From: Remi Gacogne Date: Wed, 2 Nov 2016 10:44:14 +0000 (+0100) Subject: dnsdist: Fix destination address reporting X-Git-Tag: dnsdist-1.1.0-beta2~35^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7cea4e39a78ef981ee461b49bbc193fa9903f56d;p=thirdparty%2Fpdns.git 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 759b01fabe..2e01da518d 100644 --- a/pdns/dnsdist-tcp.cc +++ b/pdns/dnsdist-tcp.cc @@ -190,9 +190,17 @@ void* tcpClientThread(int pipefd) string largerQuery; vector rewrittenResponse; shared_ptr 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 faad4d79f3..dbfb6aa88e 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -428,7 +428,11 @@ void* responderThread(std::shared_ptr 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 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(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 7c4ac6f687..bf369257db 100644 --- 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 {