diff options
6 files changed, 104 insertions, 161 deletions
diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index ee8a23c08b4a..e9ef8a7bfde1 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -1588,6 +1588,11 @@ public class LockSettingsService extends ILockSettings.Stub { return; } + // Don't send empty credentials on unlock. + if (credential.isNone()) { + return; + } + // A profile with a unified lock screen stores a randomly generated credential, so skip it. // Its parent will send credentials for the profile, as it stores the unified lock // credential. @@ -1595,12 +1600,10 @@ public class LockSettingsService extends ILockSettings.Stub { return; } - // RecoverableKeyStoreManager expects null for empty credential. - final byte[] secret = credential.isNone() ? null : credential.getCredential(); // Send credentials for the user and any child profiles that share its lock screen. for (int profileId : getProfilesWithSameLockScreen(userId)) { mRecoverableKeyStoreManager.lockScreenSecretAvailable( - credential.getType(), secret, profileId); + credential.getType(), credential.getCredential(), profileId); } } diff --git a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java index 8161503652ff..ec66599ebe82 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java @@ -44,7 +44,6 @@ import com.android.internal.util.ArrayUtils; import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.Preconditions; import com.android.internal.widget.LockPatternUtils; -import com.android.internal.widget.LockPatternUtils.CredentialType; import com.android.server.LocalServices; import com.android.server.PersistentDataBlockManagerInternal; import com.android.server.utils.WatchableImpl; @@ -71,7 +70,6 @@ class LockSettingsStorage extends WatchableImpl { private static final String TAG = "LockSettingsStorage"; private static final String TABLE = "locksettings"; - private static final boolean DEBUG = false; private static final String COLUMN_KEY = "name"; private static final String COLUMN_USERID = "user"; @@ -84,11 +82,10 @@ class LockSettingsStorage extends WatchableImpl { COLUMN_KEY, COLUMN_VALUE }; - private static final String SYSTEM_DIRECTORY = "/system/"; private static final String CHILD_PROFILE_LOCK_FILE = "gatekeeper.profile.key"; private static final String REBOOT_ESCROW_FILE = "reboot.escrow.key"; - private static final String REBOOT_ESCROW_SERVER_BLOB = "reboot.escrow.server.blob.key"; + private static final String REBOOT_ESCROW_SERVER_BLOB_FILE = "reboot.escrow.server.blob.key"; private static final String SYNTHETIC_PASSWORD_DIRECTORY = "spblob/"; @@ -108,39 +105,6 @@ class LockSettingsStorage extends WatchableImpl { private PersistentDataBlockManagerInternal mPersistentDataBlockManagerInternal; - @VisibleForTesting - public static class CredentialHash { - - private CredentialHash(byte[] hash, @CredentialType int type) { - if (type != LockPatternUtils.CREDENTIAL_TYPE_NONE) { - if (hash == null) { - throw new IllegalArgumentException("Empty hash for CredentialHash"); - } - } else /* type == LockPatternUtils.CREDENTIAL_TYPE_NONE */ { - if (hash != null) { - throw new IllegalArgumentException( - "None type CredentialHash should not have hash"); - } - } - this.hash = hash; - this.type = type; - } - - static CredentialHash create(byte[] hash, int type) { - if (type == LockPatternUtils.CREDENTIAL_TYPE_NONE) { - throw new IllegalArgumentException("Bad type for CredentialHash"); - } - return new CredentialHash(hash, type); - } - - static CredentialHash createEmptyHash() { - return new CredentialHash(null, LockPatternUtils.CREDENTIAL_TYPE_NONE); - } - - byte[] hash; - @CredentialType int type; - } - public LockSettingsStorage(Context context) { mContext = context; mOpenHelper = new DatabaseHelper(context); @@ -259,13 +223,7 @@ class LockSettingsStorage extends WatchableImpl { } public void removeChildProfileLock(int userId) { - if (DEBUG) - Slog.e(TAG, "Remove child profile lock for user: " + userId); - try { - deleteFile(getChildProfileLockFile(userId)); - } catch (Exception e) { - e.printStackTrace(); - } + deleteFile(getChildProfileLockFile(userId)); } public void writeChildProfileLock(int userId, byte[] lock) { @@ -297,39 +255,39 @@ class LockSettingsStorage extends WatchableImpl { } public void writeRebootEscrowServerBlob(byte[] serverBlob) { - writeFile(getRebootEscrowServerBlob(), serverBlob); + writeFile(getRebootEscrowServerBlobFile(), serverBlob); } public byte[] readRebootEscrowServerBlob() { - return readFile(getRebootEscrowServerBlob()); + return readFile(getRebootEscrowServerBlobFile()); } public boolean hasRebootEscrowServerBlob() { - return hasFile(getRebootEscrowServerBlob()); + return hasFile(getRebootEscrowServerBlobFile()); } public void removeRebootEscrowServerBlob() { - deleteFile(getRebootEscrowServerBlob()); + deleteFile(getRebootEscrowServerBlobFile()); } - private boolean hasFile(String name) { - byte[] contents = readFile(name); + private boolean hasFile(File path) { + byte[] contents = readFile(path); return contents != null && contents.length > 0; } - private byte[] readFile(String name) { + private byte[] readFile(File path) { int version; synchronized (mCache) { - if (mCache.hasFile(name)) { - return mCache.peekFile(name); + if (mCache.hasFile(path)) { + return mCache.peekFile(path); } version = mCache.getVersion(); } - byte[] stored = null; - try (RandomAccessFile raf = new RandomAccessFile(name, "r")) { - stored = new byte[(int) raf.length()]; - raf.readFully(stored, 0, stored.length); + byte[] data = null; + try (RandomAccessFile raf = new RandomAccessFile(path, "r")) { + data = new byte[(int) raf.length()]; + raf.readFully(data, 0, data.length); raf.close(); } catch (FileNotFoundException suppressed) { // readFile() is also called by hasFile() to check the existence of files, in this @@ -337,8 +295,8 @@ class LockSettingsStorage extends WatchableImpl { } catch (IOException e) { Slog.e(TAG, "Cannot read file " + e); } - mCache.putFileIfUnchanged(name, stored, version); - return stored; + mCache.putFileIfUnchanged(path, data, version); + return data; } private void fsyncDirectory(File directory) { @@ -352,23 +310,21 @@ class LockSettingsStorage extends WatchableImpl { } } - private void writeFile(String name, byte[] hash) { + private void writeFile(File path, byte[] data) { synchronized (mFileWriteLock) { RandomAccessFile raf = null; try { - // Write the hash to file, requiring each write to be synchronized to the + // Write the data to the file, requiring each write to be synchronized to the // underlying storage device immediately to avoid data loss in case of power loss. - // This also ensures future secdiscard operation on the file succeeds since the - // file would have been allocated on flash. - raf = new RandomAccessFile(name, "rws"); - // Truncate the file if pattern is null, to clear the lock - if (hash == null || hash.length == 0) { + raf = new RandomAccessFile(path, "rws"); + // Truncate the file if the data is empty. + if (data == null || data.length == 0) { raf.setLength(0); } else { - raf.write(hash, 0, hash.length); + raf.write(data, 0, data.length); } raf.close(); - fsyncDirectory((new File(name)).getAbsoluteFile().getParentFile()); + fsyncDirectory(path.getParentFile()); } catch (IOException e) { Slog.e(TAG, "Error writing to file " + e); } finally { @@ -380,74 +336,66 @@ class LockSettingsStorage extends WatchableImpl { } } } - mCache.putFile(name, hash); + mCache.putFile(path, data); dispatchChange(this); } } - private void deleteFile(String name) { - if (DEBUG) Slog.e(TAG, "Delete file " + name); + private void deleteFile(File path) { synchronized (mFileWriteLock) { - File file = new File(name); - if (file.exists()) { - file.delete(); - mCache.putFile(name, null); + if (path.exists()) { + // Zeroize the file to try to make its contents unrecoverable. This is *not* + // guaranteed to be effective, and in fact it usually isn't, but it doesn't hurt. + try (RandomAccessFile raf = new RandomAccessFile(path, "rws")) { + final int fileSize = (int) raf.length(); + raf.write(new byte[fileSize]); + } catch (Exception e) { + Slog.w(TAG, "Failed to zeroize " + path, e); + } + path.delete(); + dispatchChange(this); + mCache.putFile(path, null); } - dispatchChange(this); } } @VisibleForTesting - String getChildProfileLockFile(int userId) { - return getLockCredentialFilePathForUser(userId, CHILD_PROFILE_LOCK_FILE); + File getChildProfileLockFile(int userId) { + return getLockCredentialFileForUser(userId, CHILD_PROFILE_LOCK_FILE); } @VisibleForTesting - String getRebootEscrowFile(int userId) { - return getLockCredentialFilePathForUser(userId, REBOOT_ESCROW_FILE); + File getRebootEscrowFile(int userId) { + return getLockCredentialFileForUser(userId, REBOOT_ESCROW_FILE); } @VisibleForTesting - String getRebootEscrowServerBlob() { + File getRebootEscrowServerBlobFile() { // There is a single copy of server blob for all users. - return getLockCredentialFilePathForUser(UserHandle.USER_SYSTEM, REBOOT_ESCROW_SERVER_BLOB); + return getLockCredentialFileForUser(UserHandle.USER_SYSTEM, REBOOT_ESCROW_SERVER_BLOB_FILE); } - private String getLockCredentialFilePathForUser(int userId, String basename) { - String dataSystemDirectory = Environment.getDataDirectory().getAbsolutePath() + - SYSTEM_DIRECTORY; + private File getLockCredentialFileForUser(int userId, String fileName) { if (userId == 0) { - // Leave it in the same place for user 0 - return dataSystemDirectory + basename; + // The files for user 0 are stored directly in /data/system, since this is where they + // originally were, and they haven't been moved yet. + return new File(Environment.getDataSystemDirectory(), fileName); } else { - return new File(Environment.getUserSystemDirectory(userId), basename).getAbsolutePath(); + return new File(Environment.getUserSystemDirectory(userId), fileName); } } public void writeSyntheticPasswordState(int userId, long handle, String name, byte[] data) { ensureSyntheticPasswordDirectoryForUser(userId); - writeFile(getSynthenticPasswordStateFilePathForUser(userId, handle, name), data); + writeFile(getSyntheticPasswordStateFileForUser(userId, handle, name), data); } public byte[] readSyntheticPasswordState(int userId, long handle, String name) { - return readFile(getSynthenticPasswordStateFilePathForUser(userId, handle, name)); + return readFile(getSyntheticPasswordStateFileForUser(userId, handle, name)); } public void deleteSyntheticPasswordState(int userId, long handle, String name) { - String path = getSynthenticPasswordStateFilePathForUser(userId, handle, name); - File file = new File(path); - if (file.exists()) { - try (RandomAccessFile raf = new RandomAccessFile(path, "rws")) { - final int fileSize = (int) raf.length(); - raf.write(new byte[fileSize]); - } catch (Exception e) { - Slog.w(TAG, "Failed to zeroize " + path, e); - } finally { - file.delete(); - dispatchChange(this); - } - mCache.putFile(path, null); - } + deleteFile(getSyntheticPasswordStateFileForUser(userId, handle, name)); } public Map<Integer, List<Long>> listSyntheticPasswordHandlesForAllUsers(String stateName) { @@ -481,7 +429,7 @@ class LockSettingsStorage extends WatchableImpl { @VisibleForTesting protected File getSyntheticPasswordDirectoryForUser(int userId) { - return new File(Environment.getDataSystemDeDirectory(userId) ,SYNTHETIC_PASSWORD_DIRECTORY); + return new File(Environment.getDataSystemDeDirectory(userId), SYNTHETIC_PASSWORD_DIRECTORY); } /** Ensure per-user directory for synthetic password state exists */ @@ -492,12 +440,9 @@ class LockSettingsStorage extends WatchableImpl { } } - @VisibleForTesting - protected String getSynthenticPasswordStateFilePathForUser(int userId, long handle, - String name) { - final File baseDir = getSyntheticPasswordDirectoryForUser(userId); - final String baseName = formatSimple("%016x.%s", handle, name); - return new File(baseDir, baseName).getAbsolutePath(); + private File getSyntheticPasswordStateFileForUser(int userId, long handle, String name) { + String fileName = formatSimple("%016x.%s", handle, name); + return new File(getSyntheticPasswordDirectoryForUser(userId), fileName); } public void removeUser(int userId) { @@ -523,7 +468,7 @@ class LockSettingsStorage extends WatchableImpl { // The directory itself will be deleted as part of user deletion operation by the // framework, so only need to purge cache here. //TODO: (b/34600579) invoke secdiscardable - mCache.purgePath(spStateDir.getAbsolutePath()); + mCache.purgePath(spStateDir); } finally { db.endTransaction(); } @@ -704,7 +649,7 @@ class LockSettingsStorage extends WatchableImpl { final UserManager um = UserManager.get(mContext); for (UserInfo user : um.getUsers()) { File userPath = getSyntheticPasswordDirectoryForUser(user.id); - pw.println(String.format("User %d [%s]:", user.id, userPath.getAbsolutePath())); + pw.println(String.format("User %d [%s]:", user.id, userPath)); pw.increaseIndent(); File[] files = userPath.listFiles(); if (files != null) { @@ -817,20 +762,21 @@ class LockSettingsStorage extends WatchableImpl { remove(CacheKey.TYPE_KEY_VALUE, key, userId); } - byte[] peekFile(String fileName) { - return copyOf((byte[]) peek(CacheKey.TYPE_FILE, fileName, -1 /* userId */)); + byte[] peekFile(File path) { + return copyOf((byte[]) peek(CacheKey.TYPE_FILE, path.toString(), -1 /* userId */)); } - boolean hasFile(String fileName) { - return contains(CacheKey.TYPE_FILE, fileName, -1 /* userId */); + boolean hasFile(File path) { + return contains(CacheKey.TYPE_FILE, path.toString(), -1 /* userId */); } - void putFile(String key, byte[] value) { - put(CacheKey.TYPE_FILE, key, copyOf(value), -1 /* userId */); + void putFile(File path, byte[] data) { + put(CacheKey.TYPE_FILE, path.toString(), copyOf(data), -1 /* userId */); } - void putFileIfUnchanged(String key, byte[] value, int version) { - putIfUnchanged(CacheKey.TYPE_FILE, key, copyOf(value), -1 /* userId */, version); + void putFileIfUnchanged(File path, byte[] data, int version) { + putIfUnchanged(CacheKey.TYPE_FILE, path.toString(), copyOf(data), -1 /* userId */, + version); } void setFetched(int userId) { @@ -888,10 +834,11 @@ class LockSettingsStorage extends WatchableImpl { return data != null ? Arrays.copyOf(data, data.length) : null; } - synchronized void purgePath(String path) { + synchronized void purgePath(File path) { + final String pathStr = path.toString(); for (int i = mCache.size() - 1; i >= 0; i--) { CacheKey entry = mCache.keyAt(i); - if (entry.type == CacheKey.TYPE_FILE && entry.key.startsWith(path)) { + if (entry.type == CacheKey.TYPE_FILE && entry.key.startsWith(pathStr)) { mCache.removeAt(i); } } diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java index 2cb89b314b33..ec0d9856486d 100644 --- a/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java +++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java @@ -122,7 +122,7 @@ public class KeySyncTask implements Runnable { * @param recoverableKeyStoreDb Database where the keys are stored. * @param userId The uid of the user whose profile has been unlocked. * @param credentialType The type of credential as defined in {@code LockPatternUtils} - * @param credential The credential, encoded as a {@link String}. + * @param credential The credential, encoded as a byte array * @param credentialUpdated signals weather credentials were updated. * @param platformKeyManager platform key manager * @param testOnlyInsecureCertificateHelper utility class used for end-to-end tests diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java index b49bced4e567..e620c80b8e28 100644 --- a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java +++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java @@ -900,14 +900,13 @@ public class RecoverableKeyStoreManager { /** * This function can only be used inside LockSettingsService. * - * @param storedHashType from {@code CredentialHash} - * @param credential - unencrypted byte array. Password length should be at most 16 symbols - * {@code mPasswordMaxLength} - * @param userId for user who just unlocked the device. + * @param credentialType the type of credential, as defined in {@code LockPatternUtils} + * @param credential the credential, encoded as a byte array + * @param userId the ID of the user to whom the credential belongs * @hide */ public void lockScreenSecretAvailable( - int storedHashType, @NonNull byte[] credential, int userId) { + int credentialType, @NonNull byte[] credential, int userId) { // So as not to block the critical path unlocking the phone, defer to another thread. try { mExecutorService.schedule(KeySyncTask.newInstance( @@ -916,7 +915,7 @@ public class RecoverableKeyStoreManager { mSnapshotStorage, mListenersStorage, userId, - storedHashType, + credentialType, credential, /*credentialUpdated=*/ false), SYNC_DELAY_MILLIS, @@ -934,13 +933,13 @@ public class RecoverableKeyStoreManager { /** * This function can only be used inside LockSettingsService. * - * @param storedHashType from {@code CredentialHash} - * @param credential - unencrypted byte array - * @param userId for the user whose lock screen credentials were changed. + * @param credentialType the type of the new credential, as defined in {@code LockPatternUtils} + * @param credential the new credential, encoded as a byte array + * @param userId the ID of the user whose credential was changed * @hide */ public void lockScreenSecretChanged( - int storedHashType, + int credentialType, @Nullable byte[] credential, int userId) { // So as not to block the critical path unlocking the phone, defer to another thread. @@ -951,7 +950,7 @@ public class RecoverableKeyStoreManager { mSnapshotStorage, mListenersStorage, userId, - storedHashType, + credentialType, credential, /*credentialUpdated=*/ true), SYNC_DELAY_MILLIS, diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsStorageTestable.java b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsStorageTestable.java index c30af4cae59d..3f3b8d770f61 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsStorageTestable.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsStorageTestable.java @@ -32,7 +32,7 @@ import java.util.Arrays; public class LockSettingsStorageTestable extends LockSettingsStorage { - public File mStorageDir; + public final File mStorageDir; public PersistentDataBlockManagerInternal mPersistentDataBlockManager; private byte[] mPersistentData; @@ -64,25 +64,23 @@ public class LockSettingsStorageTestable extends LockSettingsStorage { } @Override - String getChildProfileLockFile(int userId) { - return makeDirs(mStorageDir, - super.getChildProfileLockFile(userId)).getAbsolutePath(); + File getChildProfileLockFile(int userId) { + return remapToStorageDir(super.getChildProfileLockFile(userId)); } @Override - String getRebootEscrowServerBlob() { - return makeDirs(mStorageDir, super.getRebootEscrowServerBlob()).getAbsolutePath(); + File getRebootEscrowServerBlobFile() { + return remapToStorageDir(super.getRebootEscrowServerBlobFile()); } @Override - String getRebootEscrowFile(int userId) { - return makeDirs(mStorageDir, super.getRebootEscrowFile(userId)).getAbsolutePath(); + File getRebootEscrowFile(int userId) { + return remapToStorageDir(super.getRebootEscrowFile(userId)); } @Override protected File getSyntheticPasswordDirectoryForUser(int userId) { - return makeDirs(mStorageDir, super.getSyntheticPasswordDirectoryForUser( - userId).getAbsolutePath()); + return remapToStorageDir(super.getSyntheticPasswordDirectoryForUser(userId)); } @Override @@ -90,14 +88,9 @@ public class LockSettingsStorageTestable extends LockSettingsStorage { return mPersistentDataBlockManager; } - private File makeDirs(File baseDir, String filePath) { - File path = new File(filePath); - if (path.getParent() == null) { - return new File(baseDir, filePath); - } else { - File mappedDir = new File(baseDir, path.getParent()); - mappedDir.mkdirs(); - return new File(mappedDir, path.getName()); - } + private File remapToStorageDir(File origPath) { + File mappedPath = new File(mStorageDir, origPath.toString()); + mappedPath.getParentFile().mkdirs(); + return mappedPath; } } diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsStorageTests.java b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsStorageTests.java index c4df05149d4b..a6638587bf22 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsStorageTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsStorageTests.java @@ -304,14 +304,15 @@ public class LockSettingsStorageTests { public void testFileLocation_Owner() { LockSettingsStorage storage = new LockSettingsStorage(InstrumentationRegistry.getContext()); - assertEquals("/data/system/gatekeeper.profile.key", storage.getChildProfileLockFile(0)); + assertEquals(new File("/data/system/gatekeeper.profile.key"), + storage.getChildProfileLockFile(0)); } @Test public void testFileLocation_SecondaryUser() { LockSettingsStorage storage = new LockSettingsStorage(InstrumentationRegistry.getContext()); - assertEquals("/data/system/users/1/gatekeeper.profile.key", + assertEquals(new File("/data/system/users/1/gatekeeper.profile.key"), storage.getChildProfileLockFile(1)); } @@ -319,7 +320,7 @@ public class LockSettingsStorageTests { public void testFileLocation_ProfileToSecondary() { LockSettingsStorage storage = new LockSettingsStorage(InstrumentationRegistry.getContext()); - assertEquals("/data/system/users/2/gatekeeper.profile.key", + assertEquals(new File("/data/system/users/2/gatekeeper.profile.key"), storage.getChildProfileLockFile(2)); } @@ -327,7 +328,7 @@ public class LockSettingsStorageTests { public void testFileLocation_ProfileToOwner() { LockSettingsStorage storage = new LockSettingsStorage(InstrumentationRegistry.getContext()); - assertEquals("/data/system/users/3/gatekeeper.profile.key", + assertEquals(new File("/data/system/users/3/gatekeeper.profile.key"), storage.getChildProfileLockFile(3)); } |