From 5def25ef3ec3f6feb58eb4377cea95ee50d2a088 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Fri, 24 Nov 2023 15:30:09 +0100 Subject: [PATCH] dnsdist: Detect and dismiss truncated UDP responses from a backend Until now we would not have detected if the response was larger than our buffer (4096 bytes or larger in some cases), which could have led to parsing errors or even forwarding a corrupted response. (cherry picked from commit 17a0b06ee3cfa43bb9ac4a0cd5186fbc42e090a5) --- pdns/dnsdist.cc | 8 +++-- regression-tests.dnsdist/test_Advanced.py | 43 +++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index b6171aff7b..d4dfbff67b 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -717,7 +717,8 @@ void responderThread(std::shared_ptr dss) auto localRespRuleActions = g_respruleactions.getLocal(); auto localCacheInsertedRespRuleActions = g_cacheInsertedRespRuleActions.getLocal(); const size_t initialBufferSize = getInitialUDPPacketBufferSize(); - PacketBuffer response(initialBufferSize); + /* allocate one more byte so we can detect truncation */ + PacketBuffer response(initialBufferSize + 1); uint16_t queryId = 0; std::vector sockets; sockets.reserve(dss->sockets.size()); @@ -746,14 +747,15 @@ void responderThread(std::shared_ptr dss) } for (const auto& fd : sockets) { - response.resize(initialBufferSize); + /* allocate one more byte so we can detect truncation */ + response.resize(initialBufferSize + 1); ssize_t got = recv(fd, response.data(), response.size(), 0); if (got == 0 && dss->isStopped()) { break; } - if (got < 0 || static_cast(got) < sizeof(dnsheader)) { + if (got < 0 || static_cast(got) < sizeof(dnsheader) || static_cast(got) == (initialBufferSize + 1)) { continue; } diff --git a/regression-tests.dnsdist/test_Advanced.py b/regression-tests.dnsdist/test_Advanced.py index f8a0be04a5..9ee2b7596a 100644 --- a/regression-tests.dnsdist/test_Advanced.py +++ b/regression-tests.dnsdist/test_Advanced.py @@ -863,3 +863,46 @@ class TestFlagsOnTimeout(DNSDistTest): self.assertEqual(len(timeouts), 2) self.assertEqual(len(queries), 2) + +class TestTruncatedUDPLargeAnswers(DNSDistTest): + _config_template = """ + newServer{address="127.0.0.1:%d"} + """ + def testVeryLargeAnswer(self): + """ + Advanced: Check that UDP responses that are too large for our buffer are dismissed + """ + name = 'very-large-answer-dismissed.advanced.tests.powerdns.com.' + query = dns.message.make_query(name, 'TXT', 'IN') + response = dns.message.make_response(query) + # we prepare a large answer + content = '' + for i in range(31): + if len(content) > 0: + content = content + ' ' + content = content + 'A' * 255 + # pad up to 8192 + content = content + ' ' + 'B' * 170 + + rrset = dns.rrset.from_text(name, + 3600, + dns.rdataclass.IN, + dns.rdatatype.TXT, + content) + response.answer.append(rrset) + self.assertEqual(len(response.to_wire()), 8192) + + # TCP should be OK + (receivedQuery, receivedResponse) = self.sendTCPQuery(query, response) + self.assertTrue(receivedQuery) + self.assertTrue(receivedResponse) + receivedQuery.id = query.id + self.assertEqual(query, receivedQuery) + self.assertEqual(receivedResponse, response) + + # UDP should never get an answer, because dnsdist will not be able to get it from the backend + (receivedQuery, receivedResponse) = self.sendUDPQuery(query, response) + self.assertTrue(receivedQuery) + self.assertFalse(receivedResponse) + receivedQuery.id = query.id + self.assertEqual(query, receivedQuery) -- 2.47.2