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: verifying certificate chains containing large RSA keys is slow [CVE-2023-29409] #61460

Closed
rolandshoemaker opened this issue Jul 19, 2023 · 27 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker Security
Milestone

Comments

@rolandshoemaker
Copy link
Member

rolandshoemaker commented Jul 19, 2023

Clients and servers which request and verify client certificates can be forced to expend a large amount of time verifying certificate chains which contain very large RSA keys during TLS handshakes.

Thanks to Mateusz Poliwczak for reporting this issue.

This is CVE-2023-29409.


This is a PRIVATE issue for CVE-2023-29409, tracked in http://b/282987216 and fixed by http://tg/1912161.

/cc @golang/security and @golang/release

@rolandshoemaker rolandshoemaker added this to the Go1.21 milestone Jul 19, 2023
@heschi heschi added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 20, 2023
@neild
Copy link
Contributor

neild commented Jul 25, 2023

@gopherbot please open backport issues

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #61579 (for 1.19), #61580 (for 1.20).

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

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/514915 mentions this issue: [release-branch.go1.19] crypto/tls: restrict RSA keys in certificates to <= 8192 bits

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/514900 mentions this issue: [release-branch.go1.20] crypto/tls: restrict RSA keys in certificates to <= 8192 bits

gopherbot pushed a commit that referenced this issue Aug 1, 2023
… to <= 8192 bits

Extremely large RSA keys in certificate chains can cause a client/server
to expend significant CPU time verifying signatures. Limit this by
restricting the size of RSA keys transmitted during handshakes to <=
8192 bits.

Based on a survey of publicly trusted RSA keys, there are currently only
three certificates in circulation with keys larger than this, and all
three appear to be test certificates that are not actively deployed. It
is possible there are larger keys in use in private PKIs, but we target
the web PKI, so causing breakage here in the interests of increasing the
default safety of users of crypto/tls seems reasonable.

Thanks to Mateusz Poliwczak for reporting this issue.

Updates #61460
Fixes #61580
Fixes CVE-2023-29409

Change-Id: Ie35038515a649199a36a12fc2c5df3af855dca6c
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1912161
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
(cherry picked from commit d865c715d92887361e4bd5596e19e513f27781b7)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1965747
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/514900
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Aug 1, 2023
… to <= 8192 bits

Extremely large RSA keys in certificate chains can cause a client/server
to expend significant CPU time verifying signatures. Limit this by
restricting the size of RSA keys transmitted during handshakes to <=
8192 bits.

Based on a survey of publicly trusted RSA keys, there are currently only
three certificates in circulation with keys larger than this, and all
three appear to be test certificates that are not actively deployed. It
is possible there are larger keys in use in private PKIs, but we target
the web PKI, so causing breakage here in the interests of increasing the
default safety of users of crypto/tls seems reasonable.

Thanks to Mateusz Poliwczak for reporting this issue.

Updates #61460
Fixes #61579
Fixes CVE-2023-29409

Change-Id: Ie35038515a649199a36a12fc2c5df3af855dca6c
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1912161
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
(cherry picked from commit d865c715d92887361e4bd5596e19e513f27781b7)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1965487
Reviewed-on: https://go-review.googlesource.com/c/go/+/514915
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Bypass: David Chase <drchase@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/515056 mentions this issue: [release-branch.go1.21] crypto/tls: restrict RSA keys in certificates to <= 8192 bits

@gopherbot
Copy link
Contributor

Closed by merging a51957f to release-branch.go1.21.

gopherbot pushed a commit that referenced this issue Aug 1, 2023
… to <= 8192 bits

Extremely large RSA keys in certificate chains can cause a client/server
to expend significant CPU time verifying signatures. Limit this by
restricting the size of RSA keys transmitted during handshakes to <=
8192 bits.

Based on a survey of publicly trusted RSA keys, there are currently only
three certificates in circulation with keys larger than this, and all
three appear to be test certificates that are not actively deployed. It
is possible there are larger keys in use in private PKIs, but we target
the web PKI, so causing breakage here in the interests of increasing the
default safety of users of crypto/tls seems reasonable.

Thanks to Mateusz Poliwczak for reporting this issue.

Updates #61460
Fixes CVE-2023-29409

Change-Id: Ie35038515a649199a36a12fc2c5df3af855dca6c
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1912161
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
(cherry picked from commit d865c715d92887361e4bd5596e19e513f27781b7)
Reviewed-on: https://go-review.googlesource.com/c/go/+/515056
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@dmitshur
Copy link
Contributor

dmitshur commented Aug 2, 2023

This is fixed on release-branch.go1.21, but still needs to be fixed on the main branch for Go 1.22. Reopening for that.

(CL 515056 said "Updates #​61460", not "Fixes #​61460", but this got closed ahead of time due anyway due to #29599.)

@dmitshur dmitshur reopened this Aug 2, 2023
@dmitshur dmitshur modified the milestones: Go1.21, Go1.22 Aug 2, 2023
bradfitz pushed a commit to tailscale/go that referenced this issue Aug 2, 2023
… to <= 8192 bits

Extremely large RSA keys in certificate chains can cause a client/server
to expend significant CPU time verifying signatures. Limit this by
restricting the size of RSA keys transmitted during handshakes to <=
8192 bits.

Based on a survey of publicly trusted RSA keys, there are currently only
three certificates in circulation with keys larger than this, and all
three appear to be test certificates that are not actively deployed. It
is possible there are larger keys in use in private PKIs, but we target
the web PKI, so causing breakage here in the interests of increasing the
default safety of users of crypto/tls seems reasonable.

Thanks to Mateusz Poliwczak for reporting this issue.

Updates golang#61460
Fixes CVE-2023-29409

Change-Id: Ie35038515a649199a36a12fc2c5df3af855dca6c
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1912161
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
(cherry picked from commit d865c715d92887361e4bd5596e19e513f27781b7)
Reviewed-on: https://go-review.googlesource.com/c/go/+/515056
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@geofffranks
Copy link

Why was this implemented as a breaking change when backported in patch releases to existing golang versions? Usually y'all implement some sort of environment variable flag to allow code to opt in/out of changes like this until the next minor version.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/515257 mentions this issue: crypto/tls: restrict RSA keys in certificates to <= 8192 bits

@FiloSottile
Copy link
Contributor

Note that the golang-announce announcement is missing a reference to this issue, or to the CVE number.

@dmitshur
Copy link
Contributor

dmitshur commented Aug 2, 2023

The fix has now landed both to release-branch.go1.21 (for the upcoming go1.21.0 release, and already included in go1.21rc4) and to the main branch (for the eventual go1.22.0 release). Moving the milestone back to Go1.21 since that's the earlier of the two. (We don't have dedicated backport issues for Go1.21 during early thaw.)

The title and description of this issue still need to be updated though. (CC @dr2chase.)

@dmitshur dmitshur modified the milestones: Go1.22, Go1.21 Aug 2, 2023
@neild neild changed the title security: fix CVE-2023-29409 crypto/tls: verifying certificate chains containing large RSA keys is slow [CVE-2023-29409] Aug 2, 2023
@rsc
Copy link
Contributor

rsc commented Aug 2, 2023

@geofffranks Generally speaking we use GODEBUGs for behavior changes that we expect to break people, and in security fixes we limit ourselves to changes we do not expect to break people. Are you using RSA keys > 8192-bit, or do you know of any common use of keys that large? Thanks.

@Gerg
Copy link

Gerg commented Aug 2, 2023

For us, we distribute software built with Golang that is intended to be run in our customers' private networks. It is unlikely that any of our customers use such large RSA keys, but it's difficult to prove that they don't.

Without an "escape hatch" for this change, we probably have to pass this on to our customers as a breaking change, which complicates our release/support process.

@geofffranks
Copy link

The CVE for this hasn't been published yet, so maybe there's information forthcomming that changes how this appears, but this change seems analogous to if a hard limit to reads on a TCP connection were implemented at some arbitrary value, in the interest of preventing clients from denying service by issuing infinite data streams.

Similar caps to protect servers from malicious client behavior exist, but they're all configurable by the server. This cap is arbitrary, and hardcoded in the language. For users wanting to use 16K RSA keys for a client or server (testing, research, extreme paranoia for highly sensitive material), golang is no longer an option.

@rolandshoemaker
Copy link
Member Author

The 8192 bit limit was picked as it was considered an upper reasonable limit for acceptable verification latency, and because it was the upper limit of observed deployed keys. Surveying the web PKI, we found only two publicly trusted certificates containing 8192 bit or larger keys (both being 8192 bits). Since we primarily target the web PKI, this was considered an acceptable limit, since there were no known deployed publicly trusted keys above the limit.

Clearly people using private PKIs may use extremely large RSA keys, but it was my assumption that it would be extremely rare for anyone to be using keys larger than 8K, since that is already 2x the largest keys you typically see.

We generally try to avoid introducing GODEBUG flags in security changes (and in minor releases in general), but if it is clear that there are people relying on this behavior in the wild, we can look into introducing a flag that configures this behavior in the next minor release.

@geofffranks
Copy link

We generally try to avoid introducing GODEBUG flags in security changes (and in minor releases in general), but if it is clear that there are people relying on this behavior in the wild, we can look into introducing a flag that configures this behavior in the next minor release.

My point is that golang has allowed users to customize via code which ciphers and TLS versions to use (including less secure options to a point). It has allowed users to customers to customize via code timeouts and maximum request sizes. Why was this any different?

@FiloSottile
Copy link
Contributor

Some things in crypto/tls are configurable for legacy or compatibility reasons, but conversely there are a number of things that are not configurable in crypto/tls. In particular, we don't support testing and research use cases through the public API, encouraging instead forks like uTLS, zcrypto/ztls, and BoGo.

To put the "extreme paranoia" angle in perspective, the NSA Commercial National Security Algorithm Suite requires 3072-bit keys all the way to Top Secret. The WebPKI root CAs have 4096-bit keys. Using keys larger than 8192 bits has no security justification, even in "extreme paranoia" threat models.

@Astaoth
Copy link

Astaoth commented Aug 22, 2023

Hi,

I would like to underline that 16k TLS keys is the standard upper limit in most OS for at least a decade. By making an arbitrary hardcoded limit to 8k keys breaks the standard compatibility. This should be a parameter and not hardcoded.

Also, limiting the RSA key size is a weird way to fix this issue. It's like "your car AC sometime leaks ? Well I'll fix this by removing the AC of all the cars of the same brand. Now there is no AC leak anymore, but you can't have fresh air when it's warm outside.". It's not a fix at all, it's limiting/removing a functionality because for a large minority of users, sometime, it takes a "long time" (whatever this means) to check the RSA key.

On a side note, qualifying 16k RSA keys of "extreme paranoia" is kind of condescendant with the users using them : you don't know the use cases, you can't make any kind of judgement.

Kind regards

@mateusz834
Copy link
Member

@Astaoth #61965

@Astaoth
Copy link

Astaoth commented Aug 22, 2023

That's not really an answer. What you suggest can be done only at the building time and not at the execution time.

@Astaoth
Copy link

Astaoth commented Aug 31, 2023

@mateusz834 @rolandshoemaker

Well, as this issue isn't really fixed but only hidden, out of curiosity, how will you react when the same issue will be reported to you with RSA 8k keys and light servers ? Will you rull out RSA 8k keys as well and states that you don't support light hardware ?
And next, when the same issue will be reported to you with IOT devices and RSA 4k keys, will you state that you don't support IOT devices and you will drop RSA support as well ?

The way this issue is fixed is ridiculous and against good developpement and security pratices.

@heschi
Copy link
Contributor

heschi commented Aug 31, 2023

@Astaoth Please follow the Code of Conduct.

For the record, you're incorrect: GODEBUGs can be applied at runtime.

@Astaoth
Copy link

Astaoth commented Aug 31, 2023

Hi @heschi,

Thank you for the information about the GODEBUG.

Apologies but I fail to see how I've broken the code of conduct. The questions about 8k keys and IOT seem to be legitimate ones and their answers would allow me a better understanding of the GO team philosophy, especially about the security vision.

@ianlancetaylor
Copy link
Contributor

@Astaoth The code of conduct asks people to be charitable and respectful. There are ways to ask questions about Go development choices that do not predict what the response will be, and do not refer to what people are doing as ridiculous. Thanks.

@Astaoth
Copy link

Astaoth commented Aug 31, 2023

@ianlancetaylor That's interresting : in this case how would judging people and organizations in a condescending manner without knowing the context or judging the people intents respect the Code of Conduct ? Or unwelcoming 15 char messages ?

Also may I underline that I would haven't made any more noise than necessary here if an appropriate answer would have been provided in first place ?

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Aug 31, 2023

I apologize on behalf of other open source contributors for making you feel condescended to or unwelcome. That is not a pleasant feeling, and I'm sorry that it happened. My only defense is that I'm sure it was not intended. The principle of charity applies here in particular: please assume that people responding to you have good intentions.

Nobody is asking you to not make noise. We are asking you to make noise respectfully and charitably.

Finally, note that this is not a support forum; for appropriate forums for questions about using Go please see https://go.dev/wiki/Questions.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/563136 mentions this issue: crypto/x509: document that Verify does not restrict key sizes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker Security
Projects
None yet
Development

No branches or pull requests