From d7948d627a8833218d26c0abaadf93df09c7732e Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 16 Jan 2020 18:17:51 +0700 Subject: [PATCH] drop 0-RTT keys when the server rejects 0-RTT --- go.mod | 2 +- go.sum | 4 ++-- internal/handshake/crypto_setup.go | 18 +++++++++++++- internal/handshake/qtls.go | 4 +++- internal/handshake/qtls_test.go | 38 +++++++++++++++++++----------- 5 files changed, 47 insertions(+), 19 deletions(-) diff --git a/go.mod b/go.mod index 2a7f1412..7432fa2c 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/golang/protobuf v1.3.0 github.com/marten-seemann/chacha20 v0.2.0 github.com/marten-seemann/qpack v0.1.0 - github.com/marten-seemann/qtls v0.5.0 + github.com/marten-seemann/qtls v0.6.0 github.com/onsi/ginkgo v1.11.0 github.com/onsi/gomega v1.8.1 golang.org/x/crypto v0.0.0-20190829043050-9756ffdc2472 diff --git a/go.sum b/go.sum index d3f5659a..dda68735 100644 --- a/go.sum +++ b/go.sum @@ -15,8 +15,8 @@ github.com/marten-seemann/chacha20 v0.2.0 h1:f40vqzzx+3GdOmzQoItkLX5WLvHgPgyYqFF github.com/marten-seemann/chacha20 v0.2.0/go.mod h1:HSdjFau7GzYRj+ahFNwsO3ouVJr1HFkWoEwNDb4TMtE= github.com/marten-seemann/qpack v0.1.0 h1:/0M7lkda/6mus9B8u34Asqm8ZhHAAt9Ho0vniNuVSVg= github.com/marten-seemann/qpack v0.1.0/go.mod h1:LFt1NU/Ptjip0C2CPkhimBz5CGE3WGDAUWqna+CNTrI= -github.com/marten-seemann/qtls v0.5.0 h1:psje4bHl6372lku0pTIt2PZ9HcVJyBIuW9pZn+u2vhY= -github.com/marten-seemann/qtls v0.5.0/go.mod h1:pxVXcHHw1pNIt8Qo0pwSYQEoZ8yYOOPXTCZLQQunvRc= +github.com/marten-seemann/qtls v0.6.0 h1:iCN+BD2aSP23uY5Gb3COVlQeOIWmKTOgD2FPIE2KjQ4= +github.com/marten-seemann/qtls v0.6.0/go.mod h1:pxVXcHHw1pNIt8Qo0pwSYQEoZ8yYOOPXTCZLQQunvRc= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.7.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.11.0 h1:JAKSXpt1YjtLA7YpPiqO9ss6sNXEsPfSGdwN0UHqzrw= diff --git a/internal/handshake/crypto_setup.go b/internal/handshake/crypto_setup.go index ff6295fc..7e209aff 100644 --- a/internal/handshake/crypto_setup.go +++ b/internal/handshake/crypto_setup.go @@ -221,7 +221,7 @@ func newCryptoSetup( writeRecord: make(chan struct{}, 1), closeChan: make(chan struct{}), } - qtlsConf := tlsConfigToQtlsConfig(tlsConf, cs, extHandler, cs.marshalPeerParamsForSessionState, cs.handlePeerParamsFromSessionState, cs.accept0RTT, enable0RTT) + qtlsConf := tlsConfigToQtlsConfig(tlsConf, cs, extHandler, cs.marshalPeerParamsForSessionState, cs.handlePeerParamsFromSessionState, cs.accept0RTT, cs.rejected0RTT, enable0RTT) cs.tlsConf = qtlsConf return cs, cs.clientHelloWrittenChan } @@ -490,6 +490,8 @@ func (h *cryptoSetup) maybeSendSessionTicket() { } } +// accept0RTT is called for the server when receiving the client's session ticket. +// It decides whether to accept 0-RTT. func (h *cryptoSetup) accept0RTT(sessionTicketData []byte) bool { var tp TransportParameters if err := tp.UnmarshalFromSessionTicket(sessionTicketData); err != nil { @@ -505,6 +507,20 @@ func (h *cryptoSetup) accept0RTT(sessionTicketData []byte) bool { return valid } +// rejected0RTT is called for the client when the server rejects 0-RTT. +func (h *cryptoSetup) rejected0RTT() { + h.logger.Debugf("0-RTT was rejected. Dropping 0-RTT keys.") + + h.mutex.Lock() + had0RTTKeys := h.zeroRTTSealer != nil + h.zeroRTTSealer = nil + h.mutex.Unlock() + + if had0RTTKeys { + h.runner.DropKeys(protocol.Encryption0RTT) + } +} + func (h *cryptoSetup) handlePostHandshakeMessage() { // make sure the handshake has already completed <-h.handshakeDone diff --git a/internal/handshake/qtls.go b/internal/handshake/qtls.go index ab6d3294..f1cbb0fb 100644 --- a/internal/handshake/qtls.go +++ b/internal/handshake/qtls.go @@ -34,6 +34,7 @@ func tlsConfigToQtlsConfig( getDataForSessionState func() []byte, setDataFromSessionState func([]byte), accept0RTT func([]byte) bool, + rejected0RTT func(), enable0RTT bool, ) *qtls.Config { if c == nil { @@ -61,7 +62,7 @@ func tlsConfigToQtlsConfig( if tlsConf == nil { return nil, nil } - return tlsConfigToQtlsConfig(tlsConf, recordLayer, extHandler, getDataForSessionState, setDataFromSessionState, accept0RTT, enable0RTT), nil + return tlsConfigToQtlsConfig(tlsConf, recordLayer, extHandler, getDataForSessionState, setDataFromSessionState, accept0RTT, rejected0RTT, enable0RTT), nil } } var csc qtls.ClientSessionCache @@ -99,6 +100,7 @@ func tlsConfigToQtlsConfig( GetExtensions: extHandler.GetExtensions, ReceivedExtensions: extHandler.ReceivedExtensions, Accept0RTT: accept0RTT, + Rejected0RTT: rejected0RTT, } if enable0RTT { conf.Enable0RTT = true diff --git a/internal/handshake/qtls_test.go b/internal/handshake/qtls_test.go index 6eb0ad1d..ebd7df35 100644 --- a/internal/handshake/qtls_test.go +++ b/internal/handshake/qtls_test.go @@ -28,19 +28,19 @@ func (*mockExtensionHandler) TransportParameters() <-chan []byte { panic("not im var _ = Describe("qtls.Config generation", func() { It("sets MinVersion and MaxVersion", func() { tlsConf := &tls.Config{MinVersion: tls.VersionTLS11, MaxVersion: tls.VersionTLS12} - qtlsConf := tlsConfigToQtlsConfig(tlsConf, nil, &mockExtensionHandler{}, nil, nil, nil, false) + qtlsConf := tlsConfigToQtlsConfig(tlsConf, nil, &mockExtensionHandler{}, nil, nil, nil, nil, false) Expect(qtlsConf.MinVersion).To(BeEquivalentTo(tls.VersionTLS13)) Expect(qtlsConf.MaxVersion).To(BeEquivalentTo(tls.VersionTLS13)) }) It("works when called with a nil config", func() { - qtlsConf := tlsConfigToQtlsConfig(nil, nil, &mockExtensionHandler{}, nil, nil, nil, false) + qtlsConf := tlsConfigToQtlsConfig(nil, nil, &mockExtensionHandler{}, nil, nil, nil, nil, false) Expect(qtlsConf).ToNot(BeNil()) }) It("sets the setter and getter function for TLS extensions", func() { extHandler := &mockExtensionHandler{} - qtlsConf := tlsConfigToQtlsConfig(&tls.Config{}, nil, extHandler, nil, nil, nil, false) + qtlsConf := tlsConfigToQtlsConfig(&tls.Config{}, nil, extHandler, nil, nil, nil, nil, false) Expect(extHandler.get).To(BeFalse()) qtlsConf.GetExtensions(10) Expect(extHandler.get).To(BeTrue()) @@ -51,31 +51,40 @@ var _ = Describe("qtls.Config generation", func() { It("sets the Accept0RTT callback", func() { accept0RTT := func([]byte) bool { return true } - qtlsConf := tlsConfigToQtlsConfig(nil, nil, &mockExtensionHandler{}, nil, nil, accept0RTT, false) + qtlsConf := tlsConfigToQtlsConfig(nil, nil, &mockExtensionHandler{}, nil, nil, accept0RTT, nil, false) Expect(qtlsConf.Accept0RTT).ToNot(BeNil()) Expect(qtlsConf.Accept0RTT(nil)).To(BeTrue()) }) + It("sets the Accept0RTT callback", func() { + var called bool + rejected0RTT := func() { called = true } + qtlsConf := tlsConfigToQtlsConfig(nil, nil, &mockExtensionHandler{}, nil, nil, nil, rejected0RTT, false) + Expect(qtlsConf.Rejected0RTT).ToNot(BeNil()) + qtlsConf.Rejected0RTT() + Expect(called).To(BeTrue()) + }) + It("enables 0-RTT", func() { - qtlsConf := tlsConfigToQtlsConfig(nil, nil, &mockExtensionHandler{}, nil, nil, nil, false) + qtlsConf := tlsConfigToQtlsConfig(nil, nil, &mockExtensionHandler{}, nil, nil, nil, nil, false) Expect(qtlsConf.Enable0RTT).To(BeFalse()) Expect(qtlsConf.MaxEarlyData).To(BeZero()) - qtlsConf = tlsConfigToQtlsConfig(nil, nil, &mockExtensionHandler{}, nil, nil, nil, true) + qtlsConf = tlsConfigToQtlsConfig(nil, nil, &mockExtensionHandler{}, nil, nil, nil, nil, true) Expect(qtlsConf.Enable0RTT).To(BeTrue()) Expect(qtlsConf.MaxEarlyData).To(Equal(uint32(0xffffffff))) }) It("initializes such that the session ticket key remains constant", func() { tlsConf := &tls.Config{} - qtlsConf1 := tlsConfigToQtlsConfig(tlsConf, nil, &mockExtensionHandler{}, nil, nil, nil, false) - qtlsConf2 := tlsConfigToQtlsConfig(tlsConf, nil, &mockExtensionHandler{}, nil, nil, nil, false) + qtlsConf1 := tlsConfigToQtlsConfig(tlsConf, nil, &mockExtensionHandler{}, nil, nil, nil, nil, false) + qtlsConf2 := tlsConfigToQtlsConfig(tlsConf, nil, &mockExtensionHandler{}, nil, nil, nil, nil, false) Expect(qtlsConf1.SessionTicketKey).ToNot(BeZero()) // should now contain a random value Expect(qtlsConf1.SessionTicketKey).To(Equal(qtlsConf2.SessionTicketKey)) }) Context("GetConfigForClient callback", func() { It("doesn't set it if absent", func() { - qtlsConf := tlsConfigToQtlsConfig(&tls.Config{}, nil, &mockExtensionHandler{}, nil, nil, nil, false) + qtlsConf := tlsConfigToQtlsConfig(&tls.Config{}, nil, &mockExtensionHandler{}, nil, nil, nil, nil, false) Expect(qtlsConf.GetConfigForClient).To(BeNil()) }) @@ -86,7 +95,7 @@ var _ = Describe("qtls.Config generation", func() { }, } extHandler := &mockExtensionHandler{} - qtlsConf := tlsConfigToQtlsConfig(tlsConf, nil, extHandler, nil, nil, nil, false) + qtlsConf := tlsConfigToQtlsConfig(tlsConf, nil, extHandler, nil, nil, nil, nil, false) Expect(qtlsConf.GetConfigForClient).ToNot(BeNil()) confForClient, err := qtlsConf.GetConfigForClient(nil) Expect(err).ToNot(HaveOccurred()) @@ -106,7 +115,7 @@ var _ = Describe("qtls.Config generation", func() { return nil, testErr }, } - qtlsConf := tlsConfigToQtlsConfig(tlsConf, nil, &mockExtensionHandler{}, nil, nil, nil, false) + qtlsConf := tlsConfigToQtlsConfig(tlsConf, nil, &mockExtensionHandler{}, nil, nil, nil, nil, false) _, err := qtlsConf.GetConfigForClient(nil) Expect(err).To(MatchError(testErr)) }) @@ -117,14 +126,14 @@ var _ = Describe("qtls.Config generation", func() { return nil, nil }, } - qtlsConf := tlsConfigToQtlsConfig(tlsConf, nil, &mockExtensionHandler{}, nil, nil, nil, false) + qtlsConf := tlsConfigToQtlsConfig(tlsConf, nil, &mockExtensionHandler{}, nil, nil, nil, nil, false) Expect(qtlsConf.GetConfigForClient(nil)).To(BeNil()) }) }) Context("ClientSessionCache", func() { It("doesn't set if absent", func() { - qtlsConf := tlsConfigToQtlsConfig(&tls.Config{}, nil, &mockExtensionHandler{}, nil, nil, nil, false) + qtlsConf := tlsConfigToQtlsConfig(&tls.Config{}, nil, &mockExtensionHandler{}, nil, nil, nil, nil, false) Expect(qtlsConf.ClientSessionCache).To(BeNil()) }) @@ -139,6 +148,7 @@ var _ = Describe("qtls.Config generation", func() { func() []byte { return []byte("foobar") }, func(p []byte) { appData = p }, nil, + nil, false, ) Expect(qtlsConf.ClientSessionCache).ToNot(BeNil()) @@ -159,7 +169,7 @@ var _ = Describe("qtls.Config generation", func() { It("puts a nil session state", func() { csc := NewMockClientSessionCache(mockCtrl) tlsConf := &tls.Config{ClientSessionCache: csc} - qtlsConf := tlsConfigToQtlsConfig(tlsConf, nil, &mockExtensionHandler{}, nil, nil, nil, false) + qtlsConf := tlsConfigToQtlsConfig(tlsConf, nil, &mockExtensionHandler{}, nil, nil, nil, nil, false) // put something csc.EXPECT().Put("foobar", nil) qtlsConf.ClientSessionCache.Put("foobar", nil)