From 3f37220c86ca38243f36e5b59c56a4ed8d7bf9b4 Mon Sep 17 00:00:00 2001 From: Alberto Leiva Popper Date: Tue, 28 Sep 2021 16:52:10 -0500 Subject: [PATCH] General review - Increase default remote object file size limit, because RRDP snapshot files can be very large. (Current: ~148 MB, double on key rollover) - Treat HTTP redirects as errors. - Before: Redirects were treated as successes, but Fort didn't bother to follow the redirect. As a result, the code was either not finding the file, or finding an empty file. - Now: Redirects are treated as errors. (Not sure if I'm meant to do something here; curl doesn't do it automatically, and RFCs are silent. In particular, I'm not in the mood to have to deal with redirect loops and whatnot.) - Remove ROA database clone operation in the SLURM code. According to the code, it was working on a clone "so that updates can be reverted" (on error, I presume.) But it was never reverting them. - Refactor SLURM cleanup code. I don't remember this one very well. Starting from the clone removal, I got distracted with some inconsistent cleanups. Patched a buggy cleanup somewhere. There's still more I want to do to the SLURM code; it looks somewhat slow. Dirty; needs testing. --- docs/usage.md | 6 +- src/config.c | 87 ++++++---- src/config.h | 2 + src/data_structure/uthash_nonfatal.h | 2 + src/http/http.c | 18 +- src/rrdp/rrdp_loader.c | 1 + src/rsync/rsync.c | 2 +- src/rtr/db/db_table.c | 75 -------- src/rtr/db/db_table.h | 2 - src/rtr/db/delta.c | 6 +- src/rtr/db/vrps.c | 13 +- src/slurm/db_slurm.c | 52 ++---- src/slurm/db_slurm.h | 7 +- src/slurm/slurm_loader.c | 251 ++++++++++----------------- src/slurm/slurm_loader.h | 2 +- src/slurm/slurm_parser.c | 8 +- src/slurm/slurm_parser.h | 9 - 17 files changed, 208 insertions(+), 335 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index 2de3c2bc..9cfcd2d7 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -733,7 +733,7 @@ The value specified (either by the argument or the default value) is utilized in - **Type:** Integer - **Availability:** `argv` and JSON -- **Default:** 30 +- **Default:** 100000 (100 kilobytes/second) - **Range:** 0--[`UINT_MAX`](http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html) The value Fort employs as [CURLOPT_LOW_SPEED_LIMIT](https://curl.haxx.se/libcurl/c/CURLOPT_LOW_SPEED_LIMIT.html) during every HTTP transfer. @@ -769,14 +769,14 @@ See [`--http.low-speed-limit`](#--httplow-speed-limit). - **Type:** Integer - **Availability:** `argv` and JSON -- **Default:** 10,000,000 (10 Megabytes) +- **Default:** 1,000,000,000 (1 Gigabyte) - **Range:** 0--2,000,000,000 (2 Gigabytes) The maximum amount of bytes files are allowed to length during HTTP transfers. Files that exceed this limit are dropped, either early (through [CURLOPT_MAXFILESIZE](https://curl.haxx.se/libcurl/c/CURLOPT_MAXFILESIZE.html)) or as they hit the limit (when the file size is not known prior to download). This is intended to prevent malicious RPKI repositories from stagnating Fort. -As of 2021-09-20, the largest legitimate file I found in the repositories was aprox. 1120 kilobytes. +As of 2021-10-05, the largest legitimate file in the repositories is an RRDP snapshot that weights ~150 megabytes. ### `--http.ca-path` diff --git a/src/config.c b/src/config.c index d4fdb7d2..47f022e7 100644 --- a/src/config.c +++ b/src/config.c @@ -215,6 +215,7 @@ struct rpki_config { bool init_tals; /* Download AS0 TALs into --tal? */ bool init_tal0s; + /* Deprecated; currently does nothing. */ unsigned int init_tal_locations; @@ -229,6 +230,11 @@ struct rpki_config { unsigned int max; } validation; } thread_pool; + + struct { + unsigned int asns_per_prefix; + unsigned int prefixes_per_asn; + } roa; }; static void print_usage(FILE *, bool); @@ -470,7 +476,7 @@ static const struct option_field options[] = { .doc = "Priority of execution to fetch repositories files, a higher value means higher priority", .min = 0, .max = 100, - },{ + }, { .id = 3002, .name = "rsync.strategy", .type = >_rsync_strategy, @@ -492,7 +498,7 @@ static const struct option_field options[] = { .doc = "Period (in seconds) to wait between retries after an RSYNC error ocurred", .min = 0, .max = UINT_MAX, - },{ + }, { .id = 3005, .name = "rsync.program", .type = >_string, @@ -560,8 +566,7 @@ static const struct option_field options[] = { .type = >_rrdp_enabled, .offset = offsetof(struct rpki_config, http.enabled), .doc = "Enables outgoing HTTP requests", - }, - { + }, { .id = 9001, .name = "http.priority", .type = >_rrdp_priority, @@ -569,8 +574,7 @@ static const struct option_field options[] = { .doc = "Priority of execution to fetch repositories files, a higher value means higher priority", .min = 0, .max = 100, - }, - { + }, { .id = 9002, .name = "http.retry.count", .type = >_rrdp_retry_count, @@ -578,8 +582,7 @@ static const struct option_field options[] = { .doc = "Maximum amount of retries whenever there's an error requesting HTTP URIs", .min = 0, .max = UINT_MAX, - }, - { + }, { .id = 9003, .name = "http.retry.interval", .type = >_rrdp_retry_interval, @@ -587,15 +590,13 @@ static const struct option_field options[] = { .doc = "Period (in seconds) to wait between retries after an error ocurred doing HTTP requests", .min = 0, .max = UINT_MAX, - }, - { + }, { .id = 9004, .name = "http.user-agent", .type = >_string, .offset = offsetof(struct rpki_config, http.user_agent), .doc = "User-Agent to use at HTTP requests, eg. Fort Validator Local/1.0", - }, - { + }, { .id = 9005, .name = "http.connect-timeout", .type = >_uint, @@ -603,8 +604,7 @@ static const struct option_field options[] = { .doc = "Timeout for the connect phase", .min = 1, .max = UINT_MAX, - }, - { + }, { .id = 9006, .name = "http.transfer-timeout", .type = >_uint, @@ -612,8 +612,7 @@ static const struct option_field options[] = { .doc = "Maximum transfer time (once the connection is established) before dropping the connection", .min = 0, .max = UINT_MAX, - }, - { + }, { .id = 9007, .name = "http.idle-timeout", /* TODO DEPRECATED. */ .type = >_uint, @@ -621,8 +620,7 @@ static const struct option_field options[] = { .doc = "Deprecated; currently an alias for --http.low-speed-time. Use --http.low-speed-time instead.", .min = 0, .max = UINT_MAX, - }, - { + }, { .id = 9009, .name = "http.low-speed-limit", .type = >_uint, @@ -630,8 +628,7 @@ static const struct option_field options[] = { .doc = "Average transfer speed (in bytes per second) that the transfer should be below during --http.low-speed-time seconds for Fort to consider it to be too slow. (Slow connections are dropped.)", .min = 0, .max = UINT_MAX, - }, - { + }, { .id = 9010, .name = "http.low-speed-time", .type = >_uint, @@ -639,8 +636,7 @@ static const struct option_field options[] = { .doc = "Seconds that the transfer speed should be below --http.low-speed-limit for the Fort to consider it too slow. (Slow connections are dropped.)", .min = 0, .max = UINT_MAX, - }, - { + }, { .id = 9011, .name = "http.max-file-size", .type = >_uint, @@ -648,8 +644,7 @@ static const struct option_field options[] = { .doc = "Fort will refuse to download files larger than this number of bytes.", .min = 0, .max = 2000000000, - }, - { + }, { .id = 9008, .name = "http.ca-path", .type = >_string, @@ -790,8 +785,7 @@ static const struct option_field options[] = { .doc = "ASN1 decoder max stack size, utilized to avoid a stack overflow on large nested ASN1 objects", .min = 1, .max = UINT_MAX, - }, - { + }, { .id = 8001, .name = "stale-repository-period", .type = >_uint, @@ -832,8 +826,7 @@ static const struct option_field options[] = { .doc = "Number of threads in the RTR client request thread pool. Also known as the maximum number of client requests the RTR server will be able to handle at the same time.", .min = 1, .max = UINT_MAX, - }, - { + }, { .id = 12001, .name = "thread-pool.validation.max", .type = >_uint, @@ -844,6 +837,24 @@ static const struct option_field options[] = { .max = 100, }, + { + .id = 13000, + .name = "roa.max_asns_per_prefix", + .type = >_uint, + .offset = offsetof(struct rpki_config, roa.asns_per_prefix), + .doc = "Maximum number of Autonomous Systems that are allowed to advertise each prefix. (Maximum number of ROAs that are allowed to share any given prefix.)", + .min = 0, + .max = UINT_MAX, + }, { + .id = 13001, + .name = "roa.max_prefixes_per_asn", + .type = >_uint, + .offset = offsetof(struct rpki_config, roa.prefixes_per_asn), + .doc = "Maximum number of prefixes that are allowed to be advertised by each Autonomous System. (Maximum number of ROAs that are allowed to share any given ASN.)", + .min = 0, + .max = UINT_MAX, + }, + { 0 }, }; @@ -1069,9 +1080,9 @@ set_default_values(void) } rpki_config.http.connect_timeout = 30; rpki_config.http.transfer_timeout = 0; - rpki_config.http.low_speed_limit = 30; + rpki_config.http.low_speed_limit = 100000; rpki_config.http.low_speed_time = 10; - rpki_config.http.max_file_size = 10000000; + rpki_config.http.max_file_size = 1000000000; rpki_config.http.ca_path = NULL; /* Use system default */ /* @@ -1123,6 +1134,10 @@ set_default_values(void) /* Usually 5 TALs, let a few more available */ rpki_config.thread_pool.validation.max = 5; + /* TODO adjust */ + rpki_config.roa.asns_per_prefix = 1024; + rpki_config.roa.prefixes_per_asn = 1024; + return 0; revert_validation_log_tag: @@ -1656,6 +1671,18 @@ config_get_thread_pool_validation_max(void) return rpki_config.thread_pool.validation.max; } +unsigned int +config_get_max_asn_per_pfx(void) +{ + return rpki_config.roa.asns_per_prefix; +} + +unsigned int +config_get_max_pfx_per_asn(void) +{ + return rpki_config.roa.prefixes_per_asn; +} + void config_set_rsync_enabled(bool value) { diff --git a/src/config.h b/src/config.h index f5ffe1b3..8af73aba 100644 --- a/src/config.h +++ b/src/config.h @@ -57,6 +57,8 @@ unsigned int config_get_asn1_decode_max_stack(void); unsigned int config_get_stale_repository_period(void); unsigned int config_get_thread_pool_server_max(void); unsigned int config_get_thread_pool_validation_max(void); +unsigned int config_get_max_asn_per_pfx(void); +unsigned int config_get_max_pfx_per_asn(void); /* Logging getters */ bool config_get_op_log_enabled(void); diff --git a/src/data_structure/uthash_nonfatal.h b/src/data_structure/uthash_nonfatal.h index 5743dda4..76fbef6b 100644 --- a/src/data_structure/uthash_nonfatal.h +++ b/src/data_structure/uthash_nonfatal.h @@ -14,6 +14,8 @@ * This validation (check for errno) must be done on ops that allocate memory, * so set 'errno' to 0 before this ops are made. The 'obj' won't be freed, * this is the caller's responsibility. + * + * TODO I think most of the code is not checking this. */ #define HASH_NONFATAL_OOM 1 #define uthash_nonfatal_oom(obj) \ diff --git a/src/http/http.c b/src/http/http.c index 9a9f81fe..3534eb0b 100644 --- a/src/http/http.c +++ b/src/http/http.c @@ -179,7 +179,7 @@ get_http_response_code(struct http_handler *handler, long *http_code, CURLcode res; res = curl_easy_getinfo(handler->curl, CURLINFO_RESPONSE_CODE, - &http_code); + http_code); if (res != CURLE_OK) { return pr_op_err("curl_easy_getinfo(CURLINFO_RESPONSE_CODE) returned %d (%s). " "I think this is supposed to be illegal, so I'll have to drop URI '%s'.", @@ -254,8 +254,22 @@ http_fetch(struct http_handler *handler, char const *uri, long *response_code, } } - if (http_code >= 400) + if (http_code >= 400) { + pr_val_err("HTTP result code: %ld", http_code); return handle_http_response_code(http_code); + } + if (http_code >= 300) { + /* + * If you're ever forced to implement this, please remember that + * a malicious server can send us on a wild chase with infinite + * redirects, so there needs to be a limit. + */ + pr_val_err("HTTP result code: %ld. I don't follow redirects; discarding file.", + http_code); + return -EINVAL; /* Do not retry. */ + } + + pr_val_debug("HTTP result code: %ld", http_code); /* * Scenario: Received an OK code, but the time condition diff --git a/src/rrdp/rrdp_loader.c b/src/rrdp/rrdp_loader.c index e0aeb439..ea68709c 100644 --- a/src/rrdp/rrdp_loader.c +++ b/src/rrdp/rrdp_loader.c @@ -251,6 +251,7 @@ upd_end: return upd_error; } else { /* Reset RSYNC visited URIs, this may force the update */ + /* TODO um, what? */ reset_downloaded(); } diff --git a/src/rsync/rsync.c b/src/rsync/rsync.c index 05640568..3fab9f03 100644 --- a/src/rsync/rsync.c +++ b/src/rsync/rsync.c @@ -93,7 +93,7 @@ is_already_downloaded(struct rpki_uri *uri, struct uri_list *visited_uris) { struct uri *cursor; - /* TODO (next iteration) this is begging for a radix trie. */ + /* TODO (next iteration) this is begging for a hash set. */ SLIST_FOREACH(cursor, visited_uris, next) if (is_descendant(cursor->uri, uri)) return true; diff --git a/src/rtr/db/db_table.c b/src/rtr/db/db_table.c index 58a095e3..60f21841 100644 --- a/src/rtr/db/db_table.c +++ b/src/rtr/db/db_table.c @@ -132,81 +132,6 @@ add_router_key(struct db_table *table, struct hashable_key *new) return 0; } -static int -duplicate_roa(struct db_table *dst, struct hashable_roa *new) -{ - struct vrp vrp; - struct ipv4_prefix prefix4; - struct ipv6_prefix prefix6; - - vrp = new->data; - switch (vrp.addr_fam) { - case AF_INET: - prefix4.addr = vrp.prefix.v4; - prefix4.len = vrp.prefix_length; - return rtrhandler_handle_roa_v4(dst, vrp.asn, &prefix4, - vrp.max_prefix_length); - case AF_INET6: - prefix6.addr = vrp.prefix.v6; - prefix6.len = vrp.prefix_length; - return rtrhandler_handle_roa_v6(dst, vrp.asn, &prefix6, - vrp.max_prefix_length); - } - - pr_crit("Unknown address family: %d", vrp.addr_fam); -} - -static int -duplicate_key(struct db_table *dst, struct hashable_key *new) -{ - return rtrhandler_handle_router_key(dst, new->data.ski, new->data.as, - new->data.spk); -} - -#define MERGE_ITER(table_prop, name, err_var) \ - struct hashable_##name *node_##name, *tmp_##name, *found_##name;\ - HASH_ITER(hh, src->table_prop, node_##name, tmp_##name) { \ - HASH_FIND(hh, dst->table_prop, &node_##name->data, \ - sizeof(node_##name->data), found_##name); \ - if (found_##name != NULL) \ - continue; \ - err_var = duplicate_##name(dst, node_##name); \ - if (err_var) \ - return err_var; \ - } - -static int -db_table_merge(struct db_table *dst, struct db_table *src) -{ - int error; - - /** Must look for elements due to the new mem allocation */ - MERGE_ITER(roas, roa, error) - MERGE_ITER(router_keys, key, error) - - return 0; -} - -int -db_table_clone(struct db_table **dst, struct db_table *src) -{ - struct db_table *result; - int error; - - result = db_table_create(); - if (result == NULL) - return pr_enomem(); - - error = db_table_merge(result, src); - if (error) { - db_table_destroy(result); - return error; - } - - *dst = result; - return 0; -} - unsigned int db_table_roa_count(struct db_table *table) { diff --git a/src/rtr/db/db_table.h b/src/rtr/db/db_table.h index 1b54f7cf..8574244d 100644 --- a/src/rtr/db/db_table.h +++ b/src/rtr/db/db_table.h @@ -9,8 +9,6 @@ struct db_table; struct db_table *db_table_create(void); void db_table_destroy(struct db_table *); -int db_table_clone(struct db_table **, struct db_table *); - unsigned int db_table_roa_count(struct db_table *); unsigned int db_table_router_key_count(struct db_table *); diff --git a/src/rtr/db/delta.c b/src/rtr/db/delta.c index e6cd3f9f..b5533320 100644 --- a/src/rtr/db/delta.c +++ b/src/rtr/db/delta.c @@ -23,9 +23,9 @@ struct delta_rk { unsigned char spk[RK_SPKI_LEN]; }; -ARRAY_LIST(deltas_v6, struct delta_v6) -ARRAY_LIST(deltas_v4, struct delta_v4) -ARRAY_LIST(deltas_rk, struct delta_rk) +STATIC_ARRAY_LIST(deltas_v6, struct delta_v6) +STATIC_ARRAY_LIST(deltas_v4, struct delta_v4) +STATIC_ARRAY_LIST(deltas_rk, struct delta_rk) struct deltas { struct { diff --git a/src/rtr/db/vrps.c b/src/rtr/db/vrps.c index a5006623..1366122e 100644 --- a/src/rtr/db/vrps.c +++ b/src/rtr/db/vrps.c @@ -173,13 +173,6 @@ vrps_destroy(void) rwlock_unlock(lock); \ return error; -#define RLOCK_HANDLER(lock, cb) \ - int error; \ - rwlock_read_lock(lock); \ - error = cb; \ - rwlock_unlock(lock); \ - return error; - int handle_roa_v4(uint32_t as, struct ipv4_prefix const *prefix, uint8_t max_length, void *arg) @@ -274,7 +267,7 @@ __vrps_update(bool *notify_clients) error = __perform_standalone_validation(&new_base); if (error) return error; - error = slurm_apply(&new_base, &state.slurm); + error = slurm_apply(new_base, &state.slurm); if (error) { db_table_destroy(new_base); return error; @@ -357,14 +350,14 @@ vrps_update(bool *changed) do { if (state.base == NULL) { rwlock_unlock(&state_lock); - pr_op_info("- Valid Prefixes: 0"); + pr_op_info("- Valid ROAs: 0"); pr_op_info("- Valid Router Keys: 0"); if (config_get_mode() == SERVER) pr_op_info("- No serial number."); break; } - pr_op_info("- Valid Prefixes: %u", db_table_roa_count(state.base)); + pr_op_info("- Valid ROAs: %u", db_table_roa_count(state.base)); pr_op_info("- Valid Router Keys: %u", db_table_router_key_count(state.base)); if (config_get_mode() == SERVER) diff --git a/src/slurm/db_slurm.c b/src/slurm/db_slurm.c index 564e46c0..df672b4a 100644 --- a/src/slurm/db_slurm.c +++ b/src/slurm/db_slurm.c @@ -1,13 +1,13 @@ -#include "db_slurm.h" +#include "slurm/db_slurm.h" #include #include #include +#include "common.h" #include "crypto/base64.h" #include "data_structure/array_list.h" #include "object/router_key.h" -#include "common.h" struct slurm_prefix_wrap { struct slurm_prefix element; @@ -34,7 +34,6 @@ struct slurm_lists { struct db_slurm { struct slurm_lists lists; struct slurm_lists *cache; - bool loaded_date_set; time_t loaded_date; struct slurm_csum_list csum_list; }; @@ -97,24 +96,35 @@ slurm_lists_destroy(struct slurm_lists *lists) } int -db_slurm_create(struct db_slurm **result) +db_slurm_create(struct slurm_csum_list *csums, struct db_slurm **result) { struct db_slurm *db; + int error; db = malloc(sizeof(struct db_slurm)); if (db == NULL) return pr_enomem(); + error = get_current_time(&db->loaded_date); + if (error) { + free(db); + return error; + } + /* Not ready yet (nor required yet) for multithreading */ al_filter_prefix_init(&db->lists.filter_pfx_al); al_assertion_prefix_init(&db->lists.assertion_pfx_al); al_filter_bgpsec_init(&db->lists.filter_bgps_al); al_assertion_bgpsec_init(&db->lists.assertion_bgps_al); - db->loaded_date_set = false; db->cache = NULL; + db->csum_list = *csums; - SLIST_INIT(&db->csum_list); - db->csum_list.list_size = 0; + /* + * Slight hack: Clean up csums, so caller can always call + * destroy_local_csum_list(). + */ + csums->slh_first = NULL; + csums->list_size = 0; *result = db; return 0; @@ -436,19 +446,6 @@ db_slurm_foreach_assertion_bgpsec(struct db_slurm *db, bgpsec_foreach_cb cb, return foreach_assertion_bgpsec(&db->lists, cb, arg); } -int -db_slurm_update_time(struct db_slurm *db) -{ - int error; - - error = get_current_time(&db->loaded_date); - if (error) - return error; - - db->loaded_date_set = true; - return 0; -} - static int print_prefix_data(struct slurm_prefix *prefix, void *arg) { @@ -529,9 +526,7 @@ print_bgpsec_data(struct slurm_bgpsec *bgpsec, void *arg) void db_slurm_log(struct db_slurm *db) { - if (db->loaded_date_set) - pr_op_info("SLURM loaded at %s", - asctime(localtime(&db->loaded_date))); + pr_op_info("SLURM loaded at %s", asctime(localtime(&db->loaded_date))); pr_op_info("Validation output filters {"); pr_op_info(" Prefix filters {"); foreach_filter_prefix(&db->lists, print_prefix_data, NULL); @@ -690,17 +685,6 @@ db_slurm_destroy(struct db_slurm *db) free(db); } -int -db_slurm_set_csum_list(struct db_slurm *db, struct slurm_csum_list *list) -{ - if (!SLIST_EMPTY(&db->csum_list)) - pr_crit("Checksum list for SLURM DB must be empty"); - - db->csum_list.slh_first = list->slh_first; - db->csum_list.list_size = list->list_size; - return 0; -} - void db_slurm_get_csum_list(struct db_slurm *db, struct slurm_csum_list *result) { diff --git a/src/slurm/db_slurm.h b/src/slurm/db_slurm.h index 65df53c2..76abea2f 100644 --- a/src/slurm/db_slurm.h +++ b/src/slurm/db_slurm.h @@ -38,6 +38,8 @@ struct slurm_file_csum { }; struct slurm_csum_list { + /* TODO (fine) why tf is this not a SLIST_HEAD */ + /* TODO (performance) Actually, why tf is this not an arraylist */ struct slurm_file_csum *slh_first; /* first element */ unsigned int list_size; }; @@ -47,7 +49,7 @@ struct db_slurm; typedef int (*prefix_foreach_cb)(struct slurm_prefix *, void *); typedef int (*bgpsec_foreach_cb)(struct slurm_bgpsec *, void *); -int db_slurm_create(struct db_slurm **); +int db_slurm_create(struct slurm_csum_list *, struct db_slurm **); int db_slurm_add_prefix_filter(struct db_slurm *, struct slurm_prefix *); int db_slurm_add_prefix_assertion(struct db_slurm *, struct slurm_prefix *); @@ -62,8 +64,6 @@ int db_slurm_foreach_assertion_prefix(struct db_slurm *, prefix_foreach_cb, int db_slurm_foreach_assertion_bgpsec(struct db_slurm *, bgpsec_foreach_cb, void *); -/* Set the last update to current datetime */ -int db_slurm_update_time(struct db_slurm *); /* Log the DB in human readable form at INFO level */ void db_slurm_log(struct db_slurm *); @@ -77,7 +77,6 @@ bool db_slurm_has_data(struct db_slurm *); void db_slurm_destroy(struct db_slurm *); -int db_slurm_set_csum_list(struct db_slurm *, struct slurm_csum_list *); void db_slurm_get_csum_list(struct db_slurm *, struct slurm_csum_list *); #endif /* SRC_SLURM_db_slurm_H_ */ diff --git a/src/slurm/slurm_loader.c b/src/slurm/slurm_loader.c index 5f4ceb90..17799da8 100644 --- a/src/slurm/slurm_loader.c +++ b/src/slurm/slurm_loader.c @@ -1,4 +1,4 @@ -#include "slurm_loader.h" +#include "slurm/slurm_loader.h" #include #include @@ -13,38 +13,44 @@ #define SLURM_FILE_EXTENSION ".slurm" +struct slurm_parser_params { + struct db_table *db_table; + struct db_slurm *db_slurm; +}; + /* - * Load the SLURM file(s) from the configured path, if the path is valid but no - * data is loaded (specific error for a SLURM folder) no error is returned and - * slurm db from @params is set as NULL. + * Load the SLURM file(s) from the configured path. + * + * If this returns zero but @result is NULL, it's fine. There are no SLURM + * rules. */ static int -load_slurm_files(struct slurm_parser_params *params) +load_slurm_files(struct slurm_csum_list *csums, struct db_slurm **result) { struct db_slurm *db; int error; - error = db_slurm_create(&db); + error = db_slurm_create(csums, &db); if (error) return error; - params->db_slurm = db; - error = process_file_or_dir(config_get_slurm(), SLURM_FILE_EXTENSION, - false, slurm_parse, params); - if (error) { - db_slurm_destroy(db); - params->db_slurm = NULL; - return error; - } + false, slurm_parse, db); + if (error) + goto cancel; /* Empty SLURM dir, or empty SLURM file(s) */ if (!db_slurm_has_data(db)) { - db_slurm_destroy(db); - params->db_slurm = NULL; + *result = NULL; + goto cancel; /* Success. */ } + *result = db; return 0; + +cancel: + db_slurm_destroy(db); + return error; } static int @@ -61,7 +67,7 @@ slurm_pfx_filters_apply(struct vrp const *vrp, void *arg) static int slurm_pfx_assertions_add(struct slurm_prefix *prefix, void *arg) { - struct slurm_parser_params *params = arg; + struct db_table *table = arg; struct ipv4_prefix prefix4; struct ipv6_prefix prefix6; struct vrp vrp; @@ -73,14 +79,14 @@ slurm_pfx_assertions_add(struct slurm_prefix *prefix, void *arg) if (vrp.addr_fam == AF_INET) { prefix4.addr = vrp.prefix.v4; prefix4.len = vrp.prefix_length; - return rtrhandler_handle_roa_v4(params->db_table, vrp.asn, - &prefix4, vrp.max_prefix_length); + return rtrhandler_handle_roa_v4(table, vrp.asn, &prefix4, + vrp.max_prefix_length); } if (vrp.addr_fam == AF_INET6) { prefix6.addr = vrp.prefix.v6; prefix6.len = vrp.prefix_length; - return rtrhandler_handle_roa_v6(params->db_table, vrp.asn, - &prefix6, vrp.max_prefix_length); + return rtrhandler_handle_roa_v6(table, vrp.asn, &prefix6, + vrp.max_prefix_length); } pr_crit("Unknown addr family type: %u", vrp.addr_fam); @@ -90,7 +96,7 @@ static int slurm_pfx_assertions_apply(struct slurm_parser_params *params) { return db_slurm_foreach_assertion_prefix(params->db_slurm, - slurm_pfx_assertions_add, params); + slurm_pfx_assertions_add, params->db_table); } static int @@ -107,33 +113,17 @@ slurm_bgpsec_filters_apply(struct router_key const *key, void *arg) static int slurm_bgpsec_assertions_add(struct slurm_bgpsec *bgpsec, void *arg) { - struct slurm_parser_params *params = arg; + struct db_table *table = arg; - return rtrhandler_handle_router_key(params->db_table, bgpsec->ski, - bgpsec->asn, bgpsec->router_public_key); + return rtrhandler_handle_router_key(table, bgpsec->ski, bgpsec->asn, + bgpsec->router_public_key); } static int slurm_bgpsec_assertions_apply(struct slurm_parser_params *params) { return db_slurm_foreach_assertion_bgpsec(params->db_slurm, - slurm_bgpsec_assertions_add, params); -} - -static int -slurm_create_parser_params(struct slurm_parser_params **result) -{ - struct slurm_parser_params *params; - - params = malloc(sizeof(struct slurm_parser_params)); - if (params == NULL) - return pr_enomem(); - - params->db_table = NULL; - params->db_slurm = NULL; - - *result = params; - return 0; + slurm_bgpsec_assertions_add, params->db_table); } static int @@ -143,12 +133,10 @@ __slurm_load_checksums(char const *location, void *arg) struct slurm_file_csum *csum; int error; - list = arg; csum = malloc(sizeof(struct slurm_file_csum)); if (csum == NULL) return pr_enomem(); - error = hash_local_file("sha256", location, csum->csum, &csum->csum_len); if (error) { @@ -156,6 +144,7 @@ __slurm_load_checksums(char const *location, void *arg) return pr_op_err("Calculating slurm hash"); } + list = arg; SLIST_INSERT_HEAD(list, csum, next); list->list_size++; @@ -175,64 +164,19 @@ destroy_local_csum_list(struct slurm_csum_list *list) } static int -slurm_load_checksums(struct slurm_csum_list *csum_list) +slurm_load_checksums(struct slurm_csum_list *csums) { - struct slurm_csum_list result; int error; - SLIST_INIT(&result); - result.list_size = 0; + SLIST_INIT(csums); + csums->list_size = 0; error = process_file_or_dir(config_get_slurm(), SLURM_FILE_EXTENSION, - false, __slurm_load_checksums, &result); - if (error) { - destroy_local_csum_list(&result); - return error; - } - - csum_list->list_size = result.list_size; - csum_list->slh_first = result.slh_first; - - return 0; -} - -/* Returns whether a new slurm was allocated */ -static void -__load_slurm_files(struct db_slurm **last_slurm, - struct slurm_parser_params *params, struct slurm_csum_list *csum_list) -{ - int error; - - error = load_slurm_files(params); + false, __slurm_load_checksums, csums); if (error) - goto use_last_slurm; - - /* Prepare the new SLURM DB */ - if (params->db_slurm != NULL) { - error = db_slurm_update_time(params->db_slurm); - if (error) - goto use_last_slurm; - db_slurm_set_csum_list(params->db_slurm, csum_list); - } + destroy_local_csum_list(csums); - /* Use new SLURM as last valid slurm */ - if (*last_slurm != NULL) - db_slurm_destroy(*last_slurm); - - *last_slurm = params->db_slurm; - - return; - -use_last_slurm: - /* Any error: use last valid SLURM */ - pr_op_info("Error loading SLURM, the validation will still continue."); - if (*last_slurm != NULL) { - pr_op_info("A previous valid version of the SLURM exists and will be applied."); - params->db_slurm = *last_slurm; - /* Log applied SLURM as info */ - db_slurm_log(params->db_slurm); - } - destroy_local_csum_list(csum_list); + return error; } static bool @@ -270,103 +214,98 @@ are_csum_lists_equals(struct slurm_csum_list *new_list, /* Load SLURM file(s) that have updates */ static int -load_updated_slurm(struct db_slurm **last_slurm, - struct slurm_parser_params *params) +update_slurm(struct db_slurm **slurm) { - struct slurm_csum_list csum_list, old_csum_list; + struct slurm_csum_list new_csums; + struct slurm_csum_list old_csums; + struct db_slurm *new_slurm = NULL; int error; - bool list_equals; - - list_equals = false; pr_op_info("Checking if there are new or modified SLURM files"); - error = slurm_load_checksums(&csum_list); + + error = slurm_load_checksums(&new_csums); if (error) return error; - if (*last_slurm != NULL) { - db_slurm_get_csum_list(*last_slurm, &old_csum_list); - list_equals = are_csum_lists_equals(&csum_list, &old_csum_list); - } + /* Empty DIR or FILE SLURM not found */ + if (new_csums.list_size == 0) + goto success; - if (list_equals) { - if (*last_slurm != NULL) { + if (*slurm != NULL) { + db_slurm_get_csum_list(*slurm, &old_csums); + if (are_csum_lists_equals(&new_csums, &old_csums)) { pr_op_info("Applying same old SLURM, no changes found."); - params->db_slurm = *last_slurm; + destroy_local_csum_list(&new_csums); + return 0; } - destroy_local_csum_list(&csum_list); - return 0; } - /* Empty DIR or FILE SLURM not found */ - if (csum_list.list_size == 0) { - if (*last_slurm != NULL) - db_slurm_destroy(*last_slurm); - *last_slurm = NULL; - params->db_slurm = NULL; + pr_op_info("Applying configured SLURM"); + + error = load_slurm_files(&new_csums, &new_slurm); + + /* + * Checksums were transferred to new_slurm on success, but they're + * still here on failure. + * Either way, new_csums is ready for cleanup. + */ + destroy_local_csum_list(&new_csums); + + if (error) { + /* Fall back to previous iteration's SLURM */ + pr_op_info("Error %d loading SLURM. The validation will continue regardless.", + error); + if (*slurm != NULL) { + pr_op_info("A previous valid version of the SLURM exists and will be applied."); + db_slurm_log(*slurm); + } + return 0; } - pr_op_info("Applying configured SLURM"); - __load_slurm_files(last_slurm, params, &csum_list); +success: + /* Use new SLURM as last valid slurm */ + if (*slurm != NULL) + db_slurm_destroy(*slurm); + *slurm = new_slurm; return 0; } int -slurm_apply(struct db_table **base, struct db_slurm **last_slurm) +slurm_apply(struct db_table *base, struct db_slurm **slurm) { - struct slurm_parser_params *params; + struct slurm_parser_params params; int error; if (config_get_slurm() == NULL) return 0; - params = NULL; - error = slurm_create_parser_params(¶ms); + error = update_slurm(slurm); if (error) return error; - error = load_updated_slurm(last_slurm, params); - if (error) - goto release_params; + if (*slurm == NULL) + return 0; - /* If there's no new SLURM loaded, stop */ - if (params->db_slurm == NULL) - goto success; + /* Ok, apply SLURM */ - /* Deep copy of the base so that updates can be reverted */ - error = db_table_clone(¶ms->db_table, *base); - if (error) - goto release_params; + params.db_table = base; + params.db_slurm = *slurm; - error = db_table_foreach_roa(params->db_table, slurm_pfx_filters_apply, - params); + /* TODO invert this. SLURM rules are few, and base is massive. */ + error = db_table_foreach_roa(base, slurm_pfx_filters_apply, ¶ms); if (error) - goto release_table; + return error; - error = db_table_foreach_router_key(params->db_table, - slurm_bgpsec_filters_apply, params); + error = db_table_foreach_router_key(base, slurm_bgpsec_filters_apply, + ¶ms); if (error) - goto release_table; + return error; - error = slurm_pfx_assertions_apply(params); + error = slurm_pfx_assertions_apply(¶ms); if (error) - goto release_table; - - error = slurm_bgpsec_assertions_apply(params); - if (error) { - goto release_table; - } + return error; - db_table_destroy(*base); - *base = params->db_table; -success: - free(params); - return 0; -release_table: - db_table_destroy(params->db_table); -release_params: - free(params); - return error; + return slurm_bgpsec_assertions_apply(¶ms); } diff --git a/src/slurm/slurm_loader.h b/src/slurm/slurm_loader.h index cf4845a3..e23dfd33 100644 --- a/src/slurm/slurm_loader.h +++ b/src/slurm/slurm_loader.h @@ -17,6 +17,6 @@ * - The @last_slurm was applied due to a syntax problem with a newer SLURM * - SLURM configured but couldn't be read (file doesn't exists, no permission) */ -int slurm_apply(struct db_table **, struct db_slurm **); +int slurm_apply(struct db_table *, struct db_slurm **); #endif /* SRC_SLURM_SLURM_LOADER_H_ */ diff --git a/src/slurm/slurm_parser.c b/src/slurm/slurm_parser.c index de86085b..6d37fb0b 100644 --- a/src/slurm/slurm_parser.c +++ b/src/slurm/slurm_parser.c @@ -1,4 +1,4 @@ -#include "slurm_parser.h" +#include "slurm/slurm_parser.h" #include #include @@ -14,6 +14,7 @@ #include "address.h" #include "json_parser.h" #include "object/router_key.h" +#include "slurm/db_slurm.h" /* JSON members */ #define SLURM_VERSION "slurmVersion" @@ -44,13 +45,10 @@ static int handle_json(json_t *, struct db_slurm *); int slurm_parse(char const *location, void *arg) { - struct slurm_parser_params *params; json_t *json_root; json_error_t json_error; int error; - params = arg; - json_root = json_load_file(location, JSON_REJECT_DUPLICATES, &json_error); if (json_root == NULL) @@ -58,7 +56,7 @@ slurm_parse(char const *location, void *arg) return pr_op_err("SLURM JSON error on line %d, column %d: %s", json_error.line, json_error.column, json_error.text); - error = handle_json(json_root, params->db_slurm); + error = handle_json(json_root, arg); json_decref(json_root); if (error) return error; /* File exists, but has a syntax error */ diff --git a/src/slurm/slurm_parser.h b/src/slurm/slurm_parser.h index 143c04f6..743a6273 100644 --- a/src/slurm/slurm_parser.h +++ b/src/slurm/slurm_parser.h @@ -1,15 +1,6 @@ #ifndef SRC_SLURM_SLURM_PARSER_H_ #define SRC_SLURM_SLURM_PARSER_H_ -#include "rtr/db/db_table.h" -#include "slurm/db_slurm.h" - -struct slurm_parser_params { - struct db_table *db_table; - struct db_slurm *db_slurm; -}; - int slurm_parse(char const *, void *); - #endif /* SRC_SLURM_SLURM_PARSER_H_ */ -- 2.47.2