-
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
Restrict ECDSA/NIST P-Curve hash function sizes for cert signing #12872
Conversation
37740cb
to
2ed3776
Compare
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.
For right now, no UI changes are required to let this go forward. We will, however, create a ticket to update the UI to make this more user-friendly when it comes to the interdependent fields of key type, key bits, and signature bits.
2ed3776
to
647067a
Compare
647067a
to
603ebd1
Compare
When using an ECDSA signature with a NIST P-Curve, we should follow recommendations from BIS (Section 4.2) and Mozilla's root store policy (section 5.1.2) to ensure that arbitrary selection of signature_bits does not exceed what the curve is capable of signing. Related: hashicorp#11245 Signed-off-by: Alexander Scheel <[email protected]>
Replaces previous calls to certutil.ValidateKeyTypeLength(...) and certutil.ValidateSignatureLength(...) with a single call, allowing for curve<->hash validation. Signed-off-by: Alexander Scheel <[email protected]>
This enables detection of whether the caller manually specified a value for signature_bits or not; when not manually specified, we can provision a value that complies with new NIST P-Curve policy. Signed-off-by: Alexander Scheel <[email protected]>
Due to our change in behavior (to default to -1 as the value to signature_bits to allow for automatic hash selection), switch ValidateKeyTypeSignatureLength(...) to accept a pointer to hashBits and provision it with valid default values. 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]>
9492992
to
8686b03
Compare
Since the race test appears to have been broken in main, and the race in question appears to be unrelated to the present changes, going ahead and merging. Thanks all! |
In #11006, it was mentioned that #11245 updated this behavior but didn't apply the size restrictions. We make the following changes (along side #11216, which I believe is still relevant):
0
signature bits, to perform auto detection if not otherwise specified. Presently this only does auto detection for ECDSA keys and leaves the default 256-bit hash function for all RSA key sizes. In the future, we could consider following NIST guidelines (and using 2048->256, 3072->384, 4096->512).ValidateKeyTypeSignatureLength
for cross Key/Signature validations (and which handles the actual autodetection)Since #11216 lacks a changelog entry,
we should likely update #11245's changelog entry to match the new behavior and provide a unified changelog entry for the combined change (as it appears 11245 hasn't yet landed in a release yet).Edit: Added a changelog entry.