From: Libor Peltan Date: Mon, 30 Mar 2020 16:47:06 +0000 (+0200) Subject: nsec3: bugfix: empty-non-terminal above unchanged deleg has NSEC3: X-Git-Tag: embedded_lmdb~35^2~103^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bbc22036c8c65728e61c6ea418e5a12e044228e8;p=thirdparty%2Fknot-dns.git nsec3: bugfix: empty-non-terminal above unchanged deleg has NSEC3: this happens when an incremental update removes a record from a node, which becomes empty-non-terminal, and there is a delegation below the node. In this situation, NSEC3 record for this empty-non-terminal was created, which is not against RFCs, but inconsistent with general Knot behaviour. The fix always iterates to a subtree of empty-non-terminal with mark_empty on incremental update. --- diff --git a/src/knot/dnssec/nsec3-chain.c b/src/knot/dnssec/nsec3-chain.c index 8b1bcc696d..f5bd7aa61c 100644 --- a/src/knot/dnssec/nsec3-chain.c +++ b/src/knot/dnssec/nsec3-chain.c @@ -578,6 +578,30 @@ static bool nsec3_is_empty(zone_node_t *node, bool opt_out) nsec3_opt_out(node, opt_out)); } +typedef struct { + bool opt_out; // NSEC3 opt-out enabled + bool mark_ent_subtrees; // also recurse to subtrees of empty-non-terminals + zone_contents_t *contents; // current zone contents + const knot_dname_t *subtree_already; // private: already iterating over this subtree +} mark_empty_ctx_t; + +static bool mark_empty_recurse(mark_empty_ctx_t *ctx, zone_node_t *node) +{ + return ctx->mark_ent_subtrees && ctx->subtree_already == NULL && + node->children > 0 && knot_nsec_empty_nsec_and_rrsigs_in_node(node); // empty non-terminal +} + +static int do_empty_recurse(mark_empty_ctx_t *ctx, zone_node_t *node, zone_tree_apply_cb_t cb) +{ + if (mark_empty_recurse(ctx, node)) { + ctx->subtree_already = node->owner; + int ret = zone_tree_sub_apply(ctx->contents->nodes, node->owner, cb, ctx); + ctx->subtree_already = NULL; + return ret; + } + return KNOT_EOK; +} + /*! * \brief Marks node and its parents as empty if NSEC3 should not be generated * for them. @@ -587,11 +611,13 @@ static bool nsec3_is_empty(zone_node_t *node, bool opt_out) */ static int nsec3_mark_empty(zone_node_t *node, void *data) { - if (node->flags & NODE_FLAGS_DELETED) { + if ((node->flags & NODE_FLAGS_DELETED) || (node->flags & NODE_FLAGS_EMPTY)) { return KNOT_EOK; } - if (!(node->flags & NODE_FLAGS_EMPTY) && nsec3_is_empty(node, (data != NULL))) { + mark_empty_ctx_t *ctx = data; + + if (nsec3_is_empty(node, ctx->opt_out)) { /*! * Mark this node and all parent nodes that meet the same * criteria as empty. @@ -607,6 +633,8 @@ static int nsec3_mark_empty(zone_node_t *node, void *data) /* Recurse using the parent node */ return nsec3_mark_empty(node_parent(node), data); } + } else { + return do_empty_recurse(ctx, node, nsec3_mark_empty); } return KNOT_EOK; @@ -621,7 +649,6 @@ static int nsec3_mark_empty(zone_node_t *node, void *data) */ static int nsec3_reset(zone_node_t *node, void *data) { - UNUSED(data); if (node->flags & NODE_FLAGS_EMPTY) { /* If node was marked as empty, increase its parent's children * count. @@ -629,6 +656,8 @@ static int nsec3_reset(zone_node_t *node, void *data) node_parent(node)->children++; /* Clear the 'empty' flag. */ node->flags &= ~NODE_FLAGS_EMPTY; + } else { + return do_empty_recurse(data, node, nsec3_reset); } return KNOT_EOK; @@ -717,8 +746,8 @@ int knot_nsec3_create_chain(const zone_contents_t *zone, * The flag will be removed when the node is encountered during NSEC3 * creation procedure. */ - int result = zone_tree_apply(zone->nodes, nsec3_mark_empty, - (opt_out ? (void *)zone : NULL)); + mark_empty_ctx_t mctx = { opt_out, false, NULL }; + int result = zone_tree_apply(zone->nodes, nsec3_mark_empty, &mctx); if (result != KNOT_EOK) { free_nsec3_tree(nsec3_nodes); return result; @@ -735,7 +764,7 @@ int knot_nsec3_create_chain(const zone_contents_t *zone, * so that flags and children count are back to normal before further * processing. */ - result = zone_tree_apply(zone->nodes, nsec3_reset, NULL); + result = zone_tree_apply(zone->nodes, nsec3_reset, &mctx); if (result != KNOT_EOK) { free_nsec3_tree(nsec3_nodes); return result; @@ -773,8 +802,8 @@ int knot_nsec3_fix_chain(zone_update_t *update, return knot_nsec3_create_chain(update->new_cont, params, ttl, update); } - int ret = zone_tree_apply(update->a_ctx->node_ptrs, nsec3_mark_empty, - (opt_out ? (void *)update : NULL)); + mark_empty_ctx_t mctx = { opt_out, true, update->new_cont }; + int ret = zone_tree_apply(update->a_ctx->node_ptrs, nsec3_mark_empty, &mctx); if (ret != KNOT_EOK) { return ret; } @@ -784,7 +813,7 @@ int knot_nsec3_fix_chain(zone_update_t *update, return ret; } - ret = zone_tree_apply(update->a_ctx->node_ptrs, nsec3_reset, NULL); + ret = zone_tree_apply(update->a_ctx->node_ptrs, nsec3_reset, &mctx); if (ret != KNOT_EOK) { return ret; } diff --git a/src/knot/zone/zone-tree.c b/src/knot/zone/zone-tree.c index f05af96d78..1c828f6b1a 100644 --- a/src/knot/zone/zone-tree.c +++ b/src/knot/zone/zone-tree.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2019 CZ.NIC, z.s.p.o. +/* Copyright (C) 2020 CZ.NIC, z.s.p.o. This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -283,11 +283,42 @@ int zone_tree_apply(zone_tree_t *tree, zone_tree_apply_cb_t function, void *data return trie_apply(tree->trie, tree_apply_cb, &f); } +int zone_tree_sub_apply(zone_tree_t *tree, const knot_dname_t *sub_root, + zone_tree_apply_cb_t function, void *data) +{ + zone_tree_it_t it = { 0 }; + int ret = zone_tree_it_sub_begin(tree, sub_root, &it); + while (ret == KNOT_EOK && !zone_tree_it_finished(&it)) { + ret = function(zone_tree_it_val(&it), data); + zone_tree_it_next(&it); + } + zone_tree_it_free(&it); + return ret; +} + int zone_tree_it_begin(zone_tree_t *tree, zone_tree_it_t *it) { return zone_tree_it_double_begin(tree, NULL, it); } +int zone_tree_it_sub_begin(zone_tree_t *tree, const knot_dname_t *sub_root, + zone_tree_it_t *it) +{ + int ret = zone_tree_it_begin(tree, it); + if (ret != KNOT_EOK) { + return ret; + } + it->sub_root = knot_dname_copy(sub_root, NULL); + knot_dname_storage_t lf_storage; + uint8_t *lf = knot_dname_lf(sub_root, lf_storage); + ret = trie_it_get_leq(it->it, lf + 1, *lf); + if ((ret != KNOT_EOK && ret != KNOT_ENOENT) || it->sub_root == NULL) { + zone_tree_it_free(it); + return ret == KNOT_EOK ? KNOT_ENOMEM : ret; + } + return KNOT_EOK; +} + int zone_tree_it_double_begin(zone_tree_t *first, zone_tree_t *second, zone_tree_it_t *it) { if (it->tree == NULL) { @@ -302,9 +333,15 @@ int zone_tree_it_double_begin(zone_tree_t *first, zone_tree_t *second, zone_tree return KNOT_EOK; } +static bool sub_done(zone_tree_it_t *it) +{ + return it->sub_root != NULL && + knot_dname_in_bailiwick(zone_tree_it_val(it)->owner, it->sub_root) < 0; +} + bool zone_tree_it_finished(zone_tree_it_t *it) { - return it->it == NULL || it->tree == NULL || trie_it_finished(it->it); + return it->it == NULL || it->tree == NULL || trie_it_finished(it->it) || sub_done(it); } zone_node_t *zone_tree_it_val(zone_tree_it_t *it) @@ -328,12 +365,14 @@ void zone_tree_it_next(zone_tree_it_t *it) it->binode_second = ((it->tree->flags & ZONE_TREE_BINO_SECOND) ? 1 : 0); it->next_tree = NULL; it->it = trie_it_begin(it->tree->trie); + assert(it->sub_root == NULL); } } void zone_tree_it_free(zone_tree_it_t *it) { trie_it_free(it->it); + knot_dname_free(it->sub_root, NULL); memset(it, 0, sizeof(*it)); } diff --git a/src/knot/zone/zone-tree.h b/src/knot/zone/zone-tree.h index 89f66ecb18..4341522244 100644 --- a/src/knot/zone/zone-tree.h +++ b/src/knot/zone/zone-tree.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2019 CZ.NIC, z.s.p.o. +/* Copyright (C) 2020 CZ.NIC, z.s.p.o. This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -49,6 +49,7 @@ typedef struct { int binode_second; zone_tree_t *next_tree; + knot_dname_t *sub_root; } zone_tree_it_t; typedef struct { @@ -211,6 +212,19 @@ int zone_tree_del_node(zone_tree_t *tree, zone_node_t *node, bool free_deleted); */ int zone_tree_apply(zone_tree_t *tree, zone_tree_apply_cb_t function, void *data); +/*! + * \brief Applies given function to each node in a subtree. + * + * \param tree Zone tree. + * \param sub_root Name denoting the subtree. + * \param function Callback to be applied. + * \param data Callback context. + * + * \return KNOT_E* + */ +int zone_tree_sub_apply(zone_tree_t *tree, const knot_dname_t *sub_root, + zone_tree_apply_cb_t function, void *data); + /*! * \brief Start zone tree iteration. * @@ -221,6 +235,18 @@ int zone_tree_apply(zone_tree_t *tree, zone_tree_apply_cb_t function, void *data */ int zone_tree_it_begin(zone_tree_t *tree, zone_tree_it_t *it); +/*! + * \brief Start iteration over a subtree. + * + * \param tree Zone tree to iterate in. + * \param sub_root Iterate over node of this name and all children. + * \param it Out: iteration context, shall be zeroed before. + * + * \return KNOT_E* + */ +int zone_tree_it_sub_begin(zone_tree_t *tree, const knot_dname_t *sub_root, + zone_tree_it_t *it); + /*! * \brief Start iteration of two zone trees. * diff --git a/tests-extra/tests/dnssec/nsec_update/test.py b/tests-extra/tests/dnssec/nsec_update/test.py index e1184da54d..75a05b0b61 100644 --- a/tests-extra/tests/dnssec/nsec_update/test.py +++ b/tests-extra/tests/dnssec/nsec_update/test.py @@ -89,6 +89,18 @@ for zone in zones: up.add("ns.deleg390281", 3600, "A", "1.2.54.31") up.send("NOERROR") +# update master by making empty-non-terminal from non-empty-non-terminal +# above a delegation (create first) +for zone in zones: + up = master.update(zone) + up.add("ent", 3600, "A", "1.2.3.4") + up.add("deleg.ent", 3600, "NS", "ns2.example.net.") + up.send("NOERROR") + t.sleep(1) + up = master.update(zone) + up.delete("ent", "A") + up.send("NOERROR") + t.sleep(1) master.ctl("zone-refresh") diff --git a/tests/knot/test_zone-tree.c b/tests/knot/test_zone-tree.c index 0663d81053..c82c54b624 100644 --- a/tests/knot/test_zone-tree.c +++ b/tests/knot/test_zone-tree.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2019 CZ.NIC, z.s.p.o. +/* Copyright (C) 2020 CZ.NIC, z.s.p.o. This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -68,9 +68,17 @@ static int ztree_iter_data(zone_node_t *node, void *data) return result; } +static int ztree_node_counter(zone_node_t *node, void *data) +{ + (void)node; + int *counter = data; + (*counter)++; + return KNOT_EOK; +} + int main(int argc, char *argv[]) { - plan(5); + plan_lazy(); ztree_init_data(); @@ -113,6 +121,12 @@ int main(int argc, char *argv[]) int ret = zone_tree_apply(t, ztree_iter_data, &i); ok (ret == KNOT_EOK, "ztree: ordered traversal"); + /* 6. subtree apply */ + int counter = 0; + ret = zone_tree_sub_apply(t, (const knot_dname_t *)"\x02""ac", + ztree_node_counter, &counter); + ok(ret == KNOT_EOK && counter == 2, "ztree: subtree iteration"); + zone_tree_free(&t); ztree_free_data(); return 0;