when forwarding, try with CD=0 first

when sending a query to a forwarder for a name within a secure domain,
the first query is now sent with CD=0. when the forwarder itself
is validating, this will give it a chance to detect bogus data and
replace it with valid data before answering. this reduces our chances
of being stuck with data that can't be validated.

if the forwarder returns SERVFAIL to the initial query, the query
will be repeated with CD=1, to allow for the possibility that the
forwarder's validator is faulty or that the bogus answer is covered
by an NTA.

note: previously, CD=1 was only sent when the query name was in a
secure domain. today, validating servers have a trust anchor at the
root by default, so virtually all queries are in a secure domain.
therefore, the code has been simplified.  as long as validation is
enabled, any forward query that receives a SERVFAIL response will be
retried with CD=1.

diff --git a/bin/tests/system/dnssec/ns2/example.db.in b/bin/tests/system/dnssec/ns2/example.db.in
index 93b7f708af14452ad84e74681c8bb83de8bce610..a7ec0e471c82ebab2cf94da953f25712225b5211 100644 (file)
--- a/bin/tests/system/dnssec/ns2/example.db.in
+++ b/bin/tests/system/dnssec/ns2/example.db.in
@@ -67,6 +67,10 @@ ns.bogus             A       10.53.0.3
 badds                  NS      ns.badds
 ns.badds               A       10.53.0.3
 
+; A subdomain with a corrupt DS, but locally trusted by the forwarder
+localkey               NS      ns.localkey
+ns.localkey            A       10.53.0.3
+
 ; A dynamic secure subdomain
 dynamic                        NS      dynamic
 dynamic                        A       10.53.0.3

diff --git a/bin/tests/system/dnssec/ns2/sign.sh b/bin/tests/system/dnssec/ns2/sign.sh
index 917da71e4be54c30fe7c0baa69030e8fb2fa177e..66fc96a676322a7c729b3b92122649dc4f20f87b 100644 (file)
--- a/bin/tests/system/dnssec/ns2/sign.sh
+++ b/bin/tests/system/dnssec/ns2/sign.sh
@@ -59,7 +59,7 @@ zonefile=example.db
 
 # Get the DS records for the "example." zone.
 for subdomain in digest-alg-unsupported ds-unsupported secure badds \
-  bogus dynamic keyless nsec3 optout \
+  bogus localkey dynamic keyless nsec3 optout \
   nsec3-unknown optout-unknown multiple rsasha256 rsasha512 \
   kskonly update-nsec3 auto-nsec auto-nsec3 secure.below-cname \
   ttlpatch split-dnssec split-smart expired expiring upper lower \

diff --git a/bin/tests/system/dnssec/ns3/named.conf.in b/bin/tests/system/dnssec/ns3/named.conf.in
index 917e2b72993730ac27b17f3b8e041b9a758ba6e6..c7f76b4638f50df6597553f74463cd534d66efe9 100644 (file)
--- a/bin/tests/system/dnssec/ns3/named.conf.in
+++ b/bin/tests/system/dnssec/ns3/named.conf.in
@@ -103,6 +103,12 @@ zone "badds.example" {
        allow-update { any; };
 };
 
+zone "localkey.example" {
+       type primary;
+       file "localkey.example.db.signed";
+       allow-update { any; };
+};
+
 zone "dynamic.example" {
        type primary;
        file "dynamic.example.db.signed";

diff --git a/bin/tests/system/dnssec/ns3/sign.sh b/bin/tests/system/dnssec/ns3/sign.sh
index 288ee6bc19bbffc86f8b1028668a2270a64df8e6..79cac88dc98769f7c40598a3e41e98a34ad43ebc 100644 (file)
--- a/bin/tests/system/dnssec/ns3/sign.sh
+++ b/bin/tests/system/dnssec/ns3/sign.sh
@@ -620,6 +620,21 @@ cat "$infile" "$keyname.key" >"$zonefile"
 "$SIGNER" -P -o "$zone" "$zonefile" >/dev/null
 sed -e 's/bogus/badds/g' <dsset-bogus.example. >dsset-badds.example.
 
+#
+# Same as badds, but locally trusted by the forwarder
+#
+zone=localkey.example.
+infile=bogus.example.db.in
+zonefile=localkey.example.db
+
+keyname=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -n zone "$zone")
+
+cat "$infile" "$keyname.key" >"$zonefile"
+
+"$SIGNER" -P -o "$zone" "$zonefile" >/dev/null
+sed -e 's/bogus/localkey/g' <dsset-bogus.example. >dsset-localkey.example.
+keyfile_to_static_keys $keyname >../ns9/trusted-localkey.conf
+
 #
 # A zone with future signatures.
 #

diff --git a/bin/tests/system/dnssec/ns4/named5.conf.in b/bin/tests/system/dnssec/ns4/named5.conf.in
new file mode 100644 (file)
index 0000000..16c8126
--- /dev/null
+++ b/bin/tests/system/dnssec/ns4/named5.conf.in
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ *
+ * 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 https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+// NS4
+
+options {
+       query-source address 10.53.0.4;
+       notify-source 10.53.0.4;
+       transfer-source 10.53.0.4;
+       port @PORT@;
+       pid-file "named.pid";
+       listen-on { 10.53.0.4; };
+       listen-on-v6 { none; };
+       recursion yes;
+       dnssec-validation yes;
+       minimal-responses no;
+
+};
+
+# Note: This is deliberately wrong! The bind.keys file contains
+# the real DNS root key, so it won't work with the local toy
+# root zones used in the tests. This is to test a forwarder
+# talking to a resolver with a misconfigured trust anchor.
+include "../../../../../bind.keys";
+
+key rndc_key {
+       secret "1234abcd8765";
+       algorithm @DEFAULT_HMAC@;
+};
+
+controls {
+       inet 10.53.0.4 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};
+
+zone "." {
+       type hint;
+       file "../../_common/root.hint";
+};
+
+zone "corp" {
+       type static-stub;
+       server-addresses { 10.53.0.2; };
+};

diff --git a/bin/tests/system/dnssec/ns9/named.conf.in b/bin/tests/system/dnssec/ns9/named.conf.in
index cdbe7ec8eaccf8a9ec55f79239324d133aa61bbf..147d328ccfa990d4a87247b0a6696ec42aa82556 100644 (file)
--- a/bin/tests/system/dnssec/ns9/named.conf.in
+++ b/bin/tests/system/dnssec/ns9/named.conf.in
@@ -38,3 +38,4 @@ controls {
 };
 
 include "trusted.conf";
+include "trusted-localkey.conf";

diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh
index 861673394277000890e922b8d595a6057c3fb698..04673b361a5df7509f39b0769d0b52b93da8886b 100644 (file)
--- a/bin/tests/system/dnssec/tests.sh
+++ b/bin/tests/system/dnssec/tests.sh
@@ -4723,5 +4723,56 @@ n=$((n + 1))
 if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
 status=$((status + ret))
 
+echo_i "checking validating forwarder behavior with mismatching NS ($n)"
+ret=0
+rndccmd 10.53.0.4 flush 2>&1 | sed 's/^/ns4 /' | cat_i
+$DIG +tcp +cd -p "$PORT" -t ns inconsistent @10.53.0.9 >dig.out.ns9.test$n.1 || ret=1
+grep "ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 1" dig.out.ns9.test$n.1 >/dev/null || ret=1
+grep "flags:.*ad.*QUERY" dig.out.ns9.test$n.1 >/dev/null && ret=1
+$DIG +tcp +cd +dnssec -p "$PORT" -t ns inconsistent @10.53.0.9 >dig.out.ns9.test$n.2 || ret=1
+grep "ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 1" dig.out.ns9.test$n.2 >/dev/null || ret=1
+grep "flags:.*ad.*QUERY" dig.out.ns9.test$n.2 >/dev/null && ret=1
+$DIG +tcp +dnssec -p "$PORT" -t ns inconsistent @10.53.0.9 >dig.out.ns9.test$n.3 || ret=1
+grep "ANSWER: 3, AUTHORITY: 0, ADDITIONAL: 1" dig.out.ns9.test$n.3 >/dev/null || ret=1
+grep "flags:.*ad.*QUERY" dig.out.ns9.test$n.3 >/dev/null || ret=1
+n=$((n + 1))
+if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+status=$((status + ret))
+
+echo_i "checking forwarder CD behavior (DS mismatch and local trust anchor) ($n)"
+ret=0
+rndccmd 10.53.0.4 flush 2>&1 | sed 's/^/ns4 /' | cat_i
+# confirm invalid DS produces SERVFAIL in resolver
+$DIG +tcp +dnssec -p "$PORT" @10.53.0.4 localkey.example soa >dig.out.ns4.test$n || ret=1
+grep "status: SERVFAIL" dig.out.ns4.test$n >/dev/null || ret=1
+# check that lookup using forwarder succeeds and that SERVFAIL was received
+nextpart ns9/named.run >/dev/null
+$DIG +tcp +dnssec -p "$PORT" @10.53.0.9 localkey.example soa >dig.out.ns9.test$n || ret=1
+grep "status: NOERROR" dig.out.ns9.test$n >/dev/null || ret=1
+grep "flags:.*ad.*QUERY" dig.out.ns9.test$n >/dev/null || ret=1
+nextpart ns9/named.run | grep 'status: SERVFAIL' >/dev/null || ret=1
+n=$((n + 1))
+if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
+status=$((status + ret))
+
+copy_setports ns4/named5.conf.in ns4/named.conf
+rndccmd 10.53.0.4 reconfig 2>&1 | sed 's/^/ns4 /' | cat_i
+sleep 3
+
+echo_i "checking forwarder CD behavior (forward server with bad trust anchor) ($n)"
+ret=0
+# confirm invalid trust anchor produces SERVFAIL in resolver
+$DIG +tcp +dnssec -p "$PORT" @10.53.0.4 a.secure.example >dig.out.ns4.test$n || ret=1
+grep "status: SERVFAIL" dig.out.ns4.test$n >/dev/null || ret=1
+# check that lookup using forwarder succeeds and that SERVFAIL was received
+nextpart ns9/named.run >/dev/null
+$DIG +tcp +dnssec -p "$PORT" @10.53.0.9 a.secure.example soa >dig.out.ns9.test$n || ret=1
+grep "status: NOERROR" dig.out.ns9.test$n >/dev/null || ret=1
+grep "flags:.*ad.*QUERY" dig.out.ns9.test$n >/dev/null || ret=1
+nextpart ns9/named.run | grep 'status: SERVFAIL' >/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

diff --git a/bin/tests/system/dnssec/tests_sh_dnssec.py b/bin/tests/system/dnssec/tests_sh_dnssec.py
index cf374f1dfb5939624744effdf589dfc801f2ea4c..91a8ba24eb60adadf962ae12d29cd10422269d16 100644 (file)
--- a/bin/tests/system/dnssec/tests_sh_dnssec.py
+++ b/bin/tests/system/dnssec/tests_sh_dnssec.py
@@ -104,6 +104,7 @@ pytestmark = pytest.mark.extra_artifacts(
         "ns3/future.example.db",
         "ns3/keyless.example.db",
         "ns3/kskonly.example.db",
+        "ns3/localkey.example.db",
         "ns3/lower.example.db",
         "ns3/managed-future.example.db",
         "ns3/multiple.example.db",
@@ -152,10 +153,10 @@ pytestmark = pytest.mark.extra_artifacts(
         "ns4/named_dump.db",
         "ns4/named_dump.db.*",
         "ns5/revoked.conf",
-        "ns5/trusted.conf",
         "ns6/optout-tld.db",
         "ns7/split-rrsig.db",
         "ns7/split-rrsig.db.unsplit",
+        "ns9/trusted-localkey.conf",
         "signer/example.db",
         "signer/example.db.after",
         "signer/example.db.before",

diff --git a/lib/dns/include/dns/resolver.h b/lib/dns/include/dns/resolver.h
index 0b00df679cebe280f245103a16184ab717d8b538..459e45173106c64ab394c4afde436eabb7d6cd59 100644 (file)
--- a/lib/dns/include/dns/resolver.h
+++ b/lib/dns/include/dns/resolver.h
@@ -131,6 +131,11 @@ enum {
                                                 * possible. */
        DNS_FETCHOPT_QMINFETCH = 1 << 16,       /*%< Qmin fetch */
        DNS_FETCHOPT_WANTZONEVERSION = 1 << 17, /*%< Request ZONEVERSION */
+       DNS_FETCHOPT_TRYCD = 1 << 18,           /*%< Send the first query
+                                                * to a forwader with
+                                                * CD=0, but retry with CD=1
+                                                * if it returns SERVFAIL.
+                                                */
 
        /*% EDNS version bits: */
        DNS_FETCHOPT_EDNSVERSIONSET = 1 << 23,

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index aa3050b8a65227cea46d79e1c7293c73bffec7f3..646e702974a7bf45b23157636911baeba244a3ce 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -2345,7 +2345,6 @@ resquery_send(resquery_t *query) {
        dns_peer_t *peer = NULL;
        dns_compress_t cctx;
        bool useedns;
-       bool secure_domain;
        bool tcp = ((query->options & DNS_FETCHOPT_TCP) != 0);
        dns_ednsopt_t ednsopts[DNS_EDNSOPTIONS];
        unsigned int ednsopt = 0;
@@ -2396,24 +2395,14 @@ resquery_send(resquery_t *query) {
         */
        if ((query->options & DNS_FETCHOPT_NOCDFLAG) != 0) {
                /* Do nothing */
-       } else if ((query->options & DNS_FETCHOPT_NOVALIDATE) != 0) {
+       } else if ((query->options & DNS_FETCHOPT_NOVALIDATE) != 0 ||
+                  (query->options & DNS_FETCHOPT_TRYCD) != 0)
+       {
                fctx->qmessage->flags |= DNS_MESSAGEFLAG_CD;
        } else if (res->view->enablevalidation &&
                   ((fctx->qmessage->flags & DNS_MESSAGEFLAG_RD) != 0))
        {
-               bool checknta = ((query->options & DNS_FETCHOPT_NONTA) == 0);
-               bool ntacovered = false;
-               result = issecuredomain(res->view, fctx->name, fctx->type,
-                                       isc_time_seconds(&query->start),
-                                       checknta, &ntacovered, &secure_domain);
-               if (result != ISC_R_SUCCESS) {
-                       secure_domain = false;
-               }
-               if (secure_domain ||
-                   (ISFORWARDER(query->addrinfo) && ntacovered))
-               {
-                       fctx->qmessage->flags |= DNS_MESSAGEFLAG_CD;
-               }
+               query->options |= DNS_FETCHOPT_TRYCD;
        }
 
        /*
@@ -7788,6 +7777,8 @@ resquery_response_continue(void *arg, isc_result_t result) {
        fetchctx_t *fctx = rctx->fctx;
        resquery_t *query = rctx->query;
 
+       QTRACE("response_continue");
+
        if (result != ISC_R_SUCCESS) {
                FCTXTRACE3("signature check failed", result);
                if (result == DNS_R_UNEXPECTEDTSIG ||
@@ -10006,6 +9997,8 @@ rctx_badserver(respctx_t *rctx, isc_result_t result) {
        char code[64];
        dns_rcode_t rcode = rctx->query->rmessage->rcode;
 
+       QTRACE("rctx_badserver");
+
        if (rcode == dns_rcode_noerror || rcode == dns_rcode_yxdomain ||
            rcode == dns_rcode_nxdomain)
        {
@@ -10100,6 +10093,16 @@ rctx_badserver(respctx_t *rctx, isc_result_t result) {
                }
                query->addrinfo->flags |= FCTX_ADDRINFO_BADCOOKIE;
                rctx->resend = true;
+       } else if (ISFORWARDER(query->addrinfo) &&
+                  query->rmessage->rcode == dns_rcode_servfail &&
+                  (query->options & DNS_FETCHOPT_TRYCD) != 0)
+       {
+               /*
+                * We got a SERVFAIL from a forwarder with
+                * CD=0; try again with CD=1.
+                */
+               rctx->retryopts |= DNS_FETCHOPT_TRYCD;
+               rctx->resend = true;
        } else {
                rctx->broken_server = DNS_R_UNEXPECTEDRCODE;
                rctx->next_server = true;