Merge "Fix UnlockedDeviceRequired with weak unlock methods" into main
diff --git a/keystore2/aidl/android/security/authorization/IKeystoreAuthorization.aidl b/keystore2/aidl/android/security/authorization/IKeystoreAuthorization.aidl
index 54894ff..a9de026 100644
--- a/keystore2/aidl/android/security/authorization/IKeystoreAuthorization.aidl
+++ b/keystore2/aidl/android/security/authorization/IKeystoreAuthorization.aidl
@@ -47,12 +47,26 @@
* 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 @@
* 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);
+ 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 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 36f94e9..f956787 100644
--- a/keystore2/src/authorization.rs
+++ b/keystore2/src/authorization.rs
@@ -164,9 +164,21 @@
}
}
- 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 @@
&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 @@
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 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(
