Merge "Fixes for the issues found while running Keystore2 client tests on a device with keymaster implementation." into main
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/keystore2/Android.bp b/keystore2/Android.bp
index ad151ad..7cb7c37 100644
--- a/keystore2/Android.bp
+++ b/keystore2/Android.bp
@@ -158,6 +158,7 @@
aconfig_declarations {
name: "keystore2_flags",
package: "android.security.keystore2",
+ container: "system",
srcs: ["aconfig/flags.aconfig"],
}
diff --git a/keystore2/aconfig/flags.aconfig b/keystore2/aconfig/flags.aconfig
index 133c4ab..b67bc6c 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"
diff --git a/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl b/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
index e612db9..abea958 100644
--- a/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
+++ b/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
@@ -151,7 +151,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/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/database.rs b/keystore2/src/database.rs
index 0a1c547..b526daa 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -761,22 +761,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 +787,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 +805,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 +832,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
}
@@ -1167,9 +1167,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.")?;
@@ -2858,14 +2858,12 @@
/// 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, BootTime)>
where
F: Fn(&AuthTokenEntry) -> bool,
{
@@ -2873,17 +2871,17 @@
}
/// 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) {
+ pub fn insert_last_off_body(&self, last_off_body: BootTime) {
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) {
+ pub fn update_last_off_body(&self, last_off_body: BootTime) {
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 {
+ fn get_last_off_body(&self) -> BootTime {
self.perboot.get_last_off_body()
}
@@ -5054,13 +5052,13 @@
#[test]
fn test_last_off_body() -> Result<()> {
let mut db = new_test_db()?;
- db.insert_last_off_body(MonotonicRawTime::now());
+ db.insert_last_off_body(BootTime::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());
+ db.update_last_off_body(BootTime::now());
let tx2 = db.conn.transaction_with_behavior(TransactionBehavior::Immediate)?;
tx2.commit()?;
let last_off_body_2 = db.get_last_off_body();
diff --git a/keystore2/src/database/perboot.rs b/keystore2/src/database/perboot.rs
index 7ff35fa..1b7c80d 100644
--- a/keystore2/src/database/perboot.rs
+++ b/keystore2/src/database/perboot.rs
@@ -15,7 +15,7 @@
//! 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.
-use super::{AuthTokenEntry, MonotonicRawTime};
+use super::{AuthTokenEntry, BootTime};
use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{
HardwareAuthToken::HardwareAuthToken, HardwareAuthenticatorType::HardwareAuthenticatorType,
};
@@ -103,11 +103,11 @@
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))
+ pub fn get_last_off_body(&self) -> BootTime {
+ BootTime(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) {
+ pub fn set_last_off_body(&self, last_off_body: BootTime) {
self.last_off_body.store(last_off_body.0, Ordering::Relaxed)
}
/// Return how many auth tokens are currently tracked.
diff --git a/keystore2/src/enforcements.rs b/keystore2/src/enforcements.rs
index 04f26e9..55c9591 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::{
@@ -614,7 +614,7 @@
})
.ok_or(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED))
.context(ks_err!("No suitable auth token found."))?;
- let now = MonotonicRawTime::now();
+ let now = BootTime::now();
let token_age = now
.checked_sub(&hat.time_received())
.ok_or_else(Error::sys)
@@ -680,7 +680,7 @@
// 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 now = BootTime::now();
let token_age = now
.checked_sub(&hat.time_received())
.ok_or_else(Error::sys)
@@ -728,7 +728,7 @@
})
}
- fn find_auth_token<F>(p: F) -> Option<(AuthTokenEntry, MonotonicRawTime)>
+ fn find_auth_token<F>(p: F) -> Option<(AuthTokenEntry, BootTime)>
where
F: Fn(&AuthTokenEntry) -> bool,
{
@@ -853,7 +853,7 @@
} 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())
@@ -889,7 +889,7 @@
&self,
secure_user_id: i64,
auth_type: HardwareAuthenticatorType,
- ) -> Option<MonotonicRawTime> {
+ ) -> Option<BootTime> {
let sids: Vec<i64> = vec![secure_user_id];
let result =
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..eb755a6 100644
--- a/keystore2/src/globals.rs
+++ b/keystore2/src/globals.rs
@@ -23,7 +23,7 @@
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::{async_task::AsyncTask, database::BootTime};
use crate::{
database::KeystoreDB,
database::Uuid,
@@ -68,7 +68,7 @@
DB_INIT.call_once(|| {
log::info!("Touching Keystore 2.0 database for this first time since boot.");
- db.insert_last_off_body(MonotonicRawTime::now());
+ db.insert_last_off_body(BootTime::now());
log::info!("Calling cleanup leftovers.");
let n = db.cleanup_leftovers().expect("Failed to cleanup database on startup.");
if n != 0 {
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..3e34cff 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::{BootTime, 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,
@@ -227,7 +228,7 @@
// 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()));
+ DB.with(|db| db.borrow_mut().update_last_off_body(BootTime::now()));
Ok(())
}
@@ -292,8 +293,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"))
}
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/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/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/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"
);