QUIC APL: Fix locking in XSO code and fix tests

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20765)

diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c
index 2dd5a4f91f5704f2e1f00806c23f1c3eef051227..98f96a770342b4edb7c6428da66adaba5752c242 100644 (file)
--- a/ssl/quic/quic_impl.c
+++ b/ssl/quic/quic_impl.c
@@ -28,6 +28,8 @@ static int quic_do_handshake(QUIC_CONNECTION *qc);
 static void qc_update_reject_policy(QUIC_CONNECTION *qc);
 static void qc_touch_default_xso(QUIC_CONNECTION *qc);
 static void qc_set_default_xso(QUIC_CONNECTION *qc, QUIC_XSO *xso, int touch);
+static SSL *quic_conn_stream_new(QUIC_CONNECTION *qc, uint64_t flags,
+                                 int need_lock);
 
 /*
  * QUIC Front-End I/O API: Common Utilities
@@ -1190,7 +1192,8 @@ static int qc_try_create_default_xso_for_write(QUIC_CONNECTION *qc)
     if (qc->default_stream_mode == SSL_DEFAULT_STREAM_MODE_AUTO_UNI)
         flags |= SSL_STREAM_FLAG_UNI;
 
-    qc_set_default_xso(qc, (QUIC_XSO *)ossl_quic_conn_stream_new(&qc->ssl, flags),
+    qc_set_default_xso(qc, (QUIC_XSO *)quic_conn_stream_new(qc, flags,
+                                                            /*needs_lock=*/0),
                        /*touch=*/0);
     if (qc->default_xso == NULL)
         return QUIC_RAISE_NON_NORMAL_ERROR(qc, ERR_R_INTERNAL_ERROR, NULL);
@@ -1321,41 +1324,55 @@ err:
     return NULL;
 }
 
-QUIC_TAKES_LOCK
-SSL *ossl_quic_conn_stream_new(SSL *s, uint64_t flags)
+/* locking depends on need_lock */
+static SSL *quic_conn_stream_new(QUIC_CONNECTION *qc, uint64_t flags,
+                                 int need_lock)
 {
-    QCTX ctx;
     QUIC_XSO *xso = NULL;
     QUIC_STREAM *qs = NULL;
     int is_uni = ((flags & SSL_STREAM_FLAG_UNI) != 0);
 
-    if (!expect_quic_conn_only(s, &ctx))
-        return NULL;
-
-    quic_lock(ctx.qc);
+    if (need_lock)
+        quic_lock(qc);
 
-    if (ossl_quic_channel_is_term_any(ctx.qc->ch)) {
-        QUIC_RAISE_NON_NORMAL_ERROR(ctx.qc, SSL_R_PROTOCOL_IS_SHUTDOWN, NULL);
+    if (ossl_quic_channel_is_term_any(qc->ch)) {
+        QUIC_RAISE_NON_NORMAL_ERROR(qc, SSL_R_PROTOCOL_IS_SHUTDOWN, NULL);
         goto err;
     }
 
-    qs = ossl_quic_channel_new_stream_local(ctx.qc->ch, is_uni);
+    qs = ossl_quic_channel_new_stream_local(qc->ch, is_uni);
     if (qs == NULL)
         goto err;
 
-    xso = create_xso_from_stream(ctx.qc, qs);
+    xso = create_xso_from_stream(qc, qs);
     if (xso == NULL)
         goto err;
 
-    qc_touch_default_xso(ctx.qc); /* inhibits default XSO */
-    quic_unlock(ctx.qc);
+    qc_touch_default_xso(qc); /* inhibits default XSO */
+    if (need_lock)
+        quic_unlock(qc);
+
     return &xso->ssl;
 
 err:
     OPENSSL_free(xso);
-    ossl_quic_stream_map_release(ossl_quic_channel_get_qsm(ctx.qc->ch), qs);
-    quic_unlock(ctx.qc);
+    ossl_quic_stream_map_release(ossl_quic_channel_get_qsm(qc->ch), qs);
+    if (need_lock)
+        quic_unlock(qc);
+
     return NULL;
+
+}
+
+QUIC_TAKES_LOCK
+SSL *ossl_quic_conn_stream_new(SSL *s, uint64_t flags)
+{
+    QCTX ctx;
+
+    if (!expect_quic_conn_only(s, &ctx))
+        return NULL;
+
+    return quic_conn_stream_new(ctx.qc, flags, /*need_lock=*/1);
 }
 
 /*

diff --git a/test/quicapitest.c b/test/quicapitest.c
index 3ce695e5e65d6decd37e17e4c238b8e8cc8070d4..c796a2aff248e887ced36743cec2f4a8ab025a29 100644 (file)
--- a/test/quicapitest.c
+++ b/test/quicapitest.c
@@ -62,13 +62,12 @@ static int test_quic_write_read(int idx)
             goto end;
     }
 
-    if (!TEST_true(ossl_quic_tserver_stream_new(qtserv, /*is_uni=*/0, &sid))
-        || !TEST_uint64_t_eq(sid, 1)) /* server-initiated, so first SID is 1 */
-        goto end;
+    sid = 0; /* client-initiated bidirectional stream */
 
     for (j = 0; j < 2; j++) {
         /* Check that sending and receiving app data is ok */
-        if (!TEST_true(SSL_write_ex(clientquic, msg, msglen, &numbytes)))
+        if (!TEST_true(SSL_write_ex(clientquic, msg, msglen, &numbytes))
+            || !TEST_size_t_eq(numbytes, msglen))
             goto end;
         if (idx == 1) {
             do {
@@ -86,6 +85,7 @@ static int test_quic_write_read(int idx)
                 goto end;
         }
 
+        ossl_quic_tserver_tick(qtserv);
         if (!TEST_true(ossl_quic_tserver_write(qtserv, sid, (unsigned char *)msg,
                                                msglen, &numbytes)))
             goto end;