DNSSEC - NSEC/NSEC3 chain fix
NSEC/NSEC3 chain fix. Takes 9 seconds for testing zone (a big one), while master
takes 4 minutes to fix it. 'Nuff said.
Merge request reports
Activity
855 knot_node_t *parent = knot_zone_contents_get_node(contents, cutted); 856 assert(parent || node == contents->apex); 857 bool parent_empty = parent && parent->rrset_count == 0; 858 while (parent_empty) { 859 knot_node_t *empty_node = NULL; 860 int ret = knot_zone_tree_remove(contents->nodes, parent->owner, 861 &empty_node); 862 if (ret != KNOT_EOK) { 863 return ret; 864 } 865 assert(empty_node && empty_node->rrset_count == 0); 866 cutted = knot_wire_next_label(cutted, NULL); 867 parent = knot_zone_contents_get_node(contents, cutted); 868 knot_node_free(&empty_node); 869 parent_empty = parent && parent->rrset_count == 0; 870 } This whole block of code is wrong. Nodes are marked empty in
xfrin_mark_empty()
, called for every node in the tree, traversed from last to first node (at least that's what should be done, maybe the traversing functionzone_contents_tree_apply_inorder_reverse()
does not work as expected), and their parents' child counts are adjusted so that when the parent is processed, it is already marked as having no children, if all were marked empty before. Thus all relevant empty non-terminals are marked as empty as well and should be deleted later in thexfrin_remove_empty_nodes()
function. If you delete the node here, there may be an invalid read when that function tries to remove it.xfrin_remove_empty_nodes()
does not work, most probably due to wrong trie iteration: Here's the order in which the function gets the nodes, after the nodea.b.c.d.e.f.test03.co.uk.
is removed:co.uk. -> noop f.test03.co.uk. -> noop d.e.f.test03.co.uk. -> children count is not 0, noop c.d.e.f.test03.co.uk. -> children count is not 0, noop a.b.c.d.e.f.test03.co.uk. -> rr_count == 0 and children count == 0, mark empty, adjust b.c.d ... e.f.test03.co.uk. -> noop test03.co.uk. -> noop b.c.d.e.f.test03.co.uk. -> mark as empty, because of a.b.c. ...
Not quite what we wanted, because it only deletes
a.b.c.d.e.f.g.test03.co.uk.
andb.c.d...
. We can either create a new walk for trie @mvavrusa ?, or use the modified code and remove thexfrin_remove_empty_nodes()
function @lslovak ? I don't see any issues with marking empty nodes during DDNS and deleting their empty parents if needed, but I'm probably missing something.
mentioned in issue #119 (closed)
mentioned in merge request !129 (merged)
mentioned in merge request !131 (closed)
216 216 knot_update_serial_t soa_up, 217 217 uint32_t *used_lifetime, 218 218 uint32_t *used_refresh, 219 uint32_t new_serial) 219 uint32_t new_serial, 220 hattrie_t **sorted_changes) 1177 1270 /*! 1178 1271 * \brief Sign changeset created by DDNS or zone-diff. 1179 1272 */ 1180 int knot_zone_sign_changeset(const knot_zone_contents_t *zone, 1273 int knot_zone_sign_changeset(const knot_zone_t *zone, 1181 1274 const knot_changeset_t *in_ch, 1182 1275 knot_changeset_t *out_ch, 1276 hattrie_t **sorted_changes, 1188 1282 } 1189 1283 1190 1284 // Create args for wrapper function - hattrie for duplicate sigs 1191 changeset_signing_data_t args = { .zone = zone, .zone_keys = zone_keys, 1285 changeset_signing_data_t args = { .zone = zone->contents, 1286 .zone_keys = zone_keys, 1192 1287 .policy = policy, 1193 1288 .changeset = out_ch, 1194 .signed_table = hattrie_create()}; 1289 .signed_tree = hattrie_create()};