]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
dnsdist: Fix handling of XFR requests over DoH dnsdist-1.9.4
authorRemi Gacogne <remi.gacogne@powerdns.com>
Tue, 23 Apr 2024 13:51:35 +0000 (15:51 +0200)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Thu, 25 Apr 2024 13:10:20 +0000 (15:10 +0200)
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.

pdns/dnsdist.cc
pdns/dnsdistdist/dnsdist-nghttp2-in.cc
pdns/dnsdistdist/dnsdist-tcp-downstream.cc
regression-tests.dnsdist/quictests.py
regression-tests.dnsdist/test_DOH.py
regression-tests.dnsdist/test_DOH3.py
regression-tests.dnsdist/test_DOQ.py

index 0c99cf03308a53fe74aefd7ac6e4e410529a7795..9369a55b249bcbd11c79b5cf7241d4b5c7ab7b19 100644 (file)
@@ -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;
     }
index 5feb96b4d3e6a03f14d696888a73109c80936c98..e40f5e64b1725cc9fadde909934af3a5adc7d0d2 100644 (file)
@@ -222,8 +222,9 @@ void IncomingHTTP2Connection::handleResponse(const struct timeval& now, TCPRespo
 
 std::unique_ptr<DOHUnitInterface> 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<IncomingHTTP2Connection::StreamID>::max());
+  if (streamID > std::numeric_limits<IncomingHTTP2Connection::StreamID>::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<IncomingHTTP2Connection::StreamID>(streamID)));
   return std::make_unique<IncomingDoHCrossProtocolContext>(std::move(query), std::dynamic_pointer_cast<IncomingHTTP2Connection>(shared_from_this()), streamID);
@@ -551,8 +552,9 @@ void NGHTTP2Headers::addDynamicHeader(std::vector<nghttp2_nv>& 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);
index 904913e3eeda61b9c6aba7045edbe21a7819fbb8..dd50c1b7d11ab356a43ef08bb81bb6ebb37d9dc5 100644 (file)
@@ -678,6 +678,7 @@ IOState TCPConnectionToBackend::handleResponse(std::shared_ptr<TCPConnectionToBa
     /* we don't move the whole IDS because we will need for the responses to come */
     response.d_idstate.qtype = it->second.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 "<<response.d_idstate.qname);
 
     it->second.d_query.d_xfrStarted = true;
index 62cf24e757a47bfa63149d5c1352d0e6b9fa4a2f..d47baa75e8f49c66b363fd25de10722b1c043f5d 100644 (file)
@@ -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)
index f1d7912e31ffcfd279ed0c67af3f337223b0a4e9..562795402d40e5da7c70fa881fa54df0a05090eb 100644 (file)
@@ -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'
index 4704c26901e292afffb2fddb6536f03e8eb0bd8f..422096206a98982ffee10dcb79fb50895cbad5d9 100644 (file)
@@ -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)
index 9af5d8a9387bfceebd462feeea7a1ad59fdb599b..033bdc7e8240ad6a74cade4f0706271bc9d77c70 100644 (file)
@@ -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')