diff --git a/pcapkit/protocols/data/transport/tcp.py b/pcapkit/protocols/data/transport/tcp.py index 922d18af7f..7b0d5eeb43 100644 --- a/pcapkit/protocols/data/transport/tcp.py +++ b/pcapkit/protocols/data/transport/tcp.py @@ -388,9 +388,16 @@ def __init__(self, kind: 'OptionNumber', length: 'int', key_id: 'int', next_key_ @info_final class FastOpenCookie(Option): - """Data model for TCP fast open cookie option.""" + """Data model for TCP fast open cookie option. - #: Cookie. + .. versionchanged:: 1.5.0 + + ``cookie`` is ``b''`` rather than :obj:`None` for a cookie request + (:issue:`2167`). + + """ + + #: Cookie, ``b''`` for a cookie request. cookie: 'bytes' if TYPE_CHECKING: diff --git a/pcapkit/protocols/internet/hopopt.py b/pcapkit/protocols/internet/hopopt.py index c933c386b8..e79e250d67 100644 --- a/pcapkit/protocols/internet/hopopt.py +++ b/pcapkit/protocols/internet/hopopt.py @@ -990,7 +990,7 @@ def _read_opt_qs(self, schema: 'Schema_QuickStartOption', *, options: 'Option') change=bool(schema.type & 0b00100000), length=self._hopopt_option_length(schema_req.len), func=func, - rate=40000 * (2 ** rate) / 1000 if rate > 0 else 0, + rate=40000 * (2 ** rate) // 1000 if rate > 0 else 0, ttl=datetime.timedelta(seconds=schema_req.ttl), nonce=schema_req.nonce['nonce'], reserved=schema_req.nonce['reserved'], @@ -1005,7 +1005,7 @@ def _read_opt_qs(self, schema: 'Schema_QuickStartOption', *, options: 'Option') change=bool(schema.type & 0b00100000), length=self._hopopt_option_length(schema_rep.len), func=func, - rate=40000 * (2 ** rate) / 1000 if rate > 0 else 0, + rate=40000 * (2 ** rate) // 1000 if rate > 0 else 0, nonce=schema_rep.nonce['nonce'], reserved=schema_rep.nonce['reserved'], unused=schema_rep.reserved, diff --git a/pcapkit/protocols/internet/ipv4.py b/pcapkit/protocols/internet/ipv4.py index d9ff253297..a90d726841 100644 --- a/pcapkit/protocols/internet/ipv4.py +++ b/pcapkit/protocols/internet/ipv4.py @@ -1334,7 +1334,7 @@ def _read_opt_qs(self, schema: 'Schema_QSOption', *, options: 'Option') -> 'Data type=self._read_ipv4_opt_type(schema.type), length=schema_req.length, func=func, - rate=40000 * (2 ** rate) / 1000 if rate > 0 else 0, + rate=40000 * (2 ** rate) // 1000 if rate > 0 else 0, ttl=datetime.timedelta(seconds=schema_req.ttl), nonce=schema_req.nonce['nonce'], reserved=schema_req.nonce['reserved'], @@ -1348,7 +1348,7 @@ def _read_opt_qs(self, schema: 'Schema_QSOption', *, options: 'Option') -> 'Data type=self._read_ipv4_opt_type(schema.type), length=schema_rep.length, func=func, - rate=40000 * (2 ** rate) / 1000 if rate > 0 else 0, + rate=40000 * (2 ** rate) // 1000 if rate > 0 else 0, unused=schema_rep.reserved, nonce=schema_rep.nonce['nonce'], reserved=schema_rep.nonce['reserved'], diff --git a/pcapkit/protocols/internet/ipv6_opts.py b/pcapkit/protocols/internet/ipv6_opts.py index 60f7609c21..212bcb0860 100644 --- a/pcapkit/protocols/internet/ipv6_opts.py +++ b/pcapkit/protocols/internet/ipv6_opts.py @@ -978,7 +978,7 @@ def _read_opt_qs(self, schema: 'Schema_QuickStartOption', *, options: 'Option') change=bool(schema.type & 0b00100000), length=self._ipv6_opts_option_length(schema_req.len), func=func, - rate=40000 * (2 ** rate) / 1000 if rate > 0 else 0, + rate=40000 * (2 ** rate) // 1000 if rate > 0 else 0, ttl=datetime.timedelta(seconds=schema_req.ttl), nonce=schema_req.nonce['nonce'], reserved=schema_req.nonce['reserved'], @@ -993,7 +993,7 @@ def _read_opt_qs(self, schema: 'Schema_QuickStartOption', *, options: 'Option') change=bool(schema.type & 0b00100000), length=self._ipv6_opts_option_length(schema_rep.len), func=func, - rate=40000 * (2 ** rate) / 1000 if rate > 0 else 0, + rate=40000 * (2 ** rate) // 1000 if rate > 0 else 0, nonce=schema_rep.nonce['nonce'], reserved=schema_rep.nonce['reserved'], unused=schema_rep.reserved, diff --git a/pcapkit/protocols/transport/tcp.py b/pcapkit/protocols/transport/tcp.py index 92b1f82f57..5107960ebc 100644 --- a/pcapkit/protocols/transport/tcp.py +++ b/pcapkit/protocols/transport/tcp.py @@ -1432,7 +1432,7 @@ def _read_mode_qs(self, schema: 'Schema_QuickStartResponse', *, options: 'Option data = Data_QuickStartResponse( kind=schema.kind, length=schema.length, - req_rate=40000 * (2 ** rate) / 1000 if rate > 0 else 0, + req_rate=40000 * (2 ** rate) // 1000 if rate > 0 else 0, ttl_diff=schema.diff, nonce=schema.nonce['nonce'], reserved=schema.flags['reserved'], @@ -2129,10 +2129,12 @@ def _read_mode_fastopen(self, schema: 'Schema_FastOpenCookie', *, options: 'Opti if not (schema.length == 2 or (6 <= schema.length <= 18 and schema.length % 2 == 0)): raise ProtocolError(f'{self.alias}: [OptNo {schema.kind}] invalid format') + # a length of 2 is a cookie request (RFC 7413 Section 4.1.1), + # which carries no cookie, so the schema field is absent (#2167) data = Data_FastOpenCookie( kind=schema.kind, length=schema.length, - cookie=schema.cookie, + cookie=schema.cookie if schema.cookie is not None else b'', ) return data @@ -2676,7 +2678,9 @@ def _make_mode_qs(self, code: 'Enum_Option', opt: 'Optional[Data_QuickStartRespo code: option code opt: option data rate: rate (in kbps) - diff: time to live (in seconds) difference + diff: TTL Diff, the IP TTL minus the QS TTL, modulo 256 (:rfc:`4782` + Section 4.2); a :class:`~datetime.timedelta` is taken as its whole + seconds nonce: nonce value reserved: reserved bits before the rate request (``Resv.``, 4 bits) nonce_reserved: reserved bits after the nonce (``R``, 2 bits) @@ -3431,7 +3435,7 @@ def _make_mode_fastopen(self, code: 'Enum_Option', opt: 'Optional[Data_FastOpenC Args: code: option code opt: option data - cookie: fast open cookie + cookie: fast open cookie; :obj:`None` or ``b''`` for a cookie request **kwargs: arbitrary keyword arguments Returns: diff --git a/tests/protocols/internet/test_quick_start_func_unit.py b/tests/protocols/internet/test_quick_start_func_unit.py index b74530b11d..03dd87c336 100644 --- a/tests/protocols/internet/test_quick_start_func_unit.py +++ b/tests/protocols/internet/test_quick_start_func_unit.py @@ -75,6 +75,26 @@ def test_quick_start_option_round_trips(self) -> None: self.assertEqual(option.ttl, datetime.timedelta(seconds=5)) + def test_rate_parses_as_an_int(self) -> None: + """Every family reads ``rate`` as an :class:`int`, as annotated, not a float. See #2167. + + TCP's Quick-Start Response is checked in + ``tests/protocols/transport/test_tcp_quick_start_response_unit.py``. + """ + from pcapkit.const.ipv6.qs_function import QSFunction as Enum_QSFunction + + for name, cls, base, code in _families(): + for func in (Enum_QSFunction.Quick_Start_Request, + Enum_QSFunction.Report_of_Approved_Rate): + for rate in (0, 80, 640): + with self.subTest(family=name, func=func.name, rate=rate): + built = cls(options=[(code, dict(func=func, rate=rate, ttl=5, + nonce=NONCE))], **base) + option = list(cls(built.data).info.options.values())[0] + self.assertIs(type(option.rate), int) + self.assertEqual(option.rate, rate) + + @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') class QuickStartWireTests(unittest.TestCase): def setUp(self) -> None: diff --git a/tests/protocols/transport/test_tcp_quick_start_response_unit.py b/tests/protocols/transport/test_tcp_quick_start_response_unit.py index eefe60bdd5..61d0d6db92 100644 --- a/tests/protocols/transport/test_tcp_quick_start_response_unit.py +++ b/tests/protocols/transport/test_tcp_quick_start_response_unit.py @@ -94,6 +94,15 @@ def test_round_trip(self) -> 'None': self.assertEqual(again, raw) self.assertEqual(bytes(tcp.payload.data), payload) + def test_rate_parses_as_an_int(self) -> 'None': + """``req_rate`` is an :class:`int`, as annotated, not a float. See #2167.""" + QS = qs() + for rate in (0, 80, 640, 1_310_720): + with self.subTest(rate=rate): + req_rate = parse(build(rate)).info.options[QS].req_rate + self.assertIs(type(req_rate), int) + self.assertEqual(req_rate, rate) + def test_rate_rounds_down_to_a_representable_code(self) -> 'None': """A rate is floored to ``40 * 2 ** N``, and below 80 kbps to ``N = 0``.""" QS = qs() diff --git a/tests/protocols/transport/test_tcp_udp_unit.py b/tests/protocols/transport/test_tcp_udp_unit.py index b53851ba67..524a739b1c 100644 --- a/tests/protocols/transport/test_tcp_udp_unit.py +++ b/tests/protocols/transport/test_tcp_udp_unit.py @@ -809,10 +809,10 @@ def assert_bad(reader, schema) -> None: key_id=1, next_key_id=2, mac=b'mac'), options=options, ).mac, b'mac') - self.assertIsNone(proto._read_mode_fastopen( + self.assertEqual(proto._read_mode_fastopen( FastOpenCookie(kind=Option.TCP_Fast_Open_Cookie, length=2, cookie=None), options=options, - ).cookie) + ).cookie, b'') self.assertEqual(proto._read_mode_fastopen( FastOpenCookie(kind=Option.TCP_Fast_Open_Cookie, length=8, cookie=b'cookie'), options=options, @@ -1303,6 +1303,45 @@ def test_construction_accepts_bare_integer_ports(self) -> None: self.assertEqual(unnamed.info.srcport.port, 53406) self.assertIs(unnamed.info.dstport, member) + def test_a_fast_open_cookie_request_parses_as_empty_bytes(self) -> None: + """A Fast Open cookie request reads ``cookie=b''``, not :obj:`None`. See #2167. + + :rfc:`7413` sends a cookie request as option ``22 02``: length 2 and no + cookie. The data model annotates ``cookie`` as :class:`bytes`, and a + segment built with ``cookie=b''`` reads back ``b''``, so a parsed one + reading :obj:`None` broke both the annotation and the parse/construct + agreement. Both round trips are checked byte for byte. + + """ + import struct + + from pcapkit.const.tcp.option import Option + from pcapkit.protocols.transport.tcp import TCP + + # data offset 6: a cookie request, then two no-operations + wire = struct.pack('!HHIIBBHHH', 1, 2, 0, 0, 6 << 4, 0x10, 0xFFFF, 0, 0) \ + + bytes([Option.TCP_Fast_Open_Cookie, 2, 1, 1]) + parsed = TCP(wire, len(wire)) + cookie = parsed.info.options[Option.TCP_Fast_Open_Cookie] + self.assertIs(type(cookie.cookie), bytes) + self.assertEqual(cookie.cookie, b'') + self.assertEqual(cookie.to_dict()['cookie'], b'') + + for value in (None, b''): + with self.subTest(cookie=value): + built = TCP(srcport=1, dstport=2, ack=True, + options=[(Option.TCP_Fast_Open_Cookie, {'cookie': value}), + (Option.No_Operation, {}), (Option.No_Operation, {})]) + self.assertEqual(built.data, wire) + self.assertEqual(built.info.options[Option.TCP_Fast_Open_Cookie], cookie) + + again = TCP(srcport=1, dstport=2, ack=True, options=parsed.info.options) + self.assertEqual(again.data, wire) + from_dict = TCP(srcport=1, dstport=2, ack=True, + options=[(Option.TCP_Fast_Open_Cookie, cookie.to_dict()), + (Option.No_Operation, {}), (Option.No_Operation, {})]) + self.assertEqual(from_dict.data, wire) + def test_a_sack_option_overrunning_the_option_area_raises(self) -> None: """An over-long ``SACK`` option must be an error, not a hang.