Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

never allow 0-RTT when using Dial, even if the session ticket allows it #4125

Merged
merged 1 commit into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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.4.0
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.4.0 h1:irfww1426oQ9Nbro3DsxySpIwVmoxRS1LCA9RL73C8Y=
github.com/quic-go/qtls-go1-20 v0.4.0/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.4.0 h1:irfww1426oQ9Nbro3DsxySpIwVmoxRS1LCA9RL73C8Y=
github.com/quic-go/qtls-go1-20 v0.4.0/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
sukunrt marked this conversation as resolved.
Show resolved Hide resolved
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
2 changes: 1 addition & 1 deletion internal/handshake/crypto_setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,8 @@ var _ = Describe("Crypto Setup TLS", func() {
Eventually(receivedSessionTicket).Should(BeClosed())
Expect(server.ConnectionState().DidResume).To(BeTrue())
Expect(client.ConnectionState().DidResume).To(BeTrue())
Expect(clientRTTStats.SmoothedRTT()).To(Equal(clientRTT))
if !strings.Contains(runtime.Version(), "go1.20") {
Expect(clientRTTStats.SmoothedRTT()).To(Equal(clientRTT))
Expect(serverRTTStats.SmoothedRTT()).To(Equal(serverRTT))
}
})
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
5 changes: 4 additions & 1 deletion internal/qtls/client_session_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ var _ = Describe("Client Session Cache", func() {
ClientSessionCache: &clientSessionCache{
wrapped: tls.NewLRUClientSessionCache(10),
getData: func() []byte { return []byte("session") },
setData: func(data []byte) { restored <- data },
setData: func(data []byte) bool {
restored <- data
return true
},
},
}
conn, err := tls.Dial(
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