[Git][NTPsec/ntpsec][master] Fixed scrambled data storage in SyncPacket()
Ian Bruene
gitlab at mg.gitlab.com
Wed Aug 23 18:22:25 UTC 2017
Ian Bruene pushed to branch master at NTPsec / ntpsec
Commits:
6dafbd9f by Ian Bruene at 2017-08-23T13:17:24-05:00
Fixed scrambled data storage in SyncPacket()
Previously SyncPacket would take data and store it in self.data, which
would only ever be used by analyze. Then analyze() would parse self.data
leaving cruft behind in self.extension. flatten() encoded the header and
tacked on self.extension. The result is that a packet could not be encoded
and decoded losslessly.
- - - - -
2 changed files:
- pylib/packet.py
- tests/pylib/test_packet.py
Changes:
=====================================
pylib/packet.py
=====================================
--- a/pylib/packet.py
+++ b/pylib/packet.py
@@ -398,7 +398,6 @@ class SyncPacket(Packet):
self.origin_timestamp = 0
self.receive_timestamp = 0
self.transmit_timestamp = 0
- self.data = polybytes(data)
self.extension = ''
self.extfields = []
self.mac = ''
@@ -407,11 +406,12 @@ class SyncPacket(Packet):
self.received = SyncPacket.posix_to_ntp(time.time())
self.trusted = True
self.rescaled = False
- if self.data:
- self.analyze()
+ if data:
+ self.analyze(polybytes(data))
- def analyze(self):
- if len(self.data) < SyncPacket.HEADER_LEN or (len(self.data) & 3) != 0:
+ def analyze(self, data):
+ datalen = len(data)
+ if datalen < SyncPacket.HEADER_LEN or (datalen & 3) != 0:
raise SyncException("impossible packet length")
(self.li_vn_mode,
self.stratum,
@@ -424,8 +424,8 @@ class SyncPacket(Packet):
self.origin_timestamp,
self.receive_timestamp,
self.transmit_timestamp) = struct.unpack(
- SyncPacket.format, self.data[:SyncPacket.HEADER_LEN])
- self.extension = self.data[SyncPacket.HEADER_LEN:]
+ SyncPacket.format, data[:SyncPacket.HEADER_LEN])
+ self.extension = data[SyncPacket.HEADER_LEN:]
# Parse the extension field if present. We figure out whether
# an extension field is present by measuring the MAC size. If
# the number of 4-octet words following the packet header is
@@ -436,18 +436,19 @@ class SyncPacket(Packet):
# or 4, the packet is a runt and discarded forthwith. If
# greater than 6, an extension field is present, so we
# subtract the length of the field and go around again.
- while len(self.extension) > 24:
- (ftype, flen) = struct.unpack("!II", self.extension[:8])
- self.extfields.append((ftype, self.extension[8:8+flen]))
- self.extension = self.extension[8+flen:]
- if len(self.extension) == 4: # Crypto-NAK
- self.mac = self.extension
- if len(self.extension) == 12: # DES
+ payload = self.extension # Keep extension intact for flatten()
+ while len(payload) > 24:
+ (ftype, flen) = struct.unpack("!II", payload[:8])
+ self.extfields.append((ftype, payload[8:8+flen]))
+ payload = payload[8+flen:]
+ if len(payload) == 4: # Crypto-NAK
+ self.mac = payload
+ if len(payload) == 12: # DES
raise SyncException("Unsupported DES authentication")
- elif len(self.extension) in (8, 16):
+ elif len(payload) in (8, 16):
raise SyncException("Packet is a runt")
- elif len(self.extension) in (20, 24): # MD5 or SHA1
- self.mac = self.extension
+ elif len(payload) in (20, 24): # MD5 or SHA1
+ self.mac = payload
@staticmethod
def ntp_to_posix(t):
=====================================
tests/pylib/test_packet.py
=====================================
--- a/tests/pylib/test_packet.py
+++ b/tests/pylib/test_packet.py
@@ -310,7 +310,6 @@ class TestSyncPacket(unittest.TestCase):
self.assertEqual(cls.origin_timestamp, 0)
self.assertEqual(cls.receive_timestamp, 0)
self.assertEqual(cls.transmit_timestamp, 0)
- self.assertEqual(cls.data, ntpp.polybytes(""))
self.assertEqual(cls.extension, '')
self.assertEqual(cls.extfields, [])
self.assertEqual(cls.mac, '')
@@ -363,11 +362,12 @@ class TestSyncPacket(unittest.TestCase):
self.assertEqual(cls.transmit_timestamp, 0x0301020304050607)
self.assertEqual(cls.extfields, [])
# Test with extension, Crypto-NAK
- data += "\x00\x00\x00\x01\x00\x00\x00\x04blah" \
- "\x00\x00\x00\x02\x00\x00\x00\x0Cjabberjabber" \
- "\x00\x00\x00\x03\x00\x00\x00\x20" \
- "In the end, our choices make us."
- data2 = data + "\x11\x22\x33\x44"
+ ext = "\x00\x00\x00\x01\x00\x00\x00\x04blah" \
+ "\x00\x00\x00\x02\x00\x00\x00\x0Cjabberjabber" \
+ "\x00\x00\x00\x03\x00\x00\x00\x20" \
+ "In the end, our choices make us."
+ mac = "\x11\x22\x33\x44"
+ data2 = data + ext + mac
cls = self.target(data2)
self.assertEqual(cls.li_vn_mode, 0x5C)
self.assertEqual(cls.stratum, 16)
@@ -383,9 +383,10 @@ class TestSyncPacket(unittest.TestCase):
self.assertEqual(cls.extfields,
[(1, "blah"), (2, "jabberjabber"),
(3, "In the end, our choices make us.")])
- self.assertEqual(cls.mac, "\x11\x22\x33\x44")
+ self.assertEqual(cls.extension, ext + mac)
+ self.assertEqual(cls.mac, mac)
# Test with extension, DES
- data2 = data + "\x11\x22\x33\x44\x55\x66\x77\x88\x99\xAA\xBB\xCC"
+ data2 = data + ext + "\x11\x22\x33\x44\x55\x66\x77\x88\x99\xAA\xBB\xCC"
try:
cls = self.target(data2)
errored = False
@@ -393,7 +394,7 @@ class TestSyncPacket(unittest.TestCase):
errored = e.message
self.assertEqual(errored, "Unsupported DES authentication")
# Test with extension, runt 8
- data2 = data + "\x11\x22\x33\x44\x55\x66\x77\x88"
+ data2 = data + ext + "\x11\x22\x33\x44\x55\x66\x77\x88"
try:
cls = self.target(data2)
errored = False
@@ -401,7 +402,7 @@ class TestSyncPacket(unittest.TestCase):
errored = e.message
self.assertEqual(errored, "Packet is a runt")
# Test with extension, runt 16
- data2 = data + "\x00\x11\x22\x33\x44\x55\x66\x77" \
+ data2 = data + ext + "\x00\x11\x22\x33\x44\x55\x66\x77" \
"\x88\x99\xAA\xBB\xCC\xDD\xEE\xFF"
try:
cls = self.target(data2)
@@ -410,16 +411,18 @@ class TestSyncPacket(unittest.TestCase):
errored = e.message
self.assertEqual(errored, "Packet is a runt")
# Test with extension, MD5 or SHA1, 20
- ext = "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09" \
+ mac = "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09" \
"\x0A\x0B\x0C\x0D\x0E\x0F\x10\x11\x12\x13"
- data2 = data + ext
+ data2 = data + ext + mac
cls = self.target(data2)
- self.assertEqual(cls.mac, ext)
+ self.assertEqual(cls.mac, mac)
+ self.assertEqual(cls.extension, ext + mac)
# Test with extension, MD5 or SHA1, 24
- ext += "\x14\x15\x16\x17"
- data2 = data + ext
+ mac += "\x14\x15\x16\x17"
+ data2 = data + ext + mac
cls = self.target(data2)
- self.assertEqual(cls.mac, ext)
+ self.assertEqual(cls.mac, mac)
+ self.assertEqual(cls.extension, ext + mac)
def test_ntp_to_posix(self):
f = self.target.ntp_to_posix
@@ -519,9 +522,9 @@ class TestSyncPacket(unittest.TestCase):
"\x00\x00\x00\x03\x00\x00\x00\x20" \
"In the end, our choices make us." \
"\x11\x22\x33\x44"
- cls = self.target(data)
- cls.extension = ext
- self.assertEqual(cls.flatten(), data + ext)
+ pkt = data + ext
+ cls = self.target(pkt)
+ self.assertEqual(cls.flatten(), pkt)
def test_refid_octets(self):
cls = self.target()
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/6dafbd9fd7481d6a59f0a4aa86cbbc4592b820ab
---
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/6dafbd9fd7481d6a59f0a4aa86cbbc4592b820ab
You're receiving this email because of your account on gitlab.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20170823/a34d8eaa/attachment.html>
More information about the vc
mailing list