From: Alberto Leiva Popper Date: Sat, 2 Feb 2019 00:08:51 +0000 (-0600) Subject: Impose a certificate chain length limit by configuration X-Git-Tag: v0.0.2~103 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=28ba7c93bf7b19ce04754f36dc5cdd2398e9a0e3;p=thirdparty%2FFORT-validator.git Impose a certificate chain length limit by configuration Also add a configuration module, and patch a ROA address iteration bug. --- diff --git a/src/Makefile.am b/src/Makefile.am index 1967fb27..afaebd34 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -10,6 +10,7 @@ rpki_validator_SOURCES += address.h address.c rpki_validator_SOURCES += array_list.h rpki_validator_SOURCES += certificate_refs.h certificate_refs.c rpki_validator_SOURCES += common.h common.c +rpki_validator_SOURCES += config.h config.c rpki_validator_SOURCES += debug.h debug.c rpki_validator_SOURCES += file.h file.c rpki_validator_SOURCES += line_file.h line_file.c diff --git a/src/asn1/signed_data.c b/src/asn1/signed_data.c index 5004181a..67af0e42 100644 --- a/src/asn1/signed_data.c +++ b/src/asn1/signed_data.c @@ -4,8 +4,10 @@ #include #include +#include "config.h" #include "log.h" #include "oid.h" +#include "thread_var.h" #include "asn1/decode.h" #include "crypto/hash.h" #include "object/certificate.h" @@ -53,10 +55,17 @@ static int handle_sdata_certificate(ANY_t *any, struct signed_object_args *args, OCTET_STRING_t *sid) { + struct validation *state; const unsigned char *tmp; X509 *cert; int error; + state = state_retrieve(); + if (state == NULL) + return -EINVAL; + if (sk_X509_num(validation_certs(state)) >= config_get_max_cert_depth()) + return pr_err("Certificate chain maximum depth exceeded."); + pr_debug_add("EE Certificate (embedded) {"); /* diff --git a/src/common.c b/src/common.c index ff9f51e5..c9138aa4 100644 --- a/src/common.c +++ b/src/common.c @@ -4,8 +4,6 @@ #include #include "log.h" -char const *repository; -size_t repository_len; int NID_rpkiManifest; int NID_signedObject; int NID_rpkiNotify; diff --git a/src/common.h b/src/common.h index e0680722..e50f7d36 100644 --- a/src/common.h +++ b/src/common.h @@ -16,8 +16,6 @@ */ #define ENOTRSYNC 3174 -extern char const *repository; -extern size_t repository_len; extern int NID_rpkiManifest; extern int NID_signedObject; extern int NID_rpkiNotify; diff --git a/src/config.c b/src/config.c new file mode 100644 index 00000000..f60731ac --- /dev/null +++ b/src/config.c @@ -0,0 +1,39 @@ +#include "config.h" + +static struct rpki_config config; + +void +config_set(struct rpki_config *new) +{ + config = *new; +} + +char const * +config_get_tal(void) +{ + return config.tal; +} + +char const * +config_get_local_repository(void) +{ + return config.local_repository; +} + +bool +config_get_disable_rsync(void) +{ + return config.disable_rsync; +} + +bool +config_get_shuffle_uris(void) +{ + return config.shuffle_uris; +} + +unsigned int +config_get_max_cert_depth(void) +{ + return config.maximum_certificate_depth; +} diff --git a/src/config.h b/src/config.h new file mode 100644 index 00000000..b49348d0 --- /dev/null +++ b/src/config.h @@ -0,0 +1,30 @@ +#ifndef SRC_CONFIG_H_ +#define SRC_CONFIG_H_ + +#include + +struct rpki_config { + /* tal file path*/ + char *tal; + /* Local repository path */ + char *local_repository; + /* Disable rsync downloads */ + bool disable_rsync; + /* Shuffle uris in tal */ + bool shuffle_uris; + /* + * rfc6487#section-7.2, last paragraph. + * Prevents arbitrarily long paths and loops. + */ + unsigned int maximum_certificate_depth; +}; + +void config_set(struct rpki_config *); + +char const *config_get_tal(void); +char const *config_get_local_repository(void); +bool config_get_disable_rsync(void); +bool config_get_shuffle_uris(void); +unsigned int config_get_max_cert_depth(void); + +#endif /* SRC_CONFIG_H_ */ diff --git a/src/main.c b/src/main.c index 5fddc47e..a1594254 100644 --- a/src/main.c +++ b/src/main.c @@ -4,6 +4,7 @@ #include #include "common.h" +#include "config.h" #include "debug.h" #include "log.h" #include "rpp.h" @@ -13,17 +14,6 @@ #include "object/tal.h" #include "rsync/rsync.h" -struct rpki_config { - /* tal file path*/ - char *tal; - /* Local repository path */ - char *local_repository; - /* Disable rsync downloads */ - bool disable_rsync; - /* Shuffle uris in tal */ - bool shuffle_uris; -}; - /** * Registers the RPKI-specific OIDs in the SSL library. * LibreSSL needs it; not sure about OpenSSL. @@ -110,8 +100,34 @@ end: } static int -handle_args(int argc, char **argv, struct rpki_config *config) +parse_max_depth(struct rpki_config *config, char *str) +{ + /* + * It cannot be UINT_MAX, because then the actual number will overflow + * and will never be bigger than this. + */ + const unsigned int MAX = UINT_MAX - 1; + unsigned long max_depth; + + errno = 0; + max_depth = strtoul(str, NULL, 10); + if (errno) { + return pr_errno(errno, + "'%s' is not an unsigned integer, or is too big (max: %u)", + str, MAX); + } + + if (max_depth > MAX) + return pr_err("The number '%s' is too big (max: %u)", str, MAX); + + config->maximum_certificate_depth = max_depth; + return 0; +} + +static int +handle_args(int argc, char **argv) { + struct rpki_config config; int opt, error = 0; static struct option long_options[] = { @@ -122,77 +138,81 @@ handle_args(int argc, char **argv, struct rpki_config *config) {0,0,0,} }; - config->disable_rsync = false; - config->shuffle_uris = false; - config->local_repository = NULL; - config->tal = NULL; + config.disable_rsync = false; + config.shuffle_uris = false; + config.local_repository = NULL; + config.tal = NULL; + config.maximum_certificate_depth = 32; - while ((opt = getopt_long(argc, argv, "t:l:rs", long_options, NULL)) + while ((opt = getopt_long(argc, argv, "t:l:rsm:", long_options, NULL)) != -1) { switch (opt) { case 't' : - config->tal = optarg; + config.tal = optarg; break; case 'l' : - config->local_repository = optarg; + config.local_repository = optarg; break; case 'r': - config->disable_rsync = true; + config.disable_rsync = true; break; case 's': - config->shuffle_uris = true; + config.shuffle_uris = true; + break; + case 'm': + error = parse_max_depth(&config, optarg); break; default: return pr_err("some usage hints.");/* TODO */ } } - if (config->tal == NULL) { + if (config.tal == NULL) { fprintf(stderr, "Missing flag --tal \n"); error = -EINVAL; } - if(config->local_repository == NULL) { + if (config.local_repository == NULL) { fprintf(stderr, "Missing flag --local_repository \n"); error = -EINVAL; } - pr_debug("TAL file : %s", config->tal); - pr_debug("Local repository : %s", config->local_repository); - pr_debug("Disable rsync : %s", config->disable_rsync + pr_debug("TAL file : %s", config.tal); + pr_debug("Local repository : %s", config.local_repository); + pr_debug("Disable rsync : %s", config.disable_rsync ? "true" : "false"); - pr_debug("shuffle uris : %s", config->shuffle_uris + pr_debug("shuffle uris : %s", config.shuffle_uris ? "true" : "false"); + pr_debug("Maximum certificate depth : %u", + config.maximum_certificate_depth); + if (!error) + config_set(&config); return error; } int main(int argc, char **argv) { - struct rpki_config config; struct tal *tal; int error; - error = handle_args(argc, argv, &config); + error = handle_args(argc, argv); if (error) return error; print_stack_trace_on_segfault(); - error = rsync_init(!config.disable_rsync); + error = rsync_init(); if (error) return error; add_rpki_oids(); thvar_init(); fnstack_store(); - fnstack_push(config.tal); - - repository = config.local_repository; - repository_len = strlen(repository); + fnstack_push(config_get_tal()); - error = tal_load(config.tal, &tal); + error = tal_load(config_get_tal(), &tal); if (!error) { - if (config.shuffle_uris) + if (config_get_shuffle_uris()) tal_shuffle_uris(tal); error = foreach_uri(tal, handle_tal_uri); error = (error >= 0) ? 0 : error; diff --git a/src/object/certificate.c b/src/object/certificate.c index 1c4fbedc..db2cb9cb 100644 --- a/src/object/certificate.c +++ b/src/object/certificate.c @@ -7,6 +7,7 @@ #include #include "common.h" +#include "config.h" #include "log.h" #include "manifest.h" #include "thread_var.h" @@ -1306,6 +1307,12 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri const *cert_uri, struct rpp *pp; int error; + state = state_retrieve(); + if (state == NULL) + return -EINVAL; + if (sk_X509_num(validation_certs(state)) >= config_get_max_cert_depth()) + return pr_err("Certificate chain maximum depth exceeded."); + pr_debug_add("%s Certificate %s {", is_ta ? "TA" : "CA", cert_uri->global); fnstack_push(cert_uri->global); @@ -1334,11 +1341,6 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri const *cert_uri, goto end3; /* -- Validate the manifest (@mft) pointed by the certificate -- */ - state = state_retrieve(); - if (state == NULL) { - error = -EINVAL; - goto end3; - } error = validation_push_cert(state, cert_uri, cert, is_ta); if (error) goto end3; diff --git a/src/object/roa.c b/src/object/roa.c index 5e6c3762..634bf235 100644 --- a/src/object/roa.c +++ b/src/object/roa.c @@ -139,7 +139,7 @@ __handle_roa(struct RouteOriginAttestation *roa, struct resources *parent) return pr_crit("ipAddrBlocks array is NULL."); for (b = 0; b < roa->ipAddrBlocks.list.count; b++) { - block = roa->ipAddrBlocks.list.array[0]; + block = roa->ipAddrBlocks.list.array[b]; if (block == NULL) return pr_err("Address block array element is NULL."); diff --git a/src/rsync/rsync.c b/src/rsync/rsync.c index 61361a61..a0770ba1 100644 --- a/src/rsync/rsync.c +++ b/src/rsync/rsync.c @@ -10,6 +10,7 @@ #include #include "common.h" +#include "config.h" #include "log.h" struct uri { @@ -21,19 +22,16 @@ struct uri { SLIST_HEAD(uri_list, uri); static struct uri_list *rsync_uris; -static bool execute_rsync = true; static char const *const RSYNC_PREFIX = "rsync://"; //static const char *rsync_command[] = {"rsync", "--recursive", "--delete", "--times", NULL}; int -rsync_init(bool is_rsync_active) +rsync_init(void) { /* Disabling rsync will forever be a useful debugging feature. */ - if (!is_rsync_active) { - execute_rsync = is_rsync_active; + if (config_get_disable_rsync()) return 0; - } rsync_uris = malloc(sizeof(struct uri_list)); if (rsync_uris == NULL) @@ -48,7 +46,7 @@ rsync_destroy(void) { struct uri *uri; - if (!execute_rsync) + if (config_get_disable_rsync()) return; while (!SLIST_EMPTY(rsync_uris)) { @@ -344,6 +342,7 @@ create_dir(char *path) static int create_dir_recursive(char *localuri) { + size_t repository_len; int i, error; bool exist = false; @@ -354,6 +353,7 @@ create_dir_recursive(char *localuri) if (exist) return 0; + repository_len = strlen(config_get_local_repository()); for (i = 1 + repository_len; localuri[i] != '\0'; i++) { if (localuri[i] == '/') { localuri[i] = '\0'; @@ -378,7 +378,7 @@ download_files(struct rpki_uri const *uri) prefix_len = strlen(RSYNC_PREFIX); - if (!execute_rsync) + if (config_get_disable_rsync()) return 0; if (uri->global_len < prefix_len || diff --git a/src/rsync/rsync.h b/src/rsync/rsync.h index 688c1df6..f4114df6 100644 --- a/src/rsync/rsync.h +++ b/src/rsync/rsync.h @@ -5,7 +5,7 @@ #include "uri.h" int download_files(struct rpki_uri const *); -int rsync_init(bool); +int rsync_init(void); void rsync_destroy(void); diff --git a/src/uri.c b/src/uri.c index ce593e78..6d942a60 100644 --- a/src/uri.c +++ b/src/uri.c @@ -1,6 +1,7 @@ #include "uri.h" #include "common.h" +#include "config.h" #include "log.h" /** @@ -74,11 +75,15 @@ static int g2l(char const *global, size_t global_len, char **result) { static char const *const PREFIX = "rsync://"; + char const *repository; char *local; size_t prefix_len; + size_t repository_len; size_t extra_slash; size_t offset; + repository = config_get_local_repository(); + repository_len = strlen(repository); prefix_len = strlen(PREFIX); if (global_len < prefix_len