Split DoHandshake into DoHandshake and DoHandshakeComplete.

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 872 matching lines @@
   return network_moved;
 }
 
 // TODO(cbentzel): Remove including "base/threading/thread_local.h" and
 // g_first_run_completed once crbug.com/424386 is fixed.
 base::LazyInstance<base::ThreadLocalBoolean>::Leaky g_first_run_completed =
     LAZY_INSTANCE_INITIALIZER;
 
 int SSLClientSocketOpenSSL::DoHandshake() {
   crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
-  int net_error = OK;
 
   int rv;
 
   // TODO(cbentzel): Leave only 1 call to SSL_do_handshake once crbug.com/424386
   // is fixed.
   if (ssl_config_.send_client_cert && ssl_config_.client_cert.get()) {
     rv = SSL_do_handshake(ssl_);
   } else {
     if (g_first_run_completed.Get().Get()) {
       // TODO(cbentzel): Remove ScopedTracker below once crbug.com/424386 is
       // fixed.
       tracked_objects::ScopedTracker tracking_profile(
           FROM_HERE_WITH_EXPLICIT_FUNCTION("424386 SSL_do_handshake()"));
 
       rv = SSL_do_handshake(ssl_);
     } else {
       g_first_run_completed.Get().Set(true);
       rv = SSL_do_handshake(ssl_);
     }
   }
 
-  if (rv == 1) {
-    if (ssl_config_.version_fallback &&
-        ssl_config_.version_max < ssl_config_.version_fallback_min) {
-      return ERR_SSL_FALLBACK_BEYOND_MINIMUM_VERSION;
-    }
-
-    // SSL handshake is completed. If NPN wasn't negotiated, see if ALPN was.
-    if (npn_status_ == kNextProtoUnsupported) {
-      const uint8_t* alpn_proto = NULL;
-      unsigned alpn_len = 0;
-      SSL_get0_alpn_selected(ssl_, &alpn_proto, &alpn_len);
-      if (alpn_len > 0) {
-        npn_proto_.assign(reinterpret_cast<const char*>(alpn_proto), alpn_len);
-        npn_status_ = kNextProtoNegotiated;
-        set_negotiation_extension(kExtensionALPN);
-      }
-    }
-
-    RecordNegotiationExtension();
-    RecordChannelIDSupport(channel_id_service_, channel_id_sent_,
-                           ssl_config_.channel_id_enabled,
-                           crypto::ECPrivateKey::IsSupported());
-
-    // Only record OCSP histograms if OCSP was requested.
-    if (ssl_config_.signed_cert_timestamps_enabled ||
-        cert_verifier_->SupportsOCSPStapling()) {
-      const uint8_t* ocsp_response;
-      size_t ocsp_response_len;
-      SSL_get0_ocsp_response(ssl_, &ocsp_response, &ocsp_response_len);
-
-      set_stapled_ocsp_response_received(ocsp_response_len != 0);
-      UMA_HISTOGRAM_BOOLEAN("Net.OCSPResponseStapled", ocsp_response_len != 0);
-    }
-
-    const uint8_t* sct_list;
-    size_t sct_list_len;
-    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 {
+  int net_error = OK;
+  if (rv <= 0) {
     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 (net_error == ERR_IO_PENDING) {
+      // If not done, stay in this state
+      GotoState(STATE_HANDSHAKE);
+      return ERR_IO_PENDING;
+    }
 
-    // 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(
-          NetLog::TYPE_SSL_HANDSHAKE_ERROR,
-          CreateNetLogOpenSSLErrorCallback(net_error, ssl_error, error_info));
+    LOG(ERROR) << "handshake failed; returned " << rv << ", SSL error code "
+               << ssl_error << ", net_error " << net_error;
+    net_log_.AddEvent(
+        NetLog::TYPE_SSL_HANDSHAKE_ERROR,
+        CreateNetLogOpenSSLErrorCallback(net_error, ssl_error, error_info));
+  }
+
+  GotoState(STATE_HANDSHAKE_COMPLETE);

[davidben, 2015/05/08 22:29:16]

TBH, this transition is kinda pointless in the failing case, since
DoHandshakeComplete will just immediately return. But going from Foo to
FooComplete seems more consistent with the rest of net.

+  return net_error;
+}
+
+int SSLClientSocketOpenSSL::DoHandshakeComplete(int result) {
+  if (result < 0)
+    return result;
+
+  if (ssl_config_.version_fallback &&
+      ssl_config_.version_max < ssl_config_.version_fallback_min) {
+    return ERR_SSL_FALLBACK_BEYOND_MINIMUM_VERSION;
+  }
+
+  // SSL handshake is completed. If NPN wasn't negotiated, see if ALPN was.
+  if (npn_status_ == kNextProtoUnsupported) {
+    const uint8_t* alpn_proto = NULL;
+    unsigned alpn_len = 0;
+    SSL_get0_alpn_selected(ssl_, &alpn_proto, &alpn_len);
+    if (alpn_len > 0) {
+      npn_proto_.assign(reinterpret_cast<const char*>(alpn_proto), alpn_len);
+      npn_status_ = kNextProtoNegotiated;
+      set_negotiation_extension(kExtensionALPN);
     }
   }
-  return net_error;
+
+  RecordNegotiationExtension();
+  RecordChannelIDSupport(channel_id_service_, channel_id_sent_,
+                         ssl_config_.channel_id_enabled,
+                         crypto::ECPrivateKey::IsSupported());
+
+  // Only record OCSP histograms if OCSP was requested.
+  if (ssl_config_.signed_cert_timestamps_enabled ||
+      cert_verifier_->SupportsOCSPStapling()) {
+    const uint8_t* ocsp_response;
+    size_t ocsp_response_len;
+    SSL_get0_ocsp_response(ssl_, &ocsp_response, &ocsp_response_len);
+
+    set_stapled_ocsp_response_received(ocsp_response_len != 0);
+    UMA_HISTOGRAM_BOOLEAN("Net.OCSPResponseStapled", ocsp_response_len != 0);
+  }
+
+  const uint8_t* sct_list;
+  size_t sct_list_len;
+  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);
+  return OK;
 }
 
 int SSLClientSocketOpenSSL::DoChannelIDLookup() {
   net_log_.AddEvent(NetLog::TYPE_SSL_CHANNEL_ID_REQUESTED);
   GotoState(STATE_CHANNEL_ID_LOOKUP_COMPLETE);
   return channel_id_service_->GetOrCreateChannelID(
       host_and_port_.host(),
       &channel_id_private_key_,
       &channel_id_cert_,
       base::Bind(&SSLClientSocketOpenSSL::OnHandshakeIOComplete,
@@ skipping 291 matching lines @@
     // (This is a quirk carried over from the windows
     // implementation.  It makes reading the logs a bit harder.)
     // State handlers can and often do call GotoState just
     // to stay in the current state.
     State state = next_handshake_state_;
     GotoState(STATE_NONE);
     switch (state) {
       case STATE_HANDSHAKE:
         rv = DoHandshake();
         break;
+      case STATE_HANDSHAKE_COMPLETE:
+        rv = DoHandshakeComplete(rv);
+        break;
       case STATE_CHANNEL_ID_LOOKUP:
         DCHECK_EQ(OK, rv);
         rv = DoChannelIDLookup();
        break;
       case STATE_CHANNEL_ID_LOOKUP_COMPLETE:
         rv = DoChannelIDLookupComplete(rv);
         break;
       case STATE_VERIFY_CERT:
         DCHECK_EQ(OK, rv);
         rv = DoVerifyCert(rv);
@@ skipping 593 matching lines @@
   }
   return false;
 }
 
 scoped_refptr<X509Certificate>
 SSLClientSocketOpenSSL::GetUnverifiedServerCertificateChain() const {
   return server_cert_;
 }
 
 }  // namespace net