From de45c8b9995a6f8ebd3bc13c15ee0deba1a5e1ea Mon Sep 17 00:00:00 2001 From: Andrew Walbran Date: Tue, 13 Apr 2021 14:42:38 +0000 Subject: [PATCH] Moving set_requesting_sid to new_binder method. Bug: 178852354 Test: mm Change-Id: Ib6af028b19d36965ad2de62c8ddc52936b6abec2 --- keystore2/src/apc.rs | 13 ++++++------- keystore2/src/authorization.rs | 10 +++++----- keystore2/src/maintenance.rs | 12 +++++++----- keystore2/src/operation.rs | 14 +++++++------- keystore2/src/remote_provisioning.rs | 4 ++-- keystore2/src/security_level.rs | 28 +++++++++++++++------------- keystore2/src/service.rs | 9 +++++---- keystore2/vpnprofilestore/lib.rs | 22 ++++++++++------------ 8 files changed, 57 insertions(+), 55 deletions(-) diff --git a/keystore2/src/apc.rs b/keystore2/src/apc.rs index 46b71dd9..848b7701 100644 --- a/keystore2/src/apc.rs +++ b/keystore2/src/apc.rs @@ -28,10 +28,10 @@ use android_security_apc::aidl::android::security::apc::{ ResponseCode::ResponseCode, }; use android_security_apc::binder::{ - ExceptionCode, Interface, Result as BinderResult, SpIBinder, Status as BinderStatus, Strong, + BinderFeatures, ExceptionCode, Interface, Result as BinderResult, SpIBinder, + Status as BinderStatus, Strong, ThreadState, }; use anyhow::{Context, Result}; -use binder::{IBinderInternal, ThreadState}; use keystore2_apc_compat::ApcHal; use keystore2_selinux as selinux; use std::time::{Duration, Instant}; @@ -203,11 +203,10 @@ impl ApcManager { pub fn new_native_binder( confirmation_token_sender: Sender>, ) -> Result> { - let result = BnProtectedConfirmation::new_binder(Self { - state: Arc::new(Mutex::new(ApcState::new(confirmation_token_sender))), - }); - result.as_binder().set_requesting_sid(true); - Ok(result) + Ok(BnProtectedConfirmation::new_binder( + Self { state: Arc::new(Mutex::new(ApcState::new(confirmation_token_sender))) }, + BinderFeatures { set_requesting_sid: true, ..BinderFeatures::default() }, + )) } fn result( diff --git a/keystore2/src/authorization.rs b/keystore2/src/authorization.rs index 06b55982..0a8fe25d 100644 --- a/keystore2/src/authorization.rs +++ b/keystore2/src/authorization.rs @@ -22,7 +22,7 @@ use crate::utils::check_keystore_permission; use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{ HardwareAuthToken::HardwareAuthToken, }; -use android_security_authorization::binder::{ExceptionCode, Interface, Result as BinderResult, +use android_security_authorization::binder::{BinderFeatures,ExceptionCode, Interface, Result as BinderResult, Strong, Status as BinderStatus}; use android_security_authorization::aidl::android::security::authorization::{ IKeystoreAuthorization::BnKeystoreAuthorization, IKeystoreAuthorization::IKeystoreAuthorization, @@ -32,7 +32,6 @@ use android_security_authorization::aidl::android::security::authorization::{ use android_system_keystore2::aidl::android::system::keystore2::{ ResponseCode::ResponseCode as KsResponseCode }; use anyhow::{Context, Result}; -use binder::IBinderInternal; use keystore2_crypto::Password; use keystore2_selinux as selinux; @@ -112,9 +111,10 @@ pub struct AuthorizationManager; impl AuthorizationManager { /// Create a new instance of Keystore Authorization service. pub fn new_native_binder() -> Result> { - let result = BnKeystoreAuthorization::new_binder(Self); - result.as_binder().set_requesting_sid(true); - Ok(result) + Ok(BnKeystoreAuthorization::new_binder( + Self, + BinderFeatures { set_requesting_sid: true, ..BinderFeatures::default() }, + )) } fn add_auth_token(&self, auth_token: &HardwareAuthToken) -> Result<()> { diff --git a/keystore2/src/maintenance.rs b/keystore2/src/maintenance.rs index 1691f9d3..9e7576e0 100644 --- a/keystore2/src/maintenance.rs +++ b/keystore2/src/maintenance.rs @@ -29,13 +29,14 @@ use android_security_maintenance::aidl::android::security::maintenance::{ IKeystoreMaintenance::{BnKeystoreMaintenance, IKeystoreMaintenance}, UserState::UserState as AidlUserState, }; -use android_security_maintenance::binder::{Interface, Result as BinderResult}; +use android_security_maintenance::binder::{ + BinderFeatures, Interface, Result as BinderResult, Strong, ThreadState, +}; 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 binder::{IBinderInternal, Strong, ThreadState}; use keystore2_crypto::Password; /// This struct is defined to implement the aforementioned AIDL interface. @@ -45,9 +46,10 @@ pub struct Maintenance; impl Maintenance { /// Create a new instance of Keystore User Manager service. pub fn new_native_binder() -> Result> { - let result = BnKeystoreMaintenance::new_binder(Self); - result.as_binder().set_requesting_sid(true); - Ok(result) + Ok(BnKeystoreMaintenance::new_binder( + Self, + BinderFeatures { set_requesting_sid: true, ..BinderFeatures::default() }, + )) } fn on_user_password_changed(user_id: i32, password: Option) -> Result<()> { diff --git a/keystore2/src/operation.rs b/keystore2/src/operation.rs index c2539a7b..0b5c77a0 100644 --- a/keystore2/src/operation.rs +++ b/keystore2/src/operation.rs @@ -133,11 +133,11 @@ use android_hardware_security_keymint::aidl::android::hardware::security::keymin IKeyMintOperation::IKeyMintOperation, KeyParameter::KeyParameter, KeyPurpose::KeyPurpose, SecurityLevel::SecurityLevel, }; +use android_hardware_security_keymint::binder::BinderFeatures; use android_system_keystore2::aidl::android::system::keystore2::{ IKeystoreOperation::BnKeystoreOperation, IKeystoreOperation::IKeystoreOperation, }; use anyhow::{anyhow, Context, Result}; -use binder::IBinderInternal; use std::{ collections::HashMap, sync::{Arc, Mutex, MutexGuard, Weak}, @@ -783,16 +783,16 @@ pub struct KeystoreOperation { impl KeystoreOperation { /// Creates a new operation instance wrapped in a - /// BnKeystoreOperation proxy object. It also - /// calls `IBinderInternal::set_requesting_sid` on the new interface, because + /// BnKeystoreOperation proxy object. It also enables + /// `BinderFeatures::set_requesting_sid` on the new interface, because /// we need it for checking Keystore permissions. pub fn new_native_binder( operation: Arc, ) -> binder::public_api::Strong { - let result = - BnKeystoreOperation::new_binder(Self { operation: Mutex::new(Some(operation)) }); - result.as_binder().set_requesting_sid(true); - result + BnKeystoreOperation::new_binder( + Self { operation: Mutex::new(Some(operation)) }, + BinderFeatures { set_requesting_sid: true, ..BinderFeatures::default() }, + ) } /// Grabs the outer operation mutex and calls `f` on the locked operation. diff --git a/keystore2/src/remote_provisioning.rs b/keystore2/src/remote_provisioning.rs index f99805da..f8ee3697 100644 --- a/keystore2/src/remote_provisioning.rs +++ b/keystore2/src/remote_provisioning.rs @@ -34,7 +34,7 @@ use android_security_remoteprovisioning::aidl::android::security::remoteprovisio AttestationPoolStatus::AttestationPoolStatus, IRemoteProvisioning::BnRemoteProvisioning, IRemoteProvisioning::IRemoteProvisioning, }; -use android_security_remoteprovisioning::binder::Strong; +use android_security_remoteprovisioning::binder::{BinderFeatures, Strong}; use android_system_keystore2::aidl::android::system::keystore2::{ Domain::Domain, KeyDescriptor::KeyDescriptor, }; @@ -233,7 +233,7 @@ impl RemoteProvisioningService { if let Ok(dev) = get_remotely_provisioned_component(&SecurityLevel::STRONGBOX) { result.device_by_sec_level.insert(SecurityLevel::STRONGBOX, dev); } - Ok(BnRemoteProvisioning::new_binder(result)) + Ok(BnRemoteProvisioning::new_binder(result, BinderFeatures::default())) } /// Populates the AttestationPoolStatus parcelable with information about how many diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs index c654c02f..8530a2eb 100644 --- a/keystore2/src/security_level.rs +++ b/keystore2/src/security_level.rs @@ -22,6 +22,7 @@ use android_hardware_security_keymint::aidl::android::hardware::security::keymin KeyMintHardwareInfo::KeyMintHardwareInfo, KeyParameter::KeyParameter, KeyParameterValue::KeyParameterValue, SecurityLevel::SecurityLevel, Tag::Tag, }; +use android_hardware_security_keymint::binder::{BinderFeatures, Strong, ThreadState}; use android_system_keystore2::aidl::android::system::keystore2::{ AuthenticatorSpec::AuthenticatorSpec, CreateOperationResponse::CreateOperationResponse, Domain::Domain, IKeystoreOperation::IKeystoreOperation, @@ -57,7 +58,6 @@ use crate::{ utils::key_characteristics_to_internal, }; use anyhow::{anyhow, Context, Result}; -use binder::{IBinderInternal, Strong, ThreadState}; /// Implementation of the IKeystoreSecurityLevel Interface. pub struct KeystoreSecurityLevel { @@ -79,8 +79,8 @@ const UNDEFINED_NOT_AFTER: i64 = 253402300799000i64; impl KeystoreSecurityLevel { /// Creates a new security level instance wrapped in a - /// BnKeystoreSecurityLevel proxy object. It also - /// calls `IBinderInternal::set_requesting_sid` on the new interface, because + /// BnKeystoreSecurityLevel proxy object. It also enables + /// `BinderFeatures::set_requesting_sid` on the new interface, because /// we need it for checking keystore permissions. pub fn new_native_binder( security_level: SecurityLevel, @@ -88,16 +88,18 @@ impl KeystoreSecurityLevel { ) -> Result<(Strong, Uuid)> { let (dev, hw_info, km_uuid) = get_keymint_device(&security_level) .context("In KeystoreSecurityLevel::new_native_binder.")?; - let result = BnKeystoreSecurityLevel::new_binder(Self { - security_level, - keymint: dev, - hw_info, - km_uuid, - operation_db: OperationDb::new(), - rem_prov_state: RemProvState::new(security_level, km_uuid), - id_rotation_state, - }); - result.as_binder().set_requesting_sid(true); + let result = BnKeystoreSecurityLevel::new_binder( + Self { + security_level, + keymint: dev, + hw_info, + km_uuid, + operation_db: OperationDb::new(), + rem_prov_state: RemProvState::new(security_level, km_uuid), + id_rotation_state, + }, + BinderFeatures { set_requesting_sid: true, ..BinderFeatures::default() }, + ); Ok((result, km_uuid)) } diff --git a/keystore2/src/service.rs b/keystore2/src/service.rs index 1debe1b5..8d3b66e2 100644 --- a/keystore2/src/service.rs +++ b/keystore2/src/service.rs @@ -37,13 +37,13 @@ use crate::{ id_rotation::IdRotationState, }; use android_hardware_security_keymint::aidl::android::hardware::security::keymint::SecurityLevel::SecurityLevel; +use android_hardware_security_keymint::binder::{BinderFeatures, Strong, ThreadState}; use android_system_keystore2::aidl::android::system::keystore2::{ Domain::Domain, IKeystoreSecurityLevel::IKeystoreSecurityLevel, IKeystoreService::BnKeystoreService, IKeystoreService::IKeystoreService, KeyDescriptor::KeyDescriptor, KeyEntryResponse::KeyEntryResponse, KeyMetadata::KeyMetadata, }; use anyhow::{Context, Result}; -use binder::{IBinderInternal, Strong, ThreadState}; use error::Error; use keystore2_selinux as selinux; @@ -90,9 +90,10 @@ impl KeystoreService { "In KeystoreService::new_native_binder: Trying to initialize the legacy migrator.", )?; - let result = BnKeystoreService::new_binder(result); - result.as_binder().set_requesting_sid(true); - Ok(result) + Ok(BnKeystoreService::new_binder( + result, + BinderFeatures { set_requesting_sid: true, ..BinderFeatures::default() }, + )) } fn uuid_to_sec_level(&self, uuid: &Uuid) -> SecurityLevel { diff --git a/keystore2/vpnprofilestore/lib.rs b/keystore2/vpnprofilestore/lib.rs index f5adc1b0..e7340a0d 100644 --- a/keystore2/vpnprofilestore/lib.rs +++ b/keystore2/vpnprofilestore/lib.rs @@ -18,9 +18,11 @@ use android_security_vpnprofilestore::aidl::android::security::vpnprofilestore:: IVpnProfileStore::BnVpnProfileStore, IVpnProfileStore::IVpnProfileStore, IVpnProfileStore::ERROR_PROFILE_NOT_FOUND, IVpnProfileStore::ERROR_SYSTEM_ERROR, }; -use android_security_vpnprofilestore::binder::{Result as BinderResult, Status as BinderStatus}; +use android_security_vpnprofilestore::binder::{ + BinderFeatures, ExceptionCode, Result as BinderResult, Status as BinderStatus, Strong, + ThreadState, +}; use anyhow::{Context, Result}; -use binder::{ExceptionCode, Strong, ThreadState}; use keystore2::{async_task::AsyncTask, legacy_blob::LegacyBlobLoader}; use rusqlite::{ params, Connection, OptionalExtension, Transaction, TransactionBehavior, NO_PARAMS, @@ -75,15 +77,11 @@ impl DB { } fn is_locked_error(e: &anyhow::Error) -> bool { - matches!(e.root_cause().downcast_ref::(), - Some(rusqlite::ffi::Error { - code: rusqlite::ErrorCode::DatabaseBusy, - .. - }) - | Some(rusqlite::ffi::Error { - code: rusqlite::ErrorCode::DatabaseLocked, - .. - })) + matches!( + e.root_cause().downcast_ref::(), + Some(rusqlite::ffi::Error { code: rusqlite::ErrorCode::DatabaseBusy, .. }) + | Some(rusqlite::ffi::Error { code: rusqlite::ErrorCode::DatabaseLocked, .. }) + ) } fn init_tables(&mut self) -> Result<()> { @@ -224,7 +222,7 @@ impl VpnProfileStore { let result = Self { db_path, async_task: Default::default() }; result.init_shelf(path); - BnVpnProfileStore::new_binder(result) + BnVpnProfileStore::new_binder(result, BinderFeatures::default()) } fn open_db(&self) -> Result {