Skip to content

Commit

Permalink
dnsdist: Detect and dismiss truncated UDP responses from a backend
Browse files Browse the repository at this point in the history
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 17a0b06)
  • Loading branch information
rgacogne committed Dec 12, 2023
1 parent 53a3c7e commit 5def25e
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 3 deletions.
8 changes: 5 additions & 3 deletions pdns/dnsdist.cc
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,8 @@ void responderThread(std::shared_ptr<DownstreamState> 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<int> sockets;
sockets.reserve(dss->sockets.size());
Expand Down Expand Up @@ -746,14 +747,15 @@ void responderThread(std::shared_ptr<DownstreamState> 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<size_t>(got) < sizeof(dnsheader)) {
if (got < 0 || static_cast<size_t>(got) < sizeof(dnsheader) || static_cast<size_t>(got) == (initialBufferSize + 1)) {
continue;
}

Expand Down
43 changes: 43 additions & 0 deletions regression-tests.dnsdist/test_Advanced.py
Original file line number Diff line number Diff line change
Expand Up @@ -863,3 +863,46 @@ def testFlags(self):

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)

0 comments on commit 5def25e

Please sign in to comment.