Implement SSL_MODE_AUTO_RETRY

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

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) {

ssl/ssl_test.cc

@@ -10458,6 +10458,83 @@ TEST(SSLTest, ErrorSyscallAfterCloseNotify) {
   write_failed = false;
 }
 
+static void TestIntermittentEmptyRead(bool auto_retry) {
+  bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
+  bssl::UniquePtr<SSL_CTX> server_ctx =
+      CreateContextWithTestCertificate(TLS_method());
+  ASSERT_TRUE(client_ctx);
+  ASSERT_TRUE(server_ctx);
+  bssl::UniquePtr<SSL> 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<BIO_METHOD> 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<BIO> 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<BIO> 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) {