From 5c3474ea563ed95bb7c86c08867139613655276b Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Fri, 26 May 2023 15:54:56 +0200 Subject: [PATCH] QUIC err handling: Properly report network errors We return SSL_ERROR_SYSCALL when network error is encountered. Reviewed-by: Paul Dale Reviewed-by: Hugo Landau Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/21087) --- include/internal/quic_channel.h | 5 +++++ ssl/quic/quic_channel.c | 7 +++++++ ssl/quic/quic_channel_local.h | 3 +++ ssl/quic/quic_impl.c | 11 ++++++++++- test/quicapitest.c | 26 ++++++++++++++++++++++---- 5 files changed, 47 insertions(+), 5 deletions(-) diff --git a/include/internal/quic_channel.h b/include/internal/quic_channel.h index 8cc165ee8fd..bfa58185749 100644 --- a/include/internal/quic_channel.h +++ b/include/internal/quic_channel.h @@ -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, diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c index 576782c4f16..0ea0c8110a3 100644 --- a/ssl/quic/quic_channel.c +++ b/ssl/quic/quic_channel.c @@ -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, diff --git a/ssl/quic/quic_channel_local.h b/ssl/quic/quic_channel_local.h index f2c84c450c4..69469780aab 100644 --- a/ssl/quic/quic_channel_local.h +++ b/ssl/quic/quic_channel_local.h @@ -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 diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c index 4866f52562f..c0f65c2138e 100644 --- a/ssl/quic/quic_impl.c +++ b/ssl/quic/quic_impl.c @@ -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; } /* diff --git a/test/quicapitest.c b/test/quicapitest.c index 6d88ee9424a..64b53c87f4c 100644 --- a/test/quicapitest.c +++ b/test/quicapitest.c @@ -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) -- 2.47.2