diff options
author | Treehugger Robot <treehugger-gerrit@google.com> | 2021-07-02 16:40:23 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2021-07-02 16:40:23 +0000 |
commit | c2bf21f433ca0281272aec8e0e7568d3c72353f9 (patch) | |
tree | 217059adca9818802d9104c5137e7a2f6cf80f35 /keystore2 | |
parent | 2edc36ce1ce3dc5b610d456573fac2c15531ef73 (diff) | |
parent | 5f3a0570108ae5ec6e9a1df2e3321ded3d8d17bc (diff) | |
download | security-c2bf21f433ca0281272aec8e0e7568d3c72353f9.tar.gz |
Merge "Keystore 2.0: Remove Asp."
Diffstat (limited to 'keystore2')
-rw-r--r-- | keystore2/src/enforcements.rs | 14 | ||||
-rw-r--r-- | keystore2/src/globals.rs | 86 | ||||
-rw-r--r-- | keystore2/src/maintenance.rs | 5 | ||||
-rw-r--r-- | keystore2/src/operation.rs | 38 | ||||
-rw-r--r-- | keystore2/src/raw_device.rs | 4 | ||||
-rw-r--r-- | keystore2/src/remote_provisioning.rs | 6 | ||||
-rw-r--r-- | keystore2/src/security_level.rs | 46 | ||||
-rw-r--r-- | keystore2/src/service.rs | 12 | ||||
-rw-r--r-- | keystore2/src/utils.rs | 40 |
9 files changed, 101 insertions, 150 deletions
diff --git a/keystore2/src/enforcements.rs b/keystore2/src/enforcements.rs index 29a3f0b0..e9a58f96 100644 --- a/keystore2/src/enforcements.rs +++ b/keystore2/src/enforcements.rs @@ -28,14 +28,13 @@ use android_hardware_security_keymint::aidl::android::hardware::security::keymin 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 @@ impl TokenReceiver { } 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 8212213e..b0af771d 100644 --- a/keystore2/src/globals.rs +++ b/keystore2/src/globals.rs @@ -21,7 +21,6 @@ use crate::legacy_blob::LegacyBlobLoader; 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 @@ use android_hardware_security_keymint::aidl::android::hardware::security::keymin 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 @@ thread_local! { 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 @@ impl DevicesMap { } } -#[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(), + } + } +} + +struct RemotelyProvisionedDevicesMap<T: FromIBinder + ?Sized> { + devices_by_sec_level: HashMap<SecurityLevel, Strong<T>>, } -impl RemotelyProvisionedDevicesMap { - fn dev_by_sec_level(&self, sec_level: &SecurityLevel) -> Option<Asp> { +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 @@ lazy_static! { /// 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 @@ lazy_static! { 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 @@ static KEYMINT_SERVICE_NAME: &str = "android.hardware.security.keymint.IKeyMintD /// 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 @@ fn connect_keymint(security_level: &SecurityLevel) -> Result<(Asp, KeyMintHardwa 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 @@ fn connect_keymint(security_level: &SecurityLevel) -> Result<(Asp, KeyMintHardwa /// 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 @@ pub fn get_keymint_device( /// 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 @@ static TIME_STAMP_SERVICE_NAME: &str = "android.hardware.security.secureclock.IS /// 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 @@ fn connect_secureclock() -> Result<Asp> { .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 @@ pub fn get_timestamp_service() -> Result<Asp> { 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 @@ fn connect_remotely_provisioned_component(security_level: &SecurityLevel) -> Res " 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/maintenance.rs b/keystore2/src/maintenance.rs index 0633bc19..afb88eca 100644 --- a/keystore2/src/maintenance.rs +++ b/keystore2/src/maintenance.rs @@ -23,7 +23,6 @@ use crate::globals::{DB, LEGACY_MIGRATOR, SUPER_KEY}; 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}, @@ -134,10 +133,8 @@ impl Maintenance { } 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()", diff --git a/keystore2/src/operation.rs b/keystore2/src/operation.rs index 8d7ad0a6..1d595b3b 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 enum Outcome { 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 @@ impl Operation { ) -> 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 @@ impl Operation { } *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 @@ impl Operation { 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 @@ impl Operation { 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 @@ impl Operation { 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 @@ impl Operation { 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 @@ impl Operation { } 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 @@ impl Operation { 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 @@ impl Operation { 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 cd549151..8cef84df 100644 --- a/keystore2/src/raw_device.rs +++ b/keystore2/src/raw_device.rs @@ -62,11 +62,11 @@ impl KeyMintDevice { /// 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 40ffd0ca..9e2424bf 100644 --- a/keystore2/src/remote_provisioning.rs +++ b/keystore2/src/remote_provisioning.rs @@ -43,7 +43,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; 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 @@ impl RemProvState { /// 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 @@ impl RemoteProvisioningService { 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 f78d98b9..2fddc186 100644 --- a/keystore2/src/security_level.rs +++ b/keystore2/src/security_level.rs @@ -28,7 +28,7 @@ use crate::remote_provisioning::RemProvState; 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 @@ use anyhow::{anyhow, Context, Result}; /// 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 @@ impl KeystoreSecurityLevel { .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 @@ impl KeystoreSecurityLevel { "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 @@ impl KeystoreSecurityLevel { .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 @@ impl KeystoreSecurityLevel { 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 @@ impl KeystoreSecurityLevel { ), 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 @@ impl KeystoreSecurityLevel { ), 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 @@ impl KeystoreSecurityLevel { ), 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 @@ impl KeystoreSecurityLevel { }) .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 @@ impl KeystoreSecurityLevel { 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 @@ impl KeystoreSecurityLevel { "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 @@ impl KeystoreSecurityLevel { 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 @@ impl KeystoreSecurityLevel { 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 d65743d2..50374fec 100644 --- a/keystore2/src/service.rs +++ b/keystore2/src/service.rs @@ -22,7 +22,7 @@ use crate::permission::{KeyPerm, KeystorePerm}; 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 @@ use keystore2_selinux as selinux; /// 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 @@ impl KeystoreService { .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 @@ impl KeystoreService { 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 @@ impl KeystoreService { .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 a110c64e..83b6853c 100644 --- a/keystore2/src/utils.rs +++ b/keystore2/src/utils.rs @@ -29,14 +29,13 @@ use android_security_apc::aidl::android::security::apc::{ 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 @@ pub fn is_device_id_attestation_tag(tag: Tag) -> bool { /// 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 @@ pub fn check_device_attestation_permissions() -> anyhow::Result<()> { } } -/// 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( |