wip: crl sealed trait + owned representation.

TODO:

* Fixing generate.py
* Checking unit test coverage
* Breaking up into reviewable chunks
* ????

src/crl.rs

@@ -14,39 +14,124 @@
 
 use crate::der::Tag;
 use crate::x509::{remember_extension, set_extension_once, Extension};
-use crate::{der, signed_data, Error, Time};
+use crate::{der, signed_data, Error, SignatureAlgorithm, Time};
 
-/// Representation of a RFC 5280[^1] profile Certificate Revocation List (CRL).
+#[cfg(feature = "alloc")]
+use std::collections::HashMap;
+
+/// Operations over a  RFC 5280[^1] profile Certificate Revocation List (CRL) required
+/// for revocation checking. Implemented by [`OwnedCertRevocationList`] and
+/// [`BorrowedCertRevocationList`].
 ///
 /// [^1]: <https://www.rfc-editor.org/rfc/rfc5280#section-5>
-pub struct BorrowedCertRevocationList<'a> {
-    /// A `SignedData` structure that can be passed to `verify_signed_data`.
-    pub(crate) signed_data: signed_data::SignedData<'a>,
-
-    /// Identifies the entity that has signed and issued this
-    /// CRL.
-    pub(crate) issuer: untrusted::Input<'a>,
+pub trait CertRevocationList: private::Sealed {
+    /// Return the DER encoded issuer of the CRL.
+    fn issuer(&self) -> &[u8];
+
+    /// Try to find a revoked certificate in the CRL by DER encoded serial number. This
+    /// may yield an error if the CRL has malformed revoked certificates.
+    fn find_serial(&self, serial: &[u8]) -> Result<Option<RevokedCert>, Error>;
+
+    /// Verify the CRL signature using the issuer's subject public key information (SPKI)
+    /// and a list of supported signature algorithms.
+    fn verify_signature(
+        &self,
+        supported_sig_algs: &[&SignatureAlgorithm],
+        issuer_spki: &[u8],
+    ) -> Result<(), Error>;
+}
 
+/// Owned representation of a RFC 5280[^1] profile Certificate Revocation List (CRL).
+///
+/// This type is only available when using the `alloc` feature.
+///
+/// [^1]: <https://www.rfc-editor.org/rfc/rfc5280#section-5>
+#[cfg(feature = "alloc")]
+pub struct OwnedCertRevocationList {
     /// Indicates the issue date of this CRL.
     pub this_update: Time,
 
     /// Indicates the date by which the next CRL will be issued.
     pub next_update: Time,
 
-    /// List of certificates revoked by the issuer in this CRL.
-    pub(crate) revoked_certs: untrusted::Input<'a>,
+    /// A map of the revoked certificates contained in then CRL, keyed by the DER encoding
+    /// of the revoked cert's serial number.
+    pub revoked_certs: HashMap<Vec<u8>, OwnedRevokedCert>,
 
     /// Provides a means of identifying the public key corresponding to the private key used to
     /// sign this CRL.
-    pub(crate) authority_key_identifier: Option<untrusted::Input<'a>>,
+    pub authority_key_identifier: Option<Vec<u8>>,
+
+    /// A monotonically increasing sequence number for a given CRL scope and CRL issuer.
+    pub crl_number: Option<Vec<u8>>,
+
+    issuer: Vec<u8>,
+
+    signed_data: signed_data::OwnedSignedData,
+}
+
+#[cfg(feature = "alloc")]
+impl CertRevocationList for OwnedCertRevocationList {
+    fn issuer(&self) -> &[u8] {
+        &self.issuer
+    }
+
+    fn find_serial(&self, serial: &[u8]) -> Result<Option<RevokedCert>, Error> {
+        // note: this is infallible for the owned representation because we process all
+        // revoked certificates at the time of construction to build the `revoked_certs` map,
+        // returning any encountered errors at that time.
+        Ok(self
+            .revoked_certs
+            .get(serial)
+            .map(|owned_revoked_cert| owned_revoked_cert.borrow()))
+    }
+
+    fn verify_signature(
+        &self,
+        supported_sig_algs: &[&SignatureAlgorithm],
+        issuer_spki: &[u8],
+    ) -> Result<(), Error> {
+        signed_data::verify_signed_data(
+            supported_sig_algs,
+            untrusted::Input::from(issuer_spki),
+            &self.signed_data.borrow(),
+        )
+    }
+}
+
+/// Borrowed representation of a RFC 5280[^1] profile Certificate Revocation List (CRL).
+///
+/// This implementation allocates no memory, but as a result may be less efficient. E.g.
+/// looking up a revoked certificate by serial number is an O(N) operation.
+///
+/// [^1]: <https://www.rfc-editor.org/rfc/rfc5280#section-5>
+pub struct BorrowedCertRevocationList<'a> {
+    /// Indicates the issue date of this CRL.
+    pub this_update: Time,
+
+    /// Indicates the date by which the next CRL will be issued.
+    pub next_update: Time,
 
     /// A monotonically increasing sequence number for a given CRL scope and CRL issuer.
     pub crl_number: Option<&'a [u8]>,
+
+    revoked_certs: untrusted::Input<'a>,
+
+    authority_key_identifier: Option<untrusted::Input<'a>>,
+
+    signed_data: signed_data::SignedData<'a>,
+
+    issuer: untrusted::Input<'a>,
 }
 
 impl<'a> BorrowedCertRevocationList<'a> {
     /// Try to parse the given bytes as a RFC 5280[^1] profile Certificate Revocation List (CRL).
     ///
+    /// Note that this will not parse or process the revoked certificate entries of the CRL, meaning
+    /// that any errors related to revoked certificates will not be surfaced until
+    /// [`BorrowedCertRevocationList.into_iter`] or [`BorrowedCertRevocationList.find_serial`] are
+    /// used.
+    ///
     /// Webpki does not support:
     ///   * CRL versions other than version 2.
     ///   * CRLs missing the next update field.
@@ -165,15 +250,44 @@ impl<'a> BorrowedCertRevocationList<'a> {
             Ok(crl)
         })?;
 
-        // Iterate through the revoked certificate entries to ensure they are valid so we can
-        // yield an error up-front instead of on first iteration by the caller.
-        for cert_result in crl.into_iter() {
-            cert_result?;
-        }
-
         Ok(crl)
     }
 
+    /// Convert the CRL to an [`OwnedCertRevocationList`]. This may error if any of the revoked
+    /// certificates in the CRL are malformed or contain unsupported features.
+    ///
+    /// This function is only available when the "alloc" feature is enabled.
+    #[cfg(feature = "alloc")]
+    pub fn to_owned(&self) -> Result<OwnedCertRevocationList, Error> {
+        // Parse and collect the CRL's revoked cert entries, ensuring there are no errors. With
+        // the full set in-hand, create a lookup map by serial number for fast revocation checking.
+        let mut revoked_certs = HashMap::new();
+        self.into_iter()
+            .collect::<Result<Vec<_>, _>>()?
+            .iter()
+            .for_each(|revoked_cert| {
+                revoked_certs.insert(revoked_cert.serial_number.to_vec(), revoked_cert.to_owned());
+            });
+
+        Ok(OwnedCertRevocationList {
+            signed_data: self.signed_data.to_owned(),
+            issuer: self.issuer.as_slice_less_safe().to_vec(),
+            this_update: self.this_update,
+            next_update: self.next_update,
+            authority_key_identifier: self
+                .authority_key_identifier
+                .map(|aki| aki.as_slice_less_safe().to_vec()),
+            crl_number: self.crl_number.map(|crl_number| crl_number.to_vec()),
+            revoked_certs,
+        })
+    }
+
+    /// DER encoding of the authority key identifier (AKI) of the CRL.
+    pub fn authority_key_identifier(&self) -> Option<&[u8]> {
+        self.authority_key_identifier
+            .map(|input| input.as_slice_less_safe())
+    }
+
     fn remember_extension(&mut self, extension: &Extension<'a>) -> Result<(), Error> {
         remember_extension(extension, |id| {
             match id {
@@ -224,29 +338,38 @@ impl<'a> BorrowedCertRevocationList<'a> {
             }
         })
     }
+}
 
-    /// Try to find a [`RevokedCert`] in the CRL that has a serial number matching `serial`. This
-    /// method will ignore any [`RevokedCert`] entries that do not parse successfully. To handle
-    /// parse errors use [`BorrowedCertRevocationList`]'s [`IntoIterator`] trait.
-    pub fn find_serial(&self, serial: &[u8]) -> Option<RevokedCert<'_>> {
-        // TODO(XXX): This linear scan is sub-optimal from a performance perspective, but avoids
-        //            any allocation. It would be nice to offer a speedier alternative for
-        //            when the alloc feature is enabled:
-        //            https://github.com/rustls/webpki/issues/80
-        self.into_iter()
-            .filter_map(|parse_res| parse_res.ok())
-            .find(|revoked_cert| revoked_cert.serial_number.eq(serial))
+impl CertRevocationList for BorrowedCertRevocationList<'_> {
+    fn issuer(&self) -> &[u8] {
+        self.issuer.as_slice_less_safe()
     }
 
-    /// Raw DER encoding of the issuer of the CRL.
-    pub fn issuer(&self) -> &[u8] {
-        self.issuer.as_slice_less_safe()
+    fn find_serial(&self, serial: &[u8]) -> Result<Option<RevokedCert>, Error> {
+        for revoked_cert_result in self.into_iter() {
+            match revoked_cert_result {
+                Err(e) => return Err(e),
+                Ok(revoked_cert) => {
+                    if revoked_cert.serial_number.eq(serial) {
+                        return Ok(Some(revoked_cert));
+                    }
+                }
+            }
+        }
+
+        Ok(None)
     }
 
-    /// DER encoding of the authority key identifier (AKI) of the CRL.
-    pub fn authority_key_identifier(&self) -> Option<&[u8]> {
-        self.authority_key_identifier
-            .map(|input| input.as_slice_less_safe())
+    fn verify_signature(
+        &self,
+        supported_sig_algs: &[&SignatureAlgorithm],
+        issuer_spki: &[u8],
+    ) -> Result<(), Error> {
+        signed_data::verify_signed_data(
+            supported_sig_algs,
+            untrusted::Input::from(issuer_spki),
+            &self.signed_data,
+        )
     }
 }
 
@@ -273,6 +396,45 @@ impl<'a> Iterator for RevokedCerts<'a> {
     }
 }
 
+/// Owned representation of a RFC 5280[^1] profile Certificate Revocation List (CRL) revoked
+/// certificate entry.
+///
+/// Only available when the "alloc" feature is enabled.
+///
+/// [^1]: <https://www.rfc-editor.org/rfc/rfc5280#section-5>
+#[cfg(feature = "alloc")]
+pub struct OwnedRevokedCert {
+    /// Serial number of the revoked certificate.
+    pub serial_number: Vec<u8>,
+
+    /// The date at which the CA processed the revocation.
+    pub revocation_date: Time,
+
+    /// Identifies the reason for the certificate revocation. When absent, the revocation reason
+    /// is assumed to be RevocationReason::Unspecified. For consistency with other extensions
+    /// and to ensure only one revocation reason extension may be present we maintain this field
+    /// as optional instead of defaulting to unspecified.
+    pub reason_code: Option<RevocationReason>,
+
+    /// Provides the date on which it is known or suspected that the private key was compromised or
+    /// that the certificate otherwise became invalid. This date may be earlier than the revocation
+    /// date which is the date at which the CA processed the revocation.
+    pub invalidity_date: Option<Time>,
+}
+
+#[cfg(feature = "alloc")]
+impl OwnedRevokedCert {
+    /// Convert the owned representation of this revoked cert to a borrowed version.
+    pub fn borrow(&self) -> RevokedCert {
+        RevokedCert {
+            serial_number: &self.serial_number,
+            revocation_date: self.revocation_date,
+            reason_code: self.reason_code,
+            invalidity_date: self.invalidity_date,
+        }
+    }
+}
+
 /// Representation of a RFC 5280[^1] profile Certificate Revocation List (CRL) revoked certificate
 /// entry.
 ///
@@ -297,6 +459,19 @@ pub struct RevokedCert<'a> {
 }
 
 impl<'a> RevokedCert<'a> {
+    /// Construct an owned represetation of the revoked certificate.
+    ///
+    /// Only available when the "alloc" feature is enabled.
+    #[cfg(feature = "alloc")]
+    pub fn to_owned(&self) -> OwnedRevokedCert {
+        OwnedRevokedCert {
+            serial_number: self.serial_number.to_vec(),
+            revocation_date: self.revocation_date,
+            reason_code: self.reason_code,
+            invalidity_date: self.invalidity_date,
+        }
+    }
+
     fn from_der(der: &mut untrusted::Reader<'a>) -> Result<Self, Error> {
         der::nested(der, Tag::Sequence, Error::BadDer, |der| {
             // RFC 5280 §4.1.2.2:
@@ -390,7 +565,7 @@ impl<'a> RevokedCert<'a> {
 /// See RFC 5280 §5.3.1[^1]
 ///
 /// [^1] <https://www.rfc-editor.org/rfc/rfc5280#section-5.3.1>
-#[derive(Debug, Clone, Eq, PartialEq)]
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
 #[allow(missing_docs)] // Not much to add above the code name.
 pub enum RevocationReason {
     /// Unspecified should not be used, and is instead assumed by the absence of a RevocationReason
@@ -443,6 +618,19 @@ impl TryFrom<u8> for RevocationReason {
     }
 }
 
+mod private {
+    use crate::crl::BorrowedCertRevocationList;
+    #[cfg(feature = "alloc")]
+    use crate::crl::OwnedCertRevocationList;
+
+    pub trait Sealed {}
+
+    impl Sealed for BorrowedCertRevocationList<'_> {}
+
+    #[cfg(feature = "alloc")]
+    impl Sealed for OwnedCertRevocationList {}
+}
+
 #[cfg(test)]
 mod tests {
     use alloc::vec::Vec;

src/end_entity.rs

@@ -15,9 +15,8 @@
 #[cfg(feature = "alloc")]
 use crate::subject_name::GeneralDnsNameRef;
 use crate::{
-    cert, signed_data, subject_name,
-    verify_cert::{self, RevocationCheckOptions},
-    Error, SignatureAlgorithm, SubjectNameRef, Time, TlsClientTrustAnchors, TlsServerTrustAnchors,
+    cert, signed_data, subject_name, verify_cert, CertRevocationList, Error, SignatureAlgorithm,
+    SubjectNameRef, Time, TlsClientTrustAnchors, TlsServerTrustAnchors,
 };
 
 /// An end-entity certificate.
@@ -98,7 +97,7 @@ impl<'a> EndEntityCert<'a> {
                 supported_sig_algs,
                 trust_anchors,
                 intermediate_certs,
-                revocation: None,
+                crls: &[],
             },
             &self.inner,
             time,
@@ -118,19 +117,19 @@ impl<'a> EndEntityCert<'a> {
     /// time).
     pub fn verify_is_valid_tls_client_cert(
         &self,
-        supported_sig_algs: &'a [&'a SignatureAlgorithm],
-        &TlsClientTrustAnchors(trust_anchors): &'a TlsClientTrustAnchors,
-        intermediate_certs: &'a [&'a [u8]],
+        supported_sig_algs: &[&SignatureAlgorithm],
+        &TlsClientTrustAnchors(trust_anchors): &TlsClientTrustAnchors,
+        intermediate_certs: &[&[u8]],
         time: Time,
-        revocation: Option<RevocationCheckOptions<'a>>,
+        crls: &[&dyn CertRevocationList],
     ) -> Result<(), Error> {
         verify_cert::build_chain(
             &verify_cert::ChainOptions {
                 required_eku_if_present: verify_cert::EKU_CLIENT_AUTH,
                 supported_sig_algs,
                 trust_anchors,
                 intermediate_certs,
-                revocation,
+                crls,
             },
             &self.inner,
             time,