Inconsistent Behavior with Changing rpz-cname-override and doing a unbound-control reload

[softov]

When attempting to change the rpz-cname-override value in the 'rpz:' section of the Unbound configuration and reloading the configuration using unbound-control reload, the override does not take effect. The DNS server continues to reply with the CNAME specified before the change.
After restarting Unbound (kill), the new configuration runs correctly.

To reproduce

rpz:
  name: "block.rpz."
  rpz-action-override: cname
  rpz-cname-override: dev01.example.com
  zonefile: "/path/zone-rpz/block.rpz.zone"

Modify the value of rpz-cname-override in the Unbound configuration file.

  rpz-cname-override: dev02.example.com

Execute unbound-control reload to reload the Unbound configuration.

The query reply a CNAME for dev01.example.com

Expected behavior
After reloading the Unbound configuration, the new value of rpz-cname-override should take effect, and the DNS server should reply with the updated CNAME for dev02.example.com.

System:

OS: FreeBSD
Unbound version -V: Version 1.19.1
Configure line: --with-libevent --with-libbsd --with-pthreads --enable-dnscrypt --enable-subnet
Linked libs: libevent 2.0.22-stable (it uses kqueue), OpenSSL 1.0.2j 26 Sep 2016
Linked modules: dns64 subnetcache respip validator iterator
DNSCrypt feature available

Additional information
Attempts to resolve the issue using commands such as rpz_disable, rpz_enable, auth_zone_reload, flush_zone, flush have been unsuccessful. The desired behavior is achieved only after restarting the Unbound service (kill), which is not desirable.

[softov]

All I can see is that, in the auth_zones_cfg function

	if(c->isrpz && !z->rpz){
		if(!(z->rpz = rpz_create(c)))
		...
	}

So I figured out the 'rpz' is a 'authzone' with a flag.
Inside the rpz_create, the 'r->action_override' is populated.

I think there is a need to a function in rpz.c to reconfigure the rpz with the changed info in 'struct config_auth'.

[softov]

I figure out, none of the rpz info was reloaded when editing conf and doing a unbound-control reload.

I was able to 'correct' the issue, creating a function int rpz_config(struct rpz* r, struct config_auth* p) and calling it in static int auth_zones_cfg(struct auth_zones* az, struct config_auth* c).

// rpz.h
int rpz_config(struct rpz* r, struct config_auth* p);

// rpz.c
int rpz_config(struct rpz* r, struct config_auth* p)
{
	if(r->taglist)
		free(r->taglist);

	if(r->log_name)
		free(r->log_name);

	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;
		}
	}

	return 0;
	err:
	// do not clear, maintain previous info
	return -1;
}

// authzone.c
	if(c->isrpz && !z->rpz){
		if(!(z->rpz = rpz_create(c))){
			fatal_exit("Could not setup RPZ zones");
			return 0;
		}
		lock_protect(&z->lock, &z->rpz->local_zones, sizeof(*z->rpz));
		/* the az->rpz_lock is locked above */
		z->rpz_az_next = az->rpz_first;
		if(az->rpz_first)
			az->rpz_first->rpz_az_prev = z;
		az->rpz_first = z;
	}
+	else if(c->isrpz) {
+		rpz_config(z->rpz, c);
+	}

I can do a PR, but, I don't know how to 'clear' some things (or even if is needed), like, taglist, log_name, r->cname_override and avoid leak.

The function can be used inside rpz_create, with some adjustments to avoid double code.

Then, in authzone.c -> auth_zones_cfg, call rpz_config if 'isrpz'

[wcawijngaards]

You are right the reload did not take rpz config variable changes into account. With the suggested code changes adjusted to allocate and free and for failure code paths, the commit makes it possible to adjust rpz configuration and then reload it. There is also a unit test. Thanks for the report, hopefully this solves the issue!