From 97e8507d467207e93f89b94528697bccca351aaf Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Thu, 18 Feb 2021 17:39:04 +0100 Subject: [PATCH] dnsdist: Fix the addition of the proxy protocol payload when reconnecting --- pdns/dnsdist-tcp.cc | 23 ++++++++++++------ pdns/dnsdistdist/dnsdist-tcp-downstream.cc | 27 ++++++++-------------- pdns/dnsdistdist/dnsdist-tcp-downstream.hh | 11 +++++---- pdns/dnsdistdist/dnsdist-tcp-upstream.hh | 2 +- 4 files changed, 34 insertions(+), 29 deletions(-) diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc index 53f23ecd35..0258585445 100644 --- a/pdns/dnsdist-tcp.cc +++ b/pdns/dnsdist-tcp.cc @@ -133,9 +133,16 @@ public: } } - static void clear() + static size_t clear() { + size_t count = 0; + for (const auto downstream : t_downstreamConnections) { + count += downstream.second.size(); + } + t_downstreamConnections.clear(); + + return count; } private: @@ -175,9 +182,9 @@ IncomingTCPConnectionState::~IncomingTCPConnectionState() d_handler.close(); } -void IncomingTCPConnectionState::clearAllDownstreamConnections() +size_t IncomingTCPConnectionState::clearAllDownstreamConnections() { - DownstreamConnectionsManager::clear(); + return DownstreamConnectionsManager::clear(); } std::shared_ptr IncomingTCPConnectionState::getDownstreamConnection(std::shared_ptr& ds, const std::unique_ptr>& tlvs, const struct timeval& now) @@ -699,16 +706,17 @@ static void handleQuery(std::shared_ptr& state, cons downstreamConnection->setProxyProtocolValuesSent(std::move(dq.proxyProtocolValues)); } + TCPQuery query(std::move(state->d_buffer), std::move(ids)); if (proxyProtocolPayloadAdded) { - downstreamConnection->setProxyProtocolPayloadAdded(true); + query.d_proxyProtocolPayloadAdded = true; } else { - downstreamConnection->setProxyProtocolPayload(std::move(proxyProtocolPayload)); + query.d_proxyProtocolPayload = std::move(proxyProtocolPayload); } ++state->d_currentQueriesCount; - vinfolog("Got query for %s|%s from %s (%s, %d bytes), relayed to %s", ids.qname.toLogString(), QType(ids.qtype).getName(), state->d_proxiedRemote.toStringWithPort(), (state->d_ci.cs->tlsFrontend ? "DoT" : "TCP"), state->d_buffer.size(), ds->getName()); - downstreamConnection->queueQuery(TCPQuery(std::move(state->d_buffer), std::move(ids)), downstreamConnection); + vinfolog("Got query for %s|%s from %s (%s, %d bytes), relayed to %s", query.d_idstate.qname.toLogString(), QType(query.d_idstate.qtype).getName(), state->d_proxiedRemote.toStringWithPort(), (state->d_ci.cs->tlsFrontend ? "DoT" : "TCP"), query.d_buffer.size(), ds->getName()); + downstreamConnection->queueQuery(std::move(query), downstreamConnection); } void IncomingTCPConnectionState::handleIOCallback(int fd, FDMultiplexer::funcparam_t& param) @@ -935,6 +943,7 @@ void IncomingTCPConnectionState::handleIO(std::shared_ptrd_ci.cs->tcpDiedReadingQuery; } else if (state->d_state == IncomingTCPConnectionState::State::sendingResponse) { + /* unlikely to happen here, the exception should be handled in sendResponse() */ ++state->d_ci.cs->tcpDiedSendingResponse; } diff --git a/pdns/dnsdistdist/dnsdist-tcp-downstream.cc b/pdns/dnsdistdist/dnsdist-tcp-downstream.cc index 74f5fe46f7..204717950a 100644 --- a/pdns/dnsdistdist/dnsdist-tcp-downstream.cc +++ b/pdns/dnsdistdist/dnsdist-tcp-downstream.cc @@ -61,6 +61,9 @@ IOState TCPConnectionToBackend::sendQuery(std::shared_ptrd_currentQuery.d_proxyProtocolPayloadAdded) { + conn->d_proxyProtocolPayloadSent = true; + } conn->incQueries(); conn->d_currentPos = 0; @@ -211,9 +214,9 @@ void TCPConnectionToBackend::handleIO(std::shared_ptr& c iostate = queueNextQuery(conn); } - if (!conn->d_proxyProtocolPayloadAdded && !conn->d_proxyProtocolPayload.empty()) { - conn->d_currentQuery.d_buffer.insert(conn->d_currentQuery.d_buffer.begin(), conn->d_proxyProtocolPayload.begin(), conn->d_proxyProtocolPayload.end()); - conn->d_proxyProtocolPayloadAdded = true; + if (conn->needProxyProtocolPayload() && !conn->d_currentQuery.d_proxyProtocolPayloadAdded && !conn->d_currentQuery.d_proxyProtocolPayload.empty()) { + conn->d_currentQuery.d_buffer.insert(conn->d_currentQuery.d_buffer.begin(), conn->d_currentQuery.d_proxyProtocolPayload.begin(), conn->d_currentQuery.d_proxyProtocolPayload.end()); + conn->d_currentQuery.d_proxyProtocolPayloadAdded = true; } reconnected = true; @@ -273,16 +276,15 @@ void TCPConnectionToBackend::queueQuery(TCPQuery&& query, std::shared_ptrupdate(IOState::NeedWrite, handleIOCallback, sharedSelf, getBackendWriteTTD(now)); } else { DEBUGLOG("Adding new query to the queue because we are in state "<<(int)d_state); @@ -301,6 +303,7 @@ bool TCPConnectionToBackend::reconnect() } d_fresh = true; + d_proxyProtocolPayloadSent = false; do { vinfolog("TCP connecting to downstream %s (%d)", d_ds->getNameWithAddr(), d_downstreamFailures); @@ -504,16 +507,6 @@ uint16_t TCPConnectionToBackend::getQueryIdFromResponse() return ntohs(dh.id); } -void TCPConnectionToBackend::setProxyProtocolPayload(std::string&& payload) -{ - d_proxyProtocolPayload = std::move(payload); -} - -void TCPConnectionToBackend::setProxyProtocolPayloadAdded(bool added) -{ - d_proxyProtocolPayloadAdded = added; -} - void TCPConnectionToBackend::setProxyProtocolValuesSent(std::unique_ptr>&& proxyProtocolValuesSent) { /* if we already have some values, we have already verified they match */ diff --git a/pdns/dnsdistdist/dnsdist-tcp-downstream.hh b/pdns/dnsdistdist/dnsdist-tcp-downstream.hh index a26b29ca06..d582ef8fb0 100644 --- a/pdns/dnsdistdist/dnsdist-tcp-downstream.hh +++ b/pdns/dnsdistdist/dnsdist-tcp-downstream.hh @@ -18,6 +18,8 @@ struct TCPQuery IDState d_idstate; PacketBuffer d_buffer; + std::string d_proxyProtocolPayload; + bool d_proxyProtocolPayloadAdded{false}; }; class TCPConnectionToBackend; @@ -165,8 +167,6 @@ public: void handleTimeout(const struct timeval& now, bool write); void release(); - void setProxyProtocolPayload(std::string&& payload); - void setProxyProtocolPayloadAdded(bool added); void setProxyProtocolValuesSent(std::unique_ptr>&& proxyProtocolValuesSent); std::string toString() const @@ -191,6 +191,10 @@ private: uint16_t getQueryIdFromResponse(); bool reconnect(); void notifyAllQueriesFailed(const struct timeval& now, FailureReason reason); + bool needProxyProtocolPayload() const + { + return !d_proxyProtocolPayloadSent && (d_ds && d_ds->useProxyProtocol); + } boost::optional getBackendReadTTD(const struct timeval& now) const { @@ -232,7 +236,6 @@ private: std::unique_ptr d_ioState{nullptr}; std::shared_ptr d_ds{nullptr}; std::shared_ptr d_clientConn; - std::string d_proxyProtocolPayload; TCPQuery d_currentQuery; struct timeval d_connectionStartTime; size_t d_currentPos{0}; @@ -244,5 +247,5 @@ private: bool d_enableFastOpen{false}; bool d_connectionDied{false}; bool d_usedForXFR{false}; - bool d_proxyProtocolPayloadAdded{false}; + bool d_proxyProtocolPayloadSent{false}; }; diff --git a/pdns/dnsdistdist/dnsdist-tcp-upstream.hh b/pdns/dnsdistdist/dnsdist-tcp-upstream.hh index 49428ed055..54c3d88f2b 100644 --- a/pdns/dnsdistdist/dnsdist-tcp-upstream.hh +++ b/pdns/dnsdistdist/dnsdist-tcp-upstream.hh @@ -151,7 +151,7 @@ public: return d_threadData.mplexer; } - static void clearAllDownstreamConnections(); + static size_t clearAllDownstreamConnections(); static void handleIO(std::shared_ptr& conn, const struct timeval& now); static void handleIOCallback(int fd, FDMultiplexer::funcparam_t& param); -- 2.47.2