From 1b3b0cef224e7a9e8279c5cfe2f7e188e3777cc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 11 Jan 2024 12:03:24 +0100 Subject: [PATCH 1/6] Split fast and slow task queues Change the taskmgr (and thus netmgr) in a way that it supports fast and slow task queues. The fast queue is used for incoming DNS traffic and it will pass the processing to the slow queue for sending outgoing DNS messages and processing resolver messages. In the future, more tasks might get moved to the slow queues, so the cached and authoritative DNS traffic can be handled without being slowed down by operations that take longer time to process. --- lib/dns/resolver.c | 4 ++-- lib/isc/include/isc/netmgr.h | 3 +++ lib/isc/netmgr/http.c | 18 ++++++++--------- lib/isc/netmgr/netmgr-int.h | 1 + lib/isc/netmgr/netmgr.c | 38 +++++++++++++++++++++++------------- lib/isc/netmgr/tcp.c | 6 +++--- lib/isc/netmgr/tcpdns.c | 4 ++-- lib/isc/netmgr/tlsdns.c | 4 ++-- lib/isc/netmgr/tlsstream.c | 12 ++++++------ lib/isc/netmgr/udp.c | 6 +++--- 10 files changed, 55 insertions(+), 41 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 4b3d1c0b40..60cac293cb 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -10408,8 +10408,8 @@ dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr, * Since we have a pool of tasks we bind them to task * queues to spread the load evenly */ - result = isc_task_create_bound(taskmgr, 0, - &res->buckets[i].task, i); + result = isc_task_create_bound( + taskmgr, 0, &res->buckets[i].task, ISC_NM_TASK_SLOW(i)); if (result != ISC_R_SUCCESS) { ntasks = i; isc_mutex_destroy(&res->buckets[i].lock); diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index eff33f6acb..d42cfe9a20 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -750,6 +750,9 @@ isc_nm_verify_tls_peer_result_string(const isc_nmhandle_t *handle); * \li 'handle' is a valid netmgr handle object. */ +#define ISC_NM_TASK_SLOW_OFFSET -2 +#define ISC_NM_TASK_SLOW(i) (ISC_NM_TASK_SLOW_OFFSET - 1 - i) + void isc_nm_task_enqueue(isc_nm_t *mgr, isc_task_t *task, int threadid); /*%< diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c index d7a33d5abe..2220edf364 100644 --- a/lib/isc/netmgr/http.c +++ b/lib/isc/netmgr/http.c @@ -2969,7 +2969,7 @@ isc__nm_http_set_max_streams(isc_nmsocket_t *listener, void isc_nm_http_set_endpoints(isc_nmsocket_t *listener, isc_nm_http_endpoints_t *eps) { - size_t nworkers; + size_t nlisteners; REQUIRE(VALID_NMSOCK(listener)); REQUIRE(listener->type == isc_nm_httplistener); @@ -2977,8 +2977,8 @@ isc_nm_http_set_endpoints(isc_nmsocket_t *listener, atomic_store(&eps->in_use, true); - nworkers = (size_t)listener->mgr->nworkers; - for (size_t i = 0; i < nworkers; i++) { + nlisteners = (size_t)listener->mgr->nlisteners; + for (size_t i = 0; i < nlisteners; i++) { isc__netievent__http_eps_t *ievent = isc__nm_get_netievent_httpendpoints(listener->mgr, listener, eps); @@ -3003,20 +3003,20 @@ isc__nm_async_httpendpoints(isc__networker_t *worker, isc__netievent_t *ev0) { static void http_init_listener_endpoints(isc_nmsocket_t *listener, isc_nm_http_endpoints_t *epset) { - size_t nworkers; + size_t nlisteners; REQUIRE(VALID_NMSOCK(listener)); REQUIRE(VALID_NM(listener->mgr)); REQUIRE(VALID_HTTP_ENDPOINTS(epset)); - nworkers = (size_t)listener->mgr->nworkers; - INSIST(nworkers > 0); + nlisteners = (size_t)listener->mgr->nlisteners; + INSIST(nlisteners > 0); listener->h2.listener_endpoints = isc_mem_get(listener->mgr->mctx, - sizeof(isc_nm_http_endpoints_t *) * nworkers); - listener->h2.n_listener_endpoints = nworkers; - for (size_t i = 0; i < nworkers; i++) { + sizeof(isc_nm_http_endpoints_t *) * nlisteners); + listener->h2.n_listener_endpoints = nlisteners; + for (size_t i = 0; i < nlisteners; i++) { listener->h2.listener_endpoints[i] = NULL; isc_nm_http_endpoints_attach( epset, &listener->h2.listener_endpoints[i]); diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 364a933128..6aca9ab92c 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -776,6 +776,7 @@ struct isc_nm { isc_refcount_t references; isc_mem_t *mctx; int nworkers; + int nlisteners; isc_mutex_t lock; isc_condition_t wkstatecond; isc_condition_t wkpausecond; diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index b19d468820..2310b4b904 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -189,12 +189,12 @@ isc__nm_force_tid(int tid) { } static void -isc__nm_threadpool_initialize(uint32_t workers) { +isc__nm_threadpool_initialize(uint32_t nworkers) { char buf[11]; int r = uv_os_getenv("UV_THREADPOOL_SIZE", buf, &(size_t){ sizeof(buf) }); if (r == UV_ENOENT) { - snprintf(buf, sizeof(buf), "%" PRIu32, workers); + snprintf(buf, sizeof(buf), "%" PRIu32, nworkers); uv_os_setenv("UV_THREADPOOL_SIZE", buf); } } @@ -212,11 +212,11 @@ isc__nm_threadpool_initialize(uint32_t workers) { #endif void -isc__netmgr_create(isc_mem_t *mctx, uint32_t workers, isc_nm_t **netmgrp) { +isc__netmgr_create(isc_mem_t *mctx, uint32_t nworkers, isc_nm_t **netmgrp) { isc_nm_t *mgr = NULL; char name[32]; - REQUIRE(workers > 0); + REQUIRE(nworkers > 0); #ifdef MAXIMAL_UV_VERSION if (uv_version() > MAXIMAL_UV_VERSION) { @@ -234,10 +234,13 @@ isc__netmgr_create(isc_mem_t *mctx, uint32_t workers, isc_nm_t **netmgrp) { uv_version_string(), UV_VERSION_STRING); } - isc__nm_threadpool_initialize(workers); + isc__nm_threadpool_initialize(nworkers); mgr = isc_mem_get(mctx, sizeof(*mgr)); - *mgr = (isc_nm_t){ .nworkers = workers }; + *mgr = (isc_nm_t){ + .nworkers = nworkers * 2, + .nlisteners = nworkers, + }; isc_mem_attach(mctx, &mgr->mctx); isc_mutex_init(&mgr->lock); @@ -272,11 +275,12 @@ isc__netmgr_create(isc_mem_t *mctx, uint32_t workers, isc_nm_t **netmgrp) { atomic_init(&mgr->keepalive, 30000); atomic_init(&mgr->advertised, 30000); - isc_barrier_init(&mgr->pausing, workers); - isc_barrier_init(&mgr->resuming, workers); + isc_barrier_init(&mgr->pausing, mgr->nworkers); + isc_barrier_init(&mgr->resuming, mgr->nworkers); - mgr->workers = isc_mem_get(mctx, workers * sizeof(isc__networker_t)); - for (size_t i = 0; i < workers; i++) { + mgr->workers = isc_mem_get(mctx, + mgr->nworkers * sizeof(isc__networker_t)); + for (int i = 0; i < mgr->nworkers; i++) { isc__networker_t *worker = &mgr->workers[i]; int r; @@ -310,7 +314,7 @@ isc__netmgr_create(isc_mem_t *mctx, uint32_t workers, isc_nm_t **netmgrp) { mgr->workers_running++; isc_thread_create(nm_thread, &mgr->workers[i], &worker->thread); - snprintf(name, sizeof(name), "isc-net-%04zu", i); + snprintf(name, sizeof(name), "isc-net-%04d", i); isc_thread_setname(worker->thread, name); } @@ -817,9 +821,15 @@ isc_nm_task_enqueue(isc_nm_t *nm, isc_task_t *task, int threadid) { isc__networker_t *worker = NULL; if (threadid == -1) { - tid = (int)isc_random_uniform(nm->nworkers); + tid = (int)isc_random_uniform(nm->nlisteners); + } else if (threadid == ISC_NM_TASK_SLOW_OFFSET) { + tid = nm->nlisteners + + (int)isc_random_uniform(nm->nworkers - nm->nlisteners); + } else if (threadid < ISC_NM_TASK_SLOW_OFFSET) { + tid = nm->nlisteners + (ISC_NM_TASK_SLOW(threadid) % + (nm->nworkers - nm->nlisteners)); } else { - tid = threadid % nm->nworkers; + tid = threadid % nm->nlisteners; } worker = &nm->workers[tid]; @@ -3778,7 +3788,7 @@ isc__nm_async_settlsctx(isc__networker_t *worker, isc__netievent_t *ev0) { static void set_tlsctx_workers(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx) { /* Update the TLS context reference for every worker thread. */ - for (size_t i = 0; i < (size_t)listener->mgr->nworkers; i++) { + for (size_t i = 0; i < (size_t)listener->mgr->nlisteners; i++) { isc__netievent__tlsctx_t *ievent = isc__nm_get_netievent_settlsctx(listener->mgr, listener, tlsctx); diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index 2a644fed3f..16b53cc579 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -341,7 +341,7 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, isc__nm_connectcb(sock, req, result, false); } else { isc__nmsocket_clearcb(sock); - sock->tid = isc_random_uniform(mgr->nworkers); + sock->tid = isc_random_uniform(mgr->nlisteners); isc__nm_connectcb(sock, req, result, true); } atomic_store(&sock->closed, true); @@ -362,7 +362,7 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, isc__nm_put_netievent_tcpconnect(mgr, ievent); } else { atomic_init(&sock->active, false); - sock->tid = isc_random_uniform(mgr->nworkers); + sock->tid = isc_random_uniform(mgr->nlisteners); isc__nm_enqueue_ievent(&mgr->workers[sock->tid], (isc__netievent_t *)ievent); } @@ -457,7 +457,7 @@ isc_nm_listentcp(isc_nm_t *mgr, isc_sockaddr_t *iface, isc__nmsocket_init(sock, mgr, isc_nm_tcplistener, iface); atomic_init(&sock->rchildren, 0); - sock->nchildren = mgr->nworkers; + sock->nchildren = mgr->nlisteners; children_size = sock->nchildren * sizeof(sock->children[0]); sock->children = isc_mem_get(mgr->mctx, children_size); memset(sock->children, 0, children_size); diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index cabc90533d..b2a0b1016d 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -324,7 +324,7 @@ isc_nm_tcpdnsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, isc__nm_put_netievent_tcpdnsconnect(mgr, ievent); } else { atomic_init(&sock->active, false); - sock->tid = isc_random_uniform(mgr->nworkers); + sock->tid = isc_random_uniform(mgr->nlisteners); isc__nm_enqueue_ievent(&mgr->workers[sock->tid], (isc__netievent_t *)ievent); } @@ -422,7 +422,7 @@ isc_nm_listentcpdns(isc_nm_t *mgr, isc_sockaddr_t *iface, isc__nmsocket_init(sock, mgr, isc_nm_tcpdnslistener, iface); atomic_init(&sock->rchildren, 0); - sock->nchildren = mgr->nworkers; + sock->nchildren = mgr->nlisteners; children_size = sock->nchildren * sizeof(sock->children[0]); sock->children = isc_mem_get(mgr->mctx, children_size); memset(sock->children, 0, children_size); diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c index cfc62eb5e9..feeb1a8d7d 100644 --- a/lib/isc/netmgr/tlsdns.c +++ b/lib/isc/netmgr/tlsdns.c @@ -419,7 +419,7 @@ isc_nm_tlsdnsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, isc__nm_put_netievent_tlsdnsconnect(mgr, ievent); } else { atomic_init(&sock->active, false); - sock->tid = isc_random_uniform(mgr->nworkers); + sock->tid = isc_random_uniform(mgr->nlisteners); isc__nm_enqueue_ievent(&mgr->workers[sock->tid], (isc__netievent_t *)ievent); } @@ -532,7 +532,7 @@ isc_nm_listentlsdns(isc_nm_t *mgr, isc_sockaddr_t *iface, isc__nmsocket_init(sock, mgr, isc_nm_tlsdnslistener, iface); atomic_init(&sock->rchildren, 0); - sock->nchildren = mgr->nworkers; + sock->nchildren = mgr->nlisteners; children_size = sock->nchildren * sizeof(sock->children[0]); sock->children = isc_mem_get(mgr->mctx, children_size); memset(sock->children, 0, children_size); diff --git a/lib/isc/netmgr/tlsstream.c b/lib/isc/netmgr/tlsstream.c index 7b490719bb..a3fc6d203c 100644 --- a/lib/isc/netmgr/tlsstream.c +++ b/lib/isc/netmgr/tlsstream.c @@ -1264,18 +1264,18 @@ isc__nm_tls_verify_tls_peer_result_string(const isc_nmhandle_t *handle) { static void tls_init_listener_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *ctx) { - size_t nworkers; + size_t nlisteners; REQUIRE(VALID_NM(listener->mgr)); REQUIRE(ctx != NULL); - nworkers = (size_t)listener->mgr->nworkers; - INSIST(nworkers > 0); + nlisteners = (size_t)listener->mgr->nlisteners; + INSIST(nlisteners > 0); listener->tlsstream.listener_tls_ctx = isc_mem_get( - listener->mgr->mctx, sizeof(isc_tlsctx_t *) * nworkers); - listener->tlsstream.n_listener_tls_ctx = nworkers; - for (size_t i = 0; i < nworkers; i++) { + listener->mgr->mctx, sizeof(isc_tlsctx_t *) * nlisteners); + listener->tlsstream.n_listener_tls_ctx = nlisteners; + for (size_t i = 0; i < nlisteners; i++) { listener->tlsstream.listener_tls_ctx[i] = NULL; isc_tlsctx_attach(ctx, &listener->tlsstream.listener_tls_ctx[i]); diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c index 476c7992f6..661de96ac6 100644 --- a/lib/isc/netmgr/udp.c +++ b/lib/isc/netmgr/udp.c @@ -157,14 +157,14 @@ isc_nm_listenudp(isc_nm_t *mgr, isc_sockaddr_t *iface, isc_nm_recv_cb_t cb, REQUIRE(VALID_NM(mgr)); /* - * We are creating mgr->nworkers duplicated sockets, one + * We are creating mgr->nlisteners duplicated sockets, one * socket for each worker thread. */ sock = isc_mem_get(mgr->mctx, sizeof(isc_nmsocket_t)); isc__nmsocket_init(sock, mgr, isc_nm_udplistener, iface); atomic_init(&sock->rchildren, 0); - sock->nchildren = mgr->nworkers; + sock->nchildren = mgr->nlisteners; children_size = sock->nchildren * sizeof(sock->children[0]); sock->children = isc_mem_get(mgr->mctx, children_size); memset(sock->children, 0, children_size); @@ -1037,7 +1037,7 @@ isc_nm_udpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, isc__nm_put_netievent_udpconnect(mgr, event); } else { atomic_init(&sock->active, false); - sock->tid = isc_random_uniform(mgr->nworkers); + sock->tid = isc_random_uniform(mgr->nlisteners); isc__nm_enqueue_ievent(&mgr->workers[sock->tid], (isc__netievent_t *)event); } From 439e16e4de525599bbb5a31575211d06cc3e2fbb Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 22 Nov 2023 16:59:03 +1100 Subject: [PATCH 2/6] Skip revoked keys when selecting DNSKEY in the validation loop Don't select revoked keys when iterating through DNSKEYs in the DNSSEC validation routines. --- lib/dns/validator.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 56a0ced7b7..7ae0b3c49c 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -1144,6 +1144,8 @@ select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) { (dns_secalg_t)dst_key_alg(val->key) && siginfo->keyid == (dns_keytag_t)dst_key_id(val->key) && + (dst_key_flags(val->key) & DNS_KEYFLAG_REVOKE) == + 0 && dst_key_iszonekey(val->key)) { if (foundold) { From 0add2934775dcfb05ea6ff9849c8c0c4a65fb009 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 22 Nov 2023 16:59:03 +1100 Subject: [PATCH 3/6] Fail processing incoming DNS message on first validation failure Stop processing the DNS validation when first validation failure occurs in the DNS message. --- lib/dns/include/dns/validator.h | 1 + lib/dns/validator.c | 21 +++++++-------------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/lib/dns/include/dns/validator.h b/lib/dns/include/dns/validator.h index 383dcb46e4..352a60a6a0 100644 --- a/lib/dns/include/dns/validator.h +++ b/lib/dns/include/dns/validator.h @@ -148,6 +148,7 @@ struct dns_validator { unsigned int depth; unsigned int authcount; unsigned int authfail; + bool failed; isc_stdtime_t start; }; diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 7ae0b3c49c..e0c3574040 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -1591,20 +1591,9 @@ validate_answer(dns_validator_t *val, bool resume) { continue; } - do { - isc_result_t tresult; - vresult = verify(val, val->key, &rdata, - val->siginfo->keyid); - if (vresult == ISC_R_SUCCESS) { - break; - } - - tresult = select_signing_key(val, val->keyset); - if (tresult != ISC_R_SUCCESS) { - break; - } - } while (1); + vresult = verify(val, val->key, &rdata, val->siginfo->keyid); if (vresult != ISC_R_SUCCESS) { + val->failed = true; validator_log(val, ISC_LOG_DEBUG(3), "failed to verify rdataset"); } else { @@ -1641,9 +1630,13 @@ validate_answer(dns_validator_t *val, bool resume) { } else { validator_log(val, ISC_LOG_DEBUG(3), "verify failure: %s", - isc_result_totext(result)); + isc_result_totext(vresult)); resume = false; } + if (val->failed) { + result = ISC_R_NOMORE; + break; + } } if (result != ISC_R_NOMORE) { validator_log(val, ISC_LOG_DEBUG(3), From 21af5c9a97ed73345799dd4dfec493cc6785e40b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 11 Jan 2024 13:34:46 +0100 Subject: [PATCH 4/6] Don't iterate from start every time we select new signing key Remember the position in the iterator when selecting the next signing key. This should speed up processing for larger DNSKEY RRSets because we don't have to iterate from start over and over again. --- lib/dns/validator.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index e0c3574040..4dd170604e 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -1104,8 +1104,8 @@ create_validator(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, * 'rdataset'. If found, build a dst_key_t for it and point val->key at * it. * - * If val->key is already non-NULL, locate it in the rdataset and then - * search past it for the *next* key that could have signed 'siginfo', then + * If val->key is already non-NULL, start searching from the next position in + * 'rdataset' to find the *next* key that could have signed 'siginfo', then * set val->key to that. * * Returns ISC_R_SUCCESS if a possible matching key has been found, @@ -1118,19 +1118,18 @@ select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) { isc_buffer_t b; dns_rdata_t rdata = DNS_RDATA_INIT; dst_key_t *oldkey = val->key; - bool foundold; if (oldkey == NULL) { - foundold = true; + result = dns_rdataset_first(rdataset); } else { - foundold = false; + dst_key_free(&oldkey); val->key = NULL; + result = dns_rdataset_next(rdataset); } - - result = dns_rdataset_first(rdataset); if (result != ISC_R_SUCCESS) { goto failure; } + do { dns_rdataset_current(rdataset, &rdata); @@ -1148,15 +1147,10 @@ select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) { 0 && dst_key_iszonekey(val->key)) { - if (foundold) { - /* - * This is the key we're looking for. - */ - return (ISC_R_SUCCESS); - } else if (dst_key_compare(oldkey, val->key)) { - foundold = true; - dst_key_free(&oldkey); - } + /* + * This is the key we're looking for. + */ + return (ISC_R_SUCCESS); } dst_key_free(&val->key); } @@ -1164,15 +1158,11 @@ select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) { result = dns_rdataset_next(rdataset); } while (result == ISC_R_SUCCESS); +failure: if (result == ISC_R_NOMORE) { result = ISC_R_NOTFOUND; } -failure: - if (oldkey != NULL) { - dst_key_free(&oldkey); - } - return (result); } From f39cd17a26959e80b3ff4ce38e98936312395610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 11 Jan 2024 13:58:52 +0100 Subject: [PATCH 5/6] Optimize selecting the signing key Don't parse the crypto data before parsing and matching the id and the algorithm for consecutive DNSKEYs. This allows us to parse the RData only in case the other parameters match allowing us to skip keys that are of no interest to us, but still would consume precious CPU time by parsing possibly garbage with OpenSSL. --- lib/dns/dst_api.c | 27 +++++++++++++++++++-------- lib/dns/include/dst/dst.h | 4 ++++ lib/dns/validator.c | 24 ++++++++++++++++-------- 3 files changed, 39 insertions(+), 16 deletions(-) diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c index 4ffda8b358..0658c691a8 100644 --- a/lib/dns/dst_api.c +++ b/lib/dns/dst_api.c @@ -164,7 +164,8 @@ computeid(dst_key_t *key); static isc_result_t frombuffer(const dns_name_t *name, unsigned int alg, unsigned int flags, unsigned int protocol, dns_rdataclass_t rdclass, - isc_buffer_t *source, isc_mem_t *mctx, dst_key_t **keyp); + isc_buffer_t *source, isc_mem_t *mctx, bool no_rdata, + dst_key_t **keyp); static isc_result_t algorithm_status(unsigned int alg); @@ -753,6 +754,13 @@ dst_key_todns(const dst_key_t *key, isc_buffer_t *target) { isc_result_t dst_key_fromdns(const dns_name_t *name, dns_rdataclass_t rdclass, isc_buffer_t *source, isc_mem_t *mctx, dst_key_t **keyp) { + return (dst_key_fromdns_ex(name, rdclass, source, mctx, false, keyp)); +} + +isc_result_t +dst_key_fromdns_ex(const dns_name_t *name, dns_rdataclass_t rdclass, + isc_buffer_t *source, isc_mem_t *mctx, bool no_rdata, + dst_key_t **keyp) { uint8_t alg, proto; uint32_t flags, extflags; dst_key_t *key = NULL; @@ -783,7 +791,7 @@ dst_key_fromdns(const dns_name_t *name, dns_rdataclass_t rdclass, } result = frombuffer(name, alg, flags, proto, rdclass, source, mctx, - &key); + no_rdata, &key); if (result != ISC_R_SUCCESS) { return (result); } @@ -804,7 +812,7 @@ dst_key_frombuffer(const dns_name_t *name, unsigned int alg, unsigned int flags, REQUIRE(dst_initialized); result = frombuffer(name, alg, flags, protocol, rdclass, source, mctx, - &key); + false, &key); if (result != ISC_R_SUCCESS) { return (result); } @@ -2351,7 +2359,8 @@ computeid(dst_key_t *key) { static isc_result_t frombuffer(const dns_name_t *name, unsigned int alg, unsigned int flags, unsigned int protocol, dns_rdataclass_t rdclass, - isc_buffer_t *source, isc_mem_t *mctx, dst_key_t **keyp) { + isc_buffer_t *source, isc_mem_t *mctx, bool no_rdata, + dst_key_t **keyp) { dst_key_t *key; isc_result_t ret; @@ -2376,10 +2385,12 @@ frombuffer(const dns_name_t *name, unsigned int alg, unsigned int flags, return (DST_R_UNSUPPORTEDALG); } - ret = key->func->fromdns(key, source); - if (ret != ISC_R_SUCCESS) { - dst_key_free(&key); - return (ret); + if (!no_rdata) { + ret = key->func->fromdns(key, source); + if (ret != ISC_R_SUCCESS) { + dst_key_free(&key); + return (ret); + } } } diff --git a/lib/dns/include/dst/dst.h b/lib/dns/include/dst/dst.h index ca292b0ef0..f845e9bd2e 100644 --- a/lib/dns/include/dst/dst.h +++ b/lib/dns/include/dst/dst.h @@ -482,6 +482,10 @@ dst_key_tofile(const dst_key_t *key, int type, const char *directory); */ isc_result_t +dst_key_fromdns_ex(const dns_name_t *name, dns_rdataclass_t rdclass, + isc_buffer_t *source, isc_mem_t *mctx, bool no_rdata, + dst_key_t **keyp); +isc_result_t dst_key_fromdns(const dns_name_t *name, dns_rdataclass_t rdclass, isc_buffer_t *source, isc_mem_t *mctx, dst_key_t **keyp); /*%< diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 4dd170604e..47c48131fb 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -1118,6 +1118,7 @@ select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) { isc_buffer_t b; dns_rdata_t rdata = DNS_RDATA_INIT; dst_key_t *oldkey = val->key; + bool no_rdata = false; if (oldkey == NULL) { result = dns_rdataset_first(rdataset); @@ -1127,7 +1128,7 @@ select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) { result = dns_rdataset_next(rdataset); } if (result != ISC_R_SUCCESS) { - goto failure; + goto done; } do { @@ -1136,8 +1137,9 @@ select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) { isc_buffer_init(&b, rdata.data, rdata.length); isc_buffer_add(&b, rdata.length); INSIST(val->key == NULL); - result = dst_key_fromdns(&siginfo->signer, rdata.rdclass, &b, - val->view->mctx, &val->key); + result = dst_key_fromdns_ex(&siginfo->signer, rdata.rdclass, &b, + val->view->mctx, no_rdata, + &val->key); if (result == ISC_R_SUCCESS) { if (siginfo->algorithm == (dns_secalg_t)dst_key_alg(val->key) && @@ -1147,18 +1149,24 @@ select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) { 0 && dst_key_iszonekey(val->key)) { - /* - * This is the key we're looking for. - */ - return (ISC_R_SUCCESS); + if (no_rdata) { + /* Retry with full key */ + dns_rdata_reset(&rdata); + dst_key_free(&val->key); + no_rdata = false; + continue; + } + /* This is the key we're looking for. */ + goto done; } dst_key_free(&val->key); } dns_rdata_reset(&rdata); result = dns_rdataset_next(rdataset); + no_rdata = true; } while (result == ISC_R_SUCCESS); -failure: +done: if (result == ISC_R_NOMORE) { result = ISC_R_NOTFOUND; } From c847092a17b98e14e1e1efea7ab06ba86595756e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 16 Jan 2024 11:46:29 +0100 Subject: [PATCH 6/6] Add CHANGES and release note for [GL #4424] --- CHANGES | 4 ++++ doc/notes/notes-current.rst | 31 +++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 doc/notes/notes-current.rst diff --git a/CHANGES b/CHANGES index c85c39a0b1..3800e70930 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6322. [security] Specific DNS answers could cause a denial-of-service + condition due to DNS validation taking a long time. + (CVE-2023-50387) [GL #4424] + 6321. [security] Change 6315 inadvertently introduced regressions that could cause named to crash. [GL #4234] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst new file mode 100644 index 0000000000..dbc1c7bdec --- /dev/null +++ b/doc/notes/notes-current.rst @@ -0,0 +1,31 @@ +.. 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. + +Notes for BIND 9.18.23 +---------------------- + +Security Fixes +~~~~~~~~~~~~~~ + +- Validating DNS messages containing a lot of DNSSEC signatures could + cause excessive CPU load, leading to a denial-of-service condition. + This has been fixed. :cve:`2023-50387` + + ISC would like to thank Elias Heftrig, Haya Schulmann, Niklas Vogel, + and Michael Waidner from the German National Research Center for + Applied Cybersecurity ATHENE. :gl:`#4424` + +Known Issues +~~~~~~~~~~~~ + +- There are no new known issues with this release. See :ref:`above + ` for a list of all known issues affecting this + BIND 9 branch.