From 9067b637d864d609feed6697b567c9145e2e3a21 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 15 Aug 2023 21:04:54 -0700 Subject: [PATCH 01/11] replace RBTs with hashmaps in dns_transport as dns_transport_find() is only concerned with finding an exact match on the specified name it doesn't need to use a tree data structure internally, we can replace the RBTs with hash tables. --- lib/dns/transport.c | 62 +++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/lib/dns/transport.c b/lib/dns/transport.c index d76f62f3d5..f961f6a3b8 100644 --- a/lib/dns/transport.c +++ b/lib/dns/transport.c @@ -13,6 +13,7 @@ #include +#include #include #include #include @@ -22,8 +23,8 @@ #include #include +#include #include -#include #include #define TRANSPORT_MAGIC ISC_MAGIC('T', 'r', 'n', 's') @@ -37,7 +38,7 @@ struct dns_transport_list { isc_refcount_t references; isc_mem_t *mctx; isc_rwlock_t lock; - dns_rbt_t *transports[DNS_TRANSPORT_COUNT]; + isc_hashmap_t *transports[DNS_TRANSPORT_COUNT]; }; typedef enum ternary { ter_none = 0, ter_true = 1, ter_false = 2 } ternary_t; @@ -47,6 +48,8 @@ struct dns_transport { isc_refcount_t references; isc_mem_t *mctx; dns_transport_type_t type; + dns_fixedname_t fn; + dns_name_t *name; struct { char *tlsname; char *certfile; @@ -64,29 +67,20 @@ struct dns_transport { } doh; }; -static void -free_dns_transport(void *node, void *arg) { - dns_transport_t *transport = node; - - REQUIRE(node != NULL); - - UNUSED(arg); - - dns_transport_detach(&transport); -} - static isc_result_t list_add(dns_transport_list_t *list, const dns_name_t *name, const dns_transport_type_t type, dns_transport_t *transport) { isc_result_t result; - dns_rbt_t *rbt = NULL; + isc_hashmap_t *hm = NULL; RWLOCK(&list->lock, isc_rwlocktype_write); - rbt = list->transports[type]; - INSIST(rbt != NULL); - - result = dns_rbt_addname(rbt, name, transport); + hm = list->transports[type]; + INSIST(hm != NULL); + transport->name = dns_fixedname_initname(&transport->fn); + dns_name_copy(name, transport->name); + result = isc_hashmap_add(hm, NULL, transport->name->ndata, + transport->name->length, transport); RWUNLOCK(&list->lock, isc_rwlocktype_write); return (result); @@ -629,15 +623,16 @@ dns_transport_find(const dns_transport_type_t type, const dns_name_t *name, dns_transport_list_t *list) { isc_result_t result; dns_transport_t *transport = NULL; - dns_rbt_t *rbt = NULL; + isc_hashmap_t *hm = NULL; REQUIRE(VALID_TRANSPORT_LIST(list)); REQUIRE(list->transports[type] != NULL); - rbt = list->transports[type]; + hm = list->transports[type]; RWLOCK(&list->lock, isc_rwlocktype_read); - result = dns_rbt_findname(rbt, name, 0, NULL, (void *)&transport); + result = isc_hashmap_find(hm, NULL, name->ndata, name->length, + (void **)&transport); if (result == ISC_R_SUCCESS) { isc_refcount_increment(&transport->references); } @@ -660,10 +655,8 @@ dns_transport_list_new(isc_mem_t *mctx) { list->magic = TRANSPORT_LIST_MAGIC; for (size_t type = 0; type < DNS_TRANSPORT_COUNT; type++) { - isc_result_t result; - result = dns_rbt_create(list->mctx, free_dns_transport, NULL, - &list->transports[type]); - RUNTIME_CHECK(result == ISC_R_SUCCESS); + isc_hashmap_create(list->mctx, 10, ISC_HASHMAP_CASE_INSENSITIVE, + &list->transports[type]); } return (list); @@ -686,9 +679,24 @@ transport_list_destroy(dns_transport_list_t *list) { list->magic = 0; for (size_t type = 0; type < DNS_TRANSPORT_COUNT; type++) { - if (list->transports[type] != NULL) { - dns_rbt_destroy(&list->transports[type]); + isc_result_t result; + isc_hashmap_iter_t *it = NULL; + + if (list->transports[type] == NULL) { + continue; } + + isc_hashmap_iter_create(list->transports[type], &it); + for (result = isc_hashmap_iter_first(it); + result == ISC_R_SUCCESS; + result = isc_hashmap_iter_delcurrent_next(it)) + { + dns_transport_t *transport = NULL; + isc_hashmap_iter_current(it, (void **)&transport); + dns_transport_detach(&transport); + } + isc_hashmap_iter_destroy(&it); + isc_hashmap_destroy(&list->transports[type]); } isc_rwlock_destroy(&list->lock); isc_mem_putanddetach(&list->mctx, list, sizeof(*list)); From 56114aaa0dba9e64f9fe6998e9c5298be18476e6 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 16 Aug 2023 12:08:53 -0700 Subject: [PATCH 02/11] add dns_nametree structure for policy match lookups this is a QP trie of boolean values to indicate whether a name is included in or excluded from some policy. this can be used for synth-from-dnssec, deny-answer-aliases, etc. --- lib/dns/Makefile.am | 2 + lib/dns/include/dns/nametree.h | 154 ++++++++++++++++++++ lib/dns/include/dns/types.h | 2 + lib/dns/nametree.c | 252 +++++++++++++++++++++++++++++++++ tests/dns/Makefile.am | 1 + tests/dns/nametree_test.c | 202 ++++++++++++++++++++++++++ 6 files changed, 613 insertions(+) create mode 100644 lib/dns/include/dns/nametree.h create mode 100644 lib/dns/nametree.c create mode 100644 tests/dns/nametree_test.c diff --git a/lib/dns/Makefile.am b/lib/dns/Makefile.am index 865a29f06a..b6814bd7ed 100644 --- a/lib/dns/Makefile.am +++ b/lib/dns/Makefile.am @@ -91,6 +91,7 @@ libdns_la_HEADERS = \ include/dns/masterdump.h \ include/dns/message.h \ include/dns/name.h \ + include/dns/nametree.h \ include/dns/ncache.h \ include/dns/nsec.h \ include/dns/nsec3.h \ @@ -196,6 +197,7 @@ libdns_la_SOURCES = \ masterdump.c \ message.c \ name.c \ + nametree.c \ ncache.c \ nsec.c \ nsec3.c \ diff --git a/lib/dns/include/dns/nametree.h b/lib/dns/include/dns/nametree.h new file mode 100644 index 0000000000..be549dab01 --- /dev/null +++ b/lib/dns/include/dns/nametree.h @@ -0,0 +1,154 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +#pragma once + +/***** +***** Module Info +*****/ + +/*! \file + * \brief + * A nametree module is a tree of DNS names containing boolean values + * or bitfields, allowing a quick lookup to see whether a name is included + * in or excluded from some policy. + */ + +#include + +#include +#include +#include +#include +#include + +#include +#include + +#include + +/* Define to 1 for detailed reference tracing */ +#undef DNS_NAMETREE_TRACE + +ISC_LANG_BEGINDECLS + +void +dns_nametree_create(isc_mem_t *mctx, const char *name, dns_nametree_t **ntp); +/*%< + * Create a nametree. + * + * If 'name' is not NULL, it will be saved as the name of the QP trie + * for debugging purposes. + * + * Requires: + * + *\li 'mctx' is a valid memory context. + *\li ntp != NULL && *ntp == NULL + */ + +isc_result_t +dns_nametree_add(dns_nametree_t *nametree, const dns_name_t *name, bool value); +/*%< + * Add a node to 'nametree'. + * + * 'value' is a single boolean value, true or false. If the name already + * exists within the tree, then return ISC_R_EXISTS. + * + * Requires: + * + *\li 'nametree' points to a valid nametree. + * + * Returns: + * + *\li ISC_R_SUCCESS + *\li ISC_R_EXISTS + * + *\li Any other result indicates failure. + */ + +isc_result_t +dns_nametree_delete(dns_nametree_t *nametree, const dns_name_t *name); +/*%< + * Delete 'name' from 'nametree'. + * + * Requires: + * + *\li 'nametree' points to a valid nametree. + *\li 'name' is not NULL + * + * Returns: + * + *\li ISC_R_SUCCESS + * + *\li Any other result indicates failure. + */ + +isc_result_t +dns_nametree_find(dns_nametree_t *nametree, const dns_name_t *name, + dns_ntnode_t **ntp); +/*%< + * Retrieve the node that exactly matches 'name' from 'nametree'. + * + * Requires: + * + *\li 'nametree' is a valid nametree. + * + *\li 'name' is a valid name. + * + *\li ntp != NULL && *ntp == NULL + * + * Returns: + * + *\li ISC_R_SUCCESS + *\li ISC_R_NOTFOUND + * + *\li Any other result indicates an error. + */ + +bool +dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name); +/*%< + * Indicates whether a 'name' is covered by 'nametree'. + * + * This returns true if 'name' has a match or a closest ancestor in + * 'nametree' with its value set to 'true'. If a name is not found, or if + * 'nametree' is NULL, the default return value is false. + * + * Requires: + * + *\li 'nametree' is a valid nametree, or is NULL. + */ + +#if DNS_NAMETREE_TRACE +#define dns_nametree_ref(ptr) \ + dns_nametree__ref(ptr, __func__, __FILE__, __LINE__) +#define dns_nametree_unref(ptr) \ + dns_nametree__unref(ptr, __func__, __FILE__, __LINE__) +#define dns_nametree_attach(ptr, ptrp) \ + dns_nametree__attach(ptr, ptrp, __func__, __FILE__, __LINE__) +#define dns_nametree_detach(ptrp) \ + dns_nametree__detach(ptrp, __func__, __FILE__, __LINE__) +#define dns_ntnode_ref(ptr) dns_ntnode__ref(ptr, __func__, __FILE__, __LINE__) +#define dns_ntnode_unref(ptr) \ + dns_ntnode__unref(ptr, __func__, __FILE__, __LINE__) +#define dns_ntnode_attach(ptr, ptrp) \ + dns_ntnode__attach(ptr, ptrp, __func__, __FILE__, __LINE__) +#define dns_ntnode_detach(ptrp) \ + dns_ntnode__detach(ptrp, __func__, __FILE__, __LINE__) +ISC_REFCOUNT_TRACE_DECL(dns_nametree); +ISC_REFCOUNT_TRACE_DECL(dns_ntnode); +#else +ISC_REFCOUNT_DECL(dns_nametree); +ISC_REFCOUNT_DECL(dns_ntnode); +#endif +ISC_LANG_ENDDECLS diff --git a/lib/dns/include/dns/types.h b/lib/dns/include/dns/types.h index 70e0b02274..f3fd6f630e 100644 --- a/lib/dns/include/dns/types.h +++ b/lib/dns/include/dns/types.h @@ -116,8 +116,10 @@ typedef struct dns_message dns_message_t; typedef uint16_t dns_messageid_t; typedef isc_region_t dns_label_t; typedef struct dns_name dns_name_t; +typedef struct dns_nametree dns_nametree_t; typedef ISC_LIST(dns_name_t) dns_namelist_t; typedef struct dns_ntatable dns_ntatable_t; +typedef struct dns_ntnode dns_ntnode_t; typedef uint16_t dns_opcode_t; typedef struct dns_order dns_order_t; typedef struct dns_peer dns_peer_t; diff --git a/lib/dns/nametree.c b/lib/dns/nametree.c new file mode 100644 index 0000000000..464690d6c8 --- /dev/null +++ b/lib/dns/nametree.c @@ -0,0 +1,252 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +/*! \file */ + +#include + +#include +#include +#include +#include +#include + +#include +#include +#include + +#define NAMETREE_MAGIC ISC_MAGIC('N', 'T', 'r', 'e') +#define VALID_NAMETREE(kt) ISC_MAGIC_VALID(kt, NAMETREE_MAGIC) + +struct dns_nametree { + unsigned int magic; + isc_mem_t *mctx; + isc_refcount_t references; + dns_qpmulti_t *table; + char name[64]; +}; + +struct dns_ntnode { + isc_mem_t *mctx; + isc_refcount_t references; + dns_fixedname_t fn; + dns_name_t *name; + union { + bool value; + unsigned char *bits; + }; +}; + +/* QP trie methods */ +static void +qp_attach(void *uctx, void *pval, uint32_t ival); +static void +qp_detach(void *uctx, void *pval, uint32_t ival); +static size_t +qp_makekey(dns_qpkey_t key, void *uctx, void *pval, uint32_t ival); +static void +qp_triename(void *uctx, char *buf, size_t size); + +static dns_qpmethods_t qpmethods = { + qp_attach, + qp_detach, + qp_makekey, + qp_triename, +}; + +static void +destroy_ntnode(dns_ntnode_t *node) { + isc_refcount_destroy(&node->references); + isc_mem_putanddetach(&node->mctx, node, sizeof(dns_ntnode_t)); +} + +#if DNS_NAMETREE_TRACE +ISC_REFCOUNT_TRACE_IMPL(dns_ntnode, destroy_ntnode); +#else +ISC_REFCOUNT_IMPL(dns_ntnode, destroy_ntnode); +#endif + +void +dns_nametree_create(isc_mem_t *mctx, const char *name, dns_nametree_t **ntp) { + dns_nametree_t *nametree = NULL; + + REQUIRE(ntp != NULL && *ntp == NULL); + + nametree = isc_mem_get(mctx, sizeof(*nametree)); + *nametree = (dns_nametree_t){ + .magic = NAMETREE_MAGIC, + }; + isc_mem_attach(mctx, &nametree->mctx); + isc_refcount_init(&nametree->references, 1); + + if (name != NULL) { + strlcpy(nametree->name, name, sizeof(nametree->name)); + } + + dns_qpmulti_create(mctx, &qpmethods, nametree, &nametree->table); + *ntp = nametree; +} + +static void +destroy_nametree(dns_nametree_t *nametree) { + nametree->magic = 0; + + dns_qpmulti_destroy(&nametree->table); + isc_refcount_destroy(&nametree->references); + + isc_mem_putanddetach(&nametree->mctx, nametree, sizeof(*nametree)); +} + +#if DNS_NAMETREE_TRACE +ISC_REFCOUNT_TRACE_IMPL(dns_nametree, destroy_nametree); +#else +ISC_REFCOUNT_IMPL(dns_nametree, destroy_nametree); +#endif + +static dns_ntnode_t * +newnode(isc_mem_t *mctx, const dns_name_t *name) { + dns_ntnode_t *node = isc_mem_get(mctx, sizeof(*node)); + *node = (dns_ntnode_t){ 0 }; + isc_mem_attach(mctx, &node->mctx); + isc_refcount_init(&node->references, 1); + + node->name = dns_fixedname_initname(&node->fn); + dns_name_copy(name, node->name); + + return (node); +} + +isc_result_t +dns_nametree_add(dns_nametree_t *nametree, const dns_name_t *name, bool value) { + isc_result_t result; + dns_qp_t *qp = NULL; + + REQUIRE(VALID_NAMETREE(nametree)); + REQUIRE(name != NULL); + + dns_qpmulti_write(nametree->table, &qp); + + result = dns_qp_getname(qp, name, NULL, NULL); + if (result == ISC_R_SUCCESS) { + result = ISC_R_EXISTS; + } else { + dns_ntnode_t *node = newnode(nametree->mctx, name); + node->value = value; + result = dns_qp_insert(qp, node, 0); + + /* + * We detach the node here, so any dns_qp_deletename() will + * destroy the node directly. + */ + dns_ntnode_detach(&node); + } + + dns_qp_compact(qp, DNS_QPGC_MAYBE); + dns_qpmulti_commit(nametree->table, &qp); + return (result); +} + +isc_result_t +dns_nametree_delete(dns_nametree_t *nametree, const dns_name_t *name) { + isc_result_t result; + dns_qp_t *qp = NULL; + void *pval = NULL; + + REQUIRE(VALID_NAMETREE(nametree)); + REQUIRE(name != NULL); + + dns_qpmulti_write(nametree->table, &qp); + result = dns_qp_deletename(qp, name, &pval, NULL); + if (result == ISC_R_SUCCESS) { + dns_ntnode_t *n = pval; + dns_ntnode_detach(&n); + } + dns_qp_compact(qp, DNS_QPGC_MAYBE); + dns_qpmulti_commit(nametree->table, &qp); + + return (result); +} + +isc_result_t +dns_nametree_find(dns_nametree_t *nametree, const dns_name_t *name, + dns_ntnode_t **ntnodep) { + isc_result_t result; + dns_qpread_t qpr; + void *pval = NULL; + + REQUIRE(VALID_NAMETREE(nametree)); + REQUIRE(name != NULL); + REQUIRE(ntnodep != NULL && *ntnodep == NULL); + + dns_qpmulti_query(nametree->table, &qpr); + result = dns_qp_getname(&qpr, name, &pval, NULL); + if (result == ISC_R_SUCCESS) { + dns_ntnode_t *knode = pval; + dns_ntnode_attach(knode, ntnodep); + } + dns_qpread_destroy(nametree->table, &qpr); + + return (result); +} + +bool +dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name) { + isc_result_t result; + dns_qpread_t qpr; + dns_ntnode_t *ntnode = NULL; + void *pval = NULL; + bool value = false; + + REQUIRE(nametree == NULL || VALID_NAMETREE(nametree)); + + if (nametree == NULL) { + return (false); + } + + dns_qpmulti_query(nametree->table, &qpr); + result = dns_qp_findname_ancestor(&qpr, name, 0, &pval, NULL); + if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { + ntnode = pval; + value = ntnode->value; + } + + dns_qpread_destroy(nametree->table, &qpr); + return (value); +} + +static void +qp_attach(void *uctx ISC_ATTR_UNUSED, void *pval, + uint32_t ival ISC_ATTR_UNUSED) { + dns_ntnode_t *ntnode = pval; + dns_ntnode_ref(ntnode); +} + +static void +qp_detach(void *uctx ISC_ATTR_UNUSED, void *pval, + uint32_t ival ISC_ATTR_UNUSED) { + dns_ntnode_t *ntnode = pval; + dns_ntnode_detach(&ntnode); +} + +static size_t +qp_makekey(dns_qpkey_t key, void *uctx ISC_ATTR_UNUSED, void *pval, + uint32_t ival ISC_ATTR_UNUSED) { + dns_ntnode_t *ntnode = pval; + return (dns_qpkey_fromname(key, ntnode->name)); +} + +static void +qp_triename(void *uctx, char *buf, size_t size) { + dns_nametree_t *nametree = uctx; + snprintf(buf, size, "%s nametree", nametree->name); +} diff --git a/tests/dns/Makefile.am b/tests/dns/Makefile.am index 921678ca90..005a97860b 100644 --- a/tests/dns/Makefile.am +++ b/tests/dns/Makefile.am @@ -29,6 +29,7 @@ check_PROGRAMS = \ dst_test \ keytable_test \ name_test \ + nametree_test \ nsec3_test \ nsec3param_test \ private_test \ diff --git a/tests/dns/nametree_test.c b/tests/dns/nametree_test.c new file mode 100644 index 0000000000..1557d2184f --- /dev/null +++ b/tests/dns/nametree_test.c @@ -0,0 +1,202 @@ +/* + * 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. + */ + +#include +#include /* IWYU pragma: keep */ +#include +#include +#include +#include +#include +#include +#include + +#define UNIT_TESTING +#include + +#include +#include +#include +#include + +#include +#include +#include + +#include + +#include + +dns_nametree_t *nametree = NULL; + +/* + * Test utilities. In general, these assume input parameters are valid + * (checking with assert_int_equal, thus aborting if not) and unlikely run time + * errors (such as memory allocation failure) won't happen. This helps keep + * the test code concise. + */ + +/* Common setup: create a nametree to test with a few keys */ +static void +create_tables(void) { + dns_fixedname_t fn; + dns_name_t *name = dns_fixedname_name(&fn); + + dns_nametree_create(mctx, "test", &nametree); + + /* Add a positive node */ + dns_test_namefromstring("example.com.", &fn); + assert_int_equal(dns_nametree_add(nametree, name, true), ISC_R_SUCCESS); + + /* Add a negative node below it */ + dns_test_namefromstring("negative.example.com.", &fn); + assert_int_equal(dns_nametree_add(nametree, name, false), + ISC_R_SUCCESS); + + /* Add a negative node with no parent */ + dns_test_namefromstring("negative.example.org.", &fn); + assert_int_equal(dns_nametree_add(nametree, name, false), + ISC_R_SUCCESS); +} + +static void +destroy_tables(void) { + if (nametree != NULL) { + dns_nametree_detach(&nametree); + } + rcu_barrier(); +} + +ISC_RUN_TEST_IMPL(add) { + dns_ntnode_t *node = NULL; + dns_fixedname_t fn; + dns_name_t *name = dns_fixedname_name(&fn); + + create_tables(); + + /* + * Getting the node for example.com should succeed. + */ + dns_test_namefromstring("example.com.", &fn); + assert_int_equal(dns_nametree_find(nametree, name, &node), + ISC_R_SUCCESS); + dns_ntnode_detach(&node); + + /* + * Try to add the same name. This should fail. + */ + assert_int_equal(dns_nametree_add(nametree, name, false), ISC_R_EXISTS); + assert_int_equal(dns_nametree_find(nametree, name, &node), + ISC_R_SUCCESS); + dns_ntnode_detach(&node); + + /* + * Try to add a new name. + */ + dns_test_namefromstring("newname.com.", &fn); + assert_int_equal(dns_nametree_add(nametree, name, true), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_find(nametree, name, &node), + ISC_R_SUCCESS); + dns_ntnode_detach(&node); + + destroy_tables(); +} + +ISC_RUN_TEST_IMPL(delete) { + dns_fixedname_t fn; + dns_name_t *name = dns_fixedname_name(&fn); + + create_tables(); + + /* name doesn't match */ + dns_test_namefromstring("example.org.", &fn); + assert_int_equal(dns_nametree_delete(nametree, name), ISC_R_NOTFOUND); + + /* subdomain match is the same as no match */ + dns_test_namefromstring("sub.example.org.", &fn); + assert_int_equal(dns_nametree_delete(nametree, name), ISC_R_NOTFOUND); + + /* + * delete requires exact match: this should return SUCCESS on + * the first try, then NOTFOUND on the second even though an + * ancestor does exist. + */ + dns_test_namefromstring("negative.example.com.", &fn); + assert_int_equal(dns_nametree_delete(nametree, name), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_delete(nametree, name), ISC_R_NOTFOUND); + + dns_test_namefromstring("negative.example.org.", &fn); + assert_int_equal(dns_nametree_delete(nametree, name), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_delete(nametree, name), ISC_R_NOTFOUND); + + destroy_tables(); +} + +ISC_RUN_TEST_IMPL(find) { + dns_ntnode_t *node = NULL; + dns_fixedname_t fn; + dns_name_t *name = dns_fixedname_name(&fn); + + create_tables(); + + /* + * dns_nametree_find() requires exact name match. It matches node + * that has a null key, too. + */ + dns_test_namefromstring("example.org.", &fn); + assert_int_equal(dns_nametree_find(nametree, name, &node), + ISC_R_NOTFOUND); + dns_test_namefromstring("sub.example.com.", &fn); + assert_int_equal(dns_nametree_find(nametree, name, &node), + ISC_R_NOTFOUND); + dns_test_namefromstring("example.com.", &fn); + assert_int_equal(dns_nametree_find(nametree, name, &node), + ISC_R_SUCCESS); + dns_ntnode_detach(&node); + + destroy_tables(); +} + +ISC_RUN_TEST_IMPL(covered) { + dns_fixedname_t fn; + dns_name_t *name = dns_fixedname_name(&fn); + const char *yesnames[] = { "example.com.", "sub.example.com.", NULL }; + const char *nonames[] = { "whatever.com.", "negative.example.com.", + "example.org.", "negative.example.org.", + NULL }; + create_tables(); + + for (const char **n = yesnames; *n != NULL; n++) { + dns_test_namefromstring(*n, &fn); + assert_true(dns_nametree_covered(nametree, name)); + } + for (const char **n = nonames; *n != NULL; n++) { + dns_test_namefromstring(*n, &fn); + assert_false(dns_nametree_covered(nametree, name)); + } + + /* If nametree is NULL, dns_nametree_covered() returns false. */ + dns_test_namefromstring("anyname.example.", &fn); + assert_false(dns_nametree_covered(NULL, name)); + + destroy_tables(); +} + +ISC_TEST_LIST_START +ISC_TEST_ENTRY(add) +ISC_TEST_ENTRY(covered) +ISC_TEST_ENTRY(find) +ISC_TEST_ENTRY(delete) +ISC_TEST_LIST_END + +ISC_TEST_MAIN From e83ac0ce65100cd289258ce420a749795a9d3db5 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 16 Aug 2023 13:28:36 -0700 Subject: [PATCH 03/11] use dns_nametree in place of RBTs replace the use of RBTs for deny-answer-aliases, the exclude lists for deny-answer-aliases and deny-answer-addresses, and dnssec-must-be-secure, with name trees. --- bin/named/server.c | 37 ++++++++++--------------- lib/dns/include/dns/view.h | 6 ++-- lib/dns/resolver.c | 57 +++++++++----------------------------- lib/dns/view.c | 7 +++-- 4 files changed, 34 insertions(+), 73 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index b2ab53b5cf..e0e12c860d 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -83,6 +83,7 @@ #include #include #include +#include #include #include #include @@ -602,20 +603,20 @@ configure_view_sortlist(const cfg_obj_t *vconfig, const cfg_obj_t *config, static isc_result_t configure_view_nametable(const cfg_obj_t *vconfig, const cfg_obj_t *config, const char *confname, const char *conftuplename, - isc_mem_t *mctx, dns_rbt_t **rbtp) { - isc_result_t result; + isc_mem_t *mctx, dns_nametree_t **ntp) { + isc_result_t result = ISC_R_SUCCESS; const cfg_obj_t *maps[3]; const cfg_obj_t *obj = NULL; - const cfg_listelt_t *element; + const cfg_listelt_t *element = NULL; int i = 0; dns_fixedname_t fixed; - dns_name_t *name; + dns_name_t *name = NULL; isc_buffer_t b; - const char *str; - const cfg_obj_t *nameobj; + const char *str = NULL; + const cfg_obj_t *nameobj = NULL; - if (*rbtp != NULL) { - dns_rbt_destroy(rbtp); + if (*ntp != NULL) { + dns_nametree_detach(ntp); } if (vconfig != NULL) { maps[i++] = cfg_tuple_get(vconfig, "options"); @@ -632,7 +633,7 @@ configure_view_nametable(const cfg_obj_t *vconfig, const cfg_obj_t *config, (void)named_config_get(maps, confname, &obj); if (obj == NULL) { /* - * No value available. *rbtp == NULL. + * No value available. *ntp == NULL. */ return (ISC_R_SUCCESS); } @@ -644,10 +645,7 @@ configure_view_nametable(const cfg_obj_t *vconfig, const cfg_obj_t *config, } } - result = dns_rbt_create(mctx, NULL, NULL, rbtp); - if (result != ISC_R_SUCCESS) { - return (result); - } + dns_nametree_create(mctx, confname, ntp); name = dns_fixedname_initname(&fixed); for (element = cfg_list_first(obj); element != NULL; @@ -658,14 +656,7 @@ configure_view_nametable(const cfg_obj_t *vconfig, const cfg_obj_t *config, isc_buffer_constinit(&b, str, strlen(str)); isc_buffer_add(&b, strlen(str)); CHECK(dns_name_fromtext(name, &b, dns_rootname, 0, NULL)); - /* - * We don't need the node data, but need to set dummy data to - * avoid a partial match with an empty node. For example, if - * we have foo.example.com and bar.example.com, we'd get a match - * for baz.example.com, which is not the expected result. - * We simply use (void *)1 as the dummy data. - */ - result = dns_rbt_addname(*rbtp, name, (void *)1); + result = dns_nametree_add(*ntp, name, true); if (result != ISC_R_SUCCESS) { cfg_obj_log(nameobj, named_g_lctx, ISC_LOG_ERROR, "failed to add %s for %s: %s", str, @@ -674,10 +665,10 @@ configure_view_nametable(const cfg_obj_t *vconfig, const cfg_obj_t *config, } } - return (result); + return (ISC_R_SUCCESS); cleanup: - dns_rbt_destroy(rbtp); + dns_nametree_detach(ntp); return (result); } diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index aed9761679..9bb9f3bf13 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -140,9 +140,9 @@ struct dns_view { dns_acl_t *denyansweracl; dns_acl_t *nocasecompress; bool msgcompression; - dns_rbt_t *answeracl_exclude; - dns_rbt_t *denyanswernames; - dns_rbt_t *answernames_exclude; + dns_nametree_t *answeracl_exclude; + dns_nametree_t *denyanswernames; + dns_nametree_t *answernames_exclude; dns_rrl_t *rrl; dns_rbt_t *sfd; isc_rwlock_t sfd_lock; diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index b5a911ccc8..a56c7d9dac 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -52,6 +52,7 @@ #include #include #include +#include #include #include #include @@ -563,7 +564,7 @@ struct dns_resolver { ISC_LIST(alternate_t) alternates; dns_rbt_t *algorithms; dns_rbt_t *digests; - dns_rbt_t *mustbesecure; + dns_nametree_t *mustbesecure; unsigned int spillatmax; unsigned int spillatmin; isc_timer_t *spillattimer; @@ -6751,15 +6752,8 @@ is_answeraddress_allowed(dns_view_t *view, dns_name_t *name, * If the owner name matches one in the exclusion list, either * exactly or partially, allow it. */ - if (view->answeracl_exclude != NULL) { - dns_rbtnode_t *node = NULL; - - result = dns_rbt_findnode(view->answeracl_exclude, name, NULL, - &node, NULL, 0, NULL, NULL); - - if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { - return (true); - } + if (dns_nametree_covered(view->answeracl_exclude, name)) { + return (true); } /* @@ -6806,7 +6800,6 @@ static bool is_answertarget_allowed(fetchctx_t *fctx, dns_name_t *qname, dns_name_t *rname, dns_rdataset_t *rdataset, bool *chainingp) { isc_result_t result; - dns_rbtnode_t *node = NULL; dns_name_t *tname = NULL; dns_rdata_cname_t cname; dns_rdata_dname_t dname; @@ -6872,12 +6865,8 @@ is_answertarget_allowed(fetchctx_t *fctx, dns_name_t *qname, dns_name_t *rname, * If the owner name matches one in the exclusion list, either * exactly or partially, allow it. */ - if (view->answernames_exclude != NULL) { - result = dns_rbt_findnode(view->answernames_exclude, qname, - NULL, &node, NULL, 0, NULL, NULL); - if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { - return (true); - } + if (dns_nametree_covered(view->answernames_exclude, qname)) { + return (true); } /* @@ -6896,9 +6885,7 @@ is_answertarget_allowed(fetchctx_t *fctx, dns_name_t *qname, dns_name_t *rname, /* * Otherwise, apply filters. */ - result = dns_rbt_findnode(view->denyanswernames, tname, NULL, &node, - NULL, 0, NULL, NULL); - if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { + if (dns_nametree_covered(view->denyanswernames, tname)) { char qnamebuf[DNS_NAME_FORMATSIZE]; char tnamebuf[DNS_NAME_FORMATSIZE]; char classbuf[64]; @@ -10957,12 +10944,10 @@ dns_resolver_resetmustbesecure(dns_resolver_t *resolver) { REQUIRE(VALID_RESOLVER(resolver)); if (resolver->mustbesecure != NULL) { - dns_rbt_destroy(&resolver->mustbesecure); + dns_nametree_detach(&resolver->mustbesecure); } } -static bool yes = true, no = false; - isc_result_t dns_resolver_setmustbesecure(dns_resolver_t *resolver, const dns_name_t *name, bool value) { @@ -10971,35 +10956,19 @@ dns_resolver_setmustbesecure(dns_resolver_t *resolver, const dns_name_t *name, REQUIRE(VALID_RESOLVER(resolver)); if (resolver->mustbesecure == NULL) { - result = dns_rbt_create(resolver->mctx, NULL, NULL, - &resolver->mustbesecure); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } + dns_nametree_create(resolver->mctx, "dnssec-must-be-secure", + &resolver->mustbesecure); } - result = dns_rbt_addname(resolver->mustbesecure, name, - value ? &yes : &no); -cleanup: + + result = dns_nametree_add(resolver->mustbesecure, name, value); return (result); } bool dns_resolver_getmustbesecure(dns_resolver_t *resolver, const dns_name_t *name) { - void *data = NULL; - bool value = false; - isc_result_t result; - REQUIRE(VALID_RESOLVER(resolver)); - if (resolver->mustbesecure == NULL) { - goto unlock; - } - result = dns_rbt_findname(resolver->mustbesecure, name, 0, NULL, &data); - if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { - value = *(bool *)data; - } -unlock: - return (value); + return (dns_nametree_covered(resolver->mustbesecure, name)); } void diff --git a/lib/dns/view.c b/lib/dns/view.c index 982c7518bc..be043cee2f 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -47,6 +47,7 @@ #include #include #include +#include #include #include #include @@ -348,13 +349,13 @@ destroy(dns_view_t *view) { dns_acl_detach(&view->pad_acl); } if (view->answeracl_exclude != NULL) { - dns_rbt_destroy(&view->answeracl_exclude); + dns_nametree_detach(&view->answeracl_exclude); } if (view->denyanswernames != NULL) { - dns_rbt_destroy(&view->denyanswernames); + dns_nametree_detach(&view->denyanswernames); } if (view->answernames_exclude != NULL) { - dns_rbt_destroy(&view->answernames_exclude); + dns_nametree_detach(&view->answernames_exclude); } if (view->sfd != NULL) { dns_rbt_destroy(&view->sfd); From 54fc02410e8cda25b81765b1a3b154b5308915a8 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 15 Aug 2023 15:30:12 -0700 Subject: [PATCH 04/11] refactor disable_algorithm and disable_ds_digest to use one data structure the functions for disabling DNSSEC signing algorithms and DS digest algorithms in resolver.c had a lot of duplicated code. this commit adds functions to implement a "bitfield tree", which is (currently) an RBT in which the node data contains arbitrary-sized bitfields to indicate whether a value has been added at the given node or not. (it can be changed to a QP trie later.) it also replaces the functions dns_resolver_disable_algorithm(), dns_resolver_algorithm_supported(), dns_resolver_disable_ds_digest() and dns_resolver_ds_digest_supported() with simple wrappers that call the new functions. --- lib/dns/resolver.c | 281 +++++++++++++++++---------------------------- 1 file changed, 106 insertions(+), 175 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index a56c7d9dac..4238a6d008 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -10718,11 +10718,11 @@ dns_resolver_printbadcache(dns_resolver_t *resolver, FILE *fp) { } static void -free_algorithm(void *node, void *arg) { - unsigned char *algorithms = node; +free_bfnode(void *node, void *arg) { + unsigned char *bfnode = node; isc_mem_t *mctx = arg; - isc_mem_put(mctx, algorithms, *algorithms); + isc_mem_put(mctx, bfnode, *bfnode); } void @@ -10734,109 +10734,6 @@ dns_resolver_reset_algorithms(dns_resolver_t *resolver) { } } -isc_result_t -dns_resolver_disable_algorithm(dns_resolver_t *resolver, const dns_name_t *name, - unsigned int alg) { - unsigned int len, mask; - isc_result_t result; - dns_rbtnode_t *node = NULL; - unsigned char *algorithms = NULL; - unsigned int algorithms_len; - - /* - * Whether an algorithm is disabled (or not) is stored in a - * per-name bitfield that is stored as the node data of an - * RBT. - */ - - REQUIRE(VALID_RESOLVER(resolver)); - if (alg > 255) { - return (ISC_R_RANGE); - } - - if (resolver->algorithms == NULL) { - result = dns_rbt_create(resolver->mctx, free_algorithm, - resolver->mctx, &resolver->algorithms); - if (result != ISC_R_SUCCESS) { - return (result); - } - } - - len = alg / 8 + 2; - mask = 1 << (alg % 8); - - result = dns_rbt_addnode(resolver->algorithms, name, &node); - - if (result != ISC_R_SUCCESS && result != ISC_R_EXISTS) { - return (result); - } - - /* If algorithms is set, algorithms[0] contains its length. */ - algorithms = node->data; - algorithms_len = (algorithms) ? algorithms[0] : 0; - - if (algorithms == NULL || len > algorithms_len) { - INSIST(len > 0); - /* - * If no bitfield exists in the node data, or if - * it is not long enough, allocate a new - * bitfield and copy the old (smaller) bitfield - * into it if one exists. - */ - node->data = algorithms = - isc_mem_creget(resolver->mctx, algorithms, - algorithms_len, len, sizeof(char)); - /* store the new length */ - algorithms[0] = len; - } - - algorithms[len - 1] |= mask; - return (ISC_R_SUCCESS); -} - -bool -dns_resolver_algorithm_supported(dns_resolver_t *resolver, - const dns_name_t *name, unsigned int alg) { - unsigned int len, mask; - unsigned char *algorithms; - void *data = NULL; - isc_result_t result; - bool found = false; - - REQUIRE(VALID_RESOLVER(resolver)); - - if ((alg == DST_ALG_DH) || (alg == DST_ALG_INDIRECT)) { - return (false); - } - - if (resolver->algorithms == NULL) { - goto unlock; - } - result = dns_rbt_findname(resolver->algorithms, name, 0, NULL, &data); - if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { - len = alg / 8 + 2; - mask = 1 << (alg % 8); - algorithms = data; - if (len <= *algorithms && (algorithms[len - 1] & mask) != 0) { - found = true; - } - } -unlock: - if (found) { - return (false); - } - - return (dst_algorithm_supported(alg)); -} - -static void -free_digest(void *node, void *arg) { - unsigned char *digests = node; - isc_mem_t *mctx = arg; - - isc_mem_put(mctx, digests, *digests); -} - void dns_resolver_reset_ds_digests(dns_resolver_t *resolver) { REQUIRE(VALID_RESOLVER(resolver)); @@ -10846,96 +10743,130 @@ dns_resolver_reset_ds_digests(dns_resolver_t *resolver) { } } +static isc_result_t +bftree_add(dns_rbt_t **bftp, isc_mem_t *mctx, const dns_name_t *name, + unsigned int val) { + isc_result_t result; + dns_rbt_t *bftree = NULL; + dns_rbtnode_t *node = NULL; + unsigned int len, mask; + unsigned char *bits = NULL; + unsigned int bits_len; + + if (*bftp == NULL) { + result = dns_rbt_create(mctx, free_bfnode, mctx, &bftree); + if (result != ISC_R_SUCCESS) { + return (result); + } + *bftp = bftree; + } else { + bftree = *bftp; + } + + len = val / 8 + 2; + mask = 1 << (val % 8); + + result = dns_rbt_addnode(bftree, name, &node); + if (result != ISC_R_SUCCESS && result != ISC_R_EXISTS) { + return (result); + } + + /* If bits is set, bits[0] contains its length. */ + bits = node->data; + bits_len = (bits != NULL) ? bits[0] : 0; + + if (bits == NULL || len > bits_len) { + INSIST(len > 0); + + /* + * If no bitfield exists in the node data, or if + * it is not long enough, allocate a new + * bitfield and copy the old (smaller) bitfield + * into it if one exists. + */ + node->data = bits = isc_mem_creget(mctx, bits, bits_len, len, + sizeof(char)); + /* store the new length */ + bits[0] = len; + } + + bits[len - 1] |= mask; + return (ISC_R_SUCCESS); +} + +static bool +bftree_present(dns_rbt_t *bftree, const dns_name_t *name, unsigned int val) { + isc_result_t result; + void *data = NULL; + + result = dns_rbt_findname(bftree, name, 0, NULL, &data); + if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { + unsigned int len = val / 8 + 2; + unsigned int mask = 1 << (val % 8); + unsigned char *bits = data; + if (len <= *bits && (bits[len - 1] & mask) != 0) { + return (true); + } + } + + return (false); +} + +isc_result_t +dns_resolver_disable_algorithm(dns_resolver_t *resolver, const dns_name_t *name, + unsigned int alg) { + REQUIRE(VALID_RESOLVER(resolver)); + + if (alg > 255) { + return (ISC_R_RANGE); + } + + return (bftree_add(&resolver->algorithms, resolver->mctx, name, alg)); +} + isc_result_t dns_resolver_disable_ds_digest(dns_resolver_t *resolver, const dns_name_t *name, unsigned int digest_type) { - unsigned int len, mask; - isc_result_t result; - dns_rbtnode_t *node = NULL; - - /* - * Whether a digest is disabled (or not) is stored in a per-name - * bitfield that is stored as the node data of an RBT. - */ - REQUIRE(VALID_RESOLVER(resolver)); + if (digest_type > 255) { return (ISC_R_RANGE); } - if (resolver->digests == NULL) { - result = dns_rbt_create(resolver->mctx, free_digest, - resolver->mctx, &resolver->digests); - if (result != ISC_R_SUCCESS) { - goto cleanup; + return (bftree_add(&resolver->digests, resolver->mctx, name, + digest_type)); +} + +bool +dns_resolver_algorithm_supported(dns_resolver_t *resolver, + const dns_name_t *name, unsigned int alg) { + REQUIRE(VALID_RESOLVER(resolver)); + + if ((alg == DST_ALG_DH) || (alg == DST_ALG_INDIRECT)) { + return (false); + } + + if (resolver->algorithms != NULL) { + if (bftree_present(resolver->algorithms, name, alg)) { + return (false); } } - len = digest_type / 8 + 2; - mask = 1 << (digest_type % 8); - - result = dns_rbt_addnode(resolver->digests, name, &node); - - if (result == ISC_R_SUCCESS || result == ISC_R_EXISTS) { - unsigned char *digests = node->data; - /* If digests is set, digests[0] contains its length. */ - if (digests == NULL || len > *digests) { - /* - * If no bitfield exists in the node data, or if - * it is not long enough, allocate a new - * bitfield and copy the old (smaller) bitfield - * into it if one exists. - */ - unsigned char *tmp = isc_mem_cget(resolver->mctx, 1, - len); - if (digests != NULL) { - memmove(tmp, digests, *digests); - } - tmp[len - 1] |= mask; - /* tmp[0] should contain the length of 'tmp'. */ - *tmp = len; - node->data = tmp; - /* Free the older bitfield. */ - if (digests != NULL) { - isc_mem_put(resolver->mctx, digests, *digests); - } - } else { - digests[len - 1] |= mask; - } - } - result = ISC_R_SUCCESS; -cleanup: - return (result); + return (dst_algorithm_supported(alg)); } bool dns_resolver_ds_digest_supported(dns_resolver_t *resolver, const dns_name_t *name, unsigned int digest_type) { - unsigned int len, mask; - unsigned char *digests; - void *data = NULL; - isc_result_t result; - bool found = false; - REQUIRE(VALID_RESOLVER(resolver)); - if (resolver->digests == NULL) { - goto unlock; - } - result = dns_rbt_findname(resolver->digests, name, 0, NULL, &data); - if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { - len = digest_type / 8 + 2; - mask = 1 << (digest_type % 8); - digests = data; - if (len <= *digests && (digests[len - 1] & mask) != 0) { - found = true; + if (resolver->digests != NULL) { + if (bftree_present(resolver->digests, name, digest_type)) { + return (false); } } -unlock: - if (found) { - return (false); - } + return (dst_ds_digest_supported(digest_type)); } From 9ed1dba9761db6011e2001e8485463ac0def07d1 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 16 Aug 2023 19:59:50 -0700 Subject: [PATCH 05/11] add semantics to dns_nametree to support bitfields name trees can now hold either boolean values or bit fields. the type is selected when the name tree is created. the behavior of dns_nametree_add() differs slightly beteween the types: in a boolean tree adding an existing name will return ISC_R_EXISTS, but in a bitfield tree it simply sets the specified bit in the bitfield and returns ISC_R_SUCCESS. --- bin/named/server.c | 2 +- lib/dns/include/dns/nametree.h | 39 ++++-- lib/dns/nametree.c | 97 +++++++++++---- lib/dns/resolver.c | 11 +- tests/dns/nametree_test.c | 220 ++++++++++++++++++++++++--------- 5 files changed, 273 insertions(+), 96 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index e0e12c860d..03483fd2b8 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -645,7 +645,7 @@ configure_view_nametable(const cfg_obj_t *vconfig, const cfg_obj_t *config, } } - dns_nametree_create(mctx, confname, ntp); + dns_nametree_create(mctx, DNS_NAMETREE_BOOL, confname, ntp); name = dns_fixedname_initname(&fixed); for (element = cfg_list_first(obj); element != NULL; diff --git a/lib/dns/include/dns/nametree.h b/lib/dns/include/dns/nametree.h index be549dab01..acd8bbd21f 100644 --- a/lib/dns/include/dns/nametree.h +++ b/lib/dns/include/dns/nametree.h @@ -40,16 +40,22 @@ /* Define to 1 for detailed reference tracing */ #undef DNS_NAMETREE_TRACE +typedef enum { DNS_NAMETREE_BOOL, DNS_NAMETREE_BITS } dns_nametree_type_t; + ISC_LANG_BEGINDECLS void -dns_nametree_create(isc_mem_t *mctx, const char *name, dns_nametree_t **ntp); +dns_nametree_create(isc_mem_t *mctx, dns_nametree_type_t type, const char *name, + dns_nametree_t **ntp); /*%< * Create a nametree. * * If 'name' is not NULL, it will be saved as the name of the QP trie * for debugging purposes. * + * 'type' indicates whether the tree will be used for storing boolean + * values (DNS_NAMETREE_BOOL) or bitfields (DNS_NAMETREE_BITS). + * * Requires: * *\li 'mctx' is a valid memory context. @@ -57,13 +63,22 @@ dns_nametree_create(isc_mem_t *mctx, const char *name, dns_nametree_t **ntp); */ isc_result_t -dns_nametree_add(dns_nametree_t *nametree, const dns_name_t *name, bool value); +dns_nametree_add(dns_nametree_t *nametree, const dns_name_t *name, + uint32_t value); /*%< * Add a node to 'nametree'. * - * 'value' is a single boolean value, true or false. If the name already + * If the nametree type was set to DNS_NAMETREE_BOOL, then 'value' + * represents a single boolean value, true or false. If the name already * exists within the tree, then return ISC_R_EXISTS. * + * If the nametree type was set to DNS_NAMETREE_BITS, then 'value' is + * a bit number within a bit field, which is sized to accomodate at least + * 'value' bits. If the name already exists, then that bit will be set + * in the bitfield, other bits will be retained, and ISC_R_SUCCESS will be + * returned. If 'value' excees the number of bits in the existing bit + * field, the field will be expanded. + * * Requires: * *\li 'nametree' points to a valid nametree. @@ -116,13 +131,21 @@ dns_nametree_find(dns_nametree_t *nametree, const dns_name_t *name, */ bool -dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name); +dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name, + uint32_t bit); /*%< - * Indicates whether a 'name' is covered by 'nametree'. + * Indicates whether a 'name' (with optional 'bit' value) is covered by + * 'nametree'. * - * This returns true if 'name' has a match or a closest ancestor in - * 'nametree' with its value set to 'true'. If a name is not found, or if - * 'nametree' is NULL, the default return value is false. + * In DNS_NAMETREE_BOOL nametrees, this returns true if 'name' has a match + * or a closest ancestor in 'nametree' with its value set to 'true'. + * 'bit' is ignored. + * + * In DNS_NAMETREE_BITS trees, this returns true if 'name' has a match or + * a closest ancestor in 'nametree' with the 'bit' set in its bitfield. + * + * If a name is not found, or if 'nametree' is NULL, the default return + * value is false. * * Requires: * diff --git a/lib/dns/nametree.c b/lib/dns/nametree.c index 464690d6c8..b148dd7565 100644 --- a/lib/dns/nametree.c +++ b/lib/dns/nametree.c @@ -32,6 +32,7 @@ struct dns_nametree { unsigned int magic; isc_mem_t *mctx; isc_refcount_t references; + dns_nametree_type_t type; dns_qpmulti_t *table; char name[64]; }; @@ -41,10 +42,8 @@ struct dns_ntnode { isc_refcount_t references; dns_fixedname_t fn; dns_name_t *name; - union { - bool value; - unsigned char *bits; - }; + bool set; + uint8_t *bits; }; /* QP trie methods */ @@ -67,6 +66,9 @@ static dns_qpmethods_t qpmethods = { static void destroy_ntnode(dns_ntnode_t *node) { isc_refcount_destroy(&node->references); + if (node->bits != NULL) { + isc_mem_cput(node->mctx, node->bits, 8, sizeof(uint32_t)); + } isc_mem_putanddetach(&node->mctx, node, sizeof(dns_ntnode_t)); } @@ -77,7 +79,8 @@ ISC_REFCOUNT_IMPL(dns_ntnode, destroy_ntnode); #endif void -dns_nametree_create(isc_mem_t *mctx, const char *name, dns_nametree_t **ntp) { +dns_nametree_create(isc_mem_t *mctx, dns_nametree_type_t type, const char *name, + dns_nametree_t **ntp) { dns_nametree_t *nametree = NULL; REQUIRE(ntp != NULL && *ntp == NULL); @@ -85,6 +88,7 @@ dns_nametree_create(isc_mem_t *mctx, const char *name, dns_nametree_t **ntp) { nametree = isc_mem_get(mctx, sizeof(*nametree)); *nametree = (dns_nametree_t){ .magic = NAMETREE_MAGIC, + .type = type, }; isc_mem_attach(mctx, &nametree->mctx); isc_refcount_init(&nametree->references, 1); @@ -126,31 +130,69 @@ newnode(isc_mem_t *mctx, const dns_name_t *name) { return (node); } +static bool +matchbit(unsigned char *bits, uint32_t val) { + unsigned int len = val / 8; + unsigned int mask = 1 << (val % 8); + + if ((bits[len] & mask) != 0) { + return (true); + } + return (false); +} + isc_result_t -dns_nametree_add(dns_nametree_t *nametree, const dns_name_t *name, bool value) { +dns_nametree_add(dns_nametree_t *nametree, const dns_name_t *name, + uint32_t value) { isc_result_t result; dns_qp_t *qp = NULL; + unsigned int len, mask; + dns_ntnode_t *old = NULL, *new = NULL; REQUIRE(VALID_NAMETREE(nametree)); REQUIRE(name != NULL); dns_qpmulti_write(nametree->table, &qp); - result = dns_qp_getname(qp, name, NULL, NULL); - if (result == ISC_R_SUCCESS) { - result = ISC_R_EXISTS; - } else { - dns_ntnode_t *node = newnode(nametree->mctx, name); - node->value = value; - result = dns_qp_insert(qp, node, 0); + switch (nametree->type) { + case DNS_NAMETREE_BOOL: + new = newnode(nametree->mctx, name); + new->set = value; + break; - /* - * We detach the node here, so any dns_qp_deletename() will - * destroy the node directly. - */ - dns_ntnode_detach(&node); + case DNS_NAMETREE_BITS: + result = dns_qp_getname(qp, name, (void **)&old, NULL); + if (result == ISC_R_SUCCESS && matchbit(old->bits, value)) { + goto out; + } + + len = value / 8; + mask = 1 << (value % 8); + + new = newnode(nametree->mctx, name); + new->bits = isc_mem_cget(nametree->mctx, 8, sizeof(value)); + if (result == ISC_R_SUCCESS) { + INSIST(old != NULL); + memmove(new->bits, old->bits, old->bits[0]); + result = dns_qp_deletename(qp, name, NULL, NULL); + INSIST(result == ISC_R_SUCCESS); + } + + new->bits[len] |= mask; + break; + default: + UNREACHABLE(); } + result = dns_qp_insert(qp, new, 0); + + /* + * We detach the node here, so any dns_qp_deletename() will + * destroy the node directly. + */ + dns_ntnode_detach(&new); + +out: dns_qp_compact(qp, DNS_QPGC_MAYBE); dns_qpmulti_commit(nametree->table, &qp); return (result); @@ -200,12 +242,12 @@ dns_nametree_find(dns_nametree_t *nametree, const dns_name_t *name, } bool -dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name) { +dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name, + uint32_t bit) { isc_result_t result; dns_qpread_t qpr; - dns_ntnode_t *ntnode = NULL; - void *pval = NULL; - bool value = false; + dns_ntnode_t *node = NULL; + bool ret = false; REQUIRE(nametree == NULL || VALID_NAMETREE(nametree)); @@ -214,14 +256,17 @@ dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name) { } dns_qpmulti_query(nametree->table, &qpr); - result = dns_qp_findname_ancestor(&qpr, name, 0, &pval, NULL); + result = dns_qp_findname_ancestor(&qpr, name, 0, (void **)&node, NULL); if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { - ntnode = pval; - value = ntnode->value; + if (nametree->type == DNS_NAMETREE_BOOL) { + ret = node->set; + } else { + ret = matchbit(node->bits, bit); + } } dns_qpread_destroy(nametree->table, &qpr); - return (value); + return (ret); } static void diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 4238a6d008..4b351ce20b 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -6752,7 +6752,7 @@ is_answeraddress_allowed(dns_view_t *view, dns_name_t *name, * If the owner name matches one in the exclusion list, either * exactly or partially, allow it. */ - if (dns_nametree_covered(view->answeracl_exclude, name)) { + if (dns_nametree_covered(view->answeracl_exclude, name, 0)) { return (true); } @@ -6865,7 +6865,7 @@ is_answertarget_allowed(fetchctx_t *fctx, dns_name_t *qname, dns_name_t *rname, * If the owner name matches one in the exclusion list, either * exactly or partially, allow it. */ - if (dns_nametree_covered(view->answernames_exclude, qname)) { + if (dns_nametree_covered(view->answernames_exclude, qname, 0)) { return (true); } @@ -6885,7 +6885,7 @@ is_answertarget_allowed(fetchctx_t *fctx, dns_name_t *qname, dns_name_t *rname, /* * Otherwise, apply filters. */ - if (dns_nametree_covered(view->denyanswernames, tname)) { + if (dns_nametree_covered(view->denyanswernames, tname, 0)) { char qnamebuf[DNS_NAME_FORMATSIZE]; char tnamebuf[DNS_NAME_FORMATSIZE]; char classbuf[64]; @@ -10887,7 +10887,8 @@ dns_resolver_setmustbesecure(dns_resolver_t *resolver, const dns_name_t *name, REQUIRE(VALID_RESOLVER(resolver)); if (resolver->mustbesecure == NULL) { - dns_nametree_create(resolver->mctx, "dnssec-must-be-secure", + dns_nametree_create(resolver->mctx, DNS_NAMETREE_BOOL, + "dnssec-must-be-secure", &resolver->mustbesecure); } @@ -10899,7 +10900,7 @@ bool dns_resolver_getmustbesecure(dns_resolver_t *resolver, const dns_name_t *name) { REQUIRE(VALID_RESOLVER(resolver)); - return (dns_nametree_covered(resolver->mustbesecure, name)); + return (dns_nametree_covered(resolver->mustbesecure, name, 0)); } void diff --git a/tests/dns/nametree_test.c b/tests/dns/nametree_test.c index 1557d2184f..840d3656e1 100644 --- a/tests/dns/nametree_test.c +++ b/tests/dns/nametree_test.c @@ -37,7 +37,8 @@ #include -dns_nametree_t *nametree = NULL; +dns_nametree_t *booltree = NULL; +dns_nametree_t *bitstree = NULL; /* * Test utilities. In general, these assume input parameters are valid @@ -46,38 +47,49 @@ dns_nametree_t *nametree = NULL; * the test code concise. */ -/* Common setup: create a nametree to test with a few keys */ +/* Common setup: create a booltree to test with a few keys */ static void create_tables(void) { dns_fixedname_t fn; dns_name_t *name = dns_fixedname_name(&fn); - dns_nametree_create(mctx, "test", &nametree); + dns_nametree_create(mctx, DNS_NAMETREE_BOOL, "bool test", &booltree); + dns_nametree_create(mctx, DNS_NAMETREE_BITS, "bits test", &bitstree); - /* Add a positive node */ + /* Add a positive boolean node */ dns_test_namefromstring("example.com.", &fn); - assert_int_equal(dns_nametree_add(nametree, name, true), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_add(booltree, name, true), ISC_R_SUCCESS); - /* Add a negative node below it */ + /* Add assorted bits to a bitfield node */ + assert_int_equal(dns_nametree_add(bitstree, name, 1), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_add(bitstree, name, 9), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_add(bitstree, name, 53), ISC_R_SUCCESS); + + /* Add negative boolean nodes with and without parents */ dns_test_namefromstring("negative.example.com.", &fn); - assert_int_equal(dns_nametree_add(nametree, name, false), + assert_int_equal(dns_nametree_add(booltree, name, false), + ISC_R_SUCCESS); + dns_test_namefromstring("negative.example.org.", &fn); + assert_int_equal(dns_nametree_add(booltree, name, false), ISC_R_SUCCESS); - /* Add a negative node with no parent */ - dns_test_namefromstring("negative.example.org.", &fn); - assert_int_equal(dns_nametree_add(nametree, name, false), - ISC_R_SUCCESS); + /* Add a bitfield nodes under a parent */ + dns_test_namefromstring("sub.example.com.", &fn); + assert_int_equal(dns_nametree_add(bitstree, name, 2), ISC_R_SUCCESS); } static void destroy_tables(void) { - if (nametree != NULL) { - dns_nametree_detach(&nametree); + if (booltree != NULL) { + dns_nametree_detach(&booltree); + } + if (bitstree != NULL) { + dns_nametree_detach(&bitstree); } rcu_barrier(); } -ISC_RUN_TEST_IMPL(add) { +ISC_RUN_TEST_IMPL(add_bool) { dns_ntnode_t *node = NULL; dns_fixedname_t fn; dns_name_t *name = dns_fixedname_name(&fn); @@ -88,15 +100,15 @@ ISC_RUN_TEST_IMPL(add) { * Getting the node for example.com should succeed. */ dns_test_namefromstring("example.com.", &fn); - assert_int_equal(dns_nametree_find(nametree, name, &node), + assert_int_equal(dns_nametree_find(booltree, name, &node), ISC_R_SUCCESS); dns_ntnode_detach(&node); /* * Try to add the same name. This should fail. */ - assert_int_equal(dns_nametree_add(nametree, name, false), ISC_R_EXISTS); - assert_int_equal(dns_nametree_find(nametree, name, &node), + assert_int_equal(dns_nametree_add(booltree, name, false), ISC_R_EXISTS); + assert_int_equal(dns_nametree_find(booltree, name, &node), ISC_R_SUCCESS); dns_ntnode_detach(&node); @@ -104,14 +116,133 @@ ISC_RUN_TEST_IMPL(add) { * Try to add a new name. */ dns_test_namefromstring("newname.com.", &fn); - assert_int_equal(dns_nametree_add(nametree, name, true), ISC_R_SUCCESS); - assert_int_equal(dns_nametree_find(nametree, name, &node), + assert_int_equal(dns_nametree_add(booltree, name, true), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_find(booltree, name, &node), ISC_R_SUCCESS); dns_ntnode_detach(&node); destroy_tables(); } +ISC_RUN_TEST_IMPL(add_bits) { + dns_ntnode_t *node = NULL; + dns_fixedname_t fn; + dns_name_t *name = dns_fixedname_name(&fn); + + create_tables(); + + /* + * Getting the node for example.com should succeed. + */ + dns_test_namefromstring("example.com.", &fn); + assert_int_equal(dns_nametree_find(booltree, name, &node), + ISC_R_SUCCESS); + dns_ntnode_detach(&node); + + /* + * Try to add the same name. This should succeed. + */ + assert_int_equal(dns_nametree_add(bitstree, name, 1), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_add(bitstree, name, 2), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_add(bitstree, name, 3), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_find(booltree, name, &node), + ISC_R_SUCCESS); + dns_ntnode_detach(&node); + + /* + * Try to add a new name. + */ + dns_test_namefromstring("newname.com.", &fn); + assert_int_equal(dns_nametree_add(booltree, name, true), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_find(booltree, name, &node), + ISC_R_SUCCESS); + dns_ntnode_detach(&node); + + destroy_tables(); +} + +ISC_RUN_TEST_IMPL(covered_bool) { + dns_fixedname_t fn; + dns_name_t *name = dns_fixedname_name(&fn); + const char *yesnames[] = { "example.com.", "sub.example.com.", NULL }; + const char *nonames[] = { "whatever.com.", "negative.example.com.", + "example.org.", "negative.example.org.", + NULL }; + create_tables(); + + for (const char **n = yesnames; *n != NULL; n++) { + dns_test_namefromstring(*n, &fn); + assert_true(dns_nametree_covered(booltree, name, 0)); + } + for (const char **n = nonames; *n != NULL; n++) { + dns_test_namefromstring(*n, &fn); + assert_false(dns_nametree_covered(booltree, name, 0)); + } + + /* If the nametree is NULL, dns_nametree_covered() returns false. */ + dns_test_namefromstring("anyname.example.", &fn); + assert_false(dns_nametree_covered(NULL, name, 0)); + + destroy_tables(); +} + +ISC_RUN_TEST_IMPL(covered_bits) { + dns_fixedname_t fn; + dns_name_t *name = dns_fixedname_name(&fn); + + create_tables(); + + /* check existing bit values */ + dns_test_namefromstring("example.com.", &fn); + assert_false(dns_nametree_covered(bitstree, name, 0)); + assert_true(dns_nametree_covered(bitstree, name, 1)); + assert_false(dns_nametree_covered(bitstree, name, 2)); + assert_false(dns_nametree_covered(bitstree, name, 3)); + assert_true(dns_nametree_covered(bitstree, name, 9)); + assert_true(dns_nametree_covered(bitstree, name, 53)); + assert_false(dns_nametree_covered(bitstree, name, 288)); + + /* add a small bit value, test again */ + assert_int_equal(dns_nametree_add(bitstree, name, 3), ISC_R_SUCCESS); + assert_true(dns_nametree_covered(bitstree, name, 3)); + + /* add a large bit value, test again */ + assert_int_equal(dns_nametree_add(bitstree, name, 615), ISC_R_SUCCESS); + assert_true(dns_nametree_covered(bitstree, name, 615)); + + /* check existing bit values for subdomain */ + dns_test_namefromstring("sub.example.com.", &fn); + assert_false(dns_nametree_covered(bitstree, name, 0)); + assert_false(dns_nametree_covered(bitstree, name, 1)); + assert_true(dns_nametree_covered(bitstree, name, 2)); + assert_false(dns_nametree_covered(bitstree, name, 3)); + assert_false(dns_nametree_covered(bitstree, name, 9)); + assert_false(dns_nametree_covered(bitstree, name, 53)); + assert_false(dns_nametree_covered(bitstree, name, 288)); + + /* check nonexistent subdomain is all false */ + dns_test_namefromstring("other.example.com", &fn); + assert_false(dns_nametree_covered(bitstree, name, 0)); + assert_false(dns_nametree_covered(bitstree, name, 1)); + assert_false(dns_nametree_covered(bitstree, name, 2)); + assert_false(dns_nametree_covered(bitstree, name, 3)); + assert_false(dns_nametree_covered(bitstree, name, 9)); + assert_false(dns_nametree_covered(bitstree, name, 53)); + assert_false(dns_nametree_covered(bitstree, name, 288)); + + /* check nonexistent domain is all false */ + dns_test_namefromstring("anyname.", &fn); + assert_false(dns_nametree_covered(bitstree, name, 0)); + assert_false(dns_nametree_covered(bitstree, name, 1)); + assert_false(dns_nametree_covered(bitstree, name, 2)); + assert_false(dns_nametree_covered(bitstree, name, 3)); + assert_false(dns_nametree_covered(bitstree, name, 9)); + assert_false(dns_nametree_covered(bitstree, name, 53)); + assert_false(dns_nametree_covered(bitstree, name, 288)); + + destroy_tables(); +} + ISC_RUN_TEST_IMPL(delete) { dns_fixedname_t fn; dns_name_t *name = dns_fixedname_name(&fn); @@ -120,11 +251,11 @@ ISC_RUN_TEST_IMPL(delete) { /* name doesn't match */ dns_test_namefromstring("example.org.", &fn); - assert_int_equal(dns_nametree_delete(nametree, name), ISC_R_NOTFOUND); + assert_int_equal(dns_nametree_delete(booltree, name), ISC_R_NOTFOUND); /* subdomain match is the same as no match */ dns_test_namefromstring("sub.example.org.", &fn); - assert_int_equal(dns_nametree_delete(nametree, name), ISC_R_NOTFOUND); + assert_int_equal(dns_nametree_delete(booltree, name), ISC_R_NOTFOUND); /* * delete requires exact match: this should return SUCCESS on @@ -132,12 +263,12 @@ ISC_RUN_TEST_IMPL(delete) { * ancestor does exist. */ dns_test_namefromstring("negative.example.com.", &fn); - assert_int_equal(dns_nametree_delete(nametree, name), ISC_R_SUCCESS); - assert_int_equal(dns_nametree_delete(nametree, name), ISC_R_NOTFOUND); + assert_int_equal(dns_nametree_delete(booltree, name), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_delete(booltree, name), ISC_R_NOTFOUND); dns_test_namefromstring("negative.example.org.", &fn); - assert_int_equal(dns_nametree_delete(nametree, name), ISC_R_SUCCESS); - assert_int_equal(dns_nametree_delete(nametree, name), ISC_R_NOTFOUND); + assert_int_equal(dns_nametree_delete(booltree, name), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_delete(booltree, name), ISC_R_NOTFOUND); destroy_tables(); } @@ -154,49 +285,26 @@ ISC_RUN_TEST_IMPL(find) { * that has a null key, too. */ dns_test_namefromstring("example.org.", &fn); - assert_int_equal(dns_nametree_find(nametree, name, &node), + assert_int_equal(dns_nametree_find(booltree, name, &node), ISC_R_NOTFOUND); dns_test_namefromstring("sub.example.com.", &fn); - assert_int_equal(dns_nametree_find(nametree, name, &node), + assert_int_equal(dns_nametree_find(booltree, name, &node), ISC_R_NOTFOUND); dns_test_namefromstring("example.com.", &fn); - assert_int_equal(dns_nametree_find(nametree, name, &node), + assert_int_equal(dns_nametree_find(booltree, name, &node), ISC_R_SUCCESS); dns_ntnode_detach(&node); destroy_tables(); } -ISC_RUN_TEST_IMPL(covered) { - dns_fixedname_t fn; - dns_name_t *name = dns_fixedname_name(&fn); - const char *yesnames[] = { "example.com.", "sub.example.com.", NULL }; - const char *nonames[] = { "whatever.com.", "negative.example.com.", - "example.org.", "negative.example.org.", - NULL }; - create_tables(); - - for (const char **n = yesnames; *n != NULL; n++) { - dns_test_namefromstring(*n, &fn); - assert_true(dns_nametree_covered(nametree, name)); - } - for (const char **n = nonames; *n != NULL; n++) { - dns_test_namefromstring(*n, &fn); - assert_false(dns_nametree_covered(nametree, name)); - } - - /* If nametree is NULL, dns_nametree_covered() returns false. */ - dns_test_namefromstring("anyname.example.", &fn); - assert_false(dns_nametree_covered(NULL, name)); - - destroy_tables(); -} - ISC_TEST_LIST_START -ISC_TEST_ENTRY(add) -ISC_TEST_ENTRY(covered) -ISC_TEST_ENTRY(find) +ISC_TEST_ENTRY(add_bool) +ISC_TEST_ENTRY(add_bits) +ISC_TEST_ENTRY(covered_bool) +ISC_TEST_ENTRY(covered_bits) ISC_TEST_ENTRY(delete) +ISC_TEST_ENTRY(find) ISC_TEST_LIST_END ISC_TEST_MAIN From bc3fd1a2efc784ef36906db3d5876a1de6f7f8da Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 16 Aug 2023 22:08:46 -0700 Subject: [PATCH 06/11] use bitfield name trees for disable-algorithms and disable-ds-digests switch disable-algorithms and disable-ds-digests to use bitfield-type name trees, replacing the RBT-based bftree. --- lib/dns/resolver.c | 112 ++++++++------------------------------------- 1 file changed, 20 insertions(+), 92 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 4b351ce20b..7984b3e4b5 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -562,8 +562,8 @@ struct dns_resolver { uint32_t lame_ttl; ISC_LIST(alternate_t) alternates; - dns_rbt_t *algorithms; - dns_rbt_t *digests; + dns_nametree_t *algorithms; + dns_nametree_t *digests; dns_nametree_t *mustbesecure; unsigned int spillatmax; unsigned int spillatmin; @@ -10717,20 +10717,12 @@ dns_resolver_printbadcache(dns_resolver_t *resolver, FILE *fp) { (void)dns_badcache_print(resolver->badcache, "Bad cache", fp); } -static void -free_bfnode(void *node, void *arg) { - unsigned char *bfnode = node; - isc_mem_t *mctx = arg; - - isc_mem_put(mctx, bfnode, *bfnode); -} - void dns_resolver_reset_algorithms(dns_resolver_t *resolver) { REQUIRE(VALID_RESOLVER(resolver)); if (resolver->algorithms != NULL) { - dns_rbt_destroy(&resolver->algorithms); + dns_nametree_detach(&resolver->algorithms); } } @@ -10739,79 +10731,10 @@ dns_resolver_reset_ds_digests(dns_resolver_t *resolver) { REQUIRE(VALID_RESOLVER(resolver)); if (resolver->digests != NULL) { - dns_rbt_destroy(&resolver->digests); + dns_nametree_detach(&resolver->digests); } } -static isc_result_t -bftree_add(dns_rbt_t **bftp, isc_mem_t *mctx, const dns_name_t *name, - unsigned int val) { - isc_result_t result; - dns_rbt_t *bftree = NULL; - dns_rbtnode_t *node = NULL; - unsigned int len, mask; - unsigned char *bits = NULL; - unsigned int bits_len; - - if (*bftp == NULL) { - result = dns_rbt_create(mctx, free_bfnode, mctx, &bftree); - if (result != ISC_R_SUCCESS) { - return (result); - } - *bftp = bftree; - } else { - bftree = *bftp; - } - - len = val / 8 + 2; - mask = 1 << (val % 8); - - result = dns_rbt_addnode(bftree, name, &node); - if (result != ISC_R_SUCCESS && result != ISC_R_EXISTS) { - return (result); - } - - /* If bits is set, bits[0] contains its length. */ - bits = node->data; - bits_len = (bits != NULL) ? bits[0] : 0; - - if (bits == NULL || len > bits_len) { - INSIST(len > 0); - - /* - * If no bitfield exists in the node data, or if - * it is not long enough, allocate a new - * bitfield and copy the old (smaller) bitfield - * into it if one exists. - */ - node->data = bits = isc_mem_creget(mctx, bits, bits_len, len, - sizeof(char)); - /* store the new length */ - bits[0] = len; - } - - bits[len - 1] |= mask; - return (ISC_R_SUCCESS); -} - -static bool -bftree_present(dns_rbt_t *bftree, const dns_name_t *name, unsigned int val) { - isc_result_t result; - void *data = NULL; - - result = dns_rbt_findname(bftree, name, 0, NULL, &data); - if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { - unsigned int len = val / 8 + 2; - unsigned int mask = 1 << (val % 8); - unsigned char *bits = data; - if (len <= *bits && (bits[len - 1] & mask) != 0) { - return (true); - } - } - - return (false); -} - isc_result_t dns_resolver_disable_algorithm(dns_resolver_t *resolver, const dns_name_t *name, unsigned int alg) { @@ -10821,7 +10744,12 @@ dns_resolver_disable_algorithm(dns_resolver_t *resolver, const dns_name_t *name, return (ISC_R_RANGE); } - return (bftree_add(&resolver->algorithms, resolver->mctx, name, alg)); + if (resolver->algorithms == NULL) { + dns_nametree_create(resolver->mctx, DNS_NAMETREE_BITS, + "algorithms", &resolver->algorithms); + } + + return (dns_nametree_add(resolver->algorithms, name, alg)); } isc_result_t @@ -10833,8 +10761,12 @@ dns_resolver_disable_ds_digest(dns_resolver_t *resolver, const dns_name_t *name, return (ISC_R_RANGE); } - return (bftree_add(&resolver->digests, resolver->mctx, name, - digest_type)); + if (resolver->digests == NULL) { + dns_nametree_create(resolver->mctx, DNS_NAMETREE_BITS, + "ds-digests", &resolver->digests); + } + + return (dns_nametree_add(resolver->digests, name, digest_type)); } bool @@ -10846,10 +10778,8 @@ dns_resolver_algorithm_supported(dns_resolver_t *resolver, return (false); } - if (resolver->algorithms != NULL) { - if (bftree_present(resolver->algorithms, name, alg)) { - return (false); - } + if (dns_nametree_covered(resolver->algorithms, name, alg)) { + return (false); } return (dst_algorithm_supported(alg)); @@ -10861,10 +10791,8 @@ dns_resolver_ds_digest_supported(dns_resolver_t *resolver, unsigned int digest_type) { REQUIRE(VALID_RESOLVER(resolver)); - if (resolver->digests != NULL) { - if (bftree_present(resolver->digests, name, digest_type)) { - return (false); - } + if (dns_nametree_covered(resolver->digests, name, digest_type)) { + return (false); } return (dst_ds_digest_supported(digest_type)); From 0ebaa26da72689a27ead6dde2ce337138ac7c448 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 16 Aug 2023 23:26:50 -0700 Subject: [PATCH 07/11] add semantics to name trees to support counters name trees can now also hold trees of counters. each time a name dns_nametree_add() is called with a given name, the counter for that name is incremented; the name is not deleted until dns_nametree_delete() is called the same number of times. this is meant to be used for synth-from-dnssec, which is incremented for each key defined at a name, and decremented when a key is removed, the name must continue to exist until the number of keys has reached zero. --- lib/dns/include/dns/nametree.h | 19 +++++++- lib/dns/nametree.c | 86 +++++++++++++++++++++++++--------- tests/dns/nametree_test.c | 44 ++++++++++++++++- 3 files changed, 125 insertions(+), 24 deletions(-) diff --git a/lib/dns/include/dns/nametree.h b/lib/dns/include/dns/nametree.h index acd8bbd21f..bae8dcefb3 100644 --- a/lib/dns/include/dns/nametree.h +++ b/lib/dns/include/dns/nametree.h @@ -40,7 +40,11 @@ /* Define to 1 for detailed reference tracing */ #undef DNS_NAMETREE_TRACE -typedef enum { DNS_NAMETREE_BOOL, DNS_NAMETREE_BITS } dns_nametree_type_t; +typedef enum { + DNS_NAMETREE_BOOL, + DNS_NAMETREE_BITS, + DNS_NAMETREE_COUNT +} dns_nametree_type_t; ISC_LANG_BEGINDECLS @@ -54,7 +58,8 @@ dns_nametree_create(isc_mem_t *mctx, dns_nametree_type_t type, const char *name, * for debugging purposes. * * 'type' indicates whether the tree will be used for storing boolean - * values (DNS_NAMETREE_BOOL) or bitfields (DNS_NAMETREE_BITS). + * values (DNS_NAMETREE_BOOL), bitfields (DNS_NAMETREE_BITS), or counters + * (DNS_NAMETREE_COUNT). * * Requires: * @@ -72,6 +77,12 @@ dns_nametree_add(dns_nametree_t *nametree, const dns_name_t *name, * represents a single boolean value, true or false. If the name already * exists within the tree, then return ISC_R_EXISTS. * + * If the nametree type was set to DNS_NAMETREE_COUNT, then 'value' + * can only be true. Each time the same name is added to the tree, + * ISC_R_SUCCESS is returned and a counter is incremented. + * dns_nametree_delete() must be deleted the same number of times + * as dns_nametree_add() before the name is removed from the tree. + * * If the nametree type was set to DNS_NAMETREE_BITS, then 'value' is * a bit number within a bit field, which is sized to accomodate at least * 'value' bits. If the name already exists, then that bit will be set @@ -96,6 +107,10 @@ dns_nametree_delete(dns_nametree_t *nametree, const dns_name_t *name); /*%< * Delete 'name' from 'nametree'. * + * If the nametree type was set to DNS_NAMETREE_COUNT, then this must + * be called for each name the same number of times as dns_nametree_add() + * was called before the name is removed. + * * Requires: * *\li 'nametree' points to a valid nametree. diff --git a/lib/dns/nametree.c b/lib/dns/nametree.c index b148dd7565..bbca177aea 100644 --- a/lib/dns/nametree.c +++ b/lib/dns/nametree.c @@ -67,7 +67,8 @@ static void destroy_ntnode(dns_ntnode_t *node) { isc_refcount_destroy(&node->references); if (node->bits != NULL) { - isc_mem_cput(node->mctx, node->bits, 8, sizeof(uint32_t)); + isc_mem_cput(node->mctx, node->bits, node->bits[0], + sizeof(char)); } isc_mem_putanddetach(&node->mctx, node, sizeof(dns_ntnode_t)); } @@ -103,8 +104,20 @@ dns_nametree_create(isc_mem_t *mctx, dns_nametree_type_t type, const char *name, static void destroy_nametree(dns_nametree_t *nametree) { + /* dns_qpread_t qpr; */ + /* dns_qpiter_t iter; */ + /* void *pval = NULL; */ + nametree->magic = 0; + /* dns_qpmulti_query(nametree->table, &qpr); */ + /* dns_qpiter_init(&qpr, &iter); */ + /* while (dns_qpiter_next(&iter, &pval, NULL) == ISC_R_SUCCESS) { */ + /* dns_ntnode_t *n = pval; */ + /* dns_ntnode_detach(&n); */ + /* } */ + /* dns_qpread_destroy(nametree->table, &qpr); */ + dns_qpmulti_destroy(&nametree->table); isc_refcount_destroy(&nametree->references); @@ -132,10 +145,10 @@ newnode(isc_mem_t *mctx, const dns_name_t *name) { static bool matchbit(unsigned char *bits, uint32_t val) { - unsigned int len = val / 8; + unsigned int len = val / 8 + 2; unsigned int mask = 1 << (val % 8); - if ((bits[len] & mask) != 0) { + if (len <= bits[0] && (bits[len - 1] & mask) != 0) { return (true); } return (false); @@ -146,7 +159,7 @@ dns_nametree_add(dns_nametree_t *nametree, const dns_name_t *name, uint32_t value) { isc_result_t result; dns_qp_t *qp = NULL; - unsigned int len, mask; + uint32_t size, pos, mask, count = 0; dns_ntnode_t *old = NULL, *new = NULL; REQUIRE(VALID_NAMETREE(nametree)); @@ -160,32 +173,44 @@ dns_nametree_add(dns_nametree_t *nametree, const dns_name_t *name, new->set = value; break; + case DNS_NAMETREE_COUNT: + new = newnode(nametree->mctx, name); + new->set = true; + result = dns_qp_deletename(qp, name, (void **)&old, &count); + if (result == ISC_R_SUCCESS) { + count += 1; + } + break; + case DNS_NAMETREE_BITS: result = dns_qp_getname(qp, name, (void **)&old, NULL); if (result == ISC_R_SUCCESS && matchbit(old->bits, value)) { goto out; } - len = value / 8; + size = pos = value / 8 + 2; mask = 1 << (value % 8); + if (old != NULL && old->bits[0] > pos) { + size = old->bits[0]; + } + new = newnode(nametree->mctx, name); - new->bits = isc_mem_cget(nametree->mctx, 8, sizeof(value)); + new->bits = isc_mem_cget(nametree->mctx, size, sizeof(char)); if (result == ISC_R_SUCCESS) { - INSIST(old != NULL); memmove(new->bits, old->bits, old->bits[0]); result = dns_qp_deletename(qp, name, NULL, NULL); INSIST(result == ISC_R_SUCCESS); } - new->bits[len] |= mask; + new->bits[pos - 1] |= mask; + new->bits[0] = size; break; default: UNREACHABLE(); } - result = dns_qp_insert(qp, new, 0); - + result = dns_qp_insert(qp, new, count); /* * We detach the node here, so any dns_qp_deletename() will * destroy the node directly. @@ -202,16 +227,30 @@ isc_result_t dns_nametree_delete(dns_nametree_t *nametree, const dns_name_t *name) { isc_result_t result; dns_qp_t *qp = NULL; - void *pval = NULL; + dns_ntnode_t *old = NULL; + uint32_t count; REQUIRE(VALID_NAMETREE(nametree)); REQUIRE(name != NULL); dns_qpmulti_write(nametree->table, &qp); - result = dns_qp_deletename(qp, name, &pval, NULL); - if (result == ISC_R_SUCCESS) { - dns_ntnode_t *n = pval; - dns_ntnode_detach(&n); + result = dns_qp_deletename(qp, name, (void **)&old, &count); + switch (nametree->type) { + case DNS_NAMETREE_BOOL: + case DNS_NAMETREE_BITS: + break; + + case DNS_NAMETREE_COUNT: + if (result == ISC_R_SUCCESS && count-- != 0) { + dns_ntnode_t *new = newnode(nametree->mctx, name); + new->set = true; + result = dns_qp_insert(qp, new, count); + INSIST(result == ISC_R_SUCCESS); + dns_ntnode_detach(&new); + } + break; + default: + UNREACHABLE(); } dns_qp_compact(qp, DNS_QPGC_MAYBE); dns_qpmulti_commit(nametree->table, &qp); @@ -223,18 +262,17 @@ isc_result_t dns_nametree_find(dns_nametree_t *nametree, const dns_name_t *name, dns_ntnode_t **ntnodep) { isc_result_t result; + dns_ntnode_t *node = NULL; dns_qpread_t qpr; - void *pval = NULL; REQUIRE(VALID_NAMETREE(nametree)); REQUIRE(name != NULL); REQUIRE(ntnodep != NULL && *ntnodep == NULL); dns_qpmulti_query(nametree->table, &qpr); - result = dns_qp_getname(&qpr, name, &pval, NULL); + result = dns_qp_getname(&qpr, name, (void **)&node, NULL); if (result == ISC_R_SUCCESS) { - dns_ntnode_t *knode = pval; - dns_ntnode_attach(knode, ntnodep); + dns_ntnode_attach(node, ntnodep); } dns_qpread_destroy(nametree->table, &qpr); @@ -258,10 +296,16 @@ dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name, dns_qpmulti_query(nametree->table, &qpr); result = dns_qp_findname_ancestor(&qpr, name, 0, (void **)&node, NULL); if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { - if (nametree->type == DNS_NAMETREE_BOOL) { + switch (nametree->type) { + case DNS_NAMETREE_BOOL: ret = node->set; - } else { + break; + case DNS_NAMETREE_COUNT: + ret = true; + break; + case DNS_NAMETREE_BITS: ret = matchbit(node->bits, bit); + break; } } diff --git a/tests/dns/nametree_test.c b/tests/dns/nametree_test.c index 840d3656e1..23df32f7e6 100644 --- a/tests/dns/nametree_test.c +++ b/tests/dns/nametree_test.c @@ -39,6 +39,7 @@ dns_nametree_t *booltree = NULL; dns_nametree_t *bitstree = NULL; +dns_nametree_t *counttree = NULL; /* * Test utilities. In general, these assume input parameters are valid @@ -55,6 +56,7 @@ create_tables(void) { dns_nametree_create(mctx, DNS_NAMETREE_BOOL, "bool test", &booltree); dns_nametree_create(mctx, DNS_NAMETREE_BITS, "bits test", &bitstree); + dns_nametree_create(mctx, DNS_NAMETREE_COUNT, "count test", &counttree); /* Add a positive boolean node */ dns_test_namefromstring("example.com.", &fn); @@ -73,7 +75,7 @@ create_tables(void) { assert_int_equal(dns_nametree_add(booltree, name, false), ISC_R_SUCCESS); - /* Add a bitfield nodes under a parent */ + /* Add a bitfield node under a parent */ dns_test_namefromstring("sub.example.com.", &fn); assert_int_equal(dns_nametree_add(bitstree, name, 2), ISC_R_SUCCESS); } @@ -86,6 +88,9 @@ destroy_tables(void) { if (bitstree != NULL) { dns_nametree_detach(&bitstree); } + if (counttree != NULL) { + dns_nametree_detach(&counttree); + } rcu_barrier(); } @@ -161,6 +166,42 @@ ISC_RUN_TEST_IMPL(add_bits) { destroy_tables(); } +ISC_RUN_TEST_IMPL(add_count) { + dns_fixedname_t fn; + dns_name_t *name = dns_fixedname_name(&fn); + + create_tables(); + + /* add a counter node five times */ + dns_test_namefromstring("example.com.", &fn); + assert_int_equal(dns_nametree_add(counttree, name, 0), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_add(counttree, name, 0), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_add(counttree, name, 0), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_add(counttree, name, 0), ISC_R_SUCCESS); + assert_int_equal(dns_nametree_add(counttree, name, 0), ISC_R_SUCCESS); + + /* delete it five times, checking coverage each time */ + assert_true(dns_nametree_covered(counttree, name, 0)); + assert_int_equal(dns_nametree_delete(counttree, name), ISC_R_SUCCESS); + + assert_true(dns_nametree_covered(counttree, name, 0)); + assert_int_equal(dns_nametree_delete(counttree, name), ISC_R_SUCCESS); + + assert_true(dns_nametree_covered(counttree, name, 0)); + assert_int_equal(dns_nametree_delete(counttree, name), ISC_R_SUCCESS); + + assert_true(dns_nametree_covered(counttree, name, 0)); + assert_int_equal(dns_nametree_delete(counttree, name), ISC_R_SUCCESS); + + assert_true(dns_nametree_covered(counttree, name, 0)); + assert_int_equal(dns_nametree_delete(counttree, name), ISC_R_SUCCESS); + + assert_false(dns_nametree_covered(counttree, name, 0)); + assert_int_equal(dns_nametree_delete(counttree, name), ISC_R_NOTFOUND); + + destroy_tables(); +} + ISC_RUN_TEST_IMPL(covered_bool) { dns_fixedname_t fn; dns_name_t *name = dns_fixedname_name(&fn); @@ -301,6 +342,7 @@ ISC_RUN_TEST_IMPL(find) { ISC_TEST_LIST_START ISC_TEST_ENTRY(add_bool) ISC_TEST_ENTRY(add_bits) +ISC_TEST_ENTRY(add_count) ISC_TEST_ENTRY(covered_bool) ISC_TEST_ENTRY(covered_bits) ISC_TEST_ENTRY(delete) From b1e4e2a9ee402b4ad510ddf7b21b0e2b8dda1366 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 16 Aug 2023 23:51:56 -0700 Subject: [PATCH 08/11] add a 'foundname' argument to dns_nametree_covered() when checking whether a name is covered, the ancestor name that was found can be set into a name object passed in. --- lib/dns/include/dns/nametree.h | 5 +- lib/dns/nametree.c | 7 +- lib/dns/resolver.c | 12 +-- tests/dns/nametree_test.c | 158 +++++++++++++++------------------ 4 files changed, 86 insertions(+), 96 deletions(-) diff --git a/lib/dns/include/dns/nametree.h b/lib/dns/include/dns/nametree.h index bae8dcefb3..3c5ac51893 100644 --- a/lib/dns/include/dns/nametree.h +++ b/lib/dns/include/dns/nametree.h @@ -147,7 +147,7 @@ dns_nametree_find(dns_nametree_t *nametree, const dns_name_t *name, bool dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name, - uint32_t bit); + dns_name_t *found, uint32_t bit); /*%< * Indicates whether a 'name' (with optional 'bit' value) is covered by * 'nametree'. @@ -162,6 +162,9 @@ dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name, * If a name is not found, or if 'nametree' is NULL, the default return * value is false. * + * If 'found' is not NULL, the name or ancestor name that was found in + * the tree is copied into it. + * * Requires: * *\li 'nametree' is a valid nametree, or is NULL. diff --git a/lib/dns/nametree.c b/lib/dns/nametree.c index bbca177aea..af7cfbddf7 100644 --- a/lib/dns/nametree.c +++ b/lib/dns/nametree.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -281,7 +282,7 @@ dns_nametree_find(dns_nametree_t *nametree, const dns_name_t *name, bool dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name, - uint32_t bit) { + dns_name_t *found, uint32_t bit) { isc_result_t result; dns_qpread_t qpr; dns_ntnode_t *node = NULL; @@ -296,6 +297,10 @@ dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name, dns_qpmulti_query(nametree->table, &qpr); result = dns_qp_findname_ancestor(&qpr, name, 0, (void **)&node, NULL); if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { + if (found != NULL) { + dns_name_copy(node->name, found); + } + switch (nametree->type) { case DNS_NAMETREE_BOOL: ret = node->set; diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 7984b3e4b5..02a578c261 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -6752,7 +6752,7 @@ is_answeraddress_allowed(dns_view_t *view, dns_name_t *name, * If the owner name matches one in the exclusion list, either * exactly or partially, allow it. */ - if (dns_nametree_covered(view->answeracl_exclude, name, 0)) { + if (dns_nametree_covered(view->answeracl_exclude, name, NULL, 0)) { return (true); } @@ -6865,7 +6865,7 @@ is_answertarget_allowed(fetchctx_t *fctx, dns_name_t *qname, dns_name_t *rname, * If the owner name matches one in the exclusion list, either * exactly or partially, allow it. */ - if (dns_nametree_covered(view->answernames_exclude, qname, 0)) { + if (dns_nametree_covered(view->answernames_exclude, qname, NULL, 0)) { return (true); } @@ -6885,7 +6885,7 @@ is_answertarget_allowed(fetchctx_t *fctx, dns_name_t *qname, dns_name_t *rname, /* * Otherwise, apply filters. */ - if (dns_nametree_covered(view->denyanswernames, tname, 0)) { + if (dns_nametree_covered(view->denyanswernames, tname, NULL, 0)) { char qnamebuf[DNS_NAME_FORMATSIZE]; char tnamebuf[DNS_NAME_FORMATSIZE]; char classbuf[64]; @@ -10778,7 +10778,7 @@ dns_resolver_algorithm_supported(dns_resolver_t *resolver, return (false); } - if (dns_nametree_covered(resolver->algorithms, name, alg)) { + if (dns_nametree_covered(resolver->algorithms, name, NULL, alg)) { return (false); } @@ -10791,7 +10791,7 @@ dns_resolver_ds_digest_supported(dns_resolver_t *resolver, unsigned int digest_type) { REQUIRE(VALID_RESOLVER(resolver)); - if (dns_nametree_covered(resolver->digests, name, digest_type)) { + if (dns_nametree_covered(resolver->digests, name, NULL, digest_type)) { return (false); } @@ -10828,7 +10828,7 @@ bool dns_resolver_getmustbesecure(dns_resolver_t *resolver, const dns_name_t *name) { REQUIRE(VALID_RESOLVER(resolver)); - return (dns_nametree_covered(resolver->mustbesecure, name, 0)); + return (dns_nametree_covered(resolver->mustbesecure, name, NULL, 0)); } void diff --git a/tests/dns/nametree_test.c b/tests/dns/nametree_test.c index 23df32f7e6..39ac4467c1 100644 --- a/tests/dns/nametree_test.c +++ b/tests/dns/nametree_test.c @@ -48,9 +48,9 @@ dns_nametree_t *counttree = NULL; * the test code concise. */ -/* Common setup: create a booltree to test with a few keys */ -static void -create_tables(void) { +/* Common setup: create trees of each type with a few keys */ +static int +setup(void **state ISC_ATTR_UNUSED) { dns_fixedname_t fn; dns_name_t *name = dns_fixedname_name(&fn); @@ -78,20 +78,17 @@ create_tables(void) { /* Add a bitfield node under a parent */ dns_test_namefromstring("sub.example.com.", &fn); assert_int_equal(dns_nametree_add(bitstree, name, 2), ISC_R_SUCCESS); + + return (0); } -static void -destroy_tables(void) { - if (booltree != NULL) { - dns_nametree_detach(&booltree); - } - if (bitstree != NULL) { - dns_nametree_detach(&bitstree); - } - if (counttree != NULL) { - dns_nametree_detach(&counttree); - } +static int +teardown(void **state ISC_ATTR_UNUSED) { + dns_nametree_detach(&booltree); + dns_nametree_detach(&bitstree); + dns_nametree_detach(&counttree); rcu_barrier(); + return (0); } ISC_RUN_TEST_IMPL(add_bool) { @@ -99,8 +96,6 @@ ISC_RUN_TEST_IMPL(add_bool) { dns_fixedname_t fn; dns_name_t *name = dns_fixedname_name(&fn); - create_tables(); - /* * Getting the node for example.com should succeed. */ @@ -125,8 +120,6 @@ ISC_RUN_TEST_IMPL(add_bool) { assert_int_equal(dns_nametree_find(booltree, name, &node), ISC_R_SUCCESS); dns_ntnode_detach(&node); - - destroy_tables(); } ISC_RUN_TEST_IMPL(add_bits) { @@ -134,8 +127,6 @@ ISC_RUN_TEST_IMPL(add_bits) { dns_fixedname_t fn; dns_name_t *name = dns_fixedname_name(&fn); - create_tables(); - /* * Getting the node for example.com should succeed. */ @@ -162,16 +153,12 @@ ISC_RUN_TEST_IMPL(add_bits) { assert_int_equal(dns_nametree_find(booltree, name, &node), ISC_R_SUCCESS); dns_ntnode_detach(&node); - - destroy_tables(); } ISC_RUN_TEST_IMPL(add_count) { dns_fixedname_t fn; dns_name_t *name = dns_fixedname_name(&fn); - create_tables(); - /* add a counter node five times */ dns_test_namefromstring("example.com.", &fn); assert_int_equal(dns_nametree_add(counttree, name, 0), ISC_R_SUCCESS); @@ -181,115 +168,116 @@ ISC_RUN_TEST_IMPL(add_count) { assert_int_equal(dns_nametree_add(counttree, name, 0), ISC_R_SUCCESS); /* delete it five times, checking coverage each time */ - assert_true(dns_nametree_covered(counttree, name, 0)); + assert_true(dns_nametree_covered(counttree, name, NULL, 0)); assert_int_equal(dns_nametree_delete(counttree, name), ISC_R_SUCCESS); - assert_true(dns_nametree_covered(counttree, name, 0)); + assert_true(dns_nametree_covered(counttree, name, NULL, 0)); assert_int_equal(dns_nametree_delete(counttree, name), ISC_R_SUCCESS); - assert_true(dns_nametree_covered(counttree, name, 0)); + assert_true(dns_nametree_covered(counttree, name, NULL, 0)); assert_int_equal(dns_nametree_delete(counttree, name), ISC_R_SUCCESS); - assert_true(dns_nametree_covered(counttree, name, 0)); + assert_true(dns_nametree_covered(counttree, name, NULL, 0)); assert_int_equal(dns_nametree_delete(counttree, name), ISC_R_SUCCESS); - assert_true(dns_nametree_covered(counttree, name, 0)); + assert_true(dns_nametree_covered(counttree, name, NULL, 0)); assert_int_equal(dns_nametree_delete(counttree, name), ISC_R_SUCCESS); - assert_false(dns_nametree_covered(counttree, name, 0)); + assert_false(dns_nametree_covered(counttree, name, NULL, 0)); assert_int_equal(dns_nametree_delete(counttree, name), ISC_R_NOTFOUND); - - destroy_tables(); } ISC_RUN_TEST_IMPL(covered_bool) { - dns_fixedname_t fn; - dns_name_t *name = dns_fixedname_name(&fn); + dns_fixedname_t fn, fn2; + dns_name_t *name = dns_fixedname_initname(&fn); + dns_name_t *found = dns_fixedname_initname(&fn2); + char buf[DNS_NAME_FORMATSIZE]; const char *yesnames[] = { "example.com.", "sub.example.com.", NULL }; const char *nonames[] = { "whatever.com.", "negative.example.com.", "example.org.", "negative.example.org.", NULL }; - create_tables(); for (const char **n = yesnames; *n != NULL; n++) { dns_test_namefromstring(*n, &fn); - assert_true(dns_nametree_covered(booltree, name, 0)); + assert_true(dns_nametree_covered(booltree, name, NULL, 0)); } for (const char **n = nonames; *n != NULL; n++) { dns_test_namefromstring(*n, &fn); - assert_false(dns_nametree_covered(booltree, name, 0)); + assert_false(dns_nametree_covered(booltree, name, NULL, 0)); } /* If the nametree is NULL, dns_nametree_covered() returns false. */ dns_test_namefromstring("anyname.example.", &fn); - assert_false(dns_nametree_covered(NULL, name, 0)); + assert_false(dns_nametree_covered(NULL, name, NULL, 0)); - destroy_tables(); + /* Check that the found name is as expected */ + dns_test_namefromstring("other.example.com.", &fn); + assert_true(dns_nametree_covered(booltree, name, found, 0)); + dns_name_format(found, buf, sizeof(buf)); + assert_string_equal(buf, "example.com"); } ISC_RUN_TEST_IMPL(covered_bits) { dns_fixedname_t fn; dns_name_t *name = dns_fixedname_name(&fn); - create_tables(); - /* check existing bit values */ dns_test_namefromstring("example.com.", &fn); - assert_false(dns_nametree_covered(bitstree, name, 0)); - assert_true(dns_nametree_covered(bitstree, name, 1)); - assert_false(dns_nametree_covered(bitstree, name, 2)); - assert_false(dns_nametree_covered(bitstree, name, 3)); - assert_true(dns_nametree_covered(bitstree, name, 9)); - assert_true(dns_nametree_covered(bitstree, name, 53)); - assert_false(dns_nametree_covered(bitstree, name, 288)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 0)); + assert_true(dns_nametree_covered(bitstree, name, NULL, 1)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 2)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 3)); + assert_true(dns_nametree_covered(bitstree, name, NULL, 9)); + assert_true(dns_nametree_covered(bitstree, name, NULL, 53)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 288)); /* add a small bit value, test again */ assert_int_equal(dns_nametree_add(bitstree, name, 3), ISC_R_SUCCESS); - assert_true(dns_nametree_covered(bitstree, name, 3)); + assert_true(dns_nametree_covered(bitstree, name, NULL, 3)); /* add a large bit value, test again */ + assert_false(dns_nametree_covered(bitstree, name, NULL, 615)); assert_int_equal(dns_nametree_add(bitstree, name, 615), ISC_R_SUCCESS); - assert_true(dns_nametree_covered(bitstree, name, 615)); + assert_true(dns_nametree_covered(bitstree, name, NULL, 615)); + assert_int_equal(dns_nametree_add(bitstree, name, 999), ISC_R_SUCCESS); + assert_true(dns_nametree_covered(bitstree, name, NULL, 999)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 998)); /* check existing bit values for subdomain */ dns_test_namefromstring("sub.example.com.", &fn); - assert_false(dns_nametree_covered(bitstree, name, 0)); - assert_false(dns_nametree_covered(bitstree, name, 1)); - assert_true(dns_nametree_covered(bitstree, name, 2)); - assert_false(dns_nametree_covered(bitstree, name, 3)); - assert_false(dns_nametree_covered(bitstree, name, 9)); - assert_false(dns_nametree_covered(bitstree, name, 53)); - assert_false(dns_nametree_covered(bitstree, name, 288)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 0)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 1)); + assert_true(dns_nametree_covered(bitstree, name, NULL, 2)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 3)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 9)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 53)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 288)); /* check nonexistent subdomain is all false */ dns_test_namefromstring("other.example.com", &fn); - assert_false(dns_nametree_covered(bitstree, name, 0)); - assert_false(dns_nametree_covered(bitstree, name, 1)); - assert_false(dns_nametree_covered(bitstree, name, 2)); - assert_false(dns_nametree_covered(bitstree, name, 3)); - assert_false(dns_nametree_covered(bitstree, name, 9)); - assert_false(dns_nametree_covered(bitstree, name, 53)); - assert_false(dns_nametree_covered(bitstree, name, 288)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 0)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 1)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 2)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 3)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 9)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 53)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 288)); /* check nonexistent domain is all false */ dns_test_namefromstring("anyname.", &fn); - assert_false(dns_nametree_covered(bitstree, name, 0)); - assert_false(dns_nametree_covered(bitstree, name, 1)); - assert_false(dns_nametree_covered(bitstree, name, 2)); - assert_false(dns_nametree_covered(bitstree, name, 3)); - assert_false(dns_nametree_covered(bitstree, name, 9)); - assert_false(dns_nametree_covered(bitstree, name, 53)); - assert_false(dns_nametree_covered(bitstree, name, 288)); - - destroy_tables(); + assert_false(dns_nametree_covered(bitstree, name, NULL, 0)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 1)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 2)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 3)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 9)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 53)); + assert_false(dns_nametree_covered(bitstree, name, NULL, 288)); } ISC_RUN_TEST_IMPL(delete) { dns_fixedname_t fn; dns_name_t *name = dns_fixedname_name(&fn); - create_tables(); - /* name doesn't match */ dns_test_namefromstring("example.org.", &fn); assert_int_equal(dns_nametree_delete(booltree, name), ISC_R_NOTFOUND); @@ -310,8 +298,6 @@ ISC_RUN_TEST_IMPL(delete) { dns_test_namefromstring("negative.example.org.", &fn); assert_int_equal(dns_nametree_delete(booltree, name), ISC_R_SUCCESS); assert_int_equal(dns_nametree_delete(booltree, name), ISC_R_NOTFOUND); - - destroy_tables(); } ISC_RUN_TEST_IMPL(find) { @@ -319,8 +305,6 @@ ISC_RUN_TEST_IMPL(find) { dns_fixedname_t fn; dns_name_t *name = dns_fixedname_name(&fn); - create_tables(); - /* * dns_nametree_find() requires exact name match. It matches node * that has a null key, too. @@ -335,18 +319,16 @@ ISC_RUN_TEST_IMPL(find) { assert_int_equal(dns_nametree_find(booltree, name, &node), ISC_R_SUCCESS); dns_ntnode_detach(&node); - - destroy_tables(); } ISC_TEST_LIST_START -ISC_TEST_ENTRY(add_bool) -ISC_TEST_ENTRY(add_bits) -ISC_TEST_ENTRY(add_count) -ISC_TEST_ENTRY(covered_bool) -ISC_TEST_ENTRY(covered_bits) -ISC_TEST_ENTRY(delete) -ISC_TEST_ENTRY(find) +ISC_TEST_ENTRY_CUSTOM(add_bool, setup, teardown) +ISC_TEST_ENTRY_CUSTOM(add_bits, setup, teardown) +ISC_TEST_ENTRY_CUSTOM(add_count, setup, teardown) +ISC_TEST_ENTRY_CUSTOM(covered_bool, setup, teardown) +ISC_TEST_ENTRY_CUSTOM(covered_bits, setup, teardown) +ISC_TEST_ENTRY_CUSTOM(delete, setup, teardown) +ISC_TEST_ENTRY_CUSTOM(find, setup, teardown) ISC_TEST_LIST_END ISC_TEST_MAIN From 1a238a0f8627e5c28ae7f521f25068cd64b5dccb Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 17 Aug 2023 00:45:38 -0700 Subject: [PATCH 09/11] use a count nametree for synthfromdnssec use the count semantics for dns_nametree to support view->sfd. --- lib/dns/include/dns/view.h | 3 +- lib/dns/view.c | 58 +++++--------------------------------- 2 files changed, 8 insertions(+), 53 deletions(-) diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index 9bb9f3bf13..f8b1d943c7 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -143,9 +143,8 @@ struct dns_view { dns_nametree_t *answeracl_exclude; dns_nametree_t *denyanswernames; dns_nametree_t *answernames_exclude; + dns_nametree_t *sfd; dns_rrl_t *rrl; - dns_rbt_t *sfd; - isc_rwlock_t sfd_lock; bool provideixfr; bool requestnsid; bool sendcookie; diff --git a/lib/dns/view.c b/lib/dns/view.c index be043cee2f..7c486acebb 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -137,8 +137,6 @@ dns_view_create(isc_mem_t *mctx, dns_dispatchmgr_t *dispatchmgr, isc_mutex_init(&view->lock); - isc_rwlock_init(&view->sfd_lock); - dns_zt_create(mctx, view, &view->zonetable); dns_fwdtable_create(mctx, view, &view->fwdtable); @@ -196,7 +194,6 @@ cleanup_new_zone_lock: dns_fwdtable_destroy(&view->fwdtable); dns_zt_detach(&view->zonetable); - isc_rwlock_destroy(&view->sfd_lock); isc_mutex_destroy(&view->lock); if (view->nta_file != NULL) { @@ -358,7 +355,7 @@ destroy(dns_view_t *view) { dns_nametree_detach(&view->answernames_exclude); } if (view->sfd != NULL) { - dns_rbt_destroy(&view->sfd); + dns_nametree_detach(&view->sfd); } if (view->secroots_priv != NULL) { dns_keytable_detach(&view->secroots_priv); @@ -408,7 +405,6 @@ destroy(dns_view_t *view) { dns_badcache_destroy(&view->failcache); } isc_mutex_destroy(&view->new_zone_lock); - isc_rwlock_destroy(&view->sfd_lock); isc_mutex_destroy(&view->lock); isc_refcount_destroy(&view->references); isc_refcount_destroy(&view->weakrefs); @@ -2318,58 +2314,26 @@ dns_view_flushonshutdown(dns_view_t *view, bool flush) { view->flush = flush; } -static void -free_sfd(void *data, void *arg) { - isc_mem_put(arg, data, sizeof(unsigned int)); -} - void dns_view_sfd_add(dns_view_t *view, const dns_name_t *name) { isc_result_t result; - dns_rbtnode_t *node = NULL; REQUIRE(DNS_VIEW_VALID(view)); - RWLOCK(&view->sfd_lock, isc_rwlocktype_write); if (view->sfd == NULL) { - result = dns_rbt_create(view->mctx, free_sfd, view->mctx, - &view->sfd); - RUNTIME_CHECK(result == ISC_R_SUCCESS); + dns_nametree_create(view->mctx, DNS_NAMETREE_COUNT, "sfd", + &view->sfd); } - result = dns_rbt_addnode(view->sfd, name, &node); - RUNTIME_CHECK(result == ISC_R_SUCCESS || result == ISC_R_EXISTS); - if (node->data != NULL) { - unsigned int *count = node->data; - (*count)++; - } else { - unsigned int *count = isc_mem_get(view->mctx, - sizeof(unsigned int)); - *count = 1; - node->data = count; - } - RWUNLOCK(&view->sfd_lock, isc_rwlocktype_write); + result = dns_nametree_add(view->sfd, name, 0); + RUNTIME_CHECK(result == ISC_R_SUCCESS); } void dns_view_sfd_del(dns_view_t *view, const dns_name_t *name) { - isc_result_t result; - void *data = NULL; - REQUIRE(DNS_VIEW_VALID(view)); - RWLOCK(&view->sfd_lock, isc_rwlocktype_write); - INSIST(view->sfd != NULL); - result = dns_rbt_findname(view->sfd, name, 0, NULL, &data); - if (result == ISC_R_SUCCESS) { - unsigned int *count = data; - INSIST(count != NULL); - if (--(*count) == 0U) { - result = dns_rbt_deletename(view->sfd, name, false); - RUNTIME_CHECK(result == ISC_R_SUCCESS); - } - } - RWUNLOCK(&view->sfd_lock, isc_rwlocktype_write); + dns_nametree_delete(view->sfd, name); } void @@ -2378,17 +2342,9 @@ dns_view_sfd_find(dns_view_t *view, const dns_name_t *name, REQUIRE(DNS_VIEW_VALID(view)); if (view->sfd != NULL) { - isc_result_t result; - void *data = NULL; - - RWLOCK(&view->sfd_lock, isc_rwlocktype_read); - result = dns_rbt_findname(view->sfd, name, 0, foundname, &data); - RWUNLOCK(&view->sfd_lock, isc_rwlocktype_read); - if (result != ISC_R_SUCCESS && result != DNS_R_PARTIALMATCH) { + if (!dns_nametree_covered(view->sfd, name, foundname, 0)) { dns_name_copy(dns_rootname, foundname); } - } else { - dns_name_copy(dns_rootname, foundname); } } From 1019c0c0b195a4115d835255560ecd80e87ba1c1 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 24 Aug 2023 13:47:19 -0700 Subject: [PATCH 10/11] unconditionally create view and resolver nametrees instead of allowing a NULL nametree in dns_nametree_covered(), require nametree to exist, and ensure that the nametrees defined for view and resolver objects are always created. --- bin/named/server.c | 8 ++--- lib/dns/include/dns/nametree.h | 3 +- lib/dns/include/dns/resolver.h | 15 --------- lib/dns/nametree.c | 6 +--- lib/dns/resolver.c | 59 +++++++--------------------------- lib/dns/view.c | 13 +++----- tests/dns/nametree_test.c | 4 --- 7 files changed, 20 insertions(+), 88 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 03483fd2b8..f751b840eb 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -618,6 +618,8 @@ configure_view_nametable(const cfg_obj_t *vconfig, const cfg_obj_t *config, if (*ntp != NULL) { dns_nametree_detach(ntp); } + dns_nametree_create(mctx, DNS_NAMETREE_BOOL, confname, ntp); + if (vconfig != NULL) { maps[i++] = cfg_tuple_get(vconfig, "options"); } @@ -645,8 +647,6 @@ configure_view_nametable(const cfg_obj_t *vconfig, const cfg_obj_t *config, } } - dns_nametree_create(mctx, DNS_NAMETREE_BOOL, confname, ntp); - name = dns_fixedname_initname(&fixed); for (element = cfg_list_first(obj); element != NULL; element = cfg_list_next(element)) @@ -4906,7 +4906,6 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, /* * Set supported DNSSEC algorithms. */ - dns_resolver_reset_algorithms(view->resolver); disabled = NULL; (void)named_config_get(maps, "disable-algorithms", &disabled); if (disabled != NULL) { @@ -4921,7 +4920,6 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, /* * Set supported DS digest types. */ - dns_resolver_reset_ds_digests(view->resolver); disabled = NULL; (void)named_config_get(maps, "disable-ds-digests", &disabled); if (disabled != NULL) { @@ -5521,7 +5519,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, */ CHECK(configure_view_dnsseckeys(view, vconfig, config, bindkeys, auto_root)); - dns_resolver_resetmustbesecure(view->resolver); + obj = NULL; result = named_config_get(maps, "dnssec-must-be-secure", &obj); if (result == ISC_R_SUCCESS) { diff --git a/lib/dns/include/dns/nametree.h b/lib/dns/include/dns/nametree.h index 3c5ac51893..228a62b9c5 100644 --- a/lib/dns/include/dns/nametree.h +++ b/lib/dns/include/dns/nametree.h @@ -159,8 +159,7 @@ dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name, * In DNS_NAMETREE_BITS trees, this returns true if 'name' has a match or * a closest ancestor in 'nametree' with the 'bit' set in its bitfield. * - * If a name is not found, or if 'nametree' is NULL, the default return - * value is false. + * If a name is not found, the default return value is false. * * If 'found' is not NULL, the name or ancestor name that was found in * the tree is copied into it. diff --git a/lib/dns/include/dns/resolver.h b/lib/dns/include/dns/resolver.h index aeee2fa620..0c6af9c816 100644 --- a/lib/dns/include/dns/resolver.h +++ b/lib/dns/include/dns/resolver.h @@ -423,18 +423,6 @@ dns_resolver_addalternate(dns_resolver_t *resolver, const isc_sockaddr_t *alt, * \li only one of 'name' or 'alt' to be valid. */ -void -dns_resolver_reset_algorithms(dns_resolver_t *resolver); -/*%< - * Clear the disabled DNSSEC algorithms. - */ - -void -dns_resolver_reset_ds_digests(dns_resolver_t *resolver); -/*%< - * Clear the disabled DS digest types. - */ - isc_result_t dns_resolver_disable_algorithm(dns_resolver_t *resolver, const dns_name_t *name, unsigned int alg); @@ -482,9 +470,6 @@ dns_resolver_ds_digest_supported(dns_resolver_t *resolver, * crypto libraries if it was not specifically disabled. */ -void -dns_resolver_resetmustbesecure(dns_resolver_t *resolver); - isc_result_t dns_resolver_setmustbesecure(dns_resolver_t *resolver, const dns_name_t *name, bool value); diff --git a/lib/dns/nametree.c b/lib/dns/nametree.c index af7cfbddf7..ec297ca6f2 100644 --- a/lib/dns/nametree.c +++ b/lib/dns/nametree.c @@ -288,11 +288,7 @@ dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name, dns_ntnode_t *node = NULL; bool ret = false; - REQUIRE(nametree == NULL || VALID_NAMETREE(nametree)); - - if (nametree == NULL) { - return (false); - } + REQUIRE(VALID_NAMETREE(nametree)); dns_qpmulti_query(nametree->table, &qpr); result = dns_qp_findname_ancestor(&qpr, name, 0, (void **)&node, NULL); diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 02a578c261..fb1e401e78 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -9809,13 +9809,12 @@ dns_resolver__destroy(dns_resolver_t *res) { REQUIRE(atomic_load_acquire(&res->nfctx) == 0); - /* These must be run before zeroing the magic number */ - dns_resolver_reset_algorithms(res); - dns_resolver_reset_ds_digests(res); - dns_resolver_resetmustbesecure(res); - res->magic = 0; + dns_nametree_detach(&res->algorithms); + dns_nametree_detach(&res->digests); + dns_nametree_detach(&res->mustbesecure); + if (res->querystats != NULL) { dns_stats_detach(&res->querystats); } @@ -9955,6 +9954,13 @@ dns_resolver_create(dns_view_t *view, isc_loopmgr_t *loopmgr, isc_mutex_init(&res->lock); isc_mutex_init(&res->primelock); + dns_nametree_create(res->mctx, DNS_NAMETREE_BITS, "algorithms", + &res->algorithms); + dns_nametree_create(res->mctx, DNS_NAMETREE_BITS, "ds-digests", + &res->digests); + dns_nametree_create(res->mctx, DNS_NAMETREE_BOOL, + "dnssec-must-be-secure", &res->mustbesecure); + res->magic = RES_MAGIC; *resp = res; @@ -10717,24 +10723,6 @@ dns_resolver_printbadcache(dns_resolver_t *resolver, FILE *fp) { (void)dns_badcache_print(resolver->badcache, "Bad cache", fp); } -void -dns_resolver_reset_algorithms(dns_resolver_t *resolver) { - REQUIRE(VALID_RESOLVER(resolver)); - - if (resolver->algorithms != NULL) { - dns_nametree_detach(&resolver->algorithms); - } -} - -void -dns_resolver_reset_ds_digests(dns_resolver_t *resolver) { - REQUIRE(VALID_RESOLVER(resolver)); - - if (resolver->digests != NULL) { - dns_nametree_detach(&resolver->digests); - } -} - isc_result_t dns_resolver_disable_algorithm(dns_resolver_t *resolver, const dns_name_t *name, unsigned int alg) { @@ -10744,11 +10732,6 @@ dns_resolver_disable_algorithm(dns_resolver_t *resolver, const dns_name_t *name, return (ISC_R_RANGE); } - if (resolver->algorithms == NULL) { - dns_nametree_create(resolver->mctx, DNS_NAMETREE_BITS, - "algorithms", &resolver->algorithms); - } - return (dns_nametree_add(resolver->algorithms, name, alg)); } @@ -10761,11 +10744,6 @@ dns_resolver_disable_ds_digest(dns_resolver_t *resolver, const dns_name_t *name, return (ISC_R_RANGE); } - if (resolver->digests == NULL) { - dns_nametree_create(resolver->mctx, DNS_NAMETREE_BITS, - "ds-digests", &resolver->digests); - } - return (dns_nametree_add(resolver->digests, name, digest_type)); } @@ -10798,15 +10776,6 @@ dns_resolver_ds_digest_supported(dns_resolver_t *resolver, return (dst_ds_digest_supported(digest_type)); } -void -dns_resolver_resetmustbesecure(dns_resolver_t *resolver) { - REQUIRE(VALID_RESOLVER(resolver)); - - if (resolver->mustbesecure != NULL) { - dns_nametree_detach(&resolver->mustbesecure); - } -} - isc_result_t dns_resolver_setmustbesecure(dns_resolver_t *resolver, const dns_name_t *name, bool value) { @@ -10814,12 +10783,6 @@ dns_resolver_setmustbesecure(dns_resolver_t *resolver, const dns_name_t *name, REQUIRE(VALID_RESOLVER(resolver)); - if (resolver->mustbesecure == NULL) { - dns_nametree_create(resolver->mctx, DNS_NAMETREE_BOOL, - "dnssec-must-be-secure", - &resolver->mustbesecure); - } - result = dns_nametree_add(resolver->mustbesecure, name, value); return (result); } diff --git a/lib/dns/view.c b/lib/dns/view.c index 7c486acebb..6855d02eda 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -162,6 +162,8 @@ dns_view_create(isc_mem_t *mctx, dns_dispatchmgr_t *dispatchmgr, goto cleanup_peerlist; } + dns_nametree_create(view->mctx, DNS_NAMETREE_COUNT, "sfd", &view->sfd); + view->magic = DNS_VIEW_MAGIC; *viewp = view; @@ -2320,11 +2322,6 @@ dns_view_sfd_add(dns_view_t *view, const dns_name_t *name) { REQUIRE(DNS_VIEW_VALID(view)); - if (view->sfd == NULL) { - dns_nametree_create(view->mctx, DNS_NAMETREE_COUNT, "sfd", - &view->sfd); - } - result = dns_nametree_add(view->sfd, name, 0); RUNTIME_CHECK(result == ISC_R_SUCCESS); } @@ -2341,10 +2338,8 @@ dns_view_sfd_find(dns_view_t *view, const dns_name_t *name, dns_name_t *foundname) { REQUIRE(DNS_VIEW_VALID(view)); - if (view->sfd != NULL) { - if (!dns_nametree_covered(view->sfd, name, foundname, 0)) { - dns_name_copy(dns_rootname, foundname); - } + if (!dns_nametree_covered(view->sfd, name, foundname, 0)) { + dns_name_copy(dns_rootname, foundname); } } diff --git a/tests/dns/nametree_test.c b/tests/dns/nametree_test.c index 39ac4467c1..147f37382b 100644 --- a/tests/dns/nametree_test.c +++ b/tests/dns/nametree_test.c @@ -206,10 +206,6 @@ ISC_RUN_TEST_IMPL(covered_bool) { assert_false(dns_nametree_covered(booltree, name, NULL, 0)); } - /* If the nametree is NULL, dns_nametree_covered() returns false. */ - dns_test_namefromstring("anyname.example.", &fn); - assert_false(dns_nametree_covered(NULL, name, NULL, 0)); - /* Check that the found name is as expected */ dns_test_namefromstring("other.example.com.", &fn); assert_true(dns_nametree_covered(booltree, name, found, 0)); From 9c25a09e5dbdaf261bba29d88685a241317760e2 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 17 Aug 2023 09:37:23 -0700 Subject: [PATCH 11/11] CHANGES for [GL !8213] --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index ecde19a3c2..6af9ebf5a1 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6238. [cleanup] Refactor several objects relying on dns_rbt trees + to instead of dns_nametree, a wrapper around dns_qp. + [GL !8213] + 6237. [bug] Address memory leaks due to not clearing OpenSSL error stack. [GL #4159]