From 543ce21a5a9ac477f37f240c07bbd86dc88c8de4 Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Wed, 24 Aug 2016 00:59:11 +0700
Subject: [PATCH] prevent opening of new streams with very low StreamIDs

ref #256
---
 protocol/server_parameters.go |  4 ++++
 streams_map.go                | 16 ++++++++++++++--
 streams_map_test.go           | 31 +++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/protocol/server_parameters.go b/protocol/server_parameters.go
index ad25001e..74546530 100644
--- a/protocol/server_parameters.go
+++ b/protocol/server_parameters.go
@@ -40,6 +40,10 @@ const MaxStreamsMultiplier = 1.1
 // MaxStreamsMinimumIncrement is the slack the client is allowed for the maximum number of streams per connection, needed e.g. when packets are out of order or dropped. The minimum of this absolute increment and the procentual increase specified by MaxStreamsMultiplier is used.
 const MaxStreamsMinimumIncrement = 10
 
+// MaxNewStreamIDDelta is the maximum difference between and a newly opened Stream and the highest StreamID that a client has ever opened
+// note that the number of streams is half this value, since the client can only open streams with open StreamID
+const MaxNewStreamIDDelta = 4 * MaxStreamsPerConnection
+
 // MaxIdleConnectionStateLifetime is the maximum value accepted for the idle connection state lifetime
 // TODO: set a reasonable value here
 const MaxIdleConnectionStateLifetime = 60 * time.Second
diff --git a/streams_map.go b/streams_map.go
index 0bb2a200..eb7f309f 100644
--- a/streams_map.go
+++ b/streams_map.go
@@ -11,8 +11,11 @@ import (
 )
 
 type streamsMap struct {
-	streams       map[protocol.StreamID]*stream
-	openStreams   []protocol.StreamID
+	streams     map[protocol.StreamID]*stream
+	openStreams []protocol.StreamID
+
+	highestStreamOpenedByClient protocol.StreamID
+
 	mutex         sync.RWMutex
 	newStream     newStreamLambda
 	maxNumStreams int
@@ -61,10 +64,19 @@ func (m *streamsMap) GetOrOpenStream(id protocol.StreamID) (*stream, error) {
 	if id%2 == 0 {
 		return nil, qerr.Error(qerr.InvalidStreamID, fmt.Sprintf("attempted to open stream %d from client-side", id))
 	}
+	if id+protocol.MaxNewStreamIDDelta < m.highestStreamOpenedByClient {
+		return nil, qerr.Error(qerr.InvalidStreamID, fmt.Sprintf("attempted to open stream %d, which is a lot smaller than the highest opened stream, %d", id, m.highestStreamOpenedByClient))
+	}
+
 	s, err := m.newStream(id)
 	if err != nil {
 		return nil, err
 	}
+
+	if id > m.highestStreamOpenedByClient {
+		m.highestStreamOpenedByClient = id
+	}
+
 	m.putStream(s)
 	return s, nil
 }
diff --git a/streams_map_test.go b/streams_map_test.go
index ddd772a7..e6259655 100644
--- a/streams_map_test.go
+++ b/streams_map_test.go
@@ -76,6 +76,37 @@ var _ = Describe("Streams Map", func() {
 				}
 			})
 		})
+
+		Context("DoS mitigation", func() {
+			It("opens and closes a lot of streams", func() {
+				for i := 1; i < 2*protocol.MaxNewStreamIDDelta; i += 2 {
+					streamID := protocol.StreamID(i)
+					_, err := m.GetOrOpenStream(streamID)
+					Expect(m.highestStreamOpenedByClient).To(Equal(streamID))
+					Expect(err).NotTo(HaveOccurred())
+					err = m.RemoveStream(streamID)
+					Expect(err).NotTo(HaveOccurred())
+				}
+			})
+
+			It("prevents opening of streams with very low StreamIDs, if higher streams have already been opened", func() {
+				for i := 1; i < protocol.MaxNewStreamIDDelta+14; i += 2 {
+					if i == 11 || i == 13 {
+						continue
+					}
+					streamID := protocol.StreamID(i)
+					_, err := m.GetOrOpenStream(streamID)
+					Expect(err).NotTo(HaveOccurred())
+					err = m.RemoveStream(streamID)
+					Expect(err).NotTo(HaveOccurred())
+				}
+				Expect(m.highestStreamOpenedByClient).To(Equal(protocol.StreamID(protocol.MaxNewStreamIDDelta + 13)))
+				_, err := m.GetOrOpenStream(11)
+				Expect(err).To(MatchError("InvalidStreamID: attempted to open stream 11, which is a lot smaller than the highest opened stream, 413"))
+				_, err = m.GetOrOpenStream(13)
+				Expect(err).ToNot(HaveOccurred())
+			})
+		})
 	})
 
 	Context("deleting streams", func() {