From: pcarana <pc.moreno2099@gmail.com>
Date: Fri, 6 Sep 2019 05:04:26 +0000 (-0500)
Subject: Fix bug, complete some unit tests, downgrade NID info logging.
X-Git-Tag: v1.1.0~1^2~9
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7b5445c867accd91df4f058444e65131207bef28;p=thirdparty%2FFORT-validator.git

Fix bug, complete some unit tests, downgrade NID info logging.

-Bug fixed: Router Key PDUs overriding each others weren't removed due to a bad 'memcmp' use.
-Complete unit tests where Router Keys data can be tested.
-Downgrade NID registering log from info to debug on initialization.
---

diff --git a/docs/doc/usage.md b/docs/doc/usage.md
index a38b5b8e..bbd551d4 100644
--- a/docs/doc/usage.md
+++ b/docs/doc/usage.md
@@ -368,7 +368,7 @@ If enabled, the logging output will contain ANSI color codes. Meant for human co
 
 <pre><code class="terminal">$ {{ page.command }} --color-output (...)
 <span style="color:cyan">DBG: Manifest '62gPOPXWxxu0sQa4vQZYUBLaMbY.mft' {</span>
-<span style="color:lightgray">INF: rpkiManifest registered. Its nid is 1061.</span>
+<span style="color:lightgray">INF: Configuration {</span>
 <span style="color:orange">WRN: H2jRmyC2M.mft: The signature algorithm has parameters.</span>
 <span style="color:red">ERR: H2jRmyC2M.mft: Certificate validation failed: certificate has expired</span>
 <span style="color:magenta">CRT: Programming error: Array size is 1 but array is NULL.</span>
diff --git a/src/nid.c b/src/nid.c
index eb279830..15084a5b 100644
--- a/src/nid.c
+++ b/src/nid.c
@@ -23,7 +23,7 @@ register_oid(const char *oid, const char *sn, const char *ln)
 	if (nid == 0)
 		return crypto_err("Unable to register the %s NID.", sn);
 
-	pr_info("%s registered. Its nid is %d.", sn, nid);
+	pr_debug("%s registered. Its nid is %d.", sn, nid);
 	return nid;
 }
 
diff --git a/src/rtr/db/vrps.c b/src/rtr/db/vrps.c
index 6efa3a55..4279f14d 100644
--- a/src/rtr/db/vrps.c
+++ b/src/rtr/db/vrps.c
@@ -423,8 +423,9 @@ router_key_ovrd_remove(struct delta_router_key const *delta, void *arg)
 	SLIST_FOREACH(ptr, filtered_keys, next) {
 		key = &delta->router_key;
 		if (key->as == ptr->delta.router_key.as &&
-		    memcmp(key->ski, ptr->delta.router_key.ski, RK_SKI_LEN) &&
-		    memcmp(key->spk, ptr->delta.router_key.spk, RK_SPKI_LEN) &&
+		    memcmp(key->ski, ptr->delta.router_key.ski, RK_SKI_LEN) == 0
+		    && memcmp(key->spk, ptr->delta.router_key.spk,
+		    RK_SPKI_LEN) == 0 &&
 		    delta->flags != ptr->delta.flags) {
 			SLIST_REMOVE(filtered_keys, ptr, rk_node, next);
 			free(ptr);
diff --git a/test/rtr/db/rtr_db_impersonator.c b/test/rtr/db/rtr_db_impersonator.c
index 9a314bf1..158b6336 100644
--- a/test/rtr/db/rtr_db_impersonator.c
+++ b/test/rtr/db/rtr_db_impersonator.c
@@ -3,6 +3,22 @@
 
 #include "address.c"
 
+static unsigned char db_imp_ski[] = {
+    0x0e, 0xe9, 0x6a, 0x8e, 0x2f, 0xac, 0x50, 0xce, 0x6c, 0x5f,
+    0x93, 0x3e, 0xde, 0x6a, 0xa7, 0x80, 0xa6, 0x85, 0x0e, 0x31 };
+
+static unsigned char db_imp_spk[] = {
+    0x30, 0x59, 0x30, 0x13, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce,
+    0x3d, 0x02, 0x01, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d,
+    0x03, 0x01, 0x07, 0x03, 0x42, 0x00, 0x04, 0xfa, 0xb9, 0x12,
+    0x2d, 0x79, 0x4f, 0xa4, 0xbf, 0xe6, 0xf8, 0xbe, 0xc2, 0x7c,
+    0x27, 0xca, 0xae, 0xfd, 0x45, 0x1e, 0xb3, 0x39, 0xe4, 0x5b,
+    0x08, 0x73, 0xc7, 0xcc, 0x96, 0x78, 0xc7, 0x13, 0xa6, 0x39,
+    0x9d, 0x3b, 0x82, 0x9f, 0x75, 0x20, 0x59, 0xf0, 0x95, 0xea,
+    0xc6, 0x2e, 0x19, 0x46, 0x73, 0x3d, 0x9d, 0x04, 0xcb, 0xa0,
+    0x2f, 0x7b, 0x39, 0x9f, 0x70, 0x42, 0xd4, 0x07, 0xce, 0xde,
+    0x04 };
+
 static int iteration = 0;
 
 static void
@@ -25,6 +41,13 @@ add_v6(struct validation_handler *handler, uint32_t as)
 	    handler->arg));
 }
 
+static void
+add_rk(struct validation_handler *handler, uint32_t as)
+{
+	ck_assert_int_eq(0, handler->handle_router_key(db_imp_ski, as,
+	    db_imp_spk, handler->arg));
+}
+
 int
 __handle_roa_v4(uint32_t as, struct ipv4_prefix const *prefix,
     uint8_t max_length, void *arg)
@@ -60,20 +83,25 @@ perform_standalone_validation(struct db_table *table)
 	case 0:
 		add_v4(&handler, 0);
 		add_v6(&handler, 0);
+		add_rk(&handler, 0);
 		break;
 	case 1:
 		add_v4(&handler, 0);
 		add_v6(&handler, 0);
+		add_rk(&handler, 0);
 		add_v4(&handler, 1);
 		add_v6(&handler, 1);
+		add_rk(&handler, 1);
 		break;
 	case 2:
 		add_v4(&handler, 1);
 		add_v6(&handler, 1);
+		add_rk(&handler, 1);
 		break;
 	case 3:
 		add_v4(&handler, 0);
 		add_v6(&handler, 0);
+		add_rk(&handler, 0);
 		break;
 	default:
 		ck_abort_msg("perform_standalone_validation() was called too many times (%d).",
diff --git a/test/rtr/db/vrps_test.c b/test/rtr/db/vrps_test.c
index b3c9c621..d6b0c669 100644
--- a/test/rtr/db/vrps_test.c
+++ b/test/rtr/db/vrps_test.c
@@ -27,40 +27,44 @@
  * 1: IPv4, ASN 1
  * 2: IPv6, ASN 0
  * 3: IPv6, ASN 1
+ * 4: Router key, ASN 0
+ * 5: Router key, ASN 1
  */
-static const bool iteration0_base[] = { 1, 0, 1, 0, };
-static const bool iteration1_base[] = { 1, 1, 1, 1, };
-static const bool iteration2_base[] = { 0, 1, 0, 1, };
-static const bool iteration3_base[] = { 1, 0, 1, 0, };
+static const bool iteration0_base[] = { 1, 0, 1, 0, 1, 0, };
+static const bool iteration1_base[] = { 1, 1, 1, 1, 1, 1, };
+static const bool iteration2_base[] = { 0, 1, 0, 1, 0, 1, };
+static const bool iteration3_base[] = { 1, 0, 1, 0, 1, 0, };
 
 /*
  * DELTA
- * 0: Withdrawal, IPv4, ASN 0    4: Announcement, IPv4, ASN 0
- * 1: Withdrawal, IPv4, ASN 1    5: Announcement, IPv4, ASN 1
- * 2: Withdrawal, IPv6, ASN 0    6: Announcement, IPv6, ASN 0
- * 3: Withdrawal, IPv6, ASN 1    7: Announcement, IPv6, ASN 1
+ * 0: Withdrawal, IPv4, ASN 0    6: Announcement, IPv4, ASN 0
+ * 1: Withdrawal, IPv4, ASN 1    7: Announcement, IPv4, ASN 1
+ * 2: Withdrawal, IPv6, ASN 0    8: Announcement, IPv6, ASN 0
+ * 3: Withdrawal, IPv6, ASN 1    9: Announcement, IPv6, ASN 1
+ * 4: Withdrawal, RK,   ASN 0   10: Announcement, RK,   ASN 0
+ * 5: Withdrawal, RK,   ASN 1   11: Announcement, RK,   ASN 1
  */
 
-static const bool deltas_0to0[] = { 0, 0, 0, 0, 0, 0, 0, 0, };
+static const bool deltas_0to0[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, };
 
-static const bool deltas_0to1[] = { 0, 0, 0, 0, 0, 1, 0, 1, };
-static const bool deltas_1to1[] = { 0, 0, 0, 0, 0, 0, 0, 0, };
+static const bool deltas_0to1[] = { 0, 0, 0, 0, 0, 0, 0, 1, 0, 1, 0, 1, };
+static const bool deltas_1to1[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, };
 
-static const bool deltas_0to2[] = { 1, 0, 1, 0, 0, 1, 0, 1, };
-static const bool deltas_1to2[] = { 1, 0, 1, 0, 0, 0, 0, 0, };
-static const bool deltas_2to2[] = { 0, 0, 0, 0, 0, 0, 0, 0, };
+static const bool deltas_0to2[] = { 1, 0, 1, 0, 1, 0, 0, 1, 0, 1, 0, 1, };
+static const bool deltas_1to2[] = { 1, 0, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, };
+static const bool deltas_2to2[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, };
 
 /* Deltas with rules that override each other */
-static const bool deltas_0to3_ovrd[] = { 1, 1, 1, 1, 1, 1, 1, 1, };
-static const bool deltas_1to3_ovrd[] = { 1, 1, 1, 1, 1, 0, 1, 0, };
-static const bool deltas_2to3_ovrd[] = { 0, 1, 0, 1, 1, 0, 1, 0, };
-static const bool deltas_3to3_ovrd[] = { 0, 0, 0, 0, 0, 0, 0, 0, };
+static const bool deltas_0to3_ovrd[] = { 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, };
+static const bool deltas_1to3_ovrd[] = { 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 1, 0, };
+static const bool deltas_2to3_ovrd[] = { 0, 1, 0, 1, 0, 1, 1, 0, 1, 0, 1, 0, };
+static const bool deltas_3to3_ovrd[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, };
 
 /* Deltas cleaned up */
-static const bool deltas_0to3_clean[] = { 0, 0, 0, 0, 0, 0, 0, 0, };
-static const bool deltas_1to3_clean[] = { 0, 1, 0, 1, 0, 0, 0, 0, };
-static const bool deltas_2to3_clean[] = { 0, 1, 0, 1, 1, 0, 1, 0, };
-static const bool deltas_3to3_clean[] = { 0, 0, 0, 0, 0, 0, 0, 0, };
+static const bool deltas_0to3_clean[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, };
+static const bool deltas_1to3_clean[] = { 0, 1, 0, 1, 0, 1, 0, 0, 0, 0, 0, 0, };
+static const bool deltas_2to3_clean[] = { 0, 1, 0, 1, 0, 1, 1, 0, 1, 0, 1, 0, };
+static const bool deltas_3to3_clean[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, };
 
 /* Impersonator functions */
 
@@ -135,14 +139,43 @@ get_vrp_index(struct vrp const *vrp)
 }
 
 static array_index
-get_delta_index(struct delta_vrp const *delta)
+get_rk_index(struct router_key const *rk)
+{
+	array_index i;
+
+	for (i = 0; i < RK_SKI_LEN; i++)
+		ck_assert_uint_eq(rk->ski[i], db_imp_ski[i]);
+
+	ck_assert_msg(rk->as <= 1, "Unexpected AS number: %u", rk->as);
+
+	for (i = 0; i < RK_SPKI_LEN; i++)
+		ck_assert_uint_eq(rk->spk[i], db_imp_spk[i]);
+
+	return rk->as + 4;
+}
+
+static array_index
+get_delta_vrp_index(struct delta_vrp const *delta)
 {
 	array_index result;
 
 	result = get_vrp_index(&delta->vrp);
-	ck_assert_msg(delta->flags <= 1, "Unexpected flags: %u", delta->flags);
+	ck_assert_msg(delta->flags <= 1, "VRP Unexpected flags: %u",
+	    delta->flags);
+
+	return result + (delta->flags ? 6 : 0);
+}
+
+static array_index
+get_delta_rk_index(struct delta_router_key const *delta)
+{
+	array_index result;
+
+	result = get_rk_index(&delta->router_key);
+	ck_assert_msg(delta->flags <= 1, "RK Unexpected flags: %u",
+	    delta->flags);
 
-	return (delta->flags << 2) | result;
+	return result + (delta->flags ? 6 : 0);
 }
 
 static int
@@ -161,17 +194,23 @@ vrp_check(struct vrp const *vrp, void *arg)
 static int
 rk_check(struct router_key const *rk, void *arg)
 {
-	/* FIXME (now) add index with Router key examples */
+	bool *array = arg;
+	array_index index;
+
+	index = get_rk_index(rk);
+	ck_assert_uint_eq(false, array[index]);
+	array[index] = true;
+
 	return 0;
 }
 
 static int
-delta_check(struct delta_vrp const *delta, void *arg)
+delta_vrp_check(struct delta_vrp const *delta, void *arg)
 {
 	bool *array = arg;
 	array_index index;
 
-	index = get_delta_index(delta);
+	index = get_delta_vrp_index(delta);
 	ck_assert_uint_eq(false, array[index]);
 	array[index] = true;
 
@@ -181,7 +220,13 @@ delta_check(struct delta_vrp const *delta, void *arg)
 static int
 delta_rk_check(struct delta_router_key const *delta, void *arg)
 {
-	/* FIXME (now) add index with Router key examples */
+	bool *array = arg;
+	array_index index;
+
+	index = get_delta_rk_index(delta);
+	ck_assert_uint_eq(false, array[index]);
+	array[index] = true;
+
 	return 0;
 }
 
@@ -197,7 +242,7 @@ static void
 check_base(serial_t expected_serial, bool const *expected_base)
 {
 	serial_t actual_serial;
-	bool actual_base[4];
+	bool actual_base[6];
 	array_index i;
 
 	memset(actual_base, 0, sizeof(actual_base));
@@ -271,7 +316,7 @@ check_deltas(serial_t from, serial_t to, bool const *expected_deltas,
     bool filter)
 {
 	serial_t actual_serial;
-	bool actual_deltas[8];
+	bool actual_deltas[12];
 	struct deltas_db deltas;
 	struct delta_group *group;
 	array_index i;
@@ -287,7 +332,7 @@ check_deltas(serial_t from, serial_t to, bool const *expected_deltas,
 	memset(actual_deltas, 0, sizeof(actual_deltas));
 	ARRAYLIST_FOREACH(&deltas, group, i)
 		ck_assert_int_eq(0, deltas_foreach(group->serial, group->deltas,
-		    delta_check, delta_rk_check, actual_deltas));
+		    delta_vrp_check, delta_rk_check, actual_deltas));
 	for (i = 0; i < ARRAY_LEN(actual_deltas); i++)
 		ck_assert_uint_eq(expected_deltas[i], actual_deltas[i]);
 }
@@ -338,7 +383,7 @@ START_TEST(test_basic)
 	struct deltas_db deltas;
 	serial_t serial;
 	bool changed;
-	bool iterated_entries[8];
+	bool iterated_entries[12];
 
 	create_deltas_0to1(&deltas, &serial, &changed, iterated_entries);
 
@@ -359,7 +404,7 @@ START_TEST(test_delta_forget)
 	struct deltas_db deltas;
 	serial_t serial;
 	bool changed;
-	bool iterated_entries[8];
+	bool iterated_entries[12];
 
 	create_deltas_0to1(&deltas, &serial, &changed, iterated_entries);
 
@@ -389,7 +434,7 @@ START_TEST(test_delta_ovrd)
 	struct deltas_db deltas;
 	serial_t serial;
 	bool changed;
-	bool iterated_entries[8];
+	bool iterated_entries[12];
 
 	create_deltas_0to1(&deltas, &serial, &changed, iterated_entries);
 
diff --git a/test/rtr/pdu_handler_test.c b/test/rtr/pdu_handler_test.c
index da04f9bd..c164af11 100644
--- a/test/rtr/pdu_handler_test.c
+++ b/test/rtr/pdu_handler_test.c
@@ -86,7 +86,7 @@ init_reset_query(struct rtr_request *request, struct reset_query_pdu *query)
 {
 	request->pdu = query;
 	request->bytes_len = 0;
-	query->header.protocol_version = RTR_V0;
+	query->header.protocol_version = RTR_V1;
 	query->header.pdu_type = PDU_TYPE_RESET_QUERY;
 	query->header.m.reserved = 0;
 	query->header.length = 8;
@@ -98,9 +98,9 @@ init_serial_query(struct rtr_request *request, struct serial_query_pdu *query,
 {
 	request->pdu = query;
 	request->bytes_len = 0;
-	query->header.protocol_version = RTR_V0;
+	query->header.protocol_version = RTR_V1;
 	query->header.pdu_type = PDU_TYPE_SERIAL_QUERY;
-	query->header.m.session_id = get_current_session_id(RTR_V0);
+	query->header.m.session_id = get_current_session_id(RTR_V1);
 	query->header.length = 12;
 	query->serial_number = serial;
 }
@@ -124,7 +124,7 @@ int
 clients_get_rtr_version_set(int fd, bool *is_set, uint8_t *rtr_version)
 {
 	(*is_set) = true;
-	(*rtr_version) = RTR_V0;
+	(*rtr_version) = RTR_V1;
 	return 0;
 }
 
@@ -180,7 +180,7 @@ static int
 handle_delta(struct delta_vrp const *delta, void *arg)
 {
 	int *fd = arg;
-	ck_assert_int_eq(0, send_prefix_pdu(*fd, RTR_V0, &delta->vrp,
+	ck_assert_int_eq(0, send_prefix_pdu(*fd, RTR_V1, &delta->vrp,
 	    delta->flags));
 	return 0;
 }
@@ -189,7 +189,7 @@ static int
 handle_delta_router_key(struct delta_router_key const *delta, void *arg)
 {
 	int *fd = arg;
-	ck_assert_int_eq(0, send_router_key_pdu(*fd, RTR_V0, &delta->router_key,
+	ck_assert_int_eq(0, send_router_key_pdu(*fd, RTR_V1, &delta->router_key,
 	    delta->flags));
 	return 0;
 }
@@ -226,7 +226,7 @@ send_error_report_pdu(int fd, uint8_t version, uint16_t code,
 
 /* Tests */
 
-/* https://tools.ietf.org/html/rfc6810#section-6.1 */
+/* https://tools.ietf.org/html/rfc8210#section-8.1 */
 START_TEST(test_start_or_restart)
 {
 	struct rtr_request request;
@@ -244,6 +244,7 @@ START_TEST(test_start_or_restart)
 	expected_pdu_add(PDU_TYPE_CACHE_RESPONSE);
 	expected_pdu_add(PDU_TYPE_IPV4_PREFIX);
 	expected_pdu_add(PDU_TYPE_IPV6_PREFIX);
+	expected_pdu_add(PDU_TYPE_ROUTER_KEY);
 	expected_pdu_add(PDU_TYPE_END_OF_DATA);
 
 	/* Run and validate */
@@ -255,7 +256,7 @@ START_TEST(test_start_or_restart)
 }
 END_TEST
 
-/* https://tools.ietf.org/html/rfc6810#section-6.2 */
+/* https://tools.ietf.org/html/rfc8210#section-8.2 */
 START_TEST(test_typical_exchange)
 {
 	struct rtr_request request;
@@ -273,8 +274,10 @@ START_TEST(test_typical_exchange)
 	expected_pdu_add(PDU_TYPE_CACHE_RESPONSE);
 	expected_pdu_add(PDU_TYPE_IPV4_PREFIX);
 	expected_pdu_add(PDU_TYPE_IPV6_PREFIX);
+	expected_pdu_add(PDU_TYPE_ROUTER_KEY);
 	expected_pdu_add(PDU_TYPE_IPV4_PREFIX);
 	expected_pdu_add(PDU_TYPE_IPV6_PREFIX);
+	expected_pdu_add(PDU_TYPE_ROUTER_KEY);
 	expected_pdu_add(PDU_TYPE_END_OF_DATA);
 
 	/* From serial 0: Run and validate */
@@ -288,6 +291,7 @@ START_TEST(test_typical_exchange)
 	expected_pdu_add(PDU_TYPE_CACHE_RESPONSE);
 	expected_pdu_add(PDU_TYPE_IPV4_PREFIX);
 	expected_pdu_add(PDU_TYPE_IPV6_PREFIX);
+	expected_pdu_add(PDU_TYPE_ROUTER_KEY);
 	expected_pdu_add(PDU_TYPE_END_OF_DATA);
 
 	/* From serial 1: Run and validate */
@@ -310,7 +314,7 @@ START_TEST(test_typical_exchange)
 }
 END_TEST
 
-/* https://tools.ietf.org/html/rfc6810#section-6.3 */
+/* https://tools.ietf.org/html/rfc8210#section-8.3 */
 START_TEST(test_no_incremental_update_available)
 {
 	struct rtr_request request;
@@ -338,7 +342,7 @@ START_TEST(test_no_incremental_update_available)
 }
 END_TEST
 
-/* https://tools.ietf.org/html/rfc6810#section-6.4 */
+/* https://tools.ietf.org/html/rfc8210#section-8.4 */
 START_TEST(test_cache_has_no_data_available)
 {
 	struct rtr_request request;
@@ -458,7 +462,7 @@ Suite *pdu_suite(void)
 	Suite *suite;
 	TCase *core, *error;
 
-	core = tcase_create("RFC6810-Defined Protocol Sequences");
+	core = tcase_create("RFC8210-Defined Protocol Sequences");
 	tcase_add_test(core, test_start_or_restart);
 	tcase_add_test(core, test_typical_exchange);
 	tcase_add_test(core, test_no_incremental_update_available);