From f3b24ba789bed8e561519909709e1671bb290c7f Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 8 May 2023 17:39:51 +1000 Subject: [PATCH 1/4] Handle FORMERR on unknown EDNS option that are echoed If the resolver received a FORMERR response to a request with an DNS COOKIE option present that echoes the option back, resend the request without an DNS COOKIE option present. --- lib/dns/include/dns/message.h | 1 + lib/dns/resolver.c | 37 ++++++++++++++++++++++++----------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/lib/dns/include/dns/message.h b/lib/dns/include/dns/message.h index 0c7cb4ee44..637675fd9a 100644 --- a/lib/dns/include/dns/message.h +++ b/lib/dns/include/dns/message.h @@ -279,6 +279,7 @@ struct dns_message { unsigned int free_saved : 1; unsigned int cc_ok : 1; unsigned int cc_bad : 1; + unsigned int cc_echoed : 1; unsigned int tkey : 1; unsigned int rdclass_set : 1; unsigned int fuzzing : 1; diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 2564f09de0..3295b55a9a 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -7996,10 +7996,15 @@ rctx_opt(respctx_t *rctx) { } /* Cookie OK */ - query->rmessage->cc_ok = 1; - inc_stats(fctx->res, dns_resstatscounter_cookieok); - dns_adb_setcookie(fctx->adb, query->addrinfo, optvalue, - optlen); + if (optlen == CLIENT_COOKIE_SIZE) { + query->rmessage->cc_echoed = 1; + } else { + query->rmessage->cc_ok = 1; + inc_stats(fctx->res, + dns_resstatscounter_cookieok); + dns_adb_setcookie(fctx->adb, query->addrinfo, + optvalue, optlen); + } break; default: break; @@ -9663,13 +9668,23 @@ rctx_badserver(respctx_t *rctx, isc_result_t result) { add_bad_edns(fctx, &query->addrinfo->sockaddr); inc_stats(fctx->res, dns_resstatscounter_edns0fail); } else if (rcode == dns_rcode_formerr) { - /* - * The server (or forwarder) doesn't understand us, - * but others might. - */ - rctx->next_server = true; - rctx->broken_server = DNS_R_REMOTEFORMERR; - log_formerr(fctx, "server sent FORMERR"); + if (query->rmessage->cc_echoed) { + /* + * Retry without DNS COOKIE. + */ + query->addrinfo->flags |= FCTX_ADDRINFO_NOCOOKIE; + rctx->resend = true; + log_formerr(fctx, "server sent FORMERR with echoed DNS " + "COOKIE"); + } else { + /* + * The server (or forwarder) doesn't understand us, + * but others might. + */ + rctx->next_server = true; + rctx->broken_server = DNS_R_REMOTEFORMERR; + log_formerr(fctx, "server sent FORMERR"); + } } else if (rcode == dns_rcode_badvers) { unsigned int version; #if DNS_EDNS_VERSION > 0 From 3328ddaf7a7b2527b9338134e0acbd2895beaa3b Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 8 May 2023 16:07:19 +1000 Subject: [PATCH 2/4] Add a server which returns FORMERR to all EDNS options The server also echoes back the EDNS options and EDNS flags. --- bin/tests/system/resolver/ans10/ans.py | 152 +++++++++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 bin/tests/system/resolver/ans10/ans.py diff --git a/bin/tests/system/resolver/ans10/ans.py b/bin/tests/system/resolver/ans10/ans.py new file mode 100644 index 0000000000..6e95dbbfc6 --- /dev/null +++ b/bin/tests/system/resolver/ans10/ans.py @@ -0,0 +1,152 @@ +# 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. + +from __future__ import print_function +import os +import sys +import signal +import socket +import select +from datetime import datetime, timedelta +import time +import functools + +import dns, dns.message, dns.query, dns.flags +from dns.rdatatype import * +from dns.rdataclass import * +from dns.rcode import * +from dns.name import * + + +# Log query to file +def logquery(type, qname): + with open("qlog", "a") as f: + f.write("%s %s\n", type, qname) + + +############################################################################ +# Respond to a DNS query. +# If there are EDNS options present return FORMERR copying the OPT record. +# Otherwise: +# SOA gets a unsigned response. +# NS gets a unsigned response. +# A gets a unsigned response. +# All other types get a unsigned NODATA response. +############################################################################ +def create_response(msg): + m = dns.message.from_wire(msg) + qname = m.question[0].name.to_text() + rrtype = m.question[0].rdtype + typename = dns.rdatatype.to_text(rrtype) + + with open("query.log", "a") as f: + f.write("%s %s\n" % (typename, qname)) + print("%s %s" % (typename, qname), end=" ") + + if m.edns != -1 and len(m.options) != 0: + r = dns.message.make_response(m) + r.use_edns( + edns=m.edns, ednsflags=m.ednsflags, payload=m.payload, options=m.options + ) + r.set_rcode(FORMERR) + else: + r = dns.message.make_response(m) + r.set_rcode(NOERROR) + if rrtype == A: + r.answer.append(dns.rrset.from_text(qname, 1, IN, A, "10.53.0.10")) + elif rrtype == NS: + 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")) + else: + r.authority.append(dns.rrset.from_text(qname, 1, IN, SOA, ". . 0 0 0 0 0")) + r.flags |= dns.flags.AA + return r + + +def sigterm(signum, frame): + print("Shutting down now...") + os.remove("ans.pid") + running = False + sys.exit(0) + + +############################################################################ +# Main +# +# Set up responder and control channel, open the pid file, and start +# the main loop, listening for queries on the query channel or commands +# on the control channel and acting on them. +############################################################################ +ip4 = "10.53.0.10" +ip6 = "fd92:7065:b8e:ffff::10" + +try: + port = int(os.environ["PORT"]) +except: + port = 5300 + +query4_socket = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) +query4_socket.bind((ip4, port)) +havev6 = True +try: + query6_socket = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM) + try: + query6_socket.bind((ip6, port)) + except: + query6_socket.close() + havev6 = False +except: + havev6 = False +signal.signal(signal.SIGTERM, sigterm) + +f = open("ans.pid", "w") +pid = os.getpid() +print(pid, file=f) +f.close() + +running = True + +print("Listening on %s port %d" % (ip4, port)) +if havev6: + print("Listening on %s port %d" % (ip6, port)) +print("Ctrl-c to quit") + +if havev6: + input = [query4_socket, query6_socket] +else: + input = [query4_socket] + +while running: + try: + inputready, outputready, exceptready = select.select(input, [], []) + except select.error as e: + break + except socket.error as e: + break + except KeyboardInterrupt: + break + + for s in inputready: + if s == query4_socket or s == query6_socket: + print( + "Query received on %s" % (ip4 if s == query4_socket else ip6), end=" " + ) + # Handle incoming queries + msg = s.recvfrom(65535) + rsp = create_response(msg[0]) + if rsp: + print(dns.rcode.to_text(rsp.rcode())) + s.sendto(rsp.to_wire(), msg[1]) + else: + print("NO RESPONSE") + if not running: + break From 9d95cd427d7f0a4b77511ac89a9d51195ba9035c Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 8 May 2023 16:26:29 +1000 Subject: [PATCH 3/4] Check fallback on FORMERR to EDNS options --- bin/tests/system/resolver/ns4/root.db | 2 ++ bin/tests/system/resolver/tests.sh | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/bin/tests/system/resolver/ns4/root.db b/bin/tests/system/resolver/ns4/root.db index 330b00e377..3c93edcf17 100644 --- a/bin/tests/system/resolver/ns4/root.db +++ b/bin/tests/system/resolver/ns4/root.db @@ -33,3 +33,5 @@ ns.targetns. A 10.53.0.6 partial-formerr. NS ns.partial-formerr. ns.partial-formerr. A 10.53.0.2 ns.partial-formerr. A 10.53.0.3 +options-formerr. NS ns.options-formerr. +ns.options-formerr. A 10.53.0.10 diff --git a/bin/tests/system/resolver/tests.sh b/bin/tests/system/resolver/tests.sh index 9bd258e8c1..f6acafeb01 100755 --- a/bin/tests/system/resolver/tests.sh +++ b/bin/tests/system/resolver/tests.sh @@ -781,6 +781,18 @@ grep "ANSWER: [12]," dig.out.2.${n} > /dev/null || ret=1 lines=$(awk '$1 == "mixedttl.tld." && $2 > 30 { print }' dig.out.2.${n} | wc -l) test ${lines:-1} -ne 0 && ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi + +n=$((n+1)) +echo_i "check resolver behavior when FORMERR for EDNS options happens (${n})" +ret=0 +msg="resolving options-formerr/A .* server sent FORMERR with echoed DNS COOKIE" +if [ $ret != 0 ]; then echo_i "failed"; fi +nextpart ns5/named.run >/dev/null +dig_with_opts +tcp @10.53.0.5 options-formerr A > dig.out.${n} || ret=1 +grep "status: NOERROR" dig.out.${n} > /dev/null || ret=1 +nextpart ns5/named.run | grep "$msg" > /dev/null || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi + status=$((status + ret)) echo_i "exit status: $status" From 97e1bb0e985416b869bf7002398163ae9b98bba1 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 8 May 2023 19:39:54 +1000 Subject: [PATCH 4/4] Add CHANGES note for [GL #4049] --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 3a07a49898..3673b44fd3 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6166. [func] Retry without DNS COOKIE on FORMERR if it appears that + the FORMERR was due to the presence of a DNS COOKIE + option. [GL #4049] + 6165. [bug] Fix a logic error in dighost.c which could call the dighost_shutdown() callback twice and cause problems if the callback function was not idempotent. [GL #4039]