Fix RPZ radix tree search() for CLIENT-IP triggers (#39481)

diff --git a/CHANGES b/CHANGES
index 9dfc1a6230699c4362c69a85f72f526f2527b066..b6814ffa72aac7d53182981045a5abbeb1051f97 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,8 @@
+4122.  [bug]           The server could match a shorter prefix than what was
+                       available in CLIENT-IP policy triggers, and so, an
+                       unexpected action could be taken. This has been
+                       corrected. [RT #39481]
+
 4121.  [bug]           When updating a response-policy zone via AXFR,
                        summary data about other policy zones could fall
                        out of sync. Ultimately this could trigger an

diff --git a/bin/named/query.c b/bin/named/query.c
index 506c0b1430cd29d24b934bb0ebe22871f417c7af..fa03a16a298c2346d01d624157b1844e8cf1ded7 100644 (file)
--- a/bin/named/query.c
+++ b/bin/named/query.c
@@ -5166,6 +5166,7 @@ rpz_rewrite(ns_client_t *client, dns_rdatatype_t qtype,
                st->m.type = DNS_RPZ_TYPE_BAD;
                st->m.policy = DNS_RPZ_POLICY_MISS;
                st->m.ttl = ~0;
+               st->m.prefix = 0;
                memset(&st->r, 0, sizeof(st->r));
                memset(&st->q, 0, sizeof(st->q));
                dns_fixedname_init(&st->_p_namef);

diff --git a/bin/tests/system/rpzrecurse/ns2/db.clientip1 b/bin/tests/system/rpzrecurse/ns2/db.clientip1
new file mode 100644 (file)
index 0000000..58f3817
--- /dev/null
+++ b/bin/tests/system/rpzrecurse/ns2/db.clientip1
@@ -0,0 +1,6 @@
+$TTL 60
+@ IN SOA root.ns ns 1996072700 3600 1800 86400 60
+     NS ns
+ns A 127.0.0.1
+32.4.0.53.10.rpz-client-ip A 10.53.0.2
+24.0.0.53.10.rpz-client-ip A 10.53.0.1

diff --git a/bin/tests/system/rpzrecurse/ns2/db.clientip2 b/bin/tests/system/rpzrecurse/ns2/db.clientip2
new file mode 100644 (file)
index 0000000..a7a676a
--- /dev/null
+++ b/bin/tests/system/rpzrecurse/ns2/db.clientip2
@@ -0,0 +1,5 @@
+$TTL 60
+@ IN SOA root.ns ns 1996072700 3600 1800 86400 60
+     NS ns
+ns A 127.0.0.1
+24.0.0.53.10.rpz-client-ip A 10.53.0.3

diff --git a/bin/tests/system/rpzrecurse/ns2/named.clientip.conf b/bin/tests/system/rpzrecurse/ns2/named.clientip.conf
new file mode 100644 (file)
index 0000000..810237b
--- /dev/null
+++ b/bin/tests/system/rpzrecurse/ns2/named.clientip.conf
@@ -0,0 +1,19 @@
+# common configuration
+include "named.conf.header";
+
+view "recursive" {
+    zone "." {
+        type hint;
+        file "root.hint";
+    };
+
+    # policy configuration to be tested
+    response-policy {
+        zone "clientip1";
+        zone "clientip2";
+    } qname-wait-recurse no;
+
+    # policy zones to be tested
+    zone "clientip1" { type master; file "db.clientip1"; };
+    zone "clientip2" { type master; file "db.clientip2"; };
+};

diff --git a/bin/tests/system/rpzrecurse/tests.sh b/bin/tests/system/rpzrecurse/tests.sh
index f833cd4bc95e270f4be0ec00877ca94b21d0a203..bd59fd53577bfb5b1042e9def79a7c729259ba8e 100644 (file)
--- a/bin/tests/system/rpzrecurse/tests.sh
+++ b/bin/tests/system/rpzrecurse/tests.sh
@@ -231,4 +231,18 @@ for n in 1 2 3 4 5 6 7 8 9; do
     }
 done
 
+# Check CLIENT-IP behavior
+t=`expr $t + 1`
+echo "I:testing CLIENT-IP behavior (${t})"
+run_server clientip
+$DIG $DIGOPTS l2.l1.l0 a @10.53.0.2 -p 5300 -b 10.53.0.4 > dig.out.${t}
+grep "status: NOERROR" dig.out.${t} > /dev/null 2>&1 || {
+    echo "I:test $t failed: query failed"
+    status=1
+}
+grep "^l2.l1.l0.[[:space:]]*[0-9]*[[:space:]]*IN[[:space:]]*A[[:space:]]*10.53.0.2" dig.out.${t} > /dev/null 2>&1 || {
+    echo "I:test $t failed: didn't get expected answer"
+    status=1
+}
+
 exit $status

diff --git a/doc/arm/notes.xml b/doc/arm/notes.xml
index 05092d56a546990849777b9b7a87b0ff7bd76a1e..f40f175e611a966536060eb0dcb7e9a3ab5227ec 100644 (file)
--- a/doc/arm/notes.xml
+++ b/doc/arm/notes.xml
@@ -652,6 +652,14 @@
            [RT #39567]
          </para>
        </listitem>
+       <listitem>
+         <para>
+           The server could match a shorter prefix than what was
+           available in CLIENT-IP policy triggers, and so, an
+           unexpected action could be taken. This has been
+           corrected. [RT #39481]
+         </para>
+       </listitem>
       </itemizedlist>
     </itemizedlist>
   </sect2>

diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c
index e547bea45b975523a6e19e0a3533f638fb73fbf1..159709dc68c83a92a38c3d15d06a6d2dd1ff3ee3 100644 (file)
--- a/lib/dns/rpz.c
+++ b/lib/dns/rpz.c
@@ -827,7 +827,7 @@ name2ipkey(int log_level,
        if (--ip_labels == 4 && !strchr(cp, 'z')) {
                /*
                 * Convert an IPv4 address
-                * from the form "prefix.w.z.y.x"
+                * from the form "prefix.z.y.x.w"
                 */
                if (prefix_num > 32U) {
                        badname(log_level, src_name,
@@ -911,6 +911,12 @@ name2ipkey(int log_level,
                prefix += DNS_RPZ_CIDR_WORD_BITS;
        }
 
+       /*
+        * XXXMUKS: Should the following check be enabled in a
+        * production build?  It can be expensive for large IP zones
+        * from 3rd parties.
+        */
+
        /*
         * Convert the address back to a canonical domain name
         * to ensure that the original name is in canonical form.
@@ -1093,7 +1099,7 @@ search(dns_rpz_zones_t *rpzs,
                        child->set.ip |= tgt_set->ip;
                        child->set.nsip |= tgt_set->nsip;
                        set_sum_pair(child);
-                       *found = cur;
+                       *found = child;
                        return (ISC_R_SUCCESS);
                }
 
@@ -1186,8 +1192,8 @@ search(dns_rpz_zones_t *rpzs,
                                 */
                                find_result = DNS_R_PARTIALMATCH;
                                *found = cur;
-                               set.client_ip = trim_zbits(set.ip,
-                                                       cur->set.client_ip);
+                               set.client_ip = trim_zbits(set.client_ip,
+                                                          cur->set.client_ip);
                                set.ip = trim_zbits(set.ip,
                                                    cur->set.ip);
                                set.nsip = trim_zbits(set.nsip,