Skip to content

Commit

Permalink
dnsdist: Fix EDNS flags confusion when editing the OPT header
Browse files Browse the repository at this point in the history 
We used to wrongly reverse the byte-ordering of the existing EDNS
flags when editing the OPT header, for example when setting an
extended DNS error status.

(cherry picked from commit 010521a)
  • Loading branch information
@rgacogne
rgacogne committed Sep 9, 2024
1 parent eb1538c commit fe5d7fe
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 3 deletions.
2 changes: 1 addition & 1 deletion pdns/dnsdist-ecs.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ bool slowRewriteEDNSOptionInQueryWithRecords(const PacketBuffer& initialPacket,
/* addOrReplaceEDNSOption will set it to false if there is already an existing option */
optionAdded = true;
addOrReplaceEDNSOption(options, optionToReplace, optionAdded, overrideExisting, newOptionContent);
pw.addOpt(ah.d_class, edns0.extRCode, edns0.extFlags, options, edns0.version);
pw.addOpt(ah.d_class, edns0.extRCode, ntohs(edns0.extFlags), options, edns0.version);
}
}

Expand Down
2 changes: 1 addition & 1 deletion pdns/test-dnsdist_cc.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2261,7 +2261,7 @@ BOOST_AUTO_TEST_CASE(test_setEDNSOption)
BOOST_REQUIRE(getEDNS0Record(dq.getData(), edns0));
BOOST_CHECK_EQUAL(edns0.version, 0U);
BOOST_CHECK_EQUAL(edns0.extRCode, 0U);
BOOST_CHECK_EQUAL(edns0.extFlags, EDNS_HEADER_FLAG_DO);
BOOST_CHECK_EQUAL(ntohs(edns0.extFlags), EDNS_HEADER_FLAG_DO);

BOOST_REQUIRE(parseEDNSOptions(dq));
BOOST_REQUIRE(dq.ednsOptions != nullptr);
Expand Down
4 changes: 4 additions & 0 deletions regression-tests.dnsdist/dnsdisttests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -862,11 +862,13 @@ def checkMessageNoEDNS(self, expected, received):
def checkMessageEDNSWithoutOptions(self, expected, received):
self.assertEqual(expected, received)
self.assertEqual(received.edns, 0)
self.assertEqual(expected.ednsflags, received.ednsflags)
self.assertEqual(expected.payload, received.payload)

def checkMessageEDNSWithoutECS(self, expected, received, withCookies=0):
self.assertEqual(expected, received)
self.assertEqual(received.edns, 0)
self.assertEqual(expected.ednsflags, received.ednsflags)
self.assertEqual(expected.payload, received.payload)
self.assertEqual(len(received.options), withCookies)
if withCookies:
Expand All @@ -879,6 +881,7 @@ def checkMessageEDNSWithoutECS(self, expected, received, withCookies=0):
def checkMessageEDNSWithECS(self, expected, received, additionalOptions=0):
self.assertEqual(expected, received)
self.assertEqual(received.edns, 0)
self.assertEqual(expected.ednsflags, received.ednsflags)
self.assertEqual(expected.payload, received.payload)
self.assertEqual(len(received.options), 1 + additionalOptions)
hasECS = False
Expand All @@ -894,6 +897,7 @@ def checkMessageEDNSWithECS(self, expected, received, additionalOptions=0):
def checkMessageEDNS(self, expected, received):
self.assertEqual(expected, received)
self.assertEqual(received.edns, 0)
self.assertEqual(expected.ednsflags, received.ednsflags)
self.assertEqual(expected.payload, received.payload)
self.assertEqual(len(expected.options), len(received.options))
self.compareOptions(expected.options, received.options)
Expand Down
4 changes: 3 additions & 1 deletion regression-tests.dnsdist/test_DOH.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ def testDOHExistingECS(self):
query.id = 0
response = dns.message.make_response(query)
response.use_edns(edns=True, payload=4096, options=[rewrittenEcso])
response.want_dnssec(True)
rrset = dns.rrset.from_text(name,
3600,
dns.rdataclass.IN,
Expand Down Expand Up @@ -899,9 +900,10 @@ def testDOHExistingECS(self):
rewrittenEcso = clientsubnetoption.ClientSubnetOption('127.0.0.0', 24)
query = dns.message.make_query(name, 'A', 'IN', use_edns=True, payload=512, options=[ecso], want_dnssec=True)
query.id = 0
expectedQuery = dns.message.make_query(name, 'A', 'IN', use_edns=True, payload=512, options=[rewrittenEcso])
expectedQuery = dns.message.make_query(name, 'A', 'IN', use_edns=True, payload=512, options=[rewrittenEcso], want_dnssec=True)
response = dns.message.make_response(query)
response.use_edns(edns=True, payload=4096, options=[rewrittenEcso])
response.want_dnssec(True)
rrset = dns.rrset.from_text(name,
3600,
dns.rdataclass.IN,
Expand Down
41 changes: 41 additions & 0 deletions regression-tests.dnsdist/test_EDE.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,47 @@ def testExtendedErrorBackendResponse(self):
(_, receivedResponse) = sender(query, response=None, useQueue=False)
self.checkMessageEDNS(expectedResponse, receivedResponse)

def testExtendedErrorBackendResponse(self):
"""
EDE: Backend response (DO)
"""
name = 'backend-response-do.ede.tests.powerdns.com.'
ede = extendederrors.ExtendedErrorOption(16, b'my extended error status')
query = dns.message.make_query(name, 'A', 'IN', use_edns=True, want_dnssec=True)

backendResponse = dns.message.make_response(query)
backendResponse.use_edns(edns=True, payload=4096, options=[])
backendResponse.want_dnssec(True)
rrset = dns.rrset.from_text(name,
60,
dns.rdataclass.IN,
dns.rdatatype.A,
'127.0.0.1')

backendResponse.answer.append(rrset)
expectedResponse = dns.message.make_response(query)
expectedResponse.use_edns(edns=True, payload=4096, options=[ede])
expectedResponse.want_dnssec(True)
rrset = dns.rrset.from_text(name,
60,
dns.rdataclass.IN,
dns.rdatatype.A,
'127.0.0.1')
expectedResponse.answer.append(rrset)

for method in ("sendUDPQuery", "sendTCPQuery"):
sender = getattr(self, method)
(receivedQuery, receivedResponse) = sender(query, backendResponse)
receivedQuery.id = query.id
self.assertEqual(query, receivedQuery)
self.checkMessageEDNS(expectedResponse, receivedResponse)

# testing the cache
for method in ("sendUDPQuery", "sendTCPQuery"):
sender = getattr(self, method)
(_, receivedResponse) = sender(query, response=None, useQueue=False)
self.checkMessageEDNS(expectedResponse, receivedResponse)

def testExtendedErrorBackendResponseWithExistingEDE(self):
"""
EDE: Backend response with existing EDE
Expand Down
8 changes: 8 additions & 0 deletions regression-tests.dnsdist/test_EDNSSelfGenerated.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ def testWithEDNSWithDO(self):
query = dns.message.make_query(name, 'A', 'IN', use_edns=True, payload=4096, want_dnssec=True)
query.flags &= ~dns.flags.RD
expectedResponse = dns.message.make_response(query, our_payload=1042)
expectedResponse.want_dnssec(True)
expectedResponse.set_rcode(dns.rcode.REFUSED)

for method in ("sendUDPQuery", "sendTCPQuery"):
Expand All @@ -159,6 +160,7 @@ def testWithEDNSWithDO(self):
# dnsdist sets RA = RD for TC responses
query.flags &= ~dns.flags.RD
expectedResponse = dns.message.make_response(query, our_payload=1042)
expectedResponse.want_dnssec(True)
expectedResponse.flags |= dns.flags.TC

(_, receivedResponse) = self.sendUDPQuery(query, response=None, useQueue=False)
Expand All @@ -169,6 +171,7 @@ def testWithEDNSWithDO(self):
name = 'edns-do.lua.edns-self.tests.powerdns.com.'
query = dns.message.make_query(name, 'A', 'IN', use_edns=True, payload=4096, want_dnssec=True)
expectedResponse = dns.message.make_response(query, our_payload=1042)
expectedResponse.want_dnssec(True)
expectedResponse.set_rcode(dns.rcode.NXDOMAIN)

for method in ("sendUDPQuery", "sendTCPQuery"):
Expand All @@ -183,6 +186,7 @@ def testWithEDNSWithDO(self):
# dnsdist set RA = RD for spoofed responses
query.flags &= ~dns.flags.RD
expectedResponse = dns.message.make_response(query, our_payload=1042)
expectedResponse.want_dnssec(True)
expectedResponse.answer.append(dns.rrset.from_text(name,
60,
dns.rdataclass.IN,
Expand All @@ -206,6 +210,7 @@ def testWithEDNSNoOptions(self):
query.flags &= ~dns.flags.RD
expectedResponse = dns.message.make_response(query, our_payload=1042)
expectedResponse.set_rcode(dns.rcode.REFUSED)
expectedResponse.want_dnssec(True)

for method in ("sendUDPQuery", "sendTCPQuery"):
sender = getattr(self, method)
Expand All @@ -219,6 +224,7 @@ def testWithEDNSNoOptions(self):
# dnsdist sets RA = RD for TC responses
query.flags &= ~dns.flags.RD
expectedResponse = dns.message.make_response(query, our_payload=1042)
expectedResponse.want_dnssec(True)
expectedResponse.flags |= dns.flags.TC

(_, receivedResponse) = self.sendUDPQuery(query, response=None, useQueue=False)
Expand All @@ -229,6 +235,7 @@ def testWithEDNSNoOptions(self):
name = 'edns-options.lua.edns-self.tests.powerdns.com.'
query = dns.message.make_query(name, 'A', 'IN', use_edns=True, options=[ecso], payload=512, want_dnssec=True)
expectedResponse = dns.message.make_response(query, our_payload=1042)
expectedResponse.want_dnssec(True)
expectedResponse.set_rcode(dns.rcode.NXDOMAIN)

for method in ("sendUDPQuery", "sendTCPQuery"):
Expand All @@ -243,6 +250,7 @@ def testWithEDNSNoOptions(self):
# dnsdist set RA = RD for spoofed responses
query.flags &= ~dns.flags.RD
expectedResponse = dns.message.make_response(query, our_payload=1042)
expectedResponse.want_dnssec(True)
expectedResponse.answer.append(dns.rrset.from_text(name,
60,
dns.rdataclass.IN,
Expand Down

0 comments on commit fe5d7fe

Please sign in to comment.