]> git.ipfire.org Git - thirdparty/FORT-validator.git/commitdiff
Add multiple improvements at SLURM, config and vrp, and fix a test bug
authorpcarana <pc.moreno2099@gmail.com>
Tue, 21 May 2019 21:11:40 +0000 (16:11 -0500)
committerpcarana <pc.moreno2099@gmail.com>
Tue, 21 May 2019 21:11:40 +0000 (16:11 -0500)
- 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.

15 files changed:
src/common.c
src/common.h
src/config.c
src/config.h
src/object/tal.c
src/rtr/db/roa_table.c
src/rtr/db/roa_table.h
src/rtr/db/vrp.h
src/rtr/db/vrps.c
src/rtr/pdu_sender.c
src/slurm/slurm_db.c
src/slurm/slurm_loader.c
src/slurm/slurm_loader.h
test/client_test.c
test/impersonator.c

index 07627fdb054c6049b8dd91d70b9efe80bf5cf643..c05f89d2a0dd662cc164b3ed26f6bd5eb75d6509 100644 (file)
@@ -4,6 +4,8 @@
 #include <dirent.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/stat.h>
+
 #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);
+}
index c13756a870820266a795a9cae37951a8c5d3a084..bc136dbba251722d1f57f42972e7d965c53b313e 100644 (file)
@@ -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_ */
index d79b506e17901356fcb0cda9e59e5c88e9167ed5..9241ac676e6b4737b3bf3ae55f1588986177390a 100644 (file)
@@ -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 = &gt_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 = &gt_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 = &gt_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 = &gt_uint32,
-               .offset = offsetof(struct rpki_config, server.retry_interval),
-               .doc = "",
-               .min = 1,
-               .max = 7200,
-       }, {
-               .id = 5007,
-               .name = "server.rtr-interval.expire",
-               .type = &gt_uint32,
-               .offset = offsetof(struct rpki_config, server.expire_interval),
-               .doc = "",
-               .min = 600,
-               .max = 172800,
-       }, {
-               .id = 5008,
-               .name = "server.slurm.location",
-               .type = &gt_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-<small number>, 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)
 {
index 89a4d25be67f673cf884e6a32d95a37ab6704ae7..866228ec9f2a9405e39c9f000a1be95103967770 100644 (file)
@@ -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);
index 9fde1634cdf9c1588ef83b32957a1cfd808f1f7b..a8e881d83c2f44fc24d573cca7553306cb613261 100644 (file)
@@ -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;
 }
index 596bd0d643071fcb6800df272cf10a5d0017ea36..905641ef241eef06e2f9e09f8f91357a6c900c83 100644 (file)
@@ -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)
 {
index 94aa7eb7d0ee6557683c3260f0efcacc488dba3c..131913044cae25a3f278becf12eeb0fa9415c82b 100644 (file)
@@ -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 *);
index 79659b0452ed62efcfb5f3417f06a3d76dde82ae..70d5503c289dbcdc3bc9c2b774a71070f15bdaf3 100644 (file)
@@ -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 {
index 117fcb86848422f090f977b552f5c86de5f74e50..aebc7d8e6e09a77bc7a4b6d4808381d24832ea29 100644 (file)
@@ -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);
index 4830a4eb7781eddd80e5a4aef259d18b68caf7e9..a54872fe57653d540be0eab081ac0de7716800b9 100644 (file)
@@ -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)
index a81b725148b0771bb7218a426136d6e2898e85b5..6d0b0f696ee2e5b7a2a4c576db09b6f4217c0c54 100644 (file)
@@ -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;
 }
index 4838fff45ad7203399e37448597810fd530ca126..6444e685c864d017ce5e766bb7554071bc7c4e93 100644 (file)
@@ -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;
index f1fcad188e3cdeacc4a7ff83c1131a3a1dfa3df3..55093ccd226554ac12d738d78b440d146e169915 100644 (file)
@@ -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_ */
index b162d585cabba67d19ffd4ad6653ffd01c055677..7acaa4ea52efbdd39cf24cdd606ab5ed5fce6b88 100644 (file)
@@ -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
 
index ca69311b8f41a31f5fa90bf81ff471379a24c3c5..61e22baf29d81e59662c9d982cae446828b4d877 100644 (file)
@@ -72,7 +72,7 @@ config_get_rsync_args(bool is_ta)
 }
 
 char const *
-config_get_slurm_location(void)
+config_get_slurm(void)
 {
        return NULL;
 }