From 11106a726b23f24a25d02fb7ea4a8c47efaaf6b9 Mon Sep 17 00:00:00 2001 From: TCY16 Date: Fri, 9 Dec 2022 14:34:35 +0100 Subject: [PATCH] intermediate commit v2; now with seemingly working private cookies --- iterator/iterator.c | 42 ++++++++++++++++++++++++------------- services/cache/infra.c | 43 ++++++++++++++++++++++++++++++-------- services/cache/infra.h | 2 -- services/mesh.c | 2 ++ services/outside_network.c | 16 ++++++++++++++ services/outside_network.h | 5 +++++ util/module.c | 2 ++ util/module.h | 2 ++ util/netevent.h | 4 ++++ 9 files changed, 93 insertions(+), 25 deletions(-) diff --git a/iterator/iterator.c b/iterator/iterator.c index f49632cfb..4d384bc55 100644 --- 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 f988a61dc..03a6b8c2c 100644 --- 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 07fb74723..d2d334545 100644 --- 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 30bcf7cda..972e3d01f 100644 --- 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 a5f00b340..196d20b9b 100644 --- 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 37162117d..9ba0f34bb 100644 --- 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 6698f9497..f44cd758e 100644 --- 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 013c65b02..129b50995 100644 --- 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 9f4d28ba9..a92e8196e 100644 --- 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 -- 2.47.2