From: Alberto Leiva Popper Date: Mon, 27 May 2019 21:47:59 +0000 (-0500) Subject: Revert "Remove the incidence framework" X-Git-Tag: v0.0.2~22 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=19154d6831ae76000dcd1534d489c9d2a2516c24;p=thirdparty%2FFORT-validator.git Revert "Remove the incidence framework" This reverts most of commit c719f7a79ea307b609be0747d1a080b3452917d7. ¯\_(ツ)_/¯ Removes old incidences, adds "Signed Object's hash algorithm has NULL object as parameters" incidence. --- diff --git a/docs/doc/incidence.md b/docs/doc/incidence.md new file mode 100644 index 00000000..0c3b3866 --- /dev/null +++ b/docs/doc/incidence.md @@ -0,0 +1,87 @@ +--- +title: Incidence +--- + +[Documentation](index.html) > {{ page.title }} + +# {{ page.title }} + +## Index + +1. [Introduction](#introduction) +2. [`incidences` definition](#incidences-definition) +3. [Incidence types](#incidence-types) + 1. [Signed Object's hash algorithm has NULL object as parameters](#signed-objects-hash-algorithm-has-null-object-as-parameters) + +## Introduction + +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 stop 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. + +## `incidences` definition + +`incidences` is a JSON array that contains (anonymous) incidence elements. Here's an example: + +``` +"incidences": [ + { + "name": "Signed Object's hash algorithm has NULL object as parameters", + "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. + +`action` is an enumeration that states the outcome of a violation of the corresponding incidence. It can take one of three values: + +1. `error`: Print error message in `error` log level, fail validation of the offending object (and all of its children). +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. + +## Incidence types + +Presently, there is only one incidence type defined. This list is expected to grow when strict DER-parsing is implemented, and might also evolve further over time, depending on the state of the global RPKI and user demand. + +### Signed Object's hash algorithm has NULL object as parameters + +[RFC 6488](https://tools.ietf.org/html/rfc6488) (RPKI Signed Objects) defers digest algorithm specification to RFC 6485: + +``` + The digestAlgorithms set contains the OIDs of the digest algorithm(s) + used in signing the encapsulated content. This set MUST contain + exactly one digest algorithm OID, and the OID MUST be selected from + those specified in [RFC6485]. +``` + +[6485](https://tools.ietf.org/html/rfc6485) has been obsoleted by [7935](https://tools.ietf.org/html/rfc7935), which states the following: + +``` + The object identifier and + parameters for SHA-256 (as defined in [RFC5754]) MUST be used for the + SignedData digestAlgorithms field and the SignerInfo digestAlgorithm + field. +``` + +[RFC 5754](https://tools.ietf.org/html/rfc5754): + +``` + There are two possible encodings for the AlgorithmIdentifier + parameters field associated with these object identifiers. (...) + some implementations encode parameters as a NULL element + while others omit them entirely. The correct encoding is to omit the + parameters field; +``` + +As of 2019-05-21, many signed objects in the global RPKI break this rule. + +If not `ignore`d, Fort will report this incidence with the following error message: + +``` +: : The hash algorithm of the '' has a NULL object as parameters +``` + +This only applies to digest parameters that have been defined as NULL objects; any other type of non-absent digest parameters will yield a different error message, and will therefore not be silenced. diff --git a/docs/doc/index.md b/docs/doc/index.md index 9111656c..0dd1f817 100644 --- a/docs/doc/index.md +++ b/docs/doc/index.md @@ -10,3 +10,4 @@ title: Documentation Index 4. [Running Fort](run.html) 5. [Fort usage](usage.html) 6. [SLURM](slurm.html) +7. [Incidences](incidence.html) diff --git a/docs/doc/usage.md b/docs/doc/usage.md index 62d4a49c..6e4f4a2f 100644 --- a/docs/doc/usage.md +++ b/docs/doc/usage.md @@ -34,6 +34,7 @@ command: fort 17. [`rsync.program`](#rsyncprogram) 18. [`rsync.arguments-recursive`](#rsyncarguments-recursive) 19. [`rsync.arguments-flat`](#rsyncarguments-flat) + 20. [`incidences`](#incidences) ## Syntax @@ -364,7 +365,14 @@ The configuration options are mostly the same as the ones from the `argv` interf "$REMOTE", "$LOCAL" ] - } + }, + + "incidences": [ + { + "name": "Signed Object's hash algorithm has NULL object as parameters", + "action": "ignore" + } + ] } @@ -432,3 +440,10 @@ Fort will replace `"$REMOTE"` with the remote URL it needs to download, and `"$L Arguments needed by [`rsync.program`](#rsyncprogram) to perform a single-file rsync. Fort will replace `"$REMOTE"` with the remote URL it needs to download, and `"$LOCAL"` with the target local directory where the file is supposed to be dropped. + +### `incidences` + +- **Type:** JSON Object +- **Availability:** JSON only + +A listing of actions to be performed by validation upon encountering certain error conditions. See [incidence](incidence.html). diff --git a/src/Makefile.am b/src/Makefile.am index d14e2319..40a8d9b1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -41,6 +41,7 @@ fort_SOURCES += asn1/signed_data.h asn1/signed_data.c fort_SOURCES += config/boolean.c config/boolean.h fort_SOURCES += config/filename_format.h config/filename_format.c +fort_SOURCES += config/incidences.h config/incidences.c fort_SOURCES += config/str.c config/str.h fort_SOURCES += config/string_array.h config/string_array.c fort_SOURCES += config/sync_strategy.h config/sync_strategy.c @@ -56,6 +57,8 @@ fort_SOURCES += data_structure/common.h fort_SOURCES += data_structure/uthash.h fort_SOURCES += data_structure/uthash_nonfatal.h +fort_SOURCES += incidence/incidence.h incidence/incidence.c + fort_SOURCES += object/certificate.h object/certificate.c fort_SOURCES += object/crl.h object/crl.c fort_SOURCES += object/ghostbusters.h object/ghostbusters.c diff --git a/src/algorithm.c b/src/algorithm.c index 493aed78..02e93eb9 100644 --- a/src/algorithm.c +++ b/src/algorithm.c @@ -4,6 +4,18 @@ #include #include "log.h" +static bool +is_asn1_null_object(ANY_t *any) +{ + return (any->size == 2) && (any->buf[0] == 5) && (any->buf[1] == 0); +} + +static bool +is_asn1_null(ANY_t *any) +{ + return (any == NULL) || is_asn1_null_object(any); +} + /** * This function can also be used for CRLs. */ @@ -52,11 +64,21 @@ validate_cms_hashing_algorithm(AlgorithmIdentifier_t *id, char const *what) * some implementations encode parameters as a NULL element * while others omit them entirely. The correct encoding is to omit the * parameters field; + * + * We will treat NULL object parameters as one type of error, and any + * other type of present parameters as a different error. The former + * will be silenceable, because many people are breaking the rule. */ - if (id->parameters != NULL) - pr_warn("The hash algorithm of the '%s' has parameters", what); - - return 0; + if (id->parameters != NULL) { + error = is_asn1_null_object(id->parameters) + ? incidence(INID_HASHALG_HAS_PARAMS, + "The hash algorithm of the '%s' has a NULL object as parameters", + what) + : pr_err("The hash algorithm of the '%s' has parameters", + what); + } + + return error; } int @@ -88,20 +110,6 @@ incorrect_oid: return pr_err("The hash algorithm of the '%s' is not SHA256.", what); } -static int -is_asn1_null(ANY_t *any) -{ - if (any == NULL) - return true; - - if (any->size != 2) - return false; - if (any->buf[0] != 5 || any->buf[1] != 0) - return false; - - return true; -} - int validate_cms_signature_algorithm(AlgorithmIdentifier_t *id) { diff --git a/src/config.c b/src/config.c index 192d4efb..9b107355 100644 --- a/src/config.c +++ b/src/config.c @@ -11,6 +11,7 @@ #include "json_handler.h" #include "log.h" #include "config/boolean.h" +#include "config/incidences.h" #include "config/str.h" #include "config/uint.h" #include "config/uint32.h" @@ -271,6 +272,15 @@ static const struct option_field options[] = { .doc = "File name variant to print during debug/error messages", }, + /* Incidences */ + { + .id = 4001, + .name = "incidences", + .type = >_incidences, + .doc = "Override actions on validation errors", + .availability = AVAILABILITY_JSON, + }, + { 0 }, }; diff --git a/src/config/incidences.c b/src/config/incidences.c new file mode 100644 index 00000000..cc068164 --- /dev/null +++ b/src/config/incidences.c @@ -0,0 +1,22 @@ +#include "config/incidences.h" + +#include +#include "incidence/incidence.h" + +static void +incidences_print(struct option_field const *field, void *_value) +{ + incidence_print(); +} + +static int +incidences_parse_json(struct option_field const *opt, json_t *json, + void *_result) +{ + return incidence_update(json); +} + +const struct global_type gt_incidences = { + .print = incidences_print, + .parse.json = incidences_parse_json, +}; diff --git a/src/config/incidences.h b/src/config/incidences.h new file mode 100644 index 00000000..9e731b5f --- /dev/null +++ b/src/config/incidences.h @@ -0,0 +1,8 @@ +#ifndef SRC_CONFIG_INCIDENCES_H_ +#define SRC_CONFIG_INCIDENCES_H_ + +#include "config/types.h" + +extern const struct global_type gt_incidences; + +#endif /* SRC_CONFIG_INCIDENCES_H_ */ diff --git a/src/incidence/incidence.c b/src/incidence/incidence.c new file mode 100644 index 00000000..5cd7fb34 --- /dev/null +++ b/src/incidence/incidence.c @@ -0,0 +1,162 @@ +#include "incidence/incidence.h" + +#include +#include +#include +#include "common.h" +#include "json_parser.h" +#include "log.h" +#include "data_structure/common.h" + +struct incidence { + const enum incidence_id id; + char const *const name; + const enum incidence_action default_action; + enum incidence_action action; +}; + +static struct incidence incidences[__INID_MAX] = { + { + INID_HASHALG_HAS_PARAMS, + "Signed Object's hash algorithm has NULL object as parameters", + INAC_ERROR, + }, +}; + +static int +name2id(char const *name, enum incidence_id *id) +{ + array_index i; + + for (i = 0; i < __INID_MAX; i++) { + if (strcmp(name, incidences[i].name) == 0) { + *id = i; + return 0; + } + } + + return pr_err("Unknown incidence name: %s", name); +} + +static char const * +action2str(enum incidence_action action) +{ + switch (action) { + case INAC_IGNORE: + return "ignore"; + case INAC_WARN: + return "warn"; + case INAC_ERROR: + return "error"; + } + + return "unknown"; +} + +static int +init_action(json_t *json) +{ + enum incidence_id id; + char const *name; + char const *action_str; + enum incidence_action action; + int error; + + error = json_get_string(json, "name", &name); + if (error) + return error; + error = name2id(name, &id); + if (error) + return error; + error = json_get_string(json, "action", &action_str); + if (error) + return error; + + if (strcmp("ignore", action_str) == 0) + action = INAC_IGNORE; + else if (strcmp("warn", action_str) == 0) + action = INAC_WARN; + else if (strcmp("error", action_str) == 0) + action = INAC_ERROR; + 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; +} + +/** + * Concurrent inits are allowed. + */ +int +incidence_init(void) +{ + array_index i; + + /* Make sure the programmer didn't desync the id enum and the array. */ + assert(__INID_MAX == ARRAY_LEN(incidences)); + for (i = 0; i < __INID_MAX; i++) { + assert(i == incidences[i].id); + /* Also init. */ + incidences[i].action = incidences[i].default_action; + } + + return 0; +} + +/** + * Concurrent calls to this function are allowed. + */ +int +incidence_update(json_t *json) +{ + array_index i; + json_t *child; + int error; + + if (!json_is_array(json)) + return pr_err("The incidences JSON element is supposed to be an array."); + + json_array_foreach(json, i, child) { + error = init_action(child); + if (error) + return error; + } + + return 0; +} + +void +incidence_print(void) +{ + array_index i; + bool printed; + + pr_info("Custom incidences:"); + pr_indent_add(); + + printed = false; + + for (i = 0; i < __INID_MAX; i++) { + if (incidences[i].action != incidences[i].default_action) { + pr_info("%s: %s", incidences[i].name, + action2str(incidences[i].action)); + printed = true; + } + } + + if (!printed) + pr_info(""); + + pr_indent_rm(); +} + +enum incidence_action +incidence_get_action(enum incidence_id id) +{ + return incidences[id].action; +} diff --git a/src/incidence/incidence.h b/src/incidence/incidence.h new file mode 100644 index 00000000..819a48f5 --- /dev/null +++ b/src/incidence/incidence.h @@ -0,0 +1,40 @@ +#ifndef SRC_INCIDENCE_INCIDENCE_H_ +#define SRC_INCIDENCE_INCIDENCE_H_ + +#include + +/* + * Note: If you need to add, modify or delete an element from this enum, + * remember that you also need to add it to the incidences array. That's all. + */ +enum incidence_id { + INID_HASHALG_HAS_PARAMS, + + __INID_MAX, +}; + +enum incidence_action { + /** + * Do not print error message, continue validation as if nothing + * happened. + */ + INAC_IGNORE, + /** + * Print error message in warning log level, continue validation as if + * nothing happened. + */ + INAC_WARN, + /** + * Print error message in error log level, fail validation of the + * offending object (and all of its children). + */ + INAC_ERROR, +}; + +int incidence_init(void); /* incidence_destroy() is not needed. */ +int incidence_update(json_t *); + +void incidence_print(void); +enum incidence_action incidence_get_action(enum incidence_id); + +#endif /* SRC_INCIDENCE_INCIDENCE_H_ */ diff --git a/src/log.c b/src/log.c index eba8efe5..118e1497 100644 --- a/src/log.c +++ b/src/log.c @@ -268,3 +268,30 @@ pr_crit(const char *format, ...) PR_SUFFIX(STDERR); return -EINVAL; } + +/** + * Prints the [format, ...] error message using the configured logging severity + * of the @id incidence. + */ +int +incidence(enum incidence_id id, const char *format, ...) +{ + enum incidence_action action; + va_list args; + + action = incidence_get_action(id); + switch (action) { + case INAC_IGNORE: + return 0; + case INAC_WARN: + PR_PREFIX(STDERR, COLOR_WARNING, "WRN", args); + PR_SUFFIX(STDERR); + return 0; + case INAC_ERROR: + PR_PREFIX(STDERR, COLOR_ERROR, "ERR", args); + PR_SUFFIX(STDERR); + return -EINVAL; + } + + return pr_crit("Unknown incidence action: %u", action); +} diff --git a/src/log.h b/src/log.h index 466a4cfc..f7f2e3a6 100644 --- a/src/log.h +++ b/src/log.h @@ -1,6 +1,8 @@ #ifndef SRC_LOG_H_ #define SRC_LOG_H_ +#include "incidence/incidence.h" + /* * I know that the OpenBSD style guide says that we shouldn't declare our own * error printing functions, but we kind of need to do it: @@ -59,6 +61,8 @@ int crypto_err(const char *, ...) CHECK_FORMAT(1, 2); int pr_enomem(void); int pr_crit(const char *, ...) CHECK_FORMAT(1, 2); +int incidence(enum incidence_id, const char *, ...) CHECK_FORMAT(2, 3); + #define PR_DEBUG printf("%s:%d (%s())\n", __FILE__, __LINE__, __func__) #define PR_DEBUG_MSG(msg, ...) printf("%s:%d (%s()): " msg "\n", \ __FILE__, __LINE__, __func__, ##__VA_ARGS__) diff --git a/src/main.c b/src/main.c index 22426ed1..4c129c4b 100644 --- a/src/main.c +++ b/src/main.c @@ -33,6 +33,9 @@ main(int argc, char **argv) print_stack_trace_on_segfault(); error = thvar_init(); + if (error) + return error; + error = incidence_init(); if (error) return error;