quic: set ServerName in client connection TLSConfig

Client connections must set tls.Config.ServerName to authenticate the identity of the server. (RFC 9001, Section 4.4.) Previously, we specified a single tls.Config per Endpoint. Change the Config passed to Listen to only apply to client connections accepted by the endpoint. Add a Config parameter to Listener.Dial to allow specifying a separate config per outbound connection, allowing the user to set the ServerName field. When the user does not set ServerName, set it ourselves. For golang/go#58547 Change-Id: Ie2500ae7c7a85400e6cc1c10cefa2bd4c746e313 Reviewed-on: https://go-review.googlesource.com/c/net/+/565796 Reviewed-by: Jonathan Amsterdam <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
golang · Feb 23, 2024 · 22cbde9 · 22cbde9
1 parent 57e4cc7
commit 22cbde9
Show file tree

Hide file tree

Showing 7 changed files with 81 additions and 42 deletions.
diff --git a/internal/quic/cmd/interop/main.go b/internal/quic/cmd/interop/main.go
@@ -148,7 +148,7 @@ func basicTest(ctx context.Context, config *quic.Config, urls []string) {
 		g.Add(1)
 		go func() {
 			defer g.Done()
-			fetchFrom(ctx, l, addr, u)
+			fetchFrom(ctx, config, l, addr, u)
 		}()
 	}
 
@@ -221,8 +221,8 @@ func parseURL(s string) (u *url.URL, authority string, err error) {
 	return u, authority, nil
 }
 
-func fetchFrom(ctx context.Context, l *quic.Endpoint, addr string, urls []*url.URL) {
-	conn, err := l.Dial(ctx, "udp", addr)
+func fetchFrom(ctx context.Context, config *quic.Config, l *quic.Endpoint, addr string, urls []*url.URL) {
+	conn, err := l.Dial(ctx, "udp", addr, config)
 	if err != nil {
 		log.Printf("%v: %v", addr, err)
 		return

diff --git a/internal/quic/config.go b/internal/quic/config.go
@@ -107,6 +107,13 @@ type Config struct {
 	QLogLogger *slog.Logger
 }
 
+// Clone returns a shallow clone of c, or nil if c is nil.
+// It is safe to clone a [Config] that is being used concurrently by a QUIC endpoint.
+func (c *Config) Clone() *Config {
+	n := *c
+	return &n
+}
+
 func configDefault[T ~int64](v, def, limit T) T {
 	switch {
 	case v == 0:

diff --git a/internal/quic/conn.go b/internal/quic/conn.go
@@ -94,7 +94,7 @@ type newServerConnIDs struct {
 	retrySrcConnID    []byte // source from server's Retry
 }
 
-func newConn(now time.Time, side connSide, cids newServerConnIDs, peerAddr netip.AddrPort, config *Config, e *Endpoint) (conn *Conn, _ error) {
+func newConn(now time.Time, side connSide, cids newServerConnIDs, peerHostname string, peerAddr netip.AddrPort, config *Config, e *Endpoint) (conn *Conn, _ error) {
 	c := &Conn{
 		side:                 side,
 		endpoint:             e,
@@ -146,7 +146,7 @@ func newConn(now time.Time, side connSide, cids newServerConnIDs, peerAddr netip
 	c.lifetimeInit()
 	c.restartIdleTimer(now)
 
-	if err := c.startTLS(now, initialConnID, transportParameters{
+	if err := c.startTLS(now, initialConnID, peerHostname, transportParameters{
 		initialSrcConnID:               c.connIDState.srcConnID(),
 		originalDstConnID:              cids.originalDstConnID,
 		retrySrcConnID:                 cids.retrySrcConnID,

diff --git a/internal/quic/conn_test.go b/internal/quic/conn_test.go
@@ -242,8 +242,10 @@ func newTestConn(t *testing.T, side connSide, opts ...any) *testConn {
 	endpoint.configTestConn = configTestConn
 	conn, err := endpoint.e.newConn(
 		endpoint.now,
+		config,
 		side,
 		cids,
+		"",
 		netip.MustParseAddrPort("127.0.0.1:443"))
 	if err != nil {
 		t.Fatal(err)

diff --git a/internal/quic/endpoint.go b/internal/quic/endpoint.go
@@ -22,11 +22,11 @@ import (
 //
 // Multiple goroutines may invoke methods on an Endpoint simultaneously.
 type Endpoint struct {
-	config     *Config
-	packetConn packetConn
-	testHooks  endpointTestHooks
-	resetGen   statelessResetTokenGenerator
-	retry      retryState
+	listenConfig *Config
+	packetConn   packetConn
+	testHooks    endpointTestHooks
+	resetGen     statelessResetTokenGenerator
+	retry        retryState
 
 	acceptQueue queue[*Conn] // new inbound connections
 	connsMap    connsMap     // only accessed by the listen loop
@@ -51,9 +51,11 @@ type packetConn interface {
 }
 
 // Listen listens on a local network address.
-// The configuration config must be non-nil.
-func Listen(network, address string, config *Config) (*Endpoint, error) {
-	if config.TLSConfig == nil {
+//
+// The config is used to for connections accepted by the endpoint.
+// If the config is nil, the endpoint will not accept connections.
+func Listen(network, address string, listenConfig *Config) (*Endpoint, error) {
+	if listenConfig != nil && listenConfig.TLSConfig == nil {
 		return nil, errors.New("TLSConfig is not set")
 	}
 	a, err := net.ResolveUDPAddr(network, address)
@@ -68,21 +70,25 @@ func Listen(network, address string, config *Config) (*Endpoint, error) {
 	if err != nil {
 		return nil, err
 	}
-	return newEndpoint(pc, config, nil)
+	return newEndpoint(pc, listenConfig, nil)
 }
 
 func newEndpoint(pc packetConn, config *Config, hooks endpointTestHooks) (*Endpoint, error) {
 	e := &Endpoint{
-		config:      config,
-		packetConn:  pc,
-		testHooks:   hooks,
-		conns:       make(map[*Conn]struct{}),
-		acceptQueue: newQueue[*Conn](),
-		closec:      make(chan struct{}),
-	}
-	e.resetGen.init(config.StatelessResetKey)
+		listenConfig: config,
+		packetConn:   pc,
+		testHooks:    hooks,
+		conns:        make(map[*Conn]struct{}),
+		acceptQueue:  newQueue[*Conn](),
+		closec:       make(chan struct{}),
+	}
+	var statelessResetKey [32]byte
+	if config != nil {
+		statelessResetKey = config.StatelessResetKey
+	}
+	e.resetGen.init(statelessResetKey)
 	e.connsMap.init()
-	if config.RequireAddressValidation {
+	if config != nil && config.RequireAddressValidation {
 		if err := e.retry.init(); err != nil {
 			return nil, err
 		}
@@ -141,14 +147,15 @@ func (e *Endpoint) Accept(ctx context.Context) (*Conn, error) {
 }
 
 // Dial creates and returns a connection to a network address.
-func (e *Endpoint) Dial(ctx context.Context, network, address string) (*Conn, error) {
+// The config cannot be nil.
+func (e *Endpoint) Dial(ctx context.Context, network, address string, config *Config) (*Conn, error) {
 	u, err := net.ResolveUDPAddr(network, address)
 	if err != nil {
 		return nil, err
 	}
 	addr := u.AddrPort()
 	addr = netip.AddrPortFrom(addr.Addr().Unmap(), addr.Port())
-	c, err := e.newConn(time.Now(), clientSide, newServerConnIDs{}, addr)
+	c, err := e.newConn(time.Now(), config, clientSide, newServerConnIDs{}, address, addr)
 	if err != nil {
 		return nil, err
 	}
@@ -159,13 +166,13 @@ func (e *Endpoint) Dial(ctx context.Context, network, address string) (*Conn, er
 	return c, nil
 }
 
-func (e *Endpoint) newConn(now time.Time, side connSide, cids newServerConnIDs, peerAddr netip.AddrPort) (*Conn, error) {
+func (e *Endpoint) newConn(now time.Time, config *Config, side connSide, cids newServerConnIDs, peerHostname string, peerAddr netip.AddrPort) (*Conn, error) {
 	e.connsMu.Lock()
 	defer e.connsMu.Unlock()
 	if e.closing {
 		return nil, errors.New("endpoint closed")
 	}
-	c, err := newConn(now, side, cids, peerAddr, e.config, e)
+	c, err := newConn(now, side, cids, peerHostname, peerAddr, config, e)
 	if err != nil {
 		return nil, err
 	}
@@ -288,11 +295,15 @@ func (e *Endpoint) handleUnknownDestinationDatagram(m *datagram) {
 		// https://www.rfc-editor.org/rfc/rfc9000#section-10.3-16
 		return
 	}
+	if e.listenConfig == nil {
+		// We are not configured to accept connections.
+		return
+	}
 	cids := newServerConnIDs{
 		srcConnID: p.srcConnID,
 		dstConnID: p.dstConnID,
 	}
-	if e.config.RequireAddressValidation {
+	if e.listenConfig.RequireAddressValidation {
 		var ok bool
 		cids.retrySrcConnID = p.dstConnID
 		cids.originalDstConnID, ok = e.validateInitialAddress(now, p, m.peerAddr)
@@ -303,7 +314,7 @@ func (e *Endpoint) handleUnknownDestinationDatagram(m *datagram) {
 		cids.originalDstConnID = p.dstConnID
 	}
 	var err error
-	c, err := e.newConn(now, serverSide, cids, m.peerAddr)
+	c, err := e.newConn(now, e.listenConfig, serverSide, cids, "", m.peerAddr)
 	if err != nil {
 		// The accept queue is probably full.
 		// We could send a CONNECTION_CLOSE to the peer to reject the connection.

diff --git a/internal/quic/endpoint_test.go b/internal/quic/endpoint_test.go
@@ -67,7 +67,8 @@ func newLocalConnPair(t testing.TB, conf1, conf2 *Config) (clientConn, serverCon
 	ctx := context.Background()
 	e1 := newLocalEndpoint(t, serverSide, conf1)
 	e2 := newLocalEndpoint(t, clientSide, conf2)
-	c2, err := e2.Dial(ctx, "udp", e1.LocalAddr().String())
+	conf2 = makeTestConfig(conf2, clientSide)
+	c2, err := e2.Dial(ctx, "udp", e1.LocalAddr().String(), conf2)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -80,9 +81,24 @@ func newLocalConnPair(t testing.TB, conf1, conf2 *Config) (clientConn, serverCon
 
 func newLocalEndpoint(t testing.TB, side connSide, conf *Config) *Endpoint {
 	t.Helper()
+	conf = makeTestConfig(conf, side)
+	e, err := Listen("udp", "127.0.0.1:0", conf)
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() {
+		e.Close(canceledContext())
+	})
+	return e
+}
+
+func makeTestConfig(conf *Config, side connSide) *Config {
+	if conf == nil {
+		return nil
+	}
+	newConf := *conf
+	conf = &newConf
 	if conf.TLSConfig == nil {
-		newConf := *conf
-		conf = &newConf
 		conf.TLSConfig = newTestTLSConfig(side)
 	}
 	if conf.QLogLogger == nil {
@@ -91,14 +107,7 @@ func newLocalEndpoint(t testing.TB, side connSide, conf *Config) *Endpoint {
 			Dir:   *qlogdir,
 		}))
 	}
-	e, err := Listen("udp", "127.0.0.1:0", conf)
-	if err != nil {
-		t.Fatal(err)
-	}
-	t.Cleanup(func() {
-		e.Close(canceledContext())
-	})
-	return e
+	return conf
 }
 
 type testEndpoint struct {

diff --git a/internal/quic/tls.go b/internal/quic/tls.go
@@ -11,14 +11,24 @@ import (
 	"crypto/tls"
 	"errors"
 	"fmt"
+	"net"
 	"time"
 )
 
 // startTLS starts the TLS handshake.
-func (c *Conn) startTLS(now time.Time, initialConnID []byte, params transportParameters) error {
+func (c *Conn) startTLS(now time.Time, initialConnID []byte, peerHostname string, params transportParameters) error {
+	tlsConfig := c.config.TLSConfig
+	if a, _, err := net.SplitHostPort(peerHostname); err == nil {
+		peerHostname = a
+	}
+	if tlsConfig.ServerName == "" && peerHostname != "" {
+		tlsConfig = tlsConfig.Clone()
+		tlsConfig.ServerName = peerHostname
+	}
+
 	c.keysInitial = initialKeys(initialConnID, c.side)
 
-	qconfig := &tls.QUICConfig{TLSConfig: c.config.TLSConfig}
+	qconfig := &tls.QUICConfig{TLSConfig: tlsConfig}
 	if c.side == clientSide {
 		c.tls = tls.QUICClient(qconfig)
 	} else {