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 74a4f4c6f384550c201e4c33550923f471317faf..cbd2f346b121fd731c7c4d53a72d48f3fd4d6eab 100644 (file)
--- 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 b4f4d04bed66bcc62bdd54d3de6543a723fa87ce..cd56c4f8d7089ede53ee5b63fbba2f61a58060a7 100644 (file)
--- 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 857557ab72be7c16b6353dc6080677d32cbbd8e0..6ec7467912b729880d23b9a8e595955ccf80ce83 100644 (file)
--- 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):