]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
dnsdist: Add support for the processing of X-Forwarded-For headers 8945/head
authorRemi Gacogne <remi.gacogne@powerdns.com>
Wed, 18 Mar 2020 13:07:57 +0000 (14:07 +0100)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Wed, 18 Mar 2020 13:07:57 +0000 (14:07 +0100)
pdns/dnsdist-lua.cc
pdns/dnsdistdist/Makefile.am
pdns/dnsdistdist/docs/reference/config.rst
pdns/dnsdistdist/doh.cc
pdns/dnsdistdist/views.hh [new file with mode: 0644]
pdns/doh.hh
regression-tests.dnsdist/test_DOH.py

index 1b4abfe68259b9eb4adb1dd0ff13e1a393249e8e..6df96431978fbbe3fc9b12faff81d0a0aaa9d1db 100644 (file)
@@ -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);
index e6bd28464e065f4b185c30391c982cabfa16e0f1..e8175a567570f0a7f28edaad577c7af31d8b9d7a 100644 (file)
@@ -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 \
index 48ff00bcb8f9a2d454462565bb14ba0a97e13e8b..ec6af0993c9b62f02a8ee3cf57dcfdb3229fa926 100644 (file)
@@ -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])
 
index 6abf4b304e25e51326bb78fdc48e2d55b790fbab..a09260e67b5b590b90cff859f45cf7b27acd8014 100644 (file)
@@ -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;
diff --git a/pdns/dnsdistdist/views.hh b/pdns/dnsdistdist/views.hh
new file mode 100644 (file)
index 0000000..afd72f6
--- /dev/null
@@ -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
index f7f9a80aaa7b2bfbda59fbf873c0a72189933889..9e51c2e065359951176d3f26343d416cf11fd7c7 100644 (file)
@@ -98,6 +98,7 @@ struct DOHFrontend
   HTTPVersionStats d_http1Stats;
   HTTPVersionStats d_http2Stats;
   bool d_sendCacheControlHeaders{true};
+  bool d_trustForwardedForHeader{false};
 
   time_t getTicketsKeyRotationDelay() const
   {
index f7d2f4f3c8fa3e7cdce4f2dda88b4816c9aa9a55..69e37a9f8678b62a0a98113add8fd4590a12e545 100644 (file)
@@ -950,3 +950,103 @@ class TestDOHFFI(DNSDistDOHTest):
         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')