From 22cbde9a565f4e40b5060a41d5e5171adcff673e Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Tue, 20 Feb 2024 14:58:00 -0800 Subject: [PATCH] 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 LUCI-TryBot-Result: Go LUCI --- internal/quic/cmd/interop/main.go | 6 ++-- internal/quic/config.go | 7 ++++ internal/quic/conn.go | 4 +-- internal/quic/conn_test.go | 2 ++ internal/quic/endpoint.go | 59 ++++++++++++++++++------------- internal/quic/endpoint_test.go | 31 ++++++++++------ internal/quic/tls.go | 14 ++++++-- 7 files changed, 81 insertions(+), 42 deletions(-) diff --git a/internal/quic/cmd/interop/main.go b/internal/quic/cmd/interop/main.go index 20f737b525..0899e0f1e4 100644 --- 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 index b045b7b92c..5d420312bb 100644 --- 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 index d462e9617a..38e8fe8f4e 100644 --- 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 index 16ee3cf2fc..a765ad60cd 100644 --- 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 index 6631708b82..a55336b240 100644 --- 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 index b9fb55fb34..b6669fc837 100644 --- 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 index a37e26fb8e..e2f2e5bde1 100644 --- 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 {