Skip to content
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

crypto/tls: Permit recently FIPS-approved protocols/algorithms #62372

Open
reedloden opened this issue Aug 30, 2023 · 28 comments
Open

crypto/tls: Permit recently FIPS-approved protocols/algorithms #62372

reedloden opened this issue Aug 30, 2023 · 28 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@reedloden
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.21.0 darwin/arm64

Does this issue reproduce with the latest release?

Yes

What did you do?

When in FIPS-mode, BoringCrypto does not support recently permitted protocols/algorithms.

What did you expect to see?

Support for TLS v1.3 (permitted by NIST SP 800-52 Rev. 2), including cipher suites TLS_AES_128_GCM_SHA256 and TLS_AES_256_GCM_SHA384. See also GSA IT Security Procedural Guide: SSL/TLS Implementation CIO-IT Security-14-69. Note that TLS 1.3 support is required starting January 1, 2024.

Support for Ed25519 signature algorithm (permitted by FIPS 186-5).

FIPS 140-3 made some changes that permitted these new protocols/algorithms.

Note that BoringSSL fips-20220613 branch already supports TLS 1.3 and some other improvements by way of SSL_CTX_set_compliance_policy(ssl_compliance_policy_fips_202205). This helps bring BoringCrypto into alignment from the TLS perspective.

What did you see instead?

Only TLS v1.2 supported. No support for TLS v1.3.

No support for Ed25519 as a signature algorithm.

reedloden added a commit to reedloden/go that referenced this issue Aug 30, 2023
…-mode

TLS 1.3 is permitted by NIST SP 800-52 Rev. 2 and will be required starting
January 1, 2024.

Ed25519 as a signature algorithm is permitted by FIPS 186-5.

Fixes golang#62372.
@gopherbot
Copy link

Change https://go.dev/cl/524355 mentions this issue: crypto/tls: support TLS 1.3 and Ed25519 signature algorithm in FIPS-mode

@ianlancetaylor
Copy link
Contributor

CC @golang/security

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 31, 2023
@dmitshur dmitshur added this to the Backlog milestone Aug 31, 2023
@dagood
Copy link
Contributor

dagood commented Aug 31, 2023

Note that TLS 1.3 support is required starting January 1, 2024.

It sounds like this change is needed in 1.21 and 1.22 1.20 and 1.21 because those versions will both still be in support by then, is that correct?

(Edit: Whoops, off by one.)

reedloden added a commit to reedloden/go that referenced this issue Sep 1, 2023
TLS 1.3 is permitted by NIST SP 800-52 Rev. 2 and will be required
starting January 1, 2024.

Updates golang#62372
@reedloden
Copy link
Author

Note that TLS 1.3 support is required starting January 1, 2024.

It sounds like this change is needed in 1.20 and 1.21 because those versions will both still be in support by then, is that correct?

As I understand Go's release policy and release cadence, then yes. I had originally included multiple changes in my PR to address the recent FIPS / NIST improvements, but I re-scoped it solely to address the TLS v1.3 support to bring it inline with what BoringSSL already supports and due to the impending deadline. I'll open up additional PRs for other things, but it's possible some of those changes might need re-certification by NIST as part of the move to FIPS 140-3.

@FiloSottile
Copy link
Contributor

The latest BoringCrypto validation, both according to the docs and to a CMVP search is certificate #4407. Its security policy specifies 853ca1ea1168dff08011e5d42d94609cc0ca2e27 as the version, which is fips-20210429.

The Approved Algorithms don't list Ed25519 (which I don't think would have been possible, since this is a FIPS 140-2 certificate). The security policy mentions TLS 1.2 in various places, but not TLS 1.3. ssl_compliance_policy_fips_202205 was added after fips-20210429 was tagged, too.

I'm afraid that we can't enable these algorithms with the current BoringCrypto module version.

The Modules In Process List has a BoringCrypto FIPS 140-3 validation In Review, for what I presume might be fips-20220613. In the past, we have upgraded to a new version of the module while it was In Review, but I can't make that call myself.

@agl, is it ok to switch to fips-20220613? What algorithms are allowed in its policy?

reedloden added a commit to reedloden/go that referenced this issue Nov 27, 2023
TLS 1.3 is permitted by NIST SP 800-52 Rev. 2 and will be required
starting January 1, 2024.

Updates golang#62372
@reedloden
Copy link
Author

@FiloSottile To be clear, this is an overarching issue covering the fact that FIPS 140-3 and other NIST standards have changed what's permitted (including Ed25519 as a new approved algorithm). Definitely understand that a validated module is required, but just wanted a tracking issue, as I didn't see anything similar. Great to see that BoringCrypto is undergoing FIPS 140-3 validation. Hopefully, that can be completed soon, and then we can way more easily update Go accordingly.

Separately, #62373 is specifically around enabling TLS 1.3 using the existing fips-20210429 certified module version. The question is whether TLS 1.3 can be used under that version (especially as the security policy doesn't mention TLS 1.2 at all), or if we need to wait for fips-20220613 (or some other BoringSSL FIPS tag).

@FiloSottile
Copy link
Contributor

Discussed this offline with @agl. My understanding is that on the Go side we have three imperfect options:

  1. wait for fips-20220613, then enable TLS 1.3, missing the NIST SP 800-52 Rev. 2 deadline
  2. upgrade to fips-20220613 while the module is in review
  3. enable TLS 1.3 with fips-20210429 under the argument that the validated KDF hasn't changed

We have precedent for upgrading to an In Review module, and most consumers I have heard from are comfortable with that, so I'm picking (2). I'll open freeze exception and backport issues next week.

@reedloden
Copy link
Author

Appreciate you following-up on this. (2) does seem like the best of the non-great options.

Just to be clear, has it been confirmed that the fips-20220613 tag is what is under review for FIPS 140-3 and not the separate fips-20230428 branch?

@FiloSottile
Copy link
Contributor

I understand that fips-20220613 will be the one we'll hopefully get a certificate for the soonest.

@reedloden
Copy link
Author

We have precedent for upgrading to an In Review module, and most consumers I have heard from are comfortable with that, so I'm picking (2). I'll open freeze exception and backport issues next week.

Did the freeze exception and backport issues get filed?

@gopherbot
Copy link

Change https://go.dev/cl/549695 mentions this issue: crypto/internal/boring: upgrade module to fips-20220613

@gopherbot
Copy link

Change https://go.dev/cl/549975 mentions this issue: crypto/tls: align FIPS-only mode with BoringSSL policy

gopherbot pushed a commit that referenced this issue Dec 18, 2023
Also, add EVP_aead_aes_*_gcm_tls13 to the build, which we will need in a
following CL, to avoid rebuilding the syso twice.

Updates #64717
Updates #62372

Change-Id: Ie4d853ad9b914c1095cad60694a1ae6f77dc22ce
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-boringcrypto
Reviewed-on: https://go-review.googlesource.com/c/go/+/549695
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
gopherbot pushed a commit that referenced this issue Dec 18, 2023
This enables TLS 1.3, disables P-521, and disables non-ECDHE suites.

Fixes #64717
Updates #62372

Change-Id: I3a65b239ef0198bbdbe5e55e0810e7128f90a091
Reviewed-on: https://go-review.googlesource.com/c/go/+/549975
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Than McIntosh <thanm@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/553856 mentions this issue: [release-branch.go1.21] crypto/tls: align FIPS-only mode with BoringSSL policy

@gopherbot
Copy link

Change https://go.dev/cl/553855 mentions this issue: [release-branch.go1.21] crypto/internal/boring: upgrade module to fips-20220613

@gopherbot
Copy link

Change https://go.dev/cl/553875 mentions this issue: [release-branch.go1.20] crypto/internal/boring: upgrade module to fips-20220613

@gopherbot
Copy link

Change https://go.dev/cl/553876 mentions this issue: [release-branch.go1.20] crypto/tls: align FIPS-only mode with BoringSSL policy

gopherbot pushed a commit that referenced this issue Jan 4, 2024
…s-20220613

Also, add EVP_aead_aes_*_gcm_tls13 to the build, which we will need in a
following CL, to avoid rebuilding the syso twice.

Updates #64717
Updates #62372
Updates #64718

Change-Id: Ie4d853ad9b914c1095cad60694a1ae6f77dc22ce
Cq-Include-Trybots: luci.golang.try:go1.20-linux-amd64-boringcrypto
Reviewed-on: https://go-review.googlesource.com/c/go/+/549695
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/553875
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
gopherbot pushed a commit that referenced this issue Jan 4, 2024
…SL policy

This enables TLS 1.3, disables P-521, and disables non-ECDHE suites.

Updates #64717
Updates #62372
Fixes #64718

Change-Id: I3a65b239ef0198bbdbe5e55e0810e7128f90a091
Reviewed-on: https://go-review.googlesource.com/c/go/+/549975
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/553876
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Jan 4, 2024
…s-20220613

Also, add EVP_aead_aes_*_gcm_tls13 to the build, which we will need in a
following CL, to avoid rebuilding the syso twice.

Updates #64717
Updates #62372
Updates #64719

Change-Id: Ie4d853ad9b914c1095cad60694a1ae6f77dc22ce
Cq-Include-Trybots: luci.golang.try:go1.21-linux-amd64-boringcrypto
Reviewed-on: https://go-review.googlesource.com/c/go/+/549695
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/553855
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
gopherbot pushed a commit that referenced this issue Jan 4, 2024
…SL policy

This enables TLS 1.3, disables P-521, and disables non-ECDHE suites.

Updates #64717
Updates #62372
Fixes #64719

Change-Id: I3a65b239ef0198bbdbe5e55e0810e7128f90a091
Reviewed-on: https://go-review.googlesource.com/c/go/+/549975
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/553856
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@leungster
Copy link

I tried pulling in this change in go 1.21.6 and it appears P521 support was dropped with GOEXPERIMENT=boringcrypto and import _ crypto/tls/fipsonly.

It looks like it was dropped intentionally from boringAllowCert. Is that expected?

@rolandshoemaker
Copy link
Member

I tried pulling in this change in go 1.21.6 and it appears P521 support was dropped with GOEXPERIMENT=boringcrypto and import _ crypto/tls/fipsonly.

It looks like it was dropped intentionally from boringAllowCert. Is that expected?

Yes, this brings us in alignment with BoringSSL which only permits P-256 and P-384

@leungster
Copy link

Thanks for the info. Unfortunately this breaks some of our existing certs in the field and will require a rotation.

@dio
Copy link

dio commented Jan 11, 2024

@FiloSottile sorry, could you help to elaborate on why we didn't go with the currently validated fips-20210429 for enabling TLS1.3? Thank you!

@FiloSottile
Copy link
Contributor

@FiloSottile sorry, could you help to elaborate on why we didn't go with the currently validated fips-20210429 for enabling TLS1.3? Thank you!

See #62372 (comment) and #62372 (comment). The fips-20210429 Security Policy does not cover the TLS 1.3 KDF.

Some auditors might accept an argument that that's ok, just like some auditors might accept In Review modules. I can't advise on what is necessary for your specific compliance goals.

@Lekensteyn
Copy link
Contributor

I tried pulling in this change in go 1.21.6 and it appears P521 support was dropped with GOEXPERIMENT=boringcrypto and import _ crypto/tls/fipsonly.
It looks like it was dropped intentionally from boringAllowCert. Is that expected?

Yes, this brings us in alignment with BoringSSL which only permits P-256 and P-384

BoringSSL presumably added it for Google Cloud, https://boringssl-review.googlesource.com/c/boringssl/+/52625

The BoringCrypto fips-20210429 certificate approves P-521. SP 800-52r2 does not forbid P-521 either:

3.2 Server Keys and Certificates

If the server is configured with an ECDSA signature certificate, either curve P-256 or curve P-384 should be used for the public key in the certificate.12

12 The recommended elliptic curves now listed in FIPS 186-4 [45] will be moved to SP 800-186. Until SP 800-186 is published, the recommended elliptic curves should be taken from FIPS 186-4.

and

3.4.2.2 Supported Groups

When elliptic curve cipher suites are configured, at least one of the NIST-approved curves, P-256 (secp256r1) and P-384 (secp384r1), shall be supported as described in RFC 8422 [52]. Additional NIST-recommended elliptic curves are listed in SP 800-56A, Appendix D [6]. Finite field groups thatare approved for TLS in SP 800-56A, Appendix D may be supported.

Presumably this was the reason to enable it before in https://go-review.googlesource.com/c/go/+/343880

Is there a compelling reason to disable support for P-521? Normally it should not be in use, but there may be edge cases where support for such client/server certificates is still needed in TLS.

@gopherbot
Copy link

Change https://go.dev/cl/558796 mentions this issue: Revert "crypto/internal/boring: upgrade module to fips-20220613" +1

gopherbot pushed a commit that referenced this issue Jan 26, 2024
This reverts commit 7383b2a
("crypto/internal/boring: upgrade module to fips-20220613") and commit
4106de9 ("crypto/tls: align FIPS-only
mode with BoringSSL policy").

Fixes #65321
Updates #64717
Updates #62372

Change-Id: I0938b97e5b4904e6532448b8ae76e920d03d0508
Reviewed-on: https://go-review.googlesource.com/c/go/+/558796
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@gopherbot
Copy link

Change https://go.dev/cl/558797 mentions this issue: [release-branch.go1.22] Revert "crypto/internal/boring: upgrade module to fips-20220613" +1

gopherbot pushed a commit that referenced this issue Jan 29, 2024
…e to fips-20220613" +1

This reverts commit 7383b2a
("crypto/internal/boring: upgrade module to fips-20220613") and commit
4106de9 ("crypto/tls: align FIPS-only
mode with BoringSSL policy").

Fixes #65324
Updates #65321
Updates #64717
Updates #62372

Change-Id: I0938b97e5b4904e6532448b8ae76e920d03d0508
Reviewed-on: https://go-review.googlesource.com/c/go/+/558796
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 09b5de4)
Reviewed-on: https://go-review.googlesource.com/c/go/+/558797
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/560275 mentions this issue: [release-branch.go1.21] Revert "crypto/internal/boring: upgrade module to fips-20220613" +1

@gopherbot
Copy link

Change https://go.dev/cl/560276 mentions this issue: [release-branch.go1.20] Revert "crypto/internal/boring: upgrade module to fips-20220613" +1

gopherbot pushed a commit that referenced this issue Feb 1, 2024
…e to fips-20220613" +1

This reverts CL 553855 ("crypto/internal/boring: upgrade module to
fips-20220613") and CL 553856 ("crypto/tls: align FIPS-only mode with
BoringSSL policy").

Fixes #65323
Updates #65321
Updates #64717
Updates #62372

Change-Id: I0938b97e5b4904e6532448b8ae76e920d03d0508
Reviewed-on: https://go-review.googlesource.com/c/go/+/558796
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 09b5de4)
Reviewed-on: https://go-review.googlesource.com/c/go/+/560275
gopherbot pushed a commit that referenced this issue Feb 1, 2024
…e to fips-20220613" +1

This reverts CL 553875 ("crypto/internal/boring: upgrade module to
fips-20220613") and CL 553876 ("crypto/tls: align FIPS-only mode with
BoringSSL policy").

Fixes #65322
Updates #65321
Updates #64717
Updates #62372

Change-Id: I0938b97e5b4904e6532448b8ae76e920d03d0508
Reviewed-on: https://go-review.googlesource.com/c/go/+/558796
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 09b5de4)
Reviewed-on: https://go-review.googlesource.com/c/go/+/560276
@sAnjAy060897
Copy link

sAnjAy060897 commented Feb 16, 2024

@FiloSottile,
I notice the boringcrypto version was downgraded back to fips-20210429 after go 1.21.6? I don't seem to find the reason for the same. We are planning to migrate to the latest FIPS 140-3 certified/in-review boring version for our products.

Can you please help us understand in which release the fips-20220613 be available? Is this version under review for FIPS 140-3 certification? Thanks!

@FiloSottile
Copy link
Contributor

@sAnjAy060897 see #65321 linked above.

ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
Also, add EVP_aead_aes_*_gcm_tls13 to the build, which we will need in a
following CL, to avoid rebuilding the syso twice.

Updates golang#64717
Updates golang#62372

Change-Id: Ie4d853ad9b914c1095cad60694a1ae6f77dc22ce
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-boringcrypto
Reviewed-on: https://go-review.googlesource.com/c/go/+/549695
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
This enables TLS 1.3, disables P-521, and disables non-ECDHE suites.

Fixes golang#64717
Updates golang#62372

Change-Id: I3a65b239ef0198bbdbe5e55e0810e7128f90a091
Reviewed-on: https://go-review.googlesource.com/c/go/+/549975
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Than McIntosh <thanm@google.com>
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
This reverts commit 7383b2a
("crypto/internal/boring: upgrade module to fips-20220613") and commit
4106de9 ("crypto/tls: align FIPS-only
mode with BoringSSL policy").

Fixes golang#65321
Updates golang#64717
Updates golang#62372

Change-Id: I0938b97e5b4904e6532448b8ae76e920d03d0508
Reviewed-on: https://go-review.googlesource.com/c/go/+/558796
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests