PawletOS/app_Settings
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix TrustedCredentialsSettings NPE

Browse Source 
[Cause of Defect]
TrustedCredentialsSettings#mKeyChainConnectionByProfileId is get/set by
more than one thread. Main thread would set it in onDestroy method, and
AsyncTask would get and set in the doInBackground method. So
mKeyChainConnectionByProfileId.get(profileId) would get null after
onDestroy method get called.

Bug: N/A
Test: Debugger to simulate concurrency
Change-Id: I0664d1e9b88b079855354ce0e6fe014a98a22327
Signed-off-by: daqi <daqi@xiaomi.com>
This commit is contained in:
daqi
2017-09-05 09:53:05 +08:00
parent 0bed3cacd7
commit be47df7395
1 changed files with 86 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

162
src/com/android/settings/TrustedCredentialsSettings.java
View File

@@ -61,6 +61,7 @@ import android.widget.Switch;
import android.widget.TabHost; import android.widget.TabHost;
import android.widget.TextView; import android.widget.TextView;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.app.UnlaunchableAppActivity; import com.android.internal.app.UnlaunchableAppActivity;
import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.internal.logging.nano.MetricsProto.MetricsEvent;
import com.android.internal.widget.LockPatternUtils; import com.android.internal.widget.LockPatternUtils;
@@ -152,6 +153,7 @@ public class TrustedCredentialsSettings extends OptionsMenuFragment
private int mConfirmingCredentialUser; private int mConfirmingCredentialUser;
private IntConsumer mConfirmingCredentialListener; private IntConsumer mConfirmingCredentialListener;
private Set<AdapterData.AliasLoader> mAliasLoaders = new ArraySet<AdapterData.AliasLoader>(2); private Set<AdapterData.AliasLoader> mAliasLoaders = new ArraySet<AdapterData.AliasLoader>(2);
@GuardedBy("mKeyChainConnectionByProfileId")
private final SparseArray<KeyChainConnection> private final SparseArray<KeyChainConnection>
mKeyChainConnectionByProfileId = new SparseArray<KeyChainConnection>(); mKeyChainConnectionByProfileId = new SparseArray<KeyChainConnection>();
@@ -256,11 +258,13 @@ public class TrustedCredentialsSettings extends OptionsMenuFragment
} }
private void closeKeyChainConnections() { private void closeKeyChainConnections() {
final int n = mKeyChainConnectionByProfileId.size(); synchronized (mKeyChainConnectionByProfileId) {
for (int i = 0; i < n; ++i) { final int n = mKeyChainConnectionByProfileId.size();
mKeyChainConnectionByProfileId.valueAt(i).close(); for (int i = 0; i < n; ++i) {
mKeyChainConnectionByProfileId.valueAt(i).close();
}
mKeyChainConnectionByProfileId.clear();
} }
mKeyChainConnectionByProfileId.clear();
} }
private void addTab(Tab tab) { private void addTab(Tab tab) {
@@ -684,62 +688,64 @@ public class TrustedCredentialsSettings extends OptionsMenuFragment
SparseArray<List<CertHolder>> certHoldersByProfile = SparseArray<List<CertHolder>> certHoldersByProfile =
new SparseArray<List<CertHolder>>(); new SparseArray<List<CertHolder>>();
try { try {
List<UserHandle> profiles = mUserManager.getUserProfiles(); synchronized(mKeyChainConnectionByProfileId) {
final int n = profiles.size(); List<UserHandle> profiles = mUserManager.getUserProfiles();
// First we get all aliases for all profiles in order to show progress final int n = profiles.size();
// correctly. Otherwise this could all be in a single loop. // First we get all aliases for all profiles in order to show progress
SparseArray<List<String>> aliasesByProfileId = new SparseArray< // correctly. Otherwise this could all be in a single loop.
List<String>>(n); SparseArray<List<String>> aliasesByProfileId = new SparseArray<
int max = 0; List<String>>(n);
int progress = 0; int max = 0;
for (int i = 0; i < n; ++i) { int progress = 0;
UserHandle profile = profiles.get(i); for (int i = 0; i < n; ++i) {
int profileId = profile.getIdentifier(); UserHandle profile = profiles.get(i);
if (shouldSkipProfile(profile)) { int profileId = profile.getIdentifier();
continue; if (shouldSkipProfile(profile)) {
continue;
}
KeyChainConnection keyChainConnection = KeyChain.bindAsUser(mContext,
profile);
// Saving the connection for later use on the certificate dialog.
mKeyChainConnectionByProfileId.put(profileId, keyChainConnection);
IKeyChainService service = keyChainConnection.getService();
List<String> aliases = mTab.getAliases(service);
if (isCancelled()) {
return new SparseArray<List<CertHolder>>();
}
max += aliases.size();
aliasesByProfileId.put(profileId, aliases);
} }
KeyChainConnection keyChainConnection = KeyChain.bindAsUser(mContext, for (int i = 0; i < n; ++i) {
profile); UserHandle profile = profiles.get(i);
// Saving the connection for later use on the certificate dialog. int profileId = profile.getIdentifier();
mKeyChainConnectionByProfileId.put(profileId, keyChainConnection); List<String> aliases = aliasesByProfileId.get(profileId);
IKeyChainService service = keyChainConnection.getService(); if (isCancelled()) {
List<String> aliases = mTab.getAliases(service); return new SparseArray<List<CertHolder>>();
if (isCancelled()) { }
return new SparseArray<List<CertHolder>>(); KeyChainConnection keyChainConnection = mKeyChainConnectionByProfileId.get(
profileId);
if (shouldSkipProfile(profile) || aliases == null
|| keyChainConnection == null) {
certHoldersByProfile.put(profileId, new ArrayList<CertHolder>(0));
continue;
}
IKeyChainService service = keyChainConnection.getService();
List<CertHolder> certHolders = new ArrayList<CertHolder>(max);
final int aliasMax = aliases.size();
for (int j = 0; j < aliasMax; ++j) {
String alias = aliases.get(j);
byte[] encodedCertificate = service.getEncodedCaCertificate(alias,
true);
X509Certificate cert = KeyChain.toCertificate(encodedCertificate);
certHolders.add(new CertHolder(service, mAdapter,
mTab, alias, cert, profileId));
publishProgress(++progress, max);
}
Collections.sort(certHolders);
certHoldersByProfile.put(profileId, certHolders);
} }
max += aliases.size(); return certHoldersByProfile;
aliasesByProfileId.put(profileId, aliases);
} }
for (int i = 0; i < n; ++i) {
UserHandle profile = profiles.get(i);
int profileId = profile.getIdentifier();
List<String> aliases = aliasesByProfileId.get(profileId);
if (isCancelled()) {
return new SparseArray<List<CertHolder>>();
}
KeyChainConnection keyChainConnection = mKeyChainConnectionByProfileId.get(
profileId);
if (shouldSkipProfile(profile) || aliases == null
|| keyChainConnection == null) {
certHoldersByProfile.put(profileId, new ArrayList<CertHolder>(0));
continue;
}
IKeyChainService service = keyChainConnection.getService();
List<CertHolder> certHolders = new ArrayList<CertHolder>(max);
final int aliasMax = aliases.size();
for (int j = 0; j < aliasMax; ++j) {
String alias = aliases.get(j);
byte[] encodedCertificate = service.getEncodedCaCertificate(alias,
true);
X509Certificate cert = KeyChain.toCertificate(encodedCertificate);
certHolders.add(new CertHolder(service, mAdapter,
mTab, alias, cert, profileId));
publishProgress(++progress, max);
}
Collections.sort(certHolders);
certHoldersByProfile.put(profileId, certHolders);
}
return certHoldersByProfile;
} catch (RemoteException e) { } catch (RemoteException e) {
Log.e(TAG, "Remote exception while loading aliases.", e); Log.e(TAG, "Remote exception while loading aliases.", e);
return new SparseArray<List<CertHolder>>(); return new SparseArray<List<CertHolder>>();
@@ -936,16 +942,18 @@ public class TrustedCredentialsSettings extends OptionsMenuFragment
public List<X509Certificate> getX509CertsFromCertHolder(CertHolder certHolder) { public List<X509Certificate> getX509CertsFromCertHolder(CertHolder certHolder) {
List<X509Certificate> certificates = null; List<X509Certificate> certificates = null;
try { try {
KeyChainConnection keyChainConnection = mKeyChainConnectionByProfileId.get( synchronized (mKeyChainConnectionByProfileId) {
certHolder.mProfileId); KeyChainConnection keyChainConnection = mKeyChainConnectionByProfileId.get(
IKeyChainService service = keyChainConnection.getService(); certHolder.mProfileId);
List<String> chain = service.getCaCertificateChainAliases(certHolder.mAlias, true); IKeyChainService service = keyChainConnection.getService();
final int n = chain.size(); List<String> chain = service.getCaCertificateChainAliases(certHolder.mAlias, true);
certificates = new ArrayList<X509Certificate>(n); final int n = chain.size();
for (int i = 0; i < n; ++i) { certificates = new ArrayList<X509Certificate>(n);
byte[] encodedCertificate = service.getEncodedCaCertificate(chain.get(i), true); for (int i = 0; i < n; ++i) {
X509Certificate certificate = KeyChain.toCertificate(encodedCertificate); byte[] encodedCertificate = service.getEncodedCaCertificate(chain.get(i), true);
certificates.add(certificate); X509Certificate certificate = KeyChain.toCertificate(encodedCertificate);
certificates.add(certificate);
}
} }
} catch (RemoteException ex) { } catch (RemoteException ex) {
Log.e(TAG, "RemoteException while retrieving certificate chain for root " Log.e(TAG, "RemoteException while retrieving certificate chain for root "
@@ -985,15 +993,17 @@ public class TrustedCredentialsSettings extends OptionsMenuFragment
@Override @Override
protected Boolean doInBackground(Void... params) { protected Boolean doInBackground(Void... params) {
try { try {
KeyChainConnection keyChainConnection = mKeyChainConnectionByProfileId.get( synchronized (mKeyChainConnectionByProfileId) {
mCertHolder.mProfileId); KeyChainConnection keyChainConnection = mKeyChainConnectionByProfileId.get(
IKeyChainService service = keyChainConnection.getService(); mCertHolder.mProfileId);
if (mCertHolder.mDeleted) { IKeyChainService service = keyChainConnection.getService();
byte[] bytes = mCertHolder.mX509Cert.getEncoded(); if (mCertHolder.mDeleted) {
service.installCaCertificate(bytes); byte[] bytes = mCertHolder.mX509Cert.getEncoded();
return true; service.installCaCertificate(bytes);
} else { return true;
return service.deleteCaCertificate(mCertHolder.mAlias); } else {
return service.deleteCaCertificate(mCertHolder.mAlias);
}
} }
} catch (CertificateEncodingException | SecurityException | IllegalStateException } catch (CertificateEncodingException | SecurityException | IllegalStateException
| RemoteException e) { | RemoteException e) {