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 b3bcd60dcb8c59eeb1597037d987794ab533c569..a22bfa0fdb327e84801c146ffea3653115d462c6 100644 (file)
--- 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 9d04a1da4fc37751e1e88fc1396898af7d9476c7..d78b125f230bd69adf93b979c00658f81265bdbc 100644 (file)
--- 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 5e033c053249646dde55c875f60141dfe9046001..d4dab681208bd8d3479183910c541cf7f03e20f1 100644 (file)
--- 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 d81751cca25977530d4547a08f38b14676b9c203..5a3eaaebeff4937eb6e43d3403f0e0317348866f 100644 (file)
--- 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 fce74a417853c93f4bf4108d40578060f12412ad..870796599b88802855784c939b61dab34a7f3198 100644 (file)
--- 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 771d934537d10d11cec4427fcabdd2758b80d7e2..5e6f0bcc911efbd28ad759883d82e3f31263ab3b 100644 (file)
--- 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 a4dd73f882f89a4f14a04fffd79a042a8fa257de..859e4e572a271e5b0a0ff2022c488baf8549e5ef 100644 (file)
--- 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 33f5ae2ee2c6903e7d25c5b4510abccd53e38d35..1410a4513288a3cfd0291f88b835d317b0f34a7f 100644 (file)
--- 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 7aa3aca3a33eb2dd11f56a32ab3e6ad8c1d8f11b..76214d990e9f512acec6048d9fdf5c7cd7e10337 100644 (file)
--- 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 a094f7b86cc0ad51b1af747c7046c4846083950a..2151f2db6ee6cff1a832864ab3bddef6910878b3 100644 (file)
--- 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 8aaf4bd264357d2229e20ea11b9058c43bff9004..ff7763875338297db9b3de3933c7686acabcfb40 100644 (file)
--- 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 2e53e2f5eeaa3b2a19abdebf1f72d1e4b2fb91d7..8da2c4585357a924bb3587f2192af6537d1cef25 100644 (file)
--- 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;
                }