From 862c7095a6da016e31ff60c8d0239bee1cfed6b9 Mon Sep 17 00:00:00 2001
From: Jan Kadlec <jan.kadlec@nic.cz>
Date: Wed, 9 Oct 2013 15:21:27 +0200
Subject: [PATCH] DNSSEC: do not add RRSIG to NSEC3 bitmap if not needed.

- Function that checks whether RR should be signed moved to node.h (made most sense)

Refs #4
---
 src/libknot/dnssec/zone-nsec.c | 21 +++++++-
 src/libknot/dnssec/zone-sign.c | 93 ++--------------------------------
 src/libknot/zone/node.c        | 81 +++++++++++++++++++++++++++++
 src/libknot/zone/node.h        | 14 +++++
 4 files changed, 118 insertions(+), 91 deletions(-)

diff --git a/src/libknot/dnssec/zone-nsec.c b/src/libknot/dnssec/zone-nsec.c
index d78476501..82a039a56 100644
--- a/src/libknot/dnssec/zone-nsec.c
+++ b/src/libknot/dnssec/zone-nsec.c
@@ -610,6 +610,25 @@ static int connect_nsec3_nodes(knot_node_t *a, knot_node_t *b, void *data)
 	return KNOT_EOK;
 }
 
+/*!
+ * \brief Check whether at least one RR type in node should be signed
+ *
+ * \param node  Node for which the check is done.
+ *
+ * \return true/false.
+ */
+static bool node_should_be_signed(const knot_node_t *n)
+{
+	knot_rrset_t **node_rrsets = knot_node_get_rrsets_no_copy(n);
+	for (int i = 0; i < n->rrset_count; i++) {
+		if (knot_node_rr_should_be_signed(n, node_rrsets[i], NULL)) {
+			return true;
+		}
+	}
+
+	return false;
+}
+
 /*!
  * \brief Create new NSEC3 node for given regular node.
  *
@@ -637,7 +656,7 @@ static knot_node_t *create_nsec3_node_for_node(knot_node_t *node,
 
 	bitmap_t rr_types = { 0 };
 	bitmap_add_node_rrsets(&rr_types, node);
-	if (node->rrset_count > 0) {
+	if (node->rrset_count > 0 && node_should_be_signed(node)) {
 		bitmap_add_type(&rr_types, KNOT_RRTYPE_RRSIG);
 	}
 	if (node == apex) {
diff --git a/src/libknot/dnssec/zone-sign.c b/src/libknot/dnssec/zone-sign.c
index 68848be57..aa6c9641c 100644
--- a/src/libknot/dnssec/zone-sign.c
+++ b/src/libknot/dnssec/zone-sign.c
@@ -395,94 +395,6 @@ static int resign_rrset(const knot_rrset_t *covered,
 	                          changeset);
 }
 
-/*!
- * \brief Checks whether RRSet is not already in the hash table, automatically
- *        stores its pointer to the table if not found, but returns false in
- *        that case.
- *
- * \param rrset  RRSet to be checked for.
- * \param table  Hash table with already signed RRs.
- *
- * \return True if RR should is signed already, false otherwise.
- */
-static bool rr_already_signed(const knot_rrset_t *rrset, ahtable_t *t)
-{
-	assert(rrset);
-	assert(t);
-
-	// Create a key = combination of owner and type mnemonic
-	int dname_size = knot_dname_size(rrset->owner);
-	assert(dname_size > 0);
-	char key[dname_size + 16];
-	memset(key, 0, sizeof(key));
-	memcpy(key, rrset->owner, dname_size);
-	int ret = knot_rrtype_to_string(rrset->type, key + dname_size, 16);
-	if (ret != KNOT_EOK) {
-		return false;
-	}
-	if (ahtable_tryget(t, key, sizeof(key))) {
-		return true;
-	}
-
-	// If not in the table, insert
-	*ahtable_get(t, (char *)key, sizeof(key)) = (value_t *)rrset;
-	return false;
-}
-
-/*!
- * \brief Checks whether RRSet in a node has to be signed.
- *
- * \param node   Node containing the RRSet.
- * \param rrset  RRSet we are checking for.
- * \param table  Optional hash table with already signed RRs.
- *
- * \return True if RR should be signed, false otherwise.
- */
-static bool rr_should_be_signed(const knot_node_t *node,
-                                const knot_rrset_t *rrset,
-                                ahtable_t *table)
-{
-	assert(node);
-	// TODO make sure this returns 'true' for newly added DSs (not in node)
-	if (rrset == NULL) {
-		return false;
-	}
-
-	// SOA entry is maintained separately
-	if (rrset->type == KNOT_RRTYPE_SOA) {
-		return false;
-	}
-
-	// DNSKEYs are maintained separately
-	if (rrset->type == KNOT_RRTYPE_DNSKEY) {
-		return false;
-	}
-
-	// We only want to sign NSECs and DSs when at delegation points
-	if (knot_node_is_deleg_point(node)) {
-		if (!(rrset->type == KNOT_RRTYPE_NSEC ||
-		    rrset->type == KNOT_RRTYPE_DS)) {
-			return false;
-		}
-	}
-
-	// These RRs have their signatures stored in changeset already
-	if (knot_node_is_replaced_nsec(node)
-	    && ((knot_rrset_type(rrset) == KNOT_RRTYPE_NSEC)
-	         || (knot_rrset_type(rrset) == KNOT_RRTYPE_NSEC3))) {
-		return false;
-	}
-
-	// Check for RRSet in the 'already_signed' table
-	if (table) {
-		if (rr_already_signed(rrset, table)) {
-			return false;
-		}
-	}
-
-	return true;
-}
-
 /*!
  * \brief Update RRSIGs in a given node by updating changeset.
  *
@@ -506,7 +418,7 @@ static int sign_node_rrsets(const knot_node_t *node,
 
 	for (int i = 0; i < node->rrset_count; i++) {
 		const knot_rrset_t *rrset = node->rrset_tree[i];
-		if (!rr_should_be_signed(node, rrset, NULL)) {
+		if (!knot_node_rr_should_be_signed(node, rrset, NULL)) {
 			continue;
 		}
 
@@ -991,7 +903,8 @@ static int sign_changeset_wrap(knot_rrset_t *chg_rrset, void *data)
 	if (node) {
 		const knot_rrset_t *zone_rrset =
 			knot_node_rrset(node, chg_rrset->type);
-		if (rr_should_be_signed(node, zone_rrset, args->signed_table)) {
+		if (knot_node_rr_should_be_signed(node, zone_rrset,
+		                                 args->signed_table)) {
 			return force_resign_rrset(zone_rrset, args->zone_keys,
 			                          args->policy, args->changeset);
 		} else if (zone_rrset && zone_rrset->rrsigs != NULL) {
diff --git a/src/libknot/zone/node.c b/src/libknot/zone/node.c
index 3c8190610..3c6556d46 100644
--- a/src/libknot/zone/node.c
+++ b/src/libknot/zone/node.c
@@ -24,7 +24,9 @@
 #include "common.h"
 #include "zone/node.h"
 #include "rrset.h"
+#include "common/descriptor.h"
 #include "util/debug.h"
+#include "common/hattrie/ahtable.h"
 
 /*----------------------------------------------------------------------------*/
 /* Non-API functions                                                          */
@@ -66,6 +68,40 @@ static inline void knot_node_flags_clear(knot_node_t *node, uint8_t flag)
 	node->flags &= ~flag;
 }
 
+/*!
+ * \brief Checks whether RRSet is not already in the hash table, automatically
+ *        stores its pointer to the table if not found, but returns false in
+ *        that case.
+ *
+ * \param rrset  RRSet to be checked for.
+ * \param table  Hash table with already signed RRs.
+ *
+ * \return True if RR should is signed already, false otherwise.
+ */
+static bool rr_already_signed(const knot_rrset_t *rrset, ahtable_t *t)
+{
+	assert(rrset);
+	assert(t);
+
+	// Create a key = combination of owner and type mnemonic
+	int dname_size = knot_dname_size(rrset->owner);
+	assert(dname_size > 0);
+	char key[dname_size + 16];
+	memset(key, 0, sizeof(key));
+	memcpy(key, rrset->owner, dname_size);
+	int ret = knot_rrtype_to_string(rrset->type, key + dname_size, 16);
+	if (ret != KNOT_EOK) {
+		return false;
+	}
+	if (ahtable_tryget(t, key, sizeof(key))) {
+		return true;
+	}
+
+	// If not in the table, insert
+	*ahtable_get(t, (char *)key, sizeof(key)) = (value_t *)rrset;
+	return false;
+}
+
 /*----------------------------------------------------------------------------*/
 /* API functions                                                              */
 /*----------------------------------------------------------------------------*/
@@ -711,3 +747,48 @@ int knot_node_shallow_copy(const knot_node_t *from, knot_node_t **to)
 
 	return KNOT_EOK;
 }
+
+/*----------------------------------------------------------------------------*/
+
+bool knot_node_rr_should_be_signed(const knot_node_t *node,
+                                   const knot_rrset_t *rrset,
+                                   ahtable_t *table)
+{
+	if (node == NULL || rrset == NULL) {
+		return false;
+	}
+
+	// SOA entry is maintained separately
+	if (rrset->type == KNOT_RRTYPE_SOA) {
+		return false;
+	}
+
+	// DNSKEYs are maintained separately
+	if (rrset->type == KNOT_RRTYPE_DNSKEY) {
+		return false;
+	}
+
+	// We only want to sign NSECs and DSs when at delegation points
+	if (knot_node_is_deleg_point(node)) {
+		if (!(rrset->type == KNOT_RRTYPE_NSEC ||
+		    rrset->type == KNOT_RRTYPE_DS)) {
+			return false;
+		}
+	}
+
+	// These RRs have their signatures stored in changeset already
+	if (knot_node_is_replaced_nsec(node)
+	    && ((knot_rrset_type(rrset) == KNOT_RRTYPE_NSEC)
+	         || (knot_rrset_type(rrset) == KNOT_RRTYPE_NSEC3))) {
+		return false;
+	}
+
+	// Check for RRSet in the 'already_signed' table
+	if (table) {
+		if (rr_already_signed(rrset, table)) {
+			return false;
+		}
+	}
+
+	return true;
+}
diff --git a/src/libknot/zone/node.h b/src/libknot/zone/node.h
index e00456c29..69fa491ca 100644
--- a/src/libknot/zone/node.h
+++ b/src/libknot/zone/node.h
@@ -30,6 +30,7 @@
 
 #include "dname.h"
 #include "rrset.h"
+#include "common/hattrie/ahtable.h"
 
 struct knot_zone;
 
@@ -429,6 +430,19 @@ int knot_node_compare(knot_node_t *node1, knot_node_t *node2);
 
 int knot_node_shallow_copy(const knot_node_t *from, knot_node_t **to);
 
+/*!
+ * \brief Checks whether RRSet in a node has to be signed.
+ *
+ * \param node   Node containing the RRSet.
+ * \param rrset  RRSet we are checking for.
+ * \param table  Optional hash table with already signed RRs.
+ *
+ * \return True if RR should be signed, false otherwise.
+ */
+bool knot_node_rr_should_be_signed(const knot_node_t *node,
+                                   const knot_rrset_t *rrset,
+                                   ahtable_t *table);
+
 #endif /* _KNOT_NODE_H_ */
 
 /*! @} */
-- 
GitLab