Use SHA-256 for subject key id on FIPS 140-3 mode #703
Conversation
This file contains hidden or 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
76c3932 to
65bc58a
Compare
This commit adds the fipsutil package that reports whether the cryptography libraries are operating in FIPS 140-3 mode. It also uses the 160 leftmost bits of SHA-256 to generate the Subject Key Ids
65bc58a to
ab27c79
Compare
Member
|
Linter issues fixed in #704. |
hslatman
reviewed
Feb 18, 2025
Comment on lines
+185
to
+188
| func Test_generateSubjectKeyID_fips(t *testing.T) { | ||
| if !fipsutil.Enabled() { | ||
| t.Skip("FIPS 140-3 mode is not enabled") | ||
| } |
Member
There was a problem hiding this comment.
Could t.Setenv help executing the tests at all times, but with the GODEBUG env var set for this test?
Contributor
Author
There was a problem hiding this comment.
No, I've tried but GODEBUG cannot be changed after initializing, it will panic if you do it. See https://github.com/golang/go/blob/279da965329a74cd75320f15cb9672a282690ab7/src/crypto/fips140/fips140.go#L26-L28
if currentlyEnabled != fips140.Enabled {
panic("crypto/fips140: GODEBUG setting changed after program start")
}
hslatman
reviewed
Feb 18, 2025
jose/encrypt.go
Outdated
| return data, nil | ||
| } | ||
| return nil, errors.New("failed to decrypt JWE: invalid password") | ||
| return nil, errors.New("failed to decrypt JWE: invalid password" + err.Error()) |
Member
There was a problem hiding this comment.
Added a space, but other options might work too. No fmt.Errorf?
Suggested change
| return nil, errors.New("failed to decrypt JWE: invalid password" + err.Error()) | |
| return nil, errors.New("failed to decrypt JWE: invalid password " + err.Error()) |
Comment on lines
13
to
15
| "github.com/stretchr/testify/assert" | ||
| "go.step.sm/crypto/fipsutil" |
Member
There was a problem hiding this comment.
Suggested change
| "github.com/stretchr/testify/assert" | |
| "go.step.sm/crypto/fipsutil" | |
| "github.com/stretchr/testify/assert" | |
| "go.step.sm/crypto/fipsutil" |
d618af5 to
21ffe8f
Compare
21ffe8f to
9fd47c3
Compare
hslatman
approved these changes
Feb 18, 2025
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
This commit adds the fips util package, which reports whether the cryptography libraries are operating in FIPS 140-3 mode.
It also uses the 160 leftmost bits of SHA-256 to generate the Subject Key IDs.
Related: