Changing code to have better error logging
This changes the error logs to show the file and line number instead of
naming a specific a function where the error originated. In some cases
the function name is kept where it made sense for readibility of the
logs.
Test: Run and tested using `atest keystore2_test` for Rust test and CTS test with `atest CtsKeystoreTestCases`
Bug: 241924261
Change-Id: I2ea970dd83e18033506555f2726c716626697cdf
diff --git a/keystore2/src/maintenance.rs b/keystore2/src/maintenance.rs
index 1fca5d9..5efb798 100644
--- a/keystore2/src/maintenance.rs
+++ b/keystore2/src/maintenance.rs
@@ -20,6 +20,7 @@
use crate::error::Error;
use crate::globals::get_keymint_device;
use crate::globals::{DB, LEGACY_IMPORTER, SUPER_KEY};
+use crate::ks_err;
use crate::permission::{KeyPerm, KeystorePerm};
use crate::super_key::{SuperKeyManager, UserState};
use crate::utils::{
@@ -71,8 +72,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::ChangePassword)
- .context("In on_user_password_changed.")?;
+ check_keystore_permission(KeystorePerm::ChangePassword).context(ks_err!())?;
let mut skm = SUPER_KEY.write().unwrap();
@@ -80,7 +80,7 @@
DB.with(|db| {
skm.unlock_screen_lock_bound_key(&mut db.borrow_mut(), user_id as u32, pw)
})
- .context("In on_user_password_changed: unlock_screen_lock_bound_key failed")?;
+ .context(ks_err!("unlock_screen_lock_bound_key failed"))?;
}
match DB
@@ -92,12 +92,11 @@
password.as_ref(),
)
})
- .context("In on_user_password_changed.")?
+ .context(ks_err!())?
{
UserState::LskfLocked => {
// Error - password can not be changed when the device is locked
- Err(Error::Rc(ResponseCode::LOCKED))
- .context("In on_user_password_changed. Device is locked.")
+ Err(Error::Rc(ResponseCode::LOCKED)).context(ks_err!("Device is locked."))
}
_ => {
// LskfLocked is the only error case for password change
@@ -109,7 +108,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::ChangeUser).context("In add_or_remove_user.")?;
+ check_keystore_permission(KeystorePerm::ChangeUser).context(ks_err!())?;
DB.with(|db| {
SUPER_KEY.write().unwrap().reset_user(
@@ -119,10 +118,10 @@
false,
)
})
- .context("In add_or_remove_user: Trying to delete keys from db.")?;
+ .context(ks_err!("Trying to delete keys from db."))?;
self.delete_listener
.delete_user(user_id as u32)
- .context("In add_or_remove_user: While invoking the delete listener.")
+ .context(ks_err!("While invoking the delete listener."))
}
fn clear_namespace(&self, domain: Domain, nspace: i64) -> Result<()> {
@@ -131,12 +130,12 @@
LEGACY_IMPORTER
.bulk_delete_uid(domain, nspace)
- .context("In clear_namespace: Trying to delete legacy keys.")?;
+ .context(ks_err!("Trying to delete legacy keys."))?;
DB.with(|db| db.borrow_mut().unbind_keys_for_namespace(domain, nspace))
- .context("In clear_namespace: Trying to delete keys from db.")?;
+ .context(ks_err!("Trying to delete keys from db."))?;
self.delete_listener
.delete_namespace(domain, nspace)
- .context("In clear_namespace: While invoking the delete listener.")
+ .context(ks_err!("While invoking the delete listener."))
}
fn get_state(user_id: i32) -> Result<AidlUserState> {
@@ -151,7 +150,7 @@
user_id as u32,
)
})
- .context("In get_state. Trying to get UserState.")?;
+ .context(ks_err!("Trying to get UserState."))?;
match state {
UserState::Uninitialized => Ok(AidlUserState::UNINITIALIZED),
@@ -164,13 +163,13 @@
where
F: Fn(Strong<dyn IKeyMintDevice>) -> binder::Result<()>,
{
- let (km_dev, _, _) = get_keymint_device(&sec_level)
- .context("In call_with_watchdog: getting keymint device")?;
+ let (km_dev, _, _) =
+ get_keymint_device(&sec_level).context(ks_err!("getting keymint device"))?;
let _wp = wd::watch_millis_with("In call_with_watchdog", 500, move || {
format!("Seclevel: {:?} Op: {}", sec_level, name)
});
- map_km_error(op(km_dev)).with_context(|| format!("In keymint device: calling {}", name))?;
+ map_km_error(op(km_dev)).with_context(|| ks_err!("calling {}", name))?;
Ok(())
}
@@ -203,7 +202,7 @@
fn early_boot_ended() -> Result<()> {
check_keystore_permission(KeystorePerm::EarlyBootEnded)
- .context("In early_boot_ended. Checking permission")?;
+ .context(ks_err!("Checking permission"))?;
log::info!("In early_boot_ended.");
if let Err(e) =
@@ -216,7 +215,7 @@
fn on_device_off_body() -> Result<()> {
// Security critical permission check. This statement must return on fail.
- check_keystore_permission(KeystorePerm::ReportOffBody).context("In on_device_off_body.")?;
+ check_keystore_permission(KeystorePerm::ReportOffBody).context(ks_err!())?;
DB.with(|db| db.borrow_mut().update_last_off_body(MonotonicRawTime::now()));
Ok(())
@@ -228,20 +227,16 @@
match source.domain {
Domain::SELINUX | Domain::KEY_ID | Domain::APP => (),
_ => {
- return Err(Error::Rc(ResponseCode::INVALID_ARGUMENT)).context(
- "In migrate_key_namespace: \
- Source domain must be one of APP, SELINUX, or KEY_ID.",
- )
+ return Err(Error::Rc(ResponseCode::INVALID_ARGUMENT))
+ .context(ks_err!("Source domain must be one of APP, SELINUX, or KEY_ID."));
}
};
match destination.domain {
Domain::SELINUX | Domain::APP => (),
_ => {
- return Err(Error::Rc(ResponseCode::INVALID_ARGUMENT)).context(
- "In migrate_key_namespace: \
- Destination domain must be one of APP or SELINUX.",
- )
+ return Err(Error::Rc(ResponseCode::INVALID_ARGUMENT))
+ .context(ks_err!("Destination domain must be one of APP or SELINUX."));
}
};
@@ -264,7 +259,7 @@
},
)
})
- .context("In migrate_key_namespace: Failed to load key blob.")?;
+ .context(ks_err!("Failed to load key blob."))?;
{
db.borrow_mut().migrate_key_namespace(key_id_guard, destination, calling_uid, |k| {
check_key_permission(KeyPerm::Rebind, k, &None)
@@ -276,7 +271,7 @@
fn delete_all_keys() -> Result<()> {
// Security critical permission check. This statement must return on fail.
check_keystore_permission(KeystorePerm::DeleteAllKeys)
- .context("In delete_all_keys. Checking permission")?;
+ .context(ks_err!("Checking permission"))?;
log::info!("In delete_all_keys.");
Maintenance::call_on_all_security_levels("deleteAllKeys", |dev| dev.deleteAllKeys())