From: Otto Moerbeek Date: Fri, 21 Mar 2025 11:05:20 +0000 (+0100) Subject: Invalid cookie case, plus test of moving to next NS in that case X-Git-Tag: rec-5.4.0-alpha1~279^2~24 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5b67901748209e3454eaa5ceb3b2d268970bcded;p=thirdparty%2Fpdns.git Invalid cookie case, plus test of moving to next NS in that case --- diff --git a/pdns/recursordist/lwres.cc b/pdns/recursordist/lwres.cc index 74a4f4c6f3..cbd2f346b1 100644 --- a/pdns/recursordist/lwres.cc +++ b/pdns/recursordist/lwres.cc @@ -497,8 +497,8 @@ static LWResult::Result asyncresolve(const ComboAddress& address, const DNSName& case CookieEntry::Support::Unknown: assert(0); case CookieEntry::Support::Unsupported: - default: - cerr << "This server does not support cookies or we don't know yet:" << endl; + cerr << "This server does not support cookies" << endl; + break; } } else { @@ -745,27 +745,33 @@ static LWResult::Result asyncresolve(const ComboAddress& address, const DNSName& uint16_t ercode = (edo.d_extRCode << 4) | lwr->d_rcode; if (ercode == ERCode::BADCOOKIE) { lwr->d_validpacket = true; - return LWResult::Result::BadCookie; + return LWResult::Result::BadCookie; // proper use of BacCookie, we did update he entry } } else { - // Server responded with a wrong client cookie, fall back to TCP - cerr << "Wrong cookie" << endl; - lwr->d_validpacket = true; - return LWResult::Result::Spoofed; + if (!doTCP) { + // Server responded with a wrong client cookie, fall back to TCP + cerr << "Wrong cookie" << endl; + lwr->d_validpacket = true; + return LWResult::Result::Spoofed; + } + // ignore bad cookie when already doing TCP } } else { - // We sent a cookie out but it's not in the table? + // We receivd cookie (we might have sent one out) but it's not in the table? cerr << "Cookie not found back"<< endl; - lwr->d_validpacket = true; - return LWResult::Result::BadCookie; // XXX + lwr->d_rcode = RCode::FormErr; + lwr->d_validpacket = false; + return LWResult::Result::Success; // success - oddly enough } } else { - cerr << "Malformed cookie in reply"<< endl; - lwr->d_validpacket = true; - return LWResult::Result::BadCookie; // XXX + cerr << "Malformed cookie in reply" << endl; + // Do something special if we get malformed repeatedly? And or consider current status: Supported + lwr->d_rcode = RCode::FormErr; + lwr->d_validpacket = false; + return LWResult::Result::Success; // succes - odly enough } } } @@ -775,8 +781,27 @@ static LWResult::Result asyncresolve(const ComboAddress& address, const DNSName& // Case: we sent out a cookie but did not get one back if (cookieSentOut && !cookieFoundInReply && !*chained) { cerr << "No cookie in reply"<< endl; - lwr->d_validpacket = true; - return LWResult::Result::BadCookie; // XXX + auto lock = s_cookiestore.lock(); + auto found = lock->find(address); + if (found != lock->end()) { + switch (found->getSupport()) { + case CookieEntry::Support::Unknown: + assert(0); + case CookieEntry::Support::Probing: + cerr << "Was probing, setting support to Unsupported" << endl; + found->setSupport(CookieEntry::Support::Unsupported); + break; + case CookieEntry::Support::Unsupported: + break; + case CookieEntry::Support::Supported: + lwr->d_validpacket = true; + return LWResult::Result::BadCookie; // XXX, we did not update cookie info... + break; + } + } + else { + // Table entry lost? XXX + } } if (outgoingLoggers) { diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index b4f4d04bed..cd56c4f8d7 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -5997,7 +5997,7 @@ int SyncRes::doResolveAt(NsSet& nameservers, DNSName auth, bool flawedNSSet, con gotAnswer = doResolveAtThisIP(prefix, qname, qtype, lwr, ednsmask, auth, sendRDQuery, wasForwarded, tns->first, *remoteIP, false, false, truncated, spoofed, context.extendedError); } - cerr << "Got spoofed?!" << spoofed << endl; + if (spoofed) cerr << "Got spoofed! " << spoofed << endl; if (forceTCP || (spoofed || (gotAnswer && truncated))) { /* retry, over TCP this time */ gotAnswer = doResolveAtThisIP(prefix, qname, qtype, lwr, ednsmask, auth, sendRDQuery, wasForwarded, diff --git a/regression-tests.recursor-dnssec/test_Cookies.py b/regression-tests.recursor-dnssec/test_Cookies.py index 857557ab72..6ec7467912 100644 --- a/regression-tests.recursor-dnssec/test_Cookies.py +++ b/regression-tests.recursor-dnssec/test_Cookies.py @@ -12,14 +12,13 @@ from recursortests import RecursorTest class CookiesTest(RecursorTest): _confdir = 'Cookies' - _config_template = """ recursor: forward_zones: - zone: cookies.example - forwarders: [%s.25] + forwarders: [%s.25, %s.26] outgoing: - cookies: true""" % (os.environ['PREFIX']) + cookies: true""" % (os.environ['PREFIX'], os.environ['PREFIX']) _expectedCookies = 'no' @classmethod @@ -48,28 +47,28 @@ outgoing: def startResponders(cls): print("Launching responders..") - address = cls._PREFIX + '.25' + address1 = cls._PREFIX + '.25' + address2 = cls._PREFIX + '.26' port = 53 - reactor.listenUDP(port, UDPResponder(), interface=address) - reactor.listenTCP(port, TCPFactory(), interface=address) + reactor.listenUDP(port, UDPResponder(), interface=address1) + reactor.listenTCP(port, TCPFactory(), interface=address1) + reactor.listenUDP(port, UDPResponder(), interface=address2) + reactor.listenTCP(port, TCPFactory(), interface=address2) if not reactor.running: cls.Responder = threading.Thread(name='Responder', target=reactor.run, args=(False,)) cls.Responder.daemon = True cls.Responder.start() - #cls._TCPResponder = threading.Thread(name='TCP Responder', target=reactor.run, args=(False,)) - #cls._TCPResponder.daemon = True - #cls._TCPResponder.start() - def checkCookies(self, support): + def checkCookies(self, support, server='127.0.0.25'): confdir = os.path.join('configs', self._confdir) output = self.recControl(confdir, 'dump-cookies', '-') for line in output.splitlines(): tokens = line.split() - if tokens[0] != '127.0.0.25': + if tokens[0] != server: continue - print(tokens) + #print(tokens) self.assertEqual(len(tokens), 5) self.assertEqual(tokens[3], support) @@ -81,7 +80,7 @@ outgoing: res = self.sendUDPQuery(query) self.assertRcodeEqual(res, dns.rcode.NOERROR) self.assertRRsetInAnswer(res, expected) - self.checkCookies('no') + self.checkCookies('Unsupported') def testAuthRepliesWithCookies(self): confdir = os.path.join('configs', self._confdir) @@ -92,7 +91,7 @@ outgoing: res = self.sendUDPQuery(query) self.assertRcodeEqual(res, dns.rcode.NOERROR) self.assertRRsetInAnswer(res, expected) - self.checkCookies('yes') + self.checkCookies('Supported') # Case: we get a an correct client and server cookie back # We do not clear the cookie tables, so the old server cookie gets re-used @@ -101,19 +100,18 @@ outgoing: res = self.sendUDPQuery(query) self.assertRcodeEqual(res, dns.rcode.NOERROR) self.assertRRsetInAnswer(res, expected) - self.checkCookies('yes') + self.checkCookies('Supported') def testAuthSendsIncorrectClientCookie(self): confdir = os.path.join('configs', self._confdir) - # Case: rec gets a an incorrect client cookie back - # Fails at the moment, as we do not do the right thing yet server side XXXX + # Case: rec gets a an incorrect client cookie back, we ignore that over TCP self.recControl(confdir, 'clear-cookies') query = dns.message.make_query('d.cookies.example.', 'A') expected = dns.rrset.from_text('d.cookies.example.', 15, dns.rdataclass.IN, 'A', '127.0.0.1') res = self.sendUDPQuery(query) self.assertRcodeEqual(res, dns.rcode.NOERROR) self.assertRRsetInAnswer(res, expected) - self.checkCookies('yes') + self.checkCookies('Probing') def testAuthSendsBADCOOKIEOverUDP(self): confdir = os.path.join('configs', self._confdir) @@ -124,7 +122,19 @@ outgoing: res = self.sendUDPQuery(query) self.assertRcodeEqual(res, dns.rcode.NOERROR) self.assertRRsetInAnswer(res, expected) - self.checkCookies('yes') + self.checkCookies('Supported') + + def testAuthSendsMalformedCookie(self): + confdir = os.path.join('configs', self._confdir) + # Case: rec gets a malformed cookie, should ignore packet + self.recControl(confdir, 'clear-cookies') + query = dns.message.make_query('f.cookies.example.', 'A') + expected = dns.rrset.from_text('f.cookies.example.', 15, dns.rdataclass.IN, 'A', '127.0.0.1') + res = self.sendUDPQuery(query) + self.assertRcodeEqual(res, dns.rcode.NOERROR) + self.assertRRsetInAnswer(res, expected) + self.checkCookies('Probing', '127.0.0.25') + self.checkCookies('Supported', '127.0.0.26') class UDPResponder(DatagramProtocol): @@ -193,6 +203,19 @@ class UDPResponder(DatagramProtocol): response.set_rcode(23) # BADCOOKIE response.answer.append(answer) + # Case send malformed cookie for server .25 + elif question.name == dns.name.from_text('f.cookies.example.') and question.rdtype == dns.rdatatype.A: + answer = dns.rrset.from_text('f.cookies.example.', 15, dns.rdataclass.IN, 'A', '127.0.0.1') + clientcookie = self.getCookie(request) + print(self.transport.getHost().host) + if self.transport.getHost().host == os.environ['PREFIX'] + '.26': + if clientcookie is not None: + response.use_edns(options = [self.createCookie(clientcookie)]) + else: + full = dns.edns.GenericOption(dns.edns.COOKIE, '') + response.use_edns(options = [full]) + response.answer.append(answer) + return response.to_wire() def datagramReceived(self, datagram, address):