From: Bob Halley Date: Sat, 21 Oct 2023 13:38:54 +0000 (-0700) Subject: Check that a relative name plus the zone's origin is not too long. (#997) X-Git-Tag: v2.5.0rc1~40 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1957961b31725209528f5efce6daa8a5047ab33c;p=thirdparty%2Fdnspython.git Check that a relative name plus the zone's origin is not too long. (#997) Previously it was possible to add very long relative names to a relative zone which could never be rendered due to being too long for wire format. Now we check this as part of _validate_name(). This code also removes duplicated name validation code from Zone and Version, consolidating it into one helper function. Finally, we fix a few comments in get methods that have cut-and-paste typos from the find variant indicating they can raise KeyError when they cannot. --- diff --git a/dns/zone.py b/dns/zone.py index 9e763f5f..03d6f1a8 100644 --- a/dns/zone.py +++ b/dns/zone.py @@ -82,6 +82,38 @@ class DigestVerificationFailure(dns.exception.DNSException): """The ZONEMD digest failed to verify.""" +def _validate_name( + name: dns.name.Name, + origin: Optional[dns.name.Name], + relativize: bool, +) -> dns.name.Name: + # This name validation code is shared by Zone and Version + if origin is None: + # This should probably never happen as other code (e.g. + # _rr_line) will notice the lack of an origin before us, but + # we check just in case! + raise KeyError("no zone origin is defined") + if name.is_absolute(): + if not name.is_subdomain(origin): + raise KeyError("name parameter must be a subdomain of the zone origin") + if relativize: + name = name.relativize(origin) + else: + # We have a relative name. Make sure that the derelativized name is + # not too long. + try: + abs_name = name.derelativize(origin) + except dns.name.NameTooLong: + # We map dns.name.NameTooLong to KeyError to be consistent with + # the other exceptions above. + raise KeyError("relative name too long for zone") + if not relativize: + # We have a relative name in a non-relative zone, so use the + # derelativized name. + name = abs_name + return name + + class Zone(dns.transaction.TransactionManager): """A DNS zone. @@ -154,26 +186,13 @@ class Zone(dns.transaction.TransactionManager): return not self.__eq__(other) def _validate_name(self, name: Union[dns.name.Name, str]) -> dns.name.Name: + # Note that any changes in this method should have corresponding changes + # made in the Version _validate_name() method. if isinstance(name, str): name = dns.name.from_text(name, None) elif not isinstance(name, dns.name.Name): raise KeyError("name parameter must be convertible to a DNS name") - if name.is_absolute(): - if self.origin is None: - # This should probably never happen as other code (e.g. - # _rr_line) will notice the lack of an origin before us, but - # we check just in case! - raise KeyError("no zone origin is defined") - if not name.is_subdomain(self.origin): - raise KeyError("name parameter must be a subdomain of the zone origin") - if self.relativize: - name = name.relativize(self.origin) - elif not self.relativize: - # We have a relative name in a non-relative zone, so derelativize. - if self.origin is None: - raise KeyError("no zone origin is defined") - name = name.derelativize(self.origin) - return name + return _validate_name(name, self.origin, self.relativize) def __getitem__(self, key): key = self._validate_name(key) @@ -252,9 +271,6 @@ class Zone(dns.transaction.TransactionManager): *create*, a ``bool``. If true, the node will be created if it does not exist. - Raises ``KeyError`` if the name is not known and create was - not specified, or if the name was not a subdomain of the origin. - Returns a ``dns.node.Node`` or ``None``. """ @@ -527,9 +543,6 @@ class Zone(dns.transaction.TransactionManager): *create*, a ``bool``. If true, the node will be created if it does not exist. - Raises ``KeyError`` if the name is not known and create was - not specified, or if the name was not a subdomain of the origin. - Returns a ``dns.rrset.RRset`` or ``None``. """ @@ -964,22 +977,7 @@ class Version: self.origin = origin def _validate_name(self, name: dns.name.Name) -> dns.name.Name: - if name.is_absolute(): - if self.origin is None: - # This should probably never happen as other code (e.g. - # _rr_line) will notice the lack of an origin before us, but - # we check just in case! - raise KeyError("no zone origin is defined") - if not name.is_subdomain(self.origin): - raise KeyError("name is not a subdomain of the zone origin") - if self.zone.relativize: - name = name.relativize(self.origin) - elif not self.zone.relativize: - # We have a relative name in a non-relative zone, so derelativize. - if self.origin is None: - raise KeyError("no zone origin is defined") - name = name.derelativize(self.origin) - return name + return _validate_name(name, self.origin, self.zone.relativize) def get_node(self, name: dns.name.Name) -> Optional[dns.node.Node]: name = self._validate_name(name) diff --git a/tests/test_zone.py b/tests/test_zone.py index eca81e01..a9029674 100644 --- a/tests/test_zone.py +++ b/tests/test_zone.py @@ -16,11 +16,11 @@ # ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT # OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -from io import BytesIO, StringIO import difflib import os import sys import unittest +from io import BytesIO, StringIO from typing import cast import dns.exception @@ -28,14 +28,12 @@ import dns.message import dns.name import dns.node import dns.rdata -import dns.rdataset import dns.rdataclass +import dns.rdataset import dns.rdatatype import dns.rrset import dns.versioned import dns.zone -import dns.node - from tests.util import here example_text = """$TTL 3600 @@ -1158,6 +1156,21 @@ class ZoneTestCase(unittest.TestCase): with self.assertRaises(KeyError): self.assertTrue(1 in z) + def testRelativeNameLengthChecks(self): + z = dns.zone.from_text(example_cname, "example.", relativize=True) + rds = dns.rdataset.from_text("in", "a", 300, "10.0.0.1") + # This name is 246 bytes long, which along with the 8 bytes for label "example" + # and 1 byte for the root name is 246 + 8 + 1 = 255 bytes long, the maximum + # legal wire format name length. + ok_long_relative = dns.name.Name(["a" * 63, "a" * 63, "a" * 63, "a" * 53]) + z.replace_rdataset(ok_long_relative, rds) + self.assertEqual(z.find_rdataset(ok_long_relative, "A"), rds) + # This is the longest possible relative name and won't work in any zone + # as there is no space left for any origin labels. + too_long_relative = dns.name.Name(["a" * 63, "a" * 63, "a" * 63, "a" * 62]) + with self.assertRaises(KeyError): + z.replace_rdataset(too_long_relative, rds) + class VersionedZoneTestCase(unittest.TestCase): def testUseTransaction(self):