From a1f072cfec931902097f781948aa0cc8165200b4 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 14 Mar 2025 12:19:36 +0100
Subject: [PATCH 1/6] Convert kasp dnssectools tests to pytest

Convert the first couple of tests from 'kasp/tests.sh' to
'kasp/tests_kasp.py', those are test cases related to 'dnssec-keygen'
and 'dnssec-settime'.

For this, we also add a new KeyProperties method,
'policy_to_properties', that takes a list of strings which represent
the keys according to the dnssec-policy and the expected key states.

(cherry picked from commit 00ea2c256482f334e3bd9ed52f20cc3a6a53660f)
---
 bin/tests/system/isctest/kasp.py    |  69 ++++++++
 bin/tests/system/kasp/tests.sh      | 172 ------------------
 bin/tests/system/kasp/tests_kasp.py | 264 ++++++++++++++++++++++++++++
 3 files changed, 333 insertions(+), 172 deletions(-)
 create mode 100644 bin/tests/system/kasp/tests_kasp.py

diff --git a/bin/tests/system/isctest/kasp.py b/bin/tests/system/isctest/kasp.py
index 1b82baf416..e4b3ead5cc 100644
--- a/bin/tests/system/isctest/kasp.py
+++ b/bin/tests/system/isctest/kasp.py
@@ -1101,3 +1101,72 @@ def keydir_to_keylist(
 
 def keystr_to_keylist(keystr: str, keydir: Optional[str] = None) -> List[Key]:
     return [Key(name, keydir) for name in keystr.split()]
+
+
+def policy_to_properties(ttl, keys: List[str]) -> List[KeyProperties]:
+    """
+    Get the policies from a list of specially formatted strings.
+    The splitted line should result in the following items:
+    line[0]: Role
+    line[1]: Lifetime
+    line[2]: Algorithm
+    line[3]: Length
+    Then, optional data for specific tests may follow:
+    - "goal", "dnskey", "krrsig", "zrrsig", "ds", followed by a value,
+      sets the given state to the specific value
+    - "offset", an offset for testing key rollover timings
+    """
+    proplist = []
+    count = 0
+    for key in keys:
+        count += 1
+        line = key.split()
+        keyprop = KeyProperties(f"KEY{count}", {}, {}, {})
+        keyprop.properties["expect"] = True
+        keyprop.properties["private"] = True
+        keyprop.properties["legacy"] = False
+        keyprop.properties["offset"] = timedelta(0)
+        keyprop.properties["role"] = line[0]
+        if line[0] == "zsk":
+            keyprop.properties["role_full"] = "zone-signing"
+            keyprop.properties["flags"] = 256
+            keyprop.metadata["ZSK"] = "yes"
+            keyprop.metadata["KSK"] = "no"
+        else:
+            keyprop.properties["role_full"] = "key-signing"
+            keyprop.properties["flags"] = 257
+            keyprop.metadata["ZSK"] = "yes" if line[0] == "csk" else "no"
+            keyprop.metadata["KSK"] = "yes"
+
+        keyprop.properties["dnskey_ttl"] = ttl
+        keyprop.metadata["Algorithm"] = line[2]
+        keyprop.metadata["Length"] = line[3]
+        keyprop.metadata["Lifetime"] = 0
+        if line[1] != "unlimited":
+            keyprop.metadata["Lifetime"] = int(line[1])
+
+        for i in range(4, len(line)):
+            if line[i].startswith("goal:"):
+                keyval = line[i].split(":")
+                keyprop.metadata["GoalState"] = keyval[1]
+            elif line[i].startswith("dnskey:"):
+                keyval = line[i].split(":")
+                keyprop.metadata["DNSKEYState"] = keyval[1]
+            elif line[i].startswith("krrsig:"):
+                keyval = line[i].split(":")
+                keyprop.metadata["KRRSIGState"] = keyval[1]
+            elif line[i].startswith("zrrsig:"):
+                keyval = line[i].split(":")
+                keyprop.metadata["ZRRSIGState"] = keyval[1]
+            elif line[i].startswith("ds:"):
+                keyval = line[i].split(":")
+                keyprop.metadata["DSState"] = keyval[1]
+            elif line[i].startswith("offset:"):
+                keyval = line[i].split(":")
+                keyprop.properties["offset"] = timedelta(seconds=int(keyval[1]))
+            else:
+                assert False, f"undefined optional data {line[i]}"
+
+        proplist.append(keyprop)
+
+    return proplist
diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh
index cdac5b7a26..bc41796d56 100644
--- a/bin/tests/system/kasp/tests.sh
+++ b/bin/tests/system/kasp/tests.sh
@@ -54,178 +54,6 @@ next_key_event_threshold=100
 # Tests                                                                       #
 ###############################################################################
 
-#
-# dnssec-keygen
-#
-set_zone "kasp"
-set_policy "kasp" "4" "200"
-set_server "keys" "10.53.0.1"
-
-n=$((n + 1))
-echo_i "check that 'dnssec-keygen -k' (configured policy) creates valid files ($n)"
-ret=0
-$KEYGEN -K keys -k "$POLICY" -l kasp.conf "$ZONE" >"keygen.out.$POLICY.test$n" 2>/dev/null || ret=1
-lines=$(wc -l <"keygen.out.$POLICY.test$n")
-test "$lines" -eq $NUM_KEYS || log_error "wrong number of keys created for policy kasp: $lines"
-# Temporarily don't log errors because we are searching multiple files.
-disable_logerror
-
-# Key properties.
-set_keyrole "KEY1" "csk"
-set_keylifetime "KEY1" "31536000"
-set_keyalgorithm "KEY1" "13" "ECDSAP256SHA256" "256"
-set_keysigning "KEY1" "yes"
-set_zonesigning "KEY1" "yes"
-
-set_keyrole "KEY2" "ksk"
-set_keylifetime "KEY2" "31536000"
-set_keyalgorithm "KEY2" "8" "RSASHA256" "2048"
-set_keysigning "KEY2" "yes"
-set_zonesigning "KEY2" "no"
-
-set_keyrole "KEY3" "zsk"
-set_keylifetime "KEY3" "2592000"
-set_keyalgorithm "KEY3" "8" "RSASHA256" "2048"
-set_keysigning "KEY3" "no"
-set_zonesigning "KEY3" "yes"
-
-set_keyrole "KEY4" "zsk"
-set_keylifetime "KEY4" "16070400"
-set_keyalgorithm "KEY4" "8" "RSASHA256" "3072"
-set_keysigning "KEY4" "no"
-set_zonesigning "KEY4" "yes"
-
-lines=$(get_keyids "$DIR" "$ZONE" | wc -l)
-test "$lines" -eq $NUM_KEYS || log_error "bad number of key ids"
-status=$((status + ret))
-
-ids=$(get_keyids "$DIR" "$ZONE")
-for id in $ids; do
-  # There are four key files with the same algorithm.
-  # Check them until a match is found.
-  ret=0 && check_key "KEY1" "$id"
-  test "$ret" -eq 0 && continue
-
-  ret=0 && check_key "KEY2" "$id"
-  test "$ret" -eq 0 && continue
-
-  ret=0 && check_key "KEY3" "$id"
-  test "$ret" -eq 0 && continue
-
-  ret=0 && check_key "KEY4" "$id"
-
-  # If ret is still non-zero, non of the files matched.
-  test "$ret" -eq 0 || echo_i "failed"
-  status=$((status + ret))
-done
-# Turn error logs on again.
-enable_logerror
-
-n=$((n + 1))
-echo_i "check that 'dnssec-keygen -k' (default policy) creates valid files ($n)"
-ret=0
-set_zone "kasp"
-set_policy "default" "1" "3600"
-set_server "." "10.53.0.1"
-# Key properties.
-key_clear "KEY1"
-set_keyrole "KEY1" "csk"
-set_keylifetime "KEY1" "0"
-set_keyalgorithm "KEY1" "13" "ECDSAP256SHA256" "256"
-set_keysigning "KEY1" "yes"
-set_zonesigning "KEY1" "yes"
-
-key_clear "KEY2"
-key_clear "KEY3"
-key_clear "KEY4"
-
-$KEYGEN -G -k "$POLICY" "$ZONE" >"keygen.out.$POLICY.test$n" 2>/dev/null || ret=1
-lines=$(wc -l <"keygen.out.$POLICY.test$n")
-test "$lines" -eq $NUM_KEYS || log_error "wrong number of keys created for policy default: $lines"
-# Temporarily adjust max search depth for this test
-MAXDEPTH=1
-ids=$(get_keyids "$DIR" "$ZONE")
-MAXDEPTH=3
-echo_i "found in dir $DIR for zone $ZONE the following keytags: $ids"
-for id in $ids; do
-  check_key "KEY1" "$id"
-  test "$ret" -eq 0 && key_save KEY1
-  check_keytimes
-done
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
-#
-# dnssec-settime
-#
-
-# These test builds upon the latest created key with dnssec-keygen and uses the
-# environment variables BASE_FILE, KEY_FILE, PRIVATE_FILE and STATE_FILE.
-CMP_FILE="${BASE_FILE}.cmp"
-n=$((n + 1))
-echo_i "check that 'dnssec-settime' by default does not edit key state file ($n)"
-ret=0
-cp "$STATE_FILE" "$CMP_FILE"
-$SETTIME -P +3600 "$BASE_FILE" >/dev/null || log_error "settime failed"
-grep "; Publish: " "$KEY_FILE" >/dev/null || log_error "mismatch published in $KEY_FILE"
-grep "Publish: " "$PRIVATE_FILE" >/dev/null || log_error "mismatch published in $PRIVATE_FILE"
-diff "$CMP_FILE" "$STATE_FILE" || log_error "unexpected file change in $STATE_FILE"
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
-n=$((n + 1))
-echo_i "check that 'dnssec-settime -s' also sets publish time metadata and states in key state file ($n)"
-ret=0
-cp "$STATE_FILE" "$CMP_FILE"
-now=$(date +%Y%m%d%H%M%S)
-$SETTIME -s -P "$now" -g "omnipresent" -k "rumoured" "$now" -z "omnipresent" "$now" -r "rumoured" "$now" -d "hidden" "$now" "$BASE_FILE" >/dev/null || log_error "settime failed"
-set_keystate "KEY1" "GOAL" "omnipresent"
-set_keystate "KEY1" "STATE_DNSKEY" "rumoured"
-set_keystate "KEY1" "STATE_KRRSIG" "rumoured"
-set_keystate "KEY1" "STATE_ZRRSIG" "omnipresent"
-set_keystate "KEY1" "STATE_DS" "hidden"
-check_key "KEY1" "$id"
-test "$ret" -eq 0 && key_save KEY1
-set_keytime "KEY1" "PUBLISHED" "${now}"
-check_keytimes
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
-n=$((n + 1))
-echo_i "check that 'dnssec-settime -s' also unsets publish time metadata and states in key state file ($n)"
-ret=0
-cp "$STATE_FILE" "$CMP_FILE"
-$SETTIME -s -P "none" -g "none" -k "none" "$now" -z "none" "$now" -r "none" "$now" -d "none" "$now" "$BASE_FILE" >/dev/null || log_error "settime failed"
-set_keystate "KEY1" "GOAL" "none"
-set_keystate "KEY1" "STATE_DNSKEY" "none"
-set_keystate "KEY1" "STATE_KRRSIG" "none"
-set_keystate "KEY1" "STATE_ZRRSIG" "none"
-set_keystate "KEY1" "STATE_DS" "none"
-check_key "KEY1" "$id"
-test "$ret" -eq 0 && key_save KEY1
-set_keytime "KEY1" "PUBLISHED" "none"
-check_keytimes
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
-n=$((n + 1))
-echo_i "check that 'dnssec-settime -s' also sets active time metadata and states in key state file (uppercase) ($n)"
-ret=0
-cp "$STATE_FILE" "$CMP_FILE"
-now=$(date +%Y%m%d%H%M%S)
-$SETTIME -s -A "$now" -g "HIDDEN" -k "UNRETENTIVE" "$now" -z "UNRETENTIVE" "$now" -r "OMNIPRESENT" "$now" -d "OMNIPRESENT" "$now" "$BASE_FILE" >/dev/null || log_error "settime failed"
-set_keystate "KEY1" "GOAL" "hidden"
-set_keystate "KEY1" "STATE_DNSKEY" "unretentive"
-set_keystate "KEY1" "STATE_KRRSIG" "omnipresent"
-set_keystate "KEY1" "STATE_ZRRSIG" "unretentive"
-set_keystate "KEY1" "STATE_DS" "omnipresent"
-check_key "KEY1" "$id"
-test "$ret" -eq 0 && key_save KEY1
-set_keytime "KEY1" "ACTIVE" "${now}"
-check_keytimes
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
 #
 # named
 #
diff --git a/bin/tests/system/kasp/tests_kasp.py b/bin/tests/system/kasp/tests_kasp.py
new file mode 100644
index 0000000000..55dbf12f89
--- /dev/null
+++ b/bin/tests/system/kasp/tests_kasp.py
@@ -0,0 +1,264 @@
+# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+#
+# SPDX-License-Identifier: MPL-2.0
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0.  If a copy of the MPL was not distributed with this
+# file, you can obtain one at https://mozilla.org/MPL/2.0/.
+#
+# See the COPYRIGHT file distributed with this work for additional
+# information regarding copyright ownership.
+
+import os
+import shutil
+
+from datetime import timedelta
+
+import pytest
+
+import isctest
+from isctest.kasp import (
+    KeyProperties,
+    KeyTimingMetadata,
+)
+
+pytestmark = pytest.mark.extra_artifacts(
+    [
+        "K*.private",
+        "K*.backup",
+        "K*.cmp",
+        "K*.key",
+        "K*.state",
+        "*.created",
+        "dig.out*",
+        "keyevent.out.*",
+        "keygen.out.*",
+        "keys",
+        "published.test*",
+        "python.out.*",
+        "retired.test*",
+        "rndc.dnssec.*.out.*",
+        "rndc.zonestatus.out.*",
+        "rrsig.out.*",
+        "created.key-*",
+        "unused.key-*",
+        "verify.out.*",
+        "zone.out.*",
+        "ns*/K*.private",
+        "ns*/K*.key",
+        "ns*/K*.state",
+        "ns*/*.db",
+        "ns*/*.db.infile",
+        "ns*/*.db.signed",
+        "ns*/*.jbk",
+        "ns*/*.jnl",
+        "ns*/dsset-*",
+        "ns*/keygen.out.*",
+        "ns*/keys",
+        "ns*/ksk",
+        "ns*/ksk/K*",
+        "ns*/zsk",
+        "ns*/zsk",
+        "ns*/zsk/K*",
+        "ns*/named-fips.conf",
+        "ns*/settime.out.*",
+        "ns*/signer.out.*",
+        "ns*/zones",
+        "ns*/policies/*.conf",
+        "ns*/*.zsk1",
+        "ns*/*.zsk2",
+        "ns3/legacy-keys.*",
+        "ns3/dynamic-signed-inline-signing.kasp.db.signed.signed",
+    ]
+)
+
+
+def test_kasp_dnssec_keygen():
+    def keygen(zone, policy, keydir=None):
+        if keydir is None:
+            keydir = "."
+
+        keygen_command = [
+            os.environ.get("KEYGEN"),
+            "-K",
+            keydir,
+            "-k",
+            policy,
+            "-l",
+            "kasp.conf",
+            zone,
+        ]
+
+        return isctest.run.cmd(keygen_command, log_stdout=True).stdout.decode("utf-8")
+
+    # check that 'dnssec-keygen -k' (configured policy) creates valid files.
+    lifetime = {
+        "P1Y": int(timedelta(days=365).total_seconds()),
+        "P30D": int(timedelta(days=30).total_seconds()),
+        "P6M": int(timedelta(days=31*6).total_seconds()),
+    }
+    keyprops = [
+        f"csk {lifetime['P1Y']} 13 256",
+        f"ksk {lifetime['P1Y']} 8 2048",
+        f"zsk {lifetime['P30D']} 8 2048",
+        f"zsk {lifetime['P6M']} 8 3072",
+    ]
+    keydir="keys"
+    out = keygen("kasp", "kasp", keydir)
+    keys = isctest.kasp.keystr_to_keylist(out, keydir)
+    expected = isctest.kasp.policy_to_properties(ttl=200, keys=keyprops)
+    isctest.kasp.check_keys("kasp", keys, expected)
+
+    # check that 'dnssec-keygen -k' (default policy) creates valid files.
+    keyprops = ["csk 0 13 256"]
+    out = keygen("kasp", "default")
+    keys = isctest.kasp.keystr_to_keylist(out)
+    expected = isctest.kasp.policy_to_properties(ttl=3600, keys=keyprops)
+    isctest.kasp.check_keys("kasp", keys, expected)
+
+    # check that 'dnssec-settime' by default does not edit key state file.
+    key = keys[0]
+    shutil.copyfile(key.privatefile, f"{key.privatefile}.backup")
+    shutil.copyfile(key.keyfile, f"{key.keyfile}.backup")
+    shutil.copyfile(key.statefile, f"{key.statefile}.backup")
+
+    created = key.get_timing("Created")
+    publish = key.get_timing("Publish") + timedelta(hours=1)
+    settime = [
+        os.environ.get("SETTIME"),
+        "-P",
+        str(publish),
+        key.path,
+    ]
+    out = isctest.run.cmd(settime, log_stdout=True).stdout.decode("utf-8")
+
+    isctest.check.file_contents_equal(f"{key.statefile}", f"{key.statefile}.backup")
+    assert key.get_metadata("Publish", file=key.privatefile) == str(publish)
+    assert key.get_metadata("Publish", file=key.keyfile, comment=True) == str(publish)
+
+    # check that 'dnssec-settime -s' also sets publish time metadata and
+    # states in key state file.
+    now = KeyTimingMetadata.now()
+    goal = "omnipresent"
+    dnskey = "rumoured"
+    krrsig = "rumoured"
+    zrrsig = "omnipresent"
+    ds = "hidden"
+    keyprops = [
+        f"csk 0 13 256 goal:{goal} dnskey:{dnskey} krrsig:{krrsig} zrrsig:{zrrsig} ds:{ds}",
+    ]
+    expected = isctest.kasp.policy_to_properties(ttl=3600, keys=keyprops)
+    expected[0].timing = {
+        "Generated": created,
+        "Published": now,
+        "Active": created,
+        "DNSKEYChange": now,
+        "KRRSIGChange": now,
+        "ZRRSIGChange": now,
+        "DSChange": now,
+    }
+
+    settime = [
+        os.environ.get("SETTIME"),
+        "-s",
+        "-P",
+        str(now),
+        "-g",
+        goal,
+        "-k",
+        dnskey,
+        str(now),
+        "-r",
+        krrsig,
+        str(now),
+        "-z",
+        zrrsig,
+        str(now),
+        "-d",
+        ds,
+        str(now),
+        key.path,
+    ]
+    out = isctest.run.cmd(settime, log_stdout=True).stdout.decode("utf-8")
+    isctest.kasp.check_keys("kasp", keys, expected)
+    isctest.kasp.check_keytimes(keys, expected)
+
+    # check that 'dnssec-settime -s' also unsets publish time metadata and
+    # states in key state file.
+    now = KeyTimingMetadata.now()
+    keyprops = ["csk 0 13 256"]
+    expected = isctest.kasp.policy_to_properties(ttl=3600, keys=keyprops)
+    expected[0].timing = {
+        "Generated": created,
+        "Active": created,
+    }
+
+    settime = [
+        os.environ.get("SETTIME"),
+        "-s",
+        "-P",
+        "none",
+        "-g",
+        "none",
+        "-k",
+        "none",
+        str(now),
+        "-z",
+        "none",
+        str(now),
+        "-r",
+        "none",
+        str(now),
+        "-d",
+        "none",
+        str(now),
+        key.path,
+    ]
+    out = isctest.run.cmd(settime, log_stdout=True).stdout.decode("utf-8")
+    isctest.kasp.check_keys("kasp", keys, expected)
+    isctest.kasp.check_keytimes(keys, expected)
+
+    # check that 'dnssec-settime -s' also sets active time metadata and states in key state file (uppercase)
+    soon = now + timedelta(hours=2)
+    goal = "hidden"
+    dnskey = "unretentive"
+    krrsig = "omnipresent"
+    zrrsig = "unretentive"
+    ds = "omnipresent"
+    keyprops = [
+        f"csk 0 13 256 goal:{goal} dnskey:{dnskey} krrsig:{krrsig} zrrsig:{zrrsig} ds:{ds}",
+    ]
+    expected = isctest.kasp.policy_to_properties(ttl=3600, keys=keyprops)
+    expected[0].timing = {
+        "Generated": created,
+        "Active": soon,
+        "DNSKEYChange": soon,
+        "KRRSIGChange": soon,
+        "ZRRSIGChange": soon,
+        "DSChange": soon,
+    }
+
+    settime = [
+        os.environ.get("SETTIME"),
+        "-s",
+        "-A",
+        str(soon),
+        "-g",
+        "HIDDEN",
+        "-k",
+        "UNRETENTIVE",
+        str(soon),
+        "-z",
+        "UNRETENTIVE",
+        str(soon),
+        "-r",
+        "OMNIPRESENT",
+        str(soon),
+        "-d",
+        "OMNIPRESENT",
+        str(soon),
+        key.path,
+    ]
+    out = isctest.run.cmd(settime, log_stdout=True).stdout.decode("utf-8")
+    isctest.kasp.check_keys("kasp", keys, expected)
+    isctest.kasp.check_keytimes(keys, expected)

From e1363e8ce970bb99617c90053c8e7c3ecb61f8a7 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 14 Mar 2025 12:52:36 +0100
Subject: [PATCH 2/6] Convert kasp default test cases to pytest

This commit deals with converting the test cases related to the default
dnssec-policy.

This requires a new method 'check_update_is_signed'. This method will
be used in future tests as well, and checks if an expected record is
in the zone and is properly signed.

Remove the counterparts for the newly added test from the kasp shell
tests script.

(cherry picked from commit 4e22b019f5123c27bda7574ae1a5750f27d5bd4b)
---
 bin/tests/system/isctest/kasp.py    |  29 +++++++
 bin/tests/system/kasp/tests.sh      |  86 +-------------------
 bin/tests/system/kasp/tests_kasp.py | 118 ++++++++++++++++++++++++++--
 3 files changed, 143 insertions(+), 90 deletions(-)

diff --git a/bin/tests/system/isctest/kasp.py b/bin/tests/system/isctest/kasp.py
index e4b3ead5cc..9e23b9a6ef 100644
--- a/bin/tests/system/isctest/kasp.py
+++ b/bin/tests/system/isctest/kasp.py
@@ -1020,6 +1020,35 @@ def check_subdomain(server, zone, ksks, zsks, tsig=None):
     check_signatures(rrsigs, qtype, fqdn, ksks, zsks)
 
 
+def verify_update_is_signed(server, fqdn, qname, qtype, rdata, ksks, zsks, tsig=None):
+    """
+    Test an RRset below the apex and verify it is updated and signed correctly.
+    """
+    response = _query(server, qname, qtype, tsig=tsig)
+
+    if response.rcode() != dns.rcode.NOERROR:
+        return False
+
+    rrtype = dns.rdatatype.to_text(qtype)
+    match = f"{qname} {DEFAULT_TTL} IN {rrtype} {rdata}"
+    rrsigs = []
+    for rrset in response.answer:
+        if rrset.match(
+            dns.name.from_text(qname), dns.rdataclass.IN, dns.rdatatype.RRSIG, qtype
+        ):
+            rrsigs.append(rrset)
+        elif not match in rrset.to_text():
+            return False
+
+    if len(rrsigs) == 0:
+        return False
+
+    # Zone is updated, ready to verify the signatures.
+    check_signatures(rrsigs, qtype, fqdn, ksks, zsks)
+
+    return True
+
+
 def next_key_event_equals(server, zone, next_event):
     if next_event is None:
         # No next key event check.
diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh
index bc41796d56..7b6176de31 100644
--- a/bin/tests/system/kasp/tests.sh
+++ b/bin/tests/system/kasp/tests.sh
@@ -64,6 +64,7 @@ next_key_event_threshold=100
 # infinite loops if there is an error.
 n=$((n + 1))
 echo_i "waiting for kasp signing changes to take effect ($n)"
+ret=0
 
 _wait_for_done_apexnsec() {
   while read -r zone; do
@@ -93,9 +94,6 @@ grep "loading from master file ${ZONE}.db failed: out of range" "ns3/named.run"
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status + ret))
 
-#
-# Zone: default.kasp.
-#
 set_keytimes_csk_policy() {
   # The first key is immediately published and activated.
   created=$(key_get KEY1 CREATED)
@@ -108,10 +106,6 @@ set_keytimes_csk_policy() {
   # Key lifetime is unlimited, so not setting RETIRED and REMOVED.
 }
 
-# Check the zone with default kasp policy has loaded and is signed.
-set_zone "default.kasp"
-set_policy "default" "1" "3600"
-set_server "ns3" "10.53.0.3"
 # Key properties.
 set_keyrole "KEY1" "csk"
 set_keylifetime "KEY1" "0"
@@ -125,59 +119,6 @@ set_keystate "KEY1" "STATE_KRRSIG" "rumoured"
 set_keystate "KEY1" "STATE_ZRRSIG" "rumoured"
 set_keystate "KEY1" "STATE_DS" "hidden"
 
-check_keys
-check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
-set_keytimes_csk_policy
-check_keytimes
-check_apex
-check_subdomain
-dnssec_verify
-
-# Trigger a keymgr run. Make sure the key files are not touched if there are
-# no modifications to the key metadata.
-n=$((n + 1))
-echo_i "make sure key files are untouched if metadata does not change ($n)"
-ret=0
-basefile=$(key_get KEY1 BASEFILE)
-privkey_stat=$(key_get KEY1 PRIVKEY_STAT)
-pubkey_stat=$(key_get KEY1 PUBKEY_STAT)
-state_stat=$(key_get KEY1 STATE_STAT)
-
-nextpart $DIR/named.run >/dev/null
-rndccmd 10.53.0.3 loadkeys "$ZONE" >/dev/null || log_error "rndc loadkeys zone ${ZONE} failed"
-wait_for_log 3 "keymgr: $ZONE done" $DIR/named.run || ret=1
-privkey_stat2=$(key_stat "${basefile}.private")
-pubkey_stat2=$(key_stat "${basefile}.key")
-state_stat2=$(key_stat "${basefile}.state")
-test "$privkey_stat" = "$privkey_stat2" || log_error "wrong private key file stat (expected $privkey_stat got $privkey_stat2)"
-test "$pubkey_stat" = "$pubkey_stat2" || log_error "wrong public key file stat (expected $pubkey_stat got $pubkey_stat2)"
-test "$state_stat" = "$state_stat2" || log_error "wrong state file stat (expected $state_stat got $state_stat2)"
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
-n=$((n + 1))
-echo_i "again ($n)"
-ret=0
-
-nextpart $DIR/named.run >/dev/null
-rndccmd 10.53.0.3 loadkeys "$ZONE" >/dev/null || log_error "rndc loadkeys zone ${ZONE} failed"
-wait_for_log 3 "keymgr: $ZONE done" $DIR/named.run || ret=1
-privkey_stat2=$(key_stat "${basefile}.private")
-pubkey_stat2=$(key_stat "${basefile}.key")
-state_stat2=$(key_stat "${basefile}.state")
-test "$privkey_stat" = "$privkey_stat2" || log_error "wrong private key file stat (expected $privkey_stat got $privkey_stat2)"
-test "$pubkey_stat" = "$pubkey_stat2" || log_error "wrong public key file stat (expected $pubkey_stat got $pubkey_stat2)"
-test "$state_stat" = "$state_stat2" || log_error "wrong state file stat (expected $state_stat got $state_stat2)"
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
-# Update zone.
-n=$((n + 1))
-echo_i "modify unsigned zone file and check that new record is signed for zone ${ZONE} ($n)"
-ret=0
-cp "${DIR}/template2.db.in" "${DIR}/${ZONE}.db"
-rndccmd 10.53.0.3 reload "$ZONE" >/dev/null || log_error "rndc reload zone ${ZONE} failed"
-
 update_is_signed() {
   ip_a=$1
   ip_d=$2
@@ -201,31 +142,6 @@ update_is_signed() {
   fi
 }
 
-retry_quiet 10 update_is_signed "10.0.0.11" "10.0.0.44" || ret=1
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
-# Move the private key file, a rekey event should not introduce replacement
-# keys.
-ret=0
-echo_i "test that if private key files are inaccessible this doesn't trigger a rollover ($n)"
-basefile=$(key_get KEY1 BASEFILE)
-mv "${basefile}.private" "${basefile}.offline"
-rndccmd 10.53.0.3 loadkeys "$ZONE" >/dev/null || log_error "rndc loadkeys zone ${ZONE} failed"
-wait_for_log 3 "zone $ZONE/IN (signed): zone_rekey:zone_verifykeys failed: some key files are missing" $DIR/named.run || ret=1
-mv "${basefile}.offline" "${basefile}.private"
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
-# Nothing has changed.
-check_keys
-check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
-set_keytimes_csk_policy
-check_keytimes
-check_apex
-check_subdomain
-dnssec_verify
-
 #
 # A zone with special characters.
 #
diff --git a/bin/tests/system/kasp/tests_kasp.py b/bin/tests/system/kasp/tests_kasp.py
index 55dbf12f89..ba9217b3ee 100644
--- a/bin/tests/system/kasp/tests_kasp.py
+++ b/bin/tests/system/kasp/tests_kasp.py
@@ -14,6 +14,7 @@ import shutil
 
 from datetime import timedelta
 
+import dns
 import pytest
 
 import isctest
@@ -29,6 +30,7 @@ pytestmark = pytest.mark.extra_artifacts(
         "K*.cmp",
         "K*.key",
         "K*.state",
+        "*.axfr",
         "*.created",
         "dig.out*",
         "keyevent.out.*",
@@ -44,14 +46,18 @@ pytestmark = pytest.mark.extra_artifacts(
         "unused.key-*",
         "verify.out.*",
         "zone.out.*",
-        "ns*/K*.private",
         "ns*/K*.key",
+        "ns*/K*.offline",
+        "ns*/K*.private",
         "ns*/K*.state",
         "ns*/*.db",
         "ns*/*.db.infile",
         "ns*/*.db.signed",
+        "ns*/*.db.signed.tmp",
         "ns*/*.jbk",
         "ns*/*.jnl",
+        "ns*/*.zsk1",
+        "ns*/*.zsk2",
         "ns*/dsset-*",
         "ns*/keygen.out.*",
         "ns*/keys",
@@ -65,14 +71,116 @@ pytestmark = pytest.mark.extra_artifacts(
         "ns*/signer.out.*",
         "ns*/zones",
         "ns*/policies/*.conf",
-        "ns*/*.zsk1",
-        "ns*/*.zsk2",
         "ns3/legacy-keys.*",
         "ns3/dynamic-signed-inline-signing.kasp.db.signed.signed",
     ]
 )
 
 
+def check_all(server, zone, policy, ksks, zsks, tsig=None):
+    isctest.kasp.check_dnssecstatus(server, zone, ksks + zsks, policy=policy)
+    isctest.kasp.check_apex(server, zone, ksks, zsks, tsig=tsig)
+    isctest.kasp.check_subdomain(server, zone, ksks, zsks, tsig=tsig)
+    isctest.kasp.check_dnssec_verify(server, zone)
+
+
+def set_keytimes_default_policy(kp):
+    # The first key is immediately published and activated.
+    kp.timing["Generated"] = kp.key.get_timing("Created")
+    kp.timing["Published"] = kp.timing["Generated"]
+    kp.timing["Active"] = kp.timing["Generated"]
+    # The DS can be published if the DNSKEY and RRSIG records are
+    # OMNIPRESENT.  This happens after max-zone-ttl (1d) plus
+    # plus zone-propagation-delay (300s).
+    kp.timing["PublishCDS"] = kp.timing["Published"] + timedelta(days=1, seconds=300)
+    # Key lifetime is unlimited, so not setting 'Retired' nor 'Removed'.
+    kp.timing["DNSKEYChange"] = kp.timing["Published"]
+    kp.timing["DSChange"] = kp.timing["Published"]
+    kp.timing["KRRSIGChange"] = kp.timing["Active"]
+    kp.timing["ZRRSIGChange"] = kp.timing["Active"]
+
+
+def test_kasp_default(servers):
+    server = servers["ns3"]
+
+    # check the zone with default kasp policy has loaded and is signed.
+    isctest.log.info("check a zone with the default policy is signed")
+    zone = "default.kasp"
+    policy = "default"
+
+    # Key properties.
+    # DNSKEY, RRSIG (ksk), RRSIG (zsk) are published. DS needs to wait.
+    keyprops = [
+        "csk 0 13 256 goal:omnipresent dnskey:rumoured krrsig:rumoured zrrsig:rumoured ds:hidden",
+    ]
+    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_keys(zone, keys, expected)
+    set_keytimes_default_policy(expected[0])
+    isctest.kasp.check_keytimes(keys, expected)
+    check_all(server, zone, policy, keys, [])
+
+    # Trigger a keymgr run. Make sure the key files are not touched if there
+    # are no modifications to the key metadata.
+    isctest.log.info(
+        "check that key files are untouched if there are no metadata changes"
+    )
+    key = keys[0]
+    privkey_stat = os.stat(key.privatefile)
+    pubkey_stat = os.stat(key.keyfile)
+    state_stat = os.stat(key.statefile)
+
+    with server.watch_log_from_here() as watcher:
+        server.rndc(f"loadkeys {zone}", log=False)
+        watcher.wait_for_line(f"keymgr: {zone} done")
+
+    assert privkey_stat.st_mtime == os.stat(key.privatefile).st_mtime
+    assert pubkey_stat.st_mtime == os.stat(key.keyfile).st_mtime
+    assert state_stat.st_mtime == os.stat(key.statefile).st_mtime
+
+    # again
+    with server.watch_log_from_here() as watcher:
+        server.rndc(f"loadkeys {zone}", log=False)
+        watcher.wait_for_line(f"keymgr: {zone} done")
+
+    assert privkey_stat.st_mtime == os.stat(key.privatefile).st_mtime
+    assert pubkey_stat.st_mtime == os.stat(key.keyfile).st_mtime
+    assert state_stat.st_mtime == os.stat(key.statefile).st_mtime
+
+    # modify unsigned zone file and check that new record is signed.
+    isctest.log.info("check that an updated zone signs the new record")
+    shutil.copyfile("ns3/template2.db.in", f"ns3/{zone}.db")
+    server.rndc(f"reload {zone}", log=False)
+
+    def update_is_signed():
+        parts = update.split()
+        qname = parts[0]
+        qtype = dns.rdatatype.from_text(parts[1])
+        rdata = parts[2]
+        return isctest.kasp.verify_update_is_signed(
+            server, zone, qname, qtype, rdata, keys, []
+        )
+
+    expected_updates = [f"a.{zone}. A 10.0.0.11", f"d.{zone}. A 10.0.0.44"]
+    for update in expected_updates:
+        isctest.run.retry_with_timeout(update_is_signed, timeout=5)
+
+    # Move the private key file, a rekey event should not introduce
+    # replacement keys.
+    isctest.log.info("check that missing private key doesn't trigger rollover")
+    shutil.move(f"{key.privatefile}", f"{key.path}.offline")
+    expectmsg = "zone_rekey:zone_verifykeys failed: some key files are missing"
+    with server.watch_log_from_here() as watcher:
+        server.rndc(f"loadkeys {zone}", log=False)
+        watcher.wait_for_line(f"zone {zone}/IN (signed): {expectmsg}")
+    # Nothing has changed.
+    expected[0].properties["private"] = False
+    isctest.kasp.check_keys(zone, keys, expected)
+    isctest.kasp.check_keytimes(keys, expected)
+    check_all(server, zone, policy, keys, [])
+
+
 def test_kasp_dnssec_keygen():
     def keygen(zone, policy, keydir=None):
         if keydir is None:
@@ -95,7 +203,7 @@ def test_kasp_dnssec_keygen():
     lifetime = {
         "P1Y": int(timedelta(days=365).total_seconds()),
         "P30D": int(timedelta(days=30).total_seconds()),
-        "P6M": int(timedelta(days=31*6).total_seconds()),
+        "P6M": int(timedelta(days=31 * 6).total_seconds()),
     }
     keyprops = [
         f"csk {lifetime['P1Y']} 13 256",
@@ -103,7 +211,7 @@ def test_kasp_dnssec_keygen():
         f"zsk {lifetime['P30D']} 8 2048",
         f"zsk {lifetime['P6M']} 8 3072",
     ]
-    keydir="keys"
+    keydir = "keys"
     out = keygen("kasp", "kasp", keydir)
     keys = isctest.kasp.keystr_to_keylist(out, keydir)
     expected = isctest.kasp.policy_to_properties(ttl=200, keys=keyprops)

From dd71177f49b30730549c92ac15786448fc0643f6 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 14 Mar 2025 13:08:44 +0100
Subject: [PATCH 3/6] Convert dynamic zone test cases to pytest

This commit deals with converting the dynamic zone test cases to
pytest. The tests for 'inline-signing.kasp' are similar to the default
case, so these are added to 'test_kasp_default'.

Unfortunately I need to add sleep calls in between freezing, updating,
and thawing a zone. Without it the intermittent failures are too
frequent.

(cherry picked from commit 0b41afbd15da6d9564952b63ebe3df5f56ad7d4e)
---
 bin/tests/system/kasp/tests.sh      | 146 --------------------------
 bin/tests/system/kasp/tests_kasp.py | 157 ++++++++++++++++++++++++++++
 2 files changed, 157 insertions(+), 146 deletions(-)

diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh
index 7b6176de31..232d6a4dff 100644
--- a/bin/tests/system/kasp/tests.sh
+++ b/bin/tests/system/kasp/tests.sh
@@ -119,29 +119,6 @@ set_keystate "KEY1" "STATE_KRRSIG" "rumoured"
 set_keystate "KEY1" "STATE_ZRRSIG" "rumoured"
 set_keystate "KEY1" "STATE_DS" "hidden"
 
-update_is_signed() {
-  ip_a=$1
-  ip_d=$2
-
-  if [ "$ip_a" != "-" ]; then
-    dig_with_opts "a.${ZONE}" "@${SERVER}" A >"dig.out.$DIR.test$n.a" || return 1
-    grep "status: NOERROR" "dig.out.$DIR.test$n.a" >/dev/null || return 1
-    grep "a.${ZONE}\..*${DEFAULT_TTL}.*IN.*A.*${ip_a}" "dig.out.$DIR.test$n.a" >/dev/null || return 1
-    lines=$(get_keys_which_signed A 0 "dig.out.$DIR.test$n.a" | wc -l)
-    test "$lines" -eq 1 || return 1
-    get_keys_which_signed A 0 "dig.out.$DIR.test$n.a" | grep "^${KEY_ID}$" >/dev/null || return 1
-  fi
-
-  if [ "$ip_d" != "-" ]; then
-    dig_with_opts "d.${ZONE}" "@${SERVER}" A >"dig.out.$DIR.test$n".d || return 1
-    grep "status: NOERROR" "dig.out.$DIR.test$n".d >/dev/null || return 1
-    grep "d.${ZONE}\..*${DEFAULT_TTL}.*IN.*A.*${ip_d}" "dig.out.$DIR.test$n".d >/dev/null || return 1
-    lines=$(get_keys_which_signed A 0 "dig.out.$DIR.test$n".d | wc -l)
-    test "$lines" -eq 1 || return 1
-    get_keys_which_signed A 0 "dig.out.$DIR.test$n".d | grep "^${KEY_ID}$" >/dev/null || return 1
-  fi
-}
-
 #
 # A zone with special characters.
 #
@@ -152,129 +129,6 @@ set_server "ns3" "10.53.0.3"
 # escaping characters, so we will just try to verify the zone.
 dnssec_verify
 
-#
-# Zone: dynamic.kasp
-#
-set_zone "dynamic.kasp"
-set_dynamic
-set_policy "default" "1" "3600"
-set_server "ns3" "10.53.0.3"
-# Key properties, timings and states same as above.
-check_keys
-check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
-set_keytimes_csk_policy
-check_keytimes
-check_apex
-check_subdomain
-dnssec_verify
-
-# Update zone with nsupdate.
-n=$((n + 1))
-echo_i "nsupdate zone and check that new record is signed for zone ${ZONE} ($n)"
-ret=0
-(
-  echo zone ${ZONE}
-  echo server 10.53.0.3 "$PORT"
-  echo update del "a.${ZONE}" 300 A 10.0.0.1
-  echo update add "a.${ZONE}" 300 A 10.0.0.101
-  echo update add "d.${ZONE}" 300 A 10.0.0.4
-  echo send
-) | $NSUPDATE
-
-retry_quiet 10 update_is_signed "10.0.0.101" "10.0.0.4" || ret=1
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
-# Update zone with nsupdate (reverting the above change).
-n=$((n + 1))
-echo_i "nsupdate zone and check that new record is signed for zone ${ZONE} ($n)"
-ret=0
-(
-  echo zone ${ZONE}
-  echo server 10.53.0.3 "$PORT"
-  echo update add "a.${ZONE}" 300 A 10.0.0.1
-  echo update del "a.${ZONE}" 300 A 10.0.0.101
-  echo update del "d.${ZONE}" 300 A 10.0.0.4
-  echo send
-) | $NSUPDATE
-
-retry_quiet 10 update_is_signed "10.0.0.1" "-" || ret=1
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
-# Update zone with freeze/thaw.
-n=$((n + 1))
-echo_i "modify zone file and check that new record is signed for zone ${ZONE} ($n)"
-ret=0
-rndccmd 10.53.0.3 freeze "$ZONE" >/dev/null || log_error "rndc freeze zone ${ZONE} failed"
-sleep 1
-echo "d.${ZONE}. 300 A 10.0.0.44" >>"${DIR}/${ZONE}.db"
-rndccmd 10.53.0.3 thaw "$ZONE" >/dev/null || log_error "rndc thaw zone ${ZONE} failed"
-
-retry_quiet 10 update_is_signed "10.0.0.1" "10.0.0.44" || ret=1
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
-#
-# Zone: dynamic-inline-signing.kasp
-#
-set_zone "dynamic-inline-signing.kasp"
-set_dynamic
-set_policy "default" "1" "3600"
-set_server "ns3" "10.53.0.3"
-# Key properties, timings and states same as above.
-check_keys
-check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
-set_keytimes_csk_policy
-check_keytimes
-check_apex
-check_subdomain
-dnssec_verify
-
-# Update zone with freeze/thaw.
-n=$((n + 1))
-echo_i "modify unsigned zone file and check that new record is signed for zone ${ZONE} ($n)"
-ret=0
-rndccmd 10.53.0.3 freeze "$ZONE" >/dev/null || log_error "rndc freeze zone ${ZONE} failed"
-sleep 1
-cp "${DIR}/template2.db.in" "${DIR}/${ZONE}.db"
-rndccmd 10.53.0.3 thaw "$ZONE" >/dev/null || log_error "rndc thaw zone ${ZONE} failed"
-
-retry_quiet 10 update_is_signed || ret=1
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
-#
-# Zone: dynamic-signed-inline-signing.kasp
-#
-set_zone "dynamic-signed-inline-signing.kasp"
-set_dynamic
-set_policy "default" "1" "3600"
-set_server "ns3" "10.53.0.3"
-dnssec_verify
-# Ensure no zone_resigninc for the unsigned version of the zone is triggered.
-n=$((n + 1))
-echo_i "check if resigning the raw version of the zone is prevented for zone ${ZONE} ($n)"
-ret=0
-grep "zone_resigninc: zone $ZONE/IN (unsigned): enter" $DIR/named.run && ret=1
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
-#
-# Zone: inline-signing.kasp
-#
-set_zone "inline-signing.kasp"
-set_policy "default" "1" "3600"
-set_server "ns3" "10.53.0.3"
-# Key properties, timings and states same as above.
-check_keys
-check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
-set_keytimes_csk_policy
-check_keytimes
-check_apex
-check_subdomain
-dnssec_verify
-
 #
 # Zone: checkds-ksk.kasp.
 #
diff --git a/bin/tests/system/kasp/tests_kasp.py b/bin/tests/system/kasp/tests_kasp.py
index ba9217b3ee..3daa5eebcc 100644
--- a/bin/tests/system/kasp/tests_kasp.py
+++ b/bin/tests/system/kasp/tests_kasp.py
@@ -11,10 +11,12 @@
 
 import os
 import shutil
+import time
 
 from datetime import timedelta
 
 import dns
+import dns.update
 import pytest
 
 import isctest
@@ -180,6 +182,161 @@ def test_kasp_default(servers):
     isctest.kasp.check_keytimes(keys, expected)
     check_all(server, zone, policy, keys, [])
 
+    # A zone that uses inline-signing.
+    isctest.log.info("check an inline-signed zone with the default policy is signed")
+    zone = "inline-signing.kasp"
+    # Key properties.
+    key1 = KeyProperties.default()
+    keys = isctest.kasp.keydir_to_keylist(zone, "ns3")
+    expected = [key1]
+    isctest.kasp.check_zone_is_signed(server, zone)
+    isctest.kasp.check_keys(zone, keys, expected)
+    set_keytimes_default_policy(key1)
+    isctest.kasp.check_keytimes(keys, expected)
+    check_all(server, zone, policy, keys, [])
+
+
+def test_kasp_dynamic(servers):
+    # Dynamic update test cases.
+    server = servers["ns3"]
+
+    # Standard dynamic zone.
+    isctest.log.info("check dynamic zone is updated and signed after update")
+    zone = "dynamic.kasp"
+    policy = "default"
+    # Key properties.
+    key1 = KeyProperties.default()
+    expected = [key1]
+    keys = isctest.kasp.keydir_to_keylist(zone, "ns3")
+    isctest.kasp.check_zone_is_signed(server, zone)
+    isctest.kasp.check_keys(zone, keys, expected)
+    set_keytimes_default_policy(key1)
+    expected = [key1]
+    isctest.kasp.check_keytimes(keys, expected)
+    check_all(server, zone, policy, keys, [])
+
+    # Update zone with nsupdate.
+    def nsupdate(updates):
+        message = dns.update.UpdateMessage(zone)
+        for update in updates:
+            if update[0] == "del":
+                message.delete(update[1], update[2], update[3])
+            else:
+                assert update[0] == "add"
+                message.add(update[1], update[2], update[3], update[4])
+
+        try:
+            response = isctest.query.udp(
+                message, server.ip, server.ports.dns, timeout=3
+            )
+            assert response.rcode() == dns.rcode.NOERROR
+        except dns.exception.Timeout:
+            assert False, f"update timeout for {zone}"
+
+        isctest.log.debug(f"update of zone {zone} to server {server.ip} successful")
+
+    def update_is_signed():
+        parts = update.split()
+        qname = parts[0]
+        qtype = dns.rdatatype.from_text(parts[1])
+        rdata = parts[2]
+        return isctest.kasp.verify_update_is_signed(
+            server, zone, qname, qtype, rdata, keys, []
+        )
+
+    updates = [
+        ["del", f"a.{zone}.", "A", "10.0.0.1"],
+        ["add", f"a.{zone}.", 300, "A", "10.0.0.101"],
+        ["add", f"d.{zone}.", 300, "A", "10.0.0.4"],
+    ]
+    nsupdate(updates)
+
+    expected_updates = [f"a.{zone}. A 10.0.0.101", f"d.{zone}. A 10.0.0.4"]
+    for update in expected_updates:
+        isctest.run.retry_with_timeout(update_is_signed, timeout=5)
+
+    # Update zone with nsupdate (reverting the above change).
+    updates = [
+        ["add", f"a.{zone}.", 300, "A", "10.0.0.1"],
+        ["del", f"a.{zone}.", "A", "10.0.0.101"],
+        ["del", f"d.{zone}.", "A", "10.0.0.4"],
+    ]
+    nsupdate(updates)
+
+    update = f"a.{zone}. A 10.0.0.1"
+    isctest.run.retry_with_timeout(update_is_signed, timeout=5)
+
+    # Update zone with freeze/thaw.
+    isctest.log.info("check dynamic zone is updated and signed after freeze and thaw")
+    with server.watch_log_from_here() as watcher:
+        server.rndc(f"freeze {zone}", log=False)
+        watcher.wait_for_line(f"freezing zone '{zone}/IN': success")
+
+    time.sleep(1)
+    with open(f"ns3/{zone}.db", "a", encoding="utf-8") as zonefile:
+        zonefile.write(f"d.{zone}. 300 A 10.0.0.44\n")
+    time.sleep(1)
+
+    with server.watch_log_from_here() as watcher:
+        server.rndc(f"thaw {zone}", log=False)
+        watcher.wait_for_line(f"thawing zone '{zone}/IN': success")
+
+    expected_updates = [f"a.{zone}. A 10.0.0.1", f"d.{zone}. A 10.0.0.44"]
+
+    for update in expected_updates:
+        isctest.run.retry_with_timeout(update_is_signed, timeout=5)
+
+    # Dynamic, and inline-signing.
+    zone = "dynamic-inline-signing.kasp"
+    # Key properties.
+    key1 = KeyProperties.default()
+    expected = [key1]
+    keys = isctest.kasp.keydir_to_keylist(zone, "ns3")
+    isctest.kasp.check_zone_is_signed(server, zone)
+    isctest.kasp.check_keys(zone, keys, expected)
+    set_keytimes_default_policy(key1)
+    expected = [key1]
+    isctest.kasp.check_keytimes(keys, expected)
+    check_all(server, zone, policy, keys, [])
+
+    # Update zone with freeze/thaw.
+    isctest.log.info(
+        "check dynamic inline-signed zone is updated and signed after freeze and thaw"
+    )
+    with server.watch_log_from_here() as watcher:
+        server.rndc(f"freeze {zone}", log=False)
+        watcher.wait_for_line(f"freezing zone '{zone}/IN': success")
+
+    time.sleep(1)
+    shutil.copyfile("ns3/template2.db.in", f"ns3/{zone}.db")
+    time.sleep(1)
+
+    with server.watch_log_from_here() as watcher:
+        server.rndc(f"thaw {zone}", log=False)
+        watcher.wait_for_line(f"thawing zone '{zone}/IN': success")
+
+    expected_updates = [f"a.{zone}. A 10.0.0.11", f"d.{zone}. A 10.0.0.44"]
+    for update in expected_updates:
+        isctest.run.retry_with_timeout(update_is_signed, timeout=5)
+
+    # Dynamic, signed, and inline-signing.
+    isctest.log.info("check dynamic signed, and inline-signed zone")
+    zone = "dynamic-signed-inline-signing.kasp"
+    # Key properties.
+    key1 = KeyProperties.default()
+    # The ns3/setup.sh script sets all states to omnipresent.
+    key1.metadata["DNSKEYState"] = "omnipresent"
+    key1.metadata["KRRSIGState"] = "omnipresent"
+    key1.metadata["ZRRSIGState"] = "omnipresent"
+    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_keys(zone, keys, expected)
+    check_all(server, zone, policy, keys, [])
+    # Ensure no zone_resigninc for the unsigned version of the zone is triggered.
+    assert f"zone_resigninc: zone {zone}/IN (unsigned): enter" not in "ns3/named.run"
+
 
 def test_kasp_dnssec_keygen():
     def keygen(zone, policy, keydir=None):

From 6650acfb2eb26c8d210c827adf6ddfe86fd44724 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 14 Mar 2025 13:42:30 +0100
Subject: [PATCH 4/6] Convert some special kasp test cases to pytest

This converts a special characters test case, a max-zone-ttl error
check, and two cases of insecure zones.

We no longer assert for having more than one DNSKEY and/or RRSIG
records. If the zone is insecure, this is no longer always true. And
we already check for the expected number of records in the
check_dnskeys/check_signatures functions.

(cherry picked from commit 07ac0e603633a701b35c92de2e3b27dc2643dcba)
---
 bin/tests/system/isctest/kasp.py    |  4 --
 bin/tests/system/kasp/tests.sh      | 64 ++---------------------------
 bin/tests/system/kasp/tests_kasp.py | 46 +++++++++++++++++++++
 3 files changed, 50 insertions(+), 64 deletions(-)

diff --git a/bin/tests/system/isctest/kasp.py b/bin/tests/system/isctest/kasp.py
index 9e23b9a6ef..2ade4dafbe 100644
--- a/bin/tests/system/isctest/kasp.py
+++ b/bin/tests/system/isctest/kasp.py
@@ -971,16 +971,13 @@ def check_apex(server, zone, ksks, zsks, tsig=None):
 
     # test dnskey query
     dnskeys, rrsigs = _query_rrset(server, fqdn, dns.rdatatype.DNSKEY, tsig=tsig)
-    assert len(dnskeys) > 0
     check_dnskeys(dnskeys, ksks, zsks)
-    assert len(rrsigs) > 0
     check_signatures(rrsigs, dns.rdatatype.DNSKEY, fqdn, ksks, zsks)
 
     # test soa query
     soa, rrsigs = _query_rrset(server, fqdn, dns.rdatatype.SOA, tsig=tsig)
     assert len(soa) == 1
     assert f"{zone}. {DEFAULT_TTL} IN SOA" in soa[0].to_text()
-    assert len(rrsigs) > 0
     check_signatures(rrsigs, dns.rdatatype.SOA, fqdn, ksks, zsks)
 
     # test cdnskey query
@@ -1016,7 +1013,6 @@ def check_subdomain(server, zone, ksks, zsks, tsig=None):
         else:
             assert match in rrset.to_text()
 
-    assert len(rrsigs) > 0
     check_signatures(rrsigs, qtype, fqdn, ksks, zsks)
 
 
diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh
index 232d6a4dff..ca294bab01 100644
--- a/bin/tests/system/kasp/tests.sh
+++ b/bin/tests/system/kasp/tests.sh
@@ -85,15 +85,6 @@ retry_quiet 30 _wait_for_done_apexnsec || ret=1
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status + ret))
 
-# Test max-zone-ttl rejects zones with too high TTL.
-n=$((n + 1))
-echo_i "check that max-zone-ttl rejects zones with too high TTL ($n)"
-ret=0
-set_zone "max-zone-ttl.kasp"
-grep "loading from master file ${ZONE}.db failed: out of range" "ns3/named.run" >/dev/null || ret=1
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
 set_keytimes_csk_policy() {
   # The first key is immediately published and activated.
   created=$(key_get KEY1 CREATED)
@@ -119,16 +110,6 @@ set_keystate "KEY1" "STATE_KRRSIG" "rumoured"
 set_keystate "KEY1" "STATE_ZRRSIG" "rumoured"
 set_keystate "KEY1" "STATE_DS" "hidden"
 
-#
-# A zone with special characters.
-#
-set_zone "i-am.\":\;?&[]\@!\$*+,|=\.\(\)special.kasp."
-set_policy "default" "1" "3600"
-set_server "ns3" "10.53.0.3"
-# It is non-trivial to adapt the tests to deal with all possible different
-# escaping characters, so we will just try to verify the zone.
-dnssec_verify
-
 #
 # Zone: checkds-ksk.kasp.
 #
@@ -474,53 +455,16 @@ if [ $RSASHA1_SUPPORTED = 1 ]; then
   dnssec_verify
 fi
 
-#
-# Zone: unsigned.kasp.
-#
-set_zone "unsigned.kasp"
-set_policy "none" "0" "0"
-set_server "ns3" "10.53.0.3"
-
-key_clear "KEY1"
-key_clear "KEY2"
-key_clear "KEY3"
-key_clear "KEY4"
-
-check_keys
-check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
-check_apex
-check_subdomain
-# Make sure the zone file is untouched.
-n=$((n + 1))
-echo_i "Make sure the zonefile for zone ${ZONE} is not edited ($n)"
-ret=0
-diff "${DIR}/${ZONE}.db.infile" "${DIR}/${ZONE}.db" || ret=1
-test "$ret" -eq 0 || echo_i "failed"
-status=$((status + ret))
-
-#
-# Zone: insecure.kasp.
-#
-set_zone "insecure.kasp"
-set_policy "insecure" "0" "0"
-set_server "ns3" "10.53.0.3"
-
-key_clear "KEY1"
-key_clear "KEY2"
-key_clear "KEY3"
-key_clear "KEY4"
-
-check_keys
-check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
-check_apex
-check_subdomain
-
 #
 # Zone: unlimited.kasp.
 #
 set_zone "unlimited.kasp"
 set_policy "unlimited" "1" "1234"
 set_server "ns3" "10.53.0.3"
+key_clear "KEY1"
+key_clear "KEY2"
+key_clear "KEY3"
+key_clear "KEY4"
 # Key properties.
 set_keyrole "KEY1" "csk"
 set_keylifetime "KEY1" "0"
diff --git a/bin/tests/system/kasp/tests_kasp.py b/bin/tests/system/kasp/tests_kasp.py
index 3daa5eebcc..adc83988c2 100644
--- a/bin/tests/system/kasp/tests_kasp.py
+++ b/bin/tests/system/kasp/tests_kasp.py
@@ -338,6 +338,52 @@ def test_kasp_dynamic(servers):
     assert f"zone_resigninc: zone {zone}/IN (unsigned): enter" not in "ns3/named.run"
 
 
+def test_kasp_special_characters(servers):
+    server = servers["ns3"]
+
+    # A zone with special characters.
+    isctest.log.info("check special characters")
+
+    zone = r'i-am.":\;?&[]\@!\$*+,|=\.\(\)special.kasp'
+    # It is non-trivial to adapt the tests to deal with all possible different
+    # escaping characters, so we will just try to verify the zone.
+    isctest.kasp.check_dnssec_verify(server, zone)
+
+
+def test_kasp_insecure(servers):
+    server = servers["ns3"]
+
+    # Insecure zones.
+    isctest.log.info("check insecure zones")
+
+    zone = "insecure.kasp"
+    expected = []
+    keys = isctest.kasp.keydir_to_keylist(zone, "ns3")
+    isctest.kasp.check_keys(zone, keys, expected)
+    isctest.kasp.check_dnssecstatus(server, zone, keys, policy="insecure")
+    isctest.kasp.check_apex(server, zone, keys, [])
+    isctest.kasp.check_subdomain(server, zone, keys, [])
+
+    zone = "unsigned.kasp"
+    expected = []
+    keys = isctest.kasp.keydir_to_keylist(zone, "ns3")
+    isctest.kasp.check_keys(zone, keys, expected)
+    isctest.kasp.check_dnssecstatus(server, zone, keys, policy=None)
+    isctest.kasp.check_apex(server, zone, keys, [])
+    isctest.kasp.check_subdomain(server, zone, keys, [])
+    # Make sure the zone file is untouched.
+    isctest.check.file_contents_equal(f"ns3/{zone}.db.infile", f"ns3/{zone}.db")
+
+
+def test_kasp_bad_maxzonettl(servers):
+    server = servers["ns3"]
+
+    # check that max-zone-ttl rejects zones with too high TTL.
+    isctest.log.info("check max-zone-ttl rejects zones with too high TTL")
+    zone = "max-zone-ttl.kasp"
+    assert f"loading from master file {zone}.db failed: out of range" in server.log
+
+
 def test_kasp_dnssec_keygen():
     def keygen(zone, policy, keydir=None):
         if keydir is None:

From 7695be3018580b73d3120271977e902bcac0e5f4 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Tue, 18 Mar 2025 10:06:29 +0100
Subject: [PATCH 5/6] The kasp tests require dnspython >= 2.0.0

The kasp tests make use of dns.update.UpdateMessage and dns.tsig.Key,
that are introduced in dnspython 2.0.0.

(cherry picked from commit 46aead5a6df2337fa3dc758f2bd839c88efe611c)
---
 bin/tests/system/kasp/tests_kasp.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/bin/tests/system/kasp/tests_kasp.py b/bin/tests/system/kasp/tests_kasp.py
index adc83988c2..7ffa22e740 100644
--- a/bin/tests/system/kasp/tests_kasp.py
+++ b/bin/tests/system/kasp/tests_kasp.py
@@ -19,6 +19,7 @@ import dns
 import dns.update
 import pytest
 
+pytest.importorskip("dns", minversion="2.0.0")
 import isctest
 from isctest.kasp import (
     KeyProperties,

From b50333e889860d55d54fbf29ba87652bbf8b63cf Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 11 Apr 2025 12:54:15 -0500
Subject: [PATCH 6/6] Fix bugs in isctest.kasp

isctest.util was not imported so file_contents_contain could not be
found. And rename verify_keys to check_keys because it asserts in
isctest.run.retry_with_timeout.

(cherry picked from commit 62a6b9faa7ccf85b2b628fb991c8f72d2976eee3)
---
 bin/tests/system/isctest/kasp.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/bin/tests/system/isctest/kasp.py b/bin/tests/system/isctest/kasp.py
index 2ade4dafbe..0ea1c774ea 100644
--- a/bin/tests/system/isctest/kasp.py
+++ b/bin/tests/system/isctest/kasp.py
@@ -24,6 +24,7 @@ import dns
 import dns.tsig
 import isctest.log
 import isctest.query
+import isctest.util
 
 DEFAULT_TTL = 300
 
@@ -612,7 +613,7 @@ def check_zone_is_signed(server, zone, tsig=None):
     assert signed
 
 
-def verify_keys(zone, keys, expected):
+def check_keys(zone, keys, expected):
     """
     Checks keys for a configured zone. This verifies:
     1. The expected number of keys exist in 'keys'.