From: Remi Gacogne Date: Tue, 19 Apr 2022 14:32:46 +0000 (+0200) Subject: dnsdist: Better coverage of the backend's discovery code X-Git-Tag: rec-4.9.0-alpha0~14^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a57367a4705e02dcd7c87963de5704c01e608398;p=thirdparty%2Fpdns.git dnsdist: Better coverage of the backend's discovery code --- diff --git a/pdns/dnsdistdist/dnsdist-discovery.cc b/pdns/dnsdistdist/dnsdist-discovery.cc index 84ddb422e1..df9b6922bc 100644 --- a/pdns/dnsdistdist/dnsdist-discovery.cc +++ b/pdns/dnsdistdist/dnsdist-discovery.cc @@ -50,10 +50,6 @@ struct DesignatedResolvers static bool parseSVCParams(const PacketBuffer& answer, std::map& resolvers) { - if (answer.size() <= sizeof(struct dnsheader)) { - throw std::runtime_error("Looking for SVC records in a packet smaller than a DNS header"); - } - std::map> hints; const struct dnsheader* dh = reinterpret_cast(answer.data()); PacketReader pr(pdns_string_view(reinterpret_cast(answer.data()), answer.size())); @@ -399,17 +395,7 @@ bool ServiceDiscovery::tryToUpgradeBackend(const UpgradeableBackend& backend) DownstreamState::Config config(backend.d_ds->d_config); config.remote = discoveredConfig.d_addr; - if (discoveredConfig.d_port != 0) { - config.remote.setPort(discoveredConfig.d_port); - } - else { - if (discoveredConfig.d_protocol == dnsdist::Protocol::DoT) { - config.remote.setPort(853); - } - else if (discoveredConfig.d_protocol == dnsdist::Protocol::DoH) { - config.remote.setPort(443); - } - } + config.remote.setPort(discoveredConfig.d_port); ComboAddress::addressOnlyEqual comparator; config.d_dohPath = discoveredConfig.d_dohPath; diff --git a/regression-tests.dnsdist/test_BackendDiscovery.py b/regression-tests.dnsdist/test_BackendDiscovery.py index fc06160a8d..b8ee41f433 100644 --- a/regression-tests.dnsdist/test_BackendDiscovery.py +++ b/regression-tests.dnsdist/test_BackendDiscovery.py @@ -17,11 +17,20 @@ class TestBackendDiscovery(DNSDistTest): _svcUpgradeDoTUnreachableBackendPort = 10606 _svcBrokenDNSResponseBackendPort = 10607 _svcUpgradeDoHBackendWithoutPathPort = 10608 + _connectionRefusedBackendPort = 10609 + _eofBackendPort = 10610 + _servfailBackendPort = 10611 + _wrongNameBackendPort = 10612 + _wrongIDBackendPort = 10613 + _tooManyQuestionsBackendPort = 10614 + _badQNameBackendPort = 10615 + _svcUpgradeDoTNoPortBackendPort = 10616 + _svcUpgradeDoHNoPortBackendPort = 10617 _upgradedBackendsPool = 'upgraded' _consoleKey = DNSDistTest.generateConsoleKey() _consoleKeyB64 = base64.b64encode(_consoleKey).decode('ascii') - _config_params = ['_consoleKeyB64', '_consolePort', '_noSVCBackendPort', '_svcNoUpgradeBackendPort', '_svcUpgradeDoTBackendPort', '_upgradedBackendsPool', '_svcUpgradeDoHBackendPort', '_svcUpgradeDoTBackendDifferentAddrPort1', '_svcUpgradeDoTBackendDifferentAddrPort2', '_svcUpgradeDoTUnreachableBackendPort', '_svcBrokenDNSResponseBackendPort', '_svcUpgradeDoHBackendWithoutPathPort'] + _config_params = ['_consoleKeyB64', '_consolePort', '_noSVCBackendPort', '_svcNoUpgradeBackendPort', '_svcUpgradeDoTBackendPort', '_upgradedBackendsPool', '_svcUpgradeDoHBackendPort', '_svcUpgradeDoTBackendDifferentAddrPort1', '_svcUpgradeDoTBackendDifferentAddrPort2', '_svcUpgradeDoTUnreachableBackendPort', '_svcBrokenDNSResponseBackendPort', '_svcUpgradeDoHBackendWithoutPathPort', '_connectionRefusedBackendPort', '_eofBackendPort', '_servfailBackendPort', '_wrongNameBackendPort', '_wrongIDBackendPort', '_tooManyQuestionsBackendPort', '_badQNameBackendPort', '_svcUpgradeDoTNoPortBackendPort', '_svcUpgradeDoHNoPortBackendPort'] _config_template = """ setKey("%s") controlSocket("127.0.0.1:%d") @@ -35,10 +44,10 @@ class TestBackendDiscovery(DNSDistTest): newServer{address="127.0.0.1:%s", caStore='ca.pem', autoUpgrade=true, autoUpgradeKeep=false}:setUp() -- SVCB upgrade to DoT, same address, keep the backend, different pool - newServer{address="127.0.0.1:%s", caStore='ca.pem', autoUpgrade=true, autoUpgradePool='%s', autoUpgradeKeep=true}:setUp() + newServer{address="127.0.0.1:%s", caStore='ca.pem', pool={'', 'another-pool'}, autoUpgrade=true, autoUpgradePool='%s', autoUpgradeKeep=true, source='127.0.0.1@lo'}:setUp() -- SVCB upgrade to DoH, same address, do not keep the backend, same pool - newServer{address="127.0.0.1:%s", caStore='ca.pem', autoUpgrade=true, autoUpgradeKeep=false}:setUp() + newServer{address="127.0.0.1:%s", caStore='ca.pem', pool={'another-pool'}, autoUpgrade=true, autoUpgradeKeep=false}:setUp() -- SVCB upgrade to DoT, different address, certificate is valid for the initial address newServer{address="127.0.0.1:%s", caStore='ca.pem', autoUpgrade=true, autoUpgradeKeep=false}:setUp() @@ -54,6 +63,33 @@ class TestBackendDiscovery(DNSDistTest): -- SVCB upgrade to DoH except the path is not specified newServer{address="127.0.0.1:%s", caStore='ca.pem', autoUpgrade=true, autoUpgradeKeep=false}:setUp() + + -- Connection refused + newServer({address="127.0.0.1:%s", caStore='ca.pem', pool={"", "other-pool"}, autoUpgrade=true, source='127.0.0.1@lo'}):setUp() + + -- EOF + newServer({address="127.0.0.1:%s", caStore='ca.pem', autoUpgrade=true}):setUp() + + -- ServFail + newServer({address="127.0.0.1:%s", autoUpgrade=true}):setUp() + + -- Wrong name + newServer({address="127.0.0.1:%s", autoUpgrade=true}):setUp() + + -- Wrong ID + newServer({address="127.0.0.1:%s", autoUpgrade=true}):setUp() + + -- Too many questions + newServer({address="127.0.0.1:%s", autoUpgrade=true}):setUp() + + -- Bad QName + newServer({address="127.0.0.1:%s", autoUpgrade=true}):setUp() + + -- SVCB upgrade to DoT, same address, no port specified via SVCB + newServer{address="127.0.0.1:%s", caStore='ca.pem', autoUpgrade=true, autoUpgradeKeep=false}:setUp() + + -- SVCB upgrade to DoH, same address, no port specified via SVCB + newServer{address="127.0.0.1:%s", caStore='ca.pem', autoUpgrade=true, autoUpgradeKeep=false}:setUp() """ _verboseMode = True @@ -98,6 +134,7 @@ class TestBackendDiscovery(DNSDistTest): dns.rdataclass.IN, dns.rdatatype.A, '127.0.0.1') + response.additional.append(rrset) rrset = dns.rrset.from_text('tls.tests.dnsdist.org.', 60, dns.rdataclass.IN, @@ -162,6 +199,62 @@ class TestBackendDiscovery(DNSDistTest): response.answer.append(rrset) return response.to_wire() + def EOFCallback(request): + return None + + def ServFailCallback(request): + response = dns.message.make_response(request) + response.set_rcode(dns.rcode.SERVFAIL) + return response.to_wire() + + def WrongNameCallback(request): + query = dns.message.make_query('not-the-right-one.', dns.rdatatype.SVCB) + response = dns.message.make_response(query) + response.id = request.id + return response.to_wire() + + def WrongIDCallback(request): + response = dns.message.make_response(request) + response.id = request.id ^ 42 + return response.to_wire() + + def WrongIDCallback(request): + response = dns.message.make_response(request) + response.id = request.id ^ 42 + return response.to_wire() + + def TooManyQuestionsCallback(request): + response = dns.message.make_response(request) + response.question.append(response.question[0]) + return response.to_wire() + + def BadQNameCallback(request): + response = dns.message.make_response(request) + wire = bytearray(response.to_wire()) + # mess up the first label length + wire[12] = 0xFF + return wire + + def UpgradeDoTNoPortCallback(request): + response = dns.message.make_response(request) + rrset = dns.rrset.from_text(request.question[0].name, + 60, + dns.rdataclass.IN, + dns.rdatatype.SVCB, + '1 tls.tests.dnsdist.org. alpn="dot" ipv4hint=127.0.0.1') + response.answer.append(rrset) + return response.to_wire() + + def UpgradeDoHNoPortCallback(request): + response = dns.message.make_response(request) + rrset = dns.rrset.from_text(request.question[0].name, + 60, + dns.rdataclass.IN, + dns.rdatatype.SVCB, + '1 tls.tests.dnsdist.org. alpn="h2" ipv4hint=127.0.0.1 key7="/dns-query{?dns}"') + response.answer.append(rrset) + return response.to_wire() + @classmethod def startResponders(cls): tlsContext = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) @@ -221,6 +314,39 @@ class TestBackendDiscovery(DNSDistTest): DOHMissingPathResponder.setDaemon(True) DOHMissingPathResponder.start() + EOFResponder = threading.Thread(name='EOF Responder', target=cls.TCPResponder, args=[cls._eofBackendPort, cls._toResponderQueue, cls._fromResponderQueue, False, False, cls.EOFCallback]) + EOFResponder.setDaemon(True) + EOFResponder.start() + + ServFailResponder = threading.Thread(name='ServFail Responder', target=cls.TCPResponder, args=[cls._servfailBackendPort, cls._toResponderQueue, cls._fromResponderQueue, False, False, cls.ServFailCallback]) + ServFailResponder.setDaemon(True) + ServFailResponder.start() + + WrongNameResponder = threading.Thread(name='Wrong Name Responder', target=cls.TCPResponder, args=[cls._wrongNameBackendPort, cls._toResponderQueue, cls._fromResponderQueue, False, False, cls.WrongNameCallback]) + WrongNameResponder.setDaemon(True) + WrongNameResponder.start() + + WrongIDResponder = threading.Thread(name='Wrong ID Responder', target=cls.TCPResponder, args=[cls._wrongIDBackendPort, cls._toResponderQueue, cls._fromResponderQueue, False, False, cls.WrongIDCallback]) + WrongIDResponder.setDaemon(True) + WrongIDResponder.start() + + TooManyQuestionsResponder = threading.Thread(name='Too many questions Responder', target=cls.TCPResponder, args=[cls._tooManyQuestionsBackendPort, cls._toResponderQueue, cls._fromResponderQueue, False, False, cls.TooManyQuestionsCallback]) + TooManyQuestionsResponder.setDaemon(True) + TooManyQuestionsResponder.start() + + badQNameResponder = threading.Thread(name='Bad QName Responder', target=cls.TCPResponder, args=[cls._badQNameBackendPort, cls._toResponderQueue, cls._fromResponderQueue, False, False, cls.BadQNameCallback]) + badQNameResponder.setDaemon(True) + badQNameResponder.start() + + TCPUpgradeToDoTNoPortResponder = threading.Thread(name='TCP upgrade to DoT (no port) Responder', target=cls.TCPResponder, args=[cls._svcUpgradeDoTNoPortBackendPort, cls._toResponderQueue, cls._fromResponderQueue, False, False, cls.UpgradeDoTNoPortCallback]) + TCPUpgradeToDoTNoPortResponder.setDaemon(True) + TCPUpgradeToDoTNoPortResponder.start() + + TCPUpgradeToDoHNoPortResponder = threading.Thread(name='TCP upgrade to DoH (no port) Responder', target=cls.TCPResponder, args=[cls._svcUpgradeDoHNoPortBackendPort, cls._toResponderQueue, cls._fromResponderQueue, False, False, cls.UpgradeDoHNoPortCallback]) + TCPUpgradeToDoHNoPortResponder.setDaemon(True) + TCPUpgradeToDoHNoPortResponder.start() + + def checkBackendsUpgraded(self): output = self.sendConsoleCommand('showServers()') print(output) @@ -240,15 +366,25 @@ class TestBackendDiscovery(DNSDistTest): expected = { '127.0.0.1:10600': '', '127.0.0.1:10601': '', - '127.0.0.1:10602': '', + '127.0.0.1:10602': 'another-pool', # 10603 has been upgraded to 10653 and removed # 10604 has been upgraded to 10654 and removed '127.0.0.2:10605': '', '127.0.0.1:10606': '', '127.0.0.1:10607': '', '127.0.0.1:10608': '', + '127.0.0.1:10609': 'other-pool', + '127.0.0.1:10610': '', + '127.0.0.1:10611': '', + '127.0.0.1:10612': '', + '127.0.0.1:10613': '', + '127.0.0.1:10614': '', + '127.0.0.1:10615': '', + # these two are not upgraded because there is no backend listening on the default ports (443 and 853) + '127.0.0.1:10616': '', + '127.0.0.1:10617': '', '127.0.0.1:10652': 'upgraded', - '127.0.0.1:10653': '', + '127.0.0.1:10653': 'another-pool', '127.0.0.2:10654': '' } print(backends) @@ -258,7 +394,6 @@ class TestBackendDiscovery(DNSDistTest): """ Backend Discovery: Upgrade """ - # enough time for discovery to happen # 5s is not enough with TSAN time.sleep(10)