From 030674b2a3249a06055c9cdd283c226c21065f5a Mon Sep 17 00:00:00 2001 From: Tony Finch Date: Mon, 22 Jun 2020 20:23:29 +0100 Subject: [PATCH 1/4] Fix re-signing when `sig-validity-interval` has two arguments Since October 2019 I have had complaints from `dnssec-cds` reporting that the signatures on some of my test zones had expired. These were zones signed by BIND 9.15 or 9.17, with a DNSKEY TTL of 24h and `sig-validity-interval 10 8`. This is the same setup we have used for our production zones since 2015, which is intended to re-sign the zones every 2 days, keeping at least 8 days signature validity. The SOA expire interval is 7 days, so even in the presence of zone transfer problems, no-one should ever see expired signatures. (These timers are a bit too tight to be completely correct, because I should have increased the expiry timers when I increased the DNSKEY TTLs from 1h to 24h. But that should only matter when zone transfers are broken, which was not the case for the error reports that led to this patch.) For example, this morning my test zone contained: dev.dns.cam.ac.uk. 86400 IN RRSIG DNSKEY 13 5 86400 ( 20200701221418 20200621213022 ...) But one of my resolvers had cached: dev.dns.cam.ac.uk. 21424 IN RRSIG DNSKEY 13 5 86400 ( 20200622063022 20200612061136 ...) This TTL was captured at 20200622105807 so the resolver cached the RRset 64976 seconds previously (18h02m56s), at 20200621165511 only about 12h before expiry. The other symptom of this error was incorrect `resign` times in the output from `rndc zonestatus`. For example, I have configured a test zone zone fast.dotat.at { file "../u/z/fast.dotat.at"; type primary; auto-dnssec maintain; sig-validity-interval 500 499; }; The zone is reset to a minimal zone containing only SOA and NS records, and when `named` starts it loads and signs the zone. After that, `rndc zonestatus` reports: next resign node: fast.dotat.at/NS next resign time: Fri, 28 May 2021 12:48:47 GMT The resign time should be within the next 24h, but instead it is near the signature expiry time, which the RRSIG(NS) says is 20210618074847. (Note 499 hours is a bit more than 20 days.) May/June 2021 is less than 500 days from now because expiry time jitter is applied to the NS records. Using this test I bisected this bug to 09990672d which contained a mistake leading to the resigning interval always being calculated in hours, when days are expected. This bug only occurs for configurations that use the two-argument form of `sig-validity-interval`. --- bin/named/zoneconf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c index 486877df55..8abbbc462b 100644 --- a/bin/named/zoneconf.c +++ b/bin/named/zoneconf.c @@ -1594,11 +1594,11 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, if (cfg_obj_isvoid(resign)) { seconds /= 4; } else if (!sigvalinsecs) { - seconds = cfg_obj_asuint32(resign); + uint32_t r = cfg_obj_asuint32(resign); if (seconds > 7 * 86400) { - seconds *= 86400; + seconds = r * 86400; } else { - seconds *= 3600; + seconds = r * 3600; } } else { seconds = cfg_obj_asuint32(resign); From 11ecf7901b3ae8bbc37ec3f78ef8ae55ae495845 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 25 Jun 2020 21:27:29 +1000 Subject: [PATCH 2/4] Add regression test for [GL !3735] Check that resign interval is actually in days rather than hours by checking that RRSIGs are all within the allowed day range. --- .../system/dnssec/ns2/hours-vs-days.db.in | 165 ++++++++++++++++++ bin/tests/system/dnssec/ns2/named.conf.in | 9 + bin/tests/system/dnssec/ns2/sign.sh | 8 + bin/tests/system/dnssec/tests.sh | 11 ++ 4 files changed, 193 insertions(+) create mode 100644 bin/tests/system/dnssec/ns2/hours-vs-days.db.in diff --git a/bin/tests/system/dnssec/ns2/hours-vs-days.db.in b/bin/tests/system/dnssec/ns2/hours-vs-days.db.in new file mode 100644 index 0000000000..2545faf6b9 --- /dev/null +++ b/bin/tests/system/dnssec/ns2/hours-vs-days.db.in @@ -0,0 +1,165 @@ +; 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. + +$TTL 300 ; 5 minutes +@ IN SOA mname1. . ( + 2000042407 ; serial + 20 ; refresh (20 seconds) + 20 ; retry (20 seconds) + 1814400 ; expire (3 weeks) + 3600 ; minimum (1 hour) + ) + NS ns2 + NS ns3 +ns2 A 10.53.0.2 +ns3 A 10.53.0.3 + +a A 10.0.0.1 +b A 10.0.0.2 +d A 10.0.0.4 + +; Used for testing ANY queries +foo TXT "testing" +foo A 10.0.1.0 + +bad-cname CNAME a +bad-dname DNAME @ + +; Used for testing CNAME queries +cname1 CNAME cname1-target +cname1-target TXT "testing cname" + +cname2 CNAME cname2-target +cname2-target TXT "testing cname" + +; Used for testing DNAME queries +dname1 DNAME dname1-target +foo.dname1-target TXT "testing dname" + +dname2 DNAME dname2-target +foo.dname2-target TXT "testing dname" + +; A secure subdomain +secure NS ns3.secure +ns3.secure A 10.53.0.3 + +; An insecure subdomain +insecure NS ns.insecure +ns.insecure A 10.53.0.3 + +; A secure subdomain we're going to inject bogus data into +bogus NS ns.bogus +ns.bogus A 10.53.0.3 + +; A subdomain with a corrupt DS +badds NS ns.badds +ns.badds A 10.53.0.3 + +; A dynamic secure subdomain +dynamic NS dynamic +dynamic A 10.53.0.3 + +; A insecure subdomain +mustbesecure NS ns.mustbesecure +ns.mustbesecure A 10.53.0.3 + +; A subdomain with expired signatures +expired NS ns.expired +ns.expired A 10.53.0.3 + +; A rfc2535 signed zone w/ CNAME +rfc2535 NS ns.rfc2535 +ns.rfc2535 A 10.53.0.3 + +z A 10.0.0.26 + +keyless NS ns.keyless +ns.keyless A 10.53.0.3 + +nsec3 NS ns.nsec3 +ns.nsec3 A 10.53.0.3 + +optout NS ns.optout +ns.optout A 10.53.0.3 + +nsec3-unknown NS ns.nsec3-unknown +ns.nsec3-unknown A 10.53.0.3 + +optout-unknown NS ns.optout-unknown +ns.optout-unknown A 10.53.0.3 + +dnskey-unknown NS ns.dnskey-unknown +ns.dnskey-unknown A 10.53.0.3 + +dnskey-unsupported NS ns.dnskey-unsupported +ns.dnskey-unsupported A 10.53.0.3 + +dnskey-nsec3-unknown NS ns.dnskey-nsec3-unknown +ns.dnskey-nsec3-unknown A 10.53.0.3 + +multiple NS ns.multiple +ns.multiple A 10.53.0.3 + +*.wild A 10.0.0.27 + +rsasha256 NS ns.rsasha256 +ns.rsasha256 A 10.53.0.3 + +rsasha512 NS ns.rsasha512 +ns.rsasha512 A 10.53.0.3 + +kskonly NS ns.kskonly +ns.kskonly A 10.53.0.3 + +update-nsec3 NS ns.update-nsec3 +ns.update-nsec3 A 10.53.0.3 + +auto-nsec NS ns.auto-nsec +ns.auto-nsec A 10.53.0.3 + +auto-nsec3 NS ns.auto-nsec3 +ns.auto-nsec3 A 10.53.0.3 + + +below-cname CNAME some.where.else. + +insecure.below-cname NS ns.insecure.below-cname +ns.insecure.below-cname A 10.53.0.3 + +secure.below-cname NS ns.secure.below-cname +ns.secure.below-cname A 10.53.0.3 + +ttlpatch NS ns.ttlpatch +ns.ttlpatch A 10.53.0.3 + +split-dnssec NS ns.split-dnssec +ns.split-dnssec A 10.53.0.3 + +split-smart NS ns.split-smart +ns.split-smart A 10.53.0.3 + +upper NS ns.upper +ns.upper A 10.53.0.3 + +LOWER NS NS.LOWER +NS.LOWER A 10.53.0.3 + +expiring NS ns.expiring +ns.expiring A 10.53.0.3 + +future NS ns.future +ns.future A 10.53.0.3 + +managed-future NS ns.managed-future +ns.managed-future A 10.53.0.3 + +revkey NS ns.revkey +ns.revkey A 10.53.0.3 + +dname-at-apex-nsec3 NS ns3 diff --git a/bin/tests/system/dnssec/ns2/named.conf.in b/bin/tests/system/dnssec/ns2/named.conf.in index 12465581cf..4b75918a56 100644 --- a/bin/tests/system/dnssec/ns2/named.conf.in +++ b/bin/tests/system/dnssec/ns2/named.conf.in @@ -182,4 +182,13 @@ zone "corp" { file "corp.db"; }; +zone "hours-vs-days" { + type master; + file "hours-vs-days.db.signed"; + auto-dnssec maintain; + /* validity 500 days, resign in 499 days */ + sig-validity-interval 500 499; + allow-update { any; }; +}; + include "trusted.conf"; diff --git a/bin/tests/system/dnssec/ns2/sign.sh b/bin/tests/system/dnssec/ns2/sign.sh index 8bb113cf39..c99f7a537c 100644 --- a/bin/tests/system/dnssec/ns2/sign.sh +++ b/bin/tests/system/dnssec/ns2/sign.sh @@ -308,3 +308,11 @@ sed 's/DNSKEY/CDNSKEY/' "$key1.key" > "$key1.cdnskey" cat "$infile" "$key1.key" "$key2.key" "$key1.cdnskey" "$key1.cds" > "$zonefile" # Don't sign, let auto-dnssec maintain do it. mv $zonefile "$zonefile.signed" + +zone=hours-vs-days +infile=hours-vs-days.db.in +zonefile=hours-vs-days.db +key1=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -n zone -f KSK "$zone") +key2=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -n zone "$zone") +$SETTIME -P sync now "$key1" > /dev/null +cat "$infile" > "$zonefile.signed" diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index 562be71122..566a4f0951 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -4270,5 +4270,16 @@ n=$((n+1)) test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) +echo_i "checking sig-validity-interval second field hours vs days ($n)" +ret=0 +# zone configured with 'sig-validity-interval 500 499;' +# 499 days in the future w/ a 20 minute runtime to now allowance +min=$(TZ=UTC $PERL -e '@lt=localtime(time() + 499*3600*24 - 20*60); printf "%.4d%0.2d%0.2d%0.2d%0.2d%0.2d\n",$lt[5]+1900,$lt[4]+1,$lt[3],$lt[2],$lt[1],$lt[0];') +dig_with_opts @10.53.0.2 hours-vs-days AXFR > dig.out.ns2.test$n +awk -v min=$min '$4 == "RRSIG" { if ($9 < min) { exit(1); } }' dig.out.ns2.test$n || ret=1 +n=$((n+1)) +test "$ret" -eq 0 || echo_i "failed" +status=$((status+ret)) + echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 From f4fbca6e1661920a7f5bb8039486827d1f729018 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 25 Jun 2020 14:50:16 +1000 Subject: [PATCH 3/4] Add CHANGES note for [GL !3735] --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index f7a4558282..f7cc269d14 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5471. [bug] The introduction of KASP support broke whether the + second field of sig-validity-interval was treated as + days or hours. (Thanks to Tony Finch.) [GL !3735] + 5470. [port] illumos: only call gsskrb5_register_acceptor_identity if we have gssapi_krb5.h. [GL #1995] From 3ff60b881fcc38fe6393ad6872237915ae077aa3 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 25 Jun 2020 14:51:19 +1000 Subject: [PATCH 4/4] Add release note for [GL !3735] --- doc/notes/notes-current.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 1ce674d1e5..ae057efdaa 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -92,3 +92,7 @@ Bug Fixes - ``named`` could crash when cleaning dead nodes in ``lib/dns/rbtdb.c`` that have been reused meanwhile. [GL #1968] + +- The introduction of KASP support broke whether the second field + of sig-validity-interval was treated as days or hours. (Thanks to + Tony Finch.) [GL !3735]