From c55490c1e6c1b1206457e1c1aae2dc418a60a873 Mon Sep 17 00:00:00 2001 From: Yorgos Thessalonikefs Date: Mon, 2 Dec 2024 12:28:11 +0100 Subject: [PATCH 1/2] - Fix #1193: log-servfail fails to log host SERVFAIL responses in Unbound 1.19.2 on Ubuntu 24.04.1 LTS, by not considering cached failures when trying to reply with expired data. --- services/mesh.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/services/mesh.c b/services/mesh.c index 5b9a02ae4..18a53b4a5 100644 --- a/services/mesh.c +++ b/services/mesh.c @@ -2203,8 +2203,13 @@ mesh_serve_expired_callback(void* arg) qstate->serve_expired_data->get_cached_answer)); msg = (*qstate->serve_expired_data->get_cached_answer)(qstate, lookup_qinfo, &is_expired); - if(!msg) + if(!msg || (FLAGS_GET_RCODE(msg->rep->flags) != LDNS_RCODE_NOERROR + && FLAGS_GET_RCODE(msg->rep->flags) != LDNS_RCODE_NXDOMAIN + && FLAGS_GET_RCODE(msg->rep->flags) != LDNS_RCODE_YXDOMAIN)) { + /* We don't care for cached failure answers at this + * stage. */ return; + } /* Reset these in case we pass a second time from here. */ encode_rep = msg->rep; memset(&actinfo, 0, sizeof(actinfo)); From c124f67f33534a9cc7a52d951865872e887a92cc Mon Sep 17 00:00:00 2001 From: Yorgos Thessalonikefs Date: Mon, 2 Dec 2024 12:30:11 +0100 Subject: [PATCH 2/2] - For #1193, introduce log-servfail.tdir and cleanup the log-servfail setting from other tests. --- testdata/iter_failreply.rpl | 1 - testdata/iter_scrub_rr_length.rpl | 1 - testdata/log_servfail.tdir/log_servfail.conf | 27 +++++++++++ testdata/log_servfail.tdir/log_servfail.dsc | 16 +++++++ testdata/log_servfail.tdir/log_servfail.post | 10 ++++ testdata/log_servfail.tdir/log_servfail.pre | 21 +++++++++ testdata/log_servfail.tdir/log_servfail.test | 47 +++++++++++++++++++ testdata/rpz_val_block.rpl | 1 - testdata/serve_expired_0ttl_nodata.rpl | 1 - testdata/serve_expired_0ttl_nxdomain.rpl | 1 - testdata/serve_expired_0ttl_servfail.rpl | 1 - testdata/serve_expired_cached_servfail.rpl | 1 - .../serve_expired_cached_servfail_refresh.rpl | 1 - .../serve_expired_client_timeout_servfail.rpl | 1 - testdata/val_failure_dnskey.rpl | 1 - testdata/val_scrub_rr_length.rpl | 1 - 16 files changed, 121 insertions(+), 11 deletions(-) create mode 100644 testdata/log_servfail.tdir/log_servfail.conf create mode 100644 testdata/log_servfail.tdir/log_servfail.dsc create mode 100644 testdata/log_servfail.tdir/log_servfail.post create mode 100644 testdata/log_servfail.tdir/log_servfail.pre create mode 100644 testdata/log_servfail.tdir/log_servfail.test diff --git a/testdata/iter_failreply.rpl b/testdata/iter_failreply.rpl index 393714196..e8ad4dd26 100644 --- a/testdata/iter_failreply.rpl +++ b/testdata/iter_failreply.rpl @@ -3,7 +3,6 @@ server: target-fetch-policy: "0 0 0 0 0" qname-minimisation: "no" minimal-responses: no - log-servfail: yes stub-zone: name: "." diff --git a/testdata/iter_scrub_rr_length.rpl b/testdata/iter_scrub_rr_length.rpl index 2ef73c2fe..ee7579f9c 100644 --- a/testdata/iter_scrub_rr_length.rpl +++ b/testdata/iter_scrub_rr_length.rpl @@ -5,7 +5,6 @@ server: minimal-responses: no rrset-roundrobin: no ede: yes - log-servfail: yes stub-zone: name: "." diff --git a/testdata/log_servfail.tdir/log_servfail.conf b/testdata/log_servfail.tdir/log_servfail.conf new file mode 100644 index 000000000..2d7c34e68 --- /dev/null +++ b/testdata/log_servfail.tdir/log_servfail.conf @@ -0,0 +1,27 @@ +server: + verbosity: 0 + use-syslog: no + directory: "" + pidfile: "unbound.pid" + chroot: "" + username: "" + do-not-query-localhost: no + use-caps-for-id: no + port: @SERVER_PORT@ + interface: 127.0.0.1 + outbound-msg-retry: 0 + + log-servfail: yes + +forward-zone: + name: "a.servfail" + forward-addr: 127.0.0.1@@SERVER_PORT@ + +forward-zone: + name: "b.servfail" + forward-addr: 127.0.0.1@@SERVER_PORT@ + +remote-control: + control-enable: yes + control-port: @CONTROL_PORT@ + control-use-cert: no diff --git a/testdata/log_servfail.tdir/log_servfail.dsc b/testdata/log_servfail.tdir/log_servfail.dsc new file mode 100644 index 000000000..cf4f455aa --- /dev/null +++ b/testdata/log_servfail.tdir/log_servfail.dsc @@ -0,0 +1,16 @@ +BaseName: log_servfail +Version: 1.0 +Description: Check the log_servfail option +CreationDate: Fri 29 Nov 11:00:00 CEST 2024 +Maintainer: +Category: +Component: +CmdDepends: +Depends: +Help: +Pre: log_servfail.pre +Post: log_servfail.post +Test: log_servfail.test +AuxFiles: +Passed: +Failure: diff --git a/testdata/log_servfail.tdir/log_servfail.post b/testdata/log_servfail.tdir/log_servfail.post new file mode 100644 index 000000000..a7bd0e88f --- /dev/null +++ b/testdata/log_servfail.tdir/log_servfail.post @@ -0,0 +1,10 @@ +# #-- log_servfail.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 +kill_from_pidfile "unbound.pid" +cat unbound.log diff --git a/testdata/log_servfail.tdir/log_servfail.pre b/testdata/log_servfail.tdir/log_servfail.pre new file mode 100644 index 000000000..540594808 --- /dev/null +++ b/testdata/log_servfail.tdir/log_servfail.pre @@ -0,0 +1,21 @@ +# #-- log_servfail.pre--# +PRE="../.." +. ../common.sh + +get_random_port 2 +SERVER_PORT=$RND_PORT +CONTROL_PORT=$(($RND_PORT + 1)) +echo "SERVER_PORT=$SERVER_PORT" >> .tpkg.var.test +echo "CONTROL_PORT=$CONTROL_PORT" >> .tpkg.var.test + +# make config file +sed \ + -e 's/@SERVER_PORT\@/'$SERVER_PORT'/' \ + -e 's/@CONTROL_PORT\@/'$CONTROL_PORT'/' \ + < log_servfail.conf > ub.conf + +# start unbound in the background +$PRE/unbound -d -c ub.conf > unbound.log 2>&1 & + +cat .tpkg.var.test +wait_unbound_up unbound.log diff --git a/testdata/log_servfail.tdir/log_servfail.test b/testdata/log_servfail.tdir/log_servfail.test new file mode 100644 index 000000000..a6156ba04 --- /dev/null +++ b/testdata/log_servfail.tdir/log_servfail.test @@ -0,0 +1,47 @@ +# #-- cookie_file.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 + +outfile=dig.out + +teststep "Check if log-servfail logs to output for iterator error" +dig a.servfail @127.0.0.1 -p $SERVER_PORT > $outfile +if ! grep "SERVFAIL" $outfile +then + cat $outfile + echo "Did not get a SERVFAIL response" + exit 1 +fi +if ! grep "SERVFAIL $outfile +if ! grep "SERVFAIL" $outfile +then + cat $outfile + echo "Did not get a SERVFAIL response" + exit 1 +fi +if ! grep "SERVFAIL