dnsdist: Fix destination address reporting

Over TCP the destination address was "0.0.0.0" when bound to an "any"
address. Over UDP, the destination address what always unset when
processing the response, except when bound to an "any" address.

diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc
index 759b01fabed7a4d8963bc25b21064258294b24fa..2e01da518d513612f0833797764dcdeee22e5ba5 100644 (file)
--- a/pdns/dnsdist-tcp.cc
+++ b/pdns/dnsdist-tcp.cc
@@ -190,9 +190,17 @@ void* tcpClientThread(int pipefd)
     string largerQuery;
     vector<uint8_t> rewrittenResponse;
     shared_ptr<DownstreamState> ds;
+    ComboAddress dest;
+    memset(&dest, 0, sizeof(dest));
+    dest.sin4.sin_family = ci.remote.sin4.sin_family;
+    socklen_t len = dest.getSocklen();
     if (!setNonBlocking(ci.fd))
       goto drop;
 
+    if (getsockname(ci.fd, (sockaddr*)&dest, &len)) {
+      dest = ci.cs->local;
+    }
+
     try {
       for(;;) {
         ds = nullptr;
@@ -258,7 +266,7 @@ void* tcpClientThread(int pipefd)
        uint16_t qtype, qclass;
        unsigned int consumed = 0;
        DNSName qname(query, qlen, sizeof(dnsheader), false, &qtype, &qclass, &consumed);
-       DNSQuestion dq(&qname, qtype, qclass, &ci.cs->local, &ci.remote, (dnsheader*)query, querySize, qlen, true);
+       DNSQuestion dq(&qname, qtype, qclass, &dest, &ci.remote, (dnsheader*)query, querySize, qlen, true);
 #ifdef HAVE_PROTOBUF
         dq.uniqueId = uuidGenerator();
 #endif
@@ -436,7 +444,7 @@ void* tcpClientThread(int pipefd)
         }
 
         dh = (struct dnsheader*) response;
-        DNSResponse dr(&qname, qtype, qclass, &ci.cs->local, &ci.remote, dh, responseSize, responseLen, true, &queryRealTime);
+        DNSResponse dr(&qname, qtype, qclass, &dest, &ci.remote, dh, responseSize, responseLen, true, &queryRealTime);
 #ifdef HAVE_PROTOBUF
         dr.uniqueId = dq.uniqueId;
 #endif

diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
index faad4d79f3def503d0813c01e7121c4d2ac6b593..dbfb6aa88ed9af1d138f92150aee244d38112ae1 100644 (file)
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -428,7 +428,11 @@ void* responderThread(std::shared_ptr<DownstreamState> state)
       continue;
     }
 #endif
-    sendUDPResponse(origFD, response, responseLen, ids->delayMsec, ids->origDest, ids->origRemote);
+    ComboAddress empty;
+    empty.sin4.sin_family = 0;
+    /* if ids->destHarvested is false, origDest holds the listening address.
+       We don't want to use that as a source since it could be 0.0.0.0 for example. */
+    sendUDPResponse(origFD, response, responseLen, ids->delayMsec, ids->destHarvested ? ids->origDest : empty, ids->origRemote);
 
     g_stats.responses++;
 
@@ -972,6 +976,11 @@ try
       }
 
       uint16_t len = (uint16_t) ret;
+      ComboAddress dest;
+      if (!HarvestDestinationAddress(&msgh, &dest)) {
+        dest.sin4.sin_family = 0;
+      }
+
 #ifdef HAVE_DNSCRYPT
       if (cs->dnscryptCtx) {
         vector<uint8_t> response;
@@ -982,10 +991,6 @@ try
 
         if (!decrypted) {
           if (response.size() > 0) {
-            ComboAddress dest;
-            if(!HarvestDestinationAddress(&msgh, &dest)) {
-              dest.sin4.sin_family = 0;
-            }
             sendUDPResponse(cs->udpFD, reinterpret_cast<char*>(response.data()), (uint16_t) response.size(), 0, dest, remote);
           }
           continue;
@@ -1015,7 +1020,7 @@ try
       const uint16_t origFlags = *flags;
       unsigned int consumed = 0;
       DNSName qname(query, len, sizeof(dnsheader), false, &qtype, &qclass, &consumed);
-      DNSQuestion dq(&qname, qtype, qclass, &cs->local, &remote, dh, sizeof(packet), len, false);
+      DNSQuestion dq(&qname, qtype, qclass, dest.sin4.sin_family != 0 ? &dest : &cs->local, &remote, dh, sizeof(packet), len, false);
 #ifdef HAVE_PROTOBUF
       dq.uniqueId = uuidGenerator();
 #endif
@@ -1037,10 +1042,6 @@ try
 
         restoreFlags(dh, origFlags);
 
-        ComboAddress dest;
-        if(!HarvestDestinationAddress(&msgh, &dest)) {
-          dest.sin4.sin_family = 0;
-        }
 #ifdef HAVE_DNSCRYPT
         if (!encryptResponse(response, &responseLen, dq.size, false, dnsCryptQuery)) {
           continue;
@@ -1072,10 +1073,6 @@ try
         uint16_t cachedResponseSize = sizeof cachedResponse;
         uint32_t allowExpired = ss ? 0 : g_staleCacheEntriesTTL;
         if (packetCache->get(dq, consumed, dh->id, cachedResponse, &cachedResponseSize, &cacheKey, allowExpired)) {
-          ComboAddress dest;
-          if(!HarvestDestinationAddress(&msgh, &dest)) {
-            dest.sin4.sin_family = 0;
-          }
 #ifdef HAVE_DNSCRYPT
           if (!encryptResponse(cachedResponse, &cachedResponseSize, sizeof cachedResponse, false, dnsCryptQuery)) {
             continue;
@@ -1115,7 +1112,6 @@ try
       ids->qname = qname;
       ids->qtype = dq.qtype;
       ids->qclass = dq.qclass;
-      ids->origDest.sin4.sin_family=0;
       ids->delayMsec = delayMsec;
       ids->origFlags = origFlags;
       ids->cacheKey = cacheKey;
@@ -1123,13 +1119,27 @@ try
       ids->packetCache = packetCache;
       ids->ednsAdded = ednsAdded;
       ids->ecsAdded = ecsAdded;
+
+      /* If we couldn't harvest the real dest addr, still
+         write down the listening addr since it will be useful
+         (especially if it's not an 'any' one).
+         We need to keep track of which one it is since we may
+         want to use the real but not the listening addr to reply.
+      */
+      if (dest.sin4.sin_family != 0) {
+        ids->origDest = dest;
+        ids->destHarvested = true;
+      }
+      else {
+        ids->origDest = cs->local;
+        ids->destHarvested = false;
+      }
 #ifdef HAVE_DNSCRYPT
       ids->dnsCryptQuery = dnsCryptQuery;
 #endif
 #ifdef HAVE_PROTOBUF
       ids->uniqueId = dq.uniqueId;
 #endif
-      HarvestDestinationAddress(&msgh, &ids->origDest);
 
       dh->id = idOffset;
 

diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh
index 7c4ac6f68782755961619e548e647fafdf7cce95..bf369257db1a3e64dd7ad27aa78e8a596a815bf1 100644 (file)
--- a/pdns/dnsdist.hh
+++ b/pdns/dnsdist.hh
@@ -269,6 +269,7 @@ struct IDState
   bool ednsAdded{false};
   bool ecsAdded{false};
   bool skipCache{false};
+  bool destHarvested{false}; // if true, origDest holds the original dest addr, otherwise the listening addr
 };
 
 struct Rings {