From 6a1c41143faa3cb84b32cf7daf46180a36b5d958 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 15 Jan 2020 08:19:19 -0800 Subject: [PATCH 1/3] fix a bug when validating negative cache entries if validator_start() is called with validator->event->message set to NULL, we can't use message->rcode to decide which negative proofs are needed, so we use the rdataset attributes instead to determine whether the rdataset was cached as NXDOMAIN or NODATA. --- lib/dns/validator.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 3e1e852531..8fe3aa46f1 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -99,6 +99,7 @@ #define CANCELED(v) (((v)->attributes & VALATTR_CANCELED) != 0) #define NEGATIVE(r) (((r)->attributes & DNS_RDATASETATTR_NEGATIVE) != 0) +#define NXDOMAIN(r) (((r)->attributes & DNS_RDATASETATTR_NXDOMAIN) != 0) static void destroy(dns_validator_t *val); @@ -3085,16 +3086,14 @@ validator_start(isc_task_t *task, isc_event_t *event) { "parent indicates it should be secure"); } } else if ((val->event->rdataset == NULL && - val->event->sigrdataset == NULL) || - (val->event->rdataset != NULL && - NEGATIVE(val->event->rdataset))) - + val->event->sigrdataset == NULL)) { /* - * This is a nonexistence validation. + * This is a validation of a negative response. */ validator_log(val, ISC_LOG_DEBUG(3), - "attempting negative response validation"); + "attempting negative response validation " + "from message"); if (val->event->message->rcode == dns_rcode_nxdomain) { val->attributes |= VALATTR_NEEDNOQNAME; @@ -3102,6 +3101,25 @@ validator_start(isc_task_t *task, isc_event_t *event) { } else { val->attributes |= VALATTR_NEEDNODATA; } + + result = validate_nx(val, false); + } else if ((val->event->rdataset != NULL && + NEGATIVE(val->event->rdataset))) + { + /* + * This is a delayed validation of a negative cache entry. + */ + validator_log(val, ISC_LOG_DEBUG(3), + "attempting negative response validation " + "from cache"); + + if (NXDOMAIN(val->event->rdataset)) { + val->attributes |= VALATTR_NEEDNOQNAME; + val->attributes |= VALATTR_NEEDNOWILDCARD; + } else { + val->attributes |= VALATTR_NEEDNODATA; + } + result = validate_nx(val, false); } else { INSIST(0); From fa04c875783330ab4cb9540ddabeec3313731ea3 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 15 Jan 2020 13:54:56 -0800 Subject: [PATCH 2/3] add system test of insecurity proof from negative cache --- bin/tests/system/dnssec/tests.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index 0d14b737f7..f46e68e591 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -1073,6 +1073,23 @@ n=$((n+1)) test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) +echo_i "checking insecurity proof works using negative cache ($n)" +ret=0 +rndccmd 10.53.0.4 flush 2>&1 | sed 's/^/ns4 /' | cat_i +dig_with_opts +cd @10.53.0.4 insecure.example. ds > dig.out.ns4.test$n.1 || ret=1 +for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 +do + dig_with_opts @10.53.0.4 nonexistent.insecure.example. > dig.out.ns4.test$n.2 || ret=1 + if grep "status: NXDOMAIN" dig.out.ns4.test$n.2 >/dev/null; then + break + fi + sleep 1 +done +grep "status: NXDOMAIN" dig.out.ns4.test$n.2 >/dev/null || ret=1 +n=$((n+1)) +test "$ret" -eq 0 || echo_i "failed" +status=$((status+ret)) + echo_i "checking positive validation RSASHA256 NSEC ($n)" ret=0 dig_with_opts +noauth a.rsasha256.example. @10.53.0.3 a > dig.out.ns3.test$n || ret=1 From 8b9a3314b1a46b35329a7f649e831729c6978838 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 15 Jan 2020 09:01:03 -0800 Subject: [PATCH 3/3] CHANGES --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index c5d2ab50c4..6a1d6880c5 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5347. [bug] Fixed a bug that could cause an intermittent crash + in validator.c when validating a negative cache + entry. [GL #1561] + 5346. [bug] Make hazard pointer array allocations dynamic, fixing a bug that caused named to crash on machines with more than 40 cores. [GL #1493]