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 0817d45980aa9970dc1a722c877a4dd3ee4e3221..e2ca0a2f020be26e69bcca63cb3ab266f385e271 100644 (file)
--- 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 cb3f0c5fb486263ba918d4949ff8ee42e14b4ae3..f839bc78075b63a7a598ee00e766d0ee67ba8ac8 100644 (file)
--- 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 99cce080f190f7c3b5e2fa13b82d988f6f662ee3..e3ca9aacf815d39a717cab7e505afcfeb580a0d5 100644 (file)
--- a/src/http/http.c
+++ b/src/http/http.c
@@ -1,5 +1,7 @@
 #include "http.h"
 
+#include <errno.h>
+#include <stdio.h>
 #include <unistd.h>
 #include <curl/curl.h>
 #include <sys/stat.h>
@@ -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 fdd7b784bf0c2d541ad57964467a09c79156695a..b31034fc59544699845cb34aca130732db8dde06 100644 (file)
--- 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 878be5176062c66c02194d4b94cff16c6faedebb..5fee91f06a1985c9e17fd4228ba5a58aa561c2ef 100644 (file)
--- 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 cdf331b083349b39c8b66f20ff9e018e5308aab1..f50bfe891ed10a8c419539fd40d023b3614dd7c2 100644 (file)
--- 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 25063ebc2c72b0ae9d6527a124454052f22ee37a..112ac6cd9293b00d367ba206abf81cfa4dc4134e 100644 (file)
--- 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 1f2f69ae1060eea35dd947f9cf9f258b2ea28da7..f595552b3313dae8b6aa500f740b306662819a6e 100644 (file)
--- 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 08b33b6138c36de1ebf63676710d20d9bcc52035..2abbcd75dc4bc0a4ec13a838aa8f67ecffefe38d 100644 (file)
--- 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 d31bedfb2536df744e6d5bc76288bb3e0e5dc494..4d51e0236f9cb2056745c992a7987e7215ef1820 100644 (file)
--- 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 b3bd6df33ca2cbacd516af88b45c851f83366db4..4fac55d8861e57abc9d31e9613374f73e3ba4a20 100644 (file)
--- 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);