diff options
author | Tomas Mraz <tomas@openssl.org> | 2023-05-30 22:14:58 +0200 |
---|---|---|
committer | Tomas Mraz <tomas@openssl.org> | 2023-07-07 15:13:29 +0200 |
commit | 9c3ea4e1d7580fc061dfb754b620adb3439e683f (patch) | |
tree | e87832dee0de7a0e5a1f0da4a71e59ac89f3345d | |
parent | 5c3474ea563ed95bb7c86c08867139613655276b (diff) | |
download | openssl-9c3ea4e1d7580fc061dfb754b620adb3439e683f.tar.gz |
QUIC err handling: Save and restore error state
We save the error state from the thread that encountered
a permanent error condition caused by system or internal
error to the QUIC_CHANNEL.
Then we restore it whenever we are returning to a user
call when protocol is shutdown.
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21087)
-rw-r--r-- | crypto/err/build.info | 2 | ||||
-rw-r--r-- | crypto/err/err.c | 10 | ||||
-rw-r--r-- | crypto/err/err_local.h | 5 | ||||
-rw-r--r-- | crypto/err/err_save.c | 89 | ||||
-rw-r--r-- | doc/build.info | 6 | ||||
-rw-r--r-- | doc/man3/OSSL_ERR_STATE_save.pod | 69 | ||||
-rw-r--r-- | include/internal/quic_channel.h | 3 | ||||
-rw-r--r-- | include/openssl/err.h.in | 5 | ||||
-rw-r--r-- | ssl/quic/quic_channel.c | 28 | ||||
-rw-r--r-- | ssl/quic/quic_channel_local.h | 3 | ||||
-rw-r--r-- | ssl/quic/quic_impl.c | 17 | ||||
-rw-r--r-- | util/libcrypto.num | 4 |
12 files changed, 224 insertions, 17 deletions
diff --git a/crypto/err/build.info b/crypto/err/build.info index ee37938418..8552d7c393 100644 --- a/crypto/err/build.info +++ b/crypto/err/build.info @@ -1,3 +1,3 @@ LIBS=../../libcrypto SOURCE[../../libcrypto]=\ - err_blocks.c err_mark.c err.c err_all.c err_all_legacy.c err_prn.c + err_blocks.c err_mark.c err.c err_all.c err_all_legacy.c err_prn.c err_save.c diff --git a/crypto/err/err.c b/crypto/err/err.c index a6c5ff6132..972856ad23 100644 --- a/crypto/err/err.c +++ b/crypto/err/err.c @@ -33,7 +33,6 @@ ERR_STATE *ERR_get_state(void); static int err_load_strings(const ERR_STRING_DATA *str); #endif -static void ERR_STATE_free(ERR_STATE *s); #ifndef OPENSSL_NO_ERR static ERR_STRING_DATA ERR_str_libraries[] = { {ERR_PACK(ERR_LIB_NONE, 0, 0), "unknown library"}, @@ -199,7 +198,7 @@ static ERR_STRING_DATA *int_err_get_item(const ERR_STRING_DATA *d) } #endif -static void ERR_STATE_free(ERR_STATE *state) +void OSSL_ERR_STATE_free(ERR_STATE *state) { int i; @@ -649,7 +648,7 @@ static void err_delete_thread_state(void *unused) return; CRYPTO_THREAD_set_local(&err_thread_local, NULL); - ERR_STATE_free(state); + OSSL_ERR_STATE_free(state); } #ifndef OPENSSL_NO_DEPRECATED_1_1_0 @@ -689,8 +688,7 @@ ERR_STATE *ossl_err_get_state_int(void) if (!CRYPTO_THREAD_set_local(&err_thread_local, (ERR_STATE*)-1)) return NULL; - /* calling CRYPTO_zalloc(.., NULL, 0) prevents mem alloc error loop */ - state = CRYPTO_zalloc(sizeof(*state), NULL, 0); + state = OSSL_ERR_STATE_new(); if (state == NULL) { CRYPTO_THREAD_set_local(&err_thread_local, NULL); return NULL; @@ -698,7 +696,7 @@ ERR_STATE *ossl_err_get_state_int(void) if (!ossl_init_thread_start(NULL, NULL, err_delete_thread_state) || !CRYPTO_THREAD_set_local(&err_thread_local, state)) { - ERR_STATE_free(state); + OSSL_ERR_STATE_free(state); CRYPTO_THREAD_set_local(&err_thread_local, NULL); return NULL; } diff --git a/crypto/err/err_local.h b/crypto/err/err_local.h index 7d785ab618..202ac35ad4 100644 --- a/crypto/err/err_local.h +++ b/crypto/err/err_local.h @@ -66,8 +66,9 @@ static ossl_inline void err_set_debug(ERR_STATE *es, size_t i, OPENSSL_free(es->err_func[i]); if (fn == NULL || fn[0] == '\0') es->err_func[i] = NULL; - else - es->err_func[i] = OPENSSL_strdup(fn); + else if ((es->err_func[i] = CRYPTO_malloc(strlen(fn) + 1, + NULL, 0)) != NULL) + strcpy(es->err_func[i], fn); } static ossl_inline void err_set_data(ERR_STATE *es, size_t i, diff --git a/crypto/err/err_save.c b/crypto/err/err_save.c new file mode 100644 index 0000000000..a471b22682 --- /dev/null +++ b/crypto/err/err_save.c @@ -0,0 +1,89 @@ +/* + * Copyright 2023 The OpenSSL Project Authors. All Rights Reserved. + * + * Licensed under the Apache License 2.0 (the "License"). You may not use + * this file except in compliance with the License. You can obtain a copy + * in the file LICENSE in the source distribution or at + * https://www.openssl.org/source/license.html + */ + +#define OSSL_FORCE_ERR_STATE + +#include <openssl/err.h> +#include "err_local.h" + +/* + * Save and restore error state. + * We are using CRYPTO_zalloc(.., NULL, 0) instead of OPENSSL_malloc() in + * these functions to prevent mem alloc error loop. + */ + +ERR_STATE *OSSL_ERR_STATE_new(void) +{ + return CRYPTO_zalloc(sizeof(ERR_STATE), NULL, 0); +} + +void OSSL_ERR_STATE_save(ERR_STATE *es) +{ + size_t i; + ERR_STATE *thread_es; + + if (es == NULL) + return; + + for (i = 0; i < ERR_NUM_ERRORS; i++) + err_clear(es, i, 1); + + thread_es = ossl_err_get_state_int(); + if (thread_es == NULL) + return; + + memcpy(es, thread_es, sizeof(*es)); + /* Taking over the pointers, just clear the thread state. */ + memset(thread_es, 0, sizeof(*thread_es)); +} + +void OSSL_ERR_STATE_restore(const ERR_STATE *es) +{ + size_t i; + ERR_STATE *thread_es; + + if (es == NULL || es->bottom == es->top) + return; + + thread_es = ossl_err_get_state_int(); + if (thread_es == NULL) + return; + + for (i = (size_t)es->bottom; i != (size_t)es->top;) { + size_t top; + + i = (i + 1) % ERR_NUM_ERRORS; + if ((es->err_flags[i] & ERR_FLAG_CLEAR) != 0) + continue; + + err_get_slot(thread_es); + top = thread_es->top; + err_clear(thread_es, top, 0); + + thread_es->err_flags[top] = es->err_flags[i]; + thread_es->err_buffer[top] = es->err_buffer[i]; + + err_set_debug(thread_es, top, es->err_file[i], es->err_line[i], + es->err_func[i]); + + if (es->err_data[i] != NULL && es->err_data_size[i] != 0) { + void *data; + size_t data_sz = es->err_data_size[i]; + + data = CRYPTO_malloc(data_sz, NULL, 0); + if (data != NULL) { + memcpy(data, es->err_data[i], data_sz); + err_set_data(thread_es, top, data, data_sz, + es->err_data_flags[i] | ERR_TXT_MALLOCED); + } + } else { + err_clear_data(thread_es, top, 0); + } + } +} diff --git a/doc/build.info b/doc/build.info index 24d4050a45..7b96381240 100644 --- a/doc/build.info +++ b/doc/build.info @@ -1687,6 +1687,10 @@ DEPEND[html/man3/OSSL_ENCODER_to_bio.html]=man3/OSSL_ENCODER_to_bio.pod GENERATE[html/man3/OSSL_ENCODER_to_bio.html]=man3/OSSL_ENCODER_to_bio.pod DEPEND[man/man3/OSSL_ENCODER_to_bio.3]=man3/OSSL_ENCODER_to_bio.pod GENERATE[man/man3/OSSL_ENCODER_to_bio.3]=man3/OSSL_ENCODER_to_bio.pod +DEPEND[html/man3/OSSL_ERR_STATE_save.html]=man3/OSSL_ERR_STATE_save.pod +GENERATE[html/man3/OSSL_ERR_STATE_save.html]=man3/OSSL_ERR_STATE_save.pod +DEPEND[man/man3/OSSL_ERR_STATE_save.3]=man3/OSSL_ERR_STATE_save.pod +GENERATE[man/man3/OSSL_ERR_STATE_save.3]=man3/OSSL_ERR_STATE_save.pod DEPEND[html/man3/OSSL_ESS_check_signing_certs.html]=man3/OSSL_ESS_check_signing_certs.pod GENERATE[html/man3/OSSL_ESS_check_signing_certs.html]=man3/OSSL_ESS_check_signing_certs.pod DEPEND[man/man3/OSSL_ESS_check_signing_certs.3]=man3/OSSL_ESS_check_signing_certs.pod @@ -3325,6 +3329,7 @@ html/man3/OSSL_ENCODER.html \ html/man3/OSSL_ENCODER_CTX.html \ html/man3/OSSL_ENCODER_CTX_new_for_pkey.html \ html/man3/OSSL_ENCODER_to_bio.html \ +html/man3/OSSL_ERR_STATE_save.html \ html/man3/OSSL_ESS_check_signing_certs.html \ html/man3/OSSL_HPKE_CTX_new.html \ html/man3/OSSL_HTTP_REQ_CTX.html \ @@ -3963,6 +3968,7 @@ man/man3/OSSL_ENCODER.3 \ man/man3/OSSL_ENCODER_CTX.3 \ man/man3/OSSL_ENCODER_CTX_new_for_pkey.3 \ man/man3/OSSL_ENCODER_to_bio.3 \ +man/man3/OSSL_ERR_STATE_save.3 \ man/man3/OSSL_ESS_check_signing_certs.3 \ man/man3/OSSL_HPKE_CTX_new.3 \ man/man3/OSSL_HTTP_REQ_CTX.3 \ diff --git a/doc/man3/OSSL_ERR_STATE_save.pod b/doc/man3/OSSL_ERR_STATE_save.pod new file mode 100644 index 0000000000..79f3aba5f8 --- /dev/null +++ b/doc/man3/OSSL_ERR_STATE_save.pod @@ -0,0 +1,69 @@ +=pod + +=head1 NAME + +OSSL_ERR_STATE_new, OSSL_ERR_STATE_save, OSSL_ERR_STATE_restore, +OSSL_ERR_STATE_free - saving and restoring error state + +=head1 SYNOPSIS + + #include <openssl/err.h> + + ERR_STATE *OSSL_ERR_STATE_new(void); + void OSSL_ERR_STATE_save(ERR_STATE *es); + void OSSL_ERR_STATE_restore(const ERR_STATE *es); + void OSSL_ERR_STATE_free(ERR_STATE *es); + +=head1 DESCRIPTION + +These functions save and restore the error state from the thread +local error state to a preallocated error state structure. + +OSSL_ERR_STATE_new() allocates an empty error state structure to +be used when saving and restoring thread error state. + +OSSL_ERR_STATE_save() saves the thread error state to I<es>. It +subsequently clears the thread error state. Any previously saved +state in I<es> is cleared prior to saving the new state. + +OSSL_ERR_STATE_restore() adds all the error entries from the +saved state I<es> to the thread error state. Existing entries in +the thread error state are not affected if there is enough space +for all the added entries. Any allocated data in the saved error +entries is duplicated on adding to the thread state. + +OSSL_ERR_STATE_free() frees the saved error state I<es>. + +=head1 RETURN VALUES + +OSSL_ERR_STATE_new() returns a pointer to the allocated ERR_STATE +structure or NULL on error. + +OSSL_ERR_STATE_save(), OSSL_ERR_STATE_restore(), OSSL_ERR_STATE_free() +do not return any values. + +=head1 NOTES + +OSSL_ERR_STATE_save() cannot fail as it takes over any allocated +data from the thread error state. + +OSSL_ERR_STATE_restore() is a best effort function. The only failure +that can happen during its operation is when memory allocation fails. +Because it manipulates the thread error state it avoids raising memory +errors on such failure. At worst the restored error entries will be +missing the auxiliary error data. + +=head1 SEE ALSO + +L<ERR_raise(3)>, L<ERR_get_error(3)>, L<ERR_clear_error(3)> + +=head1 COPYRIGHT + +Copyright 2023 The OpenSSL Project Authors. All Rights Reserved. + +Licensed under the Apache License 2.0 (the "License"). You may not use +this file except in compliance with the License. You can obtain a copy +in the file LICENSE in the source distribution or at +L<https://www.openssl.org/source/license.html>. + +=cut diff --git a/include/internal/quic_channel.h b/include/internal/quic_channel.h index bfa5818574..0606a11698 100644 --- a/include/internal/quic_channel.h +++ b/include/internal/quic_channel.h @@ -222,6 +222,9 @@ void ossl_quic_channel_raise_protocol_error(QUIC_CHANNEL *ch, */ int ossl_quic_channel_net_error(QUIC_CHANNEL *ch); +/* Restore saved error state (best effort) */ +void ossl_quic_channel_restore_err_state(QUIC_CHANNEL *ch); + /* For RXDP use. */ void ossl_quic_channel_on_remote_conn_close(QUIC_CHANNEL *ch, OSSL_QUIC_FRAME_CONN_CLOSE *f); diff --git a/include/openssl/err.h.in b/include/openssl/err.h.in index 26b98800f9..075d683f8d 100644 --- a/include/openssl/err.h.in +++ b/include/openssl/err.h.in @@ -486,6 +486,11 @@ int ERR_set_mark(void); int ERR_pop_to_mark(void); int ERR_clear_last_mark(void); +ERR_STATE *OSSL_ERR_STATE_new(void); +void OSSL_ERR_STATE_save(ERR_STATE *es); +void OSSL_ERR_STATE_restore(const ERR_STATE *es); +void OSSL_ERR_STATE_free(ERR_STATE *es); + #ifdef __cplusplus } #endif diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c index 0ea0c8110a..3edb059ad4 100644 --- a/ssl/quic/quic_channel.c +++ b/ssl/quic/quic_channel.c @@ -7,12 +7,13 @@ * https://www.openssl.org/source/license.html */ +#include <openssl/rand.h> +#include <openssl/err.h> #include "internal/quic_channel.h" #include "internal/quic_error.h" #include "internal/quic_rx_depack.h" #include "../ssl_local.h" #include "quic_channel_local.h" -#include <openssl/rand.h> /* * NOTE: While this channel implementation currently has basic server support, @@ -352,6 +353,7 @@ static void ch_cleanup(QUIC_CHANNEL *ch) ossl_qrx_free(ch->qrx); ossl_quic_demux_free(ch->demux); OPENSSL_free(ch->local_transport_params); + OSSL_ERR_STATE_free(ch->err_state); } QUIC_CHANNEL *ossl_quic_channel_new(const QUIC_CHANNEL_ARGS *args) @@ -2554,11 +2556,23 @@ void ossl_quic_channel_on_new_conn_id(QUIC_CHANNEL *ch, } } +static void ch_save_err_state(QUIC_CHANNEL *ch) +{ + if (ch->err_state == NULL) + ch->err_state = OSSL_ERR_STATE_new(); + + if (ch->err_state == NULL) + return; + + OSSL_ERR_STATE_save(ch->err_state); +} + static void ch_raise_net_error(QUIC_CHANNEL *ch) { QUIC_TERMINATE_CAUSE tcause = {0}; ch->net_error = 1; + ch_save_err_state(ch); tcause.error_code = QUIC_ERR_INTERNAL_ERROR; @@ -2574,6 +2588,14 @@ int ossl_quic_channel_net_error(QUIC_CHANNEL *ch) return ch->net_error; } +void ossl_quic_channel_restore_err_state(QUIC_CHANNEL *ch) +{ + if (ch == NULL) + return; + + OSSL_ERR_STATE_restore(ch->err_state); +} + void ossl_quic_channel_raise_protocol_error(QUIC_CHANNEL *ch, uint64_t error_code, uint64_t frame_type, @@ -2581,6 +2603,10 @@ void ossl_quic_channel_raise_protocol_error(QUIC_CHANNEL *ch, { QUIC_TERMINATE_CAUSE tcause = {0}; + if (error_code == QUIC_ERR_INTERNAL_ERROR) + /* Internal errors might leave some errors on the stack. */ + ch_save_err_state(ch); + tcause.error_code = error_code; tcause.frame_type = frame_type; diff --git a/ssl/quic/quic_channel_local.h b/ssl/quic/quic_channel_local.h index 69469780aa..44ebc23f22 100644 --- a/ssl/quic/quic_channel_local.h +++ b/ssl/quic/quic_channel_local.h @@ -402,6 +402,9 @@ struct quic_channel_st { /* Permanent net error encountered */ unsigned int net_error : 1; + + /* Saved error stack in case permanent error was encountered */ + ERR_STATE *err_state; }; # endif diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c index c0f65c2138..2eb1602764 100644 --- a/ssl/quic/quic_impl.c +++ b/ssl/quic/quic_impl.c @@ -130,20 +130,23 @@ static int quic_raise_non_normal_error(QCTX *ctx, { va_list args; - ERR_new(); - ERR_set_debug(file, line, func); - - va_start(args, fmt); - ERR_vset_error(ERR_LIB_SSL, reason, fmt, args); - va_end(args); - if (ctx != NULL) { if (ctx->is_stream && ctx->xso != NULL) ctx->xso->last_error = SSL_ERROR_SSL; else if (!ctx->is_stream && ctx->qc != NULL) ctx->qc->last_error = SSL_ERROR_SSL; + + if (reason == SSL_R_PROTOCOL_IS_SHUTDOWN && ctx->qc != NULL) + ossl_quic_channel_restore_err_state(ctx->qc->ch); } + ERR_new(); + ERR_set_debug(file, line, func); + + va_start(args, fmt); + ERR_vset_error(ERR_LIB_SSL, reason, fmt, args); + va_end(args); + return 0; } diff --git a/util/libcrypto.num b/util/libcrypto.num index d909721a36..3195ccfbd8 100644 --- a/util/libcrypto.num +++ b/util/libcrypto.num @@ -5518,3 +5518,7 @@ X509_STORE_CTX_init_rpk ? 3_2_0 EXIST::FUNCTION: X509_STORE_CTX_get0_rpk ? 3_2_0 EXIST::FUNCTION: X509_STORE_CTX_set0_rpk ? 3_2_0 EXIST::FUNCTION: CRYPTO_atomic_load_int ? 3_2_0 EXIST::FUNCTION: +OSSL_ERR_STATE_new ? 3_2_0 EXIST::FUNCTION: +OSSL_ERR_STATE_save ? 3_2_0 EXIST::FUNCTION: +OSSL_ERR_STATE_restore ? 3_2_0 EXIST::FUNCTION: +OSSL_ERR_STATE_free ? 3_2_0 EXIST::FUNCTION: |