]> 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 09:43:07 +0000 (09:43 +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).

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

index fcdfe15e046c2be1d9f3c647be3846d41d6c2ab4..748f28f3bcd5ae7906b6144c56009376e58396de 100644 (file)
@@ -419,10 +419,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);
 
@@ -540,7 +539,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);
@@ -551,8 +550,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 */
@@ -581,15 +582,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 "
@@ -602,25 +609,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 1c3876e0fba1c6b4cb37c2d1958544e184e93e59..400d9db80622fe621a772b701e600b64de9a2a3c 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 dcc418f955145006ffd50c7b4fd74537fac92203..ef3aefd3d747289e4d8541d512e57f0371b52537 100644 (file)
@@ -1121,6 +1121,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;
@@ -1409,7 +1410,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 */
@@ -1444,6 +1445,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);
@@ -1584,7 +1586,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;
                }
 
@@ -1672,9 +1674,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;
                }
@@ -1691,7 +1693,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 09281eb9cc0448c07b9f5e730289e5cc09695fb9..daacd69a5c368d9bb123871992485a0a352dca75 100644 (file)
@@ -68,6 +68,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;
 };