PKI library Mac trust store integration

net/cert/internal/trust_store_mac.cc

+// Copyright 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/internal/trust_store_mac.h"
+
+#include <CoreFoundation/CoreFoundation.h>
+#include <Security/Security.h>
+
+#include "base/logging.h"
+#include "base/mac/foundation_util.h"
+#include "base/mac/mac_logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/synchronization/lock.h"
+#include "crypto/cssm_init.h"  // for LogCSSMError.
+#include "crypto/mac_security_services_lock.h"
+#include "net/cert/internal/cert_errors.h"
+#include "net/cert/internal/parse_name.h"
+#include "net/cert/internal/parsed_certificate.h"
+#include "net/cert/test_keychain_search_list_mac.h"
+#include "net/cert/x509_certificate.h"
+
+namespace net {
+
+// XXX for SecCertificateGetData and CSSM_DATA. Could use SecCertificateCopyData
+// instead.
+// CSSM functions are deprecated as of OSX 10.7, but have no replacement.
+// https://bugs.chromium.org/p/chromium/issues/detail?id=590914#c1
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"

[Ryan Sleevi, 2017/02/06 23:59:05]

I'm happy to just jump to SecCertificateCopyData to keep this clean and blessed.
Are there other deprecated functions beyond that?

If you did want to use GetData for the optimization path, we should probably
keep its usage localized to a helper function here (e.g. that returned a
StringPiece) so that we don't accidentally introduce additional usages elsewhere
in the file (keeping the ignoring scoping hyper-local via a diagnostic pop)

[mattm, 2017/02/09 23:40:16]

Just that one. I've changed to SecCertificateCopyData.

+
+namespace {
+
+// XXX
+// The overall trust settings for a certificate are the sum of all the usage
+// constraints dictionaries that match the use for which that certificate is
+// being evaluated. Trust settings for a given use apply if any  of the
+// dictionaries in the certificate’s trust settings array satisfies the
+// specified use. Thus, when a certificate has multiple usage constraints
+// dictionaries in its trust settings array, the overall trust settings for the
+// certificate are:
+//
+// ((usage constraint dictionary 0 component 0) AND (usage constraint dictionary
+// 0 component 1) AND (...)) OR ((usage constraint dictionary 1 component 0) AND
+// (usage constraint dictionary 1 component 1) AND (...)) OR (...) ...
+//
+// If the value of the kSecTrustSettingsResult component is not
+// kSecTrustSettingsResultUnspecified for a usage constraints dictionary that
+// has no constraints, the default value kSecTrustSettingsResultTrustRoot is
+// assumed. To specify a value for the kSecTrustSettingsAllowedError component
+// without explicitly trusting or distrusting the associated certificate,
+// specify a value of kSecTrustSettingsResultUnspecified for the
+// kSecTrustSettingsResult component.
+//
+// An empty trust settings array (that is, the trustSettings parameter returns a
+// valid but empty CFArray) means "always trust this certificate” with an
+// overall trust setting for the certificate of
+// kSecTrustSettingsResultTrustRoot. Note that an empty trust settings array is
+// not the same as no trust settings (the trustSettings parameter returns NULL),
+// which means "this certificate must be verified to a known trusted
+// certificate”.
+//
+// Note the distinction between the results kSecTrustSettingsResultTrustRoot and
+// kSecTrustSettingsResultTrustAsRoot: The former can only be applied to root
+// (self-signed) certificates; the latter can only be applied to non-root
+// certificates. Therefore, an empty trust settings array for a non-root
+// certificate is invalid, because the default value of
+// kSecTrustSettingsResultTrustRoot is not valid for a non-root certificate.

[Ryan Sleevi, 2017/02/06 23:59:04]

A++ documentation - would read again.

+
+bool IsTrustDictionaryTrustedForPolicy(CFDictionaryRef trust_dict,
+                                       bool is_self_signed,
+                                       const CFStringRef target_policy_oid) {
+  DVLOG(2) << "IsTrustDictionaryTrustedForPolicy?";
+  // An empty trust dict should be interpreted as
+  // kSecTrustSettingsResultTrustRoot. This is handled by falling through all
+  // the conditions below with the default value of |trust_settings_result|.
+
+  // Ignore application-specific trust settings.
+  // TODO: should handle these? (checking if there are application settings
+  // specific to chrome?)

[Ryan Sleevi, 2017/02/06 23:59:04]

I'm OK with saying no to this, because then it gets into the broader question of
code-signing identity. As it stands, there's no explicit way to create trust
settings for Chrome, because we don't expose the trust panel from the
SecTrustRef (which is what allows the per-app trust settings to be created)

[mattm, 2017/02/09 23:40:16]

Acknowledged.

+  if (CFDictionaryContainsKey(trust_dict, kSecTrustSettingsApplication)) {
+    DVLOG(2) << "ignored trust dictionary with kSecTrustSettingsApplication";
+    return false;
+  }
+  // Ignore trust settings that contain policy-specific constraints. (E.g.,
+  // for SSL, settings that apply to a single hostname.)
+  if (CFDictionaryContainsKey(trust_dict, kSecTrustSettingsPolicyString)) {
+    DVLOG(2) << "ignored trust dictionary with kSecTrustSettingsPolicyString";
+    return false;
+  }
+
+  // XXX do we care about keyusage?
+  // CFNumberRef key_usage =
+  //    base::mac::GetValueFromDictionary<CFNumberRef>(trust_dict,
+  //    kSecTrustSettingsKeyUsage);

[Ryan Sleevi, 2017/02/06 23:59:05]

I haven't actually seen where Apple surfaces this information as far as the UI
goes. That is, my understanding is that this is (mostly) redundant with the
policy flags. I haven't the slightest where they might use this, other than
perhaps iPSec (I presume client certs use the policy string, but perhaps thats a
bad assumption)

[mattm, 2017/02/09 23:40:16]

Acknowledged.

+
+  CFTypeRef policy_typeref =
+      CFDictionaryGetValue(trust_dict, kSecTrustSettingsPolicy);
+  SecPolicyRef policy_ref = nullptr;
+  if (policy_typeref) {
+    if (CFGetTypeID(policy_typeref) != SecPolicyGetTypeID()) {
+      DVLOG(2) << "kSecTrustSettingsPolicy not a SecPolicyRef";
+      return false;
+    }
+    policy_ref =
+        reinterpret_cast<SecPolicyRef>(const_cast<void*>(policy_typeref));
+  }
+  // XXX do some manual testing that this policy stuff actually works properly.
+  // XXX is there any hierarchy to the policies? (eg, does
+  // a trust dict with kSecPolicyAppleX509Basic apply to SSL verification?)

[Ryan Sleevi, 2017/02/06 23:59:05]

AIUI, no; each policy is treated as distinct (you can see the trust panel
settings in Safari when there is a certificate error, and how they populate to
the database)

[mattm, 2017/02/09 23:40:17]

I did some manual testing. If you set a cert as trusted for basic x509 in
keychain access, it actually creates two trust dicts, one with no
kSecTrustSettingsPolicy entry. So the evaluating code doesn't need to do
anything special to handle it, but it does end up being trusted for other uses
as well.

+  if (policy_ref) {
+    CFDictionaryRef policy_dict = SecPolicyCopyProperties(policy_ref);
+    CFTypeRef value = CFDictionaryGetValue(policy_dict, kSecPolicyOid);
+    // Having a kSecTrustSettingsPolicy without a kSecPolicyOid element is an
+    // error.
+    if (!value) {
+      DVLOG(2) << "kSecTrustSettingsPolicy with no kSecPolicyOid!";
+      return false;
+    }
+    CFStringRef policy_oid = base::mac::CFCast<CFStringRef>(value);
+    if (!policy_oid) {
+      DVLOG(2) << "kSecPolicyOid value not a CFStringRef!";
+      return false;
+    }
+    base::ScopedCFTypeRef<CFDictionaryRef> scoped_policy_dict(policy_dict);
+    if (!CFEqual(policy_oid, target_policy_oid)) {
+      DVLOG(2) << "kSecPolicyOid does not match target policy oid";
+      return false;
+    }
+    // XXX care about kSecPolicyName, kSecPolicyClient, kSecPolicyKU_<foo> ?
+  }
+
+  // If kSecTrustSettingsResult is not present in the trust dict,
+  // kSecTrustSettingsResultTrustRoot is assumed.
+  int trust_settings_result = kSecTrustSettingsResultTrustRoot;
+  CFNumberRef trust_settings_result_ref =
+      base::mac::GetValueFromDictionary<CFNumberRef>(trust_dict,
+                                                     kSecTrustSettingsResult);
+
+  if (trust_settings_result_ref) {
+    if (!CFNumberGetValue(trust_settings_result_ref, kCFNumberIntType,
+                          &trust_settings_result)) {
+      DVLOG(2) << "CFNumberGetValue fail";
+      return false;
+    }
+  }
+
+  // TODO: handle distrust (kSecTrustSettingsResultDeny)
+
+  // kSecTrustSettingsResultTrustRoot can only be applied to root(self-signed)
+  // certs.
+  if (is_self_signed)
+    return trust_settings_result == kSecTrustSettingsResultTrustRoot;
+
+  // kSecTrustSettingsResultTrustAsRoot can only be applied to non-root certs.
+  return trust_settings_result == kSecTrustSettingsResultTrustAsRoot;
+}
+
+bool IsTrustSettingsTrustedForPolicy(CFArrayRef trust_settings,
+                                     bool is_self_signed,
+                                     const CFStringRef policy_oid) {
+  DVLOG(2) << "IsTrustSettingsTrustedForPolicy?";
+  // An empty trust settings array (that is, the trustSettings parameter returns
+  // a valid but empty CFArray) means "always trust this certificate” with an
+  // overall trust setting for the certificate of
+  // kSecTrustSettingsResultTrustRoot.
+  if (CFArrayGetCount(trust_settings) == 0 && is_self_signed) {
+    DVLOG(2) << "Empty trust settings array and self-signed cert = trusted";
+    return true;
+  }
+
+  for (CFIndex i = 0, settings_count = CFArrayGetCount(trust_settings);
+       i < settings_count; ++i) {
+    DVLOG(2) << "IsTrustSettingsTrustedForPolicy i=" << i << " of "
+             << settings_count;
+    CFDictionaryRef trust_dict = reinterpret_cast<CFDictionaryRef>(
+        const_cast<void*>(CFArrayGetValueAtIndex(trust_settings, i)));
+    if (IsTrustDictionaryTrustedForPolicy(trust_dict, is_self_signed,
+                                          policy_oid))
+      return true;
+  }
+  return false;
+}
+
+bool IsSecCertificateTrustedForPolicy(SecCertificateRef cert_handle,
+                                      bool is_self_signed,
+                                      const CFStringRef policy_oid) {
+  for (const auto& trust_domain :
+       {kSecTrustSettingsDomainUser, kSecTrustSettingsDomainAdmin,
+        kSecTrustSettingsDomainSystem}) {
+    DVLOG(2) << "IsSecCertificateTrustedForPolicy domain = " << trust_domain;
+    CFArrayRef trust_settings;
+    OSStatus err = SecTrustSettingsCopyTrustSettings(cert_handle, trust_domain,
+                                                     &trust_settings);
+    if (err == errSecItemNotFound) {
+      DVLOG(2) << "no trust settings for that domain";
+      // No trust settings for that domain.. try the next.
+      continue;
+    }
+    if (err) {
+      OSSTATUS_LOG(ERROR, err) << "SecTrustSettingsCopyTrustSettings error";
+      continue;
+    }
+    base::ScopedCFTypeRef<CFArrayRef> scoped_trust_settings(trust_settings);
+    if (IsTrustSettingsTrustedForPolicy(trust_settings, is_self_signed,
+                                        policy_oid))
+      return true;
+  }
+
+  // No trust settings, or none of the settings were for the correct policy, or
+  // had the correct trust result.
+  return false;
+}
+
+}  // namespace
+
+TrustStoreMac::TrustStoreMac(const void* policy_oid)
+    : policy_oid_(base::mac::CFCast<CFStringRef>(policy_oid)) {
+  DCHECK(policy_oid_);
+}
+
+TrustStoreMac::~TrustStoreMac() = default;
+
+// static
+base::ScopedCFTypeRef<CFArrayRef>
+TrustStoreMac::FindMatchingCertificatesForMacNormalizedSubject(
+    CFDataRef name_data) {
+  base::ScopedCFTypeRef<CFArrayRef> matching_items;
+  base::ScopedCFTypeRef<CFMutableDictionaryRef> query(
+      CFDictionaryCreateMutable(NULL, 0, &kCFTypeDictionaryKeyCallBacks,
+                                &kCFTypeDictionaryValueCallBacks));
+
+  CFDictionarySetValue(query, kSecClass, kSecClassCertificate);
+  CFDictionarySetValue(query, kSecReturnRef, kCFBooleanTrue);
+  CFDictionarySetValue(query, kSecMatchLimit, kSecMatchLimitAll);
+  CFDictionarySetValue(query, kSecAttrSubject, name_data);
+
+  base::ScopedCFTypeRef<CFArrayRef> scoped_alternate_keychain_search_list;
+  if (TestKeychainSearchList::HasInstance()) {
+    CFArrayRef alternate_keychain_search_list;
+    OSStatus status = TestKeychainSearchList::GetInstance()->CopySearchList(
+        &alternate_keychain_search_list);
+    if (status) {
+      OSSTATUS_LOG(ERROR, status)
+          << "TestKeychainSearchList::CopySearchList error";
+      return matching_items;
+    }
+    scoped_alternate_keychain_search_list.reset(alternate_keychain_search_list);
+  }
+  // XXX this is all a bit similar (but not quite the same) as
+  // cert_verify_proc_mac keychain stuff.... is there a way to reduce
+  // duplication?
+  //
+  // If a TestKeychainSearchList is present, it will have already set
+  // |scoped_alternate_keychain_search_list|, which will be used as the
+  // basis for reordering the keychain. Otherwise, get the current keychain
+  // search list and use that.
+  if (!scoped_alternate_keychain_search_list) {
+    CFArrayRef keychain_search_list;
+    OSStatus status = SecKeychainCopySearchList(&keychain_search_list);
+    if (status) {
+      OSSTATUS_LOG(ERROR, status) << "SecKeychainCopySearchList error";
+      return matching_items;
+    }
+    scoped_alternate_keychain_search_list.reset(keychain_search_list);
+  }
+  CFMutableArrayRef mutable_keychain_search_list = CFArrayCreateMutableCopy(
+      kCFAllocatorDefault,
+      CFArrayGetCount(scoped_alternate_keychain_search_list.get()) + 1,
+      scoped_alternate_keychain_search_list.get());
+  if (!mutable_keychain_search_list) {
+    LOG(ERROR) << "CFArrayCreateMutableCopy";
+    return matching_items;
+  }
+  scoped_alternate_keychain_search_list.reset(mutable_keychain_search_list);
+  SecKeychainRef roots_keychain;
+  // The System Roots keychain is not normally searched by SecItemCopyMatching.
+  // Get a reference to it and include in the keychain search list.
+  OSStatus status = SecKeychainOpen(
+      "/System/Library/Keychains/SystemRootCertificates.keychain",
+      &roots_keychain);
+  if (status) {
+    OSSTATUS_LOG(ERROR, status) << "SecKeychainOpen error";
+    return matching_items;
+  }
+  base::ScopedCFTypeRef<SecKeychainRef> scoped_roots_keychain(roots_keychain);
+  CFArrayAppendValue(mutable_keychain_search_list, roots_keychain);
+  CFDictionarySetValue(query, kSecMatchSearchList,
+                       scoped_alternate_keychain_search_list.get());
+
+  OSStatus err;
+  {
+    base::AutoLock lock(crypto::GetMacSecurityServicesLock());
+    err =
+        SecItemCopyMatching(query, (CFTypeRef*)matching_items.InitializeInto());

[Ryan Sleevi, 2017/02/06 23:59:04]

reinterpret_cast ? Although I think it should toll-free cast, right?

[mattm, 2017/02/09 23:40:17]

done. I think the reinterpret_cast is needed since it's a pointer to a pointer.

+  }
+  if (err == errSecItemNotFound) {
+    DVLOG(2) << "SecItemCopyMatching: no matches found";
+    return matching_items;
+  }
+  if (err) {
+    crypto::LogCSSMError("SecItemCopyMatching", err);
+    OSSTATUS_LOG(ERROR, err) << "SecItemCopyMatching error";
+    return matching_items;
+  }
+  return matching_items;
+}
+
+// static
+void TrustStoreMac::FilterTrustedCertificates(CFArrayRef matching_items,
+                                              const CFStringRef policy_oid,
+                                              TrustAnchors* out_anchors) {
+  for (CFIndex i = 0, item_count = CFArrayGetCount(matching_items);
+       i < item_count; ++i) {
+    DVLOG(2) << "i=" << i << " of " << item_count;
+    SecCertificateRef match_cert_handle = reinterpret_cast<SecCertificateRef>(
+        const_cast<void*>(CFArrayGetValueAtIndex(matching_items, i)));
+
+    if (!IsSecCertificateTrustedForPolicy(
+            match_cert_handle, X509Certificate::IsSelfSigned(match_cert_handle),
+            policy_oid)) {
+      DVLOG(2) << "not trusted for policy";
+      continue;
+    } else {
+      DVLOG(2) << "trusted!";
+    }
+
+    CSSM_DATA der_data;
+    OSStatus err = SecCertificateGetData(match_cert_handle, &der_data);
+    if (err != noErr) {
+      OSSTATUS_LOG(ERROR, err) << "SecCertificateGetData error";
+      continue;
+    }
+
+    CertErrors errors;
+    ParseCertificateOptions options;
+    options.allow_invalid_serial_numbers = true;
+    scoped_refptr<ParsedCertificate> anchor_cert = ParsedCertificate::Create(
+        der_data.Data, der_data.Length, options, &errors);
+    // TODO(mattm): if parsing fails, it would be legit to return a TrustAnchor
+    // that doesn't have an associated cert, getting the SPKI and Subject
+    // through the SecCertificate APIs.

[Ryan Sleevi, 2017/02/06 23:59:04]

I would say if parsing fails, we're good to reject it for now, and Apple's
bananas for being that lax :)

[mattm, 2017/02/09 23:40:17]

Ack.  fwiw, There were two such certs I found (on my sierra machine, haven't
checked others for this). One with an invalid serial number (which we have the
option for in parsing..), one with a duplicate extension (which we don't
allow/have an option for.)

+    if (!anchor_cert) {
+      // TODO(crbug.com/634443): return errors better.
+      LOG(ERROR) << "Error parsing issuer certificate:\n"
+                 << errors.ToDebugString();
+      continue;
+    }
+
+    DVLOG(2) << "added cert to out_anchors";
+    out_anchors->push_back(TrustAnchor::CreateFromCertificateNoConstraints(
+        std::move(anchor_cert)));
+  }
+}
+
+// static
+base::ScopedCFTypeRef<CFDataRef> TrustStoreMac::GetMacNormalizedIssuer(
+    const scoped_refptr<ParsedCertificate>& cert) {
+  base::ScopedCFTypeRef<CFDataRef> name_data;
+  // There does not appear to be any public API to get the normalized version
+  // of a Name without creating a SecCertificate.

[Ryan Sleevi, 2017/02/06 23:59:04]

Unfortunately, yeah, that's the case. I'd be hesitant to reimplement Apple's
normalization routines since it's not part of the stable API (I mean, the APIs
to get a normalized name are, but not how it's computed, even if it's
effectively static)

[mattm, 2017/02/09 23:40:16]

Acknowledged.

+  X509Certificate::OSCertHandle cert_handle =
+      X509Certificate::CreateOSCertHandleFromBytes(
+          cert->der_cert().AsStringPiece().data(), cert->der_cert().Length());
+  if (!cert_handle) {
+    LOG(ERROR) << "CreateOSCertHandleFromBytes";
+    return name_data;
+  }
+  base::ScopedCFTypeRef<SecCertificateRef> scoped_cert_handle(cert_handle);
+  name_data.reset(
+      SecCertificateCopyNormalizedIssuerContent(cert_handle, nullptr));
+  if (!name_data)
+    LOG(ERROR) << "SecCertificateCopyNormalizedIssuerContent";
+  return name_data;
+}
+
+void TrustStoreMac::FindTrustAnchorsByMacNormalizedSubject(
+    CFDataRef name_data,
+    TrustAnchors* out_anchors) const {
+  base::ScopedCFTypeRef<CFArrayRef> scoped_matching_items =
+      FindMatchingCertificatesForMacNormalizedSubject(name_data);
+  if (!scoped_matching_items)
+    return;
+
+  FilterTrustedCertificates(scoped_matching_items.get(), policy_oid_,
+                            out_anchors);
+}
+
+void TrustStoreMac::FindTrustAnchorsForCert(
+    const scoped_refptr<ParsedCertificate>& cert,
+    TrustAnchors* out_anchors) const {
+  DVLOG(2) << "FindTrustAnchorsForCert";
+
+  base::ScopedCFTypeRef<CFDataRef> name_data = GetMacNormalizedIssuer(cert);
+
+  FindTrustAnchorsByMacNormalizedSubject(name_data, out_anchors);
+}
+
+}  // namespace net