From b0478cfa34d4ba3e466b62787d3430b817766b14 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Fri, 27 Oct 2023 03:55:29 +0000 Subject: [PATCH] keystore2: fix UnlockedDeviceRequired to work without LSKF The security improvements to UnlockedDeviceRequired in Android 12 regressed its behavior by making it no longer work for unsecured users, e.g. users with a Swipe lock screen. Two different things broke it: 1. Keystore started enforcing that a HardwareAuthToken be present for all keys that use UnlockedDeviceRequired. 2. Keystore started superencrypting all keys that use UnlockedDeviceRequired. Previously, only keys that used UserAuthenticationRequired were superencrypted. The above changes apparently resulted from a misconception that for the device to be unlocked, the user must have authenticated. However, unsecured users cannot authenticate and cannot have HardwareAuthTokens, yet the device is always considered unlocked for them. This change first fixes cause (1) by making Keystore allow keys that use UnlockedDeviceRequired to be used without a HardwareAuthToken, provided that they don't also use UserAuthenticationRequired (which is the protection that actually requires a HardwareAuthToken). Regarding cause (2), superencryption is an important security enhancement for UnlockedDeviceRequired, so it's not being removed. Instead, the real problem is in the way that Keystore unnecessarily ties superencryption to the existence of the LSKF. That is, Keystore creates a user's super keys only when an LSKF is set, and Keystore deletes all the user's super keys and superencrypted keys when the LSKF is removed. Therefore, this change, in coordination with the corresponding LockSettingsService change, makes each user's Keystore super keys have the same lifetime as the user's synthetic password. That basically means they are created when the user is created and are deleted only when the user is deleted. In addition, when a user's LSKF is removed, Keystore now deletes *only* the user's auth-bound keys. The fix for cause (1) is entirely in Keystore and is guarded by the fix_unlocked_device_required_keys flag. The fix for cause (2) consists of two new IKeystoreMaintenance methods, initUserSuperKeys() and onUserLskfRemoved(), that are called by LockSettingsService and are flagged at the LockSettingsService level. Note that once the flag is removed, it will be possible to remove superseded code, including the onUserPasswordChanged() method of IKeystoreMaintenance and the init_user() and reset_user() functions that it calls. Bug: 296464083 Test: # Did the following with and without the flag enabled: atest com.android.server.locksettings \ && atest -p --include-subdirs system/security/keystore2 \ && atest CtsKeystoreTestCases Change-Id: If12824369fbad4a90e5cd0427e792655fd233b96 --- .../maintenance/IKeystoreMaintenance.aidl | 26 +++ keystore2/src/database.rs | 178 +++++++++++++++++- keystore2/src/enforcements.rs | 38 ++++ keystore2/src/maintenance.rs | 52 +++++ keystore2/src/super_key.rs | 63 +++++-- 5 files changed, 341 insertions(+), 16 deletions(-) diff --git a/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl b/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl index 86d38d77..8275e8c1 100644 --- a/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl +++ b/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl @@ -38,6 +38,20 @@ interface IKeystoreMaintenance { */ void onUserAdded(in int userId); + /** + * Allows LockSettingsService to tell Keystore to create a user's superencryption keys and store + * them encrypted by the given secret. Requires 'ChangeUser' permission. + * + * ## Error conditions: + * `ResponseCode::PERMISSION_DENIED` - if caller does not have the 'ChangeUser' permission + * `ResponseCode::SYSTEM_ERROR` - if failed to initialize the user's super keys + * + * @param userId - Android user id + * @param password - a secret derived from the synthetic password of the user + * @param allowExisting - if true, then the keys already existing is not considered an error + */ + void initUserSuperKeys(in int userId, in byte[] password, in boolean allowExisting); + /** * Allows LockSettingsService to inform keystore about removing a user. * Callers require 'ChangeUser' permission. @@ -50,6 +64,18 @@ interface IKeystoreMaintenance { */ void onUserRemoved(in int userId); + /** + * Allows LockSettingsService to tell Keystore that a user's LSKF is being removed, ie the + * user's lock screen is changing to Swipe or None. Requires 'ChangePassword' permission. + * + * ## Error conditions: + * `ResponseCode::PERMISSION_DENIED` - if caller does not have the 'ChangePassword' permission + * `ResponseCode::SYSTEM_ERROR` - if failed to delete the user's auth-bound keys + * + * @param userId - Android user id + */ + void onUserLskfRemoved(in int userId); + /** * Allows LockSettingsService to inform keystore about password change of a user. * Callers require 'ChangePassword' permission. diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs index 83963f9d..63dbf7f4 100644 --- a/keystore2/src/database.rs +++ b/keystore2/src/database.rs @@ -47,7 +47,7 @@ mod versioning; use crate::gc::Gc; use crate::impl_metadata; // This is in db_utils.rs -use crate::key_parameter::{KeyParameter, Tag}; +use crate::key_parameter::{KeyParameter, KeyParameterValue, Tag}; use crate::ks_err; use crate::permission::KeyPermSet; use crate::utils::{get_current_time_in_milliseconds, watchdog as wd, AID_USER_OFFSET}; @@ -2544,6 +2544,70 @@ impl KeystoreDB { .context(ks_err!()) } + /// Deletes all auth-bound keys, i.e. keys that require user authentication, for the given user. + /// This runs when the user's lock screen is being changed to Swipe or None. + /// + /// This intentionally does *not* delete keys that require that the device be unlocked, unless + /// such keys also require user authentication. Keystore's concept of user authentication is + /// fairly strong, and it requires that keys that require authentication be deleted as soon as + /// authentication is no longer possible. In contrast, keys that just require that the device + /// be unlocked should remain usable when the lock screen is set to Swipe or None, as the device + /// is always considered "unlocked" in that case. + pub fn unbind_auth_bound_keys_for_user(&mut self, user_id: u32) -> Result<()> { + let _wp = wd::watch_millis("KeystoreDB::unbind_auth_bound_keys_for_user", 500); + + self.with_transaction(TransactionBehavior::Immediate, |tx| { + let mut stmt = tx + .prepare(&format!( + "SELECT id from persistent.keyentry + WHERE key_type = ? + AND domain = ? + AND cast ( (namespace/{aid_user_offset}) as int) = ? + AND state = ?;", + aid_user_offset = AID_USER_OFFSET + )) + .context(concat!( + "In unbind_auth_bound_keys_for_user. ", + "Failed to prepare the query to find the keys created by apps." + ))?; + + let mut rows = stmt + .query(params![KeyType::Client, Domain::APP.0 as u32, user_id, KeyLifeCycle::Live,]) + .context(ks_err!("Failed to query the keys created by apps."))?; + + let mut key_ids: Vec = Vec::new(); + db_utils::with_rows_extract_all(&mut rows, |row| { + key_ids + .push(row.get(0).context("Failed to read key id of a key created by an app.")?); + Ok(()) + }) + .context(ks_err!())?; + + let mut notify_gc = false; + let mut num_unbound = 0; + for key_id in key_ids { + // Load the key parameters and filter out non-auth-bound keys. To identify + // auth-bound keys, use the presence of UserSecureID. The absence of NoAuthRequired + // could also be used, but UserSecureID is what Keystore treats as authoritative + // when actually enforcing the key parameters (it might not matter, though). + let params = Self::load_key_parameters(key_id, tx) + .context("Failed to load key parameters.")?; + let is_auth_bound_key = params.iter().any(|kp| { + matches!(kp.key_parameter_value(), KeyParameterValue::UserSecureID(_)) + }); + if is_auth_bound_key { + notify_gc = Self::mark_unreferenced(tx, key_id) + .context("In unbind_auth_bound_keys_for_user.")? + || notify_gc; + num_unbound += 1; + } + } + log::info!("Deleting {num_unbound} auth-bound keys for user {user_id}"); + Ok(()).do_gc(notify_gc) + }) + .context(ks_err!()) + } + fn load_key_components( tx: &Transaction, load_bits: KeyEntryLoadBits, @@ -4752,6 +4816,53 @@ pub mod tests { Ok(key_id) } + // Creates an app key that is marked as being superencrypted by the given + // super key ID and that has the given authentication and unlocked device + // parameters. This does not actually superencrypt the key blob. + fn make_superencrypted_key_entry( + db: &mut KeystoreDB, + namespace: i64, + alias: &str, + requires_authentication: bool, + requires_unlocked_device: bool, + super_key_id: i64, + ) -> Result { + let domain = Domain::APP; + let key_id = db.create_key_entry(&domain, &namespace, KeyType::Client, &KEYSTORE_UUID)?; + + let mut blob_metadata = BlobMetaData::new(); + blob_metadata.add(BlobMetaEntry::KmUuid(KEYSTORE_UUID)); + blob_metadata.add(BlobMetaEntry::EncryptedBy(EncryptedBy::KeyId(super_key_id))); + db.set_blob( + &key_id, + SubComponentType::KEY_BLOB, + Some(TEST_KEY_BLOB), + Some(&blob_metadata), + )?; + + let mut params = vec![]; + if requires_unlocked_device { + params.push(KeyParameter::new( + KeyParameterValue::UnlockedDeviceRequired, + SecurityLevel::TRUSTED_ENVIRONMENT, + )); + } + if requires_authentication { + params.push(KeyParameter::new( + KeyParameterValue::UserSecureID(42), + SecurityLevel::TRUSTED_ENVIRONMENT, + )); + } + db.insert_keyparameter(&key_id, ¶ms)?; + + let mut metadata = KeyMetaData::new(); + metadata.add(KeyMetaEntry::CreationDate(DateTime::from_millis_epoch(123456789))); + db.insert_key_metadata(&key_id, &metadata)?; + + rebind_alias(db, &key_id, alias, domain, namespace)?; + Ok(key_id) + } + fn make_bootlevel_test_key_entry_test_vector(key_id: i64, logical_only: bool) -> KeyEntry { let mut params = make_test_params(None); params.push(KeyParameter::new(KeyParameterValue::MaxBootLevel(3), SecurityLevel::KEYSTORE)); @@ -4955,6 +5066,71 @@ pub mod tests { Ok(()) } + fn app_key_exists(db: &mut KeystoreDB, nspace: i64, alias: &str) -> Result { + db.key_exists(Domain::APP, nspace, alias, KeyType::Client) + } + + // Tests the unbind_auth_bound_keys_for_user() function. + #[test] + fn test_unbind_auth_bound_keys_for_user() -> Result<()> { + let mut db = new_test_db()?; + let user_id = 1; + let nspace: i64 = (user_id * AID_USER_OFFSET).into(); + let other_user_id = 2; + let other_user_nspace: i64 = (other_user_id * AID_USER_OFFSET).into(); + let super_key_type = &USER_AFTER_FIRST_UNLOCK_SUPER_KEY; + + // Create a superencryption key. + let super_key = keystore2_crypto::generate_aes256_key()?; + let pw: keystore2_crypto::Password = (&b"xyzabc"[..]).into(); + let (encrypted_super_key, blob_metadata) = + SuperKeyManager::encrypt_with_password(&super_key, &pw)?; + db.store_super_key( + user_id, + super_key_type, + &encrypted_super_key, + &blob_metadata, + &KeyMetaData::new(), + )?; + let super_key_id = db.load_super_key(super_key_type, user_id)?.unwrap().0 .0; + + // Store 4 superencrypted app keys, one for each possible combination of + // (authentication required, unlocked device required). + make_superencrypted_key_entry(&mut db, nspace, "noauth_noud", false, false, super_key_id)?; + make_superencrypted_key_entry(&mut db, nspace, "noauth_ud", false, true, super_key_id)?; + make_superencrypted_key_entry(&mut db, nspace, "auth_noud", true, false, super_key_id)?; + make_superencrypted_key_entry(&mut db, nspace, "auth_ud", true, true, super_key_id)?; + assert!(app_key_exists(&mut db, nspace, "noauth_noud")?); + assert!(app_key_exists(&mut db, nspace, "noauth_ud")?); + assert!(app_key_exists(&mut db, nspace, "auth_noud")?); + assert!(app_key_exists(&mut db, nspace, "auth_ud")?); + + // Also store a key for a different user that requires authentication. + make_superencrypted_key_entry( + &mut db, + other_user_nspace, + "auth_ud", + true, + true, + super_key_id, + )?; + + db.unbind_auth_bound_keys_for_user(user_id)?; + + // Verify that only the user's app keys that require authentication were + // deleted. Keys that require an unlocked device but not authentication + // should *not* have been deleted, nor should the super key have been + // deleted, nor should other users' keys have been deleted. + assert!(db.load_super_key(super_key_type, user_id)?.is_some()); + assert!(app_key_exists(&mut db, nspace, "noauth_noud")?); + assert!(app_key_exists(&mut db, nspace, "noauth_ud")?); + assert!(!app_key_exists(&mut db, nspace, "auth_noud")?); + assert!(!app_key_exists(&mut db, nspace, "auth_ud")?); + assert!(app_key_exists(&mut db, other_user_nspace, "auth_ud")?); + + Ok(()) + } + #[test] fn test_store_super_key() -> Result<()> { let mut db = new_test_db()?; diff --git a/keystore2/src/enforcements.rs b/keystore2/src/enforcements.rs index 95e88373..43147e86 100644 --- a/keystore2/src/enforcements.rs +++ b/keystore2/src/enforcements.rs @@ -603,6 +603,44 @@ impl Enforcements { } } + if android_security_flags::fix_unlocked_device_required_keys() { + let (hat, state) = if user_secure_ids.is_empty() { + (None, DeferredAuthState::NoAuthRequired) + } else if let Some(key_time_out) = key_time_out { + let (hat, last_off_body) = + Self::find_auth_token(|hat: &AuthTokenEntry| match user_auth_type { + Some(auth_type) => hat.satisfies(&user_secure_ids, auth_type), + None => false, // not reachable due to earlier check + }) + .ok_or(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED)) + .context(ks_err!("No suitable auth token found."))?; + let now = MonotonicRawTime::now(); + let token_age = now + .checked_sub(&hat.time_received()) + .ok_or_else(Error::sys) + .context(ks_err!( + "Overflow while computing Auth token validity. \ + Validity cannot be established." + ))?; + + let on_body_extended = allow_while_on_body && last_off_body < hat.time_received(); + + if token_age.seconds() > key_time_out && !on_body_extended { + return Err(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED)) + .context(ks_err!("matching auth token is expired.")); + } + let state = if requires_timestamp { + DeferredAuthState::TimeStampRequired(hat.auth_token().clone()) + } else { + DeferredAuthState::NoAuthRequired + }; + (Some(hat.take_auth_token()), state) + } else { + (None, DeferredAuthState::OpAuthRequired) + }; + return Ok((hat, AuthInfo { state, key_usage_limited, confirmation_token_receiver })); + } + if !unlocked_device_required && no_auth_required { return Ok(( None, diff --git a/keystore2/src/maintenance.rs b/keystore2/src/maintenance.rs index ea48f4d4..74858de1 100644 --- a/keystore2/src/maintenance.rs +++ b/keystore2/src/maintenance.rs @@ -120,6 +120,41 @@ impl Maintenance { .context(ks_err!("While invoking the delete listener.")) } + fn init_user_super_keys( + &self, + user_id: i32, + password: Password, + allow_existing: bool, + ) -> Result<()> { + // Permission check. Must return on error. Do not touch the '?'. + check_keystore_permission(KeystorePerm::ChangeUser).context(ks_err!())?; + + let mut skm = SUPER_KEY.write().unwrap(); + DB.with(|db| { + skm.initialize_user( + &mut db.borrow_mut(), + &LEGACY_IMPORTER, + user_id as u32, + &password, + allow_existing, + ) + }) + .context(ks_err!("Failed to initialize user super keys")) + } + + // Deletes all auth-bound keys when the user's LSKF is removed. + fn on_user_lskf_removed(user_id: i32) -> Result<()> { + // Permission check. Must return on error. Do not touch the '?'. + check_keystore_permission(KeystorePerm::ChangePassword).context(ks_err!())?; + + LEGACY_IMPORTER + .bulk_delete_user(user_id as u32, true) + .context(ks_err!("Failed to delete legacy keys."))?; + + DB.with(|db| db.borrow_mut().unbind_auth_bound_keys_for_user(user_id as u32)) + .context(ks_err!("Failed to delete auth-bound keys.")) + } + fn clear_namespace(&self, domain: Domain, nspace: i64) -> Result<()> { // Permission check. Must return on error. Do not touch the '?'. check_keystore_permission(KeystorePerm::ClearUID).context("In clear_namespace.")?; @@ -272,12 +307,29 @@ impl IKeystoreMaintenance for Maintenance { map_or_log_err(self.add_or_remove_user(user_id), Ok) } + fn initUserSuperKeys( + &self, + user_id: i32, + password: &[u8], + allow_existing: bool, + ) -> BinderResult<()> { + log::info!("initUserSuperKeys(user={user_id}, allow_existing={allow_existing})"); + let _wp = wd::watch_millis("IKeystoreMaintenance::initUserSuperKeys", 500); + map_or_log_err(self.init_user_super_keys(user_id, password.into(), allow_existing), Ok) + } + fn onUserRemoved(&self, user_id: i32) -> BinderResult<()> { log::info!("onUserRemoved(user={user_id})"); let _wp = wd::watch_millis("IKeystoreMaintenance::onUserRemoved", 500); map_or_log_err(self.add_or_remove_user(user_id), Ok) } + fn onUserLskfRemoved(&self, user_id: i32) -> BinderResult<()> { + log::info!("onUserLskfRemoved(user={user_id})"); + let _wp = wd::watch_millis("IKeystoreMaintenance::onUserLskfRemoved", 500); + map_or_log_err(Self::on_user_lskf_removed(user_id), Ok) + } + fn clearNamespace(&self, domain: Domain, nspace: i64) -> BinderResult<()> { log::info!("clearNamespace({domain:?}, nspace={nspace})"); let _wp = wd::watch_millis("IKeystoreMaintenance::clearNamespace", 500); diff --git a/keystore2/src/super_key.rs b/keystore2/src/super_key.rs index 898a8c2c..3992eb2f 100644 --- a/keystore2/src/super_key.rs +++ b/keystore2/src/super_key.rs @@ -248,11 +248,12 @@ struct BiometricUnlock { #[derive(Default)] struct UserSuperKeys { - /// The AfterFirstUnlock super key is used for LSKF binding of authentication bound keys. There - /// is one key per android user. The key is stored on flash encrypted with a key derived from a - /// secret, that is itself derived from the user's lock screen knowledge factor (LSKF). When the - /// user unlocks the device for the first time, this key is unlocked, i.e., decrypted, and stays - /// memory resident until the device reboots. + /// The AfterFirstUnlock super key is used for synthetic password binding of authentication + /// bound keys. There is one key per android user. The key is stored on flash encrypted with a + /// key derived from a secret, that is itself derived from the user's synthetic password. (In + /// most cases, the user's synthetic password can, in turn, only be decrypted using the user's + /// Lock Screen Knowledge Factor or LSKF.) When the user unlocks the device for the first time, + /// this key is unlocked, i.e., decrypted, and stays memory resident until the device reboots. after_first_unlock: Option>, /// The UnlockedDeviceRequired symmetric super key works like the AfterFirstUnlock super key /// with the distinction that it is cleared from memory when the device is locked. @@ -474,7 +475,7 @@ impl SuperKeyManager { } } - /// Checks if user has setup LSKF, even when super key cache is empty for the user. + /// Checks if the user's AfterFirstUnlock super key exists in the database (or legacy database). /// The reference to self is unused but it is required to prevent calling this function /// concurrently with skm state database changes. fn super_key_exists_in_db_for_user( @@ -662,7 +663,8 @@ impl SuperKeyManager { SuperEncryptionType::None => Ok((key_blob.to_vec(), BlobMetaData::new())), SuperEncryptionType::AfterFirstUnlock => { // Encrypt the given key blob with the user's AfterFirstUnlock super key. If the - // user has not unlocked the device since boot or has no LSKF, an error is returned. + // user has not unlocked the device since boot or the super keys were never + // initialized for the user for some reason, an error is returned. match self .get_user_state(db, legacy_importer, user_id) .context(ks_err!("Failed to get user state for user {user_id}"))? @@ -676,7 +678,7 @@ impl SuperKeyManager { Err(Error::Rc(ResponseCode::LOCKED)).context(ks_err!("Device is locked.")) } UserState::Uninitialized => Err(Error::Rc(ResponseCode::UNINITIALIZED)) - .context(ks_err!("LSKF is not setup for user {user_id}")), + .context(ks_err!("User {user_id} does not have super keys")), } } SuperEncryptionType::UnlockedDeviceRequired => { @@ -1131,6 +1133,37 @@ impl SuperKeyManager { } } + /// Initializes the given user by creating their super keys, both AfterFirstUnlock and + /// UnlockedDeviceRequired. If allow_existing is true, then the user already being initialized + /// is not considered an error. + pub fn initialize_user( + &mut self, + db: &mut KeystoreDB, + legacy_importer: &LegacyImporter, + user_id: UserId, + password: &Password, + allow_existing: bool, + ) -> Result<()> { + // Create the AfterFirstUnlock super key. + if self.super_key_exists_in_db_for_user(db, legacy_importer, user_id)? { + log::info!("AfterFirstUnlock super key already exists"); + if !allow_existing { + return Err(Error::sys()).context(ks_err!("Tried to re-init an initialized user!")); + } + } else { + let super_key = self + .create_super_key(db, user_id, &USER_AFTER_FIRST_UNLOCK_SUPER_KEY, password, None) + .context(ks_err!("Failed to create AfterFirstUnlock super key"))?; + + self.install_after_first_unlock_key_for_user(user_id, super_key) + .context(ks_err!("Failed to install AfterFirstUnlock super key for user"))?; + } + + // Create the UnlockedDeviceRequired super keys. + self.unlock_unlocked_device_required_keys(db, user_id, password) + .context(ks_err!("Failed to create UnlockedDeviceRequired super keys")) + } + /// Unlocks the given user with the given password. /// /// If the user state is BeforeFirstUnlock: @@ -1186,15 +1219,15 @@ impl SuperKeyManager { /// This enum represents different states of the user's life cycle in the device. /// For now, only three states are defined. More states may be added later. pub enum UserState { - // The user has registered LSKF and has unlocked the device by entering PIN/Password, - // and hence the AfterFirstUnlock super key is available in the cache. + // The user's super keys exist, and the user has unlocked the device at least once since boot. + // Hence, the AfterFirstUnlock super key is available in the cache. AfterFirstUnlock(Arc), - // The user has registered LSKF, but has not unlocked the device using password, after reboot. - // Hence the AfterFirstUnlock and UnlockedDeviceRequired super keys are not available in the - // cache. However, they exist in the database in encrypted form. + // The user's super keys exist, but the user hasn't unlocked the device at least once since + // boot. Hence, the AfterFirstUnlock and UnlockedDeviceRequired super keys are not available in + // the cache. However, they exist in the database in encrypted form. BeforeFirstUnlock, - // There's no user in the device for the given user id, or the user with the user id has not - // setup LSKF. + // The user's super keys don't exist. I.e., there's no user with the given user ID, or the user + // is in the process of being created or destroyed. Uninitialized, }