Merge "Revisit the lifetime of the RebootEscrowProvider" am: d746cc814f am: fa83bba5bf

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1678645

Change-Id: I8d476ae93213ff09da715443943482ca436a499f

services/core/java/com/android/server/locksettings/RebootEscrowManager.java

@@ -205,6 +205,7 @@ class RebootEscrowManager {
                 Slog.i(TAG, "Using server based resume on reboot");
                 rebootEscrowProvider = new RebootEscrowProviderServerBasedImpl(mContext, mStorage);
             } else {
+                Slog.i(TAG, "Using HAL based resume on reboot");
                 rebootEscrowProvider = new RebootEscrowProviderHalImpl();
             }
 
@@ -239,7 +240,7 @@ class RebootEscrowManager {
             return mKeyStoreManager;
         }
 
-        public RebootEscrowProviderInterface getRebootEscrowProvider() {
+        public RebootEscrowProviderInterface createRebootEscrowProviderIfNeeded() {
             // Initialize for the provider lazily. Because the device_config and service
             // implementation apps may change when system server is running.
             if (mRebootEscrowProvider == null) {
@@ -249,6 +250,14 @@ class RebootEscrowManager {
             return mRebootEscrowProvider;
         }
 
+        public RebootEscrowProviderInterface getRebootEscrowProvider() {
+            return mRebootEscrowProvider;
+        }
+
+        public void clearRebootEscrowProvider() {
+            mRebootEscrowProvider = null;
+        }
+
         public int getBootCount() {
             return Settings.Global.getInt(mContext.getContentResolver(), Settings.Global.BOOT_COUNT,
                     0);
@@ -308,8 +317,6 @@ class RebootEscrowManager {
             mStorage.removeRebootEscrow(user.id);
         }
 
-        // Clear the old key in keystore.
-        mKeyStoreManager.clearKeyStoreEncryptionKey();
         onEscrowRestoreComplete(false, attemptCount);
     }
 
@@ -395,9 +402,6 @@ class RebootEscrowManager {
             allUsersUnlocked &= restoreRebootEscrowForUser(user.id, escrowKey, kk);
         }
 
-        // Clear the old key in keystore. A new key will be generated by new RoR requests.
-        mKeyStoreManager.clearKeyStoreEncryptionKey();
-
         if (!allUsersUnlocked && mLoadEscrowDataErrorCode == ERROR_NONE) {
             mLoadEscrowDataErrorCode = ERROR_UNLOCK_ALL_USERS;
         }
@@ -473,11 +477,17 @@ class RebootEscrowManager {
         if (success || (previousBootCount != -1 && bootCountDelta <= BOOT_COUNT_TOLERANCE)) {
             reportMetricOnRestoreComplete(success, attemptCount);
         }
+
+        // Clear the old key in keystore. A new key will be generated by new RoR requests.
+        mKeyStoreManager.clearKeyStoreEncryptionKey();
+        // Clear the saved reboot escrow provider
+        mInjector.clearRebootEscrowProvider();
         clearMetricsStorage();
     }
 
     private RebootEscrowKey getAndClearRebootEscrowKey(SecretKey kk) throws IOException {
-        RebootEscrowProviderInterface rebootEscrowProvider = mInjector.getRebootEscrowProvider();
+        RebootEscrowProviderInterface rebootEscrowProvider =
+                mInjector.createRebootEscrowProviderIfNeeded();
         if (rebootEscrowProvider == null) {
             Slog.w(TAG,
                     "Had reboot escrow data for users, but RebootEscrowProvider is unavailable");
@@ -529,9 +539,8 @@ class RebootEscrowManager {
             return;
         }
 
-        if (mInjector.getRebootEscrowProvider() == null) {
+        if (mInjector.createRebootEscrowProviderIfNeeded() == null) {
-            Slog.w(TAG,
+            Slog.w(TAG, "Not storing escrow data, RebootEscrowProvider is unavailable");
-                    "Had reboot escrow data for users, but RebootEscrowProvider is unavailable");
             return;
         }
 
@@ -586,13 +595,17 @@ class RebootEscrowManager {
         mRebootEscrowWanted = false;
         setRebootEscrowReady(false);
 
-        RebootEscrowProviderInterface rebootEscrowProvider = mInjector.getRebootEscrowProvider();
+        // We want to clear the internal data inside the provider, so always try to create the
+        // provider.
+        RebootEscrowProviderInterface rebootEscrowProvider =
+                mInjector.createRebootEscrowProviderIfNeeded();
         if (rebootEscrowProvider == null) {
             Slog.w(TAG, "RebootEscrowProvider is unavailable for clear request");
         } else {
             rebootEscrowProvider.clearRebootEscrowKey();
         }
 
+        mInjector.clearRebootEscrowProvider();
         clearMetricsStorage();
 
         List<UserInfo> users = mUserManager.getUsers();
@@ -610,8 +623,7 @@ class RebootEscrowManager {
 
         RebootEscrowProviderInterface rebootEscrowProvider = mInjector.getRebootEscrowProvider();
         if (rebootEscrowProvider == null) {
-            Slog.w(TAG,
+            Slog.w(TAG, "Not storing escrow key, RebootEscrowProvider is unavailable");
-                    "Had reboot escrow data for users, but RebootEscrowProvider is unavailable");
             clearRebootEscrowIfNeeded();
             return ARM_REBOOT_ERROR_NO_PROVIDER;
         }
@@ -677,11 +689,12 @@ class RebootEscrowManager {
     }
 
     boolean prepareRebootEscrow() {
-        if (mInjector.getRebootEscrowProvider() == null) {
+        clearRebootEscrowIfNeeded();
+        if (mInjector.createRebootEscrowProviderIfNeeded() == null) {
+            Slog.w(TAG, "No reboot escrow provider, skipping resume on reboot preparation.");
             return false;
         }
 
-        clearRebootEscrowIfNeeded();
         mRebootEscrowWanted = true;
         mEventLog.addEntry(RebootEscrowEvent.REQUESTED_LSKF);
         return true;
@@ -807,6 +820,10 @@ class RebootEscrowManager {
         pw.print("mPendingRebootEscrowKey is ");
         pw.println(keySet ? "set" : "not set");
 
+        RebootEscrowProviderInterface provider = mInjector.getRebootEscrowProvider();
+        String providerType = provider == null ? "null" : String.valueOf(provider.getType());
+        pw.print("RebootEscrowProvider type is " + providerType);
+
         pw.println();
         pw.println("Event log:");
         pw.increaseIndent();

services/tests/servicestests/src/com/android/server/locksettings/RebootEscrowManagerTests.java

@@ -112,14 +112,13 @@ public class RebootEscrowManagerTests {
     private MockableRebootEscrowInjected mInjected;
     private RebootEscrowManager mService;
     private SecretKey mAesKey;
+    private MockInjector mMockInjector;
 
     public interface MockableRebootEscrowInjected {
         int getBootCount();
 
         long getCurrentTimeMillis();
 
-        boolean forceServerBased();
-
         void reportMetric(boolean success, int errorCode, int serviceType, int attemptCount,
                 int escrowDurationInSeconds, int vbmetaDigestStatus, int durationSinceBootComplete);
     }
@@ -127,11 +126,12 @@ public class RebootEscrowManagerTests {
     static class MockInjector extends RebootEscrowManager.Injector {
         private final IRebootEscrow mRebootEscrow;
         private final ResumeOnRebootServiceConnection mServiceConnection;
-        private final RebootEscrowProviderInterface mRebootEscrowProvider;
+        private final RebootEscrowProviderInterface mDefaultRebootEscrowProvider;
         private final UserManager mUserManager;
         private final MockableRebootEscrowInjected mInjected;
         private final RebootEscrowKeyStoreManager mKeyStoreManager;
-        private final boolean mServerBased;
+        private boolean mServerBased;
+        private RebootEscrowProviderInterface mRebootEscrowProviderInUse;
 
         MockInjector(Context context, UserManager userManager,
                 IRebootEscrow rebootEscrow,
@@ -149,7 +149,7 @@ public class RebootEscrowManagerTests {
                             return mRebootEscrow;
                         }
                     };
-            mRebootEscrowProvider = new RebootEscrowProviderHalImpl(halInjector);
+            mDefaultRebootEscrowProvider = new RebootEscrowProviderHalImpl(halInjector);
             mUserManager = userManager;
             mKeyStoreManager = keyStoreManager;
             mInjected = injected;
@@ -166,7 +166,8 @@ public class RebootEscrowManagerTests {
             mServerBased = true;
             RebootEscrowProviderServerBasedImpl.Injector injector =
                     new RebootEscrowProviderServerBasedImpl.Injector(serviceConnection);
-            mRebootEscrowProvider = new RebootEscrowProviderServerBasedImpl(storage, injector);
+            mDefaultRebootEscrowProvider = new RebootEscrowProviderServerBasedImpl(
+                    storage, injector);
             mUserManager = userManager;
             mKeyStoreManager = keyStoreManager;
             mInjected = injected;
@@ -184,15 +185,23 @@ public class RebootEscrowManagerTests {
 
         @Override
         public boolean serverBasedResumeOnReboot() {
-            if (mInjected.forceServerBased()) {
-                return true;
-            }
             return mServerBased;
         }
 
+        @Override
+        public RebootEscrowProviderInterface createRebootEscrowProviderIfNeeded() {
+            mRebootEscrowProviderInUse = mDefaultRebootEscrowProvider;
+            return mRebootEscrowProviderInUse;
+        }
+
         @Override
         public RebootEscrowProviderInterface getRebootEscrowProvider() {
-            return mRebootEscrowProvider;
+            return mRebootEscrowProviderInUse;
+        }
+
+        @Override
+        public void clearRebootEscrowProvider() {
+            mRebootEscrowProviderInUse = null;
         }
 
         @Override
@@ -264,13 +273,15 @@ public class RebootEscrowManagerTests {
         when(mCallbacks.isUserSecure(NONSECURE_SECONDARY_USER_ID)).thenReturn(false);
         when(mCallbacks.isUserSecure(SECURE_SECONDARY_USER_ID)).thenReturn(true);
         mInjected = mock(MockableRebootEscrowInjected.class);
-        mService = new RebootEscrowManager(new MockInjector(mContext, mUserManager, mRebootEscrow,
+        mMockInjector = new MockInjector(mContext, mUserManager, mRebootEscrow,
-                mKeyStoreManager, mStorage, mInjected), mCallbacks, mStorage);
+                mKeyStoreManager, mStorage, mInjected);
+        mService = new RebootEscrowManager(mMockInjector, mCallbacks, mStorage);
     }
 
     private void setServerBasedRebootEscrowProvider() throws Exception {
-        mService = new RebootEscrowManager(new MockInjector(mContext, mUserManager,
+        mMockInjector = new MockInjector(mContext, mUserManager, mServiceConnection,
-                mServiceConnection, mKeyStoreManager, mStorage, mInjected), mCallbacks, mStorage);
+                mKeyStoreManager, mStorage, mInjected);
+        mService = new RebootEscrowManager(mMockInjector, mCallbacks, mStorage);
     }
 
     @Test
@@ -317,6 +328,7 @@ public class RebootEscrowManagerTests {
         doThrow(ServiceSpecificException.class).when(mRebootEscrow).storeKey(any());
         mService.clearRebootEscrow();
         verify(mRebootEscrow).storeKey(eq(new byte[32]));
+        assertNull(mMockInjector.getRebootEscrowProvider());
     }
 
     @Test
@@ -785,7 +797,7 @@ public class RebootEscrowManagerTests {
         assertNull(
                 mStorage.getString(RebootEscrowManager.REBOOT_ESCROW_ARMED_KEY, null, USER_SYSTEM));
         // Change the provider to server based, expect the reboot to fail
-        when(mInjected.forceServerBased()).thenReturn(true);
+        mMockInjector.mServerBased = true;
         assertEquals(ARM_REBOOT_ERROR_PROVIDER_MISMATCH, mService.armRebootEscrowIfNeeded());
         assertNull(
                 mStorage.getString(RebootEscrowManager.REBOOT_ESCROW_ARMED_KEY, null, USER_SYSTEM));