From 9a36a1bba34d178ee65d24d1dd71c7ae7953abf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 11 Mar 2019 12:04:42 +0100 Subject: [PATCH 1/5] Fix NTA-related races Changes introduced by commit 6b8e4d6e695fc13b2d2a93437418a047b5adce81 were incomplete as not all time-sensitive checks were updated to match revised "nta-lifetime" and "nta-recheck" values. Prevent rare false positives by updating all NTA-related checks so that they work reliably with "nta-lifetime 12s;" and "nta-recheck 9s;". Update comments as well to prevent confusion. --- bin/tests/system/dnssec/tests.sh | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index 81865b626e..28dff27506 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -1864,8 +1864,8 @@ echo_i "waiting for NTA rechecks/expirations" # # secure.example and badds.example used default nta-duration -# (configured as 10s in ns4/named1.conf), but nta recheck interval -# is configured to 7s, so at t=8 the NTAs for secure.example and +# (configured as 12s in ns4/named1.conf), but nta recheck interval +# is configured to 9s, so at t=10 the NTAs for secure.example and # fakenode.secure.example should both be lifted, but badds.example # should still be going. # @@ -1886,9 +1886,9 @@ status=$((status+ret)) ret=0 # -# bogus.example was set to expire in 20s, so at t=11 +# bogus.example was set to expire in 20s, so at t=13 # it should still be NTA'd, but badds.example used the default -# lifetime of 10s, so it should revert to SERVFAIL now. +# lifetime of 12s, so it should revert to SERVFAIL now. # # shellcheck disable=SC2016 $PERL -e 'my $delay = '"$start"' + 13 - time(); select(undef, undef, undef, $delay) if ($delay > 0);' @@ -2087,11 +2087,11 @@ else exit 1 fi -# nta-recheck is configured as 7s, so at t=10 the NTAs for +# nta-recheck is configured as 9s, so at t=12 the NTAs for # secure.example. should be lifted as it is not a forced NTA. -echo_i "waiting till 10s have passed after ns4 was restarted" +echo_i "waiting till 12s have passed after ns4 was restarted" # shellcheck disable=SC2016 -$PERL -e 'my $delay = '"$start"' + 10 - time(); select(undef, undef, undef, $delay) if ($delay > 0);' +$PERL -e 'my $delay = '"$start"' + 12 - time(); select(undef, undef, undef, $delay) if ($delay > 0);' # secure.example. should now return an AD=1 answer (still validates) as # the NTA has been lifted. @@ -2143,11 +2143,11 @@ else exit 1 fi -# nta-recheck is configured as 7s, but even at t=10 the NTAs for +# nta-recheck is configured as 9s, but even at t=12 the NTAs for # secure.example. should not be lifted as it is a forced NTA. -echo_i "waiting till 10s have passed after ns4 was restarted" +echo_i "waiting till 12s have passed after ns4 was restarted" # shellcheck disable=SC2016 -$PERL -e 'my $delay = '"$start"' + 10 - time(); select(undef, undef, undef, $delay) if ($delay > 0);' +$PERL -e 'my $delay = '"$start"' + 12 - time(); select(undef, undef, undef, $delay) if ($delay > 0);' # secure.example. should now return an AD=0 answer (non-authenticated) # as the NTA is still there. From a597bd52a68b76988f6ff78d18ae2368b077d32e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 11 Mar 2019 12:04:42 +0100 Subject: [PATCH 2/5] Fix message section checked in a TTL capping test Commit c032c54dda2d75c0ec68017e1331bc9880c03ae1 inadvertently changed the DNS message section inspected by one of the TTL capping checks from ADDITIONAL to ANSWER, introducing a discrepancy between that check's description and its actual meaning. Revert to inspecting the ADDITIONAL section in the aforementioned check. --- bin/tests/system/dnssec/tests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index 28dff27506..aedcf879f1 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -2880,8 +2880,8 @@ status=$((status+ret)) echo_i "testing TTL is capped at RRSIG expiry time for records in the additional section with dnssec-accept-expired yes; ($n)" ret=0 rndccmd 10.53.0.4 flush 2>&1 | sed 's/^/ns4 /' | cat_i -dig_with_answeropts +cd expiring.example mx @10.53.0.4 > dig.out.ns4.1.$n -dig_with_answeropts expiring.example mx @10.53.0.4 > dig.out.ns4.2.$n +dig_with_additionalopts +cd expiring.example mx @10.53.0.4 > dig.out.ns4.1.$n +dig_with_additionalopts expiring.example mx @10.53.0.4 > dig.out.ns4.2.$n ttls=$(awk '$1 != ";;" {print $2}' dig.out.ns4.1.$n) ttls2=$(awk '$1 != ";;" {print $2}' dig.out.ns4.2.$n) for ttl in ${ttls:-300}; do From 8baf85906306e2757ab9cce680c7f764d6e4e04e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 11 Mar 2019 12:04:42 +0100 Subject: [PATCH 3/5] Relax ADDITIONAL TTL capping checks Always expecting a TTL of exactly 300 seconds for RRsets found in the ADDITIONAL section of responses received for CD=1 queries sent during TTL capping checks is too strict since these responses will contain records cached from multiple DNS messages received during the resolution process. In responses to queries sent with CD=1, ns.expiring.example/A in the ADDITIONAL section will come from a delegation returned by ns2 while the ANSWER section will come from an authoritative answer returned by ns3. If the queries to ns2 and ns3 happen at different Unix timestamps, RRsets cached from the older response will have a different TTL by the time they are returned to dig, triggering a false positive. Allow a safety margin of 60 seconds for checks inspecting the ADDITIONAL section of responses to queries sent with CD=1 to fix the issue. A safety margin this large is likely overkill, but it is used nevertheless for consistency with similar safety margins used in other TTL capping checks. --- bin/tests/system/dnssec/tests.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index aedcf879f1..117fc0ca68 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -2813,7 +2813,7 @@ dig_with_additionalopts expiring.example ns @10.53.0.4 > dig.out.ns4.2.$n ttls=$(awk '$1 != ";;" {print $2}' dig.out.ns4.1.$n) ttls2=$(awk '$1 != ";;" {print $2}' dig.out.ns4.2.$n) for ttl in ${ttls:-300}; do - [ "$ttl" -eq 300 ] || ret=1 + [ "$ttl" -le 300 ] && [ "$ttl" -gt 240 ] || ret=1 done for ttl in ${ttls2:-0}; do [ "$ttl" -le 60 ] || ret=1 @@ -2831,7 +2831,7 @@ dig_with_additionalopts expiring.example mx @10.53.0.4 > dig.out.ns4.2.$n ttls=$(awk '$1 != ";;" {print $2}' dig.out.ns4.1.$n) ttls2=$(awk '$1 != ";;" {print $2}' dig.out.ns4.2.$n) for ttl in ${ttls:-300}; do - [ "$ttl" -eq 300 ] || ret=1 + [ "$ttl" -le 300 ] && [ "$ttl" -gt 240 ] || ret=1 done for ttl in ${ttls2:-0}; do [ "$ttl" -le 60 ] || ret=1 @@ -2885,7 +2885,7 @@ dig_with_additionalopts expiring.example mx @10.53.0.4 > dig.out.ns4.2.$n ttls=$(awk '$1 != ";;" {print $2}' dig.out.ns4.1.$n) ttls2=$(awk '$1 != ";;" {print $2}' dig.out.ns4.2.$n) for ttl in ${ttls:-300}; do - [ "$ttl" -eq 300 ] || ret=1 + [ "$ttl" -le 300 ] && [ "$ttl" -gt 240 ] || ret=1 done for ttl in ${ttls2:-0}; do [ "$ttl" -le 120 ] && [ "$ttl" -gt 60 ] || ret=1 From a85cc41486d94d6c5e5116c63fa7ef1c9fd58925 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 11 Mar 2019 12:04:42 +0100 Subject: [PATCH 4/5] Make ANSWER TTL capping checks stricter For checks querying a named instance with "dnssec-accept-expired yes;" set, authoritative responses have a TTL of 300 seconds. Assuming empty resolver cache, TTLs of RRsets in the ANSWER section of the first response to a given query will always match their authoritative counterparts. Also note that for a DNSSEC-validating named resolver, validated RRsets replace any existing non-validated RRsets with the same owner name and type, e.g. cached from responses received while resolving CD=1 queries. Since TTL capping happens before a validated RRset is inserted into the cache and RRSIG expiry time does not impose an upper TTL bound when "dnssec-accept-expired yes;" is set and, as pointed out above, the original TTLs of the relevant RRsets equal 300 seconds, the RRsets in the ANSWER section of the responses to expiring.example/SOA and expired.example/SOA queries sent with CD=0 should always be exactly 120 seconds, never a lower value. Make the relevant TTL checks stricter to reflect that. --- bin/tests/system/dnssec/tests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index 117fc0ca68..dda755f9a1 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -2855,7 +2855,7 @@ for ttl in ${ttls:-0}; do [ "$ttl" -eq 300 ] || ret=1 done for ttl in ${ttls2:-0}; do - [ "$ttl" -le 120 ] && [ "$ttl" -gt 60 ] || ret=1 + [ "$ttl" -eq 120 ] || ret=1 done n=$((n+1)) test "$ret" -eq 0 || echo_i "failed" @@ -2871,7 +2871,7 @@ for ttl in ${ttls:-0}; do [ "$ttl" -eq 300 ] || ret=1 done for ttl in ${ttls2:-0}; do - [ "$ttl" -le 120 ] && [ "$ttl" -gt 60 ] || ret=1 + [ "$ttl" -eq 120 ] || ret=1 done n=$((n+1)) test "$ret" -eq 0 || echo_i "failed" From dee1f1a49812169fc823428d6c5a3331e570612c Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 21 Feb 2019 11:14:54 +1100 Subject: [PATCH 5/5] ${ttl} must exist and be non null --- bin/tests/system/dnssec/tests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index dda755f9a1..de4d42d948 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -2795,10 +2795,10 @@ dig_with_answeropts expiring.example soa @10.53.0.4 > dig.out.ns4.2.$n ttls=$(awk '$1 != ";;" {print $2}' dig.out.ns4.1.$n) ttls2=$(awk '$1 != ";;" {print $2}' dig.out.ns4.2.$n) for ttl in ${ttls:-0}; do - [ "${ttl:-0}" -eq 300 ] || ret=1 + [ "${ttl}" -eq 300 ] || ret=1 done for ttl in ${ttls2:-0}; do - [ "${ttl:-0}" -le 60 ] || ret=1 + [ "${ttl}" -le 60 ] || ret=1 done n=$((n+1)) test "$ret" -eq 0 || echo_i "failed"