From: pcarana Date: Mon, 15 Apr 2019 21:53:15 +0000 (-0500) Subject: Complete SLURM validations and avoid memleaks X-Git-Tag: v0.0.2~49^2~3 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ed2aafa449400cde06f5d2dde64e94bfcd3c9da6;p=thirdparty%2FFORT-validator.git Complete SLURM validations and avoid memleaks --- diff --git a/src/main.c b/src/main.c index 8d02dfff..85d9d1bf 100644 --- a/src/main.c +++ b/src/main.c @@ -67,7 +67,7 @@ main(int argc, char *argv[]) err = slurm_load(); if (err) - goto end3; + goto end4; err = rtr_listen(); if (err) diff --git a/src/slurm_db.c b/src/slurm_db.c index 0d65122a..274b2f91 100644 --- a/src/slurm_db.c +++ b/src/slurm_db.c @@ -1,6 +1,7 @@ #include "slurm_db.h" #include +#include #include "array_list.h" ARRAY_LIST(al_filter_prefix, struct slurm_prefix) @@ -15,6 +16,25 @@ struct arraylist_db { struct al_assertion_bgpsec assertion_bgps_al; } array_lists_db; +#define LOCATE_FUNCS(name, type, array_list, equal_cb, filter) \ + static type * \ + name##_locate(array_list *base, type *obj) \ + { \ + type *cursor; \ + \ + ARRAYLIST_FOREACH(base, cursor) \ + if (equal_cb(cursor, obj, filter)) \ + return cursor; \ + \ + return NULL; \ + } \ + \ + static bool \ + name##_is_new(array_list *base, type *obj) \ + { \ + return name##_locate(base, obj) == NULL; \ + } + int slurm_db_init(void) { @@ -26,34 +46,175 @@ slurm_db_init(void) return 0; } +static bool +prefix_filtered_by(struct slurm_prefix *prefix, struct slurm_prefix *filter) +{ + /* Both have ASN */ + if ((prefix->data_flag & SLURM_COM_FLAG_ASN) > 0 && + (filter->data_flag & SLURM_COM_FLAG_ASN) > 0) + return prefix->asn == filter->asn; + + /* Both have a prefix of the same type */ + if ((prefix->data_flag & SLURM_PFX_FLAG_PREFIX) > 0 && + (filter->data_flag & SLURM_PFX_FLAG_PREFIX) > 0 && + prefix->addr_fam == filter->addr_fam && + prefix->prefix_length == filter->prefix_length) + return ((prefix->addr_fam == AF_INET && + prefix->ipv4_prefix.s_addr == filter->ipv4_prefix.s_addr) || + (prefix->addr_fam == AF_INET6 && + IN6_ARE_ADDR_EQUAL(prefix->ipv6_prefix.s6_addr32, + filter->ipv6_prefix.s6_addr32))); + + return false; +} + +static bool +prefix_equal(struct slurm_prefix *left, struct slurm_prefix *right, + bool filter) +{ + bool equal; + + /* Ignore the comments */ + if ((left->data_flag & ~SLURM_COM_FLAG_COMMENT) != + (right->data_flag & ~SLURM_COM_FLAG_COMMENT)) + return filter && prefix_filtered_by(left, right); + + /* It has the same data, compare it */ + equal = true; + if ((left->data_flag & SLURM_COM_FLAG_ASN) > 0) + equal = equal && left->asn == right->asn; + + if ((left->data_flag & SLURM_PFX_FLAG_PREFIX) > 0) + equal = equal && left->prefix_length == right->prefix_length + && left->addr_fam == right->addr_fam + && ((left->addr_fam == AF_INET + && left->ipv4_prefix.s_addr == right->ipv4_prefix.s_addr) + || (left->addr_fam == AF_INET6 + && IN6_ARE_ADDR_EQUAL(left->ipv6_prefix.s6_addr32, + right->ipv6_prefix.s6_addr32))); + + if ((left->data_flag & SLURM_PFX_FLAG_MAX_LENGTH) > 0) + equal = equal && + ((left->data_flag & SLURM_PFX_FLAG_MAX_LENGTH) > 0) && + left->max_prefix_length == right->max_prefix_length; + + return equal; +} + +static bool +bgpsec_filtered_by(struct slurm_bgpsec *bgpsec, struct slurm_bgpsec *filter) +{ + /* Both have ASN */ + if ((bgpsec->data_flag & SLURM_COM_FLAG_ASN) > 0 && + (filter->data_flag & SLURM_COM_FLAG_ASN) > 0) + return bgpsec->asn == filter->asn; + + /* Both have a SKI */ + if ((bgpsec->data_flag & SLURM_BGPS_FLAG_SKI) > 0 && + (filter->data_flag & SLURM_BGPS_FLAG_SKI) > 0 && + bgpsec->ski_len == filter->ski_len) + return memcmp(bgpsec->ski, filter->ski, bgpsec->ski_len) == 0; + + return false; +} + +static bool +bgpsec_equal(struct slurm_bgpsec *left, struct slurm_bgpsec *right, + bool filter) +{ + bool equal; + + /* Ignore the comments */ + if ((left->data_flag & ~SLURM_COM_FLAG_COMMENT) != + (right->data_flag & ~SLURM_COM_FLAG_COMMENT)) + return filter && bgpsec_filtered_by(left, right); + + /* It has the same data, compare it */ + equal = true; + if ((left->data_flag & SLURM_COM_FLAG_ASN) > 0) + equal = equal && left->asn == right->asn; + + if ((left->data_flag & SLURM_BGPS_FLAG_SKI) > 0) + equal = equal && left->ski_len == right->ski_len && + memcmp(left->ski, right->ski, left->ski_len) == 0; + + if ((left->data_flag & SLURM_BGPS_FLAG_ROUTER_KEY) > 0) + equal = equal && + left->router_public_key_len == + right->router_public_key_len && + memcmp(left->router_public_key, right->router_public_key, + left->router_public_key_len) == 0; + + return equal; +} + +LOCATE_FUNCS(prefix_filter, struct slurm_prefix, struct al_filter_prefix, + prefix_equal, true) +LOCATE_FUNCS(bgpsec_filter, struct slurm_bgpsec, struct al_filter_bgpsec, + bgpsec_equal, true) +LOCATE_FUNCS(prefix_assertion, struct slurm_prefix, struct al_assertion_prefix, + prefix_equal, false) +LOCATE_FUNCS(bgpsec_assertion, struct slurm_bgpsec, struct al_assertion_bgpsec, + bgpsec_equal, false) + +/* + * Try to persist the @prefix filter, if it already exists or is covered + * by another filter, then the error -EEXIST is returned; otherwise, returns + * the result of persisting the @prefix. + */ int slurm_db_add_prefix_filter(struct slurm_prefix *prefix) { - /* TODO check for exact duplicates and overwritten rules */ - return al_filter_prefix_add(&array_lists_db.filter_pfx_al, prefix); + if (prefix_filter_is_new(&array_lists_db.filter_pfx_al, prefix)) + return al_filter_prefix_add(&array_lists_db.filter_pfx_al, + prefix); + + return -EEXIST; } +/* + * Try to persist the @prefix assertion, if it already exists, then the error + * -EEXIST is returned; otherwise, returns the result of persisting the + * @prefix. + */ int slurm_db_add_prefix_assertion(struct slurm_prefix *prefix) { - /* TODO check for exact duplicates and overwritten rules */ - return al_assertion_prefix_add(&array_lists_db.assertion_pfx_al, - prefix); + if (prefix_assertion_is_new(&array_lists_db.assertion_pfx_al, prefix)) + return al_assertion_prefix_add( + &array_lists_db.assertion_pfx_al, prefix); + + return -EEXIST; } +/* + * Try to persist the @bgpsec filter, if it already exists or is covered + * by another filter, then the error -EEXIST is returned; otherwise, returns + * the result of persisting the @bgpsec. + */ int slurm_db_add_bgpsec_filter(struct slurm_bgpsec *bgpsec) { - /* TODO check for exact duplicates and overwritten rules */ - return al_filter_bgpsec_add(&array_lists_db.filter_bgps_al, bgpsec); + if (bgpsec_filter_is_new(&array_lists_db.filter_bgps_al, bgpsec)) + return al_filter_bgpsec_add(&array_lists_db.filter_bgps_al, + bgpsec); + + return -EEXIST; } +/* + * Try to persist the @bgpsec assertion, if it already exists, then the error + * -EEXIST is returned; otherwise, returns the result of persisting the + * @bgpsec. + */ int slurm_db_add_bgpsec_assertion(struct slurm_bgpsec *bgpsec) { - /* TODO check for exact duplicates and overwritten rules */ - return al_assertion_bgpsec_add(&array_lists_db.assertion_bgps_al, - bgpsec); + if (bgpsec_assertion_is_new(&array_lists_db.assertion_bgps_al, bgpsec)) + return al_assertion_bgpsec_add( + &array_lists_db.assertion_bgps_al, bgpsec); + + return -EEXIST; } static void @@ -77,76 +238,6 @@ clean_slurm_bgpsec(struct slurm_bgpsec *bgpsec) void slurm_db_cleanup(void) { - /* TODO TEST DEBUG */ - struct slurm_prefix *p; - struct slurm_bgpsec *b; - ARRAYLIST_FOREACH(&array_lists_db.filter_pfx_al, p) { - warnx("SLURM Prefix Filter:"); - if ((p->data_flag & SLURM_COM_FLAG_ASN) > 0) - warnx("-->ASN: %u", p->asn); - if ((p->data_flag & SLURM_COM_FLAG_COMMENT) > 0) - warnx("-->Comment: %s", p->comment); - if ((p->data_flag & SLURM_PFX_FLAG_PREFIX) > 0) { - warnx("-->Addr fam: %u", p->addr_fam); - warnx("-->Prefix len: %u", p->prefix_length); - } - } - - ARRAYLIST_FOREACH(&array_lists_db.filter_bgps_al, b) { - warnx("SLURM BGPsec Filter:"); - if ((b->data_flag & SLURM_COM_FLAG_ASN) > 0) - warnx("-->ASN: %u", b->asn); - if ((b->data_flag & SLURM_COM_FLAG_COMMENT) > 0) - warnx("-->Comment: %s", b->comment); - if ((b->data_flag & SLURM_BGPS_FLAG_SKI) > 0) { - warnx("-->SKI:"); - int i = 0; - for (; i < b->ski_len; i++) - warnx("---->[%d] = %02X", i, b->ski[i]); - } - if ((b->data_flag & SLURM_BGPS_FLAG_ROUTER_KEY) > 0) { - warnx("-->SPKI:"); - int i = 0; - for (; i < b->router_public_key_len; i++) - warnx("---->[%d] = %02X", i, - b->router_public_key[i]); - } - } - - ARRAYLIST_FOREACH(&array_lists_db.assertion_pfx_al, p) { - warnx("SLURM Prefix Assertion:"); - if ((p->data_flag & SLURM_COM_FLAG_ASN) > 0) - warnx("-->ASN: %u", p->asn); - if ((p->data_flag & SLURM_COM_FLAG_COMMENT) > 0) - warnx("-->Comment: %s", p->comment); - if ((p->data_flag & SLURM_PFX_FLAG_PREFIX) > 0) { - warnx("-->Addr fam: %u", p->addr_fam); - warnx("-->Prefix len: %u", p->prefix_length); - } - } - - ARRAYLIST_FOREACH(&array_lists_db.assertion_bgps_al, b) { - warnx("SLURM BGPsec Assertion:"); - if ((b->data_flag & SLURM_COM_FLAG_ASN) > 0) - warnx("-->ASN: %u", b->asn); - if ((b->data_flag & SLURM_COM_FLAG_COMMENT) > 0) - warnx("-->Comment: %s", b->comment); - if ((b->data_flag & SLURM_BGPS_FLAG_SKI) > 0) { - warnx("-->SKI:"); - int i = 0; - for (; i < b->ski_len; i++) - warnx("---->[%d] = %02X", i, b->ski[i]); - } - if ((b->data_flag & SLURM_BGPS_FLAG_ROUTER_KEY) > 0) { - warnx("-->SPKI:"); - int i = 0; - for (; i < b->router_public_key_len; i++) - warnx("---->[%d] = %02X", i, - b->router_public_key[i]); - } - } - warnx("**Deleting SLURM DB now**"); - al_filter_prefix_cleanup(&array_lists_db.filter_pfx_al, clean_slurm_prefix); al_filter_bgpsec_cleanup(&array_lists_db.filter_bgps_al, diff --git a/src/slurm_loader.c b/src/slurm_loader.c index f409d379..bae0987f 100644 --- a/src/slurm_loader.c +++ b/src/slurm_loader.c @@ -77,8 +77,11 @@ slurm_load(void) errno = 0; while ((dir_ent = readdir(dir_loc)) != NULL) { error = single_slurm_load(slurm_dir, dir_ent->d_name); - if (error) + if (error) { + warnx("The error was at SLURM file %s", + dir_ent->d_name); goto end; + } errno = 0; } if (errno) { diff --git a/src/slurm_parser.c b/src/slurm_parser.c index d88087f6..3d686008 100644 --- a/src/slurm_parser.c +++ b/src/slurm_parser.c @@ -214,7 +214,7 @@ set_max_prefix_length(json_t *object, bool is_assertion, u_int8_t addr_fam, json_int_t int_tmp; int error; - /* Ignore for filters */ + /* Handle error for filters */ if (!is_assertion) return 0; @@ -313,7 +313,7 @@ set_router_pub_key(json_t *object, bool is_assertion, char const *str_encoded; int error; - /* Ignore for filters */ + /* Handle error for filters */ if (!is_assertion) return 0; @@ -359,6 +359,17 @@ set_router_pub_key(json_t *object, bool is_assertion, return 0; } +static void +init_slurm_prefix(struct slurm_prefix *slurm_prefix) +{ + slurm_prefix->data_flag = SLURM_COM_FLAG_NONE; + slurm_prefix->asn = 0; + slurm_prefix->ipv6_prefix = in6addr_any; + slurm_prefix->prefix_length = 0; + slurm_prefix->max_prefix_length = 0; + slurm_prefix->addr_fam = 0; +} + static int load_single_prefix(json_t *object, bool is_assertion) { @@ -370,7 +381,7 @@ load_single_prefix(json_t *object, bool is_assertion) return -EINVAL; } - result.data_flag = SLURM_COM_FLAG_NONE; + init_slurm_prefix(&result); error = set_asn(object, is_assertion, &result.asn, &result.data_flag); if (error) @@ -380,29 +391,66 @@ load_single_prefix(json_t *object, bool is_assertion) if (error) return error; - error = set_comment(object, &result.comment, &result.data_flag); + error = set_max_prefix_length(object, is_assertion, result.addr_fam, + &result.max_prefix_length, &result.data_flag); if (error) return error; - error = set_max_prefix_length(object, is_assertion, result.addr_fam, - &result.max_prefix_length, &result.data_flag); + error = set_comment(object, &result.comment, &result.data_flag); if (error) return error; - if (is_assertion && (result.data_flag & SLURM_PFX_FLAG_MAX_LENGTH)) + /* A single comment isn't valid */ + if (result.data_flag == SLURM_COM_FLAG_COMMENT) { + warnx("Single comments aren't valid"); + error = -EINVAL; + goto release_comment; + } + + /* A filter must have ASN and/or prefix */ + if (!is_assertion) { + if ((result.data_flag & + (SLURM_COM_FLAG_ASN | SLURM_PFX_FLAG_PREFIX)) == 0) { + warnx("Prefix filter must have an asn and/or prefix"); + error = -EINVAL; + goto release_comment; + } + /* and can't have the max prefix length */ + if ((result.data_flag & SLURM_PFX_FLAG_MAX_LENGTH) > 0) { + warnx("Prefix filter can't have a max prefix length"); + error = -EINVAL; + goto release_comment; + } + + error = slurm_db_add_prefix_filter(&result); + if (error) + goto release_comment; + + return 0; + } + + /* + * An assertion must have ASN and prefix, the validation is done at + * set_asn and set_prefix + */ + + if ((result.data_flag & SLURM_PFX_FLAG_MAX_LENGTH) > 0) if (result.prefix_length > result.max_prefix_length) { warnx( "Prefix length is greater than max prefix length"); - return -EINVAL; + error = -EINVAL; + goto release_comment; } - /* Loaded! Now persist it */ - if (is_assertion) - slurm_db_add_prefix_assertion(&result); - else - slurm_db_add_prefix_filter(&result); + error = slurm_db_add_prefix_assertion(&result); + if (error) + goto release_comment; return 0; + +release_comment: + free((void *)result.comment); + return error; } static int @@ -414,16 +462,36 @@ load_prefix_array(json_t *array, bool is_assertion) json_array_foreach(array, index, element) { error = load_single_prefix(element, is_assertion); if (error) { - warnx( - "Error at prefix %s, element %d, ignoring content", - (is_assertion ? "assertions" : "filters"), - index + 1); + if (error == -EEXIST) + warnx( + "The prefix %s element #%d, is duplicated or covered by another %s; SLURM loading will be stopped", + (is_assertion ? "assertion" : "filter"), + index + 1, + (is_assertion ? "assertion" : "filter")); + else + warnx( + "Error at prefix %s, element #%d, SLURM loading will be stopped", + (is_assertion ? "assertions" : "filters"), + index + 1); + + return error; } } return 0; } +static void +init_slurm_bgpsec(struct slurm_bgpsec *slurm_bgpsec) +{ + slurm_bgpsec->data_flag = SLURM_COM_FLAG_NONE; + slurm_bgpsec->asn = 0; + slurm_bgpsec->ski = NULL; + slurm_bgpsec->ski_len = 0; + slurm_bgpsec->router_public_key = NULL; + slurm_bgpsec->router_public_key_len = 0; +} + static int load_single_bgpsec(json_t *object, bool is_assertion) { @@ -435,7 +503,7 @@ load_single_bgpsec(json_t *object, bool is_assertion) return -EINVAL; } - result.data_flag = SLURM_COM_FLAG_NONE; + init_slurm_bgpsec(&result); error = set_asn(object, is_assertion, &result.asn, &result.data_flag); if (error) @@ -453,16 +521,43 @@ load_single_bgpsec(json_t *object, bool is_assertion) if (error) goto release_router_key; - /* Loaded! Now persist it */ - if (is_assertion) - error = slurm_db_add_bgpsec_assertion(&result); - else + /* A single comment isn't valid */ + if (result.data_flag == SLURM_COM_FLAG_COMMENT) { + warnx("Single comments aren't valid"); + error = -EINVAL; + goto release_comment; + } + + /* A filter must have ASN and/or SKI */ + if (!is_assertion) { + if ((result.data_flag & + (SLURM_COM_FLAG_ASN | SLURM_BGPS_FLAG_SKI)) == 0) { + warnx("BGPsec filter must have an asn and/or SKI"); + error = -EINVAL; + goto release_comment; + } + /* and can't have the router public key */ + if ((result.data_flag & SLURM_BGPS_FLAG_ROUTER_KEY) > 0) { + warnx("BGPsec filter can't have a router public key"); + error = -EINVAL; + goto release_comment; + } + error = slurm_db_add_bgpsec_filter(&result); + if (error) + goto release_comment; + + return 0; + } + error = slurm_db_add_bgpsec_assertion(&result); if (error) - goto release_router_key; + goto release_comment; + return 0; +release_comment: + free((void *)result.comment); release_router_key: free(result.router_public_key); release_ski: @@ -479,10 +574,19 @@ load_bgpsec_array(json_t *array, bool is_assertion) json_array_foreach(array, index, element) { error = load_single_bgpsec(element, is_assertion); if (error) { - warnx( - "Error at bgpsec %s, element %d, ignoring content", - (is_assertion ? "assertions" : "filters"), - index + 1); + if (error == -EEXIST) + warnx( + "The bgpsec %s element #%d, is duplicated or covered by another %s; SLURM loading will be stopped", + (is_assertion ? "assertion" : "filter"), + index + 1, + (is_assertion ? "assertion" : "filter")); + else + warnx( + "Error at bgpsec %s, element #%d, SLURM loading will be stopped", + (is_assertion ? "assertions" : "filters"), + index + 1); + + return error; } } @@ -584,5 +688,12 @@ handle_json(json_t *root) if (error) return error; + /* + * TODO Any unknown members should be treated as errors, RFC8416 3.1: + * "JSON members that are not defined here MUST NOT be used in SLURM + * files. An RP MUST consider any deviations from the specifications to + * be errors." + */ + return 0; }