From 8b5f4644ae4cb7c2327f6238764f18ab624bc885 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Wed, 18 Mar 2020 14:07:57 +0100 Subject: [PATCH] dnsdist: Add support for the processing of X-Forwarded-For headers --- pdns/dnsdist-lua.cc | 4 + pdns/dnsdistdist/Makefile.am | 1 + pdns/dnsdistdist/docs/reference/config.rst | 3 +- pdns/dnsdistdist/doh.cc | 54 ++++++++++- pdns/dnsdistdist/views.hh | 37 ++++++++ pdns/doh.hh | 1 + regression-tests.dnsdist/test_DOH.py | 100 +++++++++++++++++++++ 7 files changed, 197 insertions(+), 3 deletions(-) create mode 100644 pdns/dnsdistdist/views.hh diff --git a/pdns/dnsdist-lua.cc b/pdns/dnsdist-lua.cc index 1b4abfe682..6df9643197 100644 --- a/pdns/dnsdist-lua.cc +++ b/pdns/dnsdist-lua.cc @@ -1913,6 +1913,10 @@ static void setupLuaConfig(bool client, bool configCheck) frontend->d_sendCacheControlHeaders = boost::get((*vars)["sendCacheControlHeaders"]); } + if (vars->count("trustForwardedForHeader")) { + frontend->d_trustForwardedForHeader = boost::get((*vars)["trustForwardedForHeader"]); + } + parseTLSConfig(frontend->d_tlsConfig, "addDOHLocal", vars); } g_dohlocals.push_back(frontend); diff --git a/pdns/dnsdistdist/Makefile.am b/pdns/dnsdistdist/Makefile.am index e6bd28464e..e8175a5675 100644 --- a/pdns/dnsdistdist/Makefile.am +++ b/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 \ diff --git a/pdns/dnsdistdist/docs/reference/config.rst b/pdns/dnsdistdist/docs/reference/config.rst index 48ff00bcb8..ec6af0993c 100644 --- a/pdns/dnsdistdist/docs/reference/config.rst +++ b/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]) diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc index 6abf4b304e..a09260e67b 100644 --- a/pdns/dnsdistdist/doh.cc +++ b/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(&local)); DOHServerConfig* dsc = reinterpret_cast(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(&remote)); + // ComboAddress remote; + // h2o_socket_getpeername(sock, reinterpret_cast(&remote)); // cout<<"New HTTP accept for client "<data << endl; sock->data = dsc; diff --git a/pdns/dnsdistdist/views.hh b/pdns/dnsdistdist/views.hh new file mode 100644 index 0000000000..afd72f6c2e --- /dev/null +++ b/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 +#if BOOST_VERSION >= 106100 +#include +using boost::string_view; +#else +#include +using string_view = boost::string_ref; +#endif +#else // C++17 +using std::string_view; +#endif diff --git a/pdns/doh.hh b/pdns/doh.hh index f7f9a80aaa..9e51c2e065 100644 --- a/pdns/doh.hh +++ b/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 { diff --git a/regression-tests.dnsdist/test_DOH.py b/regression-tests.dnsdist/test_DOH.py index f7d2f4f3c8..69e37a9f86 100644 --- a/regression-tests.dnsdist/test_DOH.py +++ b/regression-tests.dnsdist/test_DOH.py @@ -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') -- 2.39.2