diff options
author | Rubin Xu <rubinxu@google.com> | 2019-10-09 11:24:57 +0100 |
---|---|---|
committer | Rubin Xu <rubinxu@google.com> | 2019-10-23 15:36:17 +0100 |
commit | 7629a4b45e3d2ca4ea943724c6fa690b8355de82 (patch) | |
tree | 33f4c813f552b6599c5c9dbeee22a93a48b40f33 | |
parent | f16720faaf1496d89aefed4884890b16eaeebb63 (diff) |
Handle the case when KeyChain binding fails
Binding to keychain can fail, for example when the target user
is being removed. Handle this case gracefully and do not block
the system server.
Bug: 139554671
Test: none
Change-Id: Ib68c873e367428b82f3cb2a81cafe1a59776336c
-rw-r--r-- | keystore/java/android/security/KeyChain.java | 48 | ||||
-rw-r--r-- | services/devicepolicy/java/com/android/server/devicepolicy/CertificateMonitor.java | 11 |
2 files changed, 40 insertions, 19 deletions
diff --git a/keystore/java/android/security/KeyChain.java b/keystore/java/android/security/KeyChain.java index 254456cea536..538319c3f1d7 100644 --- a/keystore/java/android/security/KeyChain.java +++ b/keystore/java/android/security/KeyChain.java @@ -763,28 +763,33 @@ public final class KeyChain { * @see KeyChain#bind */ public static class KeyChainConnection implements Closeable { - private final Context context; - private final ServiceConnection serviceConnection; - private final IKeyChainService service; + private final Context mContext; + private final ServiceConnection mServiceConnection; + private final IKeyChainService mService; protected KeyChainConnection(Context context, ServiceConnection serviceConnection, IKeyChainService service) { - this.context = context; - this.serviceConnection = serviceConnection; - this.service = service; + this.mContext = context; + this.mServiceConnection = serviceConnection; + this.mService = service; } @Override public void close() { - context.unbindService(serviceConnection); + mContext.unbindService(mServiceConnection); } + + /** returns the service binder. */ public IKeyChainService getService() { - return service; + return mService; } } /** - * @hide for reuse by CertInstaller and Settings. - * + * Bind to KeyChainService in the current user. * Caller should call unbindService on the result when finished. + * + *@throws InterruptedException if interrupted during binding. + *@throws AssertionError if unable to bind to KeyChainService. + * @hide for reuse by CertInstaller and Settings. */ @WorkerThread public static KeyChainConnection bind(@NonNull Context context) throws InterruptedException { @@ -792,6 +797,11 @@ public final class KeyChain { } /** + * Bind to KeyChainService in the target user. + * Caller should call unbindService on the result when finished. + * + * @throws InterruptedException if interrupted during binding. + * @throws AssertionError if unable to bind to KeyChainService. * @hide */ @WorkerThread @@ -814,6 +824,16 @@ public final class KeyChain { } } } + @Override public void onBindingDied(ComponentName name) { + if (!mConnectedAtLeastOnce) { + mConnectedAtLeastOnce = true; + try { + q.put(null); + } catch (InterruptedException e) { + // will never happen, since the queue starts with one available slot + } + } + } @Override public void onServiceDisconnected(ComponentName name) {} }; Intent intent = new Intent(IKeyChainService.class.getName()); @@ -823,7 +843,13 @@ public final class KeyChain { intent, keyChainServiceConnection, Context.BIND_AUTO_CREATE, user)) { throw new AssertionError("could not bind to KeyChainService"); } - return new KeyChainConnection(context, keyChainServiceConnection, q.take()); + IKeyChainService service = q.take(); + if (service != null) { + return new KeyChainConnection(context, keyChainServiceConnection, service); + } else { + context.unbindService(keyChainServiceConnection); + throw new AssertionError("KeyChainService died while binding"); + } } private static void ensureNotOnMainThread(@NonNull Context context) { diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/CertificateMonitor.java b/services/devicepolicy/java/com/android/server/devicepolicy/CertificateMonitor.java index 4a456f720db3..aa38880d6d7d 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/CertificateMonitor.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/CertificateMonitor.java @@ -17,7 +17,6 @@ package com.android.server.devicepolicy; import android.app.Notification; -import android.app.NotificationManager; import android.app.PendingIntent; import android.content.BroadcastReceiver; import android.content.Context; @@ -26,12 +25,9 @@ import android.content.IntentFilter; import android.content.pm.ActivityInfo; import android.content.pm.PackageManager; import android.content.res.Resources; -import android.graphics.Color; -import android.os.Build; import android.os.Handler; import android.os.RemoteException; import android.os.UserHandle; -import android.os.UserManager; import android.os.storage.StorageManager; import android.provider.Settings; import android.security.Credentials; @@ -39,9 +35,9 @@ import android.security.KeyChain; import android.security.KeyChain.KeyChainConnection; import android.util.Log; +import com.android.internal.R; import com.android.internal.messages.nano.SystemMessageProto.SystemMessage; import com.android.internal.notification.SystemNotificationChannels; -import com.android.internal.R; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -49,7 +45,6 @@ import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import java.util.List; -import java.util.Set; public class CertificateMonitor { protected static final String LOG_TAG = DevicePolicyManagerService.LOG_TAG; @@ -111,13 +106,13 @@ public class CertificateMonitor { } } - public List<String> getInstalledCaCertificates(UserHandle userHandle) + private List<String> getInstalledCaCertificates(UserHandle userHandle) throws RemoteException, RuntimeException { try (KeyChainConnection conn = mInjector.keyChainBindAsUser(userHandle)) { return conn.getService().getUserCaAliases().getList(); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - return null; + throw new RuntimeException(e); } catch (AssertionError e) { throw new RuntimeException(e); } |