From 17e5dd61312c28f7171c97d21d929e3e6d0ee249 Mon Sep 17 00:00:00 2001 From: "W.C.A. Wijngaards" Date: Fri, 21 Oct 2022 10:11:47 +0200 Subject: [PATCH] - Fix that cachedb does not store failures in the external cache. --- cachedb/cachedb.c | 9 + doc/Changelog | 3 + testdata/03-testbound.tdir/03-testbound.test | 9 + testdata/cachedb_servfail_cname.crpl | 181 +++++++++++++++++++ 4 files changed, 202 insertions(+) create mode 100644 testdata/cachedb_servfail_cname.crpl diff --git a/cachedb/cachedb.c b/cachedb/cachedb.c index b07743d85..6f987fc03 100644 --- 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 a30373f52..ee5c146bd 100644 --- 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 00d362287..b9fdf214d 100644 --- 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 index 000000000..221f00d4d --- /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 -- 2.47.3