Properly handle alerts from the peer in SSL_read.

net/socket/ssl_client_socket_openssl.cc

Index: net/socket/ssl_client_socket_openssl.cc
diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index d8713f28f9e37e3fc48e0d94d38dba708b895380..d81b24b5177cf4618fd98c7938ae245527b5d87c 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -1563,11 +1563,22 @@ int SSLClientSocketOpenSSL::DoPayloadRead() {
 
     if (client_auth_cert_needed_) {
       *next_result = ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
-    } else if (*next_result < 0) {
+    } else if (*next_result <= 0) {
+      // A zero return from SSL_read may mean any of:
+      // - The underlying BIO_read returned 0.
+      // - The peer sent a close_notify.
+      // - Any arbitrary error. https://crbug.com/466303
+      //
+      // TransportReadComplete converts the first to an ERR_CONNECTION_CLOSED
+      // error, so it does not occur. The second and third are distinguished by
+      // SSL_ERROR_ZERO_RETURN.

[Ryan Sleevi, 2015/03/17 01:01:28]

Thanks, I conformed that ssl3_get_record does indeed call OPENSSL_PUT_ERROR, but
causes a zero return. This zero is propogated through ssl3_read_bytes, which
then goes through the TLS_protocol_method and up to the caller of SSL_read.

       pending_read_ssl_error_ = SSL_get_error(ssl_, *next_result);
-      *next_result = MapOpenSSLErrorWithDetails(pending_read_ssl_error_,
-                                                err_tracer,
-                                                &pending_read_error_info_);
+      if (pending_read_ssl_error_ == SSL_ERROR_ZERO_RETURN) {
+        *next_result = 0;
+      } else {
+        *next_result = MapOpenSSLErrorWithDetails(
+            pending_read_ssl_error_, err_tracer, &pending_read_error_info_);

[davidben, 2015/03/11 21:47:18]

This logic is totally incomprehensible and insane. next_result switches from
type ssl_read_return_value_t* to net_return_value_t*. (And, of course,
ssl_read_return_value_t* is a distinct type from ssl_get_error_return_value_t*.)

I'll rewrite it in a follow-up.

+      }
 
       // Many servers do not reliably send a close_notify alert when shutting
       // down a connection, and instead terminate the TCP connection. This is