-Use an ID for the registered incidence.
-When loading multiple SLURM files, the validation from RFC 8416 section 4.2 wasn't considered. A context is used to perform such validation, since every prefix or asn (for bgpsec) must be validated according to its own context/file.
-Remove dead code (structs) from SLURM.
-Validate RTR port range since this isn't validated by getaddrinfo.
The RPKI RFCs define fairly strict profiles for RPKI objects, and are unequivocal in stating that incorrectly-formed objects are supposed to be rejected by Relying Party validation. In practice, however, this does not prevent a significant amount of Certificate Authorities from issuing incorrect objects.
-By default, Fort is as pedantic as it can possibly be. The `incidence` section of its configuration file is a means to modify its behavior upon encountering profile violations that, from experience, are often overlooked.
+By default, Fort is lax with some of this bad practices. The `incidence` section of its configuration file is a means to modify its behavior upon encountering profile violations that, from experience, are often overlooked.
## `incidences` definition
```
"incidences": [
{
- "name": "Signed Object's hash algorithm has NULL object as parameters",
+ "name": "incid-hashalg-has-params",
"action": "warn"
}
]
```
-`name` is the identifier of an incidence. It is case-sensitive and developer-defined. It states the particular error condition that will be handled by the remaining field.
+`name` is the identifier of an incidence. It is case-sensitive and developer-defined. It states an ID of the particular error condition that will be handled by the remaining field.
`action` is an enumeration that states the outcome of a violation of the corresponding incidence. It can take one of three values:
2. `warn`: Print error message in `warning` log level, continue validation as if nothing happened.
3. `ignore`: Do not print error message, continue validation as if nothing happened.
-By Fort's pedantic nature, most incidences have an `action` of `error` by default.
+Since most of the incidences are result of a bad practice at the global RPKI, they have an `action` of `ignore` by default. If a strict behavior is desired, then the corresponding incidences should be configured with an `action` of `error`.
## Incidence types
### Signed Object's hash algorithm has NULL object as parameters
+- **Name:** `incid-hashalg-has-params`
+- **Default action:** `ignore`
+
[RFC 6488](https://tools.ietf.org/html/rfc6488) (RPKI Signed Objects) defers digest algorithm specification to RFC 6485:
```
"<a href="#incidences">incidences</a>": [
{
- "name": "Signed Object's hash algorithm has NULL object as parameters",
+ "name": "incid-hashalg-has-params",
"action": "ignore"
}
],
.RS 2
{
.RS 2
-"name": "Signed Object's hash algorithm has NULL object as parameters",
+"name": "incid-hashalg-has-params",
.br
"action": "warn"
.RE
happened.
.RE
.P
-By default, all the incidences have an action of \fIerror\fR. Currently there's
+By default, all the incidences have an action of \fIignore\fR. Currently there's
only one registered incidence:
-\fISigned Object's hash algorithm has NULL object as parameters\fR.
+\fIincid-hashalg-has-params\fR (Signed Object's hash algorithm has NULL object
+as parameters).
.P
More information about incidences can be consulted at FORT's web docs.
.RE
Right now, FORT accesses RPKI repositories by way of \fIrsync\fR. During each
validation cycle, FORT will literally invoke an rsync command (see
\fBrsync.program\fR and \fBrsync.arguments-recursive\fR), which will download
-the files into \fB--local-repository\fR. FORT’s validation operates on the resulting
-copy.
+the files into \fB--local-repository\fR. FORT’s validation operates on the
+resulting copy.
.P
Because rsync uses delta encoding, you’re advised to keep this cache around. It
significantly speeds up subsequent validation cycles.
},
"incidences": [
{
- "name": "Signed Object's hash algorithm has NULL object as parameters",
+ "name": "incid-hashalg-has-params",
"action": "ignore"
}
],
struct incidence {
const enum incidence_id id;
char const *const name;
+ char const *const description;
const enum incidence_action default_action;
enum incidence_action action;
};
static struct incidence incidences[__INID_MAX] = {
{
INID_HASHALG_HAS_PARAMS,
+ "incid-hashalg-has-params",
"Signed Object's hash algorithm has NULL object as parameters",
- INAC_ERROR,
+ INAC_IGNORE,
},
};
else
return pr_err("Unknown incidence action: '%s'", action_str);
- if (action > incidences[id].action)
- return pr_err("The '%s' incidence cannot have a more severe action than '%s'.",
- name, action2str(incidences[id].action));
-
incidences[id].action = action;
return 0;
}
for (i = 0; i < __INID_MAX; i++) {
if (incidences[i].action != incidences[i].default_action) {
- pr_info("%s: %s", incidences[i].name,
+ pr_info("%s (%s): %s", incidences[i].name,
+ incidences[i].description,
action2str(incidences[i].action));
printed = true;
}
{
char const *hostname;
char const *service;
+ char *tmp;
struct addrinfo hints;
+ unsigned long parsed, port;
int error;
memset(&hints, 0 , sizeof(hints));
return error;
}
+ errno = 0;
+ parsed = strtoul(service, &tmp, 10);
+ if (errno || *tmp != '\0')
+ return 0; /* Ok, not a number */
+
+ /*
+ * 'getaddrinfo' isn't very strict validating the service when a port
+ * number is indicated. If a port larger than the max (65535) is
+ * received, the 16 rightmost bits are utilized as the port and set at
+ * the addrinfo returned.
+ *
+ * So, a manual validation is implemented. Port is actually a uint16_t,
+ * so read what's necessary and compare using the same data type.
+ */
+ port = (unsigned char)((*result)->ai_addr->sa_data[0]) << 8;
+ port += (unsigned char)((*result)->ai_addr->sa_data[1]);
+ if (parsed != port)
+ return pr_err("Service port %s is out of range (max value is %d)",
+ service, USHRT_MAX);
+
return 0;
}
#include "data_structure/array_list.h"
-ARRAY_LIST(al_filter_prefix, struct slurm_prefix)
-ARRAY_LIST(al_assertion_prefix, struct slurm_prefix)
-ARRAY_LIST(al_filter_bgpsec, struct slurm_bgpsec)
-ARRAY_LIST(al_assertion_bgpsec, struct slurm_bgpsec)
+struct slurm_prefix_ctx {
+ struct slurm_prefix element;
+ int ctx;
+};
+
+struct slurm_bgpsec_ctx {
+ struct slurm_bgpsec element;
+ int ctx;
+};
+
+ARRAY_LIST(al_filter_prefix, struct slurm_prefix_ctx)
+ARRAY_LIST(al_assertion_prefix, struct slurm_prefix_ctx)
+ARRAY_LIST(al_filter_bgpsec, struct slurm_bgpsec_ctx)
+ARRAY_LIST(al_assertion_bgpsec, struct slurm_bgpsec_ctx)
struct arraylist_db {
struct al_filter_prefix filter_pfx_al;
}
static bool
-prefix_equal(struct slurm_prefix *left, struct slurm_prefix *right,
- bool filter)
+prefix_contained(struct slurm_prefix_ctx *left_ctx, struct slurm_prefix *right,
+ int ctx)
{
+ struct slurm_prefix *left;
+ struct vrp *left_vrp, *right_vrp;
+
+ /*
+ * rfc8416#section-4.2:
+ * 1. There may be conflicting changes to ROA Prefix Assertions if an
+ * IP address X and distinct SLURM files Y and Z exist such that X
+ * is contained by any prefix in any "prefixAssertions" or
+ * "prefixFilters" in file Y and X is contained by any prefix in any
+ * "prefixAssertions" or "prefixFilters" in file Z.
+ *
+ * A negative @ctx or an equal context will avoid this check.
+ */
+ if (ctx < 0 || ctx == left_ctx->ctx)
+ return false;
+
+ left = &left_ctx->element;
+ left_vrp = &left->vrp;
+ right_vrp = &right->vrp;
+
+ return (left->data_flag & SLURM_PFX_FLAG_PREFIX) > 0 &&
+ (right->data_flag & SLURM_PFX_FLAG_PREFIX) > 0 &&
+ VRP_PREFIX_EQ(left_vrp, right_vrp);
+}
+
+static bool
+prefix_equal(struct slurm_prefix_ctx *left_ctx, struct slurm_prefix *right,
+ int ctx, bool filter)
+{
+ struct slurm_prefix *left;
struct vrp *left_vrp, *right_vrp;
bool equal;
+ left = &left_ctx->element;
left_vrp = &left->vrp;
right_vrp = &right->vrp;
+ if (prefix_contained(left_ctx, right, ctx))
+ return true;
+
/* Ignore the comments */
if ((left->data_flag & ~SLURM_COM_FLAG_COMMENT) !=
(right->data_flag & ~SLURM_COM_FLAG_COMMENT))
}
static bool
-bgpsec_equal(struct slurm_bgpsec *left, struct slurm_bgpsec *right,
- bool filter)
+bgpsec_contained(struct slurm_bgpsec_ctx *left_ctx, struct slurm_bgpsec *right,
+ int ctx)
{
+ struct slurm_bgpsec *left;
+
+ /*
+ * rfc8416#section-4.2:
+ * 2. There may be conflicting changes to BGPsec Assertions if an ASN X
+ * and distinct SLURM files Y and Z exist such that X is used in any
+ * "bgpsecAssertions" or "bgpsecFilters" in file Y and X is used in
+ * any "bgpsecAssertions" or "bgpsecFilters" in file Z.
+ *
+ * A negative @ctx or an equal context will avoid this check.
+ */
+ if (ctx < 0 || ctx == left_ctx->ctx)
+ return false;
+
+ left = &left_ctx->element;
+
+ return (left->data_flag & SLURM_COM_FLAG_ASN) > 0 &&
+ (right->data_flag & SLURM_COM_FLAG_ASN) > 0 &&
+ left->asn == right->asn;
+}
+
+static bool
+bgpsec_equal(struct slurm_bgpsec_ctx *left_ctx, struct slurm_bgpsec *right,
+ int ctx, bool filter)
+{
+ struct slurm_bgpsec *left;
bool equal;
+ left = &left_ctx->element;
+
+ if (bgpsec_contained(left_ctx, right, ctx))
+ return true;
+
/* Ignore the comments */
if ((left->data_flag & ~SLURM_COM_FLAG_COMMENT) !=
(right->data_flag & ~SLURM_COM_FLAG_COMMENT))
return equal;
}
-#define ADD_FUNCS(name, type, list_name, db_list, equal_cb, filter) \
+#define ADD_FUNCS(name, type, list_name, db_list, db_alt_list, equal_cb,\
+ cont_cb, filter) \
static type * \
- name##_locate(type *obj) \
+ name##_locate(type *obj, int ctx) \
{ \
- type *cursor; \
+ type##_ctx *cursor; \
array_index i; \
\
ARRAYLIST_FOREACH(db_list, cursor, i) \
- if (equal_cb(cursor, obj, filter)) \
- return cursor; \
+ if (equal_cb(cursor, obj, ctx, filter)) \
+ return &cursor->element; \
+ \
+ ARRAYLIST_FOREACH(db_alt_list, cursor, i) \
+ if (cont_cb(cursor, obj, ctx)) \
+ return &cursor->element; \
\
return NULL; \
} \
\
static bool \
- name##_exists(type *obj) \
+ name##_exists(type *obj, int ctx) \
{ \
- return name##_locate(obj) != NULL; \
+ return name##_locate(obj, ctx) != NULL; \
} \
\
int \
- slurm_db_add_##name(type *elem) { \
- if (name##_exists(elem)) \
+ slurm_db_add_##name(type *elem, int ctx) { \
+ type##_ctx new_elem; \
+ if (name##_exists(elem, ctx)) \
return -EEXIST; \
- return list_name##_add(db_list, elem); \
+ new_elem.element = *elem; \
+ new_elem.ctx = ctx; \
+ return list_name##_add(db_list, &new_elem); \
}
ADD_FUNCS(prefix_filter, struct slurm_prefix, al_filter_prefix,
- &array_lists_db.filter_pfx_al, prefix_equal, true)
+ &array_lists_db.filter_pfx_al, &array_lists_db.assertion_pfx_al,
+ prefix_equal, prefix_contained, true)
ADD_FUNCS(bgpsec_filter, struct slurm_bgpsec, al_filter_bgpsec,
- &array_lists_db.filter_bgps_al, bgpsec_equal, true)
+ &array_lists_db.filter_bgps_al, &array_lists_db.assertion_bgps_al,
+ bgpsec_equal, bgpsec_contained, true)
ADD_FUNCS(prefix_assertion, struct slurm_prefix, al_assertion_prefix,
- &array_lists_db.assertion_pfx_al, prefix_equal, false)
+ &array_lists_db.assertion_pfx_al, &array_lists_db.filter_pfx_al,
+ prefix_equal, prefix_contained, false)
ADD_FUNCS(bgpsec_assertion, struct slurm_bgpsec, al_assertion_bgpsec,
- &array_lists_db.assertion_bgps_al, bgpsec_equal, false)
+ &array_lists_db.assertion_bgps_al, &array_lists_db.filter_bgps_al,
+ bgpsec_equal, bgpsec_contained, false)
bool
slurm_db_vrp_is_filtered(struct vrp const *vrp)
slurm_prefix.vrp = *vrp;
slurm_prefix.comment = NULL;
- return prefix_filter_exists(&slurm_prefix);
+ return prefix_filter_exists(&slurm_prefix, -1);
}
int
slurm_db_foreach_assertion_prefix(assertion_pfx_foreach_cb cb, void *arg)
{
- struct slurm_prefix *cursor;
+ struct slurm_prefix_ctx *cursor;
array_index i;
int error;
ARRAYLIST_FOREACH(&array_lists_db.assertion_pfx_al, cursor, i) {
- error = cb(cursor, arg);
+ error = cb(&cursor->element, arg);
if (error)
return error;
}
}
static void
-clean_slurm_prefix(struct slurm_prefix *prefix)
+clean_slurm_prefix(struct slurm_prefix_ctx *prefix)
{
- if ((prefix->data_flag & SLURM_COM_FLAG_COMMENT) > 0)
- free(prefix->comment);
+ if ((prefix->element.data_flag & SLURM_COM_FLAG_COMMENT) > 0)
+ free(prefix->element.comment);
}
static void
-clean_slurm_bgpsec(struct slurm_bgpsec *bgpsec)
+clean_slurm_bgpsec(struct slurm_bgpsec_ctx *bgpsec)
{
- if ((bgpsec->data_flag & SLURM_BGPS_FLAG_SKI) > 0)
- free(bgpsec->ski);
- if ((bgpsec->data_flag & SLURM_BGPS_FLAG_ROUTER_KEY) > 0)
- free(bgpsec->router_public_key);
- if ((bgpsec->data_flag & SLURM_COM_FLAG_COMMENT) > 0)
- free(bgpsec->comment);
+ if ((bgpsec->element.data_flag & SLURM_BGPS_FLAG_SKI) > 0)
+ free(bgpsec->element.ski);
+ if ((bgpsec->element.data_flag & SLURM_BGPS_FLAG_ROUTER_KEY) > 0)
+ free(bgpsec->element.router_public_key);
+ if ((bgpsec->element.data_flag & SLURM_COM_FLAG_COMMENT) > 0)
+ free(bgpsec->element.comment);
}
void
#include <stdbool.h>
#include "slurm/slurm_parser.h"
-struct slurm_prefix_list {
- struct slurm_prefix *list;
- unsigned int len;
-};
-
-struct slurm_bgpsec_list {
- struct slurm_bgpsec *list;
- unsigned int len;
-};
-
-struct slurm_db {
- struct slurm_prefix_list prefix_filters;
- struct slurm_bgpsec_list bgpsec_filters;
- struct slurm_prefix_list prefix_assertions;
- struct slurm_bgpsec_list bgpsec_assertions;
-};
-
typedef int (*assertion_pfx_foreach_cb)(struct slurm_prefix *, void *);
void slurm_db_init(void);
-int slurm_db_add_prefix_filter(struct slurm_prefix *);
-int slurm_db_add_prefix_assertion(struct slurm_prefix *);
-int slurm_db_add_bgpsec_filter(struct slurm_bgpsec *);
-int slurm_db_add_bgpsec_assertion(struct slurm_bgpsec *);
+int slurm_db_add_prefix_filter(struct slurm_prefix *, int);
+int slurm_db_add_prefix_assertion(struct slurm_prefix *, int);
+int slurm_db_add_bgpsec_filter(struct slurm_bgpsec *, int);
+int slurm_db_add_bgpsec_assertion(struct slurm_bgpsec *, int);
bool slurm_db_vrp_is_filtered(struct vrp const *vrp);
int slurm_db_foreach_assertion_prefix(assertion_pfx_foreach_cb, void *);
static int
slurm_load(bool *loaded)
{
+ int ctx = 0; /* Context (file number) */
/* Optional configuration */
*loaded = false;
if (config_get_slurm() == NULL)
*loaded = true;
slurm_db_init();
- return process_file_or_dir(config_get_slurm(),
- SLURM_FILE_EXTENSION, slurm_parse, NULL);
+ return process_file_or_dir(config_get_slurm(), SLURM_FILE_EXTENSION,
+ slurm_parse, &ctx);
}
static void
if (element == NULL) \
return pr_err("SLURM member '%s' is required", name);
-static int handle_json(json_t *);
+/* Context value, local to avoid forwarding the parameter */
+int cur_ctx;
+
+static int handle_json(json_t *, int *);
int
slurm_parse(char const *location, void *arg)
return -ENOENT;
}
- error = handle_json(json_root);
+ error = handle_json(json_root, arg);
json_decref(json_root);
return error;
goto release_comment;
}
- error = slurm_db_add_prefix_filter(&result);
+ error = slurm_db_add_prefix_filter(&result, cur_ctx);
if (error)
goto release_comment;
goto release_comment;
}
- error = slurm_db_add_prefix_assertion(&result);
+ error = slurm_db_add_prefix_assertion(&result, cur_ctx);
if (error)
goto release_comment;
json_array_foreach(array, index, element) {
error = load_single_prefix(element, is_assertion);
- if (error) {
- if (error == -EEXIST)
- pr_err(
- "The prefix %s element \"%s\", is duplicated or covered by another %s; SLURM loading will be stopped",
- (is_assertion ? "assertion" : "filter"),
- json_dumps(element, 0),
- (is_assertion ? "assertion" : "filter"));
- else
- pr_err(
- "Error at prefix %s, element \"%s\", SLURM loading will be stopped",
- (is_assertion ? "assertions" : "filters"),
- json_dumps(element, 0));
+ if (!error)
+ continue;
+ if (error == -EEXIST)
+ pr_err(
+ "The prefix %s element \"%s\", is duplicated or covered by another %s; SLURM loading will be stopped.%s",
+ (is_assertion ? "assertion" : "filter"),
+ json_dumps(element, 0),
+ (is_assertion ? "assertion" : "filter"),
+ (cur_ctx > 0
+ ? " TIP: More than 1 SLURM files were found, check if the prefix is contained in multiple files (see RFC 8416 section 4.2)."
+ : ""));
+ else
+ pr_err(
+ "Error at prefix %s, element \"%s\", SLURM loading will be stopped",
+ (is_assertion ? "assertions" : "filters"),
+ json_dumps(element, 0));
- return error;
- }
+ return error;
}
return 0;
goto release_comment;
}
- error = slurm_db_add_bgpsec_filter(&result);
+ error = slurm_db_add_bgpsec_filter(&result, cur_ctx);
if (error)
goto release_comment;
goto release_comment;
}
- error = slurm_db_add_bgpsec_assertion(&result);
+ error = slurm_db_add_bgpsec_assertion(&result, cur_ctx);
if (error)
goto release_comment;
json_array_foreach(array, index, element) {
error = load_single_bgpsec(element, is_assertion);
- if (error) {
- if (error == -EEXIST)
- pr_err(
- "The bgpsec %s element \"%s\", is duplicated or covered by another %s; SLURM loading will be stopped",
- (is_assertion ? "assertion" : "filter"),
- json_dumps(element, 0),
- (is_assertion ? "assertion" : "filter"));
- else
- pr_err(
- "Error at bgpsec %s, element \"%s\", SLURM loading will be stopped",
- (is_assertion ? "assertions" : "filters"),
- json_dumps(element, 0));
+ if (!error)
+ continue;
+ if (error == -EEXIST)
+ pr_err(
+ "The bgpsec %s element \"%s\", is duplicated or covered by another %s; SLURM loading will be stopped.%s",
+ (is_assertion ? "assertion" : "filter"),
+ json_dumps(element, 0),
+ (is_assertion ? "assertion" : "filter"),
+ (cur_ctx > 0
+ ? " TIP: More than 1 SLURM files were found, check if the ASN is contained in multiple files (see RFC 8416 section 4.2)."
+ : ""));
+ else
+ pr_err(
+ "Error at bgpsec %s, element \"%s\", SLURM loading will be stopped",
+ (is_assertion ? "assertions" : "filters"),
+ json_dumps(element, 0));
- return error;
- }
+ return error;
}
return 0;
}
static int
-handle_json(json_t *root)
+handle_json(json_t *root, int *ctx)
{
size_t expected_members;
int error;
if (!json_is_object(root))
return pr_err("The root of the SLURM is not a JSON object.");
+ cur_ctx = *ctx;
+
error = load_version(root);
if (error)
return error;
"SLURM root must have only %lu members (RFC 8416 section 3.2)",
expected_members);
+ (*ctx)++; /* Next time will be another context */
+
return 0;
}