crypto/tls: allow QUIC to configure net.Conn used on ClientHelloInfo

[marten-seemann]

The crypto/tls package's GetCertificate and GetConfigForClient callbacks receive a tls.ClientHelloInfo with a Conn field (type net.Conn). For QUIC, a full net.Conn doesn't make sense, as QUIC uses UDP and doesn't support read/write/close in the same way. Even for TCP, calling those methods during the handshake was always error-prone and nonsensical. The only useful parts are LocalAddr and RemoteAddr.

Currently, quic-go works around this by cloning the tls.Config and wrapping the callbacks to inject a fake net.Conn that exposes just the addresses (with no-ops for other methods). This cloning now breaks session resumption since Go 1.25.6, where a security fix stopped copying auto-generated session ticket keys in Config.Clone() (see #77113).

Arguably, a QUIC stack shouldn’t need to do any cloning. I propose extending tls.QUICConfig to let QUIC stacks supply the connection addresses for the handshake.

This proposal doesn’t run into any problems with connection migration as defined in RFC 9000, as the five-tuple is fixed during the handshake (migration only happens post-handshake).

Two options:

1. Add a HandshakeConn net.Conn field. The QUIC stack provides a fake conn with LocalAddr and RemoteAddr; crypto/tls uses it directly in ClientHelloInfo.Conn.

   type QUICConfig struct {
       TLSConfig          *Config
       EnableSessionEvents bool
       HandshakeConn      net.Conn // Fake conn for ClientHelloInfo.Conn during handshake
   }

2. Add LocalAddr and RemoteAddr fields (type net.Addr). The QUIC stack sets them; crypto/tls creates the fake conn internally.

   type QUICConfig struct {
       TLSConfig          *Config
       EnableSessionEvents bool
       LocalAddr          net.Addr // Local address for ClientHelloInfo.Conn during handshake
       RemoteAddr         net.Addr // Remote address for ClientHelloInfo.Conn during handshake
   }

I don't have a strong preference, but a slight lean toward option 1, as it gives QUIC stacks more flexibility for additional metadata.

[neild]

Providing a net.Conn to provide in ClientHelloInfo.Conn seems pretty reasonable to me.

Maybe we call this QUICConfig.ClientHelloInfoConn to make it extra-clear what the purpose of the field is?

[gopherbot]

Change https://go.dev/cl/745720 mentions this issue: crypto/tls: add QUICConfig.ClientHelloInfoConn

[marten-seemann]

I submitted a CL in https://go.dev/cl/745720, and tested it in quic-go (quic-go/quic-go#5571). I noticed that we still need to clone the tls.Config in order to set the minimum version to TLS 1.3, and submitted another proposal to get rid of this requirement (#77631).

With these two changes, quic-go doesn't need to clone the tls.Config anymore. I was able to verify this by running quic-go's test suite.

[aclements]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— aclements for the proposal review group

[aclements]

Proposal review discussion:

@neild helpfully led us through the maze of twisty passages: User provides a TLSConfig to the QUIC layer (e.g., x/net/quic or quic-go), possibly including a GetCertificate (etc) callback. The QUIC layer provides a tls.QUICConfig (including the TLSConfig) to the crypto/tls layer. crypto/tls calls TLSConfig.GetCertificate with a tls.ClientHelloInfo, which includes a net.Conn.

This proposal would allow the QUIC layer to inject a net.Conn that would eventually thread through to the GetCertificate (etc) callback provided by the user.

Alternative: add remote and local address to tls.QUICConfig instead of a net.Conn, and let crypto/tls make the fake net.Conn. @neild points out that if we just add the net.Conn, we'll never have to answer any more questions about what the QUIC layer might provide in a fake Conn implementation.

[neild]

Concrete proposal:

type QUICConfig struct {
  // ClientHelloInfoConn is provided in ClientHelloInfo.Conn in callbacks.
  ClientHelloInfoConn net.Conn
}

[aclements]

Have all remaining concerns about this proposal been addressed?

The proposal is given in #77363 (comment)