Adding iOS OpenSSL Implementation

net/cert/cert_verify_proc_openssl_ios.cc

+// Copyright (c) 2016 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/cert/cert_verify_proc_openssl_ios.h"
+
+#include <CommonCrypto/CommonDigest.h>
+#include <Security/Security.h>
+
+#include "base/logging.h"
+#include "base/mac/scoped_cftyperef.h"
+#include "base/strings/sys_string_conversions.h"
+#include "crypto/sha2.h"
+#include "net/base/net_errors.h"
+#include "net/cert/asn1_util.h"
+#include "net/cert/cert_verify_result.h"
+#include "net/cert/crl_set.h"
+#include "net/cert/test_root_certs.h"
+#include "net/cert/x509_certificate.h"
+#include "net/ssl/openssl_ssl_util.h"
+
+using base::ScopedCFTypeRef;
+
+namespace net {
+
+namespace {
+
+typedef OSStatus (*SecTrustCopyExtendedResultFuncPtr)(SecTrustRef,
+                                                      CFDictionaryRef*);
+
+int NetErrorFromOSStatus(OSStatus status) {
+  switch (status) {
+    case noErr:
+      return OK;
+    case errSecNotAvailable:
+      return ERR_NOT_IMPLEMENTED;
+    case errSecAuthFailed:
+      return ERR_ACCESS_DENIED;

[Ryan Sleevi, 2016/03/21 20:49:55]

This doesn't seem right, because it means mapping a non-cert error code into the
cert verification error space. Do we have precedent for this?

[svaldez, 2016/03/21 21:36:26]

This is the same thing we do  for cert_verify_proc_mac.

+    default:
+      return ERR_FAILED;
+  }
+}
+
+// Creates a series of SecPolicyRefs to be added to a SecTrustRef used to
+// validate a certificate for an SSL server. |hostname| contains the name of
+// the SSL server that the certificate should be verified against. If
+// successful, returns noErr, and stores the resultant array of SecPolicyRefs
+// in |policies|.
+OSStatus CreateTrustPolicies(const std::string& hostname,
+                             ScopedCFTypeRef<CFArrayRef>* policies) {
+  ScopedCFTypeRef<CFMutableArrayRef> local_policies(
+      CFArrayCreateMutable(kCFAllocatorDefault, 0, &kCFTypeArrayCallBacks));
+  if (!local_policies)
+    return errSecAllocate;
+
+  SecPolicyRef ssl_policy = SecPolicyCreateBasicX509();
+  CFArrayAppendValue(local_policies, ssl_policy);
+  CFRelease(ssl_policy);
+  ssl_policy = SecPolicyCreateSSL(true, base::SysUTF8ToCFStringRef(hostname));

[Ryan Sleevi, 2016/03/21 20:49:55]

I'm torn as to whether or not we want to do this, because this means we'll be
relying on iOS's hostname verification. I think we want to continue to use
X509Certificate's (as we do on every other platform)

[svaldez, 2016/03/21 21:36:26]

Will drop the hostname verification for now.

+  CFArrayAppendValue(local_policies, ssl_policy);
+  CFRelease(ssl_policy);
+
+  policies->reset(local_policies.release());
+  return noErr;
+}
+
+// Builds and evaluates a SecTrustRef for the certificate chain contained
+// in |cert_array|, using the verification policies in |trust_policies|. On
+// success, returns OK, and updates |trust_ref| and |trust_result|. On failure,
+// no output parameters are modified.
+//
+// Note: An OK return does not mean that |cert_array| is trusted, merely that
+// verification was performed successfully.
+int BuildAndEvaluateSecTrustRef(CFArrayRef cert_array,
+                                CFArrayRef trust_policies,
+                                ScopedCFTypeRef<SecTrustRef>* trust_ref,
+                                ScopedCFTypeRef<CFArrayRef>* verified_chain,
+                                SecTrustResultType* trust_result) {
+  SecTrustRef tmp_trust = NULL;
+  OSStatus status =
+      SecTrustCreateWithCertificates(cert_array, trust_policies, &tmp_trust);
+  if (status)
+    return NetErrorFromOSStatus(status);
+  ScopedCFTypeRef<SecTrustRef> scoped_tmp_trust(tmp_trust);
+
+  if (TestRootCerts::HasInstance()) {
+    status = TestRootCerts::GetInstance()->FixupSecTrustRef(tmp_trust);
+    if (status)
+      return NetErrorFromOSStatus(status);
+  }
+
+  SecTrustResultType tmp_trust_result;
+  status = SecTrustEvaluate(tmp_trust, &tmp_trust_result);
+  if (status)
+    return NetErrorFromOSStatus(status);
+
+  ScopedCFTypeRef<CFMutableArrayRef> tmp_verified_chain(
+      CFArrayCreateMutable(kCFAllocatorDefault, 0, &kCFTypeArrayCallBacks));
+  const CFIndex chain_length = SecTrustGetCertificateCount(tmp_trust);
+  for (CFIndex i = 0; i < chain_length; ++i) {
+    SecCertificateRef chain_cert = SecTrustGetCertificateAtIndex(tmp_trust, i);
+    CFArrayAppendValue(tmp_verified_chain, chain_cert);
+    CFRelease(chain_cert);
+  }
+
+  trust_ref->swap(scoped_tmp_trust);
+  *trust_result = tmp_trust_result;
+  verified_chain->reset(tmp_verified_chain.release());
+  return OK;
+}
+
+void GetCertChainInfo(CFArrayRef cert_chain,
+                      CertVerifyResult* verify_result,
+                      bool* leaf_is_weak) {
+  DCHECK_LT(0, CFArrayGetCount(cert_chain));
+
+  *leaf_is_weak = false;
+  verify_result->has_md2 = false;
+  verify_result->has_md4 = false;
+  verify_result->has_md5 = false;
+  verify_result->has_sha1 = false;
+  verify_result->has_sha1_leaf = false;
+
+  SecCertificateRef verified_cert = NULL;
+  std::vector<SecCertificateRef> verified_chain;
+  for (CFIndex i = 0, count = CFArrayGetCount(cert_chain); i < count; ++i) {
+    SecCertificateRef chain_cert =
+        X509Certificate::DupOSCertHandle(reinterpret_cast<SecCertificateRef>(

[Ryan Sleevi, 2016/03/21 20:49:55]

BUG: You end up leaking all of these certs. You shouldn't be DUPing them,
because CreateFromHandle does so internally.

[svaldez, 2016/03/21 21:36:26]

Is that the case, I had errors without this. The Retain from this Dup gets
cancelled out by the destruction of verified_chain. Then there's another
Retain/Release for the creation and destruction of the X509Certificate objects
later. If we don't Retain here, then the destruction of the input cert_chain
will result in the verified_chain references becoming invalid.

[Ryan Sleevi, 2016/03/21 21:44:14]

You mean the member |verified_chain|? If so, that doesn't get cancelled - it's
just an std::vector
Is that a bug in CreateFromHandle? The semantics are that CreateFromHandle
should CFRetain all of its input arguments (duping them). AFAICT, there are no
CFRelease()s in this function - so you should have two refs.

The only thing I can think of is some specialization of std::vector<> to
CFRelease, but that surely doesn't sound right. Just from this function, though,
it should be fully safe to create a new X509Certificate from the
GetIntermediateCertificates() of another X509Certificate (which doesn't do any
CFRetain before returning), which means it should be fully unnecessary to
CFRetain here (via DupOSCertHandle)

[svaldez, 2016/03/22 15:05:56]

Fixed. There was an extra CFRelease I was doing up above.

+            const_cast<void*>(CFArrayGetValueAtIndex(cert_chain, i))));
+    if (i == 0) {
+      verified_cert = chain_cert;
+    } else {
+      verified_chain.push_back(chain_cert);
+    }
+
+    ScopedX509 x509_cert = OSCertHandleToOpenSSL(chain_cert);
+    int sig_alg = OBJ_obj2nid(x509_cert->sig_alg->algorithm);
+    if (sig_alg == NID_md2WithRSAEncryption) {
+      verify_result->has_md2 = true;
+      if (i == 0)
+        *leaf_is_weak = true;
+    } else if (sig_alg == NID_md4WithRSAEncryption) {
+      verify_result->has_md4 = true;
+      if (i == 0)
+        *leaf_is_weak = true;
+    } else if (sig_alg == NID_md5WithRSAEncryption ||
+               sig_alg == NID_md5WithRSA) {
+      verify_result->has_md5 = true;
+      if (i == 0)
+        *leaf_is_weak = true;
+    } else if (sig_alg == NID_sha1WithRSAEncryption ||
+               sig_alg == NID_dsaWithSHA || sig_alg == NID_dsaWithSHA1 ||
+               sig_alg == NID_dsaWithSHA1_2 || sig_alg == NID_sha1WithRSA ||
+               sig_alg == NID_ecdsa_with_SHA1) {
+      verify_result->has_sha1 = true;
+      if (i == 0) {
+        verify_result->has_sha1_leaf = true;
+        *leaf_is_weak = true;
+      }
+    }
+  }
+  if (!verified_cert) {
+    NOTREACHED();
+    return;
+  }
+
+  verify_result->verified_cert =
+      X509Certificate::CreateFromHandle(verified_cert, verified_chain);
+}
+
+void AppendPublicKeyHashes(CFArrayRef chain, HashValueVector* hashes) {
+  const CFIndex n = CFArrayGetCount(chain);
+  for (CFIndex i = 0; i < n; i++) {
+    SecCertificateRef cert = reinterpret_cast<SecCertificateRef>(
+        const_cast<void*>(CFArrayGetValueAtIndex(chain, i)));
+
+    std::string der_bytes;
+    if (!X509Certificate::GetDEREncoded(cert, &der_bytes))
+      return;
+    base::StringPiece spki_bytes;
+    if (!asn1::ExtractSPKIFromDERCert(der_bytes, &spki_bytes))
+      continue;
+
+    HashValue sha1(HASH_VALUE_SHA1);
+    CC_SHA1(spki_bytes.data(), spki_bytes.size(), sha1.data());
+    hashes->push_back(sha1);
+
+    HashValue sha256(HASH_VALUE_SHA256);
+    CC_SHA256(spki_bytes.data(), spki_bytes.size(), sha256.data());
+    hashes->push_back(sha256);
+  }
+}
+
+bool CheckRevocationWithCRLSet(CFArrayRef chain, CRLSet* crl_set) {

[Ryan Sleevi, 2016/03/21 20:49:55]

I don't think we want/need this behaviour, given that Android also doesn't
support CRLSets.

Feel free to leave this off and file a bug on me, because if we do add CRLSet
behaviour to iOS, we want to make sure it does the right thing, and right now,
it won't.

[svaldez, 2016/03/21 21:36:26]

Done.

+  if (CFArrayGetCount(chain) == 0)
+    return true;
+
+  // We iterate from the root certificate down to the leaf, keeping track of
+  // the issuer's SPKI at each step.
+  std::string issuer_spki_hash;
+  for (CFIndex i = CFArrayGetCount(chain) - 1; i >= 0; i--) {
+    SecCertificateRef cert = reinterpret_cast<SecCertificateRef>(
+        const_cast<void*>(CFArrayGetValueAtIndex(chain, i)));
+
+    std::string der_bytes;
+    if (!X509Certificate::GetDEREncoded(cert, &der_bytes))
+      return false;
+    base::StringPiece spki;
+    if (!asn1::ExtractSPKIFromDERCert(der_bytes, &spki)) {
+      NOTREACHED();
+      continue;
+    }
+
+    const std::string spki_hash = crypto::SHA256HashString(spki);
+    scoped_refptr<X509Certificate::X509Certificate> x509_cert =
+        X509Certificate::CreateFromHandle(cert,
+                                          X509Certificate::OSCertHandles());
+
+    CRLSet::Result result = crl_set->CheckSPKI(spki_hash);
+
+    if (result != CRLSet::REVOKED && !issuer_spki_hash.empty()) {
+      result =
+          crl_set->CheckSerial(x509_cert->serial_number(), issuer_spki_hash);
+    }
+
+    issuer_spki_hash = spki_hash;
+
+    switch (result) {
+      case CRLSet::REVOKED:
+        return false;
+      case CRLSet::UNKNOWN:
+      case CRLSet::GOOD:
+        continue;
+      default:
+        NOTREACHED();
+        return false;
+    }
+  }
+
+  return true;
+}
+
+}  // namespace
+
+CertVerifyProcIOS::CertVerifyProcIOS() {}
+
+CertVerifyProcIOS::~CertVerifyProcIOS() {}
+
+bool CertVerifyProcIOS::SupportsAdditionalTrustAnchors() const {
+  return false;
+}
+
+bool CertVerifyProcIOS::SupportsOCSPStapling() const {
+  return false;
+}
+
+int CertVerifyProcIOS::VerifyInternal(
+    X509Certificate* cert,
+    const std::string& hostname,
+    const std::string& ocsp_response,
+    int flags,
+    CRLSet* crl_set,
+    const CertificateList& additional_trust_anchors,
+    CertVerifyResult* verify_result) {
+  ScopedCFTypeRef<CFArrayRef> trust_policies;
+  OSStatus status = CreateTrustPolicies(hostname, &trust_policies);
+  if (status)
+    return NetErrorFromOSStatus(status);
+
+  ScopedCFTypeRef<CFMutableArrayRef> cert_array(
+      cert->CreateOSCertChainForCert());
+  ScopedCFTypeRef<SecTrustRef> trust_ref;
+  SecTrustResultType trust_result = kSecTrustResultDeny;
+  ScopedCFTypeRef<CFArrayRef> final_chain;
+
+  status = BuildAndEvaluateSecTrustRef(cert_array, trust_policies, &trust_ref,
+                                       &final_chain, &trust_result);
+  if (status)
+    return NetErrorFromOSStatus(status);
+
+  if (CFArrayGetCount(final_chain) > 0) {
+    bool unused_leaf_weak;
+    GetCertChainInfo(final_chain, verify_result, &unused_leaf_weak);
+  }
+
+  if (crl_set && !CheckRevocationWithCRLSet(final_chain, crl_set))
+    verify_result->cert_status |= CERT_STATUS_REVOKED;
+
+  switch (trust_result) {

[Ryan Sleevi, 2016/03/21 20:49:55]

BUG: No expiration handling is done here, which was the one requirement from
enamel (expired vs name mismatch vs everything else)

[svaldez, 2016/03/21 21:36:26]

Added a TODO. As it stands, this is mostly focused on Cronet's use case, since
there are still a bunch of edge cases we don't handle due to the low resolution
of iOS' API. I think adding Expiration Handling and some more fine-grained
status should be handled in a separate CL for making this usable enough for
Chrome. There's a couple other things that got removed from here that would also
need to be added back in. One thing, that I'm not sure if we want to keep, is
the manual cert chain building that we do in Mac, to get around the system
library possibly returning weak chains.

[Ryan Sleevi, 2016/03/21 21:44:14]

Yeah, I debated a comment to that effect, but iOS uses the new chain building
algorithm and new weighting, so I'm happy leaving off a lot of that logic for
now.

+    case kSecTrustResultUnspecified:
+    case kSecTrustResultProceed:
+      break;
+    case kSecTrustResultDeny:
+      verify_result->cert_status |= CERT_STATUS_AUTHORITY_INVALID;

[Ryan Sleevi, 2016/03/21 20:49:55]

I don't think this this the right mapping code; a Deny error is the user has
explicitly rejected the CA, not just that it's untrusted. I think REVOKED may be
more appropriate?

[svaldez, 2016/03/21 21:36:26]

For Mac we've been using AUTHORITY_INVALID in this case.

[Ryan Sleevi, 2016/03/21 21:44:14]

OK, fair enough.

+    default:
+      verify_result->cert_status |= CERT_STATUS_INVALID;
+  }
+
+  // Perform hostname verification independent of SecTrustEvaluate. In order to
+  // do so, mask off any reported name errors first.
+  verify_result->cert_status &= ~CERT_STATUS_COMMON_NAME_INVALID;

[Ryan Sleevi, 2016/03/21 20:49:55]

This is never set (per above), so it's impossible to mask off. Instead, hostname
mismatches will end up with CERT_STATUS_INVALID all the time.

[svaldez, 2016/03/21 21:36:26]

Done.

+  if (!cert->VerifyNameMatch(hostname,
+                             &verify_result->common_name_fallback_used)) {
+    verify_result->cert_status |= CERT_STATUS_COMMON_NAME_INVALID;
+  }
+
+  verify_result->cert_status &= ~CERT_STATUS_NO_REVOCATION_MECHANISM;

[Ryan Sleevi, 2016/03/21 20:49:55]

This is never set.

[svaldez, 2016/03/21 21:36:26]

Done.

+  AppendPublicKeyHashes(final_chain, &verify_result->public_key_hashes);
+  verify_result->is_issued_by_known_root = true;

[Ryan Sleevi, 2016/03/21 20:49:55]

BUG: This isn't correct. We default to false, not true, when we don't know.

(iOS doesn't expose to us the set of trust anchors, AFAIK)

[svaldez, 2016/03/21 21:36:26]

Done.

+
+  if (IsCertStatusError(verify_result->cert_status))
+    return MapCertStatusToNetError(verify_result->cert_status);
+
+  return OK;
+}
+
+}  // namespace net