Resove FIXMEs, add some comments and unit tests.
}
static void
-update_notification_cleanup(struct update_notification *file)
+__update_notification_cleanup(struct update_notification *notif)
{
- metadata_cleanup(&file->snapshot);
- session_cleanup(&file->session);
- notification_deltas_cleanup(&file->deltas, notification_delta_cleanup);
- uri_refput(file->uri);
+ metadata_cleanup(¬if->snapshot);
+ notification_deltas_cleanup(¬if->deltas, notification_delta_cleanup);
+ uri_refput(notif->uri);
+}
+
+static void
+update_notification_cleanup(struct update_notification *notif)
+{
+ session_cleanup(¬if->session);
+ __update_notification_cleanup(notif);
}
static int
return 0;
}
+/*
+ * Initializes @old by extracting relevant data from @new.
+ * Consumes @new.
+ */
static void
-init_notif(struct update_notification *new, struct cachefile_notification *old)
+init_notif(struct cachefile_notification *old, struct update_notification *new)
{
size_t dn;
size_t i;
struct rrdp_hash *hash;
old->session = new->session;
- memset(&new->session, 0, sizeof(new->session));
STAILQ_INIT(&old->delta_hashes);
dn = config_get_rrdp_delta_threshold();
memcpy(hash->bytes, new->deltas.array[i].meta.hash, RRDP_HASH_LEN);
STAILQ_INSERT_TAIL(&old->delta_hashes, hash, hook);
}
+
+ __update_notification_cleanup(new);
}
static void
}
}
-static void
-update_notif(struct update_notification *new, struct cachefile_notification *old)
+/*
+ * Updates @old with the new information carried by @new.
+ * Consumes @new on success.
+ */
+static int
+update_notif(struct cachefile_notification *old, struct update_notification *new)
{
- BIGNUM *delta_bn;
- BN_ULONG delta;
- size_t d, dn;
+ BIGNUM *diff_bn;
+ BN_ULONG diff; /* difference between the old and new serials */
+ size_t d, dn; /* delta counter, delta "num" (total) */
struct rrdp_hash *hash;
- delta_bn = BN_new();
- if (!BN_sub(delta_bn, new->session.serial.num, old->session.serial.num)) {
- // FIXME
- }
- if (BN_is_negative(delta_bn)) {
- // FIXME
- }
+ diff_bn = BN_create();
+ if (!BN_sub(diff_bn, new->session.serial.num, old->session.serial.num))
+ return val_crypto_err("OUCH! libcrypto cannot subtract %s - %s",
+ new->session.serial.str, old->session.serial.str);
+ if (BN_is_negative(diff_bn))
+ /* The validation was the BN_cmp() in the caller. */
+ pr_crit("%s - %s < 0 despite validations.",
+ new->session.serial.str, old->session.serial.str);
- delta = BN_get_word(delta_bn);
- if (delta > new->deltas.len) {
- // FIXME
- }
+ diff = BN_get_word(diff_bn);
+ if (diff > new->deltas.len)
+ /* Should be <= because it was already compared to the delta threshold. */
+ pr_crit("%lu > %zu despite validations.",
+ diff, new->deltas.len);
BN_free(old->session.serial.num);
free(old->session.serial.str);
old->session.serial = new->session.serial;
- new->session.serial.num = NULL;
- new->session.serial.str = NULL;
- dn = delta;
+ dn = diff;
STAILQ_FOREACH(hash, &old->delta_hashes, hook)
dn++;
- for (d = new->deltas.len - delta; d < new->deltas.len; d++) {
+ for (d = new->deltas.len - diff; d < new->deltas.len; d++) {
hash = pmalloc(sizeof(struct rrdp_hash));
memcpy(hash->bytes, new->deltas.array[d].meta.hash, RRDP_HASH_LEN);
STAILQ_INSERT_TAIL(&old->delta_hashes, hash, hook);
free(hash);
dn--;
}
+
+ free(new->session.session_id);
+ __update_notification_cleanup(new);
+ return 0;
}
/*
int
rrdp_update(struct rpki_uri *uri)
{
- struct cachefile_notification **__old, *old;
+ struct cachefile_notification **cached, *old;
struct update_notification new;
bool changed;
+ int serial_cmp;
int error;
fnstack_push_uri(uri);
pr_val_debug("Processing notification.");
error = cache_download(validation_cache(state_retrieve()), uri,
- &changed, &__old);
+ &changed, &cached);
if (error)
goto end;
if (!changed) {
pr_val_debug("New session/serial: %s/%s", new.session.session_id,
new.session.serial.str);
- old = *__old;
+ old = *cached;
if (old == NULL) {
pr_val_debug("This is a new Notification.");
error = handle_snapshot(&new);
- if (!error) {
- *__old = pmalloc(sizeof(struct cachefile_notification));
- init_notif(&new, *__old);
- }
- goto revert_notification;
- }
+ if (error)
+ goto clean_notif;
- error = validate_session_desync(old, &new);
- if (error) {
- pr_val_debug("Falling back to snapshot.");
- error = handle_snapshot(&new);
- if (!error) {
- drop_notif(old);
- init_notif(&new, old);
- }
- goto revert_notification;
+ *cached = pmalloc(sizeof(struct cachefile_notification));
+ init_notif(*cached, &new);
+ goto end;
}
- if (BN_cmp(old->session.serial.num, new.session.serial.num) != 0) {
+ serial_cmp = BN_cmp(old->session.serial.num, new.session.serial.num);
+ if (serial_cmp < 0) {
pr_val_debug("The Notification's serial changed.");
+ error = validate_session_desync(old, &new);
+ if (error)
+ goto snapshot_fallback;
error = handle_deltas(&new, &old->session.serial);
- if (!error) {
- update_notif(&new, old);
- } else {
- /* Error msg already printed. */
- pr_val_debug("Falling back to snapshot.");
- error = handle_snapshot(&new);
- if (!error) {
- drop_notif(old);
- init_notif(&new, old);
- }
- }
- goto revert_notification;
+ if (error)
+ goto snapshot_fallback;
+ error = update_notif(old, &new);
+ if (!error)
+ goto end;
+ /*
+ * The files are exploded and usable, but @cached is not
+ * updatable. So drop and create it anew.
+ * We might lose some delta hashes, but it's better than
+ * re-snapshotting the next time the notification changes.
+ * Not sure if it matters. This looks so unlikely, it's
+ * practically dead code.
+ */
+ goto reset_notif;
+
+ } else if (serial_cmp > 0) {
+ pr_val_debug("Cached serial is higher than notification serial.");
+ goto snapshot_fallback;
+
+ } else {
+ pr_val_debug("The Notification changed, but the session ID and serial didn't, and no session desync was detected.");
+ goto clean_notif;
}
- pr_val_debug("The Notification changed, but the session ID and serial didn't.");
+snapshot_fallback:
+ pr_val_debug("Falling back to snapshot.");
+ error = handle_snapshot(&new);
+ if (error)
+ goto clean_notif;
+
+reset_notif:
+ drop_notif(old);
+ init_notif(old, &new);
+ goto end;
-revert_notification:
+clean_notif:
update_notification_cleanup(&new);
+
end:
fnstack_pop();
return error;
}
notif->session.serial.str = pstrdup(str);
- notif->session.serial.num = BN_new();
- if (notif->session.serial.num == NULL)
- enomem_panic();
+ notif->session.serial.num = BN_create();
if (!BN_dec2bn(¬if->session.serial.num, notif->session.serial.str)) {
error = pr_op_err("Not a serial number: %s", notif->session.serial.str);
goto revert_serial;
#include "data_structure/path_builder.h"
/**
- * Design notes:
- *
- * Because we need to generate @local from @global, @global's allowed character
- * set must be a subset of @local. Because this is Unix, @local must never
- * contain NULL (except as a terminating character). Therefore, even though IA5
- * allows NULL, @global won't.
+ * Aside from the reference counter, instances are meant to be immutable.
*
- * Because we will simply embed @global (minus "rsync://") into @local, @local's
- * encoding must be IA5-compatible. In other words, UTF-16 and UTF-32 are out of
- * the question.
+ * TODO (fine) Needs rebranding. AFAIK, RPKI does not impose significant
+ * restrictions to regular URIs (except for schema, I guess), "global URI" is
+ * pretty much tautologic, and "local URI" is a misnomer. (Because it doesn't
+ * have anything to do with 'interpretation is independent of access'.)
+ * I can't even remember if this nomenclature made sense at some point.
+ * It's more of a mapping than a URI.
*
- * Aside from the reference counter, instances are meant to be immutable.
+ * TODO (fine) Also, this structure is so intertwined with the cache module,
+ * nowadays it feels like it should be moved there.
*/
struct rpki_uri {
/**
* "Global URI".
* The one that always starts with "rsync://" or "https://".
- *
- * These things are IA5-encoded, which means you're not bound to get
- * non-ASCII characters.
+ * Normalized, ASCII-only, NULL-terminated.
*/
char *global;
/**
* "Local URI".
* The file pointed by @global, but cached in the local filesystem.
- *
- * I can't find a standard that defines this, but lots of complaints on
- * the Internet imply that Unix file paths are specifically meant to be
- * C strings.
- *
- * So just to clarify: This is a string that permits all characters,
- * printable or otherwise, except \0. (Because that's the terminating
- * character.)
- *
- * Even though it might contain characters that are non-printable
- * according to ASCII, we assume that we can just dump it into the
- * output without trouble, because the input should have the same
- * encoding as the output.
- *
- * Technically, "global" URI "https://a.b.c/d/..///./d" is not the same
- * identifier as "https://a.b.c/d", but since we're supposed to download
- * to a filesystem where "https/a.b.c/d" is the same file as
- * "https/a.b.c/d/..///./d", @local will always be normalized.
+ * Normalized, ASCII-only, NULL-terminated.
+ * Sometimes NULL, depending on @type.
*/
char *local;
- /* "local_len" is never needed right now. */
enum uri_type type;
- unsigned int references;
+ unsigned int references; /* Reference counter */
};
/*
pr_crit("Unknown URI type: %u", uri->type);
}
-/*
- * I think the reason why @guri is not a char * is to convey that it doesn't
- * need to be NULL terminated, but I'm not sure.
- *
- * FIXME callers now need to ensure @guri is NULL-terminated.
- */
int
uri_create(struct rpki_uri **result, char const *tal, enum uri_type type,
struct rpki_uri *notif, char const *guri)
#include "asn1/asn1c/IA5String.h"
#include "data_structure/array_list.h"
+/*
+ * "Long" time = seven days.
+ * Currently hardcoded, but queued for tweakability.
+ */
enum uri_type {
- UT_TA_RSYNC, /* TAL's TA URL; downloaded via rsync. */
- UT_TA_HTTP, /* TAL's TA URL; downloaded via HTTP. */
- UT_RPP, /* caRepository; downloaded via rsync. */
- UT_NOTIF, /* rpkiNotify; downloaded via HTTP. */
- UT_TMP, /* Snapshot or delta; Downloaded via HTTP. */
- UT_CAGED, /* Endangered species. */
-
- UT_AIA, /* caIssuers. Not downloaded. */
- UT_SO, /* signedObject. Not downloaded. */
- UT_MFT, /* rpkiManifest. Not downloaded. */
+ /*
+ * TAL's TA URL.
+ * The file is cached until it's untraversed for a "long" time.
+ */
+ UT_TA_RSYNC,
+ UT_TA_HTTP,
+
+ /*
+ * (rsync) Repository Publication Point. RFC 6481.
+ * The directory is cached until it's untraversed for a "long" time.
+ */
+ UT_RPP,
+
+ /*
+ * An RRDP notification file; downloaded via HTTP.
+ * The file itself is not cached, but we preserve a handful of metadata
+ * that is needed in subsequent iterations.
+ * The metadata is cached until it's untraversed for a "long" time.
+ */
+ UT_NOTIF,
+
+ /*
+ * RRDP Snapshot or Delta; downloaded via HTTP.
+ * The file itself is not cached, but we preserve some small metadata.
+ * The metadata is destroyed once the iteration finishes.
+ */
+ UT_TMP,
+
+ /*
+ * Endangered species; bound to be removed once RFC 9286 is implemented.
+ */
+ UT_CAGED,
+
+ UT_AIA, /* caIssuers. Not directly downloaded. */
+ UT_SO, /* signedObject. Not directly downloaded. */
+ UT_MFT, /* rpkiManifest. Not directly downloaded. */
};
struct rpki_uri;
MOCK(state_retrieve, struct validation *, NULL, void)
MOCK(validation_tal, struct tal *, NULL, struct validation *state)
MOCK(tal_get_file_name, char const *, "", struct tal *tal)
-MOCK_UINT(config_get_rrdp_delta_threshold, 64, void)
+MOCK_UINT(config_get_rrdp_delta_threshold, 5, void)
/* Mocks end */
ck_assert_uint_eq(expected_hash[i], hash[i]);
}
+static void
+init_serial(struct rrdp_serial *serial, unsigned long num)
+{
+ char *tmp;
+
+ serial->num = BN_create();
+ ck_assert_int_eq(1, BN_add_word(serial->num, num));
+
+ tmp = BN_bn2dec(serial->num);
+ ck_assert_ptr_ne(NULL, tmp);
+ serial->str = pstrdup(tmp);
+ OPENSSL_free(tmp);
+}
+
+static void
+init_rrdp_session(struct rrdp_session *session, unsigned long serial)
+{
+ session->session_id = pstrdup("session");
+ init_serial(&session->serial, serial);
+}
+
+static void
+init_cachefile_notif(struct cachefile_notification **result, unsigned long serial, ...)
+{
+ struct cachefile_notification *notif;
+ va_list args;
+ int hash_byte;
+ struct rrdp_hash *hash;
+ size_t i;
+
+ notif = pmalloc(sizeof(struct cachefile_notification));
+ *result = notif;
+
+ init_rrdp_session(¬if->session, serial);
+ STAILQ_INIT(¬if->delta_hashes);
+
+ va_start(args, serial);
+ while ((hash_byte = va_arg(args, int)) >= 0) {
+ hash = pmalloc(sizeof(struct rrdp_hash));
+ for (i = 0; i < RRDP_HASH_LEN; i++)
+ hash->bytes[i] = hash_byte;
+ STAILQ_INSERT_TAIL(¬if->delta_hashes, hash, hook);
+ }
+ va_end(args);
+}
+
+static void
+init_regular_notif(struct update_notification *notif, unsigned long serial, ...)
+{
+ va_list args;
+ int hash_byte;
+ struct notification_delta delta;
+ size_t i;
+
+ memset(notif, 0, sizeof(*notif));
+ init_rrdp_session(¬if->session, serial);
+ notification_deltas_init(¬if->deltas);
+
+ va_start(args, serial);
+ while ((hash_byte = va_arg(args, int)) >= 0) {
+ init_serial(&delta.serial, serial--);
+ delta.meta.uri = NULL; /* Not needed for now */
+ delta.meta.hash = pmalloc(RRDP_HASH_LEN);
+ for (i = 0; i < RRDP_HASH_LEN; i++)
+ delta.meta.hash[i] = hash_byte;
+ delta.meta.hash_len = RRDP_HASH_LEN;
+ notification_deltas_add(¬if->deltas, &delta);
+ }
+ va_end(args);
+}
+
+static void
+validate_cachefile_notif(struct cachefile_notification *notif, unsigned long __serial, ...)
+{
+ struct rrdp_serial serial;
+ va_list args;
+ int hash_byte;
+ struct rrdp_hash *hash;
+ size_t i;
+
+ ck_assert_str_eq("session", notif->session.session_id);
+ init_serial(&serial, __serial);
+ ck_assert_str_eq(serial.str, notif->session.serial.str);
+ ck_assert_int_eq(0, BN_cmp(serial.num, notif->session.serial.num));
+ serial_cleanup(&serial);
+
+ hash = STAILQ_FIRST(¬if->delta_hashes);
+
+ va_start(args, __serial);
+ while ((hash_byte = va_arg(args, int)) >= 0) {
+ ck_assert_ptr_ne(NULL, hash);
+ for (i = 0; i < RRDP_HASH_LEN; i++)
+ ck_assert_int_eq(hash_byte, hash->bytes[i]);
+ hash = STAILQ_NEXT(hash, hook);
+ }
+ va_end(args);
+
+ ck_assert_ptr_eq(NULL, hash);
+
+ rrdp_notif_free(notif);
+}
+
+START_TEST(test_update_notif)
+{
+ struct cachefile_notification *old;
+ struct update_notification new;
+
+ /* No changes */
+ init_cachefile_notif(&old, 5555, 1, 2, 3, -1);
+ init_regular_notif(&new, 5555, 1, 2, 3, -1);
+ ck_assert_int_eq(0, update_notif(old, &new));
+ validate_cachefile_notif(old, 5555, 1, 2, 3, -1);
+
+ /* Add a few serials */
+ init_cachefile_notif(&old, 5555, 1, 2, 3, -1);
+ init_regular_notif(&new, 5557, 3, 4, 5, -1);
+ ck_assert_int_eq(0, update_notif(old, &new));
+ validate_cachefile_notif(old, 5557, 1, 2, 3, 4, 5, -1);
+
+ /* Add serials, delta threshold exceeded */
+ init_cachefile_notif(&old, 5555, 1, 2, 3, -1);
+ init_regular_notif(&new, 5558, 3, 4, 5, 6, -1);
+ ck_assert_int_eq(0, update_notif(old, &new));
+ validate_cachefile_notif(old, 5558, 2, 3, 4, 5, 6, -1);
+
+ /* All new serials, but no hashes skipped */
+ init_cachefile_notif(&old, 5555, 1, 2, 3, -1);
+ init_regular_notif(&new, 5557, 4, 5, -1);
+ ck_assert_int_eq(0, update_notif(old, &new));
+ validate_cachefile_notif(old, 5557, 1, 2, 3, 4, 5, -1);
+
+ /* 2 previous tests combined */
+ init_cachefile_notif(&old, 5555, 1, 2, 3, 4, 5, -1);
+ init_regular_notif(&new, 5560, 6, 7, 8, 9, 10, -1);
+ ck_assert_int_eq(0, update_notif(old, &new));
+ validate_cachefile_notif(old, 5560, 6, 7, 8, 9, 10, -1);
+}
+END_TEST
+
START_TEST(test_parse_notification_ok)
{
struct rpki_uri uri = { 0 };
tcase_add_test(misc, test_xmlChar_NULL_assumption);
tcase_add_test(misc, test_hexstr2sha256);
tcase_add_test(misc, test_sort_deltas);
+ tcase_add_test(misc, test_update_notif);
parse = tcase_create("parse");
tcase_add_test(parse, test_parse_notification_ok);