From: W.C.A. Wijngaards Date: Mon, 11 Mar 2024 15:31:58 +0000 (+0100) Subject: - Fix #1021 Inconsistent Behavior with Changing rpz-cname-override X-Git-Tag: release-1.20.0rc1~72 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=320d0a5f1bdbf5dce3fb7a4fcac3d7173cea7846;p=thirdparty%2Funbound.git - Fix #1021 Inconsistent Behavior with Changing rpz-cname-override and doing a unbound-control reload. --- diff --git a/doc/Changelog b/doc/Changelog index bd0f7ef4b..5f7b08ff1 100644 --- a/doc/Changelog +++ b/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 diff --git a/services/authzone.c b/services/authzone.c index 93fef8ef1..f01a6d9e0 100644 --- a/services/authzone.c +++ b/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); diff --git a/services/rpz.c b/services/rpz.c index 18d76c07b..32588b7a8 100644 --- a/services/rpz.c +++ b/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,17 +515,17 @@ 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; @@ -546,9 +533,49 @@ rpz_create(struct config_auth* p) 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 @@ 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 diff --git a/services/rpz.h b/services/rpz.h index e6d8bf566..7f409087f 100644 --- a/services/rpz.h +++ b/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 diff --git a/testdata/rpz_reload.tdir/example.org.zone b/testdata/rpz_reload.tdir/example.org.zone new file mode 100644 index 000000000..21dd89938 --- /dev/null +++ b/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 diff --git a/testdata/rpz_reload.tdir/rpz.example.com.zone b/testdata/rpz_reload.tdir/rpz.example.com.zone new file mode 100644 index 000000000..ad075b18b --- /dev/null +++ b/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 diff --git a/testdata/rpz_reload.tdir/rpz_reload.conf b/testdata/rpz_reload.tdir/rpz_reload.conf new file mode 100644 index 000000000..d3c81e486 --- /dev/null +++ b/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 diff --git a/testdata/rpz_reload.tdir/rpz_reload.dsc b/testdata/rpz_reload.tdir/rpz_reload.dsc new file mode 100644 index 000000000..27f31cff1 --- /dev/null +++ b/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: diff --git a/testdata/rpz_reload.tdir/rpz_reload.post b/testdata/rpz_reload.tdir/rpz_reload.post new file mode 100644 index 000000000..ef93cd46b --- /dev/null +++ b/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 diff --git a/testdata/rpz_reload.tdir/rpz_reload.pre b/testdata/rpz_reload.tdir/rpz_reload.pre new file mode 100644 index 000000000..8f88b6094 --- /dev/null +++ b/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 diff --git a/testdata/rpz_reload.tdir/rpz_reload.test b/testdata/rpz_reload.tdir/rpz_reload.test new file mode 100644 index 000000000..f3cf9b29e --- /dev/null +++ b/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