Merge branch 'dnssec-fixes' into 'master'

DNSSEC fixes

Two fixes in this MR:

1. Continuation of fix merged in !176. Knot was still generating RRSIGs for the old (wrong) NSEC3s, as it did not know that they would be removed. Fixed by marking nodes from which NSEC3s are to be removed, so that these are not signed in the next step. (Commit 38aeecc3)
2. Function for saving removed NSEC/NSEC3 records into changeset (`knot_nsec_changeset_remove()`) was ignoring NSEC3 RRSIGs. (Commit 8b5f08ac). This probably manifested after UPDATE - a test should be added for this case.

src/knot/dnssec/nsec-chain.c

@@ -238,9 +238,8 @@ static int connect_nsec_nodes(knot_node_t *a, knot_node_t *b,
 		}
 
 		dbg_dnssec_detail("NSECs not equal, replacing.\n");
-		// current NSEC is invalid, replace it and drop RRSIG
-		// mark the node, so later we know this NSEC needs new RRSIGs
-		knot_node_set_replaced_nsec(a);
+		// Mark the node so that we do not sign this NSEC
+		knot_node_set_removed_nsec(a);
 		ret = knot_nsec_changeset_remove(a, data->changeset);
 		if (ret != KNOT_EOK) {
 			knot_rrset_deep_free(&new_nsec, 1, NULL);
@@ -651,6 +650,15 @@ int knot_nsec_changeset_remove(const knot_node_t *n,
 		knot_rrset_t *synth_rrsigs = NULL;
 		result = knot_rrset_synth_rrsig(rrsigs->owner, KNOT_RRTYPE_NSEC,
 		                                rrsigs, &synth_rrsigs, NULL);
+
+		if (result == KNOT_ENOENT) {
+			// Try removing NSEC3 RRSIGs
+			result = knot_rrset_synth_rrsig(rrsigs->owner,
+			                                KNOT_RRTYPE_NSEC3,
+			                                rrsigs, &synth_rrsigs,
+			                                NULL);
+		}
+
 		if (result != KNOT_EOK) {
 			if (result != KNOT_ENOENT) {
 				return result;

src/knot/dnssec/nsec3-chain.c

@@ -310,10 +310,8 @@ static int update_nsec3(const knot_dname_t *from, const knot_dname_t *to,
 			return KNOT_ENOMEM;
 		}
 		// Create the RRSet
-		gen_nsec3 = create_nsec3_rrset(owner,
-		                                         &zone->nsec3_params,
-		                                         &bm, binary_next,
-		                                         soa_min);
+		gen_nsec3 = create_nsec3_rrset(owner, &zone->nsec3_params,
+		                               &bm, binary_next, soa_min);
 		if (gen_nsec3 == NULL) {
 			free(binary_next);
 			knot_dname_free(&owner);
@@ -322,8 +320,7 @@ static int update_nsec3(const knot_dname_t *from, const knot_dname_t *to,
 	} else {
 		assert(old_nsec3);
 		// Reuse bitmap and data from old NSEC3
-		int ret = knot_rrset_deep_copy(old_nsec3, &gen_nsec3,
-		                                      NULL);
+		int ret = knot_rrset_deep_copy(old_nsec3, &gen_nsec3, NULL);
 		if (ret != KNOT_EOK) {
 			free(binary_next);
 			return ret;
@@ -748,7 +745,7 @@ static int create_nsec3_nodes(const knot_zone_contents_t *zone, uint32_t ttl,
 			break;
 		}
 		if (knot_node_rrset(node, KNOT_RRTYPE_NSEC)) {
-			knot_node_set_replaced_nsec(node);
+			knot_node_set_removed_nsec(node);
 		}
 
 		if (knot_node_is_non_auth(node)) {

src/knot/dnssec/zone-events.c

@@ -84,8 +84,8 @@ static int zone_sign(knot_zone_contents_t *zone, conf_zone_t *zone_config,
 	// Init needed structs
 	knot_zone_keys_t zone_keys = { '\0' };
 	knot_dnssec_policy_t policy = { '\0' };
-	int result = init_dnssec_structs(zone, zone_config, &zone_keys, &policy, soa_up,
-	                                 force);
+	int result = init_dnssec_structs(zone, zone_config, &zone_keys, &policy,
+	                                 soa_up, force);
 	if (result != KNOT_EOK) {
 		free(msgpref);
 		return result;

src/knot/dnssec/zone-nsec.c

@@ -109,6 +109,60 @@ static bool get_zone_soa_min_ttl(const knot_zone_contents_t *zone,
 	return true;
 }
 
+/*!
+ * \brief Finds a node with the same owner as the given NSEC3 RRSet and marks it
+ *        as 'removed'.
+ *
+ * \param data NSEC3 tree to search for the node in. (type knot_zone_tree_t *).
+ * \param rrset RRSet whose owner will be sought in the zone tree. non-NSEC3
+ *              RRSets are ignored.
+ *
+ * This function is constructed as a callback for the knot_changeset_apply() f
+ * function.
+ */
+static int mark_nsec3(knot_rrset_t *rrset, void *data)
+{
+	assert(rrset != NULL);
+	assert(data != NULL);
+
+	knot_zone_tree_t *nsec3s = (knot_zone_tree_t *)data;
+	knot_node_t *node = NULL;
+	int ret;
+
+	if (knot_rrset_type(rrset) == KNOT_RRTYPE_NSEC3) {
+		// Find the name in the NSEC3 tree and mark the node
+		ret = knot_zone_tree_get(nsec3s, knot_rrset_owner(rrset),
+		                         &node);
+		if (ret != KNOT_EOK) {
+			return ret;
+		}
+
+		if (node != NULL) {
+			knot_node_set_removed_nsec(node);
+		}
+	}
+
+	return KNOT_EOK;
+}
+
+/*!
+ * \brief Marks all NSEC3 nodes in zone from which RRSets are to be removed.
+ *
+ * For each NSEC3 RRSet in the changeset finds its node and marks it with the
+ * 'removed' flag.
+ */
+static int mark_removed_nsec3(knot_changeset_t *out_ch,
+                              const knot_zone_contents_t *zone)
+{
+	if (zone->nsec3_nodes == NULL) {
+		return KNOT_EOK;
+	}
+
+	int ret = knot_changeset_apply(out_ch, KNOT_CHANGESET_REMOVE,
+	                               mark_nsec3, (void *)zone->nsec3_nodes);
+	return ret;
+}
+
 /* - public API ------------------------------------------------------------ */
 
 /*!
@@ -220,6 +274,11 @@ int knot_zone_create_nsec_chain(const knot_zone_contents_t *zone,
 		result = delete_nsec3_chain(zone, changeset);
 	}
 
+	if (result == KNOT_EOK) {
+		// Mark removed NSEC3 nodes, so that they are not signed later
+		result = mark_removed_nsec3(changeset, zone);
+	}
+
 	if (result != KNOT_EOK) {
 		return result;
 	}

src/knot/dnssec/zone-sign.c

@@ -559,7 +559,7 @@ static int sign_node(knot_node_t **node, void *data)
 
 	int result = sign_node_rrsets(*node, args->zone_keys, args->policy,
 	                              args->changeset, &args->expires_at);
-	knot_node_clear_replaced_nsec(*node);
+	knot_node_clear_removed_nsec(*node);
 
 	return result;
 }
@@ -1470,7 +1470,7 @@ int knot_zone_sign_rr_should_be_signed(const knot_node_t *node,
 	}
 
 	// These RRs have their signatures stored in changeset already
-	if (knot_node_is_replaced_nsec(node)
+	if (knot_node_is_removed_nsec(node)
 	    && ((knot_rrset_type(rrset) == KNOT_RRTYPE_NSEC)
 	         || (knot_rrset_type(rrset) == KNOT_RRTYPE_NSEC3))) {
 		return KNOT_EOK;

src/knot/zone/node.c

@@ -598,23 +598,23 @@ void knot_node_clear_empty(knot_node_t *node)
 
 /*----------------------------------------------------------------------------*/
 
-int knot_node_is_replaced_nsec(const knot_node_t *node)
+int knot_node_is_removed_nsec(const knot_node_t *node)
 {
-	return knot_node_flags_get(node, KNOT_NODE_FLAGS_REPLACED_NSEC);
+	return knot_node_flags_get(node, KNOT_NODE_FLAGS_REMOVED_NSEC);
 }
 
 /*----------------------------------------------------------------------------*/
 
-void knot_node_set_replaced_nsec(knot_node_t *node)
+void knot_node_set_removed_nsec(knot_node_t *node)
 {
-	knot_node_flags_set(node, KNOT_NODE_FLAGS_REPLACED_NSEC);
+	knot_node_flags_set(node, KNOT_NODE_FLAGS_REMOVED_NSEC);
 }
 
 /*----------------------------------------------------------------------------*/
 
-void knot_node_clear_replaced_nsec(knot_node_t *node)
+void knot_node_clear_removed_nsec(knot_node_t *node)
 {
-	knot_node_flags_clear(node, KNOT_NODE_FLAGS_REPLACED_NSEC);
+	knot_node_flags_clear(node, KNOT_NODE_FLAGS_REMOVED_NSEC);
 }
 
 /*----------------------------------------------------------------------------*/

src/knot/zone/node.h

@@ -77,9 +77,9 @@ struct knot_node {
 	/*!
 	 * \brief Various flags.
 	 *
-	 * Currently only two:
 	 *   0x01 - node is a delegation point
 	 *   0x02 - node is non-authoritative (under a delegation point)
+	 *   0x04 - NSEC(3) was removed from the node.
 	 *   0x10 - node is empty and will be deleted after update
 	 */
 	uint8_t flags;
@@ -94,11 +94,11 @@ typedef enum {
 	KNOT_NODE_FLAGS_DELEG = (uint8_t)0x01,
 	/*! \brief Node is not authoritative (i.e. below a zone cut). */
 	KNOT_NODE_FLAGS_NONAUTH = (uint8_t)0x02,
+	/*! \brief NSEC/NSEC3 was removed from this node. */
+	KNOT_NODE_FLAGS_REMOVED_NSEC = (uint8_t)0x04,
 	/*! \brief Node is empty and will be deleted after update.
 	 *  \todo Remove after dname refactoring, update description in node. */
-	KNOT_NODE_FLAGS_EMPTY = (uint8_t)0x10,
-	/*! \brief NSEC in this node needs new RRSIGs. Used for signing. */
-	KNOT_NODE_FLAGS_REPLACED_NSEC = (uint8_t)0x20
+	KNOT_NODE_FLAGS_EMPTY = (uint8_t)0x10
 } knot_node_flags_t;
 
 /*----------------------------------------------------------------------------*/
@@ -371,11 +371,11 @@ void knot_node_set_auth(knot_node_t *node);
 
 int knot_node_is_auth(const knot_node_t *node);
 
-int knot_node_is_replaced_nsec(const knot_node_t *node);
+int knot_node_is_removed_nsec(const knot_node_t *node);
 
-void knot_node_set_replaced_nsec(knot_node_t *node);
+void knot_node_set_removed_nsec(knot_node_t *node);
 
-void knot_node_clear_replaced_nsec(knot_node_t *node);
+void knot_node_clear_removed_nsec(knot_node_t *node);
 
 //! \todo remove after dname refactoring
 int knot_node_is_empty(const knot_node_t *node);