auth: Thoroughly check the source of UDP answers in proxy, resolver

diff --git a/pdns/dnsproxy.cc b/pdns/dnsproxy.cc
index bee70a86a09f71f77b98bb00e2f1ecbd9c20ff4f..114c47253739dc1b947fc4d2b1832bd7310f0796 100644 (file)
--- a/pdns/dnsproxy.cc
+++ b/pdns/dnsproxy.cc
@@ -44,18 +44,21 @@ DNSProxy::DNSProxy(const string &remote)
 
   vector<string> addresses;
   stringtok(addresses, remote, " ,\t");
-  ComboAddress remaddr(addresses[0], 53);
-  
-  if((d_sock=socket(remaddr.sin4.sin_family, SOCK_DGRAM,0))<0)
+  d_remote = ComboAddress(addresses[0], 53);
+
+  if((d_sock=socket(d_remote.sin4.sin_family, SOCK_DGRAM,0))<0) {
     throw PDNSException(string("socket: ")+strerror(errno));
- 
+  }
+
   ComboAddress local;
-  if(remaddr.sin4.sin_family==AF_INET)
+  if(d_remote.sin4.sin_family==AF_INET) {
     local = ComboAddress("0.0.0.0");
-  else
+  }
+  else {
     local = ComboAddress("::");
+  }
     
-  int n=0;
+  unsigned int n=0;
   for(;n<10;n++) {
     local.sin4.sin_port = htons(10000+dns_random(50000));
     
@@ -68,11 +71,12 @@ DNSProxy::DNSProxy(const string &remote)
     throw PDNSException(string("binding dnsproxy socket: ")+strerror(errno));
   }
 
-  if(connect(d_sock, (sockaddr *)&remaddr, remaddr.getSocklen())<0) 
-    throw PDNSException("Unable to UDP connect to remote nameserver "+remaddr.toStringWithPort()+": "+stringerror());
+  if(connect(d_sock, (sockaddr *)&d_remote, d_remote.getSocklen())<0) {
+    throw PDNSException("Unable to UDP connect to remote nameserver "+d_remote.toStringWithPort()+": "+stringerror());
+  }
 
   d_xor=dns_random(0xffff);
-  L<<Logger::Error<<"DNS Proxy launched, local port "<<ntohs(local.sin4.sin_port)<<", remote "<<remaddr.toStringWithPort()<<endl;
+  L<<Logger::Error<<"DNS Proxy launched, local port "<<ntohs(local.sin4.sin_port)<<", remote "<<d_remote.toStringWithPort()<<endl;
 } 
 
 void DNSProxy::go()
@@ -179,9 +183,11 @@ void DNSProxy::mainloop(void)
     struct msghdr msgh;
     struct iovec iov;
     char cbuf[256];
+    ComboAddress fromaddr;
 
     for(;;) {
-      len=recv(d_sock, buffer, sizeof(buffer),0); // answer from our backend
+      socklen_t fromaddrSize = sizeof(fromaddr);
+      len=recvfrom(d_sock, buffer, sizeof(buffer),0, (struct sockaddr*) &fromaddr, &fromaddrSize); // answer from our backend
       if(len<(ssize_t)sizeof(dnsheader)) {
         if(len<0)
           L<<Logger::Error<<"Error receiving packet from recursor backend: "<<stringerror()<<endl;
@@ -192,6 +198,10 @@ void DNSProxy::mainloop(void)
         
         continue;
       }
+      if (fromaddr != d_remote) {
+        L<<Logger::Error<<"Got answer from unexpected host "<<fromaddr.toStringWithPort()<<" instead of our recursor backend "<<d_remote.toStringWithPort()<<endl;
+        continue;
+      }
       (*d_resanswers)++;
       (*d_udpanswers)++;
       dnsheader d;

diff --git a/pdns/dnsproxy.hh b/pdns/dnsproxy.hh
index f927c86a7a86cb1c20a458ab7e02e9bafbffa1f8..527eabadafe63f60394b27a857768c91d6d41a23 100644 (file)
--- a/pdns/dnsproxy.hh
+++ b/pdns/dnsproxy.hh
@@ -81,6 +81,7 @@ private:
   typedef map<int,ConntrackEntry> map_t;
 
   // Data
+  ComboAddress d_remote;
   AtomicCounter* d_resanswers;
   AtomicCounter* d_udpanswers;
   AtomicCounter* d_resquestions;

diff --git a/pdns/resolver.cc b/pdns/resolver.cc
index ec0343aac8e6f7a81fe4071ed76a5968037b87dd..81eabdfea634537da3459596293707812f230221 100644 (file)
--- a/pdns/resolver.cc
+++ b/pdns/resolver.cc
@@ -208,7 +208,7 @@ static int parseResult(MOADNSParser& mdp, const DNSName& origQname, uint16_t ori
   return 0;
 }
 
-bool Resolver::tryGetSOASerial(DNSName *domain, uint32_t *theirSerial, uint32_t *theirInception, uint32_t *theirExpire, uint16_t* id)
+bool Resolver::tryGetSOASerial(DNSName *domain, ComboAddress* remote, uint32_t *theirSerial, uint32_t *theirInception, uint32_t *theirExpire, uint16_t* id)
 {
   auto fds = std::unique_ptr<struct pollfd[]>(new struct pollfd[locals.size()]);
   size_t i = 0, k;
@@ -236,10 +236,10 @@ bool Resolver::tryGetSOASerial(DNSName *domain, uint32_t *theirSerial, uint32_t
   if (sock < 0) return false; // false alarm
 
   int err;
-  ComboAddress fromaddr;
-  socklen_t addrlen=fromaddr.getSocklen();
+  remote->sin6.sin6_family = AF_INET6; // make sure getSocklen() below returns a large enough value
+  socklen_t addrlen=remote->getSocklen();
   char buf[3000];
-  err = recvfrom(sock, buf, sizeof(buf), 0,(struct sockaddr*)(&fromaddr), &addrlen);
+  err = recvfrom(sock, buf, sizeof(buf), 0,(struct sockaddr*)(remote), &addrlen);
   if(err < 0) {
     if(errno == EAGAIN)
       return false;
@@ -252,13 +252,13 @@ bool Resolver::tryGetSOASerial(DNSName *domain, uint32_t *theirSerial, uint32_t
   *domain = mdp.d_qname;
   
   if(domain->empty())
-    throw ResolverException("SOA query to '" + fromaddr.toStringWithPort() + "' produced response without domain name (RCode: " + RCode::to_s(mdp.d_header.rcode) + ")");
+    throw ResolverException("SOA query to '" + remote->toStringWithPort() + "' produced response without domain name (RCode: " + RCode::to_s(mdp.d_header.rcode) + ")");
 
   if(mdp.d_answers.empty())
-    throw ResolverException("Query to '" + fromaddr.toStringWithPort() + "' for SOA of '" + domain->toString() + "' produced no results (RCode: " + RCode::to_s(mdp.d_header.rcode) + ")");
+    throw ResolverException("Query to '" + remote->toStringWithPort() + "' for SOA of '" + domain->toLogString() + "' produced no results (RCode: " + RCode::to_s(mdp.d_header.rcode) + ")");
   
   if(mdp.d_qtype != QType::SOA)
-    throw ResolverException("Query to '" + fromaddr.toStringWithPort() + "' for SOA of '" + domain->toString() + "' returned wrong record type");
+    throw ResolverException("Query to '" + remote->toStringWithPort() + "' for SOA of '" + domain->toLogString() + "' returned wrong record type");
 
   *theirInception = *theirExpire = 0;
   bool gotSOA=false;
@@ -279,7 +279,7 @@ bool Resolver::tryGetSOASerial(DNSName *domain, uint32_t *theirSerial, uint32_t
     }
   }
   if(!gotSOA)
-    throw ResolverException("Query to '" + fromaddr.toString() + "' for SOA of '" + domain->toLogString() + "' did not return a SOA");
+    throw ResolverException("Query to '" + remote->toString() + "' for SOA of '" + domain->toLogString() + "' did not return a SOA");
   return true;
 }
 
@@ -302,10 +302,14 @@ int Resolver::resolve(const string &ipport, const DNSName &domain, int type, Res
     socklen_t addrlen = sizeof(from);
     char buffer[3000];
     int len;
-    
+
     if((len=recvfrom(sock, buffer, sizeof(buffer), 0,(struct sockaddr*)(&from), &addrlen)) < 0) 
       throw ResolverException("recvfrom error waiting for answer: "+stringerror());
-  
+
+    if (from != to) {
+      throw ResolverException("Got answer from the wrong peer while resolving ("+from.toStringWithPort()+" instead of "+to.toStringWithPort()+", discarding");
+    }
+
     MOADNSParser mdp(false, buffer, len);
     return parseResult(mdp, domain, type, id, res);
   }

diff --git a/pdns/resolver.hh b/pdns/resolver.hh
index 57c28b65a553db14427924a07c1355ebbb6cddba..c9946ccf810ca7319aa3f9c17f820a8ce95d489b 100644 (file)
--- a/pdns/resolver.hh
+++ b/pdns/resolver.hh
@@ -69,7 +69,7 @@ public:
     const DNSName& tsigkeyname=DNSName(), const DNSName& tsigalgorithm=DNSName(), const string& tsigsecret="");
 
   //! see if we got a SOA response from our sendResolve
-  bool tryGetSOASerial(DNSName *theirDomain, uint32_t* theirSerial, uint32_t* theirInception, uint32_t* theirExpire, uint16_t* id);
+  bool tryGetSOASerial(DNSName *theirDomain, ComboAddress* remote, uint32_t* theirSerial, uint32_t* theirInception, uint32_t* theirExpire, uint16_t* id);
   
   //! convenience function that calls resolve above
   void getSoaSerial(const string &, const DNSName &, uint32_t *);

diff --git a/pdns/slavecommunicator.cc b/pdns/slavecommunicator.cc
index 30a32e2960211725fce99107f3f33b5d8c1a3079..748522b41a734c2d7ecaa8d215fc2f980c043d19 100644 (file)
--- a/pdns/slavecommunicator.cc
+++ b/pdns/slavecommunicator.cc
@@ -639,7 +639,7 @@ struct DomainNotificationInfo
 
 struct SlaveSenderReceiver
 {
-  typedef pair<DNSName, uint16_t> Identifier;
+  typedef std::tuple<DNSName, ComboAddress, uint16_t> Identifier;
 
   struct Answer {
     uint32_t theirSerial;
@@ -661,13 +661,16 @@ struct SlaveSenderReceiver
   {
     random_shuffle(dni.di.masters.begin(), dni.di.masters.end());
     try {
-      return make_pair(dni.di.zone,
-        d_resolver.sendResolve(ComboAddress(*dni.di.masters.begin(), 53), dni.localaddr,
-          dni.di.zone,
-          QType::SOA,
-          nullptr,
-          dni.dnssecOk, dni.tsigkeyname, dni.tsigalgname, dni.tsigsecret)
-      );
+      ComboAddress remote(*dni.di.masters.begin(), 53);
+      return std::make_tuple(dni.di.zone,
+                             remote,
+                             d_resolver.sendResolve(remote,
+                                                    dni.localaddr,
+                                                    dni.di.zone,
+                                                    QType::SOA,
+                                                    nullptr,
+                                                    dni.dnssecOk, dni.tsigkeyname, dni.tsigalgname, dni.tsigsecret)
+        );
     }
     catch(PDNSException& e) {
       throw runtime_error("While attempting to query freshness of '"+dni.di.zone.toLogString()+"': "+e.reason);
@@ -676,7 +679,7 @@ struct SlaveSenderReceiver
 
   bool receive(Identifier& id, Answer& a)
   {
-    if(d_resolver.tryGetSOASerial(&id.first, &a.theirSerial, &a.theirInception, &a.theirExpire, &id.second)) {
+    if(d_resolver.tryGetSOASerial(&(std::get<0>(id)), &(std::get<1>(id)), &a.theirSerial, &a.theirInception, &a.theirExpire, &(std::get<2>(id)))) {
       return 1;
     }
     return 0;