go: crypto/tls: support ECDHE key exchanges when ec_point_formats is missing in ClientHello extension
What did you do?
For backwards compatibility purposes, the point format list extension MAY still be included and contain exactly one value: the uncompressed point format (0). RFC 4492 specified that if this extension is missing, it means that only the uncompressed point format is supported, so interoperability with implementations that support the uncompressed format should work with or without the extension
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.go
// supportsECDHE returns whether ECDHE key exchanges can be used with this
// pre-TLS 1.3 client.
func supportsECDHE(c *Config, supportedCurves []CurveID, supportedPoints []uint8) bool {
...
supportsPointFormat := false
for _, pointFormat := range supportedPoints {
if pointFormat == pointFormatUncompressed {
supportsPointFormat = true
break
}
}
return supportsCurve && supportsPointFormat
}
What did you expect to see?
If ec_point_formats is missing in ClientHello, we will allow ECDHE key exchanges because it means that only the uncompressed point format is supported
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Reactions: 4
- Comments: 16 (5 by maintainers)
Commits related to this issue
- crypto: support ECDHE when ec_point_formats is missing in ClientHello As describe in rfc8422 5.1.2, we will support ECDHE in the case client does not include ec_point_formats extension in ClientHello... — committed to yang-wei/go by yang-wei 3 years ago
- [release-branch.go1.18] crypto/tls: support ECDHE when ec_point_formats is missing Updates #49126 Fixes #54642 Change-Id: I9d6f6392b1a6748bdac1d2c6371b22d75829a2b6 Reviewed-on: https://go-review.goo... — committed to golang/go by FiloSottile 2 years ago
- [release-branch.go1.19] crypto/tls: support ECDHE when ec_point_formats is missing Updates #49126 Fixes #54643 Change-Id: I9d6f6392b1a6748bdac1d2c6371b22d75829a2b6 Reviewed-on: https://go-review.goo... — committed to golang/go by FiloSottile 2 years ago
- crypto/tls: support ECDHE when ec_point_formats is missing Fixes #49126 Change-Id: I9d6f6392b1a6748bdac1d2c6371b22d75829a2b6 Reviewed-on: https://go-review.googlesource.com/c/go/+/425295 Run-TryBot:... — committed to rajbarik/go by FiloSottile 2 years ago
- [release-branch.go1.19] crypto/tls: support ECDHE when ec_point_formats is missing Updates #49126 Fixes #54643 Change-Id: I9d6f6392b1a6748bdac1d2c6371b22d75829a2b6 Reviewed-on: https://go-review.goo... — committed to tailscale/go by FiloSottile 2 years ago
- update qtls to versions that include the ec_point_formats fix The new qtls versions contain the standard library fix for https://github.com/golang/go/issues/49126. — committed to quic-go/quic-go by marten-seemann 2 years ago
- update qtls to versions that include the ec_point_formats fix The new qtls versions contain the standard library fix for https://github.com/golang/go/issues/49126. — committed to quic-go/quic-go by marten-seemann 2 years ago
- update qtls to versions that include the ec_point_formats fix (#3583) The new qtls versions contain the standard library fix for https://github.com/golang/go/issues/49126. — committed to quic-go/quic-go by marten-seemann 2 years ago
- update qtls to versions that include the ec_point_formats fix (#3583) The new qtls versions contain the standard library fix for https://github.com/golang/go/issues/49126. — committed to Psiphon-Labs/quic-go by marten-seemann 2 years ago
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 (https://github.com/golang/go/issues/49126#issuecomment-972987103) 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.
We’re planning on landing a fix for this in 1.20.
@cipherboy Btw, the PR #49127 above is from Oct 23, 2021 and modifies a single line.
@yang-wei Did see this part from @gopherbot reply on PR #49127?