Cleanup instances of weird pattern for getting cert chain as a vector of strings.

Also adds x509_util::CryptoBufferAsStringPiece as a more efficient replacement for X509Certificate::GetDEREncoded.

Bug: 769549
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I4c548dc67ee89ce29f7343a0031d7add5bf16d65
Reviewed-on: https://chromium-review.googlesource.com/762191Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Moe Ahmadi (slow) <mahmadi@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Commit-Queue: Matt Mueller <mattm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516984}

chrome/browser/ui/android/page_info/certificate_chain_helper.cc

@@ -11,6 +11,7 @@
 #include "content/public/browser/web_contents.h"
 #include "jni/CertificateChainHelper_jni.h"
 #include "net/cert/x509_certificate.h"
+#include "net/cert/x509_util.h"
 #include "ui/base/l10n/l10n_util.h"
 
 using base::android::JavaParamRef;
@@ -32,19 +33,11 @@ static ScopedJavaLocalRef<jobjectArray> GetCertificateChain(
     return ScopedJavaLocalRef<jobjectArray>();
 
   std::vector<std::string> cert_chain;
-  net::X509Certificate::OSCertHandles cert_handles =
-      cert->GetIntermediateCertificates();
-  // Make sure the peer's own cert is the first in the chain, if it's not
-  // already there.
-  if (cert_handles.empty() || cert_handles[0] != cert->os_cert_handle())
-    cert_handles.insert(cert_handles.begin(), cert->os_cert_handle());
-
-  cert_chain.reserve(cert_handles.size());
-  for (auto* handle : cert_handles) {
-    std::string cert_bytes;
-    net::X509Certificate::GetDEREncoded(handle, &cert_bytes);
-    cert_chain.push_back(cert_bytes);
-  }
+  cert_chain.reserve(1 + cert->GetIntermediateCertificates().size());
+  cert_chain.emplace_back(
+      net::x509_util::CryptoBufferAsStringPiece(cert->os_cert_handle()));
+  for (auto* handle : cert->GetIntermediateCertificates())
+    cert_chain.emplace_back(net::x509_util::CryptoBufferAsStringPiece(handle));
 
   return base::android::ToJavaArrayOfByteArray(env, cert_chain);
 }

ios/chrome/browser/payments/ios_payment_instrument_launcher.mm

@@ -24,6 +24,7 @@
 #include "net/base/mac/url_conversions.mm"
 #include "net/base/url_util.h"
 #include "net/cert/x509_certificate.h"
+#include "net/cert/x509_util.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -218,23 +219,18 @@ IOSPaymentInstrumentLauncher::SerializeCertificateChain(
     return cert_chain_list;
 
   scoped_refptr<net::X509Certificate> cert = item->GetSSL().certificate;
-  std::vector<std::vector<const char>> cert_chain;
-  net::X509Certificate::OSCertHandles cert_handles =
-      cert->GetIntermediateCertificates();
-  if (cert_handles.empty() || cert_handles[0] != cert->os_cert_handle())
-    cert_handles.insert(cert_handles.begin(), cert->os_cert_handle());
-
-  cert_chain.reserve(cert_handles.size());
-  for (auto* handle : cert_handles) {
-    std::string cert_bytes;
-    net::X509Certificate::GetDEREncoded(handle, &cert_bytes);
-    cert_chain.push_back(
-        std::vector<const char>(cert_bytes.begin(), cert_bytes.end()));
-  }
+  std::vector<base::StringPiece> cert_chain;
+
+  cert_chain.reserve(1 + cert->GetIntermediateCertificates().size());
+  cert_chain.push_back(
+      net::x509_util::CryptoBufferAsStringPiece(cert->os_cert_handle()));
+  for (auto* handle : cert->GetIntermediateCertificates())
+    cert_chain.push_back(net::x509_util::CryptoBufferAsStringPiece(handle));
 
   std::unique_ptr<base::ListValue> byte_array;
-  for (std::vector<const char> cert_string : cert_chain) {
+  for (const auto& cert_string : cert_chain) {
     base::ListValue byte_array;
+    byte_array.GetList().reserve(cert_string.size());
     for (const char byte : cert_string)
       byte_array.GetList().emplace_back(byte);
 

net/cert/cert_verify_proc_android.cc

@@ -315,26 +315,15 @@ bool VerifyFromAndroidTrustManager(
   return true;
 }
 
-bool GetChainDEREncodedBytes(X509Certificate* cert,
+void GetChainDEREncodedBytes(X509Certificate* cert,
                              std::vector<std::string>* chain_bytes) {
-  X509Certificate::OSCertHandle cert_handle = cert->os_cert_handle();
-  X509Certificate::OSCertHandles cert_handles =
-      cert->GetIntermediateCertificates();
-
-  // Make sure the peer's own cert is the first in the chain, if it's not
-  // already there.
-  if (cert_handles.empty() || cert_handles[0] != cert_handle)
-    cert_handles.insert(cert_handles.begin(), cert_handle);
-
-  chain_bytes->reserve(cert_handles.size());
-  for (X509Certificate::OSCertHandles::const_iterator it =
-       cert_handles.begin(); it != cert_handles.end(); ++it) {
-    std::string cert_bytes;
-    if(!X509Certificate::GetDEREncoded(*it, &cert_bytes))
-      return false;
-    chain_bytes->push_back(cert_bytes);
+  chain_bytes->reserve(1 + cert->GetIntermediateCertificates().size());
+  chain_bytes->emplace_back(
+      net::x509_util::CryptoBufferAsStringPiece(cert->os_cert_handle()));
+  for (auto* handle : cert->GetIntermediateCertificates()) {
+    chain_bytes->emplace_back(
+        net::x509_util::CryptoBufferAsStringPiece(handle));
   }
-  return true;
 }
 
 }  // namespace
@@ -360,8 +349,7 @@ int CertVerifyProcAndroid::VerifyInternal(
     const CertificateList& additional_trust_anchors,
     CertVerifyResult* verify_result) {
   std::vector<std::string> cert_bytes;
-  if (!GetChainDEREncodedBytes(cert, &cert_bytes))
-    return ERR_CERT_INVALID;
+  GetChainDEREncodedBytes(cert, &cert_bytes);
   if (!VerifyFromAndroidTrustManager(
           cert_bytes, hostname, GetGlobalCertNetFetcher(), verify_result)) {
     NOTREACHED();

net/cert/x509_util.cc

@@ -393,6 +393,12 @@ bssl::UniquePtr<CRYPTO_BUFFER> CreateCryptoBuffer(
                         data.size(), GetBufferPool()));
 }
 
+base::StringPiece CryptoBufferAsStringPiece(const CRYPTO_BUFFER* buffer) {
+  return base::StringPiece(
+      reinterpret_cast<const char*>(CRYPTO_BUFFER_data(buffer)),
+      CRYPTO_BUFFER_len(buffer));
+}
+
 scoped_refptr<X509Certificate> CreateX509CertificateFromBuffers(
     STACK_OF(CRYPTO_BUFFER) * buffers) {
   if (sk_CRYPTO_BUFFER_num(buffers) == 0) {

net/cert/x509_util.h

@@ -102,6 +102,10 @@ NET_EXPORT bssl::UniquePtr<CRYPTO_BUFFER> CreateCryptoBuffer(
 NET_EXPORT bssl::UniquePtr<CRYPTO_BUFFER> CreateCryptoBuffer(
     const char* invalid_data);
 
+// Returns a StringPiece pointing to the data in |buffer|.
+NET_EXPORT base::StringPiece CryptoBufferAsStringPiece(
+    const CRYPTO_BUFFER* buffer);
+
 // Creates a new X509Certificate from the chain in |buffers|, which must have at
 // least one element.
 scoped_refptr<X509Certificate> CreateX509CertificateFromBuffers(