Keystore 2.0: Refactor permissions. 4/5
Remove obsolete constructor functions for permissions.
Test: keystore2_test
Bug: 203555519
Change-Id: I4ff3ff91d8a5dcca99db02ddbd5894c91c405389
diff --git a/keystore2/selinux/src/lib.rs b/keystore2/selinux/src/lib.rs
index 1d9ac12..c0593b7 100644
--- a/keystore2/selinux/src/lib.rs
+++ b/keystore2/selinux/src/lib.rs
@@ -345,8 +345,6 @@
/// The example below implements `enum MyPermission with public visibility:
/// * From<i32> and Into<i32> are implemented. Where the implementation of From maps
/// any variant not specified to the default `None` with value `0`.
-/// * Every variant has a constructor with a name corresponding to its lower case SELinux string
-/// representation.
/// * `MyPermission` implements ClassPermission.
/// * An implicit default values `MyPermission::None` is created with a numeric representation
/// of `0` and a string representation of `"none"`.
@@ -632,15 +630,6 @@
stringify!($class_name)
}
}
-
- impl $enum_name {
- /// Creates an instance representing a permission with the same name.
- pub const fn none() -> Self { Self::None }
- $(
- /// Creates an instance representing a permission with the same name.
- pub const fn $selinux_name() -> Self { Self::$vname }
- )*
- }
};
}
diff --git a/keystore2/src/authorization.rs b/keystore2/src/authorization.rs
index 777089f..04626bc 100644
--- a/keystore2/src/authorization.rs
+++ b/keystore2/src/authorization.rs
@@ -119,7 +119,7 @@
fn add_auth_token(&self, auth_token: &HardwareAuthToken) -> Result<()> {
// Check keystore permission.
- check_keystore_permission(KeystorePerm::add_auth()).context("In add_auth_token.")?;
+ check_keystore_permission(KeystorePerm::AddAuth).context("In add_auth_token.")?;
ENFORCEMENTS.add_auth_token(auth_token.clone());
Ok(())
@@ -143,7 +143,7 @@
(LockScreenEvent::UNLOCK, Some(password)) => {
// This corresponds to the unlock() method in legacy keystore API.
// check permission
- check_keystore_permission(KeystorePerm::unlock())
+ check_keystore_permission(KeystorePerm::Unlock)
.context("In on_lock_screen_event: Unlock with password.")?;
ENFORCEMENTS.set_device_locked(user_id, false);
@@ -177,7 +177,7 @@
Ok(())
}
(LockScreenEvent::UNLOCK, None) => {
- check_keystore_permission(KeystorePerm::unlock())
+ check_keystore_permission(KeystorePerm::Unlock)
.context("In on_lock_screen_event: Unlock.")?;
ENFORCEMENTS.set_device_locked(user_id, false);
DB.with(|db| {
@@ -187,7 +187,7 @@
Ok(())
}
(LockScreenEvent::LOCK, None) => {
- check_keystore_permission(KeystorePerm::lock())
+ check_keystore_permission(KeystorePerm::Lock)
.context("In on_lock_screen_event: Lock")?;
ENFORCEMENTS.set_device_locked(user_id, true);
DB.with(|db| {
@@ -215,7 +215,7 @@
) -> Result<AuthorizationTokens> {
// Check permission. Function should return if this failed. Therefore having '?' at the end
// is very important.
- check_keystore_permission(KeystorePerm::get_auth_token())
+ check_keystore_permission(KeystorePerm::GetAuthToken)
.context("In get_auth_tokens_for_credstore.")?;
// If the challenge is zero, return error
diff --git a/keystore2/src/maintenance.rs b/keystore2/src/maintenance.rs
index 7ce9042..eb06784 100644
--- a/keystore2/src/maintenance.rs
+++ b/keystore2/src/maintenance.rs
@@ -69,7 +69,7 @@
fn on_user_password_changed(user_id: i32, password: Option<Password>) -> Result<()> {
//Check permission. Function should return if this failed. Therefore having '?' at the end
//is very important.
- check_keystore_permission(KeystorePerm::change_password())
+ check_keystore_permission(KeystorePerm::ChangePassword)
.context("In on_user_password_changed.")?;
if let Some(pw) = password.as_ref() {
@@ -106,7 +106,7 @@
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.")?;
+ check_keystore_permission(KeystorePerm::ChangeUser).context("In add_or_remove_user.")?;
DB.with(|db| {
UserState::reset_user(
&mut db.borrow_mut(),
@@ -124,7 +124,7 @@
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.")?;
+ check_keystore_permission(KeystorePerm::ClearUID).context("In clear_namespace.")?;
LEGACY_MIGRATOR
.bulk_delete_uid(domain, nspace)
@@ -139,7 +139,7 @@
fn get_state(user_id: i32) -> Result<AidlUserState> {
// Check permission. Function should return if this failed. Therefore having '?' at the end
// is very important.
- check_keystore_permission(KeystorePerm::get_state()).context("In get_state.")?;
+ check_keystore_permission(KeystorePerm::GetState).context("In get_state.")?;
let state = DB
.with(|db| {
UserState::get(&mut db.borrow_mut(), &LEGACY_MIGRATOR, &SUPER_KEY, user_id as u32)
@@ -195,7 +195,7 @@
}
fn early_boot_ended() -> Result<()> {
- check_keystore_permission(KeystorePerm::early_boot_ended())
+ check_keystore_permission(KeystorePerm::EarlyBootEnded)
.context("In early_boot_ended. Checking permission")?;
log::info!("In early_boot_ended.");
@@ -207,8 +207,7 @@
fn on_device_off_body() -> Result<()> {
// Security critical permission check. This statement must return on fail.
- check_keystore_permission(KeystorePerm::report_off_body())
- .context("In on_device_off_body.")?;
+ check_keystore_permission(KeystorePerm::ReportOffBody).context("In on_device_off_body.")?;
DB.with(|db| db.borrow_mut().update_last_off_body(MonotonicRawTime::now()));
Ok(())
@@ -253,7 +252,7 @@
fn delete_all_keys() -> Result<()> {
// Security critical permission check. This statement must return on fail.
- check_keystore_permission(KeystorePerm::delete_all_keys())
+ check_keystore_permission(KeystorePerm::DeleteAllKeys)
.context("In delete_all_keys. Checking permission")?;
log::info!("In delete_all_keys.");
diff --git a/keystore2/src/metrics.rs b/keystore2/src/metrics.rs
index 42295b7..3d8d6d3 100644
--- a/keystore2/src/metrics.rs
+++ b/keystore2/src/metrics.rs
@@ -41,7 +41,7 @@
fn pull_metrics(&self, atom_id: AtomID) -> Result<Vec<KeystoreAtom>> {
// Check permission. Function should return if this failed. Therefore having '?' at the end
// is very important.
- check_keystore_permission(KeystorePerm::pull_metrics()).context("In pull_metrics.")?;
+ check_keystore_permission(KeystorePerm::PullMetrics).context("In pull_metrics.")?;
METRICS_STORE.get_atoms(atom_id)
}
}
diff --git a/keystore2/src/permission.rs b/keystore2/src/permission.rs
index 89cbd5d..0a8eec7 100644
--- a/keystore2/src/permission.rs
+++ b/keystore2/src/permission.rs
@@ -600,28 +600,26 @@
#[test]
fn check_keystore_permission_test() -> Result<()> {
let system_server_ctx = Context::new("u:r:system_server:s0")?;
- assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::add_auth()).is_ok());
- assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::clear_ns()).is_ok());
- assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::get_state()).is_ok());
- assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::lock()).is_ok());
- assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::reset()).is_ok());
- assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::unlock()).is_ok());
- assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::change_user()).is_ok());
- assert!(
- check_keystore_permission(&system_server_ctx, KeystorePerm::change_password()).is_ok()
- );
- assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::clear_uid()).is_ok());
+ assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::AddAuth).is_ok());
+ assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::ClearNs).is_ok());
+ assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::GetState).is_ok());
+ assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::Lock).is_ok());
+ assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::Reset).is_ok());
+ assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::Unlock).is_ok());
+ assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::ChangeUser).is_ok());
+ assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::ChangePassword).is_ok());
+ assert!(check_keystore_permission(&system_server_ctx, KeystorePerm::ClearUID).is_ok());
let shell_ctx = Context::new("u:r:shell:s0")?;
- assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::add_auth()));
- assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::clear_ns()));
- assert!(check_keystore_permission(&shell_ctx, KeystorePerm::get_state()).is_ok());
- assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::list()));
- assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::lock()));
- assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::reset()));
- assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::unlock()));
- assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::change_user()));
- assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::change_password()));
- assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::clear_uid()));
+ assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::AddAuth));
+ assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::ClearNs));
+ assert!(check_keystore_permission(&shell_ctx, KeystorePerm::GetState).is_ok());
+ assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::List));
+ assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::Lock));
+ assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::Reset));
+ assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::Unlock));
+ assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::ChangeUser));
+ assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::ChangePassword));
+ assert_perm_failed!(check_keystore_permission(&shell_ctx, KeystorePerm::ClearUID));
Ok(())
}
diff --git a/keystore2/src/service.rs b/keystore2/src/service.rs
index b35fe36..c6d466d 100644
--- a/keystore2/src/service.rs
+++ b/keystore2/src/service.rs
@@ -274,7 +274,7 @@
if let Some(selinux::Error::PermissionDenied) =
e.root_cause().downcast_ref::<selinux::Error>()
{
- check_keystore_permission(KeystorePerm::list())
+ check_keystore_permission(KeystorePerm::List)
.context("In list_entries: While checking keystore permission.")?;
if namespace != -1 {
k.nspace = namespace;