-
Notifications
You must be signed in to change notification settings - Fork 24
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
Go 1.22.0 #160
Go 1.22.0 #160
Conversation
Relies also on golang-fips/openssl#137, will update this PR when that is merged. |
Updated since golang-fips/openssl#137 merged, this should land by the end of the week. |
func (hs *clientHandshakeStateTLS13) handshake() error { | ||
c := hs.c | ||
|
||
- if needFIPS() { |
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.
@ueno wanted to call your attention to this change and the one below, and ensure it makes sense for supporting TLS 1.3 or do we need to check for HKDF support as well?
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.
That's a good point; if HKDF from a FIPS certified module cannot be used, we probably shouldn't enable TLS 1.3 in FIPS mode, though this could probably be checked elsewhere at the library initialization, not at every handshake attempt.
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, | ||
TLS_RSA_WITH_AES_128_GCM_SHA256, | ||
- TLS_RSA_WITH_AES_256_GCM_SHA384: | ||
+ TLS_RSA_WITH_AES_256_GCM_SHA384, |
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.
@ueno also wanted to call your attention to this change to ensure cipher suite is correct.
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.
Afaik TLS_CHACHA20_POLY1305_SHA256 is still not approved in FIPS mode.
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, | ||
TLS_RSA_WITH_AES_128_GCM_SHA256, | ||
- TLS_RSA_WITH_AES_256_GCM_SHA384: | ||
+ TLS_RSA_WITH_AES_256_GCM_SHA384, |
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.
From the guidelines https://www.gsa.gov/system/files?file=SSL-TLS-Implementation-%5BCIO-IT-Security-14-69-Rev-7%5D-06-12-2023.pdf and Boring implementation
For TLS 1.3 implementations, NIST SP 800-52 and SSL Labs jointly recommend the use of the
following FIPS-approved ciphers in this order of preference (from highest to lowest):
- TLS_AES_256_GCM_SHA384
- TLS_AES_128_GCM_SHA256
- TLS_AES_128_CCM_SHA256
- TLS_AES_128_CCM_8_SHA256
patches/011-122-fixes.patch
Outdated
+ TLS_RSA_WITH_AES_256_GCM_SHA384, | ||
+ TLS_AES_128_GCM_SHA256, | ||
+ TLS_AES_256_GCM_SHA384, | ||
+ TLS_CHACHA20_POLY1305_SHA256: |
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.
needs to be removed indeed.
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.
Looks good to me!
No description provided.