Insulate the legacy Android client auth code from OpenSSL ABI changes.

net/android/keystore_openssl.cc

Index: net/android/keystore_openssl.cc
diff --git a/net/android/keystore_openssl.cc b/net/android/keystore_openssl.cc
index ec08d70236a84018adcda576bac38b7c17990edf..44a000c2b42c3cc16bf812300fe5ddf58f55fb19 100644
--- a/net/android/keystore_openssl.cc
+++ b/net/android/keystore_openssl.cc
@@ -27,6 +27,7 @@
 #include "base/logging.h"
 #include "crypto/openssl_util.h"
 #include "net/android/keystore.h"
+#include "net/android/legacy_openssl.h"
 #include "net/ssl/ssl_client_cert_type.h"
 
 // IMPORTANT NOTE: The following code will currently only work when used
@@ -95,6 +96,7 @@
 // here, which saves a lot of complexity.
 
 using base::android::ScopedJavaGlobalRef;
+using base::android::ScopedJavaLocalRef;
 
 namespace net {
 namespace android {
@@ -112,6 +114,11 @@ typedef crypto::ScopedOpenSSL<EC_GROUP, EC_GROUP_free> ScopedEC_GROUP;
 // all other method pointers are either stubs returning errors, or no-ops.
 // See <openssl/rsa.h> for exact declaration of RSA_METHOD.
 
+struct RsaAppData {
+  jobject private_key;
+  AndroidRSA* legacy_rsa;
+};
+
 int RsaMethodPubEnc(int flen,
                     const unsigned char* from,
                     unsigned char* to,
@@ -154,18 +161,33 @@ int RsaMethodPrivEnc(int flen,
   }
 
   // Retrieve private key JNI reference.
-  jobject private_key = reinterpret_cast<jobject>(RSA_get_app_data(rsa));
-  if (!private_key) {
+  RsaAppData* app_data = static_cast<RsaAppData*>(RSA_get_app_data(rsa));
+  if (!app_data || !app_data->private_key) {
     LOG(WARNING) << "Null JNI reference passed to RsaMethodPrivEnc!";
     RSAerr(RSA_F_RSA_PRIVATE_ENCRYPT, ERR_R_INTERNAL_ERROR);
     return -1;
   }
 
+  // Pre-4.2 legacy codepath.
+  if (app_data->legacy_rsa) {
+    int ret = app_data->legacy_rsa->meth->rsa_priv_enc(
+        flen, from, to, app_data->legacy_rsa, ANDROID_RSA_PKCS1_PADDING);
+    if (ret < 0) {
+      LOG(WARNING) << "Could not sign message in RsaMethodPrivEnc!";
+      // System OpenSSL will use a separate error queue, so it is still
+      // necessary to push a new error.
+      RSAerr(RSA_F_RSA_PRIVATE_ENCRYPT, ERR_R_INTERNAL_ERROR);

[agl, 2014/07/10 16:33:44]

Do we need to clear the system's error queue?

[davidben, 2014/07/10 21:47:07]

Added a TODO, but doing so seems to be difficult. I tried dlopen("libcrypto.so")
to get at the function, but libcrypto doesn't get loaded through dlopen, so we
end up with two copies of the library. I got some bizarre hangs that didn't
occur without the CL, so I assume something broke from that.

The other option I can see is to find some random Java function to call which
happens to dump the error queue as a side effect.

+      return -1;
+    }
+    return ret;
+  }
+
   base::StringPiece from_piece(reinterpret_cast<const char*>(from), flen);
   std::vector<uint8> result;
   // For RSA keys, this function behaves as RSA_private_encrypt with
   // PKCS#1 padding.
-  if (!RawSignDigestWithPrivateKey(private_key, from_piece, &result)) {
+  if (!RawSignDigestWithPrivateKey(app_data->private_key,
+                                   from_piece, &result)) {
     LOG(WARNING) << "Could not sign message in RsaMethodPrivEnc!";
     RSAerr(RSA_F_RSA_PRIVATE_ENCRYPT, ERR_R_INTERNAL_ERROR);
     return -1;
@@ -205,10 +227,11 @@ int RsaMethodInit(RSA* rsa) {
 int RsaMethodFinish(RSA* rsa) {
   // Ensure the global JNI reference created with this wrapper is
   // properly destroyed with it.
-  jobject key = reinterpret_cast<jobject>(RSA_get_app_data(rsa));
-  if (key != NULL) {
+  RsaAppData* app_data = static_cast<RsaAppData*>(RSA_get_app_data(rsa));
+  if (app_data != NULL) {
     RSA_set_app_data(rsa, NULL);
-    ReleaseKey(key);
+    ReleaseKey(app_data->private_key);
+    delete app_data;
   }
   // Actual return value is ignored by OpenSSL. There are no docs
   // explaining what this is supposed to be.
@@ -282,13 +305,18 @@ bool SwapBigNumPtrFromBytes(const std::vector<uint8>& new_bytes,
 // free |private_key| after the call.
 // IMPORTANT: The EVP_PKEY will *only* work on Android >= 4.2. For older
 // platforms, use GetRsaLegacyKey() instead.
-bool GetRsaPkeyWrapper(jobject private_key, EVP_PKEY* pkey) {
+bool GetRsaPkeyWrapper(jobject private_key,
+                       AndroidRSA* legacy_rsa,
+                       EVP_PKEY* pkey) {
   ScopedRSA rsa(RSA_new());
   RSA_set_method(rsa.get(), &android_rsa_method);
 
   // HACK: RSA_size() doesn't work with custom RSA_METHODs. To ensure that
   // it will return the right value, set the 'n' field of the RSA object
   // to match the private key's modulus.
+  //
+  // TODO(davidben): After switching to BoringSSL, consider making RSA_size call
+  // into an RSA_METHOD hook.
   std::vector<uint8> modulus;
   if (!GetRSAKeyModulus(private_key, &modulus)) {
     LOG(ERROR) << "Failed to get private key modulus";
@@ -305,7 +333,10 @@ bool GetRsaPkeyWrapper(jobject private_key, EVP_PKEY* pkey) {
     LOG(ERROR) << "Could not create global JNI reference";
     return false;
   }
-  RSA_set_app_data(rsa.get(), global_key.Release());
+  RsaAppData* app_data = new RsaAppData();
+  app_data->private_key = global_key.Release();
+  app_data->legacy_rsa = legacy_rsa;
+  RSA_set_app_data(rsa.get(), app_data);
   EVP_PKEY_assign_RSA(pkey, rsa.release());
   return true;
 }
@@ -322,30 +353,28 @@ bool GetRsaPkeyWrapper(jobject private_key, EVP_PKEY* pkey) {
 // https://crbug.com/381465
 class KeystoreEngineWorkaround {
  public:
-  KeystoreEngineWorkaround() : leaked_engine_(false) {}
+  KeystoreEngineWorkaround() {}
 
-  void LeakRsaEngine(EVP_PKEY* pkey) {
-    if (leaked_engine_)
+  void LeakEngine(jobject private_key) {
+    if (!engine_.is_null())
       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)) {
+    ScopedJavaLocalRef<jobject> engine =
+        GetOpenSSLEngineForPrivateKey(private_key);
+    if (engine.is_null()) {
       NOTREACHED();
       return;
     }
-    leaked_engine_ = true;
+    engine_.Reset(engine);
   }
 
  private:
-  bool leaked_engine_;
+  ScopedJavaGlobalRef<jobject> engine_;
 };
 
-void LeakRsaEngine(EVP_PKEY* pkey) {
+void LeakEngine(jobject private_key) {
   static base::LazyInstance<KeystoreEngineWorkaround>::Leaky s_instance =
       LAZY_INSTANCE_INITIALIZER;
-  s_instance.Get().LeakRsaEngine(pkey);
+  s_instance.Get().LeakEngine(private_key);
 }
 
 // Setup an EVP_PKEY to wrap an existing platform RSA PrivateKey object
@@ -354,31 +383,49 @@ void LeakRsaEngine(EVP_PKEY* pkey) {
 // |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 =
+  AndroidEVP_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!";
+    if (sys_pkey->type != ANDROID_EVP_PKEY_RSA) {
+      LOG(ERROR) << "Private key has wrong type!";
       return NULL;
     }
-    const unsigned char* p =
-        reinterpret_cast<const unsigned char*>(&encoded[0]);
-    int len = static_cast<int>(encoded.size());
-    sys_pkey = d2i_AutoPrivateKey(NULL, &p, len);
-    if (sys_pkey == NULL) {
-      LOG(ERROR) << "Can't convert private key data!";
-      return NULL;
+
+    AndroidRSA* sys_rsa = sys_pkey->pkey.rsa;
+    if (sys_rsa->engine) {
+      // |private_key| may not have an engine if the PrivateKey did not come
+      // from the key store, such as in unit tests.
+      if (!strcmp(sys_rsa->engine->id, "keystore")) {
+        LeakEngine(private_key);
+      } else {
+        NOTREACHED();
+      }
     }
+
+    ScopedEVP_PKEY pkey(EVP_PKEY_new());
+    if (!GetRsaPkeyWrapper(private_key, sys_rsa, pkey.get()))
+      return NULL;
+    return pkey.release();
+  }
+
+  // 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;
+  }
+  const unsigned char* p =
+      reinterpret_cast<const unsigned char*>(&encoded[0]);
+  int len = static_cast<int>(encoded.size());
+  EVP_PKEY* pkey = d2i_AutoPrivateKey(NULL, &p, len);
+  if (pkey == NULL) {
+    LOG(ERROR) << "Can't convert private key data!";
+    return NULL;
   }
-  return sys_pkey;
+  return pkey;
 }
 
 // Custom DSA_METHOD that uses the platform APIs.
@@ -710,7 +757,7 @@ EVP_PKEY* GetOpenSSLPrivateKeyWrapper(jobject private_key) {
           pkey.reset(legacy_key);
         } else {
           // Running on Android 4.2.
-          if (!GetRsaPkeyWrapper(private_key, pkey.get()))
+          if (!GetRsaPkeyWrapper(private_key, NULL, pkey.get()))
             return NULL;
         }
       }