diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 4bdd0df256..c4f1506b99 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -834,6 +834,10 @@ OPENSSL_EXPORT uint32_t SSL_get_options(const SSL *ssl); // |write|. In DTLS, it does nothing. #define SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER 0x00000002L +// SSL_MODE_AUTO_RETRY suppresses terminal errors on empty reads if the +// underlying connection state is retryable, allowing for automatic retries. +#define SSL_MODE_AUTO_RETRY 0x00000004L + // SSL_MODE_NO_AUTO_CHAIN disables automatically building a certificate chain // before sending certificates to the peer. This flag is set (and the feature // disabled) by default. @@ -5283,7 +5287,6 @@ DEFINE_STACK_OF(SSL_COMP) // The following flags do nothing and are included only to make it easier to // compile code with BoringSSL. -#define SSL_MODE_AUTO_RETRY 0 #define SSL_MODE_RELEASE_BUFFERS 0 #define SSL_MODE_SEND_CLIENTHELLO_TIME 0 #define SSL_MODE_SEND_SERVERHELLO_TIME 0 diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index 2560e6804b..9ca5533bcc 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc @@ -1387,9 +1387,16 @@ int SSL_get_error(const SSL *ssl, int ret_code) { return SSL_ERROR_ZERO_RETURN; } // An EOF was observed which violates the protocol, and the underlying - // transport does not participate in the error queue. Bubble up to the - // caller. - return SSL_ERROR_SYSCALL; + // transport does not participate in the error queue. If + // |SSL_MODE_AUTO_RETRY| is unset, bubble up to the caller. + if ((ssl->ctx->mode & SSL_MODE_AUTO_RETRY) == 0) { + return SSL_ERROR_SYSCALL; + } + // If |SSL_MODE_AUTO_RETRY| is set, proceed if in a retryable state. + if (ssl->s3->rwstate != SSL_ERROR_WANT_READ + && ssl->s3->rwstate != SSL_ERROR_WANT_WRITE) { + return SSL_ERROR_SYSCALL; + } } switch (ssl->s3->rwstate) { diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 470333971a..a1c934579a 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -10458,6 +10458,83 @@ TEST(SSLTest, ErrorSyscallAfterCloseNotify) { write_failed = false; } +static void TestIntermittentEmptyRead(bool auto_retry) { + bssl::UniquePtr client_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr server_ctx = + CreateContextWithTestCertificate(TLS_method()); + ASSERT_TRUE(client_ctx); + ASSERT_TRUE(server_ctx); + bssl::UniquePtr client, server; + ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(), + server_ctx.get())); + + // Create a fake read BIO that returns 0 on read to simulate empty read + bssl::UniquePtr method(BIO_meth_new(0, nullptr)); + ASSERT_TRUE(method); + ASSERT_TRUE(BIO_meth_set_create(method.get(), [](BIO *b) -> int { + BIO_set_init(b, 1); + return 1; + })); + ASSERT_TRUE(BIO_meth_set_read(method.get(), [](BIO *, char *, int) -> int { + return 0; + })); + bssl::UniquePtr rbio_empty(BIO_new(method.get())); + ASSERT_TRUE(rbio_empty); + BIO_set_flags(rbio_empty.get(), BIO_FLAGS_READ); + + // Save off client rbio and use empty read BIO + bssl::UniquePtr client_rbio(SSL_get_rbio(client.get())); + ASSERT_TRUE(client_rbio); + // Up-ref |client_rbio| as SSL_CTX dtor will also attempt to free it + ASSERT_TRUE(BIO_up_ref(client_rbio.get())); + SSL_set0_rbio(client.get(), rbio_empty.release()); + + if (auto_retry) { + // Set flag under test + ASSERT_TRUE(SSL_CTX_set_mode(client_ctx.get(), SSL_MODE_AUTO_RETRY)); + ASSERT_TRUE(SSL_CTX_get_mode(client_ctx.get()) & SSL_MODE_AUTO_RETRY); + } else { + // |SSL_MODE_AUTO_RETRY| is off by default + ASSERT_FALSE(SSL_CTX_get_mode(client_ctx.get()) & SSL_MODE_AUTO_RETRY); + } + + // Server writes some data to the client + const uint8_t write_data[] = {1, 2, 3}; + int ret = SSL_write(server.get(), write_data, (int) sizeof(write_data)); + EXPECT_EQ(ret, (int) sizeof(write_data)); + EXPECT_EQ(SSL_get_error(server.get(), ret), SSL_ERROR_NONE); + + uint8_t read_data[] = {0, 0, 0}; + ret = SSL_read(client.get(), read_data, sizeof(read_data)); + EXPECT_EQ(ret, 0); + if (auto_retry) { + // On empty read, client should still want a read so caller will retry + EXPECT_EQ(SSL_get_error(client.get(), ret), SSL_ERROR_WANT_READ); + } else { + // On empty read, client should error out signaling EOF + EXPECT_EQ(SSL_get_error(client.get(), ret), SSL_ERROR_SYSCALL); + } + + // Reset client rbio, read should succeed + SSL_set0_rbio(client.get(), client_rbio.release()); + ret = SSL_read(client.get(), read_data, sizeof(read_data)); + EXPECT_EQ(ret, (int) sizeof(write_data)); + EXPECT_EQ(OPENSSL_memcmp(read_data, write_data, sizeof(write_data)), 0); + EXPECT_EQ(SSL_get_error(client.get(), ret), SSL_ERROR_NONE); + + // Subsequent attempts to read should fail + ret = SSL_read(client.get(), read_data, sizeof(read_data)); + EXPECT_LT(ret, 0); + EXPECT_EQ(SSL_get_error(client.get(), ret), SSL_ERROR_WANT_READ); +} + +// Test that |SSL_MODE_AUTO_RETRY| suppresses failure on (potentially) +// transient empty reads. +TEST(SSLTest, IntermittentEmptyRead) { + TestIntermittentEmptyRead(false); + TestIntermittentEmptyRead(true); +} + // Test that |SSL_shutdown|, when quiet shutdown is enabled, simulates receiving // a close_notify, down to |SSL_read| reporting |SSL_ERROR_ZERO_RETURN|. TEST(SSLTest, QuietShutdown) {