Check time validity of TrustAnchors

The purpose of this is to avoid programs which compile-in
root certificates from trusting them indefinitely.

src/cert.rs

@@ -12,7 +12,7 @@
 // ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-use crate::{der, signed_data, Error};
+use crate::{der, signed_data, time, Error};
 use untrusted;
 
 pub enum EndEntityOrCA<'a> {
@@ -35,6 +35,26 @@ pub struct Cert<'a> {
     pub subject_alt_name: Option<untrusted::Input<'a>>,
 }
 
+pub struct Validity {
+    pub not_before: time::Time,
+    pub not_after: time::Time,
+}
+
+pub fn parse_validity(validity: untrusted::Input) -> Result<Validity, Error> {
+    validity.read_all(Error::BadDER, |input| {
+        let not_before = der::time_choice(input)?;
+        let not_after = der::time_choice(input)?;
+        if not_before < not_after {
+            Ok(Validity {
+                not_before,
+                not_after,
+            })
+        } else {
+            Err(Error::InvalidCertValidity)
+        }
+    })
+}
+
 pub fn parse_cert<'a>(
     cert_der: untrusted::Input<'a>, ee_or_ca: EndEntityOrCA<'a>,
 ) -> Result<Cert<'a>, Error> {

src/time.rs

@@ -59,4 +59,7 @@ impl Time {
     /// `std::time::SystemTime` is available (when `#![no_std]` isn't being
     /// used).
     pub fn from_seconds_since_unix_epoch(secs: u64) -> Time { Time(secs) }
+
+    /// Unwrap this type to a unix timestamp.
+    pub(crate) fn into_seconds_since_unix_epoch(self) -> u64 { self.0 }
 }

src/trust_anchor_util.rs

@@ -16,7 +16,7 @@
 
 use super::der;
 use crate::{
-    cert::{certificate_serial_number, parse_cert_internal, Cert, EndEntityOrCA},
+    cert::{certificate_serial_number, parse_cert_internal, parse_validity, Cert, EndEntityOrCA},
     Error, TrustAnchor,
 };
 use untrusted;
@@ -40,7 +40,7 @@ pub fn cert_der_as_trust_anchor(cert_der: untrusted::Input) -> Result<TrustAncho
         EndEntityOrCA::EndEntity,
         possibly_invalid_certificate_serial_number,
     ) {
-        Ok(cert) => Ok(trust_anchor_from_cert(cert)),
+        Ok(cert) => trust_anchor_from_cert(cert),
         Err(Error::BadDER) => parse_cert_v1(cert_der).or(Err(Error::BadDER)),
         Err(err) => Err(err),
     }
@@ -75,12 +75,15 @@ pub fn generate_code_for_trust_anchors(name: &str, trust_anchors: &[TrustAnchor]
     decl + &value
 }
 
-fn trust_anchor_from_cert<'a>(cert: Cert<'a>) -> TrustAnchor<'a> {
-    TrustAnchor {
+fn trust_anchor_from_cert<'a>(cert: Cert<'a>) -> Result<TrustAnchor<'a>, Error> {
+    let validity = parse_validity(cert.validity)?;
+    Ok(TrustAnchor {
         subject: cert.subject.as_slice_less_safe(),
         spki: cert.spki.as_slice_less_safe(),
         name_constraints: cert.name_constraints.map(|nc| nc.as_slice_less_safe()),
-    }
+        not_before: validity.not_before.into_seconds_since_unix_epoch(),
+        not_after: validity.not_after.into_seconds_since_unix_epoch(),
+    })
 }
 
 /// Parses a v1 certificate directly into a TrustAnchor.
@@ -94,14 +97,17 @@ fn parse_cert_v1<'a>(cert_der: untrusted::Input<'a>) -> Result<TrustAnchor<'a>,
 
                 skip(tbs, der::Tag::Sequence)?; // signature.
                 skip(tbs, der::Tag::Sequence)?; // issuer.
-                skip(tbs, der::Tag::Sequence)?; // validity.
+                let validity = der::expect_tag_and_get_value(tbs, der::Tag::Sequence)?;
                 let subject = der::expect_tag_and_get_value(tbs, der::Tag::Sequence)?;
                 let spki = der::expect_tag_and_get_value(tbs, der::Tag::Sequence)?;
+                let validity = parse_validity(validity)?;
 
                 Ok(TrustAnchor {
                     subject: subject.as_slice_less_safe(),
                     spki: spki.as_slice_less_safe(),
                     name_constraints: None,
+                    not_before: validity.not_before.into_seconds_since_unix_epoch(),
+                    not_after: validity.not_after.into_seconds_since_unix_epoch(),
                 })
             });
 

src/verify_cert.rs

@@ -56,6 +56,16 @@ pub fn build_chain(
             return Err(Error::UnknownIssuer);
         }
 
+        let not_before = time::Time::from_seconds_since_unix_epoch(trust_anchor.not_before);
+        let not_after = time::Time::from_seconds_since_unix_epoch(trust_anchor.not_after);
+
+        if time < not_before {
+            return Err(Error::CertNotValidYet);
+        }
+        if not_after < time {
+            return Err(Error::CertExpired);
+        }
+
         let name_constraints = trust_anchor.name_constraints.map(untrusted::Input::from);
 
         untrusted::read_all_optional(name_constraints, Error::BadDER, |value| {
@@ -159,8 +169,7 @@ fn check_issuer_independent_properties(
     // See the comment in `remember_extension` for why we don't check the
     // KeyUsage extension.
 
-    cert.validity
-        .read_all(Error::BadDER, |value| check_validity(value, time))?;
+    cert::parse_validity(cert.validity).and_then(|validity| check_validity(&validity, time))?;
     untrusted::read_all_optional(cert.basic_constraints, Error::BadDER, |value| {
         check_basic_constraints(value, used_as_ca, sub_ca_count)
     })?;
@@ -172,17 +181,14 @@ fn check_issuer_independent_properties(
 }
 
 // https://tools.ietf.org/html/rfc5280#section-4.1.2.5
-fn check_validity(input: &mut untrusted::Reader, time: time::Time) -> Result<(), Error> {
-    let not_before = der::time_choice(input)?;
-    let not_after = der::time_choice(input)?;
-
-    if not_before > not_after {
+fn check_validity(validity: &cert::Validity, time: time::Time) -> Result<(), Error> {
+    if validity.not_before > validity.not_after {
         return Err(Error::InvalidCertValidity);
     }
-    if time < not_before {
+    if time < validity.not_before {
         return Err(Error::CertNotValidYet);
     }
-    if time > not_after {
+    if time > validity.not_after {
         return Err(Error::CertExpired);
     }
 

src/webpki.rs

@@ -256,6 +256,14 @@ pub struct TrustAnchor<'a> {
     /// The value of a DER-encoded NameConstraints, containing name
     /// constraints to apply to the trust anchor, if any.
     pub name_constraints: Option<&'a [u8]>,
+
+    /// The root's earliest validity in seconds since the epoch. The anchor
+    /// is only considered when the validation time is after this value.
+    pub not_before: u64,
+
+    /// The root's expiry in seconds since the epoch. The anchor
+    /// is only considered when the validation time is before this value.
+    pub not_after: u64,
 }
 
 /// Trust anchors which may be used for authenticating servers.

tests/integration.rs

@@ -118,3 +118,66 @@ fn read_root_with_neg_serial() {
 #[cfg(feature = "std")]
 #[test]
 fn time_constructor() { let _ = webpki::Time::try_from(std::time::SystemTime::now()).unwrap(); }
+
+#[cfg(feature = "trust_anchor_util")]
+mod expiry {
+    use untrusted::Input;
+    use webpki::{trust_anchor_util, EndEntityCert, Error};
+
+    fn expect_expiry(when: u64, ee: &[u8], expect_err: Option<Error>) {
+        let ca = include_bytes!("expiry/ca.der");
+
+        let ee_input = Input::from(ee);
+        let inter_vec = vec![];
+        let anchors = [trust_anchor_util::cert_der_as_trust_anchor(Input::from(ca)).unwrap()];
+        let anchors = webpki::TLSServerTrustAnchors(&anchors);
+
+        let rough_time = webpki::Time::from_seconds_since_unix_epoch(when);
+
+        let cert = EndEntityCert::from(ee_input).unwrap();
+        let rc = cert.verify_is_valid_tls_server_cert(
+            super::ALL_SIGALGS,
+            &anchors,
+            &inter_vec,
+            rough_time,
+        );
+
+        assert_eq!(expect_err, rc.err());
+    }
+
+    #[test]
+    pub fn valid() {
+        let cert = include_bytes!("expiry/ee.der");
+        expect_expiry(1479496432, &cert[..], None);
+    }
+
+    #[test]
+    pub fn ee_not_valid_yet() {
+        let cert = include_bytes!("expiry/ee.der");
+        expect_expiry(1476731633, &cert[..], None);
+        expect_expiry(1476731632, &cert[..], Some(Error::CertNotValidYet));
+    }
+
+    #[test]
+    pub fn ee_expired() {
+        let cert = include_bytes!("expiry/ee.der");
+        expect_expiry(1479496433, &cert[..], None);
+        expect_expiry(1479496434, &cert[..], Some(Error::CertExpired));
+    }
+
+    #[test]
+    fn ca_not_valid_yet() {
+        let cert = include_bytes!("expiry/eelong.der");
+        expect_expiry(1476731623, &cert[..], None);
+        expect_expiry(1476731622, &cert[..], Some(Error::UnknownIssuer));
+    }
+
+    #[test]
+    fn ca_expired() {
+        // This certificate has an expiry that extends past the end
+        // of its CA cert.
+        let cert = include_bytes!("expiry/eelong.der");
+        expect_expiry(1508267623, &cert[..], None);
+        expect_expiry(1508267624, &cert[..], Some(Error::UnknownIssuer));
+    }
+}