diff options
author | snej@chromium.org <snej@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-19 21:30:48 +0000 |
---|---|---|
committer | snej@chromium.org <snej@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-19 21:30:48 +0000 |
commit | d9f99dfd148ace487c6b78c3aa069f1d5436d832 (patch) | |
tree | e36e7645901a7af1cbe5a671546659d63560628b | |
parent | a3d807d8aa6477954130b00f3b8340894818c66d (diff) | |
download | chromium_src-d9f99dfd148ace487c6b78c3aa069f1d5436d832.zip chromium_src-d9f99dfd148ace487c6b78c3aa069f1d5436d832.tar.gz chromium_src-d9f99dfd148ace487c6b78c3aa069f1d5436d832.tar.bz2 |
Improved SSL handshake processing on Mac.
We now guarantee the server cert is verified before sending a client cert.
BUG=38550
TEST=none (manual testing with five different public sites that use client certs)
Review URL: http://codereview.chromium.org/1116003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@42149 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | net/socket/ssl_client_socket_mac.cc | 124 | ||||
-rw-r--r-- | net/socket/ssl_client_socket_mac.h | 4 |
2 files changed, 77 insertions, 51 deletions
diff --git a/net/socket/ssl_client_socket_mac.cc b/net/socket/ssl_client_socket_mac.cc index 373849f..4cf7722 100644 --- a/net/socket/ssl_client_socket_mac.cc +++ b/net/socket/ssl_client_socket_mac.cc @@ -512,6 +512,7 @@ SSLClientSocketMac::SSLClientSocketMac(ClientSocket* transport_socket, next_handshake_state_(STATE_NONE), completed_handshake_(false), handshake_interrupted_(false), + client_cert_requested_(false), ssl_context_(NULL), pending_send_error_(OK) { } @@ -678,6 +679,34 @@ SSLClientSocketMac::GetNextProto(std::string* proto) { return kNextProtoUnsupported; } +OSStatus SSLClientSocketMac::EnableBreakOnAuth(bool enabled) { + // SSLSetSessionOption() was introduced in Mac OS X 10.5.7. It allows us + // to perform certificate validation during the handshake, which is + // required in order to properly enable session resumption. + // + // With the kSSLSessionOptionBreakOnServerAuth option set, SSLHandshake() + // will return errSSLServerAuthCompleted after receiving the server's + // Certificate during the handshake. That gives us an opportunity to verify + // the server certificate and then re-enter that handshake (assuming the + // certificate successfully validated). + // If the server also requests a client cert, SSLHandshake() will return + // errSSLClientCertRequested in addition to (or in some cases *instead of*) + // errSSLServerAuthCompleted. + static SSLSetSessionOptionFuncPtr ssl_set_session_options = + LookupFunction<SSLSetSessionOptionFuncPtr>(CFSTR("com.apple.security"), + CFSTR("SSLSetSessionOption")); + if (!ssl_set_session_options) + return unimpErr; // Return this if the API isn't available + OSStatus err = ssl_set_session_options(ssl_context_, + kSSLSessionOptionBreakOnServerAuth, + enabled); + if (err) + return err; + return ssl_set_session_options(ssl_context_, + kSSLSessionOptionBreakOnCertRequested, + enabled); +} + int SSLClientSocketMac::InitializeSSLContext() { SSL_LOG << "----- InitializeSSLContext"; OSStatus status = noErr; @@ -732,39 +761,15 @@ int SSLClientSocketMac::InitializeSSLContext() { if (status) return NetErrorFromOSStatus(status); - // SSLSetSessionOption() was introduced in Mac OS X 10.5.7. It allows us - // to perform certificate validation during the handshake, which is - // required in order to properly enable session resumption. - // - // With the kSSLSessionOptionBreakOnServerAuth option set, SSLHandshake() - // will return errSSLServerAuthCompleted after receiving the server's - // Certificate during the handshake. That gives us an opportunity to verify - // the server certificate and then re-enter that handshake (assuming the - // certificate successfully validated). - // - // If SSLSetSessionOption() is not present, we do not enable session + // If break-on-auth is not available, we do not enable session // resumption, because in that case 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 // to still be able to be resumed. - static SSLSetSessionOptionFuncPtr ssl_set_session_options = - LookupFunction<SSLSetSessionOptionFuncPtr>(CFSTR("com.apple.security"), - CFSTR("SSLSetSessionOption")); - if (ssl_set_session_options && !ssl_config_.send_client_cert) { - status = ssl_set_session_options(ssl_context_, - kSSLSessionOptionBreakOnServerAuth, - true); - if (!status) { - SSL_LOG << "Setting kSSLSessionOptionBreakOnCertRequested"; - status = ssl_set_session_options(ssl_context_, - kSSLSessionOptionBreakOnCertRequested, - true); - } - if (status) - return NetErrorFromOSStatus(status); - + status = EnableBreakOnAuth(true); + if (status == noErr) { // Concatenate the hostname and peer address to use as the peer ID. To // resume a session, we must connect to the same server on the same port // using the same hostname (i.e., localhost and 127.0.0.1 are considered @@ -784,13 +789,8 @@ int SSLClientSocketMac::InitializeSSLContext() { status = SSLSetPeerID(ssl_context_, peer_id.data(), peer_id.length()); if (status) return NetErrorFromOSStatus(status); - } else if (ssl_config_.send_client_cert) { - // If I have a cert, set it up-front, otherwise the server may try to get - // it later by renegotiating, which SecureTransport doesn't support well. - SSL_LOG << "Setting client cert in advance because send_client_cert is set"; - status = SetClientCert(); - if (status) - return NetErrorFromOSStatus(status); + } else if (status != unimpErr) { // it's OK if the API isn't available + return NetErrorFromOSStatus(status); } return OK; @@ -935,7 +935,9 @@ int SSLClientSocketMac::DoHandshakeStart() { // by the user, EV, etc.) available. Eliminate this step once we have // a certificate validation result cache. (Also applies to the // errSSLServerAuthCompleted case below.) + SSL_LOG << "Handshake completed (DoHandshakeStart), next verify cert"; next_handshake_state_ = STATE_VERIFY_CERT; + HandshakeFinished(); break; case errSSLServerAuthCompleted: @@ -950,18 +952,15 @@ int SSLClientSocketMac::DoHandshakeStart() { case errSSLClientCertRequested: SSL_LOG << "Received client cert request in DoHandshakeStart"; - // For some reason it's possible for SSLHandshake to return this code - // without earlier having returned errSSLServerAuthCompleted. - // Currently this isn't an issue for us because we always respond by - // aborting the connection; but if we did decide to continue we should - // first go to STATE_VERIFY_CERT and not send the client cert until that - // succeeded. --snej - - // If we knew the client cert, we would have set it already in - // InitializeSSLContext, instead of asking for this callback. - // So tell the caller that we need a client cert. - DCHECK(!ssl_config_.send_client_cert); - return ERR_SSL_CLIENT_AUTH_CERT_NEEDED; + // If we get this instead of errSSLServerAuthCompleted, the latter is + // implicit, and we should begin verification as well. + next_handshake_state_ = STATE_VERIFY_CERT; + handshake_interrupted_ = true; + status = noErr; + // We don't want to send a client cert now, because we haven't + // verified the server's cert yet. Remember it for later. + client_cert_requested_ = true; + break; case errSSLClosedGraceful: // The server unexpectedly closed on us. @@ -1003,6 +1002,18 @@ int SSLClientSocketMac::DoVerifyCertComplete(int result) { if (IsCertificateError(result) && ssl_config_.IsAllowedBadCert(server_cert_)) result = OK; + if (result == OK && client_cert_requested_) { + // It is now safe to send the server the client cert it asked for. + if (!ssl_config_.send_client_cert) { + // Caller hasn't specified a client cert, so let it know the server's + // asking for one, and abort the connection. + return ERR_SSL_CLIENT_AUTH_CERT_NEEDED; + } + OSStatus status = SetClientCert(); + if (status != noErr) + return NetErrorFromOSStatus(status); + } + if (handshake_interrupted_) { // With session resumption enabled the full handshake (i.e., the handshake // in a non-resumed session) occurs in two steps. Continue on to the second @@ -1042,11 +1053,12 @@ int SSLClientSocketMac::DoHandshakeFinish() { break; case errSSLClientCertRequested: SSL_LOG << "Server requested client cert (DoHandshakeFinish)"; - // If we knew the client cert, we would have set it already in - // InitializeSSLContext, instead of asking for this callback. - // So tell the caller that we need a client cert. - DCHECK(!ssl_config_.send_client_cert); - return ERR_SSL_CLIENT_AUTH_CERT_NEEDED; + if (!ssl_config_.send_client_cert) + return ERR_SSL_CLIENT_AUTH_CERT_NEEDED; + status = SetClientCert(); + if (status == noErr) + next_handshake_state_ = STATE_HANDSHAKE_FINISH; + break; case errSSLClosedGraceful: return ERR_SSL_PROTOCOL_ERROR; case errSSLClosedAbort: @@ -1068,6 +1080,7 @@ int SSLClientSocketMac::DoHandshakeFinish() { } case noErr: SSL_LOG << "Handshake finished! (DoHandshakeFinish)"; + HandshakeFinished(); completed_handshake_ = true; DCHECK(next_handshake_state_ == STATE_NONE); break; @@ -1078,6 +1091,15 @@ int SSLClientSocketMac::DoHandshakeFinish() { 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()"; + EnableBreakOnAuth(false); +} + int SSLClientSocketMac::DoPayloadRead() { size_t processed = 0; OSStatus status = SSLRead(ssl_context_, diff --git a/net/socket/ssl_client_socket_mac.h b/net/socket/ssl_client_socket_mac.h index 3e649ff..09a3bc3 100644 --- a/net/socket/ssl_client_socket_mac.h +++ b/net/socket/ssl_client_socket_mac.h @@ -55,6 +55,8 @@ class SSLClientSocketMac : public SSLClientSocket { // Initializes the SSLContext. Returns a net error code. int InitializeSSLContext(); + OSStatus EnableBreakOnAuth(bool enabled); + void DoConnectCallback(int result); void DoReadCallback(int result); void DoWriteCallback(int result); @@ -70,6 +72,7 @@ class SSLClientSocketMac : public SSLClientSocket { int DoVerifyCert(); int DoVerifyCertComplete(int result); int DoHandshakeFinish(); + void HandshakeFinished(); int SetClientCert(); @@ -115,6 +118,7 @@ class SSLClientSocketMac : public SSLClientSocket { bool completed_handshake_; bool handshake_interrupted_; + bool client_cert_requested_; SSLContextRef ssl_context_; // These buffers hold data retrieved from/sent to the underlying transport |