From: Remi Gacogne Date: Thu, 3 Oct 2019 09:52:08 +0000 (+0200) Subject: dnsdist: Fix ECS addition in presence of answer or authority records X-Git-Tag: auth-4.3.0-beta2~36^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f8a01c441b4dda0288800ab6ed5e475111219e94;p=thirdparty%2Fpdns.git dnsdist: Fix ECS addition in presence of answer or authority records --- diff --git a/pdns/dnsdist-ecs.cc b/pdns/dnsdist-ecs.cc index 5b97f4d1e8..b1b43c111a 100644 --- a/pdns/dnsdist-ecs.cc +++ b/pdns/dnsdist-ecs.cc @@ -503,7 +503,7 @@ bool parseEDNSOptions(DNSQuestion& dq) dq.ednsOptions = std::make_shared >(); - if (ntohs(dq.dh->arcount) != 0 && ntohs(dq.dh->arcount) != 1) { + if (ntohs(dq.dh->ancount) != 0 || ntohs(dq.dh->nscount) != 0 || (ntohs(dq.dh->arcount) != 0 && ntohs(dq.dh->arcount) != 1)) { return slowParseEDNSOptions(reinterpret_cast(dq.dh), dq.len, dq.ednsOptions); } @@ -586,7 +586,7 @@ bool handleEDNSClientSubnet(char* const packet, const size_t packetSize, const u const struct dnsheader* dh = reinterpret_cast(packet); - if (ntohs(dh->arcount) != 0 && ntohs(dh->arcount) != 1) { + if (ntohs(dh->ancount) != 0 || ntohs(dh->nscount) != 0 || (ntohs(dh->arcount) != 0 && ntohs(dh->arcount) != 1)) { vector newContent; newContent.reserve(packetSize); diff --git a/pdns/test-dnsdist_cc.cc b/pdns/test-dnsdist_cc.cc index dfa035c4de..3a5bd23c80 100644 --- a/pdns/test-dnsdist_cc.cc +++ b/pdns/test-dnsdist_cc.cc @@ -42,15 +42,15 @@ BOOST_AUTO_TEST_SUITE(test_dnsdist_cc) static const uint16_t ECSSourcePrefixV4 = 24; static const uint16_t ECSSourcePrefixV6 = 56; -static void validateQuery(const char * packet, size_t packetSize, bool hasEdns=true, bool hasXPF=false, uint16_t additionals=0) +static void validateQuery(const char * packet, size_t packetSize, bool hasEdns=true, bool hasXPF=false, uint16_t additionals=0, uint16_t answers=0, uint16_t authorities=0) { MOADNSParser mdp(true, packet, packetSize); BOOST_CHECK_EQUAL(mdp.d_qname.toString(), "www.powerdns.com."); BOOST_CHECK_EQUAL(mdp.d_header.qdcount, 1U); - BOOST_CHECK_EQUAL(mdp.d_header.ancount, 0U); - BOOST_CHECK_EQUAL(mdp.d_header.nscount, 0U); + BOOST_CHECK_EQUAL(mdp.d_header.ancount, answers); + BOOST_CHECK_EQUAL(mdp.d_header.nscount, authorities); uint16_t expectedARCount = additionals + (hasEdns ? 1U : 0U) + (hasXPF ? 1U : 0U); BOOST_CHECK_EQUAL(mdp.d_header.arcount, expectedARCount); } @@ -692,6 +692,118 @@ BOOST_AUTO_TEST_CASE(replaceECSFollowedByTSIG) { validateQuery(reinterpret_cast(query.data()), len, true, false, 1); } +BOOST_AUTO_TEST_CASE(replaceECSAfterAN) { + bool ednsAdded = false; + bool ecsAdded = false; + ComboAddress remote("192.168.1.25"); + DNSName name("www.powerdns.com."); + ComboAddress origRemote("127.0.0.1"); + string newECSOption; + generateECSOption(remote, newECSOption, remote.sin4.sin_family == AF_INET ? ECSSourcePrefixV4 : ECSSourcePrefixV6); + + vector query; + DNSPacketWriter pw(query, name, QType::A, QClass::IN, 0); + pw.getHeader()->rd = 1; + pw.startRecord(DNSName("powerdns.com."), QType::A, 0, QClass::IN, DNSResourceRecord::ANSWER, true); + pw.commit(); + EDNSSubnetOpts ecsOpts; + ecsOpts.source = Netmask(origRemote, 8); + string origECSOption = makeEDNSSubnetOptsString(ecsOpts); + DNSPacketWriter::optvect_t opts; + opts.push_back(make_pair(EDNSOptionCode::ECS, origECSOption)); + pw.addOpt(512, 0, 0, opts); + pw.commit(); + uint16_t len = query.size(); + + /* large enough packet */ + char packet[1500]; + memcpy(packet, query.data(), query.size()); + + unsigned int consumed = 0; + uint16_t qtype; + DNSName qname(packet, len, sizeof(dnsheader), false, &qtype, NULL, &consumed); + BOOST_CHECK_EQUAL(qname, name); + BOOST_CHECK(qtype == QType::A); + + BOOST_CHECK(handleEDNSClientSubnet(packet, sizeof packet, consumed, &len, ednsAdded, ecsAdded, true, newECSOption, false)); + BOOST_CHECK((size_t) len > query.size()); + BOOST_CHECK_EQUAL(ednsAdded, false); + BOOST_CHECK_EQUAL(ecsAdded, false); + validateQuery(packet, len, true, false, 0, 1, 0); + validateECS(packet, len, remote); + + /* not large enough packet */ + ednsAdded = false; + ecsAdded = false; + consumed = 0; + len = query.size(); + qname = DNSName(reinterpret_cast(query.data()), len, sizeof(dnsheader), false, &qtype, NULL, &consumed); + BOOST_CHECK_EQUAL(qname, name); + BOOST_CHECK(qtype == QType::A); + + BOOST_CHECK(!handleEDNSClientSubnet(reinterpret_cast(query.data()), query.size(), consumed, &len, ednsAdded, ecsAdded, true, newECSOption, false)); + BOOST_CHECK_EQUAL((size_t) len, query.size()); + BOOST_CHECK_EQUAL(ednsAdded, false); + BOOST_CHECK_EQUAL(ecsAdded, false); + validateQuery(reinterpret_cast(query.data()), len, true, false, 0, 1, 0); +} + +BOOST_AUTO_TEST_CASE(replaceECSAfterAuth) { + bool ednsAdded = false; + bool ecsAdded = false; + ComboAddress remote("192.168.1.25"); + DNSName name("www.powerdns.com."); + ComboAddress origRemote("127.0.0.1"); + string newECSOption; + generateECSOption(remote, newECSOption, remote.sin4.sin_family == AF_INET ? ECSSourcePrefixV4 : ECSSourcePrefixV6); + + vector query; + DNSPacketWriter pw(query, name, QType::A, QClass::IN, 0); + pw.getHeader()->rd = 1; + pw.startRecord(DNSName("powerdns.com."), QType::A, 0, QClass::IN, DNSResourceRecord::AUTHORITY, true); + pw.commit(); + EDNSSubnetOpts ecsOpts; + ecsOpts.source = Netmask(origRemote, 8); + string origECSOption = makeEDNSSubnetOptsString(ecsOpts); + DNSPacketWriter::optvect_t opts; + opts.push_back(make_pair(EDNSOptionCode::ECS, origECSOption)); + pw.addOpt(512, 0, 0, opts); + pw.commit(); + uint16_t len = query.size(); + + /* large enough packet */ + char packet[1500]; + memcpy(packet, query.data(), query.size()); + + unsigned int consumed = 0; + uint16_t qtype; + DNSName qname(packet, len, sizeof(dnsheader), false, &qtype, NULL, &consumed); + BOOST_CHECK_EQUAL(qname, name); + BOOST_CHECK(qtype == QType::A); + + BOOST_CHECK(handleEDNSClientSubnet(packet, sizeof packet, consumed, &len, ednsAdded, ecsAdded, true, newECSOption, false)); + BOOST_CHECK((size_t) len > query.size()); + BOOST_CHECK_EQUAL(ednsAdded, false); + BOOST_CHECK_EQUAL(ecsAdded, false); + validateQuery(packet, len, true, false, 0, 0, 1); + validateECS(packet, len, remote); + + /* not large enough packet */ + ednsAdded = false; + ecsAdded = false; + consumed = 0; + len = query.size(); + qname = DNSName(reinterpret_cast(query.data()), len, sizeof(dnsheader), false, &qtype, NULL, &consumed); + BOOST_CHECK_EQUAL(qname, name); + BOOST_CHECK(qtype == QType::A); + + BOOST_CHECK(!handleEDNSClientSubnet(reinterpret_cast(query.data()), query.size(), consumed, &len, ednsAdded, ecsAdded, true, newECSOption, false)); + BOOST_CHECK_EQUAL((size_t) len, query.size()); + BOOST_CHECK_EQUAL(ednsAdded, false); + BOOST_CHECK_EQUAL(ecsAdded, false); + validateQuery(reinterpret_cast(query.data()), len, true, false, 0, 0, 1); +} + BOOST_AUTO_TEST_CASE(replaceECSBetweenTwoRecords) { bool ednsAdded = false; bool ecsAdded = false; @@ -851,6 +963,7 @@ BOOST_AUTO_TEST_CASE(insertECSAfterTSIG) { validateQuery(reinterpret_cast(query.data()), len, true, false); } + BOOST_AUTO_TEST_CASE(removeEDNSWhenFirst) { DNSName name("www.powerdns.com."); diff --git a/regression-tests.dnsdist/test_EdnsClientSubnet.py b/regression-tests.dnsdist/test_EdnsClientSubnet.py index b01e6bc38b..efa909df5c 100644 --- a/regression-tests.dnsdist/test_EdnsClientSubnet.py +++ b/regression-tests.dnsdist/test_EdnsClientSubnet.py @@ -531,6 +531,90 @@ class TestEdnsClientSubnetOverride(DNSDistTest): self.checkQueryEDNSWithECS(expectedQuery, receivedQuery, 2) self.checkResponseEDNSWithECS(expectedResponse, receivedResponse, 2) + def testWithAnswerThenECS(self): + """ + ECS: Record in answer followed by an existing EDNS with ECS + + Send a query with a record in the answer section, EDNS and an existing ECS value. + Check that the query received by the responder + has a valid ECS value and that the response + received from dnsdist contains an EDNS pseudo-RR. + """ + name = 'record-in-an-withecs.ecs.tests.powerdns.com.' + ecso = clientsubnetoption.ClientSubnetOption('192.0.2.1', 24) + eco = cookiesoption.CookiesOption(b'deadbeef', b'deadbeef') + rewrittenEcso = clientsubnetoption.ClientSubnetOption('127.0.0.1', 24) + rrset = dns.rrset.from_text(name, + 3600, + dns.rdataclass.IN, + dns.rdatatype.A, + '127.0.0.1') + + query = dns.message.make_query(name, 'A', 'IN', use_edns=True, payload=4096, options=[eco,ecso,eco]) + query.answer.append(rrset) + expectedQuery = dns.message.make_query(name, 'A', 'IN', use_edns=True, payload=4096, options=[eco,eco,rewrittenEcso]) + expectedQuery.answer.append(rrset) + + response = dns.message.make_response(expectedQuery) + response.use_edns(edns=True, payload=4096, options=[eco, ecso, eco]) + expectedResponse = dns.message.make_response(query) + expectedResponse.use_edns(edns=True, payload=4096, options=[eco, ecso, eco]) + response.answer.append(rrset) + response.additional.append(rrset) + expectedResponse.answer.append(rrset) + expectedResponse.additional.append(rrset) + + for method in ("sendUDPQuery", "sendTCPQuery"): + sender = getattr(self, method) + (receivedQuery, receivedResponse) = sender(query, response) + self.assertTrue(receivedQuery) + self.assertTrue(receivedResponse) + receivedQuery.id = expectedQuery.id + self.checkQueryEDNSWithECS(expectedQuery, receivedQuery, 2) + self.checkResponseEDNSWithECS(expectedResponse, receivedResponse, 2) + + def testWithAuthThenECS(self): + """ + ECS: Record in authority followed by an existing EDNS with ECS + + Send a query with a record in the authority section, EDNS and an existing ECS value. + Check that the query received by the responder + has a valid ECS value and that the response + received from dnsdist contains an EDNS pseudo-RR. + """ + name = 'record-in-an-withecs.ecs.tests.powerdns.com.' + ecso = clientsubnetoption.ClientSubnetOption('192.0.2.1', 24) + eco = cookiesoption.CookiesOption(b'deadbeef', b'deadbeef') + rewrittenEcso = clientsubnetoption.ClientSubnetOption('127.0.0.1', 24) + rrset = dns.rrset.from_text(name, + 3600, + dns.rdataclass.IN, + dns.rdatatype.A, + '127.0.0.1') + + query = dns.message.make_query(name, 'A', 'IN', use_edns=True, payload=4096, options=[eco,ecso,eco]) + query.authority.append(rrset) + expectedQuery = dns.message.make_query(name, 'A', 'IN', use_edns=True, payload=4096, options=[eco,eco,rewrittenEcso]) + expectedQuery.authority.append(rrset) + + response = dns.message.make_response(expectedQuery) + response.use_edns(edns=True, payload=4096, options=[eco, ecso, eco]) + expectedResponse = dns.message.make_response(query) + expectedResponse.use_edns(edns=True, payload=4096, options=[eco, ecso, eco]) + response.answer.append(rrset) + response.additional.append(rrset) + expectedResponse.answer.append(rrset) + expectedResponse.additional.append(rrset) + + for method in ("sendUDPQuery", "sendTCPQuery"): + sender = getattr(self, method) + (receivedQuery, receivedResponse) = sender(query, response) + self.assertTrue(receivedQuery) + self.assertTrue(receivedResponse) + receivedQuery.id = expectedQuery.id + self.checkQueryEDNSWithECS(expectedQuery, receivedQuery, 2) + self.checkResponseEDNSWithECS(expectedResponse, receivedResponse, 2) + def testWithEDNSNoECSFollowedByAnother(self): """ ECS: Existing EDNS without ECS, followed by another record