Validate frame size

When receiving a frame from endpoint, check if length of frame is below the maximum size setting. If frame size is invalid then send a reset frame to endpoint. Add method to the connection class that sends a reset frames with an error code.

Author: jdecuyper

hyper/http20/connection.py

@@ -12,7 +12,7 @@
 from ..packages.hyperframe.frame import (
     FRAMES, DataFrame, HeadersFrame, PushPromiseFrame, RstStreamFrame,
     SettingsFrame, Frame, WindowUpdateFrame, GoAwayFrame, PingFrame,
-    BlockedFrame
+    BlockedFrame, FRAME_MAX_LEN
 )
 from ..packages.hpack.hpack_compat import Encoder, Decoder
 from .stream import Stream
@@ -123,6 +123,7 @@ def __init_state(self):
         # Values for the settings used on an HTTP/2 connection.
         self._settings = {
             SettingsFrame.INITIAL_WINDOW_SIZE: 65535,
+            SettingsFrame.SETTINGS_MAX_FRAME_SIZE: FRAME_MAX_LEN,
         }
 
         # The socket used to send data.
@@ -469,12 +470,7 @@ def _close_stream(self, stream_id, error_code=None):
         """
         Called by a stream when it would like to be 'closed'.
         """
-        if error_code is not None:
-            f = RstStreamFrame(stream_id)
-            f.error_code = error_code
-            self._send_cb(f)
-
-        del self.streams[stream_id]
+        self._send_rst_frame(stream_id, error_code)
 
     def _send_cb(self, frame, tolerate_peer_gone=False):
         """
@@ -535,6 +531,16 @@ def _consume_single_frame(self):
         # Parse the header. We can use the returned memoryview directly here.
         frame, length = Frame.parse_frame_header(header)
 
+        frame_size = self._settings[SettingsFrame.SETTINGS_MAX_FRAME_SIZE]
+        if (length > frame_size):
+            self._send_rst_frame(frame.stream_id, 6) # 6 = FRAME_SIZE_ERROR
+            log.warning(
+                "Frame size exceeded on stream %d (received: %d, max: %d)",
+                frame.stream_id,
+                length,
+                frame_size
+            )
+
         # Read the remaining data from the socket.
         data = self._recv_payload(length)
         self._consume_frame_payload(frame, data)
@@ -595,9 +601,7 @@ def _consume_frame_payload(self, frame, data):
                 # the ENABLE_PUSH setting is 0, but the spec leaves the client
                 # action undefined when they do it anyway. So we just refuse
                 # the stream and go about our business.
-                f = RstStreamFrame(frame.promised_stream_id)
-                f.error_code = 7 # REFUSED_STREAM
-                self._send_cb(f)
+                self._send_rst_frame(frame.promised_stream_id, 7)
 
         # Work out to whom this frame should go.
         if frame.stream_id != 0:
@@ -606,9 +610,7 @@ def _consume_frame_payload(self, frame, data):
             except KeyError:
                 # If we receive an unexpected stream identifier then we
                 # cancel the stream with an error of type PROTOCOL_ERROR
-                f = RstStreamFrame(frame.stream_id)
-                f.error_code = 1 # PROTOCOL_ERROR
-                self._send_cb(f)
+                self._send_rst_frame(frame.stream_id, 1)
                 log.warning(
                     "Unexpected stream identifier %d" % (frame.stream_id)
                 )
@@ -637,6 +639,21 @@ def _recv_cb(self):
             except ConnectionResetError:
                 break
 
+    def _send_rst_frame(self, stream_id, error_code):
+        """
+            Send reset stream frame with error code and remove stream from map.
+        """
+        if error_code is not None:
+            f = RstStreamFrame(stream_id)
+            f.error_code = error_code
+            self._send_cb(f)
+
+        try:
+            del self.streams[stream_id]
+        except KeyError as e:  # pragma: no cover
+            log.warn(
+                "Stream with id %d does not exist: %s",
+                stream_id, e)
 
     # The following two methods are the implementation of the context manager
     # protocol.

hyper/packages/hyperframe/frame.py

@@ -10,10 +10,10 @@
 import collections
 import struct
 
-# The maximum initial length of a frame. Some frames have shorter maximum lengths.
+# The maximum initial lengh of a frame. Some frames have shorter maximum lengths.
 FRAME_MAX_LEN = (2 ** 14) - 1
 
-# The maximum allowed length of a frame.
+# The maximum allowed lengh of a frame.
 FRAME_MAX_ALLOWED_LEN = (2 ** 24) - 1
 
 class Frame(object):

test/test_hyper.py

@@ -2,7 +2,7 @@
 from hyper.packages.hyperframe.frame import (
     Frame, DataFrame, RstStreamFrame, SettingsFrame,
     PushPromiseFrame, PingFrame, WindowUpdateFrame, HeadersFrame,
-    ContinuationFrame, BlockedFrame, GoAwayFrame,
+    ContinuationFrame, BlockedFrame, GoAwayFrame, FRAME_MAX_LEN
 )
 from hyper.packages.hpack.hpack_compat import Encoder, Decoder
 from hyper.http20.connection import HTTP20Connection
@@ -195,6 +195,7 @@ def test_closed_connections_are_reset(self):
         assert c.decoder is not decoder
         assert c._settings == {
             SettingsFrame.INITIAL_WINDOW_SIZE: 65535,
+            SettingsFrame.SETTINGS_MAX_FRAME_SIZE: FRAME_MAX_LEN,
         }
         assert c._out_flow_control_window == 65535
         assert c.window_manager is not wm
@@ -1282,6 +1283,40 @@ def data_callback(frame):
         assert isinstance(f, RstStreamFrame)
         assert f.error_code == 1 # PROTOCOL_ERROR
 
+    def test_connection_sends_rst_frame_if_frame_size_too_large(self):
+        sock = DummySocket()
+        d = DataFrame(1)
+        d.data = b'hi there frame'
+        sock.buffer = BytesIO(d.serialize())
+
+        def send_rst_frame(stream_id, error_code):
+            assert stream_id == 1
+            assert error_code == 6 #FRAME_SIZE_ERROR
+
+        c = HTTP20Connection('www.google.com')
+        # Lower the maximum frame size settings in order to force the
+        # connection to send a reset frame with FRAME_SIZE_ERROR
+        c._settings[SettingsFrame.SETTINGS_MAX_FRAME_SIZE] = 10
+        c._sock = sock
+        c._send_rst_frame = send_rst_frame
+        c.request('GET', '/')
+        c._recv_cb()
+
+    def test_connection_stream_is_removed_on_frame_size_error(self):
+        sock = DummySocket()
+        d = DataFrame(1)
+        d.data = b'hi there frame'
+        sock.buffer = BytesIO(d.serialize())
+
+        c = HTTP20Connection('www.google.com')
+        c._settings[SettingsFrame.SETTINGS_MAX_FRAME_SIZE] = 10
+        c._sock = sock
+        c.request('GET', '/')
+
+        # Make sure the stream gets removed from the map
+        assert len(c.streams) == 1
+        c._recv_cb()
+        assert len(c.streams) == 0
 
 # Some utility classes for the tests.
 class NullEncoder(object):