Add a lock for OS X CSSM wrapper APIs

They're apparently problematic with threads.

R=agl
BUG=48006
TEST=KeygenHandler.ConcurrencyTest

Review URL: http://codereview.chromium.org/2832047

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@51734 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 73 insertions, 21 deletions

diff --git a/base/crypto/cssm_init.cc b/base/crypto/cssm_init.cc
index 510ae0c..b04cbe7 100644
--- a/base/crypto/cssm_init.cc
+++ b/base/crypto/cssm_init.cc
@@ -6,6 +6,7 @@
 
 #include <Security/SecBase.h>
 
+#include "base/lock.h"
 #include "base/logging.h"
 #include "base/singleton.h"
 #include "base/sys_string_conversions.h"
@@ -75,6 +76,25 @@ class CSSMInitSingleton {
   CSSM_CSP_HANDLE csp_handle_;
 };
 
+// This singleton is separate as it pertains to Apple's wrappers over
+// their own CSSM handles, as opposed to our own CSSM_CSP_HANDLE.
+class SecurityServicesSingleton {
+ public:
+  ~SecurityServicesSingleton() {}
+
+  Lock& lock() { return lock_; }
+
+ private:
+  friend class Singleton<SecurityServicesSingleton>;
+  friend struct DefaultSingletonTraits<SecurityServicesSingleton>;
+
+  SecurityServicesSingleton() {}
+
+  Lock lock_;
+
+  DISALLOW_COPY_AND_ASSIGN(SecurityServicesSingleton);
+};
+
 }  // namespace
 
 namespace base {
@@ -124,4 +144,8 @@ void LogCSSMError(const char *fn_name, CSSM_RETURN err) {
   }
 }
 
+Lock& GetMacSecurityServicesLock() {
+  return Singleton<SecurityServicesSingleton>::get()->lock();
+}
+
 }  // namespace base
diff --git a/base/crypto/cssm_init.h b/base/crypto/cssm_init.h
index 721b2e8..b5ec03d 100644
--- a/base/crypto/cssm_init.h
+++ b/base/crypto/cssm_init.h
@@ -10,6 +10,8 @@
 #include "base/logging.h"
 #include "base/scoped_ptr.h"
 
+class Lock;
+
 namespace base {
 
 // Initialize CSSM if it isn't already initialized.  This must be called before
@@ -26,6 +28,13 @@ extern const CSSM_API_MEMORY_FUNCS kCssmMemoryFunctions;
 // Utility function to log an error message including the error name.
 void LogCSSMError(const char *function_name, CSSM_RETURN err);
 
+// The OS X certificate and key management wrappers over CSSM are not
+// thread-safe. In particular, code that accesses the CSSM database is
+// problematic.
+//
+// http://developer.apple.com/mac/library/documentation/Security/Reference/certifkeytrustservices/Reference/reference.html
+Lock& GetMacSecurityServicesLock();
+
 }  // namespace base
 
 #endif  // BASE_CRYPTO_CSSM_INIT_H_
diff --git a/net/base/cert_database_mac.cc b/net/base/cert_database_mac.cc
index 938f133..b0afd50 100644
--- a/net/base/cert_database_mac.cc
+++ b/net/base/cert_database_mac.cc
@@ -6,6 +6,8 @@
 
 #include <Security/Security.h>
 
+#include "base/crypto/cssm_init.h"
+#include "base/lock.h"
 #include "base/logging.h"
 #include "net/base/net_errors.h"
 #include "net/base/x509_certificate.h"
@@ -39,7 +41,11 @@ int CertDatabase::CheckUserCert(X509Certificate* cert) {
 }
 
 int CertDatabase::AddUserCert(X509Certificate* cert) {
-  OSStatus err = SecCertificateAddToKeychain(cert->os_cert_handle(), NULL);
+  OSStatus err;
+  {
+    AutoLock locked(base::GetMacSecurityServicesLock());
+    err = SecCertificateAddToKeychain(cert->os_cert_handle(), NULL);
+  }
   switch (err) {
     case noErr:
     case errSecDuplicateItem:
diff --git a/net/base/keygen_handler_mac.cc b/net/base/keygen_handler_mac.cc
index 94cf828..c2c63d7 100644
--- a/net/base/keygen_handler_mac.cc
+++ b/net/base/keygen_handler_mac.cc
@@ -10,6 +10,7 @@
 
 #include "base/base64.h"
 #include "base/crypto/cssm_init.h"
+#include "base/lock.h"
 #include "base/logging.h"
 #include "base/scoped_cftyperef.h"
 
@@ -206,20 +207,23 @@ static OSStatus CreateRSAKeyPair(int size_in_bits,
     return err;
   }
   scoped_cftyperef<SecKeychainRef> scoped_keychain(keychain);
-  err = SecKeyCreatePair(
-      keychain,
-      CSSM_ALGID_RSA,
-      size_in_bits,
-      0LL,
-      // public key usage and attributes:
-      CSSM_KEYUSE_ENCRYPT | CSSM_KEYUSE_VERIFY | CSSM_KEYUSE_WRAP,
-      CSSM_KEYATTR_EXTRACTABLE | CSSM_KEYATTR_PERMANENT,
-      // private key usage and attributes:
-      CSSM_KEYUSE_DECRYPT | CSSM_KEYUSE_SIGN | CSSM_KEYUSE_UNWRAP,
-      CSSM_KEYATTR_EXTRACTABLE | CSSM_KEYATTR_PERMANENT |
-          CSSM_KEYATTR_SENSITIVE,
-      NULL,
-      out_pub_key, out_priv_key);
+  {
+    AutoLock locked(base::GetMacSecurityServicesLock());
+    err = SecKeyCreatePair(
+        keychain,
+        CSSM_ALGID_RSA,
+        size_in_bits,
+        0LL,
+        // public key usage and attributes:
+        CSSM_KEYUSE_ENCRYPT | CSSM_KEYUSE_VERIFY | CSSM_KEYUSE_WRAP,
+        CSSM_KEYATTR_EXTRACTABLE | CSSM_KEYATTR_PERMANENT,
+        // private key usage and attributes:
+        CSSM_KEYUSE_DECRYPT | CSSM_KEYUSE_SIGN | CSSM_KEYUSE_UNWRAP,
+        CSSM_KEYATTR_EXTRACTABLE | CSSM_KEYATTR_PERMANENT |
+            CSSM_KEYATTR_SENSITIVE,
+        NULL,
+        out_pub_key, out_priv_key);
+  }
   if (err)
     base::LogCSSMError("SecKeyCreatePair", err);
   return err;
@@ -230,24 +234,33 @@ static OSStatus CreateSignatureContext(SecKeyRef key,
                                        CSSM_CC_HANDLE* out_cc_handle) {
   OSStatus err;
   const CSSM_ACCESS_CREDENTIALS* credentials = NULL;
-  err = SecKeyGetCredentials(key,
-                             CSSM_ACL_AUTHORIZATION_SIGN,
-                             kSecCredentialTypeDefault,
-                             &credentials);
+  {
+    AutoLock locked(base::GetMacSecurityServicesLock());
+    err = SecKeyGetCredentials(key,
+                               CSSM_ACL_AUTHORIZATION_SIGN,
+                               kSecCredentialTypeDefault,
+                               &credentials);
+  }
   if (err) {
     base::LogCSSMError("SecKeyGetCredentials", err);
     return err;
   }
 
   CSSM_CSP_HANDLE csp_handle = 0;
-  err = SecKeyGetCSPHandle(key, &csp_handle);
+  {
+    AutoLock locked(base::GetMacSecurityServicesLock());
+    err = SecKeyGetCSPHandle(key, &csp_handle);
+  }
   if (err) {
     base::LogCSSMError("SecKeyGetCSPHandle", err);
     return err;
   }
 
   const CSSM_KEY* cssm_key = NULL;
-  err = SecKeyGetCSSMKey(key, &cssm_key);
+  {
+    AutoLock locked(base::GetMacSecurityServicesLock());
+    err = SecKeyGetCSSMKey(key, &cssm_key);
+  }
   if (err) {
     base::LogCSSMError("SecKeyGetCSSMKey", err);
     return err;