ra: revoke with explicit CRL shard #7944
Conversation
jsha
force-pushed
the
revoke-with-explicit-shard
branch
from
24ec095 to
ae2e387
Compare
| Code: int64(reason), | ||
| AdminName: u.Username, | ||
| SkipBlockKey: skipBlockKey, | ||
| Malformed: malformed, | ||
| // This is a well-formed certificate so send CrlShard 0 | ||
| // to let the RA figure out the right shard from the cer. |
There was a problem hiding this comment.
| // to let the RA figure out the right shard from the cer. | |
| // to let the RA figure out the right shard from the cert. |
| assertRequestsContain(mra.revocationRequests, 0, false, false) | ||
| assertRequestsContain(mra.revocationRequests, 0, false) | ||
| } | ||
|
|
There was a problem hiding this comment.
No new TestRevokeMalformed?
| // Assumes the shard number is represented in the URL as an integer that | ||
| // occurs in the last path component, optionally followed by ".crl". |
There was a problem hiding this comment.
I admit that this is the place that had me agonizing the most of what the "right thing to do" is when I was thinking about this six months ago. Your approach here is nice and flexible, in that it would allow us to change our CRLDP "base" (e.g. the hostname or first part of the path) without having to change this code. But it's also somewhat scary in that it will happily process a certificate with a very bizarre CRLDP and not complain.
I think the approach you've settled on here is probably the best path forward for now -- the main alternative I see is configuring the crldpBaseUrl in the RA as well as in the CA, and that's a deployability mess that has only become more complex with the advent of multiple issuers -- but I think we should explicitly document this potential footgun.
There was a problem hiding this comment.
Yep, I agree on the scariness; but I think we're better off catching bizarre CRLDPs at linting time (which covers all certs, and can catch problems in advance) instead of revocation time.
In RA.RevokedCertificate, if the certificate being revoked has a crlDistributionPoints extension, parse the URL and pass the appropriate shard to the SA.
This required some changes to the
admintool. When a malformed certificate is revoked, we don't have a parsed copy of the certificate to extract a CRL URL from. So, specifically when a malformed certificate is being revoked, allow specifying a CRL shard. Because different certificates will have different shards, require one-at-a-time revocation for malformed certificates.To support that refactoring, move the serial-cleaning functionality earlier in the
admintool's flow.Also, split out one of the cases handled by the
revokeCertificatehelper in the RA. For admin revocations, we need to accept a human-specified ShardIdx, so call the SA directly in that case (and skip stat increment since admin revocations aren't useful for metrics). This allowsrevokeCertificateto be a more helpful helper, by extracting serial, issuer ID, and CRL shard automatically from an*x509.Certificate.Note: we don't yet issue certificates with the crlDistributionPoints extension, so this code will not be active until we start doing so.
Part of #7094.