From 9676d2a924904ce2ea4a8f1d89e68bc8858a0877 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Fri, 2 Aug 2019 18:01:49 +0200 Subject: [PATCH] dnsdist: Add regression tests for DoH HTTP bindings and actions --- pdns/dnsdist-lua-actions.cc | 10 +- pdns/dnsdist-lua-bindings-dnsquestion.cc | 4 +- pdns/dnsdistdist/docs/reference/dq.rst | 52 ++++++++++ pdns/dnsdistdist/docs/rules-actions.rst | 8 +- pdns/dnsdistdist/doh.cc | 79 ++++++++++++-- pdns/doh.hh | 4 +- regression-tests.dnsdist/test_DOH.py | 126 +++++++++++++++++++++-- 7 files changed, 254 insertions(+), 29 deletions(-) diff --git a/pdns/dnsdist-lua-actions.cc b/pdns/dnsdist-lua-actions.cc index 40c5412bb..be4a8785d 100644 --- a/pdns/dnsdist-lua-actions.cc +++ b/pdns/dnsdist-lua-actions.cc @@ -1081,7 +1081,7 @@ private: class HTTPStatusAction: public DNSAction { public: - HTTPStatusAction(int code, const std::string& reason, const std::string& body): d_reason(reason), d_body(body), d_code(code) + HTTPStatusAction(int code, const std::string& body, const std::string& contentType): d_body(body), d_contentType(contentType), d_code(code) { } @@ -1091,7 +1091,7 @@ public: return Action::None; } - dq->du->setHTTPResponse(d_code, d_reason, d_body); + dq->du->setHTTPResponse(d_code, d_body, d_contentType); dq->dh->qr = true; // for good measure return Action::HeaderModify; } @@ -1101,8 +1101,8 @@ public: return "return an HTTP status of " + std::to_string(d_code); } private: - std::string d_reason; std::string d_body; + std::string d_contentType; int d_code; }; #endif /* HAVE_DNS_OVER_HTTPS */ @@ -1412,8 +1412,8 @@ void setupLuaActions() }); #ifdef HAVE_DNS_OVER_HTTPS - g_lua.writeFunction("HTTPStatusAction", [](uint16_t status, std::string reason, std::string body) { - return std::shared_ptr(new HTTPStatusAction(status, reason, body)); + g_lua.writeFunction("HTTPStatusAction", [](uint16_t status, std::string body, boost::optional contentType) { + return std::shared_ptr(new HTTPStatusAction(status, body, contentType ? *contentType : "")); }); #endif /* HAVE_DNS_OVER_HTTPS */ } diff --git a/pdns/dnsdist-lua-bindings-dnsquestion.cc b/pdns/dnsdist-lua-bindings-dnsquestion.cc index 50685e6d2..4be5d9ebd 100644 --- a/pdns/dnsdist-lua-bindings-dnsquestion.cc +++ b/pdns/dnsdist-lua-bindings-dnsquestion.cc @@ -213,11 +213,11 @@ void setupLuaBindingsDNSQuestion() return dq.du->getHTTPHeaders(); }); - g_lua.registerFunction("setHTTPResponse", [](DNSQuestion& dq, uint16_t statusCode, std::string reason, std::string body) { + g_lua.registerFunction("setHTTPResponse", [](DNSQuestion& dq, uint16_t statusCode, std::string body, boost::optional contentType) { if (dq.du == nullptr) { return; } - dq.du->setHTTPResponse(statusCode, reason, body); + dq.du->setHTTPResponse(statusCode, body, contentType ? *contentType : ""); }); #endif /* HAVE_DNS_OVER_HTTPS */ } diff --git a/pdns/dnsdistdist/docs/reference/dq.rst b/pdns/dnsdistdist/docs/reference/dq.rst index a02c9d2da..6d40afc8d 100644 --- a/pdns/dnsdistdist/docs/reference/dq.rst +++ b/pdns/dnsdistdist/docs/reference/dq.rst @@ -92,6 +92,46 @@ This state can be modified from the various hooks. :returns: A table of EDNSOptionView objects, indexed on the ECS Option code + .. method:: DNSQuestion:getHTTPHeaders() -> table + + .. versionadded:: 1.4.0 + + Return the HTTP headers for a DoH query, as a table whose keys are the header names and values the header values. + + :returns: A table of HTTP headers + + .. method:: DNSQuestion:getHTTPHost() -> string + + .. versionadded:: 1.4.0 + + Return the HTTP Host for a DoH query, which may or may not contain the port. + + :returns: The host of the DoH query + + .. method:: DNSQuestion:getHTTPPath() -> string + + .. versionadded:: 1.4.0 + + Return the HTTP path for a DoH query. + + :returns: The path part of the DoH query URI + + .. method:: DNSQuestion:getHTTPQueryString() -> string + + .. versionadded:: 1.4.0 + + Return the HTTP query string for a DoH query. + + :returns: The query string part of the DoH query URI + + .. method:: DNSQuestion:getHTTPScheme() -> string + + .. versionadded:: 1.4.0 + + Return the HTTP scheme for a DoH query. + + :returns: The scheme of the DoH query, for example ''http'' or ''https'' + .. method:: DNSQuestion:getServerNameIndication() -> string .. versionadded:: 1.4.0 @@ -134,6 +174,18 @@ This state can be modified from the various hooks. :param string reason: An optional string describing the reason why this trap was sent + .. method:: DNSQuestion:setHTTPResponse(status, body, contentType="") + + .. versionadded:: 1.4.0 + + Set the HTTP status code and content to immediately send back to the client. + For HTTP redirects (3xx), the string supplied in ''body'' should be the URL to redirect to. + For 200 responses, the value of the content type header can be specified via the ''contentType'' parameter. + + :param int status: The HTTP status code to return + :param string body: The body of the HTTP response, or a URL if the status code is a redirect (3xx) + :param string contentType: The HTTP Content-Type header to return for a 200 response, ignored otherwise. Default is ''application/dns-message''. + .. method:: DNSQuestion:setTag(key, value) .. versionadded:: 1.2.0 diff --git a/pdns/dnsdistdist/docs/rules-actions.rst b/pdns/dnsdistdist/docs/rules-actions.rst index b7daf3ccd..3366d8da5 100644 --- a/pdns/dnsdistdist/docs/rules-actions.rst +++ b/pdns/dnsdistdist/docs/rules-actions.rst @@ -948,14 +948,14 @@ The following actions exist. :param int rcode: The extended RCODE to respond with. -.. function:: HTTPStatusAction(status, reason, body) +.. function:: HTTPStatusAction(status, body, contentType="") .. versionadded:: 1.4.0 - Return an HTTP response with a status code of ''status'' and a reason of ''reason''. For HTTP redirects, ''body'' should be the redirect URL. + Return an HTTP response with a status code of ''status''. For HTTP redirects, ''body'' should be the redirect URL. :param int status: The HTTP status code to return. - :param str reason: The HTTP reason. - :param str body: the body of the HTTP response, or an URL if the status code is a redirect (3xx). + :param string body: The body of the HTTP response, or a URL if the status code is a redirect (3xx). + :param string contentType: The HTTP Content-Type header to return for a 200 response, ignored otherwise. Default is ''application/dns-message''. .. function:: LogAction([filename[, binary[, append[, buffered]]]]) diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc index 08fa6c6ec..84a5bd55a 100644 --- a/pdns/dnsdistdist/doh.cc +++ b/pdns/dnsdistdist/doh.cc @@ -654,11 +654,11 @@ std::string DOHUnit::getHTTPQueryString() const } } -void DOHUnit::setHTTPResponse(uint16_t statusCode, const std::string& reason_, const std::string& body_) +void DOHUnit::setHTTPResponse(uint16_t statusCode, const std::string& body_, const std::string& contentType_) { status_code = statusCode; - reason = reason_; response = body_; + contentType = contentType_; } void dnsdistclient(int qsock, int rsock) @@ -709,6 +709,58 @@ void dnsdistclient(int qsock, int rsock) } } +static const std::string& getReasonFromStatusCode(uint16_t statusCode) +{ + /* no need to care too much about this, HTTP/2 has no 'reason' anyway */ + static const std::unordered_map reasons = { + { 200, "OK" }, + { 301, "Moved Permanently" }, + { 302, "Found" }, + { 303, "See Other" }, + { 304, "Not Modified" }, + { 305, "Use Proxy" }, + { 306, "Switch Proxy" }, + { 307, "Temporary Redirect" }, + { 308, "Permanent Redirect" }, + { 400, "Bad Request" }, + { 401, "Unauthorized" }, + { 402, "Payment Required" }, + { 403, "Forbidden" }, + { 404, "Not Found" }, + { 405, "Method Not Allowed" }, + { 406, "Not Acceptable" }, + { 407, "Proxy Authentication Required" }, + { 408, "Request Timeout" }, + { 409, "Conflict" }, + { 410, "Gone" }, + { 411, "Length Required" }, + { 412, "Precondition Failed" }, + { 413, "Payload Too Large" }, + { 414, "URI Too Long" }, + { 415, "Unsupported Media Type" }, + { 416, "Range Not Satisfiable" }, + { 417, "Expectation Failed" }, + { 418, "I'm a teapot" }, + { 451, "Unavailable For Legal Reasons" }, + { 500, "Internal Server Error" }, + { 501, "Not Implemented" }, + { 502, "Bad Gateway" }, + { 503, "Service Unavailable" }, + { 504, "Gateway Timeout" }, + { 505, "HTTP Version Not Supported" } + }; + static const std::string unknown = "Unknown"; + + const auto it = reasons.find(statusCode); + if (it == reasons.end()) { + return unknown; + } + else { + return it->second; + } +} + + // called if h2o finds that dnsdist gave us an answer static void on_dnsdist(h2o_socket_t *listener, const char *err) { @@ -734,35 +786,44 @@ static void on_dnsdist(h2o_socket_t *listener, const char *err) if (du->status_code == 200) { ++dsc->df->d_validresponses; du->req->res.status = 200; - du->req->res.reason = "OK"; // struct dnsheader* dh = (struct dnsheader*)du->query.c_str(); // cout<<"Attempt to send out "<query.size()<<" bytes over https, TC="<tc<<", RCODE="<rcode<<", qtype="<qtype<<", req="<<(void*)du->req<req->pool, &du->req->res.headers, H2O_TOKEN_CONTENT_TYPE, nullptr, H2O_STRLIT("application/dns-message")); + if (du->contentType.empty()) { + h2o_add_header(&du->req->pool, &du->req->res.headers, H2O_TOKEN_CONTENT_TYPE, nullptr, H2O_STRLIT("application/dns-message")); + } + else { + /* we need to duplicate the header content because h2o keeps a pointer and we will be deleted before the response has been sent */ + h2o_iovec_t ct = h2o_strdup(&du->req->pool, du->contentType.c_str(), du->contentType.size()); + h2o_add_header(&du->req->pool, &du->req->res.headers, H2O_TOKEN_CONTENT_TYPE, nullptr, ct.base, ct.len); + } + du->req->res.content_length = du->response.size(); h2o_send_inline(du->req, du->response.c_str(), du->response.size()); } else if (du->status_code >= 300 && du->status_code < 400) { /* in that case the response is actually a URL */ - h2o_send_redirect(du->req, du->status_code, du->reason.c_str(), du->response.c_str(), du->response.size()); + /* we need to duplicate the URL because h2o uses it for the location header, keeping a pointer, and we will be deleted before the response has been sent */ + h2o_iovec_t url = h2o_strdup(&du->req->pool, du->response.c_str(), du->response.size()); + h2o_send_redirect(du->req, du->status_code, getReasonFromStatusCode(du->status_code).c_str(), url.base, url.len); ++dsc->df->d_redirectresponses; } else { switch(du->status_code) { case 400: - h2o_send_error_400(du->req, du->reason.empty() ? "Bad Request" : du->reason.c_str(), du->response.empty() ? "invalid DNS query" : du->response.c_str(), 0); + h2o_send_error_400(du->req, getReasonFromStatusCode(du->status_code).c_str(), du->response.empty() ? "invalid DNS query" : du->response.c_str(), 0); break; case 403: - h2o_send_error_403(du->req, du->reason.empty() ? "Forbidden" : du->reason.c_str(), du->response.empty() ? "dns query not allowed" : du->response.c_str(), 0); + h2o_send_error_403(du->req, getReasonFromStatusCode(du->status_code).c_str(), du->response.empty() ? "dns query not allowed" : du->response.c_str(), 0); break; case 502: - h2o_send_error_502(du->req, du->reason.empty() ? "Bad Gateway" : du->reason.c_str(), du->response.empty() ? "no downstream server available" : du->response.c_str(), 0); + h2o_send_error_502(du->req, getReasonFromStatusCode(du->status_code).c_str(), du->response.empty() ? "no downstream server available" : du->response.c_str(), 0); break; case 500: /* fall-through */ default: - h2o_send_error_500(du->req, du->reason.empty() ? "Internal Server Error" : du->reason.c_str(), du->response.empty() ? "Internal Server Error" : du->response.c_str(), 0); + h2o_send_error_500(du->req, getReasonFromStatusCode(du->status_code).c_str(), du->response.empty() ? "Internal Server Error" : du->response.c_str(), 0); break; } diff --git a/pdns/doh.hh b/pdns/doh.hh index c1739d987..9af3239f4 100644 --- a/pdns/doh.hh +++ b/pdns/doh.hh @@ -76,7 +76,7 @@ struct DOHUnit ComboAddress dest; st_h2o_req_t* req{nullptr}; DOHUnit** self{nullptr}; - std::string reason; + std::string contentType; int rsock; uint16_t qtype; /* the status_code is set from @@ -94,7 +94,7 @@ struct DOHUnit std::string getHTTPScheme() const; std::string getHTTPQueryString() const; std::unordered_map getHTTPHeaders() const; - void setHTTPResponse(uint16_t statusCode, const std::string& reason, const std::string& body); + void setHTTPResponse(uint16_t statusCode, const std::string& body, const std::string& contentType=""); }; #endif /* HAVE_DNS_OVER_HTTPS */ diff --git a/regression-tests.dnsdist/test_DOH.py b/regression-tests.dnsdist/test_DOH.py index 8a09652bd..bf4b990c3 100644 --- a/regression-tests.dnsdist/test_DOH.py +++ b/regression-tests.dnsdist/test_DOH.py @@ -30,7 +30,7 @@ class DNSDistDOHTest(DNSDistTest): return conn @classmethod - def sendDOHQuery(cls, port, servername, baseurl, query, response=None, timeout=2.0, caFile=None, useQueue=True, rawQuery=False, customHeaders=[]): + def sendDOHQuery(cls, port, servername, baseurl, query, response=None, timeout=2.0, caFile=None, useQueue=True, rawQuery=False, rawResponse=False, customHeaders=[]): url = cls.getDOHGetURL(baseurl, query, rawQuery) conn = cls.openDOHConnection(port, caFile=caFile, timeout=timeout) response_headers = BytesIO() @@ -51,9 +51,11 @@ class DNSDistDOHTest(DNSDistTest): message = None cls._response_headers = '' data = conn.perform_rb() - rcode = conn.getinfo(pycurl.RESPONSE_CODE) - if rcode == 200: + cls._rcode = conn.getinfo(pycurl.RESPONSE_CODE) + if cls._rcode == 200 and not rawResponse: message = dns.message.from_wire(data) + elif rawResponse: + message = data if useQueue and not cls._fromResponderQueue.empty(): receivedQuery = cls._fromResponderQueue.get(True, timeout) @@ -62,14 +64,17 @@ class DNSDistDOHTest(DNSDistTest): return (receivedQuery, message) @classmethod - def sendDOHPostQuery(cls, port, servername, baseurl, query, response=None, timeout=2.0, caFile=None, useQueue=True, rawQuery=False): + def sendDOHPostQuery(cls, port, servername, baseurl, query, response=None, timeout=2.0, caFile=None, useQueue=True, rawQuery=False, rawResponse=False, customHeaders=[]): url = baseurl conn = cls.openDOHConnection(port, caFile=caFile, timeout=timeout) + response_headers = BytesIO() #conn.setopt(pycurl.VERBOSE, True) conn.setopt(pycurl.URL, url) conn.setopt(pycurl.RESOLVE, ["%s:%d:127.0.0.1" % (servername, port)]) conn.setopt(pycurl.SSL_VERIFYPEER, 1) conn.setopt(pycurl.SSL_VERIFYHOST, 2) + conn.setopt(pycurl.HTTPHEADER, customHeaders) + conn.setopt(pycurl.HEADERFUNCTION, response_headers.write) conn.setopt(pycurl.POST, True) data = query if not rawQuery: @@ -85,14 +90,18 @@ class DNSDistDOHTest(DNSDistTest): receivedQuery = None message = None + cls._response_headers = '' data = conn.perform_rb() - rcode = conn.getinfo(pycurl.RESPONSE_CODE) - if rcode == 200: + cls._rcode = conn.getinfo(pycurl.RESPONSE_CODE) + if cls._rcode == 200 and not rawResponse: message = dns.message.from_wire(data) + elif rawResponse: + message = data if useQueue and not cls._fromResponderQueue.empty(): receivedQuery = cls._fromResponderQueue.get(True, timeout) + cls._response_headers = response_headers.getvalue() return (receivedQuery, message) # @classmethod @@ -145,8 +154,30 @@ class TestDOH(DNSDistDOHTest): addAction("spoof.doh.tests.powerdns.com.", SpoofAction("1.2.3.4")) addAction(HTTPHeaderRule("X-PowerDNS", "^[a]{5}$"), SpoofAction("2.3.4.5")) addAction(HTTPPathRule("/PowerDNS"), SpoofAction("3.4.5.6")) + addAction(HTTPPathRegexRule("^/PowerDNS-[0-9]"), SpoofAction("6.7.8.9")) + addAction("http-status-action.doh.tests.powerdns.com.", HTTPStatusAction(200, "Plaintext answer", "text/plain")) + addAction("http-status-action-redirect.doh.tests.powerdns.com.", HTTPStatusAction(307, "https://doh.powerdns.org")) + + function dohHandler(dq) + if dq:getHTTPScheme() == 'https' and dq:getHTTPHost() == '%s:%d' and dq:getHTTPPath() == '/' and dq:getHTTPQueryString() == '' then + local foundct = false + for key,value in pairs(dq:getHTTPHeaders()) do + if key == 'content-type' and value == 'application/dns-message' then + foundct = true + break + end + end + if foundct then + dq:setHTTPResponse(200, 'It works!', 'text/plain') + dq.dh:setQR(true) + return DNSAction.HeaderModify + end + end + return DNSAction.None + end + addAction("http-lua.doh.tests.powerdns.com.", LuaAction(dohHandler)) """ - _config_params = ['_testServerPort', '_dohServerPort', '_serverCert', '_serverKey'] + _config_params = ['_testServerPort', '_dohServerPort', '_serverCert', '_serverKey', '_serverName', '_dohServerPort'] def testDOHSimple(self): """ @@ -450,6 +481,87 @@ class TestDOH(DNSDistDOHTest): self.checkQueryEDNSWithoutECS(expectedQuery, receivedQuery) self.assertEquals(response, receivedResponse) + def testHTTPPathRegex(self): + """ + DOH: HTTPPathRegex + """ + name = 'http-path-regex.doh.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN') + query.id = 0 + query.flags &= ~dns.flags.RD + expectedResponse = dns.message.make_response(query) + rrset = dns.rrset.from_text(name, + 3600, + dns.rdataclass.IN, + dns.rdatatype.A, + '6.7.8.9') + expectedResponse.answer.append(rrset) + + # this path should match + (_, receivedResponse) = self.sendDOHQuery(self._dohServerPort, self._serverName, self._dohBaseURL + 'PowerDNS-999', caFile=self._caCert, query=query, response=None, useQueue=False) + self.assertEquals(receivedResponse, expectedResponse) + + expectedQuery = dns.message.make_query(name, 'A', 'IN', use_edns=True, payload=4096) + expectedQuery.id = 0 + expectedQuery.flags &= ~dns.flags.RD + 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) + + # this path should NOT match + (receivedQuery, receivedResponse) = self.sendDOHQuery(self._dohServerPort, self._serverName, self._dohBaseURL + "PowerDNS2", query, response=response, caFile=self._caCert) + self.assertTrue(receivedQuery) + self.assertTrue(receivedResponse) + receivedQuery.id = expectedQuery.id + self.assertEquals(expectedQuery, receivedQuery) + self.checkQueryEDNSWithoutECS(expectedQuery, receivedQuery) + self.assertEquals(response, receivedResponse) + + def testHTTPStatusAction200(self): + """ + DOH: HTTPStatusAction 200 OK + """ + name = 'http-status-action.doh.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN', use_edns=False) + query.id = 0 + + (_, receivedResponse) = self.sendDOHQuery(self._dohServerPort, self._serverName, self._dohBaseURL, query, caFile=self._caCert, useQueue=False, rawResponse=True) + self.assertTrue(receivedResponse) + self.assertEquals(receivedResponse, b'Plaintext answer') + self.assertEquals(self._rcode, 200) + self.assertTrue('content-type: text/plain' in self._response_headers.decode()) + + def testHTTPStatusAction307(self): + """ + DOH: HTTPStatusAction 307 + """ + name = 'http-status-action-redirect.doh.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN', use_edns=False) + query.id = 0 + + (_, receivedResponse) = self.sendDOHQuery(self._dohServerPort, self._serverName, self._dohBaseURL, query, caFile=self._caCert, useQueue=False, rawResponse=True) + self.assertTrue(receivedResponse) + self.assertEquals(self._rcode, 307) + self.assertTrue('location: https://doh.powerdns.org' in self._response_headers.decode()) + + def testHTTPLuaResponse(self): + """ + DOH: Lua HTTP Response + """ + name = 'http-lua.doh.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN', use_edns=False) + query.id = 0 + + (_, receivedResponse) = self.sendDOHPostQuery(self._dohServerPort, self._serverName, self._dohBaseURL, query, caFile=self._caCert, useQueue=False, rawResponse=True) + self.assertTrue(receivedResponse) + self.assertEquals(receivedResponse, b'It works!') + self.assertEquals(self._rcode, 200) + self.assertTrue('content-type: text/plain' in self._response_headers.decode()) + class TestDOHAddingECS(DNSDistDOHTest): _serverKey = 'server.key' -- 2.40.0