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 9e763f5f0cee92b99f397af83969fb2da8e28745..03d6f1a8b0f1296fbebd5d2fd6ec75b6adc9db93 100644 (file)
--- 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 eca81e017f43d1beda72c3d93e4937d3d1c0a58b..a902967411c746bd332b090173a7d3585075cace 100644 (file)
--- 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):