Leak a reference to the ENGINE in the legacy client auth codepath.

net/android/keystore_openssl.cc

 // Copyright (c) 2013 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/android/keystore_openssl.h"
 
 #include <jni.h>
 #include <openssl/bn.h>
 // This include is required to get the ECDSA_METHOD structure definition
 // which isn't currently part of the OpenSSL official ABI. This should
@@ skipping 292 matching lines @@
   global_key.Reset(NULL, private_key);
   if (global_key.is_null()) {
     LOG(ERROR) << "Could not create global JNI reference";
     return false;
   }
   RSA_set_app_data(rsa.get(), global_key.Release());
   EVP_PKEY_assign_RSA(pkey, rsa.release());
   return true;
 }
 
+// On Android < 4.2, the libkeystore.so ENGINE uses CRYPTO_EX_DATA and is not
+// added to the global engine list. If all references to it are dropped, OpenSSL
+// will dlclose the module, leaving a dangling function pointer in the RSA
+// CRYPTO_EX_DATA class. To work around this, leak an extra reference to the
+// ENGINE we extract in GetRsaLegacyKey.
+//
+// In 4.2, this change avoids the problem:
+// https://android.googlesource.com/platform/libcore/+/106a8928fb4249f2f3d4dba1dddbe73ca5cb3d61
+//
+// https://crbug.com/381465
+class KeystoreEngineWorkaround {
+public:

[Ryan Sleevi, 2014/06/17 21:55:37]

indent 1 space

[davidben, 2014/06/17 23:43:18]

Done.

+  KeystoreEngineWorkaround() : leaked_(false) {}
+
+  void LeakRsaEngine(EVP_PKEY* pkey) {
+    if (leaked_)
+      return;
+    ScopedRSA rsa(EVP_PKEY_get1_RSA(pkey));
+    if (!rsa.get() ||
+        !rsa.get()->engine ||
+        strcmp(ENGINE_get_id(rsa.get()->engine), "keystore") ||
+        !ENGINE_init(rsa.get()->engine)) {

[Ryan Sleevi, 2014/06/17 21:55:37]

Don't use get()-> for operator->
Don't use get() for boolean

if (!rsa || !rsa->engine || strcmp(ENGINE_get_id(rsa->engine), "keystore") ||
!ENGINE_init(rsa->engine) {
}

[davidben, 2014/06/17 23:43:17]

ScopedOpenSSL doesn't actually provide a whole lot:
https://code.google.com/p/chromium/codesearch#chromium/src/crypto/openssl_uti...

+      NOTREACHED();
+      return;
+    }
+    leaked_ = true;
+  }
+
+private:

[Ryan Sleevi, 2014/06/17 21:55:37]

indent 1 space

[davidben, 2014/06/17 23:43:17]

Done.

+  bool leaked_;

[Ryan Sleevi, 2014/06/17 21:55:37]

s/leaked_/leaked_engine_/ perhaps?

Just so it's clear what you leaked (eg: you're not leaking an EVP_PKEY to keep
the engine alive)

[davidben, 2014/06/17 23:43:17]

Done.

+};
+
+void LeakRsaEngine(EVP_PKEY* pkey) {
+  static base::LazyInstance<KeystoreEngineWorkaround> s_instance =

[Ryan Sleevi, 2014/06/17 21:55:37]

this should be a ::Leaky lazy instance

[davidben, 2014/06/17 23:43:18]

Done.

+      LAZY_INSTANCE_INITIALIZER;
+  s_instance.Get().LeakRsaEngine(pkey);
+}
+
 // Setup an EVP_PKEY to wrap an existing platform RSA PrivateKey object
 // for Android 4.0 to 4.1.x. Must only be used on Android < 4.2.
 // |private_key| is a JNI reference (local or global) to the object.
 // |pkey| is the EVP_PKEY to setup as a wrapper.
 // Returns true on success, false otherwise.
 EVP_PKEY* GetRsaLegacyKey(jobject private_key) {
   EVP_PKEY* sys_pkey =
       GetOpenSSLSystemHandleForPrivateKey(private_key);
   if (sys_pkey != NULL) {
     CRYPTO_add(&sys_pkey->references, 1, CRYPTO_LOCK_EVP_PKEY);
+    LeakRsaEngine(sys_pkey);
   } else {
     // GetOpenSSLSystemHandleForPrivateKey() will fail on Android
     // 4.0.3 and earlier. However, it is possible to get the key
     // content with PrivateKey.getEncoded() on these platforms.
     // Note that this method may return NULL on 4.0.4 and later.
     std::vector<uint8> encoded;
     if (!GetPrivateKeyEncodedBytes(private_key, &encoded)) {
       LOG(ERROR) << "Can't get private key data!";
       return NULL;
     }
@@ skipping 354 matching lines @@
     default:
       LOG(WARNING)
           << "GetOpenSSLPrivateKeyWrapper() called with invalid key type";
       return NULL;
   }
   return pkey.release();
 }
 
 }  // namespace android
 }  // namespace net