From 4e3e61806ca642714311408c446a618d5879652d Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Sat, 23 Sep 2023 15:58:46 -0700 Subject: [PATCH] get predecessor name in dns_qp_findname_ancestor() dns_qp_findname_ancestor() now takes an optional 'predecessor' parameter, which if non-NULL is updated to contain the DNSSEC predecessor of the name searched for. this is done by constructing an iterator stack while carrying out the search, so it can be used to step backward if needed. --- lib/dns/forward.c | 3 +- lib/dns/include/dns/qp.h | 7 ++- lib/dns/keytable.c | 6 +- lib/dns/nametree.c | 2 +- lib/dns/nta.c | 3 +- lib/dns/qp.c | 111 +++++++++++++++++++++++++++------ lib/dns/zt.c | 2 +- tests/dns/qp_test.c | 128 ++++++++++++++++++++++++++++++++++----- 8 files changed, 223 insertions(+), 39 deletions(-) diff --git a/lib/dns/forward.c b/lib/dns/forward.c index fc21a1a4ce..0be252f3ac 100644 --- a/lib/dns/forward.c +++ b/lib/dns/forward.c @@ -168,7 +168,8 @@ dns_fwdtable_find(dns_fwdtable_t *fwdtable, const dns_name_t *name, REQUIRE(VALID_FWDTABLE(fwdtable)); dns_qpmulti_query(fwdtable->table, &qpr); - result = dns_qp_findname_ancestor(&qpr, name, NULL, NULL, &pval, NULL); + result = dns_qp_findname_ancestor(&qpr, name, NULL, NULL, NULL, &pval, + NULL); if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { dns_forwarders_t *fwdrs = pval; *forwardersp = fwdrs; diff --git a/lib/dns/include/dns/qp.h b/lib/dns/include/dns/qp.h index 51c46cf964..3e057ceccd 100644 --- a/lib/dns/include/dns/qp.h +++ b/lib/dns/include/dns/qp.h @@ -488,14 +488,17 @@ dns_qp_getname(dns_qpreadable_t qpr, const dns_name_t *name, void **pval_r, isc_result_t dns_qp_findname_ancestor(dns_qpreadable_t qpr, const dns_name_t *name, - dns_name_t *foundname, dns_qpchain_t *chain, - void **pval_r, uint32_t *ival_r); + dns_name_t *foundname, dns_name_t *predecessor, + dns_qpchain_t *chain, void **pval_r, uint32_t *ival_r); /*%< * Find a leaf in a qp-trie that is an ancestor domain of, or equal to, the * given DNS name. * * If 'foundname' is not NULL, it is updated to contain the name found. * + * If 'predecessor' is not NULL, it is updated to contain the DNSSEC + * predecessor of the searched-for name. + * * If 'chain' is not NULL, it is updated to contain a QP chain with * references to the populated nodes in the tree between the root and * the name found. diff --git a/lib/dns/keytable.c b/lib/dns/keytable.c index 0b805fd9c4..e8ce21fa19 100644 --- a/lib/dns/keytable.c +++ b/lib/dns/keytable.c @@ -518,7 +518,8 @@ dns_keytable_finddeepestmatch(dns_keytable_t *keytable, const dns_name_t *name, REQUIRE(foundname != NULL); dns_qpmulti_query(keytable->table, &qpr); - result = dns_qp_findname_ancestor(&qpr, name, NULL, NULL, &pval, NULL); + result = dns_qp_findname_ancestor(&qpr, name, NULL, NULL, NULL, &pval, + NULL); keynode = pval; if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { @@ -547,7 +548,8 @@ dns_keytable_issecuredomain(dns_keytable_t *keytable, const dns_name_t *name, REQUIRE(wantdnssecp != NULL); dns_qpmulti_query(keytable->table, &qpr); - result = dns_qp_findname_ancestor(&qpr, name, NULL, NULL, &pval, NULL); + result = dns_qp_findname_ancestor(&qpr, name, NULL, NULL, NULL, &pval, + NULL); if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { keynode = pval; if (foundname != NULL) { diff --git a/lib/dns/nametree.c b/lib/dns/nametree.c index 661253f3aa..60fa9a97f0 100644 --- a/lib/dns/nametree.c +++ b/lib/dns/nametree.c @@ -289,7 +289,7 @@ dns_nametree_covered(dns_nametree_t *nametree, const dns_name_t *name, REQUIRE(VALID_NAMETREE(nametree)); dns_qpmulti_query(nametree->table, &qpr); - result = dns_qp_findname_ancestor(&qpr, name, found, NULL, + result = dns_qp_findname_ancestor(&qpr, name, found, NULL, NULL, (void **)&node, NULL); if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { switch (nametree->type) { diff --git a/lib/dns/nta.c b/lib/dns/nta.c index 4223ec27f5..c86b0ebf7b 100644 --- a/lib/dns/nta.c +++ b/lib/dns/nta.c @@ -413,7 +413,8 @@ dns_ntatable_covered(dns_ntatable_t *ntatable, isc_stdtime_t now, RWLOCK(&ntatable->rwlock, isc_rwlocktype_read); dns_qpmulti_query(ntatable->table, &qpr); - result = dns_qp_findname_ancestor(&qpr, name, NULL, NULL, &pval, NULL); + result = dns_qp_findname_ancestor(&qpr, name, NULL, NULL, NULL, &pval, + NULL); nta = pval; switch (result) { diff --git a/lib/dns/qp.c b/lib/dns/qp.c index 3f25e8bae6..299272cf6c 100644 --- a/lib/dns/qp.c +++ b/lib/dns/qp.c @@ -1771,6 +1771,7 @@ maybe_set_name(dns_qpreader_t *qp, qp_node_t *node, dns_name_t *name) { return; } + dns_name_reset(name); len = leaf_qpkey(qp, node, key); dns_qpkey_toname(key, len, name); } @@ -1996,22 +1997,41 @@ dns_qp_getname(dns_qpreadable_t qpr, const dns_name_t *name, void **pval_r, static inline void add_link(dns_qpchain_t *chain, qp_node_t *node, size_t offset) { + /* + * prevent duplication, which could occur if we're adding a link + * for the predecessor node and it's the same as the parent. + */ + if (chain->len > 0 && node == chain->chain[chain->len - 1].node) { + return; + } chain->chain[chain->len].node = node; chain->chain[chain->len].offset = offset; chain->len++; INSIST(chain->len <= DNS_NAME_MAXLABELS); } +static inline void +prevleaf(dns_qpiter_t *it) { + isc_result_t result = dns_qpiter_prev(it, NULL, NULL, NULL); + if (result == ISC_R_NOMORE) { + result = dns_qpiter_prev(it, NULL, NULL, NULL); + } + RUNTIME_CHECK(result == ISC_R_SUCCESS); +} + isc_result_t dns_qp_findname_ancestor(dns_qpreadable_t qpr, const dns_name_t *name, - dns_name_t *foundname, dns_qpchain_t *chain, - void **pval_r, uint32_t *ival_r) { + dns_name_t *foundname, dns_name_t *predecessor, + dns_qpchain_t *chain, void **pval_r, + uint32_t *ival_r) { dns_qpreader_t *qp = dns_qpreader(qpr); dns_qpkey_t search, found; size_t searchlen, foundlen; size_t offset; qp_node_t *n = NULL; dns_qpchain_t oc; + dns_qpiter_t it; + bool matched = true; REQUIRE(QP_VALID(qp)); REQUIRE(foundname == NULL || ISC_MAGIC_VALID(name, DNS_NAME_MAGIC)); @@ -2022,11 +2042,13 @@ dns_qp_findname_ancestor(dns_qpreadable_t qpr, const dns_name_t *name, chain = &oc; } dns_qpchain_init(qp, chain); + dns_qpiter_init(qp, &it); n = get_root(qp); if (n == NULL) { return (ISC_R_NOTFOUND); } + it.stack[0] = n; /* * Like `dns_qp_insert()`, we must find a leaf. However, we don't make a @@ -2062,33 +2084,88 @@ dns_qp_findname_ancestor(dns_qpreadable_t qpr, const dns_name_t *name, { add_link(chain, twigs, offset); } - if (branch_has_twig(n, bit)) { - /* we have the twig we wanted */ - n = branch_twig_ptr(qp, n, bit); - } else if (chain->len == 0) { + + matched = branch_has_twig(n, bit); + if (matched) { /* - * the twig we're looking for isn't here, - * and we haven't found an ancestor yet either. - * but we need to end the loop at a leaf, so - * continue down from whatever this branch's - * first twig is. + * found a match: if it's a branch, we keep + * searching, and if it's a leaf, we drop out of + * the loop. */ - n = twigs; + n = branch_twig_ptr(qp, n, bit); + } else if (predecessor != NULL) { + /* + * this branch is a dead end, but the caller wants + * the predecessor to the name we were searching + * for, so let's go find that. + */ + qp_weight_t pos = branch_twig_pos(n, bit); + if (pos == 0) { + /* + * this entire branch is greater than + * the key we wanted, so we step back to + * the predecessor using the iterator. + */ + prevleaf(&it); + n = it.stack[it.sp]; + } else { + /* + * the name we want would've been between + * two twigs in this branch. point n to the + * lesser of those, then walk down to the + * highest leaf in that subtree to get the + * predecessor. + */ + n = twigs + pos - 1; + while (is_branch(n)) { + prefetch_twigs(qp, n); + it.stack[++it.sp] = n; + pos = branch_twigs_size(n) - 1; + n = ref_ptr(qp, + branch_twigs_ref(n) + pos); + } + } } else { /* - * this branch is a dead end, but we do have - * a previous leaf node saved in the chain. - * we go back to that. it's a leaf, so we'll - * fall out of the loop here. + * this branch is a dead end, and the predecessor + * doesn't matter. now we just need to find a leaf + * to end on so qpkey_leaf() will work below. */ - n = chain->chain[chain->len - 1].node; + if (chain->len > 0) { + /* we saved an ancestor leaf: use that */ + n = chain->chain[chain->len - 1].node; + } else { + /* walk down to find the leftmost leaf */ + while (is_branch(twigs)) { + twigs = branch_twigs(qp, twigs); + } + n = twigs; + } } + + it.stack[++it.sp] = n; } /* do the keys differ, and if so, where? */ foundlen = leaf_qpkey(qp, n, found); offset = qpkey_compare(search, searchlen, found, foundlen); + /* + * if we've been asked to return the predecessor name, we + * work that out here. + * + * if 'matched' is true, the search ended at a leaf. it's either + * an exact match or the immediate successor of the searched-for + * name, and in either case, we can use the qpiter stack we've + * constructed to step back to the predecessor. if 'matched' is + * false, then the search failed at a branch node, and we would + * have already found the predecessor. + */ + if (predecessor != NULL && matched) { + prevleaf(&it); + } + maybe_set_name(qp, it.stack[it.sp], predecessor); + if (offset == QPKEY_EQUAL || offset == foundlen) { SET_IF_NOT_NULL(pval_r, leaf_pval(n)); SET_IF_NOT_NULL(ival_r, leaf_ival(n)); diff --git a/lib/dns/zt.c b/lib/dns/zt.c index 7d317506fb..147e6fc830 100644 --- a/lib/dns/zt.c +++ b/lib/dns/zt.c @@ -178,7 +178,7 @@ dns_zt_find(dns_zt_t *zt, const dns_name_t *name, dns_ztfind_t options, dns_qpmulti_query(zt->multi, &qpr); - result = dns_qp_findname_ancestor(&qpr, name, NULL, &chain, &pval, + result = dns_qp_findname_ancestor(&qpr, name, NULL, NULL, &chain, &pval, NULL); if (exactopts == DNS_ZTFIND_EXACT && result == DNS_R_PARTIALMATCH) { result = ISC_R_NOTFOUND; diff --git a/tests/dns/qp_test.c b/tests/dns/qp_test.c index 782383d1a3..6c83f8f365 100644 --- a/tests/dns/qp_test.c +++ b/tests/dns/qp_test.c @@ -350,7 +350,7 @@ check_partialmatch(dns_qp_t *qp, struct check_partialmatch check[]) { dns_test_namefromstring(check[i].query, &fn1); result = dns_qp_findname_ancestor(qp, name, foundname, NULL, - &pval, NULL); + NULL, &pval, NULL); #if 0 fprintf(stderr, "%s %s (expected %s) " @@ -437,7 +437,7 @@ ISC_RUN_TEST_IMPL(partialmatch) { { "webby.foo.bar.", DNS_R_PARTIALMATCH, "foo.bar." }, { "my.web.foo.bar.", DNS_R_PARTIALMATCH, "web.foo.bar." }, { "my.other.foo.bar.", DNS_R_PARTIALMATCH, "foo.bar." }, - { NULL }, + { NULL, 0, NULL }, }; check_partialmatch(qp, check1); @@ -450,7 +450,7 @@ ISC_RUN_TEST_IMPL(partialmatch) { { "bar.", DNS_R_PARTIALMATCH, "." }, { "foo.bar.", ISC_R_SUCCESS, "foo.bar." }, { "bar", ISC_R_NOTFOUND, NULL }, - { NULL }, + { NULL, 0, NULL }, }; check_partialmatch(qp, check2); @@ -464,7 +464,7 @@ ISC_RUN_TEST_IMPL(partialmatch) { check_partialmatch(qp, (struct check_partialmatch[]){ { "bar", ISC_R_NOTFOUND, NULL }, { "bar.", ISC_R_NOTFOUND, NULL }, - { NULL }, + { NULL, 0, NULL }, }); /* what if there's a root node with an empty key? */ @@ -473,7 +473,7 @@ ISC_RUN_TEST_IMPL(partialmatch) { check_partialmatch(qp, (struct check_partialmatch[]){ { "bar", DNS_R_PARTIALMATCH, "" }, { "bar.", DNS_R_PARTIALMATCH, "" }, - { NULL }, + { NULL, 0, NULL }, }); dns_qp_destroy(&qp); @@ -496,8 +496,8 @@ check_qpchain(dns_qp_t *qp, struct check_qpchain check[]) { dns_qpchain_init(qp, &chain); dns_test_namefromstring(check[i].query, &fn1); - result = dns_qp_findname_ancestor(qp, name, NULL, &chain, NULL, - NULL); + result = dns_qp_findname_ancestor(qp, name, NULL, NULL, &chain, + NULL, NULL); #if 0 fprintf(stderr, "%s %s (expected %s), " @@ -528,17 +528,14 @@ check_qpchain(dns_qp_t *qp, struct check_qpchain check[]) { ISC_RUN_TEST_IMPL(qpchain) { dns_qp_t *qp = NULL; + const char insert[][16] = { ".", "a.", "b.", + "c.b.a.", "e.d.c.b.a.", "c.b.b.", + "c.d.", "a.b.c.d.", "a.b.c.d.e.", + "b.a.", "x.k.c.d.", "" }; int i = 0; dns_qp_create(mctx, &string_methods, NULL, &qp); - /* - * Fixed size strings [16] should ensure leaf-compatible alignment. - */ - const char insert[][16] = { ".", "a.", "b.", "c.b.a.", - "e.d.c.b.a.", "c.b.b.", "c.d.", "a.b.c.d.", - "a.b.c.d.e.", "b.a.", "" }; - while (insert[i][0] != '\0') { insert_str(qp, insert[i++]); } @@ -560,12 +557,115 @@ ISC_RUN_TEST_IMPL(qpchain) { dns_qp_destroy(&qp); } +struct check_predecessors { + const char *query; + const char *predecessor; + isc_result_t result; +}; + +static void +check_predecessors(dns_qp_t *qp, struct check_predecessors check[]) { + isc_result_t result; + dns_fixedname_t fn1, fn2; + dns_name_t *name = dns_fixedname_initname(&fn1); + dns_name_t *pred = dns_fixedname_initname(&fn2); + + for (int i = 0; check[i].query != NULL; i++) { + char *predname = NULL; + + dns_test_namefromstring(check[i].query, &fn1); + result = dns_qp_findname_ancestor(qp, name, NULL, pred, NULL, + NULL, NULL); +#if 0 + fprintf(stderr, "%s: expected %s got %s\n", check[i].query, + isc_result_totext(check[i].result), + isc_result_totext(result)); +#endif + assert_int_equal(result, check[i].result); + result = dns_name_tostring(pred, &predname, mctx); +#if 0 + fprintf(stderr, "... expected predecessor %s got %s\n", + check[i].predecessor, predname); +#endif + assert_int_equal(result, ISC_R_SUCCESS); + + assert_string_equal(predname, check[i].predecessor); + isc_mem_free(mctx, predname); + } +} + +ISC_RUN_TEST_IMPL(predecessors) { + dns_qp_t *qp = NULL; + const char insert[][16] = { + "a.", "b.", "c.b.a.", "e.d.c.b.a.", + "c.b.b.", "c.d.", "a.b.c.d.", "a.b.c.d.e.", + "b.a.", "x.k.c.d.", "" + }; + int i = 0; + + dns_qp_create(mctx, &string_methods, NULL, &qp); + while (insert[i][0] != '\0') { + insert_str(qp, insert[i++]); + } + + /* first check: no root label in the database */ + static struct check_predecessors check1[] = { + { ".", "a.b.c.d.e.", ISC_R_NOTFOUND }, + { "a.", "a.b.c.d.e.", ISC_R_SUCCESS }, + { "b.a.", "a.", ISC_R_SUCCESS }, + { "b.", "e.d.c.b.a.", ISC_R_SUCCESS }, + { "aaa.a.", "a.", DNS_R_PARTIALMATCH }, + { "ddd.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH }, + { "d.c.", "c.b.b.", ISC_R_NOTFOUND }, + { "1.2.c.b.a.", "c.b.a.", DNS_R_PARTIALMATCH }, + { "a.b.c.e.f.", "a.b.c.d.e.", ISC_R_NOTFOUND }, + { "z.y.x.", "a.b.c.d.e.", ISC_R_NOTFOUND }, + { "w.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH }, + { "z.z.z.z.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH }, + { "w.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH }, + { "d.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH }, + { "0.b.c.d.e.", "x.k.c.d.", ISC_R_NOTFOUND }, + { "b.d.", "c.b.b.", ISC_R_NOTFOUND }, + { NULL, NULL, 0 } + }; + + check_predecessors(qp, check1); + + /* second check: add a root label and try again */ + const char root[16] = "."; + insert_str(qp, root); + + static struct check_predecessors check2[] = { + { ".", "a.b.c.d.e.", ISC_R_SUCCESS }, + { "a.", ".", ISC_R_SUCCESS }, + { "b.a.", "a.", ISC_R_SUCCESS }, + { "b.", "e.d.c.b.a.", ISC_R_SUCCESS }, + { "aaa.a.", "a.", DNS_R_PARTIALMATCH }, + { "ddd.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH }, + { "d.c.", "c.b.b.", DNS_R_PARTIALMATCH }, + { "1.2.c.b.a.", "c.b.a.", DNS_R_PARTIALMATCH }, + { "a.b.c.e.f.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, + { "z.y.x.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, + { "w.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH }, + { "z.z.z.z.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH }, + { "w.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH }, + { "d.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH }, + { "0.b.c.d.e.", "x.k.c.d.", DNS_R_PARTIALMATCH }, + { NULL, NULL, 0 } + }; + + check_predecessors(qp, check2); + + dns_qp_destroy(&qp); +} + ISC_TEST_LIST_START ISC_TEST_ENTRY(qpkey_name) ISC_TEST_ENTRY(qpkey_sort) ISC_TEST_ENTRY(qpiter) ISC_TEST_ENTRY(partialmatch) ISC_TEST_ENTRY(qpchain) +ISC_TEST_ENTRY(predecessors) ISC_TEST_LIST_END ISC_TEST_MAIN