From 51c9c42adc30daccdec8876e29c2a485f133b427 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 29 Mar 2019 09:39:02 +0100 Subject: [PATCH] reject transport parameters with too large stream counts --- mock_stream_manager_test.go | 6 ++++-- session.go | 7 +++++-- streams_map.go | 6 +++++- streams_map_test.go | 26 ++++++++++++++++++-------- 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/mock_stream_manager_test.go b/mock_stream_manager_test.go index 153652b13..1f9656609 100644 --- a/mock_stream_manager_test.go +++ b/mock_stream_manager_test.go @@ -197,9 +197,11 @@ func (mr *MockStreamManagerMockRecorder) OpenUniStreamSync() *gomock.Call { } // UpdateLimits mocks base method -func (m *MockStreamManager) UpdateLimits(arg0 *handshake.TransportParameters) { +func (m *MockStreamManager) UpdateLimits(arg0 *handshake.TransportParameters) error { m.ctrl.T.Helper() - m.ctrl.Call(m, "UpdateLimits", arg0) + ret := m.ctrl.Call(m, "UpdateLimits", arg0) + ret0, _ := ret[0].(error) + return ret0 } // UpdateLimits indicates an expected call of UpdateLimits diff --git a/session.go b/session.go index 94d92ab37..4ebb52c66 100644 --- a/session.go +++ b/session.go @@ -41,7 +41,7 @@ type streamManager interface { AcceptStream() (Stream, error) AcceptUniStream() (ReceiveStream, error) DeleteStream(protocol.StreamID) error - UpdateLimits(*handshake.TransportParameters) + UpdateLimits(*handshake.TransportParameters) error HandleMaxStreamsFrame(*wire.MaxStreamsFrame) error CloseWithError(error) } @@ -930,7 +930,10 @@ func (s *session) processTransportParameters(data []byte) { } s.logger.Debugf("Received Transport Parameters: %s", params) s.peerParams = params - s.streamsMap.UpdateLimits(params) + if err := s.streamsMap.UpdateLimits(params); err != nil { + s.closeLocal(err) + return + } s.packer.HandleTransportParameters(params) s.frameParser.SetAckDelayExponent(params.AckDelayExponent) s.connFlowController.UpdateSendWindow(params.InitialMaxData) diff --git a/streams_map.go b/streams_map.go index b47596de6..846ece004 100644 --- a/streams_map.go +++ b/streams_map.go @@ -174,10 +174,14 @@ func (m *streamsMap) HandleMaxStreamsFrame(f *wire.MaxStreamsFrame) error { return nil } -func (m *streamsMap) UpdateLimits(p *handshake.TransportParameters) { +func (m *streamsMap) UpdateLimits(p *handshake.TransportParameters) error { + if p.MaxBidiStreams > protocol.MaxStreamCount || p.MaxUniStreams > protocol.MaxStreamCount { + return qerr.StreamLimitError + } // Max{Uni,Bidi}StreamID returns the highest stream ID that the peer is allowed to open. m.outgoingBidiStreams.SetMaxStream(protocol.MaxStreamID(protocol.StreamTypeBidi, p.MaxBidiStreams, m.perspective)) m.outgoingUniStreams.SetMaxStream(protocol.MaxStreamID(protocol.StreamTypeUni, p.MaxUniStreams, m.perspective)) + return nil } func (m *streamsMap) CloseWithError(err error) { diff --git a/streams_map_test.go b/streams_map_test.go index 19f16b7bb..5b8852646 100644 --- a/streams_map_test.go +++ b/streams_map_test.go @@ -295,33 +295,43 @@ var _ = Describe("Streams Map", func() { }) Context("updating stream ID limits", func() { - BeforeEach(func() { - mockSender.EXPECT().queueControlFrame(gomock.Any()) - }) - It("processes the parameter for outgoing streams, as a server", func() { + mockSender.EXPECT().queueControlFrame(gomock.Any()) m.perspective = protocol.PerspectiveServer _, err := m.OpenStream() expectTooManyStreamsError(err) - m.UpdateLimits(&handshake.TransportParameters{ + Expect(m.UpdateLimits(&handshake.TransportParameters{ MaxBidiStreams: 5, MaxUniStreams: 5, - }) + })).To(Succeed()) Expect(m.outgoingBidiStreams.maxStream).To(Equal(protocol.StreamID(17))) Expect(m.outgoingUniStreams.maxStream).To(Equal(protocol.StreamID(19))) }) It("processes the parameter for outgoing streams, as a client", func() { + mockSender.EXPECT().queueControlFrame(gomock.Any()) m.perspective = protocol.PerspectiveClient _, err := m.OpenUniStream() expectTooManyStreamsError(err) - m.UpdateLimits(&handshake.TransportParameters{ + Expect(m.UpdateLimits(&handshake.TransportParameters{ MaxBidiStreams: 5, MaxUniStreams: 5, - }) + })).To(Succeed()) Expect(m.outgoingBidiStreams.maxStream).To(Equal(protocol.StreamID(16))) Expect(m.outgoingUniStreams.maxStream).To(Equal(protocol.StreamID(18))) }) + + It("rejects parameters with too large unidirectional stream counts", func() { + Expect(m.UpdateLimits(&handshake.TransportParameters{ + MaxUniStreams: protocol.MaxStreamCount + 1, + })).To(MatchError(qerr.StreamLimitError)) + }) + + It("rejects parameters with too large unidirectional stream counts", func() { + Expect(m.UpdateLimits(&handshake.TransportParameters{ + MaxBidiStreams: protocol.MaxStreamCount + 1, + })).To(MatchError(qerr.StreamLimitError)) + }) }) Context("handling MAX_STREAMS frames", func() {