From: Remi Gacogne Date: Tue, 23 Apr 2024 13:51:35 +0000 (+0200) Subject: dnsdist: Fix handling of XFR requests over DoH X-Git-Tag: dnsdist-1.9.4^0 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4dc07fc55e007b330aa4c7a3271cb065e526363b;p=thirdparty%2Fpdns.git dnsdist: Fix handling of XFR requests over DoH We did not properly handle incoming XFR requests received over DoH When a TCP-only or DoT backend was configured, and the nghttp2 provider used. This commit fixes the assertion failure and makes sure that XFR requests are denied with `NOTIMP` when received over DNS over HTTPS, including DNS over HTTP/3. It also denies them when received over DNS over QUIC as this is not properly handled at the moment, although it does not cause a crash. --- diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index 0c99cf0330..9369a55b24 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -1670,9 +1670,18 @@ ProcessQueryResult processQuery(DNSQuestion& dq, LocalHolders& holders, std::sha /* we need an accurate ("real") value for the response and to store into the IDS, but not for insertion into the rings for example */ - struct timespec now; + timespec now{}; gettime(&now); + if ((dq.ids.qtype == QType::AXFR || dq.ids.qtype == QType::IXFR) && (dq.getProtocol() == dnsdist::Protocol::DoH || dq.getProtocol() == dnsdist::Protocol::DoQ || dq.getProtocol() == dnsdist::Protocol::DoH3)) { + dq.editHeader([](dnsheader& header) { + header.rcode = RCode::NotImp; + header.qr = true; + return true; + }); + return processQueryAfterRules(dq, holders, selectedBackend); + } + if (!applyRulesToQuery(holders, dq, now)) { return ProcessQueryResult::Drop; } diff --git a/pdns/dnsdistdist/dnsdist-nghttp2-in.cc b/pdns/dnsdistdist/dnsdist-nghttp2-in.cc index 5feb96b4d3..e40f5e64b1 100644 --- a/pdns/dnsdistdist/dnsdist-nghttp2-in.cc +++ b/pdns/dnsdistdist/dnsdist-nghttp2-in.cc @@ -222,8 +222,9 @@ void IncomingHTTP2Connection::handleResponse(const struct timeval& now, TCPRespo std::unique_ptr IncomingHTTP2Connection::getDOHUnit(uint32_t streamID) { - // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay): clang-tidy is getting confused by assert() - assert(streamID <= std::numeric_limits::max()); + if (streamID > std::numeric_limits::max()) { + throw std::runtime_error("Invalid stream ID while retrieving DoH unit"); + } // NOLINTNEXTLINE(*-narrowing-conversions): generic interface between DNS and DoH with different types auto query = std::move(d_currentStreams.at(static_cast(streamID))); return std::make_unique(std::move(query), std::dynamic_pointer_cast(shared_from_this()), streamID); @@ -551,8 +552,9 @@ void NGHTTP2Headers::addDynamicHeader(std::vector& headers, NGHTTP2H IOState IncomingHTTP2Connection::sendResponse(const struct timeval& now, TCPResponse&& response) { - // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay): clang-tidy is getting confused by assert() - assert(response.d_idstate.d_streamID != -1); + if (response.d_idstate.d_streamID == -1) { + throw std::runtime_error("Invalid DoH stream ID while sending response"); + } auto& context = d_currentStreams.at(response.d_idstate.d_streamID); uint32_t statusCode = 200U; @@ -583,8 +585,10 @@ void IncomingHTTP2Connection::notifyIOError(const struct timeval& now, TCPRespon return; } - // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay): clang-tidy is getting confused by assert() - assert(response.d_idstate.d_streamID != -1); + if (response.d_idstate.d_streamID == -1) { + throw std::runtime_error("Invalid DoH stream ID while handling I/O error notification"); + } + auto& context = d_currentStreams.at(response.d_idstate.d_streamID); context.d_buffer = std::move(response.d_buffer); sendResponse(response.d_idstate.d_streamID, context, 502, d_ci.cs->dohFrontend->d_customResponseHeaders); diff --git a/pdns/dnsdistdist/dnsdist-tcp-downstream.cc b/pdns/dnsdistdist/dnsdist-tcp-downstream.cc index 904913e3ee..dd50c1b7d1 100644 --- a/pdns/dnsdistdist/dnsdist-tcp-downstream.cc +++ b/pdns/dnsdistdist/dnsdist-tcp-downstream.cc @@ -678,6 +678,7 @@ IOState TCPConnectionToBackend::handleResponse(std::shared_ptrsecond.d_query.d_idstate.qtype; response.d_idstate.qname = it->second.d_query.d_idstate.qname; + response.d_idstate.d_streamID = it->second.d_query.d_idstate.d_streamID; DEBUGLOG("passing XFRresponse to client connection for "<second.d_query.d_xfrStarted = true; diff --git a/regression-tests.dnsdist/quictests.py b/regression-tests.dnsdist/quictests.py index 62cf24e757..d47baa75e8 100644 --- a/regression-tests.dnsdist/quictests.py +++ b/regression-tests.dnsdist/quictests.py @@ -169,3 +169,18 @@ class QUICWithCacheTests(object): total += self._responsesCounter[key] self.assertEqual(total, 1) + +class QUICXFRTests(object): + + def testXFR(self): + """ + QUIC: XFR + """ + name = 'xfr.doq.tests.powerdns.com.' + for xfrType in [dns.rdatatype.AXFR, dns.rdatatype.IXFR]: + query = dns.message.make_query(name, xfrType, 'IN') + expectedResponse = dns.message.make_response(query) + expectedResponse.set_rcode(dns.rcode.NOTIMP) + + (_, receivedResponse) = self.sendQUICQuery(query, response=None, useQueue=False) + self.assertEqual(receivedResponse, expectedResponse) diff --git a/regression-tests.dnsdist/test_DOH.py b/regression-tests.dnsdist/test_DOH.py index f1d7912e31..562795402d 100644 --- a/regression-tests.dnsdist/test_DOH.py +++ b/regression-tests.dnsdist/test_DOH.py @@ -1712,3 +1712,39 @@ class TestDOHLimitsNGHTTP2(DOHLimits, DNSDistDOHTest): class TestDOHLimitsH2O(DOHLimits, DNSDistDOHTest): _dohLibrary = 'h2o' + +class DOHXFR(object): + _serverName = 'tls.tests.dnsdist.org' + _caCert = 'ca.pem' + _dohServerPort = pickAvailablePort() + _dohBaseURL = ("https://%s:%d/" % (_serverName, _dohServerPort)) + _serverKey = 'server.key' + _serverCert = 'server.chain' + _maxTCPConnsPerClient = 3 + _config_template = """ + newServer{address="127.0.0.1:%d", tcpOnly=true} + addDOHLocal("127.0.0.1:%d", "%s", "%s", { "/" }, {library='%s'}) + """ + _config_params = ['_testServerPort', '_dohServerPort', '_serverCert', '_serverKey', '_dohLibrary'] + + def testXFR(self): + """ + DoH XFR: Check that XFR requests over DoH are refused with NotImp + """ + name = 'xfr.doh.tests.powerdns.com.' + for xfrType in [dns.rdatatype.AXFR, dns.rdatatype.IXFR]: + query = dns.message.make_query(name, xfrType, 'IN') + url = self.getDOHGetURL(self._dohBaseURL, query) + + expectedResponse = dns.message.make_response(query) + expectedResponse.set_rcode(dns.rcode.NOTIMP) + + (_, receivedResponse) = self.sendDOHQuery(self._dohServerPort, self._serverName, self._dohBaseURL, query, caFile=self._caCert, useQueue=False) + + self.assertEqual(receivedResponse, expectedResponse) + +class TestDOHXFRNGHTTP2(DOHXFR, DNSDistDOHTest): + _dohLibrary = 'nghttp2' + +class TestDOHXFRH2O(DOHXFR, DNSDistDOHTest): + _dohLibrary = 'h2o' diff --git a/regression-tests.dnsdist/test_DOH3.py b/regression-tests.dnsdist/test_DOH3.py index 4704c26901..422096206a 100644 --- a/regression-tests.dnsdist/test_DOH3.py +++ b/regression-tests.dnsdist/test_DOH3.py @@ -4,7 +4,7 @@ import clientsubnetoption from dnsdisttests import DNSDistTest from dnsdisttests import pickAvailablePort -from quictests import QUICTests, QUICWithCacheTests, QUICACLTests +from quictests import QUICTests, QUICWithCacheTests, QUICACLTests, QUICXFRTests import doh3client class TestDOH3(QUICTests, DNSDistTest): @@ -92,3 +92,24 @@ class TestDOH3Specifics(DNSDistTest): receivedQuery.id = expectedQuery.id self.assertEqual(expectedQuery, receivedQuery) self.assertEqual(receivedResponse, response) + +class TestDOH3XFR(QUICXFRTests, DNSDistTest): + _serverKey = 'server.key' + _serverCert = 'server.chain' + _serverName = 'tls.tests.dnsdist.org' + _caCert = 'ca.pem' + _doqServerPort = pickAvailablePort() + _dohBaseURL = ("https://%s:%d/" % (_serverName, _doqServerPort)) + _config_template = """ + newServer{address="127.0.0.1:%d", tcpOnly=true} + + addDOH3Local("127.0.0.1:%d", "%s", "%s") + """ + _config_params = ['_testServerPort', '_doqServerPort','_serverCert', '_serverKey'] + _verboseMode = True + + def getQUICConnection(self): + return self.getDOQConnection(self._doqServerPort, self._caCert) + + def sendQUICQuery(self, query, response=None, useQueue=True, connection=None): + return self.sendDOH3Query(self._doqServerPort, self._dohBaseURL, query, response=response, caFile=self._caCert, useQueue=useQueue, serverName=self._serverName, connection=connection) diff --git a/regression-tests.dnsdist/test_DOQ.py b/regression-tests.dnsdist/test_DOQ.py index 9af5d8a938..033bdc7e82 100644 --- a/regression-tests.dnsdist/test_DOQ.py +++ b/regression-tests.dnsdist/test_DOQ.py @@ -6,7 +6,7 @@ import clientsubnetoption from dnsdisttests import DNSDistTest from dnsdisttests import pickAvailablePort from doqclient import quic_bogus_query -from quictests import QUICTests, QUICWithCacheTests, QUICACLTests +from quictests import QUICTests, QUICWithCacheTests, QUICACLTests, QUICXFRTests import doqclient from doqclient import quic_query @@ -105,6 +105,26 @@ class TestDOQWithACL(QUICACLTests, DNSDistTest): def sendQUICQuery(self, query, response=None, useQueue=True, connection=None): return self.sendDOQQuery(self._doqServerPort, query, response=response, caFile=self._caCert, useQueue=useQueue, serverName=self._serverName, connection=connection) +class TestDOQXFR(QUICXFRTests, DNSDistTest): + _serverKey = 'server.key' + _serverCert = 'server.chain' + _serverName = 'tls.tests.dnsdist.org' + _caCert = 'ca.pem' + _doqServerPort = pickAvailablePort() + _config_template = """ + newServer{address="127.0.0.1:%d", tcpOnly=True} + + addDOQLocal("127.0.0.1:%d", "%s", "%s") + """ + _config_params = ['_testServerPort', '_doqServerPort','_serverCert', '_serverKey'] + _verboseMode = True + + def getQUICConnection(self): + return self.getDOQConnection(self._doqServerPort, self._caCert) + + def sendQUICQuery(self, query, response=None, useQueue=True, connection=None): + return self.sendDOQQuery(self._doqServerPort, query, response=response, caFile=self._caCert, useQueue=useQueue, serverName=self._serverName, connection=connection) + class TestDOQCertificateReloading(DNSDistTest): _consoleKey = DNSDistTest.generateConsoleKey() _consoleKeyB64 = base64.b64encode(_consoleKey).decode('ascii')