From: Alberto Leiva Popper Date: Mon, 26 Jun 2023 22:57:54 +0000 (-0600) Subject: Remove null handling from state_retrieve() X-Git-Tag: 1.6.0~80^2~7 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=62ff492f8ed995ee747983bc9fec9a8349fb84d4;p=thirdparty%2FFORT-validator.git Remove null handling from state_retrieve() If the function fails, it panics. Therefore, there is no reason to catch NULL and the subsequent error codes. Removes clutter. --- diff --git a/src/extension.c b/src/extension.c index d8bd58ed..7b4c59fe 100644 --- a/src/extension.c +++ b/src/extension.c @@ -330,11 +330,6 @@ handle_aki(X509_EXTENSION *ext, void *arg) } state = state_retrieve(); - if (state == NULL) { - error = -EINVAL; - goto end; - } - parent = x509stack_peek(validation_certstack(state)); if (parent == NULL) { error = pr_val_err("Certificate has no parent."); diff --git a/src/object/certificate.c b/src/object/certificate.c index a75ac7fb..01c8c4dc 100644 --- a/src/object/certificate.c +++ b/src/object/certificate.c @@ -108,10 +108,6 @@ validate_serial_number(X509 *cert) struct validation *state; BIGNUM *number; - state = state_retrieve(); - if (state == NULL) - return -EINVAL; - number = ASN1_INTEGER_to_BN(X509_get0_serialNumber(cert), NULL); if (number == NULL) return val_crypto_err("Could not parse certificate serial number"); @@ -119,6 +115,7 @@ validate_serial_number(X509 *cert) if (log_val_enabled(LOG_DEBUG)) debug_serial_number(number); + state = state_retrieve(); x509stack_store_serial(validation_certstack(state), number); return 0; } @@ -196,14 +193,9 @@ spki_cmp(X509_PUBKEY *tal_spki, X509_PUBKEY *cert_spki, static int validate_subject(X509 *cert) { - struct validation *state; struct rfc5280_name *name; int error; - state = state_retrieve(); - if (state == NULL) - return -EINVAL; - error = x509_name_decode(X509_get_subject_name(cert), "subject", &name); if (error) return error; @@ -236,8 +228,6 @@ validate_spki(X509_PUBKEY *cert_spki) size_t _tal_spki_len; state = state_retrieve(); - if (state == NULL) - return -EINVAL; tal = validation_tal(state); if (tal == NULL) @@ -922,8 +912,6 @@ certificate_validate_chain(X509 *cert, STACK_OF(X509_CRL) *crls) return 0; /* Certificate is TA; no chain validation needed. */ state = state_retrieve(); - if (state == NULL) - return -EINVAL; ctx = X509_STORE_CTX_new(); if (ctx == NULL) { @@ -1897,8 +1885,7 @@ certificate_validate_aia(struct rpki_uri *caIssuers, X509 *cert) pr_crit("Certificate's AIA was not recorded."); state = state_retrieve(); - if (state == NULL) - return -EINVAL; + parent = x509stack_peek_uri(validation_certstack(state)); if (parent == NULL) pr_crit("Certificate has no parent."); @@ -2011,9 +1998,7 @@ exec_rrdp_method(struct sia_ca_uris *sia_uris) int error; /* Start working on the RRDP local workspace */ - error = db_rrdp_uris_workspace_enable(); - if (error) - return error; + db_rrdp_uris_workspace_enable(); data_updated = false; error = rrdp_load(sia_uris->rpkiNotify.uri, &data_updated); @@ -2145,11 +2130,7 @@ use_access_method(struct sia_ca_uris *sia_uris, error = db_rrdp_uris_get_request_status( uri_get_global(sia_uris->rpkiNotify.uri), &rrdp_req_status); if (error == 0 && rrdp_req_status == RRDP_URI_REQ_VISITED) { - error = db_rrdp_uris_workspace_enable(); - if (error) { - db_rrdp_uris_workspace_disable(); - return error; - } + db_rrdp_uris_workspace_enable(); (*retry_repo_sync) = false; return replace_rrdp_mft_uri(&sia_uris->mft); } @@ -2334,8 +2315,7 @@ certificate_traverse(struct rpp *rpp_parent, struct rpki_uri *cert_uri) int error; state = state_retrieve(); - if (state == NULL) - return -EINVAL; + total_parents = certstack_get_x509_num(validation_certstack(state)); if (total_parents >= config_get_max_cert_depth()) return pr_val_err("Certificate chain maximum depth exceeded."); diff --git a/src/object/name.c b/src/object/name.c index 90609a36..5c679062 100644 --- a/src/object/name.c +++ b/src/object/name.c @@ -156,8 +156,6 @@ validate_issuer_name(char const *container, X509_NAME *issuer) */ state = state_retrieve(); - if (state == NULL) - return -EINVAL; parent = x509stack_peek(validation_certstack(state)); if (parent == NULL) { return pr_val_err("%s appears to have no parent certificate.", diff --git a/src/object/tal.c b/src/object/tal.c index d7aa7f06..7e0001f3 100644 --- a/src/object/tal.c +++ b/src/object/tal.c @@ -542,9 +542,7 @@ handle_tal_uri(struct tal *tal, struct rpki_uri *uri, * Set all RRDPs URIs to non-requested, this way we will force the * request on every cycle (to check if there are updates). */ - error = db_rrdp_uris_set_all_unvisited(); - if (error) - goto end; + db_rrdp_uris_set_all_unvisited(); /* Handle root certificate. */ error = certificate_traverse(NULL, uri); diff --git a/src/resource.c b/src/resource.c index 3a881ed4..df8d4a06 100644 --- a/src/resource.c +++ b/src/resource.c @@ -85,10 +85,7 @@ unknown: static struct resources * get_parent_resources(void) { - struct validation *state = state_retrieve(); - return (state != NULL) - ? x509stack_peek_resources(validation_certstack(state)) - : NULL; + return x509stack_peek_resources(validation_certstack(state_retrieve())); } static int diff --git a/src/rpp.c b/src/rpp.c index e3c83e6f..efa6333a 100644 --- a/src/rpp.c +++ b/src/rpp.c @@ -218,8 +218,6 @@ __cert_traverse(struct rpp *pp) return 0; state = state_retrieve(); - if (state == NULL) - return -EINVAL; certstack = validation_certstack(state); deferred.pp = pp; diff --git a/src/rrdp/db/db_rrdp_uris.c b/src/rrdp/db/db_rrdp_uris.c index 07ee0f2a..3bdae29a 100644 --- a/src/rrdp/db/db_rrdp_uris.c +++ b/src/rrdp/db/db_rrdp_uris.c @@ -63,11 +63,6 @@ find_rrdp_uri(struct db_rrdp_uri *uris, const char *search) return found; } -#define RET_NOT_FOUND_URI(uris, search, found) \ - found = find_rrdp_uri(uris, search); \ - if (found == NULL) \ - return -ENOENT; - static void add_rrdp_uri(struct db_rrdp_uri *uris, struct uris_table *new_uri) { @@ -78,30 +73,16 @@ add_rrdp_uri(struct db_rrdp_uri *uris, struct uris_table *new_uri) uris_table_destroy(old_uri); } -static int -get_thread_rrdp_uris(struct db_rrdp_uri **result) +static struct db_rrdp_uri * +get_thread_rrdp_uris(void) { - struct validation *state; - - state = state_retrieve(); - if (state == NULL) - return pr_val_err("No state related to this thread"); - - *result = validation_get_rrdp_uris(state); - return 0; + return validation_get_rrdp_uris(state_retrieve()); } -static int -get_thread_rrdp_workspace(char const **result) +static char const * +get_thread_rrdp_workspace(void) { - struct validation *state; - - state = state_retrieve(); - if (state == NULL) - return pr_val_err("No state related to this thread"); - - *result = validation_get_rrdp_workspace(state); - return 0; + return validation_get_rrdp_workspace(state_retrieve()); } struct db_rrdp_uri * @@ -128,79 +109,55 @@ db_rrdp_uris_destroy(struct db_rrdp_uri *uris) free(uris); } -int -db_rrdp_uris_cmp(char const *uri, char const *session_id, unsigned long serial, - rrdp_uri_cmp_result_t *result) +rrdp_uri_cmp_result_t +db_rrdp_uris_cmp(char const *uri, char const *session_id, unsigned long serial) { - struct db_rrdp_uri *uris; struct uris_table *found; - int error; - uris = NULL; - error = get_thread_rrdp_uris(&uris); - if (error) - return error; - - found = find_rrdp_uri(uris, uri); + found = find_rrdp_uri(get_thread_rrdp_uris(), uri); if (found == NULL) { pr_val_debug("I don't have state for this Update Notification; downloading snapshot..."); - *result = RRDP_URI_NOTFOUND; - return 0; + return RRDP_URI_NOTFOUND; } if (strcmp(session_id, found->data.session_id) != 0) { pr_val_debug("session_id changed from '%s' to '%s'.", found->data.session_id, session_id); - *result = RRDP_URI_DIFF_SESSION; - return 0; + return RRDP_URI_DIFF_SESSION; } if (serial != found->data.serial) { pr_val_debug("The serial changed from %lu to %lu.", found->data.serial, serial); - *result = RRDP_URI_DIFF_SERIAL; - return 0; + return RRDP_URI_DIFF_SERIAL; } pr_val_debug("The new Update Notification has the same session_id (%s) and serial (%lu) as the old one.", session_id, serial); - *result = RRDP_URI_EQUAL; - return 0; + return RRDP_URI_EQUAL; } -int +void db_rrdp_uris_update(char const *uri, char const *session_id, unsigned long serial, rrdp_req_status_t req_status, struct visited_uris *visited_uris) { - struct db_rrdp_uri *uris; struct uris_table *db_uri; - int error; - - uris = NULL; - error = get_thread_rrdp_uris(&uris); - if (error) - return error; db_uri = uris_table_create(uri, session_id, serial, req_status); db_uri->visited_uris = visited_uris; /* Ownership transfered */ - add_rrdp_uri(uris, db_uri); - return 0; + add_rrdp_uri(get_thread_rrdp_uris(), db_uri); } int db_rrdp_uris_get_serial(char const *uri, unsigned long *serial) { - struct db_rrdp_uri *uris; struct uris_table *found; - int error; - uris = NULL; - error = get_thread_rrdp_uris(&uris); - if (error) - return error; + found = find_rrdp_uri(get_thread_rrdp_uris(), uri); + if (found == NULL) + return -ENOENT; - RET_NOT_FOUND_URI(uris, uri, found) *serial = found->data.serial; return 0; } @@ -208,16 +165,12 @@ db_rrdp_uris_get_serial(char const *uri, unsigned long *serial) int db_rrdp_uris_get_last_update(char const *uri, long *date) { - struct db_rrdp_uri *uris; struct uris_table *found; - int error; - uris = NULL; - error = get_thread_rrdp_uris(&uris); - if (error) - return error; + found = find_rrdp_uri(get_thread_rrdp_uris(), uri); + if (found == NULL) + return -ENOENT; - RET_NOT_FOUND_URI(uris, uri, found) *date = found->last_update; return 0; } @@ -226,23 +179,20 @@ db_rrdp_uris_get_last_update(char const *uri, long *date) int db_rrdp_uris_set_last_update(char const *uri) { - struct db_rrdp_uri *uris; struct uris_table *found; time_t now; int error; - uris = NULL; - error = get_thread_rrdp_uris(&uris); - if (error) - return error; - - RET_NOT_FOUND_URI(uris, uri, found) + found = find_rrdp_uri(get_thread_rrdp_uris(), uri); + if (found == NULL) + return -ENOENT; now = 0; error = get_current_time(&now); if (error) return error; + /* TODO (#78) why are we casting */ found->last_update = (long)now; return 0; } @@ -250,16 +200,12 @@ db_rrdp_uris_set_last_update(char const *uri) int db_rrdp_uris_get_request_status(char const *uri, rrdp_req_status_t *result) { - struct db_rrdp_uri *uris; struct uris_table *found; - int error; - uris = NULL; - error = get_thread_rrdp_uris(&uris); - if (error) - return error; + found = find_rrdp_uri(get_thread_rrdp_uris(), uri); + if (found == NULL) + return -ENOENT; - RET_NOT_FOUND_URI(uris, uri, found) *result = found->request_status; return 0; } @@ -267,57 +213,40 @@ db_rrdp_uris_get_request_status(char const *uri, rrdp_req_status_t *result) int db_rrdp_uris_set_request_status(char const *uri, rrdp_req_status_t value) { - struct db_rrdp_uri *uris; struct uris_table *found; - int error; - uris = NULL; - error = get_thread_rrdp_uris(&uris); - if (error) - return error; + found = find_rrdp_uri(get_thread_rrdp_uris(), uri); + if (found == NULL) + return -ENOENT; - RET_NOT_FOUND_URI(uris, uri, found) found->request_status = value; return 0; } -int +void db_rrdp_uris_set_all_unvisited(void) { struct db_rrdp_uri *uris; struct uris_table *uri_node, *uri_tmp; - int error; - - uris = NULL; - error = get_thread_rrdp_uris(&uris); - if (error) - return error; + uris = get_thread_rrdp_uris(); HASH_ITER(hh, uris->table, uri_node, uri_tmp) uri_node->request_status = RRDP_URI_REQ_UNVISITED; - - return 0; } /* - * Returns a pointer (set in @result) to the visited_uris of the current - * thread. + * Returns a pointer to the visited_uris of the current thread. */ -int -db_rrdp_uris_get_visited_uris(char const *uri, struct visited_uris **result) +struct visited_uris * +db_rrdp_uris_get_visited_uris(char const *uri) { - struct db_rrdp_uri *uris; struct uris_table *found; - int error; - uris = NULL; - error = get_thread_rrdp_uris(&uris); - if (error) - return error; + found = find_rrdp_uri(get_thread_rrdp_uris(), uri); + if (found == NULL) + return NULL; - RET_NOT_FOUND_URI(uris, uri, found) - *result = found->visited_uris; - return 0; + return found->visited_uris; } int @@ -340,42 +269,17 @@ db_rrdp_uris_remove_all_local(struct db_rrdp_uri *uris, char const *workspace) char const * db_rrdp_uris_workspace_get(void) { - struct db_rrdp_uri *uris; - int error; - - uris = NULL; - error = get_thread_rrdp_uris(&uris); - if (error) - return NULL; - - return uris->current_workspace; + return get_thread_rrdp_uris()->current_workspace; } -int +void db_rrdp_uris_workspace_enable(void) { - struct db_rrdp_uri *uris; - int error; - - uris = NULL; - error = get_thread_rrdp_uris(&uris); - if (error) - return error; - - return get_thread_rrdp_workspace(&uris->current_workspace); + get_thread_rrdp_uris()->current_workspace = get_thread_rrdp_workspace(); } -int +void db_rrdp_uris_workspace_disable(void) { - struct db_rrdp_uri *uris; - int error; - - uris = NULL; - error = get_thread_rrdp_uris(&uris); - if (error) - return error; - - uris->current_workspace = NULL; - return 0; + get_thread_rrdp_uris()->current_workspace = NULL; } diff --git a/src/rrdp/db/db_rrdp_uris.h b/src/rrdp/db/db_rrdp_uris.h index 940f5df5..781df882 100644 --- a/src/rrdp/db/db_rrdp_uris.h +++ b/src/rrdp/db/db_rrdp_uris.h @@ -21,9 +21,9 @@ struct db_rrdp_uri; struct db_rrdp_uri *db_rrdp_uris_create(void); void db_rrdp_uris_destroy(struct db_rrdp_uri *); -int db_rrdp_uris_cmp(char const *, char const *, unsigned long, - rrdp_uri_cmp_result_t *); -int db_rrdp_uris_update(char const *, char const *session_id, unsigned long, +rrdp_uri_cmp_result_t db_rrdp_uris_cmp(char const *, char const *, + unsigned long); +void db_rrdp_uris_update(char const *, char const *session_id, unsigned long, rrdp_req_status_t, struct visited_uris *); int db_rrdp_uris_get_serial(char const *, unsigned long *); @@ -32,14 +32,14 @@ int db_rrdp_uris_set_last_update(char const *); int db_rrdp_uris_get_request_status(char const *, rrdp_req_status_t *); int db_rrdp_uris_set_request_status(char const *, rrdp_req_status_t); -int db_rrdp_uris_set_all_unvisited(void); +void db_rrdp_uris_set_all_unvisited(void); -int db_rrdp_uris_get_visited_uris(char const *, struct visited_uris **); +struct visited_uris *db_rrdp_uris_get_visited_uris(char const *); int db_rrdp_uris_remove_all_local(struct db_rrdp_uri *, char const *); char const *db_rrdp_uris_workspace_get(void); -int db_rrdp_uris_workspace_enable(void); -int db_rrdp_uris_workspace_disable(void); +void db_rrdp_uris_workspace_enable(void); +void db_rrdp_uris_workspace_disable(void); #endif /* SRC_RRDP_DB_DB_RRDP_URIS_H_ */ diff --git a/src/rrdp/rrdp_loader.c b/src/rrdp/rrdp_loader.c index cea7d830..4e47438e 100644 --- a/src/rrdp/rrdp_loader.c +++ b/src/rrdp/rrdp_loader.c @@ -24,9 +24,9 @@ process_diff_serial(struct update_notification *notification, return error; /* Work with the existent visited uris */ - error = db_rrdp_uris_get_visited_uris(notification->uri, visited); - if (error) - return error; + *visited = db_rrdp_uris_get_visited_uris(notification->uri); + if ((*visited) == NULL) + return -ENOENT; return rrdp_process_deltas(notification, serial, *visited, log_operation); @@ -57,39 +57,23 @@ static int remove_rrdp_uri_files(char const *notification_uri) { struct visited_uris *tmp; - char const *workspace; - int error; /* Work with the existent visited uris */ - error = db_rrdp_uris_get_visited_uris(notification_uri, &tmp); - if (error) - return error; + tmp = db_rrdp_uris_get_visited_uris(notification_uri); + if (tmp == NULL) + return -ENOENT; - workspace = db_rrdp_uris_workspace_get(); - - return visited_uris_delete_local(tmp, workspace); + return visited_uris_delete_local(tmp, db_rrdp_uris_workspace_get()); } /* Mark the URI as errored with dummy data, so it won't be requested again */ -static int +static void mark_rrdp_uri_request_err(char const *notification_uri) { - struct visited_uris *tmp; - int error; - pr_val_debug("RRDP data of '%s' won't be requested again during this cycle due to previous error.", notification_uri); - - tmp = visited_uris_create(); - - error = db_rrdp_uris_update(notification_uri, "", 0, - RRDP_URI_REQ_ERROR, tmp); - if (error) { - visited_uris_refput(tmp); - return error; - } - - return 0; + db_rrdp_uris_update(notification_uri, "", 0, RRDP_URI_REQ_ERROR, + visited_uris_create()); } static int @@ -211,12 +195,9 @@ __rrdp_load(struct rpki_uri *uri, bool force_snapshot, bool *data_updated) break; } - error = db_rrdp_uris_cmp(uri_get_global(uri), + res = db_rrdp_uris_cmp(uri_get_global(uri), upd_notification->global_data.session_id, - upd_notification->global_data.serial, - &res); - if (error) - goto upd_destroy; + upd_notification->global_data.serial); switch (res) { case RRDP_URI_EQUAL: @@ -255,13 +236,11 @@ __rrdp_load(struct rpki_uri *uri, bool force_snapshot, bool *data_updated) pr_val_debug("Updating local RRDP data of '%s' to:", uri_get_global(uri)); pr_val_debug("- Session ID: %s", upd_notification->global_data.session_id); pr_val_debug("- Serial: %lu", upd_notification->global_data.serial); - error = db_rrdp_uris_update(uri_get_global(uri), + db_rrdp_uris_update(uri_get_global(uri), upd_notification->global_data.session_id, upd_notification->global_data.serial, RRDP_URI_REQ_VISITED, visited); - if (error) - goto upd_destroy; set_update: /* Set the last update to now */ @@ -290,9 +269,7 @@ upd_end: reset_downloaded(); } - upd_error = mark_rrdp_uri_request_err(uri_get_global(uri)); - if (upd_error) - return upd_error; + mark_rrdp_uri_request_err(uri_get_global(uri)); return error; } diff --git a/src/rsync/rsync.c b/src/rsync/rsync.c index a24323ee..bbcc380a 100644 --- a/src/rsync/rsync.c +++ b/src/rsync/rsync.c @@ -575,9 +575,6 @@ rsync_download_files(struct rpki_uri *requested_uri, bool is_ta, bool force) return 0; state = state_retrieve(); - if (state == NULL) - return -EINVAL; - visited_uris = validation_rsync_visited_uris(state); if (!force && is_already_downloaded(requested_uri, visited_uris)) { @@ -626,15 +623,10 @@ rsync_download_files(struct rpki_uri *requested_uri, bool is_ta, bool force) void reset_downloaded(void) { - struct validation *state; struct uri_list *list; struct uri *uri; - state = state_retrieve(); - if (state == NULL) - return; - - list = validation_rsync_visited_uris(state); + list = validation_rsync_visited_uris(state_retrieve()); while (!SLIST_EMPTY(list)) { uri = SLIST_FIRST(list); diff --git a/src/thread_var.c b/src/thread_var.c index 4dcbe5fb..d4812bcd 100644 --- a/src/thread_var.c +++ b/src/thread_var.c @@ -94,7 +94,10 @@ state_store(struct validation *state) return error; } -/* Returns the current thread's validation state. */ +/* + * Returns the current thread's validation state. Never returns NULL by + * contract. + */ struct validation * state_retrieve(void) { @@ -332,12 +335,7 @@ working_repo_pop(void) static char const * addr2str(int af, void const *addr, char *(*buffer_cb)(struct validation *)) { - struct validation *state; - - state = state_retrieve(); - if (state == NULL) - return NULL; - + struct validation *state = state_retrieve(); return inet_ntop(af, addr, buffer_cb(state), INET6_ADDRSTRLEN); } diff --git a/src/validation_handler.c b/src/validation_handler.c index 364ee1c5..9461f8f0 100644 --- a/src/validation_handler.c +++ b/src/validation_handler.c @@ -4,16 +4,15 @@ #include "log.h" #include "thread_var.h" +/* + * Never returns NULL by contract. + */ static struct validation_handler const * get_current_threads_handler(void) { - struct validation *state; struct validation_handler const *handler; - state = state_retrieve(); - if (state == NULL) - return NULL; - handler = validation_get_validation_handler(state); + handler = validation_get_validation_handler(state_retrieve()); if (handler == NULL) pr_crit("This thread lacks a validation handler."); @@ -27,8 +26,6 @@ vhandler_handle_roa_v4(uint32_t as, struct ipv4_prefix const *prefix, struct validation_handler const *handler; handler = get_current_threads_handler(); - if (handler == NULL) - return -EINVAL; return (handler->handle_roa_v4 != NULL) ? handler->handle_roa_v4(as, prefix, max_length, handler->arg) @@ -42,8 +39,6 @@ vhandler_handle_roa_v6(uint32_t as, struct ipv6_prefix const *prefix, struct validation_handler const *handler; handler = get_current_threads_handler(); - if (handler == NULL) - return -EINVAL; return (handler->handle_roa_v6 != NULL) ? handler->handle_roa_v6(as, prefix, max_length, handler->arg) @@ -57,8 +52,6 @@ vhandler_handle_router_key(unsigned char const *ski, uint32_t as, struct validation_handler const *handler; handler = get_current_threads_handler(); - if (handler == NULL) - return -EINVAL; return (handler->handle_router_key != NULL) ? handler->handle_router_key(ski, as, spk, handler->arg)