From: Peter van Dijk <peter.van.dijk@powerdns.com>
Date: Mon, 7 Aug 2023 17:13:36 +0000 (+0200)
Subject: IXFR client: handle partial reads of the TCP chunk length header, plus:
X-Git-Tag: rec-4.9.1~3^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4a4034c9d1a8a94667164cc415e8593da0a76f2c;p=thirdparty%2Fpdns.git

IXFR client: handle partial reads of the TCP chunk length header, plus:
* add primarySOACount to exception text
* add indicator of current state to exception text
* a test

(cherry picked from commit 8faf5a90992b2613cf5999c8dd5e26b0025050b7)
---

diff --git a/pdns/ixfr.cc b/pdns/ixfr.cc
index fb09e35037..755fb1d155 100644
--- a/pdns/ixfr.cc
+++ b/pdns/ixfr.cc
@@ -123,6 +123,7 @@ vector<pair<vector<DNSRecord>, vector<DNSRecord> > > processIXFRRecords(const Co
 }
 
 // Returns pairs of "remove & add" vectors. If you get an empty remove, it means you got an AXFR!
+ // NOLINTNEXTLINE(readability-function-cognitive-complexity): https://github.com/PowerDNS/pdns/issues/12791
 vector<pair<vector<DNSRecord>, vector<DNSRecord>>> getIXFRDeltas(const ComboAddress& primary, const DNSName& zone, const DNSRecord& oursr,
                                                                  uint16_t xfrTimeout, bool totalTimeout,
                                                                  const TSIGTriplet& tt, const ComboAddress* laddr, size_t maxReceivedBytes)
@@ -203,24 +204,36 @@ vector<pair<vector<DNSRecord>, vector<DNSRecord>>> getIXFRDeltas(const ComboAddr
   const unsigned int expectedSOAForIXFR = 3;
   unsigned int primarySOACount = 0;
 
+  std::string state;
   for (;;) {
+    state = "start";
     // IXFR or AXFR style end reached? We don't want to process trailing data after the closing SOA
     if (style == AXFR && primarySOACount == expectedSOAForAXFR) {
+      state = "AXFRdone";
       break;
     }
-    else if (style == IXFR && primarySOACount == expectedSOAForIXFR) {
+    if (style == IXFR && primarySOACount == expectedSOAForIXFR) {
+      state = "IXFRdone";
       break;
     }
 
     elapsed = timeoutChecker();
-    if (s.readWithTimeout(reinterpret_cast<char*>(&len), sizeof(len), static_cast<int>(xfrTimeout - elapsed)) != sizeof(len)) {
+    try {
+      const struct timeval remainingTime = { .tv_sec = xfrTimeout - elapsed, .tv_usec = 0 };
+      const struct timeval idleTime = remainingTime;
+      readn2WithTimeout(s.getHandle(), &len, sizeof(len), idleTime, remainingTime, false);
+    }
+    catch (const runtime_error& ex) {
+      state = ex.what();
       break;
     }
 
     len = ntohs(len);
     if (len == 0) {
+      state = "zeroLen";
       break;
     }
+    // Currently no more break statements after this
 
     if (maxReceivedBytes > 0 && (maxReceivedBytes - receivedBytes) < (size_t) len) {
       throw std::runtime_error("Reached the maximum number of received bytes in an IXFR delta for zone '"+zone.toLogString()+"' from primary "+primary.toStringWithPort());
@@ -229,9 +242,9 @@ vector<pair<vector<DNSRecord>, vector<DNSRecord>>> getIXFRDeltas(const ComboAddr
     reply.resize(len);
 
     elapsed = timeoutChecker();
-    const struct timeval remainingTime =  { .tv_sec = xfrTimeout - elapsed, .tv_usec = 0 };
+    const struct timeval remainingTime = { .tv_sec = xfrTimeout - elapsed, .tv_usec = 0 };
     const struct timeval idleTime = remainingTime;
-    readn2WithTimeout(s.getHandle(), &reply.at(0), len, idleTime, remainingTime, false);
+    readn2WithTimeout(s.getHandle(), reply.data(), len, idleTime, remainingTime, false);
     receivedBytes += len;
 
     MOADNSParser mdp(false, reply);
@@ -306,16 +319,16 @@ vector<pair<vector<DNSRecord>, vector<DNSRecord>>> getIXFRDeltas(const ComboAddr
   switch (style) {
   case IXFR:
     if (primarySOACount != expectedSOAForIXFR) {
-      throw std::runtime_error("Incomplete IXFR transfer for '" + zone.toLogString() + "' from primary '" + primary.toStringWithPort());
+      throw std::runtime_error("Incomplete IXFR transfer (primarySOACount=" + std::to_string(primarySOACount) + ") for '" + zone.toLogString() + "' from primary '" + primary.toStringWithPort() + " state=" + state);
     }
     break;
   case AXFR:
     if (primarySOACount != expectedSOAForAXFR){
-      throw std::runtime_error("Incomplete AXFR style transfer for '" + zone.toLogString() + "' from primary '" + primary.toStringWithPort());
+      throw std::runtime_error("Incomplete AXFR style transfer (primarySOACount=" + std::to_string(primarySOACount) + ")  for '" + zone.toLogString() + "' from primary '" + primary.toStringWithPort() + " state=" + state);
     }
     break;
   case Unknown:
-    throw std::runtime_error("Incomplete XFR for '" + zone.toLogString() + "' from primary '" + primary.toStringWithPort());
+    throw std::runtime_error("Incomplete XFR (primarySOACount=" + std::to_string(primarySOACount) + ") for '" + zone.toLogString() + "' from primary '" + primary.toStringWithPort() + " state=" + state);
     break;
   }
 
diff --git a/regression-tests.recursor-dnssec/test_RPZ.py b/regression-tests.recursor-dnssec/test_RPZ.py
index 4215eacd33..da865ea74c 100644
--- a/regression-tests.recursor-dnssec/test_RPZ.py
+++ b/regression-tests.recursor-dnssec/test_RPZ.py
@@ -185,7 +185,12 @@ class RPZServer(object):
                 break
 
             wire = answer.to_wire()
-            conn.send(struct.pack("!H", len(wire)))
+            lenprefix = struct.pack("!H", len(wire))
+
+            for b in lenprefix:
+                conn.send(bytes([b]))
+                time.sleep(0.5)
+
             conn.send(wire)
             self._currentSerial = serial
             break