diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index de01fce32d..b491ebb27a 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -60,6 +60,7 @@ #include #include +#include #include #include #include diff --git a/bin/named/server.c b/bin/named/server.c index 8481cde8ba..3dce65e359 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -6936,7 +6936,7 @@ tat_send(void *arg) { result = dns_resolver_createfetch( tat->view->resolver, tatname, dns_rdatatype_null, domain, &nameservers, NULL, NULL, 0, 0, 0, NULL, NULL, - tat->loop, tat_done, tat, &tat->rdataset, + tat->loop, tat_done, tat, NULL, &tat->rdataset, &tat->sigrdataset, &tat->fetch); } diff --git a/lib/dns/Makefile.am b/lib/dns/Makefile.am index d287dc5503..1733fdccf4 100644 --- a/lib/dns/Makefile.am +++ b/lib/dns/Makefile.am @@ -71,6 +71,7 @@ libdns_la_HEADERS = \ include/dns/dnstap.h \ include/dns/dyndb.h \ include/dns/ecs.h \ + include/dns/ede.h \ include/dns/edns.h \ include/dns/fixedname.h \ include/dns/forward.h \ @@ -177,6 +178,7 @@ libdns_la_SOURCES = \ dst_parse.h \ dyndb.c \ ecs.c \ + ede.c \ fixedname.c \ forward.c \ gssapictx.c \ diff --git a/lib/dns/adb.c b/lib/dns/adb.c index 56b2edf52c..07b757308a 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -3009,7 +3009,7 @@ fetch_name(dns_adbname_t *adbname, bool start_at_zone, unsigned int depth, result = dns_resolver_createfetch( adb->res, adbname->name, type, name, nameservers, NULL, NULL, 0, options, depth, qc, gqc, isc_loop(), fetch_callback, adbname, - &fetch->rdataset, NULL, &fetch->fetch); + NULL, &fetch->rdataset, NULL, &fetch->fetch); if (result != ISC_R_SUCCESS) { DP(ENTER_LEVEL, "fetch_name: createfetch failed with %s", isc_result_totext(result)); diff --git a/lib/dns/client.c b/lib/dns/client.c index d9526b1bf0..cbdfc16bad 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -461,7 +461,7 @@ start_fetch(resctx_t *rctx) { result = dns_resolver_createfetch( rctx->view->resolver, dns_fixedname_name(&rctx->name), rctx->type, NULL, NULL, NULL, NULL, 0, fopts, 0, NULL, rctx->qc, - rctx->client->loop, fetch_done, rctx, rctx->rdataset, + rctx->client->loop, fetch_done, rctx, NULL, rctx->rdataset, rctx->sigrdataset, &rctx->fetch); return result; diff --git a/lib/dns/ede.c b/lib/dns/ede.c new file mode 100644 index 0000000000..d5c7e2da48 --- /dev/null +++ b/lib/dns/ede.c @@ -0,0 +1,168 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +/*! \file */ + +#include +#include + +#include + +#define DNS_EDE_MAGIC ISC_MAGIC('E', 'D', 'E', '!') +#define DNS_EDE_VALID(v) ISC_MAGIC_VALID(v, DNS_EDE_MAGIC) + +void +dns_ede_add(dns_edectx_t *edectx, uint16_t code, const char *text) { + REQUIRE(DNS_EDE_VALID(edectx)); + + size_t pos = 0; + uint16_t becode = htobe16(code); + dns_ednsopt_t *edns = NULL; + size_t textlen = 0; + + for (pos = 0; pos < DNS_EDE_MAX_ERRORS; pos++) { + edns = edectx->ede[pos]; + + if (edns == NULL) { + break; + } + + if (memcmp(&becode, edns->value, sizeof(becode)) == 0) { + isc_log_write(DNS_LOGCATEGORY_RESOLVER, + DNS_LOGMODULE_RESOLVER, ISC_LOG_DEBUG(1), + "ignoring duplicate ede %u %s", code, + text == NULL ? "(null)" : text); + return; + } + } + + if (pos >= DNS_EDE_MAX_ERRORS) { + isc_log_write(DNS_LOGCATEGORY_RESOLVER, DNS_LOGMODULE_RESOLVER, + ISC_LOG_DEBUG(1), "too many ede, ignoring %u %s", + code, text == NULL ? "(null)" : text); + return; + } + + isc_log_write(DNS_LOGCATEGORY_RESOLVER, DNS_LOGMODULE_RESOLVER, + ISC_LOG_DEBUG(1), "set ede: info-code %u extra-text %s", + code, text == NULL ? "(null)" : text); + + if (text != NULL) { + textlen = strlen(text); + + if (textlen > DNS_EDE_EXTRATEXT_LEN) { + isc_log_write(DNS_LOGCATEGORY_RESOLVER, + DNS_LOGMODULE_RESOLVER, ISC_LOG_DEBUG(1), + "truncate EDE code %hu text: %s", code, + text); + textlen = DNS_EDE_EXTRATEXT_LEN; + } + } + + edns = isc_mem_get(edectx->mctx, + sizeof(*edns) + sizeof(becode) + textlen); + *edns = (dns_ednsopt_t){ + .code = DNS_OPT_EDE, + .length = sizeof(becode) + textlen, + .value = (uint8_t *)edns + sizeof(*edns), + }; + + memmove(edns->value, &becode, sizeof(becode)); + if (textlen > 0) { + memmove(edns->value + sizeof(becode), text, textlen); + } + + edectx->ede[pos] = edns; +} + +void +dns_ede_init(isc_mem_t *mctx, dns_edectx_t *edectx) { + REQUIRE(mctx != NULL); + + /* + * Memory context is assigned, not attached here, + * thus there's no detach in dns_ede_reset(). + */ + *edectx = (dns_edectx_t){ + .magic = DNS_EDE_MAGIC, + .mctx = mctx, + }; +} + +void +dns_ede_reset(dns_edectx_t *edectx) { + REQUIRE(DNS_EDE_VALID(edectx)); + + for (size_t i = 0; i < DNS_EDE_MAX_ERRORS; i++) { + dns_ednsopt_t *edns = edectx->ede[i]; + if (edns == NULL) { + break; + } + + isc_mem_put(edectx->mctx, edns, sizeof(*edns) + edns->length); + edectx->ede[i] = NULL; + } +} + +void +dns_ede_invalidate(dns_edectx_t *edectx) { + REQUIRE(DNS_EDE_VALID(edectx)); + + dns_ede_reset(edectx); + + edectx->magic = 0; + edectx->mctx = NULL; +} + +void +dns_ede_copy(dns_edectx_t *edectx_to, dns_edectx_t *edectx_from) { + REQUIRE(DNS_EDE_VALID(edectx_to)); + REQUIRE(DNS_EDE_VALID(edectx_from)); + + size_t nextede = 0; + + for (nextede = 0; nextede < DNS_EDE_MAX_ERRORS; nextede++) { + if (edectx_to->ede[nextede] == NULL) { + break; + } + } + + for (size_t pos = 0; pos < DNS_EDE_MAX_ERRORS; pos++) { + if (edectx_from->ede[pos] == NULL) { + break; + } + + if (nextede >= DNS_EDE_MAX_ERRORS) { + isc_log_write(DNS_LOGCATEGORY_RESOLVER, + DNS_LOGMODULE_RESOLVER, ISC_LOG_DEBUG(1), + "too many ede from subfetch"); + break; + } + + INSIST(edectx_to->ede[nextede] == NULL); + + dns_ednsopt_t *edns = isc_mem_get( + edectx_to->mctx, + sizeof(*edns) + edectx_from->ede[pos]->length); + *edns = (dns_ednsopt_t){ + .code = DNS_OPT_EDE, + .length = edectx_from->ede[pos]->length, + .value = (uint8_t *)edns + sizeof(*edns), + }; + memmove(edns->value, edectx_from->ede[pos]->value, + edectx_from->ede[pos]->length); + + edectx_to->ede[nextede] = edns; + nextede++; + } +} diff --git a/lib/dns/include/dns/ede.h b/lib/dns/include/dns/ede.h new file mode 100644 index 0000000000..b14f16a715 --- /dev/null +++ b/lib/dns/include/dns/ede.h @@ -0,0 +1,55 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +#pragma once + +#include + +#include + +/* + * From RFC 8914: + * Because long EXTRA-TEXT fields may trigger truncation (which is undesirable + * given the supplemental nature of EDE), implementers and operators creating + * EDE options SHOULD avoid lengthy EXTRA-TEXT contents. + * + * Following this advice we limit the EXTRA-TEXT length to 64 characters. + */ +#define DNS_EDE_EXTRATEXT_LEN 64 + +#define DNS_EDE_MAX_ERRORS 3 + +typedef struct dns_edectx dns_edectx_t; +struct dns_edectx { + int magic; + isc_mem_t *mctx; + dns_ednsopt_t *ede[DNS_EDE_MAX_ERRORS]; +}; + +void +dns_ede_init(isc_mem_t *mctx, dns_edectx_t *edectx); + +void +dns_ede_reset(dns_edectx_t *edectx); + +void +dns_ede_invalidate(dns_edectx_t *edectx); + +void +dns_ede_add(dns_edectx_t *edectx, uint16_t code, const char *text); +/*%< + * Set extended error with INFO-CODE and EXTRA-TEXT . + */ + +void +dns_ede_copy(dns_edectx_t *edectx_to, dns_edectx_t *edectx_from); diff --git a/lib/dns/include/dns/message.h b/lib/dns/include/dns/message.h index aceab6cd74..3ce9a13af8 100644 --- a/lib/dns/include/dns/message.h +++ b/lib/dns/include/dns/message.h @@ -157,27 +157,6 @@ #define DNS_EDE_NETWORKERROR 23 /*%< Network Error */ #define DNS_EDE_INVALIDDATA 24 /*%< Invalid Data */ -typedef struct dns_ede dns_ede_t; -struct dns_ede { - uint16_t info_code; - char *extra_text; - ISC_LINK(dns_ede_t) link; -}; - -typedef ISC_LIST(dns_ede_t) dns_edelist_t; - -/* - * From RFC 8914: - * Because long EXTRA-TEXT fields may trigger truncation (which is undesirable - * given the supplemental nature of EDE), implementers and operators creating - * EDE options SHOULD avoid lengthy EXTRA-TEXT contents. - * - * Following this advice we limit the EXTRA-TEXT length to 64 characters. - */ -#define DNS_EDE_EXTRATEXT_LEN 64 - -#define DNS_EDE_MAX_ERRORS 3 - #define DNS_MESSAGE_REPLYPRESERVE (DNS_MESSAGEFLAG_RD | DNS_MESSAGEFLAG_CD) #define DNS_MESSAGEEXTFLAG_REPLYPRESERVE (DNS_MESSAGEEXTFLAG_DO) @@ -355,9 +334,9 @@ struct dns_message { }; struct dns_ednsopt { - uint16_t code; - uint16_t length; - unsigned char *value; + uint16_t code; + uint16_t length; + uint8_t *value; }; typedef void (*dns_message_cb_t)(void *arg, isc_result_t result); @@ -1500,31 +1479,3 @@ dns_message_createpools(isc_mem_t *mctx, isc_mempool_t **namepoolp, isc_mempool_t **rdspoolp); void dns_message_destroypools(isc_mempool_t **namepoolp, isc_mempool_t **rdspoolp); - -void -dns_ede_append(isc_mem_t *mctx, dns_edelist_t *list, uint16_t info_code, - const char *extra_text); -/*%< - * Adds a new EDE message at the end of 'list'. If 'extra_text' is non - * NULL, the string is synchronously copied internally, so the called - * doesn't have to keep it alive once this call returns. RFC8914 - * section 4 define the valid range of possible numbers for - * 'info_code'. - * - * Requires: - * \li mctx to be non NULL; - * \li list to be non NULL; - * \li info_code to be valid; - * \li extra_text can be NULL or non NULL, do not take ownership. - */ - -void -dns_ede_unlinkall(isc_mem_t *mctx, dns_edelist_t *list); -/*%< - * Unlink all elements from from 'list' and free it from - * memory. Optional text owned by elements is also freed. - * - * Requires: - * \li mctx to be non NULL; - * \li list to be non NULL; - */ diff --git a/lib/dns/include/dns/resolver.h b/lib/dns/include/dns/resolver.h index e9d0a537e7..52e0f6abae 100644 --- a/lib/dns/include/dns/resolver.h +++ b/lib/dns/include/dns/resolver.h @@ -55,6 +55,7 @@ #include #include +#include #include #include #include @@ -79,7 +80,7 @@ struct dns_fetchresponse { isc_mem_t *mctx; isc_result_t result; isc_result_t vresult; - dns_edelist_t edelist; + dns_edectx_t *edectx; dns_rdatatype_t qtype; dns_db_t *db; dns_dbnode_t *node; @@ -273,8 +274,8 @@ dns_resolver_createfetch(dns_resolver_t *res, const dns_name_t *name, unsigned int options, unsigned int depth, isc_counter_t *qc, isc_counter_t *gqc, isc_loop_t *loop, isc_job_cb cb, void *arg, - dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset, - dns_fetch_t **fetchp); + dns_edectx_t *edectx, dns_rdataset_t *rdataset, + dns_rdataset_t *sigrdataset, dns_fetch_t **fetchp); /*%< * Recurse to answer a question. * @@ -634,31 +635,3 @@ dns_resolver_freefresp(dns_fetchresponse_t **frespp); * Requires: * \li 'frespp' is valid. No-op if *frespp == NULL */ - -void -dns_resolver_edeappend(fetchctx_t *fctx, uint16_t info_code, const char *what, - const dns_name_t *name, dns_rdatatype_t type); -/*%< - * Helper for EDE message creation in resolver context. Creates message - * containing the "what" context message as well as the "name"/"type" being - * resolved - * - * Requires: - * \li "fctx" is valid - * \li "what" is valid - * \li "info_code" is within the range of defined EDE codes - * \li "name" is valid - */ - -void -dns_resolver_copyede(dns_fetch_t *from, fetchctx_t *to); -/*%< - * Copy all EDE messages from the fetchctx_t "from->private" to the fetchctx_t - * "to". The fetchctx_t from "from" is not locked. This is reponsability of the - * caller to lock it if this function is called in a context needing "from" - * synchronization. - * - * Requires: - * \li "from" is valid - * \li "to" is valid - */ diff --git a/lib/dns/include/dns/validator.h b/lib/dns/include/dns/validator.h index a0c87268ee..5ee6206b73 100644 --- a/lib/dns/include/dns/validator.h +++ b/lib/dns/include/dns/validator.h @@ -55,6 +55,7 @@ #include #include #include /* for dns_rdata_rrsig_t */ +#include #include #include @@ -155,11 +156,7 @@ struct dns_validator { isc_counter_t *qc; isc_counter_t *gqc; - /* - * opaque type here, used to send EDE errors during DNSSEC valiration - * to the fetch context. - */ - fetchctx_t *fctx; + dns_edectx_t *edectx; }; /*% @@ -176,8 +173,8 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, dns_message_t *message, unsigned int options, isc_loop_t *loop, isc_job_cb cb, void *arg, uint32_t *nvalidations, uint32_t *nfails, - isc_counter_t *qc, isc_counter_t *gqc, fetchctx_t *fctx, - dns_validator_t **validatorp); + isc_counter_t *qc, isc_counter_t *gqc, + dns_edectx_t *edectx, dns_validator_t **validatorp); /*%< * Start a DNSSEC validation. * diff --git a/lib/dns/message.c b/lib/dns/message.c index 28b3de2387..e174cf8a68 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -5072,47 +5072,3 @@ dns_message_destroypools(isc_mempool_t **namepoolp, isc_mempool_t **rdspoolp) { isc_mempool_destroy(rdspoolp); isc_mempool_destroy(namepoolp); } - -void -dns_ede_append(isc_mem_t *mctx, dns_edelist_t *list, uint16_t info_code, - const char *extra_text) { - REQUIRE(mctx); - REQUIRE(list); - REQUIRE(info_code <= 24); - - dns_ede_t *ede = isc_mem_get(mctx, sizeof(*ede)); - *ede = (dns_ede_t){ - .info_code = info_code, - .extra_text = NULL, - .link = ISC_LINK_INITIALIZER, - }; - - if (extra_text) { - ede->extra_text = isc_mem_allocate(mctx, - strlen(extra_text) + 1); - strcpy(ede->extra_text, extra_text); - } - - ISC_LIST_APPEND(*list, ede, link); -} - -void -dns_ede_unlinkall(isc_mem_t *mctx, dns_edelist_t *list) { - dns_ede_t *ede, *next; - - REQUIRE(mctx); - REQUIRE(list); - - for (ede = ISC_LIST_HEAD(*list); ede != NULL; ede = next) { - next = ISC_LIST_NEXT(ede, link); - - ISC_LIST_UNLINK(*list, ede, link); - if (ede->extra_text) { - isc_mem_free(mctx, ede->extra_text); - ede->extra_text = NULL; - } - isc_mem_put(mctx, ede, sizeof(*ede)); - } - - INSIST(ISC_LIST_EMPTY(*list)); -} diff --git a/lib/dns/nta.c b/lib/dns/nta.c index e428b5c7ef..43f663ce91 100644 --- a/lib/dns/nta.c +++ b/lib/dns/nta.c @@ -241,7 +241,7 @@ checkbogus(void *arg) { result = dns_resolver_createfetch( resolver, &nta->name, dns_rdatatype_nsec, NULL, NULL, NULL, NULL, 0, DNS_FETCHOPT_NONTA, 0, NULL, NULL, nta->loop, - fetch_done, nta, &nta->rdataset, &nta->sigrdataset, + fetch_done, nta, NULL, &nta->rdataset, &nta->sigrdataset, &nta->fetch); if (result != ISC_R_SUCCESS) { dns__nta_detach(&nta); /* for dns_resolver_createfetch() */ diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 61d0da0ae6..98abe47557 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -46,6 +46,7 @@ #include #include #include +#include #include #include #include @@ -341,6 +342,8 @@ struct fetchctx { isc_loop_t *loop; unsigned int tid; + dns_edectx_t edectx; + /* Atomic */ isc_refcount_t references; @@ -351,7 +354,6 @@ struct fetchctx { bool cloned; bool spilled; ISC_LIST(dns_fetchresponse_t) resps; - dns_edelist_t edelist; /*% Locked by loop event serialization. */ dns_fixedname_t dfname; @@ -977,7 +979,7 @@ valcreate(fetchctx_t *fctx, dns_message_t *message, dns_adbaddrinfo_t *addrinfo, result = dns_validator_create( fctx->res->view, name, type, rdataset, sigrdataset, message, valoptions, fctx->loop, validated, valarg, &fctx->nvalidations, - &fctx->nfails, fctx->qc, fctx->gqc, fctx, &validator); + &fctx->nfails, fctx->qc, fctx->gqc, &fctx->edectx, &validator); RUNTIME_CHECK(result == ISC_R_SUCCESS); inc_stats(fctx->res, dns_resstatscounter_val); ISC_LIST_APPEND(fctx->validators, validator, link); @@ -1585,14 +1587,11 @@ fctx_sendevents(fetchctx_t *fctx, isc_result_t result) { } /* - * Copy EDE that occured during the resolution to all - * clients. + * Finalize the EDE context, so it becomes "constant" and assign + * it to all clients. */ - for (dns_ede_t *ede = ISC_LIST_HEAD(fctx->edelist); ede != NULL; - ede = ISC_LIST_NEXT(ede, link)) - { - dns_ede_append(resp->mctx, &resp->edelist, - ede->info_code, ede->extra_text); + if (resp->edectx != NULL) { + dns_ede_copy(resp->edectx, &fctx->edectx); } FCTXTRACE("post response event"); @@ -1987,10 +1986,7 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, fctx_setretryinterval(fctx, srtt); if (isc_interval_iszero(&fctx->interval)) { FCTXTRACE("fetch expired"); - LOCK(&fctx->lock); - dns_ede_append(fctx->mctx, &fctx->edelist, - DNS_EDE_NOREACHABLEAUTH, NULL); - UNLOCK(&fctx->lock); + dns_ede_add(&fctx->edectx, DNS_EDE_NOREACHABLEAUTH, NULL); return ISC_R_TIMEDOUT; } @@ -4114,7 +4110,7 @@ fctx_try(fetchctx_t *fctx, bool retrying) { result = dns_resolver_createfetch( fctx->res, fctx->qminname, fctx->qmintype, fctx->domain, &fctx->nameservers, NULL, NULL, 0, options, 0, fctx->qc, - fctx->gqc, fctx->loop, resume_qmin, fctx, + fctx->gqc, fctx->loop, resume_qmin, fctx, &fctx->edectx, &fctx->qminrrset, NULL, &fctx->qminfetch); if (result != ISC_R_SUCCESS) { fetchctx_unref(fctx); @@ -4201,7 +4197,6 @@ resume_qmin(void *arg) { } UNLOCK(&fctx->lock); - dns_resolver_copyede(fctx->qminfetch, fctx); dns_resolver_destroyfetch(&fctx->qminfetch); /* @@ -4375,8 +4370,6 @@ fctx_destroy(fetchctx_t *fctx) { isc_mem_put(fctx->mctx, sa, sizeof(*sa)); } - dns_ede_unlinkall(fctx->mctx, &fctx->edelist); - isc_counter_detach(&fctx->qc); if (fctx->gqc != NULL) { isc_counter_detach(&fctx->gqc); @@ -4391,6 +4384,8 @@ fctx_destroy(fetchctx_t *fctx) { dns_resolver_detach(&fctx->res); + dns_ede_invalidate(&fctx->edectx); + isc_mutex_destroy(&fctx->lock); isc_mem_free(fctx->mctx, fctx->info); @@ -4409,10 +4404,7 @@ fctx_expired(void *arg) { "shut down hung fetch while resolving %p(%s)", fctx, fctx->info); - LOCK(&fctx->lock); - dns_ede_append(fctx->mctx, &fctx->edelist, DNS_EDE_NOREACHABLEAUTH, - NULL); - UNLOCK(&fctx->lock); + dns_ede_add(&fctx->edectx, DNS_EDE_NOREACHABLEAUTH, NULL); fctx_done_detach(&fctx, DNS_R_SERVFAIL); } @@ -4468,8 +4460,8 @@ detach: static void fctx_add_event(fetchctx_t *fctx, isc_loop_t *loop, const isc_sockaddr_t *client, dns_messageid_t id, isc_job_cb cb, void *arg, - dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset, - dns_fetch_t *fetch) { + dns_edectx_t *edectx, dns_rdataset_t *rdataset, + dns_rdataset_t *sigrdataset, dns_fetch_t *fetch) { dns_fetchresponse_t *resp = NULL; FCTXTRACE("addevent"); @@ -4477,7 +4469,6 @@ fctx_add_event(fetchctx_t *fctx, isc_loop_t *loop, const isc_sockaddr_t *client, resp = isc_mem_get(fctx->mctx, sizeof(*resp)); *resp = (dns_fetchresponse_t){ .result = DNS_R_SERVFAIL, - .edelist = ISC_LIST_INITIALIZER, .qtype = fctx->type, .rdataset = rdataset, .sigrdataset = sigrdataset, @@ -4488,6 +4479,7 @@ fctx_add_event(fetchctx_t *fctx, isc_loop_t *loop, const isc_sockaddr_t *client, .cb = cb, .arg = arg, .link = ISC_LINK_INITIALIZER, + .edectx = edectx, }; isc_mem_attach(fctx->mctx, &resp->mctx); @@ -4506,15 +4498,15 @@ fctx_add_event(fetchctx_t *fctx, isc_loop_t *loop, const isc_sockaddr_t *client, static void fctx_join(fetchctx_t *fctx, isc_loop_t *loop, const isc_sockaddr_t *client, - dns_messageid_t id, isc_job_cb cb, void *arg, + dns_messageid_t id, isc_job_cb cb, void *arg, dns_edectx_t *edectx, dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset, dns_fetch_t *fetch) { FCTXTRACE("join"); REQUIRE(!SHUTTINGDOWN(fctx)); - fctx_add_event(fctx, loop, client, id, cb, arg, rdataset, sigrdataset, - fetch); + fctx_add_event(fctx, loop, client, id, cb, arg, edectx, rdataset, + sigrdataset, fetch); fetch->magic = DNS_FETCH_MAGIC; fetchctx_attach(fctx, &fetch->private); @@ -4568,7 +4560,6 @@ fctx_create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, .loop = loop, .nvalidations = atomic_load_relaxed(&res->maxvalidations), .nfails = atomic_load_relaxed(&res->maxvalidationfails), - .edelist = ISC_LIST_INITIALIZER, }; isc_mem_attach(mctx, &fctx->mctx); @@ -4576,6 +4567,8 @@ fctx_create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, isc_mutex_init(&fctx->lock); + dns_ede_init(fctx->mctx, &fctx->edectx); + /* * Make fctx->info point to a copy of a formatted string * "name/type". FCTXTRACE won't work until this is done. @@ -7189,8 +7182,8 @@ resume_dslookup(void *arg) { result = dns_resolver_createfetch( res, fctx->nsname, dns_rdatatype_ns, domain, nsrdataset, NULL, NULL, 0, fctx->options, 0, fctx->qc, fctx->gqc, - loop, resume_dslookup, fctx, &fctx->nsrrset, NULL, - &fctx->nsfetch); + loop, resume_dslookup, fctx, &fctx->edectx, + &fctx->nsrrset, NULL, &fctx->nsfetch); if (result != ISC_R_SUCCESS) { fetchctx_unref(fctx); if (result == DNS_R_DUPLICATE) { @@ -7204,7 +7197,6 @@ resume_dslookup(void *arg) { } cleanup: - dns_resolver_copyede(fetch, fctx); dns_resolver_destroyfetch(&fetch); if (result != ISC_R_SUCCESS) { @@ -7952,10 +7944,8 @@ rctx_timedout(respctx_t *rctx) { if (isc_time_microdiff(&fctx->expires, &now) < US_PER_MS) { FCTXTRACE("query timed out; stopped trying to make " "fetch happen"); - LOCK(&fctx->lock); - dns_ede_append(fctx->mctx, &fctx->edelist, - DNS_EDE_NOREACHABLEAUTH, NULL); - UNLOCK(&fctx->lock); + dns_ede_add(&fctx->edectx, DNS_EDE_NOREACHABLEAUTH, + NULL); } else { FCTXTRACE("query timed out; trying next server"); /* try next server */ @@ -9621,7 +9611,8 @@ rctx_chaseds(respctx_t *rctx, dns_message_t *message, result = dns_resolver_createfetch( fctx->res, fctx->nsname, dns_rdatatype_ns, NULL, NULL, NULL, NULL, 0, fctx->options, 0, fctx->qc, fctx->gqc, fctx->loop, - resume_dslookup, fctx, &fctx->nsrrset, NULL, &fctx->nsfetch); + resume_dslookup, fctx, &fctx->edectx, &fctx->nsrrset, NULL, + &fctx->nsfetch); if (result != ISC_R_SUCCESS) { if (result == DNS_R_DUPLICATE) { result = DNS_R_SERVFAIL; @@ -10191,7 +10182,7 @@ dns_resolver_prime(dns_resolver_t *res) { result = dns_resolver_createfetch( res, dns_rootname, dns_rdatatype_ns, NULL, NULL, NULL, NULL, 0, DNS_FETCHOPT_NOFORWARD, 0, NULL, NULL, - isc_loop(), prime_done, res, rdataset, NULL, + isc_loop(), prime_done, res, NULL, rdataset, NULL, &res->primefetch); UNLOCK(&res->primelock); @@ -10467,8 +10458,8 @@ dns_resolver_createfetch(dns_resolver_t *res, const dns_name_t *name, unsigned int options, unsigned int depth, isc_counter_t *qc, isc_counter_t *gqc, isc_loop_t *loop, isc_job_cb cb, void *arg, - dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset, - dns_fetch_t **fetchp) { + dns_edectx_t *edectx, dns_rdataset_t *rdataset, + dns_rdataset_t *sigrdataset, dns_fetch_t **fetchp) { dns_fetch_t *fetch = NULL; fetchctx_t *fctx = NULL; isc_result_t result = ISC_R_SUCCESS; @@ -10568,8 +10559,8 @@ dns_resolver_createfetch(dns_resolver_t *res, const dns_name_t *name, fctx->depth = depth; } - fctx_join(fctx, loop, client, id, cb, arg, rdataset, sigrdataset, - fetch); + fctx_join(fctx, loop, client, id, cb, arg, edectx, rdataset, + sigrdataset, fetch); if (new_fctx) { fetchctx_ref(fctx); @@ -11077,7 +11068,7 @@ dns_resolver_getquerystats(dns_resolver_t *res, dns_stats_t **statsp) { void dns_resolver_freefresp(dns_fetchresponse_t **frespp) { - REQUIRE(frespp); + REQUIRE(frespp != NULL); if (*frespp == NULL) { return; @@ -11086,47 +11077,5 @@ dns_resolver_freefresp(dns_fetchresponse_t **frespp) { dns_fetchresponse_t *fresp = *frespp; *frespp = NULL; - dns_ede_unlinkall(fresp->mctx, &fresp->edelist); isc_mem_putanddetach(&fresp->mctx, fresp, sizeof(*fresp)); } - -void -dns_resolver_edeappend(fetchctx_t *fctx, uint16_t info_code, const char *what, - const dns_name_t *name, dns_rdatatype_t type) { - REQUIRE(VALID_FCTX(fctx)); - REQUIRE(what); - REQUIRE(name); - - char extra[DNS_NAME_FORMATSIZE + DNS_RDATATYPE_FORMATSIZE + - DNS_EDE_EXTRATEXT_LEN]; - size_t offset = 0; - - /* - * -2 to leave room for separator "/" and NULL terminator - */ - snprintf(extra, DNS_EDE_EXTRATEXT_LEN - 2, "%s ", what); - offset += strlen(extra); - dns_name_format(name, extra + offset, DNS_NAME_FORMATSIZE); - offset = strlcat(extra, "/", sizeof(extra)); - dns_rdatatype_format(type, extra + offset, - DNS_RDATATYPE_FORMATSIZE + 1); - - LOCK(&fctx->lock); - dns_ede_append(fctx->mctx, &fctx->edelist, info_code, extra); - UNLOCK(&fctx->lock) -} - -void -dns_resolver_copyede(dns_fetch_t *from, fetchctx_t *to) { - REQUIRE(DNS_FETCH_VALID(from)); - REQUIRE(VALID_FCTX(to)); - - LOCK(&to->lock); - for (dns_ede_t *ede = ISC_LIST_HEAD(from->private->edelist); - ede != NULL; ede = ISC_LIST_NEXT(ede, link)) - { - dns_ede_append(to->mctx, &to->edelist, ede->info_code, - ede->extra_text); - } - UNLOCK(&to->lock); -} diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 9cecd6a781..3dd9ad719e 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -405,7 +406,7 @@ fetch_callback_dnskey(void *arg) { } validator_log(val, ISC_LOG_DEBUG(3), "in fetch_callback_dnskey"); - dns_resolver_copyede(val->fetch, val->fctx); + dns_resolver_destroyfetch(&val->fetch); if (CANCELED(val) || CANCELING(val)) { @@ -480,7 +481,7 @@ fetch_callback_ds(void *arg) { } validator_log(val, ISC_LOG_DEBUG(3), "in fetch_callback_ds"); - dns_resolver_copyede(val->fetch, val->fctx); + dns_resolver_destroyfetch(&val->fetch); if (CANCELED(val) || CANCELING(val)) { @@ -933,7 +934,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->frdataset, &val->fsigrdataset, &val->fetch); + val->edectx, &val->frdataset, &val->fsigrdataset, &val->fetch); if (result != ISC_R_SUCCESS) { dns_validator_detach(&val); } @@ -967,10 +968,10 @@ create_validator(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, (DNS_VALIDATOR_NOCDFLAG | DNS_VALIDATOR_NONTA)); validator_logcreate(val, name, type, caller, "validator"); - 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->fctx, &val->subvalidator); + 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); if (result == ISC_R_SUCCESS) { dns_validator_attach(val, &val->subvalidator->parent); val->subvalidator->depth = val->depth + 1; @@ -3400,8 +3401,8 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, dns_message_t *message, unsigned int options, isc_loop_t *loop, isc_job_cb cb, void *arg, uint32_t *nvalidations, uint32_t *nfails, - isc_counter_t *qc, isc_counter_t *gqc, fetchctx_t *fctx, - dns_validator_t **validatorp) { + isc_counter_t *qc, isc_counter_t *gqc, + dns_edectx_t *edectx, dns_validator_t **validatorp) { isc_result_t result = ISC_R_FAILURE; dns_validator_t *val = NULL; dns_keytable_t *kt = NULL; @@ -3433,7 +3434,7 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, .rdata = DNS_RDATA_INIT, .nvalidations = nvalidations, .nfails = nfails, - .fctx = fctx, + .edectx = edectx, }; isc_refcount_init(&val->references, 1); @@ -3643,20 +3644,43 @@ validator_logcreate(dns_validator_t *val, dns_name_t *name, static void validate_extendederror(dns_validator_t *val) { - char txt[32]; - REQUIRE(VALID_VALIDATOR(val)); + char extra[DNS_NAME_FORMATSIZE + DNS_RDATATYPE_FORMATSIZE + + DNS_EDE_EXTRATEXT_LEN]; + isc_buffer_t b; + dns_validator_t *edeval = val; + + while (edeval->parent != NULL) { + edeval = edeval->parent; + } + if (val->unsupported_algorithm != 0) { - dns_secalg_format(val->unsupported_algorithm, txt, sizeof(txt)); - dns_resolver_edeappend(val->fctx, DNS_EDE_DNSKEYALG, txt, - val->name, val->type); + isc_buffer_init(&b, extra, sizeof(extra)); + dns_secalg_totext(val->unsupported_algorithm, &b); + + isc_buffer_putuint8(&b, ' '); + dns_name_totext(val->name, DNS_NAME_OMITFINALDOT, &b); + isc_buffer_putuint8(&b, '/'); + dns_rdatatype_totext(val->type, &b); + isc_buffer_putuint8(&b, '\0'); + + dns_ede_add(val->edectx, DNS_EDE_DNSKEYALG, extra); } if (val->unsupported_digest != 0) { - dns_dsdigest_format(val->unsupported_digest, txt, sizeof(txt)); - dns_resolver_edeappend(val->fctx, DNS_EDE_DSDIGESTTYPE, txt, - val->name, val->type); + isc_buffer_init(&b, extra, sizeof(extra)); + + dns_dsdigest_totext(val->unsupported_digest, &b); + isc_buffer_putuint8(&b, ' '); + dns_name_totext(val->name, DNS_NAME_OMITFINALDOT, &b); + isc_buffer_putuint8(&b, '/'); + dns_rdatatype_totext(val->type, &b); + isc_buffer_putuint8(&b, '\0'); + + dns_ede_add(val->edectx, DNS_EDE_DSDIGESTTYPE, extra); + + isc_buffer_invalidate(&b); } } diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 89122af6e4..db6da6b8c6 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -11069,7 +11069,8 @@ do_keyfetch(void *arg) { result = dns_resolver_createfetch( resolver, kname, dns_rdatatype_dnskey, NULL, NULL, NULL, NULL, 0, options, 0, NULL, NULL, zone->loop, keyfetch_done, kfetch, - &kfetch->dnskeyset, &kfetch->dnskeysigset, &kfetch->fetch); + NULL, &kfetch->dnskeyset, &kfetch->dnskeysigset, + &kfetch->fetch); dns_resolver_detach(&resolver); if (result == ISC_R_SUCCESS) { @@ -21944,7 +21945,7 @@ do_nsfetch(void *arg) { result = dns_resolver_createfetch( resolver, &nsfetch->pname, dns_rdatatype_ns, NULL, NULL, NULL, NULL, 0, options, 0, NULL, NULL, zone->loop, nsfetch_done, - nsfetch, &nsfetch->nsrrset, &nsfetch->nssigset, + nsfetch, NULL, &nsfetch->nsrrset, &nsfetch->nssigset, &nsfetch->fetch); dns_resolver_detach(&resolver); diff --git a/lib/ns/client.c b/lib/ns/client.c index eb74cdd961..8e82044971 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -218,92 +218,6 @@ ns_client_settimeout(ns_client_t *client, unsigned int seconds) { /* XXXWPK TODO use netmgr to set timeout */ } -static void -client_extendederror_reset(ns_client_t *client) { - for (size_t i = 0; i < DNS_EDE_MAX_ERRORS; i++) { - if (client->ede[i]) { - dns_ednsopt_t *ede = client->ede[i]; - - isc_mem_put(client->manager->mctx, ede->value, - ede->length); - isc_mem_put(client->manager->mctx, ede, sizeof(*ede)); - client->ede[i] = NULL; - } - } -} - -void -ns_client_extendederror(ns_client_t *client, uint16_t code, const char *text) { - uint16_t becode; - const uint16_t becodelen = sizeof(becode); - uint16_t textlen = 0; - size_t pos = 0; - unsigned char *ede = NULL; - dns_ednsopt_t *edns = NULL; - - REQUIRE(NS_CLIENT_VALID(client)); - - /* - * As ede will be directly put in the DNS message we need to make sure - * the code is in big-endian format - */ - becode = htobe16(code); - - for (pos = 0; pos < DNS_EDE_MAX_ERRORS; pos++) { - edns = client->ede[pos]; - - if (edns == NULL) { - break; - } - - if (memcmp(&becode, edns->value, becodelen) == 0) { - ns_client_log(client, NS_LOGCATEGORY_CLIENT, - NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(1), - "ignoring duplicate ede %u %s", code, - text == NULL ? "(null)" : text); - return; - } - } - - if (pos >= DNS_EDE_MAX_ERRORS) { - ns_client_log(client, NS_LOGCATEGORY_CLIENT, - NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(1), - "too many ede, ignoring %u %s", code, - text == NULL ? "(null)" : text); - return; - } - - ns_client_log(client, NS_LOGCATEGORY_CLIENT, NS_LOGMODULE_CLIENT, - ISC_LOG_DEBUG(1), "set ede: info-code %u extra-text %s", - code, text == NULL ? "(null)" : text); - - if (text != NULL && strlen(text) > 0) { - textlen = strlen(text); - - if (textlen > DNS_EDE_EXTRATEXT_LEN) { - ns_client_log(client, NS_LOGCATEGORY_CLIENT, - NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(1), - "truncate EDE code %hu text: %s", code, - text); - textlen = DNS_EDE_EXTRATEXT_LEN; - } - } - - ede = isc_mem_get(client->manager->mctx, becodelen + textlen); - - memcpy(ede, &becode, sizeof(code)); - if (textlen > 0) { - memcpy(ede + becodelen, text, textlen); - } - - edns = isc_mem_get(client->manager->mctx, sizeof(*edns)); - *edns = (dns_ednsopt_t){ .code = DNS_OPT_EDE, - .length = becodelen + textlen, - .value = ede }; - - client->ede[pos] = edns; -} - static void ns_client_endrequest(ns_client_t *client) { INSIST(client->state == NS_CLIENTSTATE_WORKING || @@ -344,7 +258,6 @@ ns_client_endrequest(ns_client_t *client) { dns_message_puttemprdataset(client->message, &client->opt); } - client_extendederror_reset(client); client->signer = NULL; client->udpsize = 512; client->extflags = 0; @@ -1267,7 +1180,7 @@ no_nsid: } for (size_t i = 0; i < DNS_EDE_MAX_ERRORS; i++) { - dns_ednsopt_t *ede = client->ede[i]; + dns_ednsopt_t *ede = client->edectx.ede[i]; if (ede == NULL) { break; @@ -1798,7 +1711,7 @@ ns__client_put_cb(void *client0) { * Call this first because it requires a valid client. */ ns_query_free(client); - client_extendederror_reset(client); + dns_ede_invalidate(&client->edectx); client->magic = 0; @@ -2263,7 +2176,7 @@ ns_client_request_continue(void *arg) { "no matching view in class"); } - ns_client_extendederror(client, DNS_EDE_PROHIBITED, NULL); + dns_ede_add(&client->edectx, DNS_EDE_PROHIBITED, NULL); ns_client_error(client, DNS_R_REFUSED); goto cleanup; @@ -2594,6 +2507,8 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) { */ client->magic = NS_CLIENT_MAGIC; ns_query_init(client); + + dns_ede_init(client->manager->mctx, &client->edectx); } else { REQUIRE(NS_CLIENT_VALID(client)); REQUIRE(client->manager->tid == isc_tid()); @@ -2606,8 +2521,11 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) { .magic = 0, .manager = client->manager, .message = client->message, + .edectx = client->edectx, .query = client->query, }; + + dns_ede_reset(&client->edectx); } client->query.attributes &= ~NS_QUERYATTR_ANSWERED; @@ -2783,7 +2701,7 @@ ns_client_checkacl(ns_client_t *client, isc_sockaddr_t *sockaddr, NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(3), "%s approved", opname); } else { - ns_client_extendederror(client, DNS_EDE_PROHIBITED, NULL); + dns_ede_add(&client->edectx, DNS_EDE_PROHIBITED, NULL); ns_client_log(client, DNS_LOGCATEGORY_SECURITY, NS_LOGMODULE_CLIENT, log_level, "%s denied", opname); diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h index 96f10665e1..86d1ff5b6e 100644 --- a/lib/ns/include/ns/client.h +++ b/lib/ns/include/ns/client.h @@ -180,7 +180,7 @@ struct ns_client { size_t tcpbuf_size; dns_message_t *message; dns_rdataset_t *opt; - dns_ednsopt_t *ede[DNS_EDE_MAX_ERRORS]; + dns_edectx_t edectx; uint16_t udpsize; uint16_t extflags; int16_t ednsversion; /* -1 noedns */ @@ -304,12 +304,6 @@ ns_client_error(ns_client_t *client, isc_result_t result); * will have an RCODE determined by 'result'. */ -void -ns_client_extendederror(ns_client_t *client, uint16_t code, const char *text); -/*%< - * Set extended error with INFO-CODE and EXTRA-TEXT . - */ - void ns_client_drop(ns_client_t *client, isc_result_t result); /*%< diff --git a/lib/ns/query.c b/lib/ns/query.c index 72b9a21ebd..23c7862242 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -42,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -899,7 +900,7 @@ ns_query_init(ns_client_t *client) { ISC_LIST_INIT(client->query.namebufs); ISC_LIST_INIT(client->query.activeversions); ISC_LIST_INIT(client->query.freeversions); - memset(client->ede, 0, sizeof(dns_ednsopt_t *) * DNS_EDE_MAX_ERRORS); + /* * This mutex is destroyed when the client is destroyed in * exit_check(). @@ -982,8 +983,7 @@ query_checkcacheaccess(ns_client_t *client, const dns_name_t *name, * since it is cleared by query_reset(), before query * processing starts. */ - ns_client_extendederror(client, DNS_EDE_PROHIBITED, - NULL); + dns_ede_add(&client->edectx, DNS_EDE_PROHIBITED, NULL); if (!options.nolog) { ns_client_aclmsg("query (cache)", name, qtype, @@ -1122,8 +1122,7 @@ query_validatezonedb(ns_client_t *client, const dns_name_t *name, ns_client_log(client, DNS_LOGCATEGORY_SECURITY, NS_LOGMODULE_QUERY, ISC_LOG_INFO, "%s denied", msg); - ns_client_extendederror(client, DNS_EDE_PROHIBITED, - NULL); + dns_ede_add(&client->edectx, DNS_EDE_PROHIBITED, NULL); } } @@ -1153,8 +1152,7 @@ query_validatezonedb(ns_client_t *client, const dns_name_t *name, result = ns_client_checkaclsilent(client, &client->destaddr, queryonacl, true); if (result != ISC_R_SUCCESS) { - ns_client_extendederror(client, DNS_EDE_PROHIBITED, - NULL); + dns_ede_add(&client->edectx, DNS_EDE_PROHIBITED, NULL); } if (!options.nolog && result != ISC_R_SUCCESS) { ns_client_log(client, DNS_LOGCATEGORY_SECURITY, @@ -2503,8 +2501,8 @@ validate(ns_client_t *client, dns_db_t *db, dns_name_t *name, isc_buffer_putstr(&buffer, " (cached)"); isc_buffer_putuint8(&buffer, 0); - ns_client_extendederror(client, DNS_EDE_DNSKEYALG, - isc_buffer_base(&buffer)); + dns_ede_add(&client->edectx, DNS_EDE_DNSKEYALG, + isc_buffer_base(&buffer)); continue; } if (!dns_name_issubdomain(name, &rrsig.signer)) { @@ -2813,7 +2811,7 @@ fetch_and_forget(ns_client_t *client, dns_name_t *qname, dns_rdatatype_t qtype, client->view->resolver, qname, qtype, NULL, NULL, NULL, peeraddr, client->message->id, options, 0, NULL, client->query.qc, client->manager->loop, cb, client, - tmprdataset, NULL, fetchp); + &client->edectx, tmprdataset, NULL, fetchp); if (result != ISC_R_SUCCESS) { ns_client_putrdataset(client, &tmprdataset); isc_nmhandle_detach(handlep); @@ -5937,8 +5935,8 @@ query_lookup(query_ctx_t *qctx) { stale_found ? "used" : "unavailable", isc_result_totext(result)); if (stale_found) { - ns_client_extendederror(qctx->client, ede, - "resolver failure"); + dns_ede_add(&qctx->client->edectx, ede, + "resolver failure"); } else if (!answer_found) { /* * Resolver failure, no stale data, nothing more we @@ -5961,9 +5959,8 @@ query_lookup(query_ctx_t *qctx) { isc_result_totext(result)); if (stale_found) { - ns_client_extendederror( - qctx->client, ede, - "query within stale refresh time window"); + dns_ede_add(&qctx->client->edectx, ede, + "query within stale refresh time window"); } else if (!answer_found) { /* * During the stale refresh window explicitly do not try @@ -6007,8 +6004,8 @@ query_lookup(query_ctx_t *qctx) { namebuf, typebuf); qctx->refresh_rrset = STALE(qctx->rdataset); if (stale_found) { - ns_client_extendederror( - qctx->client, ede, + dns_ede_add( + &qctx->client->edectx, ede, "stale data prioritized over " "lookup"); } @@ -6156,13 +6153,6 @@ fetch_callback(void *arg) { client->query.attributes &= ~NS_QUERYATTR_RECURSING; client->state = NS_CLIENTSTATE_WORKING; - for (dns_ede_t *ede = ISC_LIST_HEAD(resp->edelist); ede != NULL; - ede = ISC_LIST_NEXT(ede, link)) - { - ns_client_extendederror(client, ede->info_code, - ede->extra_text); - } - /* * Initialize a new qctx and use it to either resume from * recursion or clean up after cancelation. Transfer @@ -6380,7 +6370,7 @@ ns_query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname, client->view->resolver, qname, qtype, qdomain, nameservers, NULL, peeraddr, client->message->id, client->query.fetchoptions, 0, NULL, client->query.qc, client->manager->loop, - fetch_callback, client, rdataset, sigrdataset, + fetch_callback, client, &client->edectx, rdataset, sigrdataset, &FETCH_RECTYPE_NORMAL(client)); if (result != ISC_R_SUCCESS) { release_recursionquota(client); @@ -7247,8 +7237,8 @@ query_checkrpz(query_ctx_t *qctx, isc_result_t result) { if (qctx->rpz_st->m.rpz->ede != 0 && qctx->rpz_st->m.rpz->ede != UINT16_MAX) { - ns_client_extendederror(qctx->client, - qctx->rpz_st->m.rpz->ede, NULL); + dns_ede_add(&qctx->client->edectx, + qctx->rpz_st->m.rpz->ede, NULL); } /* @@ -11448,8 +11438,8 @@ ns_query_done(query_ctx_t *qctx) { */ partial_result_with_servfail = true; - ns_client_extendederror(qctx->client, 0, - "max. restarts reached"); + dns_ede_add(&qctx->client->edectx, 0, + "max. restarts reached"); ns_client_log(qctx->client, NS_LOGCATEGORY_CLIENT, NS_LOGMODULE_QUERY, ISC_LOG_INFO, "query iterations limit reached"); diff --git a/tests/dns/ede_test.c b/tests/dns/ede_test.c index 74194a4ee2..db1340f519 100644 --- a/tests/dns/ede_test.c +++ b/tests/dns/ede_test.c @@ -15,65 +15,77 @@ #include /* IWYU pragma: keep */ #include #include +#include #include +#include #include #include #define UNIT_TESTING #include -#include -#include -#include -#include -#include +#include -#include +#include + +#include "../../lib/dns/ede.c" #include -ISC_RUN_TEST_IMPL(ede_enqueue_unlink) { - dns_edelist_t list; - dns_ede_t *ede = NULL; - const char *msg1 = "abcd"; - const char *msg2 = "abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabc" - "dabcdabcdadcdabcd"; +const struct { + uint16_t info_code; + char *extra_text; +} vectors[DNS_EDE_MAX_ERRORS] = { + { + 22, + NULL, + }, + { + 12, + (char *)"abcd", + }, + { + 4, + (char *)"abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabc" + "dabcdabcdadcdabcd", + }, +}; - ISC_LIST_INIT(list); +ISC_RUN_TEST_IMPL(dns_edectx) { + dns_edectx_t edectx = { 0 }; + size_t pos = 0; + uint16_t becode; + uint8_t buf[sizeof(becode) + DNS_EDE_EXTRATEXT_LEN]; - dns_ede_append(mctx, &list, 22, NULL); - dns_ede_append(mctx, &list, 12, msg1); - dns_ede_append(mctx, &list, 4, msg2); + dns_ede_init(mctx, &edectx); - ede = ISC_LIST_HEAD(list); - assert_non_null(ede); - assert_int_equal(ede->info_code, 22); - assert_null(ede->extra_text); + for (size_t i = 0; i < DNS_EDE_MAX_ERRORS; i++) { + dns_ede_add(&edectx, vectors[i].info_code, + vectors[i].extra_text); + } - ede = ISC_LIST_NEXT(ede, link); - assert_non_null(ede); - assert_int_equal(ede->info_code, 12); - assert_string_equal(ede->extra_text, msg1); - assert_ptr_not_equal(ede->extra_text, msg1); + for (size_t i = 0; pos < DNS_EDE_MAX_ERRORS; pos++) { + dns_ednsopt_t *edns = edectx.ede[i]; + size_t textlen = 0; - /* - * Even though we limit the length of an EDE message to 64 bytes, - * this is done only at the ns/client.c level (to make sure to cover all - * the flows). - */ - ede = ISC_LIST_NEXT(ede, link); - assert_non_null(ede); - assert_int_equal(ede->info_code, 4); - assert_string_equal(ede->extra_text, msg2); - assert_ptr_not_equal(ede->extra_text, msg2); + becode = htobe16(vectors[i].info_code); + memmove(buf, &becode, sizeof(becode)); + if (vectors[i].extra_text != NULL) { + textlen = strlen(vectors[i].extra_text); + memcpy(edns->value + sizeof(becode), + vectors[i].extra_text, textlen); + } - dns_ede_unlinkall(mctx, &list); - assert_true(ISC_LIST_EMPTY(list)); + assert_memory_equal(buf, edectx.ede[i]->value, + sizeof(becode) + textlen); + } + + dns_ede_reset(&edectx); } ISC_TEST_LIST_START -ISC_TEST_ENTRY(ede_enqueue_unlink) +ISC_TEST_ENTRY(dns_edectx) ISC_TEST_LIST_END diff --git a/tests/ns/client_test.c b/tests/ns/client_test.c index 83a174c1d4..b19f8b8ed3 100644 --- a/tests/ns/client_test.c +++ b/tests/ns/client_test.c @@ -49,20 +49,14 @@ client_ede_test_initclient(void) { client->magic = NS_CLIENT_MAGIC; client->manager = &client_ede_test_dummy_manager; + dns_ede_init(mctx, &client->edectx); + return client; } static void client_ede_test_free(ns_client_t *client) { - for (size_t i = 0; i < DNS_EDE_MAX_ERRORS; i++) { - dns_ednsopt_t *ede = client->ede[i]; - - if (ede) { - isc_mem_put(mctx, ede->value, ede->length); - isc_mem_put(mctx, ede, sizeof(*ede)); - } - client->ede[i] = NULL; - } + dns_ede_reset(&client->edectx); isc_mem_put(mctx, client, sizeof(*client)); } @@ -72,14 +66,13 @@ client_ede_test_equals(const client_tests_ede_expected_t *expected, size_t count = 0; for (size_t i = 0; i < DNS_EDE_MAX_ERRORS; i++) { - dns_ednsopt_t *edns = client->ede[i]; + dns_ednsopt_t *edns = client->edectx.ede[i]; if (edns == NULL) { break; } uint16_t code; - const size_t codelen = sizeof(code); const unsigned char *txt; assert_in_range(count, 0, expected_count); @@ -88,11 +81,11 @@ client_ede_test_equals(const client_tests_ede_expected_t *expected, code = ISC_U8TO16_BE(edns->value); assert_int_equal(code, expected[count].code); - if (edns->length > codelen) { + if (edns->length > sizeof(code)) { assert_non_null(expected[count].txt); - txt = edns->value + codelen; + txt = edns->value + sizeof(code); assert_memory_equal(expected[count].txt, txt, - edns->length - codelen); + edns->length - sizeof(code)); } else { assert_null(expected[count].txt); } @@ -109,9 +102,9 @@ ISC_RUN_TEST_IMPL(client_ede_test_text_max_count) { const char *txt2 = "It's been a long time since I rock-and-rolled" "Ooh, let me get it back, let me get it back"; - ns_client_extendederror(client, 2, txt1); - ns_client_extendederror(client, 22, NULL); - ns_client_extendederror(client, 3, txt2); + dns_ede_add(&client->edectx, 2, txt1); + dns_ede_add(&client->edectx, 22, NULL); + dns_ede_add(&client->edectx, 3, txt2); const client_tests_ede_expected_t expected[3] = { { .code = 2, .txt = "foobar" }, @@ -128,11 +121,11 @@ ISC_RUN_TEST_IMPL(client_ede_test_text_max_count) { ISC_RUN_TEST_IMPL(client_ede_test_max_count) { ns_client_t *client = client_ede_test_initclient(); - ns_client_extendederror(client, 1, NULL); - ns_client_extendederror(client, 22, "two"); - ns_client_extendederror(client, 3, "three"); - ns_client_extendederror(client, 4, "four"); - ns_client_extendederror(client, 5, "five"); + dns_ede_add(&client->edectx, 1, NULL); + dns_ede_add(&client->edectx, 22, "two"); + dns_ede_add(&client->edectx, 3, "three"); + dns_ede_add(&client->edectx, 4, "four"); + dns_ede_add(&client->edectx, 5, "five"); const client_tests_ede_expected_t expected[3] = { { .code = 1, .txt = NULL }, @@ -147,9 +140,9 @@ ISC_RUN_TEST_IMPL(client_ede_test_max_count) { ISC_RUN_TEST_IMPL(client_ede_test_duplicates) { ns_client_t *client = client_ede_test_initclient(); - ns_client_extendederror(client, 1, NULL); - ns_client_extendederror(client, 1, "two"); - ns_client_extendederror(client, 1, "three"); + dns_ede_add(&client->edectx, 1, NULL); + dns_ede_add(&client->edectx, 1, "two"); + dns_ede_add(&client->edectx, 1, "three"); const client_tests_ede_expected_t expected[] = { { .code = 1, .txt = NULL },