From: Peter van Dijk <peter.van.dijk@powerdns.com>
Date: Mon, 12 Apr 2021 10:24:17 +0000 (+0200)
Subject: rpz dumper: stop generating double zz labels on networks that start with zeroes
X-Git-Tag: dnsdist-1.6.0-rc2~19^2~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bbe6cfec2b5a0b65a7183c04a4d088f1fcc87ba7;p=thirdparty%2Fpdns.git

rpz dumper: stop generating double zz labels on networks that start with zeroes

(partial rewrite; adds tests)
---

diff --git a/pdns/filterpo.cc b/pdns/filterpo.cc
index b5f66d8f6a..ce2867f041 100644
--- a/pdns/filterpo.cc
+++ b/pdns/filterpo.cc
@@ -697,31 +697,58 @@ DNSName DNSFilterEngine::Zone::maskToRPZ(const Netmask& nm)
   }
   else {
     DNSName temp;
-    const auto str = addr.toString();
-    const auto len = str.size();
-    std::string::size_type begin = 0;
-
-    while (begin < len) {
-      std::string::size_type end = str.find(":", begin);
-      std::string sub;
-      if (end != string::npos) {
-        sub = str.substr(begin, end - begin);
-      }
-      else {
-        sub = str.substr(begin);
+    static_assert(sizeof(addr.sin6.sin6_addr.s6_addr) == sizeof(uint16_t) * 8);
+    uint16_t *src = (uint16_t*) &addr.sin6.sin6_addr.s6_addr;
+    std::array<uint16_t,8> elems;
+
+    // this routine was adopted from glibc's inet_ntop6, written by Paul Vixie
+    // because the RPZ spec (https://datatracker.ietf.org/doc/html/draft-vixie-dnsop-dns-rpz-00#section-4.1.1) says:
+    //
+    //    If there exists more than one sequence of zero-valued fields of
+    //    identical length, then only the last such sequence is compressed.
+    //    Note that [RFC5952] specifies compressing the first such sequence,
+    //    but our notation here reverses the order of fields, and so must also
+    //    reverse the selection of which zero sequence to compress.
+    //
+    // 'cur.len > best.len' from the original code is replaced by 'cur.len >= best.len', so the last-longest wins.
+
+    struct { int base, len; } best = {-1, 0}, cur = {-1, 0};
+
+    for (int i = 0; i < (int)elems.size(); i++) {
+      auto elem = ntohs(src[i]);
+      elems[i] = elem;
+      if (elems[i] == 0) {
+        if (cur.base == -1) {  // start of a run of zeroes
+          cur = { i, 1 };
+        } else {
+          cur.len++;           // continuation of a run of zeroes
+        }
+      } else {                 // not a zero
+        if (cur.base != -1) {  // end of a run of zeroes
+          if (best.base == -1 || cur.len >= best.len) { // first run of zeroes, or a better one than we found before
+            best = cur;
+          }
+          cur.base = -1;
+        }
       }
+    }
 
-      if (sub.empty()) {
-        temp = DNSName("zz") + temp;
-      }
-      else {
-        temp = DNSName(sub) + temp;
+    if (cur.base != -1) {      // address ended with a zero
+      if (best.base == -1 || cur.len >= best.len) {     // first run of zeroes, or a better one than we found before
+        best = cur;
       }
+    }
 
-      if (end == string::npos) {
-        break;
+    if (best.base != -1 && best.len < 2) {              // if our best run is only one zero long, we do not replace it
+      best.base = -1;
+    }
+    for (int i=0; i < (int)elems.size(); i++) {
+      if (i==best.base) {
+        temp = DNSName("zz") + temp;
+        i = i + best.len - 1;
+      } else {
+        temp = DNSName((boost::format("%x") % elems.at(i)).str()) + temp;
       }
-      begin = end + 1;
     }
     res += temp;
   }
diff --git a/pdns/recursordist/test-filterpo_cc.cc b/pdns/recursordist/test-filterpo_cc.cc
index aa2fb17f0c..1f860a03b8 100644
--- a/pdns/recursordist/test-filterpo_cc.cc
+++ b/pdns/recursordist/test-filterpo_cc.cc
@@ -814,3 +814,14 @@ BOOST_AUTO_TEST_CASE(test_multiple_filter_policies_order)
     BOOST_CHECK(matchingPolicy.d_kind == DNSFilterEngine::PolicyKind::NoAction);
   }
 }
+
+BOOST_AUTO_TEST_CASE(test_mask_to_rpz)
+{
+  BOOST_CHECK_EQUAL(DNSFilterEngine::Zone::maskToRPZ(Netmask("::2/127")).toString(), "127.2.zz.");
+  BOOST_CHECK_EQUAL(DNSFilterEngine::Zone::maskToRPZ(Netmask("1::2/127")).toString(), "127.2.zz.1.");
+  BOOST_CHECK_EQUAL(DNSFilterEngine::Zone::maskToRPZ(Netmask("2::/127")).toString(), "127.zz.2.");
+  BOOST_CHECK_EQUAL(DNSFilterEngine::Zone::maskToRPZ(Netmask("1abc:2::/127")).toString(), "127.zz.2.1abc.");
+  BOOST_CHECK_EQUAL(DNSFilterEngine::Zone::maskToRPZ(Netmask("1:2:3:4:5:6:7::/127")).toString(), "127.0.7.6.5.4.3.2.1.");
+  BOOST_CHECK_EQUAL(DNSFilterEngine::Zone::maskToRPZ(Netmask("1:2:3:4:5:6::/127")).toString(), "127.zz.6.5.4.3.2.1.");
+  BOOST_CHECK_EQUAL(DNSFilterEngine::Zone::maskToRPZ(Netmask("1:0:0:0:2:0:0:0/127")).toString(), "127.zz.2.0.0.0.1.");
+}