- 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,43 +478,30 @@ new_cname_override(struct regional* region, uint8_t* ct, size_t ctlen)
 	return rrset;
 }
 
-struct rpz*
-rpz_create(struct config_auth* p)
+/** delete the cname override */
+static void
+delete_cname_override(struct rpz* r)
 {
-	struct rpz* r = calloc(1, sizeof(*r));
-	if(!r)
-		goto err;
-
-	r->region = regional_create_custom(sizeof(struct regional));
-	if(!r->region) {
-		goto err;
-	}
-
-	if(!(r->local_zones = local_zones_create())){
-		goto err;
-	}
-
-	r->nsdname_zones = local_zones_create();
-	if(r->local_zones == NULL){
-		goto err;
-	}
-
-	if(!(r->respip_set = respip_set_create())) {
-		goto err;
-	}
-
-	r->client_set = rpz_clientip_synthesized_set_create();
-	if(r->client_set == NULL) {
-		goto err;
+	if(r->cname_override) {
+		/* The cname override is what is allocated in the region. */
+		regional_free_all(r->region);
+		r->cname_override = NULL;
 	}
+}
 
-	r->ns_set = rpz_clientip_synthesized_set_create();
-	if(r->ns_set == NULL) {
-		goto err;
+/** 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;
+		}
 	}
 
-	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);
 	}
@@ -528,27 +515,67 @@ rpz_create(struct config_auth* p)
 		if(!p->rpz_cname) {
 			log_err("rpz: override with cname action found, but no "
 				"rpz-cname-override configured");
-			goto err;
+			return 0;
 		}
 
 		if(sldns_str2wire_dname_buf(p->rpz_cname, nm, &nmlen) != 0) {
 			log_err("rpz: cannot parse cname override: %s",
 				p->rpz_cname);
-			goto err;
+			return 0;
 		}
 		r->cname_override = new_cname_override(r->region, nm, nmlen);
 		if(!r->cname_override) {
-			goto err;
+			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");
-			goto err;
+			return 0;
 		}
 	}
+	return 1;
+}
+
+struct rpz*
+rpz_create(struct config_auth* p)
+{
+	struct rpz* r = calloc(1, sizeof(*r));
+	if(!r)
+		goto err;
+
+	r->region = regional_create_custom(sizeof(struct regional));
+	if(!r->region) {
+		goto err;
+	}
+
+	if(!(r->local_zones = local_zones_create())){
+		goto err;
+	}
+
+	r->nsdname_zones = local_zones_create();
+	if(r->local_zones == NULL){
+		goto err;
+	}
+
+	if(!(r->respip_set = respip_set_create())) {
+		goto err;
+	}
+
+	r->client_set = rpz_clientip_synthesized_set_create();
+	if(r->client_set == NULL) {
+		goto err;
+	}
+
+	r->ns_set = rpz_clientip_synthesized_set_create();
+	if(r->ns_set == NULL) {
+		goto err;
+	}
+
+	if(!rpz_apply_cfg_elements(r, p))
+		goto err;
 	return r;
 err:
 	if(r) {
@@ -571,6 +598,32 @@ rpz_create(struct config_auth* p)
 	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