From e864c3234c45b8969e936355ae90cd904523da38 Mon Sep 17 00:00:00 2001 From: Andrew Chen <48723787+chuenchen309@users.noreply.github.com> Date: Fri, 17 Jul 2026 21:04:34 +0800 Subject: [PATCH] Fix DNSRRTSIG length fields not auto-computing DNSRRTSIG declared its two FieldLenField length prefixes with hardcoded defaults -- mac_len=20 and other_len=0 -- instead of None. FieldLenField only derives the length from its length_of target when the value is None, so the prefixes were frozen and never matched the actual mac_data / other_data. Consequences on the build/dissect round-trip: - Even the zero-argument DNSRRTSIG() cannot re-parse its own bytes: mac_len is 20 but mac_data is empty, so on dissect the mac_data StrLenField consumes the 20 bytes belonging to the trailing fields and the next getfield unpacks past the buffer (struct.error). - A real MAC (e.g. a 32-byte HMAC-SHA256) is silently truncated to 20 bytes on the wire, because the length prefix stays 20. Every other FieldLenField in dns.py -- DNSRROPT.rdlen, DNSRRNAPTR's flags_len/services_len/regexp_len, and the header counts -- uses None. Change mac_len and other_len to None so they auto-compute like their siblings. Two existing dns_dnssec.uts assertions had baked in the buggy 0x14 (mac_len=20 with an empty MAC); corrected to 0x00 and added a round-trip case that a 32-byte MAC and 6-byte other_data survive build->dissect. Co-Authored-By: Claude Opus 4.8 (1M context) AI-Assisted: yes (Claude Opus 4.8) --- scapy/layers/dns.py | 4 ++-- test/scapy/layers/dns_dnssec.uts | 13 +++++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/scapy/layers/dns.py b/scapy/layers/dns.py index 432b0a06066..fe534abbc94 100644 --- a/scapy/layers/dns.py +++ b/scapy/layers/dns.py @@ -1138,11 +1138,11 @@ class DNSRRTSIG(_DNSRRdummy): DNSStrField("algo_name", "hmac-sha1"), TimeSignedField("time_signed", 0), ShortField("fudge", 0), - FieldLenField("mac_len", 20, fmt="!H", length_of="mac_data"), # noqa: E501 + FieldLenField("mac_len", None, fmt="!H", length_of="mac_data"), # noqa: E501 StrLenField("mac_data", "", length_from=lambda pkt: pkt.mac_len), # noqa: E501 ShortField("original_id", 0), ShortField("error", 0), - FieldLenField("other_len", 0, fmt="!H", length_of="other_data"), # noqa: E501 + FieldLenField("other_len", None, fmt="!H", length_of="other_data"), # noqa: E501 StrLenField("other_data", "", length_from=lambda pkt: pkt.other_len) # noqa: E501 ] diff --git a/test/scapy/layers/dns_dnssec.uts b/test/scapy/layers/dns_dnssec.uts index 631f7237921..2882aa3a1f6 100644 --- a/test/scapy/layers/dns_dnssec.uts +++ b/test/scapy/layers/dns_dnssec.uts @@ -132,11 +132,20 @@ t.type == 16 and t.rdlen == 312 and t.rdata[0][:5] == b"Scapy" and t.rdata[1][-1 = DNSRRTSIG basic instantiation t = DNSRRTSIG() -raw(t) == b"\x00\x00\xfa\x00\x01\x00\x00\x00\x00\x00\x1b\thmac-sha1\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x14\x00\x00\x00\x00\x00\x00" +raw(t) == b"\x00\x00\xfa\x00\x01\x00\x00\x00\x00\x00\x1b\thmac-sha1\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" = DNSRRTSIG(), check parameters t = DNSRRTSIG(rrname="SAMPLE-ALG.EXAMPLE.", time_signed=853804800, fudge=300) -raw(t) == b"\nSAMPLE-ALG\x07EXAMPLE\x00\x00\xfa\x00\x01\x00\x00\x00\x00\x00\x1b\thmac-sha1\x00\x00\x002\xe4\x07\x00\x01,\x00\x14\x00\x00\x00\x00\x00\x00" +raw(t) == b"\nSAMPLE-ALG\x07EXAMPLE\x00\x00\xfa\x00\x01\x00\x00\x00\x00\x00\x1b\thmac-sha1\x00\x00\x002\xe4\x07\x00\x01,\x00\x00\x00\x00\x00\x00\x00\x00" + += DNSRRTSIG mac_len/other_len auto-computed, round-trips +# Regression: mac_len/other_len defaulted to a hardcoded 20/0 instead of None, +# so FieldLenField never auto-computed and a non-empty MAC did not round-trip +# (the length prefix stayed 20, truncating the MAC to 20 bytes on re-dissect). +t = DNSRRTSIG(mac_data=b"M" * 32, other_data=b"O" * 6) +raw_t = raw(t) +p = DNSRRTSIG(raw_t) +p.mac_len == 32 and p.other_len == 6 and p.mac_data == b"M" * 32 and p.other_data == b"O" * 6 and raw(p) == raw_t = TimeField methods