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