crls: ensure that a certificate is covered by a CRL

[jsha]

In rustls, I was looking at the API for verifying client certificates against a CRL or set of CRLs. It seems like there's a problem: say the server provides a CRL from http://example.com/1.crl, but the client certificate is not covered by that CRL. For instance, the server could be using sharded CRLs and the client certificate could be on a different shard. Or, more likely, the server is using an out-of-date config and the client certificate is now reported on an entirely different CRL, or is issued by a different issuer.

One helpful thing here is the CRL Distribution Points extension. We could modify the API such that a CertRevocationList has an accessor that provides the Issuing Distribution Point encoded in that CRL. Then, when validating a client certificate that contains a CRLDP, we could require that there is a CertRevocationList matching a CRLDP from the client certificate. This also provides resistance against substitution attacks.

That would not solve the case where the client certificate does not contain a CRLDP. I think probably we should "fail closed" in such a case, provided the the crls list is non-empty. If the server was expecting to process revocation information from CRLs, and the client certificates do not provide CRLDP, that seems too risky in terms of misconfiguration.

Another check we could perform: if the CRLs list is non-empty, then there must be at least one CRL whose issuer matches the issuer of the certificate we are verifying. Right now it looks like we fail open: https://docs.rs/rustls-webpki/0.101.1/src/webpki/verify_cert.rs.html#203-209. I think that should be inverted. If the user provided a list of CRLs, they probably expected that list of CRLs to be complete for all the issuers of certificates they might encounter during the handshake. Or another way of putting it: if the user provided a list of CRLs, they want to treat certificates of "unknown" status as invalid.

[cpu]

These changes make sense to me. Thank you for the analysis. I'll start working on an update. I think it can probably be done in semver compatible way but if we need to do another breaking release it wouldn't be too bad.

Then, when validating a client certificate that contains a CRLDP, we could require that there is a CertRevocationList matching a CRLDP from the client certificate.

This will also require updating the Cert representation to recognize and retain that extension. I don't think that will be too difficult, just calling out that it isn't something available today.

[cpu]

That would not solve the case where the client certificate does not contain a CRLDP. I think probably we should "fail closed" in such a case, provided the the crls list is non-empty. If the server was expecting to process revocation information from CRLs, and the client certificates do not provide CRLDP, that seems too risky in terms of misconfiguration.

I think this makes sense iff the certificate being verified doesn't have a CRLDP and we additionally didn't find a CRL with an issuer matching the certificate's issuer. Looking around briefly I suspect many users of mTLS are not including the CRLDP extension in their client certificates and would still want to be able to use CRLs.

Another check we could perform: if the CRLs list is non-empty, then there must be at least one CRL whose issuer matches the issuer of the certificate we are verifying. Right now it looks like we fail open

I think this will require revisiting the decision to not offer a parameter to control the depth of revocation checking performed.

Earlier revisions of the CRL support offered an enum for depth to verify_is_valid_tls_client_cert with variants for EndEntityOnly or FullChain. @ctz suggested removing this because the trait could choose the CRL based on being provided a Cert and looking at the ee_or_ca field. We ended up removing the trait because of the complications with lifetimes without revisiting the parameter idea. In the impl as landed in tree the caller provides a list of CRLs without being able to consider the certificate(s) being looked at for revocation state and so can't omit CRLs when looking at an intermediate when they desire only EE revocation checking.

This works OK when we fail-open when we can't find a CRL with the same issuer as the certificate being considered, but it will break the common use case of only caring about the EE revocation state under your proposal to fail closed. The caller would have to provide CRLs covering the end entity, and any intermediates used in path building to the trust anchor or we would fail-closed upon encountering the first intermediate certificate not covered by a CRL.

[ctz]

can't omit CRLs when looking at an intermediate when they desire only EE revocation checking.

I think they can just only supply the CRL that is issued by the EE's immediate issuer? Though that is quite a bit of logic/configuration out of our control.

[cpu]

I think they can just only supply the CRL that is issued by the EE's immediate issuer?

The way check_signatures is written it checks the CRL status for each intermediate in the built chain, and then the EE certificate.

Today, if the caller only supplies a CRL that is issued by the EE's intermediate issuer it works fine: check_crls will be called for the intermediate, we won't find a CRL issued by the intermediate's issuer, and will continue on without error, eventually checking the EE's revocation status when we find we do have a CRL issued by the EE's intermediate issuer.

With Jsha's proposed change we'd fail closed in the step where we check the intermediate: we'd find no CRL issued by the intermediate's issuer and since CRLs were supplied (for the EE), we'd fail closed - the proposal says "there must be at least one CRL whose issuer matches the issuer of the certificate we are verifying."

I'm thinking we either need to not implement that proposal as written, or we need to add a flag to control the depth of revocation checking to allow skipping checking intermediate revocation status. Does that make sense?

[ctz]

I think we need the flexibility both ways :( Because, as I understand it, this feature is motivated by the configuration surface in mod_ssl. That allows:

• SSLCARevocationCheck chain no_crl_for_cert_ok - whole chain, missing CRLs ignored
• SSLCARevocationCheck chain - whole chain, missing CRLs are fatal
• SSLCARevocationCheck leaf - EE issuer only

[cpu]

I think we need the flexibility both ways

That makes sense, I think I can express this policy as an argument to the client cert verifier function alongside the CRLs and update validation accordingly.

[cpu]

I went down a bit of a rabbit hole implementing something close to the full algorithm from RFC 5280 6.3 (ignoring delta CRLs) to try and close out the last parts of this issue.

I had explicitly avoided implementing the full algorithm in my initial CRL support for a reason: it's complex, (IMO) not very well specified, and has no ground truth to measure against. I'm not comfortable trying to land the code I've drafted that tries to implement the whole algorithm and think we should narrow our goals.

I think there's a few opinionated choices we should make here to greatly reduce the complexity of the code we need to land to meet the requirements of this issue. I propose:

1. We should continue to not support indirect CRLs, considering certificates distribution points w/ a cRLIssuer field as not covered by any loaded CRLs since it implies a covering CRL must be indirect and we don't support parsing/loading those.

2. We should not support CRLs sharded by revocation reason. Much of the complexity in 6.3 comes from tracking whether all possible revocation reasons have been considered across a set of loaded CRLs that might be partitioned by reason code.RFC 5280 4.2.1.13 specifically recommends against this : "This profile RECOMMENDS against segmenting CRLs by reason code." If we reject CRLs with onlySomeReasons in the IDP extension a lot of complexity falls away.

I also can't find any evidence sharding this way is a popular practice. Looking at 4,800+ CRLs from CCADB I can't find any CRLs with IDP extensions that include onlySomeReasons. I can't say with certainty this isn't happening in other PKIs but I would argue the negative recommendation from the RFC and the required complexity is sufficient we should avoid implementation.

3. We should not consider distribution point names that are relative to an issuer, only considering distribution point names that are the full names variant (containing a sequence of general names). RFC 5280 4.2.1.13 says "Conforming CAs SHOULD NOT use nameRelativeToCRLIssuer to specify distribution point names." - it's also quite complex to implement, requiring appending distinguished name fragments to either the CRL issuer DN or the cRLIssuer if present, and handling the case where the issuer has more than one DN.

I find zero instances in the CCADB CRL corpus of a CRL with an IDP extension where the distribution point name specifies a nameRelativeToCRLIssuer.

4. When trying to match a certificate distribution point to a CRL issuing distribution point we should only compare URI type general names, trying to find a match by byte-for-byte comparison of the URI general name.

Jsha's original description says "when validating a client certificate that contains a CRLDP, we could require that there is a CertRevocationList matching a CRLDP from the client certificate." - this isn't specific enough to implement on its own.

Looking to the RFC for guidance is of limited help, RFC 5280 6.3.3 says:

If the distribution point name is present in the IDP CRL extension and the distribution field is present in the DP, then verify that one of the names in the IDP matches one of the names in the DP.

This is nuanced: ignoring the name relative to issuer case the "names" in both of the distribution point names are a sequence of general names that can have several forms. In webpki we only recognize a few general name types: dns name, directory name, IP address, and as of my WIP PRs: URI. The webpki::verify::GeneralName type explicitly doesn't implement any equality checking cautioning that the meaning of each type of general name differs by context.

I believe the intent of the general names in this part of the extension are to identify a "mechanism to obtain the CRL". With this in mind I can't understand how a dns general name or IP address general name could be used as a mechanism to obtain a CRL: neither specify a protocol! Similarly, how would something like a wildcard in a DNS general name be handled? I think the DNS and IP address general names are easy to consider out of scope for this reason.

That leaves directory name and URI type general names to consider. All of the CCADB CRLs I have on hand that specify an IDP extension with a full name sequence of general names use only URI general names (with the exception of a single CRL that includes both a URI general name and a directory name general name). The directory name use case is niche, and seems to be primarily useful in an environment where there's an implicit assumption of a directory server to consult. The URI type of general name allows explicitly specifying an LDAP URI so I would vote we ignore directory name general names until there is demand for implementation.

I don't expect that other folks have the relevant parts of 5280 paged in to give much input here so I'm going to proceed with an implementation that assumes the four opinionated choices above. If I'm wrong any guidance is welcome :-) I'm writing this (lengthy) comment mostly to serve as a record of my thought process and to emphasize that this is complicated and the details matter.

Thanks for reading my essay 😜

[djc]

Having no experience with CRLs in practice at all, it sounds like your choice make sense with the information we have. @icing do you happen to have more context/access to information on how CRLs get used in Apache-like scenarios?

[jsha]

We should continue to not support indirect CRLs
We should not support CRLs sharded by revocation reason
We should not consider distribution point names that are relative to an issuer, only considering distribution point names that are the full names variant
When trying to match a certificate distribution point to a CRL issuing distribution point we should only compare URI type general names, trying to find a match by byte-for-byte comparison of the URI general name.

👍🏻

These all sound good to me. We can always start strict, and if we find additional cases that we feel are important to support we can expand support later.

[cpu]

I believe this is now resolved by the combination of these three PRs:

• cert: retain CRL distribution points extension. #127
• crl: retain issuing distribution point extension #128
• revocation: more sophisticated revocation checking. #138

Thanks everyone for the discussion, input, and reviews 🌠