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 dd6420d4f5b511d269e6331b78f6c763b809844d..85987966f112d72b1268608911dc418c7389c230 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 27522c64a7faa0d601dfcded553b67265bedf424..9d72fed87822cac98c06d221b91f3a389b2aa349 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
 }
 
 #
@@ -208,6 +212,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`
 
@@ -216,24 +221,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/$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`
 
-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`
 
@@ -943,7 +947,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/bin/tests/system/inline/clean.sh b/bin/tests/system/inline/clean.sh
index 45d69843e33803ad903ecf08151bca2be1972f10..ad17c452dadf15635b5cd42d419662c52f76f368 100644 (file)
--- a/bin/tests/system/inline/clean.sh
+++ b/bin/tests/system/inline/clean.sh
@@ -52,6 +52,10 @@ rm -f ns3/expired.db
 rm -f ns3/expired.db.jnl
 rm -f ns3/expired.db.signed
 rm -f ns3/expired.db.signed.jnl
+rm -f ns3/nsec3.db
+rm -f ns3/nsec3.db.jnl
+rm -f ns3/nsec3.db.signed
+rm -f ns3/nsec3.db.signed.jnl
 rm -f ns3/retransfer.bk
 rm -f ns3/retransfer.bk.jnl
 rm -f ns3/retransfer.bk.signed

diff --git a/bin/tests/system/inline/ns1/root.db.in b/bin/tests/system/inline/ns1/root.db.in
index 404541f37c1829ffa0f0d6244e76e4343d1c2fb2..8d2eaf5dc83bd6286828a8b2ebba0fb4ab0dc2fe 100644 (file)
--- a/bin/tests/system/inline/ns1/root.db.in
+++ b/bin/tests/system/inline/ns1/root.db.in
@@ -47,3 +47,6 @@ ns3.expired.          A       10.53.0.3
 
 retransfer.                    NS      ns3.retransfer.
 ns3.retransfer.                A       10.53.0.3
+
+nsec3.                 NS      ns3.nsec3.
+ns3.nsec3.             A       10.53.0.3

diff --git a/bin/tests/system/inline/ns3/named.conf b/bin/tests/system/inline/ns3/named.conf
index 6aa0b2015832ea269fa4fd676d5e6361b1d6044b..acde7ba2525c314ccdd1fe0af3de10b9c1e9d172 100644 (file)
--- a/bin/tests/system/inline/ns3/named.conf
+++ b/bin/tests/system/inline/ns3/named.conf
@@ -95,3 +95,11 @@ zone "retransfer" {
        auto-dnssec maintain;
        file "retransfer.bk";
 };
+
+zone "nsec3" {
+       type master;
+       inline-signing yes;
+       auto-dnssec maintain;
+       allow-update { any; };
+       file "nsec3.db";
+};

diff --git a/bin/tests/system/inline/ns3/sign.sh b/bin/tests/system/inline/ns3/sign.sh
index 9e21ad29886625c8dc77081036ab51a2967cd9ff..04e61f34163d36457bdd1bbd7358dc93b18b6292 100644 (file)
--- a/bin/tests/system/inline/ns3/sign.sh
+++ b/bin/tests/system/inline/ns3/sign.sh
@@ -74,6 +74,12 @@ keyname=`$KEYGEN -q -r $RANDFILE -a RSASHA1 -b 768 -n zone $zone`
 keyname=`$KEYGEN -q -r $RANDFILE -a RSASHA1 -b 1024 -n zone -f KSK $zone`
 $DSFROMKEY -T 1200 $keyname >> ../ns1/root.db
 
+zone=nsec3
+rm -f K${zone}.+*+*.key
+rm -f K${zone}.+*+*.private
+keyname=`$KEYGEN -q -r $RANDFILE -a NSEC3RSASHA1 -b 1024 -n zone -f KSK $zone`
+$DSFROMKEY -T 1200 $keyname >> ../ns1/root.db
+
 for s in a c d h k l m q z
 do
        zone=test-$s

diff --git a/bin/tests/system/inline/setup.sh b/bin/tests/system/inline/setup.sh
index 7ba3246b7be36ecfd0f3ae1949e147d0318389ed..04b4aa77b9e5c58a566fb172105b0d2807dbc4d6 100644 (file)
--- a/bin/tests/system/inline/setup.sh
+++ b/bin/tests/system/inline/setup.sh
@@ -28,6 +28,7 @@ cp ns3/master.db.in ns3/master.db
 cp ns3/master.db.in ns3/dynamic.db
 cp ns3/master.db.in ns3/updated.db
 cp ns3/master.db.in ns3/expired.db
+cp ns3/master.db.in ns3/nsec3.db
 
 touch ns4/trusted.conf
 cp ns4/noixfr.db.in ns4/noixfr.db

diff --git a/bin/tests/system/inline/tests.sh b/bin/tests/system/inline/tests.sh
index 6529235d5f6888beb7cd1ebb8cfc825fbdb9ab80..27a1329a5da4573f5d72e540e7ea1eeb283a6910 100644 (file)
--- a/bin/tests/system/inline/tests.sh
+++ b/bin/tests/system/inline/tests.sh
@@ -25,6 +25,24 @@ RANDFILE=random.data
 status=0
 n=0
 
+$RNDC -c ../common/rndc.conf -s 10.53.0.3 -p 9953 signing -nsec3param 1 0 0 - nsec3
+
+for i in 1 2 3 4 5 6 7 8 9 0
+do
+       nsec3param=`$DIG +short @10.53.0.3 -p 5300 nsec3param nsec3.`
+       test -n "$nsec3param" && break
+       sleep 1
+done
+
+n=`expr $n + 1`
+echo "I:checking that rrsigs are replaced with ksk only"
+ret=0
+$DIG @10.53.0.3 -p 5300 axfr nsec3. |
+       awk '/RRSIG NSEC3/ {a[$1]++} END { for (i in a) {if (a[i] != 1) exit (1)}}' || ret=1
+#$DIG @10.53.0.3 -p 5300 axfr nsec3. | grep -w NSEC | grep -v "IN.RRSIG.NSEC" 
+if [ $ret != 0 ]; then echo "I:failed"; fi
+status=`expr $status + $ret`
+
 n=`expr $n + 1`
 echo "I:checking that the zone is signed on initial transfer ($n)"
 ret=0

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index e1f1fece3c8c942d5204da8744281274ff04c9b5..3bdbc3e03c28bf14ad78135087bed1ecbde1c4e3 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -729,8 +729,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);
 static isc_result_t zone_send_securedb(dns_zone_t *zone, isc_boolean_t locked,
                                       dns_db_t *db);
 
@@ -5278,19 +5276,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
@@ -5358,7 +5376,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);
@@ -5366,7 +5385,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.
@@ -5376,7 +5397,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);