From 95f851d662d5a35ff3fb74f427747f42dd857792 Mon Sep 17 00:00:00 2001
From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Thu, 27 Feb 2020 12:34:23 +0100
Subject: [PATCH] rec: Enforce 'proxy-protocol-maximum-size'

---
 pdns/pdns_recursor.cc                         | 23 ++++++-
 .../test_ProxyProtocol.py                     | 63 ++++++++++++++++---
 2 files changed, 76 insertions(+), 10 deletions(-)

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index 24a94fcd88..596fd5e25e 100644
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -2050,7 +2050,8 @@ static void handleRunningTCPQuestion(int fd, FDMultiplexer::funcparam_t& var)
          the connection was received over UDP or TCP if neede */
       bool tcp;
       bool proxy = false;
-      if (parseProxyHeader(conn->data, proxy, conn->d_source, conn->d_destination, tcp, conn->proxyProtocolValues) <= 0) {
+      size_t used = parseProxyHeader(conn->data, proxy, conn->d_source, conn->d_destination, tcp, conn->proxyProtocolValues);
+      if (used <= 0) {
         if (g_logCommonErrors) {
           g_log<<Logger::Error<<"Unable to parse proxy protocol header in packet from TCP client "<< conn->d_remote.toStringWithPort() <<endl;
         }
@@ -2058,6 +2059,14 @@ static void handleRunningTCPQuestion(int fd, FDMultiplexer::funcparam_t& var)
         t_fdm->removeReadFD(fd);
         return;
       }
+      else if (static_cast<size_t>(used) > g_proxyProtocolMaximumSize) {
+        if (g_logCommonErrors) {
+          g_log<<Logger::Error<<"Proxy protocol header in packet from TCP client "<< conn->d_remote.toStringWithPort() << " is larger than proxy-protocol-maximum-size (" << used << "), dropping"<< endl;
+        }
+        ++g_stats.proxyProtocolInvalidCount;
+        t_fdm->removeReadFD(fd);
+        return;
+      }
 
       /* check the real source */
       /* note that if the proxy header used a 'LOCAL' command, the original source and destination are untouched so everything should be fine */
@@ -2645,17 +2654,25 @@ static void handleNewUDPQuestion(int fd, FDMultiplexer::funcparam_t& var)
         if (used <= 0) {
           ++g_stats.proxyProtocolInvalidCount;
           if (!g_quiet) {
-            g_log<<Logger::Error<<"Ignoring invalid proxy protocol ("<<std::to_string(len)<<", "<<std::to_string(used)<<") query from "<<fromaddr.toString()<<endl;
+            g_log<<Logger::Error<<"Ignoring invalid proxy protocol ("<<std::to_string(len)<<", "<<std::to_string(used)<<") query from "<<fromaddr.toStringWithPort()<<endl;
           }
           return;
         }
+        else if (static_cast<size_t>(used) > g_proxyProtocolMaximumSize) {
+          if (g_quiet) {
+            g_log<<Logger::Error<<"Proxy protocol header in UDP packet from "<< fromaddr.toStringWithPort() << " is larger than proxy-protocol-maximum-size (" << used << "), dropping"<< endl;
+          }
+          ++g_stats.proxyProtocolInvalidCount;
+          return;
+        }
+
         data.erase(0, used);
       }
       else if (len > 512) {
         /* we only allow UDP packets larger than 512 for those with a proxy protocol header */
         g_stats.truncatedDrops++;
         if (!g_quiet) {
-          g_log<<Logger::Error<<"Ignoring truncated query from "<<fromaddr.toString()<<endl;
+          g_log<<Logger::Error<<"Ignoring truncated query from "<<fromaddr.toStringWithPort()<<endl;
         }
         return;
       }
diff --git a/regression-tests.recursor-dnssec/test_ProxyProtocol.py b/regression-tests.recursor-dnssec/test_ProxyProtocol.py
index d243462078..ee521ea76a 100644
--- a/regression-tests.recursor-dnssec/test_ProxyProtocol.py
+++ b/regression-tests.recursor-dnssec/test_ProxyProtocol.py
@@ -169,6 +169,7 @@ class ProxyProtocolAllowedRecursorTest(ProxyProtocolRecursorTest):
 
     _config_template = """
     proxy-protocol-from=127.0.0.1
+    proxy-protocol-maximum-size=512
     allow-from=127.0.0.0/24, ::1/128, ::42/128
 """ % ()
 
@@ -182,7 +183,6 @@ class ProxyProtocolAllowedRecursorTest(ProxyProtocolRecursorTest):
         payload = ppPayload + queryPayload
 
         # UDP
-
         self._sock.settimeout(2.0)
 
         try:
@@ -237,7 +237,6 @@ class ProxyProtocolAllowedRecursorTest(ProxyProtocolRecursorTest):
         payload = ppPayload + queryPayload
 
         # UDP
-
         self._sock.settimeout(2.0)
 
         try:
@@ -324,6 +323,61 @@ class ProxyProtocolAllowedRecursorTest(ProxyProtocolRecursorTest):
         self.assertRcodeEqual(res, dns.rcode.NOERROR)
         self.assertRRsetInAnswer(res, expected)
 
+    def testTooLargeProxyProtocol(self):
+        # the total payload (proxy protocol + DNS) is larger than proxy-protocol-maximum-size
+        # so it should be dropped
+        qname = 'too-large.proxy-protocol.recursor-tests.powerdns.com.'
+        expected = dns.rrset.from_text(qname, 0, dns.rdataclass.IN, 'A', '192.0.2.1')
+
+        query = dns.message.make_query(qname, 'A', want_dnssec=True)
+        queryPayload = query.to_wire()
+        ppPayload = ProxyProtocol.getPayload(False, True, False, '127.0.0.42', '255.255.255.255', 0, 65535, [ [0, b'foo' ], [1, b'A'*512], [ 255, b'bar'] ])
+        payload = ppPayload + queryPayload
+
+        # UDP
+        self._sock.settimeout(2.0)
+
+        try:
+            self._sock.send(payload)
+            data = self._sock.recv(4096)
+        except socket.timeout:
+            data = None
+        finally:
+            self._sock.settimeout(None)
+
+        res = None
+        if data:
+            res = dns.message.from_wire(data)
+        self.assertEqual(res, None)
+
+        # TCP
+        sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        sock.settimeout(2.0)
+        sock.connect(("127.0.0.1", self._recursorPort))
+
+        try:
+            sock.send(ppPayload)
+            sock.send(struct.pack("!H", len(queryPayload)))
+            sock.send(queryPayload)
+
+            data = sock.recv(2)
+            if data:
+                (datalen,) = struct.unpack("!H", data)
+                data = sock.recv(datalen)
+        except socket.timeout as e:
+            print("Timeout: %s" % (str(e)))
+            data = None
+        except socket.error as e:
+            print("Network error: %s" % (str(e)))
+            data = None
+        finally:
+            sock.close()
+
+        res = None
+        if data:
+            res = dns.message.from_wire(data)
+        self.assertEqual(res, None)
+
     def testNoHeaderProxyProtocol(self):
         qname = 'no-header.proxy-protocol.recursor-tests.powerdns.com.'
 
@@ -520,11 +574,6 @@ class ProxyProtocolAllowedFFIRecursorTest(ProxyProtocolAllowedRecursorTest):
     end
     """
 
-    _config_template = """
-    proxy-protocol-from=127.0.0.1
-    allow-from=127.0.0.0/24, ::1/128, ::42/128
-""" % ()
-
 class ProxyProtocolNotAllowedRecursorTest(ProxyProtocolRecursorTest):
     _confdir = 'ProxyProtocolNotAllowed'
     _lua_dns_script_file = """
-- 
2.47.2