From: pcarana Date: Fri, 24 Jul 2020 17:08:50 +0000 (-0500) Subject: Fix AIA validation, log all 'enomem', use tmp files at http downloads X-Git-Tag: v1.4.0~14 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=11231cc2121719c16a06c5d84ac99f35a3a230ad;p=thirdparty%2FFORT-validator.git Fix AIA validation, log all 'enomem', use tmp files at http downloads +The AIA validation flow didn't considered entirely the scenario where a TA child AIA extension didn't matched the actual location from where the TA was fetched (common case: when its downloaded from an HTTPS URI), so despite the TA actually existed it wasn't considered when the validator was working with local files. +Replace all '-ENOMEM' return codes with the log function 'pr_enomem'. +Use temporal files whenever an HTTPS file is being downloaded. +Fix memory leak when working with local TA files. +The message '{..} discarding any other validation results' is now sent to the operation log. +Fix GCC 10 warning related to 'strncpy', use 'memcpy' instead. --- diff --git a/src/config/string_array.c b/src/config/string_array.c index 0817d459..e2ca0a2f 100644 --- a/src/config/string_array.c +++ b/src/config/string_array.c @@ -24,13 +24,13 @@ string_array_init(struct string_array *array, char const *const *values, array->array = calloc(len, sizeof(char *)); if (array->array == NULL) - return -ENOMEM; + return pr_enomem(); for (i = 0; i < len; i++) { array->array[i] = strdup(values[i]); if (array->array[i] == NULL) { string_array_cleanup(array); - return -ENOMEM; + return pr_enomem(); } } diff --git a/src/crypto/base64.c b/src/crypto/base64.c index cb3f0c5f..f839bc78 100644 --- a/src/crypto/base64.c +++ b/src/crypto/base64.c @@ -65,7 +65,7 @@ base64_decode(BIO *in, unsigned char *out, bool has_nl, size_t out_len, b64 = BIO_new(BIO_f_base64()); if (b64 == NULL) { error = ERR_peek_last_error(); - return error ? error_ul2i(error) : -ENOMEM; + return error ? error_ul2i(error) : pr_enomem(); } /* @@ -150,7 +150,7 @@ base64url_decode(char const *str_encoded, unsigned char **result, str_copy = malloc(encoded_len + pad + 1); if (str_copy == NULL) - return -ENOMEM; + return pr_enomem(); /* Set all with pad char, then replace with the original string */ memset(str_copy, '=', encoded_len + pad); memcpy(str_copy, str_encoded, encoded_len); @@ -173,7 +173,7 @@ base64url_decode(char const *str_encoded, unsigned char **result, alloc_size = EVP_DECODE_LENGTH(strlen(str_copy)); *result = malloc(alloc_size + 1); if (*result == NULL) { - error = -ENOMEM; + error = pr_enomem(); goto free_enc; } memset(*result, 0, alloc_size); @@ -251,7 +251,7 @@ base64url_encode(unsigned char const *in, int in_len, char **result) mem = BIO_new(BIO_s_mem()); if (mem == NULL) { error = ERR_peek_last_error(); - return error ? error_ul2i(error) : -ENOMEM; + return error ? error_ul2i(error) : pr_enomem(); } b64 = BIO_new(BIO_f_base64()); @@ -274,5 +274,5 @@ base64url_encode(unsigned char const *in, int in_len, char **result) return 0; free_mem: BIO_free_all(b64); - return error ? error_ul2i(error) : -ENOMEM; + return error ? error_ul2i(error) : pr_enomem(); } diff --git a/src/http/http.c b/src/http/http.c index 99cce080..e3ca9aac 100644 --- a/src/http/http.c +++ b/src/http/http.c @@ -1,5 +1,7 @@ #include "http.h" +#include +#include #include #include #include @@ -166,10 +168,13 @@ static int __http_download_file(struct rpki_uri *uri, http_write_cb cb, long *response_code, long ims_value, long *cond_met, bool log_operation) { + char const *tmp_suffix = "_tmp"; struct http_handler handler; struct stat stat; FILE *out; unsigned int retries; + char const *original_file; + char *tmp_file, *tmp; int error; retries = 0; @@ -179,11 +184,25 @@ __http_download_file(struct rpki_uri *uri, http_write_cb cb, return 0; } - error = create_dir_recursive(uri_get_local(uri)); + original_file = uri_get_local(uri); + tmp_file = strdup(original_file); + if (tmp_file == NULL) + return pr_enomem(); + + tmp = realloc(tmp_file, strlen(tmp_file) + strlen(tmp_suffix) + 1); + if (tmp == NULL) { + error = pr_enomem(); + goto release_tmp; + } + + tmp_file = tmp; + strcat(tmp_file, tmp_suffix); + + error = create_dir_recursive(tmp_file); if (error) - return ENSURE_NEGATIVE(error); + goto release_tmp; - error = file_write(uri_get_local(uri), &out, &stat); + error = file_write(tmp_file, &out, &stat); if (error) goto delete_dir; @@ -223,11 +242,23 @@ __http_download_file(struct rpki_uri *uri, http_write_cb cb, if (error) goto delete_dir; + /* Overwrite the original file */ + error = rename(tmp_file, original_file); + if (error) { + error = errno; + pr_val_errno(error, "Renaming temporal file from '%s' to '%s'", + tmp_file, original_file); + goto delete_dir; + } + + free(tmp_file); return 0; close_file: file_close(out); delete_dir: - delete_dir_recursive_bottom_up(uri_get_local(uri)); + delete_dir_recursive_bottom_up(tmp_file); +release_tmp: + free(tmp_file); return ENSURE_NEGATIVE(error); } diff --git a/src/line_file.c b/src/line_file.c index fdd7b784..b31034fc 100644 --- a/src/line_file.c +++ b/src/line_file.c @@ -24,7 +24,7 @@ lfile_open(const char *file_name, struct line_file **result) lfile = malloc(sizeof(struct line_file)); if (lfile == NULL) - return -ENOMEM; + return pr_enomem(); lfile->file = fopen(file_name, "r"); if (lfile->file == NULL) { diff --git a/src/object/certificate.c b/src/object/certificate.c index 878be517..5fee91f0 100644 --- a/src/object/certificate.c +++ b/src/object/certificate.c @@ -1903,24 +1903,40 @@ get_certificate_type(X509 *cert, bool is_ta) * verified to comply with rfc6487#section-4.8.7 */ static int -force_aia_validation(struct rpki_uri *caIssuers, void *arg) +force_aia_validation(struct rpki_uri *caIssuers, X509 *son, bool is_ta_child) { - X509 *son = arg; X509 *parent; struct rfc5280_name *son_name; struct rfc5280_name *parent_name; int error; - pr_val_debug("AIA's URI didn't matched parent URI, doing AIA RSYNC"); + pr_val_debug("AIA's URI didn't matched parent URI, trying to SYNC"); /* RSYNC is still the prefered access mechanism */ - error = download_files(caIssuers, false, false); - if (error) + do { + error = download_files(caIssuers, false, false); + if (!error) + break; + if (error == EREQFAILED) { + pr_val_info("AIA URI couldn't be downloaded, trying to search locally"); + break; + } return error; + } while (0); + /* + * Consider the scenario where the TA was fetched from a distinct URI + * (maybe an HTTPS URI), in that case keep doing the validation since + * there's no "right" way to validate the AIA, that's where the flag + * 'is_ta_child' comes into play. + * + * A possible way is to load the certificate from the cert_stack, but + * the location itself isn't strictly the location from the AIA (the + * solution works, but maybe it isn't right. + */ error = certificate_load(caIssuers, &parent); if (error) - return error; + return !is_ta_child ? error : 0; error = x509_name_decode(X509_get_subject_name(parent), "subject", &parent_name); @@ -1997,7 +2013,8 @@ certificate_validate_aia(struct rpki_uri *caIssuers, X509 *cert) * the current issuer. This will force an RSYNC of AIA's URI, load * the certificate and do the comparison. */ - return force_aia_validation(caIssuers, cert); + return force_aia_validation(caIssuers, cert, + certstack_get_x509_num(validation_certstack(state)) == 1); } /* diff --git a/src/object/tal.c b/src/object/tal.c index cdf331b0..f50bfe89 100644 --- a/src/object/tal.c +++ b/src/object/tal.c @@ -362,7 +362,7 @@ tal_load(char const *file_name, struct tal **result) tal = malloc(sizeof(struct tal)); if (tal == NULL) { - error = -ENOMEM; + error = pr_enomem(); goto fail3; } @@ -555,19 +555,25 @@ handle_tal_uri(struct tal *tal, struct rpki_uri *uri, void *arg) if (!thread_arg->sync_files) { /* Look for local files */ if (!valid_file_or_dir(uri_get_local(uri), true, false, - pr_val_errno)) + pr_val_errno)) { + validation_destroy(state); return 0; /* Error already logged */ + } break; } /* Trying to sync, considering that the sync can be disabled */ if (uri_is_rsync(uri)) { - if (!config_get_rsync_enabled()) + if (!config_get_rsync_enabled()) { + validation_destroy(state); return 0; /* Soft error */ + } error = download_files(uri, true, false); break; } - if (!config_get_http_enabled()) + if (!config_get_http_enabled()) { + validation_destroy(state); return 0; /* Soft error */ + } error = handle_https_uri(uri); } while (0); @@ -823,7 +829,7 @@ perform_standalone_validation(struct db_table *table) SLIST_REMOVE_HEAD(&threads, next); if (thread->exit_status) { t_error = thread->exit_status; - pr_val_warn("Validation from TAL '%s' yielded error, discarding any other validation results.", + pr_op_warn("Validation from TAL '%s' yielded error, discarding any other validation results.", thread->tal_file); } thread_destroy(thread); diff --git a/src/rtr/primitive_reader.c b/src/rtr/primitive_reader.c index 25063ebc..112ac6cd 100644 --- a/src/rtr/primitive_reader.c +++ b/src/rtr/primitive_reader.c @@ -227,7 +227,7 @@ read_string(struct pdu_reader *reader, uint32_t string_len, rtr_char **result) string = malloc(string_len + 1); /* Include NULL chara. */ if (!string) - return -ENOMEM; + return pr_enomem(); memcpy(string, reader->buffer, string_len); reader->buffer += string_len; diff --git a/src/rtr/rtr.c b/src/rtr/rtr.c index 1f2f69ae..f595552b 100644 --- a/src/rtr/rtr.c +++ b/src/rtr/rtr.c @@ -243,7 +243,7 @@ parse_address(char const *full_address, char const *default_service, if (tmp_addr == NULL) return pr_enomem(); - strncpy(tmp_addr, full_address, tmp_addr_len); + memcpy(tmp_addr, full_address, tmp_addr_len); tmp_addr[tmp_addr_len] = '\0'; tmp_serv = strdup(ptr + 1); diff --git a/src/sorted_array.c b/src/sorted_array.c index 08b33b61..2abbcd75 100644 --- a/src/sorted_array.c +++ b/src/sorted_array.c @@ -114,7 +114,7 @@ sarray_add(struct sorted_array *sarray, void *element) if (sarray->count >= sarray->len) { tmp = realloc(sarray->array, 2 * sarray->len * sarray->size); if (tmp == NULL) - return -ENOMEM; + return pr_enomem(); sarray->array = tmp; sarray->len *= 2; } diff --git a/src/state.c b/src/state.c index d31bedfb..4d51e023 100644 --- a/src/state.c +++ b/src/state.c @@ -94,7 +94,7 @@ validation_prepare(struct validation **out, struct tal *tal, result = malloc(sizeof(struct validation)); if (!result) - return -ENOMEM; + return pr_enomem(); error = state_store(result); if (error) diff --git a/src/str_token.c b/src/str_token.c index b3bd6df3..4fac55d8 100644 --- a/src/str_token.c +++ b/src/str_token.c @@ -43,7 +43,7 @@ BN2string(BIGNUM *bn, char **_result) bio = BIO_new(BIO_s_mem()); if (bio == NULL) - return -ENOMEM; + return pr_enomem(); if (BN_print(bio, bn) == 0) { BIO_free(bio);