Remove client_auth_cert_needed_ from SSLClientSocketOpenSSL.

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 88965f5688d2bbe129f8a34e483e92d5df49bfe4..24c5a55beee076afa6f0cf53f39d61a084f0809e 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -358,7 +358,6 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
       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),
@@ -499,7 +498,6 @@ void SSLClientSocketOpenSSL::Disconnect() {
 
   cert_authorities_.clear();
   cert_key_types_.clear();
-  client_auth_cert_needed_ = false;
 
   start_cert_verification_time_ = base::TimeTicks();
 
@@ -960,17 +958,17 @@ int SSLClientSocketOpenSSL::DoHandshake() {
     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);
@@ -1389,9 +1387,7 @@ int SSLClientSocketOpenSSL::DoPayloadRead() {
   // 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.
@@ -1403,6 +1399,9 @@ int SSLClientSocketOpenSSL::DoPayloadRead() {
     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_);
@@ -1614,7 +1613,6 @@ int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl) {
   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);
@@ -1634,7 +1632,8 @@ int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl) {
           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.