Password security for FBE disk encryption keys
Add the means to protect FBE keys with a combination of an auth token
from Gatekeeper, and a hash of the password. Both of these must be
passed to unlock_user_key. Keys are created unprotected, and
change_user_key changes the way they are protected.
Bug: 22950892
Change-Id: Ie13bc6f82059ce941b0e664a5b60355e52b45f30
diff --git a/cmds/am/src/com/android/commands/am/Am.java b/cmds/am/src/com/android/commands/am/Am.java
index acc68cf..6206323 100644
--- a/cmds/am/src/com/android/commands/am/Am.java
+++ b/cmds/am/src/com/android/commands/am/Am.java
@@ -1126,14 +1126,19 @@
}
}
+ private byte[] argToBytes(String arg) {
+ if (arg.equals("!")) {
+ return null;
+ } else {
+ return HexDump.hexStringToByteArray(arg);
+ }
+ }
+
private void runUnlockUser() throws Exception {
int userId = Integer.parseInt(nextArgRequired());
- String tokenHex = nextArg();
- byte[] token = null;
- if (tokenHex != null) {
- token = HexDump.hexStringToByteArray(tokenHex);
- }
- boolean success = mAm.unlockUser(userId, token);
+ byte[] token = argToBytes(nextArgRequired());
+ byte[] secret = argToBytes(nextArgRequired());
+ boolean success = mAm.unlockUser(userId, token, secret);
if (success) {
System.out.println("Success: user unlocked");
} else {
diff --git a/core/java/android/app/ActivityManagerNative.java b/core/java/android/app/ActivityManagerNative.java
index a3160f4..1954774 100644
--- a/core/java/android/app/ActivityManagerNative.java
+++ b/core/java/android/app/ActivityManagerNative.java
@@ -2080,7 +2080,8 @@
data.enforceInterface(IActivityManager.descriptor);
int userId = data.readInt();
byte[] token = data.createByteArray();
- boolean result = unlockUser(userId, token);
+ byte[] secret = data.createByteArray();
+ boolean result = unlockUser(userId, token, secret);
reply.writeNoException();
reply.writeInt(result ? 1 : 0);
return true;
@@ -5571,12 +5572,13 @@
return result;
}
- public boolean unlockUser(int userId, byte[] token) throws RemoteException {
+ public boolean unlockUser(int userId, byte[] token, byte[] secret) throws RemoteException {
Parcel data = Parcel.obtain();
Parcel reply = Parcel.obtain();
data.writeInterfaceToken(IActivityManager.descriptor);
data.writeInt(userId);
data.writeByteArray(token);
+ data.writeByteArray(secret);
mRemote.transact(IActivityManager.UNLOCK_USER_TRANSACTION, data, reply, 0);
reply.readException();
boolean result = reply.readInt() != 0;
diff --git a/core/java/android/app/IActivityManager.java b/core/java/android/app/IActivityManager.java
index f5e7d78..b5ca6ee 100644
--- a/core/java/android/app/IActivityManager.java
+++ b/core/java/android/app/IActivityManager.java
@@ -426,7 +426,7 @@
// Multi-user APIs
public boolean switchUser(int userid) throws RemoteException;
public boolean startUserInBackground(int userid) throws RemoteException;
- public boolean unlockUser(int userid, byte[] token) throws RemoteException;
+ public boolean unlockUser(int userid, byte[] token, byte[] secret) throws RemoteException;
public int stopUser(int userid, boolean force, IStopUserCallback callback) throws RemoteException;
public UserInfo getCurrentUser() throws RemoteException;
public boolean isUserRunning(int userid, int flags) throws RemoteException;
diff --git a/core/java/android/os/storage/IMountService.java b/core/java/android/os/storage/IMountService.java
index dd8eb5f..fc440d2 100644
--- a/core/java/android/os/storage/IMountService.java
+++ b/core/java/android/os/storage/IMountService.java
@@ -1233,7 +1233,8 @@
}
@Override
- public void unlockUserKey(int userId, int serialNumber, byte[] token) throws RemoteException {
+ public void changeUserKey(int userId, int serialNumber,
+ byte[] token, byte[] oldSecret, byte[] newSecret) throws RemoteException {
Parcel _data = Parcel.obtain();
Parcel _reply = Parcel.obtain();
try {
@@ -1241,6 +1242,27 @@
_data.writeInt(userId);
_data.writeInt(serialNumber);
_data.writeByteArray(token);
+ _data.writeByteArray(oldSecret);
+ _data.writeByteArray(newSecret);
+ mRemote.transact(Stub.TRANSACTION_changeUserKey, _data, _reply, 0);
+ _reply.readException();
+ } finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+
+ @Override
+ public void unlockUserKey(int userId, int serialNumber,
+ byte[] token, byte[] secret) throws RemoteException {
+ Parcel _data = Parcel.obtain();
+ Parcel _reply = Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ _data.writeInt(userId);
+ _data.writeInt(serialNumber);
+ _data.writeByteArray(token);
+ _data.writeByteArray(secret);
mRemote.transact(Stub.TRANSACTION_unlockUserKey, _data, _reply, 0);
_reply.readException();
} finally {
@@ -1448,6 +1470,8 @@
static final int TRANSACTION_mountAppFuse = IBinder.FIRST_CALL_TRANSACTION + 69;
+ static final int TRANSACTION_changeUserKey = IBinder.FIRST_CALL_TRANSACTION + 70;
+
/**
* Cast an IBinder object into an IMountService interface, generating a
* proxy if needed.
@@ -2026,12 +2050,24 @@
reply.writeNoException();
return true;
}
+ case TRANSACTION_changeUserKey: {
+ data.enforceInterface(DESCRIPTOR);
+ int userId = data.readInt();
+ int serialNumber = data.readInt();
+ byte[] token = data.createByteArray();
+ byte[] oldSecret = data.createByteArray();
+ byte[] newSecret = data.createByteArray();
+ changeUserKey(userId, serialNumber, token, oldSecret, newSecret);
+ reply.writeNoException();
+ return true;
+ }
case TRANSACTION_unlockUserKey: {
data.enforceInterface(DESCRIPTOR);
int userId = data.readInt();
int serialNumber = data.readInt();
byte[] token = data.createByteArray();
- unlockUserKey(userId, serialNumber, token);
+ byte[] secret = data.createByteArray();
+ unlockUserKey(userId, serialNumber, token, secret);
reply.writeNoException();
return true;
}
@@ -2383,8 +2419,11 @@
public void createUserKey(int userId, int serialNumber, boolean ephemeral)
throws RemoteException;
public void destroyUserKey(int userId) throws RemoteException;
+ public void changeUserKey(int userId, int serialNumber,
+ byte[] token, byte[] oldSecret, byte[] newSecret) throws RemoteException;
- public void unlockUserKey(int userId, int serialNumber, byte[] token) throws RemoteException;
+ public void unlockUserKey(int userId, int serialNumber,
+ byte[] token, byte[] secret) throws RemoteException;
public void lockUserKey(int userId) throws RemoteException;
public boolean isUserKeyUnlocked(int userId) throws RemoteException;
diff --git a/core/java/android/os/storage/StorageManager.java b/core/java/android/os/storage/StorageManager.java
index b82638a..e7dfbd7 100644
--- a/core/java/android/os/storage/StorageManager.java
+++ b/core/java/android/os/storage/StorageManager.java
@@ -991,9 +991,9 @@
}
/** {@hide} */
- public void unlockUserKey(int userId, int serialNumber, byte[] token) {
+ public void unlockUserKey(int userId, int serialNumber, byte[] token, byte[] secret) {
try {
- mMountService.unlockUserKey(userId, serialNumber, token);
+ mMountService.unlockUserKey(userId, serialNumber, token, secret);
} catch (RemoteException e) {
throw e.rethrowAsRuntimeException();
}
diff --git a/services/core/java/com/android/server/LockSettingsService.java b/services/core/java/com/android/server/LockSettingsService.java
index ecba0a4..4dbb490 100644
--- a/services/core/java/com/android/server/LockSettingsService.java
+++ b/services/core/java/com/android/server/LockSettingsService.java
@@ -62,6 +62,10 @@
import com.android.internal.widget.VerifyCredentialResponse;
import com.android.server.LockSettingsStorage.CredentialHash;
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+
import java.util.Arrays;
import java.util.List;
@@ -510,9 +514,9 @@
}
}
- private void unlockUser(int userId, byte[] token) {
+ private void unlockUser(int userId, byte[] token, byte[] secret) {
try {
- ActivityManagerNative.getDefault().unlockUser(userId, token);
+ ActivityManagerNative.getDefault().unlockUser(userId, token, secret);
} catch (RemoteException e) {
throw e.rethrowAsRuntimeException();
}
@@ -560,6 +564,7 @@
getGateKeeperService().clearSecureUserId(userId);
mStorage.writePatternHash(null, userId);
setKeystorePassword(null, userId);
+ clearUserKeyProtection(userId);
return;
}
@@ -573,6 +578,7 @@
byte[] enrolledHandle = enrollCredential(currentHandle, savedCredential, pattern, userId);
if (enrolledHandle != null) {
mStorage.writePatternHash(enrolledHandle, userId);
+ setUserKeyProtection(userId, pattern, verifyPattern(pattern, 0, userId));
} else {
throw new RemoteException("Failed to enroll pattern");
}
@@ -588,6 +594,7 @@
getGateKeeperService().clearSecureUserId(userId);
mStorage.writePasswordHash(null, userId);
setKeystorePassword(null, userId);
+ clearUserKeyProtection(userId);
return;
}
@@ -601,6 +608,7 @@
byte[] enrolledHandle = enrollCredential(currentHandle, savedCredential, password, userId);
if (enrolledHandle != null) {
mStorage.writePasswordHash(enrolledHandle, userId);
+ setUserKeyProtection(userId, password, verifyPassword(password, 0, userId));
} else {
throw new RemoteException("Failed to enroll password");
}
@@ -633,6 +641,48 @@
return hash;
}
+ private void setUserKeyProtection(int userId, String credential, VerifyCredentialResponse vcr)
+ throws RemoteException {
+ if (vcr == null) {
+ throw new RemoteException("Null response verifying a credential we just set");
+ }
+ if (vcr.getResponseCode() != VerifyCredentialResponse.RESPONSE_OK) {
+ throw new RemoteException("Non-OK response verifying a credential we just set: "
+ + vcr.getResponseCode());
+ }
+ byte[] token = vcr.getPayload();
+ if (token == null) {
+ throw new RemoteException("Empty payload verifying a credential we just set");
+ }
+ changeUserKey(userId, token, secretFromCredential(credential));
+ }
+
+ private void clearUserKeyProtection(int userId) throws RemoteException {
+ changeUserKey(userId, null, null);
+ }
+
+ private static byte[] secretFromCredential(String credential) throws RemoteException {
+ try {
+ MessageDigest digest = MessageDigest.getInstance("SHA-512");
+ // Personalize the hash
+ byte[] personalization = "Android FBE credential hash"
+ .getBytes(StandardCharsets.UTF_8);
+ // Pad it to the block size of the hash function
+ personalization = Arrays.copyOf(personalization, 128);
+ digest.update(personalization);
+ digest.update(credential.getBytes(StandardCharsets.UTF_8));
+ return digest.digest();
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException("NoSuchAlgorithmException for SHA-512");
+ }
+ }
+
+ private void changeUserKey(int userId, byte[] token, byte[] secret)
+ throws RemoteException {
+ final UserInfo userInfo = UserManager.get(mContext).getUserInfo(userId);
+ getMountService().changeUserKey(userId, userInfo.serialNumber, token, null, secret);
+ }
+
@Override
public VerifyCredentialResponse checkPattern(String pattern, int userId) throws RemoteException {
return doVerifyPattern(pattern, false, 0, userId);
@@ -742,11 +792,11 @@
if (Arrays.equals(hash, storedHash.hash)) {
unlockKeystore(credentialUtil.adjustForKeystore(credential), userId);
- // TODO: pass through a meaningful token from gatekeeper to
- // unlock credential keys; for now pass through a stub value to
- // indicate that we came from a user challenge.
- final byte[] token = String.valueOf(userId).getBytes();
- unlockUser(userId, token);
+ // Users with legacy credentials don't have credential-backed
+ // FBE keys, so just pass through a fake token/secret
+ Slog.i(TAG, "Unlocking user with fake token: " + userId);
+ final byte[] fakeToken = String.valueOf(userId).getBytes();
+ unlockUser(userId, fakeToken, fakeToken);
// migrate credential to GateKeeper
credentialUtil.setCredential(credential, null, userId);
@@ -786,11 +836,9 @@
// credential has matched
unlockKeystore(credential, userId);
- // TODO: pass through a meaningful token from gatekeeper to
- // unlock credential keys; for now pass through a stub value to
- // indicate that we came from a user challenge.
- final byte[] token = String.valueOf(userId).getBytes();
- unlockUser(userId, token);
+ Slog.i(TAG, "Unlocking user " + userId +
+ " with token length " + response.getPayload().length);
+ unlockUser(userId, response.getPayload(), secretFromCredential(credential));
UserInfo info = UserManager.get(mContext).getUserInfo(userId);
if (mLockPatternUtils.isSeparateProfileChallengeEnabled(userId)) {
diff --git a/services/core/java/com/android/server/MountService.java b/services/core/java/com/android/server/MountService.java
index 5120e1b..cbd477a 100644
--- a/services/core/java/com/android/server/MountService.java
+++ b/services/core/java/com/android/server/MountService.java
@@ -2742,8 +2742,30 @@
}
}
+ private SensitiveArg encodeBytes(byte[] bytes) {
+ if (ArrayUtils.isEmpty(bytes)) {
+ return new SensitiveArg("!");
+ } else {
+ return new SensitiveArg(HexDump.toHexString(bytes));
+ }
+ }
+
@Override
- public void unlockUserKey(int userId, int serialNumber, byte[] token) {
+ public void changeUserKey(int userId, int serialNumber,
+ byte[] token, byte[] oldSecret, byte[] newSecret) {
+ enforcePermission(android.Manifest.permission.STORAGE_INTERNAL);
+ waitForReady();
+
+ try {
+ mCryptConnector.execute("cryptfs", "change_user_key", userId, serialNumber,
+ encodeBytes(token), encodeBytes(oldSecret), encodeBytes(newSecret));
+ } catch (NativeDaemonConnectorException e) {
+ throw e.rethrowAsParcelableException();
+ }
+ }
+
+ @Override
+ public void unlockUserKey(int userId, int serialNumber, byte[] token, byte[] secret) {
enforcePermission(android.Manifest.permission.STORAGE_INTERNAL);
waitForReady();
@@ -2753,16 +2775,9 @@
throw new IllegalStateException("Token required to unlock secure user " + userId);
}
- final String encodedToken;
- if (ArrayUtils.isEmpty(token)) {
- encodedToken = "!";
- } else {
- encodedToken = HexDump.toHexString(token);
- }
-
try {
mCryptConnector.execute("cryptfs", "unlock_user_key", userId, serialNumber,
- new SensitiveArg(encodedToken));
+ encodeBytes(token), encodeBytes(secret));
} catch (NativeDaemonConnectorException e) {
throw e.rethrowAsParcelableException();
}
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 5125133..9dae740 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -20340,8 +20340,8 @@
}
@Override
- public boolean unlockUser(int userId, byte[] token) {
- return mUserController.unlockUser(userId, token);
+ public boolean unlockUser(int userId, byte[] token, byte[] secret) {
+ return mUserController.unlockUser(userId, token, secret);
}
@Override
diff --git a/services/core/java/com/android/server/am/UserController.java b/services/core/java/com/android/server/am/UserController.java
index 2f63b2d3..a355fa4 100644
--- a/services/core/java/com/android/server/am/UserController.java
+++ b/services/core/java/com/android/server/am/UserController.java
@@ -783,7 +783,7 @@
return result;
}
- boolean unlockUser(final int userId, byte[] token) {
+ boolean unlockUser(final int userId, byte[] token, byte[] secret) {
if (mService.checkCallingPermission(INTERACT_ACROSS_USERS_FULL)
!= PackageManager.PERMISSION_GRANTED) {
String msg = "Permission Denial: unlockUser() from pid="
@@ -796,7 +796,7 @@
final long binderToken = Binder.clearCallingIdentity();
try {
- return unlockUserCleared(userId, token);
+ return unlockUserCleared(userId, token, secret);
} finally {
Binder.restoreCallingIdentity(binderToken);
}
@@ -810,10 +810,10 @@
*/
boolean maybeUnlockUser(final int userId) {
// Try unlocking storage using empty token
- return unlockUserCleared(userId, null);
+ return unlockUserCleared(userId, null, null);
}
- boolean unlockUserCleared(final int userId, byte[] token) {
+ boolean unlockUserCleared(final int userId, byte[] token, byte[] secret) {
synchronized (mService) {
// Bail if already running unlocked
final UserState uss = mStartedUsers.get(userId);
@@ -824,7 +824,7 @@
final UserInfo userInfo = getUserInfo(userId);
final IMountService mountService = getMountService();
try {
- mountService.unlockUserKey(userId, userInfo.serialNumber, token);
+ mountService.unlockUserKey(userId, userInfo.serialNumber, token, secret);
} catch (RemoteException | RuntimeException e) {
Slog.w(TAG, "Failed to unlock: " + e.getMessage());
return false;