Make SSL False Start work with asynchronous certificate validation

net/socket/ssl_client_socket_nss.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.
 
 // This file includes code SSLClientSocketNSS::DoVerifyCertComplete() derived
 // from AuthCertificateCallback() in
 // mozilla/security/manager/ssl/src/nsNSSCallbacks.cpp.
 
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
@@ skipping 693 matching lines @@
       CERTCertificate** result_nss_certificate,
       SECKEYPrivateKey** result_nss_private_key);
 #else
   static SECStatus ClientAuthHandler(void* arg,
                                      PRFileDesc* socket,
                                      CERTDistNames* ca_names,
                                      CERTCertificate** result_certificate,
                                      SECKEYPrivateKey** result_private_key);
 #endif
 
-  // Called by NSS once the handshake has completed.
+  // Called once the handshake has completed.
+  void HandshakeSucceeded();
+
+  // Called by NSS to determine if we can false start.

[agl, 2013/09/13 15:03:29]

In other comments, "false start" is written as "False Start".

[wtc, 2013/09/18 22:57:23]

Done.

   // |arg| contains a pointer to the current SSLClientSocketNSS::Core.
-  static void HandshakeCallback(PRFileDesc* socket, void* arg);
+  static SECStatus CanFalseStartCallback(PRFileDesc* socket,
+                                         void* arg,
+                                         PRBool* can_false_start);
 
   // Handles an NSS error generated while handshaking or performing IO.
   // Returns a network error code mapped from the original NSS error.
   int HandleNSSError(PRErrorCode error, bool handshake_error);
 
   int DoHandshakeLoop(int last_io_result);
   int DoReadLoop(int result);
   int DoWriteLoop(int result);
 
   int DoHandshake();
@@ skipping 132 matching lines @@
   std::vector<std::string> predicted_certs_;
 
   State next_handshake_state_;
 
   // True if channel ID extension was negotiated.
   bool channel_id_xtn_negotiated_;
   // True if the handshake state machine was interrupted for channel ID.
   bool channel_id_needed_;
   // True if the handshake state machine was interrupted for client auth.
   bool client_auth_cert_needed_;
-  // True if NSS has called HandshakeCallback.
-  bool handshake_callback_called_;
 
   HandshakeState nss_handshake_state_;
 
   bool transport_recv_busy_;
   bool transport_recv_eof_;
   bool transport_send_busy_;
 
   // Used by Read function.
   scoped_refptr<IOBuffer> user_read_buf_;
   int user_read_buf_len_;
@@ skipping 46 matching lines @@
       host_and_port_(host_and_port),
       ssl_config_(ssl_config),
       nss_fd_(NULL),
       nss_bufs_(NULL),
       pending_read_result_(kNoPendingReadResult),
       pending_read_nss_error_(0),
       next_handshake_state_(STATE_NONE),
       channel_id_xtn_negotiated_(false),
       channel_id_needed_(false),
       client_auth_cert_needed_(false),
-      handshake_callback_called_(false),
       transport_recv_busy_(false),
       transport_recv_eof_(false),
       transport_send_busy_(false),
       user_read_buf_len_(0),
       user_write_buf_len_(0),
       network_task_runner_(network_task_runner),
       nss_task_runner_(nss_task_runner),
       weak_net_log_(weak_net_log_factory_.GetWeakPtr()) {
 }
 
@@ skipping 74 matching lines @@
       DVLOG(1) << "System time is weird, not enabling channel ID.";
     } else {
       rv = SSL_SetClientChannelIDCallback(
           nss_fd_, SSLClientSocketNSS::Core::ClientChannelIDHandler, this);
       if (rv != SECSuccess)
         LogFailedNSSFunction(*weak_net_log_, "SSL_SetClientChannelIDCallback",
                              "");
     }
   }
 
-  rv = SSL_HandshakeCallback(
-      nss_fd_, SSLClientSocketNSS::Core::HandshakeCallback, this);
+  rv = SSL_SetCanFalseStartCallback(
+      nss_fd_, SSLClientSocketNSS::Core::CanFalseStartCallback, this);
   if (rv != SECSuccess) {
-    LogFailedNSSFunction(*weak_net_log_, "SSL_HandshakeCallback", "");
+    LogFailedNSSFunction(*weak_net_log_, "SSL_SetCanFalseStartCallback", "");
     return false;
   }
 
   return true;
 }
 
 void SSLClientSocketNSS::Core::SetPredictedCertificates(
     const std::vector<std::string>& predicted_certs) {
   if (predicted_certs.empty())
     return;
@@ skipping 102 matching lines @@
         base::Bind(IgnoreResult(&Core::Read), this, make_scoped_refptr(buf),
                    buf_len, callback));
     if (!posted) {
       nss_is_closed_ = true;
       nss_waiting_read_ = false;
     }
     return posted ? ERR_IO_PENDING : ERR_ABORTED;
   }
 
   DCHECK(OnNSSTaskRunner());
-  DCHECK(handshake_callback_called_);
   DCHECK_EQ(STATE_NONE, next_handshake_state_);
   DCHECK(user_read_callback_.is_null());
   DCHECK(user_connect_callback_.is_null());
   DCHECK(!user_read_buf_.get());
   DCHECK(nss_bufs_);
 
   user_read_buf_ = buf;
   user_read_buf_len_ = buf_len;
 
   int rv = DoReadLoop(OK);
@@ skipping 33 matching lines @@
         base::Bind(IgnoreResult(&Core::Write), this, make_scoped_refptr(buf),
                    buf_len, callback));
     if (!posted) {
       nss_is_closed_ = true;
       nss_waiting_write_ = false;
     }
     return posted ? ERR_IO_PENDING : ERR_ABORTED;
   }
 
   DCHECK(OnNSSTaskRunner());
-  DCHECK(handshake_callback_called_);
   DCHECK_EQ(STATE_NONE, next_handshake_state_);
   DCHECK(user_write_callback_.is_null());
   DCHECK(user_connect_callback_.is_null());
   DCHECK(!user_write_buf_.get());
   DCHECK(nss_bufs_);
 
   user_write_buf_ = buf;
   user_write_buf_len_ = buf_len;
 
   int rv = DoWriteLoop(OK);
@@ skipping 41 matching lines @@
 bool SSLClientSocketNSS::Core::OnNetworkTaskRunner() const {
   return network_task_runner_->RunsTasksOnCurrentThread();
 }
 
 // static
 SECStatus SSLClientSocketNSS::Core::OwnAuthCertHandler(
     void* arg,
     PRFileDesc* socket,
     PRBool checksig,
     PRBool is_server) {
-  Core* core = reinterpret_cast<Core*>(arg);
-  if (!core->handshake_callback_called_) {
-    // Only need to turn off False Start in the initial handshake. Also, it is
-    // unsafe to call SSL_OptionSet in a renegotiation because the "first
-    // handshake" lock isn't already held, which will result in an assertion
-    // failure in the ssl_Get1stHandshakeLock call in SSL_OptionSet.
-    PRBool negotiated_extension;
-    SECStatus rv = SSL_HandshakeNegotiatedExtension(socket,
-                                                    ssl_app_layer_protocol_xtn,
-                                                    &negotiated_extension);
-    if (rv != SECSuccess || !negotiated_extension) {
-      rv = SSL_HandshakeNegotiatedExtension(socket,
-                                            ssl_next_proto_nego_xtn,
-                                            &negotiated_extension);
-    }
-    if (rv != SECSuccess || !negotiated_extension) {
-      // If the server doesn't support NPN or ALPN, then we don't do False
-      // Start with it.
-      SSL_OptionSet(socket, SSL_ENABLE_FALSE_START, PR_FALSE);
-    }
-  }
-
   // Tell NSS to not verify the certificate.
   return SECSuccess;
 }
 
 #if defined(NSS_PLATFORM_CLIENT_AUTH)
 // static
 SECStatus SSLClientSocketNSS::Core::PlatformClientAuthHandler(
     void* arg,
     PRFileDesc* socket,
     CERTDistNames* ca_names,
@@ skipping 292 matching lines @@
   core->PostOrRunCallback(
       FROM_HERE, base::Bind(&Core::OnHandshakeStateUpdated, core,
                             core->nss_handshake_state_));
 
   // Tell NSS to suspend the client authentication.  We will then abort the
   // handshake by returning ERR_SSL_CLIENT_AUTH_CERT_NEEDED.
   return SECWouldBlock;
 }
 #endif  // NSS_PLATFORM_CLIENT_AUTH
 
-// static
-void SSLClientSocketNSS::Core::HandshakeCallback(
-    PRFileDesc* socket,
-    void* arg) {
-  Core* core = reinterpret_cast<Core*>(arg);
-  DCHECK(core->OnNSSTaskRunner());
-
-  core->handshake_callback_called_ = true;
-
-  HandshakeState* nss_state = &core->nss_handshake_state_;
+void SSLClientSocketNSS::Core::HandshakeSucceeded() {
+  DCHECK(OnNSSTaskRunner());
 
   PRBool last_handshake_resumed;
-  SECStatus rv = SSL_HandshakeResumedSession(socket, &last_handshake_resumed);
+  SECStatus rv = SSL_HandshakeResumedSession(nss_fd_, &last_handshake_resumed);
   if (rv == SECSuccess && last_handshake_resumed) {
-    nss_state->resumed_handshake = true;
+    nss_handshake_state_.resumed_handshake = true;
   } else {
-    nss_state->resumed_handshake = false;
+    nss_handshake_state_.resumed_handshake = false;
   }
 
-  core->RecordChannelIDSupport();
-  core->UpdateServerCert();
-  core->UpdateConnectionStatus();
-  core->UpdateNextProto();
+  RecordChannelIDSupport();
+  UpdateServerCert();
+  UpdateConnectionStatus();
+  UpdateNextProto();
 
   // Update the network task runners view of the handshake state whenever
   // a handshake has completed.
-  core->PostOrRunCallback(
-      FROM_HERE, base::Bind(&Core::OnHandshakeStateUpdated, core,
-                            *nss_state));
+  PostOrRunCallback(
+      FROM_HERE, base::Bind(&Core::OnHandshakeStateUpdated, this,
+                            nss_handshake_state_));
+}
+
+// static
+SECStatus SSLClientSocketNSS::Core::CanFalseStartCallback(
+    PRFileDesc* socket,
+    void* arg,
+    PRBool* can_false_start) {
+  // If the server doesn't support NPN or ALPN, then we don't do False Start
+  // with it.
+  PRBool negotiated_extension;
+  SECStatus rv = SSL_HandshakeNegotiatedExtension(socket,
+                                                  ssl_app_layer_protocol_xtn,
+                                                  &negotiated_extension);
+  if (rv != SECSuccess || !negotiated_extension) {
+    rv = SSL_HandshakeNegotiatedExtension(socket,
+                                          ssl_next_proto_nego_xtn,
+                                          &negotiated_extension);
+  }
+  if (rv != SECSuccess || !negotiated_extension) {
+    *can_false_start = PR_FALSE;
+    return SECSuccess;
+  }
+
+  return SSL_DefaultCanFalseStart(socket, can_false_start);
 }
 
 int SSLClientSocketNSS::Core::HandleNSSError(PRErrorCode nss_error,
                                              bool handshake_error) {
   DCHECK(OnNSSTaskRunner());
 
   int net_error = handshake_error ? MapNSSClientHandshakeError(nss_error) :
                                     MapNSSClientError(nss_error);
 
 #if defined(OS_WIN)
@@ skipping 54 matching lines @@
       // the transport IO may allow DoHandshake to make progress.
       DCHECK(rv == OK || rv == ERR_IO_PENDING);
       rv = OK;  // This causes us to stay in the loop.
     }
   } while (rv != ERR_IO_PENDING && next_handshake_state_ != STATE_NONE);
   return rv;
 }
 
 int SSLClientSocketNSS::Core::DoReadLoop(int result) {
   DCHECK(OnNSSTaskRunner());
-  DCHECK(handshake_callback_called_);
   DCHECK_EQ(STATE_NONE, next_handshake_state_);
 
   if (result < 0)
     return result;
 
   if (!nss_bufs_) {
     LOG(DFATAL) << "!nss_bufs_";
     int rv = ERR_UNEXPECTED;
     PostOrRunCallback(
         FROM_HERE,
         base::Bind(&AddLogEventWithCallback, weak_net_log_,
                    NetLog::TYPE_SSL_READ_ERROR,
                    CreateNetLogSSLErrorCallback(rv, 0)));
     return rv;
   }
 
   bool network_moved;
   int rv;
   do {
     rv = DoPayloadRead();
     network_moved = DoTransportIO();
   } while (rv == ERR_IO_PENDING && network_moved);
 
   return rv;
 }
 
 int SSLClientSocketNSS::Core::DoWriteLoop(int result) {
   DCHECK(OnNSSTaskRunner());
-  DCHECK(handshake_callback_called_);
   DCHECK_EQ(STATE_NONE, next_handshake_state_);
 
   if (result < 0)
     return result;
 
   if (!nss_bufs_) {
     LOG(DFATAL) << "!nss_bufs_";
     int rv = ERR_UNEXPECTED;
     PostOrRunCallback(
         FROM_HERE,
@@ skipping 35 matching lines @@
                    CreateNetLogSSLErrorCallback(net_error, 0)));
 
     // If the handshake already succeeded (because the server requests but
     // doesn't require a client cert), we need to invalidate the SSL session
     // so that we won't try to resume the non-client-authenticated session in
     // the next handshake.  This will cause the server to ask for a client
     // cert again.
     if (rv == SECSuccess && SSL_InvalidateSession(nss_fd_) != SECSuccess)
       LOG(WARNING) << "Couldn't invalidate SSL session: " << PR_GetError();
   } else if (rv == SECSuccess) {
-    if (!handshake_callback_called_) {
-      // Workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=562434 -
-      // SSL_ForceHandshake returned SECSuccess prematurely.
-      rv = SECFailure;
-      net_error = ERR_SSL_PROTOCOL_ERROR;
-      PostOrRunCallback(
-          FROM_HERE,
-          base::Bind(&AddLogEventWithCallback, weak_net_log_,
-                     NetLog::TYPE_SSL_HANDSHAKE_ERROR,
-                     CreateNetLogSSLErrorCallback(net_error, 0)));
-    } else {
+    HandshakeSucceeded();
   #if defined(SSL_ENABLE_OCSP_STAPLING)
-      // TODO(agl): figure out how to plumb an OCSP response into the Mac
-      // system library and update IsOCSPStaplingSupported for Mac.
-      if (IsOCSPStaplingSupported()) {
-        const SECItemArray* ocsp_responses =
-            SSL_PeerStapledOCSPResponses(nss_fd_);
-        if (ocsp_responses->len) {
+    // TODO(agl): figure out how to plumb an OCSP response into the Mac
+    // system library and update IsOCSPStaplingSupported for Mac.
+    if (IsOCSPStaplingSupported()) {
+      const SECItemArray* ocsp_responses =
+          SSL_PeerStapledOCSPResponses(nss_fd_);
+      if (ocsp_responses->len) {
   #if defined(OS_WIN)
-          if (nss_handshake_state_.server_cert) {
-            CRYPT_DATA_BLOB ocsp_response_blob;
-            ocsp_response_blob.cbData = ocsp_responses->items[0].len;
-            ocsp_response_blob.pbData = ocsp_responses->items[0].data;
-            BOOL ok = CertSetCertificateContextProperty(
-                nss_handshake_state_.server_cert->os_cert_handle(),
-                CERT_OCSP_RESPONSE_PROP_ID,
-                CERT_SET_PROPERTY_IGNORE_PERSIST_ERROR_FLAG,
-                &ocsp_response_blob);
-            if (!ok) {
-              VLOG(1) << "Failed to set OCSP response property: "
-                      << GetLastError();
-            }
+        if (nss_handshake_state_.server_cert) {
+          CRYPT_DATA_BLOB ocsp_response_blob;
+          ocsp_response_blob.cbData = ocsp_responses->items[0].len;
+          ocsp_response_blob.pbData = ocsp_responses->items[0].data;
+          BOOL ok = CertSetCertificateContextProperty(
+              nss_handshake_state_.server_cert->os_cert_handle(),
+              CERT_OCSP_RESPONSE_PROP_ID,
+              CERT_SET_PROPERTY_IGNORE_PERSIST_ERROR_FLAG,
+              &ocsp_response_blob);
+          if (!ok) {
+            VLOG(1) << "Failed to set OCSP response property: "
+                    << GetLastError();
           }
+        }
   #elif defined(USE_NSS)
-          CacheOCSPResponseFromSideChannelFunction cache_ocsp_response =
-              GetCacheOCSPResponseFromSideChannelFunction();
+        CacheOCSPResponseFromSideChannelFunction cache_ocsp_response =
+            GetCacheOCSPResponseFromSideChannelFunction();
 
-          cache_ocsp_response(
-              CERT_GetDefaultCertDB(),
-              nss_handshake_state_.server_cert_chain[0], PR_Now(),
-              &ocsp_responses->items[0], NULL);
+        cache_ocsp_response(
+            CERT_GetDefaultCertDB(),
+            nss_handshake_state_.server_cert_chain[0], PR_Now(),
+            &ocsp_responses->items[0], NULL);
   #endif
-        }
       }
+    }
   #endif
-    }
     // Done!
   } else {
     PRErrorCode prerr = PR_GetError();
     net_error = HandleNSSError(prerr, true);
 
     // Some network devices that inspect application-layer packets seem to
     // inject TCP reset packets to break the connections when they see
     // TLS 1.1 in ClientHello or ServerHello. See http://crbug.com/130293.
     //
     // Only allow ERR_CONNECTION_RESET to trigger a fallback from TLS 1.1 or
@@ skipping 1643 matching lines @@
   EnsureThreadIdAssigned();
   base::AutoLock auto_lock(lock_);
   return valid_thread_id_ == base::PlatformThread::CurrentId();
 }
 
 ServerBoundCertService* SSLClientSocketNSS::GetServerBoundCertService() const {
   return server_bound_cert_service_;
 }
 
 }  // namespace net