From 36cf1c6a5bf943ad718ddba9fbe6ea97810e3bc2 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Fri, 24 Jan 2025 18:00:14 -0800 Subject: [PATCH] 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. --- bin/tests/system/dnssec/ns2/example.db.in | 4 ++ bin/tests/system/dnssec/ns2/sign.sh | 2 +- bin/tests/system/dnssec/ns3/named.conf.in | 6 +++ bin/tests/system/dnssec/ns3/sign.sh | 15 ++++++ bin/tests/system/dnssec/ns4/named5.conf.in | 53 ++++++++++++++++++++++ bin/tests/system/dnssec/ns9/named.conf.in | 1 + bin/tests/system/dnssec/tests.sh | 51 +++++++++++++++++++++ bin/tests/system/dnssec/tests_sh_dnssec.py | 3 +- lib/dns/include/dns/resolver.h | 5 ++ lib/dns/resolver.c | 33 ++++++++------ 10 files changed, 156 insertions(+), 17 deletions(-) create mode 100644 bin/tests/system/dnssec/ns4/named5.conf.in diff --git a/bin/tests/system/dnssec/ns2/example.db.in b/bin/tests/system/dnssec/ns2/example.db.in index 93b7f708af..a7ec0e471c 100644 --- 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 917da71e4b..66fc96a676 100644 --- 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 917e2b7299..c7f76b4638 100644 --- 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 288ee6bc19..79cac88dc9 100644 --- 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-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-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 index 0000000000..16c81264e1 --- /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 cdbe7ec8ea..147d328ccf 100644 --- 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 8616733942..04673b361a 100644 --- 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 cf374f1dfb..91a8ba24eb 100644 --- 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 0b00df679c..459e451731 100644 --- 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 aa3050b8a6..646e702974 100644 --- 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;