Remove HardwareAuthToken parameter from unlockUserKey

Due to the migration to synthetic passwords, the 'token' parameter to
unlockUserKey() is no longer needed.  Remove it.

Note: I didn't change unlockUser() in IActivityManager because it is
marked with UnsupportedAppUsage, so it might not be safe to change the
method signature.  It now just ignores the 'token' parameter rather than
passing it down the stack.

Test: atest com.android.server.locksettings
Bug: 184723544
Change-Id: I35ce09412f47f2f2a17a371d518a0a518b70bfb6
This commit is contained in:
Eric Biggers
2022-01-26 01:59:17 +00:00
parent 2a8ab47782
commit b1bcec9c7d
8 changed files with 45 additions and 47 deletions

View File

@@ -173,7 +173,7 @@ interface IStorageManager {
void setDebugFlags(int flags, int mask) = 60;
void createUserKey(int userId, int serialNumber, boolean ephemeral) = 61;
void destroyUserKey(int userId) = 62;
void unlockUserKey(int userId, int serialNumber, in byte[] token, in byte[] secret) = 63;
void unlockUserKey(int userId, int serialNumber, in byte[] secret) = 63;
void lockUserKey(int userId) = 64;
boolean isUserKeyUnlocked(int userId) = 65;
void prepareUserStorage(in String volumeUuid, int userId, int serialNumber, int flags) = 66;

View File

@@ -1624,9 +1624,9 @@ public class StorageManager {
}
/** {@hide} */
public void unlockUserKey(int userId, int serialNumber, byte[] token, byte[] secret) {
public void unlockUserKey(int userId, int serialNumber, byte[] secret) {
try {
mStorageManager.unlockUserKey(userId, serialNumber, token, secret);
mStorageManager.unlockUserKey(userId, serialNumber, secret);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}

View File

@@ -1127,8 +1127,7 @@ class StorageManagerService extends IStorageManager.Stub
if (initLocked) {
mVold.lockUserKey(user.id);
} else {
mVold.unlockUserKey(user.id, user.serialNumber, encodeBytes(null),
encodeBytes(null));
mVold.unlockUserKey(user.id, user.serialNumber, encodeBytes(null));
}
} catch (Exception e) {
Slog.wtf(TAG, e);
@@ -3522,11 +3521,10 @@ class StorageManagerService extends IStorageManager.Stub
}
@Override
public void unlockUserKey(int userId, int serialNumber, byte[] token, byte[] secret) {
public void unlockUserKey(int userId, int serialNumber, byte[] secret) {
boolean isFsEncrypted = StorageManager.isFileEncryptedNativeOrEmulated();
Slog.d(TAG, "unlockUserKey: " + userId
+ " isFileEncryptedNativeOrEmulated: " + isFsEncrypted
+ " hasToken: " + (token != null)
+ " hasSecret: " + (secret != null));
enforcePermission(android.Manifest.permission.STORAGE_INTERNAL);
@@ -3546,8 +3544,7 @@ class StorageManagerService extends IStorageManager.Stub
return;
}
try {
mVold.unlockUserKey(userId, serialNumber, encodeBytes(token),
encodeBytes(secret));
mVold.unlockUserKey(userId, serialNumber, encodeBytes(secret));
} catch (Exception e) {
Slog.wtf(TAG, e);
return;

View File

@@ -15644,9 +15644,23 @@ public class ActivityManagerService extends IActivityManager.Stub
return mUserController.startUser(userId, /* foreground */ true, unlockListener);
}
/**
* Unlocks the given user.
*
* @param userId The ID of the user to unlock.
* @param token No longer used. (This parameter cannot be removed because
* this method is marked with UnsupportedAppUsage, so its
* signature might not be safe to change.)
* @param secret The secret needed to unlock the user's credential-encrypted
* storage, or null if no secret is needed.
* @param listener An optional progress listener.
*
* @return true if the user was successfully unlocked, otherwise false.
*/
@Override
public boolean unlockUser(int userId, byte[] token, byte[] secret, IProgressListener listener) {
return mUserController.unlockUser(userId, token, secret, listener);
public boolean unlockUser(int userId, @Nullable byte[] token, @Nullable byte[] secret,
@Nullable IProgressListener listener) {
return mUserController.unlockUser(userId, secret, listener);
}
@Override

View File

@@ -741,15 +741,9 @@ class UserController implements Handler.Callback {
if (!Objects.equals(info.lastLoggedInFingerprint, PackagePartitions.FINGERPRINT)
|| SystemProperties.getBoolean("persist.pm.mock-upgrade", false)) {
// Suppress double notifications for managed profiles that
// were unlocked automatically as part of their parent user
// being unlocked.
final boolean quiet;
if (info.isManagedProfile()) {
quiet = !uss.tokenProvided
|| !mLockPatternUtils.isSeparateProfileChallengeEnabled(userId);
} else {
quiet = false;
}
// were unlocked automatically as part of their parent user being
// unlocked. TODO(b/217442918): this code doesn't work correctly.
final boolean quiet = info.isManagedProfile();
mInjector.sendPreBootBroadcast(userId, quiet,
() -> finishUserUnlockedCompleted(uss));
} else {
@@ -1690,27 +1684,25 @@ class UserController implements Handler.Callback {
}
}
boolean unlockUser(final @UserIdInt int userId, byte[] token, byte[] secret,
IProgressListener listener) {
boolean unlockUser(final @UserIdInt int userId, byte[] secret, IProgressListener listener) {
checkCallingPermission(INTERACT_ACROSS_USERS_FULL, "unlockUser");
EventLog.writeEvent(EventLogTags.UC_UNLOCK_USER, userId);
final long binderToken = Binder.clearCallingIdentity();
try {
return unlockUserCleared(userId, token, secret, listener);
return unlockUserCleared(userId, secret, listener);
} finally {
Binder.restoreCallingIdentity(binderToken);
}
}
/**
* Attempt to unlock user without a credential token. This typically
* succeeds when the device doesn't have credential-encrypted storage, or
* when the credential-encrypted storage isn't tied to a user-provided
* PIN or pattern.
* Attempt to unlock user without a secret. This typically succeeds when the
* device doesn't have credential-encrypted storage, or when the
* credential-encrypted storage isn't tied to a user-provided PIN or
* pattern.
*/
private boolean maybeUnlockUser(final @UserIdInt int userId) {
// Try unlocking storage using empty token
return unlockUserCleared(userId, null, null, null);
return unlockUserCleared(userId, null, null);
}
private static void notifyFinished(@UserIdInt int userId, IProgressListener listener) {
@@ -1721,7 +1713,7 @@ class UserController implements Handler.Callback {
}
}
private boolean unlockUserCleared(final @UserIdInt int userId, byte[] token, byte[] secret,
private boolean unlockUserCleared(final @UserIdInt int userId, byte[] secret,
IProgressListener listener) {
UserState uss;
if (!StorageManager.isUserKeyUnlocked(userId)) {
@@ -1729,7 +1721,7 @@ class UserController implements Handler.Callback {
final IStorageManager storageManager = mInjector.getStorageManager();
try {
// We always want to unlock user storage, even user is not started yet
storageManager.unlockUserKey(userId, userInfo.serialNumber, token, secret);
storageManager.unlockUserKey(userId, userInfo.serialNumber, secret);
} catch (RemoteException | RuntimeException e) {
Slogf.w(TAG, "Failed to unlock: " + e.getMessage());
}
@@ -1739,7 +1731,6 @@ class UserController implements Handler.Callback {
uss = mStartedUsers.get(userId);
if (uss != null) {
uss.mUnlockProgress.addListener(listener);
uss.tokenProvided = (token != null);
}
}
// Bail if user isn't actually running

View File

@@ -56,7 +56,6 @@ public final class UserState {
public int state = STATE_BOOTING;
public int lastState = STATE_BOOTING;
public boolean switching;
public boolean tokenProvided;
/** Callback for key eviction. */
public interface KeyEvictedCallback {
@@ -149,7 +148,6 @@ public final class UserState {
@Override
public String toString() {
return "[UserState: id=" + mHandle.getIdentifier() + ", state=" + stateToString(state)
+ ", lastState=" + stateToString(lastState) + ", switching=" + switching
+ ", tokenProvided=" + tokenProvided + "]";
+ ", lastState=" + stateToString(lastState) + ", switching=" + switching + "]";
}
}

View File

@@ -1406,7 +1406,7 @@ public class LockSettingsService extends ILockSettings.Stub {
* can end up calling into other system services to process user unlock request (via
* {@link com.android.server.SystemServiceManager#unlockUser} </em>
*/
private void unlockUser(int userId, byte[] token, byte[] secret) {
private void unlockUser(int userId, byte[] secret) {
Slog.i(TAG, "Unlocking user " + userId + " with secret only, length "
+ (secret != null ? secret.length : 0));
// TODO: make this method fully async so we can update UI with progress strings
@@ -1431,7 +1431,7 @@ public class LockSettingsService extends ILockSettings.Stub {
};
try {
mActivityManager.unlockUser(userId, token, secret, listener);
mActivityManager.unlockUser(userId, null, secret, listener);
} catch (RemoteException e) {
throw e.rethrowAsRuntimeException();
}
@@ -2110,10 +2110,10 @@ public class LockSettingsService extends ILockSettings.Stub {
}
/** Unlock disk encryption */
private void unlockUserKey(int userId, byte[] token, byte[] secret) {
private void unlockUserKey(int userId, byte[] secret) {
final UserInfo userInfo = mUserManager.getUserInfo(userId);
try {
mStorageManager.unlockUserKey(userId, userInfo.serialNumber, token, secret);
mStorageManager.unlockUserKey(userId, userInfo.serialNumber, secret);
} catch (RemoteException e) {
throw new IllegalStateException("Failed to unlock user key " + userId, e);
@@ -2393,7 +2393,7 @@ public class LockSettingsService extends ILockSettings.Stub {
unlockKeystore(credential.getCredential(), userId);
Slog.i(TAG, "Unlocking user " + userId);
unlockUser(userId, null, secretFromCredential(credential));
unlockUser(userId, secretFromCredential(credential));
if (isManagedProfileWithSeparatedLock(userId)) {
setDeviceUnlockedForUser(userId);
@@ -3053,7 +3053,7 @@ public class LockSettingsService extends ILockSettings.Stub {
{
final byte[] secret = authToken.deriveDiskEncryptionKey();
unlockUser(userId, null, secret);
unlockUser(userId, secret);
Arrays.fill(secret, (byte) 0);
}
activateEscrowTokens(authToken, userId);
@@ -3119,7 +3119,7 @@ public class LockSettingsService extends ILockSettings.Stub {
// Clear key from vold so ActivityManager can just unlock the user with empty secret
// during boot. Vold storage needs to be unlocked before manipulation of the keys can
// succeed.
unlockUserKey(userId, null, auth.deriveDiskEncryptionKey());
unlockUserKey(userId, auth.deriveDiskEncryptionKey());
clearUserKeyProtection(userId, auth.deriveDiskEncryptionKey());
fixateNewestUserKeyAuth(userId);
unlockKeystore(auth.deriveKeyStorePassword(), userId);
@@ -3394,7 +3394,7 @@ public class LockSettingsService extends ILockSettings.Stub {
// If clearing credential, unlock the user manually in order to progress user start
// Call unlockUser() on a handler thread so no lock is held (either by LSS or by
// the caller like DPMS), otherwise it can lead to deadlock.
mHandler.post(() -> unlockUser(userId, null, null));
mHandler.post(() -> unlockUser(userId, null));
}
notifyPasswordChanged(userId);
notifySeparateProfileChallengeChanged(userId);

View File

@@ -776,8 +776,7 @@ public class UserControllerTest {
setUpUser(userId, 0);
mUserController.startUser(userId, /* foreground= */ false);
verify(mInjector.mStorageManagerMock, times(1))
.unlockUserKey(userId, /* serialNumber= */ 0, /* token= */ null, /* secret= */
null);
.unlockUserKey(userId, /* serialNumber= */ 0, /* secret= */ null);
mUserStates.put(userId, mUserController.getStartedUserState(userId));
}
@@ -786,8 +785,7 @@ public class UserControllerTest {
assertThat(mUserController.startProfile(userId)).isTrue();
verify(mInjector.mStorageManagerMock, times(1))
.unlockUserKey(userId, /* serialNumber= */ 0, /* token= */ null, /* secret= */
null);
.unlockUserKey(userId, /* serialNumber= */ 0, /* secret= */ null);
mUserStates.put(userId, mUserController.getStartedUserState(userId));
}