QUIC TLS: Rethink error handling

[thirdparty/openssl.git] / ssl / quic / quic_tls.c

diff --git a/ssl/quic/quic_tls.c b/ssl/quic/quic_tls.c
index d8770bf9f7d145bd8f936bee472cdb1841a0f870..fd24026fa38351f5958dfab39f4aadd068c83600 100644 (file)
--- a/ssl/quic/quic_tls.c
+++ b/ssl/quic/quic_tls.c
@@ -29,6 +29,8 @@ struct quic_tls_st {
     const unsigned char *local_transport_params;
     size_t local_transport_params_len;
 
+    ERR_STATE *error_state;
+
     /*
      * QUIC error code (usually in the TLS Alert-mapped CRYPTO_ERR range). Valid
      * only if inerror is 1.
@@ -41,10 +43,6 @@ struct quic_tls_st {
      */
     const char *error_msg;
 
-    const char *error_src_file;
-    const char *error_src_func;
-    int error_src_line;
-
     /* Whether our SSL object for TLS has been configured for use in QUIC */
     unsigned int configured : 1;
 
@@ -636,12 +634,18 @@ QUIC_TLS *ossl_quic_tls_new(const QUIC_TLS_ARGS *args)
     if (qtls == NULL)
         return NULL;
 
+    if ((qtls->error_state = OSSL_ERR_STATE_new()) == NULL) {
+        OPENSSL_free(qtls);
+        return NULL;
+    }
+
     qtls->args = *args;
     return qtls;
 }
 
 void ossl_quic_tls_free(QUIC_TLS *qtls)
 {
+    OSSL_ERR_STATE_free(qtls->error_state);
     OPENSSL_free(qtls);
 }
 
@@ -651,12 +655,28 @@ static int raise_error(QUIC_TLS *qtls, uint64_t error_code,
                        int src_line,
                        const char *src_func)
 {
+    /*
+     * When QTLS fails, add a "cover letter" error with information, potentially
+     * with any underlying libssl errors underneath it (but our cover error may
+     * be the only error in some cases). Then capture this into an ERR_STATE so
+     * we can report it later if need be when the QUIC_CHANNEL asks for it.
+     */
+    ERR_new();
+    ERR_set_debug(src_file, src_line, src_func);
+    ERR_set_error(ERR_LIB_SSL, SSL_R_QUIC_HANDSHAKE_LAYER_ERROR,
+                  "handshake layer error, error code %zu (\"%s\")",
+                  error_code, error_msg);
+    OSSL_ERR_STATE_save_to_mark(qtls->error_state);
+
+    /*
+     * We record the error information reported via the QUIC protocol
+     * separately.
+     */
     qtls->error_code        = error_code;
     qtls->error_msg         = error_msg;
-    qtls->error_src_file    = src_file;
-    qtls->error_src_line    = src_line;
-    qtls->error_src_func    = src_func;
     qtls->inerror           = 1;
+
+    ERR_pop_to_mark();
     return 0;
 }
 
@@ -669,7 +689,7 @@ static int raise_error(QUIC_TLS *qtls, uint64_t error_code,
 
 int ossl_quic_tls_tick(QUIC_TLS *qtls)
 {
-    int ret;
+    int ret, err;
     const unsigned char *alpn;
     unsigned int alpnlen;
 
@@ -683,6 +703,29 @@ int ossl_quic_tls_tick(QUIC_TLS *qtls)
     if (qtls->inerror)
         return 0;
 
+    /*
+     * SSL_get_error does not truly know what the cause of an SSL_read failure
+     * is and to some extent guesses based on contextual information. In
+     * particular, if there is _any_ ERR on the error stack, SSL_ERROR_SSL or
+     * SSL_ERROR_SYSCALL will be returned no matter what and there is no
+     * possibility of SSL_ERROR_WANT_READ/WRITE being returned, even if that was
+     * the actual cause of the SSL_read() failure.
+     *
+     * This means that ordinarily, the below code might not work right if the
+     * application has any ERR on the error stack. In order to make this code
+     * perform correctly regardless of prior ERR state, we use a variant of
+     * SSL_get_error() which ignores the error stack. However, some ERRs are
+     * raised by SSL_read() and actually indicate that something has gone wrong
+     * during the call to SSL_read(). We therefore adopt a strategy of marking
+     * the ERR stack and seeing if any errors get appended during the call to
+     * SSL_read(). If they are, we assume SSL_read() has raised an error and
+     * that we should use normal SSL_get_error() handling.
+     *
+     * NOTE: Ensure all escape paths from this function call
+     * ERR_clear_to_mark(). The RAISE macros handle this in failure cases.
+     */
+    ERR_set_mark();
+
     if (!qtls->configured) {
         SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(qtls->args.s);
         SSL_CTX *sctx = SSL_CONNECTION_GET_CTX(sc);
@@ -745,11 +788,17 @@ int ossl_quic_tls_tick(QUIC_TLS *qtls)
         ret = SSL_read(qtls->args.s, NULL, 0);
     else
         ret = SSL_do_handshake(qtls->args.s);
+
     if (ret <= 0) {
-        switch (SSL_get_error(qtls->args.s, ret)) {
+        err = ossl_ssl_get_error(qtls->args.s, ret,
+                                 /*check_err=*/ERR_count_to_mark() > 0);
+
+        switch (err) {
         case SSL_ERROR_WANT_READ:
         case SSL_ERROR_WANT_WRITE:
+            ERR_pop_to_mark();
             return 1;
+
         default:
             return RAISE_INTERNAL_ERROR(qtls);
         }
@@ -763,9 +812,11 @@ int ossl_quic_tls_tick(QUIC_TLS *qtls)
                                "no application protocol negotiated");
 
         qtls->complete = 1;
+        ERR_pop_to_mark();
         return qtls->args.handshake_complete_cb(qtls->args.handshake_complete_cb_arg);
     }
 
+    ERR_pop_to_mark();
     return 1;
 }
 
@@ -781,16 +832,12 @@ 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)
 {
     if (qtls->inerror) {
         *error_code     = qtls->error_code;
         *error_msg      = qtls->error_msg;
-        *error_src_file = qtls->error_src_file;
-        *error_src_line = qtls->error_src_line;
-        *error_src_func = qtls->error_src_func;
+        *error_state    = qtls->error_state;
     }
 
     return qtls->inerror;