Merge "Keystore 2.0: Allow by key id usage of granted keys."
diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs
index 225378f..785847d 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())),
}
@@ -2521,6 +2545,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.")?,