QUIC APL: Revise I/O error setting so that the last error is set on success
authorHugo Landau <hlandau@openssl.org>
Thu, 31 Aug 2023 10:53:07 +0000 (11:53 +0100)
committerHugo Landau <hlandau@openssl.org>
Fri, 1 Sep 2023 13:44:47 +0000 (14:44 +0100)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21915)

ssl/quic/quic_impl.c

index 5dd73fd19b8bd0d5ff06b24c0c4f6e5f84ee16bb..2f60594efa7ac404c481d661c9c38dcd79a3e5bb 100644 (file)
@@ -26,6 +26,7 @@ static int qc_try_create_default_xso_for_write(QCTX *ctx);
 static int qc_wait_for_default_xso_for_read(QCTX *ctx);
 static void quic_lock(QUIC_CONNECTION *qc);
 static void quic_unlock(QUIC_CONNECTION *qc);
+static void quic_lock_for_io(QCTX *ctx);
 static int quic_do_handshake(QCTX *ctx);
 static void qc_update_reject_policy(QUIC_CONNECTION *qc);
 static void qc_touch_default_xso(QUIC_CONNECTION *qc);
@@ -109,6 +110,18 @@ struct qctx_st {
     int             is_stream, in_io;
 };
 
+QUIC_NEEDS_LOCK
+static void quic_set_last_error(QCTX *ctx, int last_error)
+{
+    if (!ctx->in_io)
+        return;
+
+    if (ctx->is_stream && ctx->xso != NULL)
+        ctx->xso->last_error = last_error;
+    else if (!ctx->is_stream && ctx->qc != NULL)
+        ctx->qc->last_error = last_error;
+}
+
 /*
  * Raise a 'normal' error, meaning one that can be reported via SSL_get_error()
  * rather than via ERR. Note that normal errors must always be raised while
@@ -118,10 +131,8 @@ QUIC_NEEDS_LOCK
 static int quic_raise_normal_error(QCTX *ctx,
                                    int err)
 {
-    if (ctx->is_stream)
-        ctx->xso->last_error = err;
-    else
-        ctx->qc->last_error = err;
+    assert(ctx->in_io);
+    quic_set_last_error(ctx, err);
 
     return 0;
 }
@@ -148,10 +159,7 @@ static int quic_raise_non_normal_error(QCTX *ctx,
     va_list args;
 
     if (ctx != NULL) {
-        if (ctx->in_io && ctx->is_stream && ctx->xso != NULL)
-            ctx->xso->last_error = SSL_ERROR_SSL;
-        else if (ctx->in_io && !ctx->is_stream && ctx->qc != NULL)
-            ctx->qc->last_error = SSL_ERROR_SSL;
+        quic_set_last_error(ctx, SSL_ERROR_SSL);
 
         if (reason == SSL_R_PROTOCOL_IS_SHUTDOWN && ctx->qc != NULL)
             ossl_quic_channel_restore_err_state(ctx->qc->ch);
@@ -236,8 +244,10 @@ static int ossl_unused expect_quic_with_stream_lock(const SSL *s, int remote_ini
     if (!expect_quic(s, ctx))
         return 0;
 
-    ctx->in_io = in_io;
-    quic_lock(ctx->qc);
+    if (in_io)
+        quic_lock_for_io(ctx);
+    else
+        quic_lock(ctx->qc);
 
     if (ctx->xso == NULL && remote_init >= 0) {
         if (!quic_mutation_allowed(ctx->qc, /*req_active=*/0)) {
@@ -301,6 +311,20 @@ static void quic_lock(QUIC_CONNECTION *qc)
 #endif
 }
 
+static void quic_lock_for_io(QCTX *ctx)
+{
+    quic_lock(ctx->qc);
+    ctx->in_io = 1;
+
+    /*
+     * We are entering an I/O function so we must update the values returned by
+     * SSL_get_error and SSL_want. Set no error. This will be overridden later
+     * if a call to QUIC_RAISE_NORMAL_ERROR or QUIC_RAISE_NON_NORMAL_ERROR
+     * occurs during the API call.
+     */
+    quic_set_last_error(ctx, SSL_ERROR_NONE);
+}
+
 /* Precondition: Channel mutex is held (unchecked) */
 QUIC_NEEDS_LOCK
 static void quic_unlock(QUIC_CONNECTION *qc)
@@ -1677,8 +1701,7 @@ int ossl_quic_do_handshake(SSL *s)
     if (!expect_quic(s, &ctx))
         return 0;
 
-    ctx.in_io = 1;
-    quic_lock(ctx.qc);
+    quic_lock_for_io(&ctx);
 
     ret = quic_do_handshake(&ctx);
     quic_unlock(ctx.qc);
@@ -2526,8 +2549,7 @@ static int quic_read(SSL *s, void *buf, size_t len, size_t *bytes_read, int peek
     if (!expect_quic(s, &ctx))
         return 0;
 
-    ctx.in_io = 1;
-    quic_lock(ctx.qc);
+    quic_lock_for_io(&ctx);
 
     if (!quic_mutation_allowed(ctx.qc, /*req_active=*/0)) {
         ret = QUIC_RAISE_NON_NORMAL_ERROR(&ctx, SSL_R_PROTOCOL_IS_SHUTDOWN, NULL);