Add early detection of invalid http data when request line starts with binary

Author: touilleMan

h11/_readers.py

@@ -64,6 +64,8 @@ def _decode_header_lines(lines):
 def maybe_read_from_IDLE_client(buf):
     lines = buf.maybe_extract_lines()
     if lines is None:
+        if buf.is_next_line_obviously_invalid_request_line():
+            raise LocalProtocolError("illegal request line")
         return None
     if not lines:
         raise LocalProtocolError("no request line received")
@@ -81,6 +83,8 @@ def maybe_read_from_IDLE_client(buf):
 def maybe_read_from_SEND_RESPONSE_server(buf):
     lines = buf.maybe_extract_lines()
     if lines is None:
+        if buf.is_next_line_obviously_invalid_request_line():
+            raise LocalProtocolError("illegal request line")
         return None
     if not lines:
         raise LocalProtocolError("no response line received")

h11/_receivebuffer.py

@@ -133,3 +133,20 @@ def maybe_extract_lines(self):
         del lines[-2:]
 
         return lines
+
+    # In theory we should wait until `\r\n` before starting to validate
+    # incoming data. However it's interesting to detect (very) invalid data
+    # early given they might not even contain `\r\n` at all (hence only
+    # timeout will get rid of them).
+    # This is not a 100% effective detection but more of a cheap sanity check
+    # allowing for early abort in some useful cases.
+    # This is especially interesting when peer is messing up with HTTPS and
+    # sent us a TLS stream where we were expecting plain HTTP given all
+    # versions of TLS so far start handshake with a 0x16 message type code.
+    def is_next_line_obviously_invalid_request_line(self):
+        try:
+            # HTTP header line must not contain non-printable characters
+            # and should not start with a space
+            return self._data[0] < 0x21
+        except IndexError:
+            return False

h11/tests/test_connection.py

@@ -969,6 +969,38 @@ def test_empty_response():
         c.next_event()
 
 
+@pytest.mark.parametrize(
+    "data",
+    [
+        b"\x00",
+        b"\x20",
+        b"\x16\x03\x01\x00\xa5",  # Typical start of a TLS Client Hello
+    ],
+)
+def test_early_detection_of_invalid_request(data):
+    c = Connection(SERVER)
+    # Early detection should occur before even receiving a `\r\n`
+    c.receive_data(data)
+    with pytest.raises(RemoteProtocolError):
+        c.next_event()
+
+
+@pytest.mark.parametrize(
+    "data",
+    [
+        b"\x00",
+        b"\x20",
+        b"\x16\x03\x03\x00\x31",  # Typical start of a TLS Server Hello
+    ],
+)
+def test_early_detection_of_invalid_response(data):
+    c = Connection(CLIENT)
+    # Early detection should occur before even receiving a `\r\n`
+    c.receive_data(data)
+    with pytest.raises(RemoteProtocolError):
+        c.next_event()
+
+
 # This used to give different headers for HEAD and GET.
 # The correct way to handle HEAD is to put whatever headers we *would* have
 # put if it were a GET -- even though we know that for HEAD, those headers

newsfragments/122.feature.rst

@@ -0,0 +1 @@
+Add early detection of invalid http data when request line starts with binary