From: Remi Gacogne Date: Tue, 23 Apr 2024 13:28:14 +0000 (+0200) Subject: dnsdist: Fix handling of XFR requests over DoH X-Git-Tag: rec-5.1.0-alpha1~2^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F14156%2Fhead;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 commits 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/dnsdistdist/dnsdist-idstate.cc b/pdns/dnsdistdist/dnsdist-idstate.cc index 395ce3ccba..3217720a58 100644 --- a/pdns/dnsdistdist/dnsdist-idstate.cc +++ b/pdns/dnsdistdist/dnsdist-idstate.cc @@ -50,5 +50,7 @@ InternalQueryState InternalQueryState::partialCloneForXFR() const ids.d_protoBufData = std::make_unique(*d_protoBufData); } ids.cs = cs; + /* in case we want to support XFR over DoH, or the stream ID becomes used for QUIC */ + ids.d_streamID = d_streamID; return ids; } diff --git a/pdns/dnsdistdist/dnsdist-nghttp2-in.cc b/pdns/dnsdistdist/dnsdist-nghttp2-in.cc index 0e4e602a36..797def327e 100644 --- a/pdns/dnsdistdist/dnsdist-nghttp2-in.cc +++ b/pdns/dnsdistdist/dnsdist-nghttp2-in.cc @@ -222,8 +222,10 @@ 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 +553,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; @@ -584,8 +587,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.cc b/pdns/dnsdistdist/dnsdist.cc index 954450202f..d411b0b66c 100644 --- a/pdns/dnsdistdist/dnsdist.cc +++ b/pdns/dnsdistdist/dnsdist.cc @@ -1685,6 +1685,15 @@ ProcessQueryResult processQuery(DNSQuestion& dnsQuestion, LocalHolders& holders, timespec now{}; gettime(&now); + if ((dnsQuestion.ids.qtype == QType::AXFR || dnsQuestion.ids.qtype == QType::IXFR) && (dnsQuestion.getProtocol() == dnsdist::Protocol::DoH || dnsQuestion.getProtocol() == dnsdist::Protocol::DoQ || dnsQuestion.getProtocol() == dnsdist::Protocol::DoH3)) { + dnsQuestion.editHeader([](dnsheader& header) { + header.rcode = RCode::NotImp; + header.qr = true; + return true; + }); + return processQueryAfterRules(dnsQuestion, holders, selectedBackend); + } + if (!applyRulesToQuery(holders, dnsQuestion, now)) { return ProcessQueryResult::Drop; } diff --git a/regression-tests.dnsdist/quictests.py b/regression-tests.dnsdist/quictests.py index 743de28db5..b4ec180722 100644 --- a/regression-tests.dnsdist/quictests.py +++ b/regression-tests.dnsdist/quictests.py @@ -191,3 +191,18 @@ class QUICGetLocalAddressOnAnyBindTests(object): (_, receivedResponse) = self.sendQUICQuery(query, response=None, useQueue=False) self.assertEqual(receivedResponse, response) + +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 4a91c433f2..9634c914c7 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, QUICGetLocalAddressOnAnyBindTests +from quictests import QUICTests, QUICWithCacheTests, QUICACLTests, QUICGetLocalAddressOnAnyBindTests, QUICXFRTests import doh3client class TestDOH3(QUICTests, DNSDistTest): @@ -122,3 +122,24 @@ class TestDOH3GetLocalAddressOnAnyBind(QUICGetLocalAddressOnAnyBindTests, DNSDis 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) + +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 657df001a9..5a817747e0 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, QUICGetLocalAddressOnAnyBindTests +from quictests import QUICTests, QUICWithCacheTests, QUICACLTests, QUICGetLocalAddressOnAnyBindTests, 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')