Preserve transport errors for OpenSSL sockets.

net/socket/ssl_client_socket_openssl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // OpenSSL binding for SSLClientSocket. The class layout and general principle
 // of operation is derived from SSLClientSocketNSS.
 
 #include "net/socket/ssl_client_socket_openssl.h"
 
 #include <openssl/err.h>
@@ skipping 321 matching lines @@
 #endif
 }
 
 SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
     scoped_ptr<ClientSocketHandle> transport_socket,
     const HostPortPair& host_and_port,
     const SSLConfig& ssl_config,
     const SSLClientSocketContext& context)
     : transport_send_busy_(false),
       transport_recv_busy_(false),
-      transport_recv_eof_(false),
       weak_factory_(this),
       pending_read_error_(kNoPendingReadResult),
-      transport_write_error_(OK),
+      transport_read_error_(ERR_IO_PENDING),
+      transport_write_error_(ERR_IO_PENDING),
       server_cert_chain_(new PeerCertificateChain(NULL)),
       completed_handshake_(false),
       was_ever_used_(false),
       client_auth_cert_needed_(false),
       cert_verifier_(context.cert_verifier),
       server_bound_cert_service_(context.server_bound_cert_service),
       ssl_(NULL),
       transport_bio_(NULL),
       transport_(transport_socket.Pass()),
       host_and_port_(host_and_port),
@@ skipping 38 matching lines @@
   int rv = SSL_export_keying_material(
       ssl_, out, outlen, label.data(), label.size(),
       reinterpret_cast<const unsigned char*>(context.data()),
       context.length(), context.length() > 0);
 
   if (rv != 1) {
     int ssl_error = SSL_get_error(ssl_, rv);
     LOG(ERROR) << "Failed to export keying material;"
                << " returned " << rv
                << ", SSL error code " << ssl_error;
-    return MapOpenSSLError(ssl_error, err_tracer);
+    return HandleOpenSSLError(ssl_error, err_tracer);
   }
   return OK;
 }
 
 int SSLClientSocketOpenSSL::GetTLSUniqueChannelBinding(std::string* out) {
   NOTIMPLEMENTED();
   return ERR_NOT_IMPLEMENTED;
 }
 
 int SSLClientSocketOpenSSL::Connect(const CompletionCallback& callback) {
@@ skipping 34 matching lines @@
   }
 
   // Shut down anything that may call us back.
   verifier_.reset();
   transport_->socket()->Disconnect();
 
   // Null all callbacks, delete all buffers.
   transport_send_busy_ = false;
   send_buffer_ = NULL;
   transport_recv_busy_ = false;
-  transport_recv_eof_ = false;
   recv_buffer_ = NULL;
 
   user_connect_callback_.Reset();
   user_read_callback_.Reset();
   user_write_callback_.Reset();
   user_read_buf_         = NULL;
   user_read_buf_len_     = 0;
   user_write_buf_        = NULL;
   user_write_buf_len_    = 0;
 
   pending_read_error_ = kNoPendingReadResult;
-  transport_write_error_ = OK;
+  transport_read_error_ = ERR_IO_PENDING;
+  transport_write_error_ = ERR_IO_PENDING;
 
   server_cert_verify_result_.Reset();
   completed_handshake_ = false;
 
   cert_authorities_.clear();
   cert_key_types_.clear();
   client_auth_cert_needed_ = false;
 }
 
 bool SSLClientSocketOpenSSL::IsConnected() const {
@@ skipping 112 matching lines @@
       << SSLConnectionStatusToVersion(ssl_info->connection_status);
   return true;
 }
 
 int SSLClientSocketOpenSSL::Read(IOBuffer* buf,
                                  int buf_len,
                                  const CompletionCallback& callback) {
   user_read_buf_ = buf;
   user_read_buf_len_ = buf_len;
 
-  int rv = DoReadLoop(OK);
+  int rv = DoReadLoop();
 
   if (rv == ERR_IO_PENDING) {
     user_read_callback_ = callback;
   } else {
     if (rv > 0)
       was_ever_used_ = true;
     user_read_buf_ = NULL;
     user_read_buf_len_ = 0;
   }
 
   return rv;
 }
 
 int SSLClientSocketOpenSSL::Write(IOBuffer* buf,
                                   int buf_len,
                                   const CompletionCallback& callback) {
   user_write_buf_ = buf;
   user_write_buf_len_ = buf_len;
 
-  int rv = DoWriteLoop(OK);
+  int rv = DoWriteLoop();
 
   if (rv == ERR_IO_PENDING) {
     user_write_callback_ = callback;
   } else {
     if (rv > 0)
       was_ever_used_ = true;
     user_write_buf_ = NULL;
     user_write_buf_len_ = 0;
   }
 
@@ skipping 25 matching lines @@
   trying_cached_session_ = context->session_cache()->SetSSLSessionWithKey(
       ssl_, GetSocketSessionCacheKey(*this));
 
   BIO* ssl_bio = NULL;
   // 0 => use default buffer sizes.
   if (!BIO_new_bio_pair(&ssl_bio, 0, &transport_bio_, 0))
     return ERR_UNEXPECTED;
   DCHECK(ssl_bio);
   DCHECK(transport_bio_);
 
+  // Install a callback on OpenSSL's end to plumb transport errors through.
+  BIO_set_callback(ssl_bio, &SSLClientSocketOpenSSL::BIOCallbackThunk);
+  BIO_set_callback_arg(ssl_bio, reinterpret_cast<char*>(this));
+
   SSL_set_bio(ssl_, ssl_bio, ssl_bio);
 
   // OpenSSL defaults some options to on, others to off. To avoid ambiguity,
   // set everything we care about to an absolute value.
   SslSetClearMask options;
   options.ConfigureFlag(SSL_OP_NO_SSLv2, true);
   bool ssl3_enabled = (ssl_config_.version_min == SSL_PROTOCOL_VERSION_SSL3);
   options.ConfigureFlag(SSL_OP_NO_SSLv3, !ssl3_enabled);
   bool tls1_enabled = (ssl_config_.version_min <= SSL_PROTOCOL_VERSION_TLS1 &&
                        ssl_config_.version_max >= SSL_PROTOCOL_VERSION_TLS1);
@@ skipping 98 matching lines @@
 bool SSLClientSocketOpenSSL::DoTransportIO() {
   bool network_moved = false;
   int rv;
   // Read and write as much data as possible. The loop is necessary because
   // Write() may return synchronously.
   do {
     rv = BufferSend();
     if (rv != ERR_IO_PENDING && rv != 0)
       network_moved = true;
   } while (rv > 0);
-  if (!transport_recv_eof_ && BufferRecv() != ERR_IO_PENDING)
+  if (transport_read_error_ == ERR_IO_PENDING &&
+      BufferRecv() != ERR_IO_PENDING) {
     network_moved = true;
+  }
   return network_moved;
 }
 
 int SSLClientSocketOpenSSL::DoHandshake() {
   crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
   int net_error = OK;
   int rv = SSL_do_handshake(ssl_);
 
   if (client_auth_cert_needed_) {
     net_error = ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
@@ skipping 24 matching lines @@
                    base::Unretained(server_cert_.get())));
     GotoState(STATE_VERIFY_CERT);
   } else {
     int ssl_error = SSL_get_error(ssl_, rv);
 
     if (ssl_error == SSL_ERROR_WANT_CHANNEL_ID_LOOKUP) {
       // The server supports TLS channel id and the lookup is asynchronous.
       // Retrieve the error from the call to |server_bound_cert_service_|.
       net_error = channel_id_request_return_value_;
     } else {
-      net_error = MapOpenSSLError(ssl_error, err_tracer);
+      net_error = HandleOpenSSLError(ssl_error, err_tracer);
     }
 
     // If not done, stay in this state
     if (net_error == ERR_IO_PENDING) {
       GotoState(STATE_HANDSHAKE);
     } else {
       LOG(ERROR) << "handshake failed; returned " << rv
                  << ", SSL error code " << ssl_error
                  << ", net_error " << net_error;
       net_log_.AddEvent(
@@ skipping 122 matching lines @@
     // In handshake phase.
     OnHandshakeIOComplete(result);
     return;
   }
 
   // Network layer received some data, check if client requested to read
   // decrypted data.
   if (!user_read_buf_.get())
     return;
 
-  int rv = DoReadLoop(result);
+  int rv = DoReadLoop();
   if (rv != ERR_IO_PENDING)
     DoReadCallback(rv);
 }
 
 int SSLClientSocketOpenSSL::DoHandshakeLoop(int last_io_result) {
   int rv = last_io_result;
   do {
     // Default to STATE_NONE for next state.
     // (This is a quirk carried over from the windows
     // implementation.  It makes reading the logs a bit harder.)
@@ skipping 23 matching lines @@
     if (network_moved && next_handshake_state_ == STATE_HANDSHAKE) {
       // In general we exit the loop if rv is ERR_IO_PENDING.  In this
       // special case we keep looping even if rv is ERR_IO_PENDING because
       // the transport IO may allow DoHandshake to make progress.
       rv = OK;  // This causes us to stay in the loop.
     }
   } while (rv != ERR_IO_PENDING && next_handshake_state_ != STATE_NONE);
   return rv;
 }
 
-int SSLClientSocketOpenSSL::DoReadLoop(int result) {
-  if (result < 0)
-    return result;
-
+int SSLClientSocketOpenSSL::DoReadLoop() {
   bool network_moved;
   int rv;
   do {
     rv = DoPayloadRead();
     network_moved = DoTransportIO();
   } while (rv == ERR_IO_PENDING && network_moved);
 
   return rv;
 }
 
-int SSLClientSocketOpenSSL::DoWriteLoop(int result) {
-  if (result < 0)
-    return result;
-
+int SSLClientSocketOpenSSL::DoWriteLoop() {
   bool network_moved;
   int rv;
   do {
     rv = DoPayloadWrite();
     network_moved = DoTransportIO();
   } while (rv == ERR_IO_PENDING && network_moved);
 
   return rv;
 }
 
@@ skipping 36 matching lines @@
     if (total_bytes_read > 0) {
       pending_read_error_ = rv;
       rv = total_bytes_read;
       next_result = &pending_read_error_;
     }
 
     if (client_auth_cert_needed_) {
       *next_result = ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
     } else if (*next_result < 0) {
       int err = SSL_get_error(ssl_, *next_result);
-      *next_result = MapOpenSSLError(err, err_tracer);
+      *next_result = HandleOpenSSLError(err, err_tracer);
       if (rv > 0 && *next_result == ERR_IO_PENDING) {
-          // If at least some data was read from SSL_read(), do not treat
-          // insufficient data as an error to return in the next call to
-          // DoPayloadRead() - instead, let the call fall through to check
-          // SSL_read() again. This is because DoTransportIO() may complete
-          // in between the next call to DoPayloadRead(), and thus it is
-          // important to check SSL_read() on subsequent invocations to see
-          // if a complete record may now be read.
+        // If at least some data was read from SSL_read(), do not treat
+        // insufficient data as an error to return in the next call to
+        // DoPayloadRead() - instead, let the call fall through to check
+        // SSL_read() again. This is because DoTransportIO() may complete
+        // in between the next call to DoPayloadRead(), and thus it is
+        // important to check SSL_read() on subsequent invocations to see
+        // if a complete record may now be read.
         *next_result = kNoPendingReadResult;
       }
     }
   }
 
   if (rv >= 0) {
     net_log_.AddByteTransferEvent(NetLog::TYPE_SSL_SOCKET_BYTES_RECEIVED, rv,
                                   user_read_buf_->data());
   }
   return rv;
 }
 
 int SSLClientSocketOpenSSL::DoPayloadWrite() {
   crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
   int rv = SSL_write(ssl_, user_write_buf_->data(), user_write_buf_len_);
 
   if (rv >= 0) {
     net_log_.AddByteTransferEvent(NetLog::TYPE_SSL_SOCKET_BYTES_SENT, rv,
                                   user_write_buf_->data());
     return rv;
   }
 
   int err = SSL_get_error(ssl_, rv);
-  return MapOpenSSLError(err, err_tracer);
+  return HandleOpenSSLError(err, err_tracer);
 }
 
 int SSLClientSocketOpenSSL::BufferSend(void) {
   if (transport_send_busy_)
     return ERR_IO_PENDING;
 
   if (!send_buffer_.get()) {
     // Get a fresh send buffer out of the send BIO.
     size_t max_read = BIO_ctrl_pending(transport_bio_);
     if (!max_read)
@@ skipping 64 matching lines @@
 }
 
 void SSLClientSocketOpenSSL::BufferRecvComplete(int result) {
   result = TransportReadComplete(result);
   OnRecvComplete(result);
 }
 
 void SSLClientSocketOpenSSL::TransportWriteComplete(int result) {
   DCHECK(ERR_IO_PENDING != result);
   if (result < 0) {
-    // Got a socket write error; close the BIO to indicate this upward.
-    //
-    // TODO(davidben): The value of |result| gets lost. Feed the error back into
-    // the BIO so it gets (re-)detected in OnSendComplete. Perhaps with
-    // BIO_set_callback.
-    DVLOG(1) << "TransportWriteComplete error " << result;
-    (void)BIO_shutdown_wr(SSL_get_wbio(ssl_));
-
-    // Match the fix for http://crbug.com/249848 in NSS by erroring future reads
-    // from the socket after a write error.
-    //
-    // TODO(davidben): Avoid having read and write ends interact this way.
+    // Record the error. Save it to be reported in a future read or write on
+    // transport_bio_'s peer.
     transport_write_error_ = result;
-    (void)BIO_shutdown_wr(transport_bio_);
     send_buffer_ = NULL;
   } else {
     DCHECK(send_buffer_.get());
     send_buffer_->DidConsume(result);
     DCHECK_GE(send_buffer_->BytesRemaining(), 0);
     if (send_buffer_->BytesRemaining() <= 0)
       send_buffer_ = NULL;
   }
 }
 
 int SSLClientSocketOpenSSL::TransportReadComplete(int result) {
   DCHECK(ERR_IO_PENDING != result);
   if (result <= 0) {
     DVLOG(1) << "TransportReadComplete result " << result;
-    // Received 0 (end of file) or an error. Either way, bubble it up to the
-    // SSL layer via the BIO. TODO(joth): consider stashing the error code, to
-    // relay up to the SSL socket client (i.e. via DoReadCallback).
-    if (result == 0)
-      transport_recv_eof_ = true;
-    (void)BIO_shutdown_wr(transport_bio_);
-  } else if (transport_write_error_ < 0) {
-    // Mirror transport write errors as read failures; transport_bio_ has been
-    // shut down by TransportWriteComplete, so the BIO_write will fail, failing
-    // the CHECK. http://crbug.com/335557.
-    result = transport_write_error_;
+    // Received 0 (end of file) or an error. Save it to be reported in a future
+    // read on transport_bio_'s peer.
+    transport_read_error_ = result;
   } else {
     DCHECK(recv_buffer_.get());
     int ret = BIO_write(transport_bio_, recv_buffer_->data(), result);
     // A write into a memory BIO should always succeed.
     DCHECK_EQ(result, ret);
   }
   recv_buffer_ = NULL;
   transport_recv_busy_ = false;
   return result;
 }
 
+int SSLClientSocketOpenSSL::HandleOpenSSLError(
+    int ssl_error,
+    const crypto::OpenSSLErrStackTracer& tracer) {
+  // Route transport failures around OpenSSL. Use SSL_ERROR_WANT_READ and
+  // SSL_ERROR_WANT_WRITE to determine whether to use the read or write failure
+  // on the transport. This is done in lieu of routing net errors though
+  // OpenSSL's error system.
+  if (ssl_error == SSL_ERROR_WANT_READ) {
+    // If OpenSSL wanted read data, report any pending errors that were
+    // observed. Note that both |transport_read_error_| and
+    // |transport_write_error_| are checked, since the application may have
+    // encountered a socket error while writing that would otherwise not be
+    // reported until the application attempted to write again - which it may
+    // never do. This matches the fix for https://crbug.com/249848 in NSS.
+    if (transport_read_error_ != ERR_IO_PENDING)
+      return transport_read_error_;
+    if (transport_write_error_ != ERR_IO_PENDING)
+      return transport_write_error_;
+  } else if (ssl_error == SSL_ERROR_WANT_WRITE) {
+    // Because of the write buffer, this reports a failure from the previous
+    // write payload. If the current payload fails to write, the error will be
+    // reported in a future write or read.
+    if (transport_write_error_ != ERR_IO_PENDING)
+      return transport_write_error_;
+  }
+
+  // Otherwise, map the error code.
+  return MapOpenSSLError(ssl_error, tracer);
+}
+
 int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl,
                                                       X509** x509,
                                                       EVP_PKEY** pkey) {
   DVLOG(3) << "OpenSSL ClientCertRequestCallback called";
   DCHECK(ssl == ssl_);
   DCHECK(*x509 == NULL);
   DCHECK(*pkey == NULL);
   if (!ssl_config_.send_client_cert) {
     // First pass: we know that a client certificate is needed, but we do not
     // have one at hand.
@@ skipping 152 matching lines @@
         ssl_config_.next_protos[0].data()));
     *outlen = ssl_config_.next_protos[0].size();
   }
 
   npn_proto_.assign(reinterpret_cast<const char*>(*out), *outlen);
   server_protos_.assign(reinterpret_cast<const char*>(in), inlen);
   DVLOG(2) << "next protocol: '" << npn_proto_ << "' status: " << npn_status_;
   return SSL_TLSEXT_ERR_OK;
 }
 
+long SSLClientSocketOpenSSL::BIOCallback(BIO *bio,
+                                         int cmd,
+                                         const char *argp, int argi, long argl,
+                                         long retvalue) {
+  if (cmd == BIO_CB_WRITE && transport_write_error_ != ERR_IO_PENDING) {
+    // If there is a transport write failure, act as if the buffer is full. This
+    // is necessary so future writes are stopped even if there is room in the
+    // buffer. Although retrying will not be of any use, mark as a retry so
+    // SSL_get_error will return SSL_ERROR_WANT_WRITE if there is no other error
+    // in the error queue. HandleOpenSSLError will consume that and route the
+    // transport error code out of OpenSSL.

[davidben, 2014/06/06 23:31:51]

As an alternative, we can BIO_shutdown_wr(SSL_get_wbio(ssl_)) as before and then
rely on the fact that OpenSSL returns BIO_R_BROKEN_PIPE when you try to write
into a BIO you've already shutdown_wr'd.

This seems mildly preferable to me in so far as it's more consistent with how
reads are handled and arguably calling BIO_shutdown_wr(bio); BIO_write(bio); is
a misuse of the API, even though it does check for the case and return a
predictable error code.

Reads don't need any special hooking because the empty buffer behavior and error
behavior are compatible.

[agl, 2014/06/09 17:23:34]

You mean when the write error is detected in TransportWriteComplete? I also
think that's slightly preferable because it avoids this callback, although I
wouldn't worry about it if you're on the fence.

[davidben, 2014/06/10 19:16:30]

Revised to keep the BIO_shutdown_wr and condition on BIO_R_RESET_PIPE.

+    BIO_set_retry_write(bio);
+    return -1;
+  }
+  return retvalue;
+}
+
+// static
+long SSLClientSocketOpenSSL::BIOCallbackThunk(
+    BIO *bio,
+    int cmd,
+    const char *argp, int argi, long argl,
+    long retvalue) {
+  SSLClientSocketOpenSSL* socket = reinterpret_cast<SSLClientSocketOpenSSL*>(
+      BIO_get_callback_arg(bio));
+  return socket->BIOCallback(bio, cmd, argp, argi, argl, retvalue);
+}
+
 scoped_refptr<X509Certificate>
 SSLClientSocketOpenSSL::GetUnverifiedServerCertificateChain() const {
   return server_cert_;
 }
 
 }  // namespace net