From: Remi Gacogne Date: Wed, 14 Apr 2021 16:03:57 +0000 (+0200) Subject: dnsdist: Apply response rules to cross-protocol DoH responses X-Git-Tag: dnsdist-1.7.0-alpha1~45^2~43 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4df166a9bbae7e8f9374cc251585a22ca49368c6;p=thirdparty%2Fpdns.git dnsdist: Apply response rules to cross-protocol DoH responses --- diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc index 022ae68783..31c7937ce8 100644 --- a/pdns/dnsdist-tcp.cc +++ b/pdns/dnsdist-tcp.cc @@ -357,25 +357,13 @@ static void handleResponseSent(std::shared_ptr& stat if (currentResponse.d_selfGenerated == false && currentResponse.d_connection && currentResponse.d_connection->getDS()) { const auto& ds = currentResponse.d_connection->getDS(); - struct timespec answertime; - gettime(&answertime); const auto& ids = currentResponse.d_idstate; double udiff = ids.sentTime.udiff(); - g_rings.insertResponse(answertime, state->d_ci.remote, ids.qname, ids.qtype, static_cast(udiff), static_cast(currentResponse.d_buffer.size()), currentResponse.d_cleartextDH, ds->remote); vinfolog("Got answer from %s, relayed to %s (%s, %d bytes), took %f usec", ds->remote.toStringWithPort(), ids.origRemote.toStringWithPort(), (state->d_handler.isTLS() ? "DoT" : "TCP"), currentResponse.d_buffer.size(), udiff); - } - switch (currentResponse.d_cleartextDH.rcode) { - case RCode::NXDomain: - ++g_stats.frontendNXDomain; - break; - case RCode::ServFail: - ++g_stats.servfailResponses; - ++g_stats.frontendServFail; - break; - case RCode::NoError: - ++g_stats.frontendNoError; - break; + ::handleResponseSent(ids, udiff, state->d_ci.remote, ds->remote, static_cast(currentResponse.d_buffer.size()), currentResponse.d_cleartextDH); + + ds->latencyUsecTCP = (127.0 * ds->latencyUsecTCP / 128.0) + udiff/128.0; } } @@ -572,6 +560,10 @@ void IncomingTCPConnectionState::handleResponse(const struct timeval& now, TCPRe return; } + if (response.d_connection->getDS()) { + ++response.d_connection->getDS()->responses; + } + DNSResponse dr = makeDNSResponseFromIDState(ids, response.d_buffer); memcpy(&response.d_cleartextDH, dr.getHeader(), sizeof(response.d_cleartextDH)); @@ -589,9 +581,6 @@ void IncomingTCPConnectionState::handleResponse(const struct timeval& now, TCPRe ++g_stats.responses; ++state->d_ci.cs->responses; - if (response.d_connection->getDS()) { - ++response.d_connection->getDS()->responses; - } queueResponse(state, now, std::move(response)); } diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index f71ecb878a..e64620ed87 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -542,6 +542,26 @@ static void pickBackendSocketsReadyForReceiving(const std::shared_ptrmplexer.lock())->getAvailableFDs(ready, 1000); } +void handleResponseSent(const IDState& ids, double udiff, const ComboAddress& client, const ComboAddress& backend, unsigned int size, const dnsheader& cleartextDH) +{ + struct timespec ts; + gettime(&ts); + g_rings.insertResponse(ts, client, ids.qname, ids.qtype, static_cast(udiff), size, cleartextDH, backend); + + switch (cleartextDH.rcode) { + case RCode::NXDomain: + ++g_stats.frontendNXDomain; + break; + case RCode::ServFail: + ++g_stats.servfailResponses; + ++g_stats.frontendServFail; + break; + case RCode::NoError: + ++g_stats.frontendNoError; + break; + } +} + // listens on a dedicated socket, lobs answers from downstream servers to original requestors void responderThread(std::shared_ptr dss) { @@ -611,7 +631,6 @@ void responderThread(std::shared_ptr dss) continue; } - bool isDoH = du != nullptr; /* atomically mark the state as available, but only if it has not been altered in the meantime */ if (ids->tryMarkUnused(usageIndicator)) { @@ -633,12 +652,12 @@ void responderThread(std::shared_ptr dss) } dh->id = ids->origID; + ++dss->responses; - /* don't call processResponse on a truncated answer for DoH, we will retry over TCP */ - if (du && dh->tc) { + /* don't call processResponse for DOH */ + if (du) { #ifdef HAVE_DNS_OVER_HTTPS // DoH query - cerr<<"truncated answer for DoH"<handleUDPResponse(std::move(response), std::move(*ids)); #endif continue; @@ -654,48 +673,22 @@ void responderThread(std::shared_ptr dss) continue; } - if (ids->cs && !ids->cs->muted) { - if (du) { -#ifdef HAVE_DNS_OVER_HTTPS - // DoH query - du->handleUDPResponse(std::move(response), IDState()); -#endif - du = nullptr; - } - - else { - ComboAddress empty; - empty.sin4.sin_family = 0; - sendUDPResponse(origFD, response, dr.delayMsec, ids->hopLocal, ids->hopRemote); - } - } - ++g_stats.responses; if (ids->cs) { ++ids->cs->responses; } - ++dss->responses; + + if (ids->cs && !ids->cs->muted) { + ComboAddress empty; + empty.sin4.sin_family = 0; + sendUDPResponse(origFD, response, dr.delayMsec, ids->hopLocal, ids->hopRemote); + } double udiff = ids->sentTime.udiff(); - vinfolog("Got answer from %s, relayed to %s%s, took %f usec", dss->remote.toStringWithPort(), ids->origRemote.toStringWithPort(), - isDoH ? " (https)": "", udiff); + vinfolog("Got answer from %s, relayed to %s, took %f usec", dss->remote.toStringWithPort(), ids->origRemote.toStringWithPort(), udiff); - struct timespec ts; - gettime(&ts); - g_rings.insertResponse(ts, *dr.remote, *dr.qname, dr.qtype, static_cast(udiff), static_cast(got), cleartextDH, dss->remote); + handleResponseSent(*ids, udiff, *dr.remote, dss->remote, static_cast(got), cleartextDH); - switch (cleartextDH.rcode) { - case RCode::NXDomain: - ++g_stats.frontendNXDomain; - break; - case RCode::ServFail: - ++g_stats.servfailResponses; - ++g_stats.frontendServFail; - break; - case RCode::NoError: - ++g_stats.frontendNoError; - break; - } dss->latencyUsec = (127.0 * dss->latencyUsec / 128.0) + udiff/128.0; doLatencyStats(udiff); diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh index 272c9498e9..f2089e5151 100644 --- a/pdns/dnsdist.hh +++ b/pdns/dnsdist.hh @@ -704,6 +704,7 @@ struct DownstreamState pdns::stat_t_trait queryLoad{0.0}; pdns::stat_t_trait dropRate{0.0}; double latencyUsec{0.0}; + double latencyUsecTCP{0.0}; int order{1}; int weight{1}; int tcpConnectTimeout{5}; @@ -998,5 +999,6 @@ void setIDStateFromDNSQuestion(IDState& ids, DNSQuestion& dq, DNSName&& qname); int pickBackendSocketForSending(std::shared_ptr& state); ssize_t udpClientSendRequestToBackend(const std::shared_ptr& ss, const int sd, const PacketBuffer& request, bool healthCheck = false); +void handleResponseSent(const IDState& ids, double udiff, const ComboAddress& client, const ComboAddress& backend, unsigned int size, const dnsheader& cleartextDH); void carbonDumpThread(); diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc index fe237dcece..ad94b9bb7f 100644 --- a/pdns/dnsdistdist/doh.cc +++ b/pdns/dnsdistdist/doh.cc @@ -482,7 +482,6 @@ static int processDOHQuery(DOHUnit* du) if (du->response.empty()) { du->response = std::move(du->query); } - sendDoHUnitToTheMainThread(du, "DoH self-answered response"); return 0; @@ -1162,12 +1161,30 @@ public: } du->response = std::move(response.d_buffer); + du->ids = std::move(response.d_idstate); + + thread_local LocalStateHolder> localRespRuleActions = g_respruleactions.getLocal(); + DNSResponse dr = makeDNSResponseFromIDState(du->ids, du->response); + dnsheader cleartextDH; + memcpy(&cleartextDH, dr.getHeader(), sizeof(cleartextDH)); - auto sent = write(du->rsock, &du, sizeof(du)); - if (sent != sizeof(du)) { + if (!processResponse(du->response, localRespRuleActions, dr, false, false)) { du->release(); - du = nullptr; - } + return; + } + + double udiff = du->ids.sentTime.udiff(); + vinfolog("Got answer from %s, relayed to %s (https), took %f usec", du->downstream->remote.toStringWithPort(), du->ids.origRemote.toStringWithPort(), udiff); + + handleResponseSent(du->ids, udiff, *dr.remote, du->downstream->remote, du->response.size(), cleartextDH); + + ++g_stats.responses; + if (du->ids.cs) { + ++du->ids.cs->responses; + } + + sendDoHUnitToTheMainThread(du, "cross-protocol response"); + du->release(); } void handleXFRResponse(const struct timeval& now, TCPResponse&& response) override @@ -1573,23 +1590,32 @@ void DOHUnit::handleUDPResponse(PacketBuffer&& udpResponse, IDState&& state) response = std::move(udpResponse); ids = std::move(state); - auto du = this; - ssize_t sent = write(rsock, &du, sizeof(du)); - if (sent != sizeof(this)) { - if (errno == EAGAIN || errno == EWOULDBLOCK) { - ++g_stats.dohResponsePipeFull; - vinfolog("Unable to pass a DoH response to the DoH worker thread because the pipe is full"); - } - else { - vinfolog("Unable to pass a DoH response to the DoH worker thread because we couldn't write to the pipe: %s", stringerror()); + const dnsheader* dh = reinterpret_cast(response.data()); + if (!dh->tc) { + thread_local LocalStateHolder> localRespRuleActions = g_respruleactions.getLocal(); + DNSResponse dr = makeDNSResponseFromIDState(ids, response); + dnsheader cleartextDH; + memcpy(&cleartextDH, dr.getHeader(), sizeof(cleartextDH)); + + if (!processResponse(response, localRespRuleActions, dr, false, true)) { + release(); + return; } - /* at this point we have the only remaining pointer on this - DOHUnit object since we did set ids->du to nullptr earlier, - except if we got the response before the pointer could be - released by the frontend */ - release(); + double udiff = ids.sentTime.udiff(); + vinfolog("Got answer from %s, relayed to %s (https), took %f usec", downstream->remote.toStringWithPort(), ids.origRemote.toStringWithPort(), udiff); + + handleResponseSent(ids, udiff, *dr.remote, downstream->remote, response.size(), cleartextDH); + + ++g_stats.responses; + if (ids.cs) { + ++ids.cs->responses; + } } + + sendDoHUnitToTheMainThread(this, "DoH response"); + /* the reference counter has been incremented in sendDoHUnitToTheMainThread */ + release(); } #else /* HAVE_DNS_OVER_HTTPS */ diff --git a/pdns/dnsdistdist/test-dnsdisttcp_cc.cc b/pdns/dnsdistdist/test-dnsdisttcp_cc.cc index c1a1398ee8..118b063638 100644 --- a/pdns/dnsdistdist/test-dnsdisttcp_cc.cc +++ b/pdns/dnsdistdist/test-dnsdisttcp_cc.cc @@ -61,6 +61,10 @@ uint64_t getLatencyCount(const std::string&) return 0; } +void handleResponseSent(const IDState& ids, double udiff, const ComboAddress& client, const ComboAddress& backend, unsigned int size, const dnsheader& cleartextDH) +{ +} + static std::function& selectedBackend)> s_processQuery; ProcessQueryResult processQuery(DNSQuestion& dq, ClientState& cs, LocalHolders& holders, std::shared_ptr& selectedBackend)