- Fix that cachedb does not store failures in the external cache.

diff --git a/cachedb/cachedb.c b/cachedb/cachedb.c
index b07743d8525962b92ceeb99d7547d68a209244e2..6f987fc0301e1a5acef6eb660f7b16013324e1ba 100644 (file)
--- a/cachedb/cachedb.c
+++ b/cachedb/cachedb.c
@@ -390,6 +390,15 @@ prep_data(struct module_qstate* qstate, struct sldns_buffer* buf)
 
        if(!qstate->return_msg || !qstate->return_msg->rep)
                return 0;
+       /* do not store failures like SERVFAIL in the cachedb, this avoids
+        * overwriting expired, valid, content with broken content. */
+       if(FLAGS_GET_RCODE(qstate->return_msg->rep->flags) !=
+               LDNS_RCODE_NOERROR &&
+          FLAGS_GET_RCODE(qstate->return_msg->rep->flags) !=
+               LDNS_RCODE_NXDOMAIN &&
+          FLAGS_GET_RCODE(qstate->return_msg->rep->flags) !=
+               LDNS_RCODE_YXDOMAIN)
+               return 0;
        /* We don't store the reply if its TTL is 0 unless serve-expired is
         * enabled.  Such a reply won't be reusable and simply be a waste for
         * the backend.  It's also compatible with the default behavior of

diff --git a/doc/Changelog b/doc/Changelog
index a30373f527fa0d7340fe9523bf9d63371dc732df..ee5c146bd37a0289a7066dff27a09863150d2127 100644 (file)
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -1,3 +1,6 @@
+21 October 2022: Wouter
+       - Fix that cachedb does not store failures in the external cache.
+
 18 October 2022: George
        - Clarify the use of MAX_SENT_COUNT in the iterator code.
 

diff --git a/testdata/03-testbound.tdir/03-testbound.test b/testdata/03-testbound.tdir/03-testbound.test
index 00d362287634a7fcfff170576646c235ef481ca5..b9fdf214df82496feebfad6382aa373e6a7476bd 100644 (file)
--- a/testdata/03-testbound.tdir/03-testbound.test
+++ b/testdata/03-testbound.tdir/03-testbound.test
@@ -103,6 +103,15 @@ for input in $PRE/testdata/*.rpl $PRE/testdata/*.crpl; do
                fi
        fi
 
+       # detect if cachedb is needed
+       if echo $cleaninput | grep cachedb >/dev/null 2>&1; then
+               if grep "define USE_CACHEDB 1" $PRE/config.h >/dev/null 2>&1; then
+                       : # CACHEDB is supported
+               else
+                       continue
+               fi
+       fi
+
        if test $do_valgrind = "yes"; then
                echo
                if (valgrind $VALGRIND_FLAGS $PRE/testbound -p $input >tmpout 2>&1;); then

diff --git a/testdata/cachedb_servfail_cname.crpl b/testdata/cachedb_servfail_cname.crpl
new file mode 100644 (file)
index 0000000..221f00d
--- /dev/null
+++ b/testdata/cachedb_servfail_cname.crpl
@@ -0,0 +1,181 @@
+; config options
+server:
+       target-fetch-policy: "0 0 0 0 0"
+       qname-minimisation: no
+       minimal-responses: no
+       ;serve-expired: yes
+       module-config: "cachedb iterator"
+
+cachedb:
+       backend: "testframe"
+       secret-seed: "testvalue"
+
+stub-zone:
+       name: "."
+       stub-addr: 193.0.14.129
+CONFIG_END
+
+SCENARIO_BEGIN Test cachedb store and servfail reply from cname.
+; the servfail reply should not overwrite the cache contents.
+
+; K.ROOT-SERVERS.NET.
+RANGE_BEGIN 0 100
+       ADDRESS 193.0.14.129
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+. IN NS
+SECTION ANSWER
+. IN NS K.ROOT-SERVERS.NET.
+SECTION ADDITIONAL
+K.ROOT-SERVERS.NET.     IN      A       193.0.14.129
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode subdomain
+ADJUST copy_id copy_query
+REPLY QR NOERROR
+SECTION QUESTION
+com. IN NS
+SECTION AUTHORITY
+com. IN NS a.gtld-servers.net.
+SECTION ADDITIONAL
+a.gtld-servers.net.    IN      A       192.5.6.30
+ENTRY_END
+RANGE_END
+
+; a.gtld-servers.net.
+RANGE_BEGIN 0 100
+       ADDRESS 192.5.6.30
+ENTRY_BEGIN
+MATCH opcode subdomain
+ADJUST copy_id copy_query
+REPLY QR NOERROR
+SECTION QUESTION
+example.com. IN NS
+SECTION AUTHORITY
+example.com. IN NS ns2.example.com.
+SECTION ADDITIONAL
+ns2.example.com.       IN      A       1.2.3.5
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode subdomain
+ADJUST copy_id copy_query
+REPLY QR NOERROR
+SECTION QUESTION
+foo.com. IN NS
+SECTION AUTHORITY
+foo.com. IN NS ns.example.com.
+ENTRY_END
+RANGE_END
+
+; ns2.example.com.
+RANGE_BEGIN 0 20
+       ADDRESS 1.2.3.5
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN A 1.2.3.4
+ENTRY_END
+RANGE_END
+
+; ns2.example.com., now failing
+RANGE_BEGIN 20 100
+       ADDRESS 1.2.3.5
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN CNAME foo.example.com.
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA SERVFAIL
+SECTION QUESTION
+foo.example.com. IN A
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA SERVFAIL
+SECTION QUESTION
+ns2.example.com. IN A
+SECTION ANSWER
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA SERVFAIL
+SECTION QUESTION
+ns2.example.com. IN AAAA
+SECTION ANSWER
+ENTRY_END
+RANGE_END
+
+; get and entry in cache, to make it expired.
+STEP 1 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+; get the answer for it
+STEP 10 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN A 1.2.3.4
+ENTRY_END
+
+; it is now expired
+STEP 20 TIME_PASSES ELAPSE 20
+
+; get a servfail in cache for the destination
+STEP 30 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+foo.example.com. IN A
+ENTRY_END
+
+STEP 40 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA SERVFAIL
+SECTION QUESTION
+foo.example.com. IN A
+ENTRY_END
+
+; the query is now a CNAME to servfail.
+; there is a valid, but expired, entry in cache.
+STEP 50 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+STEP 60 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA SERVFAIL
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN CNAME foo.example.com.
+ENTRY_END
+
+SCENARIO_END