QUIC err handling: Save and restore error state
authorTomas Mraz <tomas@openssl.org>
Tue, 30 May 2023 20:14:58 +0000 (22:14 +0200)
committerTomas Mraz <tomas@openssl.org>
Fri, 7 Jul 2023 13:13:29 +0000 (15:13 +0200)
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)

12 files changed:
crypto/err/build.info
crypto/err/err.c
crypto/err/err_local.h
crypto/err/err_save.c [new file with mode: 0644]
doc/build.info
doc/man3/OSSL_ERR_STATE_save.pod [new file with mode: 0644]
include/internal/quic_channel.h
include/openssl/err.h.in
ssl/quic/quic_channel.c
ssl/quic/quic_channel_local.h
ssl/quic/quic_impl.c
util/libcrypto.num

index ee379384188e555ad48b6678037a6b98aa72e81e..8552d7c393fd8583fcdcc64cb9461fea7a45b75a 100644 (file)
@@ -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
index a6c5ff613283ca2fb1c0cd4cee7b95298f3a9de8..972856ad2356ea65f7897828070a48b4db25a53c 100644 (file)
@@ -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;
         }
index 7d785ab6181e30eddce25c5f6a766c3bb63dcfb9..202ac35ad484f7d8ca15547b3be8449133565001 100644 (file)
@@ -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 (file)
index 0000000..a471b22
--- /dev/null
@@ -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);
+        }
+    }
+}
index 24d4050a45a7b5559b57527447cbb2161bcefee6..7b96381240a861453140239e924f8e3894ba2daa 100644 (file)
@@ -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 (file)
index 0000000..79f3aba
--- /dev/null
@@ -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
index bfa581857497788ceae8837cd9c984705400ab7f..0606a11698dbe0d0ba0df24f77b9fba0c1a17b10 100644 (file)
@@ -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);
index 26b98800f95f52cd4b3b8ca754ff18118d0b58ce..075d683f8d4ac33145d95ba4dcd86e2e73c46f50 100644 (file)
@@ -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
index 0ea0c8110a3e4e035c2a3047011797bf39512782..3edb059ad4e34c811a135ac02fd6ed1fcc381326 100644 (file)
@@ -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;
 
index 69469780aabdb50182100a1d009e1b892555e127..44ebc23f22a1d051491fa9c1fa437ce9973ca768 100644 (file)
@@ -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
index c0f65c2138e7d55994c34c74aaadee8f1e2a77b5..2eb16027649aae4f1f83c1568019443f6755c82e 100644 (file)
@@ -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;
 }
 
index d909721a3681f4e4b7bcc9f45872d0b3b6fe4057..3195ccfbd8997d2de792c609c93ccafd5baef4cd 100644 (file)
@@ -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: