dnsdist: Fix EDNS flags confusion when editing the OPT header

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.

diff --git a/pdns/dnsdistdist/dnsdist-ecs.cc b/pdns/dnsdistdist/dnsdist-ecs.cc
index c3fe0bccc2ce9fe9d7e8748b45882de32e8f490d..10ddf4a98850a58dd0fda4b9979b4534cff1277e 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-ecs.cc
+++ b/pdns/dnsdistdist/dnsdist-ecs.cc
@@ -242,7 +242,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);
-      packetWriter.addOpt(recordHeader.d_class, edns0.extRCode, edns0.extFlags, options, edns0.version);
+      packetWriter.addOpt(recordHeader.d_class, edns0.extRCode, ntohs(edns0.extFlags), options, edns0.version);
     }
   }
 

diff --git a/pdns/dnsdistdist/test-dnsdist_cc.cc b/pdns/dnsdistdist/test-dnsdist_cc.cc
index 10c67e5c1916afe2345cad60f3b06d73ac4d0127..0ea258fbc8f1ba438519aa6fa4f40f1bbeb9637e 100644 (file)
--- a/pdns/dnsdistdist/test-dnsdist_cc.cc
+++ b/pdns/dnsdistdist/test-dnsdist_cc.cc
@@ -2289,7 +2289,7 @@ BOOST_AUTO_TEST_CASE(test_setEDNSOption)
   BOOST_REQUIRE(getEDNS0Record(dnsQuestion.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(dnsQuestion));
   BOOST_REQUIRE(dnsQuestion.ednsOptions != nullptr);

diff --git a/regression-tests.dnsdist/dnsdisttests.py b/regression-tests.dnsdist/dnsdisttests.py
index 889d40a54fa54744588862ed544f5b2424b2660c..94648db48756c42284ce2e54ce3b09935d473b64 100644 (file)
--- a/regression-tests.dnsdist/dnsdisttests.py
+++ b/regression-tests.dnsdist/dnsdisttests.py
@@ -864,11 +864,13 @@ class DNSDistTest(AssertEqualDNSMessageMixin, unittest.TestCase):
     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:
@@ -881,6 +883,7 @@ class DNSDistTest(AssertEqualDNSMessageMixin, unittest.TestCase):
     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
@@ -896,6 +899,7 @@ class DNSDistTest(AssertEqualDNSMessageMixin, unittest.TestCase):
     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)

diff --git a/regression-tests.dnsdist/test_DOH.py b/regression-tests.dnsdist/test_DOH.py
index 562795402d40e5da7c70fa881fa54df0a05090eb..c9554846ea86f52ea6140720d7b03de1c5249f2f 100644 (file)
--- a/regression-tests.dnsdist/test_DOH.py
+++ b/regression-tests.dnsdist/test_DOH.py
@@ -184,6 +184,7 @@ class DOHTests(object):
         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,
@@ -899,9 +900,10 @@ class DOHAddingECSTests(object):
         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,

diff --git a/regression-tests.dnsdist/test_EDE.py b/regression-tests.dnsdist/test_EDE.py
index e45ded5e17509ed52350e1c3533dece5c2a8c38c..675f70c6bec5cec4700df2d97a2457ff388fbd9d 100644 (file)
--- a/regression-tests.dnsdist/test_EDE.py
+++ b/regression-tests.dnsdist/test_EDE.py
@@ -90,6 +90,47 @@ class TestBasics(DNSDistTest):
             (_, 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

diff --git a/regression-tests.dnsdist/test_EDNSSelfGenerated.py b/regression-tests.dnsdist/test_EDNSSelfGenerated.py
index f1ee0aec1ddcb87aed5a9b5208b149f7beff7407..6186841b52335e0290dcd20210a7434ea61e661c 100644 (file)
--- a/regression-tests.dnsdist/test_EDNSSelfGenerated.py
+++ b/regression-tests.dnsdist/test_EDNSSelfGenerated.py
@@ -145,6 +145,7 @@ class TestEDNSSelfGenerated(DNSDistTest):
         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"):
@@ -159,6 +160,7 @@ class TestEDNSSelfGenerated(DNSDistTest):
         # 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)
@@ -169,6 +171,7 @@ class TestEDNSSelfGenerated(DNSDistTest):
         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"):
@@ -183,6 +186,7 @@ class TestEDNSSelfGenerated(DNSDistTest):
         # 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,
@@ -206,6 +210,7 @@ class TestEDNSSelfGenerated(DNSDistTest):
         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)
@@ -219,6 +224,7 @@ class TestEDNSSelfGenerated(DNSDistTest):
         # 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)
@@ -229,6 +235,7 @@ class TestEDNSSelfGenerated(DNSDistTest):
         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"):
@@ -243,6 +250,7 @@ class TestEDNSSelfGenerated(DNSDistTest):
         # 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,