Merge pull request #8945 from rgacogne/ddist-x-forwarded-for

dnsdist: Add support for the processing of X-Forwarded-For headers

pdns/dnsdist-lua.cc

@@ -1913,6 +1913,10 @@ static void setupLuaConfig(bool client, bool configCheck)
         frontend->d_sendCacheControlHeaders = boost::get<bool>((*vars)["sendCacheControlHeaders"]);
       }
 
+      if (vars->count("trustForwardedForHeader")) {
+        frontend->d_trustForwardedForHeader = boost::get<bool>((*vars)["trustForwardedForHeader"]);
+      }
+
       parseTLSConfig(frontend->d_tlsConfig, "addDOHLocal", vars);
     }
     g_dohlocals.push_back(frontend);

pdns/dnsdistdist/Makefile.am

@@ -194,6 +194,7 @@ dnsdist_SOURCES = \
 	tcpiohandler.cc tcpiohandler.hh \
 	threadname.hh threadname.cc \
 	uuid-utils.hh uuid-utils.cc \
+	views.hh \
 	xpf.cc xpf.hh \
 	ext/luawrapper/include/LuaContext.hpp \
 	ext/json11/json11.cpp \

pdns/dnsdistdist/docs/reference/config.rst

@@ -109,7 +109,7 @@ Listen Sockets
   .. versionadded:: 1.4.0
 
   .. versionchanged:: 1.5.0
-    ``sendCacheControlHeaders``, ``sessionTimeout`` options added.
+    ``sendCacheControlHeaders``, ``sessionTimeout``, ``trustForwardedForHeader`` options added.
     ``url`` now defaults to ``/dns-query`` instead of ``/``
 
   Listen on the specified address and TCP port for incoming DNS over HTTPS connections, presenting the specified X.509 certificate.
@@ -144,6 +144,7 @@ Listen Sockets
   * ``preferServerCiphers``: bool - Whether to prefer the order of ciphers set by the server instead of the one set by the client. Default is true, meaning that the order of the server is used.
   * ``keyLogFile``: str - Write the TLS keys in the specified file so that an external program can decrypt TLS exchanges, in the format described in https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format. Note that this feature requires OpenSSL >= 1.1.1.
   * ``sendCacheControlHeaders``: bool - Whether to parse the response to find the lowest TTL and set a HTTP Cache-Control header accordingly. Default is true.
+  * ``trustForwardedForHeader``: bool - Whether to parse any existing X-Forwarded-For header in the HTTP query and use the right-most value as the client source address and port, for ACL checks, rules, logging and so on. Default is false.
 
 .. function:: addTLSLocal(address, certFile(s), keyFile(s) [, options])
 

pdns/dnsdistdist/doh.cc

@@ -29,6 +29,7 @@
 #include "dnsdist-xpf.hh"
 #include "libssl.hh"
 #include "threadname.hh"
+#include "views.hh"
 
 using namespace std;
 
@@ -654,6 +655,51 @@ static void doh_dispatch_query(DOHServerConfig* dsc, h2o_handler_t* self, h2o_re
   }
 }
 
+static bool getHTTPHeaderValue(const h2o_req_t* req, const std::string& headerName, string_view& value)
+{
+  bool found = false;
+
+  for (size_t i = 0; i < req->headers.size; ++i) {
+    if (string_view(req->headers.entries[i].name->base, req->headers.entries[i].name->len) == headerName) {
+      value = string_view(req->headers.entries[i].value.base, req->headers.entries[i].value.len);
+      /* don't stop there, we might have more than one header with the same name, and we want the last one */
+      found = true;
+    }
+  }
+
+  return found;
+}
+
+static void processForwardedForHeader(const h2o_req_t* req, ComboAddress& remote)
+{
+  static const std::string headerName = "x-forwarded-for";
+  string_view value;
+
+  if (getHTTPHeaderValue(req, headerName, value)) {
+    try {
+      auto pos = value.rfind(',');
+      if (pos != string_view::npos) {
+        ++pos;
+        for (; pos < value.size() && value.at(pos) == ' '; ++pos)
+        {
+        }
+
+        if (pos < value.size()) {
+          value = value.substr(pos);
+        }
+      }
+      auto newRemote = ComboAddress(std::string(value));
+      remote = newRemote;
+    }
+    catch (const std::exception& e) {
+      vinfolog("Invalid X-Forwarded-For header ('%s') received from %s : %s", std::string(value), remote.toStringWithPort(), e.what());
+    }
+    catch (const PDNSException& e) {
+      vinfolog("Invalid X-Forwarded-For header ('%s') received from %s : %s", std::string(value), remote.toStringWithPort(), e.reason);
+    }
+  }
+}
+
 /*
   A query has been parsed by h2o.
   For GET, the base64url-encoded payload is in the 'dns' parameter, which might be the first parameter, or not.
@@ -673,6 +719,10 @@ try
   h2o_socket_getsockname(sock, reinterpret_cast<struct sockaddr*>(&local));
   DOHServerConfig* dsc = reinterpret_cast<DOHServerConfig*>(req->conn->ctx->storage.entries[0].data);
 
+  if (dsc->df->d_trustForwardedForHeader) {
+    processForwardedForHeader(req, remote);
+  }
+
   auto& holders = dsc->holders;
   if (!holders.acl->match(remote)) {
     ++g_stats.aclDrops;
@@ -1016,8 +1066,8 @@ static void on_accept(h2o_socket_t *listener, const char *err)
     return;
   }
 
-  ComboAddress remote;
-  h2o_socket_getpeername(sock, reinterpret_cast<struct sockaddr*>(&remote));
+  // ComboAddress remote;
+  // h2o_socket_getpeername(sock, reinterpret_cast<struct sockaddr*>(&remote));
   //  cout<<"New HTTP accept for client "<<remote.toStringWithPort()<<": "<< listener->data << endl;
 
   sock->data = dsc;

pdns/dnsdistdist/views.hh

@@ -0,0 +1,37 @@
+/*
+ * This file is part of PowerDNS or dnsdist.
+ * Copyright -- PowerDNS.COM B.V. and its contributors
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * In addition, for the avoidance of any doubt, permission is granted to
+ * link this program with OpenSSL and to (re)distribute the binaries
+ * produced as the result of such linking.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#pragma once
+
+// apple compiler somehow has string_view even in c++11!
+#if __cplusplus < 201703L && !defined(__APPLE__)
+#include <boost/version.hpp>
+#if BOOST_VERSION >= 106100
+#include <boost/utility/string_view.hpp>
+using boost::string_view;
+#else
+#include <boost/utility/string_ref.hpp>
+using string_view = boost::string_ref;
+#endif
+#else // C++17
+using std::string_view;
+#endif

pdns/doh.hh

@@ -98,6 +98,7 @@ struct DOHFrontend
   HTTPVersionStats d_http1Stats;
   HTTPVersionStats d_http2Stats;
   bool d_sendCacheControlHeaders{true};
+  bool d_trustForwardedForHeader{false};
 
   time_t getTicketsKeyRotationDelay() const
   {

regression-tests.dnsdist/test_DOH.py

@@ -950,3 +950,103 @@ def testHTTPLuaFFIResponse(self):
         self.assertEquals(self._rcode, 200)
         self.assertTrue('content-type: text/plain' in self._response_headers.decode())
 
+class TestDOHForwardedFor(DNSDistDOHTest):
+
+    _serverKey = 'server.key'
+    _serverCert = 'server.chain'
+    _serverName = 'tls.tests.dnsdist.org'
+    _caCert = 'ca.pem'
+    _dohServerPort = 8443
+    _dohBaseURL = ("https://%s:%d/" % (_serverName, _dohServerPort))
+    _config_template = """
+    newServer{address="127.0.0.1:%s"}
+
+    setACL('192.0.2.1/32')
+    addDOHLocal("127.0.0.1:%s", "%s", "%s", { "/" }, {trustForwardedForHeader=true})
+    """
+    _config_params = ['_testServerPort', '_dohServerPort', '_serverCert', '_serverKey']
+
+    def testDOHAllowedForwarded(self):
+        """
+        DOH with X-Forwarded-For allowed
+        """
+        name = 'allowed.forwarded.doh.tests.powerdns.com.'
+        query = dns.message.make_query(name, 'A', 'IN', use_edns=False)
+        query.id = 0
+        expectedQuery = dns.message.make_query(name, 'A', 'IN', use_edns=True, payload=4096)
+        expectedQuery.id = 0
+        response = dns.message.make_response(query)
+        rrset = dns.rrset.from_text(name,
+                                    3600,
+                                    dns.rdataclass.IN,
+                                    dns.rdatatype.A,
+                                    '127.0.0.1')
+        response.answer.append(rrset)
+
+        (receivedQuery, receivedResponse) = self.sendDOHQuery(self._dohServerPort, self._serverName, self._dohBaseURL, query, response=response, caFile=self._caCert, customHeaders=['x-forwarded-for: 127.0.0.1:42, 127.0.0.1, 192.0.2.1:4200'])
+        self.assertTrue(receivedQuery)
+        self.assertTrue(receivedResponse)
+        receivedQuery.id = expectedQuery.id
+        self.assertEquals(expectedQuery, receivedQuery)
+        self.checkQueryEDNSWithoutECS(expectedQuery, receivedQuery)
+        self.assertEquals(response, receivedResponse)
+
+    def testDOHDeniedForwarded(self):
+        """
+        DOH with X-Forwarded-For not allowed
+        """
+        name = 'not-allowed.forwarded.doh.tests.powerdns.com.'
+        query = dns.message.make_query(name, 'A', 'IN', use_edns=False)
+        query.id = 0
+        expectedQuery = dns.message.make_query(name, 'A', 'IN', use_edns=True, payload=4096)
+        expectedQuery.id = 0
+        response = dns.message.make_response(query)
+        rrset = dns.rrset.from_text(name,
+                                    3600,
+                                    dns.rdataclass.IN,
+                                    dns.rdatatype.A,
+                                    '127.0.0.1')
+        response.answer.append(rrset)
+
+        (receivedQuery, receivedResponse) = self.sendDOHQuery(self._dohServerPort, self._serverName, self._dohBaseURL, query, response=response, caFile=self._caCert, useQueue=False, rawResponse=True, customHeaders=['x-forwarded-for: 127.0.0.1:42, 127.0.0.1'])
+
+        self.assertEquals(self._rcode, 403)
+        self.assertEquals(receivedResponse, b'dns query not allowed because of ACL')
+
+class TestDOHForwardedForNoTrusted(DNSDistDOHTest):
+
+    _serverKey = 'server.key'
+    _serverCert = 'server.chain'
+    _serverName = 'tls.tests.dnsdist.org'
+    _caCert = 'ca.pem'
+    _dohServerPort = 8443
+    _dohBaseURL = ("https://%s:%d/" % (_serverName, _dohServerPort))
+    _config_template = """
+    newServer{address="127.0.0.1:%s"}
+
+    setACL('192.0.2.1/32')
+    addDOHLocal("127.0.0.1:%s", "%s", "%s", { "/" })
+    """
+    _config_params = ['_testServerPort', '_dohServerPort', '_serverCert', '_serverKey']
+
+    def testDOHForwardedUntrusted(self):
+        """
+        DOH with X-Forwarded-For not trusted
+        """
+        name = 'not-trusted.forwarded.doh.tests.powerdns.com.'
+        query = dns.message.make_query(name, 'A', 'IN', use_edns=False)
+        query.id = 0
+        expectedQuery = dns.message.make_query(name, 'A', 'IN', use_edns=True, payload=4096)
+        expectedQuery.id = 0
+        response = dns.message.make_response(query)
+        rrset = dns.rrset.from_text(name,
+                                    3600,
+                                    dns.rdataclass.IN,
+                                    dns.rdatatype.A,
+                                    '127.0.0.1')
+        response.answer.append(rrset)
+
+        (receivedQuery, receivedResponse) = self.sendDOHQuery(self._dohServerPort, self._serverName, self._dohBaseURL, query, response=response, caFile=self._caCert, useQueue=False, rawResponse=True, customHeaders=['x-forwarded-for: 192.0.2.1:4200'])
+
+        self.assertEquals(self._rcode, 403)
+        self.assertEquals(receivedResponse, b'dns query not allowed because of ACL')