intermediate commit v2; now with seemingly working private cookies

diff --git a/iterator/iterator.c b/iterator/iterator.c
index f49632cfbe3826b5b6e7a8bab3ef073ebb68ee8e..4d384bc55ede849043c94b945b54f18618f8b660 100644 (file)
--- a/iterator/iterator.c
+++ b/iterator/iterator.c
@@ -3901,14 +3901,14 @@ get_bound_ip_if(struct outside_network* outnet,
                struct sockaddr_storage addr_fake;
                socklen_t addr_fake_len = 0;
 
-
                if (!ipstrtoaddr("0.0.0.0", 0, &addr_any, &addr_any_len)) {
-                       // @TODO do something
+                       /* this shouldn't fail */
+                       return 0;
                }
 
-               if (!ipstrtoaddr("10.10.1.1", 0, &addr_fake, &addr_fake_len)) {
-                       // @TODO do something
-               }
+               // if (!ipstrtoaddr("10.10.1.1", 0, &addr_fake, &addr_fake_len)) {
+               //      // @TODO do something
+               // }
 
                log_addr(VERB_DETAIL, "!!!!! outnet->ip4_ifs->addr", &outnet->ip4_ifs->addr, bound_addrlen);
                log_addr(VERB_DETAIL, "!!!!! addr_any", &addr_any, addr_any_len);
@@ -3925,7 +3925,8 @@ get_bound_ip_if(struct outside_network* outnet,
                        memcpy(&pif_return->addr, &addr_fake, addr_fake_len);
                        pif_return->addrlen = addr_fake_len;
 
-                       log_addr(VERB_DETAIL, "!!!!! get_bound_ip_if: addr from ip4_ifs == 0.0.0.0, new is:", &pif_return->addr, outnet->ip4_ifs->addrlen);
+                       log_addr(VERB_DETAIL, "!!!!! get_bound_ip_if: addr from"
+                               " ip4_ifs == 0.0.0.0, new is:", &pif_return->addr, outnet->ip4_ifs->addrlen);
 
                        return 1;
                }
@@ -3963,6 +3964,17 @@ process_response(struct module_qstate* qstate, struct iter_qstate* iq,
        verbose(VERB_ALGO, "process_response: new external response event");
        iq->response = NULL;
        iq->state = QUERY_RESP_STATE;
+
+
+       if (event == module_event_interface_not_available) {
+               log_err("!!!!! process_response:event == module_event_interface_not_available");
+       }
+       if (!qstate->reply) {
+               log_err("!!!!! !qstate->reply");
+       }
+
+       // @TODO set renewed cookie here with infra_set_server_cookie, then bail out
+
        if(event == module_event_noreply || event == module_event_error) {
                if(event == module_event_noreply && iq->timeout_count >= 3 &&
                        qstate->env->cfg->use_caps_bits_for_id &&
@@ -3983,8 +3995,8 @@ process_response(struct module_qstate* qstate, struct iter_qstate* iq,
                }
                goto handle_it;
        }
-       if( (event != module_event_reply && event != module_event_capsfail)
-               || !qstate->reply) {
+       if( (event != module_event_reply && event != module_event_capsfail
+               && event != module_event_interface_not_available) || !qstate->reply) {
                log_err("Bad event combined with response");
                outbound_list_remove(&iq->outlist, outbound);
                errinf(qstate, "module iterator received wrong internal event with a response message");
@@ -4027,7 +4039,9 @@ process_response(struct module_qstate* qstate, struct iter_qstate* iq,
                        struct port_if pif;
                        struct port_if *pif_ptr = &pif;
 
-                       if(getsockname(qstate->reply->c->fd,
+                       /* Get the outgoing interface to store with the cookie */
+                       if(event != module_event_interface_not_available &&
+                               getsockname(qstate->reply->c->fd,
                                (struct sockaddr *) &bound_addr,
                                        &bound_addrlen) != -1) {
 
@@ -4035,14 +4049,16 @@ process_response(struct module_qstate* qstate, struct iter_qstate* iq,
 
                                if (!(get_bound_ip_if(qstate->env->worker->back,
                                        &bound_addr, bound_addrlen, pif_ptr))) {
-                                       bound_addrlen = 0;
+                                       pif.addrlen = 0;
                                }
 
                                log_addr(VERB_DETAIL, "!!!!! iterator:pif addr:", &pif.addr, pif.addrlen);
 
                        } else {
-                               bound_addrlen = 0;
+                               /* Set to zero so the cookie gets renewed */
+                               pif.addrlen = 0;
                        }
+
                        /* verify this is a 'complete cookie' (client+server)
                         * (RFC9018) with the length and store the complete
                         * cookie in the infra_cache. Do nothing when the cookie
@@ -4053,9 +4069,7 @@ process_response(struct module_qstate* qstate, struct iter_qstate* iq,
                                        &qstate->reply->addr, qstate->reply->addrlen,
                                        iq->dp->name, iq->dp->namelen, pif_ptr,
                                        cookie) >= 0) {
-                               /* log_hex() uses the verbosity levels of verbose() */
-                               log_hex("complete cookie: ", cookie->opt_data,
-                                       cookie->opt_len);
+                               // @TODO do something
                        } else {
                                log_info("upstream response server cookie is not "
                                        "added to cache; dropping response");

diff --git a/services/cache/infra.c b/services/cache/infra.c
index f988a61dc53d13a680773df7b27b006ec990aa34..03a6b8c2cd342966863d17d3c70ef8220ab6ac58 100644 (file)
--- a/services/cache/infra.c
+++ b/services/cache/infra.c
@@ -740,11 +740,12 @@ infra_get_cookie(struct infra_cache* infra, struct sockaddr_storage* addr,
 
        data = (struct infra_data*) e->data;
 
-       // @TODO fix this logic. does the cookie status matter?
-       /* renew cookie if the address isn't available isn't stored */
-       // if (data->cookie->addrlen == 0) {
-       //      infra_fill_client_cookie_random(infra, &data->cookie->data);
-       // }
+       /* renew cookie if the address that is stored isn't available */
+       if (data->cookie.pif.addrlen == 0 &&
+               data->cookie.state == SERVER_COOKIE_LEARNED) {
+               infra_fill_client_cookie_random(infra, (uint8_t*) &data->cookie.data);
+               data->cookie.state == SERVER_COOKIE_UNKNOWN;
+       }
 
        memcpy(cookie, &data->cookie, sizeof(struct edns_cookie));
 
@@ -798,10 +799,19 @@ infra_set_server_cookie(struct infra_cache* infra, struct sockaddr_storage* addr
                        return -1;
                }
 
-               if (!(data->cookie.pif.addrlen == pif->addrlen) &&
-                       memcmp(&data->cookie.pif.addr, &pif->addr, pif->addrlen)){
-
-                       // @TODO do something? this _should_ only happen on reloads?
+               /* We set the local pif addrlen to 0 if the interface is not found
+                * so it must be unequal to the stored addrlen */
+               if (data->cookie.pif.addrlen != pif->addrlen &&
+                       pif->addrlen == 0){
+                       /* don't change the status, but change to cookie length
+                        * so it gets renewed during the lookup (which is 
+                        * where all the cookie creation happens) */
+                       data->cookie.pif.addrlen = 0;
+                       lock_rw_unlock(&e->lock);
+                       log_info("the interface to the upstream response server "
+                               "that was bound to this EDNS cookie has changed;"
+                               " renewing cookie");
+                       return 0;
                }
 
                /* the server cookie has changed, but the client cookie has not
@@ -813,6 +823,11 @@ infra_set_server_cookie(struct infra_cache* infra, struct sockaddr_storage* addr
 
                        verbose(VERB_ALGO, "update new server cookie from upstream");
                        lock_rw_unlock(&e->lock);
+
+                       /* log_hex() uses the verbosity levels of verbose() */
+                       log_hex("complete cookie: ", cookie->opt_data,
+                               cookie->opt_len);
+
                        return 1;
                }
 
@@ -820,7 +835,13 @@ infra_set_server_cookie(struct infra_cache* infra, struct sockaddr_storage* addr
                 * remains unchanged */
                verbose(VERB_ALGO, "correctly received indentical cookie from"
                        " upstream; don't update");
+
                lock_rw_unlock(&e->lock);
+
+               /* log_hex() uses the verbosity levels of verbose() */
+                               log_hex("complete cookie: ", cookie->opt_data,
+                                       cookie->opt_len);
+
                return 1;
        } else { /* cookie state == SERVER_COOKIE_UNKNOWN */
 
@@ -847,6 +868,10 @@ infra_set_server_cookie(struct infra_cache* infra, struct sockaddr_storage* addr
                }
                verbose(VERB_QUERY, "storing received server cookie from upstream");
                lock_rw_unlock(&e->lock);
+
+               /* log_hex() uses the verbosity levels of verbose() */
+               log_hex("complete cookie: ", cookie->opt_data,
+                       cookie->opt_len);
                return 1;
        }
 

diff --git a/services/cache/infra.h b/services/cache/infra.h
index 07fb74723644e9655feb8fd0d6285bbb80535e64..d2d3345452aab78bb914540c28c6ab86f3ca7438 100644 (file)
--- a/services/cache/infra.h
+++ b/services/cache/infra.h
@@ -91,8 +91,6 @@ struct edns_cookie {
         enum edns_cookie_state state;
         struct edns_cookie_data data;
         struct port_if pif;
-        // struct sockaddr_storage bound_addr;
-        // socklen_t bound_addrlen;
 };
 
 

diff --git a/services/mesh.c b/services/mesh.c
index 30bcf7cda15598c75e0d544751ceecdf8e8ccdd3..972e3d01fdb76d5a15a7db2e281dc7029f00258e 100644 (file)
--- a/services/mesh.c
+++ b/services/mesh.c
@@ -886,6 +886,8 @@ void mesh_report_reply(struct mesh_area* mesh, struct outbound_entry* e,
                event = module_event_noreply;
                if(what == NETEVENT_CAPSFAIL)
                        event = module_event_capsfail;
+               if(what == NETEVENT_BOUND_INTERFACE_NOT_AVAILABLE)
+                       event = module_event_interface_not_available;
        }
        mesh_run(mesh, e->qstate->mesh_info, event, e);
 }

diff --git a/services/outside_network.c b/services/outside_network.c
index a5f00b3404fbc9a74986a01e46da043136594629..196d20b9b25d3a6f9abc0398277e069b2842d309 100644 (file)
--- a/services/outside_network.c
+++ b/services/outside_network.c
@@ -2090,6 +2090,14 @@ select_ifport(struct outside_network* outnet, struct pending* pend,
                        portno, &inuse, outnet->rnd, outnet->ip_dscp);
                if(fd == -1 && !inuse) {
                        log_err("!!!! select_ifport:nonrecoverable error making socket");
+
+                       /* we need to retry sending this message with a cookie
+                        * without a bound interface. The cookie needs to be
+                        * changed as to not leak the client cookie part that
+                        * is linked to this outgoing interface. */
+                       if (pend->sq->bound_interface != NULL) {
+                               pend->sq->bound_interface_failed = 1;
+                       }
                        /* nonrecoverable error making socket */
                        return 0;
                }
@@ -2553,6 +2561,7 @@ serviced_timer_cb(void* arg)
         * will get attached by the time we get an answer. */
        return;
 delete:
+       log_err("!!!!! serviced_timer_cb:delete serviced_udp_send");
        serviced_callbacks(sq, NETEVENT_CLOSED, NULL, NULL);
 }
 
@@ -2637,6 +2646,7 @@ serviced_create(struct outside_network* outnet, sldns_buffer* buff, int dnssec,
        } else {
                sq->bound_interface = NULL;
        }
+       sq->bound_interface_failed = 0;
        sq->padding_block_size = pad_queries_block_size;
 #ifdef UNBOUND_DEBUG
        ins =
@@ -3027,6 +3037,12 @@ serviced_callbacks(struct serviced_query* sq, int error, struct comm_point* c,
                }
                sq->outnet->svcd_overhead = backlen;
        }
+
+       /* set the error to retry the cookie with a new client cookie set */
+       if (sq->bound_interface != NULL && sq->bound_interface_failed) {
+               error = NETEVENT_BOUND_INTERFACE_NOT_AVAILABLE;
+       }
+
        /* test the actual sq->cblist, because the next elem could be deleted*/
        while((p=sq->cblist) != NULL) {
                sq->cblist = p->next; /* remove this element */

diff --git a/services/outside_network.h b/services/outside_network.h
index 37162117d71eaf473b85d5eba0b9d8553e5b4990..9ba0f34bbae09b1eb8bc487056f5f966ac98581c 100644 (file)
--- a/services/outside_network.h
+++ b/services/outside_network.h
@@ -529,6 +529,11 @@ struct serviced_query {
        int busy;
        /** interface bound to the EDNS cookie @TODO fix this */
        struct port_if* bound_interface;
+       /** flag to create a retry when the opening of the socket on the
+        * bound interface failed. This enables rewriting of the cookie without 
+        * leaking the previously sent client cookie */
+       int bound_interface_failed;
+
 };
 
 /**

diff --git a/util/module.c b/util/module.c
index 6698f94971b813f28bb864f1bd89680452095e96..f44cd758e81b88800c4ce2638841286858135bcf 100644 (file)
--- a/util/module.c
+++ b/util/module.c
@@ -70,6 +70,8 @@ strmodulevent(enum module_ev e)
        case module_event_noreply: return "module_event_noreply";
        case module_event_capsfail: return "module_event_capsfail";
        case module_event_moddone: return "module_event_moddone";
+       case module_event_interface_not_available: return 
+               "module_event_interface_not_available";
        case module_event_error: return "module_event_error";
        }
        return "bad_event_value";

diff --git a/util/module.h b/util/module.h
index 013c65b02dcffa593a1b0eb8c781ef16f54d8b8f..129b50995b85851d72e550a3b02a90f83f85a0bd 100644 (file)
--- a/util/module.h
+++ b/util/module.h
@@ -579,6 +579,8 @@ enum module_ev {
        module_event_capsfail,
        /** next module is done, and its reply is awaiting you */
        module_event_moddone,
+    /** retry of the query is needed with a rewritten (client) cookie */
+    module_event_interface_not_available,
        /** error */
        module_event_error
 };

diff --git a/util/netevent.h b/util/netevent.h
index 9f4d28ba9f8f41ebd7427df5a255f99986f6edc1..a92e8196e5a887e4d54904c7f245cc87620fd636 100644 (file)
--- a/util/netevent.h
+++ b/util/netevent.h
@@ -99,6 +99,10 @@ typedef int comm_point_callback_type(struct comm_point*, void*, int,
 /** to pass write of the write packet is done to callback function
  * used when tcp_write_and_read is enabled */
 #define NETEVENT_PKT_WRITTEN -5
+/** to pass a retry event when the bound interface of a cookie has failed
+ * and a retry is needed with a rewritten (client) cookie */
+#define NETEVENT_BOUND_INTERFACE_NOT_AVAILABLE -6
+
 
 /** timeout to slow accept calls when not possible, in msec. */
 #define NETEVENT_SLOW_ACCEPT_TIME 2000