Don't use X509Certificate in SSLConfig.

net/socket/ssl_client_socket_nss.cc

 // Copyright (c) 2011 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 1020 matching lines @@
   std::string peer_id = host_and_port_.ToString();
   SECStatus rv = SSL_SetSockPeerID(nss_fd_, const_cast<char*>(peer_id.c_str()));
   if (rv != SECSuccess)
     LogFailedNSSFunction(net_log_, "SSL_SetSockPeerID", peer_id.c_str());
 
   return OK;
 }
 
 
 // Sets server_cert_ and server_cert_nss_ if not yet set.
 // Returns server_cert_.

[wtc, 2011/07/19 21:57:01]

Remove "Returns server_cert_."

[Sergey Ulanov, 2011/07/19 23:50:21]

Done.

-X509Certificate *SSLClientSocketNSS::UpdateServerCert() {
+void SSLClientSocketNSS::UpdateServerCert() {
   // We set the server_cert_ from HandshakeCallback().
   if (server_cert_ == NULL) {
     server_cert_nss_ = SSL_PeerCertificate(nss_fd_);
     if (server_cert_nss_) {
       PeerCertificateChain certs(nss_fd_);
       server_cert_ = X509Certificate::CreateFromDERCertChain(
           certs.AsStringPieceVector());

[wtc, 2011/07/19 21:57:01]

Please add a comment here that this may fail in the sandbox.

[Sergey Ulanov, 2011/07/19 23:50:21]

Done.

     }
   }
-  return server_cert_;
 }
 
 // Sets ssl_connection_status_.
 void SSLClientSocketNSS::UpdateConnectionStatus() {
   SSLChannelInfo channel_info;
   SECStatus ok = SSL_GetChannelInfo(nss_fd_,
                                     &channel_info, sizeof(channel_info));
   if (ok == SECSuccess &&
       channel_info.length == sizeof(channel_info) &&
       channel_info.cipherSuite) {
@@ skipping 451 matching lines @@
     GotoState(STATE_VERIFY_CERT_COMPLETE);
     return OK;
   }
 
   GotoState(STATE_VERIFY_CERT);
 
   return OK;
 }
 
 int SSLClientSocketNSS::DoVerifyCert(int result) {
-  DCHECK(server_cert_);
+  DCHECK(server_cert_nss_);
 
   GotoState(STATE_VERIFY_CERT_COMPLETE);
 
-  // If the certificate is expected to be bad we can use the expectation as the
-  // cert status.
+  // If the certificate is expected to be bad we can use the
+  // expectation as the cert status. Don't use |server_cert_| here
+  // because it can be set to NULL in case we failed to create
+  // X509Certificate in UpdateServerCert(). This may happen when this
+  // code is used inside of sandbox.

[wtc, 2011/07/19 21:57:01]

Nit: remove "of" on this line and line 1546 below.

Remove the "j" at the end of line 1546.

[Sergey Ulanov, 2011/07/19 23:50:21]

Done.

+  std::string cert_der(
+      reinterpret_cast<char*>(server_cert_nss_->derCert.data),
+      server_cert_nss_->derCert.len);

[wtc, 2011/07/19 21:57:01]

Using StringPiece here would avoid the copying.

[Sergey Ulanov, 2011/07/19 23:50:21]

Done.

   int cert_status;
-  if (ssl_config_.IsAllowedBadCert(server_cert_, &cert_status)) {
+  if (ssl_config_.IsAllowedBadCert(cert_der, &cert_status)) {
     DCHECK(start_cert_verification_time_.is_null());
     VLOG(1) << "Received an expected bad cert with status: " << cert_status;
     server_cert_verify_result_ = &local_server_cert_verify_result_;
     local_server_cert_verify_result_.Reset();
     local_server_cert_verify_result_.cert_status = cert_status;
     return OK;
   }
 
+  // We may have failed to create X509Certificate object if we are
+  // running inside of sandbox.j
+  if (!server_cert_)
+    return ERR_CERT_INVALID;

[wtc, 2011/07/19 21:57:01]

This should be done as follows (compare with lines
1536-1543 above):

  if (!server_cert_) {
    server_cert_verify_result_ = &local_server_cert_verify_result_;
    local_server_cert_verify_result_.Reset();
    local_server_cert_verify_result_.cert_status = CERT_STATUS_INVALID;
    return ERR_CERT_INVALID;
  }
    

[Sergey Ulanov, 2011/07/19 23:50:21]

Done.

+
   start_cert_verification_time_ = base::TimeTicks::Now();
 
   if (ssl_host_info_.get() && !ssl_host_info_->state().certs.empty() &&
       predicted_cert_chain_correct_) {
     // If the SSLHostInfo had a prediction for the certificate chain of this
     // server then it will have optimistically started a verification of that
     // chain. So, if the prediction was correct, we should wait for that
     // verification to finish rather than start our own.
     net_log_.AddEvent(NetLog::TYPE_SSL_VERIFICATION_MERGED, NULL);
     UMA_HISTOGRAM_ENUMERATION("Net.SSLVerificationMerged", 1 /* true */, 2);
@@ skipping 719 matching lines @@
   valid_thread_id_ = base::PlatformThread::CurrentId();
 }
 
 bool SSLClientSocketNSS::CalledOnValidThread() const {
   EnsureThreadIdAssigned();
   base::AutoLock auto_lock(lock_);
   return valid_thread_id_ == base::PlatformThread::CurrentId();
 }
 
 }  // namespace net