Don't use X509Certificate in SSLConfig.

net/socket/ssl_client_socket_nss.cc

Index: net/socket/ssl_client_socket_nss.cc
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index e827f398db65eb56a946e5c1ed286f455ccccb12..2f7101c135f950817c51d5bd89ed227208161aa6 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -1039,7 +1039,7 @@ int SSLClientSocketNSS::InitializeSSLPeerName() {
 
 // 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_);
@@ -1049,7 +1049,6 @@ X509Certificate *SSLClientSocketNSS::UpdateServerCert() {
           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_.
@@ -1521,14 +1520,20 @@ int SSLClientSocketNSS::DoVerifyDNSSEC(int result) {
 }
 
 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_;
@@ -1537,6 +1542,11 @@ int SSLClientSocketNSS::DoVerifyCert(int result) {
     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() &&