-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add PSS support to PKI Secrets Engine #16519
Merged
Merged
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
cipherboy
force-pushed
the
cipherboy-add-pss-support-to-pki
branch
from
August 1, 2022 15:06
d3868c8
to
e9a2830
Compare
stevendpclark
requested changes
Aug 2, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks good, and thanks for the tests! A few nits we can discuss if you feel they are too much.
cipherboy
force-pushed
the
cipherboy-add-pss-support-to-pki
branch
from
August 3, 2022 13:46
68c1449
to
de8f8f4
Compare
Signed-off-by: Alexander Scheel <[email protected]>
We introduce a new parameter on issuers, revocation_signature_algorithm to control the signature algorithm used during CRL signing. This is because the SignatureAlgorithm value from the certificate itself is incorrect for this purpose: a RSA root could sign an ECDSA intermediate with say, SHA256WithRSA, but when the intermediate goes to sign a CRL, it must use ECDSAWithSHA256 or equivalent instead of SHA256WithRSA. When coupled with support for PSS-only keys, allowing the user to set the signature algorithm value as desired seems like the best approach. Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
When using PSS support with a managed key, sometimes the underlying device will not support PKCS#1v1.5 signatures. This results in CRL building failing, unless we update the entry's signature algorithm prior to building the CRL for the new root. With a RSA-type key and use_pss=true, we use the signature bits value to decide which hash function to use for PSS support. Signed-off-by: Alexander Scheel <[email protected]>
When CRL building fails during cert/key import, due to PSS failures, give a better indication to the user that import succeeded its just CRL building that failed. This tells them the parameter to adjust on the issuer and warns that CRL building will fail until this is fixed. Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
cipherboy
force-pushed
the
cipherboy-add-pss-support-to-pki
branch
from
August 3, 2022 13:47
de8f8f4
to
585f65e
Compare
stevendpclark
approved these changes
Aug 3, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Signed-off-by: Alexander Scheel <[email protected]>
sgmiller
reviewed
Aug 3, 2022
Signed-off-by: Alexander Scheel <[email protected]>
cipherboy
force-pushed
the
cipherboy-add-pss-support-to-pki
branch
from
August 3, 2022 15:50
ffcc841
to
cd8c973
Compare
Thanks all! Merged... |
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.
Noting that NIST is deprecating PKCS#1v1.5 signatures here at the end of December 2023, we've gone ahead and added PSS support for RSA-typed issuers in the PKI Secrets Mount.
This introduces two changes:
use_pss
on roles and signing endpoints (such as/sign-verbatim
&c) -- this allows us to create self-signed roots & intermediates using the PSS algorithm.revocation_signature_algorithm
on the Issuer, allowing us to choose override the default signature on CRLs &c.This should let us work with PSS-only keys in the future (from say, a HSM) in Vault Enterprise within the PKI mount.