From: Alberto Leiva Popper <ydahhrk@gmail.com>
Date: Mon, 3 Jun 2019 21:50:31 +0000 (-0500)
Subject: Remove the reset and merge validation handler callbacks
X-Git-Tag: v0.0.2~12^2~2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=62bff7192624b6ae83e6374db94ca88f254b091d;p=thirdparty%2FFORT-validator.git

Remove the reset and merge validation handler callbacks

The only certificate that can invalidate the tree is the root one.
This is because other certificates are considered subtrees, and
therefore isolated problems.

RFC 7730 seems to agree:

> If the connection to the preferred URI fails, or the retrieved CA
> certificate public key does not match the TAL public key, the RP
> SHOULD retrieve the CA certificate from the next URI

(Neither of those reasons can be caused by a subtree.)

By the time the first ROA is handled, the root certificate is
already validated. This means that Fort will never find itself
needing to invalidate previously handled ROAs. Hence, there's no
need to "reset" a ROA table nor "merge" it with a global one after
success.

Not having to track both a global and a temporal table also reduces
RAM usage.
---

diff --git a/src/console_handler.c b/src/console_handler.c
index a4fc44b7..fca6f5c0 100644
--- a/src/console_handler.c
+++ b/src/console_handler.c
@@ -27,7 +27,6 @@ validate_into_console(void)
 {
 	struct validation_handler handler;
 
-	handler.reset = NULL;
 	handler.handle_roa_v4 = print_v4_roa;
 	handler.handle_roa_v6 = print_v6_roa;
 	handler.arg = NULL;
diff --git a/src/object/tal.c b/src/object/tal.c
index 97091029..29f47135 100644
--- a/src/object/tal.c
+++ b/src/object/tal.c
@@ -308,10 +308,6 @@ handle_tal_uri(struct tal *tal, struct rpki_uri *uri, void *arg)
 	if (error)
 		return ENSURE_NEGATIVE(error);
 
-	error = vhandler_reset(arg);
-	if (error)
-		return ENSURE_NEGATIVE(error);
-
 	pr_debug_add("TAL URI '%s' {", uri_get_printable(uri));
 
 	if (!uri_is_certificate(uri)) {
@@ -341,6 +337,7 @@ handle_tal_uri(struct tal *tal, struct rpki_uri *uri, void *arg)
 	 * From now on, the tree should be considered valid, even if subsequent
 	 * certificates fail.
 	 * (the root validated successfully; subtrees are isolated problems.)
+	 * Only critical errors should trigger negative result codes.
 	 */
 
 	/* Handle every other certificate. */
@@ -357,7 +354,7 @@ handle_tal_uri(struct tal *tal, struct rpki_uri *uri, void *arg)
 			error = 1;
 			goto end;
 		}
-		if (error)
+		if (error) /* All other errors are critical, currently */
 			goto fail;
 
 		/*
diff --git a/src/rtr/db/roa_table.c b/src/rtr/db/roa_table.c
index 905641ef..1f738a45 100644
--- a/src/rtr/db/roa_table.c
+++ b/src/rtr/db/roa_table.c
@@ -26,8 +26,8 @@ roa_table_create(void)
 	return table;
 }
 
-static void
-roa_table_cleanup(struct roa_table *table)
+void
+roa_table_destroy(struct roa_table *table)
 {
 	struct hashable_roa *node;
 	struct hashable_roa *tmp;
@@ -36,12 +36,7 @@ roa_table_cleanup(struct roa_table *table)
 		HASH_DEL(table->roas, node);
 		free(node);
 	}
-}
 
-void
-roa_table_destroy(struct roa_table *table)
-{
-	roa_table_cleanup(table);
 	free(table);
 }
 
@@ -60,13 +55,6 @@ roa_table_foreach_roa(struct roa_table *table, vrp_foreach_cb cb, void *arg)
 	return 0;
 }
 
-int
-rtrhandler_reset(struct roa_table *table)
-{
-	roa_table_cleanup(table);
-	return 0;
-}
-
 static struct hashable_roa *
 create_roa(uint32_t asn, uint8_t max_length)
 {
@@ -158,12 +146,6 @@ roa_table_clone(struct roa_table **dst, struct roa_table *src)
 	return error;
 }
 
-int
-rtrhandler_merge(struct roa_table *dst, struct roa_table *src)
-{
-	return roa_table_merge(dst, src);
-}
-
 void
 roa_table_remove_roa(struct roa_table *table, struct vrp const *del)
 {
diff --git a/src/rtr/db/roa_table.h b/src/rtr/db/roa_table.h
index 13191304..73648756 100644
--- a/src/rtr/db/roa_table.h
+++ b/src/rtr/db/roa_table.h
@@ -13,12 +13,10 @@ int roa_table_clone(struct roa_table **, struct roa_table *);
 int roa_table_foreach_roa(struct roa_table *, vrp_foreach_cb, void *);
 void roa_table_remove_roa(struct roa_table *, struct vrp const *);
 
-int rtrhandler_reset(struct roa_table *);
 int rtrhandler_handle_roa_v4(struct roa_table *, uint32_t,
     struct ipv4_prefix const *, uint8_t);
 int rtrhandler_handle_roa_v6(struct roa_table *, uint32_t,
     struct ipv6_prefix const *, uint8_t);
-int rtrhandler_merge(struct roa_table *, struct roa_table *);
 
 int compute_deltas(struct roa_table *, struct roa_table *, struct deltas **);
 
diff --git a/src/rtr/db/vrps.c b/src/rtr/db/vrps.c
index bcbe6ede..4ee53b4f 100644
--- a/src/rtr/db/vrps.c
+++ b/src/rtr/db/vrps.c
@@ -88,12 +88,6 @@ vrps_destroy(void)
 	pthread_rwlock_destroy(&lock); /* Nothing to do with error code */
 }
 
-static int
-__reset(void *arg)
-{
-	return rtrhandler_reset(arg);
-}
-
 int
 __handle_roa_v4(uint32_t as, struct ipv4_prefix const *prefix,
     uint8_t max_length, void *arg)
@@ -111,7 +105,7 @@ __handle_roa_v6(uint32_t as, struct ipv6_prefix const * prefix,
 static int
 __perform_standalone_validation(struct roa_table **result)
 {
-	struct roa_table *roas, *global_roas;
+	struct roa_table *roas;
 	struct validation_handler validation_handler;
 	int error;
 
@@ -119,25 +113,17 @@ __perform_standalone_validation(struct roa_table **result)
 	if (roas == NULL)
 		return pr_enomem();
 
-	global_roas = roa_table_create();
-	if (global_roas == NULL) {
-		roa_table_destroy(roas);
-		return pr_enomem();
-	}
-
-	validation_handler.reset = __reset;
 	validation_handler.handle_roa_v4 = __handle_roa_v4;
 	validation_handler.handle_roa_v6 = __handle_roa_v6;
 	validation_handler.arg = roas;
 
 	error = perform_standalone_validation(&validation_handler);
-	roa_table_destroy(roas);
 	if (error) {
-		roa_table_destroy(global_roas);
+		roa_table_destroy(roas);
 		return error;
 	}
 
-	*result = global_roas;
+	*result = roas;
 	return 0;
 }
 
diff --git a/src/validation_handler.c b/src/validation_handler.c
index 94861e85..83348da4 100644
--- a/src/validation_handler.c
+++ b/src/validation_handler.c
@@ -4,12 +4,6 @@
 #include "log.h"
 #include "thread_var.h"
 
-int
-vhandler_reset(struct validation_handler *handler)
-{
-	return (handler->reset != NULL) ? handler->reset(handler->arg) : 0;
-}
-
 static int
 get_current_threads_handler(struct validation_handler const **result)
 {
diff --git a/src/validation_handler.h b/src/validation_handler.h
index 2bd44602..fc557a20 100644
--- a/src/validation_handler.h
+++ b/src/validation_handler.h
@@ -22,13 +22,6 @@
  * All of these functions can be NULL.
  */
 struct validation_handler {
-	/**
-	 * Reinitializator; called every time Fort needs to invalidate a tree
-	 * that was presumed to be correct thus far.
-	 * (Implementor should invalidate all ROAs collected by handle_roa_v4()
-	 * and handle_roa_v6().)
-	 */
-	int (*reset)(void *);
 	/** Called every time Fort has successfully validated an IPv4 ROA. */
 	int (*handle_roa_v4)(uint32_t, struct ipv4_prefix const *, uint8_t,
 	    void *);
@@ -39,7 +32,6 @@ struct validation_handler {
 	void *arg;
 };
 
-int vhandler_reset(struct validation_handler *);
 int vhandler_handle_roa_v4(uint32_t, struct ipv4_prefix const *, uint8_t);
 int vhandler_handle_roa_v6(uint32_t, struct ipv6_prefix const *, uint8_t);