From c7c26cc8e9cdcaa8621b10984be1748f55474c24 Mon Sep 17 00:00:00 2001 From: Varun Shah Date: Wed, 20 Mar 2019 11:10:33 -0700 Subject: [PATCH 01/15] Added missing permission check to isPackageDeviceAdminOnAnyUser. Added a check for the MANAGE_USERS permission to PackageManagerService#isPackageDeviceAdminOnAnyUser. To test that the method is still usable: 1) Enable virtual storage via: adb shell sm set-virtual-disk true 2) Follow instructions by clicking on notification to set up virtual storage 3) Go to Settings -> Apps & notifications -> See all X apps 4) Click on any non-system app (example Instagram) 5) Tap Storage and you should see a "Change" button (if not, choose another app) 6) Tap Change and you should see Internal and Virtual storage options listed 7) The above step confirms the method is still usable by Settings Bug: 128599183 Test: SafetyNet logging (steps listed above) Change-Id: I989f1daf52a71f6c778ebd81baa6f1bf83e9a718 Merged-In: I36521fa43daab399e08869647326a7ac32d1e512 (cherry picked from commit 18e7dedf6c35f07daf8b7239d501737745ac7f43) --- .../java/com/android/server/pm/PackageManagerService.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 82ad46f08aed0..3cde709bfeb2a 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -18141,6 +18141,12 @@ public class PackageManagerService extends IPackageManager.Stub @Override public boolean isPackageDeviceAdminOnAnyUser(String packageName) { final int callingUid = Binder.getCallingUid(); + if (checkUidPermission(android.Manifest.permission.MANAGE_USERS, callingUid) + != PERMISSION_GRANTED) { + EventLog.writeEvent(0x534e4554, "128599183", -1, ""); + throw new SecurityException(android.Manifest.permission.MANAGE_USERS + + " permission is required to call this API"); + } if (getInstantAppPackageName(callingUid) != null && !isCallerSameApp(packageName, callingUid)) { return false; From d7008c0ba385aa836a603186a007b8e14df34bcc Mon Sep 17 00:00:00 2001 From: Pavel Grafov Date: Wed, 10 Apr 2019 12:47:25 +0100 Subject: [PATCH 02/15] Limit IsSeparateProfileChallengeAllowed to system callers Fixes: 128599668 Test: build, set up separate challenge Change-Id: I2fef9ab13614627c0f1bcca04759d0974fc6181a (cherry picked from commit 1b6301cf2430f192c9842a05fc22984d782bade9) --- .../server/devicepolicy/DevicePolicyManagerService.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index cb52931433d41..00b8366ff86da 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -3930,6 +3930,9 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { @Override public boolean isSeparateProfileChallengeAllowed(int userHandle) { + if (!isCallerWithSystemUid()) { + throw new SecurityException("Caller must be system"); + } ComponentName profileOwner = getProfileOwner(userHandle); // Profile challenge is supported on N or newer release. return profileOwner != null && From 1dfce98d9dcada1571718ad97e79c5f86a3d86f3 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Wed, 27 Mar 2019 12:15:57 -0400 Subject: [PATCH 03/15] Add cross user permission check - areNotificationsEnabledForPackage Test: atest Fixes: 128599467 Change-Id: I13a0ca7590f8c4b44379730e0ee2088aba400c2a Merged-In: I13a0ca7590f8c4b44379730e0ee2088aba400c2a (cherry picked from commit 657d164136199126ae241848887de0230699cea0) (cherry picked from commit 63846a7093ca7c6d89b73fc77bdff267b3ecb4ef) --- .../NotificationManagerService.java | 5 +++++ .../NotificationManagerServiceTest.java | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 82363d102c407..1928cb7d0f0e6 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -2186,6 +2186,11 @@ public class NotificationManagerService extends SystemService { @Override public boolean areNotificationsEnabledForPackage(String pkg, int uid) { checkCallerIsSystemOrSameApp(pkg); + if (UserHandle.getCallingUserId() != UserHandle.getUserId(uid)) { + getContext().enforceCallingPermission( + android.Manifest.permission.INTERACT_ACROSS_USERS, + "canNotifyAsPackage for uid " + uid); + } return mRankingHelper.getImportance(pkg, uid) != IMPORTANCE_NONE; } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index f02c3f062f352..5622622e925ea 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -34,6 +34,7 @@ import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_SCREEN_ON import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_STATUS_BAR; import static android.content.pm.PackageManager.FEATURE_WATCH; import static android.content.pm.PackageManager.PERMISSION_DENIED; +import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.os.Build.VERSION_CODES.O_MR1; import static android.os.Build.VERSION_CODES.P; @@ -106,6 +107,7 @@ import android.testing.AndroidTestingRunner; import android.testing.TestableContext; import android.testing.TestableLooper; import android.testing.TestableLooper.RunWithLooper; +import android.testing.TestablePermissions; import android.text.Html; import android.util.ArrayMap; import android.util.AtomicFile; @@ -3145,4 +3147,21 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { assertEquals(0, captor.getValue().getNotification().flags); } + + @Test + public void testAreNotificationsEnabledForPackage_crossUser() throws Exception { + try { + mBinderService.areNotificationsEnabledForPackage(mContext.getPackageName(), + mUid + UserHandle.PER_USER_RANGE); + fail("Cannot call cross user without permission"); + } catch (SecurityException e) { + // pass + } + + // cross user, with permission, no problem + TestablePermissions perms = mContext.getTestablePermissions(); + perms.setPermission(android.Manifest.permission.INTERACT_ACROSS_USERS, PERMISSION_GRANTED); + mBinderService.areNotificationsEnabledForPackage(mContext.getPackageName(), + mUid + UserHandle.PER_USER_RANGE); + } } From 1d1faf36c7e3fe6b672e22ff9c5d39a61b58663c Mon Sep 17 00:00:00 2001 From: Varun Shah Date: Wed, 20 Mar 2019 11:10:33 -0700 Subject: [PATCH 04/15] Added missing permission check to isPackageDeviceAdminOnAnyUser. Added a check for the MANAGE_USERS permission to PackageManagerService#isPackageDeviceAdminOnAnyUser. To test that the method is still usable: 1) Enable virtual storage via: adb shell sm set-virtual-disk true 2) Follow instructions by clicking on notification to set up virtual storage 3) Go to Settings -> Apps & notifications -> See all X apps 4) Click on any non-system app (example Instagram) 5) Tap Storage and you should see a "Change" button (if not, choose another app) 6) Tap Change and you should see Internal and Virtual storage options listed 7) The above step confirms the method is still usable by Settings Bug: 128599183 Test: SafetyNet logging (steps listed above) Change-Id: I989f1daf52a71f6c778ebd81baa6f1bf83e9a718 Merged-In: I36521fa43daab399e08869647326a7ac32d1e512 (cherry picked from commit 18e7dedf6c35f07daf8b7239d501737745ac7f43) --- .../java/com/android/server/pm/PackageManagerService.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 82ad46f08aed0..3cde709bfeb2a 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -18141,6 +18141,12 @@ public class PackageManagerService extends IPackageManager.Stub @Override public boolean isPackageDeviceAdminOnAnyUser(String packageName) { final int callingUid = Binder.getCallingUid(); + if (checkUidPermission(android.Manifest.permission.MANAGE_USERS, callingUid) + != PERMISSION_GRANTED) { + EventLog.writeEvent(0x534e4554, "128599183", -1, ""); + throw new SecurityException(android.Manifest.permission.MANAGE_USERS + + " permission is required to call this API"); + } if (getInstantAppPackageName(callingUid) != null && !isCallerSameApp(packageName, callingUid)) { return false; From 0fc5dbf3d1d92ae817cdb8cf06170517a9a9251b Mon Sep 17 00:00:00 2001 From: Pavel Grafov Date: Wed, 10 Apr 2019 12:47:25 +0100 Subject: [PATCH 05/15] Limit IsSeparateProfileChallengeAllowed to system callers Fixes: 128599668 Test: build, set up separate challenge Change-Id: I2fef9ab13614627c0f1bcca04759d0974fc6181a (cherry picked from commit 1b6301cf2430f192c9842a05fc22984d782bade9) --- .../server/devicepolicy/DevicePolicyManagerService.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index cb52931433d41..00b8366ff86da 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -3930,6 +3930,9 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { @Override public boolean isSeparateProfileChallengeAllowed(int userHandle) { + if (!isCallerWithSystemUid()) { + throw new SecurityException("Caller must be system"); + } ComponentName profileOwner = getProfileOwner(userHandle); // Profile challenge is supported on N or newer release. return profileOwner != null && From 9162b8635bb6358be4f8fa2f6aa0188d6cddd641 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Wed, 27 Mar 2019 12:15:57 -0400 Subject: [PATCH 06/15] Add cross user permission check - areNotificationsEnabledForPackage Test: atest Fixes: 128599467 Change-Id: I13a0ca7590f8c4b44379730e0ee2088aba400c2a Merged-In: I13a0ca7590f8c4b44379730e0ee2088aba400c2a (cherry picked from commit 657d164136199126ae241848887de0230699cea0) (cherry picked from commit 63846a7093ca7c6d89b73fc77bdff267b3ecb4ef) --- .../NotificationManagerService.java | 5 +++++ .../NotificationManagerServiceTest.java | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 82363d102c407..1928cb7d0f0e6 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -2186,6 +2186,11 @@ public class NotificationManagerService extends SystemService { @Override public boolean areNotificationsEnabledForPackage(String pkg, int uid) { checkCallerIsSystemOrSameApp(pkg); + if (UserHandle.getCallingUserId() != UserHandle.getUserId(uid)) { + getContext().enforceCallingPermission( + android.Manifest.permission.INTERACT_ACROSS_USERS, + "canNotifyAsPackage for uid " + uid); + } return mRankingHelper.getImportance(pkg, uid) != IMPORTANCE_NONE; } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index f02c3f062f352..5622622e925ea 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -34,6 +34,7 @@ import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_SCREEN_ON import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_STATUS_BAR; import static android.content.pm.PackageManager.FEATURE_WATCH; import static android.content.pm.PackageManager.PERMISSION_DENIED; +import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.os.Build.VERSION_CODES.O_MR1; import static android.os.Build.VERSION_CODES.P; @@ -106,6 +107,7 @@ import android.testing.AndroidTestingRunner; import android.testing.TestableContext; import android.testing.TestableLooper; import android.testing.TestableLooper.RunWithLooper; +import android.testing.TestablePermissions; import android.text.Html; import android.util.ArrayMap; import android.util.AtomicFile; @@ -3145,4 +3147,21 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { assertEquals(0, captor.getValue().getNotification().flags); } + + @Test + public void testAreNotificationsEnabledForPackage_crossUser() throws Exception { + try { + mBinderService.areNotificationsEnabledForPackage(mContext.getPackageName(), + mUid + UserHandle.PER_USER_RANGE); + fail("Cannot call cross user without permission"); + } catch (SecurityException e) { + // pass + } + + // cross user, with permission, no problem + TestablePermissions perms = mContext.getTestablePermissions(); + perms.setPermission(android.Manifest.permission.INTERACT_ACROSS_USERS, PERMISSION_GRANTED); + mBinderService.areNotificationsEnabledForPackage(mContext.getPackageName(), + mUid + UserHandle.PER_USER_RANGE); + } } From 9cb88f8e11f5ad50efa858eb5f6d46628e53813e Mon Sep 17 00:00:00 2001 From: Ahming Chen Date: Thu, 2 May 2019 05:52:59 +0000 Subject: [PATCH 07/15] Revert "DO NOT MERGE Make a copy of data stored in LockSettingsStorage cache" This reverts commit 567f2357f9fda4eb40594f52342b85540b817287. Reason for revert: Change-Id: I0bc0183de916f29f79d0cee053043cf6c323f167 (cherry picked from commit bae09a535892453bf72741b0cd60e6502814f469) --- .../server/locksettings/LockSettingsStorage.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java index 2ede384f3edfa..98f1740aa9860 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java @@ -29,6 +29,7 @@ import android.database.sqlite.SQLiteOpenHelper; import android.os.Environment; import android.os.UserHandle; import android.os.UserManager; +import android.os.storage.StorageManager; import android.util.ArrayMap; import android.util.Log; import android.util.Slog; @@ -48,7 +49,6 @@ import java.io.File; import java.io.IOException; import java.io.RandomAccessFile; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Map; @@ -808,7 +808,7 @@ class LockSettingsStorage { } byte[] peekFile(String fileName) { - return copyOf((byte[]) peek(CacheKey.TYPE_FILE, fileName, -1 /* userId */)); + return (byte[]) peek(CacheKey.TYPE_FILE, fileName, -1 /* userId */); } boolean hasFile(String fileName) { @@ -816,11 +816,11 @@ class LockSettingsStorage { } void putFile(String key, byte[] value) { - put(CacheKey.TYPE_FILE, key, copyOf(value), -1 /* userId */); + put(CacheKey.TYPE_FILE, key, value, -1 /* userId */); } void putFileIfUnchanged(String key, byte[] value, int version) { - putIfUnchanged(CacheKey.TYPE_FILE, key, copyOf(value), -1 /* userId */, version); + putIfUnchanged(CacheKey.TYPE_FILE, key, value, -1 /* userId */, version); } void setFetched(int userId) { @@ -868,10 +868,6 @@ class LockSettingsStorage { mVersion++; } - private byte[] copyOf(byte[] data) { - return data != null ? Arrays.copyOf(data, data.length) : null; - } - synchronized void purgePath(String path) { for (int i = mCache.size() - 1; i >= 0; i--) { CacheKey entry = mCache.keyAt(i); From caadc532ae1608a24812e4eb46c44082f02412c9 Mon Sep 17 00:00:00 2001 From: Ahming Chen Date: Thu, 2 May 2019 05:58:13 +0000 Subject: [PATCH 08/15] Revert "DO NOT MERGE Refactor passwords/pins/patterns to byte[]" This reverts commit 820b2504f36d65ef3cd2e34ec72ee53c90fa89d9. Reason for revert: Change-Id: Ic6804433644eda4af9d82d1a356e0d3a7a72b552 (cherry picked from commit 075c346e8ff4713e2af0da662a14935485d96398) --- .../android/app/admin/PasswordMetrics.java | 25 +- .../internal/widget/ILockSettings.aidl | 12 +- .../internal/widget/LockPatternChecker.java | 43 +--- .../internal/widget/LockPatternUtils.java | 215 +++++------------- .../internal/widget/LockPatternView.java | 4 +- .../internal/widget/LockSettingsInternal.java | 6 +- .../app/admin/PasswordMetricsTest.java | 37 +-- .../keyguard/KeyguardAbsKeyInputView.java | 12 +- .../keyguard/KeyguardPasswordView.java | 18 +- .../keyguard/KeyguardPinBasedInputView.java | 18 +- .../locksettings/LockSettingsService.java | 159 ++++++------- .../LockSettingsShellCommand.java | 27 +-- .../SyntheticPasswordManager.java | 34 +-- .../recoverablekeystore/KeySyncTask.java | 29 ++- .../RecoverableKeyStoreManager.java | 10 +- .../TestOnlyInsecureCertificateHelper.java | 28 +-- .../DevicePolicyManagerService.java | 19 +- .../devicepolicy/DevicePolicyManagerTest.java | 2 +- .../CachedSyntheticPasswordTests.java | 73 +++--- .../LockSettingsServiceTestable.java | 8 +- .../LockSettingsServiceTests.java | 59 +++-- .../LockSettingsShellCommandTest.java | 22 +- .../MockSyntheticPasswordManager.java | 8 +- .../locksettings/SyntheticPasswordTests.java | 210 +++++++++-------- .../recoverablekeystore/KeySyncTaskTest.java | 50 ++-- ...TestOnlyInsecureCertificateHelperTest.java | 12 +- 26 files changed, 437 insertions(+), 703 deletions(-) diff --git a/core/java/android/app/admin/PasswordMetrics.java b/core/java/android/app/admin/PasswordMetrics.java index 69dc43d7228ee..5fee853275fb0 100644 --- a/core/java/android/app/admin/PasswordMetrics.java +++ b/core/java/android/app/admin/PasswordMetrics.java @@ -107,10 +107,7 @@ public class PasswordMetrics implements Parcelable { } }; - /** - * Returns the {@code PasswordMetrics} for a given password - */ - public static PasswordMetrics computeForPassword(@NonNull byte[] password) { + public static PasswordMetrics computeForPassword(@NonNull String password) { // Analyse the characters used int letters = 0; int upperCase = 0; @@ -118,9 +115,9 @@ public class PasswordMetrics implements Parcelable { int numeric = 0; int symbols = 0; int nonLetter = 0; - final int length = password.length; + final int length = password.length(); for (int i = 0; i < length; i++) { - switch (categoryChar((char) password[i])) { + switch (categoryChar(password.charAt(i))) { case CHAR_LOWER_CASE: letters++; lowerCase++; @@ -176,7 +173,7 @@ public class PasswordMetrics implements Parcelable { && this.nonLetter == o.nonLetter; } - /** + /* * Returns the maximum length of a sequential characters. A sequence is defined as * monotonically increasing characters with a constant interval or the same character repeated. * @@ -190,19 +187,19 @@ public class PasswordMetrics implements Parcelable { * maxLengthSequence(";;;;") == 4 (anything that repeats) * maxLengthSequence(":;<=>") == 1 (ordered, but not composed of alphas or digits) * - * @param bytes the pass + * @param string the pass * @return the number of sequential letters or digits */ - public static int maxLengthSequence(@NonNull byte[] bytes) { - if (bytes.length == 0) return 0; - char previousChar = (char) bytes[0]; + public static int maxLengthSequence(@NonNull String string) { + if (string.length() == 0) return 0; + char previousChar = string.charAt(0); @CharacterCatagory int category = categoryChar(previousChar); //current sequence category int diff = 0; //difference between two consecutive characters boolean hasDiff = false; //if we are currently targeting a sequence int maxLength = 0; //maximum length of a sequence already found int startSequence = 0; //where the current sequence started - for (int current = 1; current < bytes.length; current++) { - char currentChar = (char) bytes[current]; + for (int current = 1; current < string.length(); current++) { + char currentChar = string.charAt(current); @CharacterCatagory int categoryCurrent = categoryChar(currentChar); int currentDiff = (int) currentChar - (int) previousChar; if (categoryCurrent != category || Math.abs(currentDiff) > maxDiffCategory(category)) { @@ -221,7 +218,7 @@ public class PasswordMetrics implements Parcelable { } previousChar = currentChar; } - maxLength = Math.max(maxLength, bytes.length - startSequence); + maxLength = Math.max(maxLength, string.length() - startSequence); return maxLength; } diff --git a/core/java/com/android/internal/widget/ILockSettings.aidl b/core/java/com/android/internal/widget/ILockSettings.aidl index eca57dcab85e8..591f15fd56765 100644 --- a/core/java/com/android/internal/widget/ILockSettings.aidl +++ b/core/java/com/android/internal/widget/ILockSettings.aidl @@ -36,17 +36,17 @@ interface ILockSettings { boolean getBoolean(in String key, in boolean defaultValue, in int userId); long getLong(in String key, in long defaultValue, in int userId); String getString(in String key, in String defaultValue, in int userId); - void setLockCredential(in byte[] credential, int type, in byte[] savedCredential, int requestedQuality, int userId); + void setLockCredential(in String credential, int type, in String savedCredential, int requestedQuality, int userId); void resetKeyStore(int userId); - VerifyCredentialResponse checkCredential(in byte[] credential, int type, int userId, + VerifyCredentialResponse checkCredential(in String credential, int type, int userId, in ICheckCredentialProgressCallback progressCallback); - VerifyCredentialResponse verifyCredential(in byte[] credential, int type, long challenge, int userId); - VerifyCredentialResponse verifyTiedProfileChallenge(in byte[] credential, int type, long challenge, int userId); + VerifyCredentialResponse verifyCredential(in String credential, int type, long challenge, int userId); + VerifyCredentialResponse verifyTiedProfileChallenge(String credential, int type, long challenge, int userId); boolean checkVoldPassword(int userId); boolean havePattern(int userId); boolean havePassword(int userId); - byte[] getHashFactor(in byte[] currentCredential, int userId); - void setSeparateProfileChallengeEnabled(int userId, boolean enabled, in byte[] managedUserPassword); + byte[] getHashFactor(String currentCredential, int userId); + void setSeparateProfileChallengeEnabled(int userId, boolean enabled, String managedUserPassword); boolean getSeparateProfileChallengeEnabled(int userId); void registerStrongAuthTracker(in IStrongAuthTracker tracker); void unregisterStrongAuthTracker(in IStrongAuthTracker tracker); diff --git a/core/java/com/android/internal/widget/LockPatternChecker.java b/core/java/com/android/internal/widget/LockPatternChecker.java index bda3b5728fdc4..586ece0a274af 100644 --- a/core/java/com/android/internal/widget/LockPatternChecker.java +++ b/core/java/com/android/internal/widget/LockPatternChecker.java @@ -150,33 +150,12 @@ public final class LockPatternChecker { * @param challenge The challenge to verify against the pattern. * @param userId The user to check against the pattern. * @param callback The callback to be invoked with the verification result. - * - * @deprecated Pass the password as a byte array. */ - @Deprecated public static AsyncTask verifyPassword(final LockPatternUtils utils, final String password, final long challenge, final int userId, final OnVerifyCallback callback) { - byte[] passwordBytes = password != null ? password.getBytes() : null; - return verifyPassword(utils, passwordBytes, challenge, userId, callback); - } - - /** - * Verify a password asynchronously. - * - * @param utils The LockPatternUtils instance to use. - * @param password The password to check. - * @param challenge The challenge to verify against the pattern. - * @param userId The user to check against the pattern. - * @param callback The callback to be invoked with the verification result. - */ - public static AsyncTask verifyPassword(final LockPatternUtils utils, - final byte[] password, - final long challenge, - final int userId, - final OnVerifyCallback callback) { AsyncTask task = new AsyncTask() { private int mThrottleTimeout; @@ -209,7 +188,7 @@ public final class LockPatternChecker { * @param callback The callback to be invoked with the verification result. */ public static AsyncTask verifyTiedProfileChallenge(final LockPatternUtils utils, - final byte[] password, + final String password, final boolean isPattern, final long challenge, final int userId, @@ -243,36 +222,18 @@ public final class LockPatternChecker { * @param password The password to check. * @param userId The user to check against the pattern. * @param callback The callback to be invoked with the check result. - * @deprecated Pass passwords as byte[] */ - @Deprecated public static AsyncTask checkPassword(final LockPatternUtils utils, final String password, final int userId, final OnCheckCallback callback) { - byte[] passwordBytes = password != null ? password.getBytes() : null; - return checkPassword(utils, passwordBytes, userId, callback); - } - - /** - * Checks a password asynchronously. - * - * @param utils The LockPatternUtils instance to use. - * @param passwordBytes The password to check. - * @param userId The user to check against the pattern. - * @param callback The callback to be invoked with the check result. - */ - public static AsyncTask checkPassword(final LockPatternUtils utils, - final byte[] passwordBytes, - final int userId, - final OnCheckCallback callback) { AsyncTask task = new AsyncTask() { private int mThrottleTimeout; @Override protected Boolean doInBackground(Void... args) { try { - return utils.checkPassword(passwordBytes, userId, callback::onEarlyMatched); + return utils.checkPassword(password, userId, callback::onEarlyMatched); } catch (RequestThrottledException ex) { mThrottleTimeout = ex.getTimeoutMs(); return false; diff --git a/core/java/com/android/internal/widget/LockPatternUtils.java b/core/java/com/android/internal/widget/LockPatternUtils.java index 99a461bbcf15b..7c339fb6d6b1a 100644 --- a/core/java/com/android/internal/widget/LockPatternUtils.java +++ b/core/java/com/android/internal/widget/LockPatternUtils.java @@ -350,7 +350,7 @@ public class LockPatternUtils { null /* componentName */, userId); } - private byte[] verifyCredential(byte[] credential, int type, long challenge, int userId) + private byte[] verifyCredential(String credential, int type, long challenge, int userId) throws RequestThrottledException { try { VerifyCredentialResponse response = getLockSettings().verifyCredential(credential, @@ -367,7 +367,7 @@ public class LockPatternUtils { } } - private boolean checkCredential(byte[] credential, int type, int userId, + private boolean checkCredential(String credential, int type, int userId, @Nullable CheckCredentialProgressCallback progressCallback) throws RequestThrottledException { try { @@ -398,7 +398,7 @@ public class LockPatternUtils { public byte[] verifyPattern(List pattern, long challenge, int userId) throws RequestThrottledException { throwIfCalledOnMainThread(); - return verifyCredential(patternToByteArray(pattern), CREDENTIAL_TYPE_PATTERN, challenge, + return verifyCredential(patternToString(pattern), CREDENTIAL_TYPE_PATTERN, challenge, userId); } @@ -423,7 +423,7 @@ public class LockPatternUtils { @Nullable CheckCredentialProgressCallback progressCallback) throws RequestThrottledException { throwIfCalledOnMainThread(); - return checkCredential(patternToByteArray(pattern), CREDENTIAL_TYPE_PATTERN, userId, + return checkCredential(patternToString(pattern), CREDENTIAL_TYPE_PATTERN, userId, progressCallback); } @@ -436,7 +436,7 @@ public class LockPatternUtils { * @param challenge The challenge to verify against the password * @return the attestation that the challenge was verified, or null. */ - public byte[] verifyPassword(byte[] password, long challenge, int userId) + public byte[] verifyPassword(String password, long challenge, int userId) throws RequestThrottledException { throwIfCalledOnMainThread(); return verifyCredential(password, CREDENTIAL_TYPE_PASSWORD, challenge, userId); @@ -452,7 +452,7 @@ public class LockPatternUtils { * @param challenge The challenge to verify against the password * @return the attestation that the challenge was verified, or null. */ - public byte[] verifyTiedProfileChallenge(byte[] password, boolean isPattern, long challenge, + public byte[] verifyTiedProfileChallenge(String password, boolean isPattern, long challenge, int userId) throws RequestThrottledException { throwIfCalledOnMainThread(); try { @@ -474,31 +474,16 @@ public class LockPatternUtils { } /** - * * Check to see if a password matches the saved password. If no password exists, * always returns true. * @param password The password to check. * @return Whether the password matches the stored one. */ public boolean checkPassword(String password, int userId) throws RequestThrottledException { - byte[] passwordBytes = password != null ? password.getBytes() : null; - return checkPassword(passwordBytes, userId, null /* progressCallback */); - } - - - /** - * - * Check to see if a password matches the saved password. If no password exists, - * always returns true. - * @param password The password to check. - * @return Whether the password matches the stored one. - */ - public boolean checkPassword(byte[] password, int userId) throws RequestThrottledException { return checkPassword(password, userId, null /* progressCallback */); } - // TODO(b/120484642): This method is necessary for vendor/qcom code and is a hidden api - /* * + /** * Check to see if a password matches the saved password. If no password exists, * always returns true. * @param password The password to check. @@ -507,22 +492,6 @@ public class LockPatternUtils { public boolean checkPassword(String password, int userId, @Nullable CheckCredentialProgressCallback progressCallback) throws RequestThrottledException { - byte[] passwordBytes = password != null ? password.getBytes() : null; - throwIfCalledOnMainThread(); - return checkCredential(passwordBytes, CREDENTIAL_TYPE_PASSWORD, userId, progressCallback); - - } - - /** - * Check to see if a password matches the saved password. If no password exists, - * always returns true. - * @param password The password to check. - * @return Whether the password matches the stored one. - */ - - public boolean checkPassword(byte[] password, int userId, - @Nullable CheckCredentialProgressCallback progressCallback) - throws RequestThrottledException { throwIfCalledOnMainThread(); return checkCredential(password, CREDENTIAL_TYPE_PASSWORD, userId, progressCallback); } @@ -544,7 +513,7 @@ public class LockPatternUtils { * Returns the password history hash factor, needed to check new password against password * history with {@link #checkPasswordHistory(String, byte[], int)} */ - public byte[] getPasswordHistoryHashFactor(byte[] currentPassword, int userId) { + public byte[] getPasswordHistoryHashFactor(String currentPassword, int userId) { try { return getLockSettings().getHashFactor(currentPassword, userId); } catch (RemoteException e) { @@ -562,8 +531,8 @@ public class LockPatternUtils { * {@link ILockSettings#getHashFactor} * @return Whether the password matches any in the history. */ - public boolean checkPasswordHistory(byte[] passwordToCheck, byte[] hashFactor, int userId) { - if (passwordToCheck == null || passwordToCheck.length == 0) { + public boolean checkPasswordHistory(String passwordToCheck, byte[] hashFactor, int userId) { + if (TextUtils.isEmpty(passwordToCheck)) { Log.e(TAG, "checkPasswordHistory: empty password"); return false; } @@ -664,13 +633,13 @@ public class LockPatternUtils { /** * Clear any lock pattern or password. */ - public void clearLock(byte[] savedCredential, int userHandle) { + public void clearLock(String savedCredential, int userHandle) { final int currentQuality = getKeyguardStoredPasswordQuality(userHandle); setKeyguardStoredPasswordQuality(PASSWORD_QUALITY_UNSPECIFIED, userHandle); try{ - getLockSettings().setLockCredential(null, CREDENTIAL_TYPE_NONE, - savedCredential, PASSWORD_QUALITY_UNSPECIFIED, userHandle); + getLockSettings().setLockCredential(null, CREDENTIAL_TYPE_NONE, savedCredential, + PASSWORD_QUALITY_UNSPECIFIED, userHandle); } catch (Exception e) { Log.e(TAG, "Failed to clear lock", e); setKeyguardStoredPasswordQuality(currentQuality, userHandle); @@ -729,22 +698,21 @@ public class LockPatternUtils { /** * Save a lock pattern. * @param pattern The new pattern to save. - * @param savedPattern The previously saved pattern, converted to byte[] format + * @param savedPattern The previously saved pattern, converted to String format * @param userId the user whose pattern is to be saved. */ - public void saveLockPattern(List pattern, byte[] savedPattern, - int userId) { + public void saveLockPattern(List pattern, String savedPattern, int userId) { if (pattern == null || pattern.size() < MIN_LOCK_PATTERN_SIZE) { throw new IllegalArgumentException("pattern must not be null and at least " + MIN_LOCK_PATTERN_SIZE + " dots long."); } - final byte[] bytePattern = patternToByteArray(pattern); + final String stringPattern = patternToString(pattern); final int currentQuality = getKeyguardStoredPasswordQuality(userId); setKeyguardStoredPasswordQuality(PASSWORD_QUALITY_SOMETHING, userId); try { - getLockSettings().setLockCredential(bytePattern, CREDENTIAL_TYPE_PATTERN, savedPattern, - PASSWORD_QUALITY_SOMETHING, userId); + getLockSettings().setLockCredential(stringPattern, CREDENTIAL_TYPE_PATTERN, + savedPattern, PASSWORD_QUALITY_SOMETHING, userId); } catch (Exception e) { Log.e(TAG, "Couldn't save lock pattern", e); setKeyguardStoredPasswordQuality(currentQuality, userId); @@ -756,7 +724,7 @@ public class LockPatternUtils { if (!shouldEncryptWithCredentials(true)) { clearEncryptionPassword(); } else { - updateEncryptionPassword(StorageManager.CRYPT_TYPE_PATTERN, bytePattern); + updateEncryptionPassword(StorageManager.CRYPT_TYPE_PATTERN, stringPattern); } } @@ -828,7 +796,7 @@ public class LockPatternUtils { } /** Update the encryption password if it is enabled **/ - private void updateEncryptionPassword(final int type, final byte[] password) { + private void updateEncryptionPassword(final int type, final String password) { if (!isDeviceEncryptionEnabled()) { return; } @@ -843,9 +811,7 @@ public class LockPatternUtils { protected Void doInBackground(Void... dummy) { IStorageManager storageManager = IStorageManager.Stub.asInterface(service); try { - // TODO(b/120484642): This is a location where we still use a String for vold - String passwordString = password != null ? new String(password) : null; - storageManager.changeEncryptionPassword(type, passwordString); + storageManager.changeEncryptionPassword(type, password); } catch (RemoteException e) { Log.e(TAG, "Error changing encryption password", e); } @@ -862,30 +828,10 @@ public class LockPatternUtils { * @param savedPassword The previously saved lock password, or null if none * @param requestedQuality {@see DevicePolicyManager#getPasswordQuality(android.content.ComponentName)} * @param userHandle The userId of the user to change the password for - * - * @deprecated Pass password as a byte array */ - @Deprecated public void saveLockPassword(String password, String savedPassword, int requestedQuality, int userHandle) { - byte[] passwordBytes = password != null ? password.getBytes() : null; - byte[] savedPasswordBytes = savedPassword != null ? savedPassword.getBytes() : null; - saveLockPassword(passwordBytes, savedPasswordBytes, requestedQuality, userHandle); - } - - /** - * Save a lock password. Does not ensure that the password is as good - * as the requested mode, but will adjust the mode to be as good as the - * password. - * @param password The password to save - * @param savedPassword The previously saved lock password, or null if none - * @param requestedQuality {@see DevicePolicyManager#getPasswordQuality( - * android.content.ComponentName)} - * @param userHandle The userId of the user to change the password for - */ - public void saveLockPassword(byte[] password, byte[] savedPassword, int requestedQuality, - int userHandle) { - if (password == null || password.length < MIN_LOCK_PASSWORD_SIZE) { + if (password == null || password.length() < MIN_LOCK_PASSWORD_SIZE) { throw new IllegalArgumentException("password must not be null and at least " + "of length " + MIN_LOCK_PASSWORD_SIZE); } @@ -895,8 +841,8 @@ public class LockPatternUtils { computePasswordQuality(CREDENTIAL_TYPE_PASSWORD, password, requestedQuality), userHandle); try { - getLockSettings().setLockCredential(password, CREDENTIAL_TYPE_PASSWORD, savedPassword, - requestedQuality, userHandle); + getLockSettings().setLockCredential(password, CREDENTIAL_TYPE_PASSWORD, + savedPassword, requestedQuality, userHandle); } catch (Exception e) { Log.e(TAG, "Unable to save lock password", e); setKeyguardStoredPasswordQuality(currentQuality, userHandle); @@ -913,7 +859,7 @@ public class LockPatternUtils { * Update device encryption password if calling user is USER_SYSTEM and device supports * encryption. */ - private void updateEncryptionPasswordIfNeeded(byte[] password, int quality, int userHandle) { + private void updateEncryptionPasswordIfNeeded(String password, int quality, int userHandle) { // Update the device encryption password. if (userHandle == UserHandle.USER_SYSTEM && LockPatternUtils.isDeviceEncryptionEnabled()) { @@ -933,8 +879,8 @@ public class LockPatternUtils { * Store the hash of the *current* password in the password history list, if device policy * enforces password history requirement. */ - private void updatePasswordHistory(byte[] password, int userHandle) { - if (password == null || password.length == 0) { + private void updatePasswordHistory(String password, int userHandle) { + if (TextUtils.isEmpty(password)) { Log.e(TAG, "checkPasswordHistory: empty password"); return; } @@ -1013,7 +959,7 @@ public class LockPatternUtils { * if DevicePolicyManager has a stronger quality requirement. This value will be written * to PASSWORD_TYPE_KEY. */ - private int computePasswordQuality(int type, byte[] credential, int requestedQuality) { + private int computePasswordQuality(int type, String credential, int requestedQuality) { final int quality; if (type == CREDENTIAL_TYPE_PASSWORD) { int computedQuality = PasswordMetrics.computeForPassword(credential).quality; @@ -1036,7 +982,7 @@ public class LockPatternUtils { * true */ public void setSeparateProfileChallengeEnabled(int userHandle, boolean enabled, - byte[] managedUserPassword) { + String managedUserPassword) { if (!isManagedProfile(userHandle)) { return; } @@ -1100,28 +1046,15 @@ public class LockPatternUtils { * Deserialize a pattern. * @param string The pattern serialized with {@link #patternToString} * @return The pattern. - * @deprecated Pass patterns as byte[] and use byteArrayToPattern */ - @Deprecated public static List stringToPattern(String string) { if (string == null) { return null; } - return byteArrayToPattern(string.getBytes()); - } - - /** - * Deserialize a pattern. - * @param bytes The pattern serialized with {@link #patternToByteArray} - * @return The pattern. - */ - public static List byteArrayToPattern(byte[] bytes) { - if (bytes == null) { - return null; - } List result = Lists.newArrayList(); + final byte[] bytes = string.getBytes(); for (int i = 0; i < bytes.length; i++) { byte b = (byte) (bytes[i] - '1'); result.add(LockPatternView.Cell.of(b / 3, b % 3)); @@ -1133,22 +1066,10 @@ public class LockPatternUtils { * Serialize a pattern. * @param pattern The pattern. * @return The pattern in string form. - * @deprecated Use patternToByteArray instead. */ - @Deprecated public static String patternToString(List pattern) { - return new String(patternToByteArray(pattern)); - } - - - /** - * Serialize a pattern. - * @param pattern The pattern. - * @return The pattern in byte array form. - */ - public static byte[] patternToByteArray(List pattern) { if (pattern == null) { - return new byte[0]; + return ""; } final int patternSize = pattern.size(); @@ -1157,24 +1078,21 @@ public class LockPatternUtils { LockPatternView.Cell cell = pattern.get(i); res[i] = (byte) (cell.getRow() * 3 + cell.getColumn() + '1'); } - return res; + return new String(res); } - /** - * Transform a pattern byte array to base zero form. - * @param bytes pattern byte array. - * @return The pattern in base zero form. - */ - public static byte[] patternByteArrayToBaseZero(byte[] bytes) { - if (bytes == null) { - return new byte[0]; + public static String patternStringToBaseZero(String pattern) { + if (pattern == null) { + return ""; } - final int patternSize = bytes.length; + final int patternSize = pattern.length(); + byte[] res = new byte[patternSize]; + final byte[] bytes = pattern.getBytes(); for (int i = 0; i < patternSize; i++) { res[i] = (byte) (bytes[i] - '1'); } - return res; + return new String(res); } /* @@ -1228,18 +1146,13 @@ public class LockPatternUtils { * * @return the hash of the pattern in a byte array. */ - public String legacyPasswordToHash(byte[] password, int userId) { - if (password == null || password.length == 0) { + public String legacyPasswordToHash(String password, int userId) { + if (password == null) { return null; } try { - // Previously the password was passed as a String with the following code: - // byte[] saltedPassword = (password + getSalt(userId)).getBytes(); - // The code below creates the identical digest preimage using byte arrays: - byte[] salt = getSalt(userId).getBytes(); - byte[] saltedPassword = Arrays.copyOf(password, password.length + salt.length); - System.arraycopy(salt, 0, saltedPassword, password.length, salt.length); + byte[] saltedPassword = (password + getSalt(userId)).getBytes(); byte[] sha1 = MessageDigest.getInstance("SHA-1").digest(saltedPassword); byte[] md5 = MessageDigest.getInstance("MD5").digest(saltedPassword); @@ -1248,7 +1161,6 @@ public class LockPatternUtils { System.arraycopy(md5, 0, combined, sha1.length, md5.length); final char[] hexEncoded = HexEncoding.encode(combined); - Arrays.fill(saltedPassword, (byte) 0); return new String(hexEncoded); } catch (NoSuchAlgorithmException e) { throw new AssertionError("Missing digest algorithm: ", e); @@ -1258,19 +1170,14 @@ public class LockPatternUtils { /** * Hash the password for password history check purpose. */ - private String passwordToHistoryHash(byte[] passwordToHash, byte[] hashFactor, int userId) { - if (passwordToHash == null || passwordToHash.length == 0 || hashFactor == null) { + private String passwordToHistoryHash(String passwordToHash, byte[] hashFactor, int userId) { + if (TextUtils.isEmpty(passwordToHash) || hashFactor == null) { return null; } try { MessageDigest sha256 = MessageDigest.getInstance("SHA-256"); sha256.update(hashFactor); - byte[] salt = getSalt(userId).getBytes(); - byte[] saltedPassword = Arrays.copyOf(passwordToHash, passwordToHash.length - + salt.length); - System.arraycopy(salt, 0, saltedPassword, passwordToHash.length, salt.length); - sha256.update(saltedPassword); - Arrays.fill(saltedPassword, (byte) 0); + sha256.update((passwordToHash + getSalt(userId)).getBytes()); return new String(HexEncoding.encode(sha256.digest())); } catch (NoSuchAlgorithmException e) { throw new AssertionError("Missing digest algorithm: ", e); @@ -1703,17 +1610,17 @@ public class LockPatternUtils { * @param userId The user who's lock credential to be changed * @return {@code true} if the operation is successful. */ - public boolean setLockCredentialWithToken(byte[] credential, int type, int requestedQuality, + public boolean setLockCredentialWithToken(String credential, int type, int requestedQuality, long tokenHandle, byte[] token, int userId) { LockSettingsInternal localService = getLockSettingsInternal(); if (type != CREDENTIAL_TYPE_NONE) { - if (credential == null || credential.length < MIN_LOCK_PASSWORD_SIZE) { + if (TextUtils.isEmpty(credential) || credential.length() < MIN_LOCK_PASSWORD_SIZE) { throw new IllegalArgumentException("password must not be null and at least " + "of length " + MIN_LOCK_PASSWORD_SIZE); } final int quality = computePasswordQuality(type, credential, requestedQuality); - if (!localService.setLockCredentialWithToken(credential, type, tokenHandle, token, - quality, userId)) { + if (!localService.setLockCredentialWithToken(credential, type, tokenHandle, + token, quality, userId)) { return false; } setKeyguardStoredPasswordQuality(quality, userId); @@ -1722,11 +1629,11 @@ public class LockPatternUtils { updatePasswordHistory(credential, userId); onAfterChangingPassword(userId); } else { - if (!(credential == null || credential.length == 0)) { + if (!TextUtils.isEmpty(credential)) { throw new IllegalArgumentException("password must be emtpy for NONE type"); } - if (!localService.setLockCredentialWithToken(null, CREDENTIAL_TYPE_NONE, tokenHandle, - token, PASSWORD_QUALITY_UNSPECIFIED, userId)) { + if (!localService.setLockCredentialWithToken(null, CREDENTIAL_TYPE_NONE, + tokenHandle, token, PASSWORD_QUALITY_UNSPECIFIED, userId)) { return false; } setKeyguardStoredPasswordQuality(PASSWORD_QUALITY_UNSPECIFIED, userId); @@ -1947,22 +1854,4 @@ public class LockPatternUtils { return FRP_CREDENTIAL_ENABLED && context.getResources().getBoolean( com.android.internal.R.bool.config_enableCredentialFactoryResetProtection); } - - /** - * Converts a CharSequence to a byte array without requiring a toString(), which creates an - * additional copy. - * - * @param chars The CharSequence to convert - * @return A byte array representing the input - */ - public static byte[] charSequenceToByteArray(CharSequence chars) { - if (chars == null) { - return null; - } - byte[] bytes = new byte[chars.length()]; - for (int i = 0; i < chars.length(); i++) { - bytes[i] = (byte) chars.charAt(i); - } - return bytes; - } } diff --git a/core/java/com/android/internal/widget/LockPatternView.java b/core/java/com/android/internal/widget/LockPatternView.java index c0f57b079ccfa..e8fc5989354a6 100644 --- a/core/java/com/android/internal/widget/LockPatternView.java +++ b/core/java/com/android/internal/widget/LockPatternView.java @@ -1274,10 +1274,8 @@ public class LockPatternView extends View { @Override protected Parcelable onSaveInstanceState() { Parcelable superState = super.onSaveInstanceState(); - byte[] patternBytes = LockPatternUtils.patternToByteArray(mPattern); - String patternString = patternBytes != null ? new String(patternBytes) : null; return new SavedState(superState, - patternString, + LockPatternUtils.patternToString(mPattern), mPatternDisplayMode.ordinal(), mInputEnabled, mInStealthMode, mEnableHapticFeedback); } diff --git a/core/java/com/android/internal/widget/LockSettingsInternal.java b/core/java/com/android/internal/widget/LockSettingsInternal.java index 90397dffe5f94..9de9ef7f2aeaf 100644 --- a/core/java/com/android/internal/widget/LockSettingsInternal.java +++ b/core/java/com/android/internal/widget/LockSettingsInternal.java @@ -49,11 +49,7 @@ public abstract class LockSettingsInternal { */ public abstract boolean isEscrowTokenActive(long handle, int userId); - /** - * Set the lock credential. - * @return true if password is set. - */ - public abstract boolean setLockCredentialWithToken(byte[] credential, int type, + public abstract boolean setLockCredentialWithToken(String credential, int type, long tokenHandle, byte[] token, int requestedQuality, int userId); public abstract boolean unlockUserWithToken(long tokenHandle, byte[] token, int userId); diff --git a/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java b/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java index fc080fb1c98f0..d289f1f5defc6 100644 --- a/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java +++ b/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java @@ -80,8 +80,7 @@ public class PasswordMetricsTest { @Test public void testComputeForPassword_metrics() { - final PasswordMetrics metrics = - PasswordMetrics.computeForPassword("6B~0z1Z3*8A".getBytes()); + final PasswordMetrics metrics = PasswordMetrics.computeForPassword("6B~0z1Z3*8A"); assertEquals(11, metrics.length); assertEquals(4, metrics.letters); assertEquals(3, metrics.upperCase); @@ -94,32 +93,32 @@ public class PasswordMetricsTest { @Test public void testComputeForPassword_quality() { assertEquals(DevicePolicyManager.PASSWORD_QUALITY_ALPHANUMERIC, - PasswordMetrics.computeForPassword("a1".getBytes()).quality); + PasswordMetrics.computeForPassword("a1").quality); assertEquals(DevicePolicyManager.PASSWORD_QUALITY_ALPHABETIC, - PasswordMetrics.computeForPassword("a".getBytes()).quality); + PasswordMetrics.computeForPassword("a").quality); assertEquals(DevicePolicyManager.PASSWORD_QUALITY_ALPHABETIC, - PasswordMetrics.computeForPassword("*~&%$".getBytes()).quality); + PasswordMetrics.computeForPassword("*~&%$").quality); assertEquals(DevicePolicyManager.PASSWORD_QUALITY_NUMERIC_COMPLEX, - PasswordMetrics.computeForPassword("1".getBytes()).quality); + PasswordMetrics.computeForPassword("1").quality); // contains a long sequence so isn't complex assertEquals(DevicePolicyManager.PASSWORD_QUALITY_NUMERIC, - PasswordMetrics.computeForPassword("1234".getBytes()).quality); + PasswordMetrics.computeForPassword("1234").quality); assertEquals(DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED, - PasswordMetrics.computeForPassword("".getBytes()).quality); + PasswordMetrics.computeForPassword("").quality); } @Test public void testMaxLengthSequence() { - assertEquals(4, PasswordMetrics.maxLengthSequence("1234".getBytes())); - assertEquals(5, PasswordMetrics.maxLengthSequence("13579".getBytes())); - assertEquals(4, PasswordMetrics.maxLengthSequence("1234abd".getBytes())); - assertEquals(3, PasswordMetrics.maxLengthSequence("aabc".getBytes())); - assertEquals(1, PasswordMetrics.maxLengthSequence("qwertyuio".getBytes())); - assertEquals(3, PasswordMetrics.maxLengthSequence("@ABC".getBytes())); + assertEquals(4, PasswordMetrics.maxLengthSequence("1234")); + assertEquals(5, PasswordMetrics.maxLengthSequence("13579")); + assertEquals(4, PasswordMetrics.maxLengthSequence("1234abd")); + assertEquals(3, PasswordMetrics.maxLengthSequence("aabc")); + assertEquals(1, PasswordMetrics.maxLengthSequence("qwertyuio")); + assertEquals(3, PasswordMetrics.maxLengthSequence("@ABC")); // anything that repeats - assertEquals(4, PasswordMetrics.maxLengthSequence(";;;;".getBytes())); + assertEquals(4, PasswordMetrics.maxLengthSequence(";;;;")); // ordered, but not composed of alphas or digits - assertEquals(1, PasswordMetrics.maxLengthSequence(":;<=>".getBytes())); + assertEquals(1, PasswordMetrics.maxLengthSequence(":;<=>")); } @Test @@ -140,8 +139,8 @@ public class PasswordMetricsTest { assertNotEquals(new PasswordMetrics(DevicePolicyManager.PASSWORD_QUALITY_SOMETHING, 4), new PasswordMetrics(DevicePolicyManager.PASSWORD_QUALITY_COMPLEX, 4)); - metrics0 = PasswordMetrics.computeForPassword("1234abcd,./".getBytes()); - metrics1 = PasswordMetrics.computeForPassword("1234abcd,./".getBytes()); + metrics0 = PasswordMetrics.computeForPassword("1234abcd,./"); + metrics1 = PasswordMetrics.computeForPassword("1234abcd,./"); assertEquals(metrics0, metrics1); metrics1.letters++; assertNotEquals(metrics0, metrics1); @@ -162,5 +161,7 @@ public class PasswordMetricsTest { assertNotEquals(metrics0, metrics1); metrics1.nonLetter--; assertEquals(metrics0, metrics1); + + } } diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputView.java b/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputView.java index d9bfae2657544..48b413456755b 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputView.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputView.java @@ -33,8 +33,6 @@ import com.android.internal.util.LatencyTracker; import com.android.internal.widget.LockPatternChecker; import com.android.internal.widget.LockPatternUtils; -import java.util.Arrays; - /** * Base class for PIN and password unlock screens. */ @@ -124,19 +122,18 @@ public abstract class KeyguardAbsKeyInputView extends LinearLayout protected void verifyPasswordAndUnlock() { if (mDismissing) return; // already verified but haven't been dismissed; don't do it again. - final byte[] entry = getPasswordText(); + final String entry = getPasswordText(); setPasswordEntryInputEnabled(false); if (mPendingLockCheck != null) { mPendingLockCheck.cancel(false); } final int userId = KeyguardUpdateMonitor.getCurrentUser(); - if (entry.length <= MINIMUM_PASSWORD_LENGTH_BEFORE_REPORT) { + if (entry.length() <= MINIMUM_PASSWORD_LENGTH_BEFORE_REPORT) { // to avoid accidental lockout, only count attempts that are long enough to be a // real password. This may require some tweaking. setPasswordEntryInputEnabled(true); onPasswordChecked(userId, false /* matched */, 0, false /* not valid - too short */); - Arrays.fill(entry, (byte) 0); return; } @@ -158,7 +155,6 @@ public abstract class KeyguardAbsKeyInputView extends LinearLayout } onPasswordChecked(userId, true /* matched */, 0 /* timeoutMs */, true /* isValidPassword */); - Arrays.fill(entry, (byte) 0); } @Override @@ -173,7 +169,6 @@ public abstract class KeyguardAbsKeyInputView extends LinearLayout onPasswordChecked(userId, false /* matched */, timeoutMs, true /* isValidPassword */); } - Arrays.fill(entry, (byte) 0); } @Override @@ -184,7 +179,6 @@ public abstract class KeyguardAbsKeyInputView extends LinearLayout LatencyTracker.getInstance(mContext).onActionEnd( ACTION_CHECK_CREDENTIAL_UNLOCKED); } - Arrays.fill(entry, (byte) 0); } }); } @@ -215,7 +209,7 @@ public abstract class KeyguardAbsKeyInputView extends LinearLayout } protected abstract void resetPasswordText(boolean animate, boolean announce); - protected abstract byte[] getPasswordText(); + protected abstract String getPasswordText(); protected abstract void setPasswordEntryEnabled(boolean enabled); protected abstract void setPasswordEntryInputEnabled(boolean enabled); diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardPasswordView.java b/packages/SystemUI/src/com/android/keyguard/KeyguardPasswordView.java index 9a0abc56afdf1..81cf3aef4bc89 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardPasswordView.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardPasswordView.java @@ -231,8 +231,8 @@ public class KeyguardPasswordView extends KeyguardAbsKeyInputView } @Override - protected byte[] getPasswordText() { - return charSequenceToByteArray(mPasswordEntry.getText()); + protected String getPasswordText() { + return mPasswordEntry.getText().toString(); } @Override @@ -366,18 +366,4 @@ public class KeyguardPasswordView extends KeyguardAbsKeyInputView return getContext().getString( com.android.internal.R.string.keyguard_accessibility_password_unlock); } - - /* - * This method avoids creating a new string when getting a byte array from EditView#getText(). - */ - private static byte[] charSequenceToByteArray(CharSequence chars) { - if (chars == null) { - return null; - } - byte[] bytes = new byte[chars.length()]; - for (int i = 0; i < chars.length(); i++) { - bytes[i] = (byte) chars.charAt(i); - } - return bytes; - } } diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardPinBasedInputView.java b/packages/SystemUI/src/com/android/keyguard/KeyguardPinBasedInputView.java index cfd862eda6bdd..cb8c119d08ebc 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardPinBasedInputView.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardPinBasedInputView.java @@ -161,8 +161,8 @@ public abstract class KeyguardPinBasedInputView extends KeyguardAbsKeyInputView } @Override - protected byte[] getPasswordText() { - return charSequenceToByteArray(mPasswordEntry.getText()); + protected String getPasswordText() { + return mPasswordEntry.getText(); } @Override @@ -260,18 +260,4 @@ public abstract class KeyguardPinBasedInputView extends KeyguardAbsKeyInputView return getContext().getString( com.android.internal.R.string.keyguard_accessibility_pin_unlock); } - - /* - * This method avoids creating a new string when getting a byte array from EditView#getText(). - */ - private static byte[] charSequenceToByteArray(CharSequence chars) { - if (chars == null) { - return null; - } - byte[] bytes = new byte[chars.length()]; - for (int i = 0; i < chars.length(); i++) { - bytes[i] = (byte) chars.charAt(i); - } - return bytes; - } } diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 984d9bd89f5b8..c6a871217ebd6 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -120,6 +120,7 @@ import java.io.FileDescriptor; import java.io.FileNotFoundException; import java.io.IOException; import java.io.PrintWriter; +import java.nio.charset.StandardCharsets; import java.security.InvalidAlgorithmParameterException; import java.security.InvalidKeyException; import java.security.KeyStoreException; @@ -271,7 +272,7 @@ public class LockSettingsService extends ILockSettings.Stub { * @param managedUserPassword Managed profile original password (when it has separated lock). * NULL when it does not have a separated lock before. */ - public void tieManagedProfileLockIfNecessary(int managedUserId, byte[] managedUserPassword) { + public void tieManagedProfileLockIfNecessary(int managedUserId, String managedUserPassword) { if (DEBUG) Slog.v(TAG, "Check child profile lock for user: " + managedUserId); // Only for managed profile if (!mUserManager.getUserInfo(managedUserId).isManagedProfile()) { @@ -306,12 +307,7 @@ public class LockSettingsService extends ILockSettings.Stub { byte[] randomLockSeed = new byte[] {}; try { randomLockSeed = SecureRandom.getInstance("SHA1PRNG").generateSeed(40); - char[] newPasswordChars = HexEncoding.encode(randomLockSeed); - byte[] newPassword = new byte[newPasswordChars.length]; - for (int i = 0; i < newPasswordChars.length; i++) { - newPassword[i] = (byte) newPasswordChars[i]; - } - Arrays.fill(newPasswordChars, '\u0000'); + String newPassword = String.valueOf(HexEncoding.encode(randomLockSeed)); final int quality = DevicePolicyManager.PASSWORD_QUALITY_ALPHANUMERIC; setLockCredentialInternal(newPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, managedUserPassword, quality, managedUserId); @@ -320,7 +316,6 @@ public class LockSettingsService extends ILockSettings.Stub { // password directly, so we always store a password. setLong(LockPatternUtils.PASSWORD_TYPE_KEY, quality, managedUserId); tieProfileLockToParent(managedUserId, newPassword); - Arrays.fill(newPassword, (byte) 0); } catch (NoSuchAlgorithmException | RemoteException e) { Slog.e(TAG, "Fail to tie managed profile", e); // Nothing client can do to fix this issue, so we do not throw exception out @@ -610,7 +605,7 @@ public class LockSettingsService extends ILockSettings.Stub { try { final long handle = getSyntheticPasswordHandleLocked(userId); - final byte[] noCredential = null; + final String noCredential = null; AuthenticationResult result = mSpManager.unwrapPasswordBasedSyntheticPassword( getGateKeeperService(), handle, noCredential, userId, null); @@ -949,7 +944,7 @@ public class LockSettingsService extends ILockSettings.Stub { @Override public void setSeparateProfileChallengeEnabled(int userId, boolean enabled, - byte[] managedUserPassword) { + String managedUserPassword) { checkWritePermission(userId); synchronized (mSeparateChallengeLock) { setSeparateProfileChallengeEnabledLocked(userId, enabled, managedUserPassword); @@ -958,8 +953,8 @@ public class LockSettingsService extends ILockSettings.Stub { } @GuardedBy("mSeparateChallengeLock") - private void setSeparateProfileChallengeEnabledLocked(@UserIdInt int userId, - boolean enabled, byte[] managedUserPassword) { + private void setSeparateProfileChallengeEnabledLocked(@UserIdInt int userId, boolean enabled, + String managedUserPassword) { final boolean old = getBoolean(SEPARATE_PROFILE_CHALLENGE_KEY, false, userId); setBoolean(SEPARATE_PROFILE_CHALLENGE_KEY, enabled, userId); try { @@ -1102,28 +1097,24 @@ public class LockSettingsService extends ILockSettings.Stub { return mStorage.hasCredential(userId); } - private void setKeystorePassword(byte[] password, int userHandle) { + private void setKeystorePassword(String password, int userHandle) { final KeyStore ks = KeyStore.getInstance(); - // TODO(b/120484642): Update keystore to accept byte[] passwords - String passwordString = password == null ? null : new String(password); - ks.onUserPasswordChanged(userHandle, passwordString); + ks.onUserPasswordChanged(userHandle, password); } - private void unlockKeystore(byte[] password, int userHandle) { + private void unlockKeystore(String password, int userHandle) { if (DEBUG) Slog.v(TAG, "Unlock keystore for user: " + userHandle); - // TODO(b/120484642): Update keystore to accept byte[] passwords - String passwordString = password == null ? null : new String(password); final KeyStore ks = KeyStore.getInstance(); - ks.unlock(userHandle, passwordString); + ks.unlock(userHandle, password); } @VisibleForTesting - protected byte[] getDecryptedPasswordForTiedProfile(int userId) + protected String getDecryptedPasswordForTiedProfile(int userId) throws KeyStoreException, UnrecoverableKeyException, NoSuchAlgorithmException, NoSuchPaddingException, InvalidKeyException, InvalidAlgorithmParameterException, IllegalBlockSizeException, BadPaddingException, CertificateException, IOException { - if (DEBUG) Slog.v(TAG, "Get child profile decrypted key"); + if (DEBUG) Slog.v(TAG, "Get child profile decrytped key"); byte[] storedData = mStorage.readChildProfileLock(userId); if (storedData == null) { throw new FileNotFoundException("Child profile lock file not found"); @@ -1142,7 +1133,7 @@ public class LockSettingsService extends ILockSettings.Stub { cipher.init(Cipher.DECRYPT_MODE, decryptionKey, new GCMParameterSpec(128, iv)); decryptionResult = cipher.doFinal(encryptedPassword); - return decryptionResult; + return new String(decryptionResult, StandardCharsets.UTF_8); } private void unlockChildProfile(int profileHandle, boolean ignoreUserNotAuthenticated) @@ -1219,11 +1210,11 @@ public class LockSettingsService extends ILockSettings.Stub { && mUserManager.isUserRunning(userInfo.id); } - private Map getDecryptedPasswordsForAllTiedProfiles(int userId) { + private Map getDecryptedPasswordsForAllTiedProfiles(int userId) { if (mUserManager.getUserInfo(userId).isManagedProfile()) { return null; } - Map result = new ArrayMap(); + Map result = new ArrayMap(); final List profiles = mUserManager.getProfiles(userId); final int size = profiles.size(); for (int i = 0; i < size; i++) { @@ -1261,7 +1252,7 @@ public class LockSettingsService extends ILockSettings.Stub { * terminates when the user is a managed profile. */ private void synchronizeUnifiedWorkChallengeForProfiles(int userId, - Map profilePasswordMap) throws RemoteException { + Map profilePasswordMap) throws RemoteException { if (mUserManager.getUserInfo(userId).isManagedProfile()) { return; } @@ -1310,8 +1301,8 @@ public class LockSettingsService extends ILockSettings.Stub { // This method should be called by LockPatternUtil only, all internal methods in this class // should call setLockCredentialInternal. @Override - public void setLockCredential(byte[] credential, int type, - byte[] savedCredential, int requestedQuality, int userId) + public void setLockCredential(String credential, int type, String savedCredential, + int requestedQuality, int userId) throws RemoteException { checkWritePermission(userId); synchronized (mSeparateChallengeLock) { @@ -1322,14 +1313,14 @@ public class LockSettingsService extends ILockSettings.Stub { notifySeparateProfileChallengeChanged(userId); } - private void setLockCredentialInternal(byte[] credential, int credentialType, - byte[] savedCredential, int requestedQuality, int userId) throws RemoteException { + private void setLockCredentialInternal(String credential, int credentialType, + String savedCredential, int requestedQuality, int userId) throws RemoteException { // Normalize savedCredential and credential such that empty string is always represented // as null. - if (savedCredential == null || savedCredential.length == 0) { + if (TextUtils.isEmpty(savedCredential)) { savedCredential = null; } - if (credential == null || credential.length == 0) { + if (TextUtils.isEmpty(credential)) { credential = null; } synchronized (mSpManager) { @@ -1398,7 +1389,7 @@ public class LockSettingsService extends ILockSettings.Stub { mStorage.writeCredentialHash(willStore, userId); // push new secret and auth token to vold GateKeeperResponse gkResponse = getGateKeeperService() - .verifyChallenge(userId, 0, willStore.hash, credential); + .verifyChallenge(userId, 0, willStore.hash, credential.getBytes()); setUserKeyProtection(userId, credential, convertResponse(gkResponse)); fixateNewestUserKeyAuth(userId); // Refresh the auth token @@ -1418,8 +1409,9 @@ public class LockSettingsService extends ILockSettings.Stub { } @VisibleForTesting - protected void tieProfileLockToParent(int userId, byte[] password) { + protected void tieProfileLockToParent(int userId, String password) { if (DEBUG) Slog.v(TAG, "tieProfileLockToParent for user: " + userId); + byte[] randomLockSeed = password.getBytes(StandardCharsets.UTF_8); byte[] encryptionResult; byte[] iv; try { @@ -1453,7 +1445,7 @@ public class LockSettingsService extends ILockSettings.Stub { KeyProperties.KEY_ALGORITHM_AES + "/" + KeyProperties.BLOCK_MODE_GCM + "/" + KeyProperties.ENCRYPTION_PADDING_NONE); cipher.init(Cipher.ENCRYPT_MODE, keyStoreEncryptionKey); - encryptionResult = cipher.doFinal(password); + encryptionResult = cipher.doFinal(randomLockSeed); iv = cipher.getIV(); } finally { // The original key can now be discarded. @@ -1478,11 +1470,17 @@ public class LockSettingsService extends ILockSettings.Stub { } private byte[] enrollCredential(byte[] enrolledHandle, - byte[] enrolledCredential, byte[] toEnroll, int userId) + String enrolledCredential, String toEnroll, int userId) throws RemoteException { checkWritePermission(userId); + byte[] enrolledCredentialBytes = enrolledCredential == null + ? null + : enrolledCredential.getBytes(); + byte[] toEnrollBytes = toEnroll == null + ? null + : toEnroll.getBytes(); GateKeeperResponse response = getGateKeeperService().enroll(userId, enrolledHandle, - enrolledCredential, toEnroll); + enrolledCredentialBytes, toEnrollBytes); if (response == null) { return null; @@ -1503,7 +1501,7 @@ public class LockSettingsService extends ILockSettings.Stub { addUserKeyAuth(userId, null, key); } - private void setUserKeyProtection(int userId, byte[] credential, VerifyCredentialResponse vcr) + private void setUserKeyProtection(int userId, String credential, VerifyCredentialResponse vcr) throws RemoteException { if (DEBUG) Slog.d(TAG, "setUserKeyProtection: user=" + userId); if (vcr == null) { @@ -1525,15 +1523,16 @@ public class LockSettingsService extends ILockSettings.Stub { addUserKeyAuth(userId, null, null); } - private static byte[] secretFromCredential(byte[] credential) throws RemoteException { + 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(); + 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); + digest.update(credential.getBytes(StandardCharsets.UTF_8)); return digest.digest(); } catch (NoSuchAlgorithmException e) { throw new RuntimeException("NoSuchAlgorithmException for SHA-512"); @@ -1569,7 +1568,7 @@ public class LockSettingsService extends ILockSettings.Stub { checkWritePermission(userId); if (DEBUG) Slog.v(TAG, "Reset keystore for user: " + userId); int managedUserId = -1; - byte[] managedUserDecryptedPassword = null; + String managedUserDecryptedPassword = null; final List profiles = mUserManager.getProfiles(userId); for (UserInfo pi : profiles) { // Unlock managed profile with unified lock @@ -1606,20 +1605,17 @@ public class LockSettingsService extends ILockSettings.Stub { tieProfileLockToParent(managedUserId, managedUserDecryptedPassword); } } - if (managedUserDecryptedPassword != null && managedUserDecryptedPassword.length > 0) { - Arrays.fill(managedUserDecryptedPassword, (byte) 0); - } } @Override - public VerifyCredentialResponse checkCredential(byte[] credential, int type, int userId, + public VerifyCredentialResponse checkCredential(String credential, int type, int userId, ICheckCredentialProgressCallback progressCallback) throws RemoteException { checkPasswordReadPermission(userId); return doVerifyCredential(credential, type, false, 0, userId, progressCallback); } @Override - public VerifyCredentialResponse verifyCredential(byte[] credential, int type, long challenge, + public VerifyCredentialResponse verifyCredential(String credential, int type, long challenge, int userId) throws RemoteException { checkPasswordReadPermission(userId); return doVerifyCredential(credential, type, true, challenge, userId, @@ -1630,10 +1626,10 @@ public class LockSettingsService extends ILockSettings.Stub { * Verify user credential and unlock the user. Fix pattern bug by deprecating the old base zero * format. */ - private VerifyCredentialResponse doVerifyCredential(byte[] credential, int credentialType, + private VerifyCredentialResponse doVerifyCredential(String credential, int credentialType, boolean hasChallenge, long challenge, int userId, ICheckCredentialProgressCallback progressCallback) throws RemoteException { - if (credential == null || credential.length == 0) { + if (TextUtils.isEmpty(credential)) { throw new IllegalArgumentException("Credential can't be null or empty"); } if (userId == USER_FRP && Settings.Global.getInt(mContext.getContentResolver(), @@ -1668,9 +1664,9 @@ public class LockSettingsService extends ILockSettings.Stub { boolean shouldReEnrollBaseZero = storedHash.type == LockPatternUtils.CREDENTIAL_TYPE_PATTERN && storedHash.isBaseZeroPattern; - byte[] credentialToVerify; + String credentialToVerify; if (shouldReEnrollBaseZero) { - credentialToVerify = LockPatternUtils.patternByteArrayToBaseZero(credential); + credentialToVerify = LockPatternUtils.patternStringToBaseZero(credential); } else { credentialToVerify = credential; } @@ -1690,7 +1686,7 @@ public class LockSettingsService extends ILockSettings.Stub { } @Override - public VerifyCredentialResponse verifyTiedProfileChallenge(byte[] credential, int type, + public VerifyCredentialResponse verifyTiedProfileChallenge(String credential, int type, long challenge, int userId) throws RemoteException { checkPasswordReadPermission(userId); if (!isManagedProfileWithUnifiedLock(userId)) { @@ -1732,15 +1728,14 @@ public class LockSettingsService extends ILockSettings.Stub { * hash to GK. */ private VerifyCredentialResponse verifyCredential(int userId, CredentialHash storedHash, - byte[] credential, boolean hasChallenge, long challenge, + String credential, boolean hasChallenge, long challenge, ICheckCredentialProgressCallback progressCallback) throws RemoteException { - if ((storedHash == null || storedHash.hash.length == 0) - && (credential == null || credential.length == 0)) { + if ((storedHash == null || storedHash.hash.length == 0) && TextUtils.isEmpty(credential)) { // don't need to pass empty credentials to GateKeeper return VerifyCredentialResponse.OK; } - if (storedHash == null || credential == null || credential.length == 0) { + if (storedHash == null || TextUtils.isEmpty(credential)) { return VerifyCredentialResponse.ERROR; } @@ -1751,14 +1746,14 @@ public class LockSettingsService extends ILockSettings.Stub { if (storedHash.version == CredentialHash.VERSION_LEGACY) { final byte[] hash; if (storedHash.type == LockPatternUtils.CREDENTIAL_TYPE_PATTERN) { - hash = LockPatternUtils.patternToHash( - LockPatternUtils.byteArrayToPattern(credential)); + hash = LockPatternUtils.patternToHash(LockPatternUtils.stringToPattern(credential)); } else { - hash = mLockPatternUtils.legacyPasswordToHash(credential, userId).getBytes(); + hash = mLockPatternUtils.legacyPasswordToHash(credential, userId) + .getBytes(StandardCharsets.UTF_8); } if (Arrays.equals(hash, storedHash.hash)) { if (storedHash.type == LockPatternUtils.CREDENTIAL_TYPE_PATTERN) { - unlockKeystore(LockPatternUtils.patternByteArrayToBaseZero(credential), userId); + unlockKeystore(LockPatternUtils.patternStringToBaseZero(credential), userId); } else { unlockKeystore(credential, userId); } @@ -1789,7 +1784,7 @@ public class LockSettingsService extends ILockSettings.Stub { } } GateKeeperResponse gateKeeperResponse = getGateKeeperService() - .verifyChallenge(userId, challenge, storedHash.hash, credential); + .verifyChallenge(userId, challenge, storedHash.hash, credential.getBytes()); VerifyCredentialResponse response = convertResponse(gateKeeperResponse); boolean shouldReEnroll = gateKeeperResponse.getShouldReEnroll(); @@ -1848,7 +1843,7 @@ public class LockSettingsService extends ILockSettings.Stub { * Call this method to notify DPMS regarding the latest password metric. This should be called * when the user is authenticating or when a new password is being set. */ - private void notifyActivePasswordMetricsAvailable(byte[] password, @UserIdInt int userId) { + private void notifyActivePasswordMetricsAvailable(String password, @UserIdInt int userId) { final PasswordMetrics metrics; if (password == null) { metrics = new PasswordMetrics(); @@ -1896,7 +1891,6 @@ public class LockSettingsService extends ILockSettings.Stub { // service can't connect to vold, it restarts, and then the new instance // does successfully connect. final IStorageManager service = mInjector.getStorageManager(); - // TODO(b/120484642): Update vold to return a password as a byte array String password; long identity = Binder.clearCallingIdentity(); try { @@ -1911,8 +1905,8 @@ public class LockSettingsService extends ILockSettings.Stub { try { if (mLockPatternUtils.isLockPatternEnabled(userId)) { - if (checkCredential(password.getBytes(), LockPatternUtils.CREDENTIAL_TYPE_PATTERN, - userId, null /* progressCallback */) + if (checkCredential(password, LockPatternUtils.CREDENTIAL_TYPE_PATTERN, userId, + null /* progressCallback */) .getResponseCode() == GateKeeperResponse.RESPONSE_OK) { return true; } @@ -1922,8 +1916,8 @@ public class LockSettingsService extends ILockSettings.Stub { try { if (mLockPatternUtils.isLockPasswordEnabled(userId)) { - if (checkCredential(password.getBytes(), LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, - userId, null /* progressCallback */) + if (checkCredential(password, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, userId, + null /* progressCallback */) .getResponseCode() == GateKeeperResponse.RESPONSE_OK) { return true; } @@ -2289,7 +2283,7 @@ public class LockSettingsService extends ILockSettings.Stub { @GuardedBy("mSpManager") @VisibleForTesting protected AuthenticationToken initializeSyntheticPasswordLocked(byte[] credentialHash, - byte[] credential, int credentialType, int requestedQuality, + String credential, int credentialType, int requestedQuality, int userId) throws RemoteException { Slog.i(TAG, "Initialize SyntheticPassword for user: " + userId); final AuthenticationToken auth = mSpManager.newSyntheticPasswordAndSid( @@ -2350,7 +2344,7 @@ public class LockSettingsService extends ILockSettings.Stub { setLong(SYNTHETIC_PASSWORD_ENABLED_KEY, 1, UserHandle.USER_SYSTEM); } - private VerifyCredentialResponse spBasedDoVerifyCredential(byte[] userCredential, int + private VerifyCredentialResponse spBasedDoVerifyCredential(String userCredential, int credentialType, boolean hasChallenge, long challenge, int userId, ICheckCredentialProgressCallback progressCallback) throws RemoteException { if (DEBUG) Slog.d(TAG, "spBasedDoVerifyCredential: user=" + userId); @@ -2430,12 +2424,12 @@ public class LockSettingsService extends ILockSettings.Stub { * added back when new password is set in future. */ @GuardedBy("mSpManager") - private long setLockCredentialWithAuthTokenLocked(byte[] credential, int credentialType, + private long setLockCredentialWithAuthTokenLocked(String credential, int credentialType, AuthenticationToken auth, int requestedQuality, int userId) throws RemoteException { if (DEBUG) Slog.d(TAG, "setLockCredentialWithAuthTokenLocked: user=" + userId); long newHandle = mSpManager.createPasswordBasedSyntheticPassword(getGateKeeperService(), credential, credentialType, auth, requestedQuality, userId); - final Map profilePasswords; + final Map profilePasswords; if (credential != null) { // // not needed by synchronizeUnifiedWorkChallengeForProfiles() profilePasswords = null; @@ -2474,19 +2468,12 @@ public class LockSettingsService extends ILockSettings.Stub { synchronizeUnifiedWorkChallengeForProfiles(userId, profilePasswords); notifyActivePasswordMetricsAvailable(credential, userId); - - if (profilePasswords != null) { - for (Map.Entry entry : profilePasswords.entrySet()) { - Arrays.fill(entry.getValue(), (byte) 0); - } - } - return newHandle; } @GuardedBy("mSpManager") - private void spBasedSetLockCredentialInternalLocked(byte[] credential, int credentialType, - byte[] savedCredential, int requestedQuality, int userId) throws RemoteException { + private void spBasedSetLockCredentialInternalLocked(String credential, int credentialType, + String savedCredential, int requestedQuality, int userId) throws RemoteException { if (DEBUG) Slog.d(TAG, "spBasedSetLockCredentialInternalLocked: user=" + userId); if (isManagedProfileWithUnifiedLock(userId)) { // get credential from keystore when managed profile has unified lock @@ -2559,9 +2546,9 @@ public class LockSettingsService extends ILockSettings.Stub { * If user is a managed profile with unified challenge, currentCredential is ignored. */ @Override - public byte[] getHashFactor(byte[] currentCredential, int userId) throws RemoteException { + public byte[] getHashFactor(String currentCredential, int userId) throws RemoteException { checkPasswordReadPermission(userId); - if (currentCredential == null || currentCredential.length == 0) { + if (TextUtils.isEmpty(currentCredential)) { currentCredential = null; } if (isManagedProfileWithUnifiedLock(userId)) { @@ -2655,7 +2642,7 @@ public class LockSettingsService extends ILockSettings.Stub { } } - private boolean setLockCredentialWithToken(byte[] credential, int type, long tokenHandle, + private boolean setLockCredentialWithToken(String credential, int type, long tokenHandle, byte[] token, int requestedQuality, int userId) throws RemoteException { boolean result; synchronized (mSpManager) { @@ -2675,7 +2662,7 @@ public class LockSettingsService extends ILockSettings.Stub { return result; } - private boolean setLockCredentialWithTokenInternal(byte[] credential, int type, + private boolean setLockCredentialWithTokenInternal(String credential, int type, long tokenHandle, byte[] token, int requestedQuality, int userId) throws RemoteException { final AuthenticationResult result; synchronized (mSpManager) { @@ -2902,8 +2889,8 @@ public class LockSettingsService extends ILockSettings.Stub { } @Override - public boolean setLockCredentialWithToken(byte[] credential, int type, - long tokenHandle, byte[] token, int requestedQuality, int userId) { + public boolean setLockCredentialWithToken(String credential, int type, long tokenHandle, + byte[] token, int requestedQuality, int userId) { try { return LockSettingsService.this.setLockCredentialWithToken(credential, type, tokenHandle, token, requestedQuality, userId); diff --git a/services/core/java/com/android/server/locksettings/LockSettingsShellCommand.java b/services/core/java/com/android/server/locksettings/LockSettingsShellCommand.java index ecf6292810f47..4d2cf321b5ee7 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsShellCommand.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsShellCommand.java @@ -134,31 +134,23 @@ class LockSettingsShellCommand extends ShellCommand { mLockPatternUtils.isSyntheticPasswordEnabled())); } - private void runSetPattern() { - byte[] oldBytes = mOld != null ? mOld.getBytes() : null; - mLockPatternUtils.saveLockPattern(stringToPattern(mNew), oldBytes, mCurrentUserId); + private void runSetPattern() throws RemoteException { + mLockPatternUtils.saveLockPattern(stringToPattern(mNew), mOld, mCurrentUserId); getOutPrintWriter().println("Pattern set to '" + mNew + "'"); } - private void runSetPassword() { - byte[] newBytes = mNew != null ? mNew.getBytes() : null; - byte[] oldBytes = mOld != null ? mOld.getBytes() : null; - mLockPatternUtils.saveLockPassword(newBytes, oldBytes, PASSWORD_QUALITY_ALPHABETIC, - mCurrentUserId); + private void runSetPassword() throws RemoteException { + mLockPatternUtils.saveLockPassword(mNew, mOld, PASSWORD_QUALITY_ALPHABETIC, mCurrentUserId); getOutPrintWriter().println("Password set to '" + mNew + "'"); } - private void runSetPin() { - byte[] newBytes = mNew != null ? mNew.getBytes() : null; - byte[] oldBytes = mOld != null ? mOld.getBytes() : null; - mLockPatternUtils.saveLockPassword(newBytes, oldBytes, PASSWORD_QUALITY_NUMERIC, - mCurrentUserId); + private void runSetPin() throws RemoteException { + mLockPatternUtils.saveLockPassword(mNew, mOld, PASSWORD_QUALITY_NUMERIC, mCurrentUserId); getOutPrintWriter().println("Pin set to '" + mNew + "'"); } - private void runClear() { - byte[] oldBytes = mOld != null ? mOld.getBytes() : null; - mLockPatternUtils.clearLock(oldBytes, mCurrentUserId); + private void runClear() throws RemoteException { + mLockPatternUtils.clearLock(mOld, mCurrentUserId); getOutPrintWriter().println("Lock credential cleared"); } @@ -185,8 +177,7 @@ class LockSettingsShellCommand extends ShellCommand { try { final boolean result; if (havePassword) { - byte[] passwordBytes = mOld != null ? mOld.getBytes() : null; - result = mLockPatternUtils.checkPassword(passwordBytes, mCurrentUserId); + result = mLockPatternUtils.checkPassword(mOld, mCurrentUserId); } else { result = mLockPatternUtils.checkPattern(stringToPattern(mOld), mCurrentUserId); } diff --git a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java index 163cca6a125e2..d32c299074a96 100644 --- a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java +++ b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java @@ -96,7 +96,7 @@ public class SyntheticPasswordManager { private static final String WEAVER_SLOT_NAME = "weaver"; public static final long DEFAULT_HANDLE = 0L; - private static final byte[] DEFAULT_PASSWORD = "default-password".getBytes(); + private static final String DEFAULT_PASSWORD = "default-password"; private static final byte WEAVER_VERSION = 1; private static final int INVALID_WEAVER_SLOT = -1; @@ -164,7 +164,7 @@ public class SyntheticPasswordManager { } } - public byte[] deriveKeyStorePassword() { + public String deriveKeyStorePassword() { return bytesToHex(derivePassword(PERSONALIZATION_KEY_STORE_PASSWORD)); } @@ -453,11 +453,11 @@ public class SyntheticPasswordManager { * */ public AuthenticationToken newSyntheticPasswordAndSid(IGateKeeperService gatekeeper, - byte[] hash, byte[] credential, int userId) throws RemoteException { + byte[] hash, String credential, int userId) throws RemoteException { AuthenticationToken result = AuthenticationToken.create(); GateKeeperResponse response; if (hash != null) { - response = gatekeeper.enroll(userId, hash, credential, + response = gatekeeper.enroll(userId, hash, credential.getBytes(), result.deriveGkPassword()); if (response.getResponseCode() != GateKeeperResponse.RESPONSE_OK) { Log.w(TAG, "Fail to migrate SID, assuming no SID, user " + userId); @@ -615,7 +615,7 @@ public class SyntheticPasswordManager { * @see #clearSidForUser */ public long createPasswordBasedSyntheticPassword(IGateKeeperService gatekeeper, - byte[] credential, int credentialType, AuthenticationToken authToken, + String credential, int credentialType, AuthenticationToken authToken, int requestedQuality, int userId) throws RemoteException { if (credential == null || credentialType == LockPatternUtils.CREDENTIAL_TYPE_NONE) { @@ -669,7 +669,7 @@ public class SyntheticPasswordManager { } public VerifyCredentialResponse verifyFrpCredential(IGateKeeperService gatekeeper, - byte[] userCredential, int credentialType, + String userCredential, int credentialType, ICheckCredentialProgressCallback progressCallback) throws RemoteException { PersistentData persistentData = mStorage.readPersistentDataBlock(); if (persistentData.type == PersistentData.TYPE_SP) { @@ -838,7 +838,7 @@ public class SyntheticPasswordManager { * unknown. Caller might choose to validate it by examining AuthenticationResult.credentialType */ public AuthenticationResult unwrapPasswordBasedSyntheticPassword(IGateKeeperService gatekeeper, - long handle, byte[] credential, int userId, + long handle, String credential, int userId, ICheckCredentialProgressCallback progressCallback) throws RemoteException { if (credential == null) { credential = DEFAULT_PASSWORD; @@ -1151,7 +1151,7 @@ public class SyntheticPasswordManager { return String.format("%s%x", LockPatternUtils.SYNTHETIC_PASSWORD_KEY_PREFIX, handle); } - private byte[] computePasswordToken(byte[] password, PasswordData data) { + private byte[] computePasswordToken(String password, PasswordData data) { return scrypt(password, data.salt, 1 << data.scryptN, 1 << data.scryptR, 1 << data.scryptP, PASSWORD_TOKEN_LENGTH); } @@ -1172,8 +1172,8 @@ public class SyntheticPasswordManager { return nativeSidFromPasswordHandle(handle); } - protected byte[] scrypt(byte[] password, byte[] salt, int N, int r, int p, int outLen) { - return nativeScrypt(password, salt, N, r, p, outLen); + protected byte[] scrypt(String password, byte[] salt, int N, int r, int p, int outLen) { + return nativeScrypt(password.getBytes(), salt, N, r, p, outLen); } native long nativeSidFromPasswordHandle(byte[] handle); @@ -1195,17 +1195,17 @@ public class SyntheticPasswordManager { return result; } - protected static final byte[] HEX_ARRAY = "0123456789ABCDEF".getBytes(); - private static byte[] bytesToHex(byte[] bytes) { + final protected static char[] hexArray = "0123456789ABCDEF".toCharArray(); + public static String bytesToHex(byte[] bytes) { if (bytes == null) { - return "null".getBytes(); + return "null"; } - byte[] hexBytes = new byte[bytes.length * 2]; + char[] hexChars = new char[bytes.length * 2]; for ( int j = 0; j < bytes.length; j++ ) { int v = bytes[j] & 0xFF; - hexBytes[j * 2] = HEX_ARRAY[v >>> 4]; - hexBytes[j * 2 + 1] = HEX_ARRAY[v & 0x0F]; + hexChars[j * 2] = hexArray[v >>> 4]; + hexChars[j * 2 + 1] = hexArray[v & 0x0F]; } - return hexBytes; + return new String(hexChars); } } 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 bc6c2d24e66d4..f1951b1e31b14 100644 --- a/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java +++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java @@ -36,6 +36,7 @@ import com.android.server.locksettings.recoverablekeystore.storage.RecoverySnaps import java.io.IOException; import java.nio.ByteBuffer; import java.nio.ByteOrder; +import java.nio.charset.StandardCharsets; import java.security.GeneralSecurityException; import java.security.InvalidAlgorithmParameterException; import java.security.InvalidKeyException; @@ -48,7 +49,6 @@ import java.security.UnrecoverableKeyException; import java.security.cert.CertPath; import java.security.cert.CertificateException; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Map; @@ -83,7 +83,7 @@ public class KeySyncTask implements Runnable { private final RecoverableKeyStoreDb mRecoverableKeyStoreDb; private final int mUserId; private final int mCredentialType; - private final byte[] mCredential; + private final String mCredential; private final boolean mCredentialUpdated; private final PlatformKeyManager mPlatformKeyManager; private final RecoverySnapshotStorage mRecoverySnapshotStorage; @@ -98,7 +98,7 @@ public class KeySyncTask implements Runnable { RecoverySnapshotListenersStorage recoverySnapshotListenersStorage, int userId, int credentialType, - byte[] credential, + String credential, boolean credentialUpdated ) throws NoSuchAlgorithmException, KeyStoreException, InsecureUserException { return new KeySyncTask( @@ -132,7 +132,7 @@ public class KeySyncTask implements Runnable { RecoverySnapshotListenersStorage recoverySnapshotListenersStorage, int userId, int credentialType, - byte[] credential, + String credential, boolean credentialUpdated, PlatformKeyManager platformKeyManager, TestOnlyInsecureCertificateHelper testOnlyInsecureCertificateHelper, @@ -445,7 +445,7 @@ public class KeySyncTask implements Runnable { */ @VisibleForTesting @KeyChainProtectionParams.LockScreenUiFormat static int getUiFormat( - int credentialType, byte[] credential) { + int credentialType, String credential) { if (credentialType == LockPatternUtils.CREDENTIAL_TYPE_PATTERN) { return KeyChainProtectionParams.UI_FORMAT_PATTERN; } else if (isPin(credential)) { @@ -470,13 +470,13 @@ public class KeySyncTask implements Runnable { * Returns {@code true} if {@code credential} looks like a pin. */ @VisibleForTesting - static boolean isPin(@Nullable byte[] credential) { + static boolean isPin(@Nullable String credential) { if (credential == null) { return false; } - int length = credential.length; + int length = credential.length(); for (int i = 0; i < length; i++) { - if (!Character.isDigit((char) credential[i])) { + if (!Character.isDigit(credential.charAt(i))) { return false; } } @@ -489,7 +489,8 @@ public class KeySyncTask implements Runnable { * @return The SHA-256 hash. */ @VisibleForTesting - static byte[] hashCredentialsBySaltedSha256(byte[] salt, byte[] credentialsBytes) { + static byte[] hashCredentialsBySaltedSha256(byte[] salt, String credentials) { + byte[] credentialsBytes = credentials.getBytes(StandardCharsets.UTF_8); ByteBuffer byteBuffer = ByteBuffer.allocate( salt.length + credentialsBytes.length + LENGTH_PREFIX_BYTES * 2); byteBuffer.order(ByteOrder.LITTLE_ENDIAN); @@ -500,19 +501,17 @@ public class KeySyncTask implements Runnable { byte[] bytes = byteBuffer.array(); try { - byte[] hash = MessageDigest.getInstance(LOCK_SCREEN_HASH_ALGORITHM).digest(bytes); - Arrays.fill(bytes, (byte) 0); - return hash; + return MessageDigest.getInstance(LOCK_SCREEN_HASH_ALGORITHM).digest(bytes); } catch (NoSuchAlgorithmException e) { // Impossible, SHA-256 must be supported on Android. throw new RuntimeException(e); } } - private byte[] hashCredentialsByScrypt(byte[] salt, byte[] credentials) { + private byte[] hashCredentialsByScrypt(byte[] salt, String credentials) { return mScrypt.scrypt( - credentials, salt, SCRYPT_PARAM_N, SCRYPT_PARAM_R, SCRYPT_PARAM_P, - SCRYPT_PARAM_OUTLEN_BYTES); + credentials.getBytes(StandardCharsets.UTF_8), salt, + SCRYPT_PARAM_N, SCRYPT_PARAM_R, SCRYPT_PARAM_P, SCRYPT_PARAM_OUTLEN_BYTES); } private static SecretKey generateRecoveryKey() throws NoSuchAlgorithmException { 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 256a83f2128b4..fc5184d1438f9 100644 --- a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java +++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java @@ -851,13 +851,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 credential - unencrypted String. Password length should be at most 16 symbols {@code + * mPasswordMaxLength} * @param userId for user who just unlocked the device. * @hide */ public void lockScreenSecretAvailable( - int storedHashType, @NonNull byte[] credential, int userId) { + int storedHashType, @NonNull String credential, int userId) { // So as not to block the critical path unlocking the phone, defer to another thread. try { mExecutorService.execute(KeySyncTask.newInstance( @@ -882,13 +882,13 @@ public class RecoverableKeyStoreManager { * This function can only be used inside LockSettingsService. * * @param storedHashType from {@code CredentialHash} - * @param credential - unencrypted byte array + * @param credential - unencrypted String * @param userId for the user whose lock screen credentials were changed. * @hide */ public void lockScreenSecretChanged( int storedHashType, - @Nullable byte[] credential, + @Nullable String credential, int userId) { // So as not to block the critical path unlocking the phone, defer to another thread. try { diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelper.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelper.java index 1de5dc5bbc9a5..5ba3cceec6d0d 100644 --- a/services/core/java/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelper.java +++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelper.java @@ -84,30 +84,10 @@ public class TestOnlyInsecureCertificateHelper { || isTestOnlyCertificateAlias(rootCertificateAlias); } - /** - * Checks whether a password is in "Insecure mode" - * @param credentialType the type of credential, e.g. pattern and password - * @param credential the pattern or password - * @return true, if the credential is in "Insecure mode" - */ - public boolean doesCredentialSupportInsecureMode(int credentialType, byte[] credential) { - if (credential == null) { - return false; - } - if (credentialType != LockPatternUtils.CREDENTIAL_TYPE_PASSWORD) { - return false; - } - byte[] insecurePasswordPrefixBytes = - TrustedRootCertificates.INSECURE_PASSWORD_PREFIX.getBytes(); - if (credential.length < insecurePasswordPrefixBytes.length) { - return false; - } - for (int i = 0; i < insecurePasswordPrefixBytes.length; i++) { - if (credential[i] != insecurePasswordPrefixBytes[i]) { - return false; - } - } - return true; + public boolean doesCredentialSupportInsecureMode(int credentialType, String credential) { + return (credentialType == LockPatternUtils.CREDENTIAL_TYPE_PASSWORD) + && (credential != null) + && credential.startsWith(TrustedRootCertificates.INSECURE_PASSWORD_PREFIX); } public Map keepOnlyWhitelistedInsecureKeys(Map rawKeys) { diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 00b8366ff86da..4c0646ceed869 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -4797,8 +4797,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { if (quality == DevicePolicyManager.PASSWORD_QUALITY_MANAGED) { quality = PASSWORD_QUALITY_UNSPECIFIED; } - // TODO(b/120484642): remove getBytes() below - final PasswordMetrics metrics = PasswordMetrics.computeForPassword(password.getBytes()); + final PasswordMetrics metrics = PasswordMetrics.computeForPassword(password); final int realQuality = metrics.quality; if (realQuality < quality && quality != DevicePolicyManager.PASSWORD_QUALITY_COMPLEX) { @@ -4885,22 +4884,16 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { try { if (token == null) { if (!TextUtils.isEmpty(password)) { - mLockPatternUtils.saveLockPassword(password.getBytes(), null, quality, - userHandle); + mLockPatternUtils.saveLockPassword(password, null, quality, userHandle); } else { mLockPatternUtils.clearLock(null, userHandle); } result = true; } else { - if (!TextUtils.isEmpty(password)) { - result = mLockPatternUtils.setLockCredentialWithToken(password.getBytes(), - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, - quality, tokenHandle, token, userHandle); - } else { - result = mLockPatternUtils.setLockCredentialWithToken(null, - LockPatternUtils.CREDENTIAL_TYPE_NONE, - quality, tokenHandle, token, userHandle); - } + result = mLockPatternUtils.setLockCredentialWithToken(password, + TextUtils.isEmpty(password) ? LockPatternUtils.CREDENTIAL_TYPE_NONE + : LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, + quality, tokenHandle, token, userHandle); } boolean requireEntry = (flags & DevicePolicyManager.RESET_PASSWORD_REQUIRE_ENTRY) != 0; if (requireEntry) { diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java index e6056491a7fe0..26ce7e4cb77c4 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -4179,7 +4179,7 @@ public class DevicePolicyManagerTest extends DpmTestBase { assertTrue(dpm.isResetPasswordTokenActive(admin1)); // test reset password with token - when(getServices().lockPatternUtils.setLockCredentialWithToken(eq(password.getBytes()), + when(getServices().lockPatternUtils.setLockCredentialWithToken(eq(password), eq(LockPatternUtils.CREDENTIAL_TYPE_PASSWORD), eq(DevicePolicyManager.PASSWORD_QUALITY_ALPHABETIC), eq(handle), eq(token), eq(UserHandle.USER_SYSTEM))) diff --git a/services/tests/servicestests/src/com/android/server/locksettings/CachedSyntheticPasswordTests.java b/services/tests/servicestests/src/com/android/server/locksettings/CachedSyntheticPasswordTests.java index 94d21ddeaa2b4..d2caa0af0ba23 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/CachedSyntheticPasswordTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/CachedSyntheticPasswordTests.java @@ -18,22 +18,24 @@ package com.android.server.locksettings; import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_ALPHABETIC; import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED; +import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PASSWORD; import static com.android.server.testutils.TestUtils.assertExpectException; import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.atLeastOnce; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.verify; import android.os.RemoteException; import com.android.internal.widget.LockPatternUtils; import com.android.internal.widget.VerifyCredentialResponse; - -import org.mockito.ArgumentCaptor; +import com.android.server.locksettings.SyntheticPasswordManager.AuthenticationResult; import java.util.ArrayList; +import org.mockito.ArgumentCaptor; + /** * Run the synthetic password tests with caching enabled. * @@ -54,10 +56,10 @@ public class CachedSyntheticPasswordTests extends SyntheticPasswordTests { } public void testSyntheticPasswordClearCredentialUntrusted() throws RemoteException { - final byte[] password = "testSyntheticPasswordClearCredential-password".getBytes(); - final byte[] newPassword = "testSyntheticPasswordClearCredential-newpassword".getBytes(); + final String PASSWORD = "testSyntheticPasswordClearCredential-password"; + final String NEWPASSWORD = "testSyntheticPasswordClearCredential-newpassword"; - initializeCredentialUnderSP(password, PRIMARY_USER_ID); + initializeCredentialUnderSP(PASSWORD, PRIMARY_USER_ID); long sid = mGateKeeperService.getSecureUserId(PRIMARY_USER_ID); // clear password mService.setLockCredential(null, LockPatternUtils.CREDENTIAL_TYPE_NONE, null, @@ -65,46 +67,45 @@ public class CachedSyntheticPasswordTests extends SyntheticPasswordTests { assertEquals(0, mGateKeeperService.getSecureUserId(PRIMARY_USER_ID)); // set a new password - mService.setLockCredential(newPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, + mService.setLockCredential(NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); - assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential(newPassword, - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) - .getResponseCode()); + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + .getResponseCode()); assertNotEquals(sid, mGateKeeperService.getSecureUserId(PRIMARY_USER_ID)); } public void testSyntheticPasswordChangeCredentialUntrusted() throws RemoteException { - final byte[] password = "testSyntheticPasswordClearCredential-password".getBytes(); - final byte[] newPassword = "testSyntheticPasswordClearCredential-newpassword".getBytes(); + final String PASSWORD = "testSyntheticPasswordClearCredential-password"; + final String NEWPASSWORD = "testSyntheticPasswordClearCredential-newpassword"; - initializeCredentialUnderSP(password, PRIMARY_USER_ID); + initializeCredentialUnderSP(PASSWORD, PRIMARY_USER_ID); long sid = mGateKeeperService.getSecureUserId(PRIMARY_USER_ID); // Untrusted change password - mService.setLockCredential(newPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, + mService.setLockCredential(NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); assertNotEquals(0, mGateKeeperService.getSecureUserId(PRIMARY_USER_ID)); assertNotEquals(sid, mGateKeeperService.getSecureUserId(PRIMARY_USER_ID)); // Verify the password - assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential(newPassword, - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID).getResponseCode()); + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + .getResponseCode()); } public void testUntrustedCredentialChangeMaintainsAuthSecret() throws RemoteException { - final byte[] password = - "testUntrustedCredentialChangeMaintainsAuthSecret-password".getBytes(); - final byte[] newPassword = - "testUntrustedCredentialChangeMaintainsAuthSecret-newpassword".getBytes(); + final String PASSWORD = "testUntrustedCredentialChangeMaintainsAuthSecret-password"; + final String NEWPASSWORD = "testUntrustedCredentialChangeMaintainsAuthSecret-newpassword"; - initializeCredentialUnderSP(password, PRIMARY_USER_ID); + initializeCredentialUnderSP(PASSWORD, PRIMARY_USER_ID); // Untrusted change password - mService.setLockCredential(newPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, + mService.setLockCredential(NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); // Verify the password - assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential(newPassword, - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) - .getResponseCode()); + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + .getResponseCode()); // Ensure the same secret was passed each time ArgumentCaptor> secret = ArgumentCaptor.forClass(ArrayList.class); @@ -113,29 +114,27 @@ public class CachedSyntheticPasswordTests extends SyntheticPasswordTests { } public void testUntrustedCredentialChangeBlockedIfSpNotCached() throws RemoteException { - final byte[] password = - "testUntrustedCredentialChangeBlockedIfSpNotCached-password".getBytes(); - final byte[] newPassword = - "testUntrustedCredentialChangeBlockedIfSpNotCached-newpassword".getBytes(); + final String PASSWORD = "testUntrustedCredentialChangeBlockedIfSpNotCached-password"; + final String NEWPASSWORD = "testUntrustedCredentialChangeBlockedIfSpNotCached-newpassword"; // Disable caching for this test enableSpCaching(false); - initializeCredentialUnderSP(password, PRIMARY_USER_ID); + initializeCredentialUnderSP(PASSWORD, PRIMARY_USER_ID); long sid = mGateKeeperService.getSecureUserId(PRIMARY_USER_ID); // Untrusted change password assertExpectException(IllegalStateException.class, /* messageRegex= */ null, - () -> mService.setLockCredential(newPassword, - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, - PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID)); + () -> mService.setLockCredential( + NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, + null, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID)); assertEquals(sid, mGateKeeperService.getSecureUserId(PRIMARY_USER_ID)); // Verify the new password doesn't work but the old one still does - assertEquals(VerifyCredentialResponse.RESPONSE_ERROR, mService.verifyCredential(newPassword, - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + assertEquals(VerifyCredentialResponse.RESPONSE_ERROR, mService.verifyCredential( + NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) .getResponseCode()); - assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential(password, - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + PASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) .getResponseCode()); } diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTestable.java b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTestable.java index cf77245c9e65c..fe683abe7e1b3 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTestable.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTestable.java @@ -120,12 +120,12 @@ public class LockSettingsServiceTestable extends LockSettingsService { } @Override - protected void tieProfileLockToParent(int userId, byte[] password) { - mStorage.writeChildProfileLock(userId, password); + protected void tieProfileLockToParent(int userId, String password) { + mStorage.writeChildProfileLock(userId, password.getBytes()); } @Override - protected byte[] getDecryptedPasswordForTiedProfile(int userId) throws FileNotFoundException, + protected String getDecryptedPasswordForTiedProfile(int userId) throws FileNotFoundException, KeyPermanentlyInvalidatedException { byte[] storedData = mStorage.readChildProfileLock(userId); if (storedData == null) { @@ -138,7 +138,7 @@ public class LockSettingsServiceTestable extends LockSettingsService { } catch (RemoteException e) { // shouldn't happen. } - return storedData; + return new String(storedData); } } diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java index 6b5633cbacdec..e12f6d3be71e2 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java @@ -75,8 +75,8 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { initializeStorageWithCredential(PRIMARY_USER_ID, "password", CREDENTIAL_TYPE_PASSWORD, sid); try { - mService.setLockCredential("newpwd".getBytes(), CREDENTIAL_TYPE_PASSWORD, - "badpwd".getBytes(), PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); + mService.setLockCredential("newpwd", CREDENTIAL_TYPE_PASSWORD, "badpwd", + PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); fail("Did not fail when enrolling using incorrect credential"); } catch (RemoteException expected) { assertTrue(expected.getMessage().equals(FAILED_MESSAGE)); @@ -87,7 +87,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { public void testClearPasswordPrimaryUser() throws RemoteException { final String PASSWORD = "password"; initializeStorageWithCredential(PRIMARY_USER_ID, PASSWORD, CREDENTIAL_TYPE_PASSWORD, 1234); - mService.setLockCredential(null, CREDENTIAL_TYPE_NONE, PASSWORD.getBytes(), + mService.setLockCredential(null, CREDENTIAL_TYPE_NONE, PASSWORD, PASSWORD_QUALITY_UNSPECIFIED, PRIMARY_USER_ID); assertFalse(mService.havePassword(PRIMARY_USER_ID)); assertFalse(mService.havePattern(PRIMARY_USER_ID)); @@ -97,8 +97,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { public void testManagedProfileUnifiedChallenge() throws RemoteException { final String firstUnifiedPassword = "testManagedProfileUnifiedChallenge-pwd-1"; final String secondUnifiedPassword = "testManagedProfileUnifiedChallenge-pwd-2"; - mService.setLockCredential(firstUnifiedPassword.getBytes(), - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, + mService.setLockCredential(firstUnifiedPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, PASSWORD_QUALITY_COMPLEX, PRIMARY_USER_ID); mService.setSeparateProfileChallengeEnabled(MANAGED_PROFILE_USER_ID, false, null); final long primarySid = mGateKeeperService.getSecureUserId(PRIMARY_USER_ID); @@ -117,8 +116,8 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { mGateKeeperService.clearAuthToken(TURNED_OFF_PROFILE_USER_ID); // verify credential assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - firstUnifiedPassword.getBytes(), LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, - PRIMARY_USER_ID).getResponseCode()); + firstUnifiedPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + .getResponseCode()); // Verify that we have a new auth token for the profile assertNotNull(mGateKeeperService.getAuthToken(MANAGED_PROFILE_USER_ID)); @@ -133,16 +132,15 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { */ mStorageManager.setIgnoreBadUnlock(true); // Change primary password and verify that profile SID remains - mService.setLockCredential(secondUnifiedPassword.getBytes(), - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, - firstUnifiedPassword.getBytes(), PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); + mService.setLockCredential(secondUnifiedPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, + firstUnifiedPassword, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); mStorageManager.setIgnoreBadUnlock(false); assertEquals(profileSid, mGateKeeperService.getSecureUserId(MANAGED_PROFILE_USER_ID)); assertNull(mGateKeeperService.getAuthToken(TURNED_OFF_PROFILE_USER_ID)); // Clear unified challenge mService.setLockCredential(null, LockPatternUtils.CREDENTIAL_TYPE_NONE, - secondUnifiedPassword.getBytes(), PASSWORD_QUALITY_UNSPECIFIED, PRIMARY_USER_ID); + secondUnifiedPassword, PASSWORD_QUALITY_UNSPECIFIED, PRIMARY_USER_ID); assertEquals(0, mGateKeeperService.getSecureUserId(PRIMARY_USER_ID)); assertEquals(0, mGateKeeperService.getSecureUserId(MANAGED_PROFILE_USER_ID)); assertEquals(0, mGateKeeperService.getSecureUserId(TURNED_OFF_PROFILE_USER_ID)); @@ -151,16 +149,14 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { public void testManagedProfileSeparateChallenge() throws RemoteException { final String primaryPassword = "testManagedProfileSeparateChallenge-primary"; final String profilePassword = "testManagedProfileSeparateChallenge-profile"; - mService.setLockCredential(primaryPassword.getBytes(), - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, + mService.setLockCredential(primaryPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, PASSWORD_QUALITY_COMPLEX, PRIMARY_USER_ID); /* Currently in LockSettingsService.setLockCredential, unlockUser() is called with the new * credential as part of verifyCredential() before the new credential is committed in * StorageManager. So we relax the check in our mock StorageManager to allow that. */ mStorageManager.setIgnoreBadUnlock(true); - mService.setLockCredential(profilePassword.getBytes(), - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, + mService.setLockCredential(profilePassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, PASSWORD_QUALITY_COMPLEX, MANAGED_PROFILE_USER_ID); mStorageManager.setIgnoreBadUnlock(false); @@ -174,32 +170,31 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { mGateKeeperService.clearAuthToken(MANAGED_PROFILE_USER_ID); // verify primary credential assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - primaryPassword.getBytes(), LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, - PRIMARY_USER_ID).getResponseCode()); + primaryPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + .getResponseCode()); assertNull(mGateKeeperService.getAuthToken(MANAGED_PROFILE_USER_ID)); // verify profile credential assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - profilePassword.getBytes(), LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, + profilePassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, MANAGED_PROFILE_USER_ID).getResponseCode()); assertNotNull(mGateKeeperService.getAuthToken(MANAGED_PROFILE_USER_ID)); assertEquals(profileSid, mGateKeeperService.getSecureUserId(MANAGED_PROFILE_USER_ID)); // Change primary credential and make sure we don't affect profile mStorageManager.setIgnoreBadUnlock(true); - mService.setLockCredential("pwd".getBytes(), LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, - primaryPassword.getBytes(), PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); + mService.setLockCredential("pwd", LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, + primaryPassword, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); mStorageManager.setIgnoreBadUnlock(false); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - profilePassword.getBytes(), LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, + profilePassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, MANAGED_PROFILE_USER_ID).getResponseCode()); assertEquals(profileSid, mGateKeeperService.getSecureUserId(MANAGED_PROFILE_USER_ID)); } private void testCreateCredential(int userId, String credential, int type, int quality) throws RemoteException { - mService.setLockCredential(credential.getBytes(), type, null, quality, - userId); + mService.setLockCredential(credential, type, null, quality, userId); assertVerifyCredentials(userId, credential, type, -1); } @@ -207,16 +202,15 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { String oldCredential, int oldType, int quality) throws RemoteException { final long sid = 1234; initializeStorageWithCredential(userId, oldCredential, oldType, sid); - mService.setLockCredential(newCredential.getBytes(), newType, oldCredential.getBytes(), - quality, userId); + mService.setLockCredential(newCredential, newType, oldCredential, quality, userId); assertVerifyCredentials(userId, newCredential, newType, sid); } private void assertVerifyCredentials(int userId, String credential, int type, long sid) throws RemoteException{ final long challenge = 54321; - VerifyCredentialResponse response = mService.verifyCredential(credential.getBytes(), - type, challenge, userId); + VerifyCredentialResponse response = mService.verifyCredential(credential, type, challenge, + userId); assertEquals(GateKeeperResponse.RESPONSE_OK, response.getResponseCode()); if (sid != -1) assertEquals(sid, mGateKeeperService.getSecureUserId(userId)); @@ -235,19 +229,18 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { incorrectType = LockPatternUtils.CREDENTIAL_TYPE_PASSWORD; } // check for bad type - assertEquals(GateKeeperResponse.RESPONSE_ERROR, mService.verifyCredential( - credential.getBytes(), incorrectType, challenge, userId).getResponseCode()); + assertEquals(GateKeeperResponse.RESPONSE_ERROR, mService.verifyCredential(credential, + incorrectType, challenge, userId).getResponseCode()); // check for bad credential - assertEquals(GateKeeperResponse.RESPONSE_ERROR, mService.verifyCredential( - ("0" + credential).getBytes(), type, challenge, userId).getResponseCode()); + assertEquals(GateKeeperResponse.RESPONSE_ERROR, mService.verifyCredential("0" + credential, + type, challenge, userId).getResponseCode()); } private void initializeStorageWithCredential(int userId, String credential, int type, long sid) throws RemoteException { - byte[] credentialBytes = credential == null ? null : credential.getBytes(); byte[] oldHash = new VerifyHandle(credential.getBytes(), sid).toBytes(); if (mService.shouldMigrateToSyntheticPasswordLocked(userId)) { - mService.initializeSyntheticPasswordLocked(oldHash, credentialBytes, type, + mService.initializeSyntheticPasswordLocked(oldHash, credential, type, type == LockPatternUtils.CREDENTIAL_TYPE_PASSWORD ? PASSWORD_QUALITY_ALPHABETIC : PASSWORD_QUALITY_SOMETHING, userId); } else { diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsShellCommandTest.java b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsShellCommandTest.java index b53ba6cb3ec93..424c08c4c931d 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsShellCommandTest.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsShellCommandTest.java @@ -87,36 +87,35 @@ public class LockSettingsShellCommandTest { public void testWrongPassword() throws Exception { when(mLockPatternUtils.isLockPatternEnabled(mUserId)).thenReturn(false); when(mLockPatternUtils.isLockPasswordEnabled(mUserId)).thenReturn(true); - when(mLockPatternUtils.checkPassword("1234".getBytes(), mUserId)).thenReturn(false); + when(mLockPatternUtils.checkPassword("1234", mUserId)).thenReturn(false); assertEquals(-1, mCommand.exec(mBinder, in, out, err, new String[] { "set-pin", "--old", "1234" }, mShellCallback, mResultReceiver)); - verify(mLockPatternUtils, never()).saveLockPassword(any(byte[].class), any(byte[].class), - anyInt(), anyInt()); + verify(mLockPatternUtils, never()).saveLockPassword(any(), any(), anyInt(), anyInt()); } @Test public void testChangePin() throws Exception { when(mLockPatternUtils.isLockPatternEnabled(mUserId)).thenReturn(false); when(mLockPatternUtils.isLockPasswordEnabled(mUserId)).thenReturn(true); - when(mLockPatternUtils.checkPassword("1234".getBytes(), mUserId)).thenReturn(true); + when(mLockPatternUtils.checkPassword("1234", mUserId)).thenReturn(true); assertEquals(0, mCommand.exec(new Binder(), in, out, err, new String[] { "set-pin", "--old", "1234", "4321" }, mShellCallback, mResultReceiver)); - verify(mLockPatternUtils).saveLockPassword("4321".getBytes(), "1234".getBytes(), - PASSWORD_QUALITY_NUMERIC, mUserId); + verify(mLockPatternUtils).saveLockPassword("4321", "1234", PASSWORD_QUALITY_NUMERIC, + mUserId); } @Test public void testChangePassword() throws Exception { when(mLockPatternUtils.isLockPatternEnabled(mUserId)).thenReturn(false); when(mLockPatternUtils.isLockPasswordEnabled(mUserId)).thenReturn(true); - when(mLockPatternUtils.checkPassword("1234".getBytes(), mUserId)).thenReturn(true); + when(mLockPatternUtils.checkPassword("1234", mUserId)).thenReturn(true); assertEquals(0, mCommand.exec(new Binder(), in, out, err, new String[] { "set-password", "--old", "1234", "4321" }, mShellCallback, mResultReceiver)); - verify(mLockPatternUtils).saveLockPassword("4321".getBytes(), "1234".getBytes(), - PASSWORD_QUALITY_ALPHABETIC, mUserId); + verify(mLockPatternUtils).saveLockPassword("4321", "1234", PASSWORD_QUALITY_ALPHABETIC, + mUserId); } @Test @@ -127,8 +126,7 @@ public class LockSettingsShellCommandTest { assertEquals(0, mCommand.exec(new Binder(), in, out, err, new String[] { "set-pattern", "--old", "1234", "4321" }, mShellCallback, mResultReceiver)); - verify(mLockPatternUtils).saveLockPattern(stringToPattern("4321"), "1234".getBytes(), - mUserId); + verify(mLockPatternUtils).saveLockPattern(stringToPattern("4321"), "1234", mUserId); } @Test @@ -139,6 +137,6 @@ public class LockSettingsShellCommandTest { assertEquals(0, mCommand.exec(new Binder(), in, out, err, new String[] { "clear", "--old", "1234" }, mShellCallback, mResultReceiver)); - verify(mLockPatternUtils).clearLock("1234".getBytes(), mUserId); + verify(mLockPatternUtils).clearLock("1234", mUserId); } } diff --git a/services/tests/servicestests/src/com/android/server/locksettings/MockSyntheticPasswordManager.java b/services/tests/servicestests/src/com/android/server/locksettings/MockSyntheticPasswordManager.java index b9cb730caae1f..6f681797b88a4 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/MockSyntheticPasswordManager.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/MockSyntheticPasswordManager.java @@ -93,13 +93,9 @@ public class MockSyntheticPasswordManager extends SyntheticPasswordManager { } @Override - protected byte[] scrypt(byte[] password, byte[] salt, int n, int r, int p, int outLen) { + protected byte[] scrypt(String password, byte[] salt, int N, int r, int p, int outLen) { try { - char[] passwordChars = new char[password.length]; - for (int i = 0; i < password.length; i++) { - passwordChars[i] = (char) password[i]; - } - PBEKeySpec spec = new PBEKeySpec(passwordChars, salt, 10, outLen * 8); + PBEKeySpec spec = new PBEKeySpec(password.toCharArray(), salt, 10, outLen * 8); SecretKeyFactory f = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1"); return f.generateSecret(spec).getEncoded(); } catch (InvalidKeySpecException | NoSuchAlgorithmException e) { diff --git a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java index 3dbc881a89a6c..94e02bc4d35f6 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java @@ -65,23 +65,22 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { public void testPasswordBasedSyntheticPassword() throws RemoteException { final int USER_ID = 10; - final byte[] password = "user-password".getBytes(); - final byte[] badPassword = "bad-password".getBytes(); + final String PASSWORD = "user-password"; + final String BADPASSWORD = "bad-password"; MockSyntheticPasswordManager manager = new MockSyntheticPasswordManager(mContext, mStorage, mGateKeeperService, mUserManager); AuthenticationToken authToken = manager.newSyntheticPasswordAndSid(mGateKeeperService, null, null, USER_ID); - long handle = manager.createPasswordBasedSyntheticPassword(mGateKeeperService, - password, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, authToken, - PASSWORD_QUALITY_ALPHABETIC, USER_ID); + long handle = manager.createPasswordBasedSyntheticPassword(mGateKeeperService, PASSWORD, + LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, authToken, PASSWORD_QUALITY_ALPHABETIC, + USER_ID); AuthenticationResult result = manager.unwrapPasswordBasedSyntheticPassword( - mGateKeeperService, handle, password, USER_ID, null); - assertArrayEquals(result.authToken.deriveKeyStorePassword(), - authToken.deriveKeyStorePassword()); + mGateKeeperService, handle, PASSWORD, USER_ID, null); + assertEquals(result.authToken.deriveKeyStorePassword(), authToken.deriveKeyStorePassword()); result = manager.unwrapPasswordBasedSyntheticPassword(mGateKeeperService, handle, - badPassword, USER_ID, null); + BADPASSWORD, USER_ID, null); assertNull(result.authToken); } @@ -98,30 +97,30 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { } public void testPasswordMigration() throws RemoteException { - final byte[] password = "testPasswordMigration-password".getBytes(); + final String PASSWORD = "testPasswordMigration-password"; disableSyntheticPassword(); - mService.setLockCredential(password, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, + mService.setLockCredential(PASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); long sid = mGateKeeperService.getSecureUserId(PRIMARY_USER_ID); final byte[] primaryStorageKey = mStorageManager.getUserUnlockToken(PRIMARY_USER_ID); enableSyntheticPassword(); // Performs migration assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - password, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + PASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) .getResponseCode()); assertEquals(sid, mGateKeeperService.getSecureUserId(PRIMARY_USER_ID)); assertTrue(hasSyntheticPassword(PRIMARY_USER_ID)); // SP-based verification - assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential(password, - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + PASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) .getResponseCode()); assertArrayNotEquals(primaryStorageKey, mStorageManager.getUserUnlockToken(PRIMARY_USER_ID)); } - protected void initializeCredentialUnderSP(byte[] password, int userId) throws RemoteException { + protected void initializeCredentialUnderSP(String password, int userId) throws RemoteException { enableSyntheticPassword(); int quality = password != null ? PASSWORD_QUALITY_ALPHABETIC : PASSWORD_QUALITY_UNSPECIFIED; @@ -131,64 +130,62 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { } public void testSyntheticPasswordChangeCredential() throws RemoteException { - final byte[] password = "testSyntheticPasswordChangeCredential-password".getBytes(); - final byte[] newPassword = "testSyntheticPasswordChangeCredential-newpassword".getBytes(); + final String PASSWORD = "testSyntheticPasswordChangeCredential-password"; + final String NEWPASSWORD = "testSyntheticPasswordChangeCredential-newpassword"; - initializeCredentialUnderSP(password, PRIMARY_USER_ID); + initializeCredentialUnderSP(PASSWORD, PRIMARY_USER_ID); long sid = mGateKeeperService.getSecureUserId(PRIMARY_USER_ID); - mService.setLockCredential(newPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, password, + mService.setLockCredential(NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, PASSWORD, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - newPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) .getResponseCode()); assertEquals(sid, mGateKeeperService.getSecureUserId(PRIMARY_USER_ID)); } public void testSyntheticPasswordVerifyCredential() throws RemoteException { - final byte[] password = "testSyntheticPasswordVerifyCredential-password".getBytes(); - final byte[] badPassword = "testSyntheticPasswordVerifyCredential-badpassword".getBytes(); + final String PASSWORD = "testSyntheticPasswordVerifyCredential-password"; + final String BADPASSWORD = "testSyntheticPasswordVerifyCredential-badpassword"; - initializeCredentialUnderSP(password, PRIMARY_USER_ID); + initializeCredentialUnderSP(PASSWORD, PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - password, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + PASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) .getResponseCode()); assertEquals(VerifyCredentialResponse.RESPONSE_ERROR, mService.verifyCredential( - badPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) - .getResponseCode()); + BADPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + .getResponseCode()); } public void testSyntheticPasswordClearCredential() throws RemoteException { - final byte[] password = "testSyntheticPasswordClearCredential-password".getBytes(); - final byte[] badPassword = "testSyntheticPasswordClearCredential-newpassword".getBytes(); + final String PASSWORD = "testSyntheticPasswordClearCredential-password"; + final String NEWPASSWORD = "testSyntheticPasswordClearCredential-newpassword"; - initializeCredentialUnderSP(password, PRIMARY_USER_ID); + initializeCredentialUnderSP(PASSWORD, PRIMARY_USER_ID); long sid = mGateKeeperService.getSecureUserId(PRIMARY_USER_ID); // clear password - mService.setLockCredential(null, LockPatternUtils.CREDENTIAL_TYPE_NONE, password, + mService.setLockCredential(null, LockPatternUtils.CREDENTIAL_TYPE_NONE, PASSWORD, PASSWORD_QUALITY_UNSPECIFIED, PRIMARY_USER_ID); assertEquals(0 ,mGateKeeperService.getSecureUserId(PRIMARY_USER_ID)); // set a new password - mService.setLockCredential(badPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, + mService.setLockCredential(NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - badPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) - .getResponseCode()); + NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + .getResponseCode()); assertNotEquals(sid, mGateKeeperService.getSecureUserId(PRIMARY_USER_ID)); } public void testSyntheticPasswordChangeCredentialKeepsAuthSecret() throws RemoteException { - final byte[] password = - "testSyntheticPasswordChangeCredentialKeepsAuthSecret-password".getBytes(); - final byte[] badPassword = - "testSyntheticPasswordChangeCredentialKeepsAuthSecret-new".getBytes(); + final String PASSWORD = "testSyntheticPasswordChangeCredentialKeepsAuthSecret-password"; + final String NEWPASSWORD = "testSyntheticPasswordChangeCredentialKeepsAuthSecret-new"; - initializeCredentialUnderSP(password, PRIMARY_USER_ID); - mService.setLockCredential(badPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, password, + initializeCredentialUnderSP(PASSWORD, PRIMARY_USER_ID); + mService.setLockCredential(NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, PASSWORD, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - badPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) .getResponseCode()); // Check the same secret was passed each time @@ -198,25 +195,24 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { } public void testSyntheticPasswordVerifyPassesPrimaryUserAuthSecret() throws RemoteException { - final byte[] password = - "testSyntheticPasswordVerifyPassesPrimaryUserAuthSecret-password".getBytes(); - final byte[] newPassword = - "testSyntheticPasswordVerifyPassesPrimaryUserAuthSecret-new".getBytes(); + final String PASSWORD = "testSyntheticPasswordVerifyPassesPrimaryUserAuthSecret-password"; + final String NEWPASSWORD = "testSyntheticPasswordVerifyPassesPrimaryUserAuthSecret-new"; - initializeCredentialUnderSP(password, PRIMARY_USER_ID); + initializeCredentialUnderSP(PASSWORD, PRIMARY_USER_ID); reset(mAuthSecretService); - assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential(password, - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + PASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) .getResponseCode()); verify(mAuthSecretService).primaryUserCredential(any(ArrayList.class)); } public void testSecondaryUserDoesNotPassAuthSecret() throws RemoteException { - final byte[] password = "testSecondaryUserDoesNotPassAuthSecret-password".getBytes(); + final String PASSWORD = "testSecondaryUserDoesNotPassAuthSecret-password"; + final String NEWPASSWORD = "testSecondaryUserDoesNotPassAuthSecret-new"; - initializeCredentialUnderSP(password, SECONDARY_USER_ID); + initializeCredentialUnderSP(PASSWORD, SECONDARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - password, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, SECONDARY_USER_ID) + PASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, SECONDARY_USER_ID) .getResponseCode()); verify(mAuthSecretService, never()).primaryUserCredential(any(ArrayList.class)); } @@ -232,8 +228,8 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { } public void testSyntheticPasswordAndCredentialDoesNotPassAuthSecret() throws RemoteException { - final byte[] password = "passwordForASyntheticPassword".getBytes(); - initializeCredentialUnderSP(password, PRIMARY_USER_ID); + final String PASSWORD = "passwordForASyntheticPassword"; + initializeCredentialUnderSP(PASSWORD, PRIMARY_USER_ID); reset(mAuthSecretService); mService.onUnlockUser(PRIMARY_USER_ID); @@ -242,9 +238,9 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { } public void testSyntheticPasswordButNoCredentialPassesAuthSecret() throws RemoteException { - final byte[] password = "getASyntheticPassword".getBytes(); - initializeCredentialUnderSP(password, PRIMARY_USER_ID); - mService.setLockCredential(null, LockPatternUtils.CREDENTIAL_TYPE_NONE, password, + final String PASSWORD = "getASyntheticPassword"; + initializeCredentialUnderSP(PASSWORD, PRIMARY_USER_ID); + mService.setLockCredential(null, LockPatternUtils.CREDENTIAL_TYPE_NONE, PASSWORD, PASSWORD_QUALITY_UNSPECIFIED, PRIMARY_USER_ID); reset(mAuthSecretService); @@ -254,7 +250,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { } public void testManagedProfileUnifiedChallengeMigration() throws RemoteException { - final byte[] UnifiedPassword = "testManagedProfileUnifiedChallengeMigration-pwd".getBytes(); + final String UnifiedPassword = "testManagedProfileUnifiedChallengeMigration-pwd"; disableSyntheticPassword(); mService.setLockCredential(UnifiedPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); @@ -288,10 +284,8 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { } public void testManagedProfileSeparateChallengeMigration() throws RemoteException { - final byte[] primaryPassword = - "testManagedProfileSeparateChallengeMigration-primary".getBytes(); - final byte[] profilePassword = - "testManagedProfileSeparateChallengeMigration-profile".getBytes(); + final String primaryPassword = "testManagedProfileSeparateChallengeMigration-primary"; + final String profilePassword = "testManagedProfileSeparateChallengeMigration-profile"; disableSyntheticPassword(); mService.setLockCredential(primaryPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); @@ -332,92 +326,92 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { } public void testTokenBasedResetPassword() throws RemoteException { - final byte[] password = "password".getBytes(); - final byte[] pattern = "123654".getBytes(); - final byte[] token = "some-high-entropy-secure-token".getBytes(); - initializeCredentialUnderSP(password, PRIMARY_USER_ID); + final String PASSWORD = "password"; + final String PATTERN = "123654"; + final String TOKEN = "some-high-entropy-secure-token"; + initializeCredentialUnderSP(PASSWORD, PRIMARY_USER_ID); final byte[] storageKey = mStorageManager.getUserUnlockToken(PRIMARY_USER_ID); - long handle = mLocalService.addEscrowToken(token, PRIMARY_USER_ID); + long handle = mLocalService.addEscrowToken(TOKEN.getBytes(), PRIMARY_USER_ID); assertFalse(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); - mService.verifyCredential(password, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, + mService.verifyCredential(PASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID).getResponseCode(); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); - mLocalService.setLockCredentialWithToken(pattern, LockPatternUtils.CREDENTIAL_TYPE_PATTERN, - handle, token, PASSWORD_QUALITY_SOMETHING, PRIMARY_USER_ID); + mLocalService.setLockCredentialWithToken(PATTERN, LockPatternUtils.CREDENTIAL_TYPE_PATTERN, + handle, TOKEN.getBytes(), PASSWORD_QUALITY_SOMETHING, PRIMARY_USER_ID); // Verify DPM gets notified about new device lock mService.mHandler.runWithScissors(() -> {}, 0 /*now*/); // Flush runnables on handler - PasswordMetrics metric = PasswordMetrics.computeForPassword(pattern); + PasswordMetrics metric = PasswordMetrics.computeForPassword(PATTERN); metric.quality = PASSWORD_QUALITY_SOMETHING; verify(mDevicePolicyManager).setActivePasswordState(metric, PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - pattern, LockPatternUtils.CREDENTIAL_TYPE_PATTERN, 0, PRIMARY_USER_ID) + PATTERN, LockPatternUtils.CREDENTIAL_TYPE_PATTERN, 0, PRIMARY_USER_ID) .getResponseCode()); assertArrayEquals(storageKey, mStorageManager.getUserUnlockToken(PRIMARY_USER_ID)); } public void testTokenBasedClearPassword() throws RemoteException { - final byte[] password = "password".getBytes(); - final byte[] pattern = "123654".getBytes(); - final byte[] token = "some-high-entropy-secure-token".getBytes(); - initializeCredentialUnderSP(password, PRIMARY_USER_ID); + final String PASSWORD = "password"; + final String PATTERN = "123654"; + final String TOKEN = "some-high-entropy-secure-token"; + initializeCredentialUnderSP(PASSWORD, PRIMARY_USER_ID); final byte[] storageKey = mStorageManager.getUserUnlockToken(PRIMARY_USER_ID); - long handle = mLocalService.addEscrowToken(token, PRIMARY_USER_ID); + long handle = mLocalService.addEscrowToken(TOKEN.getBytes(), PRIMARY_USER_ID); assertFalse(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); - mService.verifyCredential(password, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, + mService.verifyCredential(PASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID).getResponseCode(); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); mLocalService.setLockCredentialWithToken(null, LockPatternUtils.CREDENTIAL_TYPE_NONE, - handle, token, PASSWORD_QUALITY_UNSPECIFIED, PRIMARY_USER_ID); - mLocalService.setLockCredentialWithToken(pattern, LockPatternUtils.CREDENTIAL_TYPE_PATTERN, - handle, token, PASSWORD_QUALITY_SOMETHING, PRIMARY_USER_ID); + handle, TOKEN.getBytes(), PASSWORD_QUALITY_UNSPECIFIED, PRIMARY_USER_ID); + mLocalService.setLockCredentialWithToken(PATTERN, LockPatternUtils.CREDENTIAL_TYPE_PATTERN, + handle, TOKEN.getBytes(), PASSWORD_QUALITY_SOMETHING, PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - pattern, LockPatternUtils.CREDENTIAL_TYPE_PATTERN, 0, PRIMARY_USER_ID) + PATTERN, LockPatternUtils.CREDENTIAL_TYPE_PATTERN, 0, PRIMARY_USER_ID) .getResponseCode()); assertArrayEquals(storageKey, mStorageManager.getUserUnlockToken(PRIMARY_USER_ID)); } public void testTokenBasedResetPasswordAfterCredentialChanges() throws RemoteException { - final byte[] password = "password".getBytes(); - final byte[] pattern = "123654".getBytes(); - final byte[] newPassword = "password".getBytes(); - final byte[] token = "some-high-entropy-secure-token".getBytes(); - initializeCredentialUnderSP(password, PRIMARY_USER_ID); + final String PASSWORD = "password"; + final String PATTERN = "123654"; + final String NEWPASSWORD = "password"; + final String TOKEN = "some-high-entropy-secure-token"; + initializeCredentialUnderSP(PASSWORD, PRIMARY_USER_ID); final byte[] storageKey = mStorageManager.getUserUnlockToken(PRIMARY_USER_ID); - long handle = mLocalService.addEscrowToken(token, PRIMARY_USER_ID); + long handle = mLocalService.addEscrowToken(TOKEN.getBytes(), PRIMARY_USER_ID); assertFalse(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); - mService.verifyCredential(password, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, + mService.verifyCredential(PASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID).getResponseCode(); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); - mService.setLockCredential(pattern, LockPatternUtils.CREDENTIAL_TYPE_PATTERN, password, + mService.setLockCredential(PATTERN, LockPatternUtils.CREDENTIAL_TYPE_PATTERN, PASSWORD, PASSWORD_QUALITY_SOMETHING, PRIMARY_USER_ID); - mLocalService.setLockCredentialWithToken(newPassword, - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, handle, token, + mLocalService.setLockCredentialWithToken(NEWPASSWORD, + LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, handle, TOKEN.getBytes(), PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - newPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + NEWPASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) .getResponseCode()); assertArrayEquals(storageKey, mStorageManager.getUserUnlockToken(PRIMARY_USER_ID)); } public void testEscrowTokenActivatedImmediatelyIfNoUserPasswordNeedsMigration() throws RemoteException { - final String token = "some-high-entropy-secure-token"; + final String TOKEN = "some-high-entropy-secure-token"; enableSyntheticPassword(); - long handle = mLocalService.addEscrowToken(token.getBytes(), PRIMARY_USER_ID); + long handle = mLocalService.addEscrowToken(TOKEN.getBytes(), PRIMARY_USER_ID); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); assertEquals(0, mGateKeeperService.getSecureUserId(PRIMARY_USER_ID)); assertTrue(hasSyntheticPassword(PRIMARY_USER_ID)); @@ -425,9 +419,9 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { public void testEscrowTokenActivatedImmediatelyIfNoUserPasswordNoMigration() throws RemoteException { - final String token = "some-high-entropy-secure-token"; + final String TOKEN = "some-high-entropy-secure-token"; initializeCredentialUnderSP(null, PRIMARY_USER_ID); - long handle = mLocalService.addEscrowToken(token.getBytes(), PRIMARY_USER_ID); + long handle = mLocalService.addEscrowToken(TOKEN.getBytes(), PRIMARY_USER_ID); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); assertEquals(0, mGateKeeperService.getSecureUserId(PRIMARY_USER_ID)); assertTrue(hasSyntheticPassword(PRIMARY_USER_ID)); @@ -435,34 +429,34 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { public void testEscrowTokenActivatedLaterWithUserPasswordNeedsMigration() throws RemoteException { - final byte[] token = "some-high-entropy-secure-token".getBytes(); - final byte[] password = "password".getBytes(); + final String TOKEN = "some-high-entropy-secure-token"; + final String PASSWORD = "password"; // Set up pre-SP user password disableSyntheticPassword(); - mService.setLockCredential(password, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, + mService.setLockCredential(PASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); enableSyntheticPassword(); - long handle = mLocalService.addEscrowToken(token, PRIMARY_USER_ID); + long handle = mLocalService.addEscrowToken(TOKEN.getBytes(), PRIMARY_USER_ID); // Token not activated immediately since user password exists assertFalse(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); // Activate token (password gets migrated to SP at the same time) assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - password, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) + PASSWORD, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, 0, PRIMARY_USER_ID) .getResponseCode()); // Verify token is activated assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); } public void testgetHashFactorPrimaryUser() throws RemoteException { - final byte[] password = "password".getBytes(); + final String password = "password"; mService.setLockCredential(password, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); final byte[] hashFactor = mService.getHashFactor(password, PRIMARY_USER_ID); assertNotNull(hashFactor); - mService.setLockCredential(null, LockPatternUtils.CREDENTIAL_TYPE_NONE, - password, PASSWORD_QUALITY_UNSPECIFIED, PRIMARY_USER_ID); + mService.setLockCredential(null, LockPatternUtils.CREDENTIAL_TYPE_NONE, password, + PASSWORD_QUALITY_UNSPECIFIED, PRIMARY_USER_ID); final byte[] newHashFactor = mService.getHashFactor(null, PRIMARY_USER_ID); assertNotNull(newHashFactor); // Hash factor should never change after password change/removal @@ -470,16 +464,16 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { } public void testgetHashFactorManagedProfileUnifiedChallenge() throws RemoteException { - final byte[] pattern = "1236".getBytes(); - mService.setLockCredential(pattern, LockPatternUtils.CREDENTIAL_TYPE_PATTERN, - null, PASSWORD_QUALITY_SOMETHING, PRIMARY_USER_ID); + final String pattern = "1236"; + mService.setLockCredential(pattern, LockPatternUtils.CREDENTIAL_TYPE_PATTERN, null, + PASSWORD_QUALITY_SOMETHING, PRIMARY_USER_ID); mService.setSeparateProfileChallengeEnabled(MANAGED_PROFILE_USER_ID, false, null); assertNotNull(mService.getHashFactor(null, MANAGED_PROFILE_USER_ID)); } public void testgetHashFactorManagedProfileSeparateChallenge() throws RemoteException { - final byte[] primaryPassword = "primary".getBytes(); - final byte[] profilePassword = "profile".getBytes(); + final String primaryPassword = "primary"; + final String profilePassword = "profile"; mService.setLockCredential(primaryPassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, PASSWORD_QUALITY_ALPHABETIC, PRIMARY_USER_ID); mService.setLockCredential(profilePassword, LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, null, diff --git a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/KeySyncTaskTest.java b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/KeySyncTaskTest.java index f86760187feb5..90947f44ef2ba 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/KeySyncTaskTest.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/KeySyncTaskTest.java @@ -135,7 +135,7 @@ public class KeySyncTaskTest { mSnapshotListenersStorage, TEST_USER_ID, TEST_CREDENTIAL_TYPE, - TEST_CREDENTIAL.getBytes(), + TEST_CREDENTIAL, /*credentialUpdated=*/ false, mPlatformKeyManager, mTestOnlyInsecureCertificateHelper, @@ -159,17 +159,17 @@ public class KeySyncTaskTest { @Test public void isPin_isTrueForNumericString() { - assertTrue(KeySyncTask.isPin("3298432574398654376547".getBytes())); + assertTrue(KeySyncTask.isPin("3298432574398654376547")); } @Test public void isPin_isFalseForStringContainingLetters() { - assertFalse(KeySyncTask.isPin("398i54369548654".getBytes())); + assertFalse(KeySyncTask.isPin("398i54369548654")); } @Test public void isPin_isFalseForStringContainingSymbols() { - assertFalse(KeySyncTask.isPin("-3987543643".getBytes())); + assertFalse(KeySyncTask.isPin("-3987543643")); } @Test @@ -178,8 +178,8 @@ public class KeySyncTaskTest { byte[] salt = randomBytes(16); assertArrayEquals( - KeySyncTask.hashCredentialsBySaltedSha256(salt, credentials.getBytes()), - KeySyncTask.hashCredentialsBySaltedSha256(salt, credentials.getBytes())); + KeySyncTask.hashCredentialsBySaltedSha256(salt, credentials), + KeySyncTask.hashCredentialsBySaltedSha256(salt, credentials)); } @Test @@ -188,8 +188,8 @@ public class KeySyncTaskTest { assertFalse( Arrays.equals( - KeySyncTask.hashCredentialsBySaltedSha256(salt, "password1234".getBytes()), - KeySyncTask.hashCredentialsBySaltedSha256(salt, "password12345".getBytes()))); + KeySyncTask.hashCredentialsBySaltedSha256(salt, "password1234"), + KeySyncTask.hashCredentialsBySaltedSha256(salt, "password12345"))); } @Test @@ -198,38 +198,34 @@ public class KeySyncTaskTest { assertFalse( Arrays.equals( - KeySyncTask.hashCredentialsBySaltedSha256(randomBytes(64), - credentials.getBytes()), - KeySyncTask.hashCredentialsBySaltedSha256(randomBytes(64), - credentials.getBytes()))); + KeySyncTask.hashCredentialsBySaltedSha256(randomBytes(64), credentials), + KeySyncTask.hashCredentialsBySaltedSha256(randomBytes(64), credentials))); } @Test public void hashCredentialsBySaltedSha256_returnsDifferentHashEvenIfConcatIsSame() { assertFalse( Arrays.equals( - KeySyncTask.hashCredentialsBySaltedSha256(utf8Bytes("123"), - "4567".getBytes()), - KeySyncTask.hashCredentialsBySaltedSha256(utf8Bytes("1234"), - "567".getBytes()))); + KeySyncTask.hashCredentialsBySaltedSha256(utf8Bytes("123"), "4567"), + KeySyncTask.hashCredentialsBySaltedSha256(utf8Bytes("1234"), "567"))); } @Test public void getUiFormat_returnsPinIfPin() { assertEquals(UI_FORMAT_PIN, - KeySyncTask.getUiFormat(CREDENTIAL_TYPE_PASSWORD, "1234".getBytes())); + KeySyncTask.getUiFormat(CREDENTIAL_TYPE_PASSWORD, "1234")); } @Test public void getUiFormat_returnsPasswordIfPassword() { assertEquals(UI_FORMAT_PASSWORD, - KeySyncTask.getUiFormat(CREDENTIAL_TYPE_PASSWORD, "1234a".getBytes())); + KeySyncTask.getUiFormat(CREDENTIAL_TYPE_PASSWORD, "1234a")); } @Test public void getUiFormat_returnsPatternIfPattern() { assertEquals(UI_FORMAT_PATTERN, - KeySyncTask.getUiFormat(CREDENTIAL_TYPE_PATTERN, "1234".getBytes())); + KeySyncTask.getUiFormat(CREDENTIAL_TYPE_PATTERN, "1234")); } @@ -291,7 +287,7 @@ public class KeySyncTaskTest { mSnapshotListenersStorage, TEST_USER_ID, CREDENTIAL_TYPE_PASSWORD, - /*credential=*/ password.getBytes(), + /*credential=*/ password, /*credentialUpdated=*/ false, mPlatformKeyManager, mTestOnlyInsecureCertificateHelper, @@ -332,7 +328,7 @@ public class KeySyncTaskTest { mSnapshotListenersStorage, TEST_USER_ID, CREDENTIAL_TYPE_PATTERN, - /*credential=*/ pattern.getBytes(), + /*credential=*/ pattern, /*credentialUpdated=*/ false, mPlatformKeyManager, mTestOnlyInsecureCertificateHelper, @@ -366,7 +362,7 @@ public class KeySyncTaskTest { mSnapshotListenersStorage, TEST_USER_ID, CREDENTIAL_TYPE_PASSWORD, - /*credential=*/ shortPassword.getBytes(), + /*credential=*/ shortPassword, /*credentialUpdated=*/ false, mPlatformKeyManager, mTestOnlyInsecureCertificateHelper, @@ -526,7 +522,7 @@ public class KeySyncTaskTest { verify(mSnapshotListenersStorage).recoverySnapshotAvailable(TEST_RECOVERY_AGENT_UID); byte[] lockScreenHash = KeySyncTask.hashCredentialsBySaltedSha256( keyDerivationParams.getSalt(), - TEST_CREDENTIAL.getBytes()); + TEST_CREDENTIAL); Long counterId = mRecoverableKeyStoreDb.getCounterId(TEST_USER_ID, TEST_RECOVERY_AGENT_UID); assertThat(counterId).isNotNull(); byte[] recoveryKey = decryptThmEncryptedKey( @@ -620,7 +616,7 @@ public class KeySyncTaskTest { mSnapshotListenersStorage, TEST_USER_ID, CREDENTIAL_TYPE_PASSWORD, - password.getBytes(), + password, /*credentialUpdated=*/ false, mPlatformKeyManager, mTestOnlyInsecureCertificateHelper, @@ -651,7 +647,7 @@ public class KeySyncTaskTest { mSnapshotListenersStorage, TEST_USER_ID, CREDENTIAL_TYPE_PASSWORD, - /*credential=*/ pin.getBytes(), + /*credential=*/ pin, /*credentialUpdated=*/ false, mPlatformKeyManager, mTestOnlyInsecureCertificateHelper, @@ -683,7 +679,7 @@ public class KeySyncTaskTest { mSnapshotListenersStorage, TEST_USER_ID, CREDENTIAL_TYPE_PATTERN, - "12345".getBytes(), + "12345", /*credentialUpdated=*/ false, mPlatformKeyManager, mTestOnlyInsecureCertificateHelper, @@ -767,7 +763,7 @@ public class KeySyncTaskTest { mSnapshotListenersStorage, TEST_USER_ID, /*credentialType=*/ 3, - "12345".getBytes(), + "12345", /*credentialUpdated=*/ false, mPlatformKeyManager, mTestOnlyInsecureCertificateHelper, diff --git a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelperTest.java b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelperTest.java index c761f0d29654e..67436cc4c8531 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelperTest.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelperTest.java @@ -24,30 +24,30 @@ public class TestOnlyInsecureCertificateHelperTest { @Test public void testDoesCredentailSupportInsecureMode_forNonWhitelistedPassword() throws Exception { assertThat(mHelper.doesCredentialSupportInsecureMode( - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, "secret12345".getBytes())).isFalse(); + LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, "secret12345")).isFalse(); assertThat(mHelper.doesCredentialSupportInsecureMode( - LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, "1234".getBytes())).isFalse(); + LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, "1234")).isFalse(); } @Test public void testDoesCredentailSupportInsecureMode_forWhitelistedPassword() throws Exception { assertThat(mHelper.doesCredentialSupportInsecureMode( LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, - TrustedRootCertificates.INSECURE_PASSWORD_PREFIX.getBytes())).isTrue(); + TrustedRootCertificates.INSECURE_PASSWORD_PREFIX)).isTrue(); assertThat(mHelper.doesCredentialSupportInsecureMode( LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, - (TrustedRootCertificates.INSECURE_PASSWORD_PREFIX + "12").getBytes())).isTrue(); + TrustedRootCertificates.INSECURE_PASSWORD_PREFIX + "12")).isTrue(); } @Test public void testDoesCredentailSupportInsecureMode_Pattern() throws Exception { assertThat(mHelper.doesCredentialSupportInsecureMode( LockPatternUtils.CREDENTIAL_TYPE_PATTERN, - TrustedRootCertificates.INSECURE_PASSWORD_PREFIX.getBytes())).isFalse(); + TrustedRootCertificates.INSECURE_PASSWORD_PREFIX)).isFalse(); assertThat(mHelper.doesCredentialSupportInsecureMode( LockPatternUtils.CREDENTIAL_TYPE_NONE, - TrustedRootCertificates.INSECURE_PASSWORD_PREFIX.getBytes())).isFalse(); + TrustedRootCertificates.INSECURE_PASSWORD_PREFIX)).isFalse(); } @Test From 11d913944f53dd189c027068aaea5fc87316e962 Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Fri, 15 Feb 2019 15:48:11 -0800 Subject: [PATCH 09/15] [RESTRICT AUTOMERGE]: Exclude secure layers from most screenshots taken by the system server. In pre-P versions of Android, it was allowed to screenshot secure layers if the buffer queue producer which was the target of the screenshot was owned by the system (in this case SurfaceFlinger). This really was a synonym for: The screen rotation animation was allowed to capture secure layers, but the other code paths weren't. In O we mistakenly changed this check to always allow the system server to capture secure layers via the captureScreen path (the captureLayers path used for TaskSnapshots was unaffected). This can result in data leakage in cases where the system server takes screenshots on behalf of other parts of the system (e.g. for the assistant). To mitigate this we provide an explicit switch for the system server to specify whether it wishes to capture Secure layers. While this is dangerous, I think it is less dangerous than the previous implicit switch of capturing secure layers based on which type of BufferQueue was passed in. The flag defaults to not capturing secure layers and we set it to true in the one place we need it (for the screen rotation animation). Non privileged clients can still not capture secure layers at all directly. Test: TransactionTest.cpp#SetFlagsSecureEUidSystem Bug: 120610669 Change-Id: I9d32c5ac2b005059be9f464859a415167d9ddbd4 (cherry picked from commit dc49e0088a05108a0616704ca5565136f89c0a1f) --- core/java/android/view/SurfaceControl.java | 26 +++++++++++++++++-- core/jni/android_view_SurfaceControl.cpp | 6 ++--- .../server/wm/ScreenRotationAnimation.java | 20 +++++++++++--- 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java index ed8b0053e5bfe..eec34cdc12cc8 100644 --- a/core/java/android/view/SurfaceControl.java +++ b/core/java/android/view/SurfaceControl.java @@ -74,7 +74,8 @@ public class SurfaceControl implements Parcelable { boolean allLayers, boolean useIdentityTransform, int rotation); private static native GraphicBuffer nativeScreenshotToBuffer(IBinder displayToken, Rect sourceCrop, int width, int height, int minLayer, int maxLayer, - boolean allLayers, boolean useIdentityTransform, int rotation); + boolean allLayers, boolean useIdentityTransform, int rotation, + boolean captureSecureLayers); private static native void nativeScreenshot(IBinder displayToken, Surface consumer, Rect sourceCrop, int width, int height, int minLayer, int maxLayer, boolean allLayers, boolean useIdentityTransform); @@ -1249,7 +1250,28 @@ public class SurfaceControl implements Parcelable { IBinder displayToken = SurfaceControl.getBuiltInDisplay( SurfaceControl.BUILT_IN_DISPLAY_ID_MAIN); return nativeScreenshotToBuffer(displayToken, sourceCrop, width, height, - minLayer, maxLayer, false, useIdentityTransform, rotation); + minLayer, maxLayer, false, useIdentityTransform, rotation, + false /* captureSecureLayers */); + } + + /** + * Like screenshotToBuffer, but if the caller is AID_SYSTEM, allows + * for the capture of secure layers. This is used for the screen rotation + * animation where the system server takes screenshots but does + * not persist them or allow them to leave the server. However in other + * cases in the system server, we mostly want to omit secure layers + * like when we take a screenshot on behalf of the assistant. + * + * @hide + */ + public static GraphicBuffer screenshotToBufferWithSecureLayersUnsafe(Rect sourceCrop, + int width, int height, int minLayer, int maxLayer, boolean useIdentityTransform, + int rotation) { + IBinder displayToken = SurfaceControl.getBuiltInDisplay( + SurfaceControl.BUILT_IN_DISPLAY_ID_MAIN); + return nativeScreenshotToBuffer(displayToken, sourceCrop, width, height, + minLayer, maxLayer, false, useIdentityTransform, rotation, + true /* captureSecureLayers */); } /** diff --git a/core/jni/android_view_SurfaceControl.cpp b/core/jni/android_view_SurfaceControl.cpp index 5b4b5f2a2264d..1529a6bcf243f 100644 --- a/core/jni/android_view_SurfaceControl.cpp +++ b/core/jni/android_view_SurfaceControl.cpp @@ -160,7 +160,7 @@ static Rect rectFromObj(JNIEnv* env, jobject rectObj) { static jobject nativeScreenshotToBuffer(JNIEnv* env, jclass clazz, jobject displayTokenObj, jobject sourceCropObj, jint width, jint height, jint minLayer, jint maxLayer, bool allLayers, bool useIdentityTransform, - int rotation) { + int rotation, bool captureSecureLayers) { sp displayToken = ibinderForJavaObject(env, displayTokenObj); if (displayToken == NULL) { return NULL; @@ -173,7 +173,7 @@ static jobject nativeScreenshotToBuffer(JNIEnv* env, jclass clazz, sp buffer; status_t res = ScreenshotClient::capture(displayToken, sourceCrop, width, height, minLayer, maxLayer, useIdentityTransform, - rotation, &buffer); + rotation, captureSecureLayers, &buffer); if (res != NO_ERROR) { return NULL; } @@ -1026,7 +1026,7 @@ static const JNINativeMethod sSurfaceControlMethods[] = { {"nativeGetHandle", "(J)Landroid/os/IBinder;", (void*)nativeGetHandle }, {"nativeScreenshotToBuffer", - "(Landroid/os/IBinder;Landroid/graphics/Rect;IIIIZZI)Landroid/graphics/GraphicBuffer;", + "(Landroid/os/IBinder;Landroid/graphics/Rect;IIIIZZIZ)Landroid/graphics/GraphicBuffer;", (void*)nativeScreenshotToBuffer }, {"nativeCaptureLayers", "(Landroid/os/IBinder;Landroid/graphics/Rect;F)Landroid/graphics/GraphicBuffer;", (void*)nativeCaptureLayers }, diff --git a/services/core/java/com/android/server/wm/ScreenRotationAnimation.java b/services/core/java/com/android/server/wm/ScreenRotationAnimation.java index 755a571cf5f75..498cda11c1b4d 100644 --- a/services/core/java/com/android/server/wm/ScreenRotationAnimation.java +++ b/services/core/java/com/android/server/wm/ScreenRotationAnimation.java @@ -27,6 +27,7 @@ import static com.android.server.wm.ScreenRotationAnimationProto.ANIMATION_RUNNI import static com.android.server.wm.ScreenRotationAnimationProto.STARTED; import android.content.Context; +import android.graphics.GraphicBuffer; import android.graphics.Matrix; import android.graphics.Rect; import android.os.IBinder; @@ -285,10 +286,21 @@ class ScreenRotationAnimation { if (displayHandle != null) { Surface sur = new Surface(); sur.copyFrom(mSurfaceControl); - SurfaceControl.screenshot(displayHandle, sur); - t.setLayer(mSurfaceControl, SCREEN_FREEZE_LAYER_SCREENSHOT); - t.setAlpha(mSurfaceControl, 0); - t.show(mSurfaceControl); + GraphicBuffer gb = SurfaceControl.screenshotToBufferWithSecureLayersUnsafe( + new Rect(), 0 /* width */, 0 /* height */, 0 /* minLayer */, + 0 /* maxLayer */, false /* useIdentityTransform */, 0 /* rotation */); + if (gb != null) { + try { + sur.attachAndQueueBuffer(gb); + } catch (RuntimeException e) { + Slog.w(TAG, "Failed to attach screenshot - " + e.getMessage()); + } + t.setLayer(mSurfaceControl, SCREEN_FREEZE_LAYER_SCREENSHOT); + t.setAlpha(mSurfaceControl, 0); + t.show(mSurfaceControl); + } else { + Slog.w(TAG, "Unable to take screenshot of display " + displayId); + } sur.destroy(); } else { Slog.w(TAG, "Built-in display " + displayId + " is null."); From 3256e59a87466b1a1d20a6cfe22617e23fa25299 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Thu, 18 Apr 2019 16:32:42 -0700 Subject: [PATCH 10/15] HwBlob: s/malloc/calloc/ Since this blob is passed between processes. We could potentially only memset portions of the blob as it is written to. However, the JHwBlob API itself doesn't have to have writes in order (even though known usages of it do write in order). Because of this, keeping track of which bytes to pad would be too expensive. Bug: 131356202 Test: boot, hidl_test_java Change-Id: I48f4d7cb20c4bfe747dd323ae3744d323ad097c9 Merged-In: I48f4d7cb20c4bfe747dd323ae3744d323ad097c9 (cherry picked from commit d8157bc094569bee74976df2585d632f1793e226) --- core/jni/android_os_HwBlob.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/jni/android_os_HwBlob.cpp b/core/jni/android_os_HwBlob.cpp index bb916d2431c5b..09589cdb08011 100644 --- a/core/jni/android_os_HwBlob.cpp +++ b/core/jni/android_os_HwBlob.cpp @@ -85,7 +85,7 @@ JHwBlob::JHwBlob(JNIEnv *env, jobject thiz, size_t size) mOwnsBuffer(true), mHandle(0) { if (size > 0) { - mBuffer = malloc(size); + mBuffer = calloc(size, 1); } } From da7203b66876a680fad56a5aafe3d84ae8354d4f Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Tue, 2 Apr 2019 14:18:56 -0700 Subject: [PATCH 11/15] [RESTRICT AUTOMERGE] Careful with screenshots containing secure layers! For purposes of the screen rotation animation the system server is allowed to capture secure (not protected) layers and trusted not to persist screenshots which may contain secure layers. However when displaying the screen rotation animation, the layer the screenshot is placed on will itself not be secure, so if we record the animation the recording will contain persisted versions of the secure content. Make sure we use the new API from SurfaceFlinger to set FLAG_SECURE if our screenshot contains secure content. Bug: 69703445 Test: Transaction_test#SetFlagsSecureEUidSystem Change-Id: I0dd36462867da52e6b1451f65f56c2c5d37538f3 (cherry picked from commit bab740f10e0812ba47d19931fdfe2fa7e02bbd0c) --- core/jni/android_view_SurfaceControl.cpp | 8 ++-- .../java/android/graphics/GraphicBuffer.java | 37 +++++++++++++++++-- .../server/wm/ScreenRotationAnimation.java | 6 +++ 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/core/jni/android_view_SurfaceControl.cpp b/core/jni/android_view_SurfaceControl.cpp index 1529a6bcf243f..614a8ff124ea1 100644 --- a/core/jni/android_view_SurfaceControl.cpp +++ b/core/jni/android_view_SurfaceControl.cpp @@ -171,9 +171,10 @@ static jobject nativeScreenshotToBuffer(JNIEnv* env, jclass clazz, maxLayer = INT32_MAX; } sp buffer; + bool capturedSecureLayers = false; status_t res = ScreenshotClient::capture(displayToken, sourceCrop, width, height, minLayer, maxLayer, useIdentityTransform, - rotation, captureSecureLayers, &buffer); + rotation, captureSecureLayers, &buffer, capturedSecureLayers); if (res != NO_ERROR) { return NULL; } @@ -184,7 +185,8 @@ static jobject nativeScreenshotToBuffer(JNIEnv* env, jclass clazz, buffer->getHeight(), buffer->getPixelFormat(), (jint)buffer->getUsage(), - (jlong)buffer.get()); + (jlong)buffer.get(), + capturedSecureLayers); } static jobject nativeScreenshotBitmap(JNIEnv* env, jclass clazz, @@ -1082,7 +1084,7 @@ int register_android_view_SurfaceControl(JNIEnv* env) jclass graphicsBufferClazz = FindClassOrDie(env, "android/graphics/GraphicBuffer"); gGraphicBufferClassInfo.clazz = MakeGlobalRefOrDie(env, graphicsBufferClazz); gGraphicBufferClassInfo.builder = GetStaticMethodIDOrDie(env, graphicsBufferClazz, - "createFromExisting", "(IIIIJ)Landroid/graphics/GraphicBuffer;"); + "createFromExisting", "(IIIIJZ)Landroid/graphics/GraphicBuffer;"); return err; } diff --git a/graphics/java/android/graphics/GraphicBuffer.java b/graphics/java/android/graphics/GraphicBuffer.java index 53d21776eed7e..61dd37fb0581a 100644 --- a/graphics/java/android/graphics/GraphicBuffer.java +++ b/graphics/java/android/graphics/GraphicBuffer.java @@ -52,6 +52,7 @@ public class GraphicBuffer implements Parcelable { private final int mHeight; private final int mFormat; private final int mUsage; + private final boolean mCapturedSecureLayers; // Note: do not rename, this field is used by native code private final long mNativeObject; @@ -82,14 +83,23 @@ public class GraphicBuffer implements Parcelable { } /** - * Private use only. See {@link #create(int, int, int, int)}. + * Private use only. See {@link #create(int, int, int, int, boolean)}. */ - private GraphicBuffer(int width, int height, int format, int usage, long nativeObject) { + private GraphicBuffer(int width, int height, int format, int usage, long nativeObject, + boolean capturedSecureLayers) { mWidth = width; mHeight = height; mFormat = format; mUsage = usage; mNativeObject = nativeObject; + mCapturedSecureLayers = capturedSecureLayers; + } + + /** + * Private use only. See {@link #create(int, int, int, int)}. + */ + private GraphicBuffer(int width, int height, int format, int usage, long nativeObject) { + this(width, height, format, usage, nativeObject, false); } /** @@ -97,14 +107,33 @@ public class GraphicBuffer implements Parcelable { * @hide */ public static GraphicBuffer createFromExisting(int width, int height, - int format, int usage, long unwrappedNativeObject) { + int format, int usage, long unwrappedNativeObject, + boolean capturedSecureLayers) { long nativeObject = nWrapGraphicBuffer(unwrappedNativeObject); if (nativeObject != 0) { - return new GraphicBuffer(width, height, format, usage, nativeObject); + return new GraphicBuffer(width, height, format, usage, nativeObject, + capturedSecureLayers); } return null; } + /** + * For SurfaceControl JNI. Provides and ignored value for capturedSecureLayers for backwards + * compatibility + * @hide + */ + public static GraphicBuffer createFromExisting(int width, int height, + int format, int usage, long unwrappedNativeObject) { + return createFromExisting(width, height, format, usage, unwrappedNativeObject, false); + } + + /** + * Returns true if the buffer contains visible secure layers. + */ + public boolean doesContainSecureLayers() { + return mCapturedSecureLayers; + } + /** * Returns the width of this buffer in pixels. */ diff --git a/services/core/java/com/android/server/wm/ScreenRotationAnimation.java b/services/core/java/com/android/server/wm/ScreenRotationAnimation.java index 498cda11c1b4d..95051dea2e9a3 100644 --- a/services/core/java/com/android/server/wm/ScreenRotationAnimation.java +++ b/services/core/java/com/android/server/wm/ScreenRotationAnimation.java @@ -295,6 +295,12 @@ class ScreenRotationAnimation { } catch (RuntimeException e) { Slog.w(TAG, "Failed to attach screenshot - " + e.getMessage()); } + // If the screenshot contains secure layers, we have to make sure the + // screenshot surface we display it in also has FLAG_SECURE so that + // the user can not screenshot secure layers via the screenshot surface. + if (gb.doesContainSecureLayers()) { + t.setSecure(mSurfaceControl, true); + } t.setLayer(mSurfaceControl, SCREEN_FREEZE_LAYER_SCREENSHOT); t.setAlpha(mSurfaceControl, 0); t.show(mSurfaceControl); From 5fc1b18b7fcc8c556f745ce1858a4cb9e99a1888 Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Fri, 15 Feb 2019 15:48:11 -0800 Subject: [PATCH 12/15] [RESTRICT AUTOMERGE]: Exclude secure layers from most screenshots taken by the system server. In pre-P versions of Android, it was allowed to screenshot secure layers if the buffer queue producer which was the target of the screenshot was owned by the system (in this case SurfaceFlinger). This really was a synonym for: The screen rotation animation was allowed to capture secure layers, but the other code paths weren't. In O we mistakenly changed this check to always allow the system server to capture secure layers via the captureScreen path (the captureLayers path used for TaskSnapshots was unaffected). This can result in data leakage in cases where the system server takes screenshots on behalf of other parts of the system (e.g. for the assistant). To mitigate this we provide an explicit switch for the system server to specify whether it wishes to capture Secure layers. While this is dangerous, I think it is less dangerous than the previous implicit switch of capturing secure layers based on which type of BufferQueue was passed in. The flag defaults to not capturing secure layers and we set it to true in the one place we need it (for the screen rotation animation). Non privileged clients can still not capture secure layers at all directly. Test: TransactionTest.cpp#SetFlagsSecureEUidSystem Bug: 120610669 Change-Id: I9d32c5ac2b005059be9f464859a415167d9ddbd4 (cherry picked from commit dc49e0088a05108a0616704ca5565136f89c0a1f) --- core/java/android/view/SurfaceControl.java | 26 +++++++++++++++++-- core/jni/android_view_SurfaceControl.cpp | 6 ++--- .../server/wm/ScreenRotationAnimation.java | 20 +++++++++++--- 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java index ed8b0053e5bfe..eec34cdc12cc8 100644 --- a/core/java/android/view/SurfaceControl.java +++ b/core/java/android/view/SurfaceControl.java @@ -74,7 +74,8 @@ public class SurfaceControl implements Parcelable { boolean allLayers, boolean useIdentityTransform, int rotation); private static native GraphicBuffer nativeScreenshotToBuffer(IBinder displayToken, Rect sourceCrop, int width, int height, int minLayer, int maxLayer, - boolean allLayers, boolean useIdentityTransform, int rotation); + boolean allLayers, boolean useIdentityTransform, int rotation, + boolean captureSecureLayers); private static native void nativeScreenshot(IBinder displayToken, Surface consumer, Rect sourceCrop, int width, int height, int minLayer, int maxLayer, boolean allLayers, boolean useIdentityTransform); @@ -1249,7 +1250,28 @@ public class SurfaceControl implements Parcelable { IBinder displayToken = SurfaceControl.getBuiltInDisplay( SurfaceControl.BUILT_IN_DISPLAY_ID_MAIN); return nativeScreenshotToBuffer(displayToken, sourceCrop, width, height, - minLayer, maxLayer, false, useIdentityTransform, rotation); + minLayer, maxLayer, false, useIdentityTransform, rotation, + false /* captureSecureLayers */); + } + + /** + * Like screenshotToBuffer, but if the caller is AID_SYSTEM, allows + * for the capture of secure layers. This is used for the screen rotation + * animation where the system server takes screenshots but does + * not persist them or allow them to leave the server. However in other + * cases in the system server, we mostly want to omit secure layers + * like when we take a screenshot on behalf of the assistant. + * + * @hide + */ + public static GraphicBuffer screenshotToBufferWithSecureLayersUnsafe(Rect sourceCrop, + int width, int height, int minLayer, int maxLayer, boolean useIdentityTransform, + int rotation) { + IBinder displayToken = SurfaceControl.getBuiltInDisplay( + SurfaceControl.BUILT_IN_DISPLAY_ID_MAIN); + return nativeScreenshotToBuffer(displayToken, sourceCrop, width, height, + minLayer, maxLayer, false, useIdentityTransform, rotation, + true /* captureSecureLayers */); } /** diff --git a/core/jni/android_view_SurfaceControl.cpp b/core/jni/android_view_SurfaceControl.cpp index 5b4b5f2a2264d..1529a6bcf243f 100644 --- a/core/jni/android_view_SurfaceControl.cpp +++ b/core/jni/android_view_SurfaceControl.cpp @@ -160,7 +160,7 @@ static Rect rectFromObj(JNIEnv* env, jobject rectObj) { static jobject nativeScreenshotToBuffer(JNIEnv* env, jclass clazz, jobject displayTokenObj, jobject sourceCropObj, jint width, jint height, jint minLayer, jint maxLayer, bool allLayers, bool useIdentityTransform, - int rotation) { + int rotation, bool captureSecureLayers) { sp displayToken = ibinderForJavaObject(env, displayTokenObj); if (displayToken == NULL) { return NULL; @@ -173,7 +173,7 @@ static jobject nativeScreenshotToBuffer(JNIEnv* env, jclass clazz, sp buffer; status_t res = ScreenshotClient::capture(displayToken, sourceCrop, width, height, minLayer, maxLayer, useIdentityTransform, - rotation, &buffer); + rotation, captureSecureLayers, &buffer); if (res != NO_ERROR) { return NULL; } @@ -1026,7 +1026,7 @@ static const JNINativeMethod sSurfaceControlMethods[] = { {"nativeGetHandle", "(J)Landroid/os/IBinder;", (void*)nativeGetHandle }, {"nativeScreenshotToBuffer", - "(Landroid/os/IBinder;Landroid/graphics/Rect;IIIIZZI)Landroid/graphics/GraphicBuffer;", + "(Landroid/os/IBinder;Landroid/graphics/Rect;IIIIZZIZ)Landroid/graphics/GraphicBuffer;", (void*)nativeScreenshotToBuffer }, {"nativeCaptureLayers", "(Landroid/os/IBinder;Landroid/graphics/Rect;F)Landroid/graphics/GraphicBuffer;", (void*)nativeCaptureLayers }, diff --git a/services/core/java/com/android/server/wm/ScreenRotationAnimation.java b/services/core/java/com/android/server/wm/ScreenRotationAnimation.java index 755a571cf5f75..498cda11c1b4d 100644 --- a/services/core/java/com/android/server/wm/ScreenRotationAnimation.java +++ b/services/core/java/com/android/server/wm/ScreenRotationAnimation.java @@ -27,6 +27,7 @@ import static com.android.server.wm.ScreenRotationAnimationProto.ANIMATION_RUNNI import static com.android.server.wm.ScreenRotationAnimationProto.STARTED; import android.content.Context; +import android.graphics.GraphicBuffer; import android.graphics.Matrix; import android.graphics.Rect; import android.os.IBinder; @@ -285,10 +286,21 @@ class ScreenRotationAnimation { if (displayHandle != null) { Surface sur = new Surface(); sur.copyFrom(mSurfaceControl); - SurfaceControl.screenshot(displayHandle, sur); - t.setLayer(mSurfaceControl, SCREEN_FREEZE_LAYER_SCREENSHOT); - t.setAlpha(mSurfaceControl, 0); - t.show(mSurfaceControl); + GraphicBuffer gb = SurfaceControl.screenshotToBufferWithSecureLayersUnsafe( + new Rect(), 0 /* width */, 0 /* height */, 0 /* minLayer */, + 0 /* maxLayer */, false /* useIdentityTransform */, 0 /* rotation */); + if (gb != null) { + try { + sur.attachAndQueueBuffer(gb); + } catch (RuntimeException e) { + Slog.w(TAG, "Failed to attach screenshot - " + e.getMessage()); + } + t.setLayer(mSurfaceControl, SCREEN_FREEZE_LAYER_SCREENSHOT); + t.setAlpha(mSurfaceControl, 0); + t.show(mSurfaceControl); + } else { + Slog.w(TAG, "Unable to take screenshot of display " + displayId); + } sur.destroy(); } else { Slog.w(TAG, "Built-in display " + displayId + " is null."); From bbcbe0e05ec001d408d1362f893af1b15dc03c83 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Thu, 18 Apr 2019 16:32:42 -0700 Subject: [PATCH 13/15] HwBlob: s/malloc/calloc/ Since this blob is passed between processes. We could potentially only memset portions of the blob as it is written to. However, the JHwBlob API itself doesn't have to have writes in order (even though known usages of it do write in order). Because of this, keeping track of which bytes to pad would be too expensive. Bug: 131356202 Test: boot, hidl_test_java Change-Id: I48f4d7cb20c4bfe747dd323ae3744d323ad097c9 Merged-In: I48f4d7cb20c4bfe747dd323ae3744d323ad097c9 (cherry picked from commit d8157bc094569bee74976df2585d632f1793e226) --- core/jni/android_os_HwBlob.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/jni/android_os_HwBlob.cpp b/core/jni/android_os_HwBlob.cpp index bb916d2431c5b..09589cdb08011 100644 --- a/core/jni/android_os_HwBlob.cpp +++ b/core/jni/android_os_HwBlob.cpp @@ -85,7 +85,7 @@ JHwBlob::JHwBlob(JNIEnv *env, jobject thiz, size_t size) mOwnsBuffer(true), mHandle(0) { if (size > 0) { - mBuffer = malloc(size); + mBuffer = calloc(size, 1); } } From b37e7650b0951d1ee5cb97269c079557e2cf4626 Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Tue, 2 Apr 2019 14:18:56 -0700 Subject: [PATCH 14/15] [RESTRICT AUTOMERGE] Careful with screenshots containing secure layers! For purposes of the screen rotation animation the system server is allowed to capture secure (not protected) layers and trusted not to persist screenshots which may contain secure layers. However when displaying the screen rotation animation, the layer the screenshot is placed on will itself not be secure, so if we record the animation the recording will contain persisted versions of the secure content. Make sure we use the new API from SurfaceFlinger to set FLAG_SECURE if our screenshot contains secure content. Bug: 69703445 Test: Transaction_test#SetFlagsSecureEUidSystem Change-Id: I0dd36462867da52e6b1451f65f56c2c5d37538f3 (cherry picked from commit bab740f10e0812ba47d19931fdfe2fa7e02bbd0c) --- core/jni/android_view_SurfaceControl.cpp | 8 ++-- .../java/android/graphics/GraphicBuffer.java | 37 +++++++++++++++++-- .../server/wm/ScreenRotationAnimation.java | 6 +++ 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/core/jni/android_view_SurfaceControl.cpp b/core/jni/android_view_SurfaceControl.cpp index 1529a6bcf243f..614a8ff124ea1 100644 --- a/core/jni/android_view_SurfaceControl.cpp +++ b/core/jni/android_view_SurfaceControl.cpp @@ -171,9 +171,10 @@ static jobject nativeScreenshotToBuffer(JNIEnv* env, jclass clazz, maxLayer = INT32_MAX; } sp buffer; + bool capturedSecureLayers = false; status_t res = ScreenshotClient::capture(displayToken, sourceCrop, width, height, minLayer, maxLayer, useIdentityTransform, - rotation, captureSecureLayers, &buffer); + rotation, captureSecureLayers, &buffer, capturedSecureLayers); if (res != NO_ERROR) { return NULL; } @@ -184,7 +185,8 @@ static jobject nativeScreenshotToBuffer(JNIEnv* env, jclass clazz, buffer->getHeight(), buffer->getPixelFormat(), (jint)buffer->getUsage(), - (jlong)buffer.get()); + (jlong)buffer.get(), + capturedSecureLayers); } static jobject nativeScreenshotBitmap(JNIEnv* env, jclass clazz, @@ -1082,7 +1084,7 @@ int register_android_view_SurfaceControl(JNIEnv* env) jclass graphicsBufferClazz = FindClassOrDie(env, "android/graphics/GraphicBuffer"); gGraphicBufferClassInfo.clazz = MakeGlobalRefOrDie(env, graphicsBufferClazz); gGraphicBufferClassInfo.builder = GetStaticMethodIDOrDie(env, graphicsBufferClazz, - "createFromExisting", "(IIIIJ)Landroid/graphics/GraphicBuffer;"); + "createFromExisting", "(IIIIJZ)Landroid/graphics/GraphicBuffer;"); return err; } diff --git a/graphics/java/android/graphics/GraphicBuffer.java b/graphics/java/android/graphics/GraphicBuffer.java index 53d21776eed7e..61dd37fb0581a 100644 --- a/graphics/java/android/graphics/GraphicBuffer.java +++ b/graphics/java/android/graphics/GraphicBuffer.java @@ -52,6 +52,7 @@ public class GraphicBuffer implements Parcelable { private final int mHeight; private final int mFormat; private final int mUsage; + private final boolean mCapturedSecureLayers; // Note: do not rename, this field is used by native code private final long mNativeObject; @@ -82,14 +83,23 @@ public class GraphicBuffer implements Parcelable { } /** - * Private use only. See {@link #create(int, int, int, int)}. + * Private use only. See {@link #create(int, int, int, int, boolean)}. */ - private GraphicBuffer(int width, int height, int format, int usage, long nativeObject) { + private GraphicBuffer(int width, int height, int format, int usage, long nativeObject, + boolean capturedSecureLayers) { mWidth = width; mHeight = height; mFormat = format; mUsage = usage; mNativeObject = nativeObject; + mCapturedSecureLayers = capturedSecureLayers; + } + + /** + * Private use only. See {@link #create(int, int, int, int)}. + */ + private GraphicBuffer(int width, int height, int format, int usage, long nativeObject) { + this(width, height, format, usage, nativeObject, false); } /** @@ -97,14 +107,33 @@ public class GraphicBuffer implements Parcelable { * @hide */ public static GraphicBuffer createFromExisting(int width, int height, - int format, int usage, long unwrappedNativeObject) { + int format, int usage, long unwrappedNativeObject, + boolean capturedSecureLayers) { long nativeObject = nWrapGraphicBuffer(unwrappedNativeObject); if (nativeObject != 0) { - return new GraphicBuffer(width, height, format, usage, nativeObject); + return new GraphicBuffer(width, height, format, usage, nativeObject, + capturedSecureLayers); } return null; } + /** + * For SurfaceControl JNI. Provides and ignored value for capturedSecureLayers for backwards + * compatibility + * @hide + */ + public static GraphicBuffer createFromExisting(int width, int height, + int format, int usage, long unwrappedNativeObject) { + return createFromExisting(width, height, format, usage, unwrappedNativeObject, false); + } + + /** + * Returns true if the buffer contains visible secure layers. + */ + public boolean doesContainSecureLayers() { + return mCapturedSecureLayers; + } + /** * Returns the width of this buffer in pixels. */ diff --git a/services/core/java/com/android/server/wm/ScreenRotationAnimation.java b/services/core/java/com/android/server/wm/ScreenRotationAnimation.java index 498cda11c1b4d..95051dea2e9a3 100644 --- a/services/core/java/com/android/server/wm/ScreenRotationAnimation.java +++ b/services/core/java/com/android/server/wm/ScreenRotationAnimation.java @@ -295,6 +295,12 @@ class ScreenRotationAnimation { } catch (RuntimeException e) { Slog.w(TAG, "Failed to attach screenshot - " + e.getMessage()); } + // If the screenshot contains secure layers, we have to make sure the + // screenshot surface we display it in also has FLAG_SECURE so that + // the user can not screenshot secure layers via the screenshot surface. + if (gb.doesContainSecureLayers()) { + t.setSecure(mSurfaceControl, true); + } t.setLayer(mSurfaceControl, SCREEN_FREEZE_LAYER_SCREENSHOT); t.setAlpha(mSurfaceControl, 0); t.show(mSurfaceControl); From e049f13eb4169492e826fd2b99f94d6969021b0f Mon Sep 17 00:00:00 2001 From: Daichi Hirono Date: Mon, 25 Jun 2018 11:16:41 +0900 Subject: [PATCH 15/15] Stop invoke initAppOps in Camera default constructor. Camera default constructor does not create the underlying native camera object. Thus calling _enableShutterSound after the default constuctor causes application crash. Bug: 132362603 Bug: 80498247 Test: ARC++ Eve/Kevin: Manually modify the code to return MODE_IGNORED for AppOpsService#checkAudioOperation() and to return false for CameraManager#supportsCamera2ApiLocked. Then start voice call in Hangouts. cf_x86_phone with camera HALv1 enabled: New CTS test Change-Id: Ia232dd71da15fda31c4dbe5568ba5083cbfaed9b Merged-In: Id738c4d46a8e3625bc3b1142b11acac9cfb0b603 (cherry picked from commit 97999565311a909a75b6214cb5076f2f7a067e16) --- core/java/android/hardware/Camera.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/java/android/hardware/Camera.java b/core/java/android/hardware/Camera.java index 2f8463629fe26..d17223671ed56 100644 --- a/core/java/android/hardware/Camera.java +++ b/core/java/android/hardware/Camera.java @@ -571,9 +571,7 @@ public class Camera { /** * An empty Camera for testing purpose. */ - Camera() { - initAppOps(); - } + Camera() {} private void initAppOps() { IBinder b = ServiceManager.getService(Context.APP_OPS_SERVICE);