fix(http): tighten HTTP/2 version guessing so arbitrary binary is not taken for a DATA frame

pcapkit/protocols/application/http.py

@@ -39,6 +39,65 @@
 #: :meth:`HTTP._guess_version` tests it before attempting any parse.
 _HTTP2_PREFACE = b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n'
 
+#: What :rfc:`9113#section-6` allows a sender to put in the frame header of each
+#: frame type it defines: the flags that type defines, and the stream identifier
+#: it is sent on -- ``True`` for a non-zero stream, ``False`` for stream 0 and
+#: :obj:`None` for either. :meth:`HTTP._guess_version` checks a bare frame
+#: against this before its HTTP/2 trial parse; see :func:`_test_http2_frame`.
+_HTTP2_FRAME_RULES = {
+    0x0: (0x09, True),    # DATA: END_STREAM, PADDED (Section 6.1)
+    0x1: (0x2D, True),    # HEADERS: END_STREAM, END_HEADERS, PADDED, PRIORITY (Section 6.2)
+    0x2: (0x00, True),    # PRIORITY (Section 6.3)
+    0x3: (0x00, True),    # RST_STREAM (Section 6.4)
+    0x4: (0x01, False),   # SETTINGS: ACK (Section 6.5)
+    0x5: (0x0C, True),    # PUSH_PROMISE: END_HEADERS, PADDED (Section 6.6)
+    0x6: (0x01, False),   # PING: ACK (Section 6.7)
+    0x7: (0x00, False),   # GOAWAY (Section 6.8)
+    0x8: (0x00, None),    # WINDOW_UPDATE: either stream (Section 6.9)
+    0x9: (0x04, True),    # CONTINUATION: END_HEADERS (Section 6.10)
+}  # type: dict[int, tuple[int, Optional[bool]]]
+
+
+def _test_http2_frame(data: 'bytes') -> 'bool':
+    """Test whether ``data`` opens with a frame header a conforming sender could write.
+
+    This is a plausibility test for :meth:`HTTP._guess_version`'s fall-through
+    only. ``httpv2.HTTP`` itself parses every frame :rfc:`9113` lets a receiver
+    accept, which includes frames this test turns down.
+
+    Args:
+        data: Payload, starting at the frame header.
+
+    Returns:
+        :data:`True` if the nine-octet header carries a frame type defined by
+        :rfc:`9113#section-6`, sets no flag that type leaves undefined, leaves the
+        reserved bit clear, and names a stream that type may be sent on.
+
+    Notes:
+        Undefined flags and the reserved bit "MUST be left unset (0x00) when
+        sending" (:rfc:`9113#section-4.1`), and each stream rule is one a
+        receiver "MUST treat as a connection error" when broken (e.g. ``DATA``
+        on stream 0, :rfc:`9113#section-6.1`), so no frame a conforming peer
+        sends fails here. Frame types outside the ten that :rfc:`9113` defines
+        are legal and ignored by a receiver (:rfc:`9113#section-5.5`), but on a
+        payload with no other evidence they are what arbitrary binary looks
+        like most of the time, so they are not guessed.
+
+    """
+    if len(data) < 9:
+        return False
+    rules = _HTTP2_FRAME_RULES.get(data[3])
+    if rules is None:
+        return False
+    flags, stream = rules
+    if data[4] & ~flags:
+        return False
+    if data[5] & 0x80:
+        return False
+    if stream is None:
+        return True
+    return any(data[5:9]) is stream
+
 
 class HTTP(Application[_PT, _ST], Generic[_PT, _ST]):
     """This class implements all protocols in HTTP family.
@@ -253,14 +312,16 @@ def _guess_version(self, length: 'int', **kwargs: 'Any') -> 'HTTP':
               preface ahead of it, or an opaque HTTP/1 message body -- is
               genuinely undecidable from one payload, and the honest answer is
               the ``Raw`` that an escaping ``ProtocolError`` becomes under
-              :func:`~pcapkit.protocols.misc.raw.beholder`. In particular there
-              is deliberately *no* heuristic on the nine-octet frame header
-              ("type at most 9, reserved bit clear"): that misfires on binary
-              HTTP/1 bodies, which is precisely how garbage text was classified
-              HTTP/2 to begin with. A self-consistent bare frame is still parsed
-              as HTTP/2, but by the fall-through below -- on the parser's own
-              length/type consistency rules -- rather than by a guess dressed up
-              as identification.
+              :func:`~pcapkit.protocols.misc.raw.beholder`. The nine-octet frame
+              header is therefore never used to *identify* HTTP/2 ahead of
+              HTTP/1: as identification it misfires on binary HTTP/1 bodies,
+              which is precisely how garbage text was classified HTTP/2 to begin
+              with. A bare frame is parsed as HTTP/2 only by the fall-through
+              below, and only if its header is one a conforming sender could
+              write (see :func:`_test_http2_frame`): a frame type :rfc:`9113`
+              defines, no undefined flag, and a stream that type may be sent on.
+              Nine zero octets -- a ``DATA`` frame on stream 0, a protocol error
+              under :rfc:`9113#section-6.1` -- are not guessed as HTTP/2.
 
         """
         # NOTE: Positive identification, before any parse attempt. The preface is
@@ -368,6 +429,16 @@ def _guess_version(self, length: 'int', **kwargs: 'Any') -> 'HTTP':
         with contextlib.suppress(ProtocolError):
             return HTTPv1(self._data, length, **kwargs)
 
+        # NOTE: The HTTP/2 trial runs only on a header that passes
+        # :func:`_test_http2_frame`. Since a frame's Length counts the payload
+        # alone (:rfc:`9113#section-4.1`), any nine octets with a small enough
+        # Length are self-consistent, so the parser's own checks accept most
+        # zero-heavy binary as a ``DATA`` frame or a frame of unknown type. The
+        # test rejects only headers that no conforming sender writes, and it
+        # gates this guess alone: an explicit ``version=2`` is not affected.
+        if not _test_http2_frame(self._data[:length]):
+            raise ProtocolError("unknown HTTP version")
+
         from pcapkit.protocols.application.httpv2 import HTTP as HTTPv2  # isort: skip # pylint: disable=line-too-long,import-outside-toplevel
         with contextlib.suppress(ProtocolError, struct.error):
             return HTTPv2(self._data, length, **kwargs)

tests/protocols/application/test_http_guess_http2_1141_unit.py

@@ -0,0 +1,169 @@
+# -*- coding: utf-8 -*-
+"""HTTP's version guess takes a bare frame for HTTP/2 only if a sender could write it. C.f. #1141.
+
+Since #1138 an HTTP/2 frame's Length counts the payload alone
+(:rfc:`9113#section-4.1`), so any nine octets whose Length fits the buffer are
+self-consistent. :meth:`HTTP._guess_version
+<pcapkit.protocols.application.http.HTTP._guess_version>` falls through to an
+HTTP/2 trial parse when neither the preface nor an HTTP/1 start line matches, and
+that trial accepted most zero-heavy binary: nine zero octets came back as
+``HTTP/2 type=0``, a ``DATA`` frame on stream 0, which :rfc:`9113#section-6.1`
+makes a protocol error.
+
+The guess now requires a frame type :rfc:`9113` defines, no flag that type
+leaves undefined, a clear reserved bit, and a stream that type may be sent on.
+Explicit ``version=2`` parsing is not affected, and is asserted unchanged here.
+
+The legal frames in :data:`LEGAL_FRAMES` were built with Scapy 2.7.0's
+:mod:`scapy.contrib.http2` (``H2Frame`` over each frame class) and are embedded
+as bytes, so this module does not depend on Scapy, which the per-PR test job
+does not install.
+
+"""
+
+from __future__ import annotations
+
+import importlib.util
+import random
+import unittest
+import warnings
+
+from tests._support import reimport_once_per_class, time_limit
+
+RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper')
+HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS)
+
+#: One frame of each type :rfc:`9113` defines, with the flag and stream
+#: variants each type allows, as Scapy writes them: name, frame type, octets.
+LEGAL_FRAMES = (
+    ('DATA', 0x0, '00000500000000000168656c6c6f'),
+    ('DATA, empty, END_STREAM', 0x0, '000000000100000001'),
+    ('DATA, PADDED', 0x0, '000006000900000003047800000000'),
+    ('HEADERS', 0x1, '00000101050000000182'),
+    ('HEADERS, PADDED, PRIORITY', 0x1, '000009012c0000000102000000000f820000'),
+    ('PRIORITY', 0x2, '0000050200000000050000000110'),
+    ('RST_STREAM', 0x3, '00000403000000000100000008'),
+    ('SETTINGS', 0x4, '000006040000000000000300000064'),
+    ('SETTINGS, empty', 0x4, '000000040000000000'),
+    ('SETTINGS, ACK', 0x4, '000000040100000000'),
+    ('PUSH_PROMISE', 0x5, '0000050504000000010000000282'),
+    ('PING', 0x6, '0000080600000000000000000000000001'),
+    ('PING, ACK', 0x6, '0000080601000000000000000000000001'),
+    ('GOAWAY', 0x7, '0000080700000000000000000100000000'),
+    ('GOAWAY, debug data', 0x7, '00000b0700000000000000000100000002627965'),
+    ('WINDOW_UPDATE, stream 0', 0x8, '000004080000000000000003e8'),
+    ('WINDOW_UPDATE, stream 1', 0x8, '000004080000000001000003e8'),
+    ('CONTINUATION', 0x9, '00000109040000000182'),
+)
+
+#: Frame headers no conforming sender writes, each with a zero-length payload
+#: unless the type needs one to parse: name, octets.
+REJECTED_FRAMES = (
+    ('nine zero octets: DATA on stream 0', '000000000000000000'),
+    ('HEADERS on stream 0', '000000010400000000'),
+    ('CONTINUATION on stream 0', '000000090400000000'),
+    ('GOAWAY on stream 1', '0000080700000000010000000000000000'),
+    ('DATA with undefined flag 0x02', '000000000200000001'),
+    ('SETTINGS with undefined flag 0x02', '000000040200000000'),
+    ('DATA with the reserved bit set', '000000000080000001'),
+    ('unknown frame type 0xEE', '0000000aee0000000100000000000000000000'),
+    ('ORIGIN, not an RFC 9113 type', '0000000c0000000000'),
+)
+
+#: The seeded fuzz. Pre-fix, this corpus produced 566 HTTP/2 labels, of which
+#: 249 were ``DATA`` (most on stream 0) and 317 frames of unknown type.
+FUZZ_SEED = 1141
+FUZZ_COUNT = 5000
+PRE_FIX_HTTP2_LABELS = 566
+
+
+def _fuzz_buffers() -> 'list[bytes]':
+    """Zero-heavy random buffers of 0-64 octets, the shape that reaches HTTP/2."""
+    rng = random.Random(FUZZ_SEED)
+    buffers = []
+    for _ in range(FUZZ_COUNT):
+        size = rng.randrange(0, 65)
+        buffers.append(bytes(0 if rng.random() < 0.5 else rng.randrange(256)
+                             for _ in range(size)))
+    return buffers
+
+
+@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed')
+class HTTPGuessHTTP2Tests(unittest.TestCase):
+    """The HTTP/2 fall-through in ``_guess_version`` takes only plausible frames."""
+
+    def setUp(self) -> None:
+        reimport_once_per_class(self)
+
+    def _guess(self, data: bytes) -> 'object':
+        from pcapkit.protocols.application.http import HTTP
+
+        with warnings.catch_warnings():
+            warnings.simplefilter('ignore')
+            with time_limit():
+                return HTTP(data, len(data))
+
+    def test_nine_zero_octets_are_not_guessed_as_http2(self) -> None:
+        """The #1141 reproduction: a stream-0 ``DATA`` frame is not HTTP/2 by guess."""
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        with self.assertRaisesRegex(ProtocolError, 'unknown HTTP version'):
+            self._guess(bytes(9))
+
+    def test_explicit_version_still_parses_nine_zero_octets(self) -> None:
+        """Explicit HTTP/2 parsing stays RFC-exact: only the guess is tightened."""
+        from pcapkit.protocols.application.http import HTTP
+
+        http = HTTP(bytes(9), 9, version=2)
+        self.assertEqual(http.version, '2')
+        self.assertEqual((int(http.info.type), http.info.sid), (0, 0))
+
+    def test_headers_no_sender_writes_are_not_guessed(self) -> None:
+        """Wrong stream, undefined flag, reserved bit or unknown type: refused."""
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        for name, octets in REJECTED_FRAMES:
+            with self.subTest(frame=name):
+                with self.assertRaisesRegex(ProtocolError, 'unknown HTTP version'):
+                    self._guess(bytes.fromhex(octets))
+
+    def test_every_legal_frame_type_is_still_guessed_as_http2(self) -> None:
+        """Each Scapy-built frame of the ten RFC 9113 types comes back HTTP/2."""
+        seen = set()
+        for name, frame_type, octets in LEGAL_FRAMES:
+            with self.subTest(frame=name):
+                http = self._guess(bytes.fromhex(octets))
+                self.assertEqual(http.version, '2')  # type: ignore[attr-defined]
+                self.assertEqual(int(http.info.type), frame_type)  # type: ignore[attr-defined]
+                seen.add(frame_type)
+        self.assertEqual(seen, set(range(10)))
+
+    def test_seeded_fuzz_false_positive_rate_drops(self) -> None:
+        """Far fewer random buffers are labelled HTTP/2, and each is a stream-bound ``DATA``.
+
+        What is left is a ``DATA`` frame on a non-zero stream with defined flags,
+        which is a legal frame and so cannot be refused without refusing real
+        traffic.
+
+        """
+        from pcapkit.utilities.exceptions import ProtocolError
+
+        labels = []
+        for data in _fuzz_buffers():
+            try:
+                http = self._guess(data)
+            except ProtocolError:
+                continue
+            if http.version == '2':  # type: ignore[attr-defined]
+                labels.append(data)
+
+        self.assertLess(len(labels), PRE_FIX_HTTP2_LABELS // 4)
+        for data in labels:
+            with self.subTest(frame=data[:9].hex()):
+                self.assertEqual(data[3], 0x0)
+                self.assertTrue(any(data[5:9]))
+                self.assertEqual(data[4] & ~0x09, 0)
+
+
+if __name__ == '__main__':
+    unittest.main()

tests/protocols/application/test_http_unit.py

@@ -147,18 +147,26 @@ def make(**kwargs):
         with mock.patch('pcapkit.protocols.application.httpv2.HTTP', FakeHTTPv2):
             self.assertEqual(http.make(version=2, sid=1), ('v2-made', {'sid': 1}))
 
+        # The HTTP/2 arm runs only on a frame header a sender could write
+        # (#1141), so the fall-through is driven with one: an empty ``DATA``
+        # frame on stream 1.
+        frame = object.__new__(HTTP)
+        frame._data = http2_frame_bytes(0x0, 0, 1, b'')
+        frame._file = object()
+        frame.__cached__ = {}
+
         with mock.patch('pcapkit.protocols.application.httpv1.HTTP',
                         mock.Mock(side_effect=ProtocolError('bad v1'))), \
                 mock.patch('pcapkit.protocols.application.httpv2.HTTP', FakeHTTPv2):
-            self.assertIsInstance(HTTP._guess_version(http, 9), FakeHTTPv2)
+            self.assertIsInstance(HTTP._guess_version(frame, 9), FakeHTTPv2)
         with mock.patch('pcapkit.protocols.application.httpv1.HTTP', FakeHTTPv1):
             self.assertIsInstance(HTTP._guess_version(http, 9), FakeHTTPv1)
         with mock.patch('pcapkit.protocols.application.httpv1.HTTP',
                         mock.Mock(side_effect=ProtocolError('bad v1'))), \
                 mock.patch('pcapkit.protocols.application.httpv2.HTTP',
                            mock.Mock(side_effect=ProtocolError('bad v2'))):
             with self.assertRaises(ProtocolError):
-                HTTP._guess_version(http, 9)
+                HTTP._guess_version(frame, 9)
 
     def test_http_make_dispatches_to_real_versioned_classes(self) -> None:
         """Regression test for GH-452.
@@ -427,12 +435,19 @@ def test_http_guess_version_falls_through_to_the_http2_arm(self) -> None:
         ``test_guess_version_reaches_http2_on_the_connection_preface``, which is
         where that assertion belongs.
 
+        Since #1141 the HTTP/2 arm runs only on a frame header a sender could
+        write, and ``b'not http at all'`` (type ``0x20``) is turned away before
+        it. So the payload is now a ``DATA`` frame header on stream 1 that
+        declares 255 octets of payload, with six behind it. That header passes
+        the plausibility test, so arm 2 is entered, and the declared length then
+        makes the arm decline it, as "not"'s did.
+
         """
         from pcapkit.protocols.application.http import HTTP
         from pcapkit.utilities.exceptions import ProtocolError
 
         http = object.__new__(HTTP)
-        http._data = b'not http at all'
+        http._data = bytes.fromhex('0000ff000000000001') + b'x' * 6
 
         entered = []  # type: list[str]
         real_v1 = importlib.import_module('pcapkit.protocols.application.httpv1').HTTP

tests/protocols/transport/test_tcp_http_dispatch_unit.py

@@ -11,11 +11,10 @@
 :class:`pcapkit.protocols.application.http.HTTP` does.
 
 Nothing is parsed from :file:`examples/captures/`, so this is unit tier. The
-segments below are built in memory, and the HTTP/2 payload is the *verbatim*
-nine octets that :file:`examples/captures/options-transport.pcap` frame 34
-carries -- a fixture this library's own ``httpv2.HTTP.make`` produced -- rather
-than a hand-rolled frame, so the frame is whatever the library itself writes and
-cannot fail for a reason that has nothing to do with dispatch.
+segments below are built in memory, and the HTTP/2 payload is the nine octets
+this library's own ``httpv2.HTTP`` writes for an empty ``DATA`` frame on stream
+1, rather than a hand-rolled frame, so the frame is whatever the library itself
+writes and cannot fail for a reason that has nothing to do with dispatch.
 
 """
 
@@ -36,9 +35,11 @@
 _TCP_TO_8080 = bytes.fromhex('c3501f9000000001000000015010ffff00000000')
 
 #: A nine-octet HTTP/2 ``DATA`` frame: declared length 0 (the payload, the
-#: header excluded), type 0, no flags, stream 0. Copied verbatim from
-#: ``options-transport.pcap`` frame 34.
-_HTTP2_FRAME = bytes.fromhex('000000000000000000')
+#: header excluded), type 0, no flags, stream 1, as
+#: ``httpv2.HTTP(type=Frame.DATA, sid=1, frame={'data': b''})`` writes it. A
+#: ``DATA`` frame needs a non-zero stream (:rfc:`9113#section-6.1`), so the
+#: version guess does not take a stream-0 one for HTTP/2.
+_HTTP2_FRAME = bytes.fromhex('000000000000000001')
 #: A minimal HTTP/1.1 request, which must keep decoding as HTTP/1.1.
 _HTTP1_REQUEST = b'GET / HTTP/1.1\r\nHost: example.invalid\r\n\r\n'