From: pcarana Date: Tue, 21 May 2019 21:11:40 +0000 (-0500) Subject: Add multiple improvements at SLURM, config and vrp, and fix a test bug X-Git-Tag: v0.0.2~26 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=cea21a8d9911f91d29d9adfd03f7d0c732f79cec;p=thirdparty%2FFORT-validator.git Add multiple improvements at SLURM, config and vrp, and fix a test bug - Fix bug at client_test, the module wasn't updated with several changes from other commits. - Add common function to load data from a file or directory, use this for TAL and SLURM locations (both configurations can have a file path or a dir path). - Update some config parameters: + 'server.slurm.location' renamed to 'slurm' and it can be a file path or a directory path. + 'server.queue' renamed to 'server.backlog' with a default value of SOMAXCONN. + Delete 'server.rtr-interval.*' (RTRv1 isn't supported yet). - Create macros to compare VRPs and to compare each of its properties. - If the SLURM has errors, don't drop the whole ROA tree, just don't apply SLURM on the tree. --- diff --git a/src/common.c b/src/common.c index 07627fdb..c05f89d2 100644 --- a/src/common.c +++ b/src/common.c @@ -4,6 +4,8 @@ #include #include #include +#include + #include "log.h" int @@ -125,7 +127,7 @@ process_file(char const *dir_name, char const *file_name, char const *file_ext, return error; } -int +static int process_dir_files(char const *location, char const *file_ext, process_file_cb cb, void *arg) { @@ -158,3 +160,20 @@ close_dir: end: return error; } + +int +process_file_or_dir(char const *location, char const *file_ext, + process_file_cb cb, void *arg) +{ + struct stat attr; + int error; + + error = stat(location, &attr); + if (error) + return pr_errno(errno, "Error reading path '%s'", location); + + if (S_ISDIR(attr.st_mode) == 0) + return cb(location, arg); + + return process_dir_files(location, file_ext, cb, arg); +} diff --git a/src/common.h b/src/common.h index c13756a8..bc136dbb 100644 --- a/src/common.h +++ b/src/common.h @@ -36,6 +36,6 @@ void rwlock_unlock(pthread_rwlock_t *); int close_thread(pthread_t thread, char const *what); typedef int (*process_file_cb)(char const *, void *); -int process_dir_files(char const *, char const *, process_file_cb, void *); +int process_file_or_dir(char const *, char const *, process_file_cb, void *); #endif /* SRC_RTR_COMMON_H_ */ diff --git a/src/config.c b/src/config.c index d79b506e..9241ac67 100644 --- a/src/config.c +++ b/src/config.c @@ -44,25 +44,26 @@ struct rpki_config { * Prevents arbitrarily long paths and loops. */ unsigned int maximum_certificate_depth; + /** File or directory where the .slurm file(s) is(are) located */ + char *slurm; struct { /** The bound listening address of the RTR server. */ char *address; /** The bound listening port of the RTR server. */ char *port; - /** Maximum accepted client connections */ - unsigned int queue; + /** Outstanding connections in the socket's listen queue */ + unsigned int backlog; /** Interval used to look for updates at VRPs location */ unsigned int validation_interval; - /** Intervals use at RTR v1 End of data PDU **/ - uint32_t refresh_interval; - uint32_t retry_interval; - uint32_t expire_interval; - - /** Directory where the .slurm files are located */ - char *slurm_location; + /* + * TODO (next iteration) Intervals used at End of data PDU + * uint32_t refresh_interval; + * uint32_t retry_interval; + * uint32_t expire_interval; + */ } server; struct { @@ -178,6 +179,12 @@ static const struct option_field options[] = { * overflow and will never be bigger than this. */ .max = UINT_MAX - 1, + }, { + .id = 1003, + .name = "slurm", + .type = >_string, + .offset = offsetof(struct rpki_config, slurm), + .doc = "Path to the SLURM file or SLURMs directory (files must have the extension .slurm)", }, /* Server fields */ @@ -195,10 +202,10 @@ static const struct option_field options[] = { .doc = "Port the RTR server will bind itself to. Can be a string, in which case a number will be resolved.", }, { .id = 5003, - .name = "server.queue", + .name = "server.backlog", .type = >_uint, - .offset = offsetof(struct rpki_config, server.queue), - .doc = "Maximum accepted client connections", + .offset = offsetof(struct rpki_config, server.backlog), + .doc = "Maximum connections in the socket's listen queue", .min = 1, .max = SOMAXCONN, }, { @@ -215,39 +222,15 @@ static const struct option_field options[] = { * We do this by not getting new information more than once per * minute. */ - .min = 60, + .min = 10, .max = 7200, - }, { - .id = 5005, - .name = "server.rtr-interval.refresh", - .type = >_uint32, - .offset = offsetof(struct rpki_config, server.refresh_interval), - .doc = "Intervals use at RTR v1 End of data PDU", - .min = 1, - .max = 86400, - }, { - .id = 5006, - .name = "server.rtr-interval.retry", - .type = >_uint32, - .offset = offsetof(struct rpki_config, server.retry_interval), - .doc = "", - .min = 1, - .max = 7200, - }, { - .id = 5007, - .name = "server.rtr-interval.expire", - .type = >_uint32, - .offset = offsetof(struct rpki_config, server.expire_interval), - .doc = "", - .min = 600, - .max = 172800, - }, { - .id = 5008, - .name = "server.slurm.location", - .type = >_string, - .offset = offsetof(struct rpki_config, server.slurm_location), - .doc = "Directory where the .slurm files are located", - }, + }, + /* + * TODO (next iteration) RTRv1 intervals with values: + * - refresh: min = 1, max = 86400, default = 3600 + * - retry: min = 1, max = 7200, default = 600 + * - expire: min = 600, max = 172800, default = 7200 + */ /* RSYNC fields */ { @@ -451,14 +434,11 @@ set_default_values(void) if (rpki_config.server.port == NULL) return pr_enomem(); - rpki_config.server.queue = 10; + rpki_config.server.backlog = SOMAXCONN; rpki_config.server.validation_interval = 60; - rpki_config.server.refresh_interval = 3600; - rpki_config.server.retry_interval = 600; - rpki_config.server.expire_interval = 7200; - rpki_config.server.slurm_location = NULL; rpki_config.tal = NULL; + rpki_config.slurm = NULL; rpki_config.local_repository = strdup("/tmp/fort/repository"); if (rpki_config.local_repository == NULL) { @@ -639,7 +619,7 @@ config_get_server_queue(void) /* * The range of this is 1-, so adding signedness is safe. */ - return rpki_config.server.queue; + return rpki_config.server.backlog; } unsigned int @@ -648,29 +628,12 @@ config_get_validation_interval(void) return rpki_config.server.validation_interval; } -uint32_t -config_get_refresh_interval(void) -{ - return rpki_config.server.refresh_interval; -} - -uint32_t -config_get_retry_interval(void) -{ - return rpki_config.server.retry_interval; -} - -uint32_t -config_get_expire_interval(void) -{ - return rpki_config.server.expire_interval; -} - char const * -config_get_slurm_location(void) +config_get_slurm(void) { - return rpki_config.server.slurm_location; + return rpki_config.slurm; } + char const * config_get_tal(void) { diff --git a/src/config.h b/src/config.h index 89a4d25b..866228ec 100644 --- a/src/config.h +++ b/src/config.h @@ -18,10 +18,7 @@ char const *config_get_server_address(void); char const *config_get_server_port(void); int config_get_server_queue(void); unsigned int config_get_validation_interval(void); -uint32_t config_get_refresh_interval(void); -uint32_t config_get_retry_interval(void); -uint32_t config_get_expire_interval(void); -char const *config_get_slurm_location(void); +char const *config_get_slurm(void); char const *config_get_tal(void); char const *config_get_local_repository(void); diff --git a/src/object/tal.c b/src/object/tal.c index 9fde1634..a8e881d8 100644 --- a/src/object/tal.c +++ b/src/object/tal.c @@ -371,22 +371,12 @@ end: tal_destroy(tal); int perform_standalone_validation(struct validation_handler *handler) { - char const *config_tal; - struct stat attr; int error; - config_tal = config_get_tal(); - error = stat(config_tal, &attr); - if (error) - return pr_errno(errno, "Error reading path '%s'", config_tal); - fnstack_init(); - if (S_ISDIR(attr.st_mode) == 0) - error = do_file_validation(config_tal, handler); - else - error = process_dir_files(config_tal, TAL_FILE_EXTENSION, - do_file_validation, handler); - + error = process_file_or_dir(config_get_tal(), TAL_FILE_EXTENSION, + do_file_validation, handler); fnstack_cleanup(); + return error; } diff --git a/src/rtr/db/roa_table.c b/src/rtr/db/roa_table.c index 596bd0d6..905641ef 100644 --- a/src/rtr/db/roa_table.c +++ b/src/rtr/db/roa_table.c @@ -142,6 +142,22 @@ roa_table_merge(struct roa_table *dst, struct roa_table *src) return 0; } +int +roa_table_clone(struct roa_table **dst, struct roa_table *src) +{ + int error; + + *dst = roa_table_create(); + if (*dst == NULL) + return pr_enomem(); + + error = roa_table_merge(*dst, src); + if (error) + free(*dst); + + return error; +} + int rtrhandler_merge(struct roa_table *dst, struct roa_table *src) { diff --git a/src/rtr/db/roa_table.h b/src/rtr/db/roa_table.h index 94aa7eb7..13191304 100644 --- a/src/rtr/db/roa_table.h +++ b/src/rtr/db/roa_table.h @@ -8,6 +8,7 @@ struct roa_table; struct roa_table *roa_table_create(void); void roa_table_destroy(struct roa_table *); +int roa_table_clone(struct roa_table **, struct roa_table *); int roa_table_foreach_roa(struct roa_table *, vrp_foreach_cb, void *); void roa_table_remove_roa(struct roa_table *, struct vrp const *); diff --git a/src/rtr/db/vrp.h b/src/rtr/db/vrp.h index 79659b04..70d5503c 100644 --- a/src/rtr/db/vrp.h +++ b/src/rtr/db/vrp.h @@ -7,6 +7,30 @@ #define FLAG_WITHDRAWAL 0 #define FLAG_ANNOUNCEMENT 1 +#define VRP_ASN_EQ(a, b) \ + (a)->asn == (b)->asn + +#define VRP_MAX_PREFIX_LEN_EQ(a, b) \ + (a)->max_prefix_length == (b)->max_prefix_length + +#define VRP_PREFIX_V4_EQ(a, b) \ + ((a)->addr_fam == AF_INET && \ + (b)->addr_fam == AF_INET && \ + (a)->prefix.v4.s_addr == (b)->prefix.v4.s_addr && \ + (a)->prefix_length == (b)->prefix_length) + +#define VRP_PREFIX_V6_EQ(a, b) \ + ((a)->addr_fam == AF_INET6 && \ + (b)->addr_fam == AF_INET6 && \ + IN6_ARE_ADDR_EQUAL(&(a)->prefix.v6, &(b)->prefix.v6) && \ + (a)->prefix_length == (b)->prefix_length) + +#define VRP_PREFIX_EQ(a, b) \ + (VRP_PREFIX_V4_EQ(a, b) || VRP_PREFIX_V6_EQ(a, b)) + +#define VRP_EQ(a, b) \ + (VRP_ASN_EQ(a, b) && VRP_PREFIX_EQ(a, b) && VRP_MAX_PREFIX_LEN_EQ(a, b)) + typedef uint32_t serial_t; struct vrp { diff --git a/src/rtr/db/vrps.c b/src/rtr/db/vrps.c index 117fcb86..aebc7d8e 100644 --- a/src/rtr/db/vrps.c +++ b/src/rtr/db/vrps.c @@ -249,11 +249,16 @@ vrps_update(bool *changed) rwlock_write_lock(&lock); - error = slurm_apply(new_base); - if (error) { - rwlock_unlock(&lock); - goto revert_base; - } + /* + * TODO (next iteration) Remember the last valid SLURM + * + * Currently SLURM is ignored if it has errors, the error is logged and + * the new_base isn't altered. Instead of this, the last valid SLURM + * should be remembered, and will be applied when a new SLURM has + * errors; a warning should be logged to indicate which version of the + * SLURM is being applied. + */ + slurm_apply(&new_base); if (state.base != NULL) { error = compute_deltas(state.base, new_base, &deltas); diff --git a/src/rtr/pdu_sender.c b/src/rtr/pdu_sender.c index 4830a4eb..a54872fe 100644 --- a/src/rtr/pdu_sender.c +++ b/src/rtr/pdu_sender.c @@ -172,19 +172,6 @@ send_prefix_pdu(int fd, struct vrp const *vrp, uint8_t flags) return -EINVAL; } -static bool -vrp_equals(struct vrp const *left, struct vrp const *right) -{ - return left->asn == right->asn - && left->addr_fam == right->addr_fam - && left->prefix_length == right->prefix_length - && left->max_prefix_length == right->max_prefix_length - && ((left->addr_fam == AF_INET - && left->prefix.v4.s_addr == right->prefix.v4.s_addr) - || (left->addr_fam == AF_INET6 - && IN6_ARE_ADDR_EQUAL(&left->prefix.v6, &right->prefix.v6))); -} - static int vrp_simply_send(struct delta const *delta, void *arg) { @@ -205,7 +192,7 @@ vrp_ovrd_remove(struct delta const *delta, void *arg) struct vrp_slist *filtered_vrps = arg; SLIST_FOREACH(ptr, filtered_vrps, next) - if (vrp_equals(&delta->vrp, &ptr->delta.vrp) && + if (VRP_EQ(&delta->vrp, &ptr->delta.vrp) && delta->flags != ptr->delta.flags) { SLIST_REMOVE(filtered_vrps, ptr, vrp_node, next); free(ptr); @@ -283,11 +270,7 @@ send_end_of_data_pdu(int fd, serial_t end_serial) pdu.header.length = RTRPDU_END_OF_DATA_LEN; pdu.serial_number = end_serial; - if (pdu.header.protocol_version == RTR_V1) { - pdu.refresh_interval = config_get_refresh_interval(); - pdu.retry_interval = config_get_retry_interval(); - pdu.expire_interval = config_get_expire_interval(); - } + /* TODO (next iteration) Add RTRv1 intervals */ len = serialize_end_of_data_pdu(&pdu, data); if (len != RTRPDU_END_OF_DATA_LEN) diff --git a/src/slurm/slurm_db.c b/src/slurm/slurm_db.c index a81b7251..6d0b0f69 100644 --- a/src/slurm/slurm_db.c +++ b/src/slurm/slurm_db.c @@ -38,17 +38,12 @@ prefix_filtered_by(struct slurm_prefix *filter, struct slurm_prefix *prefix) /* Both have ASN */ if ((filter->data_flag & SLURM_COM_FLAG_ASN) > 0 && (prefix->data_flag & SLURM_COM_FLAG_ASN) > 0) - return filter_vrp->asn == prefix_vrp->asn; + return VRP_ASN_EQ(filter_vrp, prefix_vrp); /* Both have a prefix of the same type */ if ((filter->data_flag & SLURM_PFX_FLAG_PREFIX) > 0 && - (prefix->data_flag & SLURM_PFX_FLAG_PREFIX) > 0 && - filter_vrp->addr_fam == prefix_vrp->addr_fam && - filter_vrp->prefix_length == prefix_vrp->prefix_length) - return ((filter_vrp->addr_fam == AF_INET && - filter_vrp->prefix.v4.s_addr == prefix_vrp->prefix.v4.s_addr) || - (filter_vrp->addr_fam == AF_INET6 && - IN6_ARE_ADDR_EQUAL(&filter_vrp->prefix.v6, &prefix_vrp->prefix.v6))); + (prefix->data_flag & SLURM_PFX_FLAG_PREFIX) > 0) + return VRP_PREFIX_EQ(filter_vrp, prefix_vrp); return false; } @@ -71,21 +66,15 @@ prefix_equal(struct slurm_prefix *left, struct slurm_prefix *right, /* It has the same data, compare it */ equal = true; if ((left->data_flag & SLURM_COM_FLAG_ASN) > 0) - equal = equal && left_vrp->asn == right_vrp->asn; + equal = equal && VRP_ASN_EQ(left_vrp, right_vrp); if ((left->data_flag & SLURM_PFX_FLAG_PREFIX) > 0) - equal = equal - && left_vrp->prefix_length == right_vrp->prefix_length - && left_vrp->addr_fam == right_vrp->addr_fam - && ((left_vrp->addr_fam == AF_INET - && left_vrp->prefix.v4.s_addr == right_vrp->prefix.v4.s_addr) - || (left_vrp->addr_fam == AF_INET6 - && IN6_ARE_ADDR_EQUAL(&left_vrp->prefix.v6, &right_vrp->prefix.v6))); + equal = equal && VRP_PREFIX_EQ(left_vrp, right_vrp); if ((left->data_flag & SLURM_PFX_FLAG_MAX_LENGTH) > 0) equal = equal && ((left->data_flag & SLURM_PFX_FLAG_MAX_LENGTH) > 0) && - left_vrp->max_prefix_length == right_vrp->max_prefix_length; + VRP_MAX_PREFIX_LEN_EQ(left_vrp, right_vrp); return equal; } diff --git a/src/slurm/slurm_loader.c b/src/slurm/slurm_loader.c index 4838fff4..6444e685 100644 --- a/src/slurm/slurm_loader.c +++ b/src/slurm/slurm_loader.c @@ -18,13 +18,13 @@ slurm_load(bool *loaded) { /* Optional configuration */ *loaded = false; - if (config_get_slurm_location() == NULL) + if (config_get_slurm() == NULL) return 0; *loaded = true; slurm_db_init(); - return process_dir_files(config_get_slurm_location(), + return process_file_or_dir(config_get_slurm(), SLURM_FILE_EXTENSION, slurm_parse, NULL); } @@ -32,7 +32,7 @@ static void slurm_cleanup(void) { /* Only if the SLURM was configured */ - if (config_get_slurm_location() != NULL) + if (config_get_slurm() != NULL) slurm_db_cleanup(); } @@ -81,9 +81,15 @@ slurm_pfx_assertions_apply(struct roa_table *base) base); } +/* + * Load the SLURM file/dir and try to apply it on @base. + * + * On any error the SLURM won't be applied to @base. + */ int -slurm_apply(struct roa_table *base) +slurm_apply(struct roa_table **base) { + struct roa_table *new_base; bool loaded; int error; @@ -95,14 +101,26 @@ slurm_apply(struct roa_table *base) if (!loaded) return 0; - error = roa_table_foreach_roa(base, slurm_pfx_filters_apply, base); + /* Deep copy of the base so that updates can be reverted */ + error = roa_table_clone(&new_base, *base); if (error) goto cleanup; - error = slurm_pfx_assertions_apply(base); + error = roa_table_foreach_roa(new_base, slurm_pfx_filters_apply, + new_base); + if (error) + goto release_new; - /** TODO (next iteration) Apply BGPsec filters and assertions */ + error = slurm_pfx_assertions_apply(new_base); + if (!error) { + roa_table_destroy(*base); + *base = new_base; + goto cleanup; + } + /** TODO (next iteration) Apply BGPsec filters and assertions */ +release_new: + roa_table_destroy(new_base); cleanup: slurm_cleanup(); return error; diff --git a/src/slurm/slurm_loader.h b/src/slurm/slurm_loader.h index f1fcad18..55093ccd 100644 --- a/src/slurm/slurm_loader.h +++ b/src/slurm/slurm_loader.h @@ -3,6 +3,6 @@ #include "rtr/db/roa_table.h" -int slurm_apply(struct roa_table *); +int slurm_apply(struct roa_table **); #endif /* SRC_SLURM_SLURM_LOADER_H_ */ diff --git a/test/client_test.c b/test/client_test.c index b162d585..7acaa4ea 100644 --- a/test/client_test.c +++ b/test/client_test.c @@ -26,6 +26,13 @@ handle_foreach(struct client const *client, void *arg) return 0; } +static int +join_threads(pthread_t tid, void *arg) +{ + /* Empty, since no threads are alive */ + return 0; +} + START_TEST(basic_test) { /* @@ -33,15 +40,12 @@ START_TEST(basic_test) * I'm mostly just concerned about uthash usage; I've never used uthash * before. */ - - struct sockaddr_in addr; - struct rtr_client client; + struct sockaddr_storage addr; unsigned int i; unsigned int state; memset(&addr, 0, sizeof(addr)); - addr.sin_family = AF_INET; - memcpy(&client.addr, &addr, sizeof(client.addr)); + addr.ss_family = AF_INET; ck_assert_int_eq(0, clients_db_init()); @@ -51,14 +55,10 @@ START_TEST(basic_test) */ for (i = 0; i < 4; i++) { - client.fd = 1; - ck_assert_int_eq(0, clients_add(&client)); - client.fd = 2; - ck_assert_int_eq(0, clients_add(&client)); - client.fd = 3; - ck_assert_int_eq(0, clients_add(&client)); - client.fd = 4; - ck_assert_int_eq(0, clients_add(&client)); + ck_assert_int_eq(0, clients_add(1, addr, 10)); + ck_assert_int_eq(0, clients_add(2, addr, 20)); + ck_assert_int_eq(0, clients_add(3, addr, 30)); + ck_assert_int_eq(0, clients_add(4, addr, 40)); } clients_forget(3); @@ -67,7 +67,7 @@ START_TEST(basic_test) ck_assert_int_eq(0, clients_foreach(handle_foreach, &state)); ck_assert_uint_eq(3, state); - clients_db_destroy(); + clients_db_destroy(join_threads, NULL); } END_TEST diff --git a/test/impersonator.c b/test/impersonator.c index ca69311b..61e22baf 100644 --- a/test/impersonator.c +++ b/test/impersonator.c @@ -72,7 +72,7 @@ config_get_rsync_args(bool is_ta) } char const * -config_get_slurm_location(void) +config_get_slurm(void) { return NULL; }