Separate CertVerifyProcAndroid from CertVerifyProcOpenSSL

net/base/cert_verify_proc_android.cc

Index: net/base/cert_verify_proc_android.cc
diff --git a/net/base/cert_verify_proc_android.cc b/net/base/cert_verify_proc_android.cc
new file mode 100644
index 0000000000000000000000000000000000000000..1bdbcef87b22eb71937be686f02be26d6d194943
--- /dev/null
+++ b/net/base/cert_verify_proc_android.cc
@@ -0,0 +1,94 @@
+// 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_android.h"
+
+#include <string>
+#include <vector>
+
+#include "base/logging.h"
+#include "net/android/network_library.h"
+#include "net/base/cert_status_flags.h"
+#include "net/base/cert_verify_result.h"
+#include "net/base/net_errors.h"
+#include "net/base/x509_certificate.h"
+
+namespace net {
+
+namespace {
+
+// Returns true if the certificate verification call was successful (regardless
+// of its result), i.e. if |verify_result| was set. 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;
+}
+
+bool 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)

[Ryan Sleevi, 2012/12/13 19:50:50]

This second condition should always be false. It's a bug if cert_handles[0] ever
== cert_handle.

For further cleanup though.

[ppi, 2012/12/13 21:06:39]

Thanks! If you don't mind I will address this separately.

+    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);
+  }
+  return true;
+}
+
+}  // namespace
+
+CertVerifyProcAndroid::CertVerifyProcAndroid() {}
+
+CertVerifyProcAndroid::~CertVerifyProcAndroid() {}
+
+int CertVerifyProcAndroid::VerifyInternal(X509Certificate* cert,
+                                          const std::string& hostname,
+                                          int flags,
+                                          CRLSet* crl_set,
+                                          CertVerifyResult* verify_result) {
+  if (!cert->VerifyNameMatch(hostname))
+    verify_result->cert_status |= CERT_STATUS_COMMON_NAME_INVALID;
+
+  std::vector<std::string> cert_bytes;
+  if (!GetChainDEREncodedBytes(cert, &cert_bytes))
+    return ERR_CERT_INVALID;
+  if (!VerifyFromAndroidTrustManager(cert_bytes, verify_result)) {
+    NOTREACHED();
+    return ERR_FAILED;
+  }
+  if (IsCertStatusError(verify_result->cert_status))
+    return MapCertStatusToNetError(verify_result->cert_status);

[joth, 2012/12/14 00:45:01]

Bug?
I think prior to this patch even on OS_ANDROID we'd be setting
verify_result->is_issued_by_known_root = true here.

[ppi, 2012/12/14 04:02:32]

My bad, thanks for catching that! I am fixing this in
https://codereview.chromium.org/11570019/ .

+

[joth, 2012/12/14 00:45:01]

should there be a TODO / bug link here to implement the other missing bits?
- GetCertChainInfo(ctx.get(), verify_result);
- AppendPublicKeyHashes()


they depend on  crbug.com/116838 IIRC.

[ppi, 2012/12/14 04:02:32]

Good idea, thanks. Being addressed in https://codereview.chromium.org/11570019/.

+  return OK;
+}
+
+}  // namespace net