- Fix #1021 Inconsistent Behavior with Changing rpz-cname-override

and doing a unbound-control reload.

doc/Changelog

@@ -1,3 +1,7 @@
+11 March 2024: Wouter
+	- Fix #1021 Inconsistent Behavior with Changing rpz-cname-override
+	  and doing a unbound-control reload.
+
 8 March 2024: Wouter
 	- Fix unbound-control-setup.cmd to use 3072 bits so that certificates
 	  are long enough for newer OpenSSL versions. This fix is included

services/authzone.c

@@ -2152,6 +2152,16 @@ auth_zones_cfg(struct auth_zones* az, struct config_auth* c)
 		if(az->rpz_first)
 			az->rpz_first->rpz_az_prev = z;
 		az->rpz_first = z;
+	} else if(c->isrpz && z->rpz) {
+		if(!rpz_config(z->rpz, c)) {
+			log_err("Could not change rpz config");
+			if(x) {
+				lock_basic_unlock(&x->lock);
+			}
+			lock_rw_unlock(&z->lock);
+			lock_rw_unlock(&az->rpz_lock);
+			return 0;
+		}
 	}
 	if(c->isrpz) {
 		lock_rw_unlock(&az->rpz_lock);

services/rpz.c

@@ -478,6 +478,67 @@ new_cname_override(struct regional* region, uint8_t* ct, size_t ctlen)
 	return rrset;
 }
 
+/** delete the cname override */
+static void
+delete_cname_override(struct rpz* r)
+{
+	if(r->cname_override) {
+		/* The cname override is what is allocated in the region. */
+		regional_free_all(r->region);
+		r->cname_override = NULL;
+	}
+}
+
+/** Apply rpz config elements to the rpz structure, false on failure. */
+static int
+rpz_apply_cfg_elements(struct rpz* r, struct config_auth* p)
+{
+	if(p->rpz_taglist && p->rpz_taglistlen) {
+		r->taglistlen = p->rpz_taglistlen;
+		r->taglist = memdup(p->rpz_taglist, r->taglistlen);
+		if(!r->taglist) {
+			log_err("malloc failure on RPZ taglist alloc");
+			return 0;
+		}
+	}
+
+	if(p->rpz_action_override) {
+		r->action_override = rpz_config_to_action(p->rpz_action_override);
+	}
+	else
+		r->action_override = RPZ_NO_OVERRIDE_ACTION;
+
+	if(r->action_override == RPZ_CNAME_OVERRIDE_ACTION) {
+		uint8_t nm[LDNS_MAX_DOMAINLEN+1];
+		size_t nmlen = sizeof(nm);
+
+		if(!p->rpz_cname) {
+			log_err("rpz: override with cname action found, but no "
+				"rpz-cname-override configured");
+			return 0;
+		}
+
+		if(sldns_str2wire_dname_buf(p->rpz_cname, nm, &nmlen) != 0) {
+			log_err("rpz: cannot parse cname override: %s",
+				p->rpz_cname);
+			return 0;
+		}
+		r->cname_override = new_cname_override(r->region, nm, nmlen);
+		if(!r->cname_override) {
+			return 0;
+		}
+	}
+	r->log = p->rpz_log;
+	r->signal_nxdomain_ra = p->rpz_signal_nxdomain_ra;
+	if(p->rpz_log_name) {
+		if(!(r->log_name = strdup(p->rpz_log_name))) {
+			log_err("malloc failure on RPZ log_name strdup");
+			return 0;
+		}
+	}
+	return 1;
+}
+
 struct rpz*
 rpz_create(struct config_auth* p)
 {
@@ -513,42 +574,8 @@ rpz_create(struct config_auth* p)
 		goto err;
 	}
 
-	r->taglistlen = p->rpz_taglistlen;
-	r->taglist = memdup(p->rpz_taglist, r->taglistlen);
-	if(p->rpz_action_override) {
-		r->action_override = rpz_config_to_action(p->rpz_action_override);
-	}
-	else
-		r->action_override = RPZ_NO_OVERRIDE_ACTION;
-
-	if(r->action_override == RPZ_CNAME_OVERRIDE_ACTION) {
-		uint8_t nm[LDNS_MAX_DOMAINLEN+1];
-		size_t nmlen = sizeof(nm);
-
-		if(!p->rpz_cname) {
-			log_err("rpz: override with cname action found, but no "
-				"rpz-cname-override configured");
-			goto err;
-		}
-
-		if(sldns_str2wire_dname_buf(p->rpz_cname, nm, &nmlen) != 0) {
-			log_err("rpz: cannot parse cname override: %s",
-				p->rpz_cname);
-			goto err;
-		}
-		r->cname_override = new_cname_override(r->region, nm, nmlen);
-		if(!r->cname_override) {
-			goto err;
-		}
-	}
-	r->log = p->rpz_log;
-	r->signal_nxdomain_ra = p->rpz_signal_nxdomain_ra;
-	if(p->rpz_log_name) {
-		if(!(r->log_name = strdup(p->rpz_log_name))) {
-			log_err("malloc failure on RPZ log_name strdup");
-			goto err;
-		}
-	}
+	if(!rpz_apply_cfg_elements(r, p))
+		goto err;
 	return r;
 err:
 	if(r) {
@@ -571,6 +598,32 @@ err:
 	return NULL;
 }
 
+int
+rpz_config(struct rpz* r, struct config_auth* p)
+{
+	/* If the zonefile changes, it is read later, after which
+	 * rpz_clear and rpz_finish_config is called. */
+
+	/* free taglist, if any */
+	if(r->taglist) {
+		free(r->taglist);
+		r->taglist = NULL;
+		r->taglistlen = 0;
+	}
+
+	/* free logname, if any */
+	if(r->log_name) {
+		free(r->log_name);
+		r->log_name = NULL;
+	}
+
+	delete_cname_override(r);
+
+	if(!rpz_apply_cfg_elements(r, p))
+		return 0;
+	return 1;
+}
+
 /**
  * Remove RPZ zone name from dname
  * Copy dname to newdname, without the originlen number of trailing bytes

services/rpz.h

@@ -225,6 +225,14 @@ int rpz_clear(struct rpz* r);
  */
 struct rpz* rpz_create(struct config_auth* p);
 
+/**
+ * Change config on rpz, after reload.
+ * @param r: the rpz structure.
+ * @param p: the config that was read.
+ * @return false on failure.
+ */
+int rpz_config(struct rpz* r, struct config_auth* p);
+
 /**
  * String for RPZ action enum
  * @param a: RPZ action to get string for

testdata/rpz_reload.tdir/example.org.zone

@@ -0,0 +1,2 @@
+example.org. 3600 IN SOA ns1.example.org. hostmaster.example.org. 1379078166 28800 7200 604800 7200
+www.example.org. A 1.2.3.5

testdata/rpz_reload.tdir/rpz.example.com.zone

@@ -0,0 +1,6 @@
+; example rpz file
+rpz.example.com. 3600 IN SOA ns1.rpz.example.com. hostmaster.rpz.example.com. 1379078166 28800 7200 604800 7200
+	NS ns1.rpz.example.com.
+	NS ns2.rpz.example.com.
+foo.example.net	CNAME .
+www.example.net	A	1.2.3.4

testdata/rpz_reload.tdir/rpz_reload.conf

@@ -0,0 +1,30 @@
+server:
+	verbosity: 7
+	# num-threads: 1
+	interface: 127.0.0.1
+	port: @PORT@
+	use-syslog: no
+	directory: ""
+	pidfile: "unbound.pid"
+	chroot: ""
+	username: ""
+	module-config: "respip iterator"
+	log-time-ascii: yes
+
+remote-control:
+	control-enable: yes
+	control-interface: @CONTROL_PATH@/controlpipe.@CONTROL_PID@
+	control-use-cert: no
+
+rpz:
+	name: "rpz.example.com"
+	zonefile: "rpz.example.com.zone"
+	rpz-action-override: cname
+	rpz-cname-override: "www.example.org"
+	rpz-log: yes
+	rpz-log-name: "example policy"
+
+auth-zone:
+	name: "example.org"
+	zonefile: "example.org.zone"
+	for-upstream: yes

testdata/rpz_reload.tdir/rpz_reload.dsc

@@ -0,0 +1,16 @@
+BaseName: rpz_reload
+Version: 1.0
+Description: check rpz reload change
+CreationDate: Mon 11 Mar 16:00:00 CET 2024
+Maintainer: dr. W.C.A. Wijngaards
+Category: 
+Component:
+CmdDepends: 
+Depends: 
+Help:
+Pre: rpz_reload.pre
+Post: rpz_reload.post
+Test: rpz_reload.test
+AuxFiles: 
+Passed:
+Failure:

testdata/rpz_reload.tdir/rpz_reload.post

@@ -0,0 +1,12 @@
+# #-- rpz_reload.post --#
+# source the master var file when it's there
+[ -f ../.tpkg.var.master ] && source ../.tpkg.var.master
+# source the test var file when it's there
+[ -f .tpkg.var.test ] && source .tpkg.var.test
+#
+# do your teardown here
+. ../common.sh
+echo "> cat logfiles"
+cat unbound.log
+kill_pid $UNBOUND_PID
+rm -f $CONTROL_PATH/controlpipe.$CONTROL_PID

testdata/rpz_reload.tdir/rpz_reload.pre

@@ -0,0 +1,26 @@
+# #-- rpz_reload.pre--#
+# source the master var file when it's there
+[ -f ../.tpkg.var.master ] && source ../.tpkg.var.master
+# use .tpkg.var.test for in test variable passing
+[ -f .tpkg.var.test ] && source .tpkg.var.test
+
+. ../common.sh
+
+get_random_port 1
+UNBOUND_PORT=$RND_PORT
+echo "UNBOUND_PORT=$UNBOUND_PORT" >> .tpkg.var.test
+
+# make config file
+CONTROL_PATH=/tmp
+CONTROL_PID=$$
+sed -e 's/@PORT\@/'$UNBOUND_PORT'/' -e 's?@CONTROL_PATH\@?'$CONTROL_PATH'?' -e 's/@CONTROL_PID@/'$CONTROL_PID'/' < rpz_reload.conf > ub.conf
+# start unbound in the background
+PRE="../.."
+$PRE/unbound -d -c ub.conf >unbound.log 2>&1 &
+UNBOUND_PID=$!
+echo "UNBOUND_PID=$UNBOUND_PID" >> .tpkg.var.test
+echo "CONTROL_PATH=$CONTROL_PATH" >> .tpkg.var.test
+echo "CONTROL_PID=$CONTROL_PID" >> .tpkg.var.test
+
+cat .tpkg.var.test
+wait_unbound_up unbound.log

testdata/rpz_reload.tdir/rpz_reload.test

@@ -0,0 +1,109 @@
+# #-- rpz_reload.test --#
+# source the master var file when it's there
+[ -f ../.tpkg.var.master ] && source ../.tpkg.var.master
+# use .tpkg.var.test for in test variable passing
+[ -f .tpkg.var.test ] && source .tpkg.var.test
+
+PRE="../.."
+. ../common.sh
+# do the test
+echo "> dig . SOA"
+dig @127.0.0.1 -p $UNBOUND_PORT localhost. A | tee outfile
+echo "> check answer"
+if grep localhost outfile | grep "127.0.0.1"; then
+	echo "OK"
+else
+	echo "Not OK"
+	exit 1
+fi
+
+echo ""
+echo "> unbound-control status"
+$PRE/unbound-control -c ub.conf status
+if test $? -ne 0; then
+	echo "wrong exit value."
+	exit 1
+else
+	echo "exit value: OK"
+fi
+
+# Have the RPZ block some things.
+dig @127.0.0.1 -p $UNBOUND_PORT foo.example.net. A | tee outfile
+echo "> check answer"
+if grep "www.example.org" outfile | grep "1.2.3.5"; then
+	echo "OK"
+else
+	echo "Not OK"
+	exit 1
+fi
+if grep "rpz: applied .example policy." unbound.log | grep "foo.example.net. A"; then
+	echo "log line OK"
+else
+	echo "log line not OK"
+	exit 1
+fi
+
+dig @127.0.0.1 -p $UNBOUND_PORT www.example.net. A | tee outfile
+if grep "www.example.org" outfile | grep "1.2.3.5"; then
+	echo "OK"
+else
+	echo "Not OK"
+	exit 1
+fi
+if grep "rpz: applied .example policy." unbound.log | grep "www.example.net. A"; then
+	echo "log line OK"
+else
+	echo "log line not OK"
+	exit 1
+fi
+
+# Modify the config
+cp ub.conf ub2.conf
+sed -e 's/rpz-action-override: cname/#rpz-action-override: ""/' \
+    -e 's/rpz-cname-override: "www.example.org"/rpz-cname-override: ""/' \
+    -e 's/rpz-log-name: "example policy"/rpz-log-name: "exrpz"/' \
+    < ub2.conf > ub.conf
+echo ""
+echo "> Modified config"
+grep "rpz" ub.conf
+echo ""
+
+echo "> unbound-control reload"
+$PRE/unbound-control -c ub.conf reload 2>&1 | tee outfile
+if test $? -ne 0; then
+	echo "wrong exit value."
+	exit 1
+fi
+wait_logfile unbound.log "Restart of unbound" 60
+
+# Check the output after reload
+dig @127.0.0.1 -p $UNBOUND_PORT foo.example.net. A | tee outfile
+echo "> check answer"
+if grep "NXDOMAIN" outfile; then
+	echo "OK"
+else
+	echo "Not OK"
+	exit 1
+fi
+if grep "rpz: applied .exrpz." unbound.log | grep "foo.example.net. A"; then
+	echo "log line OK"
+else
+	echo "log line not OK"
+	exit 1
+fi
+
+dig @127.0.0.1 -p $UNBOUND_PORT www.example.net. A | tee outfile
+if grep "1.2.3.4" outfile; then
+	echo "OK"
+else
+	echo "Not OK"
+	exit 1
+fi
+if grep "rpz: applied .exrpz." unbound.log | grep "www.example.net. A"; then
+	echo "log line OK"
+else
+	echo "log line not OK"
+	exit 1
+fi
+
+exit 0