From b1635df2f55e0b75548ba7a1a42ec7f181e0e14c Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 16 Aug 2023 09:53:41 +0700 Subject: [PATCH] ignore QUICConn.SendSessionTicket error if session tickets are disabled (#4030) --- integrationtests/self/resumption_test.go | 58 ++++++++++++++++++++++-- internal/handshake/crypto_setup.go | 12 +++-- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/integrationtests/self/resumption_test.go b/integrationtests/self/resumption_test.go index 264f832ebf5..bedf04704b0 100644 --- a/integrationtests/self/resumption_test.go +++ b/integrationtests/self/resumption_test.go @@ -56,7 +56,7 @@ var _ = Describe("TLS session resumption", func() { context.Background(), fmt.Sprintf("localhost:%d", server.Addr().(*net.UDPAddr).Port), tlsConf, - nil, + getQuicConfig(nil), ) Expect(err).ToNot(HaveOccurred()) var sessionKey string @@ -71,7 +71,7 @@ var _ = Describe("TLS session resumption", func() { context.Background(), fmt.Sprintf("localhost:%d", server.Addr().(*net.UDPAddr).Port), tlsConf, - nil, + getQuicConfig(nil), ) Expect(err).ToNot(HaveOccurred()) Expect(gets).To(Receive(Equal(sessionKey))) @@ -85,7 +85,7 @@ var _ = Describe("TLS session resumption", func() { It("doesn't use session resumption, if the config disables it", func() { sConf := getTLSConfig() sConf.SessionTicketsDisabled = true - server, err := quic.ListenAddr("localhost:0", sConf, nil) + server, err := quic.ListenAddr("localhost:0", sConf, getQuicConfig(nil)) Expect(err).ToNot(HaveOccurred()) defer server.Close() @@ -98,7 +98,7 @@ var _ = Describe("TLS session resumption", func() { context.Background(), fmt.Sprintf("localhost:%d", server.Addr().(*net.UDPAddr).Port), tlsConf, - nil, + getQuicConfig(nil), ) Expect(err).ToNot(HaveOccurred()) Consistently(puts).ShouldNot(Receive()) @@ -114,7 +114,55 @@ var _ = Describe("TLS session resumption", func() { context.Background(), fmt.Sprintf("localhost:%d", server.Addr().(*net.UDPAddr).Port), tlsConf, - nil, + getQuicConfig(nil), + ) + Expect(err).ToNot(HaveOccurred()) + Expect(conn.ConnectionState().TLS.DidResume).To(BeFalse()) + + serverConn, err = server.Accept(context.Background()) + Expect(err).ToNot(HaveOccurred()) + Expect(serverConn.ConnectionState().TLS.DidResume).To(BeFalse()) + }) + + It("doesn't use session resumption, if the config returned by GetConfigForClient disables it", func() { + sConf := &tls.Config{ + GetConfigForClient: func(*tls.ClientHelloInfo) (*tls.Config, error) { + conf := getTLSConfig() + conf.SessionTicketsDisabled = true + return conf, nil + }, + } + + server, err := quic.ListenAddr("localhost:0", sConf, getQuicConfig(nil)) + Expect(err).ToNot(HaveOccurred()) + defer server.Close() + + gets := make(chan string, 100) + puts := make(chan string, 100) + cache := newClientSessionCache(tls.NewLRUClientSessionCache(10), gets, puts) + tlsConf := getTLSClientConfig() + tlsConf.ClientSessionCache = cache + conn, err := quic.DialAddr( + context.Background(), + fmt.Sprintf("localhost:%d", server.Addr().(*net.UDPAddr).Port), + tlsConf, + getQuicConfig(nil), + ) + Expect(err).ToNot(HaveOccurred()) + Consistently(puts).ShouldNot(Receive()) + Expect(conn.ConnectionState().TLS.DidResume).To(BeFalse()) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + serverConn, err := server.Accept(ctx) + Expect(err).ToNot(HaveOccurred()) + Expect(serverConn.ConnectionState().TLS.DidResume).To(BeFalse()) + + conn, err = quic.DialAddr( + context.Background(), + fmt.Sprintf("localhost:%d", server.Addr().(*net.UDPAddr).Port), + tlsConf, + getQuicConfig(nil), ) Expect(err).ToNot(HaveOccurred()) Expect(conn.ConnectionState().TLS.DidResume).To(BeFalse()) diff --git a/internal/handshake/crypto_setup.go b/internal/handshake/crypto_setup.go index 35d65b5eecb..a1179257b5a 100644 --- a/internal/handshake/crypto_setup.go +++ b/internal/handshake/crypto_setup.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "net" + "strings" "sync" "sync/atomic" "time" @@ -356,10 +357,15 @@ func (h *cryptoSetup) getDataForSessionTicket() []byte { // Due to limitations in crypto/tls, it's only possible to generate a single session ticket per connection. // It is only valid for the server. func (h *cryptoSetup) GetSessionTicket() ([]byte, error) { - if h.tlsConf.SessionTicketsDisabled { - return nil, nil - } if err := qtls.SendSessionTicket(h.conn, h.allow0RTT); err != nil { + // Session tickets might be disabled by tls.Config.SessionTicketsDisabled. + // We can't check h.tlsConfig here, since the actual config might have been obtained from + // the GetConfigForClient callback. + // See https://github.com/golang/go/issues/62032. + // Once that issue is resolved, this error assertion can be removed. + if strings.Contains(err.Error(), "session ticket keys unavailable") { + return nil, nil + } return nil, err } ev := h.conn.NextEvent()