revocation: use cert CRL DP and CRL IDP extensions.

This commit updates the CRL validation process so that when selecting
a CRL we consider more than just whether the cert issuer matches the CRL
issuer.

As of this commit we consider a CRL authoritative for a certificate
when:

* The certificate issuer matches the CRL issuer and,
* The certificate has no CRL distribution points, and the CRL has no
  issuing distribution point extension.
* Or, the certificate has no CRL distribution points, but the the CRL
  has an issuing distribution point extension with a scope that includes
  the certificate.
* Or, the certificate has CRL distribution points, and the CRL has an
  issuing distribution point extension with a scope that includes the
  certificate, and at least one distribution point full name is a URI
  type general name that can also be found in the CRL issuing
  distribution point full name general name sequence.

In all other circumstances the CRL is not considered authoritative. If
we can't find an authoritative CRL from the set of loaded CRLs the
certificate will be considered as having unknown revocation status.
Whether this is an error or not is controlled by the user provided
revocation policy.

TODO:
* test coverage.
* tidying up comments.

src/cert.rs

@@ -149,7 +149,6 @@ impl<'a> Cert<'a> {
     }
 
     /// Returns an iterator over the certificate's cRLDistributionPoints extension values, if any.
-    #[allow(dead_code)] // TODO(@cpu): remove once used in CRL validation.
     pub(crate) fn crl_distribution_points(&self) -> Option<CrlDistributionPoints> {
         self.crl_distribution_points
             .map(|crl_distribution_points| CrlDistributionPoints {
@@ -313,7 +312,6 @@ impl<'a> CrlDistributionPoint<'a> {
     }
 
     /// Return the distribution point names (if any).
-    #[allow(dead_code)] // TODO(@cpu): remove this once used in CRL validation.
     pub(crate) fn names(&self) -> Result<Option<DistributionPointName<'a>>, Error> {
         self.distribution_point
             .map(DistributionPointName::from_der)

src/crl.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::cert::lenient_certificate_serial_number;
+use crate::cert::{lenient_certificate_serial_number, Cert, EndEntityOrCa};
 use crate::der::{self, Tag, CONSTRUCTED, CONTEXT_SPECIFIC};
 use crate::signed_data::{self, SignedData};
 use crate::x509::{remember_extension, set_extension_once, DistributionPointName, Extension};
@@ -623,7 +623,6 @@ impl TryFrom<u8> for RevocationReason {
     }
 }
 
-#[allow(dead_code)] // TODO(@cpu): remove this once used in CRL validation.
 pub(crate) struct IssuingDistributionPoint<'a> {
     distribution_point: Option<untrusted::Input<'a>>,
     pub(crate) only_contains_user_certs: bool,
@@ -634,7 +633,6 @@ pub(crate) struct IssuingDistributionPoint<'a> {
 }
 
 impl<'a> IssuingDistributionPoint<'a> {
-    #[allow(dead_code)] // TODO(@cpu): remove this once used in CRL validation.
     pub(crate) fn from_der(der: untrusted::Input<'a>) -> Result<IssuingDistributionPoint, Error> {
         const DISTRIBUTION_POINT_TAG: u8 = CONTEXT_SPECIFIC | CONSTRUCTED;
         const ONLY_CONTAINS_USER_CERTS_TAG: u8 = CONTEXT_SPECIFIC | 1;
@@ -731,12 +729,73 @@ impl<'a> IssuingDistributionPoint<'a> {
     }
 
     /// Return the distribution point names (if any).
-    #[allow(dead_code)] // TODO(@cpu): remove this once used in CRL validation.
     pub(crate) fn names(&self) -> Result<Option<DistributionPointName<'a>>, Error> {
         self.distribution_point
             .map(DistributionPointName::from_der)
             .transpose()
     }
+
+    /// Returns true if the CRL issuing distribution point has a scope that could include the cert
+    /// and if the cert has CRL distribution points, that at least one CRL DP has a valid distribution
+    /// point full name where one of the general names is a Uniform Resource Identifier (URI) general
+    /// name that can also be found in the CRL issuing distribution point.
+    ///
+    /// We do not consider:
+    /// * Distribution point names relative to an issuer.
+    /// * General names of a type other than URI.
+    /// * Malformed names or invalid IDP or CRL DP extensions.
+    pub(crate) fn authoritative_for(&self, cert: &Cert) -> bool {
+        assert!(!self.only_contains_attribute_certs); // We check this at time of parse.
+
+        // Check that the scope of the CRL issuing distribution point could include the cert.
+        matches!(
+            (
+                self.only_contains_ca_certs,
+                self.only_contains_user_certs,
+                &cert.ee_or_ca,
+            ),
+            (true, _, EndEntityOrCa::Ca(_)) | (_, true, EndEntityOrCa::EndEntity)
+        );
+
+        let cert_dps = match cert.crl_distribution_points() {
+            // If the certificate has no distribution points, then the CRL can be authoritative
+            // based on the issuer matching and the scope including the cert.
+            None => return true,
+            Some(cert_dps) => cert_dps,
+        };
+
+        let mut idp_general_names = match self.names() {
+            Ok(Some(DistributionPointName::FullName(general_names))) => general_names,
+            _ => return false, // Either no full names, or malformed.
+        };
+
+        for cert_dp in cert_dps {
+            let cert_dp = match cert_dp {
+                Ok(cert_dp) => cert_dp,
+                // certificate CRL DP was invalid, can't match.
+                Err(_) => return false,
+            };
+
+            // If the certificate CRL DP was for an indirect CRL, or a CRL
+            // sharded by revocation reason, it can't match.
+            if cert_dp.crl_issuer.is_some() || cert_dp.reasons.is_some() {
+                return false;
+            }
+
+            let mut dp_general_names = match cert_dp.names() {
+                Ok(Some(DistributionPointName::FullName(general_names))) => general_names,
+                _ => return false, // Either no full names, or malformed.
+            };
+
+            // At least one URI type name in the IDP full names must match a URI type name in the
+            // DP full names.
+            if idp_general_names.uri_name_in_common_with(&mut dp_general_names) {
+                return true;
+            }
+        }
+
+        false
+    }
 }
 
 mod private {

src/verify_cert.rs

@@ -14,6 +14,7 @@
 
 use crate::{
     cert::{Cert, EndEntityOrCa},
+    crl::IssuingDistributionPoint,
     der, signed_data, subject_name, time, CertRevocationList, Error, SignatureAlgorithm,
     TrustAnchor,
 };
@@ -37,13 +38,15 @@ impl<'a> RevocationOptionsBuilder<'a> {
     /// Create a builder that will perform revocation checking using the provided certificate
     /// revocation lists (CRLs). At least one CRL must be provided.
     ///
-    /// Use [build] to create a [RevocationOptions] instance.
+    /// Use [RevocationOptionsBuilder::build] to create a [RevocationOptions] instance.
     ///
     /// By default revocation checking will be performed on both the end-entity (leaf) certificate
-    /// and intermediate certificates. This can be customized using the [with_depth] method.
+    /// and intermediate certificates. This can be customized using the
+    /// [RevocationOptionsBuilder::with_depth] method.
     ///
     /// By default revocation checking will fail if the revocation status of a certificate cannot
-    /// be determined. This can be customized using the [allow_unknown_status] method.
+    /// be determined. This can be customized using the
+    /// [RevocationOptionsBuilder::allow_unknown_status] method.
     pub fn new(crls: &'a [&'a dyn CertRevocationList]) -> Result<Self, CrlsRequired> {
         match crls.len() {
             0 => Err(CrlsRequired),
@@ -303,21 +306,16 @@ fn check_crls(
         return Ok(None);
     }
 
-    let crl = match revocation
+    let crl = revocation
         .crls
         .iter()
-        .find(|candidate_crl| candidate_crl.issuer() == cert.issuer())
-    {
-        Some(crl) => crl,
-        None => {
-            return match revocation.status_requirement {
-                // If the policy allows unknown, return Ok(None) to indicate that the certificate
-                // was not confirmed as CertNotRevoked, but that this isn't an error condition.
-                RevocationStatusRequirement::AllowUnknown => Ok(None),
-                // Otherwise, this is an error condition based on the provided policy.
-                _ => Err(Error::UnknownRevocationStatus),
-            };
-        }
+        .find(|candidate_crl| crl_authoritative(**candidate_crl, cert));
+
+    use RevocationStatusRequirement::*;
+    let crl = match (crl, revocation.status_requirement) {
+        (Some(crl), _) => crl,
+        (None, AllowUnknown) => return Ok(None),
+        _ => return Err(Error::UnknownRevocationStatus),
     };
 
     // Verify the CRL signature with the issuer SPKI.
@@ -338,6 +336,48 @@ fn check_crls(
     }
 }
 
+/// Returns true if the CRL can be considered authoritative for the given certificate.
+///
+/// A CRL is considered authoritative for a certificate when:
+///   * The certificate issuer matches the CRL issuer and,
+///     * The certificate has no CRL distribution points, and the CRL has no issuing distribution
+///       point extension.
+///     * Or, the certificate has no CRL distribution points, but the the CRL has an issuing
+///       distribution point extension with a scope that includes the certificate.
+///     * Or, the certificate has CRL distribution points, and the CRL has an issuing
+///       distribution point extension with a scope that includes the certificate, and at least
+///       one distribution point full name is a URI type general name that can also be found in
+///       the CRL issuing distribution point full name general name sequence.
+///
+/// In all other circumstances the CRL is not considered authoritative.
+fn crl_authoritative(crl: &dyn CertRevocationList, cert: &Cert<'_>) -> bool {
+    // In all cases we require that the authoritative CRL have the same issuer
+    // as the certificate. Recall we do not support indirect CRLs.
+    if crl.issuer() != cert.issuer() {
+        return false;
+    }
+
+    let crl_idp = match (
+        cert.crl_distribution_points(),
+        crl.issuing_distribution_point(),
+    ) {
+        // If the certificate has no CRL distribution points, and the CRL has no issuing distribution point,
+        // then we can consider this CRL authoritative based on the issuer matching.
+        (cert_dps, None) => return cert_dps.is_none(),
+
+        // If the CRL has an issuing distribution point, parse it so we can consider its scope
+        // and compare against the cert CRL distribution points, if present.
+        (_, Some(crl_idp)) => {
+            match IssuingDistributionPoint::from_der(untrusted::Input::from(crl_idp)) {
+                Ok(crl_idp) => crl_idp,
+                Err(_) => return false,
+            }
+        }
+    };
+
+    crl_idp.authoritative_for(cert)
+}
+
 // When verifying CRL signed data we want to disambiguate the context of possible errors by mapping
 // them to CRL specific variants that a consumer can use to tell the issue was with the CRL's
 // signature, not a certificate.

src/x509.rs

@@ -80,7 +80,6 @@ pub(crate) fn remember_extension(
 /// CRL information for a given certificate as described in RFC 5280 section 4.2.3.13[^1].
 ///
 /// [^1]: <https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.13>
-#[allow(dead_code)] // TODO(@cpu): remove this once used in CRL validation.
 pub(crate) enum DistributionPointName<'a> {
     /// The distribution point name is a relative distinguished name, relative to the CRL issuer.
     NameRelativeToCrlIssuer(untrusted::Input<'a>),
@@ -115,6 +114,29 @@ pub(crate) struct GeneralNames<'a> {
     reader: untrusted::Reader<'a>,
 }
 
+impl GeneralNames<'_> {
+    /// Returns true if this sequence of general names has at least one Uniform Resource Indicator
+    /// (URI) type general names that match a URI type general name in `other_names`.
+    ///
+    /// General names of a different type, or invalid general names are ignored.
+    pub(crate) fn uri_name_in_common_with(&mut self, other_names: &mut GeneralNames<'_>) -> bool {
+        for name in self.flatten() {
+            if let GeneralName::UniformResourceIdentifier(uri) = name {
+                if other_names.any(|other_name| {
+                    if let Ok(GeneralName::UniformResourceIdentifier(other_uri)) = other_name {
+                        uri.as_slice_less_safe() == other_uri.as_slice_less_safe()
+                    } else {
+                        false
+                    }
+                }) {
+                    return true;
+                }
+            }
+        }
+        false
+    }
+}
+
 impl<'a> Iterator for GeneralNames<'a> {
     type Item = Result<GeneralName<'a>, Error>;