Fix errors on SSL_accept() and SSL_get_error()

Calling SSL_accept() was raising two errors on the stack if you passed
the wrong object type. Similarly SSL_get_error() was adding an error to
the stack if the wrong object type was passed and returning the wrong
result.

We also ensure SSL_set_accept_state() and SSL_set_connect_state() don't
raise spurious errors since these are void functions.

Fixes #27347
Fixes #27348

Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27351)

(cherry picked from commit cb5bb8916fa0e044e6658c8b3db6d7c672cb25fe)

diff --git a/include/internal/quic_ssl.h b/include/internal/quic_ssl.h
index acfeb0d5150529f5999f09107e3d1e22e00987ec..e7fd260ab600061f682fd38741da514dba434a8d 100644 (file)
--- a/include/internal/quic_ssl.h
+++ b/include/internal/quic_ssl.h
@@ -72,8 +72,8 @@ __owur const SSL_CIPHER *ossl_quic_get_cipher(unsigned int u);
 int ossl_quic_renegotiate_check(SSL *ssl, int initok);
 
 int ossl_quic_do_handshake(SSL *s);
-void ossl_quic_set_connect_state(SSL *s);
-void ossl_quic_set_accept_state(SSL *s);
+int ossl_quic_set_connect_state(SSL *s, int raiseerrs);
+int ossl_quic_set_accept_state(SSL *s, int raiseerrs);
 
 __owur int ossl_quic_has_pending(const SSL *s);
 __owur int ossl_quic_handle_events(SSL *s);

diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c
index 847639b89e51b705bb0c18ecb036c8ad6988a6c2..64da2be4afcd2d3ff91b5f08365d6b0af6613ad7 100644 (file)
--- a/ssl/quic/quic_impl.c
+++ b/ssl/quic/quic_impl.c
@@ -186,6 +186,10 @@ static int quic_raise_non_normal_error(QCTX *ctx,
  *      This determines whether SSL_get_error() is updated; the value it returns
  *      is modified only by an I/O call.
  *
+ *   QCTX_NO_ERROR
+ *      Don't raise an error if the object type is wrong. Should not be used in
+ *      conjunction with any flags that may raise errors not related to a wrong
+ *      object type.
  */
 #define QCTX_C              (1U << 0)
 #define QCTX_S              (1U << 1)
@@ -195,6 +199,7 @@ static int quic_raise_non_normal_error(QCTX *ctx,
 #define QCTX_LOCK           (1U << 5)
 #define QCTX_IO             (1U << 6)
 #define QCTX_D              (1U << 7)
+#define QCTX_NO_ERROR       (1U << 8)
 
 /*
  * Called when expect_quic failed. Used to diagnose why such a call failed and
@@ -205,7 +210,9 @@ static int wrong_type(const SSL *s, uint32_t flags)
     const uint32_t mask = QCTX_C | QCTX_S | QCTX_L | QCTX_D;
     int code = ERR_R_UNSUPPORTED;
 
-    if ((flags & mask) == QCTX_D)
+    if ((flags & QCTX_NO_ERROR) != 0)
+        return 1;
+    else if ((flags & mask) == QCTX_D)
         code = SSL_R_DOMAIN_USE_ONLY;
     else if ((flags & mask) == QCTX_L)
         code = SSL_R_LISTENER_USE_ONLY;
@@ -225,7 +232,8 @@ static int wrong_type(const SSL *s, uint32_t flags)
  *
  * After this returns 1, all fields of the passed QCTX are initialised.
  * Returns 0 on failure. This function is intended to be used to provide API
- * semantics and as such, it invokes QUIC_RAISE_NON_NORMAL_ERROR() on failure.
+ * semantics and as such, it invokes QUIC_RAISE_NON_NORMAL_ERROR() on failure
+ * unless the QCTX_NO_ERROR flag is set.
  *
  * The flags argument controls the preconditions and postconditions of this
  * function. See above for the different flags.
@@ -378,6 +386,25 @@ err:
     return ok;
 }
 
+static int is_quic_c(const SSL *s, QCTX *ctx, int raiseerrs)
+{
+    uint32_t flags = QCTX_C;
+
+    if (!raiseerrs)
+        flags |= QCTX_NO_ERROR;
+    return expect_quic_as(s, ctx, flags);
+}
+
+/* Same as expect_quic_cs except that errors are not raised if raiseerrs == 0 */
+static int is_quic_cs(const SSL *s, QCTX *ctx, int raiseerrs)
+{
+    uint32_t flags = QCTX_C | QCTX_S;
+
+    if (!raiseerrs)
+        flags |= QCTX_NO_ERROR;
+    return expect_quic_as(s, ctx, flags);
+}
+
 static int expect_quic_cs(const SSL *s, QCTX *ctx)
 {
     return expect_quic_as(s, ctx, QCTX_C | QCTX_S);
@@ -1672,33 +1699,47 @@ long ossl_quic_ctrl(SSL *s, int cmd, long larg, void *parg)
 }
 
 /* SSL_set_connect_state */
-void ossl_quic_set_connect_state(SSL *s)
+int ossl_quic_set_connect_state(SSL *s, int raiseerrs)
 {
     QCTX ctx;
 
-    if (!expect_quic_cs(s, &ctx))
-        return;
+    if (!is_quic_c(s, &ctx, raiseerrs))
+        return 0;
+
+    if (ctx.qc->as_server_state == 0)
+        return 1;
 
     /* Cannot be changed after handshake started */
-    if (ctx.qc->started || ctx.is_stream)
-        return;
+    if (ctx.qc->started) {
+        if (raiseerrs)
+            QUIC_RAISE_NON_NORMAL_ERROR(NULL, SSL_R_INVALID_COMMAND, NULL);
+        return 0;
+    }
 
     ctx.qc->as_server_state = 0;
+    return 1;
 }
 
 /* SSL_set_accept_state */
-void ossl_quic_set_accept_state(SSL *s)
+int ossl_quic_set_accept_state(SSL *s, int raiseerrs)
 {
     QCTX ctx;
 
-    if (!expect_quic_cs(s, &ctx))
-        return;
+    if (!is_quic_c(s, &ctx, raiseerrs))
+        return 0;
+
+    if (ctx.qc->as_server_state == 1)
+        return 1;
 
     /* Cannot be changed after handshake started */
-    if (ctx.qc->started || ctx.is_stream)
-        return;
+    if (ctx.qc->started) {
+        if (raiseerrs)
+            QUIC_RAISE_NON_NORMAL_ERROR(NULL, SSL_R_INVALID_COMMAND, NULL);
+        return 0;
+    }
 
     ctx.qc->as_server_state = 1;
+    return 1;
 }
 
 /* SSL_do_handshake */
@@ -1983,7 +2024,8 @@ int ossl_quic_do_handshake(SSL *s)
 int ossl_quic_connect(SSL *s)
 {
     /* Ensure we are in connect state (no-op if non-idle). */
-    ossl_quic_set_connect_state(s);
+    if (!ossl_quic_set_connect_state(s, 1))
+        return -1;
 
     /* Begin or continue the handshake */
     return ossl_quic_do_handshake(s);
@@ -1993,7 +2035,8 @@ int ossl_quic_connect(SSL *s)
 int ossl_quic_accept(SSL *s)
 {
     /* Ensure we are in accept state (no-op if non-idle). */
-    ossl_quic_set_accept_state(s);
+    if (!ossl_quic_set_accept_state(s, 1))
+        return -1;
 
     /* Begin or continue the handshake */
     return ossl_quic_do_handshake(s);
@@ -2325,8 +2368,9 @@ int ossl_quic_get_error(const SSL *s, int i)
     QCTX ctx;
     int net_error, last_error;
 
-    if (!expect_quic_cs(s, &ctx))
-        return 0;
+    /* SSL_get_errors() should not raise new errors */
+    if (!is_quic_cs(s, &ctx, 0 /* suppress errors */))
+        return SSL_ERROR_SSL;
 
     qctx_lock(&ctx);
     net_error = ossl_quic_channel_net_error(ctx.qc->ch);

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index a81b16c10974a5d7ac75879ee4ca7ffd655146bb..efc6d66fb9c54a6e738f7a8d4f1575d7af8d3dbe 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -4980,7 +4980,8 @@ void SSL_set_accept_state(SSL *s)
 
 #ifndef OPENSSL_NO_QUIC
     if (IS_QUIC(s)) {
-        ossl_quic_set_accept_state(s);
+        /* We suppress errors because this is a void function */
+        (void)ossl_quic_set_accept_state(s, 0 /* suppress errors */);
         return;
     }
 #endif
@@ -4999,7 +5000,8 @@ void SSL_set_connect_state(SSL *s)
 
 #ifndef OPENSSL_NO_QUIC
     if (IS_QUIC(s)) {
-        ossl_quic_set_connect_state(s);
+        /* We suppress errors because this is a void function */
+        (void)ossl_quic_set_connect_state(s, 0 /* suppress errors */);
         return;
     }
 #endif