Merge "Optimize dependencies in rkp_factory_extraction_tool"
diff --git a/keystore2/legacykeystore/Android.bp b/keystore2/legacykeystore/Android.bp
index c2890cc..fb6f60f 100644
--- a/keystore2/legacykeystore/Android.bp
+++ b/keystore2/legacykeystore/Android.bp
@@ -31,6 +31,7 @@
"android.security.legacykeystore-rust",
"libanyhow",
"libbinder_rs",
+ "libcutils_bindgen",
"libkeystore2",
"liblog_rust",
"librusqlite",
@@ -48,6 +49,7 @@
"android.security.legacykeystore-rust",
"libanyhow",
"libbinder_rs",
+ "libcutils_bindgen",
"libkeystore2",
"libkeystore2_test_utils",
"liblog_rust",
diff --git a/keystore2/legacykeystore/TEST_MAPPING b/keystore2/legacykeystore/TEST_MAPPING
new file mode 100644
index 0000000..37d1439
--- /dev/null
+++ b/keystore2/legacykeystore/TEST_MAPPING
@@ -0,0 +1,7 @@
+{
+ "presubmit": [
+ {
+ "name": "legacykeystore_test"
+ }
+ ]
+}
diff --git a/keystore2/legacykeystore/lib.rs b/keystore2/legacykeystore/lib.rs
index 8a4d7d8..efa0870 100644
--- a/keystore2/legacykeystore/lib.rs
+++ b/keystore2/legacykeystore/lib.rs
@@ -24,10 +24,14 @@
ThreadState,
};
use anyhow::{Context, Result};
-use keystore2::{async_task::AsyncTask, legacy_blob::LegacyBlobLoader, utils::watchdog as wd};
+use keystore2::{
+ async_task::AsyncTask, legacy_blob::LegacyBlobLoader, maintenance::DeleteListener,
+ maintenance::Domain, utils::watchdog as wd,
+};
use rusqlite::{
params, Connection, OptionalExtension, Transaction, TransactionBehavior, NO_PARAMS,
};
+use std::sync::Arc;
use std::{
collections::HashSet,
path::{Path, PathBuf},
@@ -144,6 +148,25 @@
})?;
Ok(removed == 1)
}
+
+ fn remove_uid(&mut self, uid: u32) -> Result<()> {
+ self.with_transaction(TransactionBehavior::Immediate, |tx| {
+ tx.execute("DELETE FROM profiles WHERE owner = ?;", params![uid])
+ .context("In remove_uid: Failed to delete.")
+ })?;
+ Ok(())
+ }
+
+ fn remove_user(&mut self, user_id: u32) -> Result<()> {
+ self.with_transaction(TransactionBehavior::Immediate, |tx| {
+ tx.execute(
+ "DELETE FROM profiles WHERE cast ( ( owner/? ) as int) = ?;",
+ params![cutils_bindgen::AID_USER_OFFSET, user_id],
+ )
+ .context("In remove_uid: Failed to delete.")
+ })?;
+ Ok(())
+ }
}
/// This is the main LegacyKeystore error type, it wraps binder exceptions and the
@@ -209,6 +232,19 @@
)
}
+struct LegacyKeystoreDeleteListener {
+ legacy_keystore: Arc<LegacyKeystore>,
+}
+
+impl DeleteListener for LegacyKeystoreDeleteListener {
+ fn delete_namespace(&self, domain: Domain, namespace: i64) -> Result<()> {
+ self.legacy_keystore.delete_namespace(domain, namespace)
+ }
+ fn delete_user(&self, user_id: u32) -> Result<()> {
+ self.legacy_keystore.delete_user(user_id)
+ }
+}
+
/// Implements ILegacyKeystore AIDL interface.
pub struct LegacyKeystore {
db_path: PathBuf,
@@ -226,14 +262,23 @@
/// It is kept for backward compatibility with early adopters.
const LEGACY_KEYSTORE_FILE_NAME: &'static str = "vpnprofilestore.sqlite";
+ const WIFI_NAMESPACE: i64 = 102;
+ const AID_WIFI: u32 = 1010;
+
/// Creates a new LegacyKeystore instance.
- pub fn new_native_binder(path: &Path) -> Strong<dyn ILegacyKeystore> {
+ pub fn new_native_binder(
+ path: &Path,
+ ) -> (Box<dyn DeleteListener + Send + Sync + 'static>, Strong<dyn ILegacyKeystore>) {
let mut db_path = path.to_path_buf();
db_path.push(Self::LEGACY_KEYSTORE_FILE_NAME);
- let result = Self { db_path, async_task: Default::default() };
- result.init_shelf(path);
- BnLegacyKeystore::new_binder(result, BinderFeatures::default())
+ let legacy_keystore = Arc::new(Self { db_path, async_task: Default::default() });
+ legacy_keystore.init_shelf(path);
+ let service = LegacyKeystoreService { legacy_keystore: legacy_keystore.clone() };
+ (
+ Box::new(LegacyKeystoreDeleteListener { legacy_keystore }),
+ BnLegacyKeystore::new_binder(service, BinderFeatures::default()),
+ )
}
fn open_db(&self) -> Result<DB> {
@@ -242,18 +287,17 @@
fn get_effective_uid(uid: i32) -> Result<u32> {
const AID_SYSTEM: u32 = 1000;
- const AID_WIFI: u32 = 1010;
let calling_uid = ThreadState::get_calling_uid();
let uid = uid as u32;
if uid == UID_SELF as u32 || uid == calling_uid {
Ok(calling_uid)
- } else if calling_uid == AID_SYSTEM && uid == AID_WIFI {
+ } else if calling_uid == AID_SYSTEM && uid == Self::AID_WIFI {
// The only exception for legacy reasons is allowing SYSTEM to access
// the WIFI namespace.
// IMPORTANT: If you attempt to add more exceptions, it means you are adding
// more callers to this deprecated feature. DON'T!
- Ok(AID_WIFI)
+ Ok(Self::AID_WIFI)
} else {
Err(Error::perm()).with_context(|| {
format!("In get_effective_uid: caller: {}, requested uid: {}.", calling_uid, uid)
@@ -303,6 +347,36 @@
}
}
+ fn delete_namespace(&self, domain: Domain, namespace: i64) -> Result<()> {
+ let uid = match domain {
+ Domain::APP => namespace as u32,
+ Domain::SELINUX => {
+ if namespace == Self::WIFI_NAMESPACE {
+ // Namespace WIFI gets mapped to AID_WIFI.
+ Self::AID_WIFI
+ } else {
+ // Nothing to do for any other namespace.
+ return Ok(());
+ }
+ }
+ _ => return Ok(()),
+ };
+
+ if let Err(e) = self.bulk_delete_uid(uid) {
+ log::warn!("In LegacyKeystore::delete_namespace: {:?}", e);
+ }
+ let mut db = self.open_db().context("In LegacyKeystore::delete_namespace.")?;
+ db.remove_uid(uid).context("In LegacyKeystore::delete_namespace.")
+ }
+
+ fn delete_user(&self, user_id: u32) -> Result<()> {
+ if let Err(e) = self.bulk_delete_user(user_id) {
+ log::warn!("In LegacyKeystore::delete_user: {:?}", e);
+ }
+ let mut db = self.open_db().context("In LegacyKeystore::delete_user.")?;
+ db.remove_user(user_id).context("In LegacyKeystore::delete_user.")
+ }
+
fn list(&self, prefix: &str, uid: i32) -> Result<Vec<String>> {
let mut db = self.open_db().context("In list.")?;
let uid = Self::get_effective_uid(uid).context("In list.")?;
@@ -340,7 +414,7 @@
self.do_serialized(move |state| {
state
.legacy_loader
- .list_legacy_keystore_entries(uid)
+ .list_legacy_keystore_entries_for_uid(uid)
.context("Trying to list legacy keystore entries.")
})
.context("In list_legacy.")
@@ -364,6 +438,38 @@
.context("In get_legacy.")
}
+ fn bulk_delete_uid(&self, uid: u32) -> Result<()> {
+ self.do_serialized(move |state| {
+ let entries = state
+ .legacy_loader
+ .list_legacy_keystore_entries_for_uid(uid)
+ .context("In bulk_delete_uid: Trying to list entries.")?;
+ for alias in entries.iter() {
+ if let Err(e) = state.legacy_loader.remove_legacy_keystore_entry(uid, alias) {
+ log::warn!("In bulk_delete_uid: Failed to delete legacy entry. {:?}", e);
+ }
+ }
+ Ok(())
+ })
+ }
+
+ fn bulk_delete_user(&self, user_id: u32) -> Result<()> {
+ self.do_serialized(move |state| {
+ let entries = state
+ .legacy_loader
+ .list_legacy_keystore_entries_for_user(user_id)
+ .context("In bulk_delete_user: Trying to list entries.")?;
+ for (uid, entries) in entries.iter() {
+ for alias in entries.iter() {
+ if let Err(e) = state.legacy_loader.remove_legacy_keystore_entry(*uid, alias) {
+ log::warn!("In bulk_delete_user: Failed to delete legacy entry. {:?}", e);
+ }
+ }
+ }
+ Ok(())
+ })
+ }
+
fn migrate_one_legacy_entry(
uid: u32,
alias: &str,
@@ -386,24 +492,28 @@
}
}
-impl binder::Interface for LegacyKeystore {}
+struct LegacyKeystoreService {
+ legacy_keystore: Arc<LegacyKeystore>,
+}
-impl ILegacyKeystore for LegacyKeystore {
+impl binder::Interface for LegacyKeystoreService {}
+
+impl ILegacyKeystore for LegacyKeystoreService {
fn get(&self, alias: &str, uid: i32) -> BinderResult<Vec<u8>> {
let _wp = wd::watch_millis("ILegacyKeystore::get", 500);
- map_or_log_err(self.get(alias, uid), Ok)
+ map_or_log_err(self.legacy_keystore.get(alias, uid), Ok)
}
fn put(&self, alias: &str, uid: i32, entry: &[u8]) -> BinderResult<()> {
let _wp = wd::watch_millis("ILegacyKeystore::put", 500);
- map_or_log_err(self.put(alias, uid, entry), Ok)
+ map_or_log_err(self.legacy_keystore.put(alias, uid, entry), Ok)
}
fn remove(&self, alias: &str, uid: i32) -> BinderResult<()> {
let _wp = wd::watch_millis("ILegacyKeystore::remove", 500);
- map_or_log_err(self.remove(alias, uid), Ok)
+ map_or_log_err(self.legacy_keystore.remove(alias, uid), Ok)
}
fn list(&self, prefix: &str, uid: i32) -> BinderResult<Vec<String>> {
let _wp = wd::watch_millis("ILegacyKeystore::list", 500);
- map_or_log_err(self.list(prefix, uid), Ok)
+ map_or_log_err(self.legacy_keystore.list(prefix, uid), Ok)
}
}
@@ -466,6 +576,52 @@
}
#[test]
+ fn test_delete_uid() {
+ let test_dir = TempDir::new("test_delete_uid_").expect("Failed to create temp dir.");
+ let mut db = DB::new(&test_dir.build().push(LegacyKeystore::LEGACY_KEYSTORE_FILE_NAME))
+ .expect("Failed to open database.");
+
+ // Insert three entries for owner 2.
+ db.put(2, "test1", TEST_BLOB1).expect("Failed to insert test1.");
+ db.put(2, "test2", TEST_BLOB2).expect("Failed to insert test2.");
+ db.put(3, "test3", TEST_BLOB3).expect("Failed to insert test3.");
+
+ db.remove_uid(2).expect("Failed to remove uid 2");
+
+ assert_eq!(Vec::<String>::new(), db.list(2).expect("Failed to list entries."));
+
+ assert_eq!(vec!["test3".to_string(),], db.list(3).expect("Failed to list entries."));
+ }
+
+ #[test]
+ fn test_delete_user() {
+ let test_dir = TempDir::new("test_delete_user_").expect("Failed to create temp dir.");
+ let mut db = DB::new(&test_dir.build().push(LegacyKeystore::LEGACY_KEYSTORE_FILE_NAME))
+ .expect("Failed to open database.");
+
+ // Insert three entries for owner 2.
+ db.put(2 + 2 * cutils_bindgen::AID_USER_OFFSET, "test1", TEST_BLOB1)
+ .expect("Failed to insert test1.");
+ db.put(4 + 2 * cutils_bindgen::AID_USER_OFFSET, "test2", TEST_BLOB2)
+ .expect("Failed to insert test2.");
+ db.put(3, "test3", TEST_BLOB3).expect("Failed to insert test3.");
+
+ db.remove_user(2).expect("Failed to remove user 2");
+
+ assert_eq!(
+ Vec::<String>::new(),
+ db.list(2 + 2 * cutils_bindgen::AID_USER_OFFSET).expect("Failed to list entries.")
+ );
+
+ assert_eq!(
+ Vec::<String>::new(),
+ db.list(4 + 2 * cutils_bindgen::AID_USER_OFFSET).expect("Failed to list entries.")
+ );
+
+ assert_eq!(vec!["test3".to_string(),], db.list(3).expect("Failed to list entries."));
+ }
+
+ #[test]
fn concurrent_legacy_keystore_entry_test() -> Result<()> {
let temp_dir = Arc::new(
TempDir::new("concurrent_legacy_keystore_entry_test_")
diff --git a/keystore2/src/enforcements.rs b/keystore2/src/enforcements.rs
index 29a3f0b..e9a58f9 100644
--- a/keystore2/src/enforcements.rs
+++ b/keystore2/src/enforcements.rs
@@ -28,14 +28,13 @@
KeyParameter::KeyParameter as KmKeyParameter, KeyPurpose::KeyPurpose, Tag::Tag,
};
use android_hardware_security_secureclock::aidl::android::hardware::security::secureclock::{
- ISecureClock::ISecureClock, TimeStampToken::TimeStampToken,
+ TimeStampToken::TimeStampToken,
};
use android_security_authorization::aidl::android::security::authorization::ResponseCode::ResponseCode as AuthzResponseCode;
use android_system_keystore2::aidl::android::system::keystore2::{
Domain::Domain, IKeystoreSecurityLevel::KEY_FLAG_AUTH_BOUND_WITHOUT_CRYPTOGRAPHIC_LSKF_BINDING,
OperationChallenge::OperationChallenge,
};
-use android_system_keystore2::binder::Strong;
use anyhow::{Context, Result};
use std::{
collections::{HashMap, HashSet},
@@ -219,13 +218,10 @@
}
fn get_timestamp_token(challenge: i64) -> Result<TimeStampToken, Error> {
- let dev: Strong<dyn ISecureClock> = get_timestamp_service()
- .expect(concat!(
- "Secure Clock service must be present ",
- "if TimeStampTokens are required."
- ))
- .get_interface()
- .expect("Fatal: Timestamp service does not implement ISecureClock.");
+ let dev = get_timestamp_service().expect(concat!(
+ "Secure Clock service must be present ",
+ "if TimeStampTokens are required."
+ ));
map_binder_status(dev.generateTimeStamp(challenge))
}
diff --git a/keystore2/src/globals.rs b/keystore2/src/globals.rs
index 8212213..b0af771 100644
--- a/keystore2/src/globals.rs
+++ b/keystore2/src/globals.rs
@@ -21,7 +21,6 @@
use crate::legacy_migrator::LegacyMigrator;
use crate::super_key::SuperKeyManager;
use crate::utils::watchdog as wd;
-use crate::utils::Asp;
use crate::{async_task::AsyncTask, database::MonotonicRawTime};
use crate::{
database::KeystoreDB,
@@ -33,6 +32,9 @@
IKeyMintDevice::IKeyMintDevice, IRemotelyProvisionedComponent::IRemotelyProvisionedComponent,
KeyMintHardwareInfo::KeyMintHardwareInfo, SecurityLevel::SecurityLevel,
};
+use android_hardware_security_secureclock::aidl::android::hardware::security::secureclock::{
+ ISecureClock::ISecureClock,
+};
use android_hardware_security_keymint::binder::{StatusCode, Strong};
use android_security_compat::aidl::android::security::compat::IKeystoreCompatService::IKeystoreCompatService;
use anyhow::{Context, Result};
@@ -85,34 +87,33 @@
RefCell::new(create_thread_local_db());
}
-#[derive(Default)]
-struct DevicesMap {
- devices_by_uuid: HashMap<Uuid, (Asp, KeyMintHardwareInfo)>,
+struct DevicesMap<T: FromIBinder + ?Sized> {
+ devices_by_uuid: HashMap<Uuid, (Strong<T>, KeyMintHardwareInfo)>,
uuid_by_sec_level: HashMap<SecurityLevel, Uuid>,
}
-impl DevicesMap {
+impl<T: FromIBinder + ?Sized> DevicesMap<T> {
fn dev_by_sec_level(
&self,
sec_level: &SecurityLevel,
- ) -> Option<(Asp, KeyMintHardwareInfo, Uuid)> {
+ ) -> Option<(Strong<T>, KeyMintHardwareInfo, Uuid)> {
self.uuid_by_sec_level.get(sec_level).and_then(|uuid| self.dev_by_uuid(uuid))
}
- fn dev_by_uuid(&self, uuid: &Uuid) -> Option<(Asp, KeyMintHardwareInfo, Uuid)> {
+ fn dev_by_uuid(&self, uuid: &Uuid) -> Option<(Strong<T>, KeyMintHardwareInfo, Uuid)> {
self.devices_by_uuid
.get(uuid)
.map(|(dev, hw_info)| ((*dev).clone(), (*hw_info).clone(), *uuid))
}
- fn devices<T: FromIBinder + ?Sized>(&self) -> Vec<Strong<T>> {
- self.devices_by_uuid.values().filter_map(|(asp, _)| asp.get_interface::<T>().ok()).collect()
+ fn devices(&self) -> Vec<Strong<T>> {
+ self.devices_by_uuid.values().map(|(dev, _)| dev.clone()).collect()
}
/// The requested security level and the security level of the actual implementation may
/// differ. So we map the requested security level to the uuid of the implementation
/// so that there cannot be any confusion as to which KeyMint instance is requested.
- fn insert(&mut self, sec_level: SecurityLevel, dev: Asp, hw_info: KeyMintHardwareInfo) {
+ fn insert(&mut self, sec_level: SecurityLevel, dev: Strong<T>, hw_info: KeyMintHardwareInfo) {
// For now we use the reported security level of the KM instance as UUID.
// TODO update this section once UUID was added to the KM hardware info.
let uuid: Uuid = sec_level.into();
@@ -121,17 +122,31 @@
}
}
-#[derive(Default)]
-struct RemotelyProvisionedDevicesMap {
- devices_by_sec_level: HashMap<SecurityLevel, Asp>,
+impl<T: FromIBinder + ?Sized> Default for DevicesMap<T> {
+ fn default() -> Self {
+ Self {
+ devices_by_uuid: HashMap::<Uuid, (Strong<T>, KeyMintHardwareInfo)>::new(),
+ uuid_by_sec_level: Default::default(),
+ }
+ }
}
-impl RemotelyProvisionedDevicesMap {
- fn dev_by_sec_level(&self, sec_level: &SecurityLevel) -> Option<Asp> {
+struct RemotelyProvisionedDevicesMap<T: FromIBinder + ?Sized> {
+ devices_by_sec_level: HashMap<SecurityLevel, Strong<T>>,
+}
+
+impl<T: FromIBinder + ?Sized> Default for RemotelyProvisionedDevicesMap<T> {
+ fn default() -> Self {
+ Self { devices_by_sec_level: HashMap::<SecurityLevel, Strong<T>>::new() }
+ }
+}
+
+impl<T: FromIBinder + ?Sized> RemotelyProvisionedDevicesMap<T> {
+ fn dev_by_sec_level(&self, sec_level: &SecurityLevel) -> Option<Strong<T>> {
self.devices_by_sec_level.get(sec_level).map(|dev| (*dev).clone())
}
- fn insert(&mut self, sec_level: SecurityLevel, dev: Asp) {
+ fn insert(&mut self, sec_level: SecurityLevel, dev: Strong<T>) {
self.devices_by_sec_level.insert(sec_level, dev);
}
}
@@ -143,11 +158,13 @@
/// Runtime database of unwrapped super keys.
pub static ref SUPER_KEY: Arc<SuperKeyManager> = Default::default();
/// Map of KeyMint devices.
- static ref KEY_MINT_DEVICES: Mutex<DevicesMap> = Default::default();
+ static ref KEY_MINT_DEVICES: Mutex<DevicesMap<dyn IKeyMintDevice>> = Default::default();
/// Timestamp service.
- static ref TIME_STAMP_DEVICE: Mutex<Option<Asp>> = Default::default();
+ static ref TIME_STAMP_DEVICE: Mutex<Option<Strong<dyn ISecureClock>>> = Default::default();
/// RemotelyProvisionedComponent HAL devices.
- static ref REMOTELY_PROVISIONED_COMPONENT_DEVICES: Mutex<RemotelyProvisionedDevicesMap> = Default::default();
+ static ref REMOTELY_PROVISIONED_COMPONENT_DEVICES:
+ Mutex<RemotelyProvisionedDevicesMap<dyn IRemotelyProvisionedComponent>> =
+ Default::default();
/// A single on-demand worker thread that handles deferred tasks with two different
/// priorities.
pub static ref ASYNC_TASK: Arc<AsyncTask> = Default::default();
@@ -166,8 +183,7 @@
static ref GC: Arc<Gc> = Arc::new(Gc::new_init_with(ASYNC_TASK.clone(), || {
(
Box::new(|uuid, blob| {
- let km_dev: Strong<dyn IKeyMintDevice> =
- get_keymint_dev_by_uuid(uuid).map(|(dev, _)| dev)?.get_interface()?;
+ let km_dev = get_keymint_dev_by_uuid(uuid).map(|(dev, _)| dev)?;
let _wp = wd::watch_millis("In invalidate key closure: calling deleteKey", 500);
map_km_error(km_dev.deleteKey(&*blob))
.context("In invalidate key closure: Trying to invalidate key blob.")
@@ -184,7 +200,9 @@
/// Make a new connection to a KeyMint device of the given security level.
/// If no native KeyMint device can be found this function also brings
/// up the compatibility service and attempts to connect to the legacy wrapper.
-fn connect_keymint(security_level: &SecurityLevel) -> Result<(Asp, KeyMintHardwareInfo)> {
+fn connect_keymint(
+ security_level: &SecurityLevel,
+) -> Result<(Strong<dyn IKeyMintDevice>, KeyMintHardwareInfo)> {
let keymint_instances =
get_aidl_instances("android.hardware.security.keymint", 1, "IKeyMintDevice");
@@ -251,7 +269,7 @@
hw_info.versionNumber = hal_version;
}
- Ok((Asp::new(keymint.as_binder()), hw_info))
+ Ok((keymint, hw_info))
}
/// Get a keymint device for the given security level either from our cache or
@@ -259,7 +277,7 @@
/// TODO the latter can be removed when the uuid is part of the hardware info.
pub fn get_keymint_device(
security_level: &SecurityLevel,
-) -> Result<(Asp, KeyMintHardwareInfo, Uuid)> {
+) -> Result<(Strong<dyn IKeyMintDevice>, KeyMintHardwareInfo, Uuid)> {
let mut devices_map = KEY_MINT_DEVICES.lock().unwrap();
if let Some((dev, hw_info, uuid)) = devices_map.dev_by_sec_level(&security_level) {
Ok((dev, hw_info, uuid))
@@ -275,7 +293,9 @@
/// attempt to establish a new connection. It is assumed that the cache is already populated
/// when this is called. This is a fair assumption, because service.rs iterates through all
/// security levels when it gets instantiated.
-pub fn get_keymint_dev_by_uuid(uuid: &Uuid) -> Result<(Asp, KeyMintHardwareInfo)> {
+pub fn get_keymint_dev_by_uuid(
+ uuid: &Uuid,
+) -> Result<(Strong<dyn IKeyMintDevice>, KeyMintHardwareInfo)> {
let devices_map = KEY_MINT_DEVICES.lock().unwrap();
if let Some((dev, hw_info, _)) = devices_map.dev_by_uuid(uuid) {
Ok((dev, hw_info))
@@ -294,7 +314,7 @@
/// Make a new connection to a secure clock service.
/// If no native SecureClock device can be found brings up the compatibility service and attempts
/// to connect to the legacy wrapper.
-fn connect_secureclock() -> Result<Asp> {
+fn connect_secureclock() -> Result<Strong<dyn ISecureClock>> {
let secureclock_instances =
get_aidl_instances("android.hardware.security.secureclock", 1, "ISecureClock");
@@ -325,12 +345,12 @@
.context("In connect_secureclock: Trying to get Legacy wrapper.")
}?;
- Ok(Asp::new(secureclock.as_binder()))
+ Ok(secureclock)
}
/// Get the timestamp service that verifies auth token timeliness towards security levels with
/// different clocks.
-pub fn get_timestamp_service() -> Result<Asp> {
+pub fn get_timestamp_service() -> Result<Strong<dyn ISecureClock>> {
let mut ts_device = TIME_STAMP_DEVICE.lock().unwrap();
if let Some(dev) = &*ts_device {
Ok(dev.clone())
@@ -344,7 +364,9 @@
static REMOTE_PROVISIONING_HAL_SERVICE_NAME: &str =
"android.hardware.security.keymint.IRemotelyProvisionedComponent";
-fn connect_remotely_provisioned_component(security_level: &SecurityLevel) -> Result<Asp> {
+fn connect_remotely_provisioned_component(
+ security_level: &SecurityLevel,
+) -> Result<Strong<dyn IRemotelyProvisionedComponent>> {
let remotely_prov_instances =
get_aidl_instances("android.hardware.security.keymint", 1, "IRemotelyProvisionedComponent");
@@ -375,12 +397,14 @@
" RemotelyProvisionedComponent service."
))
.map_err(|e| e)?;
- Ok(Asp::new(rem_prov_hal.as_binder()))
+ Ok(rem_prov_hal)
}
/// Get a remote provisiong component device for the given security level either from the cache or
/// by making a new connection. Returns the device.
-pub fn get_remotely_provisioned_component(security_level: &SecurityLevel) -> Result<Asp> {
+pub fn get_remotely_provisioned_component(
+ security_level: &SecurityLevel,
+) -> Result<Strong<dyn IRemotelyProvisionedComponent>> {
let mut devices_map = REMOTELY_PROVISIONED_COMPONENT_DEVICES.lock().unwrap();
if let Some(dev) = devices_map.dev_by_sec_level(&security_level) {
Ok(dev)
diff --git a/keystore2/src/keystore2_main.rs b/keystore2/src/keystore2_main.rs
index 45338c4..dab6867 100644
--- a/keystore2/src/keystore2_main.rs
+++ b/keystore2/src/keystore2_main.rs
@@ -96,7 +96,11 @@
panic!("Failed to register service {} because of {:?}.", AUTHORIZATION_SERVICE_NAME, e);
});
- let maintenance_service = Maintenance::new_native_binder().unwrap_or_else(|e| {
+ let (delete_listener, legacykeystore) = LegacyKeystore::new_native_binder(
+ &keystore2::globals::DB_PATH.read().expect("Could not get DB_PATH."),
+ );
+
+ let maintenance_service = Maintenance::new_native_binder(delete_listener).unwrap_or_else(|e| {
panic!("Failed to create service {} because of {:?}.", USER_MANAGER_SERVICE_NAME, e);
});
binder::add_service(USER_MANAGER_SERVICE_NAME, maintenance_service.as_binder()).unwrap_or_else(
@@ -120,9 +124,6 @@
});
}
- let legacykeystore = LegacyKeystore::new_native_binder(
- &keystore2::globals::DB_PATH.read().expect("Could not get DB_PATH."),
- );
binder::add_service(LEGACY_KEYSTORE_SERVICE_NAME, legacykeystore.as_binder()).unwrap_or_else(
|e| {
panic!(
diff --git a/keystore2/src/legacy_blob.rs b/keystore2/src/legacy_blob.rs
index e0d2133..6b16d2e 100644
--- a/keystore2/src/legacy_blob.rs
+++ b/keystore2/src/legacy_blob.rs
@@ -678,7 +678,7 @@
}
/// List all entries belonging to the given uid.
- pub fn list_legacy_keystore_entries(&self, uid: u32) -> Result<Vec<String>> {
+ pub fn list_legacy_keystore_entries_for_uid(&self, uid: u32) -> Result<Vec<String>> {
let mut path = self.path.clone();
let user_id = uid_to_android_user(uid);
path.push(format!("user_{}", user_id));
@@ -690,7 +690,7 @@
_ => {
return Err(e).context(format!(
concat!(
- "In list_legacy_keystore_entries: ,",
+ "In list_legacy_keystore_entries_for_uid: ,",
"Failed to open legacy blob database: {:?}"
),
path
@@ -701,21 +701,54 @@
let mut result: Vec<String> = Vec::new();
for entry in dir {
let file_name = entry
- .context("In list_legacy_keystore_entries: Trying to access dir entry")?
+ .context("In list_legacy_keystore_entries_for_uid: Trying to access dir entry")?
.file_name();
if let Some(f) = file_name.to_str() {
let encoded_alias = &f[uid_str.len() + 1..];
if f.starts_with(&uid_str) && !Self::is_keystore_alias(encoded_alias) {
- result.push(
- Self::decode_alias(encoded_alias)
- .context("In list_legacy_keystore_entries: Trying to decode alias.")?,
- )
+ result.push(Self::decode_alias(encoded_alias).context(
+ "In list_legacy_keystore_entries_for_uid: Trying to decode alias.",
+ )?)
}
}
}
Ok(result)
}
+ fn extract_legacy_alias(encoded_alias: &str) -> Option<String> {
+ if !Self::is_keystore_alias(encoded_alias) {
+ Self::decode_alias(encoded_alias).ok()
+ } else {
+ None
+ }
+ }
+
+ /// Lists all keystore entries belonging to the given user. Returns a map of UIDs
+ /// to sets of decoded aliases. Only returns entries that do not begin with
+ /// KNOWN_KEYSTORE_PREFIXES.
+ pub fn list_legacy_keystore_entries_for_user(
+ &self,
+ user_id: u32,
+ ) -> Result<HashMap<u32, HashSet<String>>> {
+ let user_entries = self
+ .list_user(user_id)
+ .context("In list_legacy_keystore_entries_for_user: Trying to list user.")?;
+
+ let result =
+ user_entries.into_iter().fold(HashMap::<u32, HashSet<String>>::new(), |mut acc, v| {
+ if let Some(sep_pos) = v.find('_') {
+ if let Ok(uid) = v[0..sep_pos].parse::<u32>() {
+ if let Some(alias) = Self::extract_legacy_alias(&v[sep_pos + 1..]) {
+ let entry = acc.entry(uid).or_default();
+ entry.insert(alias);
+ }
+ }
+ }
+ acc
+ });
+ Ok(result)
+ }
+
/// This function constructs the legacy blob file name which has the form:
/// user_<android user id>/<uid>_<alias>. Legacy blob file names must not use
/// known keystore prefixes.
@@ -798,10 +831,10 @@
.is_none())
}
- fn extract_alias(encoded_alias: &str) -> Option<String> {
+ fn extract_keystore_alias(encoded_alias: &str) -> Option<String> {
// We can check the encoded alias because the prefixes we are interested
// in are all in the printable range that don't get mangled.
- for prefix in &["USRPKEY_", "USRSKEY_", "USRCERT_", "CACERT_"] {
+ for prefix in Self::KNOWN_KEYSTORE_PREFIXES {
if let Some(alias) = encoded_alias.strip_prefix(prefix) {
return Self::decode_alias(&alias).ok();
}
@@ -849,7 +882,7 @@
user_entries.into_iter().fold(HashMap::<u32, HashSet<String>>::new(), |mut acc, v| {
if let Some(sep_pos) = v.find('_') {
if let Ok(uid) = v[0..sep_pos].parse::<u32>() {
- if let Some(alias) = Self::extract_alias(&v[sep_pos + 1..]) {
+ if let Some(alias) = Self::extract_keystore_alias(&v[sep_pos + 1..]) {
let entry = acc.entry(uid).or_default();
entry.insert(alias);
}
@@ -877,7 +910,7 @@
return None;
}
let encoded_alias = &v[uid_str.len()..];
- Self::extract_alias(encoded_alias)
+ Self::extract_keystore_alias(encoded_alias)
})
.collect();
@@ -1388,7 +1421,7 @@
let temp_dir = TempDir::new("list_legacy_keystore_entries_on_non_existing_user")?;
let legacy_blob_loader = LegacyBlobLoader::new(temp_dir.path());
- assert!(legacy_blob_loader.list_legacy_keystore_entries(20)?.is_empty());
+ assert!(legacy_blob_loader.list_legacy_keystore_entries_for_user(20)?.is_empty());
Ok(())
}
diff --git a/keystore2/src/maintenance.rs b/keystore2/src/maintenance.rs
index 0633bc1..9abc5aa 100644
--- a/keystore2/src/maintenance.rs
+++ b/keystore2/src/maintenance.rs
@@ -23,7 +23,6 @@
use crate::permission::{KeyPerm, KeystorePerm};
use crate::super_key::UserState;
use crate::utils::{check_key_permission, check_keystore_permission, watchdog as wd};
-use android_hardware_security_keymint::aidl::android::hardware::security::keymint::IKeyMintDevice::IKeyMintDevice;
use android_hardware_security_keymint::aidl::android::hardware::security::keymint::SecurityLevel::SecurityLevel;
use android_security_maintenance::aidl::android::security::maintenance::{
IKeystoreMaintenance::{BnKeystoreMaintenance, IKeystoreMaintenance},
@@ -32,22 +31,35 @@
use android_security_maintenance::binder::{
BinderFeatures, Interface, Result as BinderResult, Strong, ThreadState,
};
+use android_system_keystore2::aidl::android::system::keystore2::KeyDescriptor::KeyDescriptor;
use android_system_keystore2::aidl::android::system::keystore2::ResponseCode::ResponseCode;
-use android_system_keystore2::aidl::android::system::keystore2::{
- Domain::Domain, KeyDescriptor::KeyDescriptor,
-};
use anyhow::{Context, Result};
use keystore2_crypto::Password;
+/// Reexport Domain for the benefit of DeleteListener
+pub use android_system_keystore2::aidl::android::system::keystore2::Domain::Domain;
+
+/// The Maintenance module takes a delete listener argument which observes user and namespace
+/// deletion events.
+pub trait DeleteListener {
+ /// Called by the maintenance module when an app/namespace is deleted.
+ fn delete_namespace(&self, domain: Domain, namespace: i64) -> Result<()>;
+ /// Called by the maintenance module when a user is deleted.
+ fn delete_user(&self, user_id: u32) -> Result<()>;
+}
+
/// This struct is defined to implement the aforementioned AIDL interface.
-/// As of now, it is an empty struct.
-pub struct Maintenance;
+pub struct Maintenance {
+ delete_listener: Box<dyn DeleteListener + Send + Sync + 'static>,
+}
impl Maintenance {
- /// Create a new instance of Keystore User Manager service.
- pub fn new_native_binder() -> Result<Strong<dyn IKeystoreMaintenance>> {
+ /// Create a new instance of Keystore Maintenance service.
+ pub fn new_native_binder(
+ delete_listener: Box<dyn DeleteListener + Send + Sync + 'static>,
+ ) -> Result<Strong<dyn IKeystoreMaintenance>> {
Ok(BnKeystoreMaintenance::new_binder(
- Self,
+ Self { delete_listener },
BinderFeatures { set_requesting_sid: true, ..BinderFeatures::default() },
))
}
@@ -89,7 +101,7 @@
}
}
- fn add_or_remove_user(user_id: i32) -> Result<()> {
+ fn add_or_remove_user(&self, user_id: i32) -> Result<()> {
// Check permission. Function should return if this failed. Therefore having '?' at the end
// is very important.
check_keystore_permission(KeystorePerm::change_user()).context("In add_or_remove_user.")?;
@@ -102,10 +114,13 @@
false,
)
})
- .context("In add_or_remove_user: Trying to delete keys from db.")
+ .context("In add_or_remove_user: Trying to delete keys from db.")?;
+ self.delete_listener
+ .delete_user(user_id as u32)
+ .context("In add_or_remove_user: While invoking the delete listener.")
}
- fn clear_namespace(domain: Domain, nspace: i64) -> Result<()> {
+ fn clear_namespace(&self, domain: Domain, nspace: i64) -> Result<()> {
// Permission check. Must return on error. Do not touch the '?'.
check_keystore_permission(KeystorePerm::clear_uid()).context("In clear_namespace.")?;
@@ -113,7 +128,10 @@
.bulk_delete_uid(domain, nspace)
.context("In clear_namespace: Trying to delete legacy keys.")?;
DB.with(|db| db.borrow_mut().unbind_keys_for_namespace(domain, nspace))
- .context("In clear_namespace: Trying to delete keys from db.")
+ .context("In clear_namespace: Trying to delete keys from db.")?;
+ self.delete_listener
+ .delete_namespace(domain, nspace)
+ .context("In clear_namespace: While invoking the delete listener.")
}
fn get_state(user_id: i32) -> Result<AidlUserState> {
@@ -134,10 +152,8 @@
}
fn early_boot_ended_help(sec_level: SecurityLevel) -> Result<()> {
- let (dev, _, _) = get_keymint_device(&sec_level)
+ let (km_dev, _, _) = get_keymint_device(&sec_level)
.context("In early_boot_ended: getting keymint device")?;
- let km_dev: Strong<dyn IKeyMintDevice> =
- dev.get_interface().context("In early_boot_ended: getting keymint device interface")?;
let _wp = wd::watch_millis_with(
"In early_boot_ended_help: calling earlyBootEnded()",
@@ -231,17 +247,17 @@
fn onUserAdded(&self, user_id: i32) -> BinderResult<()> {
let _wp = wd::watch_millis("IKeystoreMaintenance::onUserAdded", 500);
- map_or_log_err(Self::add_or_remove_user(user_id), Ok)
+ map_or_log_err(self.add_or_remove_user(user_id), Ok)
}
fn onUserRemoved(&self, user_id: i32) -> BinderResult<()> {
let _wp = wd::watch_millis("IKeystoreMaintenance::onUserRemoved", 500);
- map_or_log_err(Self::add_or_remove_user(user_id), Ok)
+ map_or_log_err(self.add_or_remove_user(user_id), Ok)
}
fn clearNamespace(&self, domain: Domain, nspace: i64) -> BinderResult<()> {
let _wp = wd::watch_millis("IKeystoreMaintenance::clearNamespace", 500);
- map_or_log_err(Self::clear_namespace(domain, nspace), Ok)
+ map_or_log_err(self.clear_namespace(domain, nspace), Ok)
}
fn getState(&self, user_id: i32) -> BinderResult<AidlUserState> {
diff --git a/keystore2/src/operation.rs b/keystore2/src/operation.rs
index 8d7ad0a..1d595b3 100644
--- a/keystore2/src/operation.rs
+++ b/keystore2/src/operation.rs
@@ -128,12 +128,12 @@
use crate::enforcements::AuthInfo;
use crate::error::{map_err_with, map_km_error, map_or_log_err, Error, ErrorCode, ResponseCode};
use crate::metrics::log_key_operation_event_stats;
-use crate::utils::{watchdog as wd, Asp};
+use crate::utils::watchdog as wd;
use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{
IKeyMintOperation::IKeyMintOperation, KeyParameter::KeyParameter, KeyPurpose::KeyPurpose,
SecurityLevel::SecurityLevel,
};
-use android_hardware_security_keymint::binder::BinderFeatures;
+use android_hardware_security_keymint::binder::{BinderFeatures, Strong};
use android_system_keystore2::aidl::android::system::keystore2::{
IKeystoreOperation::BnKeystoreOperation, IKeystoreOperation::IKeystoreOperation,
};
@@ -170,7 +170,7 @@
pub struct Operation {
// The index of this operation in the OperationDb.
index: usize,
- km_op: Asp,
+ km_op: Strong<dyn IKeyMintOperation>,
last_usage: Mutex<Instant>,
outcome: Mutex<Outcome>,
owner: u32, // Uid of the operation's owner.
@@ -222,7 +222,7 @@
) -> Self {
Self {
index,
- km_op: Asp::new(km_op.as_binder()),
+ km_op,
last_usage: Mutex::new(Instant::now()),
outcome: Mutex::new(Outcome::Unknown),
owner,
@@ -282,19 +282,10 @@
}
*locked_outcome = Outcome::Pruned;
- let km_op: binder::public_api::Strong<dyn IKeyMintOperation> =
- match self.km_op.get_interface() {
- Ok(km_op) => km_op,
- Err(e) => {
- log::error!("In prune: Failed to get KeyMintOperation interface.\n {:?}", e);
- return Err(Error::sys());
- }
- };
-
let _wp = wd::watch_millis("In Operation::prune: calling abort()", 500);
// We abort the operation. If there was an error we log it but ignore it.
- if let Err(e) = map_km_error(km_op.abort()) {
+ if let Err(e) = map_km_error(self.km_op.abort()) {
log::error!("In prune: KeyMint::abort failed with {:?}.", e);
}
@@ -362,9 +353,6 @@
Self::check_input_length(aad_input).context("In update_aad")?;
self.touch();
- let km_op: binder::public_api::Strong<dyn IKeyMintOperation> =
- self.km_op.get_interface().context("In update: Failed to get KeyMintOperation.")?;
-
let (hat, tst) = self
.auth_info
.lock()
@@ -374,7 +362,7 @@
self.update_outcome(&mut *outcome, {
let _wp = wd::watch_millis("Operation::update_aad: calling updateAad", 500);
- map_km_error(km_op.updateAad(aad_input, hat.as_ref(), tst.as_ref()))
+ map_km_error(self.km_op.updateAad(aad_input, hat.as_ref(), tst.as_ref()))
})
.context("In update_aad: KeyMint::update failed.")?;
@@ -388,9 +376,6 @@
Self::check_input_length(input).context("In update")?;
self.touch();
- let km_op: binder::public_api::Strong<dyn IKeyMintOperation> =
- self.km_op.get_interface().context("In update: Failed to get KeyMintOperation.")?;
-
let (hat, tst) = self
.auth_info
.lock()
@@ -401,7 +386,7 @@
let output = self
.update_outcome(&mut *outcome, {
let _wp = wd::watch_millis("Operation::update: calling update", 500);
- map_km_error(km_op.update(input, hat.as_ref(), tst.as_ref()))
+ map_km_error(self.km_op.update(input, hat.as_ref(), tst.as_ref()))
})
.context("In update: KeyMint::update failed.")?;
@@ -421,9 +406,6 @@
}
self.touch();
- let km_op: binder::public_api::Strong<dyn IKeyMintOperation> =
- self.km_op.get_interface().context("In finish: Failed to get KeyMintOperation.")?;
-
let (hat, tst, confirmation_token) = self
.auth_info
.lock()
@@ -434,7 +416,7 @@
let output = self
.update_outcome(&mut *outcome, {
let _wp = wd::watch_millis("Operation::finish: calling finish", 500);
- map_km_error(km_op.finish(
+ map_km_error(self.km_op.finish(
input,
signature,
hat.as_ref(),
@@ -462,12 +444,10 @@
fn abort(&self, outcome: Outcome) -> Result<()> {
let mut locked_outcome = self.check_active().context("In abort")?;
*locked_outcome = outcome;
- let km_op: binder::public_api::Strong<dyn IKeyMintOperation> =
- self.km_op.get_interface().context("In abort: Failed to get KeyMintOperation.")?;
{
let _wp = wd::watch_millis("Operation::abort: calling abort", 500);
- map_km_error(km_op.abort()).context("In abort: KeyMint::abort failed.")
+ map_km_error(self.km_op.abort()).context("In abort: KeyMint::abort failed.")
}
}
}
diff --git a/keystore2/src/raw_device.rs b/keystore2/src/raw_device.rs
index cd54915..8cef84d 100644
--- a/keystore2/src/raw_device.rs
+++ b/keystore2/src/raw_device.rs
@@ -62,11 +62,11 @@
/// Get a [`KeyMintDevice`] for the given [`SecurityLevel`]
pub fn get(security_level: SecurityLevel) -> Result<KeyMintDevice> {
- let (asp, hw_info, km_uuid) = get_keymint_device(&security_level)
+ let (km_dev, hw_info, km_uuid) = get_keymint_device(&security_level)
.context("In KeyMintDevice::get: get_keymint_device failed")?;
Ok(KeyMintDevice {
- km_dev: asp.get_interface()?,
+ km_dev,
km_uuid,
version: hw_info.versionNumber,
security_level: hw_info.securityLevel,
diff --git a/keystore2/src/remote_provisioning.rs b/keystore2/src/remote_provisioning.rs
index 40ffd0c..9e2424b 100644
--- a/keystore2/src/remote_provisioning.rs
+++ b/keystore2/src/remote_provisioning.rs
@@ -43,7 +43,7 @@
use crate::database::{CertificateChain, KeystoreDB, Uuid};
use crate::error::{self, map_or_log_err, map_rem_prov_error, Error};
use crate::globals::{get_keymint_device, get_remotely_provisioned_component, DB};
-use crate::utils::{watchdog as wd, Asp};
+use crate::utils::watchdog as wd;
/// Contains helper functions to check if remote provisioning is enabled on the system and, if so,
/// to assign and retrieve attestation keys and certificate chains.
@@ -204,7 +204,7 @@
/// Implementation of the IRemoteProvisioning service.
#[derive(Default)]
pub struct RemoteProvisioningService {
- device_by_sec_level: HashMap<SecurityLevel, Asp>,
+ device_by_sec_level: HashMap<SecurityLevel, Strong<dyn IRemotelyProvisionedComponent>>,
}
impl RemoteProvisioningService {
@@ -213,7 +213,7 @@
sec_level: &SecurityLevel,
) -> Result<Strong<dyn IRemotelyProvisionedComponent>> {
if let Some(dev) = self.device_by_sec_level.get(sec_level) {
- dev.get_interface().context("In get_dev_by_sec_level.")
+ Ok(dev.clone())
} else {
Err(error::Error::sys()).context(concat!(
"In get_dev_by_sec_level: Remote instance for requested security level",
diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index f78d98b..2fddc18 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -28,7 +28,7 @@
use crate::super_key::{KeyBlob, SuperKeyManager};
use crate::utils::{
check_device_attestation_permissions, check_key_permission, is_device_id_attestation_tag,
- key_characteristics_to_internal, uid_to_android_user, watchdog as wd, Asp,
+ key_characteristics_to_internal, uid_to_android_user, watchdog as wd,
};
use crate::{
database::{
@@ -61,7 +61,7 @@
/// Implementation of the IKeystoreSecurityLevel Interface.
pub struct KeystoreSecurityLevel {
security_level: SecurityLevel,
- keymint: Asp,
+ keymint: Strong<dyn IKeyMintDevice>,
hw_info: KeyMintHardwareInfo,
km_uuid: Uuid,
operation_db: OperationDb,
@@ -304,14 +304,9 @@
.unwrap_key_if_required(&blob_metadata, km_blob)
.context("In create_operation. Failed to handle super encryption.")?;
- let km_dev: Strong<dyn IKeyMintDevice> = self
- .keymint
- .get_interface()
- .context("In create_operation: Failed to get KeyMint device")?;
-
let (begin_result, upgraded_blob) = self
.upgrade_keyblob_if_required_with(
- &*km_dev,
+ &*self.keymint,
key_id_guard,
&km_blob,
&blob_metadata,
@@ -322,7 +317,12 @@
"In KeystoreSecurityLevel::create_operation: calling begin",
500,
);
- km_dev.begin(purpose, blob, &operation_parameters, immediate_hat.as_ref())
+ self.keymint.begin(
+ purpose,
+ blob,
+ &operation_parameters,
+ immediate_hat.as_ref(),
+ )
}) {
Err(Error::Km(ErrorCode::TOO_MANY_OPERATIONS)) => {
self.operation_db.prune(caller_uid, forced)?;
@@ -508,8 +508,6 @@
.add_certificate_parameters(caller_uid, params, &key)
.context("In generate_key: Trying to get aaid.")?;
- let km_dev: Strong<dyn IKeyMintDevice> = self.keymint.get_interface()?;
-
let creation_result = match attestation_key_info {
Some(AttestationKeyInfo::UserGenerated {
key_id_guard,
@@ -518,7 +516,7 @@
issuer_subject,
}) => self
.upgrade_keyblob_if_required_with(
- &*km_dev,
+ &*self.keymint,
Some(key_id_guard),
&KeyBlob::Ref(&blob),
&blob_metadata,
@@ -537,7 +535,7 @@
),
5000, // Generate can take a little longer.
);
- km_dev.generateKey(¶ms, attest_key.as_ref())
+ self.keymint.generateKey(¶ms, attest_key.as_ref())
})
},
)
@@ -552,7 +550,7 @@
),
5000, // Generate can take a little longer.
);
- km_dev.generateKey(¶ms, Some(&attestation_key))
+ self.keymint.generateKey(¶ms, Some(&attestation_key))
})
.context("While generating Key with remote provisioned attestation key.")
.map(|mut creation_result| {
@@ -568,7 +566,7 @@
),
5000, // Generate can take a little longer.
);
- km_dev.generateKey(¶ms, None)
+ self.keymint.generateKey(¶ms, None)
})
.context("While generating Key without explicit attestation key."),
}
@@ -625,8 +623,7 @@
})
.context("In import_key.")?;
- let km_dev: Strong<dyn IKeyMintDevice> =
- self.keymint.get_interface().context("In import_key: Trying to get the KM device")?;
+ let km_dev = &self.keymint;
let creation_result = map_km_error({
let _wp =
self.watch_millis("In KeystoreSecurityLevel::import_key: calling importKey.", 500);
@@ -736,10 +733,9 @@
let masking_key = masking_key.unwrap_or(ZERO_BLOB_32);
- let km_dev: Strong<dyn IKeyMintDevice> = self.keymint.get_interface()?;
let (creation_result, _) = self
.upgrade_keyblob_if_required_with(
- &*km_dev,
+ &*self.keymint,
Some(wrapping_key_id_guard),
&wrapping_key_blob,
&wrapping_blob_metadata,
@@ -749,7 +745,7 @@
"In KeystoreSecurityLevel::import_wrapped_key: calling importWrappedKey.",
500,
);
- let creation_result = map_km_error(km_dev.importWrappedKey(
+ let creation_result = map_km_error(self.keymint.importWrappedKey(
wrapped_data,
wrapping_blob,
masking_key,
@@ -883,10 +879,7 @@
check_key_permission(KeyPerm::convert_storage_key_to_ephemeral(), storage_key, &None)
.context("In convert_storage_key_to_ephemeral: Check permission")?;
- let km_dev: Strong<dyn IKeyMintDevice> = self.keymint.get_interface().context(concat!(
- "In IKeystoreSecurityLevel convert_storage_key_to_ephemeral: ",
- "Getting keymint device interface"
- ))?;
+ let km_dev = &self.keymint;
match {
let _wp = self.watch_millis(
concat!(
@@ -945,10 +938,7 @@
check_key_permission(KeyPerm::delete(), key, &None)
.context("In IKeystoreSecurityLevel delete_key: Checking delete permissions")?;
- let km_dev: Strong<dyn IKeyMintDevice> = self
- .keymint
- .get_interface()
- .context("In IKeystoreSecurityLevel delete_key: Getting keymint device interface")?;
+ let km_dev = &self.keymint;
{
let _wp =
self.watch_millis("In KeystoreSecuritylevel::delete_key: calling deleteKey", 500);
diff --git a/keystore2/src/service.rs b/keystore2/src/service.rs
index d65743d..50374fe 100644
--- a/keystore2/src/service.rs
+++ b/keystore2/src/service.rs
@@ -22,7 +22,7 @@
use crate::security_level::KeystoreSecurityLevel;
use crate::utils::{
check_grant_permission, check_key_permission, check_keystore_permission,
- key_parameters_to_authorizations, watchdog as wd, Asp,
+ key_parameters_to_authorizations, watchdog as wd,
};
use crate::{
database::Uuid,
@@ -51,7 +51,7 @@
/// Implementation of the IKeystoreService.
#[derive(Default)]
pub struct KeystoreService {
- i_sec_level_by_uuid: HashMap<Uuid, Asp>,
+ i_sec_level_by_uuid: HashMap<Uuid, Strong<dyn IKeystoreSecurityLevel>>,
uuid_by_sec_level: HashMap<SecurityLevel, Uuid>,
}
@@ -68,15 +68,13 @@
.context(concat!(
"In KeystoreService::new_native_binder: ",
"Trying to construct mandatory security level TEE."
- ))
- .map(|(dev, uuid)| (Asp::new(dev.as_binder()), uuid))?;
+ ))?;
result.i_sec_level_by_uuid.insert(uuid, dev);
result.uuid_by_sec_level.insert(SecurityLevel::TRUSTED_ENVIRONMENT, uuid);
// Strongbox is optional, so we ignore errors and turn the result into an Option.
if let Ok((dev, uuid)) =
KeystoreSecurityLevel::new_native_binder(SecurityLevel::STRONGBOX, id_rotation_state)
- .map(|(dev, uuid)| (Asp::new(dev.as_binder()), uuid))
{
result.i_sec_level_by_uuid.insert(uuid, dev);
result.uuid_by_sec_level.insert(SecurityLevel::STRONGBOX, uuid);
@@ -107,7 +105,7 @@
fn get_i_sec_level_by_uuid(&self, uuid: &Uuid) -> Result<Strong<dyn IKeystoreSecurityLevel>> {
if let Some(dev) = self.i_sec_level_by_uuid.get(uuid) {
- dev.get_interface().context("In get_i_sec_level_by_uuid.")
+ Ok(dev.clone())
} else {
Err(error::Error::sys())
.context("In get_i_sec_level_by_uuid: KeyMint instance for key not found.")
@@ -123,7 +121,7 @@
.get(&sec_level)
.and_then(|uuid| self.i_sec_level_by_uuid.get(uuid))
{
- dev.get_interface().context("In get_security_level.")
+ Ok(dev.clone())
} else {
Err(error::Error::Km(ErrorCode::HARDWARE_TYPE_UNAVAILABLE))
.context("In get_security_level: No such security level.")
diff --git a/keystore2/src/utils.rs b/keystore2/src/utils.rs
index a110c64..83b6853 100644
--- a/keystore2/src/utils.rs
+++ b/keystore2/src/utils.rs
@@ -29,14 +29,13 @@
use android_system_keystore2::aidl::android::system::keystore2::{
Authorization::Authorization, KeyDescriptor::KeyDescriptor,
};
-use anyhow::{anyhow, Context};
-use binder::{FromIBinder, SpIBinder, ThreadState};
+use anyhow::Context;
+use binder::{Strong, ThreadState};
use keystore2_apc_compat::{
ApcCompatUiOptions, APC_COMPAT_ERROR_ABORTED, APC_COMPAT_ERROR_CANCELLED,
APC_COMPAT_ERROR_IGNORED, APC_COMPAT_ERROR_OK, APC_COMPAT_ERROR_OPERATION_PENDING,
APC_COMPAT_ERROR_SYSTEM_ERROR,
};
-use std::sync::Mutex;
/// This function uses its namesake in the permission module and in
/// combination with with_calling_sid from the binder crate to check
@@ -103,7 +102,7 @@
/// identifiers. It throws an error if the permissions cannot be verified, or if the caller doesn't
/// have the right permissions, and returns silently otherwise.
pub fn check_device_attestation_permissions() -> anyhow::Result<()> {
- let permission_controller: binder::Strong<dyn IPermissionController::IPermissionController> =
+ let permission_controller: Strong<dyn IPermissionController::IPermissionController> =
binder::get_interface("permission")?;
let binder_result = {
@@ -128,39 +127,6 @@
}
}
-/// Thread safe wrapper around SpIBinder. It is safe to have SpIBinder smart pointers to the
-/// same object in multiple threads, but cloning a SpIBinder is not thread safe.
-/// Keystore frequently hands out binder tokens to the security level interface. If this
-/// is to happen from a multi threaded thread pool, the SpIBinder needs to be protected by a
-/// Mutex.
-#[derive(Debug)]
-pub struct Asp(Mutex<SpIBinder>);
-
-impl Asp {
- /// Creates a new instance owning a SpIBinder wrapped in a Mutex.
- pub fn new(i: SpIBinder) -> Self {
- Self(Mutex::new(i))
- }
-
- /// Clones the owned SpIBinder and attempts to convert it into the requested interface.
- pub fn get_interface<T: FromIBinder + ?Sized>(&self) -> anyhow::Result<binder::Strong<T>> {
- // We can use unwrap here because we never panic when locked, so the mutex
- // can never be poisoned.
- let lock = self.0.lock().unwrap();
- (*lock)
- .clone()
- .into_interface()
- .map_err(|e| anyhow!(format!("get_interface failed with error code {:?}", e)))
- }
-}
-
-impl Clone for Asp {
- fn clone(&self) -> Self {
- let lock = self.0.lock().unwrap();
- Self(Mutex::new((*lock).clone()))
- }
-}
-
/// Converts a set of key characteristics as returned from KeyMint into the internal
/// representation of the keystore service.
pub fn key_characteristics_to_internal(