Keystore 2.0: Allow by key id usage of granted keys.
When keys are loaded by grant they may be used by key id subsequently.
This patch adds a check of the grant database when loading the access
tuple. If one is found the access vector is populated allowing
the permission callback to perform access control based on the grant.
Test: keystore2_test
Change-Id: If70dfbc035aed5aa3842663d475b489df3e3dd4e
diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs
index ffec931..db30e07 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -1229,18 +1229,18 @@
// Domain::KEY_ID. In this case we load the domain and namespace from the
// keyentry database because we need them for access control.
Domain::KEY_ID => {
- let mut stmt = tx
- .prepare(
- "SELECT domain, namespace FROM persistent.keyentry
- WHERE
- id = ?
- AND state = ?;",
- )
- .context("Domain::KEY_ID: prepare statement failed")?;
- let mut rows = stmt
- .query(params![key.nspace, KeyLifeCycle::Live])
- .context("Domain::KEY_ID: query failed.")?;
- let (domain, namespace): (Domain, i64) =
+ let (domain, namespace): (Domain, i64) = {
+ let mut stmt = tx
+ .prepare(
+ "SELECT domain, namespace FROM persistent.keyentry
+ WHERE
+ id = ?
+ AND state = ?;",
+ )
+ .context("Domain::KEY_ID: prepare statement failed")?;
+ let mut rows = stmt
+ .query(params![key.nspace, KeyLifeCycle::Live])
+ .context("Domain::KEY_ID: query failed.")?;
db_utils::with_rows_extract_one(&mut rows, |row| {
let r =
row.map_or_else(|| Err(KsError::Rc(ResponseCode::KEY_NOT_FOUND)), Ok)?;
@@ -1249,13 +1249,37 @@
r.get(1).context("Failed to unpack namespace.")?,
))
})
- .context("Domain::KEY_ID.")?;
+ .context("Domain::KEY_ID.")?
+ };
+
+ // We may use a key by id after loading it by grant.
+ // In this case we have to check if the caller has a grant for this particular
+ // key. We can skip this if we already know that the caller is the owner.
+ // But we cannot know this if domain is anything but App. E.g. in the case
+ // of Domain::SELINUX we have to speculatively check for grants because we have to
+ // consult the SEPolicy before we know if the caller is the owner.
+ let access_vector: Option<KeyPermSet> =
+ if domain != Domain::APP || namespace != caller_uid as i64 {
+ let access_vector: Option<i32> = tx
+ .query_row(
+ "SELECT access_vector FROM persistent.grant
+ WHERE grantee = ? AND keyentryid = ?;",
+ params![caller_uid as i64, key.nspace],
+ |row| row.get(0),
+ )
+ .optional()
+ .context("Domain::KEY_ID: query grant failed.")?;
+ access_vector.map(|p| p.into())
+ } else {
+ None
+ };
+
let key_id = key.nspace;
let mut access_key = key;
access_key.domain = domain;
access_key.nspace = namespace;
- Ok((key_id, access_key, None))
+ Ok((key_id, access_key, access_vector))
}
_ => Err(anyhow!(KsError::sys())),
}
@@ -2436,6 +2460,90 @@
Ok(())
}
+ // This test attempts to load a key by key id while the caller is not the owner
+ // but a grant exists for the given key and the caller.
+ #[test]
+ fn test_insert_and_load_full_keyentry_from_grant_by_key_id() -> Result<()> {
+ let mut db = new_test_db()?;
+ const OWNER_UID: u32 = 1u32;
+ const GRANTEE_UID: u32 = 2u32;
+ const SOMEONE_ELSE_UID: u32 = 3u32;
+ let key_id = make_test_key_entry(&mut db, Domain::APP, OWNER_UID as i64, TEST_ALIAS, None)
+ .context("test_insert_and_load_full_keyentry_from_grant_by_key_id")?
+ .0;
+
+ db.grant(
+ KeyDescriptor {
+ domain: Domain::APP,
+ nspace: 0,
+ alias: Some(TEST_ALIAS.to_string()),
+ blob: None,
+ },
+ OWNER_UID,
+ GRANTEE_UID,
+ key_perm_set![KeyPerm::use_()],
+ |_k, _av| Ok(()),
+ )
+ .unwrap();
+
+ debug_dump_grant_table(&mut db)?;
+
+ let id_descriptor =
+ KeyDescriptor { domain: Domain::KEY_ID, nspace: key_id, ..Default::default() };
+
+ let (_, key_entry) = db
+ .load_key_entry(
+ id_descriptor.clone(),
+ KeyType::Client,
+ KeyEntryLoadBits::BOTH,
+ GRANTEE_UID,
+ |k, av| {
+ assert_eq!(Domain::APP, k.domain);
+ assert_eq!(OWNER_UID as i64, k.nspace);
+ assert!(av.unwrap().includes(KeyPerm::use_()));
+ Ok(())
+ },
+ )
+ .unwrap();
+
+ assert_eq!(key_entry, make_test_key_entry_test_vector(key_id, None));
+
+ let (_, key_entry) = db
+ .load_key_entry(
+ id_descriptor.clone(),
+ KeyType::Client,
+ KeyEntryLoadBits::BOTH,
+ SOMEONE_ELSE_UID,
+ |k, av| {
+ assert_eq!(Domain::APP, k.domain);
+ assert_eq!(OWNER_UID as i64, k.nspace);
+ assert!(av.is_none());
+ Ok(())
+ },
+ )
+ .unwrap();
+
+ assert_eq!(key_entry, make_test_key_entry_test_vector(key_id, None));
+
+ db.unbind_key(id_descriptor.clone(), KeyType::Client, OWNER_UID, |_, _| Ok(())).unwrap();
+
+ assert_eq!(
+ Some(&KsError::Rc(ResponseCode::KEY_NOT_FOUND)),
+ db.load_key_entry(
+ id_descriptor,
+ KeyType::Client,
+ KeyEntryLoadBits::NONE,
+ GRANTEE_UID,
+ |_k, _av| Ok(()),
+ )
+ .unwrap_err()
+ .root_cause()
+ .downcast_ref::<KsError>()
+ );
+
+ Ok(())
+ }
+
static KEY_LOCK_TEST_ALIAS: &str = "my super duper locked key";
#[test]
diff --git a/keystore2/src/permission.rs b/keystore2/src/permission.rs
index 0917256..a81954f 100644
--- a/keystore2/src/permission.rs
+++ b/keystore2/src/permission.rs
@@ -482,25 +482,41 @@
/// was supplied. It is also produced if `Domain::KEY_ID` was selected, and
/// on various unexpected backend failures.
pub fn check_key_permission(
+ caller_uid: u32,
caller_ctx: &CStr,
perm: KeyPerm,
key: &KeyDescriptor,
access_vector: &Option<KeyPermSet>,
) -> anyhow::Result<()> {
+ // If an access vector was supplied, the key is either accessed by GRANT or by KEY_ID.
+ // In the former case, key.domain was set to GRANT and we check the failure cases
+ // further below. If the access is requested by KEY_ID, key.domain would have been
+ // resolved to APP or SELINUX depending on where the key actually resides.
+ // Either way we can return here immediately if the access vector covers the requested
+ // permission. If it does not, we can still check if the caller has access by means of
+ // ownership.
+ if let Some(access_vector) = access_vector {
+ if access_vector.includes(perm) {
+ return Ok(());
+ }
+ }
+
let target_context = match key.domain {
// apps get the default keystore context
- Domain::APP => getcon().context("check_key_permission: getcon failed.")?,
+ Domain::APP => {
+ if caller_uid as i64 != key.nspace {
+ return Err(selinux::Error::perm())
+ .context("Trying to access key without ownership.");
+ }
+ getcon().context("check_key_permission: getcon failed.")?
+ }
Domain::SELINUX => lookup_keystore2_key_context(key.nspace)
.context("check_key_permission: Domain::SELINUX: Failed to lookup namespace.")?,
Domain::GRANT => {
match access_vector {
- Some(pv) => {
- if pv.includes(perm) {
- return Ok(());
- } else {
- return Err(selinux::Error::perm())
- .context(format!("\"{}\" not granted", perm.to_selinux()));
- }
+ Some(_) => {
+ return Err(selinux::Error::perm())
+ .context(format!("\"{}\" not granted", perm.to_selinux()));
}
None => {
// If DOMAIN_GRANT was selected an access vector must be supplied.
@@ -685,6 +701,7 @@
let key = KeyDescriptor { domain: Domain::GRANT, nspace: 0, alias: None, blob: None };
assert_perm_failed!(check_key_permission(
+ 0,
&selinux::Context::new("ignored").unwrap(),
KeyPerm::grant(),
&key,
@@ -692,6 +709,7 @@
));
check_key_permission(
+ 0,
&selinux::Context::new("ignored").unwrap(),
KeyPerm::use_(),
&key,
@@ -707,38 +725,82 @@
let key = KeyDescriptor { domain: Domain::APP, nspace: 0, alias: None, blob: None };
- assert!(check_key_permission(&system_server_ctx, KeyPerm::use_(), &key, &None).is_ok());
- assert!(check_key_permission(&system_server_ctx, KeyPerm::delete(), &key, &None).is_ok());
- assert!(check_key_permission(&system_server_ctx, KeyPerm::get_info(), &key, &None).is_ok());
- assert!(check_key_permission(&system_server_ctx, KeyPerm::rebind(), &key, &None).is_ok());
- assert!(check_key_permission(&system_server_ctx, KeyPerm::update(), &key, &None).is_ok());
- assert!(check_key_permission(&system_server_ctx, KeyPerm::grant(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &system_server_ctx, KeyPerm::use_(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &system_server_ctx, KeyPerm::delete(), &key, &None).is_ok());
assert!(
- check_key_permission(&system_server_ctx, KeyPerm::use_dev_id(), &key, &None).is_ok()
+ check_key_permission(0, &system_server_ctx, KeyPerm::get_info(), &key, &None).is_ok()
);
- assert!(check_key_permission(&gmscore_app, KeyPerm::gen_unique_id(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &system_server_ctx, KeyPerm::rebind(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &system_server_ctx, KeyPerm::update(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &system_server_ctx, KeyPerm::grant(), &key, &None).is_ok());
+ assert!(
+ check_key_permission(0, &system_server_ctx, KeyPerm::use_dev_id(), &key, &None).is_ok()
+ );
+ assert!(
+ check_key_permission(0, &gmscore_app, KeyPerm::gen_unique_id(), &key, &None).is_ok()
+ );
- assert!(check_key_permission(&shell_ctx, KeyPerm::use_(), &key, &None).is_ok());
- assert!(check_key_permission(&shell_ctx, KeyPerm::delete(), &key, &None).is_ok());
- assert!(check_key_permission(&shell_ctx, KeyPerm::get_info(), &key, &None).is_ok());
- assert!(check_key_permission(&shell_ctx, KeyPerm::rebind(), &key, &None).is_ok());
- assert!(check_key_permission(&shell_ctx, KeyPerm::update(), &key, &None).is_ok());
- assert_perm_failed!(check_key_permission(&shell_ctx, KeyPerm::grant(), &key, &None));
+ assert!(check_key_permission(0, &shell_ctx, KeyPerm::use_(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &shell_ctx, KeyPerm::delete(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &shell_ctx, KeyPerm::get_info(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &shell_ctx, KeyPerm::rebind(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &shell_ctx, KeyPerm::update(), &key, &None).is_ok());
+ assert_perm_failed!(check_key_permission(0, &shell_ctx, KeyPerm::grant(), &key, &None));
assert_perm_failed!(check_key_permission(
+ 0,
&shell_ctx,
KeyPerm::req_forced_op(),
&key,
&None
));
- assert_perm_failed!(check_key_permission(&shell_ctx, KeyPerm::manage_blob(), &key, &None));
- assert_perm_failed!(check_key_permission(&shell_ctx, KeyPerm::use_dev_id(), &key, &None));
assert_perm_failed!(check_key_permission(
+ 0,
+ &shell_ctx,
+ KeyPerm::manage_blob(),
+ &key,
+ &None
+ ));
+ assert_perm_failed!(check_key_permission(
+ 0,
+ &shell_ctx,
+ KeyPerm::use_dev_id(),
+ &key,
+ &None
+ ));
+ assert_perm_failed!(check_key_permission(
+ 0,
&shell_ctx,
KeyPerm::gen_unique_id(),
&key,
&None
));
+ // Also make sure that the permission fails if the caller is not the owner.
+ assert_perm_failed!(check_key_permission(
+ 1, // the owner is 0
+ &system_server_ctx,
+ KeyPerm::use_(),
+ &key,
+ &None
+ ));
+ // Unless there was a grant.
+ assert!(check_key_permission(
+ 1,
+ &system_server_ctx,
+ KeyPerm::use_(),
+ &key,
+ &Some(key_perm_set![KeyPerm::use_()])
+ )
+ .is_ok());
+ // But fail if the grant did not cover the requested permission.
+ assert_perm_failed!(check_key_permission(
+ 1,
+ &system_server_ctx,
+ KeyPerm::use_(),
+ &key,
+ &Some(key_perm_set![KeyPerm::get_info()])
+ ));
+
Ok(())
}
@@ -753,27 +815,45 @@
};
if is_su {
- assert!(check_key_permission(&sctx, KeyPerm::use_(), &key, &None).is_ok());
- assert!(check_key_permission(&sctx, KeyPerm::delete(), &key, &None).is_ok());
- assert!(check_key_permission(&sctx, KeyPerm::get_info(), &key, &None).is_ok());
- assert!(check_key_permission(&sctx, KeyPerm::rebind(), &key, &None).is_ok());
- assert!(check_key_permission(&sctx, KeyPerm::update(), &key, &None).is_ok());
- assert!(check_key_permission(&sctx, KeyPerm::grant(), &key, &None).is_ok());
- assert!(check_key_permission(&sctx, KeyPerm::manage_blob(), &key, &None).is_ok());
- assert!(check_key_permission(&sctx, KeyPerm::use_dev_id(), &key, &None).is_ok());
- assert!(check_key_permission(&sctx, KeyPerm::gen_unique_id(), &key, &None).is_ok());
- assert!(check_key_permission(&sctx, KeyPerm::req_forced_op(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &sctx, KeyPerm::use_(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &sctx, KeyPerm::delete(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &sctx, KeyPerm::get_info(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &sctx, KeyPerm::rebind(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &sctx, KeyPerm::update(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &sctx, KeyPerm::grant(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &sctx, KeyPerm::manage_blob(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &sctx, KeyPerm::use_dev_id(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &sctx, KeyPerm::gen_unique_id(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &sctx, KeyPerm::req_forced_op(), &key, &None).is_ok());
} else {
- assert!(check_key_permission(&sctx, KeyPerm::use_(), &key, &None).is_ok());
- assert!(check_key_permission(&sctx, KeyPerm::delete(), &key, &None).is_ok());
- assert!(check_key_permission(&sctx, KeyPerm::get_info(), &key, &None).is_ok());
- assert!(check_key_permission(&sctx, KeyPerm::rebind(), &key, &None).is_ok());
- assert!(check_key_permission(&sctx, KeyPerm::update(), &key, &None).is_ok());
- assert_perm_failed!(check_key_permission(&sctx, KeyPerm::grant(), &key, &None));
- assert_perm_failed!(check_key_permission(&sctx, KeyPerm::req_forced_op(), &key, &None));
- assert_perm_failed!(check_key_permission(&sctx, KeyPerm::manage_blob(), &key, &None));
- assert_perm_failed!(check_key_permission(&sctx, KeyPerm::use_dev_id(), &key, &None));
- assert_perm_failed!(check_key_permission(&sctx, KeyPerm::gen_unique_id(), &key, &None));
+ assert!(check_key_permission(0, &sctx, KeyPerm::use_(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &sctx, KeyPerm::delete(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &sctx, KeyPerm::get_info(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &sctx, KeyPerm::rebind(), &key, &None).is_ok());
+ assert!(check_key_permission(0, &sctx, KeyPerm::update(), &key, &None).is_ok());
+ assert_perm_failed!(check_key_permission(0, &sctx, KeyPerm::grant(), &key, &None));
+ assert_perm_failed!(check_key_permission(
+ 0,
+ &sctx,
+ KeyPerm::req_forced_op(),
+ &key,
+ &None
+ ));
+ assert_perm_failed!(check_key_permission(
+ 0,
+ &sctx,
+ KeyPerm::manage_blob(),
+ &key,
+ &None
+ ));
+ assert_perm_failed!(check_key_permission(0, &sctx, KeyPerm::use_dev_id(), &key, &None));
+ assert_perm_failed!(check_key_permission(
+ 0,
+ &sctx,
+ KeyPerm::gen_unique_id(),
+ &key,
+ &None
+ ));
}
Ok(())
}
@@ -789,9 +869,9 @@
};
if is_su {
- check_key_permission(&sctx, KeyPerm::use_(), &key, &None)
+ check_key_permission(0, &sctx, KeyPerm::use_(), &key, &None)
} else {
- assert_perm_failed!(check_key_permission(&sctx, KeyPerm::use_(), &key, &None));
+ assert_perm_failed!(check_key_permission(0, &sctx, KeyPerm::use_(), &key, &None));
Ok(())
}
}
@@ -803,6 +883,7 @@
assert_eq!(
Some(&KsError::sys()),
check_key_permission(
+ 0,
&selinux::Context::new("ignored").unwrap(),
KeyPerm::use_(),
&key,
diff --git a/keystore2/src/utils.rs b/keystore2/src/utils.rs
index 080348c..870b7fc 100644
--- a/keystore2/src/utils.rs
+++ b/keystore2/src/utils.rs
@@ -77,6 +77,7 @@
) -> anyhow::Result<()> {
ThreadState::with_calling_sid(|calling_sid| {
permission::check_key_permission(
+ ThreadState::get_calling_uid(),
&calling_sid
.ok_or_else(Error::sys)
.context("In check_key_permission: Cannot check permission without calling_sid.")?,