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: randomly generate ticket_age_add [freeze exception] #52814
Comments
cc @golang/security |
Ah, yes, that's an oversight, thank you. I would argue this is worth a freeze exception, because the fix is very simple, we are early in the freeze, and it fixes a privacy issue (allowing network observers to correlate successive connections even if the client changed location). Maybe even worth a CVE and a backport? Not sure. /cc @golang/release If we ever expose the session ticket structure (which might be something that happens as part of #25351 #46718 #19199 #6379), we might want to add the obfuscated_ticket_age to it (or start generating it from the key material?) in case other implementations want to use it, but for now we are the only consumers of the tickets and indeed don't care about it. (While at it, let's also add a note about why ticket_nonce is always zero, which is simply that we only ever send one ticket. It's a good comment to have in there in case that invariant ever changes.) |
Here's my patch, for what it's worth: https://github.com/nervuri/go/commit/1cfb6b392b1d22127c9f6afcf45619d190ed9bfe Not sure if I should submit it as a pull request... for such a small change, the contribution setup is a tad overkill. :) And you may want to write the patch differently. What do you think? |
cc @golang/security If the fix is easy and we can get a CL in before the end of the week I think it's fine. Up to the security team to decide about a CVE/backport. |
We will work to get a CL in this week. Backporting seems reasonable given how small the fix is. @gopherbot Please open backport issues for Go 1.17 and Go 1.18, this is a privacy issue. |
Backport issue(s) opened: #52832 (for 1.17), #52833 (for 1.18). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases. |
The Go project does accept normal github pull requests: https://go.dev/doc/contribute#sending_a_change_github |
Yes, but to contribute I need to sign a CLA, for which I need a Google account, for which I have to provide my phone number. I would not have found this bug if I didn't care about privacy. Requiring all contributors to get an account with Big Brother is just rude. My pull request won't go through, but do feel free to use the code. I am providing it to the project under the project's license (although the patch is so small that it's probably not copyrightable). |
@nervuri I completely understand not wanting to sign the CLA, but in that situation please don't send us code via GitHub pull requests or e-mail or in any other way. Our legal advice is very clear: we can only accept code changes from people who have signed the CLA. When you send us code without signing the CLA we have to be careful to not look at that code and not rely on it in any way in making our own changes. Thanks for understanding. |
I would be willing to sign the CLA if it didn't require using a Google account and providing Google my phone number. Golang could use Github's cla-bot, for instance. Or I'll just do it here: I hereby sign the CLA currently hosted at https://cla.developers.google.com/about/google-individual Anyway, I closed the pull request - hope that makes it easier for you. I'm sure nobody looked at the code. :) |
Change https://go.dev/cl/405994 mentions this issue: |
This freeze exception has been approved. |
Change https://go.dev/cl/408574 mentions this issue: |
Change https://go.dev/cl/408575 mentions this issue: |
As required by RFC 8446, section 4.6.1, ticket_age_add now holds a random 32-bit value. Before this change, this value was always set to 0. This change also documents the reasoning for always setting ticket_nonce to 0. The value ticket_nonce must be unique per connection, but we only ever send one ticket per connection. Updates #52814 Fixes #52833 Fixes CVE-2022-30629 Change-Id: I6c2fc6ca0376b7b968abd59d6d3d3854c1ab68bb Reviewed-on: https://go-review.googlesource.com/c/go/+/405994 Reviewed-by: Tatiana Bradley <tatiana@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> Run-TryBot: Tatiana Bradley <tatiana@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit fe4de36) Reviewed-on: https://go-review.googlesource.com/c/go/+/408575 Run-TryBot: Roland Shoemaker <roland@golang.org>
As required by RFC 8446, section 4.6.1, ticket_age_add now holds a random 32-bit value. Before this change, this value was always set to 0. This change also documents the reasoning for always setting ticket_nonce to 0. The value ticket_nonce must be unique per connection, but we only ever send one ticket per connection. Updates #52814 Fixes #52832 Fixes CVE-2022-30629 Change-Id: I6c2fc6ca0376b7b968abd59d6d3d3854c1ab68bb Reviewed-on: https://go-review.googlesource.com/c/go/+/405994 Reviewed-by: Tatiana Bradley <tatiana@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> Run-TryBot: Tatiana Bradley <tatiana@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit fe4de36) Reviewed-on: https://go-review.googlesource.com/c/go/+/408574 Run-TryBot: Roland Shoemaker <roland@golang.org>
Would it be possible to cast some more light for the reason for issuing CVE-2022-30629? As an outsider, this seems more like RFC 8446 non-compliance issue since the server does not use |
As required by RFC 8446, section 4.6.1, ticket_age_add now holds a random 32-bit value. Before this change, this value was always set to 0. This change also documents the reasoning for always setting ticket_nonce to 0. The value ticket_nonce must be unique per connection, but we only ever send one ticket per connection. Updates golang#52814 Fixes golang#52832 Fixes CVE-2022-30629 Change-Id: I6c2fc6ca0376b7b968abd59d6d3d3854c1ab68bb Reviewed-on: https://go-review.googlesource.com/c/go/+/405994 Reviewed-by: Tatiana Bradley <tatiana@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> Run-TryBot: Tatiana Bradley <tatiana@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit fe4de36) Reviewed-on: https://go-review.googlesource.com/c/go/+/408574 Run-TryBot: Roland Shoemaker <roland@golang.org>
As required by RFC 8446, section 4.6.1, ticket_age_add now holds a random 32-bit value. Before this change, this value was always set to 0. This change also documents the reasoning for always setting ticket_nonce to 0. The value ticket_nonce must be unique per connection, but we only ever send one ticket per connection. Updates golang#52814 Fixes golang#52832 Fixes CVE-2022-30629 Change-Id: I6c2fc6ca0376b7b968abd59d6d3d3854c1ab68bb Reviewed-on: https://go-review.googlesource.com/c/go/+/405994 Reviewed-by: Tatiana Bradley <tatiana@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> Run-TryBot: Tatiana Bradley <tatiana@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit fe4de36) Reviewed-on: https://go-review.googlesource.com/c/go/+/408574 Run-TryBot: Roland Shoemaker <roland@golang.org>
# AWS EKS Backported To: go-1.15.15-eks Backported On: Thu, 22 Sept 2022 Backported By: budris@amazon.com Backported From: release-branch.go1.17 EKS Patch Source Commit: danbudris@e25bfe0 Upstream Source Commit: golang@c15a8e2 # Original Information As required by RFC 8446, section 4.6.1, ticket_age_add now holds a random 32-bit value. Before this change, this value was always set to 0. This change also documents the reasoning for always setting ticket_nonce to 0. The value ticket_nonce must be unique per connection, but we only ever send one ticket per connection. Updates golang#52814 Fixes golang#52832 Fixes CVE-2022-30629 Change-Id: I6c2fc6ca0376b7b968abd59d6d3d3854c1ab68bb Reviewed-on: https://go-review.googlesource.com/c/go/+/405994 Reviewed-by: Tatiana Bradley <tatiana@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> Run-TryBot: Tatiana Bradley <tatiana@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit fe4de36) Reviewed-on: https://go-review.googlesource.com/c/go/+/408574 Run-TryBot: Roland Shoemaker <roland@golang.org>
# AWS EKS Backported To: go-1.15.15-eks Backported On: Thu, 22 Sept 2022 Backported By: budris@amazon.com Backported From: release-branch.go1.17 EKS Patch Source Commit: danbudris@e25bfe0 Upstream Source Commit: golang@c15a8e2 # Original Information As required by RFC 8446, section 4.6.1, ticket_age_add now holds a random 32-bit value. Before this change, this value was always set to 0. This change also documents the reasoning for always setting ticket_nonce to 0. The value ticket_nonce must be unique per connection, but we only ever send one ticket per connection. Updates golang#52814 Fixes golang#52832 Fixes CVE-2022-30629 Change-Id: I6c2fc6ca0376b7b968abd59d6d3d3854c1ab68bb Reviewed-on: https://go-review.googlesource.com/c/go/+/405994 Reviewed-by: Tatiana Bradley <tatiana@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> Run-TryBot: Tatiana Bradley <tatiana@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit fe4de36) Reviewed-on: https://go-review.googlesource.com/c/go/+/408574 Run-TryBot: Roland Shoemaker <roland@golang.org>
# AWS EKS Backported To: go-1.16.15-eks Backported On: Tue, 04 Oct 2022 Backported By: budris@amazon.com Backported From: release-branch.go1.17 EKS Patch Source Commit: danbudris@3beb7c1 Upstream Source Commit: golang@c15a8e2 # Original Information As required by RFC 8446, section 4.6.1, ticket_age_add now holds a random 32-bit value. Before this change, this value was always set to 0. This change also documents the reasoning for always setting ticket_nonce to 0. The value ticket_nonce must be unique per connection, but we only ever send one ticket per connection. Updates golang#52814 Fixes golang#52832 Fixes CVE-2022-30629 Change-Id: I6c2fc6ca0376b7b968abd59d6d3d3854c1ab68bb Reviewed-on: https://go-review.googlesource.com/c/go/+/405994 Reviewed-by: Tatiana Bradley <tatiana@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> Run-TryBot: Tatiana Bradley <tatiana@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit fe4de36) Reviewed-on: https://go-review.googlesource.com/c/go/+/408574 Run-TryBot: Roland Shoemaker <roland@golang.org>
crypto/tls always sets
newSessionTicketMsgTLS13.ageAdd
to 0, which makes it so that clients resuming a session can't obfuscate theobfusacted_ticket_age
. This violates the TLS 1.3 spec (RFC 8446, section 4.6.1):See the sendSessionTickets() function.
How to reproduce
srv.SetKeepAlivesEnabled(false)
; we don't want connection reuse)tls.handshake
curl -k https://localhost:8443 https://localhost:8443
In Wireshark, open the second Client Hello message, look at the
pre_shared_key
extension and you'll see thatobfuscated_ticket_age
is 0 (or very close to 0).Proposed fix
Given that you don't check the obfuscated_ticket_age, it's enough to assign
ageAdd
a random value each time.The text was updated successfully, but these errors were encountered: