From 17d4d178b97870313995873e946450bd15b32134 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 2 Apr 2025 16:31:54 +0200 Subject: [PATCH 1/3] Add static ede context into each validator layer Instead of passing the edectx from the fetchctx into all subvalidators, make the ede context ownership explict for dns_resolver_createfetch() callers, and copy the ede result codes from the children validators to the parent when finishing the validation process. (cherry picked from commit d7593196a19497814efbe5d19dcb82adc243bd18) --- lib/dns/include/dns/validator.h | 4 +++- lib/dns/validator.c | 15 +++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/dns/include/dns/validator.h b/lib/dns/include/dns/validator.h index 1a25793212..fe9261556c 100644 --- a/lib/dns/include/dns/validator.h +++ b/lib/dns/include/dns/validator.h @@ -158,7 +158,9 @@ struct dns_validator { isc_counter_t *qc; isc_counter_t *gqc; - dns_edectx_t *edectx; + dns_edectx_t edectx; + + dns_edectx_t *cb_edectx; }; /*% diff --git a/lib/dns/validator.c b/lib/dns/validator.c index e677da1ce2..dea8128832 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -251,6 +251,8 @@ validator_done(dns_validator_t *val, isc_result_t result) { val->attributes |= VALATTR_COMPLETE; val->result = result; + dns_ede_copy(val->cb_edectx, &val->edectx); + isc_async_run(val->loop, val->cb, val); } @@ -962,7 +964,7 @@ create_fetch(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, result = dns_resolver_createfetch( val->view->resolver, name, type, NULL, NULL, NULL, NULL, 0, fopts, 0, val->qc, val->gqc, val->loop, callback, val, - val->edectx, &val->frdataset, &val->fsigrdataset, &val->fetch); + &val->edectx, &val->frdataset, &val->fsigrdataset, &val->fetch); if (result != ISC_R_SUCCESS) { dns_validator_detach(&val); } @@ -999,7 +1001,7 @@ create_validator(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, result = dns_validator_create( val->view, name, type, rdataset, sig, NULL, vopts, val->loop, cb, val, val->nvalidations, val->nfails, val->qc, val->gqc, - val->edectx, &val->subvalidator); + &val->edectx, &val->subvalidator); if (result == ISC_R_SUCCESS) { dns_validator_attach(val, &val->subvalidator->parent); val->subvalidator->depth = val->depth + 1; @@ -3425,6 +3427,7 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, REQUIRE(rdataset != NULL || (rdataset == NULL && sigrdataset == NULL && message != NULL)); REQUIRE(validatorp != NULL && *validatorp == NULL); + REQUIRE(edectx != NULL); result = dns_view_getsecroots(view, &kt); if (result != ISC_R_SUCCESS) { @@ -3446,9 +3449,11 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, .cb = cb, .arg = arg, .rdata = DNS_RDATA_INIT, - .edectx = edectx, + .cb_edectx = edectx, }; + dns_ede_init(view->mctx, &val->edectx); + isc_refcount_init(&val->references, 1); dns_view_attach(view, &val->view); if (message != NULL) { @@ -3569,6 +3574,8 @@ destroy_validator(dns_validator_t *val) { isc_counter_detach(&val->gqc); } + dns_ede_invalidate(&val->edectx); + dns_view_detach(&val->view); isc_loop_detach(&val->loop); @@ -3690,7 +3697,7 @@ validator_addede(dns_validator_t *val, uint16_t code, const char *extra) { dns_rdatatype_totext(val->type, &b); isc_buffer_putuint8(&b, '\0'); - dns_ede_add(val->edectx, code, bdata); + dns_ede_add(&val->edectx, code, bdata); } static void From 01a579d126fcf09eb85f45ae19e30cb6ed846e19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 2 Apr 2025 17:38:31 +0200 Subject: [PATCH 2/3] Don't pass edectx from fetch_and_forget Pass NULL as edectx for the fetch_and_forget() fetches as nobody is reading the EDE contexts and it can mess the main client buffer. (cherry picked from commit fe48290140632f8ffb9da66567f5ce4a1d40c075) --- lib/ns/query.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ns/query.c b/lib/ns/query.c index 6de4d2bd01..3759ee371b 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -2820,8 +2820,8 @@ fetch_and_forget(ns_client_t *client, dns_name_t *qname, dns_rdatatype_t qtype, result = dns_resolver_createfetch( client->view->resolver, qname, qtype, NULL, NULL, NULL, peeraddr, client->message->id, options, 0, NULL, - client->query.qc, client->manager->loop, cb, client, - &client->edectx, tmprdataset, NULL, fetchp); + client->query.qc, client->manager->loop, cb, client, NULL, + tmprdataset, NULL, fetchp); if (result != ISC_R_SUCCESS) { ns_client_putrdataset(client, &tmprdataset); isc_nmhandle_detach(handlep); From 81468fca598b0a6f41bb7835dce0377dace71995 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 2 Apr 2025 18:06:52 +0200 Subject: [PATCH 3/3] Don't copy EDE codes if source is same as destination If the nested DNS validator ends up in the same fetch because of the loops, the code could be copying the EDE codes from the same source EDE context as the destination EDE context. Skip copying the EDE codes if the source and the destination is the same. (cherry picked from commit 2988ebae214846d6f8efa41d058aa34f7285a7dc) --- lib/dns/ede.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/dns/ede.c b/lib/dns/ede.c index 4b6e7faad7..22e90bb888 100644 --- a/lib/dns/ede.c +++ b/lib/dns/ede.c @@ -137,6 +137,10 @@ dns_ede_copy(dns_edectx_t *edectx_to, const dns_edectx_t *edectx_from) { REQUIRE(DNS_EDE_VALID(edectx_to)); REQUIRE(DNS_EDE_VALID(edectx_from)); + if (edectx_to == edectx_from) { + return; + } + for (size_t pos = 0; pos < DNS_EDE_MAX_ERRORS; pos++) { uint16_t fromcode;