diff --git a/pdns/dnsdist-ecs.cc b/pdns/dnsdist-ecs.cc index cf8b73e8a6a8..b94b0eeeed1d 100644 --- a/pdns/dnsdist-ecs.cc +++ b/pdns/dnsdist-ecs.cc @@ -559,23 +559,23 @@ int rewriteResponseWithoutEDNSOption(const std::string& initialPacket, const uin return 0; } -bool addEDNS(DNSQuestion& dq, bool dnssecOK) +bool addEDNS(dnsheader* dh, uint16_t& len, const size_t size, bool dnssecOK, uint16_t payloadSize) { - if (dq.dh->arcount != 0) { + if (dh->arcount != 0) { return false; } std::string optRecord; - generateOptRR(std::string(), optRecord, g_PayloadSizeSelfGenAnswers, dnssecOK); + generateOptRR(std::string(), optRecord, payloadSize, dnssecOK); - if (optRecord.size() >= dq.size || (dq.size - optRecord.size()) < dq.len) { + if (optRecord.size() >= size || (size - optRecord.size()) < len) { return false; } - char * optPtr = reinterpret_cast(dq.dh) + dq.len; + char * optPtr = reinterpret_cast(dh) + len; memcpy(optPtr, optRecord.data(), optRecord.size()); - dq.len += optRecord.size(); - dq.dh->arcount = htons(1); + len += optRecord.size(); + dh->arcount = htons(1); return true; } @@ -611,7 +611,7 @@ bool addEDNSToQueryTurnedResponse(DNSQuestion& dq) if (g_addEDNSToSelfGeneratedResponses) { /* now we need to add a new OPT record */ - return addEDNS(dq, dnssecOK); + return addEDNS(dq.dh, dq.len, dq.size, dnssecOK, g_PayloadSizeSelfGenAnswers); } /* otherwise we are just fine */ diff --git a/pdns/dnsdist-ecs.hh b/pdns/dnsdist-ecs.hh index e0b92a42c625..c9193f61d65c 100644 --- a/pdns/dnsdist-ecs.hh +++ b/pdns/dnsdist-ecs.hh @@ -32,7 +32,7 @@ int removeEDNSOptionFromOPT(char* optStart, size_t* optLen, const uint16_t optio int rewriteResponseWithoutEDNSOption(const std::string& initialPacket, const uint16_t optionCodeToSkip, vector& newContent); int getEDNSOptionsStart(char* packet, const size_t offset, const size_t len, char ** optRDLen, size_t * remaining); bool isEDNSOptionInOpt(const std::string& packet, const size_t optStart, const size_t optLen, const uint16_t optionCodeToFind); -bool addEDNS(DNSQuestion& dq, bool dnssecOK); +bool addEDNS(dnsheader* dh, uint16_t& len, const size_t size, bool dnssecOK, uint16_t payloadSize); bool addEDNSToQueryTurnedResponse(DNSQuestion& dq); int getEDNSZ(const DNSQuestion& dq); diff --git a/pdns/dnsdist-lua-actions.cc b/pdns/dnsdist-lua-actions.cc index 7724fda54c10..a9f64a40f333 100644 --- a/pdns/dnsdist-lua-actions.cc +++ b/pdns/dnsdist-lua-actions.cc @@ -458,7 +458,7 @@ DNSAction::Action SpoofAction::operator()(DNSQuestion* dq, string* ruleresult) c dq->dh->ancount = htons(dq->dh->ancount); if (hadEDNS && g_addEDNSToSelfGeneratedResponses) { - addEDNS(*dq, dnssecOK); + addEDNS(dq->dh, dq->len, dq->size, dnssecOK, g_PayloadSizeSelfGenAnswers); } return Action::HeaderModify; diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index 8abb56e41bf3..39018f851b42 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -52,6 +52,7 @@ #include "delaypipe.hh" #include "dolog.hh" #include "dnsname.hh" +#include "dnsparser.hh" #include "dnswriter.hh" #include "ednsoptions.hh" #include "gettime.hh" @@ -142,12 +143,24 @@ bool g_servFailOnNoPolicy{false}; bool g_truncateTC{false}; bool g_fixupCase{0}; -static void truncateTC(char* packet, uint16_t* len, unsigned int consumed) +static void truncateTC(char* packet, uint16_t* len, size_t responseSize, unsigned int consumed) try { + bool hadEDNS = false; + uint16_t payloadSize = 0; + uint16_t z = 0; + + if (g_addEDNSToSelfGeneratedResponses) { + hadEDNS = getEDNSUDPPayloadSizeAndZ(packet, *len, &payloadSize, &z); + } + *len=(uint16_t) (sizeof(dnsheader)+consumed+DNS_TYPE_SIZE+DNS_CLASS_SIZE); struct dnsheader* dh = reinterpret_cast(packet); dh->ancount = dh->arcount = dh->nscount = 0; + + if (hadEDNS) { + addEDNS(dh, *len, responseSize, z & EDNS_HEADER_FLAG_DO, payloadSize); + } } catch(...) { @@ -475,7 +488,7 @@ try { } if(dh->tc && g_truncateTC) { - truncateTC(response, &responseLen, consumed); + truncateTC(response, &responseLen, responseSize, consumed); } dh->id = ids->origID; diff --git a/pdns/dnsparser.cc b/pdns/dnsparser.cc index 2fc13ac381d4..e26bdf7089b3 100644 --- a/pdns/dnsparser.cc +++ b/pdns/dnsparser.cc @@ -934,12 +934,15 @@ uint16_t getRecordsOfTypeCount(const char* packet, size_t length, uint8_t sectio return result; } -uint16_t getEDNSUDPPayloadSize(const char* packet, size_t length) +bool getEDNSUDPPayloadSizeAndZ(const char* packet, size_t length, uint16_t* payloadSize, uint16_t* z) { if (length < sizeof(dnsheader)) { - return 0; + return false; } + *payloadSize = 0; + *z = 0; + try { const dnsheader* dh = (const dnsheader*) packet; @@ -958,7 +961,11 @@ uint16_t getEDNSUDPPayloadSize(const char* packet, size_t length) const uint16_t dnsclass = dpm.get16BitInt(); if(dnstype == QType::OPT) { - return dnsclass; + /* skip extended rcode and version */ + dpm.skipBytes(2); + *z = dpm.get16BitInt(); + *payloadSize = dnsclass; + return true; } /* TTL */ @@ -969,5 +976,6 @@ uint16_t getEDNSUDPPayloadSize(const char* packet, size_t length) catch(...) { } - return 0; + + return false; } diff --git a/pdns/dnsparser.hh b/pdns/dnsparser.hh index b0182ebb5eda..3cc710f438c7 100644 --- a/pdns/dnsparser.hh +++ b/pdns/dnsparser.hh @@ -401,7 +401,7 @@ void editDNSPacketTTL(char* packet, size_t length, std::function std::shared_ptr getRR(const DNSRecord& dr) diff --git a/pdns/test-dnsdist_cc.cc b/pdns/test-dnsdist_cc.cc index 9bd45c2e5e03..0f8901c56d88 100644 --- a/pdns/test-dnsdist_cc.cc +++ b/pdns/test-dnsdist_cc.cc @@ -780,6 +780,8 @@ static int getZ(const DNSName& qname, const uint16_t qtype, const uint16_t qclas BOOST_AUTO_TEST_CASE(test_getEDNSZ) { + uint16_t z; + uint16_t udpPayloadSize; DNSName qname("www.powerdns.com."); uint16_t qtype = QType::A; uint16_t qclass = QClass::IN; @@ -801,7 +803,9 @@ BOOST_AUTO_TEST_CASE(test_getEDNSZ) { pw.commit(); BOOST_CHECK_EQUAL(getZ(qname, qtype, qclass, query), 0); - BOOST_CHECK_EQUAL(getEDNSUDPPayloadSize(reinterpret_cast(query.data()), query.size()), 0); + BOOST_CHECK_EQUAL(getEDNSUDPPayloadSizeAndZ(reinterpret_cast(query.data()), query.size(), &udpPayloadSize, &z), false); + BOOST_CHECK_EQUAL(z, 0); + BOOST_CHECK_EQUAL(udpPayloadSize, 0); } { @@ -813,7 +817,9 @@ BOOST_AUTO_TEST_CASE(test_getEDNSZ) { query.resize(query.size() - (/* RDLEN */ sizeof(uint16_t) + /* last byte of TTL / Z */ 1)); BOOST_CHECK_EQUAL(getZ(qname, qtype, qclass, query), 0); - BOOST_CHECK_EQUAL(getEDNSUDPPayloadSize(reinterpret_cast(query.data()), query.size()), 512); + BOOST_CHECK_EQUAL(getEDNSUDPPayloadSizeAndZ(reinterpret_cast(query.data()), query.size(), &udpPayloadSize, &z), false); + BOOST_CHECK_EQUAL(z, 0); + BOOST_CHECK_EQUAL(udpPayloadSize, 0); } { @@ -824,7 +830,9 @@ BOOST_AUTO_TEST_CASE(test_getEDNSZ) { pw.commit(); BOOST_CHECK_EQUAL(getZ(qname, qtype, qclass, query), 0); - BOOST_CHECK_EQUAL(getEDNSUDPPayloadSize(reinterpret_cast(query.data()), query.size()), 512); + BOOST_CHECK_EQUAL(getEDNSUDPPayloadSizeAndZ(reinterpret_cast(query.data()), query.size(), &udpPayloadSize, &z), true); + BOOST_CHECK_EQUAL(z, 0); + BOOST_CHECK_EQUAL(udpPayloadSize, 512); } { @@ -835,7 +843,9 @@ BOOST_AUTO_TEST_CASE(test_getEDNSZ) { pw.commit(); BOOST_CHECK_EQUAL(getZ(qname, qtype, qclass, query), EDNS_HEADER_FLAG_DO); - BOOST_CHECK_EQUAL(getEDNSUDPPayloadSize(reinterpret_cast(query.data()), query.size()), 512); + BOOST_CHECK_EQUAL(getEDNSUDPPayloadSizeAndZ(reinterpret_cast(query.data()), query.size(), &udpPayloadSize, &z), true); + BOOST_CHECK_EQUAL(z, EDNS_HEADER_FLAG_DO); + BOOST_CHECK_EQUAL(udpPayloadSize, 512); } { @@ -846,7 +856,9 @@ BOOST_AUTO_TEST_CASE(test_getEDNSZ) { pw.commit(); BOOST_CHECK_EQUAL(getZ(qname, qtype, qclass, query), 0); - BOOST_CHECK_EQUAL(getEDNSUDPPayloadSize(reinterpret_cast(query.data()), query.size()), 512); + BOOST_CHECK_EQUAL(getEDNSUDPPayloadSizeAndZ(reinterpret_cast(query.data()), query.size(), &udpPayloadSize, &z), true); + BOOST_CHECK_EQUAL(z, 0); + BOOST_CHECK_EQUAL(udpPayloadSize, 512); } { @@ -857,13 +869,17 @@ BOOST_AUTO_TEST_CASE(test_getEDNSZ) { pw.commit(); BOOST_CHECK_EQUAL(getZ(qname, qtype, qclass, query), EDNS_HEADER_FLAG_DO); - BOOST_CHECK_EQUAL(getEDNSUDPPayloadSize(reinterpret_cast(query.data()), query.size()), 512); + BOOST_CHECK_EQUAL(getEDNSUDPPayloadSizeAndZ(reinterpret_cast(query.data()), query.size(), &udpPayloadSize, &z), true); + BOOST_CHECK_EQUAL(z, EDNS_HEADER_FLAG_DO); + BOOST_CHECK_EQUAL(udpPayloadSize, 512); } } BOOST_AUTO_TEST_CASE(test_addEDNSToQueryTurnedResponse) { + uint16_t z; + uint16_t udpPayloadSize; DNSName qname("www.powerdns.com."); uint16_t qtype = QType::A; uint16_t qclass = QClass::IN; @@ -892,7 +908,9 @@ BOOST_AUTO_TEST_CASE(test_addEDNSToQueryTurnedResponse) { auto dq = turnIntoResponse(qname, qtype, qclass, lc, rem, queryRealTime, query); BOOST_CHECK_EQUAL(getEDNSZ(dq), 0); - BOOST_CHECK_EQUAL(getEDNSUDPPayloadSize(reinterpret_cast(dq.dh), dq.len), 0); + BOOST_CHECK_EQUAL(getEDNSUDPPayloadSizeAndZ(reinterpret_cast(dq.dh), dq.len, &udpPayloadSize, &z), false); + BOOST_CHECK_EQUAL(z, 0); + BOOST_CHECK_EQUAL(udpPayloadSize, 0); } { @@ -905,8 +923,9 @@ BOOST_AUTO_TEST_CASE(test_addEDNSToQueryTurnedResponse) { query.resize(query.size() - (/* RDLEN */ sizeof(uint16_t) + /* last byte of TTL / Z */ 1)); auto dq = turnIntoResponse(qname, qtype, qclass, lc, rem, queryRealTime, query); BOOST_CHECK_EQUAL(getEDNSZ(dq), 0); - /* 512, because we don't touch a broken OPT record */ - BOOST_CHECK_EQUAL(getEDNSUDPPayloadSize(reinterpret_cast(dq.dh), dq.len), 512); + BOOST_CHECK_EQUAL(getEDNSUDPPayloadSizeAndZ(reinterpret_cast(dq.dh), dq.len, &udpPayloadSize, &z), false); + BOOST_CHECK_EQUAL(z, 0); + BOOST_CHECK_EQUAL(udpPayloadSize, 0); } { @@ -918,7 +937,9 @@ BOOST_AUTO_TEST_CASE(test_addEDNSToQueryTurnedResponse) { auto dq = turnIntoResponse(qname, qtype, qclass, lc, rem, queryRealTime, query); BOOST_CHECK_EQUAL(getEDNSZ(dq), 0); - BOOST_CHECK_EQUAL(getEDNSUDPPayloadSize(reinterpret_cast(dq.dh), dq.len), g_PayloadSizeSelfGenAnswers); + BOOST_CHECK_EQUAL(getEDNSUDPPayloadSizeAndZ(reinterpret_cast(dq.dh), dq.len, &udpPayloadSize, &z), true); + BOOST_CHECK_EQUAL(z, 0); + BOOST_CHECK_EQUAL(udpPayloadSize, g_PayloadSizeSelfGenAnswers); } { @@ -930,7 +951,9 @@ BOOST_AUTO_TEST_CASE(test_addEDNSToQueryTurnedResponse) { auto dq = turnIntoResponse(qname, qtype, qclass, lc, rem, queryRealTime, query); BOOST_CHECK_EQUAL(getEDNSZ(dq), EDNS_HEADER_FLAG_DO); - BOOST_CHECK_EQUAL(getEDNSUDPPayloadSize(reinterpret_cast(dq.dh), dq.len), g_PayloadSizeSelfGenAnswers); + BOOST_CHECK_EQUAL(getEDNSUDPPayloadSizeAndZ(reinterpret_cast(dq.dh), dq.len, &udpPayloadSize, &z), true); + BOOST_CHECK_EQUAL(z, EDNS_HEADER_FLAG_DO); + BOOST_CHECK_EQUAL(udpPayloadSize, g_PayloadSizeSelfGenAnswers); } { @@ -942,7 +965,9 @@ BOOST_AUTO_TEST_CASE(test_addEDNSToQueryTurnedResponse) { auto dq = turnIntoResponse(qname, qtype, qclass, lc, rem, queryRealTime, query); BOOST_CHECK_EQUAL(getEDNSZ(dq), 0); - BOOST_CHECK_EQUAL(getEDNSUDPPayloadSize(reinterpret_cast(dq.dh), dq.len), g_PayloadSizeSelfGenAnswers); + BOOST_CHECK_EQUAL(getEDNSUDPPayloadSizeAndZ(reinterpret_cast(dq.dh), dq.len, &udpPayloadSize, &z), true); + BOOST_CHECK_EQUAL(z, 0); + BOOST_CHECK_EQUAL(udpPayloadSize, g_PayloadSizeSelfGenAnswers); } { @@ -954,7 +979,9 @@ BOOST_AUTO_TEST_CASE(test_addEDNSToQueryTurnedResponse) { auto dq = turnIntoResponse(qname, qtype, qclass, lc, rem, queryRealTime, query); BOOST_CHECK_EQUAL(getEDNSZ(dq), EDNS_HEADER_FLAG_DO); - BOOST_CHECK_EQUAL(getEDNSUDPPayloadSize(reinterpret_cast(dq.dh), dq.len), g_PayloadSizeSelfGenAnswers); + BOOST_CHECK_EQUAL(getEDNSUDPPayloadSizeAndZ(reinterpret_cast(dq.dh), dq.len, &udpPayloadSize, &z), true); + BOOST_CHECK_EQUAL(z, EDNS_HEADER_FLAG_DO); + BOOST_CHECK_EQUAL(udpPayloadSize, g_PayloadSizeSelfGenAnswers); } } diff --git a/regression-tests.dnsdist/test_Basics.py b/regression-tests.dnsdist/test_Basics.py index e9c122e705af..ea4b7b08b9a9 100644 --- a/regression-tests.dnsdist/test_Basics.py +++ b/regression-tests.dnsdist/test_Basics.py @@ -142,6 +142,48 @@ def testTruncateTC(self): response.answer.append(rrset) response.flags |= dns.flags.TC + expectedResponse = dns.message.make_response(query) + expectedResponse.flags |= dns.flags.TC + + (receivedQuery, receivedResponse) = self.sendUDPQuery(query, response) + receivedQuery.id = query.id + self.assertEquals(query, receivedQuery) + self.assertEquals(expectedResponse.flags, receivedResponse.flags) + self.assertEquals(expectedResponse.question, receivedResponse.question) + self.assertFalse(response.answer == receivedResponse.answer) + self.assertEquals(len(receivedResponse.answer), 0) + self.assertEquals(len(receivedResponse.authority), 0) + self.assertEquals(len(receivedResponse.additional), 0) + self.checkMessageNoEDNS(expectedResponse, receivedResponse) + + def testTruncateTCEDNS(self): + """ + Basics: Truncate TC with EDNS + + dnsdist is configured to truncate TC (default), + we make the backend send responses + with TC set and additional content, + and check that the received response has been fixed. + Note that the query and initial response had EDNS, + so the final response should have it too. + """ + name = 'atruncatetc.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN', use_edns=True, payload=4096, want_dnssec=True) + response = dns.message.make_response(query) + # force a different responder payload than the one in the query, + # so we check that we don't just mirror it + response.payload = 4242 + rrset = dns.rrset.from_text(name, + 3600, + dns.rdataclass.IN, + dns.rdatatype.A, + '127.0.0.1') + + response.answer.append(rrset) + response.flags |= dns.flags.TC + expectedResponse = dns.message.make_response(query) + expectedResponse.payload = 4242 + expectedResponse.flags |= dns.flags.TC (receivedQuery, receivedResponse) = self.sendUDPQuery(query, response) receivedQuery.id = query.id @@ -152,6 +194,11 @@ def testTruncateTC(self): self.assertEquals(len(receivedResponse.answer), 0) self.assertEquals(len(receivedResponse.authority), 0) self.assertEquals(len(receivedResponse.additional), 0) + print(expectedResponse) + print(receivedResponse) + self.checkMessageEDNSWithoutOptions(expectedResponse, receivedResponse) + self.assertFalse(receivedResponse.ednsflags & dns.flags.DO) + self.assertEquals(receivedResponse.payload, 4242) def testRegexReturnsRefused(self): """