From 3588cddd438e4e454f117addc35b5265c138859e Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 10 Mar 2021 14:16:18 +0800 Subject: [PATCH] allow 0-RTT resumption if the server's stream limit was increased --- integrationtests/self/zero_rtt_test.go | 55 ++++++++++++++++++++++- internal/handshake/crypto_setup.go | 2 +- internal/wire/transport_parameter_test.go | 32 ++++++++----- internal/wire/transport_parameters.go | 16 +++---- 4 files changed, 83 insertions(+), 22 deletions(-) diff --git a/integrationtests/self/zero_rtt_test.go b/integrationtests/self/zero_rtt_test.go index a76eb72e..3cbae9c6 100644 --- a/integrationtests/self/zero_rtt_test.go +++ b/integrationtests/self/zero_rtt_test.go @@ -433,7 +433,58 @@ var _ = Describe("0-RTT", func() { Expect(zeroRTTPackets[0]).To(BeNumerically(">=", protocol.PacketNumber(5))) }) - It("rejects 0-RTT when the server's transport parameters changed", func() { + It("doesn't reject 0-RTT when the server's transport stream limit increased", func() { + const maxStreams = 1 + tlsConf, clientConf := dialAndReceiveSessionTicket(getQuicConfig(&quic.Config{ + MaxIncomingUniStreams: maxStreams, + AcceptToken: func(_ net.Addr, _ *quic.Token) bool { return true }, + })) + + tracer := newRcvdPacketTracer() + ln, err := quic.ListenAddrEarly( + "localhost:0", + tlsConf, + getQuicConfig(&quic.Config{ + Versions: []protocol.VersionNumber{version}, + AcceptToken: func(_ net.Addr, _ *quic.Token) bool { return true }, + MaxIncomingUniStreams: maxStreams + 1, + Tracer: newTracer(func() logging.ConnectionTracer { return tracer }), + }), + ) + Expect(err).ToNot(HaveOccurred()) + defer ln.Close() + proxy, num0RTTPackets := runCountingProxy(ln.Addr().(*net.UDPAddr).Port) + defer proxy.Close() + + sess, err := quic.DialAddrEarly( + fmt.Sprintf("localhost:%d", proxy.LocalPort()), + clientConf, + getQuicConfig(&quic.Config{Versions: []protocol.VersionNumber{version}}), + ) + Expect(err).ToNot(HaveOccurred()) + str, err := sess.OpenUniStream() + Expect(err).ToNot(HaveOccurred()) + _, err = str.Write([]byte("foobar")) + Expect(err).ToNot(HaveOccurred()) + Expect(str.Close()).To(Succeed()) + // The client remembers the old limit and refuses to open a new stream. + _, err = sess.OpenUniStream() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("too many open streams")) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + _, err = sess.OpenUniStreamSync(ctx) + Expect(err).ToNot(HaveOccurred()) + Expect(sess.CloseWithError(0, "")).To(Succeed()) + + // The client should send 0-RTT packets. + num0RTT := atomic.LoadUint32(num0RTTPackets) + fmt.Fprintf(GinkgoWriter, "Sent %d 0-RTT packets.", num0RTT) + Expect(num0RTT).ToNot(BeZero()) + Expect(get0RTTPackets(tracer.getRcvdPackets())).ToNot(BeEmpty()) + }) + + It("rejects 0-RTT when the server's stream limit decreased", func() { const maxStreams = 42 tlsConf, clientConf := dialAndReceiveSessionTicket(getQuicConfig(&quic.Config{ MaxIncomingStreams: maxStreams, @@ -447,7 +498,7 @@ var _ = Describe("0-RTT", func() { getQuicConfig(&quic.Config{ Versions: []protocol.VersionNumber{version}, AcceptToken: func(_ net.Addr, _ *quic.Token) bool { return true }, - MaxIncomingStreams: maxStreams + 1, + MaxIncomingStreams: maxStreams - 1, Tracer: newTracer(func() logging.ConnectionTracer { return tracer }), }), ) diff --git a/internal/handshake/crypto_setup.go b/internal/handshake/crypto_setup.go index f2300ffd..699807ac 100644 --- a/internal/handshake/crypto_setup.go +++ b/internal/handshake/crypto_setup.go @@ -466,7 +466,7 @@ func (h *cryptoSetup) GetSessionTicket() ([]byte, error) { func (h *cryptoSetup) accept0RTT(sessionTicketData []byte) bool { var t sessionTicket if err := t.Unmarshal(sessionTicketData); err != nil { - h.logger.Debugf("Unmarshaling transport parameters from session ticket failed: %s", err.Error()) + h.logger.Debugf("Unmarshalling transport parameters from session ticket failed: %s", err.Error()) return false } valid := h.ourParams.ValidFor0RTT(t.Parameters) diff --git a/internal/wire/transport_parameter_test.go b/internal/wire/transport_parameter_test.go index 2dca346d..f4b3a80f 100644 --- a/internal/wire/transport_parameter_test.go +++ b/internal/wire/transport_parameter_test.go @@ -483,7 +483,7 @@ var _ = Describe("Transport Parameters", func() { Context("rejects the parameters if they changed", func() { var p TransportParameters - params := &TransportParameters{ + saved := &TransportParameters{ InitialMaxStreamDataBidiLocal: 1, InitialMaxStreamDataBidiRemote: 2, InitialMaxStreamDataUni: 3, @@ -494,43 +494,53 @@ var _ = Describe("Transport Parameters", func() { } BeforeEach(func() { - p = *params - Expect(params.ValidFor0RTT(&p)).To(BeTrue()) + p = *saved + Expect(p.ValidFor0RTT(saved)).To(BeTrue()) }) It("rejects the parameters if the InitialMaxStreamDataBidiLocal changed", func() { p.InitialMaxStreamDataBidiLocal = 0 - Expect(params.ValidFor0RTT(&p)).To(BeFalse()) + Expect(p.ValidFor0RTT(saved)).To(BeFalse()) }) It("rejects the parameters if the InitialMaxStreamDataBidiRemote changed", func() { p.InitialMaxStreamDataBidiRemote = 0 - Expect(params.ValidFor0RTT(&p)).To(BeFalse()) + Expect(p.ValidFor0RTT(saved)).To(BeFalse()) }) It("rejects the parameters if the InitialMaxStreamDataUni changed", func() { p.InitialMaxStreamDataUni = 0 - Expect(params.ValidFor0RTT(&p)).To(BeFalse()) + Expect(p.ValidFor0RTT(saved)).To(BeFalse()) }) It("rejects the parameters if the InitialMaxData changed", func() { p.InitialMaxData = 0 - Expect(params.ValidFor0RTT(&p)).To(BeFalse()) + Expect(p.ValidFor0RTT(saved)).To(BeFalse()) }) It("rejects the parameters if the MaxBidiStreamNum changed", func() { p.MaxBidiStreamNum = 0 - Expect(params.ValidFor0RTT(&p)).To(BeFalse()) + Expect(p.ValidFor0RTT(saved)).To(BeFalse()) + }) + + It("accepts the parameters if the MaxBidiStreamNum was increased", func() { + p.MaxBidiStreamNum = saved.MaxBidiStreamNum + 1 + Expect(p.ValidFor0RTT(saved)).To(BeTrue()) }) It("rejects the parameters if the MaxUniStreamNum changed", func() { - p.MaxUniStreamNum = 0 - Expect(params.ValidFor0RTT(&p)).To(BeFalse()) + p.MaxUniStreamNum = saved.MaxUniStreamNum - 1 + Expect(p.ValidFor0RTT(saved)).To(BeFalse()) + }) + + It("accepts the parameters if the MaxUniStreamNum was increased", func() { + p.MaxUniStreamNum = saved.MaxUniStreamNum + 1 + Expect(p.ValidFor0RTT(saved)).To(BeTrue()) }) It("rejects the parameters if the ActiveConnectionIDLimit changed", func() { p.ActiveConnectionIDLimit = 0 - Expect(params.ValidFor0RTT(&p)).To(BeFalse()) + Expect(p.ValidFor0RTT(saved)).To(BeFalse()) }) }) }) diff --git a/internal/wire/transport_parameters.go b/internal/wire/transport_parameters.go index 759d4dd3..6bf437dc 100644 --- a/internal/wire/transport_parameters.go +++ b/internal/wire/transport_parameters.go @@ -440,14 +440,14 @@ func (p *TransportParameters) UnmarshalFromSessionTicket(r *bytes.Reader) error } // ValidFor0RTT checks if the transport parameters match those saved in the session ticket. -func (p *TransportParameters) ValidFor0RTT(tp *TransportParameters) bool { - return p.InitialMaxStreamDataBidiLocal == tp.InitialMaxStreamDataBidiLocal && - p.InitialMaxStreamDataBidiRemote == tp.InitialMaxStreamDataBidiRemote && - p.InitialMaxStreamDataUni == tp.InitialMaxStreamDataUni && - p.InitialMaxData == tp.InitialMaxData && - p.MaxBidiStreamNum == tp.MaxBidiStreamNum && - p.MaxUniStreamNum == tp.MaxUniStreamNum && - p.ActiveConnectionIDLimit == tp.ActiveConnectionIDLimit +func (p *TransportParameters) ValidFor0RTT(saved *TransportParameters) bool { + return p.InitialMaxStreamDataBidiLocal == saved.InitialMaxStreamDataBidiLocal && + p.InitialMaxStreamDataBidiRemote == saved.InitialMaxStreamDataBidiRemote && + p.InitialMaxStreamDataUni == saved.InitialMaxStreamDataUni && + p.InitialMaxData == saved.InitialMaxData && + p.MaxBidiStreamNum >= saved.MaxBidiStreamNum && + p.MaxUniStreamNum >= saved.MaxUniStreamNum && + p.ActiveConnectionIDLimit == saved.ActiveConnectionIDLimit } // String returns a string representation, intended for logging.