From d04aaa323c3a788d26f18fc66e0a59b47e525b38 Mon Sep 17 00:00:00 2001 From: Amith Yamasani Date: Mon, 13 Jun 2016 12:09:36 -0700 Subject: [PATCH] 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 --- core/java/android/content/Context.java | 6 ++ core/java/android/os/Environment.java | 16 ++++ core/java/android/os/UserManager.java | 4 +- .../java/android/security/GateKeeper.java | 3 +- .../android/server/LockSettingsService.java | 2 +- .../accounts/AccountManagerService.java | 2 +- .../server/pm/PackageManagerService.java | 12 +++ .../android/server/pm/UserManagerService.java | 90 +++++++++++++------ .../android/server/devicepolicy/Owners.java | 2 +- .../java/com/android/server/SystemServer.java | 2 +- 10 files changed, 108 insertions(+), 31 deletions(-) diff --git a/core/java/android/content/Context.java b/core/java/android/content/Context.java index 108350a2b9ce1..0881c9cb2208a 100644 --- a/core/java/android/content/Context.java +++ b/core/java/android/content/Context.java @@ -3645,6 +3645,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 f6e8940db4087..d8be2b6dcc4c4 100644 --- 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 7146448ad5d7c..bcc8d463e51db 100644 --- 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)}. + *

* 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 c1df28c387e51..7a2cbd06eb924 100644 --- 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 eeb20bf03d123..1e715f93d0411 100644 --- 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 408454286aeed..0cf517274f30e 100644 --- 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 users = getUserManager().getUsers(); + final List 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 f2fde0f91e402..a560c83ef92ed 100644 --- 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 a8cf110655f80..a85064b70f22a 100644 --- 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; * */ 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 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 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 1ae1a773b23ea..b53933e07f2bd 100644 --- 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 users = mUserManager.getUsers(); + final List 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 476a559eaea3f..f59b2ff16482f 100644 --- 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.