Skip to content

Commit

Permalink
never allow 0-RTT when using Dial, even if the session ticket allows it
Browse files Browse the repository at this point in the history
When resuming a TLS session using Dial (and not DialEarly), 0-RTT should
be disabled at the TLS layer, even if the session ticket allows for
0-RTT resumption.

This bug is not critical, since Dial doesn't return an EarlyConnection,
so the client wouldn't be able to actually send 0-RTT data in practice.
  • Loading branch information
marten-seemann committed Oct 24, 2023
1 parent 99cb5c7 commit 13a8bca
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 12 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ require (
github.com/onsi/ginkgo/v2 v2.9.5
github.com/onsi/gomega v1.27.6
github.com/quic-go/qpack v0.4.0
github.com/quic-go/qtls-go1-20 v0.3.4
github.com/quic-go/qtls-go1-20 v0.3.5-0.20231024051010-174a6a10e95d
go.uber.org/mock v0.3.0
golang.org/x/crypto v0.4.0
golang.org/x/exp v0.0.0-20221205204356-47842c84f3db
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ github.com/prometheus/common v0.0.0-20180801064454-c7de2306084e/go.mod h1:daVV7q
github.com/prometheus/procfs v0.0.0-20180725123919-05ee40e3a273/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
github.com/quic-go/qpack v0.4.0 h1:Cr9BXA1sQS2SmDUWjSofMPNKmvF6IiIfDRmgU0w1ZCo=
github.com/quic-go/qpack v0.4.0/go.mod h1:UZVnYIfi5GRk+zI9UMaCPsmZ2xKJP7XBUvVyT1Knj9A=
github.com/quic-go/qtls-go1-20 v0.3.4 h1:MfFAPULvst4yoMgY9QmtpYmfij/em7O8UUi+bNVm7Cg=
github.com/quic-go/qtls-go1-20 v0.3.4/go.mod h1:X9Nh97ZL80Z+bX/gUXMbipO6OxdiDi58b/fMC9mAL+k=
github.com/quic-go/qtls-go1-20 v0.3.5-0.20231024051010-174a6a10e95d h1:S9Y4/Fn/PXtTANpU3GdTwubVuPkwsGA6PnxC4Gz6goc=
github.com/quic-go/qtls-go1-20 v0.3.5-0.20231024051010-174a6a10e95d/go.mod h1:X9Nh97ZL80Z+bX/gUXMbipO6OxdiDi58b/fMC9mAL+k=
github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g=
github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
github.com/shurcooL/component v0.0.0-20170202220835-f88ec8f54cc4/go.mod h1:XhFIlyj5a1fBNx5aJTbKoIq0mNaPvOagO+HjB3EtxrY=
Expand Down
4 changes: 2 additions & 2 deletions integrationtests/gomodvendor/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ github.com/prometheus/common v0.0.0-20180801064454-c7de2306084e/go.mod h1:daVV7q
github.com/prometheus/procfs v0.0.0-20180725123919-05ee40e3a273/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
github.com/quic-go/qpack v0.4.0 h1:Cr9BXA1sQS2SmDUWjSofMPNKmvF6IiIfDRmgU0w1ZCo=
github.com/quic-go/qpack v0.4.0/go.mod h1:UZVnYIfi5GRk+zI9UMaCPsmZ2xKJP7XBUvVyT1Knj9A=
github.com/quic-go/qtls-go1-20 v0.3.4 h1:MfFAPULvst4yoMgY9QmtpYmfij/em7O8UUi+bNVm7Cg=
github.com/quic-go/qtls-go1-20 v0.3.4/go.mod h1:X9Nh97ZL80Z+bX/gUXMbipO6OxdiDi58b/fMC9mAL+k=
github.com/quic-go/qtls-go1-20 v0.3.5-0.20231024051010-174a6a10e95d h1:S9Y4/Fn/PXtTANpU3GdTwubVuPkwsGA6PnxC4Gz6goc=
github.com/quic-go/qtls-go1-20 v0.3.5-0.20231024051010-174a6a10e95d/go.mod h1:X9Nh97ZL80Z+bX/gUXMbipO6OxdiDi58b/fMC9mAL+k=
github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g=
github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
github.com/shurcooL/component v0.0.0-20170202220835-f88ec8f54cc4/go.mod h1:XhFIlyj5a1fBNx5aJTbKoIq0mNaPvOagO+HjB3EtxrY=
Expand Down
31 changes: 31 additions & 0 deletions integrationtests/self/zero_rtt_oldgo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,37 @@ var _ = Describe("0-RTT", func() {
Expect(zeroRTTPackets[0]).To(BeNumerically(">=", protocol.PacketNumber(5)))
})

It("doesn't use 0-RTT when Dial is used for the resumed connection", func() {
tlsConf, clientConf := dialAndReceiveSessionTicket(nil)

ln, err := quic.ListenAddrEarly(
"localhost:0",
tlsConf,
getQuicConfig(&quic.Config{Allow0RTT: true}),
)
Expect(err).ToNot(HaveOccurred())
defer ln.Close()
proxy, num0RTTPackets := runCountingProxy(ln.Addr().(*net.UDPAddr).Port)
defer proxy.Close()

conn, err := quic.DialAddr(
context.Background(),
fmt.Sprintf("localhost:%d", proxy.LocalPort()),
clientConf,
getQuicConfig(nil),
)
Expect(err).ToNot(HaveOccurred())
defer conn.CloseWithError(0, "")
Expect(conn.ConnectionState().TLS.DidResume).To(BeTrue())
Expect(conn.ConnectionState().Used0RTT).To(BeFalse())
Expect(num0RTTPackets.Load()).To(BeZero())

serverConn, err := ln.Accept(context.Background())
Expect(err).ToNot(HaveOccurred())
Expect(serverConn.ConnectionState().TLS.DidResume).To(BeTrue())
Expect(serverConn.ConnectionState().Used0RTT).To(BeFalse())
})

It("doesn't reject 0-RTT when the server's transport stream limit increased", func() {
const maxStreams = 1
tlsConf, clientConf := dialAndReceiveSessionTicket(getQuicConfig(&quic.Config{
Expand Down
33 changes: 33 additions & 0 deletions integrationtests/self/zero_rtt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,39 @@ var _ = Describe("0-RTT", func() {
Expect(zeroRTTPackets[0]).To(BeNumerically(">=", protocol.PacketNumber(5)))
})

It("doesn't use 0-RTT when Dial is used for the resumed connection", func() {
tlsConf := getTLSConfig()
clientConf := getTLSClientConfig()
dialAndReceiveSessionTicket(tlsConf, getQuicConfig(nil), clientConf)

ln, err := quic.ListenAddrEarly(
"localhost:0",
tlsConf,
getQuicConfig(&quic.Config{Allow0RTT: true}),
)
Expect(err).ToNot(HaveOccurred())
defer ln.Close()
proxy, num0RTTPackets := runCountingProxy(ln.Addr().(*net.UDPAddr).Port)
defer proxy.Close()

conn, err := quic.DialAddr(
context.Background(),
fmt.Sprintf("localhost:%d", proxy.LocalPort()),
clientConf,
getQuicConfig(nil),
)
Expect(err).ToNot(HaveOccurred())
defer conn.CloseWithError(0, "")
Expect(conn.ConnectionState().TLS.DidResume).To(BeTrue())
Expect(conn.ConnectionState().Used0RTT).To(BeFalse())
Expect(num0RTTPackets.Load()).To(BeZero())

serverConn, err := ln.Accept(context.Background())
Expect(err).ToNot(HaveOccurred())
Expect(serverConn.ConnectionState().TLS.DidResume).To(BeTrue())
Expect(serverConn.ConnectionState().Used0RTT).To(BeFalse())
})

It("doesn't reject 0-RTT when the server's transport stream limit increased", func() {
const maxStreams = 1
tlsConf := getTLSConfig()
Expand Down
16 changes: 13 additions & 3 deletions internal/handshake/crypto_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ func NewCryptoSetupClient(
quicConf := &qtls.QUICConfig{TLSConfig: tlsConf}
qtls.SetupConfigForClient(quicConf, cs.marshalDataForSessionState, cs.handleDataFromSessionState)
cs.tlsConf = tlsConf
cs.allow0RTT = enable0RTT

cs.conn = qtls.QUICClient(quicConf)
cs.conn.SetTransportParameters(cs.ourParams.Marshal(protocol.PerspectiveClient))
Expand Down Expand Up @@ -316,13 +317,20 @@ func (h *cryptoSetup) marshalDataForSessionState() []byte {
return h.peerParams.MarshalForSessionTicket(b)
}

func (h *cryptoSetup) handleDataFromSessionState(data []byte) {
func (h *cryptoSetup) handleDataFromSessionState(data []byte) (allowEarlyData bool) {
tp, err := h.handleDataFromSessionStateImpl(data)
if err != nil {
h.logger.Debugf("Restoring of transport parameters from session ticket failed: %s", err.Error())
return
}
h.zeroRTTParameters = tp
// The session ticket might have been saved from a connection that allowed 0-RTT,
// and therefore contain transport parameters.
// Only use them if 0-RTT is actually used on the new connection.
if tp != nil && h.allow0RTT {
h.zeroRTTParameters = tp
return true
}
return false
}

func (h *cryptoSetup) handleDataFromSessionStateImpl(data []byte) (*wire.TransportParameters, error) {
Expand Down Expand Up @@ -383,7 +391,9 @@ func (h *cryptoSetup) GetSessionTicket() ([]byte, error) {
}

// handleSessionTicket is called for the server when receiving the client's session ticket.
// It reads parameters from the session ticket and decides whether to accept 0-RTT when the session ticket is used for 0-RTT.
// It reads parameters from the session ticket and checks whether to accept 0-RTT if the session ticket enabled 0-RTT.
// Note that the fact that the session ticket allows 0-RTT doesn't mean that the actual TLS handshake enables 0-RTT:
// A client may use a 0-RTT enabled session to resume a TLS session without using 0-RTT.
func (h *cryptoSetup) handleSessionTicket(sessionTicketData []byte, using0RTT bool) bool {
var t sessionTicket
if err := t.Unmarshal(sessionTicketData, using0RTT); err != nil {
Expand Down
6 changes: 4 additions & 2 deletions internal/qtls/client_session_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (

type clientSessionCache struct {
getData func() []byte
setData func([]byte)
setData func([]byte) (allowEarlyData bool)
wrapped tls.ClientSessionCache
}

Expand Down Expand Up @@ -46,10 +46,12 @@ func (c clientSessionCache) Get(key string) (*tls.ClientSessionState, bool) {
c.wrapped.Put(key, nil)
return nil, false
}
var earlyData bool
// restore QUIC transport parameters and RTT stored in state.Extra
if extra := findExtraData(state.Extra); extra != nil {
c.setData(extra)
earlyData = c.setData(extra)
}
state.EarlyData = earlyData
session, err := tls.NewResumptionState(ticket, state)
if err != nil {
// It's not clear why this would error.
Expand Down
2 changes: 1 addition & 1 deletion internal/qtls/go120.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func SetupConfigForServer(conf *QUICConfig, enable0RTT bool, getDataForSessionTi
}
}

func SetupConfigForClient(conf *QUICConfig, getDataForSessionState func() []byte, setDataFromSessionState func([]byte)) {
func SetupConfigForClient(conf *QUICConfig, getDataForSessionState func() []byte, setDataFromSessionState func([]byte) bool) {
conf.ExtraConfig = &qtls.ExtraConfig{
GetAppDataForSessionState: getDataForSessionState,
SetAppDataFromSessionState: setDataFromSessionState,
Expand Down
2 changes: 1 addition & 1 deletion internal/qtls/go121.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func SetupConfigForServer(qconf *QUICConfig, _ bool, getData func() []byte, hand
}
}

func SetupConfigForClient(qconf *QUICConfig, getData func() []byte, setData func([]byte)) {
func SetupConfigForClient(qconf *QUICConfig, getData func() []byte, setData func([]byte) bool) {
conf := qconf.TLSConfig
if conf.ClientSessionCache != nil {
origCache := conf.ClientSessionCache
Expand Down

0 comments on commit 13a8bca

Please sign in to comment.