Fix NTP Authenticator parsing for non-MD5 digest types

scapy/layers/ntp.py

@@ -40,6 +40,7 @@
     StrFixedLenField,
     StrLenField,
     XByteField,
+    XStrField,
     XStrFixedLenField,
 )
 from scapy.layers.inet import UDP
@@ -57,6 +58,14 @@
 _NTP_HDR_WITH_EXT_MIN_SIZE = _NTP_AUTH_MD5_MIN_SIZE + _NTP_EXT_MIN_SIZE
 _NTP_AUTH_MD5_TAIL_SIZE = 20
 _NTP_AUTH_MD5_DGST_SIZE = 16
+
+# Valid NTP MAC sizes (key_id + digest)
+_NTP_AUTH_MD5_SIZE = 20      # 4 + 16
+_NTP_AUTH_SHA1_SIZE = 24     # 4 + 20
+_NTP_AUTH_SHA256_SIZE = 36   # 4 + 32
+_NTP_AUTH_SHA384_SIZE = 52   # 4 + 48
+_NTP_AUTH_SHA512_SIZE = 68   # 4 + 64
+
 _NTP_PRIVATE_PACKET_MIN_SIZE = 8
 
 # ntpd "Private" messages are the shortest
@@ -76,6 +85,35 @@
 #     Fields and utilities
 #############################################################################
 
+
+def _ntp_auth_tail_size(length):
+    """
+    Dynamically compute the NTP authenticator tail size (key_id + digest).
+
+    Valid MAC sizes are defined as constants:
+    - _NTP_AUTH_MD5_SIZE (20): MD5 (4 + 16)
+    - _NTP_AUTH_SHA1_SIZE (24): SHA1 (4 + 20)
+    - _NTP_AUTH_SHA256_SIZE (36): SHA256 (4 + 32)
+    - _NTP_AUTH_SHA384_SIZE (52): SHA384 (4 + 48)
+    - _NTP_AUTH_SHA512_SIZE (68): SHA512 (4 + 64)
+
+    Returns the tail size if it matches a known valid size, otherwise
+    returns _NTP_AUTH_MD5_TAIL_SIZE as a fallback.
+    """
+    valid_mac_sizes = [
+        _NTP_AUTH_MD5_SIZE,
+        _NTP_AUTH_SHA1_SIZE,
+        _NTP_AUTH_SHA256_SIZE,
+        _NTP_AUTH_SHA384_SIZE,
+        _NTP_AUTH_SHA512_SIZE
+    ]
+    # Check for exact match with a known MAC size
+    if length in valid_mac_sizes:
+        return length
+    # Otherwise, default to MD5 size (backward compatibility)
+    return _NTP_AUTH_MD5_TAIL_SIZE
+
+
 class XLEShortField(LEShortField):
     """
     XShortField which value is encoded in little endian.
@@ -246,8 +284,9 @@ def getfield(self, pkt, s):
         remain = s
         length = len(s)
 
-        if length > _NTP_AUTH_MD5_TAIL_SIZE:
-            start = length - _NTP_AUTH_MD5_TAIL_SIZE
+        tail_size = _ntp_auth_tail_size(length)
+        if length > tail_size:
+            start = length - tail_size
             ret = s[:start]
             remain = s[start:]
         return remain, ret
@@ -263,7 +302,7 @@ class NTPAuthenticator(Packet):
     fields_desc = [
         _NTPAuthenticatorPaddingField("padding", ""),
         IntField("key_id", 0),
-        XStrFixedLenField("dgst", "", length_from=lambda x: 16)
+        XStrField("dgst", "")
     ]
 
     def extract_padding(self, s):
@@ -335,8 +374,9 @@ def getfield(self, pkt, s):
         lst = []
         remain = s
         length = len(s)
-        if length > _NTP_AUTH_MD5_TAIL_SIZE:
-            end = length - _NTP_AUTH_MD5_TAIL_SIZE
+        tail_size = _ntp_auth_tail_size(length)
+        if length > tail_size:
+            end = length - tail_size
             extensions = s[:end]
             remain = s[end:]
 
@@ -476,7 +516,8 @@ def guess_payload_class(self, payload):
         """
         plen = len(payload)
 
-        if plen - 4 in [16, 20, 32, 64]:  # length of MD5, SHA1, SHA256, SHA512
+        # length of MD5, SHA1, SHA256, SHA384, SHA512
+        if plen - 4 in [16, 20, 32, 48, 64]:
             return NTPAuthenticator
         elif plen > _NTP_AUTH_MD5_TAIL_SIZE:
             return NTPExtensions

test/scapy/layers/ntp.uts

@@ -103,11 +103,56 @@ assert p.version == 4
 assert p.mode == 3
 assert p.stratum == 2
 
-= NTPAuthenticator
+= NTPAuthenticator - MD5 with padding (old test, updated for correct parsing)
 
+# This packet has 24 bytes of authenticator data
+# The old (incorrect) code interpreted this as: 4 padding + 4 key_id + 16 MD5 digest  
+# The new (correct) code interprets 24 bytes as SHA1 MAC: 4 key_id + 20 SHA1 digest
+# Note: This test packet may have been created with MD5 intent, but with 24 bytes
+# total, it's now correctly parsed as SHA1 according to RFC 5905 standards
 s = hex_bytes("000c2962f268d094666d23750800450000640db640004011a519c0a80364c0a80305a51e007b0050731a2300072000000000000000000000000000000000000000000000000000000000000000000000000052c7bc1dda64b97d0000000bcdc3825dbf6b7ad02886ff45aa8b2eaf7ac78bc1")
 p = Ether(s)
-assert NTPAuthenticator in p and p[NTPAuthenticator].key_id == 3452142173
+assert NTPAuthenticator in p
+# With 24 bytes, this is now interpreted as SHA1 (4 + 20), not MD5 with padding
+assert p[NTPAuthenticator].key_id == 11  # First 4 bytes: 0000000b
+assert len(p[NTPAuthenticator].dgst) == 20  # SHA1 digest
+assert bytes_hex(p[NTPAuthenticator].dgst) == b'cdc3825dbf6b7ad02886ff45aa8b2eaf7ac78bc1'
+
+= NTPAuthenticator - SHA1 (24 bytes: 4 key_id + 20 digest)
+# Create an NTP packet with SHA1 authenticator
+ntp_header = b"!\x0b\x06\xea\x00\x00\x00\x00\x00\x00\xf2\xc1\x7f\x7f\x01\x00\xdb9\xe8\xa21\x02\xe6\xbc\xdb9\xe8\x81\x02U8\xef\xdb9\xe8\x80\xdcl+\x06\xdb9\xe8\xa91\xcbI\xbf"
+sha1_key_id = b"\x00\x00\x00\x02"  # key_id = 2
+sha1_digest = b"\x11\x22\x33\x44\x55\x66\x77\x88\x99\xaa\xbb\xcc\xdd\xee\xff\x00\x01\x02\x03\x04"  # 20 bytes
+s = ntp_header + sha1_key_id + sha1_digest
+p = NTP(s)
+assert isinstance(p, NTPHeader)
+assert NTPAuthenticator in p
+assert p[NTPAuthenticator].key_id == 2
+assert len(p[NTPAuthenticator].dgst) == 20
+assert bytes_hex(p[NTPAuthenticator].dgst) == b'112233445566778899aabbccddeeff0001020304'
+# Test round-trip (build and parse)
+rebuilt = NTP(raw(p))
+assert rebuilt[NTPAuthenticator].key_id == 2
+assert len(rebuilt[NTPAuthenticator].dgst) == 20
+assert bytes_hex(rebuilt[NTPAuthenticator].dgst) == b'112233445566778899aabbccddeeff0001020304'
+
+= NTPAuthenticator - SHA256 (36 bytes: 4 key_id + 32 digest)
+# Create an NTP packet with SHA256 authenticator
+ntp_header = b"!\x0b\x06\xea\x00\x00\x00\x00\x00\x00\xf2\xc1\x7f\x7f\x01\x00\xdb9\xe8\xa21\x02\xe6\xbc\xdb9\xe8\x81\x02U8\xef\xdb9\xe8\x80\xdcl+\x06\xdb9\xe8\xa91\xcbI\xbf"
+sha256_key_id = b"\x00\x00\x00\x03"  # key_id = 3
+sha256_digest = b"\xaa\xbb\xcc\xdd\xee\xff\x00\x11\x22\x33\x44\x55\x66\x77\x88\x99\x11\x22\x33\x44\x55\x66\x77\x88\x99\xaa\xbb\xcc\xdd\xee\xff\x00"  # 32 bytes
+s = ntp_header + sha256_key_id + sha256_digest
+p = NTP(s)
+assert isinstance(p, NTPHeader)
+assert NTPAuthenticator in p
+assert p[NTPAuthenticator].key_id == 3
+assert len(p[NTPAuthenticator].dgst) == 32
+assert bytes_hex(p[NTPAuthenticator].dgst) == b'aabbccddeeff00112233445566778899112233445566778899aabbccddeeff00'
+# Test round-trip (build and parse)
+rebuilt = NTP(raw(p))
+assert rebuilt[NTPAuthenticator].key_id == 3
+assert len(rebuilt[NTPAuthenticator].dgst) == 32
+assert bytes_hex(rebuilt[NTPAuthenticator].dgst) == b'aabbccddeeff00112233445566778899112233445566778899aabbccddeeff00'
 
 
 ############
@@ -343,8 +388,11 @@ assert p.more == 0
 assert p.op_code == 9
 assert p.count == 15
 assert p.data == b'ntp.test.2.conf'
-assert p.authenticator.key_id == 1
-assert bytes_hex(p.authenticator.dgst) == b'c9fb8abe3c605ffa36d218c3b7648923'
+# After data padding to 4-byte alignment, there are 24 bytes for authenticator
+# With dynamic parsing, 24 bytes = SHA1 (4 key_id + 20 digest)
+assert p.authenticator.key_id == 0
+assert len(p.authenticator.dgst) == 20
+assert bytes_hex(p.authenticator.dgst) == b'00000001c9fb8abe3c605ffa36d218c3b7648923'
 
 
 = NTP Control (mode 6) - CTL_OP_SAVECONFIG (2) - response