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.

Sign up for GitHub

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

Jump to bottom

crypto/tls: boringcrypto restricts RSA key sizes to 2048 and 3072 #41147

Closed
riraccuia opened this issue Aug 31, 2020 · 24 comments
Closed

crypto/tls: boringcrypto restricts RSA key sizes to 2048 and 3072 #41147

riraccuia opened this issue Aug 31, 2020 · 24 comments
Assignees
@rsc
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.20

Comments

@riraccuia
Copy link

Is there a reason why the IsBoringCertificate() check would not allow RSA key sizes > 3072 ?

Specifically, I am trying to establish a TLS connection to a corporate server that has an intermediate CA whose key size is 4096 and it throws:
tls handshake failed: x509: certificate specifies an incompatible key usage

Seems like this was recently discussed in golang-nuts ( https://groups.google.com/g/golang-nuts/c/DbzPtRDtVgQ ) but i found no open issue here.

@FiloSottile
@FiloSottile
Copy link
Contributor

@agl, it looks like there's a good argument for NIST having clarified they'll take 4096. Should we allow it?

@dmitshur dmitshur changed the title dev.boringcrypto - /crypto/tls/boring.go RSA key sizes restricted to 2048 and 3072 dev.boringcrypto: crypto/tls: in boring.go, RSA key sizes restricted to 2048 and 3072 Sep 1, 2020
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 1, 2020
@dmitshur dmitshur added this to the Backlog milestone Sep 1, 2020
@kumpfdp
Copy link

kumpfdp commented Sep 2, 2020

This would be great to be included. Today, we're having to manually apply a patch to that line of code.

@webvictim webvictim mentioned this issue Sep 2, 2020
Closed
@agl
Copy link
Contributor

agl commented Sep 15, 2020

Having looked into this, it doesn't appear that allowing other modulus sizes is strictly compliant with the current validation. However, future validations can be updated to take advantage of the increased flexibility now allowed by the IG. We expect to do this, but have no timelines to announce and do not currently have a revalidation in progress.

@sfc-gh-dwu
Copy link

It's 2021 now, any update on when we can get 4096bit validated?

@evanye
Copy link

evanye commented Jan 6, 2022

It's 2022 now. Any update on when we can get 4096 bits validated?

@agl
Copy link
Contributor

agl commented Jan 6, 2022

It's 2022 now. Any update on when we can get 4096 bits validated?

It's been nearly a year since we did a new validation that includes RSA 4096. I'm afraid NIST can take as long as they take—we've no ability to speed up their processing.

@qmuntal qmuntal mentioned this issue Jul 8, 2022
Closed
@rolandshoemaker rolandshoemaker changed the title dev.boringcrypto: crypto/tls: in boring.go, RSA key sizes restricted to 2048 and 3072 crypto/tls: boringcrypto restricts RSA key sizes to 2048 and 3072 Jul 8, 2022
@cipherboy
Copy link
Contributor

Per closed duplicate #53755:

Per Hashicorp's recent discussions with Leidos around a Letter of Attestation for Vault based on BoringCrypto's 3678 certificate, larger key sizes are allowed. In particular, this limitation shouldn't be necessary as, per our lab contact (and with permission to quote in the interest of getting this change upstreamed):

P. said:

FIPS 140-2 IG A.14 allows for one to use RSA key sizes >= 2048 bits, so long as the ones that are testable have been tested. At the time of the BoringCrypto certification, only up to 3072 was testable, so IG A.14 would allow you to use anything above that as well.

Since the relevant BC certificate has both 2048 and 3072 tested, we sould simplify the check to >= 2048.

Let me know if this is agreeable and I can open a PR.

(TL;DR: Leidos confirmed any key size >= 2048 is safe to allow under FIPS 140-2 IG A.14, and the existing cert without 4096-bit tested is sufficient for 4096-bit keys, as 4096-bit wasn't a testing target when the existing cert was released).

@evanye
Copy link

evanye commented Jul 8, 2022

@cipherboy that's great news! please keep us updated on which go version this can be merged into, so that we can stop using our fork of boringcrypto :)

@rolandshoemaker
Copy link
Member

@agl as the resident FIPS person, do you have an opinion on this?

@irbekrm
Copy link
Contributor

irbekrm commented Aug 12, 2022

Awesome to hear that we might be able to use 4096 bit RSA keys!
Would be great to know when/if the TLS check in boring Go is going to updated- we also would like to use 4096 bit keys and now that we know it might be compliant, we have to decide between forking 'boring' Go to apply a patch or waiting for an update to the TLS check 😅

@danisevas
Copy link

So with what @cipherboy wrote, can a fix be added to version 1.20?

@Lovesmoring
Copy link

hi, will this be added to 1.20? I see #53755 has milestone go1.20 but this one hasn't

@ianlancetaylor
Copy link
Contributor

Based on the discussion on this issue, no decision has been made yet.

@cipherboy
Copy link
Contributor

@agl / @ianlancetaylor / @rolandshoemaker - Feel free to drop me a mail at my work email (alex.scheel@hashicorp.com) if you want me to put you in touch with Leidos and our contact there if that'll help move things along.

@JINHAOHU JINHAOHU mentioned this issue Oct 13, 2022
Closed
@jaredpar
Copy link

Reading through this issue I'm confused if the sticking point is:

  1. Whether FIPS 140-2 allows for this key length
  2. Or whether BoringSSL has been certified for this key length

I understand a few people have asserted that (1) is allowed and that is my understanding as well. So it would seem that it's about (2) but wanted to verify that is where the issue is sitting at.

Or possible (3) just not the right resource trade off right now 😄

@cipherboy
Copy link
Contributor

@jaredpar My statement above from our lab states not only that 1 is allowed, but 2 is as well. I've offered to put the Go team in touch with this lab if they desire to validate this, but gotten no inquiries. And I've also volunteered to submit a patch fixing this issue.

I believe the issue is thus 3, resource tradeoff and are likely busy with other higher priorities (purely speculation here, but perhaps they're considering a newer BC cert that might invalidate 2 due to omission of certifying higher RSA key sizes under later FIPS 140-2 IG)...

But not working on the Go team, your guess is as good as mine.

@rsc
Copy link
Contributor

rsc commented Nov 3, 2022

Talked to @agl about this. I will send a CL for Go 1.20.

@rsc rsc self-assigned this Nov 3, 2022
@rsc rsc added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 3, 2022
@rsc rsc modified the milestones: Backlog, Go1.20 Nov 3, 2022
@gopherbot
Copy link

Change https://go.dev/cl/447655 mentions this issue: crypto/x509: allow BoringCrypto to use 4096-bit keys

@cipherboy
Copy link
Contributor

Thank you @rsc!

@rsc
Copy link
Contributor

rsc commented Nov 9, 2022

@gopherbot please backport

This was referenced Nov 9, 2022
Closed
Closed
@gopherbot
Copy link

Backport issue(s) opened: #56671 (for 1.18), #56672 (for 1.19).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@rsc
Copy link
Contributor

rsc commented Nov 9, 2022

Only needs Go 1.19; closed the Go 1.18 backport.

@gopherbot
Copy link

Change https://go.dev/cl/449016 mentions this issue: [release-branch.go1.19] crypto/x509: allow BoringCrypto to use 4096-bit keys

@gopherbot
Copy link

Change https://go.dev/cl/449639 mentions this issue: [dev.boringcrypto.go1.18] crypto/tls: allow BoringCrypto to use 4096-bit keys

gopherbot pushed a commit that referenced this issue Nov 11, 2022
@rsc
[dev.boringcrypto.go1.18] crypto/tls: allow BoringCrypto to use 4096-…
f241e00 
…bit keys

FIPS-140 has been updated to allow 4096-bit RSA keys.
Allow them in certificate processing.

This is the Go 1.18 boringcrypto branch version of CL 447655.
Not a straight cherry-pick, because the code in the boringcrypto branch
is different from the code that merged into the main branch.

Fixes #41147 for the Go 1.18 boringcrypto branch.

Change-Id: Iae8a6406a2885e6546df2c28c1791c19cfafb6b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/449639
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
gopherbot pushed a commit that referenced this issue Nov 13, 2022
@rsc
[release-branch.go1.19] crypto/x509: allow BoringCrypto to use 4096-b…
861ba02 
…it keys

FIPS-140 has been updated to allow 4096-bit RSA keys.
Allow them in certificate processing.

For #41147.
Fixes #56672.

Change-Id: I4c6bcb1b137a200dfe70cebc605ae57f49871184
Reviewed-on: https://go-review.googlesource.com/c/go/+/447655
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/449016
andrew-d pushed a commit to tailscale/go that referenced this issue Dec 7, 2022
@rsc @andrew-d
[release-branch.go1.19] crypto/x509: allow BoringCrypto to use 4096-b…
a8ce6fd 
…it keys

FIPS-140 has been updated to allow 4096-bit RSA keys.
Allow them in certificate processing.

For golang#41147.
Fixes golang#56672.

Change-Id: I4c6bcb1b137a200dfe70cebc605ae57f49871184
Reviewed-on: https://go-review.googlesource.com/c/go/+/447655
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/449016
@FiloSottile FiloSottile mentioned this issue Mar 1, 2023
Closed
@golang golang locked and limited conversation to collaborators Nov 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@rsc rsc

Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.20
Development

No branches or pull requests