]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Fix intermittent kasp pytest failures
authorMatthijs Mekking <matthijs@isc.org>
Wed, 7 May 2025 15:17:27 +0000 (17:17 +0200)
committerNicki Křížek <nicki@isc.org>
Thu, 29 May 2025 11:44:56 +0000 (11:44 +0000)
The pytest cases checks if a zone is signed by looking at the NSEC
record at the apex. If that has an RRSIG record, it is considered
signed. But 'named' signs zones incrementally (in batches) and so
the zone may still lack some signatures. In other words, the tests
may consider a zone signed while in fact signing is not yet complete,
then performs additional checks such as is a subdomain signed with the
right key. If this check happens before the zone is actually fully
signed, the check will fail.

Fix this by using 'check_dnssec_verify' instead of
'check_is_zone_signed'. We were already doing this check, but we now
move it up. This will transfer the zone and then run 'dnssec-verify'
on the response. If the zone is partially signed, the check will fail,
and it will retry for up to ten times.

bin/tests/system/isctest/kasp.py
bin/tests/system/kasp/tests_kasp.py
bin/tests/system/ksr/tests_ksr.py

index 51bf22f0897226cedfbcbc6004a33a670aba11f3..d6bd930d87745955548ebf8cbee7b1e36c7f7ca8 100644 (file)
@@ -611,57 +611,6 @@ class Key:
         return self.path
 
 
-def check_zone_is_signed(server, zone, tsig=None):
-    addr = server.ip
-    fqdn = f"{zone}."
-
-    # wait until zone is fully signed
-    signed = False
-    for _ in range(10):
-        response = _query(server, fqdn, dns.rdatatype.NSEC, tsig=tsig)
-        if not isinstance(response, dns.message.Message):
-            isctest.log.debug(f"no response for {fqdn} NSEC from {addr}")
-        elif response.rcode() != dns.rcode.NOERROR:
-            rcode = dns.rcode.to_text(response.rcode())
-            isctest.log.debug(f"{rcode} response for {fqdn} NSEC from {addr}")
-        else:
-            has_nsec = False
-            has_rrsig = False
-            for rr in response.answer:
-                if not has_nsec:
-                    has_nsec = rr.match(
-                        dns.name.from_text(fqdn),
-                        dns.rdataclass.IN,
-                        dns.rdatatype.NSEC,
-                        dns.rdatatype.NONE,
-                    )
-                if not has_rrsig:
-                    has_rrsig = rr.match(
-                        dns.name.from_text(fqdn),
-                        dns.rdataclass.IN,
-                        dns.rdatatype.RRSIG,
-                        dns.rdatatype.NSEC,
-                    )
-
-            if not has_nsec:
-                isctest.log.debug(
-                    f"missing apex {fqdn} NSEC record in response from {addr}"
-                )
-            if not has_rrsig:
-                isctest.log.debug(
-                    f"missing {fqdn} NSEC signature in response from {addr}"
-                )
-
-            signed = has_nsec and has_rrsig
-
-        if signed:
-            break
-
-        time.sleep(1)
-
-    assert signed
-
-
 def check_keys(zone, keys, expected):
     """
     Checks keys for a configured zone. This verifies:
index 178e602f48304a312daae49c06af4257e1eba9f9..6cbb87080b228b5c198967103d7fce3393719535 100644 (file)
@@ -166,7 +166,6 @@ def check_all(server, zone, policy, ksks, zsks, zsk_missing=False, tsig=None):
         server, zone, ksks, zsks, zsk_missing=zsk_missing, tsig=tsig
     )
     isctest.kasp.check_subdomain(server, zone, ksks, zsks, tsig=tsig)
-    isctest.kasp.check_dnssec_verify(server, zone, tsig=tsig)
 
 
 def set_keytimes_default_policy(kp):
@@ -661,7 +660,7 @@ def test_kasp_case(servers, params):
     isctest.log.info(f"check test case zone {zone} policy {policy}")
 
     # First make sure the zone is signed.
-    isctest.kasp.check_zone_is_signed(server, zone)
+    isctest.kasp.check_dnssec_verify(server, zone)
 
     # Key properties.
     expected = isctest.kasp.policy_to_properties(ttl=ttl, keys=params["key-properties"])
@@ -679,6 +678,7 @@ def test_kasp_case(servers, params):
         ksks = [k for k in keys if k.is_ksk()]
         zsks = [k for k in keys if not k.is_ksk()]
 
+    isctest.kasp.check_dnssec_verify(server, zone)
     isctest.kasp.check_keys(zone, keys, expected)
 
     offset = params["offset"] if "offset" in params else None
@@ -759,7 +759,7 @@ def test_kasp_inherit_signed(zone, policy, server_id, alg, tsig_kind, servers):
     key1.metadata["Length"] = alg.bits
     keys = isctest.kasp.keydir_to_keylist(zone, server.identifier)
 
-    isctest.kasp.check_zone_is_signed(server, zone, tsig=tsig)
+    isctest.kasp.check_dnssec_verify(server, zone, tsig=tsig)
     isctest.kasp.check_keys(zone, keys, [key1])
     set_keytimes_default_policy(key1)
     isctest.kasp.check_keytimes(keys, [key1])
@@ -786,7 +786,7 @@ def test_kasp_inherit_view(number, dynamic, inline_signing, txt_rdata, servers):
     key1.metadata["Length"] = ECDSAP384SHA384.bits
     keys = isctest.kasp.keydir_to_keylist(zone, server.identifier)
 
-    isctest.kasp.check_zone_is_signed(server, zone, tsig=tsig)
+    isctest.kasp.check_dnssec_verify(server, zone, tsig=tsig)
     isctest.kasp.check_keys(zone, keys, [key1])
     set_keytimes_default_policy(key1)
     isctest.kasp.check_keytimes(keys, [key1])
@@ -839,7 +839,7 @@ def test_kasp_default(servers):
     ]
     expected = isctest.kasp.policy_to_properties(ttl=3600, keys=keyprops)
     keys = isctest.kasp.keydir_to_keylist(zone, "ns3")
-    isctest.kasp.check_zone_is_signed(server, zone)
+    isctest.kasp.check_dnssec_verify(server, zone)
     isctest.kasp.check_keys(zone, keys, expected)
     set_keytimes_default_policy(expected[0])
     isctest.kasp.check_keytimes(keys, expected)
@@ -900,6 +900,7 @@ def test_kasp_default(servers):
         watcher.wait_for_line(f"zone {zone}/IN (signed): {expectmsg}")
     # Nothing has changed.
     expected[0].properties["private"] = False
+    isctest.kasp.check_dnssec_verify(server, zone)
     isctest.kasp.check_keys(zone, keys, expected)
     isctest.kasp.check_keytimes(keys, expected)
     check_all(server, zone, policy, keys, [])
@@ -911,7 +912,7 @@ def test_kasp_default(servers):
     key1 = KeyProperties.default()
     keys = isctest.kasp.keydir_to_keylist(zone, "ns3")
     expected = [key1]
-    isctest.kasp.check_zone_is_signed(server, zone)
+    isctest.kasp.check_dnssec_verify(server, zone)
     isctest.kasp.check_keys(zone, keys, expected)
     set_keytimes_default_policy(key1)
     isctest.kasp.check_keytimes(keys, expected)
@@ -930,7 +931,7 @@ def test_kasp_dynamic(servers):
     key1 = KeyProperties.default()
     expected = [key1]
     keys = isctest.kasp.keydir_to_keylist(zone, "ns3")
-    isctest.kasp.check_zone_is_signed(server, zone)
+    isctest.kasp.check_dnssec_verify(server, zone)
     isctest.kasp.check_keys(zone, keys, expected)
     set_keytimes_default_policy(key1)
     expected = [key1]
@@ -1014,7 +1015,7 @@ def test_kasp_dynamic(servers):
     key1 = KeyProperties.default()
     expected = [key1]
     keys = isctest.kasp.keydir_to_keylist(zone, "ns3")
-    isctest.kasp.check_zone_is_signed(server, zone)
+    isctest.kasp.check_dnssec_verify(server, zone)
     isctest.kasp.check_keys(zone, keys, expected)
     set_keytimes_default_policy(key1)
     expected = [key1]
@@ -1053,7 +1054,7 @@ def test_kasp_dynamic(servers):
     key1.metadata["DSState"] = "omnipresent"
     expected = [key1]
     keys = isctest.kasp.keydir_to_keylist(zone, "ns3/keys")
-    isctest.kasp.check_zone_is_signed(server, zone)
+    isctest.kasp.check_dnssec_verify(server, zone)
     isctest.kasp.check_keys(zone, keys, expected)
     check_all(server, zone, policy, keys, [])
     # Ensure no zone_resigninc for the unsigned version of the zone is triggered.
@@ -1079,7 +1080,7 @@ def test_kasp_checkds(servers):
     keys = isctest.kasp.keydir_to_keylist(zone, "ns3")
     ksks = [k for k in keys if k.is_ksk()]
     zsks = [k for k in keys if k.is_zsk()]
-    isctest.kasp.check_zone_is_signed(server, zone)
+    isctest.kasp.check_dnssec_verify(server, zone)
     isctest.kasp.check_keys(zone, keys, expected)
     check_all(server, zone, policy, ksks, zsks)
 
@@ -1123,7 +1124,7 @@ def test_kasp_checkds_doubleksk(servers):
     keys = isctest.kasp.keydir_to_keylist(zone, "ns3")
     ksks = [k for k in keys if k.is_ksk()]
     zsks = [k for k in keys if k.is_zsk()]
-    isctest.kasp.check_zone_is_signed(server, zone)
+    isctest.kasp.check_dnssec_verify(server, zone)
     isctest.kasp.check_keys(zone, keys, expected)
     check_all(server, zone, policy, ksks, zsks)
 
@@ -1196,7 +1197,7 @@ def test_kasp_checkds_csk(servers):
     ]
     expected = isctest.kasp.policy_to_properties(ttl=303, keys=policy_keys)
     keys = isctest.kasp.keydir_to_keylist(zone, "ns3")
-    isctest.kasp.check_zone_is_signed(server, zone)
+    isctest.kasp.check_dnssec_verify(server, zone)
     isctest.kasp.check_keys(zone, keys, expected)
     check_all(server, zone, policy, keys, [])
 
@@ -1482,7 +1483,7 @@ def test_kasp_zsk_retired(servers):
     keys = isctest.kasp.keydir_to_keylist(zone, "ns3")
     ksks = [k for k in keys if k.is_ksk()]
     zsks = [k for k in keys if not k.is_ksk()]
-    isctest.kasp.check_zone_is_signed(server, zone)
+    isctest.kasp.check_dnssec_verify(server, zone)
     isctest.kasp.check_keys(zone, keys, expected)
 
     offset = -timedelta(days=30 * 6)
index 15ccc02b28a68fd681a72c94a5ed97687159d8d6..a5aefa0b219fb036c74a58ae2709d29448761d69 100644 (file)
@@ -756,8 +756,6 @@ def test_ksr_common(servers):
     # test zone is correctly signed
     # - check rndc dnssec -status output
     isctest.kasp.check_dnssecstatus(ns1, zone, overlapping_zsks, policy=policy)
-    # - zone is signed
-    isctest.kasp.check_zone_is_signed(ns1, zone)
     # - dnssec_verify
     isctest.kasp.check_dnssec_verify(ns1, zone)
     # - check keys
@@ -831,8 +829,6 @@ def test_ksr_lastbundle(servers):
     # test zone is correctly signed
     # - check rndc dnssec -status output
     isctest.kasp.check_dnssecstatus(ns1, zone, zsks, policy=policy)
-    # - zone is signed
-    isctest.kasp.check_zone_is_signed(ns1, zone)
     # - dnssec_verify
     isctest.kasp.check_dnssec_verify(ns1, zone)
     # - check keys
@@ -911,8 +907,6 @@ def test_ksr_inthemiddle(servers):
     # test zone is correctly signed
     # - check rndc dnssec -status output
     isctest.kasp.check_dnssecstatus(ns1, zone, zsks, policy=policy)
-    # - zone is signed
-    isctest.kasp.check_zone_is_signed(ns1, zone)
     # - dnssec_verify
     isctest.kasp.check_dnssec_verify(ns1, zone)
     # - check keys
@@ -1106,8 +1100,6 @@ def test_ksr_unlimited(servers):
     # test zone is correctly signed
     # - check rndc dnssec -status output
     isctest.kasp.check_dnssecstatus(ns1, zone, zsks, policy=policy)
-    # - zone is signed
-    isctest.kasp.check_zone_is_signed(ns1, zone)
     # - dnssec_verify
     isctest.kasp.check_dnssec_verify(ns1, zone)
     # - check keys
@@ -1218,8 +1210,6 @@ def test_ksr_twotone(servers):
     # test zone is correctly signed
     # - check rndc dnssec -status output
     isctest.kasp.check_dnssecstatus(ns1, zone, zsks, policy=policy)
-    # - zone is signed
-    isctest.kasp.check_zone_is_signed(ns1, zone)
     # - dnssec_verify
     isctest.kasp.check_dnssec_verify(ns1, zone)
     # - check keys
@@ -1298,8 +1288,6 @@ def test_ksr_kskroll(servers):
     # test zone is correctly signed
     # - check rndc dnssec -status output
     isctest.kasp.check_dnssecstatus(ns1, zone, zsks, policy=policy)
-    # - zone is signed
-    isctest.kasp.check_zone_is_signed(ns1, zone)
     # - dnssec_verify
     isctest.kasp.check_dnssec_verify(ns1, zone)
     # - check keys