net/http: go1.24 breaks compatibility by modifying in-place the tls.Config{NextProtos}
#72100
Comments
dethi
changed the title
tls.Config{NextProtos}tls.Config{NextProtos}
|
The problem can also be summarised by this tiny example: https://go.dev/play/p/eg_dXkGompq |
|
CC @neild |
gabyhelp
added
the
BugReport
|
Thanks. I agree that this is a regression. |
|
Change https://go.dev/cl/654875 mentions this issue: |
|
@gopherbot please backport to go 1.24, this is a regression |
|
Backport issue(s) opened: #72103 (for 1.24). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases. |
dmitshur
added
NeedsFix
|
Hello, We are seeing a similar problem but for http.Transport. t := http.DefaultTransport.(*http.Transport).Clone()
protocols := &http.Protocols{}
protocols.SetHTTP2(true)
protocols.SetUnencryptedHTTP2(true)
t.Protocols = protocolsand used it for our purposes. But this caused other clients, If we add the following line, the issue disappears. t.TLSClientConfig = &tls.Config{}Does the same problem exist in |
|
Change https://go.dev/cl/657215 mentions this issue: |
…xtProtos Clone the input slice before adjusting NextProtos to add or remove "http/1.1" and "h2" entries, so as not to modify a slice that the caller might be using. (We clone the tls.Config that contains the slice, but that's a shallow clone.) For #72100 Fixes #72103 Change-Id: I9f228b8fb6f6f2ca5023179ec114929c002dbda9 Reviewed-on: https://go-review.googlesource.com/c/go/+/654875 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Jonathan Amsterdam <jba@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/657215
Go version
go version go1.24.0 darwin/arm64
Output of
go envin your module/workspace:What did you do?
Setup:
Run the following program locally: https://go.dev/play/p/sqxPDLLHzwX
Simulate a client connecting on the gRPC port with TLS1.3, ALPN H2:
Observe the following error:
Repeat the same test using Go 1.23 or older, TLS handshake will happen successfully.
What did you see happen?
net/http server added
Protocolsto control the allowed protocols. The server modifies the TLS ConfigNextProtosbased on this field (func adjustNextProtos).This is a different behaviour than before and cause issue when two servers (in my example net/http and gRPC server) utilise the same TLS Config. The TLS Config is soft-cloned in both server, but net/http use
slices.DeleteFuncwhich delete in-place values from the slice, thus reflecting the same change on the gRPC server.In the example I shared, h2 is disabled on the net/http server (using the old way), and so
adjustNextProtosremoves it from the TLS ConfigNextProtos. On the other side, gRPC server appendh2if it's not already present. Because the TLS Config is a shallow clone, the outcome is racy.What did you expect to see?
The expectation is that the gRPC server has h2 set in the NextProtos and so that the ALPN negotiation works correctly. This was the case up until Go 1.24.
The text was updated successfully, but these errors were encountered: