Merge "Added `not_multi_abi` configuration for keystore2_client_tests module." into main
diff --git a/keystore2/legacykeystore/lib.rs b/keystore2/legacykeystore/lib.rs
index f7a8198..db3eff6 100644
--- a/keystore2/legacykeystore/lib.rs
+++ b/keystore2/legacykeystore/lib.rs
@@ -55,7 +55,7 @@
F: Fn(&Transaction) -> Result<T>,
{
loop {
- match self
+ let result = self
.conn
.transaction_with_behavior(behavior)
.context("In with_transaction.")
@@ -63,7 +63,8 @@
.and_then(|(result, tx)| {
tx.commit().context("In with_transaction: Failed to commit transaction.")?;
Ok(result)
- }) {
+ });
+ match result {
Ok(result) => break Ok(result),
Err(e) => {
if Self::is_locked_error(&e) {
diff --git a/keystore2/src/authorization.rs b/keystore2/src/authorization.rs
index f956787..243abf1 100644
--- a/keystore2/src/authorization.rs
+++ b/keystore2/src/authorization.rs
@@ -128,7 +128,8 @@
fn add_auth_token(&self, auth_token: &HardwareAuthToken) -> Result<()> {
// Check keystore permission.
- check_keystore_permission(KeystorePerm::AddAuth).context(ks_err!())?;
+ check_keystore_permission(KeystorePerm::AddAuth)
+ .context(ks_err!("caller missing AddAuth permissions"))?;
log::info!(
"add_auth_token(challenge={}, userId={}, authId={}, authType={:#x}, timestamp={}ms)",
@@ -149,7 +150,8 @@
user_id,
password.is_some(),
);
- check_keystore_permission(KeystorePerm::Unlock).context(ks_err!("Unlock."))?;
+ check_keystore_permission(KeystorePerm::Unlock)
+ .context(ks_err!("caller missing Unlock permissions"))?;
ENFORCEMENTS.set_device_locked(user_id, false);
let mut skm = SUPER_KEY.write().unwrap();
@@ -160,7 +162,7 @@
.context(ks_err!("Unlock with password."))
} else {
DB.with(|db| skm.try_unlock_user_with_biometric(&mut db.borrow_mut(), user_id as u32))
- .context(ks_err!("try_unlock_user_with_biometric failed"))
+ .context(ks_err!("try_unlock_user_with_biometric failed user_id={user_id}"))
}
}
@@ -179,7 +181,8 @@
if !android_security_flags::fix_unlocked_device_required_keys_v2() {
weak_unlock_enabled = false;
}
- check_keystore_permission(KeystorePerm::Lock).context(ks_err!("Lock"))?;
+ check_keystore_permission(KeystorePerm::Lock)
+ .context(ks_err!("caller missing Lock permission"))?;
ENFORCEMENTS.set_device_locked(user_id, true);
let mut skm = SUPER_KEY.write().unwrap();
DB.with(|db| {
@@ -198,7 +201,8 @@
if !android_security_flags::fix_unlocked_device_required_keys_v2() {
return Ok(());
}
- check_keystore_permission(KeystorePerm::Lock).context(ks_err!("Lock"))?;
+ check_keystore_permission(KeystorePerm::Lock)
+ .context(ks_err!("caller missing Lock permission"))?;
SUPER_KEY.write().unwrap().wipe_plaintext_unlocked_device_required_keys(user_id as u32);
Ok(())
}
@@ -208,7 +212,8 @@
if !android_security_flags::fix_unlocked_device_required_keys_v2() {
return Ok(());
}
- check_keystore_permission(KeystorePerm::Lock).context(ks_err!("Lock"))?;
+ check_keystore_permission(KeystorePerm::Lock)
+ .context(ks_err!("caller missing Lock permission"))?;
SUPER_KEY.write().unwrap().wipe_all_unlocked_device_required_keys(user_id as u32);
Ok(())
}
@@ -221,7 +226,8 @@
) -> Result<AuthorizationTokens> {
// Check permission. Function should return if this failed. Therefore having '?' at the end
// is very important.
- check_keystore_permission(KeystorePerm::GetAuthToken).context(ks_err!("GetAuthToken"))?;
+ check_keystore_permission(KeystorePerm::GetAuthToken)
+ .context(ks_err!("caller missing GetAuthToken permission"))?;
// If the challenge is zero, return error
if challenge == 0 {
@@ -240,7 +246,8 @@
auth_types: &[HardwareAuthenticatorType],
) -> Result<i64> {
// Check keystore permission.
- check_keystore_permission(KeystorePerm::GetLastAuthTime).context(ks_err!())?;
+ check_keystore_permission(KeystorePerm::GetLastAuthTime)
+ .context(ks_err!("caller missing GetLastAuthTime permission"))?;
let mut max_time: i64 = -1;
for auth_type in auth_types.iter() {
diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs
index b526daa..2757313 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -1467,7 +1467,7 @@
F: Fn(&Transaction) -> Result<(bool, T)>,
{
loop {
- match self
+ let result = self
.conn
.transaction_with_behavior(behavior)
.context(ks_err!())
@@ -1475,7 +1475,8 @@
.and_then(|(result, tx)| {
tx.commit().context(ks_err!("Failed to commit transaction."))?;
Ok(result)
- }) {
+ });
+ match result {
Ok(result) => break Ok(result),
Err(e) => {
if Self::is_locked_error(&e) {
diff --git a/keystore2/src/globals.rs b/keystore2/src/globals.rs
index eb755a6..8535df1 100644
--- a/keystore2/src/globals.rs
+++ b/keystore2/src/globals.rs
@@ -247,7 +247,11 @@
}
e => e,
})
- .context(ks_err!("Trying to get Legacy wrapper."))?,
+ .context(ks_err!(
+ "Trying to get Legacy wrapper. Attempt to get keystore \
+ compat service for security level {:?}",
+ *security_level
+ ))?,
None,
)
};
@@ -394,7 +398,7 @@
}
e => e,
})
- .context(ks_err!("Trying to get Legacy wrapper."))
+ .context(ks_err!("Failed attempt to get legacy secure clock."))
}?;
Ok(secureclock)
@@ -437,5 +441,5 @@
_ => None,
}
.ok_or(Error::Km(ErrorCode::HARDWARE_TYPE_UNAVAILABLE))
- .context(ks_err!())
+ .context(ks_err!("Failed to get rpc for sec level {:?}", *security_level))
}
diff --git a/keystore2/src/operation.rs b/keystore2/src/operation.rs
index eabc1ab..11eaf17 100644
--- a/keystore2/src/operation.rs
+++ b/keystore2/src/operation.rs
@@ -290,7 +290,7 @@
// We abort the operation. If there was an error we log it but ignore it.
if let Err(e) = map_km_error(self.km_op.abort()) {
- log::error!("In prune: KeyMint::abort failed with {:?}.", e);
+ log::warn!("In prune: KeyMint::abort failed with {:?}.", e);
}
Ok(())
diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index 6fb0eb2..5f9745f 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -927,7 +927,7 @@
.context(ks_err!("Check permission"))?;
let km_dev = &self.keymint;
- match {
+ let res = {
let _wp = self.watch_millis(
concat!(
"In IKeystoreSecurityLevel::convert_storage_key_to_ephemeral: ",
@@ -936,7 +936,8 @@
500,
);
map_km_error(km_dev.convertStorageKeyToEphemeral(key_blob))
- } {
+ };
+ match res {
Ok(result) => {
Ok(EphemeralStorageKeyResponse { ephemeralKey: result, upgradedBlob: None })
}
diff --git a/keystore2/tests/keystore2_client_test_utils.rs b/keystore2/tests/keystore2_client_test_utils.rs
index f270297..7534da3 100644
--- a/keystore2/tests/keystore2_client_test_utils.rs
+++ b/keystore2/tests/keystore2_client_test_utils.rs
@@ -95,14 +95,11 @@
// only system update and not vendor update, newly added attestation properties
// (ro.product.*_for_attestation) reading logic would not be available for such devices
// hence skipping this test for such scenario.
- let api_level = std::str::from_utf8(&get_system_prop("ro.board.first_api_level"))
- .unwrap()
- .parse::<i32>()
- .unwrap();
- // This file is only present on GSI builds.
- let path_buf = PathBuf::from("/system/system_ext/etc/init/init.gsi.rc");
- api_level < 34 && path_buf.as_path().is_file()
+ // This file is only present on GSI builds.
+ let gsi_marker = PathBuf::from("/system/system_ext/etc/init/init.gsi.rc");
+
+ get_vsr_api_level() < 34 && gsi_marker.as_path().is_file()
}
#[macro_export]
@@ -514,15 +511,38 @@
}
}
+fn get_integer_system_prop(name: &str) -> Option<i32> {
+ let val = get_system_prop(name);
+ if val.is_empty() {
+ return None;
+ }
+ let val = std::str::from_utf8(&val).ok()?;
+ val.parse::<i32>().ok()
+}
+
+pub fn get_vsr_api_level() -> i32 {
+ if let Some(api_level) = get_integer_system_prop("ro.vendor.api_level") {
+ return api_level;
+ }
+
+ let vendor_api_level = get_integer_system_prop("ro.board.api_level")
+ .or_else(|| get_integer_system_prop("ro.board.first_api_level"));
+ let product_api_level = get_integer_system_prop("ro.product.first_api_level")
+ .or_else(|| get_integer_system_prop("ro.build.version.sdk"));
+
+ match (vendor_api_level, product_api_level) {
+ (Some(v), Some(p)) => std::cmp::min(v, p),
+ (Some(v), None) => v,
+ (None, Some(p)) => p,
+ _ => panic!("Could not determine VSR API level"),
+ }
+}
+
/// Determines whether the SECOND-IMEI can be used as device attest-id.
pub fn is_second_imei_id_attestation_required(
keystore2: &binder::Strong<dyn IKeystoreService>,
) -> bool {
- let api_level = std::str::from_utf8(&get_system_prop("ro.vendor.api_level"))
- .unwrap()
- .parse::<i32>()
- .unwrap();
- keystore2.getInterfaceVersion().unwrap() >= 3 && api_level > 33
+ keystore2.getInterfaceVersion().unwrap() >= 3 && get_vsr_api_level() > 33
}
/// Run a service command and collect the output.
diff --git a/ondevice-signing/odsign.rc b/ondevice-signing/odsign.rc
index b96c62f..b95cf9d 100644
--- a/ondevice-signing/odsign.rc
+++ b/ondevice-signing/odsign.rc
@@ -3,13 +3,10 @@
user root
group system
disabled # does not start with the core class
- # Explicitly specify empty capabilities, otherwise odsign will inherit all
- # the capabilities from init.
- # Note: whether a process can use capabilities is controlled by SELinux, so
- # inheriting all the capabilities from init is not a security issue.
- # However, for defense-in-depth and just for the sake of bookkeeping it's
- # better to explicitly state that odsign doesn't need any capabilities.
- capabilities
+ # We need SYS_NICE in order to allow the crosvm child process to use it.
+ # (b/322197421). odsign itself never uses it (and isn't allowed to by
+ # SELinux).
+ capabilities SYS_NICE
# Note that odsign is not oneshot, but stopped manually when it exits. This
# ensures that if odsign crashes during a module update, apexd will detect