From 8e9ba42ba9914b51b1e34bcb7d6907f279bc61de Mon Sep 17 00:00:00 2001 From: pcarana Date: Fri, 2 Aug 2019 11:51:11 -0500 Subject: [PATCH] Fix minor issues and add some enhancements. -Display a warning when a directory doesn't have files with the desired extension (used at TAL and SLURM configuration). -Set the minimum allowed value of 'maximum-certificate-depth' to 5 to allow a normal operation. -Validate ROA output file path at initialization instead of doing it when the whole validation process has terminated. -Add a note to indicate 64-bit OS support. -Print the real address where the RTR server will be bounded to. -If there's an error loading the SLURM data, show the element that has the error. -Fix X509_VERIFY_PARAM memory leak. -Update doc reference to UINT_MAX definition. -Fix bug: when a TAL couldn't be loaded its references where trying to be released, but such references didn't existed. --- README.md | 2 ++ docs/doc/installation.md | 2 ++ docs/doc/usage.md | 19 ++++++++++--------- man/fort.8 | 10 ++++++---- src/common.c | 14 ++++++++++---- src/config.c | 13 ++++++++++++- src/file.c | 22 ++++++++++++++++++++++ src/file.h | 3 +++ src/object/tal.c | 3 ++- src/rtr/rtr.c | 1 + src/slurm/slurm_parser.c | 16 ++++++++-------- src/state.c | 28 +++++++++++++++++----------- 12 files changed, 95 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index d11676e5..ef79257a 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,8 @@ Dependencies: 2. [jansson](https://github.com/akheron/jansson) 3. [rsync](http://rsync.samba.org/) +The validator is currently supported in *64-bit* OS. A 32-bit OS may face the [Year 2038 problem](https://en.wikipedia.org/wiki/Year_2038_problem) when handling dates at certificates. + After all the dependencies are installed, run: ``` diff --git a/docs/doc/installation.md b/docs/doc/installation.md index 6dd3ee74..29be8a44 100644 --- a/docs/doc/installation.md +++ b/docs/doc/installation.md @@ -25,6 +25,8 @@ The dependencies are 2. libcrypto (Either [LibreSSL](http://www.libressl.org/) or [OpenSSL](https://www.openssl.org/)) 3. [rsync](http://rsync.samba.org/) +Fort is currently supported in *64-bit* OS. A 32-bit OS may face the [Year 2038 problem](https://en.wikipedia.org/wiki/Year_2038_problem) when handling dates at certificates, and currently there's no work around for this. + ## Option 1: Installing the Debian package > TODO Upload to Debian, add more archs diff --git a/docs/doc/usage.md b/docs/doc/usage.md index 357f76db..03171a26 100644 --- a/docs/doc/usage.md +++ b/docs/doc/usage.md @@ -237,6 +237,7 @@ Of course, this is only relevant if the TAL lists more than one URL. - **Type:** Integer - **Availability:** `argv` and JSON - **Default:** 32 +- **Range:** 5--([`UINT_MAX`](http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html)--1) Maximum allowable RPKI tree height. Meant to protect Fort from iterating infinitely due to certificate chain loops. @@ -292,7 +293,7 @@ See the corresponding manual page from your operating system (likely `man 2 list - **Type:** Integer - **Availability:** `argv` and JSON - **Default:** 3600 -- **Range:** 60--[`UINT_MAX`](http://pubs.opengroup.org/onlinepubs/9699919799/) +- **Range:** 60--[`UINT_MAX`](http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html) Number of seconds the server will sleep between validation cycles. @@ -373,11 +374,11 @@ Path to a JSON file from which additional configuration will be read. The configuration options are mostly the same as the ones from the `argv` interface. (See the "Availability" metadata of each field.) Here's a full configuration file example: 
{
-	"tal": "/tmp/tal/test.tal",
-	"local-repository": "/tmp/repository",
+	"tal": "/tmp/fort/tal/test.tal",
+	"local-repository": "/tmp/fort/repository",
 	"sync-strategy": "root",
 	"shuffle-uris": true,
-	"slurm": "/tmp/test.slurm",
+	"slurm": "/tmp/fort/test.slurm",
 	"mode": "server",
 
 	"server": {
@@ -416,7 +417,7 @@ The configuration options are mostly the same as the ones from the `argv` interf
 	],
 
 	"output": {
-		"roa": "/tmp/fort_roas.csv"
+		"roa": "/tmp/fort/roas.csv"
 	}
 }
@@ -437,25 +438,25 @@ $ cat a.json { "local-repository": "a", "sync-strategy": "root", - "maximum-certificate-depth": 1 + "maximum-certificate-depth": 5 } $ cat b.json { "sync-strategy": "strict" - "maximum-certificate-depth": 2 + "maximum-certificate-depth": 6 } $ cat c.json { - "maximum-certificate-depth": 4 + "maximum-certificate-depth": 8 } $ {{ page.command }} \ --configuration-file="a.json" \ --configuration-file="b.json" \ --configuration-file="c.json" -$ # local-repository is "a", sync-strategy is "strict" and maximum-certificate-depth is 4 +$ # local-repository is "a", sync-strategy is "strict" and maximum-certificate-depth is 8 {% endhighlight %} ### rsync.program diff --git a/man/fort.8 b/man/fort.8 index 1eca2865..e363d30f 100644 --- a/man/fort.8 +++ b/man/fort.8 @@ -179,6 +179,8 @@ copy. .P Because rsync uses delta encoding, you’re advised to keep this cache around. It significantly speeds up subsequent validation cycles. +.P +By default, the path is \fI/tmp/fort/repository\fR. .RE .P @@ -279,7 +281,7 @@ that yields a successful traversal. Maximum allowable certificate chain length. Meant to protect FORT from iterating infinitely due to certificate chain loops. .P -By default, it has a value of \fI32\fR. +By default, it has a value of \fI32\fR. The minimum allowed value is 5. .P (Required to prevent loops and "other degenerate forms of the logical RPKI hierarchy." (RFC 6481)) @@ -462,8 +464,8 @@ to a specific value: .nf { - "tal": "/tmp/tal/", - "local-repository": "/tmp/repository", + "tal": "/tmp/fort/tal/", + "local-repository": "/tmp/fort/repository", "sync-strategy": "root", "shuffle-uris": true, "maximum-certificate-depth": 32, @@ -504,7 +506,7 @@ to a specific value: } ], "output": { - "roa": "/tmp/roas.csv" + "roa": "/tmp/fort/roas.csv" } } .fi diff --git a/src/common.c b/src/common.c index 19f50226..8ef84aca 100644 --- a/src/common.c +++ b/src/common.c @@ -86,7 +86,7 @@ close_thread(pthread_t thread, char const *what) static int process_file(char const *dir_name, char const *file_name, char const *file_ext, - process_file_cb cb, void *arg) + int *fcount, process_file_cb cb, void *arg) { char *ext, *fullpath, *tmp; int error; @@ -98,6 +98,8 @@ process_file(char const *dir_name, char const *file_name, char const *file_ext, return 0; } + (*fcount)++; /* Increment the found count */ + /* Get the full file path */ tmp = strdup(dir_name); if (tmp == NULL) @@ -129,7 +131,7 @@ process_dir_files(char const *location, char const *file_ext, { DIR *dir_loc; struct dirent *dir_ent; - int error; + int found, error; dir_loc = opendir(location); if (dir_loc == NULL) { @@ -138,9 +140,10 @@ process_dir_files(char const *location, char const *file_ext, } errno = 0; + found = 0; while ((dir_ent = readdir(dir_loc)) != NULL) { - error = process_file(location, dir_ent->d_name, file_ext, cb, - arg); + error = process_file(location, dir_ent->d_name, file_ext, + &found, cb, arg); if (error) { pr_err("The error was at file %s", dir_ent->d_name); goto close_dir; @@ -151,6 +154,9 @@ process_dir_files(char const *location, char const *file_ext, pr_err("Error reading dir %s", location); error = -errno; } + if (!error && found == 0) + pr_warn("Location '%s' doesn't have files with extension '%s'", + location, file_ext); close_dir: closedir(dir_loc); end: diff --git a/src/config.c b/src/config.c index d1440dfd..6e363735 100644 --- a/src/config.c +++ b/src/config.c @@ -9,6 +9,7 @@ #include "common.h" #include "configure_ac.h" +#include "file.h" #include "json_handler.h" #include "log.h" #include "config/boolean.h" @@ -182,7 +183,7 @@ static const struct option_field options[] = { .offset = offsetof(struct rpki_config, maximum_certificate_depth), .doc = "Maximum allowable certificate chain length", - .min = 1, + .min = 5, /** * It cannot be UINT_MAX, because then the actual number will * overflow and will never be bigger than this. @@ -519,9 +520,19 @@ revert_port: return error; } +static bool +valid_output_file(char const *path) +{ + return strcmp(path, "-") == 0 || file_valid(path); +} + static int validate_config(void) { + if (rpki_config.output.roa != NULL && + !valid_output_file(rpki_config.output.roa)) + return pr_err("Invalid output.roa file."); + return (rpki_config.tal != NULL) ? 0 : pr_err("The TAL file/directory (--tal) is mandatory."); diff --git a/src/file.c b/src/file.c index d1e26b6f..8df0f966 100644 --- a/src/file.c +++ b/src/file.c @@ -110,3 +110,25 @@ file_free(struct file_contents *fc) { free(fc->buffer); } + +/* + * Validate @file_name, if it doesn't exist, this function will create it and + * close it. + */ +bool +file_valid(char const *file_name) +{ + FILE *tmp; + struct stat stat; + int error; + + if (file_name == NULL) + return false; + + error = file_write(file_name, &tmp, &stat); + if (error) + return false; + + file_close(tmp); + return true; +} diff --git a/src/file.h b/src/file.h index 8440dfe4..1699587c 100644 --- a/src/file.h +++ b/src/file.h @@ -1,6 +1,7 @@ #ifndef SRC_FILE_H_ #define SRC_FILE_H_ +#include #include #include #include @@ -22,4 +23,6 @@ void file_close(FILE *); int file_load(char const *, struct file_contents *); void file_free(struct file_contents *); +bool file_valid(char const *); + #endif /* SRC_FILE_H_ */ diff --git a/src/object/tal.c b/src/object/tal.c index 7f8e4789..1bff8efd 100644 --- a/src/object/tal.c +++ b/src/object/tal.c @@ -389,7 +389,8 @@ do_file_validation(char const *tal_file, void *arg) error = pr_err("None of the URIs of the TAL '%s' yielded a successful traversal.", tal_file); -end: tal_destroy(tal); + tal_destroy(tal); +end: fnstack_pop(); return error; } diff --git a/src/rtr/rtr.c b/src/rtr/rtr.c index cd0e1d33..f72fdfaa 100644 --- a/src/rtr/rtr.c +++ b/src/rtr/rtr.c @@ -38,6 +38,7 @@ init_addrinfo(struct addrinfo **result) memset(&hints, 0 , sizeof(hints)); hints.ai_family = AF_UNSPEC; /* hints.ai_socktype = SOCK_DGRAM; */ + hints.ai_flags |= AI_CANONNAME; hints.ai_flags |= AI_PASSIVE; hostname = config_get_server_address(); diff --git a/src/slurm/slurm_parser.c b/src/slurm/slurm_parser.c index 74285ca3..2cc52e90 100644 --- a/src/slurm/slurm_parser.c +++ b/src/slurm/slurm_parser.c @@ -431,15 +431,15 @@ load_prefix_array(json_t *array, bool is_assertion) if (error) { if (error == -EEXIST) pr_err( - "The prefix %s element #%d, is duplicated or covered by another %s; SLURM loading will be stopped", + "The prefix %s element \"%s\", is duplicated or covered by another %s; SLURM loading will be stopped", (is_assertion ? "assertion" : "filter"), - index + 1, + json_dumps(element, 0), (is_assertion ? "assertion" : "filter")); else pr_err( - "Error at prefix %s, element #%d, SLURM loading will be stopped", + "Error at prefix %s, element \"%s\", SLURM loading will be stopped", (is_assertion ? "assertions" : "filters"), - index + 1); + json_dumps(element, 0)); return error; } @@ -555,15 +555,15 @@ load_bgpsec_array(json_t *array, bool is_assertion) if (error) { if (error == -EEXIST) pr_err( - "The bgpsec %s element #%d, is duplicated or covered by another %s; SLURM loading will be stopped", + "The bgpsec %s element \"%s\", is duplicated or covered by another %s; SLURM loading will be stopped", (is_assertion ? "assertion" : "filter"), - index + 1, + json_dumps(element, 0), (is_assertion ? "assertion" : "filter")); else pr_err( - "Error at bgpsec %s, element #%d, SLURM loading will be stopped", + "Error at bgpsec %s, element \"%s\", SLURM loading will be stopped", (is_assertion ? "assertions" : "filters"), - index + 1); + json_dumps(element, 0)); return error; } diff --git a/src/state.c b/src/state.c index 4fb3e77d..f3551392 100644 --- a/src/state.c +++ b/src/state.c @@ -14,8 +14,11 @@ struct validation { struct tal *tal; - /** https://www.openssl.org/docs/man1.1.1/man3/X509_STORE_load_locations.html */ - X509_STORE *store; + struct x509_data { + /** https://www.openssl.org/docs/man1.1.1/man3/X509_STORE_load_locations.html */ + X509_STORE *store; + X509_VERIFY_PARAM *params; + } x509_data; struct cert_stack *certstack; @@ -92,8 +95,8 @@ validation_prepare(struct validation **out, struct tal *tal, result->tal = tal; - result->store = X509_STORE_new(); - if (!result->store) { + result->x509_data.store = X509_STORE_new(); + if (!result->x509_data.store) { error = crypto_err("X509_STORE_new() returned NULL"); goto abort1; } @@ -105,21 +108,23 @@ validation_prepare(struct validation **out, struct tal *tal, } X509_VERIFY_PARAM_set_flags(params, X509_V_FLAG_CRL_CHECK); - X509_STORE_set1_param(result->store, params); - X509_STORE_set_verify_cb(result->store, cb); + X509_STORE_set1_param(result->x509_data.store, params); + X509_STORE_set_verify_cb(result->x509_data.store, cb); error = certstack_create(&result->certstack); if (error) - goto abort2; + goto abort3; result->pubkey_state = PKS_UNTESTED; result->validation_handler = *validation_handler; + result->x509_data.params = params; /* Ownership transfered */ *out = result; return 0; - +abort3: + X509_VERIFY_PARAM_free(params); abort2: - X509_STORE_free(result->store); + X509_STORE_free(result->x509_data.store); abort1: free(result); return error; @@ -128,7 +133,8 @@ abort1: void validation_destroy(struct validation *state) { - X509_STORE_free(state->store); + X509_VERIFY_PARAM_free(state->x509_data.params); + X509_STORE_free(state->x509_data.store); certstack_destroy(state->certstack); free(state); } @@ -142,7 +148,7 @@ validation_tal(struct validation *state) X509_STORE * validation_store(struct validation *state) { - return state->store; + return state->x509_data.store; } struct cert_stack * -- 2.47.3