From ec28eb05db2c9b5b3c298c264bc7a3b3ffc79435 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 29 Jun 2023 09:15:38 +1000 Subject: [PATCH 01/36] Address race in dns_tsigkey_find() Restart the process with a write lock if we discover an expired key while holding the read lock. (cherry picked from commit d2ba96488ea30eaeb225c12ea77e71de399b6175) --- lib/dns/tsig.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index e102d09399..8f4c3b9d1b 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -1744,8 +1744,9 @@ isc_result_t dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, const dns_name_t *algorithm, dns_tsig_keyring_t *ring) { dns_tsigkey_t *key; - isc_stdtime_t now; isc_result_t result; + isc_rwlocktype_t locktype = isc_rwlocktype_read; + isc_stdtime_t now; REQUIRE(tsigkey != NULL); REQUIRE(*tsigkey == NULL); @@ -1757,25 +1758,30 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, RWUNLOCK(&ring->lock, isc_rwlocktype_write); isc_stdtime_get(&now); - RWLOCK(&ring->lock, isc_rwlocktype_read); + +again: + RWLOCK(&ring->lock, locktype); key = NULL; result = dns_rbt_findname(ring->keys, name, 0, NULL, (void *)&key); if (result == DNS_R_PARTIALMATCH || result == ISC_R_NOTFOUND) { - RWUNLOCK(&ring->lock, isc_rwlocktype_read); + RWUNLOCK(&ring->lock, locktype); return (ISC_R_NOTFOUND); } if (algorithm != NULL && !dns_name_equal(key->algorithm, algorithm)) { - RWUNLOCK(&ring->lock, isc_rwlocktype_read); + RWUNLOCK(&ring->lock, locktype); return (ISC_R_NOTFOUND); } if (key->inception != key->expire && isc_serial_lt(key->expire, now)) { /* * The key has expired. */ - RWUNLOCK(&ring->lock, isc_rwlocktype_read); - RWLOCK(&ring->lock, isc_rwlocktype_write); + if (locktype == isc_rwlocktype_read) { + RWUNLOCK(&ring->lock, locktype); + locktype = isc_rwlocktype_write; + goto again; + } remove_fromring(key); - RWUNLOCK(&ring->lock, isc_rwlocktype_write); + RWUNLOCK(&ring->lock, locktype); return (ISC_R_NOTFOUND); } #if 0 @@ -1790,7 +1796,7 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, } #endif /* if 0 */ isc_refcount_increment(&key->refs); - RWUNLOCK(&ring->lock, isc_rwlocktype_read); + RWUNLOCK(&ring->lock, locktype); adjust_lru(key); *tsigkey = key; return (ISC_R_SUCCESS); From 12a476fa9b48b94a2c3911b299edbcadca3779e8 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 29 Jun 2023 09:27:29 +1000 Subject: [PATCH 02/36] Add CHANGES note for [GL #4182] (cherry picked from commit a62cda787ffdced3fc2ae31d8da86cba17a3f95f) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index a526008103..87c859c1ea 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6314. [bug] Address race conditions in dns_tsigkey_find(). + [GL #4182] + 6304. [bug] The wrong time was being used to determine what RRSIGs where to be generated when dnssec-policy was in use. [GL #4494] From 608707b4f5b473e416563bfe0d43e26d6dc4a5c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 11 Sep 2023 10:35:28 +0200 Subject: [PATCH 03/36] Use hashtable when parsing a message When parsing messages use a hashtable instead of a linear search to reduce the amount of work done in findname when there's more than one name in the section. There are two hashtables: 1) hashtable for owner names - that's constructed for each section when we hit the second name in the section and destroyed right after parsing that section; 2) per-name hashtable - for each name in the section, we construct a new hashtable for that name if there are more than one rdataset for that particular name. (cherry picked from commit b8a96317544c7b310b4f74360825a87b6402ddc2) --- lib/dns/include/dns/message.h | 38 ---- lib/dns/include/dns/name.h | 37 ++-- lib/dns/message.c | 369 ++++++++++++++++++++++------------ lib/dns/name.c | 1 + 4 files changed, 261 insertions(+), 184 deletions(-) diff --git a/lib/dns/include/dns/message.h b/lib/dns/include/dns/message.h index fafa86a252..ea45742439 100644 --- a/lib/dns/include/dns/message.h +++ b/lib/dns/include/dns/message.h @@ -801,44 +801,6 @@ dns_message_findtype(const dns_name_t *name, dns_rdatatype_t type, *\li #ISC_R_NOTFOUND -- the desired type does not exist. */ -isc_result_t -dns_message_find(const dns_name_t *name, dns_rdataclass_t rdclass, - dns_rdatatype_t type, dns_rdatatype_t covers, - dns_rdataset_t **rdataset); -/*%< - * Search the name for the specified rdclass and type. If it is found, - * *rdataset is filled in with a pointer to that rdataset. - * - * Requires: - *\li if '**rdataset' is non-NULL, *rdataset needs to be NULL. - * - *\li 'type' be a valid type, and NOT dns_rdatatype_any. - * - *\li If 'type' is dns_rdatatype_rrsig, 'covers' must be a valid type. - * Otherwise it should be 0. - * - * Returns: - *\li #ISC_R_SUCCESS -- all is well. - *\li #ISC_R_NOTFOUND -- the desired type does not exist. - */ - -void -dns_message_movename(dns_message_t *msg, dns_name_t *name, - dns_section_t fromsection, dns_section_t tosection); -/*%< - * Move a name from one section to another. - * - * Requires: - * - *\li 'msg' be valid. - * - *\li 'name' must be a name already in 'fromsection'. - * - *\li 'fromsection' must be a valid section. - * - *\li 'tosection' must be a valid section. - */ - void dns_message_addname(dns_message_t *msg, dns_name_t *name, dns_section_t section); diff --git a/lib/dns/include/dns/name.h b/lib/dns/include/dns/name.h index 683f71d79f..2e1c5f882b 100644 --- a/lib/dns/include/dns/name.h +++ b/lib/dns/include/dns/name.h @@ -69,6 +69,7 @@ #include #include +#include #include #include #include /* Required for storage size of dns_label_t. */ @@ -112,6 +113,7 @@ struct dns_name { isc_buffer_t *buffer; ISC_LINK(dns_name_t) link; ISC_LIST(dns_rdataset_t) list; + isc_ht_t *ht; }; #define DNS_NAME_MAGIC ISC_MAGIC('D', 'N', 'S', 'n') @@ -167,30 +169,24 @@ LIBDNS_EXTERNAL_DATA extern const dns_name_t *dns_wildcardname; * unsigned char offsets[] = { 0, 6 }; * dns_name_t value = DNS_NAME_INITABSOLUTE(data, offsets); */ -#define DNS_NAME_INITNONABSOLUTE(A, B) \ - { \ - DNS_NAME_MAGIC, A, (sizeof(A) - 1), sizeof(B), \ - DNS_NAMEATTR_READONLY, B, NULL, \ - { (void *)-1, (void *)-1 }, { \ - NULL, NULL \ - } \ +#define DNS_NAME_INITNONABSOLUTE(A, B) \ + { \ + DNS_NAME_MAGIC, A, (sizeof(A) - 1), sizeof(B), \ + DNS_NAMEATTR_READONLY, B, NULL, \ + { (void *)-1, (void *)-1 }, { NULL, NULL }, NULL \ } -#define DNS_NAME_INITABSOLUTE(A, B) \ - { \ - DNS_NAME_MAGIC, A, sizeof(A), sizeof(B), \ - DNS_NAMEATTR_READONLY | DNS_NAMEATTR_ABSOLUTE, B, \ - NULL, { (void *)-1, (void *)-1 }, { \ - NULL, NULL \ - } \ +#define DNS_NAME_INITABSOLUTE(A, B) \ + { \ + DNS_NAME_MAGIC, A, sizeof(A), sizeof(B), \ + DNS_NAMEATTR_READONLY | DNS_NAMEATTR_ABSOLUTE, B, \ + NULL, { (void *)-1, (void *)-1 }, { NULL, NULL }, NULL \ } -#define DNS_NAME_INITEMPTY \ - { \ - DNS_NAME_MAGIC, NULL, 0, 0, 0, NULL, NULL, \ - { (void *)-1, (void *)-1 }, { \ - NULL, NULL \ - } \ +#define DNS_NAME_INITEMPTY \ + { \ + DNS_NAME_MAGIC, NULL, 0, 0, 0, NULL, NULL, \ + { (void *)-1, (void *)-1 }, { NULL, NULL }, NULL \ } /*% @@ -1357,6 +1353,7 @@ ISC_LANG_ENDDECLS _n->buffer = NULL; \ ISC_LINK_INIT(_n, link); \ ISC_LIST_INIT(_n->list); \ + _n->ht = NULL; \ } while (0) #define DNS_NAME_RESET(n) \ diff --git a/lib/dns/message.c b/lib/dns/message.c index 7f34cd7bf8..2f0075d5b3 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -22,6 +22,8 @@ #include #include +#include +#include #include #include #include /* Required for HP/UX (and others?) */ @@ -507,9 +509,11 @@ msgresetsigs(dns_message_t *msg, bool replying) { } else { dns_rdataset_disassociate(msg->tsig); isc_mempool_put(msg->rdspool, msg->tsig); + msg->tsig = NULL; if (msg->querytsig != NULL) { dns_rdataset_disassociate(msg->querytsig); isc_mempool_put(msg->rdspool, msg->querytsig); + msg->querytsig = NULL; } } dns_message_puttempname(msg, &msg->tsigname); @@ -798,6 +802,22 @@ dns_message_detach(dns_message_t **messagep) { } } +static isc_result_t +name_hash_add(isc_ht_t *ht, dns_name_t *name, dns_name_t **foundp) { + dns_fixedname_t fixed; + dns_name_t *key = dns_fixedname_initname(&fixed); + dns_name_downcase(name, key, NULL); + + isc_result_t result = isc_ht_find(ht, key->ndata, key->length, + (void **)foundp); + if (result == ISC_R_SUCCESS) { + return (ISC_R_EXISTS); + } + result = isc_ht_add(ht, key->ndata, key->length, (void *)name); + INSIST(result == ISC_R_SUCCESS); + return (ISC_R_SUCCESS); +} + static isc_result_t findname(dns_name_t **foundname, const dns_name_t *target, dns_namelist_t *section) { @@ -817,29 +837,26 @@ findname(dns_name_t **foundname, const dns_name_t *target, return (ISC_R_NOTFOUND); } -isc_result_t -dns_message_find(const dns_name_t *name, dns_rdataclass_t rdclass, - dns_rdatatype_t type, dns_rdatatype_t covers, - dns_rdataset_t **rdataset) { - dns_rdataset_t *curr; +typedef struct __attribute__((__packed__)) rds_key { + dns_rdataclass_t rdclass; + dns_rdatatype_t type; + dns_rdatatype_t covers; +} rds_key_t; - REQUIRE(name != NULL); - REQUIRE(rdataset == NULL || *rdataset == NULL); - - for (curr = ISC_LIST_TAIL(name->list); curr != NULL; - curr = ISC_LIST_PREV(curr, link)) - { - if (curr->rdclass == rdclass && curr->type == type && - curr->covers == covers) - { - if (rdataset != NULL) { - *rdataset = curr; - } - return (ISC_R_SUCCESS); - } +static isc_result_t +rds_hash_add(isc_ht_t *ht, dns_rdataset_t *rds, dns_rdataset_t **foundp) { + rds_key_t key = { .rdclass = rds->rdclass, + .type = rds->type, + .covers = rds->covers }; + isc_result_t result = isc_ht_find(ht, (const unsigned char *)&key, + sizeof(key), (void **)foundp); + if (result == ISC_R_SUCCESS) { + return (ISC_R_EXISTS); } - - return (ISC_R_NOTFOUND); + result = isc_ht_add(ht, (const unsigned char *)&key, sizeof(key), + (void *)rds); + INSIST(result == ISC_R_SUCCESS); + return (ISC_R_SUCCESS); } isc_result_t @@ -966,6 +983,18 @@ getrdata(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, } \ } while (0) +static void +cleanup_name_hashmaps(dns_namelist_t *section) { + dns_name_t *name = NULL; + for (name = ISC_LIST_HEAD(*section); name != NULL; + name = ISC_LIST_NEXT(name, link)) + { + if (name->ht != NULL) { + isc_ht_destroy(&name->ht); + } + } +} + static isc_result_t getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, unsigned int options) { @@ -975,13 +1004,19 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, dns_name_t *name2 = NULL; dns_rdataset_t *rdataset = NULL; dns_rdatalist_t *rdatalist = NULL; - isc_result_t result; + isc_result_t result = ISC_R_SUCCESS; dns_rdatatype_t rdtype; dns_rdataclass_t rdclass; dns_namelist_t *section = &msg->sections[DNS_SECTION_QUESTION]; bool best_effort = ((options & DNS_MESSAGEPARSE_BESTEFFORT) != 0); bool seen_problem = false; bool free_name = false; + bool free_ht = false; + isc_ht_t *name_map = NULL; + + if (msg->counts[DNS_SECTION_QUESTION] > 1) { + isc_ht_init(&name_map, msg->mctx, 1, ISC_HT_CASE_INSENSITIVE); + } for (count = 0; count < msg->counts[DNS_SECTION_QUESTION]; count++) { name = NULL; @@ -1002,13 +1037,19 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, goto cleanup; } + /* If there is only one QNAME, skip the duplicity checks */ + if (name_map == NULL) { + result = ISC_R_SUCCESS; + goto skip_name_check; + } + /* * Run through the section, looking to see if this name * is already there. If it is found, put back the allocated * name since we no longer need it, and set our name pointer * to point to the name we found. */ - result = findname(&name2, name, section); + result = name_hash_add(name_map, name, &name2); /* * If it is the first name in the section, accept it. @@ -1020,19 +1061,25 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, * this should be legal or not. In either case we no longer * need this name pointer. */ - if (result != ISC_R_SUCCESS) { + skip_name_check: + switch (result) { + case ISC_R_SUCCESS: if (!ISC_LIST_EMPTY(*section)) { DO_ERROR(DNS_R_FORMERR); } ISC_LIST_APPEND(*section, name, link); - free_name = false; - } else { + break; + case ISC_R_EXISTS: dns_message_puttempname(msg, &name); name = name2; name2 = NULL; - free_name = false; + break; + default: + UNREACHABLE(); } + free_name = false; + /* * Get type and class. */ @@ -1062,14 +1109,6 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, msg->tkey = 1; } - /* - * Can't ask the same question twice. - */ - result = dns_message_find(name, rdclass, rdtype, 0, NULL); - if (result == ISC_R_SUCCESS) { - DO_ERROR(DNS_R_FORMERR); - } - /* * Allocate a new rdatalist. */ @@ -1083,6 +1122,7 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, result = ISC_R_NOMEMORY; goto cleanup; } + dns_rdataset_init(rdataset); /* * Convert rdatalist to rdataset, and attach the latter to @@ -1091,7 +1131,6 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, rdatalist->type = rdtype; rdatalist->rdclass = rdclass; - dns_rdataset_init(rdataset); result = dns_rdatalist_tordataset(rdatalist, rdataset); if (result != ISC_R_SUCCESS) { goto cleanup; @@ -1099,14 +1138,45 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, rdataset->attributes |= DNS_RDATASETATTR_QUESTION; + /* + * Skip the duplicity check for first rdataset + */ + if (ISC_LIST_EMPTY(name->list)) { + result = ISC_R_SUCCESS; + goto skip_rds_check; + } + + /* + * Can't ask the same question twice. + */ + if (name->ht == NULL) { + isc_ht_init(&name->ht, msg->mctx, 1); + free_ht = true; + + INSIST(ISC_LIST_HEAD(name->list) == + ISC_LIST_TAIL(name->list)); + + dns_rdataset_t *old_rdataset = + ISC_LIST_HEAD(name->list); + + result = rds_hash_add(name->ht, old_rdataset, NULL); + + INSIST(result == ISC_R_SUCCESS); + } + result = rds_hash_add(name->ht, rdataset, NULL); + if (result == ISC_R_EXISTS) { + DO_ERROR(DNS_R_FORMERR); + } + + skip_rds_check: ISC_LIST_APPEND(name->list, rdataset, link); + rdataset = NULL; } if (seen_problem) { - return (DNS_R_RECOVERABLE); + result = DNS_R_RECOVERABLE; } - return (ISC_R_SUCCESS); cleanup: if (rdataset != NULL) { @@ -1117,6 +1187,14 @@ cleanup: dns_message_puttempname(msg, &name); } + if (free_ht) { + cleanup_name_hashmaps(section); + } + + if (name_map != NULL) { + isc_ht_destroy(&name_map); + } + return (result); } @@ -1196,17 +1274,24 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, dns_name_t *name = NULL; dns_name_t *name2 = NULL; dns_rdataset_t *rdataset = NULL; + dns_rdataset_t *found_rdataset = NULL; dns_rdatalist_t *rdatalist = NULL; - isc_result_t result; + isc_result_t result = ISC_R_SUCCESS; dns_rdatatype_t rdtype, covers; dns_rdataclass_t rdclass; dns_rdata_t *rdata = NULL; dns_ttl_t ttl; dns_namelist_t *section = &msg->sections[sectionid]; - bool free_name = false, free_rdataset = false, seen_problem = false; + bool free_name = false, seen_problem = false; + bool free_ht = false; bool preserve_order = ((options & DNS_MESSAGEPARSE_PRESERVEORDER) != 0); bool best_effort = ((options & DNS_MESSAGEPARSE_BESTEFFORT) != 0); bool isedns, issigzero, istsig; + isc_ht_t *name_map = NULL; + + if (msg->counts[sectionid] > 1) { + isc_ht_init(&name_map, msg->mctx, 1, ISC_HT_CASE_INSENSITIVE); + } for (count = 0; count < msg->counts[sectionid]; count++) { int recstart = source->current; @@ -1214,7 +1299,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, skip_name_search = false; skip_type_search = false; - free_rdataset = false; isedns = false; issigzero = false; istsig = false; @@ -1257,8 +1341,8 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, if (msg->rdclass_set == 0 && rdtype != dns_rdatatype_opt && /* class is UDP SIZE */ rdtype != dns_rdatatype_tsig && /* class is ANY */ - rdtype != dns_rdatatype_tkey) - { /* class is undefined */ + rdtype != dns_rdatatype_tkey) /* class is undefined */ + { msg->rdclass = rdclass; msg->rdclass_set = 1; } @@ -1456,61 +1540,132 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, free_name = false; } } else { + if (name_map == NULL) { + result = ISC_R_SUCCESS; + goto skip_name_check; + } + /* * Run through the section, looking to see if this name * is already there. If it is found, put back the * allocated name since we no longer need it, and set * our name pointer to point to the name we found. */ - result = findname(&name2, name, section); + result = name_hash_add(name_map, name, &name2); /* * If it is a new name, append to the section. */ - if (result == ISC_R_SUCCESS) { + skip_name_check: + switch (result) { + case ISC_R_SUCCESS: + ISC_LIST_APPEND(*section, name, link); + break; + case ISC_R_EXISTS: dns_message_puttempname(msg, &name); name = name2; - } else { - ISC_LIST_APPEND(*section, name, link); + name2 = NULL; + break; + default: + UNREACHABLE(); } free_name = false; } + rdatalist = newrdatalist(msg); + if (rdatalist == NULL) { + result = ISC_R_NOMEMORY; + goto cleanup; + } + dns_message_gettemprdataset(msg, &rdataset); + if (rdataset == NULL) { + result = ISC_R_NOMEMORY; + goto cleanup; + } + + rdatalist->type = rdtype; + rdatalist->covers = covers; + rdatalist->rdclass = rdclass; + rdatalist->ttl = ttl; + + RUNTIME_CHECK(dns_rdatalist_tordataset(rdatalist, rdataset) == + ISC_R_SUCCESS); + dns_rdataset_setownercase(rdataset, name); + rdatalist = NULL; + /* * Search name for the particular type and class. * Skip this stage if in update mode or this is a meta-type. */ - if (preserve_order || msg->opcode == dns_opcode_update || - skip_type_search) + if (isedns || istsig || issigzero) { + /* Skip adding the rdataset to the tables */ + } else if (preserve_order || msg->opcode == dns_opcode_update || + skip_type_search) { - result = ISC_R_NOTFOUND; + result = ISC_R_SUCCESS; + + ISC_LIST_APPEND(name->list, rdataset, link); } else { /* * If this is a type that can only occur in * the question section, fail. */ if (dns_rdatatype_questiononly(rdtype)) { + dns_message_puttemprdataset(msg, &rdataset); DO_ERROR(DNS_R_FORMERR); } - rdataset = NULL; - result = dns_message_find(name, rdclass, rdtype, covers, - &rdataset); - } + if (ISC_LIST_EMPTY(name->list)) { + result = ISC_R_SUCCESS; + goto skip_rds_check; + } + + if (name->ht == NULL) { + isc_ht_init(&name->ht, msg->mctx, 1); + free_ht = true; + + INSIST(ISC_LIST_HEAD(name->list) == + ISC_LIST_TAIL(name->list)); + + dns_rdataset_t *old_rdataset = + ISC_LIST_HEAD(name->list); + + result = rds_hash_add(name->ht, old_rdataset, + NULL); + + INSIST(result == ISC_R_SUCCESS); + } + found_rdataset = NULL; + result = rds_hash_add(name->ht, rdataset, + &found_rdataset); + + /* + * If we found an rdataset that matches, we need to + * append this rdata to that set. If we did not, we + * need to create a new rdatalist, store the important + * bits there, convert it to an rdataset, and link the + * latter to the name. Yuck. When appending, make + * certain that the type isn't a singleton type, such as + * SOA or CNAME. + * + * Note that this check will be bypassed when preserving + * order, the opcode is an update, or the type search is + * skipped. + */ + skip_rds_check: + switch (result) { + case ISC_R_EXISTS: + /* Free the rdataset we used as the key */ + dns_rdataset_disassociate(rdataset); + isc_mempool_put(msg->rdspool, rdataset); + rdataset = found_rdataset; + + result = ISC_R_SUCCESS; + + if (!dns_rdatatype_issingleton(rdtype)) { + break; + } - /* - * If we found an rdataset that matches, we need to - * append this rdata to that set. If we did not, we need - * to create a new rdatalist, store the important bits there, - * convert it to an rdataset, and link the latter to the name. - * Yuck. When appending, make certain that the type isn't - * a singleton type, such as SOA or CNAME. - * - * Note that this check will be bypassed when preserving order, - * the opcode is an update, or the type search is skipped. - */ - if (result == ISC_R_SUCCESS) { - if (dns_rdatatype_issingleton(rdtype)) { dns_rdata_t *first; dns_rdatalist_fromrdataset(rdataset, &rdatalist); @@ -1519,37 +1674,12 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, if (dns_rdata_compare(rdata, first) != 0) { DO_ERROR(DNS_R_FORMERR); } - } - } - - if (result == ISC_R_NOTFOUND) { - rdataset = isc_mempool_get(msg->rdspool); - if (rdataset == NULL) { - result = ISC_R_NOMEMORY; - goto cleanup; - } - free_rdataset = true; - - rdatalist = newrdatalist(msg); - if (rdatalist == NULL) { - result = ISC_R_NOMEMORY; - goto cleanup; - } - - rdatalist->type = rdtype; - rdatalist->covers = covers; - rdatalist->rdclass = rdclass; - rdatalist->ttl = ttl; - - dns_rdataset_init(rdataset); - RUNTIME_CHECK( - dns_rdatalist_tordataset(rdatalist, rdataset) == - ISC_R_SUCCESS); - dns_rdataset_setownercase(rdataset, name); - - if (!isedns && !istsig && !issigzero) { + break; + case ISC_R_SUCCESS: ISC_LIST_APPEND(name->list, rdataset, link); - free_rdataset = false; + break; + default: + UNREACHABLE(); } } @@ -1585,7 +1715,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, msg->opt = rdataset; rdataset = NULL; - free_rdataset = false; ercode = (dns_rcode_t)((msg->opt->ttl & DNS_MESSAGE_EDNSRCODE_MASK) >> 20); @@ -1597,7 +1726,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, msg->sig0name = name; msg->sigstart = recstart; rdataset = NULL; - free_rdataset = false; free_name = false; } else if (istsig) { msg->tsig = rdataset; @@ -1608,7 +1736,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, */ msg->tsigname->attributes |= DNS_NAMEATTR_NOCOMPRESS; rdataset = NULL; - free_rdataset = false; free_name = false; } @@ -1616,13 +1743,11 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, if (free_name) { dns_message_puttempname(msg, &name); } - if (free_rdataset) { - isc_mempool_put(msg->rdspool, rdataset); - } - free_name = free_rdataset = false; + free_name = false; } INSIST(!free_name); - INSIST(!free_rdataset); + + rdataset = NULL; } /* @@ -1640,16 +1765,20 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, } if (seen_problem) { - return (DNS_R_RECOVERABLE); + result = DNS_R_RECOVERABLE; } - return (ISC_R_SUCCESS); cleanup: if (free_name) { dns_message_puttempname(msg, &name); } - if (free_rdataset) { - isc_mempool_put(msg->rdspool, rdataset); + + if (free_ht) { + cleanup_name_hashmaps(section); + } + + if (name_map != NULL) { + isc_ht_destroy(&name_map); } return (result); @@ -2446,7 +2575,7 @@ dns_message_findname(dns_message_t *msg, dns_section_t section, const dns_name_t *target, dns_rdatatype_t type, dns_rdatatype_t covers, dns_name_t **name, dns_rdataset_t **rdataset) { - dns_name_t *foundname; + dns_name_t *foundname = NULL; isc_result_t result; /* @@ -2493,22 +2622,6 @@ dns_message_findname(dns_message_t *msg, dns_section_t section, return (result); } -void -dns_message_movename(dns_message_t *msg, dns_name_t *name, - dns_section_t fromsection, dns_section_t tosection) { - REQUIRE(msg != NULL); - REQUIRE(msg->from_to_wire == DNS_MESSAGE_INTENTRENDER); - REQUIRE(name != NULL); - REQUIRE(VALID_NAMED_SECTION(fromsection)); - REQUIRE(VALID_NAMED_SECTION(tosection)); - - /* - * Unlink the name from the old section - */ - ISC_LIST_UNLINK(msg->sections[fromsection], name, link); - ISC_LIST_APPEND(msg->sections[tosection], name, link); -} - void dns_message_addname(dns_message_t *msg, dns_name_t *name, dns_section_t section) { @@ -2600,6 +2713,10 @@ dns_message_puttempname(dns_message_t *msg, dns_name_t **itemp) { REQUIRE(!ISC_LINK_LINKED(item, link)); REQUIRE(ISC_LIST_HEAD(item->list) == NULL); + if (item->ht != NULL) { + isc_ht_destroy(&item->ht); + } + /* * we need to check this in case dns_name_dup() was used. */ diff --git a/lib/dns/name.c b/lib/dns/name.c index 96f95b34b9..a1702699f9 100644 --- a/lib/dns/name.c +++ b/lib/dns/name.c @@ -188,6 +188,7 @@ dns_name_invalidate(dns_name_t *name) { name->offsets = NULL; name->buffer = NULL; ISC_LINK_INIT(name, link); + INSIST(name->ht == NULL); } bool From a4baf324159ec3764195c949cb56c861d9f173ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 11 Oct 2023 13:54:50 +0200 Subject: [PATCH 04/36] Backport isc_ht API changes from BIND 9.18 To prevent allocating large hashtable in dns_message, we need to backport the improvements to isc_ht API from BIND 9.18+ that includes support for case insensitive keys and incremental rehashing of the hashtables. --- bin/plugins/filter-aaaa.c | 2 +- lib/dns/catz.c | 8 +- lib/dns/message.c | 14 +- lib/dns/rpz.c | 5 +- lib/isc/ht.c | 521 +++++++++++++++++++++++++++----------- lib/isc/include/isc/ht.h | 28 +- lib/isc/tests/ht_test.c | 4 +- 7 files changed, 410 insertions(+), 172 deletions(-) diff --git a/bin/plugins/filter-aaaa.c b/bin/plugins/filter-aaaa.c index 265da26d60..449124d0ae 100644 --- a/bin/plugins/filter-aaaa.c +++ b/bin/plugins/filter-aaaa.c @@ -350,7 +350,7 @@ plugin_register(const char *parameters, const void *cfg, const char *cfg_file, cfg_line, mctx, lctx, actx)); } - isc_ht_init(&inst->ht, mctx, 16); + isc_ht_init(&inst->ht, mctx, 16, ISC_HT_CASE_SENSITIVE); isc_mutex_init(&inst->hlock); /* diff --git a/lib/dns/catz.c b/lib/dns/catz.c index 2c00d6eba9..0275a4fdbf 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -423,9 +423,9 @@ dns_catz_zones_merge(dns_catz_zone_t *target, dns_catz_zone_t *newzone) { dns_name_format(&target->name, czname, DNS_NAME_FORMATSIZE); - isc_ht_init(&toadd, target->catzs->mctx, 16); + isc_ht_init(&toadd, target->catzs->mctx, 16, ISC_HT_CASE_SENSITIVE); - isc_ht_init(&tomod, target->catzs->mctx, 16); + isc_ht_init(&tomod, target->catzs->mctx, 16, ISC_HT_CASE_SENSITIVE); isc_ht_iter_create(newzone->entries, &iter1); @@ -610,7 +610,7 @@ dns_catz_new_zones(dns_catz_zones_t **catzsp, dns_catz_zonemodmethods_t *zmm, isc_refcount_init(&new_zones->refs, 1); - isc_ht_init(&new_zones->zones, mctx, 4); + isc_ht_init(&new_zones->zones, mctx, 4, ISC_HT_CASE_SENSITIVE); isc_mem_attach(mctx, &new_zones->mctx); new_zones->zmm = zmm; @@ -662,7 +662,7 @@ dns_catz_new_zone(dns_catz_zones_t *catzs, dns_catz_zone_t **zonep, dns_name_init(&new_zone->name, NULL); dns_name_dup(name, catzs->mctx, &new_zone->name); - isc_ht_init(&new_zone->entries, catzs->mctx, 16); + isc_ht_init(&new_zone->entries, catzs->mctx, 16, ISC_HT_CASE_SENSITIVE); new_zone->updatetimer = NULL; result = isc_timer_create(catzs->timermgr, isc_timertype_inactive, NULL, diff --git a/lib/dns/message.c b/lib/dns/message.c index 2f0075d5b3..d216cb1742 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -804,16 +804,12 @@ dns_message_detach(dns_message_t **messagep) { static isc_result_t name_hash_add(isc_ht_t *ht, dns_name_t *name, dns_name_t **foundp) { - dns_fixedname_t fixed; - dns_name_t *key = dns_fixedname_initname(&fixed); - dns_name_downcase(name, key, NULL); - - isc_result_t result = isc_ht_find(ht, key->ndata, key->length, + isc_result_t result = isc_ht_find(ht, name->ndata, name->length, (void **)foundp); if (result == ISC_R_SUCCESS) { return (ISC_R_EXISTS); } - result = isc_ht_add(ht, key->ndata, key->length, (void *)name); + result = isc_ht_add(ht, name->ndata, name->length, (void *)name); INSIST(result == ISC_R_SUCCESS); return (ISC_R_SUCCESS); } @@ -1150,7 +1146,8 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, * Can't ask the same question twice. */ if (name->ht == NULL) { - isc_ht_init(&name->ht, msg->mctx, 1); + isc_ht_init(&name->ht, msg->mctx, 1, + ISC_HT_CASE_SENSITIVE); free_ht = true; INSIST(ISC_LIST_HEAD(name->list) == @@ -1621,7 +1618,8 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, } if (name->ht == NULL) { - isc_ht_init(&name->ht, msg->mctx, 1); + isc_ht_init(&name->ht, msg->mctx, 1, + ISC_HT_CASE_SENSITIVE); free_ht = true; INSIST(ISC_LIST_HEAD(name->list) == diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c index 20db72fb15..fdc975976b 100644 --- a/lib/dns/rpz.c +++ b/lib/dns/rpz.c @@ -1544,7 +1544,7 @@ dns_rpz_new_zone(dns_rpz_zones_t *rpzs, dns_rpz_zone_t **rpzp) { * simplifies update_from_db */ - isc_ht_init(&zone->nodes, rpzs->mctx, 1); + isc_ht_init(&zone->nodes, rpzs->mctx, 1, ISC_HT_CASE_SENSITIVE); dns_name_init(&zone->origin, NULL); dns_name_init(&zone->client_ip, NULL); @@ -1722,7 +1722,8 @@ setup_update(dns_rpz_zone_t *rpz) { ISC_LOG_DEBUG(1), "rpz: %s: using hashtable size %d", domain, hashsize); - isc_ht_init(&rpz->newnodes, rpz->rpzs->mctx, hashsize); + isc_ht_init(&rpz->newnodes, rpz->rpzs->mctx, hashsize, + ISC_HT_CASE_SENSITIVE); result = dns_db_createiterator(rpz->updb, DNS_DB_NONSEC3, &rpz->updbit); if (result != ISC_R_SUCCESS) { diff --git a/lib/isc/ht.c b/lib/isc/ht.c index 07a36b4cff..eaf2b3c508 100644 --- a/lib/isc/ht.c +++ b/lib/isc/ht.c @@ -27,51 +27,231 @@ typedef struct isc_ht_node isc_ht_node_t; #define ISC_HT_MAGIC ISC_MAGIC('H', 'T', 'a', 'b') #define ISC_HT_VALID(ht) ISC_MAGIC_VALID(ht, ISC_HT_MAGIC) +#define HT_NO_BITS 0 +#define HT_MIN_BITS 1 +#define HT_MAX_BITS 32 +#define HT_OVERCOMMIT 3 + +#define HT_NEXTTABLE(idx) ((idx == 0) ? 1 : 0) +#define TRY_NEXTTABLE(idx, ht) (idx == ht->hindex && rehashing_in_progress(ht)) + +#define GOLDEN_RATIO_32 0x61C88647 + +#define HASHSIZE(bits) (UINT64_C(1) << (bits)) + struct isc_ht_node { void *value; isc_ht_node_t *next; + uint32_t hashval; size_t keysize; - unsigned char key[FLEXIBLE_ARRAY_MEMBER]; + unsigned char key[]; }; struct isc_ht { unsigned int magic; isc_mem_t *mctx; - size_t size; - size_t mask; - unsigned int count; - isc_ht_node_t **table; + size_t count; + bool case_sensitive; + size_t size[2]; + uint8_t hashbits[2]; + isc_ht_node_t **table[2]; + uint8_t hindex; + uint32_t hiter; /* rehashing iterator */ }; struct isc_ht_iter { isc_ht_t *ht; size_t i; + uint8_t hindex; isc_ht_node_t *cur; }; +static isc_ht_node_t * +isc__ht_find(const isc_ht_t *ht, const unsigned char *key, + const uint32_t keysize, const uint32_t hashval, const uint8_t idx); +static void +isc__ht_add(isc_ht_t *ht, const unsigned char *key, const uint32_t keysize, + const uint32_t hashval, const uint8_t idx, void *value); +static isc_result_t +isc__ht_delete(isc_ht_t *ht, const unsigned char *key, const uint32_t keysize, + const uint32_t hashval, const uint8_t idx); + +static uint32_t +rehash_bits(isc_ht_t *ht, size_t newcount); + +static void +hashtable_new(isc_ht_t *ht, const uint8_t idx, const uint8_t bits); +static void +hashtable_free(isc_ht_t *ht, const uint8_t idx); +static void +hashtable_rehash(isc_ht_t *ht, uint32_t newbits); +static void +hashtable_rehash_one(isc_ht_t *ht); +static void +maybe_rehash(isc_ht_t *ht, size_t newcount); + +static isc_result_t +isc__ht_iter_next(isc_ht_iter_t *it); + +static bool +isc__ht_node_match(isc_ht_node_t *node, const uint32_t hashval, + const uint8_t *key, uint32_t keysize) { + return (node->hashval == hashval && node->keysize == keysize && + memcmp(node->key, key, keysize) == 0); +} + +static uint32_t +hash_32(uint32_t val, unsigned int bits) { + REQUIRE(bits <= HT_MAX_BITS); + /* High bits are more random. */ + return (val * GOLDEN_RATIO_32 >> (32 - bits)); +} + +static bool +rehashing_in_progress(const isc_ht_t *ht) { + return (ht->table[HT_NEXTTABLE(ht->hindex)] != NULL); +} + +static bool +hashtable_is_overcommited(isc_ht_t *ht) { + return (ht->count >= (ht->size[ht->hindex] * HT_OVERCOMMIT)); +} + +static uint32_t +rehash_bits(isc_ht_t *ht, size_t newcount) { + uint32_t newbits = ht->hashbits[ht->hindex]; + + while (newcount >= HASHSIZE(newbits) && newbits <= HT_MAX_BITS) { + newbits += 1; + } + + return (newbits); +} + +/* + * Rebuild the hashtable to reduce the load factor + */ +static void +hashtable_rehash(isc_ht_t *ht, uint32_t newbits) { + uint8_t oldindex = ht->hindex; + uint32_t oldbits = ht->hashbits[oldindex]; + uint8_t newindex = HT_NEXTTABLE(oldindex); + + REQUIRE(ht->hashbits[oldindex] >= HT_MIN_BITS); + REQUIRE(ht->hashbits[oldindex] <= HT_MAX_BITS); + REQUIRE(ht->table[oldindex] != NULL); + + REQUIRE(newbits <= HT_MAX_BITS); + REQUIRE(ht->hashbits[newindex] == HT_NO_BITS); + REQUIRE(ht->table[newindex] == NULL); + + REQUIRE(newbits > oldbits); + + hashtable_new(ht, newindex, newbits); + + ht->hindex = newindex; + + hashtable_rehash_one(ht); +} + +static void +hashtable_rehash_one(isc_ht_t *ht) { + isc_ht_node_t **newtable = ht->table[ht->hindex]; + uint32_t oldsize = ht->size[HT_NEXTTABLE(ht->hindex)]; + isc_ht_node_t **oldtable = ht->table[HT_NEXTTABLE(ht->hindex)]; + isc_ht_node_t *node = NULL; + isc_ht_node_t *nextnode; + + /* Find first non-empty node */ + while (ht->hiter < oldsize && oldtable[ht->hiter] == NULL) { + ht->hiter++; + } + + /* Rehashing complete */ + if (ht->hiter == oldsize) { + hashtable_free(ht, HT_NEXTTABLE(ht->hindex)); + ht->hiter = 0; + return; + } + + /* Move the first non-empty node from old hashtable to new hashtable */ + for (node = oldtable[ht->hiter]; node != NULL; node = nextnode) { + uint32_t hash = hash_32(node->hashval, + ht->hashbits[ht->hindex]); + nextnode = node->next; + node->next = newtable[hash]; + newtable[hash] = node; + } + + oldtable[ht->hiter] = NULL; + + ht->hiter++; +} + +static void +maybe_rehash(isc_ht_t *ht, size_t newcount) { + uint32_t newbits = rehash_bits(ht, newcount); + + if (ht->hashbits[ht->hindex] < newbits && newbits <= HT_MAX_BITS) { + hashtable_rehash(ht, newbits); + } +} + +static void +hashtable_new(isc_ht_t *ht, const uint8_t idx, const uint8_t bits) { + size_t size; + REQUIRE(ht->hashbits[idx] == HT_NO_BITS); + REQUIRE(ht->table[idx] == NULL); + REQUIRE(bits >= HT_MIN_BITS); + REQUIRE(bits <= HT_MAX_BITS); + + ht->hashbits[idx] = bits; + ht->size[idx] = HASHSIZE(ht->hashbits[idx]); + + size = ht->size[idx] * sizeof(isc_ht_node_t *); + + ht->table[idx] = isc_mem_get(ht->mctx, size); + memset(ht->table[idx], 0, size); +} + +static void +hashtable_free(isc_ht_t *ht, const uint8_t idx) { + size_t size = ht->size[idx] * sizeof(isc_ht_node_t *); + + for (size_t i = 0; i < ht->size[idx]; i++) { + isc_ht_node_t *node = ht->table[idx][i]; + while (node != NULL) { + isc_ht_node_t *next = node->next; + ht->count--; + isc_mem_put(ht->mctx, node, + sizeof(*node) + node->keysize); + node = next; + } + } + + isc_mem_put(ht->mctx, ht->table[idx], size); + ht->hashbits[idx] = HT_NO_BITS; + ht->table[idx] = NULL; +} + void -isc_ht_init(isc_ht_t **htp, isc_mem_t *mctx, uint8_t bits) { +isc_ht_init(isc_ht_t **htp, isc_mem_t *mctx, uint8_t bits, + unsigned int options) { isc_ht_t *ht = NULL; - size_t i; + bool case_sensitive = ((options & ISC_HT_CASE_INSENSITIVE) == 0); REQUIRE(htp != NULL && *htp == NULL); REQUIRE(mctx != NULL); - REQUIRE(bits >= 1 && bits <= (sizeof(size_t) * 8 - 1)); + REQUIRE(bits >= 1 && bits <= HT_MAX_BITS); - ht = isc_mem_get(mctx, sizeof(struct isc_ht)); + ht = isc_mem_get(mctx, sizeof(*ht)); + *ht = (isc_ht_t){ + .case_sensitive = case_sensitive, + }; - ht->mctx = NULL; isc_mem_attach(mctx, &ht->mctx); - ht->size = ((size_t)1 << bits); - ht->mask = ((size_t)1 << bits) - 1; - ht->count = 0; - - ht->table = isc_mem_get(ht->mctx, ht->size * sizeof(isc_ht_node_t *)); - - for (i = 0; i < ht->size; i++) { - ht->table[i] = NULL; - } + hashtable_new(ht, 0, bits); ht->magic = ISC_HT_MAGIC; @@ -81,126 +261,180 @@ isc_ht_init(isc_ht_t **htp, isc_mem_t *mctx, uint8_t bits) { void isc_ht_destroy(isc_ht_t **htp) { isc_ht_t *ht; - size_t i; REQUIRE(htp != NULL); + REQUIRE(ISC_HT_VALID(*htp)); ht = *htp; *htp = NULL; - - REQUIRE(ISC_HT_VALID(ht)); - ht->magic = 0; - for (i = 0; i < ht->size; i++) { - isc_ht_node_t *node = ht->table[i]; - while (node != NULL) { - isc_ht_node_t *next = node->next; - ht->count--; - isc_mem_put(ht->mctx, node, - offsetof(isc_ht_node_t, key) + - node->keysize); - node = next; + for (size_t i = 0; i <= 1; i++) { + if (ht->table[i] != NULL) { + hashtable_free(ht, i); } } INSIST(ht->count == 0); - isc_mem_put(ht->mctx, ht->table, ht->size * sizeof(isc_ht_node_t *)); - isc_mem_putanddetach(&ht->mctx, ht, sizeof(struct isc_ht)); + isc_mem_putanddetach(&ht->mctx, ht, sizeof(*ht)); +} + +static void +isc__ht_add(isc_ht_t *ht, const unsigned char *key, const uint32_t keysize, + const uint32_t hashval, const uint8_t idx, void *value) { + isc_ht_node_t *node; + uint32_t hash; + + hash = hash_32(hashval, ht->hashbits[idx]); + + node = isc_mem_get(ht->mctx, sizeof(*node) + keysize); + *node = (isc_ht_node_t){ + .keysize = keysize, + .hashval = hashval, + .next = ht->table[idx][hash], + .value = value, + }; + + memmove(node->key, key, keysize); + + ht->count++; + ht->table[idx][hash] = node; } isc_result_t -isc_ht_add(isc_ht_t *ht, const unsigned char *key, uint32_t keysize, +isc_ht_add(isc_ht_t *ht, const unsigned char *key, const uint32_t keysize, void *value) { - isc_ht_node_t *node; - uint32_t hash; + uint32_t hashval; REQUIRE(ISC_HT_VALID(ht)); REQUIRE(key != NULL && keysize > 0); - hash = isc_hash_function(key, keysize, true); - node = ht->table[hash & ht->mask]; - while (node != NULL) { - if (keysize == node->keysize && - memcmp(key, node->key, keysize) == 0) - { - return (ISC_R_EXISTS); - } - node = node->next; + if (rehashing_in_progress(ht)) { + /* Rehash in progress */ + hashtable_rehash_one(ht); + } else if (hashtable_is_overcommited(ht)) { + /* Rehash requested */ + maybe_rehash(ht, ht->count); } - node = isc_mem_get(ht->mctx, offsetof(isc_ht_node_t, key) + keysize); + hashval = isc_hash32(key, keysize, ht->case_sensitive); - memmove(node->key, key, keysize); - node->keysize = keysize; - node->next = ht->table[hash & ht->mask]; - node->value = value; + if (isc__ht_find(ht, key, keysize, hashval, ht->hindex) != NULL) { + return (ISC_R_EXISTS); + } + + isc__ht_add(ht, key, keysize, hashval, ht->hindex, value); - ht->count++; - ht->table[hash & ht->mask] = node; return (ISC_R_SUCCESS); } -isc_result_t -isc_ht_find(const isc_ht_t *ht, const unsigned char *key, uint32_t keysize, - void **valuep) { - isc_ht_node_t *node; +static isc_ht_node_t * +isc__ht_find(const isc_ht_t *ht, const unsigned char *key, + const uint32_t keysize, const uint32_t hashval, + const uint8_t idx) { uint32_t hash; + uint8_t findex = idx; + +nexttable: + hash = hash_32(hashval, ht->hashbits[findex]); + for (isc_ht_node_t *node = ht->table[findex][hash]; node != NULL; + node = node->next) + { + if (isc__ht_node_match(node, hashval, key, keysize)) { + return (node); + } + } + if (TRY_NEXTTABLE(findex, ht)) { + /* + * Rehashing in progress, check the other table + */ + findex = HT_NEXTTABLE(findex); + goto nexttable; + } + + return (NULL); +} + +isc_result_t +isc_ht_find(const isc_ht_t *ht, const unsigned char *key, + const uint32_t keysize, void **valuep) { + uint32_t hashval; + isc_ht_node_t *node; REQUIRE(ISC_HT_VALID(ht)); REQUIRE(key != NULL && keysize > 0); REQUIRE(valuep == NULL || *valuep == NULL); - hash = isc_hash_function(key, keysize, true); - node = ht->table[hash & ht->mask]; - while (node != NULL) { - if (keysize == node->keysize && - memcmp(key, node->key, keysize) == 0) - { - if (valuep != NULL) { - *valuep = node->value; + hashval = isc_hash32(key, keysize, ht->case_sensitive); + + node = isc__ht_find(ht, key, keysize, hashval, ht->hindex); + if (node == NULL) { + return (ISC_R_NOTFOUND); + } + + if (valuep != NULL) { + *valuep = node->value; + } + return (ISC_R_SUCCESS); +} + +static isc_result_t +isc__ht_delete(isc_ht_t *ht, const unsigned char *key, const uint32_t keysize, + const uint32_t hashval, const uint8_t idx) { + isc_ht_node_t *prev = NULL; + uint32_t hash; + + hash = hash_32(hashval, ht->hashbits[idx]); + + for (isc_ht_node_t *node = ht->table[idx][hash]; node != NULL; + prev = node, node = node->next) + { + if (isc__ht_node_match(node, hashval, key, keysize)) { + if (prev == NULL) { + ht->table[idx][hash] = node->next; + } else { + prev->next = node->next; } + isc_mem_put(ht->mctx, node, + sizeof(*node) + node->keysize); + ht->count--; + return (ISC_R_SUCCESS); } - node = node->next; } return (ISC_R_NOTFOUND); } isc_result_t -isc_ht_delete(isc_ht_t *ht, const unsigned char *key, uint32_t keysize) { - isc_ht_node_t *node, *prev; - uint32_t hash; +isc_ht_delete(isc_ht_t *ht, const unsigned char *key, const uint32_t keysize) { + uint32_t hashval; + uint8_t hindex; + isc_result_t result; REQUIRE(ISC_HT_VALID(ht)); REQUIRE(key != NULL && keysize > 0); - prev = NULL; - hash = isc_hash_function(key, keysize, true); - node = ht->table[hash & ht->mask]; - while (node != NULL) { - if (keysize == node->keysize && - memcmp(key, node->key, keysize) == 0) - { - if (prev == NULL) { - ht->table[hash & ht->mask] = node->next; - } else { - prev->next = node->next; - } - isc_mem_put(ht->mctx, node, - offsetof(isc_ht_node_t, key) + - node->keysize); - ht->count--; - - return (ISC_R_SUCCESS); - } - - prev = node; - node = node->next; + if (rehashing_in_progress(ht)) { + /* Rehash in progress */ + hashtable_rehash_one(ht); } - return (ISC_R_NOTFOUND); + + hindex = ht->hindex; + hashval = isc_hash32(key, keysize, ht->case_sensitive); +nexttable: + result = isc__ht_delete(ht, key, keysize, hashval, hindex); + + if (result == ISC_R_NOTFOUND && TRY_NEXTTABLE(hindex, ht)) { + /* + * Rehashing in progress, check the other table + */ + hindex = HT_NEXTTABLE(hindex); + goto nexttable; + } + + return (result); } void @@ -211,10 +445,10 @@ isc_ht_iter_create(isc_ht_t *ht, isc_ht_iter_t **itp) { REQUIRE(itp != NULL && *itp == NULL); it = isc_mem_get(ht->mctx, sizeof(isc_ht_iter_t)); - - it->ht = ht; - it->i = 0; - it->cur = NULL; + *it = (isc_ht_iter_t){ + .ht = ht, + .hindex = ht->hindex, + }; *itp = it; } @@ -229,25 +463,46 @@ isc_ht_iter_destroy(isc_ht_iter_t **itp) { it = *itp; *itp = NULL; ht = it->ht; - isc_mem_put(ht->mctx, it, sizeof(isc_ht_iter_t)); + isc_mem_put(ht->mctx, it, sizeof(*it)); } isc_result_t isc_ht_iter_first(isc_ht_iter_t *it) { + isc_ht_t *ht; + REQUIRE(it != NULL); + ht = it->ht; + + it->hindex = ht->hindex; it->i = 0; - while (it->i < it->ht->size && it->ht->table[it->i] == NULL) { + + return (isc__ht_iter_next(it)); +} + +static isc_result_t +isc__ht_iter_next(isc_ht_iter_t *it) { + isc_ht_t *ht = it->ht; + + while (it->i < ht->size[it->hindex] && + ht->table[it->hindex][it->i] == NULL) + { it->i++; } - if (it->i == it->ht->size) { - return (ISC_R_NOMORE); + if (it->i < ht->size[it->hindex]) { + it->cur = ht->table[it->hindex][it->i]; + + return (ISC_R_SUCCESS); } - it->cur = it->ht->table[it->i]; + if (TRY_NEXTTABLE(it->hindex, ht)) { + it->hindex = HT_NEXTTABLE(it->hindex); + it->i = 0; + return (isc__ht_iter_next(it)); + } - return (ISC_R_SUCCESS); + return (ISC_R_NOMORE); } isc_result_t @@ -256,60 +511,36 @@ isc_ht_iter_next(isc_ht_iter_t *it) { REQUIRE(it->cur != NULL); it->cur = it->cur->next; - if (it->cur == NULL) { - do { - it->i++; - } while (it->i < it->ht->size && it->ht->table[it->i] == NULL); - if (it->i >= it->ht->size) { - return (ISC_R_NOMORE); - } - it->cur = it->ht->table[it->i]; + + if (it->cur != NULL) { + return (ISC_R_SUCCESS); } - return (ISC_R_SUCCESS); + it->i++; + + return (isc__ht_iter_next(it)); } isc_result_t isc_ht_iter_delcurrent_next(isc_ht_iter_t *it) { isc_result_t result = ISC_R_SUCCESS; - isc_ht_node_t *to_delete = NULL; - isc_ht_node_t *prev = NULL; - isc_ht_node_t *node = NULL; - uint32_t hash; + isc_ht_node_t *dnode = NULL; + uint8_t dindex; isc_ht_t *ht; + isc_result_t dresult; + REQUIRE(it != NULL); REQUIRE(it->cur != NULL); - to_delete = it->cur; + ht = it->ht; + dnode = it->cur; + dindex = it->hindex; - it->cur = it->cur->next; - if (it->cur == NULL) { - do { - it->i++; - } while (it->i < ht->size && ht->table[it->i] == NULL); - if (it->i >= ht->size) { - result = ISC_R_NOMORE; - } else { - it->cur = ht->table[it->i]; - } - } + result = isc_ht_iter_next(it); - hash = isc_hash_function(to_delete->key, to_delete->keysize, true); - node = ht->table[hash & ht->mask]; - while (node != to_delete) { - prev = node; - node = node->next; - INSIST(node != NULL); - } - - if (prev == NULL) { - ht->table[hash & ht->mask] = node->next; - } else { - prev->next = node->next; - } - isc_mem_put(ht->mctx, node, - offsetof(isc_ht_node_t, key) + node->keysize); - ht->count--; + dresult = isc__ht_delete(ht, dnode->key, dnode->keysize, dnode->hashval, + dindex); + INSIST(dresult == ISC_R_SUCCESS); return (result); } @@ -334,8 +565,8 @@ isc_ht_iter_currentkey(isc_ht_iter_t *it, unsigned char **key, *keysize = it->cur->keysize; } -unsigned int -isc_ht_count(isc_ht_t *ht) { +size_t +isc_ht_count(const isc_ht_t *ht) { REQUIRE(ISC_HT_VALID(ht)); return (ht->count); diff --git a/lib/isc/include/isc/ht.h b/lib/isc/include/isc/ht.h index f1386bb515..163fbefb79 100644 --- a/lib/isc/include/isc/ht.h +++ b/lib/isc/include/isc/ht.h @@ -13,8 +13,7 @@ /* ! \file */ -#ifndef ISC_HT_H -#define ISC_HT_H 1 +#pragma once #include #include @@ -25,9 +24,15 @@ typedef struct isc_ht isc_ht_t; typedef struct isc_ht_iter isc_ht_iter_t; +enum { ISC_HT_CASE_SENSITIVE = 0x00, ISC_HT_CASE_INSENSITIVE = 0x01 }; + /*% * Initialize hashtable at *htp, using memory context and size of (1< Date: Wed, 11 Oct 2023 09:15:13 +0200 Subject: [PATCH 05/36] Add CHANGES and release note for [GL #4234] (cherry picked from commit 30d27928cff8a82774131b401c26b171a2367e31) --- CHANGES | 3 +++ doc/notes/notes-current.rst | 8 +++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 87c859c1ea..eb5c775ceb 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6315. [security] Speed up parsing of DNS messages with many different + names. (CVE-2023-4408) [GL #4234] + 6314. [bug] Address race conditions in dns_tsigkey_find(). [GL #4182] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index d69151dfe3..23847e23c0 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -15,7 +15,13 @@ Notes for BIND 9.16.46 Security Fixes ~~~~~~~~~~~~~~ -- None. +- Parsing DNS messages with many different names could cause excessive + CPU load. This has been fixed. :cve:`2023-4408` + + ISC would like to thank Shoham Danino from Reichman University, Anat + Bremler-Barr from Tel-Aviv University, Yehuda Afek from Tel-Aviv + University, and Yuval Shavitt from Tel-Aviv University for bringing + this vulnerability to our attention. :gl:`#4234` New Features ~~~~~~~~~~~~ From c44965af3314b516f6de5f3bf618df17fc026718 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 14 Nov 2023 15:35:57 +0100 Subject: [PATCH 06/36] Fix windows build, remove external symbols The functions dns_message_find and dns_message_movename have been removed. Remove the symbols from libdns.def.in to fix the windows build. --- lib/dns/win32/libdns.def.in | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index 5c0ba7c451..582bf90f45 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -535,7 +535,6 @@ dns_message_clonebuffer dns_message_create dns_message_currentname dns_message_detach -dns_message_find dns_message_findname dns_message_findtype dns_message_firstname @@ -554,7 +553,6 @@ dns_message_gettsigkey dns_message_headertotext dns_message_logfmtpacket dns_message_logpacket -dns_message_movename dns_message_nextname dns_message_parse dns_message_peekheader From c73262493658cb8623927ef6cc2f023501f7e809 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 10 Oct 2023 10:58:18 +1100 Subject: [PATCH 07/36] Save the correct result value to resume with nxdomain-redirect The wrong result value was being saved for resumption with nxdomain-redirect when performing the fetch. This lead to an assert when checking that RFC 1918 reverse queries where not leaking to the global internet. (cherry picked from commit 9d0fa07c5e7a39db89862a4f843d2190059afb4b) --- lib/ns/query.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/lib/ns/query.c b/lib/ns/query.c index 62b5ea8463..55d815e6c0 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -455,10 +455,10 @@ static void query_addnxrrsetnsec(query_ctx_t *qctx); static isc_result_t -query_nxdomain(query_ctx_t *qctx, bool empty_wild); +query_nxdomain(query_ctx_t *qctx, isc_result_t result); static isc_result_t -query_redirect(query_ctx_t *qctx); +query_redirect(query_ctx_t *qctx, isc_result_t result); static isc_result_t query_ncache(query_ctx_t *qctx, isc_result_t result); @@ -7345,8 +7345,7 @@ query_usestale(query_ctx_t *qctx, isc_result_t result) { * result from the search. */ static isc_result_t -query_gotanswer(query_ctx_t *qctx, isc_result_t res) { - isc_result_t result = res; +query_gotanswer(query_ctx_t *qctx, isc_result_t result) { char errmsg[256]; CCTRACE(ISC_LOG_DEBUG(3), "query_gotanswer"); @@ -7416,16 +7415,16 @@ root_key_sentinel: return (query_nodata(qctx, DNS_R_NXRRSET)); case DNS_R_EMPTYWILD: - return (query_nxdomain(qctx, true)); + return (query_nxdomain(qctx, DNS_R_EMPTYWILD)); case DNS_R_NXDOMAIN: - return (query_nxdomain(qctx, false)); + return (query_nxdomain(qctx, DNS_R_NXDOMAIN)); case DNS_R_COVERINGNSEC: return (query_coveringnsec(qctx)); case DNS_R_NCACHENXDOMAIN: - result = query_redirect(qctx); + result = query_redirect(qctx, result); if (result != ISC_R_COMPLETE) { return (result); } @@ -9243,10 +9242,10 @@ query_addnxrrsetnsec(query_ctx_t *qctx) { * Handle NXDOMAIN and empty wildcard responses. */ static isc_result_t -query_nxdomain(query_ctx_t *qctx, bool empty_wild) { +query_nxdomain(query_ctx_t *qctx, isc_result_t result) { dns_section_t section; uint32_t ttl; - isc_result_t result; + bool empty_wild = (result == DNS_R_EMPTYWILD); CCTRACE(ISC_LOG_DEBUG(3), "query_nxdomain"); @@ -9255,7 +9254,7 @@ query_nxdomain(query_ctx_t *qctx, bool empty_wild) { INSIST(qctx->is_zone || REDIRECT(qctx->client)); if (!empty_wild) { - result = query_redirect(qctx); + result = query_redirect(qctx, result); if (result != ISC_R_COMPLETE) { return (result); } @@ -9343,7 +9342,7 @@ cleanup: * redirecting, so query processing should continue past it. */ static isc_result_t -query_redirect(query_ctx_t *qctx) { +query_redirect(query_ctx_t *qctx, isc_result_t saved_result) { isc_result_t result; CCTRACE(ISC_LOG_DEBUG(3), "query_redirect"); @@ -9384,7 +9383,7 @@ query_redirect(query_ctx_t *qctx) { SAVE(qctx->client->query.redirect.rdataset, qctx->rdataset); SAVE(qctx->client->query.redirect.sigrdataset, qctx->sigrdataset); - qctx->client->query.redirect.result = DNS_R_NCACHENXDOMAIN; + qctx->client->query.redirect.result = saved_result; dns_name_copynf(qctx->fname, qctx->client->query.redirect.fname); qctx->client->query.redirect.authoritative = @@ -10005,7 +10004,7 @@ query_coveringnsec(query_ctx_t *qctx) { * We now have the proof that we have an NXDOMAIN. Apply * NXDOMAIN redirection if configured. */ - result = query_redirect(qctx); + result = query_redirect(qctx, DNS_R_COVERINGNSEC); if (result != ISC_R_COMPLETE) { redirected = true; goto cleanup; From 47d4c0e5a676bd664a949d9fc472064c53ac3a3d Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 10 Oct 2023 11:09:11 +1100 Subject: [PATCH 08/36] Add CHANGES note for [GL #4281] (cherry picked from commit 0748965b7ce7b80ff0062c7ffc661d52335c780e) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index eb5c775ceb..fda7a3ff18 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6316. [security] Specific queries could trigger an assertion check with + nxdomain-redirect enabled. (CVE-2023-5517) [GL #4281] + 6315. [security] Speed up parsing of DNS messages with many different names. (CVE-2023-4408) [GL #4234] From 20754ea186b68dd2e00b498c463ccefe48d8fc85 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 10 Oct 2023 11:12:24 +1100 Subject: [PATCH 09/36] Add release note for [GL #4281] (cherry picked from commit 2fbafc2675d19136993980765c0589da599f8403) --- doc/notes/notes-current.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 23847e23c0..f7729e9f52 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -23,6 +23,10 @@ Security Fixes University, and Yuval Shavitt from Tel-Aviv University for bringing this vulnerability to our attention. :gl:`#4234` +- Specific queries could cause :iscman:`named` to crash with an + assertion failure when ``nxdomain-redirect`` was enabled. This has + been fixed. :cve:`2023-5517` :gl:`#4281` + New Features ~~~~~~~~~~~~ From 7db2796507127b40e2f091dafb842c6a7e86b9a8 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 12 Oct 2023 12:01:46 +1100 Subject: [PATCH 10/36] Restore dns64 state during serve-stale processing If we are in the process of looking for the A records as part of dns64 processing and the server-stale timeout triggers, redo the dns64 changes that had been made to the orignal qctx. (cherry picked from commit 1fcc483df13e049b96f620e515f0d4d45f3680b7) --- lib/ns/query.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/ns/query.c b/lib/ns/query.c index 55d815e6c0..1290c308af 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -6095,6 +6095,13 @@ query_lookup_stale(ns_client_t *client) { query_ctx_t qctx; qctx_init(client, NULL, client->query.qtype, &qctx); + if (DNS64(client)) { + qctx.qtype = qctx.type = dns_rdatatype_a; + qctx.dns64 = true; + } + if (DNS64EXCLUDE(client)) { + qctx.dns64_exclude = true; + } dns_db_attach(client->view->cachedb, &qctx.db); client->query.attributes &= ~NS_QUERYATTR_RECURSIONOK; client->query.dboptions |= DNS_DBFIND_STALETIMEOUT; From 10bfb3de950617ab807ababacfcda11d25b7af39 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 12 Oct 2023 13:39:57 +1100 Subject: [PATCH 11/36] Add CHANGES note for [GL #4334] (cherry picked from commit 26671f8c478a66296ee5874adbe741c890e435d1) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index fda7a3ff18..e1fce27f79 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6317. [security] Restore DNS64 state when handling a serve-stale timeout. + (CVE-2023-5679) [GL #4334] + 6316. [security] Specific queries could trigger an assertion check with nxdomain-redirect enabled. (CVE-2023-5517) [GL #4281] From 549d860b33694e0f30ba9f035165badc137c4f5f Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 12 Oct 2023 13:42:38 +1100 Subject: [PATCH 12/36] Add release note for [GL #4334] (cherry picked from commit c4faf5c69f7eba3b23b8a932e66fc89ec3bee4a9) --- doc/notes/notes-current.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index f7729e9f52..738f2ef042 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -27,6 +27,11 @@ Security Fixes assertion failure when ``nxdomain-redirect`` was enabled. This has been fixed. :cve:`2023-5517` :gl:`#4281` +- A bad interaction between DNS64 and serve-stale could cause + :iscman:`named` to crash with an assertion failure, when both of these + features were enabled. This has been fixed. :cve:`2023-5679` + :gl:`#4334` + New Features ~~~~~~~~~~~~ From c3377cbfaa44dcb033f5abfb2db031612c8f47d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 4 Jan 2024 13:39:27 +0100 Subject: [PATCH 13/36] Limit isc_task_send() overhead for tree pruning Instead of issuing a separate isc_task_send() call for every RBTDB node that triggers tree pruning, maintain a list of nodes from which tree pruning can be started from and only issue an isc_task_send() call if pruning has not yet been triggered by another RBTDB node. The extra queuing overhead eliminated by this change could be remotely exploited to cause excessive memory use. As this change modifies struct dns_rbtnode by adding a new 'prunelink' member to it, bump MAPAPI to prevent any attempts of loading map-format zone files created using older BIND 9 versions. (cherry picked from commit 24381cc36d8528f5a4046fb2614451aeac4cdfc1) --- lib/dns/include/dns/rbt.h | 6 ++ lib/dns/mapapi | 2 +- lib/dns/rbt.c | 1 + lib/dns/rbtdb.c | 149 +++++++++++++++++++++++++------------- 4 files changed, 107 insertions(+), 51 deletions(-) diff --git a/lib/dns/include/dns/rbt.h b/lib/dns/include/dns/rbt.h index 4a6b0781c7..6cfd40ffdd 100644 --- a/lib/dns/include/dns/rbt.h +++ b/lib/dns/include/dns/rbt.h @@ -140,6 +140,12 @@ struct dns_rbtnode { */ ISC_LINK(dns_rbtnode_t) deadlink; + /*% + * This linked list is used to store nodes from which tree pruning can + * be started. + */ + ISC_LINK(dns_rbtnode_t) prunelink; + /*@{*/ /*! * These values are used in the RBT DB implementation. The appropriate diff --git a/lib/dns/mapapi b/lib/dns/mapapi index 1b502d34cc..a46e190937 100644 --- a/lib/dns/mapapi +++ b/lib/dns/mapapi @@ -13,4 +13,4 @@ # Whenever releasing a new major release of BIND9, set this value # back to 1.0 when releasing the first alpha. Map files are *never* # compatible across major releases. -MAPAPI=3.0 +MAPAPI=4.0 diff --git a/lib/dns/rbt.c b/lib/dns/rbt.c index d5d18b836e..780a950a48 100644 --- a/lib/dns/rbt.c +++ b/lib/dns/rbt.c @@ -2307,6 +2307,7 @@ create_node(isc_mem_t *mctx, const dns_name_t *name, dns_rbtnode_t **nodep) { HASHVAL(node) = 0; ISC_LINK_INIT(node, deadlink); + ISC_LINK_INIT(node, prunelink); LOCKNUM(node) = 0; WILD(node) = 0; diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index fbb4b84259..723d169532 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -521,6 +521,10 @@ struct dns_rbtdb { */ rbtnodelist_t *deadnodes; + /* List of nodes from which recursive tree pruning can be started from. + * Locked by tree_lock. */ + rbtnodelist_t prunenodes; + /* * Heaps. These are used for TTL based expiry in a cache, * or for zone resigning in a zone DB. hmctx is the memory @@ -1067,6 +1071,7 @@ free_rbtdb(dns_rbtdb_t *rbtdb, bool log, isc_event_t *event) { unsigned int i; isc_result_t result; char buf[DNS_NAME_FORMATSIZE]; + dns_rbtnode_t *node = NULL; dns_rbt_t **treep; isc_time_t start; @@ -1092,8 +1097,6 @@ free_rbtdb(dns_rbtdb_t *rbtdb, bool log, isc_event_t *event) { * the overhead of unlinking all nodes here should be negligible. */ for (i = 0; i < rbtdb->node_lock_count; i++) { - dns_rbtnode_t *node; - node = ISC_LIST_HEAD(rbtdb->deadnodes[i]); while (node != NULL) { ISC_LIST_UNLINK(rbtdb->deadnodes[i], node, deadlink); @@ -1101,6 +1104,12 @@ free_rbtdb(dns_rbtdb_t *rbtdb, bool log, isc_event_t *event) { } } + node = ISC_LIST_HEAD(rbtdb->prunenodes); + while (node != NULL) { + ISC_LIST_UNLINK(rbtdb->prunenodes, node, prunelink); + node = ISC_LIST_HEAD(rbtdb->prunenodes); + } + if (event == NULL) { rbtdb->quantum = (rbtdb->task != NULL) ? 100 : 0; } @@ -1935,19 +1944,32 @@ is_leaf(dns_rbtnode_t *node) { node->left == NULL && node->right == NULL); } +/*% + * The tree lock must be held when this function is called as it reads and + * updates rbtdb->prunenodes. + */ static void send_to_prune_tree(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node, isc_rwlocktype_t locktype) { - isc_event_t *ev; - dns_db_t *db; + bool pruning_queued = (ISC_LIST_HEAD(rbtdb->prunenodes) != NULL); + + INSIST(locktype == isc_rwlocktype_write); - ev = isc_event_allocate(rbtdb->common.mctx, NULL, DNS_EVENT_RBTPRUNE, - prune_tree, node, sizeof(isc_event_t)); new_reference(rbtdb, node, locktype); - db = NULL; - attach((dns_db_t *)rbtdb, &db); - ev->ev_sender = db; - isc_task_send(rbtdb->task, &ev); + INSIST(!ISC_LINK_LINKED(node, prunelink)); + ISC_LIST_APPEND(rbtdb->prunenodes, node, prunelink); + + if (!pruning_queued) { + isc_event_t *ev = NULL; + dns_db_t *db = NULL; + + attach((dns_db_t *)rbtdb, &db); + + ev = isc_event_allocate(rbtdb->common.mctx, NULL, + DNS_EVENT_RBTPRUNE, prune_tree, db, + sizeof(isc_event_t)); + isc_task_send(rbtdb->task, &ev); + } } /*% @@ -2222,17 +2244,26 @@ restore_locks: } /* - * Prune the tree by recursively cleaning-up single leaves. In the worst - * case, the number of iteration is the number of tree levels, which is at - * most the maximum number of domain name labels, i.e, 127. In practice, this - * should be much smaller (only a few times), and even the worst case would be - * acceptable for a single event. + * Prune the tree by recursively cleaning up single leaves. Go through all + * nodes stored in the rbtdb->prunenodes list; for each of them, in the worst + * case, it will be necessary to traverse a number of tree levels equal to the + * maximum legal number of domain name labels (127); in practice, the number of + * tree levels to traverse will virtually always be much smaller (a few levels + * at most). While holding the tree lock throughout this entire operation is + * less than ideal, so is splitting the latter up by queueing a separate + * prune_tree() run for each node to start pruning from (as queueing requires + * allocating memory and can therefore potentially be exploited to exhaust + * available memory). Also note that actually freeing up the memory used by + * RBTDB nodes (which is what this function does) is essential to keeping cache + * memory use in check, so since the tree lock needs to be acquired anyway, + * freeing as many nodes as possible before the tree lock gets released is + * prudent. */ static void prune_tree(isc_task_t *task, isc_event_t *event) { - dns_rbtdb_t *rbtdb = event->ev_sender; - dns_rbtnode_t *node = event->ev_arg; - dns_rbtnode_t *parent; + dns_rbtdb_t *rbtdb = (dns_rbtdb_t *)event->ev_arg; + dns_rbtnode_t *node = NULL; + dns_rbtnode_t *parent = NULL; unsigned int locknum; UNUSED(task); @@ -2240,44 +2271,60 @@ prune_tree(isc_task_t *task, isc_event_t *event) { isc_event_free(&event); RWLOCK(&rbtdb->tree_lock, isc_rwlocktype_write); - locknum = node->locknum; - NODE_LOCK(&rbtdb->node_locks[locknum].lock, isc_rwlocktype_write); - do { - parent = node->parent; - decrement_reference(rbtdb, node, 0, isc_rwlocktype_write, - isc_rwlocktype_write, true); - if (parent != NULL && parent->down == NULL) { - /* - * node was the only down child of the parent and has - * just been removed. We'll then need to examine the - * parent. Keep the lock if possible; otherwise, - * release the old lock and acquire one for the parent. - */ - if (parent->locknum != locknum) { - NODE_UNLOCK(&rbtdb->node_locks[locknum].lock, - isc_rwlocktype_write); - locknum = parent->locknum; - NODE_LOCK(&rbtdb->node_locks[locknum].lock, - isc_rwlocktype_write); + while ((node = ISC_LIST_HEAD(rbtdb->prunenodes)) != NULL) { + locknum = node->locknum; + NODE_LOCK(&rbtdb->node_locks[locknum].lock, + isc_rwlocktype_write); + do { + if (ISC_LINK_LINKED(node, prunelink)) { + ISC_LIST_UNLINK(rbtdb->prunenodes, node, + prunelink); } - /* - * We need to gain a reference to the node before - * decrementing it in the next iteration. - */ - if (ISC_LINK_LINKED(parent, deadlink)) { - ISC_LIST_UNLINK(rbtdb->deadnodes[locknum], + parent = node->parent; + decrement_reference(rbtdb, node, 0, + isc_rwlocktype_write, + isc_rwlocktype_write, true); + + if (parent != NULL && parent->down == NULL) { + /* + * node was the only down child of the parent + * and has just been removed. We'll then need + * to examine the parent. Keep the lock if + * possible; otherwise, release the old lock and + * acquire one for the parent. + */ + if (parent->locknum != locknum) { + NODE_UNLOCK( + &rbtdb->node_locks[locknum].lock, + isc_rwlocktype_write); + locknum = parent->locknum; + NODE_LOCK( + &rbtdb->node_locks[locknum].lock, + isc_rwlocktype_write); + } + + /* + * We need to gain a reference to the node + * before decrementing it in the next iteration. + */ + if (ISC_LINK_LINKED(parent, deadlink)) { + ISC_LIST_UNLINK( + rbtdb->deadnodes[locknum], parent, deadlink); + } + new_reference(rbtdb, parent, + isc_rwlocktype_write); + } else { + parent = NULL; } - new_reference(rbtdb, parent, isc_rwlocktype_write); - } else { - parent = NULL; - } - node = parent; - } while (node != NULL); - NODE_UNLOCK(&rbtdb->node_locks[locknum].lock, isc_rwlocktype_write); + node = parent; + } while (node != NULL); + NODE_UNLOCK(&rbtdb->node_locks[locknum].lock, + isc_rwlocktype_write); + } RWUNLOCK(&rbtdb->tree_lock, isc_rwlocktype_write); detach((dns_db_t **)&rbtdb); @@ -8735,6 +8782,8 @@ dns_rbtdb_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type, ISC_LIST_INIT(rbtdb->deadnodes[i]); } + ISC_LIST_INIT(rbtdb->prunenodes); + rbtdb->active = rbtdb->node_lock_count; for (i = 0; i < (int)(rbtdb->node_lock_count); i++) { From f66150204c2e94aab2cda97b40d0f02bcd057c92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 4 Jan 2024 13:39:27 +0100 Subject: [PATCH 14/36] Add CHANGES entry and release note for GL #4383 (cherry picked from commit 04df558d57105ccfb9853ba892021c9de01d30c9) --- CHANGES | 4 ++++ doc/notes/notes-current.rst | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/CHANGES b/CHANGES index e1fce27f79..c73d3f52b8 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6319. [security] Query patterns that continuously triggered cache + database maintenance could exhaust all available memory + on the host running named. (CVE-2023-6516) [GL #4383] + 6317. [security] Restore DNS64 state when handling a serve-stale timeout. (CVE-2023-5679) [GL #4334] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 738f2ef042..9cbc363108 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -32,6 +32,15 @@ Security Fixes features were enabled. This has been fixed. :cve:`2023-5679` :gl:`#4334` +- Query patterns that continuously triggered cache database maintenance + could cause an excessive amount of memory to be allocated, exceeding + ``max-cache-size`` and potentially leading to all available memory on + the host running :iscman:`named` being exhausted. This has been fixed. + :cve:`2023-6516` + + ISC would like to thank Infoblox for bringing this vulnerability to + our attention. :gl:`#4383` + New Features ~~~~~~~~~~~~ From 1237d73cd1120b146ee699bbae7b2fe837cf2f98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 4 Jan 2024 13:39:27 +0100 Subject: [PATCH 15/36] Fix map offsets in the "masterformat" system test The "masterformat" system test attempts to check named-checkzone behavior when it is fed corrupt map-format zone files. However, despite the RBTDB and RBT structures having evolved over the years, the offsets at which a valid map-format zone file is malformed by the "masterformat" test have not been updated accordingly, causing the relevant checks to introduce a different type of corruption than they were originally meant to cause: - the "bad node header" check originally mangled the 'type' member of the rdatasetheader_t structure for cname.example.nil, - the "bad node data" check originally mangled the 'serial' and 'rdh_ttl' members of the rdatasetheader_t structure for aaaa.example.nil. Update the offsets at which the map-format zone file is malformed at by the "masterformat" system test so that the relevant checks fulfill their original purpose again. --- bin/tests/system/masterformat/tests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/tests/system/masterformat/tests.sh b/bin/tests/system/masterformat/tests.sh index ab999fc057..e83f551f61 100755 --- a/bin/tests/system/masterformat/tests.sh +++ b/bin/tests/system/masterformat/tests.sh @@ -293,7 +293,7 @@ status=$((status + ret)) echo_i "checking corrupt map files fail to load (bad node header) ($n)" ret=0 cp map.5 badmap -stomp badmap 2754 2 99 +stomp badmap 3706 2 99 $CHECKZONE -D -f map -F text -o text.5 example.nil badmap >/dev/null [ $? = 1 ] || ret=1 n=$((n + 1)) @@ -303,7 +303,7 @@ status=$((status + ret)) echo_i "checking corrupt map files fail to load (bad node data) ($n)" ret=0 cp map.5 badmap -stomp badmap 2897 5 127 +stomp badmap 3137 5 127 $CHECKZONE -D -f map -F text -o text.5 example.nil badmap >/dev/null [ $? = 1 ] || ret=1 n=$((n + 1)) From c53e81a80084f6233fcdd3c9fa5b9c56d1834c55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 5 Jan 2024 13:01:26 +0100 Subject: [PATCH 16/36] Prepare release notes for BIND 9.16.46 --- doc/arm/notes.rst | 2 +- .../{notes-current.rst => notes-9.16.46.rst} | 15 --------------- 2 files changed, 1 insertion(+), 16 deletions(-) rename doc/notes/{notes-current.rst => notes-9.16.46.rst} (94%) diff --git a/doc/arm/notes.rst b/doc/arm/notes.rst index b0fc8b6ec9..8532fec4f8 100644 --- a/doc/arm/notes.rst +++ b/doc/arm/notes.rst @@ -46,7 +46,7 @@ for Microsoft Windows operating systems. .. include:: ../notes/notes-known-issues.rst -.. include:: ../notes/notes-current.rst +.. include:: ../notes/notes-9.16.46.rst .. include:: ../notes/notes-9.16.45.rst .. include:: ../notes/notes-9.16.44.rst .. include:: ../notes/notes-9.16.43.rst diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-9.16.46.rst similarity index 94% rename from doc/notes/notes-current.rst rename to doc/notes/notes-9.16.46.rst index 9cbc363108..8a2d3459a9 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-9.16.46.rst @@ -41,27 +41,12 @@ Security Fixes ISC would like to thank Infoblox for bringing this vulnerability to our attention. :gl:`#4383` -New Features -~~~~~~~~~~~~ - -- None. - Removed Features ~~~~~~~~~~~~~~~~ - The support for AES algorithm for DNS cookies has been deprecated. :gl:`#4421` -Feature Changes -~~~~~~~~~~~~~~~ - -- None. - -Bug Fixes -~~~~~~~~~ - -- None. - Known Issues ~~~~~~~~~~~~ From 86a78021fe6a63447f40552f8e6c44b374f50712 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 5 Jan 2024 13:01:26 +0100 Subject: [PATCH 17/36] Tweak and reword release notes --- doc/notes/notes-9.16.46.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/notes/notes-9.16.46.rst b/doc/notes/notes-9.16.46.rst index 8a2d3459a9..314f89e0c7 100644 --- a/doc/notes/notes-9.16.46.rst +++ b/doc/notes/notes-9.16.46.rst @@ -44,8 +44,9 @@ Security Fixes Removed Features ~~~~~~~~~~~~~~~~ -- The support for AES algorithm for DNS cookies has been deprecated. - :gl:`#4421` +- Support for using AES as the DNS COOKIE algorithm (``cookie-algorithm + aes;``) has been deprecated and will be removed in a future release. + Please use the current default, SipHash-2-4, instead. :gl:`#4421` Known Issues ~~~~~~~~~~~~ From 0b43bf1a382e77571c5808e8ca73ea29d3a95c47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 5 Jan 2024 12:51:13 +0100 Subject: [PATCH 18/36] Fix Danger rules for flagging release note issues The logic contained in dangerfile.py incorrectly warns about missing release note changes for merge requests preparing release documentation as such merge requests rename files in the doc/notes/ directory. This (correctly) causes these files to be passed to dangerfile.py via danger.git.created_files and danger.git.deleted_files rather than via danger.git.modified_files, which in turn causes the logic checking the use of the "Release Notes" label to assume that no release notes are added, removed, or modified by a given merge request. Fix by considering all types of file changes (modifications, additions, and removals - which also covers file renaming) when checking whether a given merge request modifies release notes. Update the warning messages accordingly. However, when trying to find release notes added by a given merge request, deleted files must not be considered. Tweak the logic looking for GitLab identifiers in the release notes added by a given merge request so that it only scans modified and added (or renamed) files. (cherry picked from commit 0fec404c64cf367b9c2d676535c50f228355b6b5) --- dangerfile.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/dangerfile.py b/dangerfile.py index 119f6e23a3..cb25ffb3cc 100644 --- a/dangerfile.py +++ b/dangerfile.py @@ -44,6 +44,9 @@ relnotes_issue_or_mr_id_regex = re.compile(rb":gl:`[#!][0-9]+`") release_notes_regex = re.compile(r"doc/(arm|notes)/notes-.*\.(rst|xml)") modified_files = danger.git.modified_files +affected_files = ( + danger.git.modified_files + danger.git.created_files + danger.git.deleted_files +) mr_labels = danger.gitlab.mr.labels target_branch = danger.gitlab.mr.target_branch is_backport = "Backport" in mr_labels or "Backport::Partial" in mr_labels @@ -341,18 +344,18 @@ if changes_added_lines: # MR. release_notes_regex = re.compile(r"doc/(arm|notes)/notes-.*\.(rst|xml)") -release_notes_changed = list(filter(release_notes_regex.match, modified_files)) +release_notes_changed = list(filter(release_notes_regex.match, affected_files)) release_notes_label_set = "Release Notes" in mr_labels if not release_notes_changed: if release_notes_label_set: fail( "This merge request has the *Release Notes* label set. " - "Add a release note or unset the *Release Notes* label." + "Update release notes or unset the *Release Notes* label." ) elif "Customer" in mr_labels: warn( "This merge request has the *Customer* label set. " - "Add a release note unless the changes introduced are trivial." + "Update release notes unless the changes introduced are trivial." ) if release_notes_changed and not release_notes_label_set: fail( @@ -361,7 +364,9 @@ if release_notes_changed and not release_notes_label_set: ) if release_notes_changed: - notes_added_lines = added_lines(target_branch, release_notes_changed) + modified_or_new_files = danger.git.modified_files + danger.git.created_files + release_notes_added = list(filter(release_notes_regex.match, modified_or_new_files)) + notes_added_lines = added_lines(target_branch, release_notes_added) identifiers_found = filter(relnotes_issue_or_mr_id_regex.search, notes_added_lines) if notes_added_lines and not any(identifiers_found): warn("No valid issue/MR identifiers found in added release notes.") From ac85b33c277e325758baf79b83ca106ac7fea2dd Mon Sep 17 00:00:00 2001 From: Michal Nowak Date: Fri, 5 Jan 2024 15:19:34 +0100 Subject: [PATCH 19/36] prep 9.16.46 --- CHANGES | 2 ++ version | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index c73d3f52b8..d1e601dc31 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,5 @@ + --- 9.16.46 released --- + 6319. [security] Query patterns that continuously triggered cache database maintenance could exhaust all available memory on the host running named. (CVE-2023-6516) [GL #4383] diff --git a/version b/version index 2561f7377b..698a143ff8 100644 --- a/version +++ b/version @@ -8,4 +8,4 @@ MINORVER=16 PATCHVER=46 RELEASETYPE= RELEASEVER= -EXTENSIONS=-dev +EXTENSIONS= From 0bbb0065e63c3231b320bd20d1121aed6c4d00d8 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 16 Jan 2024 16:03:15 -0800 Subject: [PATCH 20/36] fix a message parsing regression the fix for CVE-2023-4408 introduced a regression in the message parser, which could cause a crash if duplicate rdatasets were found in the question section. this commit ensures that rdatasets are correctly disassociated and freed when this occurs. (cherry picked from commit 4c19d35614f8cd80d8748156a5bad361e19abc28) --- lib/dns/message.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/dns/message.c b/lib/dns/message.c index d216cb1742..af8ac3d881 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -1128,9 +1128,7 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, rdatalist->rdclass = rdclass; result = dns_rdatalist_tordataset(rdatalist, rdataset); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } + RUNTIME_CHECK(result == ISC_R_SUCCESS); rdataset->attributes |= DNS_RDATASETATTR_QUESTION; @@ -1177,7 +1175,9 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, cleanup: if (rdataset != NULL) { - INSIST(!dns_rdataset_isassociated(rdataset)); + if (dns_rdataset_isassociated(rdataset)) { + dns_rdataset_disassociate(rdataset); + } isc_mempool_put(msg->rdspool, rdataset); } if (free_name) { From f397ff5bb81413004fa6367f63a833fe70a3ac59 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 29 Jan 2024 12:26:00 -0800 Subject: [PATCH 21/36] fix another message parsing regression The fix for CVE-2023-4408 introduced a regression in the message parser, which could cause a crash if an rdata type that can only occur in the question was found in another section. (cherry picked from commit 510f1de8a6add516b842a55750366944701d3d9a) --- lib/dns/message.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/lib/dns/message.c b/lib/dns/message.c index af8ac3d881..22aa552f3f 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -1299,6 +1299,7 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, isedns = false; issigzero = false; istsig = false; + found_rdataset = NULL; name = NULL; result = dns_message_gettempname(msg, &name); @@ -1446,10 +1447,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, * Then put the meta-class back into the finished rdata. */ rdata = newrdata(msg); - if (rdata == NULL) { - result = ISC_R_NOMEMORY; - goto cleanup; - } if (msg->opcode == dns_opcode_update && update(sectionid, rdclass)) { @@ -1608,7 +1605,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, * the question section, fail. */ if (dns_rdatatype_questiononly(rdtype)) { - dns_message_puttemprdataset(msg, &rdataset); DO_ERROR(DNS_R_FORMERR); } @@ -1656,18 +1652,17 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, /* Free the rdataset we used as the key */ dns_rdataset_disassociate(rdataset); isc_mempool_put(msg->rdspool, rdataset); - rdataset = found_rdataset; - result = ISC_R_SUCCESS; + rdataset = found_rdataset; if (!dns_rdatatype_issingleton(rdtype)) { break; } - dns_rdata_t *first; dns_rdatalist_fromrdataset(rdataset, &rdatalist); - first = ISC_LIST_HEAD(rdatalist->rdata); + dns_rdata_t *first = + ISC_LIST_HEAD(rdatalist->rdata); INSIST(first != NULL); if (dns_rdata_compare(rdata, first) != 0) { DO_ERROR(DNS_R_FORMERR); @@ -1712,7 +1707,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, dns_rcode_t ercode; msg->opt = rdataset; - rdataset = NULL; ercode = (dns_rcode_t)((msg->opt->ttl & DNS_MESSAGE_EDNSRCODE_MASK) >> 20); @@ -1723,7 +1717,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, msg->sig0 = rdataset; msg->sig0name = name; msg->sigstart = recstart; - rdataset = NULL; free_name = false; } else if (istsig) { msg->tsig = rdataset; @@ -1733,9 +1726,9 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, * Windows doesn't like TSIG names to be compressed. */ msg->tsigname->attributes |= DNS_NAMEATTR_NOCOMPRESS; - rdataset = NULL; free_name = false; } + rdataset = NULL; if (seen_problem) { if (free_name) { @@ -1744,8 +1737,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, free_name = false; } INSIST(!free_name); - - rdataset = NULL; } /* @@ -1767,6 +1758,10 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, } cleanup: + if (rdataset != NULL && rdataset != found_rdataset) { + dns_rdataset_disassociate(rdataset); + isc_mempool_put(msg->rdspool, rdataset); + } if (free_name) { dns_message_puttempname(msg, &name); } From 010a6606245d6775864c89afc3e799fa640a43e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 31 Jan 2024 11:52:19 +0100 Subject: [PATCH 22/36] Add a CHANGES entry (cherry picked from commit 04ba284e1ace177d095599c8708126093a91336d) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index d1e601dc31..adbae5e334 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6321. [security] Change 6315 inadvertently introduced regressions that + could cause named to crash. [GL #4234] + --- 9.16.46 released --- 6319. [security] Query patterns that continuously triggered cache From c12608ca934c0433d280e65fe6c631013e200cfe 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 23/36] 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. (cherry picked from commit 1b3b0cef224e7a9e8279c5cfe2f7e188e3777cc7) --- lib/dns/resolver.c | 4 ++-- lib/isc/include/isc/netmgr.h | 3 +++ lib/isc/netmgr/netmgr-int.h | 1 + lib/isc/netmgr/netmgr.c | 36 +++++++++++++++++++++++------------- lib/isc/netmgr/tcp.c | 6 +++--- lib/isc/netmgr/tcpdns.c | 4 ++-- lib/isc/netmgr/udp.c | 6 +++--- 7 files changed, 37 insertions(+), 23 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index a97aaa85e9..0952624689 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -10634,8 +10634,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) { isc_mutex_destroy(&res->buckets[i].lock); goto cleanup_buckets; diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index f1747be058..efeb5f3d00 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -479,6 +479,9 @@ isc_nm_tcpdnsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, * 'cb'. */ +#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/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 05fde1a791..c3a176260b 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -655,6 +655,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 6f42ec90ff..7bff1cc9b3 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -231,12 +231,12 @@ isc__nm_winsock_destroy(void) { #endif /* WIN32 */ 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); } } @@ -254,11 +254,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) { @@ -282,10 +282,13 @@ isc__netmgr_create(isc_mem_t *mctx, uint32_t workers, isc_nm_t **netmgrp) { isc__nm_winsock_initialize(); #endif /* WIN32 */ - 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); @@ -316,11 +319,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; @@ -354,7 +358,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); } @@ -840,9 +844,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]; diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index 821d6c4568..1666318453 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -323,7 +323,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); @@ -341,7 +341,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); } @@ -445,7 +445,7 @@ isc_nm_listentcp(isc_nm_t *mgr, isc_sockaddr_t *iface, #if defined(WIN32) sock->nchildren = 1; #else - sock->nchildren = mgr->nworkers; + sock->nchildren = mgr->nlisteners; #endif children_size = sock->nchildren * sizeof(sock->children[0]); sock->children = isc_mem_get(mgr->mctx, children_size); diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index bd593ebf4f..037d74c607 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -303,7 +303,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); } @@ -410,7 +410,7 @@ isc_nm_listentcpdns(isc_nm_t *mgr, isc_sockaddr_t *iface, #if defined(WIN32) sock->nchildren = 1; #else - sock->nchildren = mgr->nworkers; + sock->nchildren = mgr->nlisteners; #endif children_size = sock->nchildren * sizeof(sock->children[0]); sock->children = isc_mem_get(mgr->mctx, children_size); diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c index 00f9d40884..bc59fca543 100644 --- a/lib/isc/netmgr/udp.c +++ b/lib/isc/netmgr/udp.c @@ -136,7 +136,7 @@ isc_nm_listenudp(isc_nm_t *mgr, isc_sockaddr_t *iface, isc_nm_recv_cb_t cb, uv_os_sock_t fd = -1; /* - * 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)); @@ -146,7 +146,7 @@ isc_nm_listenudp(isc_nm_t *mgr, isc_sockaddr_t *iface, isc_nm_recv_cb_t cb, #if defined(WIN32) sock->nchildren = 1; #else - sock->nchildren = mgr->nworkers; + sock->nchildren = mgr->nlisteners; #endif children_size = sock->nchildren * sizeof(sock->children[0]); @@ -847,7 +847,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 751b7cc4750ede6d8c5232751d60aad8ad84aa67 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 22 Nov 2023 16:59:03 +1100 Subject: [PATCH 24/36] Skip revoked keys when selecting DNSKEY in the validation loop Don't select revoked keys when iterating through DNSKEYs in the DNSSEC validation routines. (cherry picked from commit 439e16e4de525599bbb5a31575211d06cc3e2fbb) --- lib/dns/validator.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 6cf717f870..8bec8fed6c 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 6a65a425283d70da86bf732449acd6d7c8dec718 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 22 Nov 2023 16:59:03 +1100 Subject: [PATCH 25/36] Fail processing incoming DNS message on first validation failure Stop processing the DNS validation when first validation failure occurs in the DNS message. (cherry picked from commit 0add2934775dcfb05ea6ff9849c8c0c4a65fb009) --- 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 b435cd67fb..c5d7a31a41 100644 --- a/lib/dns/include/dns/validator.h +++ b/lib/dns/include/dns/validator.h @@ -149,6 +149,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 8bec8fed6c..e8e840837d 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 3d206e918b3efbc20074629ad9d99095fbd2e5fd 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 26/36] 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. (cherry picked from commit 21af5c9a97ed73345799dd4dfec493cc6785e40b) --- 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 e8e840837d..9dce558128 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 a520fbc0470a0d6b72db6aa0b8deda8798551614 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 27/36] 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. (cherry picked from commit f39cd17a26959e80b3ff4ce38e98936312395610) --- 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 d49beb2ec9..df5a358e5f 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); @@ -780,6 +781,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; @@ -810,7 +818,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); } @@ -831,7 +839,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); } @@ -2337,7 +2345,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; @@ -2362,10 +2371,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 3185e9f91b..3e3cfe6a68 100644 --- a/lib/dns/include/dst/dst.h +++ b/lib/dns/include/dst/dst.h @@ -471,6 +471,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 9dce558128..243b19f64e 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 0ab4125801b1b158fde89afbb335401fe77da2e6 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 28/36] Add CHANGES and release note for [GL #4424] (cherry picked from commit c847092a17b98e14e1e1efea7ab06ba86595756e) --- 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 adbae5e334..60f0a6a230 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..9bd9752d33 --- /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.16.47 +---------------------- + +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. From 83cad74ae21b445d993898ca4d8a610cedf7d363 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 2 Feb 2024 09:23:00 +0100 Subject: [PATCH 29/36] Prepare release notes for BIND 9.16.47 --- doc/arm/notes.rst | 1 + doc/notes/notes-9.16.46.rst | 45 +++---------------------- doc/notes/notes-9.16.47.rst | 65 +++++++++++++++++++++++++++++++++++++ doc/notes/notes-current.rst | 31 ------------------ 4 files changed, 70 insertions(+), 72 deletions(-) create mode 100644 doc/notes/notes-9.16.47.rst delete mode 100644 doc/notes/notes-current.rst diff --git a/doc/arm/notes.rst b/doc/arm/notes.rst index 8532fec4f8..94e0ce43af 100644 --- a/doc/arm/notes.rst +++ b/doc/arm/notes.rst @@ -46,6 +46,7 @@ for Microsoft Windows operating systems. .. include:: ../notes/notes-known-issues.rst +.. include:: ../notes/notes-9.16.47.rst .. include:: ../notes/notes-9.16.46.rst .. include:: ../notes/notes-9.16.45.rst .. include:: ../notes/notes-9.16.44.rst diff --git a/doc/notes/notes-9.16.46.rst b/doc/notes/notes-9.16.46.rst index 314f89e0c7..b0af65aec3 100644 --- a/doc/notes/notes-9.16.46.rst +++ b/doc/notes/notes-9.16.46.rst @@ -12,45 +12,8 @@ Notes for BIND 9.16.46 ---------------------- -Security Fixes -~~~~~~~~~~~~~~ +.. note:: -- Parsing DNS messages with many different names could cause excessive - CPU load. This has been fixed. :cve:`2023-4408` - - ISC would like to thank Shoham Danino from Reichman University, Anat - Bremler-Barr from Tel-Aviv University, Yehuda Afek from Tel-Aviv - University, and Yuval Shavitt from Tel-Aviv University for bringing - this vulnerability to our attention. :gl:`#4234` - -- Specific queries could cause :iscman:`named` to crash with an - assertion failure when ``nxdomain-redirect`` was enabled. This has - been fixed. :cve:`2023-5517` :gl:`#4281` - -- A bad interaction between DNS64 and serve-stale could cause - :iscman:`named` to crash with an assertion failure, when both of these - features were enabled. This has been fixed. :cve:`2023-5679` - :gl:`#4334` - -- Query patterns that continuously triggered cache database maintenance - could cause an excessive amount of memory to be allocated, exceeding - ``max-cache-size`` and potentially leading to all available memory on - the host running :iscman:`named` being exhausted. This has been fixed. - :cve:`2023-6516` - - ISC would like to thank Infoblox for bringing this vulnerability to - our attention. :gl:`#4383` - -Removed Features -~~~~~~~~~~~~~~~~ - -- Support for using AES as the DNS COOKIE algorithm (``cookie-algorithm - aes;``) has been deprecated and will be removed in a future release. - Please use the current default, SipHash-2-4, instead. :gl:`#4421` - -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. + The BIND 9.16.46 release was withdrawn after the discovery of a + regression in a security fix in it during pre-release testing. ISC + would like to acknowledge the assistance of Curtis Tuplin of SaskTel. diff --git a/doc/notes/notes-9.16.47.rst b/doc/notes/notes-9.16.47.rst new file mode 100644 index 0000000000..69a8a8b0f5 --- /dev/null +++ b/doc/notes/notes-9.16.47.rst @@ -0,0 +1,65 @@ +.. 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.16.47 +---------------------- + +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 for bringing this vulnerability to our + attention. :gl:`#4424` + +- Parsing DNS messages with many different names could cause excessive + CPU load. This has been fixed. :cve:`2023-4408` + + ISC would like to thank Shoham Danino from Reichman University, Anat + Bremler-Barr from Tel-Aviv University, Yehuda Afek from Tel-Aviv + University, and Yuval Shavitt from Tel-Aviv University for bringing + this vulnerability to our attention. :gl:`#4234` + +- Specific queries could cause :iscman:`named` to crash with an + assertion failure when ``nxdomain-redirect`` was enabled. This has + been fixed. :cve:`2023-5517` :gl:`#4281` + +- A bad interaction between DNS64 and serve-stale could cause + :iscman:`named` to crash with an assertion failure, when both of these + features were enabled. This has been fixed. :cve:`2023-5679` + :gl:`#4334` + +- Query patterns that continuously triggered cache database maintenance + could cause an excessive amount of memory to be allocated, exceeding + ``max-cache-size`` and potentially leading to all available memory on + the host running :iscman:`named` being exhausted. This has been fixed. + :cve:`2023-6516` + + ISC would like to thank Infoblox for bringing this vulnerability to + our attention. :gl:`#4383` + +Removed Features +~~~~~~~~~~~~~~~~ + +- Support for using AES as the DNS COOKIE algorithm (``cookie-algorithm + aes;``) has been deprecated and will be removed in a future release. + Please use the current default, SipHash-2-4, instead. :gl:`#4421` + +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. diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst deleted file mode 100644 index 9bd9752d33..0000000000 --- a/doc/notes/notes-current.rst +++ /dev/null @@ -1,31 +0,0 @@ -.. 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.16.47 ----------------------- - -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. From bab1aa96660c16e26d93a08aed2bbf32e57c8486 Mon Sep 17 00:00:00 2001 From: Michal Nowak Date: Fri, 2 Feb 2024 11:09:46 +0100 Subject: [PATCH 30/36] prep 9.16.47 --- CHANGES | 2 ++ lib/dns/win32/libdns.def.in | 1 + version | 2 +- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 60f0a6a230..aaaa4252bc 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,5 @@ + --- 9.16.47 released --- + 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] diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index 582bf90f45..610d3edb3b 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -1465,6 +1465,7 @@ dst_key_format dst_key_free dst_key_frombuffer dst_key_fromdns +dst_key_fromdns_ex dst_key_fromfile dst_key_fromgssapi dst_key_fromlabel diff --git a/version b/version index 698a143ff8..29fc6a264a 100644 --- a/version +++ b/version @@ -5,7 +5,7 @@ PRODUCT=BIND DESCRIPTION="(Extended Support Version)" MAJORVER=9 MINORVER=16 -PATCHVER=46 +PATCHVER=47 RELEASETYPE= RELEASEVER= EXTENSIONS= From f493a8394102b0aeb101d5dc2f963004c8741175 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Sun, 11 Feb 2024 00:49:32 +0100 Subject: [PATCH 31/36] Fix case insensitive matching in isc_ht hash table implementation The case insensitive matching in isc_ht was basically completely broken as only the hashvalue computation was case insensitive, but the key comparison was always case sensitive. (cherry picked from commit 175655b771fd17b06dfb8cfb29eaadf0f3b6a8b5) --- lib/isc/ht.c | 55 ++++++++++++++++++++++++++++++++++++++--- lib/isc/tests/ht_test.c | 53 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 4 deletions(-) diff --git a/lib/isc/ht.c b/lib/isc/ht.c index eaf2b3c508..e11050fc93 100644 --- a/lib/isc/ht.c +++ b/lib/isc/ht.c @@ -93,11 +93,54 @@ maybe_rehash(isc_ht_t *ht, size_t newcount); static isc_result_t isc__ht_iter_next(isc_ht_iter_t *it); +static uint8_t maptolower[] = { + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, + 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20, 0x21, 0x22, 0x23, + 0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f, + 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x3a, 0x3b, + 0x3c, 0x3d, 0x3e, 0x3f, 0x40, 0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67, + 0x68, 0x69, 0x6a, 0x6b, 0x6c, 0x6d, 0x6e, 0x6f, 0x70, 0x71, 0x72, 0x73, + 0x74, 0x75, 0x76, 0x77, 0x78, 0x79, 0x7a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f, + 0x60, 0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67, 0x68, 0x69, 0x6a, 0x6b, + 0x6c, 0x6d, 0x6e, 0x6f, 0x70, 0x71, 0x72, 0x73, 0x74, 0x75, 0x76, 0x77, + 0x78, 0x79, 0x7a, 0x7b, 0x7c, 0x7d, 0x7e, 0x7f, 0x80, 0x81, 0x82, 0x83, + 0x84, 0x85, 0x86, 0x87, 0x88, 0x89, 0x8a, 0x8b, 0x8c, 0x8d, 0x8e, 0x8f, + 0x90, 0x91, 0x92, 0x93, 0x94, 0x95, 0x96, 0x97, 0x98, 0x99, 0x9a, 0x9b, + 0x9c, 0x9d, 0x9e, 0x9f, 0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, + 0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf, 0xb0, 0xb1, 0xb2, 0xb3, + 0xb4, 0xb5, 0xb6, 0xb7, 0xb8, 0xb9, 0xba, 0xbb, 0xbc, 0xbd, 0xbe, 0xbf, + 0xc0, 0xc1, 0xc2, 0xc3, 0xc4, 0xc5, 0xc6, 0xc7, 0xc8, 0xc9, 0xca, 0xcb, + 0xcc, 0xcd, 0xce, 0xcf, 0xd0, 0xd1, 0xd2, 0xd3, 0xd4, 0xd5, 0xd6, 0xd7, + 0xd8, 0xd9, 0xda, 0xdb, 0xdc, 0xdd, 0xde, 0xdf, 0xe0, 0xe1, 0xe2, 0xe3, + 0xe4, 0xe5, 0xe6, 0xe7, 0xe8, 0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, 0xef, + 0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7, 0xf8, 0xf9, 0xfa, 0xfb, + 0xfc, 0xfd, 0xfe, 0xff +}; + +static int +memcasecmp(const void *vs1, const void *vs2, size_t len) { + uint8_t const *s1 = vs1; + uint8_t const *s2 = vs2; + for (size_t i = 0; i < len; i++) { + uint8_t u1 = s1[i]; + uint8_t u2 = s2[i]; + int U1 = maptolower[u1]; + int U2 = maptolower[u2]; + int diff = U1 - U2; + if (diff) { + return diff; + } + } + return 0; +} + static bool isc__ht_node_match(isc_ht_node_t *node, const uint32_t hashval, - const uint8_t *key, uint32_t keysize) { + const uint8_t *key, uint32_t keysize, bool case_sensitive) { return (node->hashval == hashval && node->keysize == keysize && - memcmp(node->key, key, keysize) == 0); + (case_sensitive ? (memcmp(node->key, key, keysize) == 0) + : (memcasecmp(node->key, key, keysize) == 0))); } static uint32_t @@ -341,7 +384,9 @@ nexttable: for (isc_ht_node_t *node = ht->table[findex][hash]; node != NULL; node = node->next) { - if (isc__ht_node_match(node, hashval, key, keysize)) { + if (isc__ht_node_match(node, hashval, key, keysize, + ht->case_sensitive)) + { return (node); } } @@ -390,7 +435,9 @@ isc__ht_delete(isc_ht_t *ht, const unsigned char *key, const uint32_t keysize, for (isc_ht_node_t *node = ht->table[idx][hash]; node != NULL; prev = node, node = node->next) { - if (isc__ht_node_match(node, hashval, key, keysize)) { + if (isc__ht_node_match(node, hashval, key, keysize, + ht->case_sensitive)) + { if (prev == NULL) { ht->table[idx][hash] = node->next; } else { diff --git a/lib/isc/tests/ht_test.c b/lib/isc/tests/ht_test.c index bd96aa8615..cc824c346a 100644 --- a/lib/isc/tests/ht_test.c +++ b/lib/isc/tests/ht_test.c @@ -333,9 +333,62 @@ isc_ht_iterator_test(void **state) { test_ht_iterator(); } +static void +isc_ht_case(void **state) { + UNUSED(state); + + isc_ht_t *ht = NULL; + void *f = NULL; + isc_result_t result = ISC_R_UNSET; + + unsigned char lower[16] = { "test case" }; + unsigned char same[16] = { "test case" }; + unsigned char upper[16] = { "TEST CASE" }; + unsigned char mixed[16] = { "tEsT CaSe" }; + + isc_ht_init(&ht, test_mctx, 8, ISC_HT_CASE_SENSITIVE); + assert_non_null(ht); + + result = isc_ht_add(ht, lower, 16, (void *)lower); + assert_int_equal(result, ISC_R_SUCCESS); + + result = isc_ht_add(ht, same, 16, (void *)same); + assert_int_equal(result, ISC_R_EXISTS); + + result = isc_ht_add(ht, upper, 16, (void *)upper); + assert_int_equal(result, ISC_R_SUCCESS); + + result = isc_ht_find(ht, mixed, 16, &f); + assert_int_equal(result, ISC_R_NOTFOUND); + assert_null(f); + + isc_ht_destroy(&ht); + assert_null(ht); + + isc_ht_init(&ht, test_mctx, 8, ISC_HT_CASE_INSENSITIVE); + assert_non_null(ht); + + result = isc_ht_add(ht, lower, 16, (void *)lower); + assert_int_equal(result, ISC_R_SUCCESS); + + result = isc_ht_add(ht, same, 16, (void *)same); + assert_int_equal(result, ISC_R_EXISTS); + + result = isc_ht_add(ht, upper, 16, (void *)upper); + assert_int_equal(result, ISC_R_EXISTS); + + result = isc_ht_find(ht, mixed, 16, &f); + assert_int_equal(result, ISC_R_SUCCESS); + assert_ptr_equal(f, &lower); + + isc_ht_destroy(&ht); + assert_null(ht); +} + int main(void) { const struct CMUnitTest tests[] = { + cmocka_unit_test(isc_ht_case), cmocka_unit_test(isc_ht_20), cmocka_unit_test(isc_ht_8), cmocka_unit_test(isc_ht_1), From b9c10a194da3358204f5ba7d91e55332db435614 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Sun, 11 Feb 2024 09:13:43 +0100 Subject: [PATCH 32/36] Add a system test for mixed-case data for the same owner We were missing a test where a single owner name would have multiple types with a different case. The generated RRSIGs and NSEC records will then have different case than the signed records and message parser have to cope with that and treat everything as the same owner. (cherry picked from commit a114042059ecbbc94ae0f604ca681323a75af480) --- bin/tests/system/dnssec/ns3/secure.example.db.in | 5 +++++ bin/tests/system/dnssec/ns3/sign.sh | 4 +++- bin/tests/system/dnssec/tests.sh | 15 +++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/bin/tests/system/dnssec/ns3/secure.example.db.in b/bin/tests/system/dnssec/ns3/secure.example.db.in index ec39308e9a..883e06790b 100644 --- a/bin/tests/system/dnssec/ns3/secure.example.db.in +++ b/bin/tests/system/dnssec/ns3/secure.example.db.in @@ -47,3 +47,8 @@ rrsigonly A 10.0.0.29 cnameandkey CNAME @ cnamenokey CNAME @ dnameandkey DNAME @ + +mixedcase A 10.0.0.30 +mixedCASE TXT "mixed case" +MIXEDcase AAAA 2002:: +mIxEdCaSe LOC 37 52 56.788 N 121 54 55.02 W 1120m 10m 100m 10m diff --git a/bin/tests/system/dnssec/ns3/sign.sh b/bin/tests/system/dnssec/ns3/sign.sh index 400c5d2cbd..278e8dd5a9 100644 --- a/bin/tests/system/dnssec/ns3/sign.sh +++ b/bin/tests/system/dnssec/ns3/sign.sh @@ -87,7 +87,9 @@ keyname=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -n zone "$zone cat "$infile" "$cnameandkey.key" "$dnameandkey.key" "$keyname.key" >"$zonefile" -"$SIGNER" -P -o "$zone" "$zonefile" >/dev/null +"$SIGNER" -P -D -o "$zone" "$zonefile" >/dev/null +cat "$zonefile" "$zonefile".signed >"$zonefile".tmp +mv "$zonefile".tmp "$zonefile".signed zone=bogus.example. infile=bogus.example.db.in diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index 7a2be13b7d..28d3b6bd71 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -771,6 +771,21 @@ n=$((n + 1)) test "$ret" -eq 0 || echo_i "failed" status=$((status + ret)) +echo_i "checking mixed-case positive validation ($n)" +ret=0 +for type in a txt aaaa loc; do + dig_with_opts +noauth mixedcase.secure.example. \ + @10.53.0.3 $type >dig.out.$type.ns3.test$n || ret=1 + dig_with_opts +noauth mixedcase.secure.example. \ + @10.53.0.4 $type >dig.out.$type.ns4.test$n || ret=1 + digcomp --lc dig.out.$type.ns3.test$n dig.out.$type.ns4.test$n || ret=1 + grep "status: NOERROR" dig.out.$type.ns4.test$n >/dev/null || ret=1 + grep "flags:.*ad.*QUERY" dig.out.$type.ns4.test$n >/dev/null || ret=1 +done +n=$((n + 1)) +test "$ret" -eq 0 || echo_i "failed" +status=$((status + ret)) + echo_i "checking multi-stage positive validation NSEC/NSEC3 ($n)" ret=0 dig_with_opts +noauth a.nsec3.example. \ From 81479eaa639fb2c77781cfb4898d934e6a3c8e5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Sun, 11 Feb 2024 00:59:30 +0100 Subject: [PATCH 33/36] Add CHANGES note for [GL #4568] (cherry picked from commit e91884553f8b53d8e7f595a36914d351c1a1789d) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index aaaa4252bc..4d96b74512 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6343. [bug] Fix case insensitive setting for isc_ht hashtable. + [GL #4568] + --- 9.16.47 released --- 6322. [security] Specific DNS answers could cause a denial-of-service From 659e0d80fde14b5221eb3dfd08cdcdcd94cb2674 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Sun, 11 Feb 2024 12:10:17 +0100 Subject: [PATCH 34/36] Prepare release notes for BIND 9.16.48 --- doc/arm/notes.rst | 1 + doc/notes/notes-9.16.47.rst | 55 +++---------------------------- doc/notes/notes-9.16.48.rst | 65 +++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 50 deletions(-) create mode 100644 doc/notes/notes-9.16.48.rst diff --git a/doc/arm/notes.rst b/doc/arm/notes.rst index 94e0ce43af..1fecc77586 100644 --- a/doc/arm/notes.rst +++ b/doc/arm/notes.rst @@ -46,6 +46,7 @@ for Microsoft Windows operating systems. .. include:: ../notes/notes-known-issues.rst +.. include:: ../notes/notes-9.16.48.rst .. include:: ../notes/notes-9.16.47.rst .. include:: ../notes/notes-9.16.46.rst .. include:: ../notes/notes-9.16.45.rst diff --git a/doc/notes/notes-9.16.47.rst b/doc/notes/notes-9.16.47.rst index 69a8a8b0f5..bf39c3d815 100644 --- a/doc/notes/notes-9.16.47.rst +++ b/doc/notes/notes-9.16.47.rst @@ -12,54 +12,9 @@ Notes for BIND 9.16.47 ---------------------- -Security Fixes -~~~~~~~~~~~~~~ +.. note:: -- 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 for bringing this vulnerability to our - attention. :gl:`#4424` - -- Parsing DNS messages with many different names could cause excessive - CPU load. This has been fixed. :cve:`2023-4408` - - ISC would like to thank Shoham Danino from Reichman University, Anat - Bremler-Barr from Tel-Aviv University, Yehuda Afek from Tel-Aviv - University, and Yuval Shavitt from Tel-Aviv University for bringing - this vulnerability to our attention. :gl:`#4234` - -- Specific queries could cause :iscman:`named` to crash with an - assertion failure when ``nxdomain-redirect`` was enabled. This has - been fixed. :cve:`2023-5517` :gl:`#4281` - -- A bad interaction between DNS64 and serve-stale could cause - :iscman:`named` to crash with an assertion failure, when both of these - features were enabled. This has been fixed. :cve:`2023-5679` - :gl:`#4334` - -- Query patterns that continuously triggered cache database maintenance - could cause an excessive amount of memory to be allocated, exceeding - ``max-cache-size`` and potentially leading to all available memory on - the host running :iscman:`named` being exhausted. This has been fixed. - :cve:`2023-6516` - - ISC would like to thank Infoblox for bringing this vulnerability to - our attention. :gl:`#4383` - -Removed Features -~~~~~~~~~~~~~~~~ - -- Support for using AES as the DNS COOKIE algorithm (``cookie-algorithm - aes;``) has been deprecated and will be removed in a future release. - Please use the current default, SipHash-2-4, instead. :gl:`#4421` - -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. + The BIND 9.16.47 release was withdrawn after the discovery of a + regression in a security fix in it during pre-release testing. ISC + would like to acknowledge the assistance of Vinzenz Vogel and Daniel + Stirnimann of SWITCH. diff --git a/doc/notes/notes-9.16.48.rst b/doc/notes/notes-9.16.48.rst new file mode 100644 index 0000000000..6521b42f4b --- /dev/null +++ b/doc/notes/notes-9.16.48.rst @@ -0,0 +1,65 @@ +.. 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.16.48 +---------------------- + +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 for bringing this vulnerability to our + attention. :gl:`#4424` + +- Parsing DNS messages with many different names could cause excessive + CPU load. This has been fixed. :cve:`2023-4408` + + ISC would like to thank Shoham Danino from Reichman University, Anat + Bremler-Barr from Tel-Aviv University, Yehuda Afek from Tel-Aviv + University, and Yuval Shavitt from Tel-Aviv University for bringing + this vulnerability to our attention. :gl:`#4234` + +- Specific queries could cause :iscman:`named` to crash with an + assertion failure when ``nxdomain-redirect`` was enabled. This has + been fixed. :cve:`2023-5517` :gl:`#4281` + +- A bad interaction between DNS64 and serve-stale could cause + :iscman:`named` to crash with an assertion failure, when both of these + features were enabled. This has been fixed. :cve:`2023-5679` + :gl:`#4334` + +- Query patterns that continuously triggered cache database maintenance + could cause an excessive amount of memory to be allocated, exceeding + ``max-cache-size`` and potentially leading to all available memory on + the host running :iscman:`named` being exhausted. This has been fixed. + :cve:`2023-6516` + + ISC would like to thank Infoblox for bringing this vulnerability to + our attention. :gl:`#4383` + +Removed Features +~~~~~~~~~~~~~~~~ + +- Support for using AES as the DNS COOKIE algorithm (``cookie-algorithm + aes;``) has been deprecated and will be removed in a future release. + Please use the current default, SipHash-2-4, instead. :gl:`#4421` + +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. From 43a6b02728a2d4c87550a76f81d41bbcc8dc7532 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Sun, 11 Feb 2024 12:10:17 +0100 Subject: [PATCH 35/36] Add release note for GL #4459 --- doc/notes/notes-9.16.48.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/notes/notes-9.16.48.rst b/doc/notes/notes-9.16.48.rst index 6521b42f4b..917e551f63 100644 --- a/doc/notes/notes-9.16.48.rst +++ b/doc/notes/notes-9.16.48.rst @@ -24,6 +24,10 @@ Security Fixes Applied Cybersecurity ATHENE for bringing this vulnerability to our attention. :gl:`#4424` +- Preparing an NSEC3 closest encloser proof could cause excessive CPU + load, leading to a denial-of-service condition. This has been fixed. + :cve:`2023-50868` :gl:`#4459` + - Parsing DNS messages with many different names could cause excessive CPU load. This has been fixed. :cve:`2023-4408` From 7fc5ef173b4039ae6ebd0aa40cb023fa14bb369d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Sun, 11 Feb 2024 12:31:23 +0100 Subject: [PATCH 36/36] prep 9.16.48 --- CHANGES | 2 ++ version | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 4d96b74512..4e03556ab8 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,5 @@ + --- 9.16.48 released --- + 6343. [bug] Fix case insensitive setting for isc_ht hashtable. [GL #4568] diff --git a/version b/version index 29fc6a264a..d67f3a57c7 100644 --- a/version +++ b/version @@ -5,7 +5,7 @@ PRODUCT=BIND DESCRIPTION="(Extended Support Version)" MAJORVER=9 MINORVER=16 -PATCHVER=47 +PATCHVER=48 RELEASETYPE= RELEASEVER= EXTENSIONS=