Optimize CT & OCSP handling code

net/socket/ssl_client_socket_impl.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.
 
 #include "net/socket/ssl_client_socket_impl.h"
 
 #include <errno.h>
 #include <string.h>
 
 #include <algorithm>
@@ skipping 1154 matching lines @@
                             alpn_len);
     negotiated_protocol_ = NextProtoFromString(proto);
   }
 
   RecordNegotiatedProtocol();
   RecordChannelIDSupport();
 
   const uint8_t* ocsp_response_raw;
   size_t ocsp_response_len;
   SSL_get0_ocsp_response(ssl_.get(), &ocsp_response_raw, &ocsp_response_len);
-  std::string ocsp_response;
-  if (ocsp_response_len > 0) {
-    ocsp_response_.assign(reinterpret_cast<const char*>(ocsp_response_raw),
-                          ocsp_response_len);
-  }
   set_stapled_ocsp_response_received(ocsp_response_len != 0);
   UMA_HISTOGRAM_BOOLEAN("Net.OCSPResponseStapled", ocsp_response_len != 0);
 
   const uint8_t* sct_list;
   size_t sct_list_len;
   SSL_get0_signed_cert_timestamp_list(ssl_.get(), &sct_list, &sct_list_len);
   set_signed_cert_timestamps_received(sct_list_len != 0);
 
   if (IsRenegotiationAllowed())
     SSL_set_renegotiate_mode(ssl_.get(), ssl_renegotiate_freely);
@@ skipping 61 matching lines @@
   CertStatus cert_status;
   if (ssl_config_.IsAllowedBadCert(server_cert_.get(), &cert_status)) {
     server_cert_verify_result_.Reset();
     server_cert_verify_result_.cert_status = cert_status;
     server_cert_verify_result_.verified_cert = server_cert_;
     return OK;
   }
 
   start_cert_verification_time_ = base::TimeTicks::Now();
 
+  const uint8_t* ocsp_response_raw;
+  size_t ocsp_response_len;
+  SSL_get0_ocsp_response(ssl_.get(), &ocsp_response_raw, &ocsp_response_len);
+  base::StringPiece ocsp_response(
+      reinterpret_cast<const char*>(ocsp_response_raw), ocsp_response_len);
+
   return cert_verifier_->Verify(
       CertVerifier::RequestParams(server_cert_, host_and_port_.host(),
                                   ssl_config_.GetCertVerifyFlags(),
-                                  ocsp_response_, CertificateList()),
+                                  ocsp_response.as_string(), CertificateList()),
       // TODO(davidben): Route the CRLSet through SSLConfig so
       // SSLClientSocket doesn't depend on SSLConfigService.
       SSLConfigService::GetCRLSet().get(), &server_cert_verify_result_,
       base::Bind(&SSLClientSocketImpl::OnHandshakeIOComplete,
                  base::Unretained(this)),
       &cert_verifier_request_, net_log_);
 }
 
 int SSLClientSocketImpl::DoVerifyCertComplete(int result) {
   cert_verifier_request_.reset();
@@ skipping 37 matching lines @@
       result = ct_result;
   }
 
   if (result == OK) {
     DCHECK(!certificate_verified_);
     certificate_verified_ = true;
     MaybeCacheSession();
     SSLInfo ssl_info;
     bool ok = GetSSLInfo(&ssl_info);
     DCHECK(ok);
+
+    const uint8_t* ocsp_response_raw;
+    size_t ocsp_response_len;
+    SSL_get0_ocsp_response(ssl_.get(), &ocsp_response_raw, &ocsp_response_len);
+    base::StringPiece ocsp_response(
+        reinterpret_cast<const char*>(ocsp_response_raw), ocsp_response_len);
+
     transport_security_state_->CheckExpectStaple(host_and_port_, ssl_info,
-                                                 ocsp_response_);
+                                                 ocsp_response);
   }
 
   completed_connect_ = true;
   // Exit DoHandshakeLoop and return the result to the caller to Connect.
   DCHECK_EQ(STATE_NONE, next_handshake_state_);
   return result;
 }
 
 void SSLClientSocketImpl::DoConnectCallback(int rv) {
   if (!user_connect_callback_.is_null()) {
@@ skipping 216 matching lines @@
     return;
 
   if (rv_write != ERR_IO_PENDING)
     DoWriteCallback(rv_write);
 }
 
 int SSLClientSocketImpl::VerifyCT() {
   const uint8_t* sct_list_raw;
   size_t sct_list_len;
   SSL_get0_signed_cert_timestamp_list(ssl_.get(), &sct_list_raw, &sct_list_len);
-  std::string sct_list;
-  if (sct_list_len > 0)
-    sct_list.assign(reinterpret_cast<const char*>(sct_list_raw), sct_list_len);
+  base::StringPiece sct_list(reinterpret_cast<const char*>(sct_list_raw),
+                             sct_list_len);
+
+  const uint8_t* ocsp_response_raw;

[eroman, 2016/12/27 22:00:41]

Extract to a helper?

[Ryan Sleevi, 2016/12/27 22:17:19]

I don't believe the helper here helps readability. The previous attempt to
extract added unnecessary memory overheads; this code makes it clearer.

+  size_t ocsp_response_len;
+  SSL_get0_ocsp_response(ssl_.get(), &ocsp_response_raw, &ocsp_response_len);

[eroman, 2016/12/27 22:00:41]

Not very familiar with the interactions here, but I am assuming the OCSP
response is accessible here and has the same value as where it was snapshotted
in the old code.

[Ryan Sleevi, 2016/12/27 22:17:19]

Correct.

+  base::StringPiece ocsp_response(
+      reinterpret_cast<const char*>(ocsp_response_raw), ocsp_response_len);
 
   // Note that this is a completely synchronous operation: The CT Log Verifier
   // gets all the data it needs for SCT verification and does not do any
   // external communication.
   cert_transparency_verifier_->Verify(
-      server_cert_verify_result_.verified_cert.get(), ocsp_response_, sct_list,
+      server_cert_verify_result_.verified_cert.get(), ocsp_response, sct_list,
       &ct_verify_result_.scts, net_log_);
 
   ct_verify_result_.ct_policies_applied = true;
   ct_verify_result_.ev_policy_compliance =
       ct::EVPolicyCompliance::EV_POLICY_DOES_NOT_APPLY;
 
   SCTList verified_scts =
       ct::SCTsMatchingStatus(ct_verify_result_.scts, ct::SCT_STATUS_OK);
 
   if (server_cert_verify_result_.cert_status & CERT_STATUS_IS_EV) {
@@ skipping 466 matching lines @@
     if (ERR_GET_REASON(info->error_code) == SSL_R_TLSV1_ALERT_ACCESS_DENIED &&
         !certificate_requested_) {
       net_error = ERR_SSL_PROTOCOL_ERROR;
     }
   }
 
   return net_error;
 }
 
 }  // namespace net