Merge "Freeze AIDL APIs for SC" into sc-dev
diff --git a/keystore2/src/async_task.rs b/keystore2/src/async_task.rs
index 45f0274..e130024 100644
--- a/keystore2/src/async_task.rs
+++ b/keystore2/src/async_task.rs
@@ -19,7 +19,6 @@
//! processed all tasks before it terminates.
//! Note that low priority tasks are processed only when the high priority queue is empty.
-use crate::utils::watchdog as wd;
use std::{any::Any, any::TypeId, time::Duration};
use std::{
collections::{HashMap, VecDeque},
@@ -241,14 +240,11 @@
// Now that the lock has been dropped, perform the action.
match action {
Action::QueuedFn(f) => {
- let _wd = wd::watch_millis("async_task thread: calling queued fn", 500);
f(&mut shelf);
done_idle = false;
}
Action::IdleFns(idle_fns) => {
for idle_fn in idle_fns {
- let _wd =
- wd::watch_millis("async_task thread: calling idle_fn", 500);
idle_fn(&mut shelf);
}
done_idle = true;
diff --git a/keystore2/src/audit_log.rs b/keystore2/src/audit_log.rs
index 30fc155..3d7d26e 100644
--- a/keystore2/src/audit_log.rs
+++ b/keystore2/src/audit_log.rs
@@ -25,14 +25,15 @@
const TAG_KEY_GENERATED: u32 = 210024;
const TAG_KEY_IMPORTED: u32 = 210025;
const TAG_KEY_DESTROYED: u32 = 210026;
+const TAG_KEY_INTEGRITY_VIOLATION: u32 = 210032;
-const NAMESPACE_MASK: i64 = 0x80000000;
+const FLAG_NAMESPACE: i64 = 0x80000000;
-/// For app domain returns calling app uid, for SELinux domain returns masked namespace.
-fn key_owner(key: &KeyDescriptor, calling_app: uid_t) -> i32 {
- match key.domain {
- Domain::APP => calling_app as i32,
- Domain::SELINUX => (key.nspace | NAMESPACE_MASK) as i32,
+/// Encode key owner as either uid or namespace with a flag.
+fn key_owner(domain: Domain, nspace: i64, uid: i32) -> i32 {
+ match domain {
+ Domain::APP => uid,
+ Domain::SELINUX => (nspace | FLAG_NAMESPACE) as i32,
_ => {
log::info!("Not logging audit event for key with unexpected domain");
0
@@ -55,12 +56,29 @@
log_key_event(TAG_KEY_DESTROYED, key, calling_app, success);
}
+/// 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)
+ })
+}
+
fn log_key_event(tag: u32, key: &KeyDescriptor, calling_app: uid_t, success: bool) {
- if let Some(ctx) = LogContext::new(LogIdSecurity, tag) {
- let event = ctx
- .append_i32(if success { 1 } else { 0 })
+ with_log_context(tag, |ctx| {
+ let owner = key_owner(key.domain, key.nspace, calling_app as i32);
+ ctx.append_i32(if success { 1 } else { 0 })
.append_str(key.alias.as_ref().map_or("none", String::as_str))
- .append_i32(key_owner(key, calling_app));
+ .append_i32(owner)
+ })
+}
+
+fn with_log_context<F>(tag: u32, f: F)
+where
+ F: Fn(LogContext) -> LogContext,
+{
+ if let Some(ctx) = LogContext::new(LogIdSecurity, tag) {
+ let event = f(ctx);
LOGS_HANDLER.queue_lo(move |_| {
event.write();
});
diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs
index e2e9349..2930162 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -3168,6 +3168,30 @@
fn get_last_off_body(&self) -> MonotonicRawTime {
self.perboot.get_last_off_body()
}
+
+ /// Load descriptor of a key by key id
+ pub fn load_key_descriptor(&mut self, key_id: i64) -> Result<Option<KeyDescriptor>> {
+ let _wp = wd::watch_millis("KeystoreDB::load_key_descriptor", 500);
+
+ self.with_transaction(TransactionBehavior::Deferred, |tx| {
+ tx.query_row(
+ "SELECT domain, namespace, alias FROM persistent.keyentry WHERE id = ?;",
+ params![key_id],
+ |row| {
+ Ok(KeyDescriptor {
+ domain: Domain(row.get(0)?),
+ nspace: row.get(1)?,
+ alias: row.get(2)?,
+ blob: None,
+ })
+ },
+ )
+ .optional()
+ .context("Trying to load key descriptor")
+ .no_gc()
+ })
+ .context("In load_key_descriptor.")
+ }
}
#[cfg(test)]
@@ -5520,4 +5544,20 @@
assert_eq!(mode, "wal");
Ok(())
}
+
+ #[test]
+ fn test_load_key_descriptor() -> Result<()> {
+ let mut db = new_test_db()?;
+ let key_id = make_test_key_entry(&mut db, Domain::APP, 1, TEST_ALIAS, None)?.0;
+
+ let key = db.load_key_descriptor(key_id)?.unwrap();
+
+ assert_eq!(key.domain, Domain::APP);
+ assert_eq!(key.nspace, 1);
+ assert_eq!(key.alias, Some(TEST_ALIAS.to_string()));
+
+ // No such id
+ assert_eq!(db.load_key_descriptor(key_id + 1)?, None);
+ Ok(())
+ }
}
diff --git a/keystore2/src/keystore2_main.rs b/keystore2/src/keystore2_main.rs
index 53461da..3d53a36 100644
--- a/keystore2/src/keystore2_main.rs
+++ b/keystore2/src/keystore2_main.rs
@@ -132,12 +132,7 @@
},
);
- std::thread::spawn(|| {
- match metrics::register_pull_metrics_callbacks() {
- Err(e) => error!("register_pull_metrics_callbacks failed: {:?}.", e),
- _ => info!("Pull metrics callbacks successfully registered."),
- };
- });
+ metrics::register_pull_metrics_callbacks();
info!("Successfully registered Keystore 2.0 service.");
diff --git a/keystore2/src/metrics.rs b/keystore2/src/metrics.rs
index 07c3d64..10a465c 100644
--- a/keystore2/src/metrics.rs
+++ b/keystore2/src/metrics.rs
@@ -23,7 +23,7 @@
KeyParameter::KeyParameter, KeyPurpose::KeyPurpose, PaddingMode::PaddingMode,
SecurityLevel::SecurityLevel,
};
-use anyhow::Result;
+use anyhow::{anyhow, Result};
use keystore2_system_property::PropertyWatcher;
use statslog_rust::{
keystore2_key_creation_event_reported::{
@@ -40,6 +40,47 @@
use statslog_rust_header::Atoms;
use statspull_rust::{set_pull_atom_callback, StatsPullResult};
+// Waits and returns Ok if boot is completed.
+fn wait_for_boot_completed() -> Result<()> {
+ let watcher = PropertyWatcher::new("sys.boot_completed");
+ match watcher {
+ Ok(mut watcher) => {
+ loop {
+ let wait_result = watcher.wait();
+ match wait_result {
+ Ok(_) => {
+ let value_result =
+ watcher.read(|_name, value| Ok(value.trim().to_string()));
+ match value_result {
+ Ok(value) => {
+ if value == "1" {
+ break;
+ }
+ }
+ Err(e) => {
+ log::error!(
+ "In wait_for_boot_completed: Failed while reading property. {}",
+ e
+ );
+ return Err(anyhow!("Error in waiting for boot completion."));
+ }
+ }
+ }
+ Err(e) => {
+ log::error!("In wait_for_boot_completed: Failed while waiting. {}", e);
+ return Err(anyhow!("Error in waiting for boot completion."));
+ }
+ }
+ }
+ Ok(())
+ }
+ Err(e) => {
+ log::error!("In wait_for_boot_completed: Failed to create PropertyWatcher. {}", e);
+ Err(anyhow!("Error in waiting for boot completion."))
+ }
+ }
+}
+
fn create_default_key_creation_atom() -> Keystore2KeyCreationEventReported {
// If a value is not present, fields represented by bitmaps and i32 fields
// will take 0, except error_code which defaults to 1 indicating NO_ERROR and key_size,
@@ -89,10 +130,10 @@
construct_key_creation_event_stats(sec_level, key_params, result);
LOGS_HANDLER.queue_lo(move |_| {
- let logging_result = key_creation_event_stats.stats_write();
-
- if let Err(e) = logging_result {
- log::error!("Error in logging key creation event in the async task. {:?}", e);
+ if let Ok(()) = wait_for_boot_completed() {
+ if let Err(e) = key_creation_event_stats.stats_write() {
+ log::error!("Error in logging key creation event in the async task. {:?}", e);
+ }
}
});
}
@@ -114,10 +155,10 @@
);
LOGS_HANDLER.queue_lo(move |_| {
- let logging_result = key_operation_event_stats.stats_write();
-
- if let Err(e) = logging_result {
- log::error!("Error in logging key operation event in the async task. {:?}", e);
+ if let Ok(()) = wait_for_boot_completed() {
+ if let Err(e) = key_operation_event_stats.stats_write() {
+ log::error!("Error in logging key operation event in the async task. {:?}", e);
+ }
}
});
}
@@ -383,21 +424,17 @@
}
/// Registers pull metrics callbacks
-pub fn register_pull_metrics_callbacks() -> Result<()> {
+pub fn register_pull_metrics_callbacks() {
// Before registering the callbacks with statsd, we have to wait for the system to finish
// booting up. This avoids possible races that may occur at startup. For example, statsd
// depends on a companion service, and if registration happens too soon it will fail since
// the companion service isn't up yet.
- let mut watcher = PropertyWatcher::new("sys.boot_completed")?;
- loop {
- watcher.wait()?;
- let value = watcher.read(|_name, value| Ok(value.trim().to_string()));
- if value? == "1" {
+ LOGS_HANDLER.queue_lo(move |_| {
+ if let Ok(()) = wait_for_boot_completed() {
set_pull_atom_callback(Atoms::Keystore2StorageStats, None, pull_metrics_callback);
- break;
+ log::info!("Pull metrics callbacks successfully registered.")
}
- }
- Ok(())
+ });
}
fn pull_metrics_callback() -> StatsPullResult {
diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index d10aba0..00b26e4 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -15,7 +15,9 @@
//! This crate implements the IKeystoreSecurityLevel interface.
use crate::attestation_key_utils::{get_attest_key_info, AttestationKeyInfo};
-use crate::audit_log::{log_key_deleted, log_key_generated, log_key_imported};
+use crate::audit_log::{
+ log_key_deleted, log_key_generated, log_key_imported, log_key_integrity_violation,
+};
use crate::database::{CertificateInfo, KeyIdGuard};
use crate::error::{self, map_km_error, map_or_log_err, Error, ErrorCode};
use crate::globals::{DB, ENFORCEMENTS, LEGACY_MIGRATOR, SUPER_KEY};
@@ -325,6 +327,18 @@
self.operation_db.prune(caller_uid, forced)?;
continue;
}
+ v @ Err(Error::Km(ErrorCode::INVALID_KEY_BLOB)) => {
+ if let Some((key_id, _)) = key_properties {
+ if let Ok(Some(key)) =
+ DB.with(|db| db.borrow_mut().load_key_descriptor(key_id))
+ {
+ log_key_integrity_violation(&key);
+ } else {
+ log::error!("Failed to load key descriptor for audit log");
+ }
+ }
+ return v;
+ }
v => return v,
}
},