From f39d02eca4733115cf31bdd20f8b7e4d01b8dff7 Mon Sep 17 00:00:00 2001 From: pcarana Date: Wed, 15 Jan 2020 18:59:20 -0600 Subject: [PATCH] Load previous valid SLURM on any error, validate tal/slurm conf args. +Previous valid SLURM was applied only if a newer SLURM had syntax errors; this has changed, now it's applied on any error. +Log error when the version isn't set at SLURM file. +Validate configured location (can be a file or directory) of 'tal' and 'slurm' args when the application starts. --- src/common.c | 33 ++++++++++++++++++++++++++ src/common.h | 2 ++ src/config.c | 13 +++++++--- src/slurm/slurm_loader.c | 51 +++++++++++++++++----------------------- src/slurm/slurm_parser.c | 22 +++++++---------- 5 files changed, 74 insertions(+), 47 deletions(-) diff --git a/src/common.c b/src/common.c index a0bf2f0f..54ccebd7 100644 --- a/src/common.c +++ b/src/common.c @@ -185,6 +185,39 @@ process_file_or_dir(char const *location, char const *file_ext, return process_dir_files(location, file_ext, cb, arg); } + +bool +valid_file_or_dir(char const *location) +{ + FILE *file; + struct stat attr; + + file = fopen(location, "rb"); + if (file == NULL) { + pr_errno(errno, "Could not open location '%s'", + location); + return false; + } + + if (fstat(fileno(file), &attr) == -1) { + pr_errno(errno, "fstat(%s) failed", location); + goto fail; + } + + if (!S_ISREG(attr.st_mode) && !S_ISDIR(attr.st_mode)) { + pr_err("'%s' does not seem to be a file or directory", + location); + goto fail; + } + + return true; + +fail: + if (fclose(file) == -1) + pr_errno(errno, "fclose() failed"); + return false; +} + char const * addr2str4(struct in_addr const *addr, char *buffer) { diff --git a/src/common.h b/src/common.h index 95a9bb19..f925f2dd 100644 --- a/src/common.h +++ b/src/common.h @@ -3,6 +3,7 @@ #include #include +#include #include /* "I think that this is not supposed to be implemented." */ @@ -45,6 +46,7 @@ void close_thread(pthread_t thread, char const *); typedef int (*process_file_cb)(char const *, void *); int process_file_or_dir(char const *, char const *, process_file_cb, void *); +bool valid_file_or_dir(char const *); char const *addr2str4(struct in_addr const *, char *); char const *addr2str6(struct in6_addr const *, char *); diff --git a/src/config.c b/src/config.c index 787099ff..8edc7252 100644 --- a/src/config.c +++ b/src/config.c @@ -798,6 +798,12 @@ valid_output_file(char const *path) static int validate_config(void) { + if (rpki_config.tal == NULL) + return pr_err("The TAL file/directory (--tal) is mandatory."); + + if (!valid_file_or_dir(rpki_config.tal)) + return pr_err("Invalid TAL file/directory."); + if (rpki_config.server.interval.expire < rpki_config.server.interval.refresh || rpki_config.server.interval.expire < @@ -812,9 +818,10 @@ validate_config(void) !valid_output_file(rpki_config.output.bgpsec)) return pr_err("Invalid output.bgpsec file."); - return (rpki_config.tal != NULL) - ? 0 - : pr_err("The TAL file/directory (--tal) is mandatory."); + if (rpki_config.slurm != NULL && !valid_file_or_dir(rpki_config.slurm)) + return pr_err("Invalid slurm location."); + + return 0; } static void diff --git a/src/slurm/slurm_loader.c b/src/slurm/slurm_loader.c index e1420bd8..a9436f58 100644 --- a/src/slurm/slurm_loader.c +++ b/src/slurm/slurm_loader.c @@ -14,9 +14,8 @@ /* * Load the SLURM file(s) from the configured path, if the path is valid but no - * data is loaded (specific error for a SLURM folder) return -ENOENT error. - * - * Expect an EEXIST error from slurm_parse() if there's a syntax error. + * data is loaded (specific error for a SLURM folder) no error is returned and + * slurm db from @params is set as NULL. */ static int slurm_load(struct slurm_parser_params *params) @@ -32,20 +31,19 @@ slurm_load(struct slurm_parser_params *params) error = process_file_or_dir(config_get_slurm(), SLURM_FILE_EXTENSION, slurm_parse, params); - if (error) - goto err; + if (error) { + db_slurm_destroy(db); + params->db_slurm = NULL; + return error; + } - /* A unmodified context means that no SLURM was loaded */ + /* A unmodified context means that no SLURM was loaded (empty dir) */ if(params->cur_ctx == 0) { - error = -ENOENT; - goto err; + db_slurm_destroy(db); + params->db_slurm = NULL; } return 0; -err: - db_slurm_destroy(db); - params->db_slurm = NULL; - return error; } static int @@ -147,38 +145,31 @@ slurm_apply(struct db_table **base, struct db_slurm **last_slurm) if (config_get_slurm() == NULL) return 0; + pr_info("Applying configured SLURM"); error = slurm_create_parser_params(¶ms); if (error) return error; error = slurm_load(params); - switch (error) { - case 0: - /* Use as last valid slurm */ + if (!error) { + /* Use new SLURM as last valid slurm */ if (*last_slurm != NULL) db_slurm_destroy(*last_slurm); *last_slurm = params->db_slurm; - db_slurm_update_time(*last_slurm); - break; - case -EEXIST: - /* Syntax error, use last valid slurm, log as info */ + if (*last_slurm != NULL) + db_slurm_update_time(*last_slurm); + } else { + /* Any error: use last valid SLURM */ + pr_warn("Error loading SLURM, the validation will still continue."); if (*last_slurm != NULL) { - pr_warn("A previous valid version of the SLURM exists and will be applied"); + pr_warn("A previous valid version of the SLURM exists and will be applied."); params->db_slurm = *last_slurm; + /* Log applied SLURM as info */ db_slurm_log(params->db_slurm); } - break; - default: - /* Some other error, discard SLURM */ - if (*last_slurm != NULL) { - pr_info("Discarding previous valid SLURM"); - db_slurm_destroy(*last_slurm); - *last_slurm = NULL; - } - goto success; } - /* If there's no SLURM, stop */ + /* If there's no new SLURM loaded, stop */ if (params->db_slurm == NULL) goto success; diff --git a/src/slurm/slurm_parser.c b/src/slurm/slurm_parser.c index 2ef266c5..d9eba1fa 100644 --- a/src/slurm/slurm_parser.c +++ b/src/slurm/slurm_parser.c @@ -43,7 +43,7 @@ unsigned int cur_ctx; static int handle_json(json_t *, struct db_slurm *, unsigned int *); /* - * Try to parse the SLURM file(s), any syntax error will return an EEXIST error + * Try to parse the SLURM file(s) */ int slurm_parse(char const *location, void *arg) @@ -58,27 +58,19 @@ slurm_parse(char const *location, void *arg) json_root = json_load_file(location, JSON_REJECT_DUPLICATES, &json_error); - if (json_root == NULL) { - pr_err("SLURM JSON error on line %d, column %d: %s", + if (json_root == NULL) + /* File wasn't read or has a content error */ + return pr_err("SLURM JSON error on line %d, column %d: %s", json_error.line, json_error.column, json_error.text); - /* File was read, but has a content error */ - if (json_error.position > 0) - goto syntax_err; - return -ENOENT; - } ctx = params->cur_ctx; error = handle_json(json_root, params->db_slurm, &ctx); json_decref(json_root); if (error) - goto syntax_err; + return error; /* File exists, but has a syntax error */ params->cur_ctx = ctx; return 0; - -syntax_err: - /* File exists, but has a syntax/content error */ - return -EEXIST; } static int @@ -610,7 +602,9 @@ load_version(json_t *root) version = -1; error = json_get_int(root, SLURM_VERSION, &version); if (error) - return error; + return error == -ENOENT ? + pr_err("SLURM member '"SLURM_VERSION"' is required.") : + error; /* Validate data */ if (version != 1) -- 2.47.2