From 958f3e4d50364e887b0a605113c9cce78cfa5cbb Mon Sep 17 00:00:00 2001 From: Alberto Leiva Popper Date: Mon, 11 Mar 2024 14:22:26 -0600 Subject: [PATCH] Force RRDP files to be hosted by the same origin Reject RRDP snapshot and deltas if they're not hosted in the same origin as the notification. Prevents malicious notifications from wasting other servers' bandwidth. First half of draft-spaghetti-sidrops-rrdp-same-origin-00. Thanks to Job Snijders for reporting this. --- src/rrdp.c | 24 +++++++++++++++++++++-- src/types/uri.c | 30 +++++++++++++++++++++++++++++ src/types/uri.h | 1 + test/rrdp_test.c | 44 +++++++++++++++++++++---------------------- test/types/uri_test.c | 33 ++++++++++++++++++++++++++++++++ 5 files changed, 107 insertions(+), 25 deletions(-) diff --git a/src/rrdp.c b/src/rrdp.c index 7f3d52df..f99849d0 100644 --- a/src/rrdp.c +++ b/src/rrdp.c @@ -589,6 +589,23 @@ handle_withdraw(xmlTextReaderPtr reader, struct rpki_uri *notif) return error; } +static int +parse_notification_snapshot(xmlTextReaderPtr reader, + struct update_notification *notif) +{ + int error; + + error = parse_file_metadata(reader, NULL, HR_MANDATORY, ¬if->snapshot); + if (error) + return error; + + if (!uri_same_origin(notif->uri, notif->snapshot.uri)) + return pr_val_err("Notification %s and Snapshot %s are not hosted by the same origin.", + uri_get_global(notif->uri), uri_get_global(notif->snapshot.uri)); + + return 0; +} + static int parse_notification_delta(xmlTextReaderPtr reader, struct update_notification *notif) @@ -606,6 +623,10 @@ parse_notification_delta(xmlTextReaderPtr reader, return error; } + if (!uri_same_origin(notif->uri, delta.meta.uri)) + return pr_val_err("Notification %s and Delta %s are not hosted by the same origin.", + uri_get_global(notif->uri), uri_get_global(delta.meta.uri)); + notification_deltas_add(¬if->deltas, &delta); return 0; } @@ -710,8 +731,7 @@ xml_read_notif(xmlTextReaderPtr reader, void *arg) if (xmlStrEqual(name, BAD_CAST RRDP_ELEM_DELTA)) { return parse_notification_delta(reader, notif); } else if (xmlStrEqual(name, BAD_CAST RRDP_ELEM_SNAPSHOT)) { - return parse_file_metadata(reader, NULL, HR_MANDATORY, - ¬if->snapshot); + return parse_notification_snapshot(reader, notif); } else if (xmlStrEqual(name, BAD_CAST RRDP_ELEM_NOTIFICATION)) { /* No need to validate session ID and serial */ return parse_session(reader, ¬if->session); diff --git a/src/types/uri.c b/src/types/uri.c index 919eef99..4b64d863 100644 --- a/src/types/uri.c +++ b/src/types/uri.c @@ -529,6 +529,36 @@ uri_equals(struct rpki_uri *u1, struct rpki_uri *u2) return strcmp(u1->global, u2->global) == 0; } +bool +uri_same_origin(struct rpki_uri *u1, struct rpki_uri *u2) +{ + char const *g1, *g2; + size_t c, slashes; + + g1 = u1->global; + g2 = u2->global; + slashes = 0; + + for (c = 0; g1[c] == g2[c]; c++) { + switch (g1[c]) { + case '/': + slashes++; + if (slashes == 3) + return true; + break; + case '\0': + return slashes == 2; + } + } + + if (g1[c] == '\0') + return (slashes == 2) && g2[c] == '/'; + if (g2[c] == '\0') + return (slashes == 2) && g1[c] == '/'; + + return false; +} + /* @ext must include the period. */ bool uri_has_extension(struct rpki_uri *uri, char const *ext) diff --git a/src/types/uri.h b/src/types/uri.h index ea770692..3105757c 100644 --- a/src/types/uri.h +++ b/src/types/uri.h @@ -71,6 +71,7 @@ char const *uri_get_global(struct rpki_uri *); char const *uri_get_local(struct rpki_uri *); bool uri_equals(struct rpki_uri *, struct rpki_uri *); +bool uri_same_origin(struct rpki_uri *, struct rpki_uri *); bool uri_has_extension(struct rpki_uri *, char const *); bool uri_is_certificate(struct rpki_uri *); diff --git a/test/rrdp_test.c b/test/rrdp_test.c index b752646f..f15bcf0e 100644 --- a/test/rrdp_test.c +++ b/test/rrdp_test.c @@ -458,15 +458,24 @@ START_TEST(test_update_notif) } END_TEST +static void +init_uri(struct rpki_uri *uri, char *global, char *local, enum uri_type type) +{ + uri->global = global; + uri->local = local; + uri->type = type; + uri->references = 1; +} + +#define init_notif_uri(u, g, l) init_uri(u, g, l, UT_NOTIF) + START_TEST(test_parse_notification_ok) { - struct rpki_uri uri = { 0 }; + struct rpki_uri uri; struct update_notification notif; ck_assert_int_eq(0, relax_ng_init()); - uri.local = "resources/rrdp/notif-ok.xml"; - uri.type = UT_NOTIF; - uri.references = 1; + init_notif_uri(&uri, "https://host/notification.xml", "resources/rrdp/notif-ok.xml"); ck_assert_int_eq(0, parse_notification(&uri, ¬if)); ck_assert_str_eq("9df4b597-af9e-4dca-bdda-719cce2c4e28", (char const *)notif.session.session_id); @@ -495,13 +504,11 @@ END_TEST START_TEST(test_parse_notification_0deltas) { - struct rpki_uri uri = { 0 }; + struct rpki_uri uri; struct update_notification notif; ck_assert_int_eq(0, relax_ng_init()); - uri.local = "resources/rrdp/notif-0deltas.xml"; - uri.type = UT_NOTIF; - uri.references = 1; + init_notif_uri(&uri, "https://host/notification.xml", "resources/rrdp/notif-0deltas.xml"); ck_assert_int_eq(0, parse_notification(&uri, ¬if)); ck_assert_str_eq("9df4b597-af9e-4dca-bdda-719cce2c4e28", (char const *)notif.session.session_id); @@ -520,13 +527,11 @@ END_TEST START_TEST(test_parse_notification_large_serial) { - struct rpki_uri uri = { 0 }; + struct rpki_uri uri; struct update_notification notif; ck_assert_int_eq(0, relax_ng_init()); - uri.local = "resources/rrdp/notif-large-serial.xml"; - uri.type = UT_NOTIF; - uri.references = 1; + init_notif_uri(&uri, "https://host/notification.xml", "resources/rrdp/notif-large-serial.xml"); ck_assert_int_eq(0, parse_notification(&uri, ¬if)); ck_assert_str_eq("9df4b597-af9e-4dca-bdda-719cce2c4e28", (char const *)notif.session.session_id); @@ -552,13 +557,11 @@ END_TEST static void test_parse_notification_error(char *file) { - struct rpki_uri uri = { 0 }; + struct rpki_uri uri; struct update_notification notif; ck_assert_int_eq(0, relax_ng_init()); - uri.local = file; - uri.type = UT_NOTIF; - uri.references = 1; + init_notif_uri(&uri, "https://host/notification.xml", file); ck_assert_int_eq(-EINVAL, parse_notification(&uri, ¬if)); relax_ng_cleanup(); @@ -620,13 +623,8 @@ START_TEST(test_parse_snapshot_bad_publish) ck_assert_int_eq(0, relax_ng_init()); - notif_uri.global = "https://example.com/notification.xml"; - notif_uri.local = "cache/example.com/notification.xml"; - notif_uri.type = UT_NOTIF; - notif_uri.references = 1; - - snapshot_uri.local = "resources/rrdp/snapshot-bad-publish.xml"; - snapshot_uri.references = 1; + init_notif_uri(¬if_uri, "https://example.com/notification.xml", "cache/example.com/notification.xml"); + init_uri(&snapshot_uri, "https://example.com/snapshot.xml", "resources/rrdp/snapshot-bad-publish.xml", UT_TMP); notif.session.session_id = "9df4b597-af9e-4dca-bdda-719cce2c4e28"; notif.session.serial.str = "2"; diff --git a/test/types/uri_test.c b/test/types/uri_test.c index 46585074..fcc47a5a 100644 --- a/test/types/uri_test.c +++ b/test/types/uri_test.c @@ -196,6 +196,38 @@ START_TEST(check_caged) } END_TEST +static bool +same_origin(char *g1, char *g2) +{ + struct rpki_uri u1 = { .type = UT_NOTIF, .references = 1 }; + struct rpki_uri u2 = { .type = UT_TMP, .references = 1 }; + u1.global = g1; + u2.global = g2; + return uri_same_origin(&u1, &u2); +} + +START_TEST(test_same_origin) +{ + ck_assert_int_eq(true, same_origin("https://a.b.c/d/e/f", "https://a.b.c/g/h/i")); + ck_assert_int_eq(false, same_origin("https://a.b.cc/d/e/f", "https://a.b.c/g/h/i")); + ck_assert_int_eq(false, same_origin("https://a.b.c/d/e/f", "https://a.b.cc/g/h/i")); + ck_assert_int_eq(true, same_origin("https://a.b.c", "https://a.b.c")); + ck_assert_int_eq(true, same_origin("https://a.b.c/", "https://a.b.c")); + ck_assert_int_eq(true, same_origin("https://a.b.c", "https://a.b.c/")); + ck_assert_int_eq(true, same_origin("https://", "https://")); + ck_assert_int_eq(false, same_origin("https://", "https://a")); + ck_assert_int_eq(false, same_origin("https://a", "https://b")); + + /* Undefined, but manhandle the code anyway */ + ck_assert_int_eq(false, same_origin("", "")); + ck_assert_int_eq(false, same_origin("ht", "ht")); + ck_assert_int_eq(false, same_origin("https:", "https:")); + ck_assert_int_eq(false, same_origin("https:/", "https:/")); + ck_assert_int_eq(false, same_origin("https:/a", "https:/a")); + ck_assert_int_eq(true, same_origin("https:/a/", "https:/a/")); +} +END_TEST + static Suite *address_load_suite(void) { Suite *suite; @@ -205,6 +237,7 @@ static Suite *address_load_suite(void) tcase_add_test(core, test_constructor); tcase_add_test(core, check_validate_current_directory); tcase_add_test(core, check_caged); + tcase_add_test(core, test_same_origin); suite = suite_create("Encoding checking"); suite_add_tcase(suite, core); -- 2.47.2