From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Wed, 7 May 2025 08:52:56 +0000 (+0200)
Subject: dnsdist: Only set the proxy protocol payload size when actually added
X-Git-Tag: dnsdist-2.0.0-alpha2~20^2~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3b193fcb2c46237e40fda55a6bf59b79a6d942a7;p=thirdparty%2Fpdns.git

dnsdist: Only set the proxy protocol payload size when actually added

I can think of two cases where we got this wrong:
- the query was initially assigned to a backend using the proxy protocol
  payload, then later restarted and assigned to a backend not using it.
  The proxy protocol payload size was then kept from the first assignment.
- we failed to actually prepend the proxy protocol payload but the payload
  size was updated.

Both cases could cause a corrupted payload to be sent, or an exception to
be raised if the size of the proxy protocol payload was larger than the
size of the initial query.
---

diff --git a/pdns/dnsdistdist/dnsdist-proxy-protocol.cc b/pdns/dnsdistdist/dnsdist-proxy-protocol.cc
index 01056b1d5a..ec927341bd 100644
--- a/pdns/dnsdistdist/dnsdist-proxy-protocol.cc
+++ b/pdns/dnsdistdist/dnsdist-proxy-protocol.cc
@@ -40,14 +40,19 @@ bool addProxyProtocol(DNSQuestion& dq, const std::string& payload)
   return addProxyProtocol(dq.getMutableData(), payload);
 }
 
-bool addProxyProtocol(DNSQuestion& dq, size_t* payloadSize)
+bool addProxyProtocol(DNSQuestion& dnsQuestion, size_t* proxyProtocolPayloadSize)
 {
-  auto payload = getProxyProtocolPayload(dq);
-  if (payloadSize != nullptr) {
-    *payloadSize = payload.size();
+  auto payload = getProxyProtocolPayload(dnsQuestion);
+  size_t payloadSize = payload.size();
+
+  if (!addProxyProtocol(dnsQuestion, payload)) {
+    return false;
   }
 
-  return addProxyProtocol(dq, payload);
+  if (proxyProtocolPayloadSize != nullptr) {
+    *proxyProtocolPayloadSize = payloadSize;
+  }
+  return true;
 }
 
 bool addProxyProtocol(PacketBuffer& buffer, const std::string& payload)
diff --git a/pdns/dnsdistdist/dnsdist-proxy-protocol.hh b/pdns/dnsdistdist/dnsdist-proxy-protocol.hh
index 6e4234d6be..3404729461 100644
--- a/pdns/dnsdistdist/dnsdist-proxy-protocol.hh
+++ b/pdns/dnsdistdist/dnsdist-proxy-protocol.hh
@@ -31,7 +31,7 @@ struct DNSQuestion;
 
 std::string getProxyProtocolPayload(const DNSQuestion& dq);
 
-bool addProxyProtocol(DNSQuestion& dq, size_t* proxyProtocolPayloadSize = nullptr);
+bool addProxyProtocol(DNSQuestion& dnsQuestion, size_t* proxyProtocolPayloadSize = nullptr);
 bool addProxyProtocol(DNSQuestion& dq, const std::string& payload);
 bool addProxyProtocol(PacketBuffer& buffer, const std::string& payload);
 bool addProxyProtocol(PacketBuffer& buffer, bool tcp, const ComboAddress& source, const ComboAddress& destination, const std::vector<ProxyProtocolValue>& values);
diff --git a/pdns/dnsdistdist/dnsdist.cc b/pdns/dnsdistdist/dnsdist.cc
index b35b40628b..219a0549d7 100644
--- a/pdns/dnsdistdist/dnsdist.cc
+++ b/pdns/dnsdistdist/dnsdist.cc
@@ -1720,9 +1720,13 @@ bool assignOutgoingUDPQueryToBackend(std::shared_ptr<DownstreamState>& downstrea
   bool doh = dnsQuestion.ids.du != nullptr;
 
   bool failed = false;
+  dnsQuestion.ids.d_proxyProtocolPayloadSize = 0;
   if (downstream->d_config.useProxyProtocol) {
     try {
-      addProxyProtocol(dnsQuestion, &dnsQuestion.ids.d_proxyProtocolPayloadSize);
+      size_t proxyProtocolPayloadSize = 0;
+      if (addProxyProtocol(dnsQuestion, &proxyProtocolPayloadSize)) {
+        dnsQuestion.ids.d_proxyProtocolPayloadSize = proxyProtocolPayloadSize;
+      }
     }
     catch (const std::exception& e) {
       vinfolog("Adding proxy protocol payload to %s query from %s failed: %s", (dnsQuestion.ids.du ? "DoH" : ""), dnsQuestion.ids.origDest.toStringWithPort(), e.what());