From 1ef7c7c2becff3f100d4ce5b9bb80e1bc2f85bf0 Mon Sep 17 00:00:00 2001
From: Jan Vcelak <jan.vcelak@nic.cz>
Date: Wed, 9 Oct 2013 12:58:37 +0200
Subject: [PATCH] DNSSEC: refactor create_nsec3_owner() and add tests

refs #4, #180
---
 src/libknot/dnssec/zone-nsec.c             | 108 ++++++++-------------
 src/libknot/dnssec/zone-nsec.h             |   7 +-
 src/libknot/zone/zone-contents.c           |  11 +--
 src/tests/Makefile.am                      |   2 +
 src/tests/libknot/dnssec_zone_nsec_tests.c |  63 ++++++++++++
 src/tests/libknot/dnssec_zone_nsec_tests.h |  24 +++++
 src/tests/unittests_main.c                 |   2 +
 7 files changed, 139 insertions(+), 78 deletions(-)
 create mode 100644 src/tests/libknot/dnssec_zone_nsec_tests.c
 create mode 100644 src/tests/libknot/dnssec_zone_nsec_tests.h

diff --git a/src/libknot/dnssec/zone-nsec.c b/src/libknot/dnssec/zone-nsec.c
index d70cda3cd..d78476501 100644
--- a/src/libknot/dnssec/zone-nsec.c
+++ b/src/libknot/dnssec/zone-nsec.c
@@ -403,35 +403,49 @@ static void copy_signatures(const knot_zone_tree_t *from, knot_zone_tree_t *to)
  *
  * \param hash       Raw hash.
  * \param hash_size  Size of the hash.
- * \param apex       Zone apex.
- * \param apex_size  Size of the zone apex.
+ * \param zone_apex  Zone apex.
  *
  * \return NSEC3 owner name, NULL in case of error.
  */
 static knot_dname_t *nsec3_hash_to_dname(const uint8_t *hash, size_t hash_size,
-					 const char *apex, size_t apex_size)
+					 const knot_dname_t *zone_apex)
 {
 	assert(hash);
-	assert(apex);
+	assert(zone_apex);
 
-	char name[KNOT_DNAME_MAX_LENGTH];
-	int32_t endp;
+	// encode raw hash to first label
 
-	endp = base32hex_encode(hash, hash_size, (uint8_t *)name, sizeof(name));
-	if (endp <= 0) {
+	uint8_t label[KNOT_DNAME_MAX_LENGTH];
+	int32_t label_size;
+	label_size = base32hex_encode(hash, hash_size, label, sizeof(label));
+	if (label_size <= 0) {
 		return NULL;
 	}
 
-	name[endp] = '.';
-	endp += 1;
+	// allocate result
+
+	size_t zone_apex_size = knot_dname_size(zone_apex);
+	size_t result_size = 1 + label_size + zone_apex_size;
+	knot_dname_t *result = malloc(result_size);
+	if (!result) {
+		return NULL;
+	}
 
-	memcpy(name + endp, apex, apex_size);
-	endp += apex_size;
+	// build the result
 
-	knot_dname_t *dname = knot_dname_from_str(name, endp);
-	knot_dname_to_lower(dname);
+	uint8_t *write = result;
 
-	return dname;
+	*write = (uint8_t)label_size;
+	write += 1;
+	memcpy(write, label, label_size);
+	write += label_size;
+	memcpy(write, zone_apex, zone_apex_size);
+	write += zone_apex_size;
+
+	assert(write == result + result_size);
+	knot_dname_to_lower(result);
+
+	return result;
 }
 
 /* - NSEC3 nodes construction ---------------------------------------------- */
@@ -596,54 +610,27 @@ static int connect_nsec3_nodes(knot_node_t *a, knot_node_t *b, void *data)
 	return KNOT_EOK;
 }
 
-/*!
- * \brief Get zone apex as a string.
- */
-static bool get_zone_apex_str(const knot_zone_contents_t *zone,
-                              char **apex, size_t *apex_size)
-{
-	assert(zone);
-	assert(zone->apex);
-	assert(apex);
-	assert(apex_size);
-
-	*apex = knot_dname_to_str(zone->apex->owner);
-	if (!*apex) {
-		return false;
-	}
-
-	*apex_size = strlen(*apex);
-
-	return true;
-}
-
 /*!
  * \brief Create new NSEC3 node for given regular node.
  *
- * \note Parameters 'apex' and 'apex_size' are added for performace reasons.
- *
  * \param node       Node for which the NSEC3 node is created.
- * \param apex_node  Zone apex node.
- * \param apex       Zone apex.
- * \param apex_size  Size fo zone apex.
+ * \param apex       Zone apex node.
  * \param params     NSEC3 hash function parameters.
  * \param ttl        TTL of the new NSEC3 node.
  *
  * \return Error code, KNOT_EOK if successful.
  */
 static knot_node_t *create_nsec3_node_for_node(knot_node_t *node,
-                                               knot_node_t *apex_node,
-                                               char *apex, size_t apex_size,
+                                               knot_node_t *apex,
                                                const knot_nsec3_params_t *params,
                                                uint32_t ttl)
 {
 	assert(node);
-	assert(apex_node);
 	assert(apex);
 	assert(params);
 
 	knot_dname_t *nsec3_owner;
-	nsec3_owner = create_nsec3_owner(node->owner, params, apex, apex_size);
+	nsec3_owner = create_nsec3_owner(node->owner, apex->owner, params);
 	if (!nsec3_owner) {
 		return NULL;
 	}
@@ -653,13 +640,12 @@ static knot_node_t *create_nsec3_node_for_node(knot_node_t *node,
 	if (node->rrset_count > 0) {
 		bitmap_add_type(&rr_types, KNOT_RRTYPE_RRSIG);
 	}
-	if (node == apex_node) {
+	if (node == apex) {
 		bitmap_add_type(&rr_types, KNOT_RRTYPE_DNSKEY);
 	}
 
 	knot_node_t *nsec3_node;
-	nsec3_node = create_nsec3_node(nsec3_owner, params, apex_node,
-	                               &rr_types, ttl);
+	nsec3_node = create_nsec3_node(nsec3_owner, params, apex, &rr_types, ttl);
 
 	return nsec3_node;
 }
@@ -683,12 +669,6 @@ static int create_nsec3_nodes(const knot_zone_contents_t *zone, uint32_t ttl,
 
 	assert(params);
 
-	char *apex = NULL;
-	size_t apex_size;
-	if (!get_zone_apex_str(zone, &apex, &apex_size)) {
-		return KNOT_ENOMEM;
-	}
-
 	int result = KNOT_EOK;
 
 	int sorted = false;
@@ -697,8 +677,8 @@ static int create_nsec3_nodes(const knot_zone_contents_t *zone, uint32_t ttl,
 		knot_node_t *node = (knot_node_t *)*hattrie_iter_val(it);
 
 		knot_node_t *nsec3_node;
-		nsec3_node = create_nsec3_node_for_node(node, zone->apex, apex,
-		                                        apex_size, params, ttl);
+		nsec3_node = create_nsec3_node_for_node(node, zone->apex,
+		                                        params, ttl);
 		if (!nsec3_node) {
 			result = KNOT_ENOMEM;
 			break;
@@ -713,7 +693,6 @@ static int create_nsec3_nodes(const knot_zone_contents_t *zone, uint32_t ttl,
 	}
 
 	hattrie_iter_free(it);
-	free(apex);
 
 	return result;
 }
@@ -857,27 +836,26 @@ static bool get_zone_soa_min_ttl(const knot_zone_contents_t *zone,
  * \brief Create NSEC3 owner name from regular owner name.
  */
 knot_dname_t *create_nsec3_owner(const knot_dname_t *owner,
-                                 const knot_nsec3_params_t *params,
-                                 const char *apex, size_t apex_size)
+				 const knot_dname_t *zone_apex,
+                                 const knot_nsec3_params_t *params)
 {
-	if (owner == NULL || params == NULL || apex == NULL) {
+	if (owner == NULL || zone_apex == NULL || params == NULL) {
 		return NULL;
 	}
 
 	uint8_t *hash = NULL;
 	size_t hash_size = 0;
-	int name_size = knot_dname_size(owner);
+	int owner_size = knot_dname_size(owner);
 
-	if (name_size < 0) {
+	if (owner_size < 0) {
 		return NULL;
 	}
 
-	if (knot_nsec3_hash(params, owner, name_size, &hash, &hash_size)
-	    != KNOT_EOK) {
+	if (knot_nsec3_hash(params, owner, owner_size, &hash, &hash_size) != KNOT_EOK) {
 		return NULL;
 	}
 
-	knot_dname_t *result = nsec3_hash_to_dname(hash, hash_size, apex, apex_size);
+	knot_dname_t *result = nsec3_hash_to_dname(hash, hash_size, zone_apex);
 	free(hash);
 
 	return result;
diff --git a/src/libknot/dnssec/zone-nsec.h b/src/libknot/dnssec/zone-nsec.h
index 0cea6944d..2fe8113ef 100644
--- a/src/libknot/dnssec/zone-nsec.h
+++ b/src/libknot/dnssec/zone-nsec.h
@@ -48,15 +48,14 @@ bool is_nsec3_enabled(const knot_zone_contents_t *zone);
  * \brief Create NSEC3 owner name from regular owner name.
  *
  * \param owner      Node owner name.
+ * \param zone_apex  Zone apex name.
  * \param params     Params for NSEC3 hashing function.
- * \param apex       Apex name.
- * \param apex_size  Size of the zone apex name.
  *
  * \return NSEC3 owner name, NULL in case of error.
  */
 knot_dname_t *create_nsec3_owner(const knot_dname_t *owner,
-                                 const knot_nsec3_params_t *params,
-                                 const char *apex, size_t apex_size);
+                                 const knot_dname_t *zone_apex,
+                                 const knot_nsec3_params_t *params);
 
 /*!
  * \brief Create NSEC or NSEC3 chain in the zone.
diff --git a/src/libknot/zone/zone-contents.c b/src/libknot/zone/zone-contents.c
index 959a04ab5..fb8f3ed30 100644
--- a/src/libknot/zone/zone-contents.c
+++ b/src/libknot/zone/zone-contents.c
@@ -144,15 +144,8 @@ static int knot_zone_contents_nsec3_name(const knot_zone_contents_t *zone,
 		return KNOT_ENSEC3PAR;
 	}
 
-	char *apex_name = knot_dname_to_str(knot_node_owner(
-	                                        knot_zone_contents_apex(zone)));
-	assert(apex_name);
-
-	*nsec3_name = create_nsec3_owner(name, nsec3_params, apex_name,
-	                                 strlen(apex_name));
-	free(apex_name);
-
-	if (*nsec3_name == NULL) {
+	*nsec3_name = create_nsec3_owner(name, zone->apex->owner, nsec3_params);
+	if (nsec3_name == NULL) {
 		return KNOT_ERROR;
 	}
 
diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am
index cbc1d0661..1931bdb2e 100644
--- a/src/tests/Makefile.am
+++ b/src/tests/Makefile.am
@@ -60,6 +60,8 @@ unittests_SOURCES =			\
 	libknot/dnssec_nsec3_tests.h	\
 	libknot/dnssec_sign_tests.c	\
 	libknot/dnssec_sign_tests.h	\
+	libknot/dnssec_zone_nsec_tests.c \
+	libknot/dnssec_zone_nsec_tests.h \
 	unittests_main.c
 
 unittests_LDADD = ../libknotd.la ../libknots.la @LIBOBJS@
diff --git a/src/tests/libknot/dnssec_zone_nsec_tests.c b/src/tests/libknot/dnssec_zone_nsec_tests.c
new file mode 100644
index 000000000..e91b82bd5
--- /dev/null
+++ b/src/tests/libknot/dnssec_zone_nsec_tests.c
@@ -0,0 +1,63 @@
+/*  Copyright (C) 2013 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
+
+    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
+    the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <config.h>
+
+#include "dnssec_zone_nsec_tests.h"
+#include "libknot/dname.h"
+#include "libknot/dnssec/zone-nsec.h"
+
+static int dnssec_zone_nsec_tests_count(int argc, char *argv[]);
+static int dnssec_zone_nsec_tests_run(int argc, char *argv[]);
+
+unit_api dnssec_zone_nsec_tests_api = {
+	"libknot/dnssec/zone-nsec",
+	&dnssec_zone_nsec_tests_count,
+	&dnssec_zone_nsec_tests_run
+};
+
+static int dnssec_zone_nsec_tests_count(int argc, char *argv[])
+{
+	return 1;
+}
+
+static knot_dname_t *get_dname(const char *str)
+{
+	size_t length = strlen(str);
+	return knot_dname_from_str(str, length);
+}
+
+static int dnssec_zone_nsec_tests_run(int argc, char *argv[])
+{
+	knot_dname_t *owner  = get_dname("name.example.com");
+	knot_dname_t *apex   = get_dname("example.com");
+	knot_dname_t *expect = get_dname("sv9o5lv8kgv6lm1t9dkst43b3c0aagbj.example.com");
+
+	knot_nsec3_params_t params = {
+		.algorithm = 1, .flags = 0, .iterations = 10,
+		.salt = (uint8_t *)"\xc0\x01", .salt_length = 2
+	};
+
+	knot_dname_t *result = create_nsec3_owner(owner, apex, &params);
+	ok(knot_dname_cmp(result, expect) == 0, "create_nsec3_owner()");
+
+	knot_dname_free(&result);
+	knot_dname_free(&owner);
+	knot_dname_free(&apex);
+	knot_dname_free(&expect);
+
+	return 0;
+}
diff --git a/src/tests/libknot/dnssec_zone_nsec_tests.h b/src/tests/libknot/dnssec_zone_nsec_tests.h
new file mode 100644
index 000000000..7badb4f7a
--- /dev/null
+++ b/src/tests/libknot/dnssec_zone_nsec_tests.h
@@ -0,0 +1,24 @@
+/*  Copyright (C) 2013 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
+
+    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
+    the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef _KNOTD_DNSSEC_ZONE_NSEC_TESTS_
+#define _KNOTD_DNSSEC_ZONE_NSEC_TESTS_
+
+#include "common/libtap/tap_unit.h"
+
+unit_api dnssec_zone_nsec_tests_api;
+
+#endif // _KNOTD_DNSSEC_ZONE_NSEC_TESTS_
diff --git a/src/tests/unittests_main.c b/src/tests/unittests_main.c
index cd9729c79..5513fc19d 100644
--- a/src/tests/unittests_main.c
+++ b/src/tests/unittests_main.c
@@ -38,6 +38,7 @@
 #include "tests/libknot/dnssec_keys_tests.h"
 #include "tests/libknot/dnssec_nsec3_tests.h"
 #include "tests/libknot/dnssec_sign_tests.h"
+#include "tests/libknot/dnssec_zone_nsec_tests.h"
 #include "tests/libknot/rrset_tests.h"
 
 // Run all loaded units
@@ -75,6 +76,7 @@ int main(int argc, char *argv[])
 	        &dnssec_keys_tests_api,  //! DNSSEC key manipulation.
 	        &dnssec_nsec3_tests_api, //! DNSSEC NSEC3 operations.
 	        &dnssec_sign_tests_api,  //! DNSSEC signing/verification.
+	        &dnssec_zone_nsec_tests_api, //! Zone NSEC functions.
 //	        &rrset_tests_api,
 
 	        NULL
-- 
GitLab