fix handling of revoked keys

when a key is revoked its key ID changes, due to the inclusion
of the "revoke" flag. a collision between this changed key ID and
that of an unrelated public-only key could cause a crash in
dnssec-signzone.

diff --git a/bin/tests/system/dnssec/signer/general/K.+013+23640.key b/bin/tests/system/dnssec/signer/general/K.+013+23640.key
new file mode 100644 (file)
index 0000000..df4ff32
--- /dev/null
+++ b/bin/tests/system/dnssec/signer/general/K.+013+23640.key
@@ -0,0 +1,6 @@
+; This is a key-signing key, keyid 23640, for .
+; Created: 20250310185208 (Mon Mar 10 18:52:08 2025)
+; Publish: 20250310185208 (Mon Mar 10 18:52:08 2025)
+; Activate: 20250310185208 (Mon Mar 10 18:52:08 2025)
+; Revoke: 20250310185208 (Mon Mar 10 18:52:08 2025)
+. IN DNSKEY 257 3 13 uKwpRtMH+9iuUk/Xj6LciIP5ZckaBtXaUqxUxzJYexXjvxGZGX4470Jv hq2NCI3HBZQNaCCP/h9sluhIzRGPTA==

diff --git a/bin/tests/system/dnssec/signer/general/K.+013+23640.private b/bin/tests/system/dnssec/signer/general/K.+013+23640.private
new file mode 100644 (file)
index 0000000..36f932f
--- /dev/null
+++ b/bin/tests/system/dnssec/signer/general/K.+013+23640.private
@@ -0,0 +1,7 @@
+Private-key-format: v1.3
+Algorithm: 13 (ECDSAP256SHA256)
+PrivateKey: m5udfGNSijISQ8Tfp4kx09O1em4PErLUw/mCj3SKmqw=
+Created: 20250310185208
+Publish: 20250310185208
+Activate: 20250310185208
+Revoke: 20250310185208

diff --git a/bin/tests/system/dnssec/signer/general/K.+013+23768.key b/bin/tests/system/dnssec/signer/general/K.+013+23768.key
new file mode 100644 (file)
index 0000000..85e460a
--- /dev/null
+++ b/bin/tests/system/dnssec/signer/general/K.+013+23768.key
@@ -0,0 +1,5 @@
+; This is a zone-signing key, keyid 23768, for .
+; Created: 20250310185208 (Mon Mar 10 18:52:08 2025)
+; Publish: 20250310185208 (Mon Mar 10 18:52:08 2025)
+; Activate: 20250310185208 (Mon Mar 10 18:52:08 2025)
+. IN DNSKEY 256 3 13 TFelYtTRBWeA9A307vvuWIcaNwW4txW4RgSELtsi46ZQs24ncRxmxtFf uJuPyVXePNiE4HNI9CIowGUsn5WuBw==

diff --git a/bin/tests/system/dnssec/signer/general/test12.zone b/bin/tests/system/dnssec/signer/general/test12.zone
new file mode 100644 (file)
index 0000000..4e4c9ba
--- /dev/null
+++ b/bin/tests/system/dnssec/signer/general/test12.zone
@@ -0,0 +1,37 @@
+; 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.
+
+;      This is a zone which has two DNSKEY records, both of which have
+; existing private key files available.  They should be loaded automatically
+; and the zone correctly signed.
+;
+$TTL 30        ; 30 seconds
+.                      IN SOA  a.root.servers.nil. each.isc.org. (
+                               2000042101 ; serial
+                               600        ; refresh (10 minutes)
+                               600        ; retry (10 minutes)
+                               1200       ; expire (20 minutes)
+                               600        ; minimum (10 minutes)
+                               )
+                       NS      a.root-servers.nil.
+                       DNSKEY  256 3 13 (
+                               TFelYtTRBWeA9A307vvuWIcaNwW4txW4RgSELtsi46ZQ
+                               s24ncRxmxtFfuJuPyVXePNiE4HNI9CIowGUsn5WuBw==
+                               ) ; ZSK; alg = ECDSAP256SHA256 ; key id = 23768
+                       DNSKEY  257 3 13 (
+                               OSmhpULEDCUzHCBeDU5uJXzkCcGuW2qrkQznKRPGhRZN
+                               j7ZUIGInGzM5Um5m02ULWt8tKbi55NJUeifKWegQ0g==
+                               ) ; KSK; alg = ECDSAP256SHA256 ; key id = 22255
+                       DNSKEY  385 3 13 (
+                               uKwpRtMH+9iuUk/Xj6LciIP5ZckaBtXaUqxUxzJYexXj
+                               vxGZGX4470Jvhq2NCI3HBZQNaCCP/h9sluhIzRGPTA==
+                               ) ; revoked KSK; alg = ECDSAP256SHA256 ; key id = 23768
+a.root-servers.nil.    A       10.53.0.1

diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh
index c389c5f6c7a107b64d334f9b12d863df089210ee..09c8be1250bdb4c01cfb516c35f19368e0e89807 100644 (file)
--- a/bin/tests/system/dnssec/tests.sh
+++ b/bin/tests/system/dnssec/tests.sh
@@ -1564,6 +1564,18 @@ n=$((n + 1))
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status + ret))
 
+echo_ic "revoked KSK ID collides with ZSK ($n)"
+ret=0
+# signing should fail, but should not coredump
+(
+  cd signer/general || exit 0
+  rm -f signed.zone
+  $SIGNER -S -f signed.zone -o . test12.zone >signer.out.$n
+) && ret=1
+n=$((n + 1))
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status + ret))
+
 echo_ic "check that dnssec-signzone rejects excessive NSEC3 iterations ($n)"
 ret=0
 (

diff --git a/lib/dns/dnssec.c b/lib/dns/dnssec.c
index 2220dbb233f9358aad987ce79c37e90e0d5cd4a8..b609f60f663c8d7c28e60342e0dbb6d4cd7e11a2 100644 (file)
--- a/lib/dns/dnssec.c
+++ b/lib/dns/dnssec.c
@@ -1428,29 +1428,35 @@ addkey(dns_dnsseckeylist_t *keylist, dst_key_t **newkey, bool savekeys,
 
        if (key != NULL) {
                /*
-                * Found a match.  If the old key was only public and the
-                * new key is private, replace the old one; otherwise
-                * leave it.  But either way, mark the key as having
-                * been found in the zone.
+                * Found a match. If we already had a private key, then
+                * the new key can't be an improvement. If the existing
+                * key was public-only but the new key is too, then it's
+                * still not an improvement. Mark the old key as having
+                * been found in the zone and stop.
                 */
-               if (dst_key_isprivate(key->key)) {
-                       dst_key_free(newkey);
-               } else if (dst_key_isprivate(*newkey)) {
-                       dst_key_free(&key->key);
-                       key->key = *newkey;
+               if (dst_key_isprivate(key->key) || !dst_key_isprivate(*newkey))
+               {
+                       key->source = dns_keysource_zoneapex;
+                       return;
                }
 
-               key->source = dns_keysource_zoneapex;
-               return;
+               /*
+                * However, if the old key was public-only, and the new key
+                * is private, then we're throwing away the old key.
+                */
+               dst_key_free(&key->key);
+               ISC_LIST_UNLINK(*keylist, key, link);
+               dns_dnsseckey_destroy(mctx, &key);
        }
 
+       /* Store the new key. */
        dns_dnsseckey_create(mctx, newkey, &key);
+       key->source = dns_keysource_zoneapex;
        key->pubkey = pubkey_only;
        if (key->legacy || savekeys) {
                key->force_publish = true;
                key->force_sign = dst_key_isprivate(key->key);
        }
-       key->source = dns_keysource_zoneapex;
        ISC_LIST_APPEND(*keylist, key, link);
        *newkey = NULL;
 }