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
diff --git a/keystore2/src/super_key.rs b/keystore2/src/super_key.rs
index 9c17ccb..44ce9ab 100644
--- a/keystore2/src/super_key.rs
+++ b/keystore2/src/super_key.rs
@@ -868,85 +868,114 @@
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 @@
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(