From 2c42324e26f7290ad282538f92eda29d864d3bcf Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 19 Feb 2025 10:34:47 +1100 Subject: [PATCH 1/2] Remove check for missing RRSIG records from getsection Checking whether the authority section is properly signed should be left to the validator. Checking in getsection (dns_message_parse) was way too early and resulted in resolution failures of lookups that should have otherwise succeeded. (cherry picked from commit 83159d0a545be2845f08386f5dffdc2ac3721ba5) --- lib/dns/message.c | 71 ----------------------------------------------- 1 file changed, 71 deletions(-) diff --git a/lib/dns/message.c b/lib/dns/message.c index 1ac370bc05..63c07abecb 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -1197,62 +1197,6 @@ update(dns_section_t section, dns_rdataclass_t rdclass) { return false; } -/* - * Check to confirm that all DNSSEC records (DS, NSEC, NSEC3) have - * covering RRSIGs. - */ -static bool -auth_signed(dns_namelist_t *section) { - dns_name_t *name; - - for (name = ISC_LIST_HEAD(*section); name != NULL; - name = ISC_LIST_NEXT(name, link)) - { - int auth_dnssec = 0, auth_rrsig = 0; - dns_rdataset_t *rds; - - for (rds = ISC_LIST_HEAD(name->list); rds != NULL; - rds = ISC_LIST_NEXT(rds, link)) - { - switch (rds->type) { - case dns_rdatatype_ds: - auth_dnssec |= 0x1; - break; - case dns_rdatatype_nsec: - auth_dnssec |= 0x2; - break; - case dns_rdatatype_nsec3: - auth_dnssec |= 0x4; - break; - case dns_rdatatype_rrsig: - break; - default: - continue; - } - - switch (rds->covers) { - case dns_rdatatype_ds: - auth_rrsig |= 0x1; - break; - case dns_rdatatype_nsec: - auth_rrsig |= 0x2; - break; - case dns_rdatatype_nsec3: - auth_rrsig |= 0x4; - break; - default: - break; - } - } - - if (auth_dnssec != auth_rrsig) { - return false; - } - } - - return true; -} - static isc_result_t getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, dns_section_t sectionid, unsigned int options) { @@ -1723,21 +1667,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, INSIST(!free_name); } - /* - * If any of DS, NSEC or NSEC3 appeared in the - * authority section of a query response without - * a covering RRSIG, FORMERR - */ - if (sectionid == DNS_SECTION_AUTHORITY && - msg->opcode == dns_opcode_query && - ((msg->flags & DNS_MESSAGEFLAG_QR) != 0) && - ((msg->flags & DNS_MESSAGEFLAG_TC) == 0) && !preserve_order && - !auth_signed(section)) - { - /* XXX test coverage */ - DO_ERROR(DNS_R_FORMERR); - } - if (seen_problem) { result = DNS_R_RECOVERABLE; } From fef505206799c6a7eb95318a54dae8dcd3761f0f Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 19 Feb 2025 12:31:16 +1100 Subject: [PATCH 2/2] Check insecure response with missing RRSIG in authority This scenario should succeed but wasn't due rejection of the message at the message parsing stage. (cherry picked from commit 4271d93f00909fad74d694121da970b1a633c495) --- bin/tests/system/dnssec/ans10/ans.py | 6 ++++++ bin/tests/system/dnssec/ns1/root.db.in | 2 ++ bin/tests/system/dnssec/tests.sh | 16 ++++++++++++++++ 3 files changed, 24 insertions(+) diff --git a/bin/tests/system/dnssec/ans10/ans.py b/bin/tests/system/dnssec/ans10/ans.py index dbe49e5e5a..84bf0a2642 100644 --- a/bin/tests/system/dnssec/ans10/ans.py +++ b/bin/tests/system/dnssec/ans10/ans.py @@ -38,6 +38,7 @@ def logquery(type, qname): # NS gets a unsigned response. # DNSKEY get a unsigned NODATA response. # A gets a signed response. +# TXT gets a signed NODATA response without RRSIG. # All other types get a unsigned NODATA response. ############################################################################ def create_response(msg): @@ -72,6 +73,11 @@ def create_response(msg): r.answer.append(dns.rrset.from_text(qname, 1, IN, NS, ".")) elif rrtype == SOA: r.answer.append(dns.rrset.from_text(qname, 1, IN, SOA, ". . 0 0 0 0 0")) + elif rrtype == TXT: + r.authority.append(dns.rrset.from_text(qname, 1, IN, SOA, ". . 0 0 0 0 0")) + r.authority.append( + dns.rrset.from_text(qname, 1, IN, NSEC, qname + " A NS SOA RRSIG NSEC") + ) else: r.authority.append(dns.rrset.from_text(qname, 1, IN, SOA, ". . 0 0 0 0 0")) r.flags |= dns.flags.AA diff --git a/bin/tests/system/dnssec/ns1/root.db.in b/bin/tests/system/dnssec/ns1/root.db.in index 52d88ab500..1e7a5c79cf 100644 --- a/bin/tests/system/dnssec/ns1/root.db.in +++ b/bin/tests/system/dnssec/ns1/root.db.in @@ -43,3 +43,5 @@ ds-rrsigs-stripped. NS ns2.ds-rrsigs-stripped. ns2.ds-rrsigs-stripped. A 10.53.0.2 inconsistent. NS ns2.inconsistent. ns2.inconsistent. A 10.53.0.2 +nsec-rrsigs-stripped. NS ns10.nsec-rrsigs-stripped. +ns10.nsec-rrsigs-stripped. A 10.53.0.10 diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index c022e8afca..2b2ff2fbb6 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -4553,5 +4553,21 @@ n=$((n + 1)) if [ "$ret" -ne 0 ]; then echo_i "failed"; fi status=$((status + ret)) +echo_i "checking that a insecure negative response where there is a NSEC without a RRSIG succeeds ($n)" +ret=0 +# check server preconditions +dig_with_opts +notcp @10.53.0.10 nsec-rrsigs-stripped. TXT +dnssec >dig.out.ns10.test$n +grep "status: NOERROR" dig.out.ns10.test$n >/dev/null || ret=1 +grep "QUERY: 1, ANSWER: 0, AUTHORITY: 2, ADDITIONAL: 1" dig.out.ns10.test$n >/dev/null || ret=1 +grep "IN.RRSIG.NSEC" dig.out.ns10.test$n >/dev/null && ret=1 +# check resolver succeeds +dig_with_opts @10.53.0.4 nsec-rrsigs-stripped. TXT +dnssec >dig.out.ns4.test$n +grep "status: NOERROR" dig.out.ns4.test$n >/dev/null || ret=1 +grep "QUERY: 1, ANSWER: 0, AUTHORITY: 2, ADDITIONAL: 1" dig.out.ns4.test$n >/dev/null || ret=1 +grep "IN.RRSIG.NSEC" dig.out.ns4.test$n >/dev/null && ret=1 +n=$((n + 1)) +if [ "$ret" -ne 0 ]; then echo_i "failed"; fi +status=$((status + ret)) + echo_i "exit status: $status" [ $status -eq 0 ] || exit 1