Don't break on auth if we are about to do client auth....

net/socket/ssl_client_socket_mac.cc

 // Copyright (c) 2010 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.
 
 #include "net/socket/ssl_client_socket_mac.h"
 
 #include <CoreServices/CoreServices.h>
 #include <netdb.h>
 #include <sys/socket.h>
 #include <sys/types.h>
@@ skipping 784 matching lines @@
                                 hostname_.length());
   if (status)
     return NetErrorFromOSStatus(status);
 
   // Disable certificate verification within Secure Transport; we'll
   // be handling that ourselves.
   status = SSLSetEnableCertVerify(ssl_context_, false);
   if (status)
     return NetErrorFromOSStatus(status);
 
+  // It is tricky to handle client cert request over renegotiation due to bugs
+  // in Secure Transport.  From Ken McLeod on apple-cdsa:
+  // http://lists.apple.com/archives/apple-cdsa/2010/Feb/msg00058.html
+  //   A possible workaround would be to set the
+  //   kSSLSessionOptionBreakOnCertRequested option initially, then if you get
+  //   that status, ask for a client cert, abort the connection yourself and
+  //   retry it (this time calling SSLSetCertificate before the handshake
+  //   starts, and *not* setting the kSSLSessionOptionBreakOnCertRequested
+  //   option.)
   if (ssl_config_.send_client_cert) {
-    // Provide the client cert up-front if we have one, even though we'll get
-    // notified later when the server requests it, and set it again; this is
-    // seemingly redundant but works around a problem with SecureTransport
-    // and provides correct behavior on both 10.5 and 10.6:
-    // http://lists.apple.com/archives/apple-cdsa/2010/Feb/msg00058.html
-    // http://code.google.com/p/chromium/issues/detail?id=38905
     SSL_LOG << "Setting client cert in advance because send_client_cert is set";
     status = SetClientCert();
     if (status)
       return NetErrorFromOSStatus(status);
+    return OK;
   }
 
   status = EnableBreakOnAuth(true);
   if (status == noErr) {
     // Only enable session resumption if break-on-auth is available,
     // because without break-on-auth we are verifying the server's certificate
     // after the handshake completes (but before any application data is
     // exchanged). If we were to enable session resumption in this situation,
     // the session would be cached before we verified the certificate, leaving
     // the potential for a session in which the certificate failed to validate
@@ skipping 276 matching lines @@
 
 int SSLClientSocketMac::DoHandshakeFinish() {
   OSStatus status = SSLHandshake(ssl_context_);
 
   switch (status) {
     case errSSLWouldBlock:
       next_handshake_state_ = STATE_HANDSHAKE_FINISH;
       break;
     case errSSLClientCertRequested:
       SSL_LOG << "Server requested client cert (DoHandshakeFinish)";
-      if (!ssl_config_.send_client_cert)
-        return ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
-      // (We already called SetClientCert during InitializeSSLContext.)
-      status = noErr;
-      next_handshake_state_ = STATE_HANDSHAKE_FINISH;
+      DCHECK(!ssl_config_.send_client_cert);
+      return ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
       break;
     case errSSLClosedGraceful:
       return ERR_SSL_PROTOCOL_ERROR;
     case errSSLClosedAbort:
     case errSSLPeerHandshakeFail: {
       // See if the server aborted due to client cert checking.
       SSLClientCertificateState client_state;
       if (SSLGetClientCertificateState(ssl_context_, &client_state) == noErr &&
           client_state > kSSLClientCertNone) {
         if (client_state == kSSLClientCertRequested &&
@@ skipping 14 matching lines @@
       DCHECK(next_handshake_state_ == STATE_NONE);
       break;
     default:
       break;
   }
 
   return NetErrorFromOSStatus(status);
 }
 
 void SSLClientSocketMac::HandshakeFinished() {
-  // After the handshake's finished, disable breaking on server or client
-  // auth. Otherwise it might be triggered during a subsequent renegotiation,
-  // and SecureTransport doesn't handle that very well (there's usually no way
-  // to proceed without aborting the connection, at least not on 10.5.)
   SSL_LOG << "HandshakeFinished()";
-  OSStatus status = EnableBreakOnAuth(false);
-  if (status != noErr)
-    SSL_LOG << "EnableBreakOnAuth failed: " << status;
-  // Note- this will actually always return an error, up through OS 10.6.3,
-  // because the option can't be changed after the context opens.
 }
 
 int SSLClientSocketMac::DoPayloadRead() {
   size_t processed = 0;
   OSStatus status = SSLRead(ssl_context_,
                             user_read_buf_->data(),
                             user_read_buf_len_,
                             &processed);
 
   // There's a subtle difference here in semantics of the "would block" errors.
   // In our code, ERR_IO_PENDING means the whole operation is async, while
   // errSSLWouldBlock means that the stream isn't ending (and is often returned
   // along with partial data). So even though "would block" is returned, if we
   // have data, let's just return it.
 
   if (processed > 0)
     return processed;
 
   switch (status) {
     case errSSLClosedNoNotify:
       // TODO(wtc): Unless we have received the close_notify alert, we need to
       // return an error code indicating that the SSL connection ended
       // uncleanly, a potential truncation attack.  See http://crbug.com/18586.
       return OK;
 
     case errSSLServerAuthCompleted:
     case errSSLClientCertRequested:
       // Server wants to renegotiate, probably to ask for a client cert,
       // but SecureTransport doesn't support renegotiation so we have to close.
-      if (ssl_config_.send_client_cert) {
-        // We already gave SecureTransport a client cert. At this point there's
-        // nothing we can do; the renegotiation will fail regardless, due to
-        // bugs in Apple's SecureTransport library.
-        SSL_LOG << "Server renegotiating (status=" << status
-                << "), but I've already set a client cert. Fatal error.";
-        return ERR_SSL_PROTOCOL_ERROR;
-      }
+      DCHECK(!ssl_config_.send_client_cert);
       // Tell my caller the server wants a client cert so it can reconnect.
       SSL_LOG << "Server renegotiating; assuming it wants a client cert...";
       return ERR_SSL_CLIENT_AUTH_CERT_NEEDED;
 
     default:
       return NetErrorFromOSStatus(status);
   }
 }
 
 int SSLClientSocketMac::DoPayloadWrite() {
@@ skipping 110 matching lines @@
   if (rv < 0 && rv != ERR_IO_PENDING) {
     us->write_io_buf_ = NULL;
     return OSStatusFromNetError(rv);
   }
 
   // always lie to our caller
   return noErr;
 }
 
 }  // namespace net