More thorough cleansing of expired users

If any /data/system_[c|d]e folders were not erased
when the user was removed (maybe due to a reboot),
make sure they're cleaned up on restart as well
as when the userId is recycled later.

Mark the users' system folders with the correct
serial number for later verification.

AccountManager shouldn't be querying accounts of
partially created/destroyed users.

Change-Id: I4313756b7464f34cd5ce4fb296d61daa50b41fcb
Fixes: 29285673

diff --git a/core/java/android/content/Context.java b/core/java/android/content/Context.java
index 108350a..0881c9c 100644 (file)
--- a/core/java/android/content/Context.java
+++ b/core/java/android/content/Context.java
@@ -3646,6 +3646,12 @@ public abstract class Context {
     public static final String SYSTEM_HEALTH_SERVICE = "systemhealth";
 
     /**
+     * Gatekeeper Service.
+     * @hide
+     */
+    public static final String GATEKEEPER_SERVICE = "android.service.gatekeeper.IGateKeeperService";
+
+    /**
      * Determine whether the given permission is allowed for a particular
      * process and user ID running in the system.
      *

diff --git a/core/java/android/os/Environment.java b/core/java/android/os/Environment.java
index f6e8940..d8be2b6 100644 (file)
--- a/core/java/android/os/Environment.java
+++ b/core/java/android/os/Environment.java
@@ -240,6 +240,22 @@ public class Environment {
         return new File(getDataDirectory(), "system");
     }
 
+    /**
+     * Returns the base directory for per-user system directory, device encrypted.
+     * {@hide}
+     */
+    public static File getDataSystemDeDirectory() {
+        return buildPath(getDataDirectory(), "system_de");
+    }
+
+    /**
+     * Returns the base directory for per-user system directory, credential encrypted.
+     * {@hide}
+     */
+    public static File getDataSystemCeDirectory() {
+        return buildPath(getDataDirectory(), "system_ce");
+    }
+
     /** {@hide} */
     public static File getDataSystemCeDirectory(int userId) {
         return buildPath(getDataDirectory(), "system_ce", String.valueOf(userId));

diff --git a/core/java/android/os/UserManager.java b/core/java/android/os/UserManager.java
index 7146448..bcc8d46 100644 (file)
--- a/core/java/android/os/UserManager.java
+++ b/core/java/android/os/UserManager.java
@@ -1496,7 +1496,9 @@ public class UserManager {
     }
 
     /**
-     * Returns information for all users on this device.
+     * Returns information for all users on this device, including ones marked for deletion.
+     * To retrieve only users that are alive, use {@link #getUsers(boolean)}.
+     * <p>
      * Requires {@link android.Manifest.permission#MANAGE_USERS} permission.
      * @return the list of users that exist on the device.
      * @hide

diff --git a/keystore/java/android/security/GateKeeper.java b/keystore/java/android/security/GateKeeper.java
index c1df28c..7a2cbd0 100644 (file)
--- a/keystore/java/android/security/GateKeeper.java
+++ b/keystore/java/android/security/GateKeeper.java
@@ -16,6 +16,7 @@
 
 package android.security;
 
+import android.content.Context;
 import android.os.RemoteException;
 import android.os.ServiceManager;
 import android.os.UserHandle;
@@ -32,7 +33,7 @@ public abstract class GateKeeper {
 
     public static IGateKeeperService getService() {
         IGateKeeperService service = IGateKeeperService.Stub.asInterface(
-                ServiceManager.getService("android.service.gatekeeper.IGateKeeperService"));
+                ServiceManager.getService(Context.GATEKEEPER_SERVICE));
         if (service == null) {
             throw new IllegalStateException("Gatekeeper service not available");
         }

diff --git a/services/core/java/com/android/server/LockSettingsService.java b/services/core/java/com/android/server/LockSettingsService.java
index eeb20bf..1e715f9 100644 (file)
--- a/services/core/java/com/android/server/LockSettingsService.java
+++ b/services/core/java/com/android/server/LockSettingsService.java
@@ -1588,7 +1588,7 @@ public class LockSettingsService extends ILockSettings.Stub {
         }
 
         final IBinder service =
-            ServiceManager.getService("android.service.gatekeeper.IGateKeeperService");
+            ServiceManager.getService(Context.GATEKEEPER_SERVICE);
         if (service != null) {
             service.linkToDeath(new GateKeeperDiedRecipient(), 0);
             mGateKeeperService = IGateKeeperService.Stub.asInterface(service);

diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java
index 4084542..0cf5172 100644 (file)
--- a/services/core/java/com/android/server/accounts/AccountManagerService.java
+++ b/services/core/java/com/android/server/accounts/AccountManagerService.java
@@ -3425,7 +3425,7 @@ public class AccountManagerService
     /** {@hide} */
     @NonNull
     public AccountAndUser[] getAllAccounts() {
-        final List<UserInfo> users = getUserManager().getUsers();
+        final List<UserInfo> users = getUserManager().getUsers(true);
         final int[] userIds = new int[users.size()];
         for (int i = 0; i < userIds.length; i++) {
             userIds[i] = users.get(i).id;

diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index f2fde0f..a560c83 100644 (file)
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -19352,10 +19352,18 @@ Slog.v(TAG, ":: stepped forward, applying functor at tag " + parser.getName());
             if ((flags & StorageManager.FLAG_STORAGE_DE) != 0 && !mOnlyCore) {
                 UserManagerService.enforceSerialNumber(
                         Environment.getDataUserDeDirectory(volumeUuid, userId), userSerial);
+                if (Objects.equals(volumeUuid, StorageManager.UUID_PRIVATE_INTERNAL)) {
+                    UserManagerService.enforceSerialNumber(
+                            Environment.getDataSystemDeDirectory(userId), userSerial);
+                }
             }
             if ((flags & StorageManager.FLAG_STORAGE_CE) != 0 && !mOnlyCore) {
                 UserManagerService.enforceSerialNumber(
                         Environment.getDataUserCeDirectory(volumeUuid, userId), userSerial);
+                if (Objects.equals(volumeUuid, StorageManager.UUID_PRIVATE_INTERNAL)) {
+                    UserManagerService.enforceSerialNumber(
+                            Environment.getDataSystemCeDirectory(userId), userSerial);
+                }
             }
 
             synchronized (mInstallLock) {
@@ -19424,6 +19432,10 @@ Slog.v(TAG, ":: stepped forward, applying functor at tag " + parser.getName());
                 .listFilesOrEmpty(Environment.getDataUserDeDirectory(volumeUuid)));
         Collections.addAll(files, FileUtils
                 .listFilesOrEmpty(Environment.getDataUserCeDirectory(volumeUuid)));
+        Collections.addAll(files, FileUtils
+                .listFilesOrEmpty(Environment.getDataSystemDeDirectory()));
+        Collections.addAll(files, FileUtils
+                .listFilesOrEmpty(Environment.getDataSystemCeDirectory()));
         for (File file : files) {
             if (!file.isDirectory()) continue;
 

diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java
index a8cf110..a85064b 100644 (file)
--- a/services/core/java/com/android/server/pm/UserManagerService.java
+++ b/services/core/java/com/android/server/pm/UserManagerService.java
@@ -65,6 +65,8 @@ import android.os.UserManager;
 import android.os.UserManagerInternal;
 import android.os.UserManagerInternal.UserRestrictionsListener;
 import android.os.storage.StorageManager;
+import android.security.GateKeeper;
+import android.service.gatekeeper.IGateKeeperService;
 import android.system.ErrnoException;
 import android.system.Os;
 import android.system.OsConstants;
@@ -87,6 +89,7 @@ import com.android.internal.util.Preconditions;
 import com.android.internal.util.XmlUtils;
 import com.android.internal.widget.LockPatternUtils;
 import com.android.server.LocalServices;
+import com.android.server.SystemService;
 import com.android.server.am.UserState;
 
 import libcore.io.IoUtils;
@@ -119,6 +122,7 @@ import java.util.List;
  * </ul>
  */
 public class UserManagerService extends IUserManager.Stub {
+
     private static final String LOG_TAG = "UserManagerService";
     static final boolean DBG = false; // DO NOT SUBMIT WITH TRUE
     private static final boolean DBG_WITH_STACKTRACE = false; // DO NOT SUBMIT WITH TRUE
@@ -367,6 +371,31 @@ public class UserManagerService extends IUserManager.Stub {
         }
     }
 
+    public static class LifeCycle extends SystemService {
+
+        private UserManagerService mUms;
+
+        /**
+         * @param context
+         */
+        public LifeCycle(Context context) {
+            super(context);
+        }
+
+        @Override
+        public void onStart() {
+            mUms = UserManagerService.getInstance();
+            publishBinderService(Context.USER_SERVICE, mUms);
+        }
+
+        @Override
+        public void onBootPhase(int phase) {
+            if (phase == SystemService.PHASE_ACTIVITY_MANAGER_READY) {
+                mUms.cleanupPartialUsers();
+            }
+        }
+    }
+
     @VisibleForTesting
     UserManagerService(File dataDir) {
         this(null, null, new Object(), dataDir);
@@ -408,25 +437,6 @@ public class UserManagerService extends IUserManager.Stub {
     }
 
     void systemReady() {
-        // Prune out any partially created, partially removed and ephemeral users.
-        ArrayList<UserInfo> partials = new ArrayList<>();
-        synchronized (mUsersLock) {
-            final int userSize = mUsers.size();
-            for (int i = 0; i < userSize; i++) {
-                UserInfo ui = mUsers.valueAt(i).info;
-                if ((ui.partial || ui.guestToRemove || ui.isEphemeral()) && i != 0) {
-                    partials.add(ui);
-                }
-            }
-        }
-        final int partialsSize = partials.size();
-        for (int i = 0; i < partialsSize; i++) {
-            UserInfo ui = partials.get(i);
-            Slog.w(LOG_TAG, "Removing partially created user " + ui.id
-                    + " (name=" + ui.name + ")");
-            removeUserState(ui.id);
-        }
-
         mAppOpsService = IAppOpsService.Stub.asInterface(
                 ServiceManager.getService(Context.APP_OPS_SERVICE));
 
@@ -447,6 +457,27 @@ public class UserManagerService extends IUserManager.Stub {
                 null, mHandler);
     }
 
+    void cleanupPartialUsers() {
+        // Prune out any partially created, partially removed and ephemeral users.
+        ArrayList<UserInfo> partials = new ArrayList<>();
+        synchronized (mUsersLock) {
+            final int userSize = mUsers.size();
+            for (int i = 0; i < userSize; i++) {
+                UserInfo ui = mUsers.valueAt(i).info;
+                if ((ui.partial || ui.guestToRemove || ui.isEphemeral()) && i != 0) {
+                    partials.add(ui);
+                }
+            }
+        }
+        final int partialsSize = partials.size();
+        for (int i = 0; i < partialsSize; i++) {
+            UserInfo ui = partials.get(i);
+            Slog.w(LOG_TAG, "Removing partially created user " + ui.id
+                    + " (name=" + ui.name + ")");
+            removeUserState(ui.id);
+        }
+    }
+
     @Override
     public String getUserAccount(int userId) {
         checkManageUserAndAcrossUsersFullPermission("get user account");
@@ -2472,8 +2503,23 @@ public class UserManagerService extends IUserManager.Stub {
                 "Destroying key for user " + userHandle + " failed, continuing anyway", e);
         }
 
+        // Cleanup gatekeeper secure user id
+        try {
+            final IGateKeeperService gk = GateKeeper.getService();
+            if (gk != null) {
+                gk.clearSecureUserId(userHandle);
+            }
+        } catch (Exception ex) {
+            Slog.w(LOG_TAG, "unable to clear GK secure user id");
+        }
+
         // Cleanup package manager settings
         mPm.cleanUpUser(this, userHandle);
+
+        // Clean up all data before removing metadata
+        mPm.destroyUserData(userHandle,
+                StorageManager.FLAG_STORAGE_DE | StorageManager.FLAG_STORAGE_CE);
+
         // Remove this user from the list
         synchronized (mUsersLock) {
             mUsers.remove(userHandle);
@@ -2496,12 +2542,6 @@ public class UserManagerService extends IUserManager.Stub {
         AtomicFile userFile = new AtomicFile(new File(mUsersDir, userHandle + XML_SUFFIX));
         userFile.delete();
         updateUserIds();
-
-        // Now that we've purged all the metadata above, destroy the actual data
-        // on disk; if we battery pull in here we'll finish cleaning up when
-        // reconciling after reboot.
-        mPm.destroyUserData(userHandle,
-                StorageManager.FLAG_STORAGE_DE | StorageManager.FLAG_STORAGE_CE);
     }
 
     private void sendProfileRemovedBroadcast(int parentUserId, int removedUserId) {

diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/Owners.java b/services/devicepolicy/java/com/android/server/devicepolicy/Owners.java
index 1ae1a77..b53933e 100644 (file)
--- a/services/devicepolicy/java/com/android/server/devicepolicy/Owners.java
+++ b/services/devicepolicy/java/com/android/server/devicepolicy/Owners.java
@@ -120,7 +120,7 @@ class Owners {
             // First, try to read from the legacy file.
             final File legacy = getLegacyConfigFileWithTestOverride();
 
-            final List<UserInfo> users = mUserManager.getUsers();
+            final List<UserInfo> users = mUserManager.getUsers(true);
 
             if (readLegacyOwnerFileLocked(legacy)) {
                 if (DEBUG) {

diff --git a/services/java/com/android/server/SystemServer.java b/services/java/com/android/server/SystemServer.java
index 476a559..f59b2ff 100644 (file)
--- a/services/java/com/android/server/SystemServer.java
+++ b/services/java/com/android/server/SystemServer.java
@@ -470,7 +470,7 @@ public final class SystemServer {
         }
 
         traceBeginAndSlog("StartUserManagerService");
-        ServiceManager.addService(Context.USER_SERVICE, UserManagerService.getInstance());
+        mSystemServiceManager.startService(UserManagerService.LifeCycle.class);
         Trace.traceEnd(Trace.TRACE_TAG_SYSTEM_SERVER);
 
         // Initialize attribute cache used to cache resources from packages.