]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
dnssec-signzone retain signature if key is offline
authorMatthijs Mekking <matthijs@isc.org>
Tue, 14 Jan 2025 14:18:40 +0000 (15:18 +0100)
committerMatthijs Mekking <matthijs@isc.org>
Thu, 23 Jan 2025 14:04:03 +0000 (14:04 +0000)
Track inside the dns_dnsseckey structure whether we have seen the
private key, or if this key only has a public key file.

If the key only has a public key file, or a DNSKEY reference in the
zone, mark the key 'pubkey'. In dnssec-signzone, if the key only
has a public key available, consider the key to be offline. Any
signatures that should be refreshed for which the key is not available,
retain the signature.

So in the code, 'expired' becomes 'refresh', and the new 'expired'
is only used to determine whether we need to keep the signature if
the corresponding key is not available (retaining the signature if
it is not expired).

In the 'keysthatsigned' function, we can remove:
  - key->force_publish = false;
  - key->force_sign = false;

because they are redundant ('dns_dnsseckey_create' already sets these
values to false).

(cherry picked from commit 5e3aef364fc3c6382a5761eb339ca780df0829ef)

bin/dnssec/dnssec-signzone.c
bin/tests/system/dnssec/tests.sh
lib/dns/dnssec.c
lib/dns/include/dns/dnssec.h

index 0cd92cd7459b1f9be7533d1c7713354208a8476e..7e906dfd1de574e3520718fc9709c6bcbbc00ec3 100644 (file)
@@ -420,10 +420,9 @@ keythatsigned(dns_rdata_rrsig_t *rrsig) {
                dns_dnsseckey_create(mctx, &privkey, &key);
        } else {
                dns_dnsseckey_create(mctx, &pubkey, &key);
+               key->pubkey = true;
        }
 
-       key->force_publish = false;
-       key->force_sign = false;
        key->index = keycount++;
        ISC_LIST_APPEND(keylist, key, link);
 
@@ -541,7 +540,7 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
        }
 
        while (result == ISC_R_SUCCESS) {
-               bool expired, future;
+               bool expired, refresh, future, offline;
                bool keep = false, resign = false;
 
                dns_rdataset_current(&sigset, &sigrdata);
@@ -552,8 +551,10 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
                future = isc_serial_lt(now, rrsig.timesigned);
 
                key = keythatsigned(&rrsig);
+               offline = key->pubkey;
                sig_format(&rrsig, sigstr, sizeof(sigstr));
-               expired = isc_serial_gt(now + cycle, rrsig.timeexpire);
+               expired = isc_serial_gt(now, rrsig.timeexpire);
+               refresh = isc_serial_gt(now + cycle, rrsig.timeexpire);
 
                if (isc_serial_gt(rrsig.timesigned, rrsig.timeexpire)) {
                        /* rrsig is dropped and not replaced */
@@ -582,15 +583,21 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
                } else if (issigningkey(key)) {
                        wassignedby[key->index] = true;
 
-                       if (!expired && rrsig.originalttl == set->ttl &&
+                       if (!refresh && rrsig.originalttl == set->ttl &&
                            setverifies(name, set, key->key, &sigrdata))
                        {
                                vbprintf(2, "\trrsig by %s retained\n", sigstr);
                                keep = true;
+                       } else if (offline) {
+                               vbprintf(2,
+                                        "\trrsig by %s retained - private key "
+                                        "missing\n",
+                                        sigstr);
+                               keep = true;
                        } else {
                                vbprintf(2, "\trrsig by %s dropped - %s\n",
                                         sigstr,
-                                        expired ? "expired"
+                                        refresh ? "refresh"
                                         : rrsig.originalttl != set->ttl
                                                 ? "ttl change"
                                                 : "failed to "
@@ -603,25 +610,32 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
                } else if (iszonekey(key)) {
                        wassignedby[key->index] = true;
 
-                       if (!expired && rrsig.originalttl == set->ttl &&
+                       if (!refresh && rrsig.originalttl == set->ttl &&
                            setverifies(name, set, key->key, &sigrdata))
                        {
                                vbprintf(2, "\trrsig by %s retained\n", sigstr);
                                keep = true;
+                       } else if (offline) {
+                               vbprintf(2,
+                                        "\trrsig by %s retained - private key "
+                                        "missing\n",
+                                        sigstr);
+                               keep = true;
                        } else {
                                vbprintf(2, "\trrsig by %s dropped - %s\n",
                                         sigstr,
-                                        expired ? "expired"
+                                        refresh ? "refresh"
                                         : rrsig.originalttl != set->ttl
                                                 ? "ttl change"
                                                 : "failed to "
                                                   "verify");
                        }
-               } else if (!expired) {
+               } else if (!refresh) {
                        vbprintf(2, "\trrsig by %s retained\n", sigstr);
                        keep = true;
                } else {
-                       vbprintf(2, "\trrsig by %s expired\n", sigstr);
+                       vbprintf(2, "\trrsig by %s %s\n", sigstr,
+                                expired ? "expired" : "needs refresh");
                }
 
                if (keep) {
index 84e08d2669866ddeefabafbc5a3bc5c630753664..37fc64de2951d001f35d09cfb341b395f288a867 100644 (file)
@@ -1361,14 +1361,14 @@ status=$((status + ret))
 echo_ic "two DNSKEYs, DNSKEY RRset only by KSK ($n)"
 ret=0
 (
-cd signer/general || exit 1
-rm -f signed.zone
-$SIGNER -s now-1mo -e now+2d -P -x -f signed.zone -O full -o example.com. test1.zone >signer.out.$n
-test -f signed.zone
+  cd signer/general || exit 1
+  rm -f signed.zone
+  $SIGNER -s now-1mo -e now+2d -P -x -f signed.zone -O full -o example.com. test1.zone >signer.out.$n
+  test -f signed.zone
 ) || ret=1
-n=$((n+1))
+n=$((n + 1))
 test "$ret" -eq 0 || echo_i "failed"
-status=$((status+ret))
+status=$((status + ret))
 
 echo_ic "two DNSKEYs, DNSKEY RRset only by KSK, private key missing ($n)"
 ret=0
index 4b2261d17ee3db868eeedc1cf6b2767ee4490398..f80f317eea70c1eba064790da73a4f92f334d43d 100644 (file)
@@ -1123,6 +1123,7 @@ dns_dnsseckey_create(isc_mem_t *mctx, dst_key_t **dstkey,
        dk->hint_remove = false;
        dk->first_sign = false;
        dk->is_active = false;
+       dk->pubkey = false;
        dk->purge = false;
        dk->prepublish = 0;
        dk->source = dns_keysource_unknown;
@@ -1411,7 +1412,7 @@ failure:
  */
 static void
 addkey(dns_dnsseckeylist_t *keylist, dst_key_t **newkey, bool savekeys,
-       isc_mem_t *mctx) {
+       bool pubkey_only, isc_mem_t *mctx) {
        dns_dnsseckey_t *key = NULL;
 
        /* Skip duplicates */
@@ -1446,6 +1447,7 @@ addkey(dns_dnsseckeylist_t *keylist, dst_key_t **newkey, bool savekeys,
        }
 
        dns_dnsseckey_create(mctx, newkey, &key);
+       key->pubkey = pubkey_only;
        if (key->legacy || savekeys) {
                key->force_publish = true;
                key->force_sign = dst_key_isprivate(key->key);
@@ -1586,7 +1588,7 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, dns_kasp_t *kasp,
                }
 
                if (publickey) {
-                       addkey(keylist, &dnskey, savekeys, mctx);
+                       addkey(keylist, &dnskey, savekeys, true, mctx);
                        goto skip;
                }
 
@@ -1674,9 +1676,9 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, dns_kasp_t *kasp,
        addkey:
                if (result == ISC_R_FILENOTFOUND || result == ISC_R_NOPERM) {
                        if (pubkey != NULL) {
-                               addkey(keylist, &pubkey, savekeys, mctx);
+                               addkey(keylist, &pubkey, savekeys, true, mctx);
                        } else {
-                               addkey(keylist, &dnskey, savekeys, mctx);
+                               addkey(keylist, &dnskey, savekeys, false, mctx);
                        }
                        goto skip;
                }
@@ -1693,7 +1695,7 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, dns_kasp_t *kasp,
                 */
                dst_key_setttl(privkey, dst_key_getttl(dnskey));
 
-               addkey(keylist, &privkey, savekeys, mctx);
+               addkey(keylist, &privkey, savekeys, false, mctx);
        skip:
                if (dnskey != NULL) {
                        dst_key_free(&dnskey);
index 3525672dcff5313301e144d82407bbc6a3d539fe..55e7fb842b90db04f4a02e836482df940715383f 100644 (file)
@@ -71,6 +71,7 @@ struct dns_dnsseckey {
                                      *  an older version of BIND9) and
                                      *  should be ignored when searching
                                      *  for keys to import into the zone */
+       bool         pubkey;         /*% public key only */
        unsigned int index;          /*% position in list */
        ISC_LINK(dns_dnsseckey_t) link;
 };