Merge branch '117-running-dnssec-keymgr-with-old-keys-inactivates-deletes-them-immediately' into 'master'

Resolve "Running dnssec-keymgr with old keys inactivates/deletes them immediately"

Closes #117

See merge request isc-projects/bind9!1378

CHANGES

@@ -1,3 +1,7 @@
+5140.	[bug]		Don't immediately mark existing keys as inactive and
+			deleted when running dnssec-keymgr for the first
+			time. [GL #117]
+
 5139.	[bug]		If possible, don't use forwarders when priming.
 			This ensures we can get root server IP addresses
 			from priming query response glue, which may not

bin/python/isc/keyseries.py.in

@@ -77,15 +77,39 @@ class keyseries:
         a = key.activate()
         if not p or p > now:
             key.setpublish(now)
+            p = now
         if not a or a > now:
             key.setactivate(now)
+            a = now
 
+        i = key.inactive()
         if not rp:
             key.setinactive(None, **kwargs)
             key.setdelete(None, **kwargs)
+        elif not i or a + rp != i:
+            if not i and a + rp > now + prepub:
+                key.setinactive(a + rp, **kwargs)
+                key.setdelete(a + rp + postpub, **kwargs)
+            elif not i:
+                key.setinactive(now + prepub, **kwargs)
+                key.setdelete(now + prepub + postpub, **kwargs)
+            elif a + rp > i:
+                key.setinactive(a + rp, **kwargs)
+                key.setdelete(a + rp + postpub, **kwargs)
+            elif a + rp > now + prepub:
+                key.setinactive(a + rp, **kwargs)
+                key.setdelete(a + rp + postpub, **kwargs)
+            else:
+                key.setinactive(now + prepub, **kwargs)
+                key.setdelete(now + prepub + postpub, **kwargs)
         else:
-            key.setinactive(a + rp, **kwargs)
+            d = key.delete()
-            key.setdelete(a + rp + postpub, **kwargs)
+            if not d or i + postpub > now:
+                key.setdelete(i + postpub, **kwargs)
+            elif not d:
+                key.setdelete(now + postpub, **kwargs)
+            elif d < i + postpub:
+                key.setdelete(i + postpub, **kwargs)
 
         if policy.keyttl != key.ttl:
             key.setttl(policy.keyttl)

bin/tests/system/keymgr/19-old-keys/README

@@ -0,0 +1,7 @@
+Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+
+See COPYRIGHT in the source root or http://isc.org/copyright.html for terms.
+
+This directory has a key set which is valid, but which was published
+and activated more than one rollover period ago. dnssec-keymgr should
+not mark the keys as already being inactive and deleted.

bin/tests/system/keymgr/19-old-keys/expect

@@ -0,0 +1,12 @@
+kargs="-c policy.conf example.com"
+kmatch=""
+kret=0
+cargs="-d 1w -m 2w example.com"
+cmatch="4,Publish
+4,Activate
+2,Inactive
+2,Delete"
+cret=0
+warn=0
+error=0
+ok=2

bin/tests/system/keymgr/19-old-keys/extra.sh

@@ -0,0 +1,19 @@
+# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+#
+# 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 http://mozilla.org/MPL/2.0/.
+#
+# See the COPYRIGHT file distributed with this work for additional
+# information regarding copyright ownership.
+
+now=`$PERL -e 'print time()."\n";'`
+for keyfile in K*.key; do
+    inactive=`$SETTIME -upI $keyfile | awk '{print $2}'`
+    if [ "$inactive" = UNSET ]; then
+        continue
+    elif [ "$inactive" -lt "$now" ]; then
+        echo_d "inactive date is in the past"
+        ret=1
+    fi
+done

bin/tests/system/keymgr/19-old-keys/policy.conf

@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * 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 http://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+policy default {
+	policy global;
+	algorithm nsec3rsasha1;
+	pre-publish zsk 2w;
+	roll-period zsk 6mo;
+	coverage 364d;
+};

bin/tests/system/keymgr/clean.sh

@@ -11,6 +11,8 @@
 
 rm -f */K*.key
 rm -f */K*.private
-rm -f coverage.* keymgr.*
+rm -f Kexample.com.*.key
-rm -f policy.out
+rm -f Kexample.com.*.private
+rm -f coverage.* keymgr.* settime.*
 rm -f ns*/managed-keys.bind*
+rm -f policy.out

bin/tests/system/keymgr/setup.sh

@@ -214,3 +214,13 @@ rm -f $dir/K*.private
 ksk1=`$KEYGEN -K $dir -a rsasha1 -3fk example.com`
 zsk1=`$KEYGEN -K $dir -a rsasha1 -3 example.com`
 $SETTIME -K $dir -I now+2mo -D now+3mo $zsk1 > /dev/null
+
+# Test 19: Key has been published/active a long time
+dir=19-old-keys
+echo_i "set up $dir"
+rm -f $dir/K*.key
+rm -f $dir/K*.private
+ksk1=`$KEYGEN -K $dir -a rsasha1 -3fk example.com`
+zsk1=`$KEYGEN -K $dir -a rsasha1 -3 example.com`
+$SETTIME -K $dir -P now-2y -A now-2y $ksk1 > /dev/null
+$SETTIME -K $dir -P now-2y -A now-2y $zsk1 > /dev/null

bin/tests/system/keymgr/tests.sh

@@ -16,13 +16,19 @@ status=0
 n=1
 
 matchall () {
+    match_result=ok
     file=$1
-    echo "$2" | while read matchline; do
+    while IFS="," read expect matchline; do
-        grep "$matchline" $file > /dev/null 2>&1 || {
+        [ -z "$matchline" ] && continue
-            echo "FAIL"
+        matches=`grep "$matchline" $file | wc -l`
-            return
+        [ "$matches" -ne "$expect" ] && {
+            echo "'$matchline': expected $expect found $matches"
+            return 1
         }
-    done
+    done << EOF
+    $2
+EOF
+    return 0
 }
 
 echo_i "checking for DNSSEC key coverage issues"
@@ -51,11 +57,8 @@ for dir in [0-9][0-9]-*; do
             ret=1
         fi
 
-        found=`matchall keymgr.$n "$kmatch"`
+        # check for matches in keymgr output
-        if [ "$found" = "FAIL" ]; then
+        matchall keymgr.$n "$kmatch" || ret=1
-            echo "no match on '$kmatch'"
-            ret=1
-        fi
 
         # now check coverage
         $COVERAGE -K $dir $cargs > coverage.$n 2>&1
@@ -87,10 +90,13 @@ for dir in [0-9][0-9]-*; do
             ret=1
         fi
 
-        found=`matchall coverage.$n "$cmatch"`
+        # check for matches in coverage output
-        if [ "$found" = "FAIL" ]; then
+        matchall coverage.$n "$cmatch" || ret=1
-            echo "no match on '$cmatch'"
+
-            ret=1
+        if [ -f $dir/extra.sh ]; then
+           cd $dir
+           . ./extra.sh
+           cd ..
         fi
 
         n=`expr $n + 1`

util/copyrights

@@ -742,6 +742,9 @@
 ./bin/tests/system/keymgr/17-noforce/expect	X	2016,2018,2019
 ./bin/tests/system/keymgr/18-nonstd-prepub/README	TXT.BRIEF	2016,2018,2019
 ./bin/tests/system/keymgr/18-nonstd-prepub/expect	X	2016,2018,2019
+./bin/tests/system/keymgr/19-old-keys/README	TXT.BRIEF	2019
+./bin/tests/system/keymgr/19-old-keys/expect	X	2019
+./bin/tests/system/keymgr/19-old-keys/extra.sh	SH	2019
 ./bin/tests/system/keymgr/clean.sh		SH	2016,2018,2019
 ./bin/tests/system/keymgr/policy.good		X	2016,2018,2019
 ./bin/tests/system/keymgr/policy.sample		X	2016,2017,2018,2019