summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEric Biggers <ebiggers@google.com>2024-01-17 22:51:37 +0000
committerEric Biggers <ebiggers@google.com>2024-01-17 22:51:37 +0000
commit6946daa1ab6b2f2e4282b5fc1b12d6f16590a177 (patch)
tree27d3e135f52099fac2b3e14907205cd7e767b06f
parent0e77b347e7aa0b11e8353f208deb1677187c7ceb (diff)
downloadsecurity-6946daa1ab6b2f2e4282b5fc1b12d6f16590a177.tar.gz
Fix UnlockedDeviceRequired with weak unlock methods
Starting in Android 12, unlocking the device with a class 1 ("convenience") biometric, class 2 ("weak") biometric, or a trust agent unexpectedly doesn't allow the use of UnlockedDeviceRequired keys. The cause of this bug is that the cryptographic protection that Keystore now applies to UnlockedDeviceRequired keys incorrectly assumes that the device can only be unlocked using LSKF or via a biometric that participates in Keystore (has a SID and uses HardwareAuthTokens). Actually, Keyguard also allows the device to be unlocked using weaker biometrics that do not particiate in Keystore, if they are enrolled. Similarly, there are also cases where a trust agent can actively unlock the device, e.g. unlocking a phone using a paired watch. In combination with the system_server changes in I34dc49f1338e94755e96c1cf84de0638dc70d311, this CL fixes the bug by making Keystore retain the UnlockedDeviceRequired super keys in memory if a weak unlock method is enabled at device lock time. This does mean that UnlockedDeviceRequired is enforced only logically when a weak unlock method is enabled, but this is the best we can do in this case. This CL also adds methods by which Keystore can be notified of the expiration of unlock methods, causing the security level of UnlockedDeviceRequired keys to be upgraded. A future CL for system_server is planned to use these. Test: see I34dc49f1338e94755e96c1cf84de0638dc70d311 Bug: 296464083 Change-Id: I1b0d9ec4f9e31dc91642e865045766bd17e34cad
-rw-r--r--keystore2/aidl/android/security/authorization/IKeystoreAuthorization.aidl81
-rw-r--r--keystore2/src/authorization.rs58
-rw-r--r--keystore2/src/super_key.rs173
3 files changed, 225 insertions, 87 deletions
diff --git a/keystore2/aidl/android/security/authorization/IKeystoreAuthorization.aidl b/keystore2/aidl/android/security/authorization/IKeystoreAuthorization.aidl
index 54894ff5..a9de0263 100644
--- a/keystore2/aidl/android/security/authorization/IKeystoreAuthorization.aidl
+++ b/keystore2/aidl/android/security/authorization/IKeystoreAuthorization.aidl
@@ -47,12 +47,26 @@ interface IKeystoreAuthorization {
* disabled the use of such keys. In addition, once per boot, this method must be called with a
* password before keys that require user authentication can be used.
*
- * To enable access to these keys, this method attempts to decrypt and cache the user's super
- * keys. If the password is given, i.e. if the unlock occurred using an LSKF-equivalent
- * mechanism, then both the AfterFirstUnlock and UnlockedDeviceRequired super keys are decrypted
- * (if not already done). Otherwise, only the UnlockedDeviceRequired super keys are decrypted,
- * and this only works if a valid HardwareAuthToken has been added to Keystore for one of the
- * 'unlockingSids' that was passed to the last call to onDeviceLocked() for the user.
+ * This method does two things to restore access to UnlockedDeviceRequired keys. First, it sets
+ * a flag that indicates the user is unlocked. This is always done, and it makes Keystore's
+ * logical enforcement of UnlockedDeviceRequired start passing. Second, it recovers and caches
+ * the user's UnlockedDeviceRequired super keys. This succeeds only in the following cases:
+ *
+ * - The (correct) password is provided, proving that the user has authenticated using LSKF or
+ * equivalent. This is the most powerful type of unlock. Keystore uses the password to
+ * decrypt the user's UnlockedDeviceRequired super keys from disk. It also uses the password
+ * to decrypt the user's AfterFirstUnlock super key from disk, if not already done.
+ *
+ * - The user's UnlockedDeviceRequired super keys are cached in biometric-encrypted form, and a
+ * matching valid HardwareAuthToken has been added to Keystore. I.e., class 3 biometric
+ * unlock is enabled and the user recently authenticated using a class 3 biometric. The keys
+ * are cached in biometric-encrypted form if onDeviceLocked() was called with a nonempty list
+ * of unlockingSids, and onNonLskfUnlockMethodsExpired() was not called later.
+ *
+ * - The user's UnlockedDeviceRequired super keys are already cached in plaintext. This is the
+ * case if onDeviceLocked() was called with weakUnlockEnabled=true, and
+ * onWeakUnlockMethodsExpired() was not called later. This case provides only
+ * Keystore-enforced logical security for UnlockedDeviceRequired.
*
* ## Error conditions:
* `ResponseCode::PERMISSION_DENIED` - if the caller does not have the 'Unlock' permission.
@@ -69,22 +83,59 @@ interface IKeystoreAuthorization {
* Tells Keystore that the device is now locked for a user. Requires the 'Lock' permission.
*
* This method makes Keystore stop allowing the use of the given user's keys that require an
- * unlocked device. This is done through logical enforcement, and also through cryptographic
- * enforcement by wiping the UnlockedDeviceRequired super keys from memory.
+ * unlocked device. This is enforced logically, and when possible it's also enforced
+ * cryptographically by wiping the UnlockedDeviceRequired super keys from memory.
*
- * unlockingSids is the list of SIDs of the user's biometrics with which the device may be
- * unlocked later. If this list is non-empty, then instead of completely wiping the
- * UnlockedDeviceRequired super keys from memory, this method re-encrypts these super keys with
- * a new AES key that is imported into KeyMint and bound to the given SIDs. This allows the
- * UnlockedDeviceRequired super keys to be recovered if the device is unlocked with a biometric.
+ * unlockingSids and weakUnlockEnabled specify the methods by which the device can become
+ * unlocked for the user, in addition to LSKF-equivalent authentication.
+ *
+ * unlockingSids is the list of SIDs of class 3 (strong) biometrics that can unlock. If
+ * unlockingSids is non-empty, then this method saves a copy of the UnlockedDeviceRequired super
+ * keys in memory encrypted by a new AES key that is imported into KeyMint and configured to be
+ * usable only when user authentication has occurred using any of the SIDs. This allows the
+ * keys to be recovered if the device is unlocked using a class 3 biometric.
+ *
+ * weakUnlockEnabled is true if the unlock can happen using a method that does not have an
+ * associated SID, such as a class 1 (convenience) biometric, class 2 (weak) biometric, or trust
+ * agent. These methods don't count as "authentication" from Keystore's perspective. In this
+ * case, Keystore keeps a copy of the UnlockedDeviceRequired super keys in memory in plaintext,
+ * providing only logical security for UnlockedDeviceRequired.
*
* ## Error conditions:
* `ResponseCode::PERMISSION_DENIED` - if the caller does not have the 'Lock' permission.
*
* @param userId The Android user ID of the user for which the device is now locked
- * @param unlockingSids The user's list of biometric SIDs
+ * @param unlockingSids SIDs of class 3 biometrics that can unlock the device for the user
+ * @param weakUnlockEnabled Whether a weak unlock method can unlock the device for the user
+ */
+ void onDeviceLocked(in int userId, in long[] unlockingSids, in boolean weakUnlockEnabled);
+
+ /**
+ * Tells Keystore that weak unlock methods can no longer unlock the device for the given user.
+ * This is intended to be called after an earlier call to onDeviceLocked() with
+ * weakUnlockEnabled=true. It upgrades the security level of UnlockedDeviceRequired keys to
+ * that which would have resulted from calling onDeviceLocked() with weakUnlockEnabled=false.
+ *
+ * ## Error conditions:
+ * `ResponseCode::PERMISSION_DENIED` - if the caller does not have the 'Lock' permission.
+ *
+ * @param userId The Android user ID of the user for which weak unlock methods have expired
+ */
+ void onWeakUnlockMethodsExpired(in int userId);
+
+ /**
+ * Tells Keystore that non-LSKF-equivalent unlock methods can no longer unlock the device for
+ * the given user. This is intended to be called after an earlier call to onDeviceLocked() with
+ * nonempty unlockingSids. It upgrades the security level of UnlockedDeviceRequired keys to
+ * that which would have resulted from calling onDeviceLocked() with unlockingSids=[] and
+ * weakUnlockEnabled=false.
+ *
+ * ## Error conditions:
+ * `ResponseCode::PERMISSION_DENIED` - if the caller does not have the 'Lock' permission.
+ *
+ * @param userId The Android user ID of the user for which non-LSKF unlock methods have expired
*/
- void onDeviceLocked(in int userId, in long[] unlockingSids);
+ void onNonLskfUnlockMethodsExpired(in int userId);
/**
* Allows Credstore to retrieve a HardwareAuthToken and a TimestampToken.
diff --git a/keystore2/src/authorization.rs b/keystore2/src/authorization.rs
index 36f94e99..f9567875 100644
--- a/keystore2/src/authorization.rs
+++ b/keystore2/src/authorization.rs
@@ -164,9 +164,21 @@ impl AuthorizationManager {
}
}
- fn on_device_locked(&self, user_id: i32, unlocking_sids: &[i64]) -> Result<()> {
- log::info!("on_device_locked(user_id={}, unlocking_sids={:?})", user_id, unlocking_sids);
-
+ fn on_device_locked(
+ &self,
+ user_id: i32,
+ unlocking_sids: &[i64],
+ mut weak_unlock_enabled: bool,
+ ) -> Result<()> {
+ log::info!(
+ "on_device_locked(user_id={}, unlocking_sids={:?}, weak_unlock_enabled={})",
+ user_id,
+ unlocking_sids,
+ weak_unlock_enabled
+ );
+ if !android_security_flags::fix_unlocked_device_required_keys_v2() {
+ weak_unlock_enabled = false;
+ }
check_keystore_permission(KeystorePerm::Lock).context(ks_err!("Lock"))?;
ENFORCEMENTS.set_device_locked(user_id, true);
let mut skm = SUPER_KEY.write().unwrap();
@@ -175,11 +187,32 @@ impl AuthorizationManager {
&mut db.borrow_mut(),
user_id as u32,
unlocking_sids,
+ weak_unlock_enabled,
);
});
Ok(())
}
+ fn on_weak_unlock_methods_expired(&self, user_id: i32) -> Result<()> {
+ log::info!("on_weak_unlock_methods_expired(user_id={})", user_id);
+ if !android_security_flags::fix_unlocked_device_required_keys_v2() {
+ return Ok(());
+ }
+ check_keystore_permission(KeystorePerm::Lock).context(ks_err!("Lock"))?;
+ SUPER_KEY.write().unwrap().wipe_plaintext_unlocked_device_required_keys(user_id as u32);
+ Ok(())
+ }
+
+ fn on_non_lskf_unlock_methods_expired(&self, user_id: i32) -> Result<()> {
+ log::info!("on_non_lskf_unlock_methods_expired(user_id={})", user_id);
+ if !android_security_flags::fix_unlocked_device_required_keys_v2() {
+ return Ok(());
+ }
+ check_keystore_permission(KeystorePerm::Lock).context(ks_err!("Lock"))?;
+ SUPER_KEY.write().unwrap().wipe_all_unlocked_device_required_keys(user_id as u32);
+ Ok(())
+ }
+
fn get_auth_tokens_for_credstore(
&self,
challenge: i64,
@@ -240,9 +273,24 @@ impl IKeystoreAuthorization for AuthorizationManager {
map_or_log_err(self.on_device_unlocked(user_id, password.map(|pw| pw.into())), Ok)
}
- fn onDeviceLocked(&self, user_id: i32, unlocking_sids: &[i64]) -> BinderResult<()> {
+ fn onDeviceLocked(
+ &self,
+ user_id: i32,
+ unlocking_sids: &[i64],
+ weak_unlock_enabled: bool,
+ ) -> BinderResult<()> {
let _wp = wd::watch_millis("IKeystoreAuthorization::onDeviceLocked", 500);
- map_or_log_err(self.on_device_locked(user_id, unlocking_sids), Ok)
+ map_or_log_err(self.on_device_locked(user_id, unlocking_sids, weak_unlock_enabled), Ok)
+ }
+
+ fn onWeakUnlockMethodsExpired(&self, user_id: i32) -> BinderResult<()> {
+ let _wp = wd::watch_millis("IKeystoreAuthorization::onWeakUnlockMethodsExpired", 500);
+ map_or_log_err(self.on_weak_unlock_methods_expired(user_id), Ok)
+ }
+
+ fn onNonLskfUnlockMethodsExpired(&self, user_id: i32) -> BinderResult<()> {
+ let _wp = wd::watch_millis("IKeystoreAuthorization::onNonLskfUnlockMethodsExpired", 500);
+ map_or_log_err(self.on_non_lskf_unlock_methods_expired(user_id), Ok)
}
fn getAuthTokensForCredStore(
diff --git a/keystore2/src/super_key.rs b/keystore2/src/super_key.rs
index 9c17ccbf..44ce9abf 100644
--- a/keystore2/src/super_key.rs
+++ b/keystore2/src/super_key.rs
@@ -868,85 +868,114 @@ impl SuperKeyManager {
Ok(())
}
- /// Wipe the user's UnlockedDeviceRequired super keys from memory.
+ /// Protects the user's UnlockedDeviceRequired super keys in a way such that they can only be
+ /// unlocked by the enabled unlock methods.
pub fn lock_unlocked_device_required_keys(
&mut self,
db: &mut KeystoreDB,
user_id: UserId,
unlocking_sids: &[i64],
+ weak_unlock_enabled: bool,
) {
- log::info!(
- "Locking UnlockedDeviceRequired super keys for user {}; unlocking_sids={:?}",
- user_id,
- unlocking_sids
- );
let entry = self.data.user_keys.entry(user_id).or_default();
- if !unlocking_sids.is_empty() {
- if let (Some(aes), Some(ecdh)) = (
- entry.unlocked_device_required_symmetric.as_ref().cloned(),
- entry.unlocked_device_required_private.as_ref().cloned(),
- ) {
- let res = (|| -> Result<()> {
- let key_desc = KeyMintDevice::internal_descriptor(format!(
- "biometric_unlock_key_{}",
- user_id
- ));
- let encrypting_key = generate_aes256_key()?;
- let km_dev: KeyMintDevice =
- KeyMintDevice::get(SecurityLevel::TRUSTED_ENVIRONMENT)
- .context(ks_err!("KeyMintDevice::get failed"))?;
- let mut key_params = vec![
- KeyParameterValue::Algorithm(Algorithm::AES),
- KeyParameterValue::KeySize(256),
- KeyParameterValue::BlockMode(BlockMode::GCM),
- KeyParameterValue::PaddingMode(PaddingMode::NONE),
- KeyParameterValue::CallerNonce,
- KeyParameterValue::KeyPurpose(KeyPurpose::DECRYPT),
- KeyParameterValue::MinMacLength(128),
- KeyParameterValue::AuthTimeout(BIOMETRIC_AUTH_TIMEOUT_S),
- KeyParameterValue::HardwareAuthenticatorType(
- HardwareAuthenticatorType::FINGERPRINT,
- ),
- ];
- for sid in unlocking_sids {
- key_params.push(KeyParameterValue::UserSecureID(*sid));
- }
- let key_params: Vec<KmKeyParameter> =
- key_params.into_iter().map(|x| x.into()).collect();
- km_dev.create_and_store_key(
- db,
- &key_desc,
- KeyType::Client, /* TODO Should be Super b/189470584 */
- |dev| {
- let _wp = wd::watch_millis(
- "In lock_unlocked_device_required_keys: calling importKey.",
- 500,
- );
- dev.importKey(
- key_params.as_slice(),
- KeyFormat::RAW,
- &encrypting_key,
- None,
- )
- },
- )?;
- entry.biometric_unlock = Some(BiometricUnlock {
- sids: unlocking_sids.into(),
- key_desc,
- symmetric: LockedKey::new(&encrypting_key, &aes)?,
- private: LockedKey::new(&encrypting_key, &ecdh)?,
- });
- Ok(())
- })();
- // There is no reason to propagate an error here upwards. We must clear the keys
- // from memory in any case.
- if let Err(e) = res {
- log::error!("Error setting up biometric unlock: {:#?}", e);
+ if unlocking_sids.is_empty() {
+ if android_security_flags::fix_unlocked_device_required_keys_v2() {
+ entry.biometric_unlock = None;
+ }
+ } else if let (Some(aes), Some(ecdh)) = (
+ entry.unlocked_device_required_symmetric.as_ref().cloned(),
+ entry.unlocked_device_required_private.as_ref().cloned(),
+ ) {
+ // If class 3 biometric unlock methods are enabled, create a biometric-encrypted copy of
+ // the keys. Do this even if weak unlock methods are enabled too; in that case we'll
+ // also retain a plaintext copy of the keys, but that copy will be wiped later if weak
+ // unlock methods expire. So we need the biometric-encrypted copy too just in case.
+ let res = (|| -> Result<()> {
+ let key_desc =
+ KeyMintDevice::internal_descriptor(format!("biometric_unlock_key_{}", user_id));
+ let encrypting_key = generate_aes256_key()?;
+ let km_dev: KeyMintDevice = KeyMintDevice::get(SecurityLevel::TRUSTED_ENVIRONMENT)
+ .context(ks_err!("KeyMintDevice::get failed"))?;
+ let mut key_params = vec![
+ KeyParameterValue::Algorithm(Algorithm::AES),
+ KeyParameterValue::KeySize(256),
+ KeyParameterValue::BlockMode(BlockMode::GCM),
+ KeyParameterValue::PaddingMode(PaddingMode::NONE),
+ KeyParameterValue::CallerNonce,
+ KeyParameterValue::KeyPurpose(KeyPurpose::DECRYPT),
+ KeyParameterValue::MinMacLength(128),
+ KeyParameterValue::AuthTimeout(BIOMETRIC_AUTH_TIMEOUT_S),
+ KeyParameterValue::HardwareAuthenticatorType(
+ HardwareAuthenticatorType::FINGERPRINT,
+ ),
+ ];
+ for sid in unlocking_sids {
+ key_params.push(KeyParameterValue::UserSecureID(*sid));
}
+ let key_params: Vec<KmKeyParameter> =
+ key_params.into_iter().map(|x| x.into()).collect();
+ km_dev.create_and_store_key(
+ db,
+ &key_desc,
+ KeyType::Client, /* TODO Should be Super b/189470584 */
+ |dev| {
+ let _wp = wd::watch_millis(
+ "In lock_unlocked_device_required_keys: calling importKey.",
+ 500,
+ );
+ dev.importKey(key_params.as_slice(), KeyFormat::RAW, &encrypting_key, None)
+ },
+ )?;
+ entry.biometric_unlock = Some(BiometricUnlock {
+ sids: unlocking_sids.into(),
+ key_desc,
+ symmetric: LockedKey::new(&encrypting_key, &aes)?,
+ private: LockedKey::new(&encrypting_key, &ecdh)?,
+ });
+ Ok(())
+ })();
+ if let Err(e) = res {
+ log::error!("Error setting up biometric unlock: {:#?}", e);
+ // The caller can't do anything about the error, and for security reasons we still
+ // wipe the keys (unless a weak unlock method is enabled). So just log the error.
}
}
+ // Wipe the plaintext copy of the keys, unless a weak unlock method is enabled.
+ if !weak_unlock_enabled {
+ entry.unlocked_device_required_symmetric = None;
+ entry.unlocked_device_required_private = None;
+ }
+ Self::log_status_of_unlocked_device_required_keys(user_id, entry);
+ }
+
+ pub fn wipe_plaintext_unlocked_device_required_keys(&mut self, user_id: UserId) {
+ let entry = self.data.user_keys.entry(user_id).or_default();
+ entry.unlocked_device_required_symmetric = None;
+ entry.unlocked_device_required_private = None;
+ Self::log_status_of_unlocked_device_required_keys(user_id, entry);
+ }
+
+ pub fn wipe_all_unlocked_device_required_keys(&mut self, user_id: UserId) {
+ let entry = self.data.user_keys.entry(user_id).or_default();
entry.unlocked_device_required_symmetric = None;
entry.unlocked_device_required_private = None;
+ entry.biometric_unlock = None;
+ Self::log_status_of_unlocked_device_required_keys(user_id, entry);
+ }
+
+ fn log_status_of_unlocked_device_required_keys(user_id: UserId, entry: &UserSuperKeys) {
+ let status = match (
+ // Note: the status of the symmetric and private keys should always be in sync.
+ // So we only check one here.
+ entry.unlocked_device_required_symmetric.is_some(),
+ entry.biometric_unlock.is_some(),
+ ) {
+ (false, false) => "fully protected",
+ (false, true) => "biometric-encrypted",
+ (true, false) => "retained in plaintext",
+ (true, true) => "retained in plaintext, with biometric-encrypted copy too",
+ };
+ log::info!("UnlockedDeviceRequired super keys for user {user_id} are {status}.");
}
/// User has unlocked, not using a password. See if any of our stored auth tokens can be used
@@ -957,6 +986,16 @@ impl SuperKeyManager {
user_id: UserId,
) -> Result<()> {
let entry = self.data.user_keys.entry(user_id).or_default();
+ if android_security_flags::fix_unlocked_device_required_keys_v2()
+ && entry.unlocked_device_required_symmetric.is_some()
+ && entry.unlocked_device_required_private.is_some()
+ {
+ // If the keys are already cached in plaintext, then there is no need to decrypt the
+ // biometric-encrypted copy. Both copies can be present here if the user has both
+ // class 3 biometric and weak unlock methods enabled, and the device was unlocked before
+ // the weak unlock methods expired.
+ return Ok(());
+ }
if let Some(biometric) = entry.biometric_unlock.as_ref() {
let (key_id_guard, key_entry) = db
.load_key_entry(