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

[marten-seemann]

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

[gopherbot]

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

[marten-seemann]

@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]

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.

[espadolini]

Now that #77113 was merged, does this mean that there's no way to keep session resumption working in scenarios where cloning a TLS config is the only option?

edit: other than manually managing session tickets, that is

[neild]

It shouldn't be necessary to clone the TLS config for each connection received. As I recall it, the original motivation for cloning the config was to use WrapSession and UnwrapSession to add additional data (the QUIC transport parameters) into session tickets, but #63691 added the ability for QUIC stacks to manage data in the ticket while processing QUICStoreSession and QUICResumeSession events events.

[marten-seemann]

Unfortunately, this is not the only reason a QUIC stack needs to clone the config. We also need to do it to allow the application to inspect the remote and local address in the GetConfigForClient and GetCertificate callback. See https://github.com/quic-go/quic-go/blob/7659dd8e0fa06b41290ad29af323d93d673c6b36/internal/handshake/tls_config.go#L16 for the logic currently implemented in quic-go.

[espadolini]

We also need to do it to allow the application to inspect the remote and local address in the GetConfigForClient and GetCertificate callback.

It would be a compatibility break in quic-go but we could pass in arbitrary info as a context value, which would then be available to GetConfigForClient (but not VerifyConnection, alas).

[marten-seemann]

It's not so much a compatibility break for quic-go but for the applications building on top of quic-go and expecting the TLS callbacks to work the same. It's pretty much a non-starter to expect them to write different code and not expect a remote and local address on these callbacks.

That said, it would be nice if we could move the fakeConn logic from quic-go into the standard library. We would then need to pass the local and remote address to the constructor. As far as I can tell, quic-go would then not need to clone the tls.Config at all.