From: Alberto Leiva Popper <ydahhrk@gmail.com>
Date: Wed, 24 Sep 2025 18:03:50 +0000 (-0600)
Subject: Misc. testing
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6d361ea67861d25a67de48ff8a613d71409998fd;p=thirdparty%2FFORT-validator.git

Misc. testing

Back in May, I tried creating unit tests for the cache's new stateful features.
This scaled poorly, which later led to the creation of the Barry/RPT framework.

I'm throwing away those failed unit tests, but I'm keeping the few bugfixes I
managed to whip up while I was coding them.

The only special mention is "separate cache_setup() into two steps."
This is needed because all involved processes need to work in the cache
directory. Ensuring the directory exists and has been chdir'd into before
subprocesses are spawned is a simple means to achieve that.
---

diff --git a/src/cache.c b/src/cache.c
index b3bcd60d..a22bfa0f 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -272,8 +272,6 @@ node2json(struct cache_node *node)
 		goto fail;
 	if (json_add_str(json, "path", node->path))
 		goto fail;
-	if (node->verdict && json_add_str(json, "error", node->verdict))
-		goto fail;
 	if (node->attempt_ts && json_add_ts(json, "attempt", node->attempt_ts))
 		goto fail;
 	if (node->success_ts && json_add_ts(json, "success", node->success_ts))
@@ -427,7 +425,7 @@ cache_atexit(void)
 }
 
 int
-cache_setup(void)
+cache_setup1(void)
 {
 	char const *cachedir;
 	int error;
@@ -445,6 +443,14 @@ cache_setup(void)
 		return error;
 	}
 
+	return 0;
+}
+
+int
+cache_setup2(void)
+{
+	int error;
+
 	init_tables();
 
 	errno = 0;
@@ -1004,30 +1010,36 @@ get_fallback(struct extension_uris *uris)
 	return (difftime(rsync->success_ts, rrdp->success_ts) > 0) ? rsync : rrdp;
 }
 
-/* Do not free nor modify the result. */
 validation_verdict
-cache_refresh_by_url(struct uri const *url, char const **result)
+cache_refresh_by_url(struct uri const *url, char **result)
 {
 	struct cache_node *node = NULL;
 	validation_verdict vv;
 
-	if (uri_is_https(url))
+	if (uri_is_https(url)) {
 		vv = do_refresh(&cache.https, url, &node);
-	else if (uri_is_rsync(url))
+		if (vv != VV_CONTINUE)
+			goto oops;
+		*result = node ? pstrdup(node->path) : NULL;
+		return VV_CONTINUE;
+	}
+
+	if (uri_is_rsync(url)) {
 		vv = do_refresh(&cache.rsync, url, &node);
-	else
-		vv = VV_FAIL;
+		if (vv != VV_CONTINUE)
+			goto oops;
+		*result = path_join(node->path, strip_rsync_module(uri_str(url)));
+		return VV_CONTINUE;
+	}
 
-	*result = node ? node->path : NULL;
+	vv = VV_FAIL;
+oops:	*result = NULL;
 	return vv;
 }
 
-/*
- * HTTPS (TAs) and rsync only; don't use this for RRDP.
- * Do not free nor modify the result.
- */
+/* HTTPS (TAs) and rsync only; don't use this for RRDP. */
 validation_verdict
-cache_get_fallback(struct uri const *url, char const **result)
+cache_get_fallback(struct uri const *url, char **result)
 {
 	struct cache_node *node;
 
@@ -1045,7 +1057,7 @@ cache_get_fallback(struct uri const *url, char const **result)
 		return VV_CONTINUE;
 	}
 
-	*result = node->path;
+	*result = pstrdup(node->path);
 	return VV_CONTINUE;
 }
 
diff --git a/src/cache.h b/src/cache.h
index 9d04a1da..d78b125f 100644
--- a/src/cache.h
+++ b/src/cache.h
@@ -7,7 +7,8 @@
 #include "types/rpp.h"
 #include "types/uri.h"
 
-int cache_setup(void);		/* Init this module */
+int cache_setup1(void);
+int cache_setup2(void);
 void cache_atexit(void);
 
 int cache_prepare(void);	/* Prepare cache for new validation cycle */
@@ -38,8 +39,8 @@ struct extension_uris {
 void exturis_init(struct extension_uris *);
 void exturis_cleanup(struct extension_uris *);
 
-validation_verdict cache_refresh_by_url(struct uri const *, char const **);
-validation_verdict cache_get_fallback(struct uri const *, char const **);
+validation_verdict cache_refresh_by_url(struct uri const *, char **);
+validation_verdict cache_get_fallback(struct uri const *, char **);
 
 struct cache_cage;
 validation_verdict cache_refresh_by_uris(struct extension_uris *,
diff --git a/src/ext.c b/src/ext.c
index 5e033c05..d4dab681 100644
--- a/src/ext.c
+++ b/src/ext.c
@@ -9,7 +9,6 @@
 #include "libcrypto_util.h"
 #include "log.h"
 #include "nid.h"
-#include "thread_var.h"
 
 static json_t *
 unimplemented(void const *arg)
diff --git a/src/main.c b/src/main.c
index d81751cc..5a3eaaeb 100644
--- a/src/main.c
+++ b/src/main.c
@@ -128,6 +128,10 @@ main(int argc, char **argv)
 	if (error)
 		goto revert_log;
 
+	error = cache_setup1();
+	if (error)
+		goto revert_config;
+
 	rsync_setup(NULL, NULL); /* Fork rsync spawner ASAP */
 	register_signal_handlers();
 
@@ -152,7 +156,7 @@ main(int argc, char **argv)
 	error = vrps_init();
 	if (error)
 		goto revert_relax_ng;
-	error = cache_setup();
+	error = cache_setup2();
 	if (error)
 		goto revert_vrps;
 	error = output_setup();
@@ -188,6 +192,7 @@ revert_nid:
 	nid_destroy();
 revert_rsync:
 	rsync_teardown();
+revert_config:
 	free_rpki_config();
 revert_log:
 	log_teardown();
diff --git a/src/object/certificate.c b/src/object/certificate.c
index fce74a41..87079659 100644
--- a/src/object/certificate.c
+++ b/src/object/certificate.c
@@ -129,10 +129,6 @@ validate_issuer(struct rpki_certificate *cert)
 	return 0;
 }
 
-/*
- * Compare public keys, call @diff_alg_cb when the algorithm is different, call
- * @diff_pk_cb when the public key is different; return 0 if both are equal.
- */
 static int
 spki_cmp(X509_PUBKEY *tal_spki, X509_PUBKEY *cert_spki)
 {
@@ -201,12 +197,12 @@ decode_spki(struct tal *tal)
 	spki = d2i_X509_PUBKEY(NULL, &cursor, len);
 
 	if (spki == NULL) {
-		op_crypto_err("The public key cannot be decoded.");
+		op_crypto_err("The TAL's public key cannot be decoded.");
 		goto fail;
 	}
 	if (cursor != origin + len) {
 		X509_PUBKEY_free(spki);
-		op_crypto_err("The public key contains trailing garbage.");
+		op_crypto_err("The TAL's public key contains trailing garbage.");
 		goto fail;
 	}
 
@@ -440,7 +436,7 @@ validate_public_key(struct rpki_certificate *cert)
 		if ((evppkey = X509_get0_pubkey(cert->x509)) == NULL)
 			return val_crypto_err("X509_get0_pubkey() returned NULL");
 		if (X509_verify(cert->x509, evppkey) != 1)
-			return EINVAL;
+			return val_crypto_err("Invalid signature.");
 	}
 
 	return 0;
@@ -1232,7 +1228,7 @@ handle_caRepository(struct uri const *uri, void *arg)
 	char const *cr;
 
 	cr = uri_str(uri);
-	pr_clutter("caRepository: %s", caRepo);
+	pr_clutter("caRepository: %s", cr);
 
 	if (!uri_is_rsync(uri)) {
 		pr_val_debug("Ignoring non-rsync caRepository '%s'.", cr);
diff --git a/src/object/tal.c b/src/object/tal.c
index 771d9345..5e6f0bcc 100644
--- a/src/object/tal.c
+++ b/src/object/tal.c
@@ -175,10 +175,10 @@ validate_ta(struct tal *tal, struct cache_mapping const *ta_map)
 
 static validation_verdict
 try_urls(struct tal *tal, bool (*url_is_protocol)(struct uri const *),
-    validation_verdict (*get_path)(struct uri const *, char const **))
+    validation_verdict (*get_path)(struct uri const *, char **))
 {
 	struct uri *url;
-	char const *path;
+	char *path;
 	struct cache_mapping map;
 	validation_verdict vv;
 
@@ -193,15 +193,20 @@ try_urls(struct tal *tal, bool (*url_is_protocol)(struct uri const *),
 			continue;
 
 		map.url = *url;
-		map.path = (char *)path;
+		map.path = path;
 
 		vv = validate_ta(tal, &map);
-		if (vv == VV_BUSY)
+		if (vv == VV_BUSY) {
+			free(path);
 			return VV_BUSY;
-		if (vv == VV_FAIL)
+		}
+		if (vv == VV_FAIL) {
+			free(path);
 			continue;
+		}
 
 		cache_commit_file(&map);
+		free(path);
 		return VV_CONTINUE;
 	}
 
diff --git a/src/rsync.c b/src/rsync.c
index a4dd73f8..859e4e57 100644
--- a/src/rsync.c
+++ b/src/rsync.c
@@ -335,10 +335,12 @@ post_task(struct cache_mapping *map, struct rsync_tasks *tasks,
 
 	if (tasks->a >= config_rsync_max()) {
 		LIST_INSERT_HEAD(&tasks->queued, task, lh);
-		pr_op_debug(RSP "Queued new task.");
+		pr_op_debug(RSP "Queued task %d: %s -> %s",
+		    task->pid, uri_str(&task->url), task->path);
 	} else {
 		activate_task(tasks, task, now);
-		pr_op_debug(RSP "Got new task: %d", task->pid);
+		pr_op_debug(RSP "Got new task %d: %s -> %s",
+		    task->pid, uri_str(&task->url), task->path);
 	}
 }
 
@@ -743,6 +745,9 @@ rsync_setup(char const *program, ...)
 	char const *arg;
 	int error;
 
+	if (!config_get_rsync_enabled())
+		return;
+
 	if (program != NULL) {
 		rsync_args[0] = arg = program;
 		va_start(args, program);
@@ -858,6 +863,9 @@ end:	mutex_unlock(&pssk.wrlock);
 void
 rsync_teardown(void)
 {
+	if (!config_get_rsync_enabled())
+		return;
+
 	spsk_cleanup();
 	wait_subprocess("rsync spawner", spawner);
 }
diff --git a/src/task.c b/src/task.c
index 33f5ae2e..1410a451 100644
--- a/src/task.c
+++ b/src/task.c
@@ -23,6 +23,20 @@ static bool enabled = true;
 static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_cond_t awakener = PTHREAD_COND_INITIALIZER;
 
+static char const *
+task_name(struct validation_task *task)
+{
+	switch (task->type) {
+	case VTT_RPP:
+		return uri_str(&task->u.ca->map.url);
+	case VTT_TAL:
+		return task->u.tal;
+	}
+
+	pr_crit("Unknown task type: %u", task->type);
+	return NULL;
+}
+
 static void
 task_free(struct validation_task *task)
 {
@@ -231,7 +245,7 @@ task_dequeue(struct validation_task *prev)
 			STAILQ_REMOVE_HEAD(&waiting, lh);
 			mutex_unlock(&lock);
 			pr_op_debug("task_dequeue(): Claimed task '%s'.",
-			    uri_str(&task->u.ca->map.url));
+			    task_name(task));
 			return task;
 		}
 
diff --git a/src/types/name.c b/src/types/name.c
index 7aa3aca3..76214d99 100644
--- a/src/types/name.c
+++ b/src/types/name.c
@@ -7,7 +7,6 @@
 
 #include "alloc.h"
 #include "log.h"
-#include "thread_var.h"
 
 /**
  * It's an RFC5280 name, but from RFC 6487's perspective.
@@ -186,11 +185,11 @@ end:	x509_name_put(parent_subject);
 void
 x509_name_pr_clutter(const char *prefix, X509_NAME *name)
 {
+	struct rfc5280_name *printable;
+
 	if (!pr_clutter_enabled())
 		return;
 
-	struct rfc5280_name *printable;
-
 	if (name == NULL) {
 		pr_clutter("%s: (null)", prefix);
 		return;
diff --git a/test/cache_test.c b/test/cache_test.c
index a094f7b8..2151f2db 100644
--- a/test/cache_test.c
+++ b/test/cache_test.c
@@ -136,6 +136,7 @@ static struct cache_cage *
 rsync_dance(char *url)
 {
 	/* Queue the rsync (includes rsync simulation) */
+	dl_error = 0;
 	ck_assert_ptr_eq(NULL, run_dl_rsync(url, VV_BUSY, 1));
 	/* Signal rsync completion; no need to wait */
 	finish_rsync();
@@ -148,7 +149,7 @@ run_dl_https(char const *url, unsigned int expected_calls,
     validation_verdict expected_vv, char const *expected_result)
 {
 	struct uri uri;
-	char const *result;
+	char *result = NULL;
 
 	ck_assert_ptr_eq(NULL, uri_init(&uri, url));
 
@@ -161,6 +162,7 @@ run_dl_https(char const *url, unsigned int expected_calls,
 	ck_assert_uint_eq(expected_calls, https_counter);
 
 	ck_assert_pstr_eq(expected_result, result);
+	free(result);
 	ck_assert_str_eq(VV_CONTINUE, cache_get_fallback(&uri, &result));
 	ck_assert_ptr_eq(NULL, result);
 
diff --git a/test/mock.c b/test/mock.c
index 8aaf4bd2..ff776387 100644
--- a/test/mock.c
+++ b/test/mock.c
@@ -3,6 +3,7 @@
 #include <errno.h>
 #include <arpa/inet.h>
 #include <fcntl.h>
+#include <openssl/err.h>
 #include <stdarg.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -66,7 +67,45 @@ MOCK_VOID_PRINT(pr_val_debug, PR_COLOR_DBG)
 MOCK_VOID_PRINT(pr_val_info, PR_COLOR_INF)
 MOCK_INT_PRINT(pr_val_warn, PR_COLOR_WRN, 0)
 MOCK_INT_PRINT(pr_val_err, PR_COLOR_ERR, EINVAL)
-MOCK_INT_PRINT(val_crypto_err, PR_COLOR_ERR, EINVAL)
+
+struct crypto_cb_arg {
+	unsigned int stack_size;
+	int (*error_fn)(const char *, ...);
+};
+
+static int
+log_crypto_error(const char *str, size_t len, void *_arg)
+{
+	struct crypto_cb_arg *arg = _arg;
+	arg->error_fn("-> %s", str);
+	arg->stack_size++;
+	return 1;
+}
+
+static int
+crypto_err(int (*error_fn)(const char *, ...))
+{
+	struct crypto_cb_arg arg;
+
+	error_fn("libcrypto error stack:");
+
+	arg.stack_size = 0;
+	arg.error_fn = error_fn;
+	ERR_print_errors_cb(log_crypto_error, &arg);
+	if (arg.stack_size == 0)
+		error_fn("   <Empty>");
+	else
+		error_fn("End of libcrypto stack.");
+
+	return EINVAL;
+}
+
+int
+val_crypto_err(const char *format, ...)
+{
+	MOCK_PRINT(PR_COLOR_ERR);
+	return crypto_err(pr_val_err);
+}
 
 void
 enomem_panic(void)
diff --git a/test/task_test.c b/test/task_test.c
index 2e53e2f5..8da2c458 100644
--- a/test/task_test.c
+++ b/test/task_test.c
@@ -206,12 +206,11 @@ user_thread(void *arg)
 	printf("th%d: Started.\n", thid);
 
 	while ((task = task_dequeue(task)) != NULL) {
-		printf("- th%d: Dequeued '%s'\n", thid, uri_str(&task->u.ca->map.url));
+		printf("- th%d: Dequeued '%s'\n", thid, task_name(task));
 		total_dequeued++;
 
 		if (certificate_traverse_mock(task->u.ca, thid) == EBUSY) {
-			printf("+ th%d: Requeuing '%s'\n",
-			    thid, uri_str(&task->u.ca->map.url));
+			printf("+ th%d: Requeuing '%s'\n", thid, task_name(task));
 			task_requeue_dormant(task);
 			task = NULL;
 		}