Fix three problems with DNSSEC: (#946)

* Fix three problems with DNSSEC:

1) Signing a relative zone didn't quite work.
2) The signer generated the wrong RRSIG labels length for a wild name.
3) The validator failed to detect 2).

* fix issues detected by mypy

diff --git a/dns/dnssec.py b/dns/dnssec.py
index 13b731339d2dad08a2c18e7c62dad26a39a18f69..55fd7b57d4f98fee07337e22ff13a60ce9869e4d 100644 (file)
--- a/dns/dnssec.py
+++ b/dns/dnssec.py
@@ -682,6 +682,7 @@ def _sign(
     lifetime: Optional[int] = None,
     verify: bool = False,
     policy: Optional[Policy] = None,
+    origin: Optional[dns.name.Name] = None,
 ) -> RRSIG:
     """Sign RRset using private key.
 
@@ -717,6 +718,10 @@ def _sign(
     *policy*, a ``dns.dnssec.Policy`` or ``None``.  If ``None``, the default policy,
     ``dns.dnssec.default_policy`` is used; this policy defaults to that of RFC 8624.
 
+    *origin*, a ``dns.name.Name`` or ``None``.  If ``None``, the default, then all
+    names in the rrset (including its owner name) must be absolute; otherwise the
+    specified origin will be used to make names absolute when signing.
+
     Raises ``DeniedByPolicy`` if the signature is denied by policy.
     """
 
@@ -748,12 +753,22 @@ def _sign(
     else:
         raise ValueError("expiration or lifetime must be specified")
 
+    # Derelativize now because we need a correct labels length for the
+    # rrsig_template.
+    if origin is not None:
+        rrname = rrname.derelativize(origin)
+    labels = len(rrname) - 1
+
+    # Adjust labels appropriately for wildcards.
+    if rrname.is_wild():
+        labels -= 1
+
     rrsig_template = RRSIG(
         rdclass=rdclass,
         rdtype=dns.rdatatype.RRSIG,
         type_covered=rdtype,
         algorithm=dnskey.algorithm,
-        labels=len(rrname) - 1,
+        labels=labels,
         original_ttl=original_ttl,
         expiration=rrsig_expiration,
         inception=rrsig_inception,
@@ -762,7 +777,7 @@ def _sign(
         signature=b"",
     )
 
-    data = dns.dnssec._make_rrsig_signature_data(rrset, rrsig_template)
+    data = dns.dnssec._make_rrsig_signature_data(rrset, rrsig_template, origin)
     chosen_hash = _make_hash(rrsig_template.algorithm)
     signature = None
 
@@ -867,9 +882,12 @@ def _make_rrsig_signature_data(
             raise ValidationFailure("relative RR name without an origin specified")
         rrname = rrname.derelativize(origin)
 
-    if len(rrname) - 1 < rrsig.labels:
+    name_len = len(rrname)
+    if rrname.is_wild() and rrsig.labels != name_len - 2:
+        raise ValidationFailure("wild owner name has wrong label length")
+    if name_len - 1 < rrsig.labels:
         raise ValidationFailure("owner name longer than RRSIG labels")
-    elif rrsig.labels < len(rrname) - 1:
+    elif rrsig.labels < name_len - 1:
         suffix = rrname.split(rrsig.labels + 1)[1]
         rrname = dns.name.from_text("*", suffix)
     rrnamebuf = rrname.to_digestable()
@@ -1235,6 +1253,7 @@ def default_rrset_signer(
     expiration: Optional[Union[datetime, str, int, float]] = None,
     lifetime: Optional[int] = None,
     policy: Optional[Policy] = None,
+    origin: Optional[dns.name.Name] = None,
 ) -> None:
     """Default RRset signer"""
 
@@ -1259,6 +1278,7 @@ def default_rrset_signer(
             lifetime=lifetime,
             signer=signer,
             policy=policy,
+            origin=origin,
         )
         txn.add(rrset.name, rrset.ttl, rrsig)
 
@@ -1364,6 +1384,7 @@ def sign_zone(
                 expiration=expiration,
                 lifetime=lifetime,
                 policy=policy,
+                origin=zone.origin,
             )
             return _sign_zone_nsec(zone, _txn, _rrset_signer)
 
@@ -1436,7 +1457,8 @@ def _sign_zone_nsec(
                         rrset = dns.rrset.from_rdata(name, rdataset.ttl, *rdataset)
                         rrset_signer(txn, rrset)
 
-        if last_secure:
+        # We need "is not None" as the empty name is False because its length is 0.
+        if last_secure is not None:
             _txn_add_nsec(txn, last_secure, name, zone.rdclass, rrsig_ttl, rrset_signer)
         last_secure = name
 

diff --git a/tests/test_dnssec.py b/tests/test_dnssec.py
index 3074741f4f394ed6bfa555f8b0a5bf7cb567537c..7177dcdabbd6f6587ce8956aab140545a4bd6494 100644 (file)
--- a/tests/test_dnssec.py
+++ b/tests/test_dnssec.py
@@ -824,6 +824,26 @@ class DNSSECValidatorTestCase(unittest.TestCase):
                 com_txt, com_txt_rrsig[0], wildcard_keys, None, wildcard_when
             )
 
+        # check some bogus label lengths
+        a_name = dns.name.from_text("a.example.com")
+        a_txt = clone_rrset(wildcard_txt, a_name)
+        a_txt_rrsig = clone_rrset(wildcard_txt_rrsig, a_name)
+        bad_rrsig = a_txt_rrsig[0].replace(labels=99)
+        with self.assertRaises(dns.dnssec.ValidationFailure):
+            dns.dnssec.validate_rrsig(
+                a_txt, bad_rrsig, wildcard_keys, None, wildcard_when
+            )
+        bad_rrsig = a_txt_rrsig[0].replace(labels=3)
+        with self.assertRaises(dns.dnssec.ValidationFailure):
+            dns.dnssec.validate_rrsig(
+                a_txt, bad_rrsig, wildcard_keys, None, wildcard_when
+            )
+        bad_rrsig = a_txt_rrsig[0].replace(labels=1)
+        with self.assertRaises(dns.dnssec.ValidationFailure):
+            dns.dnssec.validate_rrsig(
+                a_txt, bad_rrsig, wildcard_keys, None, wildcard_when
+            )
+
     def testAlternateParameterFormats(self):  # type: () -> None
         # Pass rrset and rrsigset as (name, rdataset) tuples, not rrsets
         rrset = (abs_soa.name, abs_soa.to_rdataset())
@@ -936,8 +956,10 @@ class DNSSECMiscTestCase(unittest.TestCase):
         ts = dns.dnssec.to_timestamp(441812220)
         self.assertEqual(ts, REFERENCE_TIMESTAMP)
 
-    def test_sign_zone(self):
-        zone = dns.zone.from_text(test_zone_sans_nsec, "example.", relativize=False)
+    def do_test_sign_zone(self, relativize):
+        zone = dns.zone.from_text(
+            test_zone_sans_nsec, "example.", relativize=relativize
+        )
 
         algorithm = dns.dnssec.Algorithm.ED25519
         lifetime = 3600
@@ -966,9 +988,10 @@ class DNSSECMiscTestCase(unittest.TestCase):
                 lifetime=lifetime,
             )
 
+        print(zone.to_text())
         rrsigs = set(
             [
-                (str(name), rdataset.covers)
+                (str(name.derelativize(zone.origin)), rdataset.covers)
                 for (name, rdataset) in zone.iterate_rdatasets()
                 if rdataset.rdtype == dns.rdatatype.RRSIG
             ]
@@ -977,7 +1000,11 @@ class DNSSECMiscTestCase(unittest.TestCase):
 
         signers = set(
             [
-                (str(name), rdataset.covers, rdataset[0].key_tag)
+                (
+                    str(name.derelativize(zone.origin)),
+                    rdataset.covers,
+                    rdataset[0].key_tag,
+                )
                 for (name, rdataset) in zone.iterate_rdatasets()
                 if rdataset.rdtype == dns.rdatatype.RRSIG
             ]
@@ -992,6 +1019,12 @@ class DNSSECMiscTestCase(unittest.TestCase):
             else:
                 self.assertEqual(key_tag, dns.dnssec.key_id(zsk_dnskey))
 
+    def test_sign_zone_absolute(self):
+        self.do_test_sign_zone(False)
+
+    def test_sign_zone_relative(self):
+        self.do_test_sign_zone(True)
+
     def test_sign_zone_nsec_null_signer(self):
         def rrset_signer(
             txn: dns.transaction.Transaction,
@@ -1334,6 +1367,14 @@ class DNSSECSignatureTestCase(unittest.TestCase):
         rrset = (name, rdataset)
         self._test_signature(key, dns.dnssec.Algorithm.ED448, rrset)
 
+    def testSignWildRdataset(self):  # type: () -> None
+        key = ed448.Ed448PrivateKey.generate()
+        name = dns.name.from_text("*.example.com")
+        rdataset = dns.rdataset.from_text_list("in", "a", 30, ["10.0.0.1", "10.0.0.2"])
+        rrset = (name, rdataset)
+        rrsigset = self._test_signature(key, dns.dnssec.Algorithm.ED448, rrset)
+        self.assertEqual(rrsigset[0].labels, 2)
+
     def _test_signature(self, key, algorithm, rrset, signer=None, policy=None):
         ttl = 60
         lifetime = 3600
@@ -1358,6 +1399,7 @@ class DNSSECSignatureTestCase(unittest.TestCase):
         keys = {signer: dnskey_rrset}
         rrsigset = dns.rrset.from_rdata(rrname, ttl, rrsig)
         dns.dnssec.validate(rrset=rrset, rrsigset=rrsigset, keys=keys, policy=policy)
+        return rrsigset
 
 
 if __name__ == "__main__":