From a3fda086f7af09271a0261587563b5f36516f40d Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 1 Feb 2021 13:05:08 +1100 Subject: [PATCH 1/5] Check that there was no OPT record before falling back to plain DNS on FORMERR. --- lib/dns/resolver.c | 2 +- lib/dns/zone.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 847ead38a8..ebc7fbc2cf 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -10005,7 +10005,7 @@ rctx_badserver(respctx_t *rctx, isc_result_t result) { return (ISC_R_SUCCESS); } - if ((rcode == dns_rcode_formerr) && + if ((rcode == dns_rcode_formerr) && rctx->opt == NULL && (rctx->retryopts & DNS_FETCHOPT_NOEDNS0) == 0) { /* diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 70528f448b..6f32655a18 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -13606,7 +13606,7 @@ stub_callback(isc_task_t *task, isc_event_t *event) { if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOEDNS) && (msg->rcode == dns_rcode_servfail || msg->rcode == dns_rcode_notimp || - msg->rcode == dns_rcode_formerr)) + (msg->rcode == dns_rcode_formerr && msg->opt == NULL))) { dns_zone_log(zone, ISC_LOG_DEBUG(1), "refreshing stub: rcode (%.*s) retrying " @@ -13995,7 +13995,7 @@ refresh_callback(isc_task_t *task, isc_event_t *event) { if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOEDNS) && (msg->rcode == dns_rcode_servfail || msg->rcode == dns_rcode_notimp || - msg->rcode == dns_rcode_formerr)) + (msg->rcode == dns_rcode_formerr && msg->opt == NULL))) { dns_zone_log(zone, ISC_LOG_DEBUG(1), "refresh: rcode (%.*s) retrying without " From 0477938e2f99f6db43eb31dd5b688821d90d623e Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 1 Feb 2021 14:37:20 +1100 Subject: [PATCH 2/5] Adjust expected queries for no fallback to plain DNS --- bin/tests/system/qmin/tests.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/bin/tests/system/qmin/tests.sh b/bin/tests/system/qmin/tests.sh index 82eb657cbe..e6a2684643 100755 --- a/bin/tests/system/qmin/tests.sh +++ b/bin/tests/system/qmin/tests.sh @@ -233,7 +233,6 @@ sort ans2/query.log > ans2/query.log.sorted cat << __EOF | $DIFF ans2/query.log.sorted - > /dev/null || ret=1 ADDR ns2.ugly. NS boing.ugly. -NS boing.ugly. NS ugly. __EOF for ans in ans2 ans3 ans4; do mv -f $ans/query.log query-$ans-$n.log 2>/dev/null || true; done From 3c942a3e3acc1684e6655ae1e08e4291a96fdbb5 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 12 Jul 2021 12:29:55 +1000 Subject: [PATCH 3/5] Update out of date comment --- lib/dns/resolver.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index ebc7fbc2cf..f8b9c4d9a9 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -10010,14 +10010,6 @@ rctx_badserver(respctx_t *rctx, isc_result_t result) { { /* * It's very likely they don't like EDNS0. - * If the response code is SERVFAIL, also check if the - * response contains an OPT RR and don't cache the - * failure since it can be returned for various other - * reasons. - * - * XXXRTH We should check if the question - * we're asking requires EDNS0, and - * if so, we should bail out. */ rctx->retryopts |= DNS_FETCHOPT_NOEDNS0; rctx->resend = true; From 5ab0c9fdfe8e7c5e52665581d7aeb82b7a4d03d5 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 1 Feb 2021 13:35:56 +1100 Subject: [PATCH 4/5] Add CHANGES note for [GL #2249] --- CHANGES | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGES b/CHANGES index 594c850699..0dd12c7292 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,10 @@ +5677. [func] Only accept FORMERR without a OPT record as an + indication that the server does net support EDNS. + This will break communication with servers that + don't understand EDNS and incorrectly echo back + the request message with the rcode field set to + FORMERR and the QR bit set to 1. [GL #2249] + 5676. [func] Memory allocation has been substantially refactored, and is now based on the memory allocation API provided by 'libjemalloc'. This is now a build From e9c72ca95c11d8992ceed60e0a959c50e456aca7 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 1 Feb 2021 13:37:26 +1100 Subject: [PATCH 5/5] Add release note for [GL #2249] --- doc/notes/notes-current.rst | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 58b24f4ffb..b337129c16 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -49,13 +49,21 @@ Feature Changes - DNS over HTTPS support can be disabled at the compile time via the new configuration option ``--disable-doh``. This allows BIND 9 to be - compiled without libnghttp2 library. [GL #2478] + compiled without libnghttp2 library. :gl:`#2478` - Memory allocation has been substantially refactored, and is now based on the memory allocation API provided by the `jemalloc` library on platforms where it is available. This library is now recommended for building BIND 9. :gl:`#2433` +- Previously, named accepted FORMERR responses both with and without + an OPT record, as an indication that a given server did not support + EDNS. To implement full compliance with RFC 6891, only FORMERR + responses without an OPT record are now accepted. This intentionally + breaks communication with servers that do not support EDNS and + that incorrectly echo back the query message with the RCODE field + set to FORMERR and the QR bit set to 1. :gl:`#2249` + Bug Fixes ~~~~~~~~~