DHCP: implement RFC 3396 encoding of long options#4950
DHCP: implement RFC 3396 encoding of long options#4950StrugglingKeyboard wants to merge 1 commit intosecdev:masterfrom
Conversation
There was a problem hiding this comment.
I think it's really cool that
DHCP(options=[('captive-portal', b'a'*256)])can automatically split the option. That being said I'm not sure how safe it is to start concatenating the options by default. Personally in some cases I think it would still be better to be able to see all the options (and the existing scripts actually expect that).
The commit message should probably point to #4343 as well.
(The tests fail because zero-length options get lost
>>> raw(DHCP(options=[('rapid_commit', b'')]))
b''It should be
>>> raw(DHCP(options=[('rapid_commit', b'')]))
b'P\x00'Those tests do pass with the master branch)
2f4ba1d to
e58cf84
Compare
|
Updated the PR based on @evverx's review:
I also updated the PR description above to reflect these changes. A note on the current design: RFC 3396 splitting is always applied during encoding (since the output is valid regardless of receiver support), but concatenation during decoding is opt-in. I'm not fully sure about the naming of the toggle, like I'd be glad to add documentation for these changes if needed. Let me know what you think, or if there's anything else I should address. |
…ecdev#4343) Split DHCP options longer than 255 bytes into multiple consecutive TLV entries during serialization (i2m), as specified by RFC 3396. Add opt-in RFC 3396 decoding via conf.contribs["dhcp"]["rfc3396"]. When enabled, all options sharing the same code are concatenated globally before interpretation, and the aggregate option buffer is built from options/file/sname fields when option overload is present (RFC 3396 section 5). When disabled (the default), decoding behavior is unchanged.
e58cf84 to
ed2d19e
Compare
Description
Implements RFC 3396 (Encoding Long Options in DHCPv4) in Scapy's DHCP layer.
Fixes #4642 and fixes #4343
Problem
raw(DHCP(options=[('captive-portal', 'a'*256)]))raisesstruct.error: ubyte format requires 0 <= number <= 255because the option length field is encoded as a single unsigned byte, limiting option values to 255 bytes.On the decoding side, DHCP packets containing multiple options with the same code (as produced by RFC 3396 encoding) are not reassembled, leading to incorrect or incomplete dissection.
Changes
All changes are in
scapy/layers/dhcp.py, within the classDHCPOptionsField.Encoding (
i2m)Options with values exceeding 255 bytes are automatically split into consecutive TLV fragments. Zero-length options (such as
rapid_commit) are correctly preserved. This is always active and does not require any toggle, since the produced output is valid for any receiver regardless of RFC 3396 support.Decoding (
m2i)Controlled by
conf.contribs["dhcp"]["rfc3396"], defaulting toTrue.When
False, the original decoding behavior is preserved exactly: each TLV is an independent entry in the options list. This avoids breaking existing scripts that rely on seeing individual TLV entries.When
True, decoding follows RFC 3396 strictly:The concatenation is global rather than consecutive-only because RFC 3396 section 7 states that when a decoding agent "finds two or more options with the same option code, it MUST consider them to be split portions of an option", without requiring them to be adjacent. This also matches Wireshark's implementation.
Why opt-in rather than default?
On the wire, two separate options with the same code are indistinguishable from a single option split into two fragments. Enabling concatenation by default would silently change the output of existing scripts that inspect individual options. Making it opt-in lets users choose the behavior they need.
New helper methods:
_extract_raw_entries: extracts TLV entries from a raw buffer without interpreting them. Reused for the main options buffer and for file/sname when overload is present._merge_entries: merges all entries with the same option code, preserving order of first appearance. Pads, ends and malformed trailing data are kept in place._entries_to_raw: reconstructs raw bytes from extracted entries, used to preserve the legacy error-handling behavior (appending the remaining raw buffer and stopping when a field interpretation fails).Error handling:
When
f.getfieldfails during interpretation, the remaining entries are reconstructed as raw bytes via_entries_to_raw, appended to the options list, and parsing stops. This matches the original behavior whereopt.append(x); breakwas used.Tests
Added a single test block in
test/scapy/layers/dhcp.utscovering:i2m: splitting at 256 bytes, zero-length option preservationm2iwithrfc3396=True: global concatenation (typed fields, non-consecutive fragments, unknown option codes)m2iwithrfc3396=False: legacy behavior preservedThe
conf.contribs["dhcp"]["rfc3396"]value is saved and restored around the tests.