Stop LockSettingsService from calling DevicePolicyManager directly
This is a violation of layering (LSS is considered a lower level
component than DPM) and source of deadlock due to lock inversion.
This change tries to remove most of the direct calls into DPM from
LSS. After this, there will only be a handful non-critical invocations
remaining:
1. DPM.reportPasswordChanged()
This is always called on a handler thread so it's OK (LSS does not
hold any hold while calling out). Consider this as a (asynchronous)
broadcast.
2. DPMi.reportSeparateProfileChallengeChanged()
This is now moved to the handler thread, similar to
DPM.reportPasswordChanged().
3. DPMi.canUserHaveUntrustedCredentialReset()
This is still a violation but it will soon be removed as we
remove the caching of syhnthetic password alltogether (deprecating
old resetPassword()). So I'll leave it for now.
Test: atest com.android.server.locksettings
Test: atest com.android.server.devicepolicy.DevicePolicyManagerTest
Test: atest MixedManagedProfileOwnerTest#testResetPasswordWithToken
Test: atest MixedDeviceOwnerTest#testResetPasswordWithToken
Bug: 37090873
Bug: 141537958
Change-Id: Ie44cb418ab255bd016c5dd448674beabd362b74c
diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java
index b7eca29..378d9eb 100644
--- a/services/core/java/com/android/server/locksettings/LockSettingsService.java
+++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java
@@ -44,6 +44,7 @@
import android.app.PendingIntent;
import android.app.admin.DevicePolicyManager;
import android.app.admin.DevicePolicyManagerInternal;
+import android.app.admin.DeviceStateCache;
import android.app.admin.PasswordMetrics;
import android.app.backup.BackupManager;
import android.app.trust.IStrongAuthTracker;
@@ -76,6 +77,7 @@
import android.os.SystemProperties;
import android.os.UserHandle;
import android.os.UserManager;
+import android.os.UserManagerInternal;
import android.os.storage.IStorageManager;
import android.os.storage.StorageManager;
import android.provider.Settings;
@@ -429,6 +431,10 @@
return (UserManager) mContext.getSystemService(Context.USER_SERVICE);
}
+ public UserManagerInternal getUserManagerInternal() {
+ return LocalServices.getService(UserManagerInternal.class);
+ }
+
/**
* Return the {@link DevicePolicyManager} object.
*
@@ -440,6 +446,10 @@
return (DevicePolicyManager) mContext.getSystemService(Context.DEVICE_POLICY_SERVICE);
}
+ public DeviceStateCache getDeviceStateCache() {
+ return DeviceStateCache.getInstance();
+ }
+
public KeyStore getKeyStore() {
return KeyStore.getInstance();
}
@@ -1023,11 +1033,16 @@
}
private void notifySeparateProfileChallengeChanged(int userId) {
- final DevicePolicyManagerInternal dpmi = LocalServices.getService(
- DevicePolicyManagerInternal.class);
- if (dpmi != null) {
- dpmi.reportSeparateProfileChallengeChanged(userId);
- }
+ // LSS cannot call into DPM directly, otherwise it will cause deadlock.
+ // In this case, calling DPM on a handler thread is OK since DPM doesn't
+ // expect reportSeparateProfileChallengeChanged() to happen synchronously.
+ mHandler.post(() -> {
+ final DevicePolicyManagerInternal dpmi = LocalServices.getService(
+ DevicePolicyManagerInternal.class);
+ if (dpmi != null) {
+ dpmi.reportSeparateProfileChallengeChanged(userId);
+ }
+ });
}
@Override
@@ -2038,7 +2053,6 @@
* reporting the password changed.
*/
private void notifyPasswordChanged(@UserIdInt int userId) {
- // Same handler as notifyActivePasswordMetricsAvailable to ensure correct ordering
mHandler.post(() -> {
mInjector.getDevicePolicyManager().reportPasswordChanged(userId);
LocalServices.getService(WindowManagerInternal.class).reportPasswordChanged(userId);
@@ -3026,45 +3040,43 @@
pw.decreaseIndent();
}
+ /**
+ * Cryptographically disable escrow token support for the current user, if the user is not
+ * managed (either user has a profile owner, or if device is managed). Do not disable
+ * if we are running an automotive build.
+ */
private void disableEscrowTokenOnNonManagedDevicesIfNeeded(int userId) {
- long ident = Binder.clearCallingIdentity();
- try {
- // Managed profile should have escrow enabled
- if (mUserManager.getUserInfo(userId).isManagedProfile()) {
- Slog.i(TAG, "Managed profile can have escrow token");
- return;
- }
- DevicePolicyManager dpm = mInjector.getDevicePolicyManager();
- // Devices with Device Owner should have escrow enabled on all users.
- if (dpm.getDeviceOwnerComponentOnAnyUser() != null) {
- Slog.i(TAG, "Corp-owned device can have escrow token");
- return;
- }
- // We could also have a profile owner on the given (non-managed) user for unicorn cases
- if (dpm.getProfileOwnerAsUser(userId) != null) {
- Slog.i(TAG, "User with profile owner can have escrow token");
- return;
- }
- // If the device is yet to be provisioned (still in SUW), there is still
- // a chance that Device Owner will be set on the device later, so postpone
- // disabling escrow token for now.
- if (!dpm.isDeviceProvisioned()) {
- Slog.i(TAG, "Postpone disabling escrow tokens until device is provisioned");
- return;
- }
+ final UserManagerInternal userManagerInternal = mInjector.getUserManagerInternal();
- // Escrow tokens are enabled on automotive builds.
- if (mContext.getPackageManager().hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE)) {
- return;
- }
+ // Managed profile should have escrow enabled
+ if (userManagerInternal.isUserManaged(userId)) {
+ Slog.i(TAG, "Managed profile can have escrow token");
+ return;
+ }
- // Disable escrow token permanently on all other device/user types.
- Slog.i(TAG, "Disabling escrow token on user " + userId);
- if (isSyntheticPasswordBasedCredentialLocked(userId)) {
- mSpManager.destroyEscrowData(userId);
- }
- } finally {
- Binder.restoreCallingIdentity(ident);
+ // Devices with Device Owner should have escrow enabled on all users.
+ if (userManagerInternal.isDeviceManaged()) {
+ Slog.i(TAG, "Corp-owned device can have escrow token");
+ return;
+ }
+
+ // If the device is yet to be provisioned (still in SUW), there is still
+ // a chance that Device Owner will be set on the device later, so postpone
+ // disabling escrow token for now.
+ if (!mInjector.getDeviceStateCache().isDeviceProvisioned()) {
+ Slog.i(TAG, "Postpone disabling escrow tokens until device is provisioned");
+ return;
+ }
+
+ // Escrow tokens are enabled on automotive builds.
+ if (mContext.getPackageManager().hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE)) {
+ return;
+ }
+
+ // Disable escrow token permanently on all other device/user types.
+ Slog.i(TAG, "Disabling escrow token on user " + userId);
+ if (isSyntheticPasswordBasedCredentialLocked(userId)) {
+ mSpManager.destroyEscrowData(userId);
}
}
diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java
index dd1adb7..35e6a12 100644
--- a/services/core/java/com/android/server/pm/UserManagerService.java
+++ b/services/core/java/com/android/server/pm/UserManagerService.java
@@ -3939,6 +3939,13 @@
}
@Override
+ public boolean isDeviceManaged() {
+ synchronized (mUsersLock) {
+ return mIsDeviceManaged;
+ }
+ }
+
+ @Override
public void setUserManaged(@UserIdInt int userId, boolean isManaged) {
synchronized (mUsersLock) {
mIsUserManaged.put(userId, isManaged);
@@ -3946,6 +3953,13 @@
}
@Override
+ public boolean isUserManaged(@UserIdInt int userId) {
+ synchronized (mUsersLock) {
+ return mIsUserManaged.get(userId);
+ }
+ }
+
+ @Override
public void setUserIcon(@UserIdInt int userId, Bitmap bitmap) {
long ident = Binder.clearCallingIdentity();
try {
@@ -4165,6 +4179,7 @@
return restrictions != null && restrictions.getBoolean(restrictionKey);
}
+ @Override
public @Nullable UserInfo getUserInfo(@UserIdInt int userId) {
UserData userData;
synchronized (mUsersLock) {