3635. [bug] Signatures were not being removed from a zone with

                        only KSK keys for a algorithm. [RT #24439]

(cherry picked from commit d1e22676de16e6dee54c58b27cca11c5fb8f1ff5)

diff --git a/CHANGES b/CHANGES
index 3be22a95bf8bb4199d33d41af994229c1719bd06..56d80f87ee4d0c8edfb4b231b0cdcaae1b3d9ad5 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+3635.  [bug]           Signatures were not being removed from a zone with
+                       only KSK keys for a algorithm. [RT #24439]
+
 3634.  [func]          Report build-id in rndc status. Report build-id
                        when building from a git repository. [RT #20422]
 

diff --git a/bin/tests/system/autosign/tests.sh b/bin/tests/system/autosign/tests.sh
index 899d20b7dd32309d7762b5a847d1077ae5e118cf..4774817b237141c6d87f4f422a0b653ce18c0cde 100644 (file)
--- a/bin/tests/system/autosign/tests.sh
+++ b/bin/tests/system/autosign/tests.sh
@@ -44,14 +44,18 @@ showprivate () {
 
 # check that signing records are marked as complete
 checkprivate () {
-    ret=0
+    _ret=0
+    expected="${3:-0}"
     x=`showprivate "$@"`
-    echo $x | grep incomplete >&- 2>&- && ret=1
-    [ $ret = 1 ] && {
-        echo "$x"
-        echo "I:failed"
-    }
-    return $ret
+    echo $x | grep incomplete > /dev/null && _ret=1
+
+    if [ $_ret = $expected ]; then
+        return 0
+    fi
+
+    echo "$x"
+    echo "I:failed"
+    return 1
 }
 
 #
@@ -161,6 +165,7 @@ ret=0
 missing=`sed 's/^K.*+007+0*\([0-9]\)/\1/' < missingzsk.key`
 $JOURNALPRINT ns3/nozsk.example.db.jnl | \
    awk '{if ($1 == "del" && $5 == "RRSIG" && $12 == id) {exit 1}} END {exit 0}' id=$missing || ret=1
+n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`
 
@@ -169,26 +174,23 @@ ret=0
 inactive=`sed 's/^K.*+007+0*\([0-9]\)/\1/' < inactivezsk.key`
 $JOURNALPRINT ns3/inaczsk.example.db.jnl | \
    awk '{if ($1 == "del" && $5 == "RRSIG" && $12 == id) {exit 1}} END {exit 0}' id=$inactive || ret=1
+n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`
 
-echo "I:checking that non-replaceable RRSIGs are logged only once ($n)"
+echo "I:checking that non-replaceable RRSIGs are logged only once (missing private key) ($n)"
 ret=0
 loglines=`grep "Key nozsk.example/NSEC3RSASHA1/$missing .* retaining signatures" ns3/named.run | wc -l`
 [ "$loglines" -eq 1 ] || ret=1
-loglines=`grep "Key inaczsk.example/NSEC3RSASHA1/$missing .* retaining signatures" ns3/named.run | wc -l`
-[ "$loglines" -eq 1 ] || ret=1
+n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`
 
-# This test is above the rndc freeze/thaw calls because the apex node
-# will be resigned on thaw, increasing the serial number again.
-echo "I:checking serial is not incremented when signatures are unchanged ($n)"
+echo "I:checking that non-replaceable RRSIGs are logged only once (inactive private key) ($n)"
 ret=0
-newserial=`$DIG $DIGOPTS +short soa nozsk.example @10.53.0.3 | awk '$0 !~ /SOA/ {print $3}'`
-[ "$newserial" -eq 2 ] || ret=1
-newserial=`$DIG $DIGOPTS +short soa inaczsk.example @10.53.0.3 | awk '$0 !~ /SOA/ {print $3}'`
-[ "$newserial" -eq 2 ] || ret=1
+loglines=`grep "Key inaczsk.example/NSEC3RSASHA1/$inactive .* retaining signatures" ns3/named.run | wc -l`
+[ "$loglines" -eq 1 ] || ret=1
+n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`
 
@@ -825,7 +827,7 @@ checkprivate oldsigs.example 10.53.0.3 || ret=1
 checkprivate optout.example 10.53.0.3 || ret=1
 checkprivate optout.nsec3.example 10.53.0.3 || ret=1
 checkprivate optout.optout.example 10.53.0.3 || ret=1
-checkprivate prepub.example 10.53.0.3 || ret=1
+checkprivate prepub.example 10.53.0.3 1 || ret=1
 checkprivate rsasha256.example 10.53.0.3 || ret=1
 checkprivate rsasha512.example 10.53.0.3 || ret=1
 checkprivate secure.example 10.53.0.3 || ret=1

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index a654167e989902459ad48e9ad783f9cf895f3a5a..726fe41aaadcbff91264a5d1e2227d333f6550c7 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -686,8 +686,6 @@ 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);
 static void zone_rekey(dns_zone_t *zone);
-static isc_boolean_t delsig_ok(dns_rdata_rrsig_t *rrsig_ptr,
-                              dst_key_t **keys, unsigned int nkeys);
 
 #define ENTER zone_debuglog(zone, me, 1, "enter")
 
@@ -4743,19 +4741,39 @@ set_key_expiry_warning(dns_zone_t *zone, isc_stdtime_t when, isc_stdtime_t now)
  * have no new key.
  */
 static isc_boolean_t
-delsig_ok(dns_rdata_rrsig_t *rrsig_ptr, dst_key_t **keys, unsigned int nkeys) {
+delsig_ok(dns_rdata_rrsig_t *rrsig_ptr, dst_key_t **keys, unsigned int nkeys,
+         isc_boolean_t *warn)
+{
        unsigned int i = 0;
+       isc_boolean_t have_ksk = ISC_FALSE, have_zsk = ISC_FALSE;
+       isc_boolean_t have_pksk = ISC_FALSE, have_pzsk = ISC_FALSE;
 
-       /*
-        * It's okay to delete a signature if there is an active ZSK
-        * with the same algorithm
-        */
        for (i = 0; i < nkeys; i++) {
-               if (rrsig_ptr->algorithm == dst_key_alg(keys[i]) &&
-                   (dst_key_isprivate(keys[i])) && !KSK(keys[i]))
-                       return (ISC_TRUE);
+               if (rrsig_ptr->algorithm != dst_key_alg(keys[i]))
+                       continue;
+               if (dst_key_isprivate(keys[i])) {
+                       if (KSK(keys[i]))
+                               have_ksk = have_pksk = ISC_TRUE;
+                       else
+                               have_zsk = have_pzsk = ISC_TRUE;
+               } else {
+                       if (KSK(keys[i]))
+                               have_ksk = ISC_TRUE;
+                       else
+                               have_zsk = ISC_TRUE;
+               }
        }
 
+       if (have_zsk && have_ksk && !have_pzsk)
+               *warn = ISC_TRUE;
+
+       /*
+        * It's okay to delete a signature if there is an active key
+        * with the same algorithm to replace it.
+        */
+       if (have_pksk || have_pzsk)
+               return (ISC_TRUE);
+
        /*
         * Failing that, it is *not* okay to delete a signature
         * if the associated public key is still in the DNSKEY RRset
@@ -4823,7 +4841,8 @@ del_sigs(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
                RUNTIME_CHECK(result == ISC_R_SUCCESS);
 
                if (type != dns_rdatatype_dnskey) {
-                       if (delsig_ok(&rrsig, keys, nkeys)) {
+                       isc_boolean_t warn = ISC_FALSE, deleted = ISC_FALSE;
+                       if (delsig_ok(&rrsig, keys, nkeys, &warn)) {
                                result = update_one_rr(db, ver, zonediff->diff,
                                               DNS_DIFFOP_DELRESIGN, name,
                                               rdataset.ttl, &rdata);
@@ -4831,7 +4850,9 @@ del_sigs(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
                                        changed = ISC_TRUE;
                                if (result != ISC_R_SUCCESS)
                                        break;
-                       } else {
+                               deleted = ISC_TRUE;
+                       }
+                       if (warn) {
                                /*
                                 * At this point, we've got an RRSIG,
                                 * which is signed by an inactive key.
@@ -4841,7 +4862,7 @@ del_sigs(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
                                 * offline will prevent us spinning waiting
                                 * for the private part.
                                 */
-                               if (incremental) {
+                               if (incremental && !deleted) {
                                        result = offline(db, ver, zonediff,
                                                         name, rdataset.ttl,
                                                         &rdata);