diff --git a/keystore2/legacykeystore/lib.rs b/keystore2/legacykeystore/lib.rs index d4074165..41b86850 100644 --- a/keystore2/legacykeystore/lib.rs +++ b/keystore2/legacykeystore/lib.rs @@ -210,41 +210,25 @@ impl Error { } } -/// This function should be used by legacykeystore service calls to translate error conditions -/// into service specific exceptions. +/// Translate errors into service specific exceptions. /// -/// All error conditions get logged by this function, except for ERROR_ENTRY_NOT_FOUND error. -/// -/// `Error::Error(x)` variants get mapped onto a service specific error code of `x`. -/// -/// All non `Error` error conditions get mapped onto `ERROR_SYSTEM_ERROR`. -/// -/// `handle_ok` will be called if `result` is `Ok(value)` where `value` will be passed -/// as argument to `handle_ok`. `handle_ok` must generate a `BinderResult`, but it -/// typically returns Ok(value). -fn map_or_log_err(result: Result, handle_ok: F) -> BinderResult -where - F: FnOnce(U) -> BinderResult, -{ - result.map_or_else( - |e| { - let root_cause = e.root_cause(); - let (rc, log_error) = match root_cause.downcast_ref::() { - // Make the entry not found errors silent. - Some(Error::Error(ERROR_ENTRY_NOT_FOUND)) => (ERROR_ENTRY_NOT_FOUND, false), - Some(Error::Error(e)) => (*e, true), - Some(Error::Binder(_, _)) | None => (ERROR_SYSTEM_ERROR, true), - }; - if log_error { - log::error!("{:?}", e); - } - Err(BinderStatus::new_service_specific_error( - rc, - anyhow_error_to_cstring(&e).as_deref(), - )) - }, - handle_ok, - ) +/// All errors are logged, except for ERROR_ENTRY_NOT_FOUND error. `Error::Error(x)` variants get +/// mapped onto a service specific error code of `x`, other errors are mapped to +/// `ERROR_SYSTEM_ERROR`. +fn map_or_log_err(result: Result) -> BinderResult { + result.map_err(|e| { + let root_cause = e.root_cause(); + let (rc, log_error) = match root_cause.downcast_ref::() { + // Make the entry not found errors silent. + Some(Error::Error(ERROR_ENTRY_NOT_FOUND)) => (ERROR_ENTRY_NOT_FOUND, false), + Some(Error::Error(e)) => (*e, true), + Some(Error::Binder(_, _)) | None => (ERROR_SYSTEM_ERROR, true), + }; + if log_error { + log::error!("{:?}", e); + } + BinderStatus::new_service_specific_error(rc, anyhow_error_to_cstring(&e).as_deref()) + }) } fn ensure_keystore_put_is_enabled() -> Result<()> { @@ -552,19 +536,19 @@ impl binder::Interface for LegacyKeystoreService {} impl ILegacyKeystore for LegacyKeystoreService { fn get(&self, alias: &str, uid: i32) -> BinderResult> { let _wp = wd::watch("ILegacyKeystore::get"); - map_or_log_err(self.legacy_keystore.get(alias, uid), Ok) + map_or_log_err(self.legacy_keystore.get(alias, uid)) } fn put(&self, alias: &str, uid: i32, entry: &[u8]) -> BinderResult<()> { let _wp = wd::watch("ILegacyKeystore::put"); - map_or_log_err(self.legacy_keystore.put(alias, uid, entry), Ok) + map_or_log_err(self.legacy_keystore.put(alias, uid, entry)) } fn remove(&self, alias: &str, uid: i32) -> BinderResult<()> { let _wp = wd::watch("ILegacyKeystore::remove"); - map_or_log_err(self.legacy_keystore.remove(alias, uid), Ok) + map_or_log_err(self.legacy_keystore.remove(alias, uid)) } fn list(&self, prefix: &str, uid: i32) -> BinderResult> { let _wp = wd::watch("ILegacyKeystore::list"); - map_or_log_err(self.legacy_keystore.list(prefix, uid), Ok) + map_or_log_err(self.legacy_keystore.list(prefix, uid)) } } diff --git a/keystore2/src/apc.rs b/keystore2/src/apc.rs index bdde5ae4..8fda1f25 100644 --- a/keystore2/src/apc.rs +++ b/keystore2/src/apc.rs @@ -82,33 +82,20 @@ impl Error { /// `selinux::Error::perm()` is mapped on `ResponseCode::PERMISSION_DENIED`. /// /// All non `Error` error conditions get mapped onto ResponseCode::SYSTEM_ERROR`. -/// -/// `handle_ok` will be called if `result` is `Ok(value)` where `value` will be passed -/// as argument to `handle_ok`. `handle_ok` must generate a `BinderResult`, but it -/// typically returns Ok(value). -pub fn map_or_log_err(result: Result, handle_ok: F) -> BinderResult -where - F: FnOnce(U) -> BinderResult, -{ - result.map_or_else( - |e| { - log::error!("{:#?}", e); - let root_cause = e.root_cause(); - let rc = match root_cause.downcast_ref::() { - Some(Error::Rc(rcode)) => rcode.0, - Some(Error::Binder(_, _)) => ResponseCode::SYSTEM_ERROR.0, - None => match root_cause.downcast_ref::() { - Some(selinux::Error::PermissionDenied) => ResponseCode::PERMISSION_DENIED.0, - _ => ResponseCode::SYSTEM_ERROR.0, - }, - }; - Err(BinderStatus::new_service_specific_error( - rc, - anyhow_error_to_cstring(&e).as_deref(), - )) - }, - handle_ok, - ) +pub fn map_or_log_err(result: Result) -> BinderResult { + result.map_err(|e| { + log::error!("{:#?}", e); + let root_cause = e.root_cause(); + let rc = match root_cause.downcast_ref::() { + Some(Error::Rc(rcode)) => rcode.0, + Some(Error::Binder(_, _)) => ResponseCode::SYSTEM_ERROR.0, + None => match root_cause.downcast_ref::() { + Some(selinux::Error::PermissionDenied) => ResponseCode::PERMISSION_DENIED.0, + _ => ResponseCode::SYSTEM_ERROR.0, + }, + }; + BinderStatus::new_service_specific_error(rc, anyhow_error_to_cstring(&e).as_deref()) + }) } /// Rate info records how many failed attempts a client has made to display a protected @@ -354,20 +341,23 @@ impl IProtectedConfirmation for ApcManager { ) -> BinderResult<()> { // presentPrompt can take more time than other operations. let _wp = wd::watch_millis("IProtectedConfirmation::presentPrompt", 3000); - map_or_log_err( - self.present_prompt(listener, prompt_text, extra_data, locale, ui_option_flags), - Ok, - ) + map_or_log_err(self.present_prompt( + listener, + prompt_text, + extra_data, + locale, + ui_option_flags, + )) } fn cancelPrompt( &self, listener: &binder::Strong, ) -> BinderResult<()> { let _wp = wd::watch("IProtectedConfirmation::cancelPrompt"); - map_or_log_err(self.cancel_prompt(listener), Ok) + map_or_log_err(self.cancel_prompt(listener)) } fn isSupported(&self) -> BinderResult { let _wp = wd::watch("IProtectedConfirmation::isSupported"); - map_or_log_err(Self::is_supported(), Ok) + map_or_log_err(Self::is_supported()) } } diff --git a/keystore2/src/authorization.rs b/keystore2/src/authorization.rs index 5810c353..4636a665 100644 --- a/keystore2/src/authorization.rs +++ b/keystore2/src/authorization.rs @@ -64,38 +64,27 @@ pub enum Error { /// `selinux::Error::perm()` is mapped on `ResponseCode::PERMISSION_DENIED`. /// /// All non `Error` error conditions get mapped onto ResponseCode::SYSTEM_ERROR`. -/// -/// `handle_ok` will be called if `result` is `Ok(value)` where `value` will be passed -/// as argument to `handle_ok`. `handle_ok` must generate a `BinderResult`, but it -/// typically returns Ok(value). -pub fn map_or_log_err(result: Result, handle_ok: F) -> BinderResult -where - F: FnOnce(U) -> BinderResult, -{ - result.map_or_else( - |e| { - log::error!("{:#?}", e); - let root_cause = e.root_cause(); - if let Some(KeystoreError::Rc(ks_rcode)) = root_cause.downcast_ref::() { - let rc = match *ks_rcode { - // Although currently keystore2/ResponseCode.aidl and - // authorization/ResponseCode.aidl share the same integer values for the - // common response codes, this may deviate in the future, hence the - // conversion here. - KsResponseCode::SYSTEM_ERROR => ResponseCode::SYSTEM_ERROR.0, - KsResponseCode::KEY_NOT_FOUND => ResponseCode::KEY_NOT_FOUND.0, - KsResponseCode::VALUE_CORRUPTED => ResponseCode::VALUE_CORRUPTED.0, - KsResponseCode::INVALID_ARGUMENT => ResponseCode::INVALID_ARGUMENT.0, - // If the code paths of IKeystoreAuthorization aidl's methods happen to return - // other error codes from KsResponseCode in the future, they should be converted - // as well. - _ => ResponseCode::SYSTEM_ERROR.0, - }; - return Err(BinderStatus::new_service_specific_error( - rc, - anyhow_error_to_cstring(&e).as_deref(), - )); - } +pub fn map_or_log_err(result: Result) -> BinderResult { + result.map_err(|e| { + log::error!("{:#?}", e); + let root_cause = e.root_cause(); + if let Some(KeystoreError::Rc(ks_rcode)) = root_cause.downcast_ref::() { + let rc = match *ks_rcode { + // Although currently keystore2/ResponseCode.aidl and + // authorization/ResponseCode.aidl share the same integer values for the + // common response codes, this may deviate in the future, hence the + // conversion here. + KsResponseCode::SYSTEM_ERROR => ResponseCode::SYSTEM_ERROR.0, + KsResponseCode::KEY_NOT_FOUND => ResponseCode::KEY_NOT_FOUND.0, + KsResponseCode::VALUE_CORRUPTED => ResponseCode::VALUE_CORRUPTED.0, + KsResponseCode::INVALID_ARGUMENT => ResponseCode::INVALID_ARGUMENT.0, + // If the code paths of IKeystoreAuthorization aidl's methods happen to return + // other error codes from KsResponseCode in the future, they should be converted + // as well. + _ => ResponseCode::SYSTEM_ERROR.0, + }; + BinderStatus::new_service_specific_error(rc, anyhow_error_to_cstring(&e).as_deref()) + } else { let rc = match root_cause.downcast_ref::() { Some(Error::Rc(rcode)) => rcode.0, Some(Error::Binder(_, _)) => ResponseCode::SYSTEM_ERROR.0, @@ -104,13 +93,9 @@ where _ => ResponseCode::SYSTEM_ERROR.0, }, }; - Err(BinderStatus::new_service_specific_error( - rc, - anyhow_error_to_cstring(&e).as_deref(), - )) - }, - handle_ok, - ) + BinderStatus::new_service_specific_error(rc, anyhow_error_to_cstring(&e).as_deref()) + } + }) } /// This struct is defined to implement the aforementioned AIDL interface. @@ -272,12 +257,12 @@ impl Interface for AuthorizationManager {} impl IKeystoreAuthorization for AuthorizationManager { fn addAuthToken(&self, auth_token: &HardwareAuthToken) -> BinderResult<()> { let _wp = wd::watch("IKeystoreAuthorization::addAuthToken"); - map_or_log_err(self.add_auth_token(auth_token), Ok) + map_or_log_err(self.add_auth_token(auth_token)) } fn onDeviceUnlocked(&self, user_id: i32, password: Option<&[u8]>) -> BinderResult<()> { let _wp = wd::watch("IKeystoreAuthorization::onDeviceUnlocked"); - map_or_log_err(self.on_device_unlocked(user_id, password.map(|pw| pw.into())), Ok) + map_or_log_err(self.on_device_unlocked(user_id, password.map(|pw| pw.into()))) } fn onDeviceLocked( @@ -287,17 +272,17 @@ impl IKeystoreAuthorization for AuthorizationManager { weak_unlock_enabled: bool, ) -> BinderResult<()> { let _wp = wd::watch("IKeystoreAuthorization::onDeviceLocked"); - map_or_log_err(self.on_device_locked(user_id, unlocking_sids, weak_unlock_enabled), Ok) + map_or_log_err(self.on_device_locked(user_id, unlocking_sids, weak_unlock_enabled)) } fn onWeakUnlockMethodsExpired(&self, user_id: i32) -> BinderResult<()> { let _wp = wd::watch("IKeystoreAuthorization::onWeakUnlockMethodsExpired"); - map_or_log_err(self.on_weak_unlock_methods_expired(user_id), Ok) + map_or_log_err(self.on_weak_unlock_methods_expired(user_id)) } fn onNonLskfUnlockMethodsExpired(&self, user_id: i32) -> BinderResult<()> { let _wp = wd::watch("IKeystoreAuthorization::onNonLskfUnlockMethodsExpired"); - map_or_log_err(self.on_non_lskf_unlock_methods_expired(user_id), Ok) + map_or_log_err(self.on_non_lskf_unlock_methods_expired(user_id)) } fn getAuthTokensForCredStore( @@ -307,14 +292,11 @@ impl IKeystoreAuthorization for AuthorizationManager { auth_token_max_age_millis: i64, ) -> binder::Result { let _wp = wd::watch("IKeystoreAuthorization::getAuthTokensForCredStore"); - map_or_log_err( - self.get_auth_tokens_for_credstore( - challenge, - secure_user_id, - auth_token_max_age_millis, - ), - Ok, - ) + map_or_log_err(self.get_auth_tokens_for_credstore( + challenge, + secure_user_id, + auth_token_max_age_millis, + )) } fn getLastAuthTime( @@ -323,7 +305,7 @@ impl IKeystoreAuthorization for AuthorizationManager { auth_types: &[HardwareAuthenticatorType], ) -> binder::Result { if aconfig_android_hardware_biometrics_rust::last_authentication_time() { - map_or_log_err(self.get_last_auth_time(secure_user_id, auth_types), Ok) + map_or_log_err(self.get_last_auth_time(secure_user_id, auth_types)) } else { Err(BinderStatus::new_service_specific_error( ResponseCode::PERMISSION_DENIED.0, diff --git a/keystore2/src/error.rs b/keystore2/src/error.rs index f0d0d27b..b9a22916 100644 --- a/keystore2/src/error.rs +++ b/keystore2/src/error.rs @@ -167,42 +167,17 @@ pub fn map_binder_status_code(r: Result) -> Result { /// into service specific exceptions. /// /// All error conditions get logged by this function, except for KEY_NOT_FOUND error. -/// -/// `handle_ok` will be called if `result` is `Ok(value)` where `value` will be passed -/// as argument to `handle_ok`. `handle_ok` must generate a `BinderResult`, but it -/// typically returns Ok(value). -/// -/// # Examples -/// -/// ``` -/// fn loadKey() -> anyhow::Result> { -/// if (good_but_auth_required) { -/// Ok(vec!['k', 'e', 'y']) -/// } else { -/// Err(anyhow!(Error::Rc(ResponseCode::KEY_NOT_FOUND))) -/// } -/// } -/// -/// map_or_log_err(loadKey(), Ok) -/// ``` -pub fn map_or_log_err(result: anyhow::Result, handle_ok: F) -> BinderResult -where - F: FnOnce(U) -> BinderResult, -{ - map_err_with( - result, - |e| { - // Make the key not found errors silent. - if !matches!( - e.root_cause().downcast_ref::(), - Some(Error::Rc(ResponseCode::KEY_NOT_FOUND)) - ) { - log::error!("{:?}", e); - } - e - }, - handle_ok, - ) +pub fn map_or_log_err(result: anyhow::Result) -> BinderResult { + map_err_with(result, |e| { + // Make the key not found errors silent. + if !matches!( + e.root_cause().downcast_ref::(), + Some(Error::Rc(ResponseCode::KEY_NOT_FOUND)) + ) { + log::error!("{:?}", e); + } + e + }) } /// This function turns an anyhow error into an optional CString. @@ -222,26 +197,15 @@ pub fn anyhow_error_to_cstring(e: &anyhow::Error) -> Option { /// This function behaves similar to map_or_log_error, but it does not log the errors, instead /// it calls map_err on the error before mapping it to a binder result allowing callers to /// log or transform the error before mapping it. -pub fn map_err_with( - result: anyhow::Result, - map_err: F1, - handle_ok: F2, -) -> BinderResult +pub fn map_err_with(result: anyhow::Result, map_err: F1) -> BinderResult where F1: FnOnce(anyhow::Error) -> anyhow::Error, - F2: FnOnce(U) -> BinderResult, { - result.map_or_else( - |e| { - let e = map_err(e); - let rc = anyhow_error_to_serialized_error(&e); - Err(BinderStatus::new_service_specific_error( - rc.0, - anyhow_error_to_cstring(&e).as_deref(), - )) - }, - handle_ok, - ) + result.map_err(|e| { + let e = map_err(e); + let rc = anyhow_error_to_serialized_error(&e); + BinderStatus::new_service_specific_error(rc.0, anyhow_error_to_cstring(&e).as_deref()) + }) } /// This type is used to send error codes on the wire. @@ -359,8 +323,7 @@ pub mod tests { for rc in ResponseCode::LOCKED.0..ResponseCode::BACKEND_BUSY.0 { assert_eq!( Result::<(), i32>::Err(rc), - map_or_log_err(nested_rc(ResponseCode(rc)), |_| Err(BinderStatus::ok())) - .map_err(|s| s.service_specific_error()) + map_or_log_err(nested_rc(ResponseCode(rc))).map_err(|s| s.service_specific_error()) ); } @@ -369,8 +332,7 @@ pub mod tests { for ec in ErrorCode::UNKNOWN_ERROR.0..ErrorCode::ROOT_OF_TRUST_ALREADY_SET.0 { assert_eq!( Result::<(), i32>::Err(ec), - map_or_log_err(nested_ec(ErrorCode(ec)), |_| Err(BinderStatus::ok())) - .map_err(|s| s.service_specific_error()) + map_or_log_err(nested_ec(ErrorCode(ec))).map_err(|s| s.service_specific_error()) ); } @@ -381,8 +343,7 @@ pub mod tests { Result::<(), i32>::Err(ec), map_or_log_err( map_km_error(binder_sse_error(ec)) - .with_context(|| format!("Km error code: {}.", ec)), - |_| Err(BinderStatus::ok()) + .with_context(|| format!("Km error code: {}.", ec)) ) .map_err(|s| s.service_specific_error()) ); @@ -396,10 +357,8 @@ pub mod tests { // map_or_log_err then maps it on a service specific error of ResponseCode::SYSTEM_ERROR. assert_eq!( Result::<(), ResponseCode>::Err(ResponseCode::SYSTEM_ERROR), - map_or_log_err(sse.context("Non negative service specific error."), |_| Err( - BinderStatus::ok() - )) - .map_err(|s| ResponseCode(s.service_specific_error())) + map_or_log_err(sse.context("Non negative service specific error.")) + .map_err(|s| ResponseCode(s.service_specific_error())) ); // map_km_error creates a Error::Binder variant storing the given exception code. @@ -408,31 +367,29 @@ pub mod tests { // map_or_log_err then maps it on a service specific error of ResponseCode::SYSTEM_ERROR. assert_eq!( Result::<(), ResponseCode>::Err(ResponseCode::SYSTEM_ERROR), - map_or_log_err(binder_exception.context("Binder Exception."), |_| Err( - BinderStatus::ok() - )) - .map_err(|s| ResponseCode(s.service_specific_error())) + map_or_log_err(binder_exception.context("Binder Exception.")) + .map_err(|s| ResponseCode(s.service_specific_error())) ); // selinux::Error::Perm() needs to be mapped to ResponseCode::PERMISSION_DENIED assert_eq!( Result::<(), ResponseCode>::Err(ResponseCode::PERMISSION_DENIED), - map_or_log_err(nested_selinux_perm(), |_| Err(BinderStatus::ok())) + map_or_log_err(nested_selinux_perm()) .map_err(|s| ResponseCode(s.service_specific_error())) ); // All other errors get mapped on System Error. assert_eq!( Result::<(), ResponseCode>::Err(ResponseCode::SYSTEM_ERROR), - map_or_log_err(nested_other_error(), |_| Err(BinderStatus::ok())) + map_or_log_err(nested_other_error()) .map_err(|s| ResponseCode(s.service_specific_error())) ); // Result::Ok variants get passed to the ok handler. - assert_eq!(Ok(ResponseCode::LOCKED), map_or_log_err(nested_ok(ResponseCode::LOCKED), Ok)); + assert_eq!(Ok(ResponseCode::LOCKED), map_or_log_err(nested_ok(ResponseCode::LOCKED))); assert_eq!( Ok(ResponseCode::SYSTEM_ERROR), - map_or_log_err(nested_ok(ResponseCode::SYSTEM_ERROR), Ok) + map_or_log_err(nested_ok(ResponseCode::SYSTEM_ERROR)) ); Ok(()) diff --git a/keystore2/src/km_compat.rs b/keystore2/src/km_compat.rs index 03c9d027..02bcb1aa 100644 --- a/keystore2/src/km_compat.rs +++ b/keystore2/src/km_compat.rs @@ -226,7 +226,7 @@ where ) -> binder::Result { if self.emu.emulation_required(key_params, &KeyImportData::None) { let mut result = self.soft.generateKey(key_params, attestation_key)?; - result.keyBlob = map_or_log_err(wrap_keyblob(&result.keyBlob), Ok)?; + result.keyBlob = map_or_log_err(wrap_keyblob(&result.keyBlob))?; Ok(result) } else { self.real.generateKey(key_params, attestation_key) @@ -242,7 +242,7 @@ where if self.emu.emulation_required(key_params, &KeyImportData::new(key_format, key_data)?) { let mut result = self.soft.importKey(key_params, key_format, key_data, attestation_key)?; - result.keyBlob = map_or_log_err(wrap_keyblob(&result.keyBlob), Ok)?; + result.keyBlob = map_or_log_err(wrap_keyblob(&result.keyBlob))?; Ok(result) } else { self.real.importKey(key_params, key_format, key_data, attestation_key) @@ -281,7 +281,7 @@ where KeyBlob::Wrapped(keyblob) => { // Re-wrap the upgraded keyblob. let upgraded_keyblob = self.soft.upgradeKey(keyblob, upgrade_params)?; - map_or_log_err(wrap_keyblob(&upgraded_keyblob), Ok) + map_or_log_err(wrap_keyblob(&upgraded_keyblob)) } } } diff --git a/keystore2/src/maintenance.rs b/keystore2/src/maintenance.rs index 644e7e5e..c384a037 100644 --- a/keystore2/src/maintenance.rs +++ b/keystore2/src/maintenance.rs @@ -303,13 +303,13 @@ impl IKeystoreMaintenance for Maintenance { password.is_some() ); let _wp = wd::watch("IKeystoreMaintenance::onUserPasswordChanged"); - map_or_log_err(Self::on_user_password_changed(user_id, password.map(|pw| pw.into())), Ok) + map_or_log_err(Self::on_user_password_changed(user_id, password.map(|pw| pw.into()))) } fn onUserAdded(&self, user_id: i32) -> BinderResult<()> { log::info!("onUserAdded(user={user_id})"); let _wp = wd::watch("IKeystoreMaintenance::onUserAdded"); - map_or_log_err(self.add_or_remove_user(user_id), Ok) + map_or_log_err(self.add_or_remove_user(user_id)) } fn initUserSuperKeys( @@ -320,31 +320,31 @@ impl IKeystoreMaintenance for Maintenance { ) -> BinderResult<()> { log::info!("initUserSuperKeys(user={user_id}, allow_existing={allow_existing})"); let _wp = wd::watch("IKeystoreMaintenance::initUserSuperKeys"); - map_or_log_err(self.init_user_super_keys(user_id, password.into(), allow_existing), Ok) + map_or_log_err(self.init_user_super_keys(user_id, password.into(), allow_existing)) } fn onUserRemoved(&self, user_id: i32) -> BinderResult<()> { log::info!("onUserRemoved(user={user_id})"); let _wp = wd::watch("IKeystoreMaintenance::onUserRemoved"); - map_or_log_err(self.add_or_remove_user(user_id), Ok) + map_or_log_err(self.add_or_remove_user(user_id)) } fn onUserLskfRemoved(&self, user_id: i32) -> BinderResult<()> { log::info!("onUserLskfRemoved(user={user_id})"); let _wp = wd::watch("IKeystoreMaintenance::onUserLskfRemoved"); - map_or_log_err(Self::on_user_lskf_removed(user_id), Ok) + map_or_log_err(Self::on_user_lskf_removed(user_id)) } fn clearNamespace(&self, domain: Domain, nspace: i64) -> BinderResult<()> { log::info!("clearNamespace({domain:?}, nspace={nspace})"); let _wp = wd::watch("IKeystoreMaintenance::clearNamespace"); - map_or_log_err(self.clear_namespace(domain, nspace), Ok) + map_or_log_err(self.clear_namespace(domain, nspace)) } fn earlyBootEnded(&self) -> BinderResult<()> { log::info!("earlyBootEnded()"); let _wp = wd::watch("IKeystoreMaintenance::earlyBootEnded"); - map_or_log_err(Self::early_boot_ended(), Ok) + map_or_log_err(Self::early_boot_ended()) } fn migrateKeyNamespace( @@ -354,13 +354,13 @@ impl IKeystoreMaintenance for Maintenance { ) -> BinderResult<()> { log::info!("migrateKeyNamespace(src={source:?}, dest={destination:?})"); let _wp = wd::watch("IKeystoreMaintenance::migrateKeyNamespace"); - map_or_log_err(Self::migrate_key_namespace(source, destination), Ok) + map_or_log_err(Self::migrate_key_namespace(source, destination)) } fn deleteAllKeys(&self) -> BinderResult<()> { log::warn!("deleteAllKeys()"); let _wp = wd::watch("IKeystoreMaintenance::deleteAllKeys"); - map_or_log_err(Self::delete_all_keys(), Ok) + map_or_log_err(Self::delete_all_keys()) } fn getAppUidsAffectedBySid( @@ -370,6 +370,6 @@ impl IKeystoreMaintenance for Maintenance { ) -> BinderResult> { log::info!("getAppUidsAffectedBySid(secure_user_id={secure_user_id:?})"); let _wp = wd::watch("IKeystoreMaintenance::getAppUidsAffectedBySid"); - map_or_log_err(Self::get_app_uids_affected_by_sid(user_id, secure_user_id), Ok) + map_or_log_err(Self::get_app_uids_affected_by_sid(user_id, secure_user_id)) } } diff --git a/keystore2/src/metrics.rs b/keystore2/src/metrics.rs index c114c8ba..bbe12aa2 100644 --- a/keystore2/src/metrics.rs +++ b/keystore2/src/metrics.rs @@ -52,6 +52,6 @@ impl Interface for Metrics {} impl IKeystoreMetrics for Metrics { fn pullMetrics(&self, atom_id: AtomID) -> BinderResult> { let _wp = wd::watch("IKeystoreMetrics::pullMetrics"); - map_or_log_err(self.pull_metrics(atom_id), Ok) + map_or_log_err(self.pull_metrics(atom_id)) } } diff --git a/keystore2/src/operation.rs b/keystore2/src/operation.rs index 290bc74a..94bd7c33 100644 --- a/keystore2/src/operation.rs +++ b/keystore2/src/operation.rs @@ -822,24 +822,18 @@ impl binder::Interface for KeystoreOperation {} impl IKeystoreOperation for KeystoreOperation { fn updateAad(&self, aad_input: &[u8]) -> binder::Result<()> { let _wp = wd::watch("IKeystoreOperation::updateAad"); - map_or_log_err( - self.with_locked_operation( - |op| op.update_aad(aad_input).context(ks_err!("KeystoreOperation::updateAad")), - false, - ), - Ok, - ) + map_or_log_err(self.with_locked_operation( + |op| op.update_aad(aad_input).context(ks_err!("KeystoreOperation::updateAad")), + false, + )) } fn update(&self, input: &[u8]) -> binder::Result>> { let _wp = wd::watch("IKeystoreOperation::update"); - map_or_log_err( - self.with_locked_operation( - |op| op.update(input).context(ks_err!("KeystoreOperation::update")), - false, - ), - Ok, - ) + map_or_log_err(self.with_locked_operation( + |op| op.update(input).context(ks_err!("KeystoreOperation::update")), + false, + )) } fn finish( &self, @@ -847,13 +841,10 @@ impl IKeystoreOperation for KeystoreOperation { signature: Option<&[u8]>, ) -> binder::Result>> { let _wp = wd::watch("IKeystoreOperation::finish"); - map_or_log_err( - self.with_locked_operation( - |op| op.finish(input, signature).context(ks_err!("KeystoreOperation::finish")), - true, - ), - Ok, - ) + map_or_log_err(self.with_locked_operation( + |op| op.finish(input, signature).context(ks_err!("KeystoreOperation::finish")), + true, + )) } fn abort(&self) -> binder::Result<()> { @@ -873,7 +864,6 @@ impl IKeystoreOperation for KeystoreOperation { }; e }, - Ok, ) } } diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs index 59f23151..71d6dba6 100644 --- a/keystore2/src/security_level.rs +++ b/keystore2/src/security_level.rs @@ -996,7 +996,7 @@ impl IKeystoreSecurityLevel for KeystoreSecurityLevel { forced: bool, ) -> binder::Result { let _wp = self.watch("IKeystoreSecurityLevel::createOperation"); - map_or_log_err(self.create_operation(key, operation_parameters, forced), Ok) + map_or_log_err(self.create_operation(key, operation_parameters, forced)) } fn generateKey( &self, @@ -1012,7 +1012,7 @@ impl IKeystoreSecurityLevel for KeystoreSecurityLevel { let result = self.generate_key(key, attestation_key, params, flags, entropy); log_key_creation_event_stats(self.security_level, params, &result); log_key_generated(key, ThreadState::get_calling_uid(), result.is_ok()); - map_or_log_err(result, Ok) + map_or_log_err(result) } fn importKey( &self, @@ -1026,7 +1026,7 @@ impl IKeystoreSecurityLevel for KeystoreSecurityLevel { let result = self.import_key(key, attestation_key, params, flags, key_data); log_key_creation_event_stats(self.security_level, params, &result); log_key_imported(key, ThreadState::get_calling_uid(), result.is_ok()); - map_or_log_err(result, Ok) + map_or_log_err(result) } fn importWrappedKey( &self, @@ -1041,20 +1041,20 @@ impl IKeystoreSecurityLevel for KeystoreSecurityLevel { self.import_wrapped_key(key, wrapping_key, masking_key, params, authenticators); log_key_creation_event_stats(self.security_level, params, &result); log_key_imported(key, ThreadState::get_calling_uid(), result.is_ok()); - map_or_log_err(result, Ok) + map_or_log_err(result) } fn convertStorageKeyToEphemeral( &self, storage_key: &KeyDescriptor, ) -> binder::Result { let _wp = self.watch("IKeystoreSecurityLevel::convertStorageKeyToEphemeral"); - map_or_log_err(self.convert_storage_key_to_ephemeral(storage_key), Ok) + map_or_log_err(self.convert_storage_key_to_ephemeral(storage_key)) } fn deleteKey(&self, key: &KeyDescriptor) -> binder::Result<()> { let _wp = self.watch("IKeystoreSecurityLevel::deleteKey"); let result = self.delete_key(key); log_key_deleted(key, ThreadState::get_calling_uid(), result.is_ok()); - map_or_log_err(result, Ok) + map_or_log_err(result) } } diff --git a/keystore2/src/service.rs b/keystore2/src/service.rs index 1eab8a83..2ca9ccf8 100644 --- a/keystore2/src/service.rs +++ b/keystore2/src/service.rs @@ -384,11 +384,11 @@ impl IKeystoreService for KeystoreService { let _wp = wd::watch_millis_with("IKeystoreService::getSecurityLevel", 500, move || { format!("security_level: {}", security_level.0) }); - map_or_log_err(self.get_security_level(security_level), Ok) + map_or_log_err(self.get_security_level(security_level)) } fn getKeyEntry(&self, key: &KeyDescriptor) -> binder::Result { let _wp = wd::watch("IKeystoreService::get_key_entry"); - map_or_log_err(self.get_key_entry(key), Ok) + map_or_log_err(self.get_key_entry(key)) } fn updateSubcomponent( &self, @@ -397,17 +397,17 @@ impl IKeystoreService for KeystoreService { certificate_chain: Option<&[u8]>, ) -> binder::Result<()> { let _wp = wd::watch("IKeystoreService::updateSubcomponent"); - map_or_log_err(self.update_subcomponent(key, public_cert, certificate_chain), Ok) + map_or_log_err(self.update_subcomponent(key, public_cert, certificate_chain)) } fn listEntries(&self, domain: Domain, namespace: i64) -> binder::Result> { let _wp = wd::watch("IKeystoreService::listEntries"); - map_or_log_err(self.list_entries(domain, namespace), Ok) + map_or_log_err(self.list_entries(domain, namespace)) } fn deleteKey(&self, key: &KeyDescriptor) -> binder::Result<()> { let _wp = wd::watch("IKeystoreService::deleteKey"); let result = self.delete_key(key); log_key_deleted(key, ThreadState::get_calling_uid(), result.is_ok()); - map_or_log_err(result, Ok) + map_or_log_err(result) } fn grant( &self, @@ -416,11 +416,11 @@ impl IKeystoreService for KeystoreService { access_vector: i32, ) -> binder::Result { let _wp = wd::watch("IKeystoreService::grant"); - map_or_log_err(self.grant(key, grantee_uid, access_vector.into()), Ok) + map_or_log_err(self.grant(key, grantee_uid, access_vector.into())) } fn ungrant(&self, key: &KeyDescriptor, grantee_uid: i32) -> binder::Result<()> { let _wp = wd::watch("IKeystoreService::ungrant"); - map_or_log_err(self.ungrant(key, grantee_uid), Ok) + map_or_log_err(self.ungrant(key, grantee_uid)) } fn listEntriesBatched( &self, @@ -429,11 +429,11 @@ impl IKeystoreService for KeystoreService { start_past_alias: Option<&str>, ) -> binder::Result> { let _wp = wd::watch("IKeystoreService::listEntriesBatched"); - map_or_log_err(self.list_entries_batched(domain, namespace, start_past_alias), Ok) + map_or_log_err(self.list_entries_batched(domain, namespace, start_past_alias)) } fn getNumberOfEntries(&self, domain: Domain, namespace: i64) -> binder::Result { let _wp = wd::watch("IKeystoreService::getNumberOfEntries"); - map_or_log_err(self.count_num_entries(domain, namespace), Ok) + map_or_log_err(self.count_num_entries(domain, namespace)) } }