From 0f9c2ffc728d71eec096b9d513df88ca52de70e6 Mon Sep 17 00:00:00 2001 From: Rubin Xu Date: Wed, 14 Aug 2019 16:25:57 +0100 Subject: [PATCH 1/2] Clean up exception usage in LockSettingsService (part 2) Replace RuntimeException which cannot propagate across binder to binder-compatible IllegalStateException/IllegalArgumentException Bug: 128831839 Test: atest frameworks/base/services/tests/servicestests/src/com/android/server/locksettings Change-Id: I2e88997acb27a7b97eafa7a7de441ac641544ddd --- .../locksettings/LockSettingsService.java | 8 +++--- .../locksettings/LockSettingsStorage.java | 6 ++-- .../locksettings/PasswordSlotManager.java | 4 +-- .../locksettings/SyntheticPasswordCrypto.java | 28 ++++++++++--------- .../SyntheticPasswordManager.java | 16 +++++------ 5 files changed, 32 insertions(+), 30 deletions(-) diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 00d39e2d8f481..ebe5a1dc23587 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -1614,17 +1614,17 @@ public class LockSettingsService extends ILockSettings.Stub { } catch (CertificateException | UnrecoverableKeyException | IOException | BadPaddingException | IllegalBlockSizeException | KeyStoreException | NoSuchPaddingException | NoSuchAlgorithmException | InvalidKeyException e) { - throw new RuntimeException("Failed to encrypt key", e); + throw new IllegalStateException("Failed to encrypt key", e); } ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); try { if (iv.length != PROFILE_KEY_IV_SIZE) { - throw new RuntimeException("Invalid iv length: " + iv.length); + throw new IllegalArgumentException("Invalid iv length: " + iv.length); } outputStream.write(iv); outputStream.write(encryptionResult); } catch (IOException e) { - throw new RuntimeException("Failed to concatenate byte arrays", e); + throw new IllegalStateException("Failed to concatenate byte arrays", e); } mStorage.writeChildProfileLock(userId, outputStream.toByteArray()); } @@ -1692,7 +1692,7 @@ public class LockSettingsService extends ILockSettings.Stub { digest.update(credential); return digest.digest(); } catch (NoSuchAlgorithmException e) { - throw new RuntimeException("NoSuchAlgorithmException for SHA-512"); + throw new IllegalStateException("NoSuchAlgorithmException for SHA-512"); } } diff --git a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java index 29b8aa2e12f4f..ba2ebc8f51b20 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java @@ -140,7 +140,7 @@ class LockSettingsStorage { dos.close(); return os.toByteArray(); } catch (IOException e) { - throw new RuntimeException(e); + throw new IllegalStateException("Fail to serialze credential hash", e); } } @@ -157,7 +157,7 @@ class LockSettingsStorage { } return new CredentialHash(hash, type); } catch (IOException e) { - throw new RuntimeException(e); + throw new IllegalStateException("Fail to deserialze credential hash", e); } } } @@ -666,7 +666,7 @@ class LockSettingsStorage { dos.writeInt(qualityForUi); dos.write(payload); } catch (IOException e) { - throw new RuntimeException("ByteArrayOutputStream cannot throw IOException"); + throw new IllegalStateException("ByteArrayOutputStream cannot throw IOException"); } return os.toByteArray(); } diff --git a/services/core/java/com/android/server/locksettings/PasswordSlotManager.java b/services/core/java/com/android/server/locksettings/PasswordSlotManager.java index 4ef63c05325c4..17aca1576e400 100644 --- a/services/core/java/com/android/server/locksettings/PasswordSlotManager.java +++ b/services/core/java/com/android/server/locksettings/PasswordSlotManager.java @@ -109,7 +109,7 @@ public class PasswordSlotManager { public void markSlotInUse(int slot) throws RuntimeException { ensureSlotMapLoaded(); if (mSlotMap.containsKey(slot) && !mSlotMap.get(slot).equals(getMode())) { - throw new RuntimeException("password slot " + slot + " is not available"); + throw new IllegalStateException("password slot " + slot + " is not available"); } mSlotMap.put(slot, getMode()); saveSlotMap(); @@ -123,7 +123,7 @@ public class PasswordSlotManager { public void markSlotDeleted(int slot) throws RuntimeException { ensureSlotMapLoaded(); if (mSlotMap.containsKey(slot) && !mSlotMap.get(slot).equals(getMode())) { - throw new RuntimeException("password slot " + slot + " cannot be deleted"); + throw new IllegalStateException("password slot " + slot + " cannot be deleted"); } mSlotMap.remove(slot); saveSlotMap(); diff --git a/services/core/java/com/android/server/locksettings/SyntheticPasswordCrypto.java b/services/core/java/com/android/server/locksettings/SyntheticPasswordCrypto.java index 388e51f203cac..ea0fb47a49b30 100644 --- a/services/core/java/com/android/server/locksettings/SyntheticPasswordCrypto.java +++ b/services/core/java/com/android/server/locksettings/SyntheticPasswordCrypto.java @@ -18,6 +18,7 @@ package com.android.server.locksettings; import android.security.keystore.KeyProperties; import android.security.keystore.KeyProtection; +import android.util.Slog; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -43,6 +44,7 @@ import javax.crypto.spec.GCMParameterSpec; import javax.crypto.spec.SecretKeySpec; public class SyntheticPasswordCrypto { + private static final String TAG = "SyntheticPasswordCrypto"; private static final int PROFILE_KEY_IV_SIZE = 12; private static final int DEFAULT_TAG_LENGTH_BITS = 128; private static final int AES_KEY_LENGTH = 32; // 256-bit AES key @@ -80,12 +82,12 @@ public class SyntheticPasswordCrypto { byte[] ciphertext = cipher.doFinal(blob); byte[] iv = cipher.getIV(); if (iv.length != PROFILE_KEY_IV_SIZE) { - throw new RuntimeException("Invalid iv length: " + iv.length); + throw new IllegalArgumentException("Invalid iv length: " + iv.length); } final GCMParameterSpec spec = cipher.getParameters().getParameterSpec( GCMParameterSpec.class); if (spec.getTLen() != DEFAULT_TAG_LENGTH_BITS) { - throw new RuntimeException("Invalid tag length: " + spec.getTLen()); + throw new IllegalArgumentException("Invalid tag length: " + spec.getTLen()); } ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); outputStream.write(iv); @@ -102,7 +104,7 @@ public class SyntheticPasswordCrypto { } catch (InvalidKeyException | NoSuchAlgorithmException | NoSuchPaddingException | IllegalBlockSizeException | BadPaddingException | IOException | InvalidParameterSpecException e) { - e.printStackTrace(); + Slog.e(TAG, "Failed to encrypt", e); return null; } } @@ -116,7 +118,7 @@ public class SyntheticPasswordCrypto { } catch (InvalidKeyException | NoSuchAlgorithmException | NoSuchPaddingException | IllegalBlockSizeException | BadPaddingException | InvalidAlgorithmParameterException e) { - e.printStackTrace(); + Slog.e(TAG, "Failed to decrypt", e); return null; } } @@ -130,8 +132,8 @@ public class SyntheticPasswordCrypto { byte[] intermediate = decrypt(applicationId, APPLICATION_ID_PERSONALIZATION, blob); return decrypt(decryptionKey, intermediate); } catch (Exception e) { - e.printStackTrace(); - throw new RuntimeException("Failed to decrypt blob", e); + Slog.e(TAG, "Failed to decrypt V1 blob", e); + throw new IllegalStateException("Failed to decrypt blob", e); } } @@ -148,8 +150,8 @@ public class SyntheticPasswordCrypto { | KeyStoreException | NoSuchPaddingException | NoSuchAlgorithmException | InvalidKeyException | UnrecoverableKeyException | InvalidAlgorithmParameterException e) { - e.printStackTrace(); - throw new RuntimeException("Failed to decrypt blob", e); + Slog.e(TAG, "Failed to decrypt blob", e); + throw new IllegalStateException("Failed to decrypt blob", e); } } @@ -180,8 +182,8 @@ public class SyntheticPasswordCrypto { | KeyStoreException | NoSuchPaddingException | NoSuchAlgorithmException | InvalidKeyException | InvalidParameterSpecException e) { - e.printStackTrace(); - throw new RuntimeException("Failed to encrypt blob", e); + Slog.e(TAG, "Failed to create blob", e); + throw new IllegalStateException("Failed to encrypt blob", e); } } @@ -193,7 +195,7 @@ public class SyntheticPasswordCrypto { keyStore.deleteEntry(keyAlias); } catch (KeyStoreException | NoSuchAlgorithmException | CertificateException | IOException e) { - e.printStackTrace(); + Slog.e(TAG, "Failed to destroy blob", e); } } @@ -202,7 +204,7 @@ public class SyntheticPasswordCrypto { final int PADDING_LENGTH = 128; MessageDigest digest = MessageDigest.getInstance("SHA-512"); if (personalisation.length > PADDING_LENGTH) { - throw new RuntimeException("Personalisation too long"); + throw new IllegalArgumentException("Personalisation too long"); } // Personalize the hash // Pad it to the block size of the hash function @@ -213,7 +215,7 @@ public class SyntheticPasswordCrypto { } return digest.digest(); } catch (NoSuchAlgorithmException e) { - throw new RuntimeException("NoSuchAlgorithmException for SHA-512", e); + throw new IllegalStateException("NoSuchAlgorithmException for SHA-512", e); } } } diff --git a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java index 9bf9ca8047fa8..955a9aa8d0de6 100644 --- a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java +++ b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java @@ -353,12 +353,12 @@ public class SyntheticPasswordManager { */ private byte[] weaverEnroll(int slot, byte[] key, @Nullable byte[] value) { if (slot == INVALID_WEAVER_SLOT || slot >= mWeaverConfig.slots) { - throw new RuntimeException("Invalid slot for weaver"); + throw new IllegalArgumentException("Invalid slot for weaver"); } if (key == null) { key = new byte[mWeaverConfig.keySize]; } else if (key.length != mWeaverConfig.keySize) { - throw new RuntimeException("Invalid key size for weaver"); + throw new IllegalArgumentException("Invalid key size for weaver"); } if (value == null) { value = secureRandom(mWeaverConfig.valueSize); @@ -383,12 +383,12 @@ public class SyntheticPasswordManager { */ private VerifyCredentialResponse weaverVerify(int slot, byte[] key) { if (slot == INVALID_WEAVER_SLOT || slot >= mWeaverConfig.slots) { - throw new RuntimeException("Invalid slot for weaver"); + throw new IllegalArgumentException("Invalid slot for weaver"); } if (key == null) { key = new byte[mWeaverConfig.keySize]; } else if (key.length != mWeaverConfig.keySize) { - throw new RuntimeException("Invalid key size for weaver"); + throw new IllegalArgumentException("Invalid key size for weaver"); } final VerifyCredentialResponse[] response = new VerifyCredentialResponse[1]; try { @@ -620,7 +620,7 @@ public class SyntheticPasswordManager { return i; } } - throw new RuntimeException("Run out of weaver slots."); + throw new IllegalStateException("Run out of weaver slots."); } /** @@ -1029,10 +1029,10 @@ public class SyntheticPasswordManager { if (version != SYNTHETIC_PASSWORD_VERSION_V3 && version != SYNTHETIC_PASSWORD_VERSION_V2 && version != SYNTHETIC_PASSWORD_VERSION_V1) { - throw new RuntimeException("Unknown blob version"); + throw new IllegalArgumentException("Unknown blob version"); } if (blob[1] != type) { - throw new RuntimeException("Invalid blob type"); + throw new IllegalArgumentException("Invalid blob type"); } final byte[] secret; if (version == SYNTHETIC_PASSWORD_VERSION_V1) { @@ -1237,7 +1237,7 @@ public class SyntheticPasswordManager { private byte[] passwordTokenToWeaverKey(byte[] token) { byte[] key = SyntheticPasswordCrypto.personalisedHash(PERSONALISATION_WEAVER_KEY, token); if (key.length < mWeaverConfig.keySize) { - throw new RuntimeException("weaver key length too small"); + throw new IllegalArgumentException("weaver key length too small"); } return Arrays.copyOf(key, mWeaverConfig.keySize); } From e2f925f280ee24e493d4f20e450610cf57a934f9 Mon Sep 17 00:00:00 2001 From: Rubin Xu Date: Wed, 14 Aug 2019 16:53:55 +0100 Subject: [PATCH 2/2] Dump some extra information from LockSettingsService To help diagnose device unlock problems. Things included: 1. Last password change time and previous SP handle 2. Current password type 3. Separate work challenge state 4. File metadata about the wrapped SP blob Bug: 62410009 Test: adb shell dumpsys lock_settings Change-Id: I8bb56554c9b26b4bc1dcd92df677b5190159fca7 --- .../locksettings/LockSettingsService.java | 55 ++++++++++++++++--- .../locksettings/LockSettingsStorage.java | 21 +++++++ 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index ebe5a1dc23587..f38f2f9a0de55 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -106,6 +106,7 @@ import com.android.internal.messages.nano.SystemMessageProto.SystemMessage; import com.android.internal.notification.SystemNotificationChannels; import com.android.internal.util.ArrayUtils; import com.android.internal.util.DumpUtils; +import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.Preconditions; import com.android.internal.widget.ICheckCredentialProgressCallback; import com.android.internal.widget.ILockSettings; @@ -137,8 +138,10 @@ import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.security.UnrecoverableKeyException; import java.security.cert.CertificateException; +import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Arrays; +import java.util.Date; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; @@ -169,6 +172,8 @@ public class LockSettingsService extends ILockSettings.Stub { private static final int PROFILE_KEY_IV_SIZE = 12; private static final String SEPARATE_PROFILE_CHALLENGE_KEY = "lockscreen.profilechallenge"; private static final int SYNTHETIC_PASSWORD_ENABLED_BY_DEFAULT = 1; + private static final String PREV_SYNTHETIC_PASSWORD_HANDLE_KEY = "prev-sp-handle"; + private static final String SYNTHETIC_PASSWORD_UPDATE_TIME_KEY = "sp-handle-ts"; // No challenge provided private static final int CHALLENGE_NONE = 0; @@ -2474,7 +2479,7 @@ public class LockSettingsService extends ILockSettings.Stub { gateKeeperClearSecureUserId(userId); } fixateNewestUserKeyAuth(userId); - setLong(SYNTHETIC_PASSWORD_HANDLE_KEY, handle, userId); + setSyntheticPasswordHandleLocked(handle, userId); return auth; } @@ -2483,6 +2488,14 @@ public class LockSettingsService extends ILockSettings.Stub { SyntheticPasswordManager.DEFAULT_HANDLE, userId); } + private void setSyntheticPasswordHandleLocked(long handle, int userId) { + final long oldHandle = getSyntheticPasswordHandleLocked(userId); + setLong(SYNTHETIC_PASSWORD_HANDLE_KEY, handle, userId); + setLong(PREV_SYNTHETIC_PASSWORD_HANDLE_KEY, oldHandle, userId); + setLong(SYNTHETIC_PASSWORD_UPDATE_TIME_KEY, System.currentTimeMillis(), userId); + + } + private boolean isSyntheticPasswordBasedCredentialLocked(int userId) { if (userId == USER_FRP) { final int type = mStorage.readPersistentDataBlock().type; @@ -2660,7 +2673,7 @@ public class LockSettingsService extends ILockSettings.Stub { unlockKeystore(auth.deriveKeyStorePassword(), userId); setKeystorePassword(null, userId); } - setLong(SYNTHETIC_PASSWORD_HANDLE_KEY, newHandle, userId); + setSyntheticPasswordHandleLocked(newHandle, userId); synchronizeUnifiedWorkChallengeForProfiles(userId, profilePasswords); notifyActivePasswordMetricsAvailable(credentialType, credential, userId); @@ -2928,29 +2941,57 @@ public class LockSettingsService extends ILockSettings.Stub { return true; } + static String timestampToString(long timestamp) { + return new SimpleDateFormat("yyyy-MM-dd HH:mm:ss").format(new Date(timestamp)); + } + @Override - protected void dump(FileDescriptor fd, PrintWriter pw, String[] args){ - if (!DumpUtils.checkDumpPermission(mContext, TAG, pw)) return; + protected void dump(FileDescriptor fd, PrintWriter printWriter, String[] args) { + if (!DumpUtils.checkDumpPermission(mContext, TAG, printWriter)) return; + IndentingPrintWriter pw = new IndentingPrintWriter(printWriter, " "); pw.println("Current lock settings service state:"); + pw.println(String.format("SP Enabled = %b", mLockPatternUtils.isSyntheticPasswordEnabled())); + pw.println(); + pw.println("User State:"); + pw.increaseIndent(); List users = mUserManager.getUsers(); for (int user = 0; user < users.size(); user++) { final int userId = users.get(user).id; - pw.println(" User " + userId); + pw.println("User " + userId); + pw.increaseIndent(); synchronized (mSpManager) { - pw.println(String.format(" SP Handle = %x", + pw.println(String.format("SP Handle: %x", getSyntheticPasswordHandleLocked(userId))); + pw.println(String.format("Last changed: %s (%x)", + timestampToString(getLong(SYNTHETIC_PASSWORD_UPDATE_TIME_KEY, 0, userId)), + getLong(PREV_SYNTHETIC_PASSWORD_HANDLE_KEY, 0, userId))); } try { - pw.println(String.format(" SID = %x", + pw.println(String.format("SID: %x", getGateKeeperService().getSecureUserId(userId))); } catch (RemoteException e) { // ignore. } + // It's OK to dump the password type since anyone with physical access can just + // observe it from the keyguard directly. + pw.println("PasswordType: " + getLong(LockPatternUtils.PASSWORD_TYPE_KEY, 0, userId)); + pw.println("hasPassword: " + havePassword(userId)); + pw.println("hasPattern: " + havePattern(userId)); // print raw credential type instead? + pw.println("SeparateChallenge: " + getSeparateProfileChallengeEnabled(userId)); + pw.decreaseIndent(); } + pw.println(); + pw.decreaseIndent(); + + pw.println("Storage:"); + pw.increaseIndent(); + mStorage.dump(pw); + pw.println(); + pw.decreaseIndent(); } private void disableEscrowTokenOnNonManagedDevicesIfNeeded(int userId) { diff --git a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java index ba2ebc8f51b20..fe12a945bd4cc 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java @@ -35,6 +35,7 @@ import android.util.Slog; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.ArrayUtils; +import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.Preconditions; import com.android.internal.widget.LockPatternUtils; import com.android.internal.widget.LockPatternUtils.CredentialType; @@ -676,6 +677,26 @@ class LockSettingsStorage { void initialize(SQLiteDatabase db); } + public void dump(IndentingPrintWriter pw) { + final UserManager um = UserManager.get(mContext); + for (UserInfo user : um.getUsers(false)) { + File userPath = getSyntheticPasswordDirectoryForUser(user.id); + pw.println(String.format("User %d [%s]:", user.id, userPath.getAbsolutePath())); + pw.increaseIndent(); + File[] files = userPath.listFiles(); + if (files != null) { + for (File file : files) { + pw.println(String.format("%4d %s %s", file.length(), + LockSettingsService.timestampToString(file.lastModified()), + file.getName())); + } + } else { + pw.println("[Not found]"); + } + pw.decreaseIndent(); + } + } + static class DatabaseHelper extends SQLiteOpenHelper { private static final String TAG = "LockSettingsDB"; private static final String DATABASE_NAME = "locksettings.db";