diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs index 225378f4..785847db 100644 --- a/keystore2/src/database.rs +++ b/keystore2/src/database.rs @@ -1229,18 +1229,18 @@ impl KeystoreDB { // 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 @@ impl KeystoreDB { 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 = + if domain != Domain::APP || namespace != caller_uid as i64 { + let access_vector: Option = 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 @@ mod tests { 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::() + ); + + 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 09172567..a81954fe 100644 --- a/keystore2/src/permission.rs +++ b/keystore2/src/permission.rs @@ -482,25 +482,41 @@ pub fn check_grant_permission( /// 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, ) -> 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 @@ mod tests { 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 @@ mod tests { )); check_key_permission( + 0, &selinux::Context::new("ignored").unwrap(), KeyPerm::use_(), &key, @@ -707,38 +725,82 @@ mod tests { 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(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(&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 @@ mod tests { }; 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 @@ mod tests { }; 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 @@ mod tests { 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 080348ca..870b7fc5 100644 --- a/keystore2/src/utils.rs +++ b/keystore2/src/utils.rs @@ -77,6 +77,7 @@ pub fn check_key_permission( ) -> 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.")?,