Refactor KeyChainKeyManager()'s sClientCertificateReferenceWorkaround
The referenced issue states that it is only applicable to Android < 4.2 (testing confirms the problem on 4.1.2, but not on 4.2.2). A test was added for the version code, primarily as a finder's aid for a day when K-9 Mail no longer supports Android < 4.2 and the work-around can be removed. The referenced issue also states that it is only necessary to hold a reference to the first PrivateKey retrieved. (Testing indicates that the problem is avoided so long at least one reference is always maintained to a PrivateKey -- it doesn't actually need to be a continuous reference to the first PrivateKey.) From my understanding, a normal class loader never unloads a class, so the static reference can be safely kept privately in KeyChainKeyManager.
This commit is contained in:
parent
51829a2451
commit
ada74db8d5
2 changed files with 25 additions and 21 deletions
|
@ -2,7 +2,6 @@
|
|||
package com.fsck.k9;
|
||||
|
||||
import java.io.File;
|
||||
import java.security.PrivateKey;
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
|
@ -1435,14 +1434,4 @@ public class K9 extends Application {
|
|||
editor.commit();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Holding a reference to PrivateKey selected for client certificate
|
||||
* authentication. We need to keep reference to this key so it won't get
|
||||
* garbage collected. If it will then the whole app will crash
|
||||
* on Android <= 4.2 with "Fatal signal 11 code=1".
|
||||
*
|
||||
* see https://code.google.com/p/android/issues/detail?id=62319
|
||||
*/
|
||||
public static ArrayList<PrivateKey> sClientCertificateReferenceWorkaround = new ArrayList<PrivateKey>();
|
||||
}
|
||||
|
|
|
@ -27,6 +27,8 @@ import com.fsck.k9.mail.ClientCertificateRequiredException;
|
|||
@TargetApi(Build.VERSION_CODES.ICE_CREAM_SANDWICH)
|
||||
public class KeyChainKeyManager extends X509ExtendedKeyManager {
|
||||
|
||||
private static PrivateKey sClientCertificateReferenceWorkaround;
|
||||
|
||||
private String mAlias;
|
||||
|
||||
public KeyChainKeyManager() {
|
||||
|
@ -87,20 +89,24 @@ public class KeyChainKeyManager extends X509ExtendedKeyManager {
|
|||
if (K9.DEBUG)
|
||||
Log.d(K9.LOG_TAG, "KeyChainKeyManager.getPrivateKey for " + alias);
|
||||
|
||||
PrivateKey key = KeyChain.getPrivateKey(K9.app, alias);
|
||||
PrivateKey key;
|
||||
|
||||
/*
|
||||
* We need to keep reference to the first private key retrieved so
|
||||
* it won't get garbage collected. If it will then the whole app
|
||||
* will crash on Android < 4.2 with "Fatal signal 11 code=1". See
|
||||
* https://code.google.com/p/android/issues/detail?id=62319
|
||||
*/
|
||||
if (sClientCertificateReferenceWorkaround == null
|
||||
&& Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1) {
|
||||
key = retrieveFirstPrivateKey(alias);
|
||||
} else {
|
||||
key = KeyChain.getPrivateKey(K9.app, alias);
|
||||
}
|
||||
|
||||
if (key == null) {
|
||||
throw new IllegalStateException("No private key found for: " + alias);
|
||||
}
|
||||
|
||||
/*
|
||||
* We need to keep reference to this key so it won't get garbage
|
||||
* collected. If it will then the whole app will crash on Android <=
|
||||
* 4.2 with "Fatal signal 11 code=1". See
|
||||
* https://code.google.com/p/android/issues/detail?id=62319
|
||||
*/
|
||||
K9.sClientCertificateReferenceWorkaround.add(key);
|
||||
|
||||
return key;
|
||||
} catch (KeyChainException e) {
|
||||
throw new RuntimeException(e);
|
||||
|
@ -110,6 +116,15 @@ public class KeyChainKeyManager extends X509ExtendedKeyManager {
|
|||
}
|
||||
}
|
||||
|
||||
private synchronized PrivateKey retrieveFirstPrivateKey(String alias)
|
||||
throws KeyChainException, InterruptedException {
|
||||
PrivateKey key = KeyChain.getPrivateKey(K9.app, alias);
|
||||
if (sClientCertificateReferenceWorkaround == null) {
|
||||
sClientCertificateReferenceWorkaround = key;
|
||||
}
|
||||
return key;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String chooseServerAlias(String keyType, Principal[] issuers, Socket socket) {
|
||||
// not valid for client side
|
||||
|
|
Loading…
Reference in a new issue