From 44f36e27633cb3eba0f2687e6e13419789ca0d5f Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 19 Aug 2025 12:42:35 +0200 Subject: [PATCH 1/4] Test rndc sign updates the signatures Add a check to the ZSK rollover test case that ensures the zone is signed with the successor key only, after a 'rndc sign' is commanded. (cherry picked from commit 008d3d2a9c0bb807ce392374bcc63a6f79b9d088) --- .../tests_rollover_zsk_prepublication.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py b/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py index c8643022e6..e5d842c535 100644 --- a/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py +++ b/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py @@ -222,6 +222,14 @@ def test_zsk_prepub_step3(tld, alg, size, ns3): } isctest.kasp.check_rollover_step(ns3, CONFIG, policy, step) + # Force full resign and check all signatures have been replaced. + with ns3.watch_log_from_here() as watcher: + ns3.rndc(f"sign {zone}", log=False) + watcher.wait_for_line(f"zone {zone}/IN (signed): sending notifies") + + step["smooth"] = False + isctest.kasp.check_rollover_step(ns3, CONFIG, POLICY, step) + @pytest.mark.parametrize( "tld", From 76ecde48af58ad444a4619b9ff1325e3bbc84bc2 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 19 Aug 2025 12:37:29 +0200 Subject: [PATCH 2/4] Force full sign to generate new signatures When introducing the kasp logic, a full sign of the zone did not generate new signatures for the new active keys during a ZSK rollover. The introduced kasp logic ensured that the rollover is performed smoothly, as in the signatures are only replaced if the old signature is close to expiring (depending on the signatures-refresh option). Fix by maintaining a fullsign boolean value in the signing structure, that will ensure the RRsets are signed with the correct key, rather than a similar good key. In case of a fullsign, we can also remove signatures from inactive keys. Remove the unused dns_zone_signwithkey function. (cherry picked from commit 844bde0c70bca83f9f46d1b6356804c0a44e9dcd) --- lib/dns/include/dns/zone.h | 8 ----- lib/dns/zone.c | 65 +++++++++++++++++++++----------------- 2 files changed, 36 insertions(+), 37 deletions(-) diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index b83e447f01..e5d99e4d3f 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -2301,14 +2301,6 @@ dns_zone_getsignatures(dns_zone_t *zone); * Get the number of signatures that will be generated per quantum. */ -isc_result_t -dns_zone_signwithkey(dns_zone_t *zone, dns_secalg_t algorithm, uint16_t keyid, - bool deleteit); -/*%< - * Initiate/resume signing of the entire zone with the zone DNSKEY(s) - * that match the given algorithm and keyid. - */ - isc_result_t dns_zone_addnsec3chain(dns_zone_t *zone, dns_rdata_nsec3param_t *nsec3param); /*%< diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 1b20918f43..09cf734eaa 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -750,6 +750,7 @@ struct dns_signing { dns_secalg_t algorithm; uint16_t keyid; bool deleteit; + bool fullsign; bool done; ISC_LINK(dns_signing_t) link; }; @@ -992,7 +993,7 @@ static void dump_done(void *arg, isc_result_t result); static isc_result_t zone_signwithkey(dns_zone_t *zone, dns_secalg_t algorithm, uint16_t keyid, - bool deleteit); + bool deleteit, bool fullsign); static isc_result_t delete_nsec(dns_db_t *db, dns_dbversion_t *ver, dns_dbnode_t *node, dns_name_t *name, dns_diff_t *diff); @@ -3678,7 +3679,7 @@ resume_signingwithkey(dns_zone_t *zone) { result = zone_signwithkey(zone, rdata.data[0], (rdata.data[1] << 8) | rdata.data[2], - rdata.data[3]); + rdata.data[3], false); if (result != ISC_R_SUCCESS) { dnssec_log(zone, ISC_LOG_ERROR, "zone_signwithkey failed: %s", @@ -7505,7 +7506,7 @@ failure: static bool signed_with_good_key(dns_zone_t *zone, dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, dns_rdatatype_t type, - dst_key_t *key) { + dst_key_t *key, bool fullsign) { isc_result_t result; dns_rdataset_t rdataset; dns_rdata_t rdata = DNS_RDATA_INIT; @@ -7538,7 +7539,7 @@ signed_with_good_key(dns_zone_t *zone, dns_db_t *db, dns_dbnode_t *node, dns_rdata_reset(&rdata); } - if (zone->kasp != NULL) { + if (zone->kasp != NULL && !fullsign) { dns_kasp_key_t *kkey; int zsk_count = 0; bool approved; @@ -7650,8 +7651,9 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name, dns_dbnode_t *node, dns_dbversion_t *version, bool build_nsec3, bool build_nsec, dst_key_t *key, isc_stdtime_t now, isc_stdtime_t inception, isc_stdtime_t expire, dns_ttl_t nsecttl, - bool both, bool is_ksk, bool is_zsk, bool is_bottom_of_zone, - dns_diff_t *diff, int32_t *signatures, isc_mem_t *mctx) { + bool both, bool is_ksk, bool is_zsk, bool fullsign, + bool is_bottom_of_zone, dns_diff_t *diff, int32_t *signatures, + isc_mem_t *mctx) { isc_result_t result; dns_rdatasetiter_t *iterator = NULL; dns_rdataset_t rdataset; @@ -7762,7 +7764,7 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name, goto next_rdataset; } if (signed_with_good_key(zone, db, node, version, rdataset.type, - key)) + key, fullsign)) { goto next_rdataset; } @@ -9816,8 +9818,8 @@ zone_sign(dns_zone_t *zone) { db, zone, name, node, version, build_nsec3, build_nsec, zone_keys[i], now, inception, expire, zone_nsecttl(zone), both, is_ksk, - is_zsk, is_bottom_of_zone, zonediff.diff, - &signatures, zone->mctx)); + is_zsk, signing->fullsign, is_bottom_of_zone, + zonediff.diff, &signatures, zone->mctx)); /* * If we are adding we are done. Look for other keys * of the same algorithm if deleting. @@ -20412,22 +20414,6 @@ dns_zone_setnotifydelay(dns_zone_t *zone, uint32_t delay) { UNLOCK_ZONE(zone); } -isc_result_t -dns_zone_signwithkey(dns_zone_t *zone, dns_secalg_t algorithm, uint16_t keyid, - bool deleteit) { - isc_result_t result; - REQUIRE(DNS_ZONE_VALID(zone)); - - dnssec_log(zone, ISC_LOG_NOTICE, - "dns_zone_signwithkey(algorithm=%u, keyid=%u)", algorithm, - keyid); - LOCK_ZONE(zone); - result = zone_signwithkey(zone, algorithm, keyid, deleteit); - UNLOCK_ZONE(zone); - - return result; -} - /* * Called when a dynamic update for an NSEC3PARAM record is received. * @@ -20499,7 +20485,7 @@ dns_zone_getprivatetype(dns_zone_t *zone) { static isc_result_t zone_signwithkey(dns_zone_t *zone, dns_secalg_t algorithm, uint16_t keyid, - bool deleteit) { + bool deleteit, bool fullsign) { dns_signing_t *signing; dns_signing_t *current; isc_result_t result = ISC_R_SUCCESS; @@ -20514,6 +20500,7 @@ zone_signwithkey(dns_zone_t *zone, dns_secalg_t algorithm, uint16_t keyid, signing->algorithm = algorithm; signing->keyid = keyid; signing->deleteit = deleteit; + signing->fullsign = fullsign; signing->done = false; now = isc_time_now(); @@ -22952,7 +22939,7 @@ zone_rekey(dns_zone_t *zone) { { result = zone_signwithkey( zone, dst_key_alg(key->key), - dst_key_id(key->key), true); + dst_key_id(key->key), true, false); if (result != ISC_R_SUCCESS) { dnssec_log(zone, ISC_LOG_ERROR, "zone_signwithkey failed: " @@ -22992,7 +22979,7 @@ zone_rekey(dns_zone_t *zone) { result = zone_signwithkey( zone, dst_key_alg(key->key), - dst_key_id(key->key), false); + dst_key_id(key->key), false, true); if (result != ISC_R_SUCCESS) { dnssec_log(zone, ISC_LOG_ERROR, "zone_signwithkey failed: " @@ -23000,6 +22987,26 @@ zone_rekey(dns_zone_t *zone) { isc_result_totext(result)); } } + /* + * ...and remove signatures for all inactive keys. + */ + ISC_LIST_FOREACH(dnskeys, key, link) { + if (!key->force_sign && !key->hint_sign) { + result = zone_signwithkey( + zone, dst_key_alg(key->key), + dst_key_id(key->key), true, + false); + if (result != ISC_R_SUCCESS) { + dnssec_log(zone, ISC_LOG_ERROR, + "zone_signwithkey " + "failed: " + "%s", + isc_result_totext( + result)); + } + } + } + } else if (newalg) { /* * We haven't been told to sign fully, but a new @@ -23016,7 +23023,7 @@ zone_rekey(dns_zone_t *zone) { result = zone_signwithkey( zone, dst_key_alg(key->key), - dst_key_id(key->key), false); + dst_key_id(key->key), false, false); if (result != ISC_R_SUCCESS) { dnssec_log(zone, ISC_LOG_ERROR, "zone_signwithkey failed: " From aea6f4f18b5ebcc8a897c9f354fd1855aa4dc3e0 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 19 Aug 2025 15:10:59 +0200 Subject: [PATCH 3/4] Test the next key event after full sign After a full sign we no longer have to need to take the sign delay into account. (cherry picked from commit acbf110b1828946341ac22d568dacd4dfff9c387) --- bin/tests/system/isctest/kasp.py | 9 ++++++--- .../tests_rollover_zsk_prepublication.py | 3 ++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/bin/tests/system/isctest/kasp.py b/bin/tests/system/isctest/kasp.py index 4f22193fed..e95dc2174e 100644 --- a/bin/tests/system/isctest/kasp.py +++ b/bin/tests/system/isctest/kasp.py @@ -72,11 +72,12 @@ def IpubC(config, rollover=True): return config["zone-propagation-delay"] + max(ttl1, ttl2) -def Iret(config, zsk=True, ksk=False, rollover=True): +def Iret(config, zsk=True, ksk=False, rollover=True, smooth=True): sign_delay = timedelta(0) safety_interval = timedelta(0) if rollover: - sign_delay = config["signatures-validity"] - config["signatures-refresh"] + if smooth: + sign_delay = config["signatures-validity"] - config["signatures-refresh"] safety_interval = config["retire-safety"] iretKSK = timedelta(0) @@ -246,7 +247,9 @@ class KeyProperties: if "Lifetime" not in self.metadata or self.metadata["Lifetime"] == 0: return - iret = Iret(config, zsk=self.key.is_zsk(), ksk=self.key.is_ksk()) + sigdel = self.key.get_timing("SigRemoved", must_exist=False) + smooth = sigdel is None + iret = Iret(config, zsk=self.key.is_zsk(), ksk=self.key.is_ksk(), smooth=smooth) self.timing["Removed"] = self.timing["Retired"] + iret def set_expected_keytimes( diff --git a/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py b/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py index e5d842c535..702c0f26ad 100644 --- a/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py +++ b/bin/tests/system/rollover-zsk-prepub/tests_rollover_zsk_prepublication.py @@ -41,7 +41,7 @@ POLICY = "zsk-prepub" ZSK_LIFETIME = TIMEDELTA["P30D"] LIFETIME_POLICY = int(ZSK_LIFETIME.total_seconds()) IPUB = Ipub(CONFIG) -IRET = Iret(CONFIG, rollover=True) +IRET = Iret(CONFIG) KEYTTLPROP = CONFIG["dnskey-ttl"] + CONFIG["zone-propagation-delay"] OFFSETS = {} OFFSETS["step1-p"] = -int(TIMEDELTA["P7D"].total_seconds()) @@ -228,6 +228,7 @@ def test_zsk_prepub_step3(tld, alg, size, ns3): watcher.wait_for_line(f"zone {zone}/IN (signed): sending notifies") step["smooth"] = False + step["nextev"] = Iret(CONFIG, smooth=False) isctest.kasp.check_rollover_step(ns3, CONFIG, POLICY, step) From bd13d0d2af7d697c3986f91d0610e73ea76ddedd Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 19 Aug 2025 15:16:39 +0200 Subject: [PATCH 4/4] Update the retire interval after full sign After a full sign we no longer have to need to take the sign delay into account. Update the timing checks in keymgr_transition_time to determine the start of the interval: Either the last change, or if SigPublish/ SigDelete is set. The latter case indicates a full sign was done and so we no longer have to take the sign delay into account. (cherry picked from commit 489752eb1f7f4e09bcf96d723ae64158cf785f97) --- lib/dns/dst_api.c | 4 ++- lib/dns/dst_parse.c | 7 +++-- lib/dns/include/dns/keymgr.h | 3 ++- lib/dns/include/dst/dst.h | 4 ++- lib/dns/keymgr.c | 52 +++++++++++++++++++++++++++++++++--- lib/dns/zone.c | 3 +++ 6 files changed, 62 insertions(+), 11 deletions(-) diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c index 11edd56994..c823fdac5f 100644 --- a/lib/dns/dst_api.c +++ b/lib/dns/dst_api.c @@ -117,7 +117,7 @@ static const char *timingtags[TIMING_NTAGS] = { "DNSKEYChange:", "ZRRSIGChange:", "KRRSIGChange:", "DSChange:", - "DSRemoved:" + "DSRemoved:", "ZRRSIGPublish", "ZRRSIGRemoved" }; #define KEYSTATES_NTAGS (DST_MAX_KEYSTATES + 1) @@ -2116,6 +2116,8 @@ write_key_state(const dst_key_t *key, int type, const char *directory) { printtime(key, DST_TIME_DELETE, "Removed", fp); printtime(key, DST_TIME_DSPUBLISH, "DSPublish", fp); printtime(key, DST_TIME_DSDELETE, "DSRemoved", fp); + printtime(key, DST_TIME_SIGPUBLISH, "SigPublish", fp); + printtime(key, DST_TIME_SIGDELETE, "SigRemoved", fp); printtime(key, DST_TIME_SYNCPUBLISH, "PublishCDS", fp); printtime(key, DST_TIME_SYNCDELETE, "DeleteCDS", fp); diff --git a/lib/dns/dst_parse.c b/lib/dns/dst_parse.c index eb2a116b19..a6967ad47e 100644 --- a/lib/dns/dst_parse.c +++ b/lib/dns/dst_parse.c @@ -54,10 +54,9 @@ #define TIMING_NTAGS (DST_MAX_TIMES + 1) static const char *timetags[TIMING_NTAGS] = { - "Created:", "Publish:", "Activate:", "Revoke:", - "Inactive:", "Delete:", "DSPublish:", "SyncPublish:", - "SyncDelete:", NULL, NULL, NULL, - NULL + "Created:", "Publish:", "Activate:", "Revoke:", "Inactive:", + "Delete:", "DSPublish:", "SyncPublish:", "SyncDelete:", NULL, + NULL, NULL, NULL, NULL, NULL }; #define NUMERIC_NTAGS (DST_MAX_NUMERIC + 1) diff --git a/lib/dns/include/dns/keymgr.h b/lib/dns/include/dns/keymgr.h index a0508f1f5f..30d05b78cd 100644 --- a/lib/dns/include/dns/keymgr.h +++ b/lib/dns/include/dns/keymgr.h @@ -27,7 +27,8 @@ ISC_LANG_BEGINDECLS #define DNS_KEYMGRATTR_NONE 0x00 /*%< No ordering. */ #define DNS_KEYMGRATTR_S2I 0x01 /*%< Secure to insecure. */ #define DNS_KEYMGRATTR_NOROLL 0x02 /*%< No rollover allowed. */ -#define DNS_KEYMGRATTR_FORCESTEP 0x04 /*%< Force next step in manual-mode */ +#define DNS_KEYMGRATTR_FORCESTEP 0x04 /*%< Force next step in manual-mode. */ +#define DNS_KEYMGRATTR_FULLSIGN 0x08 /*%< Full sign was issued. */ void dns_keymgr_settime_syncpublish(dst_key_t *key, dns_kasp_t *kasp, bool first); diff --git a/lib/dns/include/dst/dst.h b/lib/dns/include/dst/dst.h index a08a6092fa..1a6920753c 100644 --- a/lib/dns/include/dst/dst.h +++ b/lib/dns/include/dst/dst.h @@ -147,7 +147,9 @@ typedef enum dst_algorithm { #define DST_TIME_KRRSIG 11 #define DST_TIME_DS 12 #define DST_TIME_DSDELETE 13 -#define DST_MAX_TIMES 13 +#define DST_TIME_SIGPUBLISH 14 +#define DST_TIME_SIGDELETE 15 +#define DST_MAX_TIMES 15 /* Numeric metadata definitions */ #define DST_NUM_PREDECESSOR 0 diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c index 72edb27c66..4f8f7cad76 100644 --- a/lib/dns/keymgr.c +++ b/lib/dns/keymgr.c @@ -1314,9 +1314,9 @@ keymgr_transition_time(dns_dnsseckey_t *key, int type, dst_key_state_t next_state, dns_kasp_t *kasp, isc_stdtime_t now, isc_stdtime_t *when) { isc_result_t ret; - isc_stdtime_t lastchange, dstime, nexttime = now; + isc_stdtime_t lastchange, dstime, sigtime, nexttime = now; dns_ttl_t ttlsig = dns_kasp_zonemaxttl(kasp, true); - uint32_t dsstate; + uint32_t dsstate, sigstate, signdelay = 0; /* * No need to wait if we move things into an uncertain state. @@ -1370,6 +1370,17 @@ keymgr_transition_time(dns_dnsseckey_t *key, int type, switch (next_state) { case OMNIPRESENT: case HIDDEN: + /* Was there a full sign? */ + sigstate = (next_state == HIDDEN) ? DST_TIME_SIGDELETE + : DST_TIME_SIGPUBLISH; + ret = dst_key_gettime(key->key, sigstate, &sigtime); + if (ret == ISC_R_SUCCESS && sigtime <= now) { + signdelay = 0; + } else { + sigtime = lastchange; + signdelay = dns_kasp_signdelay(kasp); + } + /* * RFC 7583: The retire interval (Iret) is the amount * of time that must elapse after a DNSKEY or @@ -1387,7 +1398,7 @@ keymgr_transition_time(dns_dnsseckey_t *key, int type, * * Dsgn + zone-propagation-delay + max-zone-ttl. */ - nexttime = lastchange + ttlsig + + nexttime = sigtime + ttlsig + dns_kasp_zonepropagationdelay(kasp); /* * Only add the sign delay Dsgn and retire-safety if @@ -1401,7 +1412,7 @@ keymgr_transition_time(dns_dnsseckey_t *key, int type, DST_NUM_SUCCESSOR, &tag); } if (ret == ISC_R_SUCCESS) { - nexttime += dns_kasp_signdelay(kasp) + + nexttime += signdelay + dns_kasp_retiresafety(kasp); } break; @@ -2137,6 +2148,34 @@ dst_key_doublematch(dns_dnsseckey_t *key, dns_kasp_t *kasp) { return matches > 1; } +static void +keymgr_zrrsig(dns_dnsseckeylist_t *keyring, isc_stdtime_t now) { + for (dns_dnsseckey_t *dkey = ISC_LIST_HEAD(*keyring); dkey != NULL; + dkey = ISC_LIST_NEXT(dkey, link)) + { + isc_result_t ret; + bool zsk = false; + + ret = dst_key_getbool(dkey->key, DST_BOOL_ZSK, &zsk); + if (ret == ISC_R_SUCCESS && zsk) { + dst_key_state_t state; + isc_result_t result = dst_key_getstate( + dkey->key, DST_KEY_ZRRSIG, &state); + if (result == ISC_R_SUCCESS) { + if (state == RUMOURED) { + dst_key_settime(dkey->key, + DST_TIME_SIGPUBLISH, + now); + } else if (state == UNRETENTIVE) { + dst_key_settime(dkey->key, + DST_TIME_SIGDELETE, + now); + } + } + } + } +} + /* * Examine 'keys' and match 'kasp' policy. * @@ -2359,6 +2398,11 @@ dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass, opts |= DNS_KEYMGRATTR_S2I; } + /* In case of a full sign, store ZRRSIGPublish/ZRRSIGDelete. */ + if ((opts & DNS_KEYMGRATTR_FULLSIGN) != 0) { + keymgr_zrrsig(keyring, now); + } + /* Read to update key states. */ isc_result_t retval = keymgr_update(keyring, kasp, now, nexttime, opts); diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 09cf734eaa..96fdf6b66d 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -22553,6 +22553,9 @@ zone_rekey(dns_zone_t *zone) { * fully signed now. */ fullsign = DNS_ZONEKEY_OPTION(zone, DNS_ZONEKEY_FULLSIGN); + if (fullsign) { + options |= DNS_KEYMGRATTR_FULLSIGN; + } /* * True when called from "rndc dnssec -step". Indicates the zone