diff options
-rw-r--r-- | dns/dnssec.py | 8 | ||||
-rw-r--r-- | dns/rdata.py | 74 | ||||
-rw-r--r-- | dns/zone.py | 9 | ||||
-rw-r--r-- | tests/test_rdata.py | 89 | ||||
-rw-r--r-- | tests/test_zonedigest.py | 15 |
5 files changed, 180 insertions, 15 deletions
diff --git a/dns/dnssec.py b/dns/dnssec.py index f09ecd6..6e9946f 100644 --- a/dns/dnssec.py +++ b/dns/dnssec.py @@ -404,13 +404,13 @@ def _validate_rrsig(rrset, rrsig, keys, origin=None, now=None): rrnamebuf = rrname.to_digestable() rrfixed = struct.pack('!HHI', rdataset.rdtype, rdataset.rdclass, rrsig.original_ttl) - for rr in sorted(rdataset): + rdatas = [rdata.to_digestable(origin) for rdata in rdataset] + for rdata in sorted(rdatas): data += rrnamebuf data += rrfixed - rrdata = rr.to_digestable(origin) - rrlen = struct.pack('!H', len(rrdata)) + rrlen = struct.pack('!H', len(rdata)) data += rrlen - data += rrdata + data += rdata chosen_hash = _make_hash(rrsig.algorithm) diff --git a/dns/rdata.py b/dns/rdata.py index 0831c41..624063e 100644 --- a/dns/rdata.py +++ b/dns/rdata.py @@ -38,6 +38,22 @@ import dns.ttl _chunksize = 32 +# We currently allow comparisons for rdata with relative names for backwards +# compatibility, but in the future we will not, as these kinds of comparisons +# can lead to subtle bugs if code is not carefully written. +# +# This switch allows the future behavior to be turned on so code can be +# tested with it. +_allow_relative_comparisons = True + + +class NoRelativeRdataOrdering(dns.exception.DNSException): + """An attempt was made to do an ordered comparison of one or more + rdata with relative names. The only reliable way of sorting rdata + is to use non-relativized rdata. + + """ + def _wordbreak(data, chunksize=_chunksize, separator=b' '): """Break a binary string into chunks of chunksize characters separated by @@ -232,12 +248,42 @@ class Rdata: """Compare an rdata with another rdata of the same rdtype and rdclass. - Return < 0 if self < other in the DNSSEC ordering, 0 if self - == other, and > 0 if self > other. - + For rdata with only absolute names: + Return < 0 if self < other in the DNSSEC ordering, 0 if self + == other, and > 0 if self > other. + For rdata with at least one relative names: + The rdata sorts before any rdata with only absolute names. + When compared with another relative rdata, all names are + made absolute as if they were relative to the root, as the + proper origin is not available. While this creates a stable + ordering, it is NOT guaranteed to be the DNSSEC ordering. + In the future, all ordering comparisons for rdata with + relative names will be disallowed. """ - our = self.to_digestable(dns.name.root) - their = other.to_digestable(dns.name.root) + try: + our = self.to_digestable() + our_relative = False + except dns.name.NeedAbsoluteNameOrOrigin: + if _allow_relative_comparisons: + our = self.to_digestable(dns.name.root) + our_relative = True + try: + their = other.to_digestable() + their_relative = False + except dns.name.NeedAbsoluteNameOrOrigin: + if _allow_relative_comparisons: + their = other.to_digestable(dns.name.root) + their_relative = True + if _allow_relative_comparisons: + if our_relative != their_relative: + # For the purpose of comparison, all rdata with at least one + # relative name is less than an rdata with only absolute names. + if our_relative: + return -1 + else: + return 1 + elif our_relative or their_relative: + raise NoRelativeRdataOrdering if our == their: return 0 elif our > their: @@ -250,14 +296,28 @@ class Rdata: return False if self.rdclass != other.rdclass or self.rdtype != other.rdtype: return False - return self._cmp(other) == 0 + our_relative = False + their_relative = False + try: + our = self.to_digestable() + except dns.name.NeedAbsoluteNameOrOrigin: + our = self.to_digestable(dns.name.root) + our_relative = True + try: + their = other.to_digestable() + except dns.name.NeedAbsoluteNameOrOrigin: + their = other.to_digestable(dns.name.root) + their_relative = True + if our_relative != their_relative: + return False + return our == their def __ne__(self, other): if not isinstance(other, Rdata): return True if self.rdclass != other.rdclass or self.rdtype != other.rdtype: return True - return self._cmp(other) != 0 + return not self.__eq__(other) def __lt__(self, other): if not isinstance(other, Rdata) or \ diff --git a/dns/zone.py b/dns/zone.py index 9c3204b..d154928 100644 --- a/dns/zone.py +++ b/dns/zone.py @@ -733,10 +733,11 @@ class Zone(dns.transaction.TransactionManager): continue rrfixed = struct.pack('!HHI', rdataset.rdtype, rdataset.rdclass, rdataset.ttl) - for rr in sorted(rdataset): - rrdata = rr.to_digestable(self.origin) - rrlen = struct.pack('!H', len(rrdata)) - hasher.update(rrnamebuf + rrfixed + rrlen + rrdata) + rdatas = [rdata.to_digestable(self.origin) + for rdata in rdataset] + for rdata in sorted(rdatas): + rrlen = struct.pack('!H', len(rdata)) + hasher.update(rrnamebuf + rrfixed + rrlen + rdata) return hasher.digest() def compute_digest(self, hash_algorithm, scheme=DigestScheme.SIMPLE): diff --git a/tests/test_rdata.py b/tests/test_rdata.py index 05ec6ca..f87ff56 100644 --- a/tests/test_rdata.py +++ b/tests/test_rdata.py @@ -696,7 +696,96 @@ class RdataTestCase(unittest.TestCase): rr = dns.rdata.from_text('IN', 'DNSKEY', input_variation) new_text = rr.to_text(chunksize=chunksize) self.assertEqual(output, new_text) + + def test_relative_vs_absolute_compare_unstrict(self): + try: + saved = dns.rdata._allow_relative_comparisons + dns.rdata._allow_relative_comparisons = True + r1 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'www.') + r2 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'www') + self.assertFalse(r1 == r2) + self.assertTrue(r1 != r2) + self.assertFalse(r1 < r2) + self.assertFalse(r1 <= r2) + self.assertTrue(r1 > r2) + self.assertTrue(r1 >= r2) + self.assertTrue(r2 < r1) + self.assertTrue(r2 <= r1) + self.assertFalse(r2 > r1) + self.assertFalse(r2 >= r1) + finally: + dns.rdata._allow_relative_comparisons = saved + + def test_relative_vs_absolute_compare_strict(self): + try: + saved = dns.rdata._allow_relative_comparisons + dns.rdata._allow_relative_comparisons = False + r1 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'www.') + r2 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'www') + self.assertFalse(r1 == r2) + self.assertTrue(r1 != r2) + def bad1(): + r1 < r2 + def bad2(): + r1 <= r2 + def bad3(): + r1 > r2 + def bad4(): + r1 >= r2 + self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad1) + self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad2) + self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad3) + self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad4) + finally: + dns.rdata._allow_relative_comparisons = saved + + def test_absolute_vs_absolute_compare(self): + r1 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'www.') + r2 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'xxx.') + self.assertFalse(r1 == r2) + self.assertTrue(r1 != r2) + self.assertTrue(r1 < r2) + self.assertTrue(r1 <= r2) + self.assertFalse(r1 > r2) + self.assertFalse(r1 >= r2) + def test_relative_vs_relative_compare_unstrict(self): + try: + saved = dns.rdata._allow_relative_comparisons + dns.rdata._allow_relative_comparisons = True + r1 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'www') + r2 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'xxx') + self.assertFalse(r1 == r2) + self.assertTrue(r1 != r2) + self.assertTrue(r1 < r2) + self.assertTrue(r1 <= r2) + self.assertFalse(r1 > r2) + self.assertFalse(r1 >= r2) + finally: + dns.rdata._allow_relative_comparisons = saved + + def test_relative_vs_relative_compare_strict(self): + try: + saved = dns.rdata._allow_relative_comparisons + dns.rdata._allow_relative_comparisons = False + r1 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'www') + r2 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'xxx') + self.assertFalse(r1 == r2) + self.assertTrue(r1 != r2) + def bad1(): + r1 < r2 + def bad2(): + r1 <= r2 + def bad3(): + r1 > r2 + def bad4(): + r1 >= r2 + self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad1) + self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad2) + self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad3) + self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad4) + finally: + dns.rdata._allow_relative_comparisons = saved class UtilTestCase(unittest.TestCase): diff --git a/tests/test_zonedigest.py b/tests/test_zonedigest.py index f98e5f7..d94be24 100644 --- a/tests/test_zonedigest.py +++ b/tests/test_zonedigest.py @@ -176,3 +176,18 @@ class ZoneDigestTestCase(unittest.TestCase): with self.assertRaises(dns.exception.SyntaxError): dns.rdata.from_text('IN', 'ZONEMD', '100 1 0 ' + self.sha384_hash) + sorting_zone = textwrap.dedent(''' + @ 86400 IN SOA ns1 admin 2018031900 ( + 1800 900 604800 86400 ) + 86400 IN NS ns1 + 86400 IN NS ns2 + 86400 IN RP n1.example. a. + 86400 IN RP n1. b. + ''') + + def test_relative_zone_sorting(self): + z1 = dns.zone.from_text(self.sorting_zone, 'example.', relativize=True) + z2 = dns.zone.from_text(self.sorting_zone, 'example.', relativize=False) + zmd1 = z1.compute_digest(dns.zone.DigestHashAlgorithm.SHA384) + zmd2 = z2.compute_digest(dns.zone.DigestHashAlgorithm.SHA384) + self.assertEqual(zmd1, zmd2) |