wip: refactoring extension handling #1
Closed
Conversation
This file contains 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
Welcome to Codecov 🎉Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment. Thanks for integrating Codecov - We've got you covered ☂️ |
cpu
commented
Sep 10, 2023
4 tasks
This commit creates a new crate-internal module, `ext`, for managing X.509 extensions. In this commit we wire up emitting extensions managed by this module, but do not yet convert any existing extensions to the new arrangement. This will begin in subsequent commits. Notably the new representation for a collection of extensions uses a `BTreeMap` keyed by OID. We use this to reject additions of extensions that are already present in the map, since RFC 5280 forbids duplicate extensions. Using a `BTreeMap` instead of another map type allows preserving order.
This commit lifts the authority key identifier extension into the `ext` module.
This commit lifts the subject alternative name extension into the `ext` module.
This commit lifts the key usage extension into the `ext` module.
This commit lifts the extended key usage extension into the `ext` module.
This commit lifts the name constraints extension into the `ext` module.
This commit lifts the CRL distribution points extension into the `ext` module.
This commit lifts the subject key identifier extension into the `ext` module. It additionally adds support for specifying a custom SKI value in `CertificateParams`, and reading it from a CSR's SKI ext. Diverging from the existing code we now adhere to the RFC 5280 advice and always emit the SKI extension when generating a certificate. Previously this was only done if the basic constraints specified `IsCa::Ca` or `IsCa::ExplicitNoCa`, but not when using `IsCa::NoCa`.
This commit lifts the basic constraints extension into the `ext` module.
This commit lifts the custom extension handling into the `ext` module.
Now that all extensions in certs and CSRs are managed through `Extensions` we can have that type manage writing the outer `SEQUENCE` and each `Extension`.
Previously the params were interrogated for parameters that would indicate needing an extension to be emitted. Keeping this logic up to date was error prone. This commit reworks the code to emit extensions whenever the `Extensions` generated from the params isn't empty. This has the advantage of not needing to be updated when new parameter -> extension instances are added.
This commit lifts the CRL number extension handling into the `ext` module.
This commit lifts the CRL issuing distribution point extension handling into the `ext` module.
We always have an issuer `Certificate` when invoking `extensions` on a `CertificateRevocationListParams`, and RFC 5280 says issuers are REQUIRED to include the AKI ext.
Now that all of the base CRL extensions are handled by `Extensions` we can use that type to emit the `SEQUENCE` directly. Similarly, since the only call-site always provides an issuer, simplify some logic.
This commit lifts the CRL entry reason code extension handling into the `ext` module.
This commit lifts the CRL entry invalidity date extension into the `ext` module. There are no longer any references to the lib.rs `write_x509_extension` helper, so it is also removed.
Now that all of the CRL entry extensions have been migrated to `Extensions` we can let that type write the `SEQUENCE` and extension values. There are no longer any callers to `Extensions.iter()` so we remove that fn.
In preparation for broader CSR extension support this commit updates the logic for detecting unsupported CSR exts to only forbid serial number.
This commit adds a unit test that ensures when an `rcgen::CertificateParams` specifies parameters that result in X.509 extensions, we find each expected extension in a generated certificate *and* certificate signing request when parsing both with `x509-parser`.
This commit updates the CSR parsing to populate unknown requested extensions as custom extensions in the built CSR `CertificateParams.custom_extensions`.
|
I've opened a draft on the main repo (rustls#164) Closing this PR on my fork. |
14 tasks
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
Work in progress refactor of extension handling (for certificates, CSRs and CRLs).
Goals:
lib.rsCertificateParamsare written to both certificates and CSRs (resolves X509v3 extensions from certificate not transferred to CSR rustls/rcgen#122)Todo:
CustomExtension