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
diff --git a/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl b/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
index 86d38d7..8275e8c 100644
--- a/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
+++ b/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
@@ -39,6 +39,20 @@
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.
*
@@ -51,6 +65,18 @@
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 83963f9..63dbf7f 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -47,7 +47,7 @@
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 @@
.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<i64> = 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 @@
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<KeyIdGuard> {
+ 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 @@
Ok(())
}
+ fn app_key_exists(db: &mut KeystoreDB, nspace: i64, alias: &str) -> Result<bool> {
+ 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 95e8837..43147e8 100644
--- a/keystore2/src/enforcements.rs
+++ b/keystore2/src/enforcements.rs
@@ -603,6 +603,44 @@
}
}
+ 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 ea48f4d..74858de 100644
--- a/keystore2/src/maintenance.rs
+++ b/keystore2/src/maintenance.rs
@@ -120,6 +120,41 @@
.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 @@
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 898a8c2..3992eb2 100644
--- a/keystore2/src/super_key.rs
+++ b/keystore2/src/super_key.rs
@@ -248,11 +248,12 @@
#[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<Arc<SuperKey>>,
/// 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 @@
}
}
- /// 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 @@
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 @@
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 @@
}
}
+ /// 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 @@
/// 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<SuperKey>),
- // 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,
}