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 <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/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 {
+
+namespace {
+
+// The rules for interpreting trust settings are documented at:
+// https://developer.apple.com/reference/security/1400261-sectrustsettingscopytrustsetting?language=objc
+
+// Returns true if a usage constraints dictionary |trust_dict| for a certificate
+// that |is_self_signed| should be treated as a trust anchor.
+bool IsTrustDictionaryTrustedForPolicy(CFDictionaryRef trust_dict,
+                                       bool is_self_signed,
+                                       const CFStringRef target_policy_oid) {
+  // 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|.
+
+  // Trust settings may be scoped to a single application, by checking that the
+  // code signing identity of the current application matches the serialized
+  // code signing identity in the kSecTrustSettingsApplication key.
+  // As this is not presently supported, skip any trust settings scoped to the
+  // application.
+  if (CFDictionaryContainsKey(trust_dict, kSecTrustSettingsApplication))
+    return false;
+
+  // Trust settings may be scoped using policy-specific constraints. For
+  // example, SSL trust settings might be scoped to a single hostname, or EAP
+  // settings specific to a particular WiFi network.
+  // As this is not presently supported, skip any policy-specific trust
+  // settings.
+  if (CFDictionaryContainsKey(trust_dict, kSecTrustSettingsPolicyString))
+    return false;
+
+  // Ignoring kSecTrustSettingsKeyUsage for now; it does not seem relevant to
+  // the TLS case.
+
+  // If the trust settings are scoped to a specific policy (via
+  // kSecTrustSettingsPolicy), ensure that the policy is the same policy as
+  // |target_policy_oid|. If there is no kSecTrustSettingsPolicy key, it's
+  // considered a match for all policies.
+  SecPolicyRef policy_ref = base::mac::GetValueFromDictionary<SecPolicyRef>(
+      trust_dict, kSecTrustSettingsPolicy);
+  if (policy_ref) {
+    base::ScopedCFTypeRef<CFDictionaryRef> policy_dict(
+        SecPolicyCopyProperties(policy_ref));
+
+    // kSecPolicyOid is guaranteed to be present in the policy dictionary.
+    //
+    // TODO(mattm): remove the CFCastStrict below once Chromium builds against
+    // the 10.11 SDK.
+    CFStringRef policy_oid = base::mac::GetValueFromDictionary<CFStringRef>(
+        policy_dict, base::mac::CFCastStrict<CFStringRef>(kSecPolicyOid));
+
+    if (!CFEqual(policy_oid, target_policy_oid))
+      return false;
+  }
+
+  // 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,

[Ryan Sleevi, 2017/02/16 22:27:22]

Perhaps fold this in to line 84 with an &&

[mattm, 2017/02/17 00:04:18]

Done.

+                          &trust_settings_result)) {
+      return false;
+    }
+  }
+
+  // TODO: handle distrust (kSecTrustSettingsResultDeny)

[Ryan Sleevi, 2017/02/16 22:27:22]

It looks like you're already handling that, by way of line 96 & 99, right?

[mattm, 2017/02/17 00:04:18]

Isn't there still the case where you have a distrust record for some non-anchor
cert, so we'd want to be able to fail any chain that got built with that cert
even if it chains to a valid trust anchor?

[Ryan Sleevi, 2017/02/17 00:54:13]

Yup. I was thinking solely for the low-hanging fruit of CA records.

+
+  // 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;
+}
+
+// Returns true if the trust settings array |trust_settings| for a certificate
+// that |is_self_signed| should be treated as a trust anchor.
+bool IsTrustSettingsTrustedForPolicy(CFArrayRef trust_settings,
+                                     bool is_self_signed,
+                                     const CFStringRef policy_oid) {
+  // An empty trust settings array (that is, the trust_settings 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)
+    return true;
+
+  for (CFIndex i = 0, settings_count = CFArrayGetCount(trust_settings);
+       i < settings_count; ++i) {
+    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;
+}
+
+// Returns true if the certificate |cert_handle| is trusted for the policy
+// |policy_oid|.
+bool IsSecCertificateTrustedForPolicy(SecCertificateRef cert_handle,
+                                      const CFStringRef policy_oid) {
+  const bool is_self_signed = X509Certificate::IsSelfSigned(cert_handle);
+  // Evaluate trust domains in user, admin, system order. Admin settings can
+  // override system ones, and user settings can override both admin and system.
+  for (const auto& trust_domain :
+       {kSecTrustSettingsDomainUser, kSecTrustSettingsDomainAdmin,
+        kSecTrustSettingsDomainSystem}) {
+    base::ScopedCFTypeRef<CFArrayRef> trust_settings;
+    OSStatus err = SecTrustSettingsCopyTrustSettings(
+        cert_handle, trust_domain, trust_settings.InitializeInto());
+    if (err == errSecItemNotFound) {
+      // No trust settings for that domain.. try the next.
+      continue;
+    }
+    if (err) {
+      OSSTATUS_LOG(ERROR, err) << "SecTrustSettingsCopyTrustSettings error";
+      continue;
+    }
+    if (IsTrustSettingsTrustedForPolicy(trust_settings, is_self_signed,
+                                        policy_oid))

[Ryan Sleevi, 2017/02/16 22:27:22]

To the general point of handling distrust records - I suspect that if you
modified IsTrustSettingsTrustedForPolicy() to return the tristate ('yes', 'no',
'continue'), then we could appropriately handle distrust records by way of
short-circuiting on yes/no and otherwise continuing to the next trust domain on
'continue'

That should work, right?

[mattm, 2017/02/17 00:04:18]

Would work for this part, but see the question above.

[mattm, 2017/02/17 02:03:36]

I added the tri-state.

+      return true;
+  }
+
+  // No trust settings, or none of the settings were for the correct policy, or
+  // had the correct trust result.
+  return false;
+}
+
+// Filters an array of SecCertificateRef by trust for |policy_oid|, returning
+// the results as TrustAnchors in |out_anchors|.
+void FilterTrustedCertificates(CFArrayRef matching_items,
+                               const CFStringRef policy_oid,
+                               TrustAnchors* out_anchors) {
+  for (CFIndex i = 0, item_count = CFArrayGetCount(matching_items);
+       i < item_count; ++i) {
+    SecCertificateRef match_cert_handle = reinterpret_cast<SecCertificateRef>(
+        const_cast<void*>(CFArrayGetValueAtIndex(matching_items, i)));
+
+    if (!IsSecCertificateTrustedForPolicy(match_cert_handle, policy_oid))
+      continue;
+
+    base::ScopedCFTypeRef<CFDataRef> der_data(
+        SecCertificateCopyData(match_cert_handle));
+    if (!der_data) {
+      LOG(ERROR) << "SecCertificateCopyData error";
+      continue;
+    }
+
+    CertErrors errors;
+    ParseCertificateOptions options;
+    options.allow_invalid_serial_numbers = true;
+    scoped_refptr<ParsedCertificate> anchor_cert = ParsedCertificate::Create(
+        CFDataGetBytePtr(der_data.get()), CFDataGetLength(der_data.get()),
+        options, &errors);
+    if (!anchor_cert) {
+      // TODO(crbug.com/634443): return errors better.
+      LOG(ERROR) << "Error parsing issuer certificate:\n"
+                 << errors.ToDebugString();
+      continue;
+    }
+
+    out_anchors->push_back(TrustAnchor::CreateFromCertificateNoConstraints(
+        std::move(anchor_cert)));
+  }
+}
+
+}  // namespace
+
+TrustStoreMac::TrustStoreMac(const CFTypeRef policy_oid)
+    : policy_oid_(base::mac::CFCastStrict<CFStringRef>(policy_oid)) {
+  DCHECK(policy_oid_);
+}
+
+TrustStoreMac::~TrustStoreMac() = default;
+
+void TrustStoreMac::FindTrustAnchorsForCert(
+    const scoped_refptr<ParsedCertificate>& cert,
+    TrustAnchors* out_anchors) const {
+  base::ScopedCFTypeRef<CFDataRef> name_data = GetMacNormalizedIssuer(cert);
+
+  FindTrustAnchorsByMacNormalizedSubject(name_data, out_anchors);
+}
+
+// static
+base::ScopedCFTypeRef<CFArrayRef>
+TrustStoreMac::FindMatchingCertificatesForMacNormalizedSubject(
+    CFDataRef name_data) {
+  base::ScopedCFTypeRef<CFArrayRef> matching_items;
+  base::ScopedCFTypeRef<CFMutableDictionaryRef> query(
+      CFDictionaryCreateMutable(nullptr, 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()) {
+    OSStatus status = TestKeychainSearchList::GetInstance()->CopySearchList(
+        scoped_alternate_keychain_search_list.InitializeInto());
+    if (status) {
+      OSSTATUS_LOG(ERROR, status)
+          << "TestKeychainSearchList::CopySearchList error";
+      return matching_items;
+    }
+  }
+
+  // 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) {
+    OSStatus status = SecKeychainCopySearchList(
+        scoped_alternate_keychain_search_list.InitializeInto());
+    if (status) {
+      OSSTATUS_LOG(ERROR, status) << "SecKeychainCopySearchList error";
+      return matching_items;
+    }
+  }
+  CFMutableArrayRef mutable_keychain_search_list = CFArrayCreateMutableCopy(

[Ryan Sleevi, 2017/02/16 22:27:22]

newline between 247/248

[mattm, 2017/02/17 00:04:18]

Done.

+      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);
+  base::ScopedCFTypeRef<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.InitializeInto());
+  if (status) {
+    OSSTATUS_LOG(ERROR, status) << "SecKeychainOpen error";
+    return matching_items;
+  }
+  CFArrayAppendValue(mutable_keychain_search_list, roots_keychain);
+  CFDictionarySetValue(query, kSecMatchSearchList,
+                       scoped_alternate_keychain_search_list.get());
+
+  OSStatus err;
+  {
+    base::AutoLock lock(crypto::GetMacSecurityServicesLock());

[Ryan Sleevi, 2017/02/16 22:27:22]

Line 260 and 241 would also need to be guarded w/ this lock, as they also both
hit the security services locks

[mattm, 2017/02/17 00:04:18]

Done.

+    err = SecItemCopyMatching(
+        query, reinterpret_cast<CFTypeRef*>(matching_items.InitializeInto()));
+  }
+  if (err == errSecItemNotFound) {
+    // No matches found.
+    return matching_items;
+  }
+  if (err) {
+    OSSTATUS_LOG(ERROR, err) << "SecItemCopyMatching error";
+    return matching_items;
+  }
+  return matching_items;
+}
+
+// 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.
+  base::ScopedCFTypeRef<SecCertificateRef> cert_handle(
+      X509Certificate::CreateOSCertHandleFromBytes(
+          cert->der_cert().AsStringPiece().data(), cert->der_cert().Length()));
+  if (!cert_handle) {
+    LOG(ERROR) << "CreateOSCertHandleFromBytes";
+    return name_data;
+  }
+  name_data.reset(
+      SecCertificateCopyNormalizedIssuerContent(cert_handle, nullptr));

[Ryan Sleevi, 2017/02/16 22:27:23]

This would also need the CSSM lock

[mattm, 2017/02/17 00:04:18]

Hm, that seems a little surprising unless you're saying that every Sec* function
that touchs certs needs the lock? This function just seems to copy some bytes
out of the cert structure.. (the normalization is done during parsing)

What about the SecTrustSettingsCopyTrustSettings and SecPolicyCopyProperties
calls?

[Ryan Sleevi, 2017/02/17 00:54:13]

Yup, at least until 10.12 it seems.

At the lowest level, the issue is in the ref-counting in the CSSM/CDSA code, and
any properties that touch any of those objects (or cause them to be
allocated/deallocated as part of the call, which isn't really something we can
guarantee with the API contract) can end up tripping it up.
Yeah, basically. SecTrustSettings* is "safe" because it doesn't touch any of the
CSSM bits, but that's not guaranteed. SecPolicy* is less-so, because it (used
to) have a CSSM path in it.

[mattm, 2017/02/17 02:03:36]

Acknowledged.
Sorry, it wasn't clear to me if this should be interpeted as "yes add the locks"
or "no don't need to add the locks".

+  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);
+}
+
+}  // namespace net