From: Alberto Leiva Popper Date: Tue, 14 May 2019 23:02:15 +0000 (-0500) Subject: Add incidence framework X-Git-Tag: v0.0.2~34 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=310853a925fdefad866a799df988c8683f081f1b;p=thirdparty%2FFORT-validator.git Add incidence framework It's a configurable means to define the outcome of a validation error. At present, it has only been coded to handle the signature algorithm having parameters error, since it's technically a profile violation, and yet there's an insurmountable amount of certificates breaking it at present. Unrelatedly, the commit also prevents some RTR errors from being responded to RTR errors. --- diff --git a/docs/doc/installation.md b/docs/doc/installation.md index 75092cc6..5e12ecb5 100644 --- a/docs/doc/installation.md +++ b/docs/doc/installation.md @@ -44,8 +44,6 @@ sudo make install There are no packages just yet. -### Validator - {% highlight bash %} git clone https://github.com/ydahhrk/rpki-validator.git cd rpki-validator @@ -54,14 +52,3 @@ cd rpki-validator make sudo make install {% endhighlight %} - -### RTR-Server - -{% highlight bash %} -git clone https://github.com/ydahhrk/rtr-server.git -cd rtr-server -./autogen.sh -./configure -make -sudo make install -{% endhighlight %} diff --git a/src/Makefile.am b/src/Makefile.am index 61962fe6..a41a1bfa 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 cec068b7..e2439cb3 100644 --- a/src/algorithm.c +++ b/src/algorithm.c @@ -101,6 +101,7 @@ validate_cms_signature_algorithm(AlgorithmIdentifier_t *id) 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, }; uint8_t last; + int error; if (id == NULL) return pr_err("The signature algorithm is absent."); @@ -132,8 +133,12 @@ validate_cms_signature_algorithm(AlgorithmIdentifier_t *id) * AlgorithmIdentifier, the parameters MUST be NULL. Implementations * MUST accept the parameters being absent as well as present. */ - if (id->parameters != NULL) - pr_warn("The signature algorithm has parameters."); + if (id->parameters != NULL) { + error = incidence(INID_SIGNATURE_ALGORITHM_HAS_PARAMS, + "The signature algorithm has parameters."); + if (error) + return error; + } return 0; diff --git a/src/config.c b/src/config.c index a05239d6..8faf9e7d 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" @@ -96,7 +97,7 @@ static char const *program_name; static struct rpki_config rpki_config; /** - * An option that takes no arguments, is not correlated to any rpki_config + * An ARGP option that takes no arguments, is not correlated to any rpki_config * fields, and is entirely managed by its handler function. */ static const struct global_type gt_callback = { @@ -279,7 +280,7 @@ static const struct option_field options[] = { .name = "log.color-output", .type = >_bool, .offset = offsetof(struct rpki_config, log.color), - .doc = "Print ANSI color codes.", + .doc = "Print ANSI color codes", }, { .id = 4000, .name = "log.file-name-format", @@ -288,6 +289,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..f6e36a7d --- /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_SIGNATURE_ALGORITHM_HAS_PARAMS, + "signature algorithm has parameters", + INAC_WARN, + }, +}; + +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..dccd12aa --- /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_SIGNATURE_ALGORITHM_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 8ae24c91..a26fdec5 100644 --- a/src/main.c +++ b/src/main.c @@ -38,6 +38,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; diff --git a/src/rtr/err_pdu.c b/src/rtr/err_pdu.c index f6172b6d..3e521c65 100644 --- a/src/rtr/err_pdu.c +++ b/src/rtr/err_pdu.c @@ -31,11 +31,10 @@ err_pdu_send(int fd, rtr_error_code_t code, struct rtr_request const *request, /* * This function must always return error so callers can interrupt * themselves easily. - * But note that not all callers should use this. - * TODO (now) Prevent errors to errors - * (It's harder than it seems, because request->pdu is sometimes NULL.) + * But note that not all callers should use this feature. */ + /* Need a clone to remove the const. */ message = (message_const != NULL) ? strdup(message_const) : NULL; send_error_report_pdu(fd, code, request, message); free(message); diff --git a/src/rtr/pdu.c b/src/rtr/pdu.c index 1f52d8da..536dba7e 100644 --- a/src/rtr/pdu.c +++ b/src/rtr/pdu.c @@ -18,6 +18,9 @@ pdu_header_from_reader(struct pdu_reader *reader, struct pdu_header *header) || read_int32(reader, &header->length); } +#define ERROR(report_cb) \ + ((header.pdu_type != PDU_TYPE_ERROR_REPORT) ? (report_cb) : -EINVAL); + int pdu_load(int fd, struct rtr_request *request, struct pdu_metadata const **metadata) @@ -49,20 +52,21 @@ pdu_load(int fd, struct rtr_request *request, return err_pdu_send_unsupported_proto_version(fd); if (header.length < RTRPDU_HEADER_LEN) - return err_pdu_send_invalid_request_truncated(fd, hdr_bytes, - "PDU is too small. (< 8 bytes)"); + return ERROR(err_pdu_send_invalid_request_truncated(fd, + hdr_bytes, "PDU is too small. (< 8 bytes)")); /* * Error messages can be quite large. * But they're probably not legitimate, so drop 'em. * 512 is like a 5-paragraph error message, so it's probably enough. + * Most error messages are bound to be two phrases tops. * (Warning: I'm assuming english tho.) */ if (header.length > 512) { pr_warn("Got an extremely large PDU (%u bytes). WTF?", header.length); - return err_pdu_send_invalid_request_truncated(fd, hdr_bytes, - "PDU is too large. (> 512 bytes)"); + return ERROR(err_pdu_send_invalid_request_truncated(fd, + hdr_bytes, "PDU is too large. (> 512 bytes)")); } /* Read the rest of the PDU into its buffer. */ @@ -83,7 +87,7 @@ pdu_load(int fd, struct rtr_request *request, /* Deserialize the PDU. */ meta = pdu_get_metadata(header.pdu_type); if (!meta) { - error = err_pdu_send_unsupported_pdu_type(fd, request); + error = ERROR(err_pdu_send_unsupported_pdu_type(fd, request)); goto revert_bytes; } @@ -95,7 +99,7 @@ pdu_load(int fd, struct rtr_request *request, error = meta->from_stream(&header, &reader, request->pdu); if (error) { - err_pdu_send_internal_error(fd); + ERROR(err_pdu_send_internal_error(fd)); goto revert_pdu; } diff --git a/src/rtr/pdu.h b/src/rtr/pdu.h index 2f982183..283fb973 100644 --- a/src/rtr/pdu.h +++ b/src/rtr/pdu.h @@ -143,12 +143,17 @@ struct error_report_pdu { struct pdu_metadata { size_t length; /** + * Builds the PDU from @header, and the bytes remaining in the reader. + * * Caller assumes that from_stream functions are only allowed to fail - * on programming errors. (Because of the internal error response.) + * on programming errors. (Because failure results in an internal error + * response.) */ int (*from_stream)(struct pdu_header *, struct pdu_reader *, void *); /** - * Handlers are expected to send error PDUs on discretion. + * Handlers must return 0 to maintain the connection, nonzero to close + * the socket. + * Also, they are supposed to send error PDUs on discretion. */ int (*handle)(int, struct rtr_request const *); void (*destructor)(void *);