QUIC TLS: Rethink error handling

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #21547)

crypto/err/err_mark.c

@@ -46,6 +46,25 @@ int ERR_pop_to_mark(void)
     return 1;
 }
 
+int ERR_count_to_mark(void)
+{
+    ERR_STATE *es;
+    int count = 0, top;
+
+    es = ossl_err_get_state_int();
+    if (es == NULL)
+        return 0;
+
+    top = es->top;
+    while (es->bottom != top
+           && es->err_marks[top] == 0) {
+        ++count;
+        top = top > 0 ? top - 1 : ERR_NUM_ERRORS - 1;
+    }
+
+    return count;
+}
+
 int ERR_clear_last_mark(void)
 {
     ERR_STATE *es;

crypto/err/err_save.c

@@ -43,6 +43,71 @@ void OSSL_ERR_STATE_save(ERR_STATE *es)
     memset(thread_es, 0, sizeof(*thread_es));
 }
 
+void OSSL_ERR_STATE_save_to_mark(ERR_STATE *es)
+{
+    size_t i, j, count;
+    int top;
+    ERR_STATE *thread_es;
+
+    if (es == NULL)
+        return;
+
+    thread_es = ossl_err_get_state_int();
+    if (thread_es == NULL) {
+        for (i = 0; i < ERR_NUM_ERRORS; ++i)
+            err_clear(es, i, 1);
+
+        es->top = es->bottom = 0;
+        return;
+    }
+
+    /* Determine number of errors we are going to move. */
+    for (count = 0, top = thread_es->top;
+         thread_es->bottom != top
+         && thread_es->err_marks[top] == 0;
+         ++count)
+        top = top > 0 ? top - 1 : ERR_NUM_ERRORS - 1;
+
+    /* Move the errors, preserving order. */
+    for (i = 0, j = top; i < count; ++i) {
+        j = (j + 1) % ERR_NUM_ERRORS;
+
+        err_clear(es, i, 1);
+
+        /* Move the error entry to the given ERR_STATE. */
+        es->err_flags[i]        = thread_es->err_flags[j];
+        es->err_marks[i]        = 0;
+        es->err_buffer[i]       = thread_es->err_buffer[j];
+        es->err_data[i]         = thread_es->err_data[j];
+        es->err_data_size[i]    = thread_es->err_data_size[j];
+        es->err_data_flags[i]   = thread_es->err_data_flags[j];
+        es->err_file[i]         = thread_es->err_file[j];
+        es->err_line[i]         = thread_es->err_line[j];
+        es->err_func[i]         = thread_es->err_func[j];
+
+        thread_es->err_flags[j]     = 0;
+        thread_es->err_buffer[j]    = 0;
+        thread_es->err_data[j]      = NULL;
+        thread_es->err_data_size[j] = 0;
+        thread_es->err_file[j]      = NULL;
+        thread_es->err_line[j]      = 0;
+        thread_es->err_func[j]      = NULL;
+    }
+
+    if (i > 0) {
+        /* If we moved anything, es's stack always starts at [0]. */
+        es->top     = i - 1;
+        es->bottom  = ERR_NUM_ERRORS - 1;
+    } else {
+        /* Didn't move anything - empty stack */
+        es->top = es->bottom = 0;
+    }
+
+    /* Erase extra space as a precaution. */
+    for (; i < ERR_NUM_ERRORS; ++i)
+        err_clear(es, i, 1);
+}
+
 void OSSL_ERR_STATE_restore(const ERR_STATE *es)
 {
     size_t i;

crypto/err/openssl.txt

@@ -1498,6 +1498,7 @@ SSL_R_PROTOCOL_IS_SHUTDOWN:207:protocol is shutdown
 SSL_R_PSK_IDENTITY_NOT_FOUND:223:psk identity not found
 SSL_R_PSK_NO_CLIENT_CB:224:psk no client cb
 SSL_R_PSK_NO_SERVER_CB:225:psk no server cb
+SSL_R_QUIC_HANDSHAKE_LAYER_ERROR:393:quic handshake layer error
 SSL_R_QUIC_NETWORK_ERROR:387:quic network error
 SSL_R_QUIC_PROTOCOL_ERROR:382:quic protocol error
 SSL_R_READ_BIO_NOT_SET:211:read bio not set

doc/man3/ERR_set_mark.pod

@@ -2,8 +2,8 @@
 
 =head1 NAME
 
-ERR_set_mark, ERR_clear_last_mark, ERR_pop_to_mark
-- set mark, clear mark and pop errors until mark
+ERR_set_mark, ERR_clear_last_mark, ERR_pop_to_mark, ERR_count_to_mark - set
+mark, clear mark and pop errors until mark
 
 =head1 SYNOPSIS
 
@@ -12,6 +12,7 @@ ERR_set_mark, ERR_clear_last_mark, ERR_pop_to_mark
  int ERR_set_mark(void);
  int ERR_pop_to_mark(void);
  int ERR_clear_last_mark(void);
+ int ERR_count_to_mark(void);
 
 =head1 DESCRIPTION
 
@@ -23,13 +24,20 @@ The mark is then removed.  If there is no mark, the whole stack is removed.
 
 ERR_clear_last_mark() removes the last mark added if there is one.
 
+ERR_count_to_mark() returns the number of entries on the error stack above the
+most recently marked entry, not including that entry. If there is no mark in the
+error stack, the number of entries in the error stack is returned.
+
 =head1 RETURN VALUES
 
 ERR_set_mark() returns 0 if the error stack is empty, otherwise 1.
 
 ERR_clear_last_mark() and ERR_pop_to_mark() return 0 if there was no mark in the
 error stack, which implies that the stack became empty, otherwise 1.
 
+ERR_count_to_mark() returns the number of error stack entries found above the
+most recent mark, if any, or the total number of error stack entries.
+
 =head1 COPYRIGHT
 
 Copyright 2003-2021 The OpenSSL Project Authors. All Rights Reserved.

doc/man3/OSSL_ERR_STATE_save.pod

@@ -2,15 +2,16 @@
 
 =head1 NAME
 
-OSSL_ERR_STATE_new, OSSL_ERR_STATE_save, OSSL_ERR_STATE_restore,
-OSSL_ERR_STATE_free - saving and restoring error state
+OSSL_ERR_STATE_new, OSSL_ERR_STATE_save, OSSL_ERR_STATE_save_to_mark,
+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_save_to_mark(ERR_STATE *es);
  void OSSL_ERR_STATE_restore(const ERR_STATE *es);
  void OSSL_ERR_STATE_free(ERR_STATE *es);
 
@@ -26,6 +27,18 @@ 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_save_to_mark() is similar to OSSL_ERR_STATE_save() but only saves
+ERR entries up to the most recent mark on the ERR stack. These entries are moved
+to I<es> and removed from the thread error state. However, the most recent
+marked ERR and any ERR state before it remains part of the thread error state
+and is not moved to the ERR_STATE. The mark is not cleared and must be cleared
+explicitly after a call to this function using L<ERR_pop_to_mark(3)> or
+L<ERR_clear_last_mark(3)>. (Since a call to OSSL_ERR_STATE_save_to_mark() leaves
+the marked ERR as the top error, either of these functions will have the same
+effect.) If there is no marked ERR in the thread local error state, all ERR
+entries are copied and the effect is the same as for a call to
+OSSL_ERR_STATE_save().
+
 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
@@ -39,13 +52,13 @@ OSSL_ERR_STATE_free() frees the saved error state I<es>.
 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.
+OSSL_ERR_STATE_save(), OSSL_ERR_STATE_save_to_mark(), 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_save() and OSSL_ERR_STATE_save_to_mark() 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.

include/internal/quic_channel.h

@@ -221,6 +221,7 @@ void ossl_quic_channel_raise_protocol_error_loc(QUIC_CHANNEL *ch,
                                                 uint64_t error_code,
                                                 uint64_t frame_type,
                                                 const char *reason,
+                                                ERR_STATE *err_state,
                                                 const char *src_file,
                                                 int src_line,
                                                 const char *src_func);
@@ -229,10 +230,21 @@ void ossl_quic_channel_raise_protocol_error_loc(QUIC_CHANNEL *ch,
     ossl_quic_channel_raise_protocol_error_loc((ch), (error_code),  \
                                                (frame_type),        \
                                                (reason),            \
+                                               NULL,                \
                                                OPENSSL_FILE,        \
                                                OPENSSL_LINE,        \
                                                OPENSSL_FUNC)
 
+#define ossl_quic_channel_raise_protocol_error_state(ch, error_code, frame_type, reason, state) \
+    ossl_quic_channel_raise_protocol_error_loc((ch), (error_code),  \
+                                               (frame_type),        \
+                                               (reason),            \
+                                               (state),             \
+                                               OPENSSL_FILE,        \
+                                               OPENSSL_LINE,        \
+                                               OPENSSL_FUNC)
+
+
 /*
  * Returns 1 if permanent net error was detected on the QUIC_CHANNEL,
  * 0 otherwise.

include/internal/quic_tls.h

@@ -98,8 +98,6 @@ int ossl_quic_tls_set_transport_params(QUIC_TLS *qtls,
 int ossl_quic_tls_get_error(QUIC_TLS *qtls,
                             uint64_t *error_code,
                             const char **error_msg,
-                            const char **error_src_file,
-                            int *error_src_line,
-                            const char **error_src_func);
+                            ERR_STATE **error_state);
 
 #endif

include/internal/ssl.h

@@ -16,4 +16,6 @@
 typedef void (*ossl_msg_cb)(int write_p, int version, int content_type,
                             const void *buf, size_t len, SSL *ssl, void *arg);
 
+int ossl_ssl_get_error(const SSL *s, int i, int check_err);
+
 #endif

include/openssl/err.h.in

@@ -485,9 +485,11 @@ int ERR_get_next_error_library(void);
 int ERR_set_mark(void);
 int ERR_pop_to_mark(void);
 int ERR_clear_last_mark(void);
+int ERR_count_to_mark(void);
 
 ERR_STATE *OSSL_ERR_STATE_new(void);
 void OSSL_ERR_STATE_save(ERR_STATE *es);
+void OSSL_ERR_STATE_save_to_mark(ERR_STATE *es);
 void OSSL_ERR_STATE_restore(const ERR_STATE *es);
 void OSSL_ERR_STATE_free(ERR_STATE *es);
 

include/openssl/sslerr.h

@@ -230,6 +230,7 @@
 # define SSL_R_PSK_IDENTITY_NOT_FOUND                     223
 # define SSL_R_PSK_NO_CLIENT_CB                           224
 # define SSL_R_PSK_NO_SERVER_CB                           225
+# define SSL_R_QUIC_HANDSHAKE_LAYER_ERROR                 393
 # define SSL_R_QUIC_NETWORK_ERROR                         387
 # define SSL_R_QUIC_PROTOCOL_ERROR                        382
 # define SSL_R_READ_BIO_NOT_SET                           211

ssl/quic/quic_channel.c

@@ -1616,8 +1616,8 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags)
     QUIC_CHANNEL *ch = arg;
     int channel_only = (flags & QUIC_REACTOR_TICK_FLAG_CHANNEL_ONLY) != 0;
     uint64_t error_code;
-    const char *error_msg, *error_src_file, *error_src_func;
-    int error_src_line;
+    const char *error_msg;
+    ERR_STATE *error_state = NULL;
 
     /*
      * When we tick the QUIC connection, we do everything we need to do
@@ -1674,15 +1674,9 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags)
                 ossl_quic_tls_tick(ch->qtls);
 
                 if (ossl_quic_tls_get_error(ch->qtls, &error_code, &error_msg,
-                                            &error_src_file,
-                                            &error_src_line,
-                                            &error_src_func)) {
-                    ossl_quic_channel_raise_protocol_error_loc(ch, error_code, 0,
-                                                               error_msg,
-                                                               error_src_file,
-                                                               error_src_line,
-                                                               error_src_func);
-                }
+                                            &error_state))
+                    ossl_quic_channel_raise_protocol_error_state(ch, error_code, 0,
+                                                                 error_msg, error_state);
             }
 
             /*
@@ -2916,6 +2910,7 @@ void ossl_quic_channel_raise_protocol_error_loc(QUIC_CHANNEL *ch,
                                                 uint64_t error_code,
                                                 uint64_t frame_type,
                                                 const char *reason,
+                                                ERR_STATE *err_state,
                                                 const char *src_file,
                                                 int src_line,
                                                 const char *src_func)
@@ -2934,6 +2929,13 @@ void ossl_quic_channel_raise_protocol_error_loc(QUIC_CHANNEL *ch,
         err_str_sfx = "";
     }
 
+    /*
+     * If we were provided an underlying error state, restore it and then append
+     * our ERR on top as a "cover letter" error.
+     */
+    if (err_state != NULL)
+        OSSL_ERR_STATE_restore(err_state);
+
     if (frame_type != 0) {
         ft_str = ossl_quic_frame_type_to_string(frame_type);
         if (ft_str == NULL) {