Separate CertVerifyProcAndroid from CertVerifyProcOpenSSL

net/base/cert_verify_proc_openssl.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/base/cert_verify_proc_openssl.h"
 
 #include <openssl/x509v3.h>
 
 #include <string>
 #include <vector>
 
 #include "base/logging.h"
 #include "base/sha1.h"
 #include "crypto/openssl_util.h"
 #include "crypto/sha2.h"
 #include "net/base/asn1_util.h"
 #include "net/base/cert_status_flags.h"
 #include "net/base/cert_verifier.h"
 #include "net/base/cert_verify_result.h"
 #include "net/base/net_errors.h"
 #include "net/base/x509_certificate.h"
 
-#if defined(OS_ANDROID)
-#include "net/android/network_library.h"
-#endif
-
 namespace net {
 
 namespace {
 
 // Maps X509_STORE_CTX_get_error() return values to our cert status flags.
 CertStatus MapCertErrorToCertStatus(int err) {
   switch (err) {
     case X509_V_ERR_SUBJECT_ISSUER_MISMATCH:
       return CERT_STATUS_COMMON_NAME_INVALID;
     case X509_V_ERR_CERT_NOT_YET_VALID:
@@ skipping 117 matching lines @@
     base::SHA1HashBytes(reinterpret_cast<const uint8*>(spki_bytes.data()),
                         spki_bytes.size(), sha1.data());
     hashes->push_back(sha1);
 
     HashValue sha256(HASH_VALUE_SHA256);
     crypto::SHA256HashString(spki_bytes, sha1.data(), crypto::kSHA256Length);
     hashes->push_back(sha256);
   }
 }
 
-#if defined(OS_ANDROID)
-// Returns true if we have verification result in |verify_result| from Android
-// Trust Manager. Otherwise returns false.
-bool VerifyFromAndroidTrustManager(const std::vector<std::string>& cert_bytes,
-                                   CertVerifyResult* verify_result) {
-  // TODO(joth): Fetch the authentication type from SSL rather than hardcode.
-  bool verified = true;
-  android::VerifyResult result =
-      android::VerifyX509CertChain(cert_bytes, "RSA");
-  switch (result) {
-    case android::VERIFY_OK:
-      break;
-    case android::VERIFY_NO_TRUSTED_ROOT:
-      verify_result->cert_status |= CERT_STATUS_AUTHORITY_INVALID;
-      break;
-    case android::VERIFY_INVOCATION_ERROR:
-      verified = false;
-      break;
-    default:
-      verify_result->cert_status |= CERT_STATUS_INVALID;
-      break;
-  }
-  return verified;
-}
-
-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;
-    X509Certificate::X509Certificate::GetDEREncoded(*it, &cert_bytes);
-    chain_bytes->push_back(cert_bytes);
-  }
-}
-#endif  // defined(OS_ANDROID)
-
 }  // namespace
 
 CertVerifyProcOpenSSL::CertVerifyProcOpenSSL() {}
 
 CertVerifyProcOpenSSL::~CertVerifyProcOpenSSL() {}
 
 int CertVerifyProcOpenSSL::VerifyInternal(X509Certificate* cert,
                                           const std::string& hostname,
                                           int flags,
                                           CRLSet* crl_set,
                                           CertVerifyResult* verify_result) {
   crypto::EnsureOpenSSLInit();
 
   if (!cert->VerifyNameMatch(hostname))
     verify_result->cert_status |= CERT_STATUS_COMMON_NAME_INVALID;
 
-  bool verify_attempted = false;
+  crypto::ScopedOpenSSL<X509_STORE_CTX, X509_STORE_CTX_free> ctx(
+      X509_STORE_CTX_new());
 
-#if defined(OS_ANDROID)
-  std::vector<std::string> cert_bytes;
-  GetChainDEREncodedBytes(cert, &cert_bytes);
+  crypto::ScopedOpenSSL<STACK_OF(X509), sk_X509_free_fn> intermediates(
+      sk_X509_new_null());
+  if (!intermediates.get())
+    return ERR_OUT_OF_MEMORY;
 
-  verify_attempted = VerifyFromAndroidTrustManager(cert_bytes, verify_result);
-#endif
+  const X509Certificate::OSCertHandles& os_intermediates =
+      cert->GetIntermediateCertificates();
+  for (X509Certificate::OSCertHandles::const_iterator it =
+       os_intermediates.begin(); it != os_intermediates.end(); ++it) {
+    if (!sk_X509_push(intermediates.get(), *it))
+      return ERR_OUT_OF_MEMORY;
+  }
+  int rv = X509_STORE_CTX_init(ctx.get(), X509Certificate::cert_store(),
+                               cert->os_cert_handle(), intermediates.get());
+  CHECK_EQ(1, rv);

[Ryan Sleevi, 2012/12/13 02:16:36]

[existing?] BUG: I don't think this should be a CHECK - both
|cert->os_Cert_handle()| and |intermediates.get()| represent potentially
'attacker' (read: user) input.

[ppi, 2012/12/13 04:49:09]

Nice catch! I will address this separately: crbug.com/165858 .

 
-  if (verify_attempted) {
-    if (IsCertStatusError(verify_result->cert_status))
-      return MapCertStatusToNetError(verify_result->cert_status);
-  } else {
-    crypto::ScopedOpenSSL<X509_STORE_CTX, X509_STORE_CTX_free> ctx(
-        X509_STORE_CTX_new());
+  if (X509_verify_cert(ctx.get()) != 1) {
+    int x509_error = X509_STORE_CTX_get_error(ctx.get());
+    CertStatus cert_status = MapCertErrorToCertStatus(x509_error);
+    LOG(ERROR) << "X509 Verification error "
+        << X509_verify_cert_error_string(x509_error)
+        << " : " << x509_error
+        << " : " << X509_STORE_CTX_get_error_depth(ctx.get())
+        << " : " << cert_status;
+    verify_result->cert_status |= cert_status;
+  }
 
-    crypto::ScopedOpenSSL<STACK_OF(X509), sk_X509_free_fn> intermediates(
-        sk_X509_new_null());
-    if (!intermediates.get())
-      return ERR_OUT_OF_MEMORY;
-
-    const X509Certificate::OSCertHandles& os_intermediates =
-        cert->GetIntermediateCertificates();
-    for (X509Certificate::OSCertHandles::const_iterator it =
-         os_intermediates.begin(); it != os_intermediates.end(); ++it) {
-      if (!sk_X509_push(intermediates.get(), *it))
-        return ERR_OUT_OF_MEMORY;
-    }
-    int rv = X509_STORE_CTX_init(ctx.get(), X509Certificate::cert_store(),
-                                 cert->os_cert_handle(), intermediates.get());
-    CHECK_EQ(1, rv);
-
-    if (X509_verify_cert(ctx.get()) != 1) {
-      int x509_error = X509_STORE_CTX_get_error(ctx.get());
-      CertStatus cert_status = MapCertErrorToCertStatus(x509_error);
-      LOG(ERROR) << "X509 Verification error "
-          << X509_verify_cert_error_string(x509_error)
-          << " : " << x509_error
-          << " : " << X509_STORE_CTX_get_error_depth(ctx.get())
-          << " : " << cert_status;
-      verify_result->cert_status |= cert_status;
-    }
-
-    GetCertChainInfo(ctx.get(), verify_result);
-    AppendPublicKeyHashes(ctx.get(), &verify_result->public_key_hashes);
-    if (IsCertStatusError(verify_result->cert_status))
-      return MapCertStatusToNetError(verify_result->cert_status);
-  }
+  GetCertChainInfo(ctx.get(), verify_result);
+  AppendPublicKeyHashes(ctx.get(), &verify_result->public_key_hashes);
+  if (IsCertStatusError(verify_result->cert_status))
+    return MapCertStatusToNetError(verify_result->cert_status);
 
   // Currently we only ues OpenSSL's default root CA paths, so treat all
   // correctly verified certs as being from a known root.
   // TODO(joth): if the motivations described in
   // http://src.chromium.org/viewvc/chrome?view=rev&revision=80778 become an
   // issue on OpenSSL builds, we will need to embed a hardcoded list of well
   // known root CAs, as per the _mac and _win versions.
   verify_result->is_issued_by_known_root = true;
 
   return OK;
 }
 
 }  // namespace net