Condition SSL_ERROR_SYSCALL suppression on 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.
@@ -5275,7 +5279,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,10 +1387,12 @@ 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. Do not consider retryable |rwstate| EOF.
-    if (ssl->s3->rwstate != SSL_ERROR_WANT_READ
-            && ssl->s3->rwstate != SSL_ERROR_WANT_WRITE) {
+    // transport does not participate in the error queue. If
+    // |SSL_MODE_AUTO_RETRY| is unset or we're in a non-retryable state, bubble
+    // up to the caller.
+    if ((ssl->ctx->mode & SSL_MODE_AUTO_RETRY) == 0
+            || (ssl->s3->rwstate != SSL_ERROR_WANT_READ
+                && ssl->s3->rwstate != SSL_ERROR_WANT_WRITE)) {
       return SSL_ERROR_SYSCALL;
     }
   }

ssl/ssl_test.cc

@@ -10458,7 +10458,8 @@ TEST(SSLTest, ErrorSyscallAfterCloseNotify) {
   write_failed = false;
 }
 
-// Test that intermittent inability to read results in retryable error
+// Test that |SSL_MODE_AUTO_RETRY| suppresses failure on (potentially)
+// transient empty reads.
 TEST(SSLTest, IntermittentEmptyRead) {
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
   bssl::UniquePtr<SSL_CTX> server_ctx =
@@ -10490,16 +10491,60 @@ TEST(SSLTest, IntermittentEmptyRead) {
   ASSERT_TRUE(BIO_up_ref(client_rbio.get()));
   SSL_set0_rbio(client.get(), rbio_empty.release());
 
+  // |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);
 
-  // On empty read, client should still want a read so caller will retry
+  // On empty read, client should error out signaling EOF
   uint8_t read_data[] = {0, 0, 0};
   ret = SSL_read(client.get(), read_data, sizeof(read_data));
   EXPECT_EQ(ret, 0);
+  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);
+
+  // Next, setu up the same test with |SSL_MODE_AUTO_RETRY| set
+  client_ctx.reset(SSL_CTX_new(TLS_method()));
+  server_ctx = CreateContextWithTestCertificate(TLS_method());
+  ASSERT_TRUE(client_ctx);
+  ASSERT_TRUE(server_ctx);
+  ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
+                                     server_ctx.get()));
+  rbio_empty.reset(BIO_new(method.get()));
+  ASSERT_TRUE(rbio_empty);
+  BIO_set_flags(rbio_empty.get(), BIO_FLAGS_READ);
+  client_rbio.reset(SSL_get_rbio(client.get()));
+  ASSERT_TRUE(client_rbio);
+  ASSERT_TRUE(BIO_up_ref(client_rbio.get()));
+  SSL_set0_rbio(client.get(), rbio_empty.release());
+
+  // 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);
+
+  // Server writes some data to the client
+  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);
+
+  // On empty read, client should still want a read so caller will retry
+  ret = SSL_read(client.get(), read_data, sizeof(read_data));
+  EXPECT_EQ(ret, 0);
   EXPECT_EQ(SSL_get_error(client.get(), ret), SSL_ERROR_WANT_READ);
 
   // Reset client rbio, read should succeed