QUIC err handling: Properly report network errors
authorTomas Mraz <tomas@openssl.org>
Fri, 26 May 2023 13:54:56 +0000 (15:54 +0200)
committerTomas Mraz <tomas@openssl.org>
Fri, 7 Jul 2023 13:13:29 +0000 (15:13 +0200)
We return SSL_ERROR_SYSCALL when network error is encountered.

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21087)

include/internal/quic_channel.h
ssl/quic/quic_channel.c
ssl/quic/quic_channel_local.h
ssl/quic/quic_impl.c
test/quicapitest.c

index 8cc165ee8fd605429ecc16491772f508b98c782f..bfa581857497788ceae8837cd9c984705400ab7f 100644 (file)
@@ -216,6 +216,11 @@ void ossl_quic_channel_raise_protocol_error(QUIC_CHANNEL *ch,
                                             uint64_t error_code,
                                             uint64_t frame_type,
                                             const char *reason);
+/*
+ * Returns 1 if permanent net error was detected on the QUIC_CHANNEL,
+ * 0 otherwise.
+ */
+int ossl_quic_channel_net_error(QUIC_CHANNEL *ch);
 
 /* For RXDP use. */
 void ossl_quic_channel_on_remote_conn_close(QUIC_CHANNEL *ch,
index 576782c4f164f3f6e8db3c33ec7f5148078e3050..0ea0c8110a3e4e035c2a3047011797bf39512782 100644 (file)
@@ -2558,6 +2558,8 @@ static void ch_raise_net_error(QUIC_CHANNEL *ch)
 {
     QUIC_TERMINATE_CAUSE tcause = {0};
 
+    ch->net_error = 1;
+
     tcause.error_code = QUIC_ERR_INTERNAL_ERROR;
 
     /*
@@ -2567,6 +2569,11 @@ static void ch_raise_net_error(QUIC_CHANNEL *ch)
     ch_start_terminating(ch, &tcause, 1);
 }
 
+int ossl_quic_channel_net_error(QUIC_CHANNEL *ch)
+{
+    return ch->net_error;
+}
+
 void ossl_quic_channel_raise_protocol_error(QUIC_CHANNEL *ch,
                                             uint64_t error_code,
                                             uint64_t frame_type,
index f2c84c450c4ec18d255c0e268dcedf6e5455bc07..69469780aabdb50182100a1d009e1b892555e127 100644 (file)
@@ -399,6 +399,9 @@ struct quic_channel_st {
      * If set, RXKU is expected (because we initiated a spontaneous TXKU).
      */
     unsigned int                    rxku_expected                       : 1;
+
+    /* Permanent net error encountered */
+    unsigned int                    net_error                           : 1;
 };
 
 # endif
index 4866f52562fad5dcef8637af41ac2a93d6258376..c0f65c2138e7d55994c34c74aaadee8f1e2a77b5 100644 (file)
@@ -1664,11 +1664,20 @@ SSL *ossl_quic_conn_stream_new(SSL *s, uint64_t flags)
 int ossl_quic_get_error(const SSL *s, int i)
 {
     QCTX ctx;
+    int net_error, last_error;
 
     if (!expect_quic(s, &ctx))
         return 0;
 
-    return ctx.is_stream ? ctx.xso->last_error : ctx.qc->last_error;
+    quic_lock(ctx.qc);
+    net_error = ossl_quic_channel_net_error(ctx.qc->ch);
+    last_error = ctx.is_stream ? ctx.xso->last_error : ctx.qc->last_error;
+    quic_unlock(ctx.qc);
+
+    if (net_error)
+        return SSL_ERROR_SYSCALL;
+
+    return last_error;
 }
 
 /*
index 6d88ee9424a96dfe35eadd42e9789c10e126deae..64b53c87f4c96d202319d2ba2b405a50324a9179 100644 (file)
@@ -40,6 +40,7 @@ static int is_fips = 0;
  * Test that we read what we've written.
  * Test 0: Non-blocking
  * Test 1: Blocking
+ * Test 2: Blocking, introduce socket error, test error handling.
  */
 static int test_quic_write_read(int idx)
 {
@@ -54,7 +55,7 @@ static int test_quic_write_read(int idx)
     int ssock = 0, csock = 0;
     uint64_t sid = UINT64_MAX;
 
-    if (idx == 1 && !qtest_supports_blocking())
+    if (idx >= 1 && !qtest_supports_blocking())
         return TEST_skip("Blocking tests not supported in this build");
 
     if (!TEST_ptr(cctx)
@@ -65,7 +66,7 @@ static int test_quic_write_read(int idx)
             || !TEST_true(qtest_create_quic_connection(qtserv, clientquic)))
         goto end;
 
-    if (idx == 1) {
+    if (idx >= 1) {
         if (!TEST_true(BIO_get_fd(ossl_quic_tserver_get0_rbio(qtserv), &ssock)))
             goto end;
         if (!TEST_int_gt(csock = SSL_get_rfd(clientquic), 0))
@@ -79,7 +80,7 @@ static int test_quic_write_read(int idx)
         if (!TEST_true(SSL_write_ex(clientquic, msg, msglen, &numbytes))
             || !TEST_size_t_eq(numbytes, msglen))
             goto end;
-        if (idx == 1) {
+        if (idx >= 1) {
             do {
                 if (!TEST_true(wait_until_sock_readable(ssock)))
                     goto end;
@@ -95,12 +96,29 @@ static int test_quic_write_read(int idx)
                 goto end;
         }
 
+        if (idx >= 2 && j > 0)
+            /* Introduce permanent socket error */
+            BIO_closesocket(csock);
+
         ossl_quic_tserver_tick(qtserv);
         if (!TEST_true(ossl_quic_tserver_write(qtserv, sid, (unsigned char *)msg,
                                                msglen, &numbytes)))
             goto end;
         ossl_quic_tserver_tick(qtserv);
         SSL_handle_events(clientquic);
+
+        if (idx >= 2 && j > 0) {
+            if (!TEST_false(SSL_read_ex(clientquic, buf, 1, &numbytes))
+                    || !TEST_int_eq(SSL_get_error(clientquic, 0),
+                                    SSL_ERROR_SYSCALL)
+                    || !TEST_false(SSL_write_ex(clientquic, msg, msglen,
+                                                &numbytes))
+                    || !TEST_int_eq(SSL_get_error(clientquic, 0),
+                                    SSL_ERROR_SYSCALL))
+                goto end;
+            break;
+        }
+
         /*
          * In blocking mode the SSL_read_ex call will block until the socket is
          * readable and has our data. In non-blocking mode we're doing everything
@@ -641,7 +659,7 @@ int setup_tests(void)
     if (privkey == NULL)
         goto err;
 
-    ADD_ALL_TESTS(test_quic_write_read, 2);
+    ADD_ALL_TESTS(test_quic_write_read, 3);
     ADD_TEST(test_ciphersuites);
     ADD_TEST(test_version);
 #if defined(DO_SSL_TRACE_TEST)