From: Otto Date: Wed, 17 Mar 2021 15:00:16 +0000 (+0100) Subject: Make sure we take the right minimum for the PC TTL data in the SERVFAIL case. X-Git-Tag: rec-4.5.0-beta1~16^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F10185%2Fhead;p=thirdparty%2Fpdns.git Make sure we take the right minimum for the PC TTL data in the SERVFAIL case. Also add safety belt to the ageing code to not wrap TTLs, adjust one dnsdist test for ageDNSPacket no longer underflowing, and stop dnsdist from relying on ageDNSPacket wrapping around. --- diff --git a/pdns/dnsdist-cache.cc b/pdns/dnsdist-cache.cc index 915fc30d95..c1356a8874 100644 --- a/pdns/dnsdist-cache.cc +++ b/pdns/dnsdist-cache.cc @@ -285,7 +285,13 @@ bool DNSDistPacketCache::get(DNSQuestion& dq, uint16_t queryId, uint32_t* keyOut } if (!d_dontAge && !skipAging) { - ageDNSPacket(reinterpret_cast(&response[0]), response.size(), age); + if (!stale) { + ageDNSPacket(reinterpret_cast(&response[0]), response.size(), age); + } + else { + editDNSPacketTTL(reinterpret_cast(&response[0]), response.size(), + [staleTTL = d_staleTTL](uint8_t section, uint16_t class_, uint16_t type, uint32_t ttl) { return staleTTL; }); + } } d_hits++; diff --git a/pdns/dnsparser.hh b/pdns/dnsparser.hh index 50c6bfd521..ad2aff9f80 100644 --- a/pdns/dnsparser.hh +++ b/pdns/dnsparser.hh @@ -514,7 +514,11 @@ public: uint32_t tmp; memcpy(&tmp, (void*) p, sizeof(tmp)); tmp = ntohl(tmp); - tmp-=decrease; + if (tmp > decrease) { + tmp -= decrease; + } else { + tmp = 0; + } tmp = htonl(tmp); memcpy(d_packet + d_offset-4, (const char*)&tmp, sizeof(tmp)); } diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc index aadd7a2e7c..e29a72e947 100644 --- a/pdns/pdns_recursor.cc +++ b/pdns/pdns_recursor.cc @@ -2125,12 +2125,13 @@ static void startDoResolve(void *p) g_stats.variableResponses++; } if (!SyncRes::s_nopacketcache && !variableAnswer && !sr.wasVariable()) { + minTTL = min(minTTL, pw.getHeader()->rcode == RCode::ServFail ? SyncRes::s_packetcacheservfailttl : + SyncRes::s_packetcachettl); t_packetCache->insertResponsePacket(dc->d_tag, dc->d_qhash, std::move(dc->d_query), dc->d_mdp.d_qname, dc->d_mdp.d_qtype, dc->d_mdp.d_qclass, string((const char*)&*packet.begin(), packet.size()), g_now.tv_sec, - pw.getHeader()->rcode == RCode::ServFail ? SyncRes::s_packetcacheservfailttl : - min(minTTL,SyncRes::s_packetcachettl), + minTTL, dq.validationState, std::move(pbDataForCache), dc->d_tcp); } diff --git a/pdns/test-dnsparser_cc.cc b/pdns/test-dnsparser_cc.cc index c1ac47079b..e71bf19fdb 100644 --- a/pdns/test-dnsparser_cc.cc +++ b/pdns/test-dnsparser_cc.cc @@ -131,13 +131,12 @@ BOOST_AUTO_TEST_CASE(test_ageDNSPacket) { BOOST_CHECK(firstPacket == expectedAlteredPacket); - /* now remove more than the remaining TTL, not that while TTL are, - per rfc1035 errata, "a 32 bit unsigned integer" so we should be - able to expect unsigned overflow to apply, but rfc2181 specifies - a maximum of "2^31 - 1". */ + /* now remove more than the remaining TTL. We expect ageDNSPacket + to cap this at zero and not cause an unsigned underflow into + the 2^32-1 neighbourhood */ ageDNSPacket(reinterpret_cast(firstPacket.data()), firstPacket.size(), 1801); - uint32_t ttl = std::numeric_limits::max(); + uint32_t ttl = 0; expectedAlteredPacket = generatePacket(ttl); BOOST_REQUIRE_EQUAL(firstPacket.size(), expectedAlteredPacket.size());