diff --git a/CHANGES b/CHANGES index 3ec56b9c9e..339a5046f1 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5169. [bug] The presence of certain types in an otherwise + empty node could cause a crash while processing a + type ANY query. [GL #901] + 5168. [bug] Do not crash on shutdown when RPZ fails to load. Also, keep previous version of the database if RPZ fails to load. [GL #813] diff --git a/bin/tests/system/dnssec/ns3/insecure.example.db b/bin/tests/system/dnssec/ns3/insecure.example.db index 86552149e1..98777d674f 100644 --- a/bin/tests/system/dnssec/ns3/insecure.example.db +++ b/bin/tests/system/dnssec/ns3/insecure.example.db @@ -21,4 +21,5 @@ ns A 10.53.0.3 a A 10.0.0.1 b A 10.0.0.2 d A 10.0.0.4 +x DNSKEY 258 3 5 Cg== z A 10.0.0.26 diff --git a/bin/tests/system/dnssec/ns3/secure.example.db.in b/bin/tests/system/dnssec/ns3/secure.example.db.in index 9d310d8cb2..27f2b2401c 100644 --- a/bin/tests/system/dnssec/ns3/secure.example.db.in +++ b/bin/tests/system/dnssec/ns3/secure.example.db.in @@ -28,6 +28,7 @@ g A 10.0.0.7 z A 10.0.0.26 a.a.a.a.a.a.a.a.a.a.e A 10.0.0.27 x CNAME a +zz DNSKEY 258 3 5 Cg== private NS ns.private ns.private A 10.53.0.2 diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index 1f39bd535b..81865b626e 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -3635,6 +3635,19 @@ n=$((n+1)) test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) +echo_i "checking DNSSEC records are occluded from ANY in an insecure zone ($n)" +ret=0 +dig_with_opts any x.insecure.example. @10.53.0.3 > dig.out.ns3.1.test$n || ret=1 +grep "status: NOERROR" dig.out.ns3.1.test$n > /dev/null || ret=1 +grep "ANSWER: 0," dig.out.ns3.1.test$n > /dev/null || ret=1 +dig_with_opts any zz.secure.example. @10.53.0.3 > dig.out.ns3.2.test$n || ret=1 +grep "status: NOERROR" dig.out.ns3.2.test$n > /dev/null || ret=1 +# DNSKEY+RRSIG, NSEC+RRSIG +grep "ANSWER: 4," dig.out.ns3.2.test$n > /dev/null || ret=1 +n=$((n+1)) +test "$ret" -eq 0 || echo_i "failed" +status=$((status+ret)) + # Note: after this check, ns4 will not be validating any more; do not add any # further validation tests employing ns4 below this check. echo_i "check that validation defaults to off when dnssec-enable is off ($n)" diff --git a/doc/arm/notes.xml b/doc/arm/notes.xml index 8ae3174604..3dc8716a07 100644 --- a/doc/arm/notes.xml +++ b/doc/arm/notes.xml @@ -127,7 +127,9 @@ - None. + The presence of certain record types in an otherwise empty + node of a zone could trigger an assertion failure while processing + a query of type ANY. [GL #901] diff --git a/lib/ns/query.c b/lib/ns/query.c index f1c640b43f..0efcafd578 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -6727,7 +6727,7 @@ query_addnoqnameproof(query_ctx_t *qctx) { */ static isc_result_t query_respond_any(query_ctx_t *qctx) { - bool found = false; + bool found = false, hidden = false; dns_rdatasetiter_t *rdsiter = NULL; isc_result_t result; dns_rdatatype_t onetype = 0; /* type to use for minimal-any */ @@ -6782,11 +6782,11 @@ query_respond_any(query_ctx_t *qctx) { dns_rdatatype_isdnssec(qctx->rdataset->type)) { /* - * The zone is transitioning from insecure - * to secure. Hide the dnssec records from - * ANY queries. + * The zone may be transitioning from insecure + * to secure. Hide DNSSEC records from ANY queries. */ dns_rdataset_disassociate(qctx->rdataset); + hidden = true; } else if (qctx->view->minimal_any && !TCP(qctx->client) && !WANTDNSSEC(qctx->client) && qctx->qtype == dns_rdatatype_any && @@ -6808,7 +6808,8 @@ query_respond_any(query_ctx_t *qctx) { qctx->rdataset->type == qctx->qtype) && qctx->rdataset->type != 0) { - if (NOQNAME(qctx->rdataset) && WANTDNSSEC(qctx->client)) + if (NOQNAME(qctx->rdataset) && + WANTDNSSEC(qctx->client)) { qctx->noqname = qctx->rdataset; } else { @@ -6864,8 +6865,9 @@ query_respond_any(query_ctx_t *qctx) { } qctx->rdataset = ns_client_newrdataset(qctx->client); - if (qctx->rdataset == NULL) + if (qctx->rdataset == NULL) { break; + } } else { /* * We're not interested in this rdataset. @@ -6886,48 +6888,59 @@ query_respond_any(query_ctx_t *qctx) { } if (found) { + /* + * Call hook if any answers were found. + * Do this before releasing qctx->fname, in case + * the hook function needs it. + */ CALL_HOOK(NS_QUERY_RESPOND_ANY_FOUND, qctx); - - if (qctx->fname != NULL) { - dns_message_puttempname(qctx->client->message, - &qctx->fname); - } - - query_addauth(qctx); - return (ns_query_done(qctx)); } - /* - * If we're here, no matching rdatasets were found. If we were - * searching for RRSIG/SIG, that may be okay, but otherwise - * something's gone wrong. - */ - INSIST(qctx->qtype == dns_rdatatype_rrsig || - qctx->qtype == dns_rdatatype_sig); - if (qctx->fname != NULL) { - dns_message_puttempname(qctx->client->message, - &qctx->fname); + dns_message_puttempname(qctx->client->message, &qctx->fname); } - if (!qctx->is_zone) { - qctx->authoritative = false; - qctx->client->attributes &= ~NS_CLIENTATTR_RA; + if (found) { + /* + * At least one matching rdataset was found + */ query_addauth(qctx); - return (ns_query_done(qctx)); + } else if (qctx->qtype == dns_rdatatype_rrsig || + qctx->qtype == dns_rdatatype_sig) + { + /* + * No matching rdatasets were found, but we got + * here on a search for RRSIG/SIG, so that's okay. + */ + if (!qctx->is_zone) { + qctx->authoritative = false; + qctx->client->attributes &= ~NS_CLIENTATTR_RA; + query_addauth(qctx); + return (ns_query_done(qctx)); + } + + if (qctx->qtype == dns_rdatatype_rrsig && + dns_db_issecure(qctx->db)) + { + char namebuf[DNS_NAME_FORMATSIZE]; + dns_name_format(qctx->client->query.qname, + namebuf, sizeof(namebuf)); + ns_client_log(qctx->client, DNS_LOGCATEGORY_DNSSEC, + NS_LOGMODULE_QUERY, ISC_LOG_WARNING, + "missing signature for %s", namebuf); + } + + qctx->fname = ns_client_newname(qctx->client, qctx->dbuf, &b); + return (query_sign_nodata(qctx)); + } else if (!hidden) { + /* + * No matching rdatasets were found and nothing was + * deliberately hidden: something must have gone wrong. + */ + QUERY_ERROR(qctx, DNS_R_SERVFAIL); } - if (qctx->qtype == dns_rdatatype_rrsig && dns_db_issecure(qctx->db)) { - char namebuf[DNS_NAME_FORMATSIZE]; - dns_name_format(qctx->client->query.qname, - namebuf, sizeof(namebuf)); - ns_client_log(qctx->client, DNS_LOGCATEGORY_DNSSEC, - NS_LOGMODULE_QUERY, ISC_LOG_WARNING, - "missing signature for %s", namebuf); - } - - qctx->fname = ns_client_newname(qctx->client, qctx->dbuf, &b); - return (query_sign_nodata(qctx)); + return (ns_query_done(qctx)); cleanup: return (result);