Merge "Revert "Deprecating the aidl for Android Protected Confirmation"" into main
diff --git a/fsverity/fsverity_manifest_generator.py b/fsverity/fsverity_manifest_generator.py
index 181758a..ca7ac5c 100644
--- a/fsverity/fsverity_manifest_generator.py
+++ b/fsverity/fsverity_manifest_generator.py
@@ -35,7 +35,7 @@
return bytes(bytearray.fromhex(out))
if __name__ == '__main__':
- p = argparse.ArgumentParser()
+ p = argparse.ArgumentParser(fromfile_prefix_chars='@')
p.add_argument(
'--output',
help='Path to the output manifest',
@@ -52,7 +52,7 @@
'inputs',
nargs='*',
help='input file for the build manifest')
- args = p.parse_args(sys.argv[1:])
+ args = p.parse_args()
digests = FSVerityDigests()
for f in sorted(args.inputs):
diff --git a/fsverity_init/Android.bp b/fsverity_init/Android.bp
index d9bff3b..5588493 100644
--- a/fsverity_init/Android.bp
+++ b/fsverity_init/Android.bp
@@ -22,12 +22,17 @@
"libkeyutils",
"liblog",
],
- cflags: ["-Werror", "-Wall", "-Wextra"],
+ cflags: [
+ "-Werror",
+ "-Wall",
+ "-Wextra",
+ ],
}
aconfig_declarations {
name: "aconfig_fsverity_init",
package: "android.security.flag",
+ container: "system",
srcs: ["flags.aconfig"],
}
diff --git a/fsverity_init/flags.aconfig b/fsverity_init/flags.aconfig
index 20640d7..495c71c 100644
--- a/fsverity_init/flags.aconfig
+++ b/fsverity_init/flags.aconfig
@@ -1,4 +1,5 @@
package: "android.security.flag"
+container: "system"
flag {
name: "deprecate_fsverity_init"
diff --git a/identity/Android.bp b/identity/Android.bp
index f7a540a..a563532 100644
--- a/identity/Android.bp
+++ b/identity/Android.bp
@@ -1,4 +1,5 @@
package {
+ default_team: "trendy_team_android_hardware_backed_security",
// See: http://go/android-license-faq
// A large-scale-change added 'default_applicable_licenses' to import
// all of the 'license_kinds' from "system_security_license"
@@ -17,7 +18,7 @@
"-Wno-deprecated-declarations",
],
sanitize: {
- misc_undefined : ["integer"],
+ misc_undefined: ["integer"],
},
}
@@ -58,7 +59,7 @@
"android.hardware.identity-support-lib",
"android.hardware.security.rkp-V3-cpp",
"android.security.rkp_aidl-cpp",
- "libcppbor_external",
+ "libcppbor",
"libcredstore_aidl",
"libkeymaster4support",
"librkp_support",
@@ -102,7 +103,7 @@
name: "libcredstore_aidl",
srcs: [
":credstore_aidl",
- ],
+ ],
aidl: {
export_aidl_headers: true,
include_dirs: [
@@ -136,6 +137,6 @@
cc: [
"trong@google.com",
"zeuthen@google.com",
- ]
+ ],
},
}
diff --git a/identity/util/Android.bp b/identity/util/Android.bp
index 71d7718..771fe79 100644
--- a/identity/util/Android.bp
+++ b/identity/util/Android.bp
@@ -13,6 +13,7 @@
// limitations under the License.
package {
+ default_team: "trendy_team_android_hardware_backed_security",
default_applicable_licenses: ["Android-Apache-2.0"],
}
diff --git a/keystore/tests/Android.bp b/keystore/tests/Android.bp
index e641f44..c3a9e66 100644
--- a/keystore/tests/Android.bp
+++ b/keystore/tests/Android.bp
@@ -1,6 +1,7 @@
// Unit test for AuthTokenTable
package {
+ default_team: "trendy_team_android_hardware_backed_security",
// See: http://go/android-license-faq
// A large-scale-change added 'default_applicable_licenses' to import
// all of the 'license_kinds' from "system_security_license"
@@ -41,9 +42,9 @@
"libkeystore-attestation-application-id",
"libvndksupport",
],
- sanitize: {
- cfi: false,
- }
+ sanitize: {
+ cfi: false,
+ },
}
cc_test {
@@ -68,7 +69,7 @@
shared_libs: [
"libbinder_ndk",
],
- sanitize: {
- cfi: false,
- }
+ sanitize: {
+ cfi: false,
+ },
}
diff --git a/keystore/tests/fuzzer/Android.bp b/keystore/tests/fuzzer/Android.bp
index d459f75..55d8f83 100644
--- a/keystore/tests/fuzzer/Android.bp
+++ b/keystore/tests/fuzzer/Android.bp
@@ -15,6 +15,7 @@
*/
package {
+ default_team: "trendy_team_android_hardware_backed_security",
// See: http://go/android-license-faq
// A large-scale-change added 'default_applicable_licenses' to import
// all of the 'license_kinds' from "system_security_license"
diff --git a/keystore2/Android.bp b/keystore2/Android.bp
index ad151ad..ed9cd88 100644
--- a/keystore2/Android.bp
+++ b/keystore2/Android.bp
@@ -28,6 +28,7 @@
defaults: [
"keymint_use_latest_hal_aidl_rust",
"keystore2_use_latest_aidl_rust",
+ "structured_log_rust_defaults",
],
rustlibs: [
@@ -54,7 +55,6 @@
"libkeystore2_selinux",
"liblazy_static",
"liblibc",
- "liblog_event_list",
"liblog_rust",
"libmessage_macro",
"librand",
@@ -158,6 +158,7 @@
aconfig_declarations {
name: "keystore2_flags",
package: "android.security.keystore2",
+ container: "system",
srcs: ["aconfig/flags.aconfig"],
}
diff --git a/keystore2/OWNERS b/keystore2/OWNERS
index 6b1a95b..bf9d61b 100644
--- a/keystore2/OWNERS
+++ b/keystore2/OWNERS
@@ -5,5 +5,4 @@
hasinitg@google.com
jbires@google.com
sethmo@google.com
-trong@google.com
swillden@google.com
diff --git a/keystore2/aconfig/flags.aconfig b/keystore2/aconfig/flags.aconfig
index 133c4ab..856b42e 100644
--- a/keystore2/aconfig/flags.aconfig
+++ b/keystore2/aconfig/flags.aconfig
@@ -1,4 +1,5 @@
package: "android.security.keystore2"
+container: "system"
flag {
name: "wal_db_journalmode_v3"
@@ -22,4 +23,12 @@
description: "Include support for importing keys that were previously software-emulated into KeyMint"
bug: "283077822"
is_fixed_read_only: true
+}
+
+flag {
+ name: "database_loop_timeout"
+ namespace: "hardware_backed_security"
+ description: "Abandon Keystore database retry loop after an interval"
+ bug: "319563050"
+ is_fixed_read_only: true
}
\ No newline at end of file
diff --git a/keystore2/aidl/android/security/authorization/IKeystoreAuthorization.aidl b/keystore2/aidl/android/security/authorization/IKeystoreAuthorization.aidl
index a9de026..fd532f6 100644
--- a/keystore2/aidl/android/security/authorization/IKeystoreAuthorization.aidl
+++ b/keystore2/aidl/android/security/authorization/IKeystoreAuthorization.aidl
@@ -18,8 +18,6 @@
import android.hardware.security.keymint.HardwareAuthenticatorType;
import android.security.authorization.AuthorizationTokens;
-// TODO: mark the interface with @SensitiveData when the annotation is ready (b/176110256).
-
/**
* IKeystoreAuthorization interface exposes the methods for other system components to
* provide keystore with the information required to enforce authorizations on key usage.
diff --git a/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl b/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
index e612db9..50e9828 100644
--- a/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
+++ b/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
@@ -112,16 +112,6 @@
void earlyBootEnded();
/**
- * Informs Keystore 2.0 that the an off body event was detected.
- *
- * ## Error conditions:
- * `ResponseCode::PERMISSION_DENIED` - if the caller does not have the `ReportOffBody`
- * permission.
- * `ResponseCode::SYSTEM_ERROR` - if an unexpected error occurred.
- */
- void onDeviceOffBody();
-
- /**
* Migrate a key from one namespace to another. The caller must have use, grant, and delete
* permissions on the source namespace and rebind permissions on the destination namespace.
* The source may be specified by Domain::APP, Domain::SELINUX, or Domain::KEY_ID. The target
@@ -151,7 +141,8 @@
* (addition of a fingerprint, for example), authentication-bound keys may be invalidated.
* This method allows the platform to find out which apps would be affected (for a given user)
* when a given user secure ID is removed.
- * Callers require 'ChangeUser' permission.
+ * Callers require the `android.permission.MANAGE_USERS` Android permission
+ * (not SELinux policy).
*
* @param userId The affected user.
* @param sid The user secure ID - identifier of the authentication method.
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/selinux/src/concurrency_test.rs b/keystore2/selinux/src/concurrency_test.rs
index a5d2df2..fa97f3a 100644
--- a/keystore2/selinux/src/concurrency_test.rs
+++ b/keystore2/selinux/src/concurrency_test.rs
@@ -69,7 +69,7 @@
android_logger::init_once(
android_logger::Config::default()
.with_tag("keystore2_selinux_concurrency_test")
- .with_min_level(log::Level::Debug),
+ .with_max_level(log::LevelFilter::Debug),
);
let cpus = num_cpus::get();
diff --git a/keystore2/selinux/src/lib.rs b/keystore2/selinux/src/lib.rs
index 32fdb59..695e029 100644
--- a/keystore2/selinux/src/lib.rs
+++ b/keystore2/selinux/src/lib.rs
@@ -720,7 +720,7 @@
android_logger::init_once(
android_logger::Config::default()
.with_tag("keystore_selinux_tests")
- .with_min_level(log::Level::Debug),
+ .with_max_level(log::LevelFilter::Debug),
);
let scontext = Context::new("u:r:shell:s0")?;
let backend = KeystoreKeyBackend::new()?;
diff --git a/keystore2/src/audit_log.rs b/keystore2/src/audit_log.rs
index 0e5dfeb..8d9735e 100644
--- a/keystore2/src/audit_log.rs
+++ b/keystore2/src/audit_log.rs
@@ -20,7 +20,7 @@
Domain::Domain, KeyDescriptor::KeyDescriptor,
};
use libc::uid_t;
-use log_event_list::{LogContext, LogContextError, LogIdSecurity};
+use structured_log::{structured_log, LOG_ID_SECURITY};
const TAG_KEY_GENERATED: u32 = 210024;
const TAG_KEY_IMPORTED: u32 = 210025;
@@ -58,30 +58,19 @@
/// Logs key integrity violation to NIAP audit log.
pub fn log_key_integrity_violation(key: &KeyDescriptor) {
- with_log_context(TAG_KEY_INTEGRITY_VIOLATION, |ctx| {
- let owner = key_owner(key.domain, key.nspace, key.nspace as i32);
- ctx.append_str(key.alias.as_ref().map_or("none", String::as_str))?.append_i32(owner)
- })
+ let owner = key_owner(key.domain, key.nspace, key.nspace as i32);
+ let alias = String::from(key.alias.as_ref().map_or("none", String::as_str));
+ LOGS_HANDLER.queue_lo(move |_| {
+ let _result =
+ structured_log!(log_id: LOG_ID_SECURITY, TAG_KEY_INTEGRITY_VIOLATION, alias, owner);
+ });
}
fn log_key_event(tag: u32, key: &KeyDescriptor, calling_app: uid_t, success: bool) {
- with_log_context(tag, |ctx| {
- let owner = key_owner(key.domain, key.nspace, calling_app as i32);
- ctx.append_i32(i32::from(success))?
- .append_str(key.alias.as_ref().map_or("none", String::as_str))?
- .append_i32(owner)
- })
-}
-
-fn with_log_context<F>(tag: u32, f: F)
-where
- F: Fn(LogContext) -> Result<LogContext, LogContextError>,
-{
- if let Some(ctx) = LogContext::new(LogIdSecurity, tag) {
- if let Ok(event) = f(ctx) {
- LOGS_HANDLER.queue_lo(move |_| {
- let _result = event.write();
- });
- }
- }
+ let owner = key_owner(key.domain, key.nspace, calling_app as i32);
+ let alias = String::from(key.alias.as_ref().map_or("none", String::as_str));
+ LOGS_HANDLER.queue_lo(move |_| {
+ let _result =
+ structured_log!(log_id: LOG_ID_SECURITY, tag, i32::from(success), alias, owner);
+ });
}
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/crypto/zvec.rs b/keystore2/src/crypto/zvec.rs
index c917a89..00cbb1c 100644
--- a/keystore2/src/crypto/zvec.rs
+++ b/keystore2/src/crypto/zvec.rs
@@ -20,6 +20,7 @@
use std::fmt;
use std::ops::{Deref, DerefMut};
use std::ptr::write_volatile;
+use std::ptr::NonNull;
/// A semi fixed size u8 vector that is zeroed when dropped. It can shrink in
/// size but cannot grow larger than the original size (and if it shrinks it
@@ -46,7 +47,7 @@
let b = v.into_boxed_slice();
if size > 0 {
// SAFETY: The address range is part of our address space.
- unsafe { mlock(b.as_ptr() as *const std::ffi::c_void, b.len()) }?;
+ unsafe { mlock(NonNull::from(&b).cast(), b.len()) }?;
}
Ok(Self { elems: b, len: size })
}
@@ -79,9 +80,7 @@
if let Err(e) =
// SAFETY: The address range is part of our address space, and was previously locked
// by `mlock` in `ZVec::new` or the `TryFrom<Vec<u8>>` implementation.
- unsafe {
- munlock(self.elems.as_ptr() as *const std::ffi::c_void, self.elems.len())
- }
+ unsafe { munlock(NonNull::from(&self.elems).cast(), self.elems.len()) }
{
log::error!("In ZVec::drop: `munlock` failed: {:?}.", e);
}
@@ -137,7 +136,7 @@
let b = v.into_boxed_slice();
if !b.is_empty() {
// SAFETY: The address range is part of our address space.
- unsafe { mlock(b.as_ptr() as *const std::ffi::c_void, b.len()) }?;
+ unsafe { mlock(NonNull::from(&b).cast(), b.len()) }?;
}
Ok(Self { elems: b, len })
}
diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs
index 0a1c547..43eaa55 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -95,6 +95,24 @@
#[cfg(test)]
use tests::random;
+/// If the database returns a busy error code, retry after this interval.
+const DB_BUSY_RETRY_INTERVAL: Duration = Duration::from_micros(500);
+/// If the database returns a busy error code, keep retrying for this long.
+const MAX_DB_BUSY_RETRY_PERIOD: Duration = Duration::from_secs(15);
+
+/// Check whether a database lock has timed out.
+fn check_lock_timeout(start: &std::time::Instant, timeout: Duration) -> Result<()> {
+ if keystore2_flags::database_loop_timeout() {
+ let elapsed = start.elapsed();
+ if elapsed >= timeout {
+ error!("Abandon locked DB after {elapsed:?}");
+ return Err(&KsError::Rc(ResponseCode::BACKEND_BUSY))
+ .context(ks_err!("Abandon locked DB after {elapsed:?}",));
+ }
+ }
+ Ok(())
+}
+
impl_metadata!(
/// A set of metadata for key entries.
#[derive(Debug, Default, Eq, PartialEq)]
@@ -761,22 +779,22 @@
}
/// Database representation of the monotonic time retrieved from the system call clock_gettime with
-/// CLOCK_MONOTONIC_RAW. Stores monotonic time as i64 in milliseconds.
+/// CLOCK_BOOTTIME. Stores monotonic time as i64 in milliseconds.
#[derive(Debug, Copy, Clone, Default, Eq, PartialEq, Ord, PartialOrd)]
-pub struct MonotonicRawTime(i64);
+pub struct BootTime(i64);
-impl MonotonicRawTime {
- /// Constructs a new MonotonicRawTime
+impl BootTime {
+ /// Constructs a new BootTime
pub fn now() -> Self {
Self(get_current_time_in_milliseconds())
}
- /// Returns the value of MonotonicRawTime in milliseconds as i64
+ /// Returns the value of BootTime in milliseconds as i64
pub fn milliseconds(&self) -> i64 {
self.0
}
- /// Returns the integer value of MonotonicRawTime as i64
+ /// Returns the integer value of BootTime as i64
pub fn seconds(&self) -> i64 {
self.0 / 1000
}
@@ -787,13 +805,13 @@
}
}
-impl ToSql for MonotonicRawTime {
+impl ToSql for BootTime {
fn to_sql(&self) -> rusqlite::Result<ToSqlOutput> {
Ok(ToSqlOutput::Owned(Value::Integer(self.0)))
}
}
-impl FromSql for MonotonicRawTime {
+impl FromSql for BootTime {
fn column_result(value: ValueRef) -> FromSqlResult<Self> {
Ok(Self(i64::column_result(value)?))
}
@@ -805,11 +823,11 @@
pub struct AuthTokenEntry {
auth_token: HardwareAuthToken,
// Time received in milliseconds
- time_received: MonotonicRawTime,
+ time_received: BootTime,
}
impl AuthTokenEntry {
- fn new(auth_token: HardwareAuthToken, time_received: MonotonicRawTime) -> Self {
+ fn new(auth_token: HardwareAuthToken, time_received: BootTime) -> Self {
AuthTokenEntry { auth_token, time_received }
}
@@ -832,7 +850,7 @@
}
/// Returns the time that this auth token was received.
- pub fn time_received(&self) -> MonotonicRawTime {
+ pub fn time_received(&self) -> BootTime {
self.time_received
}
@@ -842,11 +860,6 @@
}
}
-/// Shared in-memory databases get destroyed as soon as the last connection to them gets closed.
-/// This object does not allow access to the database connection. But it keeps a database
-/// connection alive in order to keep the in memory per boot database alive.
-pub struct PerBootDbKeepAlive(Connection);
-
impl KeystoreDB {
const UNASSIGNED_KEY_ID: i64 = -1i64;
const CURRENT_DB_VERSION: u32 = 1;
@@ -1035,7 +1048,7 @@
.context("Failed to attach database persistent.")
{
if Self::is_locked_error(&e) {
- std::thread::sleep(std::time::Duration::from_micros(500));
+ std::thread::sleep(DB_BUSY_RETRY_INTERVAL);
continue;
} else {
return Err(e);
@@ -1167,9 +1180,9 @@
"DELETE FROM persistent.blobmetadata WHERE blobentryid = ?;",
params![blob_id],
)
- .context("Trying to delete blob metadata.")?;
+ .context(ks_err!("Trying to delete blob metadata: {:?}", blob_id))?;
tx.execute("DELETE FROM persistent.blobentry WHERE id = ?;", params![blob_id])
- .context("Trying to blob.")?;
+ .context(ks_err!("Trying to delete blob: {:?}", blob_id))?;
}
Self::cleanup_unreferenced(tx).context("Trying to cleanup unreferenced.")?;
@@ -1466,8 +1479,20 @@
where
F: Fn(&Transaction) -> Result<(bool, T)>,
{
+ self.with_transaction_timeout(behavior, MAX_DB_BUSY_RETRY_PERIOD, f)
+ }
+ fn with_transaction_timeout<T, F>(
+ &mut self,
+ behavior: TransactionBehavior,
+ timeout: Duration,
+ f: F,
+ ) -> Result<T>
+ where
+ F: Fn(&Transaction) -> Result<(bool, T)>,
+ {
+ let start = std::time::Instant::now();
loop {
- match self
+ let result = self
.conn
.transaction_with_behavior(behavior)
.context(ks_err!())
@@ -1475,11 +1500,13 @@
.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) {
- std::thread::sleep(std::time::Duration::from_micros(500));
+ check_lock_timeout(&start, timeout)?;
+ std::thread::sleep(DB_BUSY_RETRY_INTERVAL);
continue;
} else {
return Err(e).context(ks_err!());
@@ -2228,6 +2255,7 @@
check_permission: impl Fn(&KeyDescriptor, Option<KeyPermSet>) -> Result<()>,
) -> Result<(KeyIdGuard, KeyEntry)> {
let _wp = wd::watch_millis("KeystoreDB::load_key_entry", 500);
+ let start = std::time::Instant::now();
loop {
match self.load_key_entry_internal(
@@ -2240,7 +2268,8 @@
Ok(result) => break Ok(result),
Err(e) => {
if Self::is_locked_error(&e) {
- std::thread::sleep(std::time::Duration::from_micros(500));
+ check_lock_timeout(&start, MAX_DB_BUSY_RETRY_PERIOD)?;
+ std::thread::sleep(DB_BUSY_RETRY_INTERVAL);
continue;
} else {
return Err(e).context(ks_err!());
@@ -2858,33 +2887,16 @@
/// Insert or replace the auth token based on (user_id, auth_id, auth_type)
pub fn insert_auth_token(&mut self, auth_token: &HardwareAuthToken) {
- self.perboot.insert_auth_token_entry(AuthTokenEntry::new(
- auth_token.clone(),
- MonotonicRawTime::now(),
- ))
+ self.perboot
+ .insert_auth_token_entry(AuthTokenEntry::new(auth_token.clone(), BootTime::now()))
}
/// Find the newest auth token matching the given predicate.
- pub fn find_auth_token_entry<F>(&self, p: F) -> Option<(AuthTokenEntry, MonotonicRawTime)>
+ pub fn find_auth_token_entry<F>(&self, p: F) -> Option<AuthTokenEntry>
where
F: Fn(&AuthTokenEntry) -> bool,
{
- self.perboot.find_auth_token_entry(p).map(|entry| (entry, self.get_last_off_body()))
- }
-
- /// Insert last_off_body into the metadata table at the initialization of auth token table
- pub fn insert_last_off_body(&self, last_off_body: MonotonicRawTime) {
- self.perboot.set_last_off_body(last_off_body)
- }
-
- /// Update last_off_body when on_device_off_body is called
- pub fn update_last_off_body(&self, last_off_body: MonotonicRawTime) {
- self.perboot.set_last_off_body(last_off_body)
- }
-
- /// Get last_off_body time when finding auth tokens
- fn get_last_off_body(&self) -> MonotonicRawTime {
- self.perboot.get_last_off_body()
+ self.perboot.find_auth_token_entry(p)
}
/// Load descriptor of a key by key id
@@ -5034,7 +5046,7 @@
// This allows us to test repeated elements.
thread_local! {
- static RANDOM_COUNTER: RefCell<i64> = RefCell::new(0);
+ static RANDOM_COUNTER: RefCell<i64> = const { RefCell::new(0) };
}
fn reset_random() {
@@ -5052,23 +5064,6 @@
}
#[test]
- fn test_last_off_body() -> Result<()> {
- let mut db = new_test_db()?;
- db.insert_last_off_body(MonotonicRawTime::now());
- let tx = db.conn.transaction_with_behavior(TransactionBehavior::Immediate)?;
- tx.commit()?;
- let last_off_body_1 = db.get_last_off_body();
- let one_second = Duration::from_secs(1);
- thread::sleep(one_second);
- db.update_last_off_body(MonotonicRawTime::now());
- let tx2 = db.conn.transaction_with_behavior(TransactionBehavior::Immediate)?;
- tx2.commit()?;
- let last_off_body_2 = db.get_last_off_body();
- assert!(last_off_body_1 < last_off_body_2);
- Ok(())
- }
-
- #[test]
fn test_unbind_keys_for_user() -> Result<()> {
let mut db = new_test_db()?;
db.unbind_keys_for_user(1, false)?;
@@ -5492,7 +5487,7 @@
// All three entries are in the database
assert_eq!(db.perboot.auth_tokens_len(), 3);
// It selected the most recent timestamp
- assert_eq!(db.find_auth_token_entry(|_| true).unwrap().0.auth_token.mac, b"mac2".to_vec());
+ assert_eq!(db.find_auth_token_entry(|_| true).unwrap().auth_token.mac, b"mac2".to_vec());
Ok(())
}
@@ -5618,4 +5613,85 @@
assert_eq!(third_sid_apps, vec![second_app_id]);
Ok(())
}
+
+ #[test]
+ fn test_key_id_guard_immediate() -> Result<()> {
+ if !keystore2_flags::database_loop_timeout() {
+ eprintln!("Skipping test as loop timeout flag disabled");
+ return Ok(());
+ }
+ // Emit logging from test.
+ android_logger::init_once(
+ android_logger::Config::default()
+ .with_tag("keystore_database_tests")
+ .with_max_level(log::LevelFilter::Debug),
+ );
+
+ // Preparation: put a single entry into a test DB.
+ let temp_dir = Arc::new(TempDir::new("key_id_guard_immediate")?);
+ let temp_dir_clone_a = temp_dir.clone();
+ let temp_dir_clone_b = temp_dir.clone();
+ let mut db = KeystoreDB::new(temp_dir.path(), None)?;
+ let key_id = make_test_key_entry(&mut db, Domain::APP, 1, TEST_ALIAS, None)?.0;
+
+ let (a_sender, b_receiver) = std::sync::mpsc::channel();
+ let (b_sender, a_receiver) = std::sync::mpsc::channel();
+
+ // First thread starts an immediate transaction, then waits on a synchronization channel
+ // before trying to get the `KeyIdGuard`.
+ let handle_a = thread::spawn(move || {
+ let temp_dir = temp_dir_clone_a;
+ let mut db = KeystoreDB::new(temp_dir.path(), None).unwrap();
+
+ // Make sure the other thread has initialized its database access before we lock it out.
+ a_receiver.recv().unwrap();
+
+ let _result = db.with_transaction_timeout(
+ TransactionBehavior::Immediate,
+ Duration::from_secs(3),
+ |_tx| {
+ // Notify the other thread that we're inside the immediate transaction...
+ a_sender.send(()).unwrap();
+ // ...then wait to be sure that the other thread has the `KeyIdGuard` before
+ // this thread also tries to get it.
+ a_receiver.recv().unwrap();
+
+ let _guard = KEY_ID_LOCK.get(key_id);
+ Ok(()).no_gc()
+ },
+ );
+ });
+
+ // Second thread gets the `KeyIdGuard`, then waits before trying to perform an immediate
+ // transaction.
+ let handle_b = thread::spawn(move || {
+ let temp_dir = temp_dir_clone_b;
+ let mut db = KeystoreDB::new(temp_dir.path(), None).unwrap();
+ // Notify the other thread that we are initialized (so it can lock the immediate
+ // transaction).
+ b_sender.send(()).unwrap();
+
+ let _guard = KEY_ID_LOCK.get(key_id);
+ // Notify the other thread that we have the `KeyIdGuard`...
+ b_sender.send(()).unwrap();
+ // ...then wait to be sure that the other thread is in the immediate transaction before
+ // this thread also tries to do one.
+ b_receiver.recv().unwrap();
+
+ let result = db.with_transaction_timeout(
+ TransactionBehavior::Immediate,
+ Duration::from_secs(3),
+ |_tx| Ok(()).no_gc(),
+ );
+ // Expect the attempt to get an immediate transaction to fail, and then this thread will
+ // exit and release the `KeyIdGuard`, allowing the other thread to complete.
+ assert!(result.is_err());
+ check_result_is_error_containing_string(result, "BACKEND_BUSY");
+ });
+
+ let _ = handle_a.join();
+ let _ = handle_b.join();
+
+ Ok(())
+ }
}
diff --git a/keystore2/src/database/perboot.rs b/keystore2/src/database/perboot.rs
index 7ff35fa..4727015 100644
--- a/keystore2/src/database/perboot.rs
+++ b/keystore2/src/database/perboot.rs
@@ -13,15 +13,14 @@
// limitations under the License.
//! This module implements a per-boot, shared, in-memory storage of auth tokens
-//! and last-time-on-body for the main Keystore 2.0 database module.
+//! for the main Keystore 2.0 database module.
-use super::{AuthTokenEntry, MonotonicRawTime};
+use super::AuthTokenEntry;
use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{
HardwareAuthToken::HardwareAuthToken, HardwareAuthenticatorType::HardwareAuthenticatorType,
};
use lazy_static::lazy_static;
use std::collections::HashSet;
-use std::sync::atomic::{AtomicI64, Ordering};
use std::sync::Arc;
use std::sync::RwLock;
@@ -62,17 +61,13 @@
impl Eq for AuthTokenEntryWrap {}
-/// Per-boot state structure. Currently only used to track auth tokens and
-/// last-off-body.
+/// Per-boot state structure. Currently only used to track auth tokens.
#[derive(Default)]
pub struct PerbootDB {
// We can use a .unwrap() discipline on this lock, because only panicking
// while holding a .write() lock will poison it. The only write usage is
// an insert call which inserts a pre-constructed pair.
auth_tokens: RwLock<HashSet<AuthTokenEntryWrap>>,
- // Ordering::Relaxed is appropriate for accessing this atomic, since it
- // does not currently need to be synchronized with anything else.
- last_off_body: AtomicI64,
}
lazy_static! {
@@ -102,14 +97,6 @@
matches.sort_by_key(|x| x.0.time_received);
matches.last().map(|x| x.0.clone())
}
- /// Get the last time the device was off the user's body
- pub fn get_last_off_body(&self) -> MonotonicRawTime {
- MonotonicRawTime(self.last_off_body.load(Ordering::Relaxed))
- }
- /// Set the last time the device was off the user's body
- pub fn set_last_off_body(&self, last_off_body: MonotonicRawTime) {
- self.last_off_body.store(last_off_body.0, Ordering::Relaxed)
- }
/// Return how many auth tokens are currently tracked.
pub fn auth_tokens_len(&self) -> usize {
self.auth_tokens.read().unwrap().len()
diff --git a/keystore2/src/enforcements.rs b/keystore2/src/enforcements.rs
index 04f26e9..95dd026 100644
--- a/keystore2/src/enforcements.rs
+++ b/keystore2/src/enforcements.rs
@@ -20,7 +20,7 @@
use crate::key_parameter::{KeyParameter, KeyParameterValue};
use crate::{authorization::Error as AuthzError, super_key::SuperEncryptionType};
use crate::{
- database::{AuthTokenEntry, MonotonicRawTime},
+ database::{AuthTokenEntry, BootTime},
globals::SUPER_KEY,
};
use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{
@@ -476,7 +476,6 @@
let mut user_id: i32 = -1;
let mut user_secure_ids = Vec::<i64>::new();
let mut key_time_out: Option<i64> = None;
- let mut allow_while_on_body = false;
let mut unlocked_device_required = false;
let mut key_usage_limited: Option<i64> = None;
let mut confirmation_token_receiver: Option<Arc<Mutex<Option<Receiver<Vec<u8>>>>>> = None;
@@ -533,9 +532,6 @@
KeyParameterValue::UnlockedDeviceRequired => {
unlocked_device_required = true;
}
- KeyParameterValue::AllowWhileOnBody => {
- allow_while_on_body = true;
- }
KeyParameterValue::UsageCountLimit(_) => {
// We don't examine the limit here because this is enforced on finish.
// Instead, we store the key_id so that finish can look up the key
@@ -607,14 +603,13 @@
let (hat, state) = if user_secure_ids.is_empty() {
(None, DeferredAuthState::NoAuthRequired)
} else if let Some(key_time_out) = key_time_out {
- let (hat, last_off_body) =
- Self::find_auth_token(|hat: &AuthTokenEntry| match user_auth_type {
- Some(auth_type) => hat.satisfies(&user_secure_ids, auth_type),
- None => false, // not reachable due to earlier check
- })
- .ok_or(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED))
- .context(ks_err!("No suitable auth token found."))?;
- let now = MonotonicRawTime::now();
+ let hat = Self::find_auth_token(|hat: &AuthTokenEntry| match user_auth_type {
+ Some(auth_type) => hat.satisfies(&user_secure_ids, auth_type),
+ None => false, // not reachable due to earlier check
+ })
+ .ok_or(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED))
+ .context(ks_err!("No suitable auth token found."))?;
+ let now = BootTime::now();
let token_age = now
.checked_sub(&hat.time_received())
.ok_or_else(Error::sys)
@@ -623,9 +618,7 @@
Validity cannot be established."
))?;
- let on_body_extended = allow_while_on_body && last_off_body < hat.time_received();
-
- if token_age.seconds() > key_time_out && !on_body_extended {
+ if token_age.seconds() > key_time_out {
return Err(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED))
.context(ks_err!("matching auth token is expired."));
}
@@ -660,8 +653,8 @@
let need_auth_token = timeout_bound || unlocked_device_required;
- let hat_and_last_off_body = if need_auth_token {
- let hat_and_last_off_body = Self::find_auth_token(|hat: &AuthTokenEntry| {
+ let hat = if need_auth_token {
+ let hat = Self::find_auth_token(|hat: &AuthTokenEntry| {
if let (Some(auth_type), true) = (user_auth_type, timeout_bound) {
hat.satisfies(&user_secure_ids, auth_type)
} else {
@@ -669,8 +662,7 @@
}
});
Some(
- hat_and_last_off_body
- .ok_or(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED))
+ hat.ok_or(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED))
.context(ks_err!("No suitable auth token found."))?,
)
} else {
@@ -678,9 +670,9 @@
};
// Now check the validity of the auth token if the key is timeout bound.
- let hat = match (hat_and_last_off_body, key_time_out) {
- (Some((hat, last_off_body)), Some(key_time_out)) => {
- let now = MonotonicRawTime::now();
+ let hat = match (hat, key_time_out) {
+ (Some(hat), Some(key_time_out)) => {
+ let now = BootTime::now();
let token_age = now
.checked_sub(&hat.time_received())
.ok_or_else(Error::sys)
@@ -689,15 +681,13 @@
Validity cannot be established."
))?;
- let on_body_extended = allow_while_on_body && last_off_body < hat.time_received();
-
- if token_age.seconds() > key_time_out && !on_body_extended {
+ if token_age.seconds() > key_time_out {
return Err(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED))
.context(ks_err!("matching auth token is expired."));
}
Some(hat)
}
- (Some((hat, _)), None) => Some(hat),
+ (Some(hat), None) => Some(hat),
// If timeout_bound is true, above code must have retrieved a HAT or returned with
// KEY_USER_NOT_AUTHENTICATED. This arm should not be reachable.
(None, Some(_)) => panic!("Logical error."),
@@ -728,7 +718,7 @@
})
}
- fn find_auth_token<F>(p: F) -> Option<(AuthTokenEntry, MonotonicRawTime)>
+ fn find_auth_token<F>(p: F) -> Option<AuthTokenEntry>
where
F: Fn(&AuthTokenEntry) -> bool,
{
@@ -848,12 +838,12 @@
(challenge == hat.challenge()) && hat.satisfies(&sids, auth_type)
});
- let auth_token = if let Some((auth_token_entry, _)) = result {
+ let auth_token = if let Some(auth_token_entry) = result {
auth_token_entry.take_auth_token()
} else {
// Filter the matching auth tokens by age.
if auth_token_max_age_millis != 0 {
- let now_in_millis = MonotonicRawTime::now();
+ let now_in_millis = BootTime::now();
let result = Self::find_auth_token(|auth_token_entry: &AuthTokenEntry| {
let token_valid = now_in_millis
.checked_sub(&auth_token_entry.time_received())
@@ -863,7 +853,7 @@
token_valid && auth_token_entry.satisfies(&sids, auth_type)
});
- if let Some((auth_token_entry, _)) = result {
+ if let Some(auth_token_entry) = result {
auth_token_entry.take_auth_token()
} else {
return Err(AuthzError::Rc(AuthzResponseCode::NO_AUTH_TOKEN_FOUND))
@@ -889,17 +879,13 @@
&self,
secure_user_id: i64,
auth_type: HardwareAuthenticatorType,
- ) -> Option<MonotonicRawTime> {
+ ) -> Option<BootTime> {
let sids: Vec<i64> = vec![secure_user_id];
let result =
Self::find_auth_token(|entry: &AuthTokenEntry| entry.satisfies(&sids, auth_type));
- if let Some((auth_token_entry, _)) = result {
- Some(auth_token_entry.time_received())
- } else {
- None
- }
+ result.map(|auth_token_entry| auth_token_entry.time_received())
}
}
diff --git a/keystore2/src/error.rs b/keystore2/src/error.rs
index b4c57fb..f0d0d27 100644
--- a/keystore2/src/error.rs
+++ b/keystore2/src/error.rs
@@ -352,7 +352,7 @@
android_logger::init_once(
android_logger::Config::default()
.with_tag("keystore_error_tests")
- .with_min_level(log::Level::Debug),
+ .with_max_level(log::LevelFilter::Debug),
);
// All Error::Rc(x) get mapped on a service specific error
// code of x.
diff --git a/keystore2/src/globals.rs b/keystore2/src/globals.rs
index 0f899ed..7ac1038 100644
--- a/keystore2/src/globals.rs
+++ b/keystore2/src/globals.rs
@@ -16,6 +16,7 @@
//! database connections and connections to services that Keystore needs
//! to talk to.
+use crate::async_task::AsyncTask;
use crate::gc::Gc;
use crate::km_compat::{BacklevelKeyMintWrapper, KeyMintV1};
use crate::ks_err;
@@ -23,7 +24,6 @@
use crate::legacy_importer::LegacyImporter;
use crate::super_key::SuperKeyManager;
use crate::utils::watchdog as wd;
-use crate::{async_task::AsyncTask, database::MonotonicRawTime};
use crate::{
database::KeystoreDB,
database::Uuid,
@@ -68,7 +68,6 @@
DB_INIT.call_once(|| {
log::info!("Touching Keystore 2.0 database for this first time since boot.");
- db.insert_last_off_body(MonotonicRawTime::now());
log::info!("Calling cleanup leftovers.");
let n = db.cleanup_leftovers().expect("Failed to cleanup database on startup.");
if n != 0 {
@@ -247,7 +246,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 +397,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 +440,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/key_parameter.rs b/keystore2/src/key_parameter.rs
index 02a1f16..bd45207 100644
--- a/keystore2/src/key_parameter.rs
+++ b/keystore2/src/key_parameter.rs
@@ -912,7 +912,8 @@
/// The time in seconds for which the key is authorized for use, after user authentication
#[key_param(tag = AUTH_TIMEOUT, field = Integer)]
AuthTimeout(i32),
- /// The key may be used after authentication timeout if device is still on-body
+ /// The key's authentication timeout, if it has one, is automatically expired when the device is
+ /// removed from the user's body. No longer implemented; this tag is no longer enforced.
#[key_param(tag = ALLOW_WHILE_ON_BODY, field = BoolValue)]
AllowWhileOnBody,
/// The key must be unusable except when the user has provided proof of physical presence
diff --git a/keystore2/src/keystore2_main.rs b/keystore2/src/keystore2_main.rs
index 059d59d..178b36c 100644
--- a/keystore2/src/keystore2_main.rs
+++ b/keystore2/src/keystore2_main.rs
@@ -40,8 +40,8 @@
android_logger::init_once(
android_logger::Config::default()
.with_tag("keystore2")
- .with_min_level(log::Level::Debug)
- .with_log_id(android_logger::LogId::System)
+ .with_max_level(log::LevelFilter::Debug)
+ .with_log_buffer(android_logger::LogId::System)
.format(|buf, record| {
writeln!(
buf,
diff --git a/keystore2/src/maintenance.rs b/keystore2/src/maintenance.rs
index 8c0ac48..8780e9e 100644
--- a/keystore2/src/maintenance.rs
+++ b/keystore2/src/maintenance.rs
@@ -14,7 +14,7 @@
//! This module implements IKeystoreMaintenance AIDL interface.
-use crate::database::{KeyEntryLoadBits, KeyType, MonotonicRawTime};
+use crate::database::{KeyEntryLoadBits, KeyType};
use crate::error::map_km_error;
use crate::error::map_or_log_err;
use crate::error::Error;
@@ -24,7 +24,8 @@
use crate::permission::{KeyPerm, KeystorePerm};
use crate::super_key::{SuperKeyManager, UserState};
use crate::utils::{
- check_key_permission, check_keystore_permission, uid_to_android_user, watchdog as wd,
+ check_get_app_uids_affected_by_sid_permissions, check_key_permission,
+ check_keystore_permission, uid_to_android_user, watchdog as wd,
};
use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{
IKeyMintDevice::IKeyMintDevice, SecurityLevel::SecurityLevel,
@@ -223,14 +224,6 @@
Maintenance::call_on_all_security_levels("earlyBootEnded", |dev| dev.earlyBootEnded())
}
- fn on_device_off_body() -> Result<()> {
- // Security critical permission check. This statement must return on fail.
- check_keystore_permission(KeystorePerm::ReportOffBody).context(ks_err!())?;
-
- DB.with(|db| db.borrow_mut().update_last_off_body(MonotonicRawTime::now()));
- Ok(())
- }
-
fn migrate_key_namespace(source: &KeyDescriptor, destination: &KeyDescriptor) -> Result<()> {
let calling_uid = ThreadState::get_calling_uid();
@@ -292,8 +285,9 @@
secure_user_id: i64,
) -> Result<std::vec::Vec<i64>> {
// This method is intended to be called by Settings and discloses a list of apps
- // associated with a user, so it requires the ChangeUser permission.
- check_keystore_permission(KeystorePerm::ChangeUser).context(ks_err!())?;
+ // associated with a user, so it requires the "android.permission.MANAGE_USERS"
+ // permission (to avoid leaking list of apps to unauthorized callers).
+ check_get_app_uids_affected_by_sid_permissions().context(ks_err!())?;
DB.with(|db| db.borrow_mut().get_app_uids_affected_by_sid(user_id, secure_user_id))
.context(ks_err!("Failed to get app UIDs affected by SID"))
}
@@ -353,12 +347,6 @@
map_or_log_err(Self::early_boot_ended(), Ok)
}
- fn onDeviceOffBody(&self) -> BinderResult<()> {
- log::info!("onDeviceOffBody()");
- let _wp = wd::watch_millis("IKeystoreMaintenance::onDeviceOffBody", 500);
- map_or_log_err(Self::on_device_off_body(), Ok)
- }
-
fn migrateKeyNamespace(
&self,
source: &KeyDescriptor,
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/permission.rs b/keystore2/src/permission.rs
index bc73744..982bc82 100644
--- a/keystore2/src/permission.rs
+++ b/keystore2/src/permission.rs
@@ -137,10 +137,7 @@
/// Checked when earlyBootEnded() is called.
#[selinux(name = early_boot_ended)]
EarlyBootEnded,
- /// Checked when IKeystoreMaintenance::onDeviceOffBody is called.
- #[selinux(name = report_off_body)]
- ReportOffBody,
- /// Checked when IkeystoreMetrics::pullMetrics is called.
+ /// Checked when IKeystoreMetrics::pullMetrics is called.
#[selinux(name = pull_metrics)]
PullMetrics,
/// Checked when IKeystoreMaintenance::deleteAllKeys is called.
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/src/super_key.rs b/keystore2/src/super_key.rs
index 44ce9ab..11ab734 100644
--- a/keystore2/src/super_key.rs
+++ b/keystore2/src/super_key.rs
@@ -1011,7 +1011,7 @@
let mut errs = vec![];
for sid in &biometric.sids {
let sid = *sid;
- if let Some((auth_token_entry, _)) = db.find_auth_token_entry(|entry| {
+ if let Some(auth_token_entry) = db.find_auth_token_entry(|entry| {
entry.auth_token().userId == sid || entry.auth_token().authenticatorId == sid
}) {
let res: Result<(Arc<SuperKey>, Arc<SuperKey>)> = (|| {
diff --git a/keystore2/src/utils.rs b/keystore2/src/utils.rs
index 174a22b..a3fd882 100644
--- a/keystore2/src/utils.rs
+++ b/keystore2/src/utils.rs
@@ -129,6 +129,15 @@
check_android_permission("android.permission.REQUEST_UNIQUE_ID_ATTESTATION")
}
+/// This function checks whether the calling app has the Android permissions needed to manage
+/// users. Only callers that can manage users are allowed to get a list of apps affected
+/// by a user's SID changing.
+/// It throws an error if the permissions cannot be verified or if the caller doesn't
+/// have the right permissions. Otherwise it returns silently.
+pub fn check_get_app_uids_affected_by_sid_permissions() -> anyhow::Result<()> {
+ check_android_permission("android.permission.MANAGE_USERS")
+}
+
fn check_android_permission(permission: &str) -> anyhow::Result<()> {
let permission_controller: Strong<dyn IPermissionController::IPermissionController> =
binder::get_interface("permission")?;
diff --git a/keystore2/test_utils/ffi_test_utils.cpp b/keystore2/test_utils/ffi_test_utils.cpp
index 4e781d1..ea03069 100644
--- a/keystore2/test_utils/ffi_test_utils.cpp
+++ b/keystore2/test_utils/ffi_test_utils.cpp
@@ -155,11 +155,19 @@
}
if (!X509_verify(key_cert.get(), signing_pubkey.get())) {
- LOG(ERROR) << "Verification of certificate " << i << " failed "
- << "OpenSSL error string: " << ERR_error_string(ERR_get_error(), NULL)
- << '\n'
- << cert_data.str();
- return false;
+ // Handles the case of device-unique attestation chain which is not expected to be
+ // self-signed - b/191361618
+ // For device-unique attestation chain `strict_issuer_check` is not set, so ignore the
+ // root certificate signature verification result and in all other cases return the
+ // error.
+ bool is_root_cert = (i == chain.size() - 1);
+ if (strict_issuer_check || !is_root_cert) {
+ LOG(ERROR) << "Verification of certificate " << i << " failed "
+ << "OpenSSL error string: " << ERR_error_string(ERR_get_error(), NULL)
+ << '\n'
+ << cert_data.str();
+ return false;
+ }
}
string cert_issuer = x509NameToStr(X509_get_issuer_name(key_cert.get()));
diff --git a/keystore2/test_utils/key_generations.rs b/keystore2/test_utils/key_generations.rs
index 9ddc87a..a733be3 100644
--- a/keystore2/test_utils/key_generations.rs
+++ b/keystore2/test_utils/key_generations.rs
@@ -410,6 +410,11 @@
) {
// Make sure key authorizations contains only `ALLOWED_TAGS_IN_KEY_AUTHS`
authorizations.iter().all(|auth| {
+ // Ignore `INVALID` tag if the backend is Keymaster and not KeyMint.
+ // Keymaster allows INVALID tag for unsupported key parameters.
+ if !has_default_keymint() && auth.keyParameter.tag == Tag::INVALID {
+ return true;
+ }
assert!(
ALLOWED_TAGS_IN_KEY_AUTHS.contains(&auth.keyParameter.tag),
"key authorization is not allowed: {:#?}",
@@ -427,6 +432,21 @@
{
return true;
}
+
+ // Ignore below parameters if the backend is Keymaster and not KeyMint.
+ // Keymaster does not support these parameters. These key parameters are introduced in
+ // KeyMint1.0.
+ if !has_default_keymint() {
+ if matches!(key_param.tag, Tag::RSA_OAEP_MGF_DIGEST | Tag::USAGE_COUNT_LIMIT) {
+ return true;
+ }
+ if key_param.tag == Tag::PURPOSE
+ && key_param.value == KeyParameterValue::KeyPurpose(KeyPurpose::ATTEST_KEY)
+ {
+ return true;
+ }
+ }
+
if ALLOWED_TAGS_IN_KEY_AUTHS.contains(&key_param.tag) {
assert!(
check_key_param(authorizations, key_param),
diff --git a/keystore2/test_utils/run_as.rs b/keystore2/test_utils/run_as.rs
index be643b6..d39d069 100644
--- a/keystore2/test_utils/run_as.rs
+++ b/keystore2/test_utils/run_as.rs
@@ -29,13 +29,14 @@
use keystore2_selinux as selinux;
use nix::sys::wait::{waitpid, WaitStatus};
use nix::unistd::{
- close, fork, pipe as nix_pipe, read as nix_read, setgid, setuid, write as nix_write,
- ForkResult, Gid, Pid, Uid,
+ fork, pipe as nix_pipe, read as nix_read, setgid, setuid, write as nix_write, ForkResult, Gid,
+ Pid, Uid,
};
use serde::{de::DeserializeOwned, Serialize};
use std::io::{Read, Write};
use std::marker::PhantomData;
-use std::os::unix::io::RawFd;
+use std::os::fd::AsRawFd;
+use std::os::fd::OwnedFd;
fn transition(se_context: selinux::Context, uid: Uid, gid: Gid) {
setgid(gid).expect("Failed to set GID. This test might need more privileges.");
@@ -48,35 +49,23 @@
/// PipeReader is a simple wrapper around raw pipe file descriptors.
/// It takes ownership of the file descriptor and closes it on drop. It provides `read_all`, which
/// reads from the pipe into an expending vector, until no more data can be read.
-struct PipeReader(RawFd);
+struct PipeReader(OwnedFd);
impl Read for PipeReader {
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
- let bytes = nix_read(self.0, buf)?;
+ let bytes = nix_read(self.0.as_raw_fd(), buf)?;
Ok(bytes)
}
}
-impl Drop for PipeReader {
- fn drop(&mut self) {
- close(self.0).expect("Failed to close reader pipe fd.");
- }
-}
-
/// PipeWriter is a simple wrapper around raw pipe file descriptors.
/// It takes ownership of the file descriptor and closes it on drop. It provides `write`, which
/// writes the given buffer into the pipe, returning the number of bytes written.
-struct PipeWriter(RawFd);
-
-impl Drop for PipeWriter {
- fn drop(&mut self) {
- close(self.0).expect("Failed to close writer pipe fd.");
- }
-}
+struct PipeWriter(OwnedFd);
impl Write for PipeWriter {
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
- let written = nix_write(self.0, buf)?;
+ let written = nix_write(&self.0, buf)?;
Ok(written)
}
diff --git a/keystore2/tests/AndroidTest.xml b/keystore2/tests/AndroidTest.xml
index 7db36f7..dde18a9 100644
--- a/keystore2/tests/AndroidTest.xml
+++ b/keystore2/tests/AndroidTest.xml
@@ -14,6 +14,7 @@
limitations under the License.
-->
<configuration description="Config to run keystore2_client_tests device tests.">
+ <option name="config-descriptor:metadata" key="parameter" value="not_multi_abi" />
<target_preparer class="com.android.tradefed.targetprep.RootTargetPreparer">
</target_preparer>
diff --git a/keystore2/tests/keystore2_client_attest_key_tests.rs b/keystore2/tests/keystore2_client_attest_key_tests.rs
index 3532a35..454248a 100644
--- a/keystore2/tests/keystore2_client_attest_key_tests.rs
+++ b/keystore2/tests/keystore2_client_attest_key_tests.rs
@@ -31,12 +31,13 @@
use keystore2_test_utils::ffi_test_utils::{get_value_from_attest_record, validate_certchain};
use crate::{
- skip_test_if_no_app_attest_key_feature, skip_test_if_no_device_id_attestation_feature,
+ skip_device_id_attestation_tests, skip_test_if_no_app_attest_key_feature,
+ skip_test_if_no_device_id_attestation_feature,
};
use crate::keystore2_client_test_utils::{
app_attest_key_feature_exists, device_id_attestation_feature_exists, get_attest_id_value,
- is_second_imei_id_attestation_required,
+ is_second_imei_id_attestation_required, skip_device_id_attest_tests,
};
/// Generate RSA and EC attestation keys and use them for signing RSA-signing keys.
@@ -522,6 +523,8 @@
/// INVALID_TAG`
fn generate_attested_key_with_device_attest_ids(algorithm: Algorithm) {
skip_test_if_no_device_id_attestation_feature!();
+ skip_device_id_attestation_tests!();
+ skip_test_if_no_app_attest_key_feature!();
let keystore2 = get_keystore_service();
let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
diff --git a/keystore2/tests/keystore2_client_authorizations_tests.rs b/keystore2/tests/keystore2_client_authorizations_tests.rs
index 2291a08..32be99e 100644
--- a/keystore2/tests/keystore2_client_authorizations_tests.rs
+++ b/keystore2/tests/keystore2_client_authorizations_tests.rs
@@ -41,11 +41,14 @@
};
use crate::keystore2_client_test_utils::{
- delete_app_key, perform_sample_asym_sign_verify_op, perform_sample_hmac_sign_verify_op,
- perform_sample_sym_key_decrypt_op, perform_sample_sym_key_encrypt_op,
- verify_certificate_serial_num, verify_certificate_subject_name, SAMPLE_PLAIN_TEXT,
+ app_attest_key_feature_exists, delete_app_key, perform_sample_asym_sign_verify_op,
+ perform_sample_hmac_sign_verify_op, perform_sample_sym_key_decrypt_op,
+ perform_sample_sym_key_encrypt_op, verify_certificate_serial_num,
+ verify_certificate_subject_name, SAMPLE_PLAIN_TEXT,
};
+use crate::{skip_test_if_no_app_attest_key_feature, skip_tests_if_keymaster_impl_present};
+
use keystore2_test_utils::ffi_test_utils::get_value_from_attest_record;
fn gen_key_including_unique_id(
@@ -112,8 +115,9 @@
let auth = key_generations::get_key_auth(&key_metadata.authorizations, Tag::USAGE_COUNT_LIMIT)
.unwrap();
- if check_attestation {
+ if check_attestation && key_generations::has_default_keymint() {
// Check usage-count-limit is included in attest-record.
+ // `USAGE_COUNT_LIMIT` is supported from KeyMint1.0
assert_ne!(
gen_params.iter().filter(|kp| kp.tag == Tag::ATTESTATION_CHALLENGE).count(),
0,
@@ -438,40 +442,12 @@
delete_app_key(&keystore2, alias).unwrap();
}
-/// Generate a key with `BOOTLOADER_ONLY`. Test should successfully generate
-/// a key and verify the key characteristics. Test should fail with error code `INVALID_KEY_BLOB`
-/// during creation of an operation using this key.
-#[test]
-fn keystore2_gen_key_auth_boot_loader_only_op_fail() {
- let keystore2 = get_keystore_service();
- let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
-
- let gen_params = authorizations::AuthSetBuilder::new()
- .no_auth_required()
- .algorithm(Algorithm::EC)
- .purpose(KeyPurpose::SIGN)
- .purpose(KeyPurpose::VERIFY)
- .digest(Digest::SHA_2_256)
- .ec_curve(EcCurve::P_256)
- .attestation_challenge(b"foo".to_vec())
- .boot_loader_only();
-
- let alias = "ks_test_auth_tags_test";
- let result = key_generations::map_ks_error(key_generations::create_key_and_operation(
- &sec_level,
- &gen_params,
- &authorizations::AuthSetBuilder::new().purpose(KeyPurpose::SIGN).digest(Digest::SHA_2_256),
- alias,
- ));
- assert!(result.is_err());
- assert_eq!(Error::Km(ErrorCode::INVALID_KEY_BLOB), result.unwrap_err());
-}
-
/// Generate a key with `EARLY_BOOT_ONLY`. Test should successfully generate
/// a key and verify the key characteristics. Test should fail with error code `EARLY_BOOT_ENDED`
/// during creation of an operation using this key.
#[test]
fn keystore2_gen_key_auth_early_boot_only_op_fail() {
+ skip_tests_if_keymaster_impl_present!();
let keystore2 = get_keystore_service();
let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
@@ -815,6 +791,7 @@
/// generated attestation-key.
#[test]
fn keystore2_gen_attested_key_auth_app_id_app_data_test_success() {
+ skip_test_if_no_app_attest_key_feature!();
let keystore2 = get_keystore_service();
let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
@@ -869,6 +846,7 @@
/// be provided to generateKey for an attestation key that was generated with them.
#[test]
fn keystore2_gen_attestation_key_with_auth_app_id_app_data_test_fail() {
+ skip_test_if_no_app_attest_key_feature!();
let keystore2 = get_keystore_service();
let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
@@ -973,6 +951,7 @@
/// generate a key successfully and verify the specified key parameters.
#[test]
fn keystore2_gen_key_auth_serial_number_subject_test_success() {
+ skip_tests_if_keymaster_impl_present!();
let keystore2 = get_keystore_service();
let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
diff --git a/keystore2/tests/keystore2_client_device_unique_attestation_tests.rs b/keystore2/tests/keystore2_client_device_unique_attestation_tests.rs
index cf88fc5..b784adf 100644
--- a/keystore2/tests/keystore2_client_device_unique_attestation_tests.rs
+++ b/keystore2/tests/keystore2_client_device_unique_attestation_tests.rs
@@ -27,6 +27,8 @@
perform_sample_asym_sign_verify_op,
};
+use crate::skip_tests_if_keymaster_impl_present;
+
/// This macro is used for generating device unique attested EC key with device id attestation.
macro_rules! test_ec_key_device_unique_attestation_id {
( $test_name:ident, $tag:expr, $prop_name:expr ) => {
@@ -158,6 +160,7 @@
/// Test should fail to generate a key with error code `INVALID_ARGUMENT`
#[test]
fn keystore2_gen_key_device_unique_attest_with_default_sec_level_unimplemented() {
+ skip_tests_if_keymaster_impl_present!();
let keystore2 = get_keystore_service();
let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
@@ -178,7 +181,10 @@
alias,
));
assert!(result.is_err());
- assert_eq!(Error::Km(ErrorCode::INVALID_ARGUMENT), result.unwrap_err());
+ assert!(matches!(
+ result.unwrap_err(),
+ Error::Km(ErrorCode::INVALID_ARGUMENT) | Error::Km(ErrorCode::UNSUPPORTED_TAG)
+ ));
}
/// Generate a EC key with `DEVICE_UNIQUE_ATTESTATION` using `STRONGBOX` security level.
@@ -324,32 +330,32 @@
test_ec_key_device_unique_attestation_id!(
keystore2_device_unique_attest_ecdsa_attest_id_brand,
Tag::ATTESTATION_ID_BRAND,
- "ro.product.brand_for_attestation"
+ "brand"
);
test_ec_key_device_unique_attestation_id!(
keystore2_device_unique_attest_ecdsa_attest_id_device,
Tag::ATTESTATION_ID_DEVICE,
- "ro.product.device"
+ "device"
);
test_ec_key_device_unique_attestation_id!(
keystore2_device_unique_attest_ecdsa_attest_id_product,
Tag::ATTESTATION_ID_PRODUCT,
- "ro.product.name_for_attestation"
+ "name"
);
test_ec_key_device_unique_attestation_id!(
keystore2_device_unique_attest_ecdsa_attest_id_serial,
Tag::ATTESTATION_ID_SERIAL,
- "ro.serialno"
+ "serialno"
);
test_ec_key_device_unique_attestation_id!(
keystore2_device_unique_attest_ecdsa_attest_id_manufacturer,
Tag::ATTESTATION_ID_MANUFACTURER,
- "ro.product.manufacturer"
+ "manufacturer"
);
test_ec_key_device_unique_attestation_id!(
keystore2_device_unique_attest_ecdsa_attest_id_model,
Tag::ATTESTATION_ID_MODEL,
- "ro.product.model_for_attestation"
+ "model"
);
test_ec_key_device_unique_attestation_id!(
keystore2_device_unique_attest_ecdsa_attest_id_imei,
@@ -367,32 +373,32 @@
test_rsa_key_device_unique_attestation_id!(
keystore2_device_unique_attest_rsa_attest_id_brand,
Tag::ATTESTATION_ID_BRAND,
- "ro.product.brand_for_attestation"
+ "brand"
);
test_rsa_key_device_unique_attestation_id!(
keystore2_device_unique_attest_rsa_attest_id_device,
Tag::ATTESTATION_ID_DEVICE,
- "ro.product.device"
+ "device"
);
test_rsa_key_device_unique_attestation_id!(
keystore2_device_unique_attest_rsa_attest_id_product,
Tag::ATTESTATION_ID_PRODUCT,
- "ro.product.name_for_attestation"
+ "name"
);
test_rsa_key_device_unique_attestation_id!(
keystore2_device_unique_attest_rsa_attest_id_serial,
Tag::ATTESTATION_ID_SERIAL,
- "ro.serialno"
+ "serialno"
);
test_rsa_key_device_unique_attestation_id!(
keystore2_device_unique_attest_rsa_attest_id_manufacturer,
Tag::ATTESTATION_ID_MANUFACTURER,
- "ro.product.manufacturer"
+ "manufacturer"
);
test_rsa_key_device_unique_attestation_id!(
keystore2_device_unique_attest_rsa_attest_id_model,
Tag::ATTESTATION_ID_MODEL,
- "ro.product.model_for_attestation"
+ "model"
);
test_rsa_key_device_unique_attestation_id!(
keystore2_device_unique_attest_rsa_attest_id_imei,
diff --git a/keystore2/tests/keystore2_client_ec_key_tests.rs b/keystore2/tests/keystore2_client_ec_key_tests.rs
index 8267140..f2c6d0f 100644
--- a/keystore2/tests/keystore2_client_ec_key_tests.rs
+++ b/keystore2/tests/keystore2_client_ec_key_tests.rs
@@ -30,8 +30,8 @@
};
use crate::keystore2_client_test_utils::{
- delete_app_key, execute_op_run_as_child, perform_sample_sign_operation, BarrierReached,
- ForcedOp, TestOutcome,
+ delete_app_key, execute_op_run_as_child, get_vsr_api_level, perform_sample_sign_operation,
+ BarrierReached, ForcedOp, TestOutcome,
};
macro_rules! test_ec_sign_key_op_success {
@@ -374,13 +374,18 @@
)
.unwrap();
- let result = key_generations::map_ks_error(sec_level.createOperation(
- &key_metadata.key,
- &authorizations::AuthSetBuilder::new().purpose(KeyPurpose::SIGN).digest(digest),
- false,
- ));
- assert!(result.is_err());
- assert_eq!(Error::Km(ErrorCode::UNSUPPORTED_DIGEST), result.unwrap_err());
+ // The KeyMint v2 API added `CURVE_25519` and specified that "Ed25519 keys only support
+ // Digest::NONE". However, this was not checked at the time so we can only be strict about
+ // checking this for more recent implementations.
+ if get_vsr_api_level() >= 35 {
+ let result = key_generations::map_ks_error(sec_level.createOperation(
+ &key_metadata.key,
+ &authorizations::AuthSetBuilder::new().purpose(KeyPurpose::SIGN).digest(digest),
+ false,
+ ));
+ assert!(result.is_err(), "unexpected success for digest {digest:?}");
+ assert_eq!(Error::Km(ErrorCode::UNSUPPORTED_DIGEST), result.unwrap_err());
+ }
}
}
diff --git a/keystore2/tests/keystore2_client_import_keys_tests.rs b/keystore2/tests/keystore2_client_import_keys_tests.rs
index 31d57a2..bf787d2 100644
--- a/keystore2/tests/keystore2_client_import_keys_tests.rs
+++ b/keystore2/tests/keystore2_client_import_keys_tests.rs
@@ -37,9 +37,9 @@
};
use crate::keystore2_client_test_utils::{
- encrypt_secure_key, encrypt_transport_key, perform_sample_asym_sign_verify_op,
- perform_sample_hmac_sign_verify_op, perform_sample_sym_key_decrypt_op,
- perform_sample_sym_key_encrypt_op, SAMPLE_PLAIN_TEXT,
+ encrypt_secure_key, encrypt_transport_key, get_vsr_api_level,
+ perform_sample_asym_sign_verify_op, perform_sample_hmac_sign_verify_op,
+ perform_sample_sym_key_decrypt_op, perform_sample_sym_key_encrypt_op, SAMPLE_PLAIN_TEXT,
};
pub fn import_rsa_sign_key_and_perform_sample_operation(
@@ -306,6 +306,13 @@
let alias = format!("ks_ec_key_test_import_1_{}{}", getuid(), 256);
+ if get_vsr_api_level() < 35 {
+ // The KeyMint spec was previously not clear as to whether EC_CURVE was optional on import
+ // of EC keys. However, this was not checked at the time so we can only be strict about
+ // checking this for implementations at VSR-V or later.
+ println!("Skipping EC_CURVE on import only strict >= VSR-V");
+ return;
+ }
// Don't specify ec-curve.
let import_params = authorizations::AuthSetBuilder::new()
.no_auth_required()
diff --git a/keystore2/tests/keystore2_client_list_entries_tests.rs b/keystore2/tests/keystore2_client_list_entries_tests.rs
index 1c0c496..8b3f700 100644
--- a/keystore2/tests/keystore2_client_list_entries_tests.rs
+++ b/keystore2/tests/keystore2_client_list_entries_tests.rs
@@ -140,7 +140,7 @@
let key_descriptors = keystore2.listEntries(Domain::APP, -1).unwrap();
assert_eq!(1, key_descriptors.len());
- let key = key_descriptors.get(0).unwrap();
+ let key = key_descriptors.first().unwrap();
assert_eq!(key.alias, Some(alias));
assert_eq!(key.nspace, GRANTEE_UID.try_into().unwrap());
assert_eq!(key.domain, Domain::APP);
diff --git a/keystore2/tests/keystore2_client_test_utils.rs b/keystore2/tests/keystore2_client_test_utils.rs
index 037482a..7534da3 100644
--- a/keystore2/tests/keystore2_client_test_utils.rs
+++ b/keystore2/tests/keystore2_client_test_utils.rs
@@ -15,6 +15,7 @@
use nix::unistd::{Gid, Uid};
use serde::{Deserialize, Serialize};
+use std::path::PathBuf;
use std::process::{Command, Output};
use openssl::bn::BigNum;
@@ -88,6 +89,19 @@
pm.hasSystemFeature(DEVICE_ID_ATTESTATION_FEATURE, 0).expect("hasSystemFeature failed.")
}
+/// Determines whether to skip device id attestation tests on GSI build with API level < 34.
+pub fn skip_device_id_attest_tests() -> bool {
+ // b/298586194, there are some devices launched with Android T, and they will be receiving
+ // 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.
+
+ // 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]
macro_rules! skip_test_if_no_app_attest_key_feature {
() => {
@@ -106,6 +120,24 @@
};
}
+#[macro_export]
+macro_rules! skip_device_id_attestation_tests {
+ () => {
+ if skip_device_id_attest_tests() {
+ return;
+ }
+ };
+}
+
+#[macro_export]
+macro_rules! skip_tests_if_keymaster_impl_present {
+ () => {
+ if !key_generations::has_default_keymint() {
+ return;
+ }
+ };
+}
+
/// Generate EC key and grant it to the list of users with given access vector.
/// Returns the list of granted keys `nspace` values in the order of given grantee uids.
pub fn generate_ec_key_and_grant_to_users(
@@ -479,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/keystore2/tests/legacy_blobs/keystore2_legacy_blob_tests.rs b/keystore2/tests/legacy_blobs/keystore2_legacy_blob_tests.rs
index 0335159..3be99ee 100644
--- a/keystore2/tests/legacy_blobs/keystore2_legacy_blob_tests.rs
+++ b/keystore2/tests/legacy_blobs/keystore2_legacy_blob_tests.rs
@@ -46,6 +46,10 @@
static AUTH_SERVICE_NAME: &str = "android.security.authorization";
const SELINUX_SHELL_NAMESPACE: i64 = 1;
+fn rkp_only() -> bool {
+ matches!(rustutils::system_properties::read("remote_provisioning.tee.rkp_only"), Ok(Some(v)) if v == "1")
+}
+
fn get_maintenance() -> binder::Strong<dyn IKeystoreMaintenance> {
binder::get_interface(USER_MANAGER_SERVICE_NAME).unwrap()
}
@@ -162,13 +166,13 @@
.getSecurityLevel(SecurityLevel::SecurityLevel::TRUSTED_ENVIRONMENT)
.unwrap();
// Generate Key BLOB and prepare legacy keystore blob files.
- let att_challenge: &[u8] = b"foo";
+ let att_challenge: Option<&[u8]> = if rkp_only() { None } else { Some(b"foo") };
let key_metadata = key_generations::generate_ec_p256_signing_key(
&sec_level,
Domain::BLOB,
SELINUX_SHELL_NAMESPACE,
None,
- Some(att_challenge),
+ att_challenge,
)
.expect("Failed to generate key blob");
@@ -212,14 +216,12 @@
.unwrap();
}
- let mut path_buf = PathBuf::from("/data/misc/keystore/user_99");
- path_buf.push("9910001_CACERT_authbound");
- if !path_buf.as_path().is_file() {
- make_cert_blob_file(
- path_buf.as_path(),
- key_metadata.certificateChain.as_ref().unwrap(),
- )
- .unwrap();
+ if let Some(chain) = key_metadata.certificateChain.as_ref() {
+ let mut path_buf = PathBuf::from("/data/misc/keystore/user_99");
+ path_buf.push("9910001_CACERT_authbound");
+ if !path_buf.as_path().is_file() {
+ make_cert_blob_file(path_buf.as_path(), chain).unwrap();
+ }
}
// Keystore2 disables the legacy importer when it finds the legacy database empty.
@@ -246,7 +248,7 @@
KeygenResult {
cert: key_metadata.certificate.unwrap(),
- cert_chain: key_metadata.certificateChain.unwrap(),
+ cert_chain: key_metadata.certificateChain.unwrap_or_default(),
key_parameters: key_params,
}
})
@@ -275,7 +277,7 @@
gen_key_result.cert
);
assert_eq!(
- key_entry_response.metadata.certificateChain.unwrap(),
+ key_entry_response.metadata.certificateChain.unwrap_or_default(),
gen_key_result.cert_chain
);
assert_eq!(key_entry_response.metadata.key.domain, Domain::KEY_ID);
@@ -415,13 +417,13 @@
.getSecurityLevel(SecurityLevel::SecurityLevel::TRUSTED_ENVIRONMENT)
.unwrap();
// Generate Key BLOB and prepare legacy keystore blob files.
- let att_challenge: &[u8] = b"foo";
+ let att_challenge: Option<&[u8]> = if rkp_only() { None } else { Some(b"foo") };
let key_metadata = key_generations::generate_ec_p256_signing_key(
&sec_level,
Domain::BLOB,
SELINUX_SHELL_NAMESPACE,
None,
- Some(att_challenge),
+ att_challenge,
)
.expect("Failed to generate key blob");
@@ -468,15 +470,12 @@
.unwrap();
}
- let mut path_buf = PathBuf::from("/data/misc/keystore/user_98");
- path_buf.push("9810001_CACERT_authboundcertenc");
- if !path_buf.as_path().is_file() {
- make_encrypted_ca_cert_file(
- path_buf.as_path(),
- &super_key,
- key_metadata.certificateChain.as_ref().unwrap(),
- )
- .unwrap();
+ if let Some(chain) = key_metadata.certificateChain.as_ref() {
+ let mut path_buf = PathBuf::from("/data/misc/keystore/user_98");
+ path_buf.push("9810001_CACERT_authboundcertenc");
+ if !path_buf.as_path().is_file() {
+ make_encrypted_ca_cert_file(path_buf.as_path(), &super_key, chain).unwrap();
+ }
}
// Keystore2 disables the legacy importer when it finds the legacy database empty.
@@ -503,7 +502,7 @@
KeygenResult {
cert: key_metadata.certificate.unwrap(),
- cert_chain: key_metadata.certificateChain.unwrap(),
+ cert_chain: key_metadata.certificateChain.unwrap_or_default(),
key_parameters: key_params,
}
})
@@ -532,7 +531,7 @@
gen_key_result.cert
);
assert_eq!(
- key_entry_response.metadata.certificateChain.unwrap(),
+ key_entry_response.metadata.certificateChain.unwrap_or_default(),
gen_key_result.cert_chain
);
diff --git a/keystore2/watchdog/src/lib.rs b/keystore2/watchdog/src/lib.rs
index 01043c5..fa4620a 100644
--- a/keystore2/watchdog/src/lib.rs
+++ b/keystore2/watchdog/src/lib.rs
@@ -335,7 +335,7 @@
android_logger::init_once(
android_logger::Config::default()
.with_tag("keystore2_watchdog_tests")
- .with_min_level(log::Level::Debug),
+ .with_max_level(log::LevelFilter::Debug),
);
let wd = Watchdog::new(Watchdog::NOISY_REPORT_TIMEOUT.checked_mul(3).unwrap());
diff --git a/ondevice-signing/Android.bp b/ondevice-signing/Android.bp
index f56cfab..6901b17 100644
--- a/ondevice-signing/Android.bp
+++ b/ondevice-signing/Android.bp
@@ -142,6 +142,8 @@
"libfsverity",
"liblogwrap",
"libprotobuf-cpp-lite",
+ "libstatspull",
+ "libstatssocket",
"libutils",
],
}
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
diff --git a/ondevice-signing/tests/Android.bp b/ondevice-signing/tests/Android.bp
index 4027220..bcfe8e4 100644
--- a/ondevice-signing/tests/Android.bp
+++ b/ondevice-signing/tests/Android.bp
@@ -13,6 +13,7 @@
// limitations under the License.
package {
+ default_team: "trendy_team_art_mainline",
// See: http://go/android-license-faq
// A large-scale-change added 'default_applicable_licenses' to import
// all of the 'license_kinds' from "system_security_license"
@@ -22,23 +23,23 @@
}
cc_test {
- name: "libsigningutils_test",
- srcs: ["SigningUtilsTest.cpp"],
- test_suites: ["device-tests"],
- compile_multilib: "both",
- defaults: [
- "odsign_flags_defaults",
- ],
- static_libs: [
- "libsigningutils",
- ],
- shared_libs: [
- "libbase",
- "libcrypto",
- ],
- data: [
- "test_file",
- "test_file.sig",
- "SigningUtils.cert.der",
- ],
+ name: "libsigningutils_test",
+ srcs: ["SigningUtilsTest.cpp"],
+ test_suites: ["device-tests"],
+ compile_multilib: "both",
+ defaults: [
+ "odsign_flags_defaults",
+ ],
+ static_libs: [
+ "libsigningutils",
+ ],
+ shared_libs: [
+ "libbase",
+ "libcrypto",
+ ],
+ data: [
+ "test_file",
+ "test_file.sig",
+ "SigningUtils.cert.der",
+ ],
}
diff --git a/prng_seeder/Android.bp b/prng_seeder/Android.bp
index 3c8179f..4f9b7e1 100644
--- a/prng_seeder/Android.bp
+++ b/prng_seeder/Android.bp
@@ -13,6 +13,7 @@
// limitations under the License.
package {
+ default_team: "trendy_team_java_core_libraries",
// See: http://go/android-license-faq
// SPDX-license-identifier-Apache-2.0
default_applicable_licenses: ["system_security_license"],
diff --git a/prng_seeder/src/main.rs b/prng_seeder/src/main.rs
index f8b0c63..cb7f38d 100644
--- a/prng_seeder/src/main.rs
+++ b/prng_seeder/src/main.rs
@@ -31,7 +31,7 @@
use anyhow::{ensure, Context, Result};
use clap::Parser;
-use log::{error, info, Level};
+use log::{error, info, LevelFilter};
use nix::sys::signal;
use tokio::{io::AsyncWriteExt, net::UnixListener as TokioUnixListener};
@@ -48,7 +48,9 @@
fn configure_logging() -> Result<()> {
ensure!(
logger::init(
- logger::Config::default().with_tag_on_device("prng_seeder").with_min_level(Level::Info)
+ logger::Config::default()
+ .with_tag_on_device("prng_seeder")
+ .with_max_level(LevelFilter::Info)
),
"log configuration failed"
);
diff --git a/provisioner/Android.bp b/provisioner/Android.bp
index 605abb4..ede1ae6 100644
--- a/provisioner/Android.bp
+++ b/provisioner/Android.bp
@@ -15,6 +15,7 @@
//
package {
+ default_team: "trendy_team_foundation_security_rust_pkvm_",
// See: http://go/android-license-faq
// A large-scale-change added 'default_applicable_licenses' to import
// all of the 'license_kinds' from "system_security_license"
@@ -38,7 +39,7 @@
"android.hardware.drm-V1-ndk",
"android.hardware.security.rkp-V3-ndk",
"libbase",
- "libcppbor_external",
+ "libcppbor",
"libcppcose_rkp",
"libjsoncpp",
"libkeymint_remote_prov_support",
@@ -84,7 +85,7 @@
"dist_files",
"rkp_factory_extraction_tool",
],
- dest: "rkp_factory_extraction_tool"
+ dest: "rkp_factory_extraction_tool",
},
compile_multilib: "both",
multilib: {
diff --git a/provisioner/rkp_factory_extraction_tool.cpp b/provisioner/rkp_factory_extraction_tool.cpp
index 0a3a59a..62d62cf 100644
--- a/provisioner/rkp_factory_extraction_tool.cpp
+++ b/provisioner/rkp_factory_extraction_tool.cpp
@@ -24,6 +24,7 @@
#include <remote_prov/remote_prov_utils.h>
#include <sys/random.h>
+#include <future>
#include <string>
#include <vector>
@@ -91,7 +92,13 @@
// for every IRemotelyProvisionedComponent.
void getCsrForInstance(const char* name, void* /*context*/) {
auto fullName = getFullServiceName(IRemotelyProvisionedComponent::descriptor, name);
- AIBinder* rkpAiBinder = AServiceManager_getService(fullName.c_str());
+ std::future<AIBinder*> wait_for_service_func =
+ std::async(std::launch::async, AServiceManager_waitForService, fullName.c_str());
+ if (wait_for_service_func.wait_for(std::chrono::seconds(10)) == std::future_status::timeout) {
+ std::cerr << "Wait for service timed out after 10 seconds: " << fullName;
+ exit(-1);
+ }
+ AIBinder* rkpAiBinder = wait_for_service_func.get();
::ndk::SpAIBinder rkp_binder(rkpAiBinder);
auto rkp_service = IRemotelyProvisionedComponent::fromBinder(rkp_binder);
if (!rkp_service) {
diff --git a/provisioner/support/Android.bp b/provisioner/support/Android.bp
index 52b204f..24cfd03 100644
--- a/provisioner/support/Android.bp
+++ b/provisioner/support/Android.bp
@@ -13,6 +13,7 @@
// limitations under the License.
package {
+ default_team: "trendy_team_android_hardware_backed_security",
// See: http://go/android-license-faq
// A large-scale-change added 'default_applicable_licenses' to import
// all of the 'license_kinds' from "hardware_interfaces_license"