Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1071)

Side by Side Diff: net/android/keystore_openssl.cc

Issue 322533003: Leak a reference to the ENGINE in the legacy client auth codepath. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: sleevi comments Created 6 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2013 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "net/android/keystore_openssl.h" 5 #include "net/android/keystore_openssl.h"
6 6
7 #include <jni.h> 7 #include <jni.h>
8 #include <openssl/bn.h> 8 #include <openssl/bn.h>
9 // This include is required to get the ECDSA_METHOD structure definition 9 // This include is required to get the ECDSA_METHOD structure definition
10 // which isn't currently part of the OpenSSL official ABI. This should 10 // which isn't currently part of the OpenSSL official ABI. This should
(...skipping 292 matching lines...) Expand 10 before | Expand all | Expand 10 after
303 global_key.Reset(NULL, private_key); 303 global_key.Reset(NULL, private_key);
304 if (global_key.is_null()) { 304 if (global_key.is_null()) {
305 LOG(ERROR) << "Could not create global JNI reference"; 305 LOG(ERROR) << "Could not create global JNI reference";
306 return false; 306 return false;
307 } 307 }
308 RSA_set_app_data(rsa.get(), global_key.Release()); 308 RSA_set_app_data(rsa.get(), global_key.Release());
309 EVP_PKEY_assign_RSA(pkey, rsa.release()); 309 EVP_PKEY_assign_RSA(pkey, rsa.release());
310 return true; 310 return true;
311 } 311 }
312 312
313 // On Android < 4.2, the libkeystore.so ENGINE uses CRYPTO_EX_DATA and is not
314 // added to the global engine list. If all references to it are dropped, OpenSSL
315 // will dlclose the module, leaving a dangling function pointer in the RSA
316 // CRYPTO_EX_DATA class. To work around this, leak an extra reference to the
317 // ENGINE we extract in GetRsaLegacyKey.
318 //
319 // In 4.2, this change avoids the problem:
320 // https://android.googlesource.com/platform/libcore/+/106a8928fb4249f2f3d4dba1d ddbe73ca5cb3d61
321 //
322 // https://crbug.com/381465
323 class KeystoreEngineWorkaround {
324 public:
325 KeystoreEngineWorkaround() : leaked_engine_(false) {}
326
327 void LeakRsaEngine(EVP_PKEY* pkey) {
328 if (leaked_engine_)
329 return;
330 ScopedRSA rsa(EVP_PKEY_get1_RSA(pkey));
331 if (!rsa.get() ||
332 !rsa.get()->engine ||
333 strcmp(ENGINE_get_id(rsa.get()->engine), "keystore") ||
334 !ENGINE_init(rsa.get()->engine)) {
335 NOTREACHED();
336 return;
337 }
338 leaked_engine_ = true;
339 }
340
341 private:
342 bool leaked_engine_;
343 };
344
345 void LeakRsaEngine(EVP_PKEY* pkey) {
346 static base::LazyInstance<KeystoreEngineWorkaround>::Leaky s_instance =
347 LAZY_INSTANCE_INITIALIZER;
348 s_instance.Get().LeakRsaEngine(pkey);
349 }
350
313 // Setup an EVP_PKEY to wrap an existing platform RSA PrivateKey object 351 // Setup an EVP_PKEY to wrap an existing platform RSA PrivateKey object
314 // for Android 4.0 to 4.1.x. Must only be used on Android < 4.2. 352 // for Android 4.0 to 4.1.x. Must only be used on Android < 4.2.
315 // |private_key| is a JNI reference (local or global) to the object. 353 // |private_key| is a JNI reference (local or global) to the object.
316 // |pkey| is the EVP_PKEY to setup as a wrapper. 354 // |pkey| is the EVP_PKEY to setup as a wrapper.
317 // Returns true on success, false otherwise. 355 // Returns true on success, false otherwise.
318 EVP_PKEY* GetRsaLegacyKey(jobject private_key) { 356 EVP_PKEY* GetRsaLegacyKey(jobject private_key) {
319 EVP_PKEY* sys_pkey = 357 EVP_PKEY* sys_pkey =
320 GetOpenSSLSystemHandleForPrivateKey(private_key); 358 GetOpenSSLSystemHandleForPrivateKey(private_key);
321 if (sys_pkey != NULL) { 359 if (sys_pkey != NULL) {
322 CRYPTO_add(&sys_pkey->references, 1, CRYPTO_LOCK_EVP_PKEY); 360 CRYPTO_add(&sys_pkey->references, 1, CRYPTO_LOCK_EVP_PKEY);
361 LeakRsaEngine(sys_pkey);
323 } else { 362 } else {
324 // GetOpenSSLSystemHandleForPrivateKey() will fail on Android 363 // GetOpenSSLSystemHandleForPrivateKey() will fail on Android
325 // 4.0.3 and earlier. However, it is possible to get the key 364 // 4.0.3 and earlier. However, it is possible to get the key
326 // content with PrivateKey.getEncoded() on these platforms. 365 // content with PrivateKey.getEncoded() on these platforms.
327 // Note that this method may return NULL on 4.0.4 and later. 366 // Note that this method may return NULL on 4.0.4 and later.
328 std::vector<uint8> encoded; 367 std::vector<uint8> encoded;
329 if (!GetPrivateKeyEncodedBytes(private_key, &encoded)) { 368 if (!GetPrivateKeyEncodedBytes(private_key, &encoded)) {
330 LOG(ERROR) << "Can't get private key data!"; 369 LOG(ERROR) << "Can't get private key data!";
331 return NULL; 370 return NULL;
332 } 371 }
(...skipping 354 matching lines...) Expand 10 before | Expand all | Expand 10 after
687 default: 726 default:
688 LOG(WARNING) 727 LOG(WARNING)
689 << "GetOpenSSLPrivateKeyWrapper() called with invalid key type"; 728 << "GetOpenSSLPrivateKeyWrapper() called with invalid key type";
690 return NULL; 729 return NULL;
691 } 730 }
692 return pkey.release(); 731 return pkey.release();
693 } 732 }
694 733
695 } // namespace android 734 } // namespace android
696 } // namespace net 735 } // namespace net
OLDNEW
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698