]> git.ipfire.org Git - thirdparty/unbound.git/commitdiff
- Fix cachedb for serve-expired with serve-expired-reply-ttl.
authorW.C.A. Wijngaards <wouter@nlnetlabs.nl>
Wed, 10 Apr 2024 15:01:57 +0000 (17:01 +0200)
committerW.C.A. Wijngaards <wouter@nlnetlabs.nl>
Wed, 10 Apr 2024 15:01:57 +0000 (17:01 +0200)
cachedb/cachedb.c
doc/Changelog
doc/unbound.conf.5.in
services/cache/rrset.c
services/mesh.c
testdata/cachedb_expired.crpl
testdata/cachedb_expired_reply_ttl.crpl [new file with mode: 0644]

index f07535240db27b6c2de7f38d16703004e7abedcb..548ec198d93c89736b75c083fbca6c89e176a2cd 100644 (file)
@@ -267,10 +267,6 @@ cachedb_init(struct module_env* env, int id)
                return 0;
        }
        cachedb_env->enabled = 1;
-       if(env->cfg->serve_expired && env->cfg->serve_expired_reply_ttl)
-               log_warn(
-                       "cachedb: serve-expired-reply-ttl is set but not working for data "
-                       "originating from the external cache; 0 TTL is used for those.");
        if(env->cfg->serve_expired && env->cfg->serve_expired_client_timeout)
                log_warn(
                        "cachedb: serve-expired-client-timeout is set but not working for "
@@ -513,9 +509,38 @@ adjust_msg_ttl(struct dns_msg* msg, time_t adjust)
        }
 }
 
+/* Set the TTL of the given RRset to fixed value. */
+static void
+packed_rrset_ttl_set(struct packed_rrset_data* data, time_t ttl)
+{
+       size_t i;
+       size_t total = data->count + data->rrsig_count;
+       data->ttl = ttl;
+       for(i=0; i<total; i++) {
+               data->rr_ttl[i] = ttl;
+       }
+       data->ttl_add = 0;
+}
+
+/* Set the TTL of a DNS message and its RRs by to a fixed value. */
+static void
+set_msg_ttl(struct dns_msg* msg, time_t ttl)
+{
+       size_t i;
+       msg->rep->ttl = ttl;
+       msg->rep->prefetch_ttl = PREFETCH_TTL_CALC(msg->rep->ttl);
+       msg->rep->serve_expired_ttl = msg->rep->ttl + SERVE_EXPIRED_TTL;
+
+       for(i=0; i<msg->rep->rrset_count; i++) {
+               packed_rrset_ttl_set((struct packed_rrset_data*)msg->
+                       rep->rrsets[i]->entry.data, ttl);
+       }
+}
+
 /** convert dns message in buffer to return_msg */
 static int
-parse_data(struct module_qstate* qstate, struct sldns_buffer* buf)
+parse_data(struct module_qstate* qstate, struct sldns_buffer* buf,
+       int* msg_expired)
 {
        struct msg_parse* prs;
        struct edns_data edns;
@@ -585,6 +610,7 @@ parse_data(struct module_qstate* qstate, struct sldns_buffer* buf)
        adjust = *qstate->env->now - (time_t)timestamp;
        if(qstate->return_msg->rep->ttl < adjust) {
                verbose(VERB_ALGO, "cachedb msg expired");
+               *msg_expired = 1;
                /* If serve-expired is enabled, we still use an expired message
                 * setting the TTL to 0. */
                if(!qstate->env->cfg->serve_expired ||
@@ -619,7 +645,8 @@ parse_data(struct module_qstate* qstate, struct sldns_buffer* buf)
  * return true if lookup was successful.
  */
 static int
-cachedb_extcache_lookup(struct module_qstate* qstate, struct cachedb_env* ie)
+cachedb_extcache_lookup(struct module_qstate* qstate, struct cachedb_env* ie,
+       int* msg_expired)
 {
        char key[(CACHEDB_HASHSIZE/8)*2+1];
        calc_hash(qstate, key, sizeof(key));
@@ -636,7 +663,7 @@ cachedb_extcache_lookup(struct module_qstate* qstate, struct cachedb_env* ie)
        }
 
        /* parse dns message into return_msg */
-       if( !parse_data(qstate, qstate->env->scratch_buffer) ) {
+       if( !parse_data(qstate, qstate->env->scratch_buffer, msg_expired) ) {
                return 0;
        }
        return 1;
@@ -707,7 +734,7 @@ cachedb_intcache_lookup(struct module_qstate* qstate, struct cachedb_env* cde)
  * Store query into the internal cache of unbound.
  */
 static void
-cachedb_intcache_store(struct module_qstate* qstate)
+cachedb_intcache_store(struct module_qstate* qstate, int msg_expired)
 {
        uint32_t store_flags = qstate->query_flags;
 
@@ -715,9 +742,23 @@ cachedb_intcache_store(struct module_qstate* qstate)
                store_flags |= DNSCACHE_STORE_ZEROTTL;
        if(!qstate->return_msg)
                return;
+       if(msg_expired) {
+               /* Set TTLs to a value such that value + *env->now is
+                * going to be now-3 seconds. Making it expired
+                * in the cache. */
+               set_msg_ttl(qstate->return_msg, (time_t)-3);
+       }
        (void)dns_cache_store(qstate->env, &qstate->qinfo,
                qstate->return_msg->rep, 0, qstate->prefetch_leeway, 0,
                qstate->region, store_flags, qstate->qstarttime);
+       if(msg_expired) {
+               /* set TTLs to zero again */
+               adjust_msg_ttl(qstate->return_msg, -1);
+               /* Send serve expired responses based on the cachedb
+                * returned message, that was just stored in the cache.
+                * It can then continue to work on this query. */
+               mesh_respond_serve_expired(qstate->mesh_info);
+       }
 }
 
 /**
@@ -733,6 +774,7 @@ cachedb_handle_query(struct module_qstate* qstate,
        struct cachedb_qstate* ATTR_UNUSED(iq),
        struct cachedb_env* ie, int id)
 {
+       int msg_expired = 0;
        qstate->is_cachedb_answer = 0;
        /* check if we are enabled, and skip if so */
        if(!ie->enabled) {
@@ -767,13 +809,13 @@ cachedb_handle_query(struct module_qstate* qstate,
        }
 
        /* ask backend cache to see if we have data */
-       if(cachedb_extcache_lookup(qstate, ie)) {
+       if(cachedb_extcache_lookup(qstate, ie, &msg_expired)) {
                if(verbosity >= VERB_ALGO)
                        log_dns_msg(ie->backend->name,
                                &qstate->return_msg->qinfo,
                                qstate->return_msg->rep);
                /* store this result in internal cache */
-               cachedb_intcache_store(qstate);
+               cachedb_intcache_store(qstate, msg_expired);
                /* In case we have expired data but there is a client timer for expired
                 * answers, pass execution to next module in order to try updating the
                 * data first.
index 310e6379feae498e80805195cc9f9f3caf73f873..ee2b5fb6eec85f9b0b1c07671a03e048e4cdd236 100644 (file)
@@ -6,6 +6,7 @@
        - Add test for cachedb serve expired.
        - Extended test for cachedb serve expired.
        - Fix makefile dependencies for fake_event.c.
+       - Fix cachedb for serve-expired with serve-expired-reply-ttl.
 
 9 April 2024: Yorgos
        - Merge #1043 from xiaoxiaoafeifei: Add loongarch support; updates
index 2420712e74606af495c9861b992b79f3568f2aae..5da551baabc0ad21f351671f0719f20778a864c7 100644 (file)
@@ -2641,10 +2641,10 @@ query as usual, and stores the answer in the backend.
 .P
 This module interacts with the \fBserve\-expired\-*\fR options and will reply
 with expired data if Unbound is configured for that.  Currently the use
-of \fBserve\-expired\-client\-timeout:\fR and
-\fBserve\-expired\-reply\-ttl:\fR is not consistent for data originating from
-the external cache as these will result in a reply with 0 TTL without trying to
-update the data first, ignoring the configured values.
+of \fBserve\-expired\-client\-timeout:\fR
+is not consistent for data originating from
+the external cache as it will result in a reply without trying to
+update the data first, ignoring the configured value.
 .P
 If Unbound was built with
 \fB\-\-with\-libhiredis\fR
@@ -2703,7 +2703,7 @@ is retrieved. The default is no.
 .TP
 .B cachedb-check-when-serve-expired: \fI<yes or no>\fR
 If enabled, the cachedb is checked before an expired response is returned.
-When serve-expired is enabled, without serve-expired-client-timeout, it then
+When \fBserve\-expired\fR is enabled, without \fBserve\-expired\-client\-timeout\fR, it then
 does not immediately respond with an expired response from cache, but instead
 first checks the cachedb for valid contents, and if so returns it. If the
 cachedb also has no valid contents, the serve expired response is sent.
index 4e3d08bdaaf5b04ed7d37d362ca7e648d8e609f3..f41a1955cfc6646d055c437c5230c9167ae414fa 100644 (file)
@@ -127,6 +127,9 @@ need_to_update_rrset(void* nd, void* cd, time_t timenow, int equal, int ns)
 {
        struct packed_rrset_data* newd = (struct packed_rrset_data*)nd;
        struct packed_rrset_data* cached = (struct packed_rrset_data*)cd;
+       /*      o if new data is expired, current data is better */
+       if( newd->ttl < timenow && cached->ttl >= timenow)
+               return 0;
        /*      o store if rrset has been validated 
         *              everything better than bogus data 
         *              secure is preferred */
index 80d4a5aa5d82cdbe930e0e829421cf4ade8f6e00..eb4315a495bb0e82c1648a1e00e632882719d5a3 100644 (file)
@@ -2264,6 +2264,8 @@ mesh_serve_expired_callback(void* arg)
 void
 mesh_respond_serve_expired(struct mesh_state* mstate)
 {
+       if(!mstate->s.serve_expired_data)
+               mesh_serve_expired_init(mstate, -1);
        mesh_serve_expired_callback(mstate);
 }
 
index 60fdba7dcec343ba4b16a56233433ad51e0c3118..9f9ff677c6d19e5b137148c1d0ce96910f73168e 100644 (file)
@@ -150,7 +150,7 @@ REPLY QR RD RA NOERROR
 SECTION QUESTION
 www2.example.com. IN A
 SECTION ANSWER
-www2.example.com. 0 IN A 1.2.3.5
+www2.example.com. 30 IN A 1.2.3.5
 ENTRY_END
 
 ; cache is expired, cachedb has no answer
@@ -232,7 +232,7 @@ REPLY QR RD RA NOERROR
 SECTION QUESTION
 www.example.com. IN A
 SECTION ANSWER
-www.example.com. 0 IN A 1.2.3.4
+www.example.com. 30 IN A 1.2.3.4
 ENTRY_END
 
 STEP 190 TRAFFIC
@@ -297,7 +297,7 @@ REPLY QR RD RA NOERROR
 SECTION QUESTION
 www.example.com. IN A
 SECTION ANSWER
-www.example.com. 0 IN A 1.2.3.4
+www.example.com. 30 IN A 1.2.3.4
 ENTRY_END
 
 STEP 290 TRAFFIC
diff --git a/testdata/cachedb_expired_reply_ttl.crpl b/testdata/cachedb_expired_reply_ttl.crpl
new file mode 100644 (file)
index 0000000..b5f3405
--- /dev/null
@@ -0,0 +1,259 @@
+; config options
+server:
+       target-fetch-policy: "0 0 0 0 0"
+       qname-minimisation: no
+       minimal-responses: no
+       serve-expired: yes
+       serve-expired-reply-ttl: 30
+       module-config: "cachedb iterator"
+
+cachedb:
+       backend: "testframe"
+       secret-seed: "testvalue"
+       cachedb-check-when-serve-expired: yes
+
+stub-zone:
+       name: "."
+       stub-addr: 193.0.14.129
+CONFIG_END
+
+SCENARIO_BEGIN Test cachedb and serve-expired-reply-ttl.
+
+; K.ROOT-SERVERS.NET.
+RANGE_BEGIN 0 400
+       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 400
+       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 400
+       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
+
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 10 IN A 1.2.3.5
+ENTRY_END
+RANGE_END
+
+; make time not 0
+STEP 2 TIME_PASSES ELAPSE 212
+
+; Get an entry in cache, to make it expired.
+STEP 4 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
+
+; Get another query in cache to make it expired.
+STEP 20 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www2.example.com. IN A
+ENTRY_END
+
+; get the answer for it
+STEP 30 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 10 IN A 1.2.3.5
+ENTRY_END
+
+; it is now expired
+STEP 40 TIME_PASSES ELAPSE 20
+
+; cache is expired, and cachedb is expired.
+STEP 50 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www2.example.com. IN A
+ENTRY_END
+
+STEP 60 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 30 IN A 1.2.3.5
+ENTRY_END
+
+; got an answer from upstream
+STEP 61 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www2.example.com. IN A
+ENTRY_END
+
+STEP 62 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 10 IN A 1.2.3.5
+ENTRY_END
+
+; cache is expired, cachedb has no answer
+STEP 70 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+STEP 80 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 30 IN A 1.2.3.4
+ENTRY_END
+
+STEP 90 TRAFFIC
+; the entry should be refreshed in cache now.
+; cache is valid and cachedb is valid.
+STEP 100 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+STEP 110 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+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
+
+; make both cache and cachedb expired.
+STEP 120 TIME_PASSES ELAPSE 20
+STEP 130 FLUSH_MESSAGE www.example.com. IN A
+
+; cache has no entry and cachedb is expired.
+STEP 140 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+STEP 150 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 30 IN A 1.2.3.4
+ENTRY_END
+
+; the name is resolved
+STEP 160 TRAFFIC
+
+; the resolve name has been updated.
+STEP 170 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+STEP 180 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+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
+
+SCENARIO_END