-
Notifications
You must be signed in to change notification settings - Fork 17.5k
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: support ECDHE key exchanges when ec_point_formats is missing in ClientHello extension #49126
Comments
As describe in rfc8422 5.1.2, we will support ECDHE in the case client does not include ec_point_formats extension in ClientHello extension. This make sure ECDHE will work with (uncompressed point format is listed explicitly) or without extension. rfc8422 5.1.2: https://datatracker.ietf.org/doc/html/rfc8422#section-5.1.2. Fixes golang#49126
Change https://golang.org/cl/358116 mentions this issue: |
cc @FiloSottile |
Hey, how is everything? Any updates on this? |
This issue is affecting Windows 11 and Windows Server 2022 built-in Schannel library latest TLS 1.3 implementation. It seems TLS1_3_CLIENT sends Sites unreachable by TLS1_3_CLIENT: |
@yang-wei Did see this part from @gopherbot reply on PR #49127?
|
CC @golang/security Looks like this didn't make 1.19. Moving to backlog. Please recategorize as appropriate. |
@ianlancetaylor Any movement on this? Hashicorp is also affected, per duplicate ticket. Though, in this case it is TLSv1.2-only client, not a Microsoft TLSv1.3 client (unlike this OP). |
@cipherboy Btw, the PR #49127 above is from Oct 23, 2021 and modifies a single line. |
We're planning on landing a fix for this in 1.20. |
So am i right in thinking 1.20 will be about early 2023? Is there really no way to get it out before then? Go is clearly broken and doesn't support the TLS spec properly and there is a one line fix... |
Interestingly, this is only hit when using the Windows TLS1_3_CLIENT (or a different client that omits the extension) to reach a TLS 1.2-only Go server, because the extension is ignored entirely when negotiating TLS 1.3. This probably explains why it was only noticed for test sites (#49126 (comment)) which might disable TLS 1.3 to test for vulnerabilities, and why it didn't cause widespread breakage. Still, considering the duplicates that were filed, the fact that we're the odd one out in getting this wrong, the fact that we're off-spec, the fact that this is not something applications can workaround (unless they can upgrade to TLS 1.3), how this kind of issue tends to add complexity to the ecosystem in the form of workarounds, and the simplicity of the fix, I think we should backport it. PR #49127 / CL 358116 is an incomplete fix because we should not send the extension back when the client didn't send it. I'll send a new fix in a second. @gopherbot please open backport issues for both supported releases. |
Backport issue(s) opened: #54642 (for 1.18), #54643 (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. |
Change https://go.dev/cl/425295 mentions this issue: |
Change https://go.dev/cl/425635 mentions this issue: |
Change https://go.dev/cl/425636 mentions this issue: |
…ts is missing Updates #49126 Fixes #54642 Change-Id: I9d6f6392b1a6748bdac1d2c6371b22d75829a2b6 Reviewed-on: https://go-review.googlesource.com/c/go/+/425295 Run-TryBot: Filippo Valsorda <[email protected]> Auto-Submit: Filippo Valsorda <[email protected]> Reviewed-by: Alex Scheel <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Reviewed-by: David Chase <[email protected]> (cherry picked from commit 1df2a03) Reviewed-on: https://go-review.googlesource.com/c/go/+/425636
…ts is missing Updates #49126 Fixes #54643 Change-Id: I9d6f6392b1a6748bdac1d2c6371b22d75829a2b6 Reviewed-on: https://go-review.googlesource.com/c/go/+/425295 Run-TryBot: Filippo Valsorda <[email protected]> Auto-Submit: Filippo Valsorda <[email protected]> Reviewed-by: Alex Scheel <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Reviewed-by: David Chase <[email protected]> (cherry picked from commit 1df2a03) Reviewed-on: https://go-review.googlesource.com/c/go/+/425635
Fixes golang#49126 Change-Id: I9d6f6392b1a6748bdac1d2c6371b22d75829a2b6 Reviewed-on: https://go-review.googlesource.com/c/go/+/425295 Run-TryBot: Filippo Valsorda <[email protected]> Auto-Submit: Filippo Valsorda <[email protected]> Reviewed-by: Alex Scheel <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Reviewed-by: David Chase <[email protected]>
…ts is missing Updates golang#49126 Fixes golang#54643 Change-Id: I9d6f6392b1a6748bdac1d2c6371b22d75829a2b6 Reviewed-on: https://go-review.googlesource.com/c/go/+/425295 Run-TryBot: Filippo Valsorda <[email protected]> Auto-Submit: Filippo Valsorda <[email protected]> Reviewed-by: Alex Scheel <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Reviewed-by: David Chase <[email protected]> (cherry picked from commit 1df2a03) Reviewed-on: https://go-review.googlesource.com/c/go/+/425635
The new qtls versions contain the standard library fix for golang/go#49126.
The new qtls versions contain the standard library fix for golang/go#49126.
The new qtls versions contain the standard library fix for golang/go#49126.
…o#3583) The new qtls versions contain the standard library fix for golang/go#49126.
What did you do?
Per rfc8422#section-5.1.2,
We are seeing TLS handshake failure (client and server failed to agree on ECDHE_ECDSA key exchange algorithem) when
ec_point_formats
is missing because we expect it to be listed in tls/handshake_server.goWhat did you expect to see?
If
ec_point_formats
is missing in ClientHello, we will allow ECDHE key exchanges becauseit means that only the uncompressed point format is supported
The text was updated successfully, but these errors were encountered: