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

Jump to bottom

crypto/tls: session resumption fails with a cloned tls.Config #60506

Open
marten-seemann opened this issue May 30, 2023 · 3 comments
Open

crypto/tls: session resumption fails with a cloned tls.Config #60506

marten-seemann opened this issue May 30, 2023 · 3 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@marten-seemann
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version go1.20.3 darwin/arm64

Does this issue reproduce with the latest release?

Yes, reproduced with tip.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/marten/Library/Caches/go-build"
GOENV="/Users/marten/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/marten/src/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/marten/src/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/marten/bin/go1.20ex"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/marten/bin/go1.20ex/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.20.3"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/q0/b5ynf00142l7bl9sp8y098zr0000gn/T/go-build2554136632=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I take a (potentially user-provided) tls.Config, clone it and start the first TLS server. I then connect to this server with a TLS client and obtain a session ticket.
I then start a second server using a clone of the same Config under the same IP and port, and have the TLS client connect to it.

This is exactly what a 0-RTT enabled QUIC stack needs to do to support 0-RTT resumption: every incoming QUIC connection needs to use a clone of the original tls.Config. This is because it needs to wrap the user-provided WrapSession and UnwrapSession callback on the tls.Config (see here for the corresponding code in quic-go).

What did you expect to see?

I expected session resumption to succeed.

What did you see instead?

The client uses the ticket obtained from the first connection, but session resumption fails. This is because the session ticket keys on the tls.Config are lazily initialized.

I currently work around this by explicitly calling tls.Config.DecryptTicket with an arbitrary value for the ticket, since this triggers initialization of the session ticket keys. A cleaner way would be to initialize session ticket keys on Clone (potentially restricted to configs that can be used for the server, i.e. have Certificates or GetCertificate configured for performance reasons).

cc @FiloSottile
@marten-seemann marten-seemann mentioned this issue May 30, 2023
Merged
@gopherbot
Copy link

Change https://go.dev/cl/499235 mentions this issue: crypto/tls: initialize session ticket keys on Config.Clone

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 30, 2023
@mknyszek mknyszek added this to the Backlog milestone May 30, 2023
@marten-seemann
Copy link
Contributor Author

@FiloSottile @neild Any chance we can get this fix into the Go 1.21 release?

Without this fix, any 0-RTT enabled QUIC implementation will jump through hoops to make session resumption work (the workaround I described above).

@neild
Copy link
Contributor

neild commented Jul 6, 2023

Commented on https://go.dev/cl/499235.

I do believe that this is a further indication that using WrapSession and UnwrapSession to add data to the ticket is not the right API for the future. You shouldn't need to clone the config on each new connection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@neild @mknyszek @marten-seemann @gopherbot