wip: refactoring extension handling #164
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
10 tasks
|
This is the work I started in cpu#1, now rebased, updated to forbid duplicate extensions, and moved to a draft PR on this repo. I've listed some TODO items I'd like to tackle before calling for broader review. Stay tuned. I have some other pressing work to handle first. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #164 +/- ##
==========================================
- Coverage 75.51% 70.72% -4.80%
==========================================
Files 7 8 +1
Lines 1981 2169 +188
==========================================
+ Hits 1496 1534 +38
- Misses 485 635 +150 ☔ View full report in Codecov by Sentry. |
cpu
force-pushed
the
cpu-rework-ext-handling
branch
from
ae702ea to
eba80ef
Compare
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. This adds a dedicated `Extensions` struct and `Extension` trait that handle: * tracking extensions maintaining insertion order. * ensuring the invariant that we never add more than one instance of the same extension OID. * writing the DER encoded SEQUENCE of extensions. * writing each DER encoded extension SEQUENCE - including the OID, criticality, and value. The `Extension` trait allows common operations across all extensions like: * getting the ext OID. * getting the criticality (using a new `Criticality` enum). * getting the raw DER value.
This commit lifts the authority key identifier extension into the `ext` module.
This commit lifts the subject alternative name extension into the `ext` module. It additionally ensures we never write an empty SAN extension, if the `CertificateParams` contain an empty vec of SAN names. For the time being SAN extensions are always written as non-criticial, but the required plumbing to handle the RFC5280 guidance on SAN ext criticality is added for follow-up adjustment.
This commit lifts the key usage extension into the `ext` module.
TODO: Split out the non-eku related bits. 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. 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.
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.
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.
cpu
force-pushed
the
cpu-rework-ext-handling
branch
from
9d2c80b to
0225a26
Compare
|
This needs a rebase. I'm going to close this for now and will revisit when I have time to follow through on finishing the remaining parts. |
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.
Not yet ready for review
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 #122)Todo: