Reinstate guard, abandon SSL_MODE_AUTO_RETRY

Author: WillChilds-Klein

include/openssl/ssl.h

@@ -869,9 +869,6 @@ OPENSSL_EXPORT uint32_t SSL_get_options(const SSL *ssl);
 // session resumption is used for a given SSL*.
 #define SSL_MODE_NO_SESSION_CREATION 0x00000200L
 
-// TODO [childw]
-#define SSL_MODE_AUTO_RETRY 0x00000004L
-
 // SSL_MODE_SEND_FALLBACK_SCSV sends TLS_FALLBACK_SCSV in the ClientHello.
 // To be set only by applications that reconnect with a downgraded protocol
 // version; see RFC 7507 for details.
@@ -5278,6 +5275,7 @@ 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

@@ -1027,10 +1027,7 @@ static int ssl_read_impl(SSL *ssl) {
                                  &alert, ssl->s3->read_buffer.span());
     bool retry;
     int bio_ret = ssl_handle_open_record(ssl, &retry, ret, consumed, alert);
-
-    if (bio_ret == 0 && retry && (ssl->ctx->mode & SSL_MODE_AUTO_RETRY)) {
-      continue;
-    } else if (bio_ret <= 0) {
+    if (bio_ret <= 0) {
       return bio_ret;
     }
     if (!retry) {
@@ -1391,8 +1388,11 @@ int SSL_get_error(const SSL *ssl, int ret_code) {
     }
     // 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;
+    // caller. Do not consider retryable |rwstate| EOF.
+    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

@@ -10491,26 +10491,28 @@ TEST(SSLTest, IntermittentEmptyRead) {
   SSL_set0_rbio(client.get(), rbio_empty.release());
 
   // Server writes some data to the client
-  const uint8_t data[1] = {0};
-  int ret = SSL_write(server.get(), data, (int) sizeof(data));
-  EXPECT_EQ(ret, (int) sizeof(data));
+  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
-  uint8_t buf[1];
-  ret = SSL_read(client.get(), buf, sizeof(buf));
+  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_WANT_READ);
 
   // Reset client rbio, read should succeed
   SSL_set0_rbio(client.get(), client_rbio.release());
-  ret = SSL_read(client.get(), buf, sizeof(buf));
-  EXPECT_EQ(ret, (int) sizeof(buf));
+  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);
 
-  ret = SSL_read(client.get(), buf, sizeof(buf));
-  EXPECT_LE(ret, 0);
-  EXPECT_EQ(SSL_get_error(client.get(), ret), SSL_ERROR_SSL);
+  // 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_shutdown|, when quiet shutdown is enabled, simulates receiving