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 915fc30d95f7a8df6f02fab5dc7004f594259037..c1356a887498b929e42e2d97d7569907eb4de6cb 100644 (file)
--- 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<char *>(&response[0]), response.size(), age);
+    if (!stale) {
+      ageDNSPacket(reinterpret_cast<char *>(&response[0]), response.size(), age);
+    }
+    else {
+      editDNSPacketTTL(reinterpret_cast<char *>(&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 50c6bfd521b1c01b892432f3ba28adcedaeaff8b..ad2aff9f8032aaf4f8aa61b633bdf64055874554 100644 (file)
--- 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 aadd7a2e7c83598be79374797946dd6a29e993ad..e29a72e947a155aa0c2d3c1365835af8a1d772a8 100644 (file)
--- 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 c1ac47079b135a7917002f8fa427c44136746fd0..e71bf19fdb002e7fea2c5ab8a494985ca6986b2c 100644 (file)
--- 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<char*>(firstPacket.data()), firstPacket.size(), 1801);
 
-  uint32_t ttl = std::numeric_limits<uint32_t>::max();
+  uint32_t ttl = 0;
 
   expectedAlteredPacket = generatePacket(ttl);
   BOOST_REQUIRE_EQUAL(firstPacket.size(), expectedAlteredPacket.size());