cert: use key_identifier_method of issuer for AKI #262
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
cpu
force-pushed
the
cpu-261-fix-prespec-aki
branch
from
3b77545 to
40c3100
Compare
cpu
changed the title
Previously when issuing a certificate with an authority key identifier (AKI) extension that's signed by an issuer certificate we had a small bug where we used the to-be-issued certificate's param's `key_identifier_method` to derive the key identifier of the issuing certificate to use for the issued certificate's AKI. Instead we should be using the issuer certificate's param's `key_identifier_method`, taking care to mind the pre-specified variant. We missed this with our unit testing of the pre-specified key identifier method because we only issued a self-signed test certificate, never issuing a certificate signed by the CA that has the customization. This commit fixes the bug and extends test coverage to prevent further regression.
cpu
force-pushed
the
cpu-261-fix-prespec-aki
branch
from
40c3100 to
ed5446a
Compare
djc
approved these changes
Apr 3, 2024
Also fixes the release date for 0.13.0.
Good call. Added one and proactively updated the changelog. I also double checked this was a regression (though I didn't bother digging up which commit was the breaker. (Edit: I think it was me in 30489d7)). Backporting the unit test from this branch to 0.12.1 shows it passing, just like described in 261. |
|
|
Thanks for publishing the fix 👍 |
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.
Previously when issuing a certificate with an authority key identifier (AKI) extension that's signed by an issuer certificate we had a small bug where we used the to-be-issued certificate's param's
key_identifier_methodto derive the key identifier of the issuing certificate to use for the issued certificate's AKI. Instead we should be using the issuer certificate's param'skey_identifier_method, taking care to mind the pre-specified variant.We missed this with our unit testing of the pre-specified key identifier method because we only issued a self-signed test certificate, never issuing a certificate signed by the CA that has the customization. We principally exercised that the subject key identifier (SKI) of the self-signed cert matched the pre-specified value, but never tested that an issued cert's AKI matches the pre-specified SKI of the issuer.
This branch fixes the bug and extends test coverage to prevent further regression.
Resolves #261