Certificate Revocation List (CRL) generation support.#128
Merged
Conversation
Previously the `CertificateParams` type defined a member function, `write_extension`, that could serialize X509 extensions per RFC 5280. Conveniently, Certificate revocation lists (CRLs) use the exact same mechanism for both CRL and revoked cert extensions. In preparation for supporting generation of CRLs this commit lifts the existing function into a free-standing version, renamed to `write_x509_extension`. The existing `CertificateParams` code is updated to use this function.
Previously the `CertificateParams` type's `write_cert` fn had a code fragment used to write the authority key identifier (AKI) extension defined by RFC 5280 Section 4.2.1.1[0] Later, in RFC 5280 Section 5.2.1, there is text saying: > Conforming CRL issuers MUST use the key identifier method, and MUST > include this extension in all CRLs issued. Conveniently, the AKI extension implementation for CRLs is identical to certificates. Both use the definition from 4.2.1.1. In preparation for supporting generation of CRLs that adhere to the specification this commit lifts the existing code for AKI serialization to a free-standing version named `write_x509_authority_key_identifier`. The existing `CertificateParams.write_cert` code is updated to use this function. [0]: https://www.rfc-editor.org/rfc/rfc5280#section-4.2.1.1 [1]: https://www.rfc-editor.org/rfc/rfc5280#section-5.2.1
cpu
commented
Jun 18, 2023
est31
requested changes
Jun 18, 2023
est31
left a comment
Member
There was a problem hiding this comment.
Initial review looks mostly good, I haven't given it a close look though (yet)
This was referenced Jun 22, 2023
This commit introduces support for generating signed Certificate Revocation Lists (CRLs) according to the profile defined in RFC 5280[0]. This implementation closely follows the feature set supported by the Rustls `webpki` crate. It is _not_ a complete implementation of all possible CRL features. Notably it doesn't support: * Indirect CRLs - that is, CRLs that have revoked certificates issued by an issuer separate from the issuer of the CRL. * Delta CRLs - that is, CRLs that only include the differences from a previously issued CRL. There is also limited flexibility for issuing CRLs that don't adhere to 5280. E.g. specifying an incorrect version, omitting a CRL number, omitting the authority key identifier extenion. These are required by 5280 so let's please not pollute the world with more out-of-spec DER. I assume anyone needing to generate malformed things on purpose can use tools like `der-ascii` instead. [0]: https://www.rfc-editor.org/rfc/rfc5280#section-5 [1]: https://github.com/google/der-ascii
This commit adds tests that ensure rcgen created CRLs parse/validate successfully with OpenSSL and x509-parser. Coverage is not added for Botan because it appears the Botan rust bindings don't expose the required `X509_CRL` type. Coverage is not (yet) added for Webpki because the upstream crate hasn't yet cut a release that includes CRL parsing support.
Member
|
@est31 do you have an idea of when you'll be able to look at this in more detail? We're discussing using this support in the rustls test suite. |
Member
|
@djc merged. Do you need a crates.io release with this PR? |
Member
|
Not for now, will ping you if/when we do. Thanks for the fast turnaround! |
Member
Author
+1 - thank you 🎉 |
This was referenced Jul 4, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This branch implements support for generating RFC 5280 Certificate Revocation Lists (CRLs) using rcgen.
My primary motivation for this work is to offer the ecosystem a pure-Rust method for generating CRLs that will work with Rustls as part of in-progress support for client certificate revocation checking. As such, this work doesn't attempt to offer all possible CRL features but a minimal subset I think makes the most sense as a starting point.
I've done my best to match the style of the crate for this new feature, but as this is my first contribution there may be room for further improvements.
Resolves #109
CertificateParams: lift write_extension fn.
Previously the
CertificateParamstype defined a member function,write_extension, that could serialize X509 extensions per RFC 5280.Conveniently, Certificate revocation lists (CRLs) use the exact same mechanism for both CRL and revoked cert extensions.
In preparation for supporting generation of CRLs this commit lifts the existing function into a free-standing version, renamed to
write_x509_extension. The existingCertificateParamscode is updated to use this function.CertificateParams: lift AKI extension fn.
Previously the
CertificateParamstype'swrite_certfn had a code fragment used to write the authority key identifier (AKI) extension defined by RFC 5280 Section 4.2.1.1.Later, in RFC 5280 Section 5.2.1, there is text saying:
Conveniently, the AKI extension implementation for CRLs is identical to certificates. Both use the definition from 4.2.1.1.
In preparation for supporting generation of CRLs that adhere to the specification this commit lifts the existing code for AKI serialization to a free-standing version named
write_x509_authority_key_identifier. The existingCertificateParams.write_certcode is updated to use this function.lib: add support for creating CRLs.
This commit introduces support for generating signed Certificate Revocation Lists (CRLs) according to the profile defined in RFC 5280 Section 5.
This implementation closely follows the feature set (soon to be) supported by the Rustls
webpkicrate. It is not a complete implementation of all possible CRL features. Notably it doesn't support:There is also limited flexibility for issuing CRLs that don't adhere to 5280. E.g. specifying an incorrect version, omitting a CRL number, omitting the authority key identifier extenion. These are required by 5280 so let's please not pollute the world with more out-of-spec DER. I assume anyone needing to generate malformed things on purpose can use tools like
der-asciiinstead.tests: add certificate revocation list tests.
This commit adds tests that ensure rcgen created CRLs parse/validate successfully with OpenSSL and x509-parser.
Coverage is not added for Botan because it appears the Botan rust bindings don't expose the required
X509_CRLtype (randombit/botan-rs#95).Coverage is not (yet) added for Webpki because the upstream crate hasn't yet cut a release that includes CRL parsing support. I've started on this test coverage (see 37ada8d) but since it will require a
Cargopatch to use the unreleasedrustls/webpkiI've held off on completing that work or including it for review.