Use SecKeyCreateSignature on macOS 10.12 and later.

net/ssl/ssl_platform_key_mac.cc

Index: net/ssl/ssl_platform_key_mac.cc
diff --git a/net/ssl/ssl_platform_key_mac.cc b/net/ssl/ssl_platform_key_mac.cc
index 4000c61496450ac4f1621f3f140084b35b5d09f5..31633fd0c6760a759868933fc632331b1d1142c9 100644
--- a/net/ssl/ssl_platform_key_mac.cc
+++ b/net/ssl/ssl_platform_key_mac.cc
@@ -2,8 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "net/ssl/ssl_platform_key.h"
+#include "net/ssl/ssl_platform_key_mac.h"
 
+#include <dlfcn.h>
+#include <CoreFoundation/CoreFoundation.h>
 #include <Security/cssm.h>
 #include <Security/SecBase.h>
 #include <Security/SecCertificate.h>
@@ -12,18 +14,23 @@
 
 #include <memory>
 
+#include "base/lazy_instance.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/mac/foundation_util.h"
 #include "base/mac/mac_logging.h"
+#include "base/mac/mac_util.h"
 #include "base/mac/scoped_cftyperef.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/scoped_policy.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/synchronization/lock.h"
 #include "crypto/mac_security_services_lock.h"
 #include "crypto/openssl_util.h"
 #include "net/base/net_errors.h"
 #include "net/cert/x509_certificate.h"
+#include "net/ssl/ssl_platform_key.h"
 #include "net/ssl/ssl_platform_key_util.h"
 #include "net/ssl/ssl_private_key.h"
 #include "net/ssl/threaded_ssl_private_key.h"
@@ -32,6 +39,12 @@
 #include "third_party/boringssl/src/include/openssl/nid.h"
 #include "third_party/boringssl/src/include/openssl/rsa.h"
 
+// TODO(davidben): Remove this after https://crbug.com/669240 is fixed.
+#if !defined(MAC_OS_X_VERSION_10_12) || \
+    MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_12
+typedef CFStringRef SecKeyAlgorithm;
+#endif
+
 namespace net {
 
 // CSSM functions are deprecated as of OSX 10.7, but have no replacement.
@@ -62,16 +75,17 @@ class ScopedCSSM_CC_HANDLE {
   DISALLOW_COPY_AND_ASSIGN(ScopedCSSM_CC_HANDLE);
 };
 
-// Looks up the private key for |certificate| in KeyChain and returns
+// Looks up the private key for |certificate| in |keychain| and returns
 // a SecKeyRef or nullptr on failure. The caller takes ownership of the
 // result.
-SecKeyRef FetchSecKeyRefForCertificate(const X509Certificate* certificate) {
+SecKeyRef FetchSecKeyRefForCertificate(const X509Certificate* certificate,
+                                       SecKeychainRef keychain) {
   OSStatus status;
   base::ScopedCFTypeRef<SecIdentityRef> identity;
   {
     base::AutoLock lock(crypto::GetMacSecurityServicesLock());
     status = SecIdentityCreateWithCertificate(
-        nullptr, certificate->os_cert_handle(), identity.InitializeInto());
+        keychain, certificate->os_cert_handle(), identity.InitializeInto());
   }
   if (status != noErr) {
     OSSTATUS_LOG(WARNING, status);
@@ -88,18 +102,81 @@ SecKeyRef FetchSecKeyRefForCertificate(const X509Certificate* certificate) {
   return private_key.release();
 }
 
-class SSLPlatformKeyMac : public ThreadedSSLPrivateKey::Delegate {
+// These symbols were added in the 10.12 SDK, but we currently use an older SDK,
+// so look them up with dlsym.
+//
+// TODO(davidben): After https://crbug.com/669240 is fixed, use the APIs
+// directly.
+struct SecKeyAPIs {
+  SecKeyAPIs() { Init(); }
+
+  void Init() {
+    SecKeyCreateSignature = reinterpret_cast<SecKeyCreateSignatureFunc>(
+        dlsym(RTLD_DEFAULT, "SecKeyCreateSignature"));
+    if (!SecKeyCreateSignature) {
+      NOTREACHED();
+      return;
+    }
+
+#define LOOKUP_ALGORITHM(name)                                          \
+  do {                                                                  \
+    SecKeyAlgorithm* algorithm =                                        \
+        reinterpret_cast<SecKeyAlgorithm*>(dlsym(RTLD_DEFAULT, #name)); \
+    if (!algorithm) {                                                   \
+      NOTREACHED();                                                     \
+      return;                                                           \

[Ryan Sleevi, 2016/12/13 22:21:07]

Is it really fatal if these can't be looked up?

[davidben, 2016/12/14 00:59:43]

If it happens, we got the version check wrong. I think that's fine for
NOTREACHED() that is checked at runtime.

+    }                                                                   \
+    name = *algorithm;                                                  \
+  } while (0)
+
+    LOOKUP_ALGORITHM(kSecKeyAlgorithmRSASignatureDigestPKCS1v15Raw);
+    LOOKUP_ALGORITHM(kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA1);
+    LOOKUP_ALGORITHM(kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA256);
+    LOOKUP_ALGORITHM(kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA384);
+    LOOKUP_ALGORITHM(kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA512);
+    LOOKUP_ALGORITHM(kSecKeyAlgorithmECDSASignatureDigestX962SHA1);
+    LOOKUP_ALGORITHM(kSecKeyAlgorithmECDSASignatureDigestX962SHA256);
+    LOOKUP_ALGORITHM(kSecKeyAlgorithmECDSASignatureDigestX962SHA384);
+    LOOKUP_ALGORITHM(kSecKeyAlgorithmECDSASignatureDigestX962SHA512);
+
+#undef LOOKUP_ALGORITHM
+
+    valid = true;
+  }
+
+  using SecKeyCreateSignatureFunc = CFDataRef (*)(SecKeyRef key,
+                                                  SecKeyAlgorithm algorithm,
+                                                  CFDataRef dataToSign,
+                                                  CFErrorRef* error);
+
+  bool valid = false;
+  SecKeyCreateSignatureFunc SecKeyCreateSignature = NULL;

[Ryan Sleevi, 2016/12/13 22:21:07]

nullptr?

[davidben, 2016/12/14 00:59:43]

Done.

+  SecKeyAlgorithm kSecKeyAlgorithmRSASignatureDigestPKCS1v15Raw = NULL;
+  SecKeyAlgorithm kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA1 = NULL;
+  SecKeyAlgorithm kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA256 = NULL;
+  SecKeyAlgorithm kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA384 = NULL;
+  SecKeyAlgorithm kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA512 = NULL;
+  SecKeyAlgorithm kSecKeyAlgorithmECDSASignatureDigestX962SHA1 = NULL;
+  SecKeyAlgorithm kSecKeyAlgorithmECDSASignatureDigestX962SHA256 = NULL;
+  SecKeyAlgorithm kSecKeyAlgorithmECDSASignatureDigestX962SHA384 = NULL;
+  SecKeyAlgorithm kSecKeyAlgorithmECDSASignatureDigestX962SHA512 = NULL;
+};
+
+base::LazyInstance<SecKeyAPIs>::Leaky g_sec_key_apis =
+    LAZY_INSTANCE_INITIALIZER;
+
+class SSLPlatformKeyCSSM : public ThreadedSSLPrivateKey::Delegate {
  public:
-  SSLPlatformKeyMac(SSLPrivateKey::Type type,
-                    size_t max_length,
-                    SecKeyRef key,
-                    const CSSM_KEY* cssm_key)
+  SSLPlatformKeyCSSM(SSLPrivateKey::Type type,
+                     size_t max_length,
+                     SecKeyRef key,
+                     const CSSM_KEY* cssm_key)
       : type_(type),
         max_length_(max_length),
         key_(key, base::scoped_policy::RETAIN),
         cssm_key_(cssm_key) {}
 
-  ~SSLPlatformKeyMac() override {}
+  ~SSLPlatformKeyCSSM() override {}
 
   SSLPrivateKey::Type GetType() override { return type_; }
 
@@ -206,19 +283,134 @@ class SSLPlatformKeyMac : public ThreadedSSLPrivateKey::Delegate {
   base::ScopedCFTypeRef<SecKeyRef> key_;
   const CSSM_KEY* cssm_key_;
 
-  DISALLOW_COPY_AND_ASSIGN(SSLPlatformKeyMac);
+  DISALLOW_COPY_AND_ASSIGN(SSLPlatformKeyCSSM);
+};
+
+class SSLPlatformKeySecKey : public ThreadedSSLPrivateKey::Delegate {
+ public:
+  SSLPlatformKeySecKey(SSLPrivateKey::Type type,
+                       size_t max_length,
+                       SecKeyRef key)
+      : type_(type),
+        max_length_(max_length),
+        key_(key, base::scoped_policy::RETAIN) {}
+
+  ~SSLPlatformKeySecKey() override {}
+
+  SSLPrivateKey::Type GetType() override { return type_; }
+
+  std::vector<SSLPrivateKey::Hash> GetDigestPreferences() override {
+    static const SSLPrivateKey::Hash kHashes[] = {
+        SSLPrivateKey::Hash::SHA512, SSLPrivateKey::Hash::SHA384,
+        SSLPrivateKey::Hash::SHA256, SSLPrivateKey::Hash::SHA1};
+    return std::vector<SSLPrivateKey::Hash>(kHashes,
+                                            kHashes + arraysize(kHashes));

[Ryan Sleevi, 2016/12/13 22:21:07]

pedantry nit: Can you not std::begin(kHashes), std::end(kHashes) ?

Alternatively, can you not
return std::vector<SSLPrivateKey::Hash>{
  SSLPrivateKey::Hash::SHA512, SSLPrivateKey::Hash::SHA384, ... };

[davidben, 2016/12/14 00:59:43]

Done. (This was copy-pasted from code that predated us being able to use that.
Fixed in both.)

+  }
+
+  size_t GetMaxSignatureLengthInBytes() override { return max_length_; }
+
+  Error SignDigest(SSLPrivateKey::Hash hash,
+                   const base::StringPiece& input,
+                   std::vector<uint8_t>* signature) override {
+    const SecKeyAPIs& apis = g_sec_key_apis.Get();
+    if (!apis.valid) {
+      LOG(ERROR) << "SecKey APIs not found";
+      return ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED;
+    }
+
+    SecKeyAlgorithm algorithm = NULL;
+    if (type_ == SSLPrivateKey::Type::RSA) {
+      switch (hash) {
+        case SSLPrivateKey::Hash::SHA512:
+          algorithm = apis.kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA512;
+          break;
+        case SSLPrivateKey::Hash::SHA384:
+          algorithm = apis.kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA384;
+          break;
+        case SSLPrivateKey::Hash::SHA256:
+          algorithm = apis.kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA256;
+          break;
+        case SSLPrivateKey::Hash::SHA1:
+          algorithm = apis.kSecKeyAlgorithmRSASignatureDigestPKCS1v15SHA1;
+          break;
+        case SSLPrivateKey::Hash::MD5_SHA1:
+          algorithm = apis.kSecKeyAlgorithmRSASignatureDigestPKCS1v15Raw;
+          break;
+      }
+    } else if (SSLPrivateKey::IsECDSAType(type_)) {
+      switch (hash) {
+        case SSLPrivateKey::Hash::SHA512:
+          algorithm = apis.kSecKeyAlgorithmECDSASignatureDigestX962SHA512;
+          break;
+        case SSLPrivateKey::Hash::SHA384:
+          algorithm = apis.kSecKeyAlgorithmECDSASignatureDigestX962SHA384;
+          break;
+        case SSLPrivateKey::Hash::SHA256:
+          algorithm = apis.kSecKeyAlgorithmECDSASignatureDigestX962SHA256;
+          break;
+        case SSLPrivateKey::Hash::SHA1:
+          algorithm = apis.kSecKeyAlgorithmECDSASignatureDigestX962SHA1;
+          break;
+        case SSLPrivateKey::Hash::MD5_SHA1:
+          // MD5-SHA1 is not used with ECDSA.
+          break;
+      }
+    }
+
+    if (!algorithm) {
+      NOTREACHED();
+      return ERR_FAILED;
+    }
+
+    base::ScopedCFTypeRef<CFDataRef> input_ref(CFDataCreateWithBytesNoCopy(
+        kCFAllocatorDefault, reinterpret_cast<const uint8_t*>(input.data()),
+        base::checked_cast<CFIndex>(input.size()), kCFAllocatorNull));
+
+    base::ScopedCFTypeRef<CFErrorRef> error;
+    base::ScopedCFTypeRef<CFDataRef> signature_ref(apis.SecKeyCreateSignature(
+        key_, algorithm, input_ref, error.InitializeInto()));
+    if (!signature_ref) {
+      LOG(ERROR) << error;
+      return ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED;
+    }
+
+    signature->assign(
+        CFDataGetBytePtr(signature_ref),
+        CFDataGetBytePtr(signature_ref) + CFDataGetLength(signature_ref));
+    return OK;
+  }
+
+ private:
+  SSLPrivateKey::Type type_;
+  size_t max_length_;
+  base::ScopedCFTypeRef<SecKeyRef> key_;
+
+  DISALLOW_COPY_AND_ASSIGN(SSLPlatformKeySecKey);
 };
 
 }  // namespace
 
-scoped_refptr<SSLPrivateKey> FetchClientCertPrivateKey(
-    X509Certificate* certificate) {
+scoped_refptr<SSLPrivateKey> FetchClientCertPrivateKeyFromKeychain(
+    X509Certificate* certificate,
+    SecKeychainRef keychain) {
   // Look up the private key.
   base::ScopedCFTypeRef<SecKeyRef> private_key(
-      FetchSecKeyRefForCertificate(certificate));
+      FetchSecKeyRefForCertificate(certificate, keychain));
   if (!private_key)
     return nullptr;
 
+  SSLPrivateKey::Type key_type;
+  size_t max_length;
+  if (!GetClientCertInfo(certificate, &key_type, &max_length))
+    return nullptr;
+
+  if (base::mac::IsAtLeastOS10_12()) {
+    return make_scoped_refptr(
+        new ThreadedSSLPrivateKey(base::MakeUnique<SSLPlatformKeySecKey>(
+                                      key_type, max_length, private_key.get()),
+                                  GetSSLPlatformKeyTaskRunner()));
+  }
+
   const CSSM_KEY* cssm_key;
   OSStatus status = SecKeyGetCSSMKey(private_key.get(), &cssm_key);
   if (status != noErr) {
@@ -226,17 +418,17 @@ scoped_refptr<SSLPrivateKey> FetchClientCertPrivateKey(
     return nullptr;
   }
 
-  SSLPrivateKey::Type key_type;
-  size_t max_length;
-  if (!GetClientCertInfo(certificate, &key_type, &max_length))
-    return nullptr;
-
   return make_scoped_refptr(new ThreadedSSLPrivateKey(
-      base::MakeUnique<SSLPlatformKeyMac>(key_type, max_length,
-                                          private_key.get(), cssm_key),
+      base::MakeUnique<SSLPlatformKeyCSSM>(key_type, max_length,
+                                           private_key.get(), cssm_key),
       GetSSLPlatformKeyTaskRunner()));
 }
 
+scoped_refptr<SSLPrivateKey> FetchClientCertPrivateKey(
+    X509Certificate* certificate) {
+  return FetchClientCertPrivateKeyFromKeychain(certificate, nullptr);
+}
+
 #pragma clang diagnostic pop  // "-Wdeprecated-declarations"
 
 }  // namespace net