- Fix EDNS to upstream where the same option could be attached more than

  once.
- Add a region to serviced_query for allocations.

diff --git a/services/outside_network.c b/services/outside_network.c
index d2389dd2d9501a072d8ad407b09c4b3e480b2b98..ca92707f33d09f0a589f1068b478e9ba0e8d1571 100644 (file)
--- a/services/outside_network.c
+++ b/services/outside_network.c
@@ -1714,16 +1714,7 @@ static void
 serviced_node_del(rbnode_type* node, void* ATTR_UNUSED(arg))
 {
        struct serviced_query* sq = (struct serviced_query*)node;
-       struct service_callback* p = sq->cblist, *np;
-       free(sq->qbuf);
-       free(sq->zone);
-       free(sq->tls_auth_name);
-       edns_opt_list_free(sq->opt_list);
-       while(p) {
-               np = p->next;
-               free(p);
-               p = np;
-       }
+       alloc_reg_release(sq->alloc, sq->region);
        free(sq);
 }
 
@@ -2468,7 +2459,8 @@ serviced_create(struct outside_network* outnet, sldns_buffer* buff, int dnssec,
        int want_dnssec, int nocaps, int tcp_upstream, int ssl_upstream,
        char* tls_auth_name, struct sockaddr_storage* addr, socklen_t addrlen,
        uint8_t* zone, size_t zonelen, int qtype, struct edns_option* opt_list,
-       size_t pad_queries_block_size)
+       size_t pad_queries_block_size, struct alloc_cache* alloc,
+       struct regional* region)
 {
        struct serviced_query* sq = (struct serviced_query*)malloc(sizeof(*sq));
 #ifdef UNBOUND_DEBUG
@@ -2477,15 +2469,19 @@ serviced_create(struct outside_network* outnet, sldns_buffer* buff, int dnssec,
        if(!sq) 
                return NULL;
        sq->node.key = sq;
-       sq->qbuf = memdup(sldns_buffer_begin(buff), sldns_buffer_limit(buff));
+       sq->alloc = alloc;
+       sq->region = region;
+       sq->qbuf = regional_alloc_init(region, sldns_buffer_begin(buff),
+               sldns_buffer_limit(buff));
        if(!sq->qbuf) {
+               alloc_reg_release(alloc, region);
                free(sq);
                return NULL;
        }
        sq->qbuflen = sldns_buffer_limit(buff);
-       sq->zone = memdup(zone, zonelen);
+       sq->zone = regional_alloc_init(region, zone, zonelen);
        if(!sq->zone) {
-               free(sq->qbuf);
+               alloc_reg_release(alloc, region);
                free(sq);
                return NULL;
        }
@@ -2497,10 +2493,9 @@ serviced_create(struct outside_network* outnet, sldns_buffer* buff, int dnssec,
        sq->tcp_upstream = tcp_upstream;
        sq->ssl_upstream = ssl_upstream;
        if(tls_auth_name) {
-               sq->tls_auth_name = strdup(tls_auth_name);
+               sq->tls_auth_name = regional_strdup(region, tls_auth_name);
                if(!sq->tls_auth_name) {
-                       free(sq->zone);
-                       free(sq->qbuf);
+                       alloc_reg_release(alloc, region);
                        free(sq);
                        return NULL;
                }
@@ -2509,17 +2504,7 @@ serviced_create(struct outside_network* outnet, sldns_buffer* buff, int dnssec,
        }
        memcpy(&sq->addr, addr, addrlen);
        sq->addrlen = addrlen;
-       sq->opt_list = NULL;
-       if(opt_list) {
-               sq->opt_list = edns_opt_copy_alloc(opt_list);
-               if(!sq->opt_list) {
-                       free(sq->tls_auth_name);
-                       free(sq->zone);
-                       free(sq->qbuf);
-                       free(sq);
-                       return NULL;
-               }
-       }
+       sq->opt_list = opt_list;
        sq->outnet = outnet;
        sq->cblist = NULL;
        sq->pending = NULL;
@@ -2528,7 +2513,7 @@ serviced_create(struct outside_network* outnet, sldns_buffer* buff, int dnssec,
        sq->to_be_deleted = 0;
        sq->padding_block_size = pad_queries_block_size;
 #ifdef UNBOUND_DEBUG
-       ins = 
+       ins =
 #else
        (void)
 #endif
@@ -2898,7 +2883,8 @@ serviced_callbacks(struct serviced_query* sq, int error, struct comm_point* c,
                 * use secondary buffer to store the query.
                 * This is a data copy, but faster than packet to server */
                backlen = sldns_buffer_limit(c->buffer);
-               backup_p = memdup(sldns_buffer_begin(c->buffer), backlen);
+               backup_p = regional_alloc_init(sq->region,
+                       sldns_buffer_begin(c->buffer), backlen);
                if(!backup_p) {
                        log_err("malloc failure in serviced query callbacks");
                        error = NETEVENT_CLOSED;
@@ -2916,10 +2902,8 @@ serviced_callbacks(struct serviced_query* sq, int error, struct comm_point* c,
                }
                fptr_ok(fptr_whitelist_serviced_query(p->cb));
                (void)(*p->cb)(c, p->cb_arg, error, rep);
-               free(p);
        }
        if(backup_p) {
-               free(backup_p);
                sq->outnet->svcd_overhead = 0;
        }
        verbose(VERB_ALGO, "svcd callbacks end");
@@ -3260,44 +3244,71 @@ outnet_serviced_query(struct outside_network* outnet,
        int nocaps, int tcp_upstream, int ssl_upstream, char* tls_auth_name,
        struct sockaddr_storage* addr, socklen_t addrlen, uint8_t* zone,
        size_t zonelen, struct module_qstate* qstate,
-       comm_point_callback_type* callback, void* callback_arg, sldns_buffer* buff,
-       struct module_env* env)
+       comm_point_callback_type* callback, void* callback_arg,
+       sldns_buffer* buff, struct module_env* env)
 {
        struct serviced_query* sq;
        struct service_callback* cb;
        struct edns_string_addr* client_string_addr;
-
-       if(!inplace_cb_query_call(env, qinfo, flags, addr, addrlen, zone, zonelen,
-               qstate, qstate->region))
+       struct regional* region;
+       struct edns_option* backed_up_opt_list = qstate->edns_opts_back_out;
+       struct edns_option* per_upstream_opt_list = NULL;
+
+       /* If we have an already populated EDNS option list make a copy since
+        * we may now add upstream specific EDNS options. */
+       /* Use a region that could be attached to a serviced_query, if it needs
+        * to be created. If an existing one is found then this region will be
+        * destroyed here. */
+       region = alloc_reg_obtain(env->alloc);
+       if(!region) return NULL;
+       if(qstate->edns_opts_back_out) {
+               per_upstream_opt_list = edns_opt_copy_region(
+                       qstate->edns_opts_back_out, region);
+               if(!per_upstream_opt_list) {
+                       alloc_reg_release(env->alloc, region);
                        return NULL;
+               }
+               qstate->edns_opts_back_out = per_upstream_opt_list;
+       }
+
+       if(!inplace_cb_query_call(env, qinfo, flags, addr, addrlen, zone,
+               zonelen, qstate, region)) {
+               alloc_reg_release(env->alloc, region);
+               return NULL;
+       }
+       /* Restore the option list; we can explicitly use the copied one from
+        * now on. */
+       qstate->edns_opts_back_out = backed_up_opt_list;
 
        if((client_string_addr = edns_string_addr_lookup(
                &env->edns_strings->client_strings, addr, addrlen))) {
-               edns_opt_list_append(&qstate->edns_opts_back_out,
+               edns_opt_list_append(&per_upstream_opt_list,
                        env->edns_strings->client_string_opcode,
                        client_string_addr->string_len,
-                       client_string_addr->string, qstate->region);
+                       client_string_addr->string, region);
        }
 
        serviced_gen_query(buff, qinfo->qname, qinfo->qname_len, qinfo->qtype,
                qinfo->qclass, flags);
        sq = lookup_serviced(outnet, buff, dnssec, addr, addrlen,
-               qstate->edns_opts_back_out);
-       /* duplicate entries are included in the callback list, because
-        * there is a counterpart registration by our caller that needs to
-        * be doubly-removed (with callbacks perhaps). */
-       if(!(cb = (struct service_callback*)malloc(sizeof(*cb))))
-               return NULL;
+               per_upstream_opt_list);
        if(!sq) {
                /* make new serviced query entry */
                sq = serviced_create(outnet, buff, dnssec, want_dnssec, nocaps,
                        tcp_upstream, ssl_upstream, tls_auth_name, addr,
                        addrlen, zone, zonelen, (int)qinfo->qtype,
-                       qstate->edns_opts_back_out,
+                       per_upstream_opt_list,
                        ( ssl_upstream && env->cfg->pad_queries
-                       ? env->cfg->pad_queries_block_size : 0 ));
+                       ? env->cfg->pad_queries_block_size : 0 ),
+                       env->alloc, region);
                if(!sq) {
-                       free(cb);
+                       alloc_reg_release(env->alloc, region);
+                       return NULL;
+               }
+               if(!(cb = (struct service_callback*)regional_alloc(
+                       sq->region, sizeof(*cb)))) {
+                       (void)rbtree_delete(outnet->serviced, sq);
+                       serviced_node_del(&sq->node, NULL);
                        return NULL;
                }
                /* perform first network action */
@@ -3305,17 +3316,25 @@ outnet_serviced_query(struct outside_network* outnet,
                        if(!serviced_udp_send(sq, buff)) {
                                (void)rbtree_delete(outnet->serviced, sq);
                                serviced_node_del(&sq->node, NULL);
-                               free(cb);
                                return NULL;
                        }
                } else {
                        if(!serviced_tcp_send(sq, buff)) {
                                (void)rbtree_delete(outnet->serviced, sq);
                                serviced_node_del(&sq->node, NULL);
-                               free(cb);
                                return NULL;
                        }
                }
+       } else {
+               /* We don't need this region anymore. */
+               alloc_reg_release(env->alloc, region);
+               /* duplicate entries are included in the callback list, because
+                * there is a counterpart registration by our caller that needs
+                * to be doubly-removed (with callbacks perhaps). */
+               if(!(cb = (struct service_callback*)regional_alloc(
+                       sq->region, sizeof(*cb)))) {
+                       return NULL;
+               }
        }
        /* add callback to list of callbacks */
        cb->cb = callback;
@@ -3334,7 +3353,6 @@ callback_list_remove(struct serviced_query* sq, void* cb_arg)
                if((*pp)->cb_arg == cb_arg) {
                        struct service_callback* del = *pp;
                        *pp = del->next;
-                       free(del);
                        return;
                }
                pp = &(*pp)->next;

diff --git a/services/outside_network.h b/services/outside_network.h
index d0d532e6425f1f5946629528057cf73e8997dde6..10d9d5ca6433040f11aebe8d8f1e6b89e4fa565b 100644 (file)
--- a/services/outside_network.h
+++ b/services/outside_network.h
@@ -43,7 +43,9 @@
 #ifndef OUTSIDE_NETWORK_H
 #define OUTSIDE_NETWORK_H
 
+#include "util/alloc.h"
 #include "util/rbtree.h"
+#include "util/regional.h"
 #include "util/netevent.h"
 #include "dnstap/dnstap_config.h"
 struct pending;
@@ -512,6 +514,11 @@ struct serviced_query {
        void* pending;
        /** block size with which to pad encrypted queries (default: 128) */
        size_t padding_block_size;
+       /** region for this serviced query. Will be cleared when this
+        * serviced_query will be deleted */
+       struct regional* region;
+       /** allocation service for the region */
+       struct alloc_cache* alloc;
 };
 
 /**

diff --git a/testcode/fake_event.c b/testcode/fake_event.c
index 6a8245d4a6ab432b2f22ffde24531629acf2dcf4..e1b4443bd83a43c643be7188b3c3e02b845316e2 100644 (file)
--- a/testcode/fake_event.c
+++ b/testcode/fake_event.c
@@ -1222,11 +1222,36 @@ struct serviced_query* outnet_serviced_query(struct outside_network* outnet,
        if(1) {
                struct edns_data edns;
                struct edns_string_addr* client_string_addr;
+               struct edns_option* backed_up_opt_list =
+                       qstate->edns_opts_back_out;
+               struct edns_option* per_upstream_opt_list = NULL;
+               /* If we have an already populated EDNS option list make a copy
+                * since we may now add upstream specific EDNS options. */
+               if(qstate->edns_opts_back_out) {
+                       per_upstream_opt_list = edns_opt_copy_region(
+                               qstate->edns_opts_back_out, qstate->region);
+                       if(!per_upstream_opt_list) {
+                               free(pend);
+                               fatal_exit("out of memory");
+                       }
+                       qstate->edns_opts_back_out = per_upstream_opt_list;
+               }
                if(!inplace_cb_query_call(env, qinfo, flags, addr, addrlen,
                        zone, zonelen, qstate, qstate->region)) {
                        free(pend);
                        return NULL;
                }
+               /* Restore the option list; we can explicitly use the copied
+                * one from now on. */
+               qstate->edns_opts_back_out = backed_up_opt_list;
+               if((client_string_addr = edns_string_addr_lookup(
+                       &env->edns_strings->client_strings,
+                       addr, addrlen))) {
+                       edns_opt_list_append(&per_upstream_opt_list,
+                               env->edns_strings->client_string_opcode,
+                               client_string_addr->string_len,
+                               client_string_addr->string, qstate->region);
+               }
                /* add edns */
                edns.edns_present = 1;
                edns.ext_rcode = 0;
@@ -1236,16 +1261,8 @@ struct serviced_query* outnet_serviced_query(struct outside_network* outnet,
                if(dnssec)
                        edns.bits = EDNS_DO;
                edns.padding_block_size = 0;
-               if((client_string_addr = edns_string_addr_lookup(
-                       &env->edns_strings->client_strings,
-                       addr, addrlen))) {
-                       edns_opt_list_append(&qstate->edns_opts_back_out,
-                               env->edns_strings->client_string_opcode,
-                               client_string_addr->string_len,
-                               client_string_addr->string, qstate->region);
-               }
                edns.opt_list_in = NULL;
-               edns.opt_list_out = qstate->edns_opts_back_out;
+               edns.opt_list_out = per_upstream_opt_list;
                edns.opt_list_inplace_cb_out = NULL;
                attach_edns_record(pend->buffer, &edns);
        }

diff --git a/testdata/edns_attached_once_per_upstream.rpl b/testdata/edns_attached_once_per_upstream.rpl
new file mode 100644 (file)
index 0000000..19f1ba7
--- /dev/null
+++ b/testdata/edns_attached_once_per_upstream.rpl
@@ -0,0 +1,90 @@
+; config options
+server:
+       edns-client-string: 10.0.0.0/24 "abc d"
+       outbound-msg-retry: 1
+
+stub-zone:
+       name: "edns-string-abc."
+       stub-addr: 10.0.0.3
+       stub-first: yes
+
+forward-zone:
+       name: "."
+       forward-addr: 10.0.0.1
+
+CONFIG_END
+
+SCENARIO_BEGIN Test that upstream specific EDNS is attached once; uses string tag option
+
+RANGE_BEGIN 0 1000
+       ADDRESS 10.0.0.3
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR SERVFAIL
+SECTION QUESTION
+edns-string-abc. IN A
+ENTRY_END
+RANGE_END
+
+RANGE_BEGIN 0 1000
+       ADDRESS 10.0.0.1
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+edns-string-abc. IN A
+SECTION ANSWER
+edns-string-abc. IN A 10.20.30.40
+SECTION ADDITIONAL
+ENTRY_END
+RANGE_END
+
+STEP 10 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+edns-string-abc. IN A
+ENTRY_END
+
+; This will receive SERVFAIL and the next address will be queried
+STEP 20 CHECK_OUT_QUERY ADDRESS  10.0.0.3
+ENTRY_BEGIN
+MATCH qname qtype opcode ednsdata
+SECTION QUESTION
+edns-string-abc. IN A
+SECTION ADDITIONAL
+       HEX_EDNSDATA_BEGIN
+               fd e9           ; Opcode 65001
+               00 05           ; Length 5
+               61 62 63 20 64  ; "abc d"
+       HEX_EDNSDATA_END
+ENTRY_END
+
+; This will receive the answer; makes sure that EDNS is attached once
+STEP 22 CHECK_OUT_QUERY ADDRESS  10.0.0.1
+ENTRY_BEGIN
+MATCH qname qtype opcode ednsdata
+SECTION QUESTION
+edns-string-abc. IN A
+SECTION ADDITIONAL
+       HEX_EDNSDATA_BEGIN
+               fd e9           ; Opcode 65001
+               00 05           ; Length 5
+               61 62 63 20 64  ; "abc d"
+       HEX_EDNSDATA_END
+ENTRY_END
+
+
+STEP 30 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+edns-string-abc. IN A
+SECTION ANSWER
+edns-string-abc. IN A 10.20.30.40
+ENTRY_END
+
+SCENARIO_END