Remove client_auth_cert_needed_ from SSLClientSocketOpenSSL.

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 <errno.h>
@@ skipping 340 matching lines @@
     const SSLClientSocketContext& context)
     : transport_send_busy_(false),
       transport_recv_busy_(false),
       pending_read_error_(kNoPendingReadResult),
       pending_read_ssl_error_(SSL_ERROR_NONE),
       transport_read_error_(OK),
       transport_write_error_(OK),
       server_cert_chain_(new PeerCertificateChain(NULL)),
       completed_connect_(false),
       was_ever_used_(false),
-      client_auth_cert_needed_(false),
       cert_verifier_(context.cert_verifier),
       cert_transparency_verifier_(context.cert_transparency_verifier),
       channel_id_service_(context.channel_id_service),
       ssl_(NULL),
       transport_bio_(NULL),
       transport_(transport_socket.Pass()),
       host_and_port_(host_and_port),
       ssl_config_(ssl_config),
       ssl_session_cache_shard_(context.ssl_session_cache_shard),
       next_handshake_state_(STATE_NONE),
@@ skipping 120 matching lines @@
   pending_read_error_info_ = OpenSSLErrorInfo();
 
   transport_read_error_ = OK;
   transport_write_error_ = OK;
 
   server_cert_verify_result_.Reset();
   completed_connect_ = false;
 
   cert_authorities_.clear();
   cert_key_types_.clear();
-  client_auth_cert_needed_ = false;
 
   start_cert_verification_time_ = base::TimeTicks();
 
   npn_status_ = kNextProtoUnsupported;
   npn_proto_.clear();
 
   channel_id_sent_ = false;
   channel_id_request_handle_.Cancel();
 }
 
@@ skipping 440 matching lines @@
     SSL_get0_signed_cert_timestamp_list(ssl_, &sct_list, &sct_list_len);
     set_signed_cert_timestamps_received(sct_list_len != 0);
 
     if (IsRenegotiationAllowed())
       SSL_set_reject_peer_renegotiations(ssl_, 0);
 
     // Verify the certificate.
     UpdateServerCert();
     GotoState(STATE_VERIFY_CERT);
   } else {
-    if (client_auth_cert_needed_)
-      return ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
-
     int ssl_error = SSL_get_error(ssl_, rv);
-
     if (ssl_error == SSL_ERROR_WANT_CHANNEL_ID_LOOKUP) {
       // The server supports channel ID. Stop to look one up before returning to
       // the handshake.
       GotoState(STATE_CHANNEL_ID_LOOKUP);
       return OK;
     }
+    if (ssl_error == SSL_ERROR_WANT_X509_LOOKUP &&
+        !ssl_config_.send_client_cert) {
+      return ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
+    }
 
     OpenSSLErrorInfo error_info;
     net_error = MapOpenSSLErrorWithDetails(ssl_error, err_tracer, &error_info);
 
     // 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
@@ skipping 398 matching lines @@
   do {
     ssl_ret = SSL_read(ssl_, user_read_buf_->data() + total_bytes_read,
                        user_read_buf_len_ - total_bytes_read);
     if (ssl_ret > 0)
       total_bytes_read += ssl_ret;
   } while (total_bytes_read < user_read_buf_len_ && ssl_ret > 0);
 
   // Although only the final SSL_read call may have failed, the failure needs to
   // processed immediately, while the information still available in OpenSSL's
   // error queue.
-  if (client_auth_cert_needed_) {
-    pending_read_error_ = ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
-  } else if (ssl_ret <= 0) {
+  if (ssl_ret <= 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.
     pending_read_ssl_error_ = SSL_get_error(ssl_, ssl_ret);
     if (pending_read_ssl_error_ == SSL_ERROR_ZERO_RETURN) {
       pending_read_error_ = 0;
+    } else if (pending_read_ssl_error_ == SSL_ERROR_WANT_X509_LOOKUP &&
+               !ssl_config_.send_client_cert) {

[davidben, 2015/05/08 22:15:52]

This is the same as checking it before the ssl_ret check. If
client_auth_cert_needed_ was true, we just returned -1 from a callback, which
means that ssl_ret is in a failing state. This is just more correct w.r.t. how
one is supposed to do OpenSSL error-handling.

+      pending_read_error_ = ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
     } else {
       pending_read_error_ = MapOpenSSLErrorWithDetails(
           pending_read_ssl_error_, err_tracer, &pending_read_error_info_);
     }
 
     // Many servers do not reliably send a close_notify alert when shutting down
     // a connection, and instead terminate the TCP connection. This is reported
     // as ERR_CONNECTION_CLOSED. Because of this, map the unclean shutdown to a
     // graceful EOF, instead of treating it as an error as it should be.
     if (pending_read_error_ == ERR_CONNECTION_CLOSED)
@@ skipping 191 matching lines @@
   // Clear any currently configured certificates.
   SSL_certs_clear(ssl_);
 
 #if defined(OS_IOS)
   // TODO(droger): Support client auth on iOS. See http://crbug.com/145954).
   LOG(WARNING) << "Client auth is not supported";
 #else  // !defined(OS_IOS)
   if (!ssl_config_.send_client_cert) {
     // First pass: we know that a client certificate is needed, but we do not
     // have one at hand.
-    client_auth_cert_needed_ = true;
     STACK_OF(X509_NAME) *authorities = SSL_get_client_CA_list(ssl);
     for (size_t i = 0; i < sk_X509_NAME_num(authorities); i++) {
       X509_NAME *ca_name = (X509_NAME *)sk_X509_NAME_value(authorities, i);
       unsigned char* str = NULL;
       int length = i2d_X509_NAME(ca_name, &str);
       cert_authorities_.push_back(std::string(
           reinterpret_cast<const char*>(str),
           static_cast<size_t>(length)));
       OPENSSL_free(str);
     }
 
     const unsigned char* client_cert_types;
     size_t num_client_cert_types =
         SSL_get0_certificate_types(ssl, &client_cert_types);
     for (size_t i = 0; i < num_client_cert_types; i++) {
       cert_key_types_.push_back(
           static_cast<SSLClientCertType>(client_cert_types[i]));
     }
 
-    return -1;  // Suspends handshake.
+    // Suspends handshake. SSL_get_error will return SSL_ERROR_WANT_X509_LOOKUP.
+    return -1;
   }
 
   // Second pass: a client certificate should have been selected.
   if (ssl_config_.client_cert.get()) {
     ScopedX509 leaf_x509 =
         OSCertHandleToOpenSSL(ssl_config_.client_cert->os_cert_handle());
     if (!leaf_x509) {
       LOG(WARNING) << "Failed to import certificate";
       OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_CERT_BAD_FORMAT);
       return -1;
@@ skipping 256 matching lines @@
   }
   return false;
 }
 
 scoped_refptr<X509Certificate>
 SSLClientSocketOpenSSL::GetUnverifiedServerCertificateChain() const {
   return server_cert_;
 }
 
 }  // namespace net