Merge changes I5b5018d4,I688fff83,Ib99d689d
* changes:
On-device signing: Remove Keymaster implementation.
On-device signing: verify the public key.
On-device signing: Switch to using a TEE-backed keystore key.
diff --git a/keystore/keystore_cli_v2.cpp b/keystore/keystore_cli_v2.cpp
index 6e45ee2..43f72a9 100644
--- a/keystore/keystore_cli_v2.cpp
+++ b/keystore/keystore_cli_v2.cpp
@@ -56,7 +56,7 @@
keymint::AuthorizationSet parameters;
};
-constexpr const char keystore2_service_name[] = "android.system.keystore2";
+constexpr const char keystore2_service_name[] = "android.system.keystore2.IKeystoreService/default";
int unwrapError(const ndk::ScopedAStatus& status) {
if (status.isOk()) return 0;
@@ -769,7 +769,7 @@
sec_level->generateKey(keyDescriptor(name), {} /* attestationKey */, params.vector_data(),
0 /* flags */, {} /* entropy */, &keyMetadata);
- if (rc.isOk()) {
+ if (!rc.isOk()) {
std::cerr << "GenerateKey failed: " << rc.getDescription() << std::endl;
return unwrapError(rc);
}
diff --git a/keystore/tests/Android.bp b/keystore/tests/Android.bp
index 249cb77..39601eb 100644
--- a/keystore/tests/Android.bp
+++ b/keystore/tests/Android.bp
@@ -62,9 +62,9 @@
"libgtest_main",
"libutils",
"liblog",
+ "android.security.apc-ndk_platform",
],
shared_libs: [
- "android.security.apc-ndk_platform",
"libbinder_ndk",
],
sanitize: {
diff --git a/keystore2/Android.bp b/keystore2/Android.bp
index 0ba49ed..32493c0 100644
--- a/keystore2/Android.bp
+++ b/keystore2/Android.bp
@@ -145,4 +145,6 @@
],
vintf_fragments: ["android.system.keystore2-service.xml"],
+
+ required: ["keystore_cli_v2"],
}
diff --git a/keystore2/selinux/Android.bp b/keystore2/selinux/Android.bp
index 18063d3..0810855 100644
--- a/keystore2/selinux/Android.bp
+++ b/keystore2/selinux/Android.bp
@@ -34,6 +34,7 @@
rustlibs: [
"libanyhow",
+ "liblazy_static",
"liblog_rust",
"libselinux_bindgen",
"libthiserror",
@@ -56,8 +57,30 @@
rustlibs: [
"libandroid_logger",
"libanyhow",
+ "liblazy_static",
"liblog_rust",
"libselinux_bindgen",
"libthiserror",
],
}
+
+rust_test {
+ name: "keystore2_selinux_concurrency_test",
+ srcs: [
+ "src/concurrency_test.rs",
+ ],
+ crate_name: "keystore2_selinux_concurrency_test",
+ test_suites: ["deneral-tests"],
+ auto_gen_config: true,
+
+ rustlibs: [
+ "libandroid_logger",
+ "libanyhow",
+ "libkeystore2_selinux",
+ "liblazy_static",
+ "liblog_rust",
+ "libnix",
+ "libnum_cpus",
+ "libthiserror",
+ ],
+}
diff --git a/keystore2/selinux/src/concurrency_test.rs b/keystore2/selinux/src/concurrency_test.rs
new file mode 100644
index 0000000..a5d2df2
--- /dev/null
+++ b/keystore2/selinux/src/concurrency_test.rs
@@ -0,0 +1,190 @@
+// Copyright 2021, The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use keystore2_selinux::{check_access, Context};
+use nix::sched::sched_setaffinity;
+use nix::sched::CpuSet;
+use nix::unistd::getpid;
+use std::thread;
+use std::{
+ sync::{atomic::AtomicU8, atomic::Ordering, Arc},
+ time::{Duration, Instant},
+};
+
+#[derive(Clone, Copy)]
+struct CatCount(u8, u8, u8, u8);
+
+impl CatCount {
+ fn next(&mut self) -> CatCount {
+ let result = *self;
+ if self.3 == 255 {
+ if self.2 == 254 {
+ if self.1 == 253 {
+ if self.0 == 252 {
+ self.0 = 255;
+ }
+ self.0 += 1;
+ self.1 = self.0;
+ }
+ self.1 += 1;
+ self.2 = self.1;
+ }
+ self.2 += 1;
+ self.3 = self.2;
+ }
+ self.3 += 1;
+ result
+ }
+
+ fn make_string(&self) -> String {
+ format!("c{},c{},c{},c{}", self.0, self.1, self.2, self.3)
+ }
+}
+
+impl Default for CatCount {
+ fn default() -> Self {
+ Self(0, 1, 2, 3)
+ }
+}
+
+/// This test calls selinux_check_access concurrently causing access vector cache misses
+/// in libselinux avc. The test then checks if any of the threads fails to report back
+/// after a burst of access checks. The purpose of the test is to draw out a specific
+/// access vector cache corruption that sends a calling thread into an infinite loop.
+/// This was observed when keystore2 used libselinux concurrently in a non thread safe
+/// way. See b/184006658.
+#[test]
+fn test_concurrent_check_access() {
+ android_logger::init_once(
+ android_logger::Config::default()
+ .with_tag("keystore2_selinux_concurrency_test")
+ .with_min_level(log::Level::Debug),
+ );
+
+ let cpus = num_cpus::get();
+ let turnpike = Arc::new(AtomicU8::new(0));
+ let complete_count = Arc::new(AtomicU8::new(0));
+ let mut threads: Vec<thread::JoinHandle<()>> = Vec::new();
+
+ for i in 0..cpus {
+ log::info!("Spawning thread {}", i);
+ let turnpike_clone = turnpike.clone();
+ let complete_count_clone = complete_count.clone();
+ threads.push(thread::spawn(move || {
+ let mut cpu_set = CpuSet::new();
+ cpu_set.set(i).unwrap();
+ sched_setaffinity(getpid(), &cpu_set).unwrap();
+ let mut cat_count: CatCount = Default::default();
+
+ log::info!("Thread 0 reached turnpike");
+ loop {
+ turnpike_clone.fetch_add(1, Ordering::Relaxed);
+ loop {
+ match turnpike_clone.load(Ordering::Relaxed) {
+ 0 => break,
+ 255 => return,
+ _ => {}
+ }
+ }
+
+ for _ in 0..250 {
+ let (tctx, sctx, perm, class) = (
+ Context::new("u:object_r:keystore:s0").unwrap(),
+ Context::new(&format!(
+ "u:r:untrusted_app:s0:{}",
+ cat_count.next().make_string()
+ ))
+ .unwrap(),
+ "use",
+ "keystore2_key",
+ );
+
+ check_access(&sctx, &tctx, class, perm).unwrap();
+ }
+
+ complete_count_clone.fetch_add(1, Ordering::Relaxed);
+ while complete_count_clone.load(Ordering::Relaxed) as usize != cpus {
+ thread::sleep(Duration::from_millis(5));
+ }
+ }
+ }));
+ }
+
+ let mut i = 0;
+ let run_time = Instant::now();
+
+ loop {
+ const TEST_ITERATIONS: u32 = 500;
+ const MAX_SLEEPS: u64 = 500;
+ const SLEEP_MILLISECONDS: u64 = 5;
+ let mut sleep_count: u64 = 0;
+ while turnpike.load(Ordering::Relaxed) as usize != cpus {
+ thread::sleep(Duration::from_millis(SLEEP_MILLISECONDS));
+ sleep_count += 1;
+ assert!(
+ sleep_count < MAX_SLEEPS,
+ "Waited too long to go ready on iteration {}, only {} are ready",
+ i,
+ turnpike.load(Ordering::Relaxed)
+ );
+ }
+
+ if i % 100 == 0 {
+ let elapsed = run_time.elapsed().as_secs();
+ println!("{:02}:{:02}: Iteration {}", elapsed / 60, elapsed % 60, i);
+ }
+
+ // Give the threads some time to reach and spin on the turn pike.
+ assert_eq!(turnpike.load(Ordering::Relaxed) as usize, cpus, "i = {}", i);
+ if i >= TEST_ITERATIONS {
+ turnpike.store(255, Ordering::Relaxed);
+ break;
+ }
+
+ // Now go.
+ complete_count.store(0, Ordering::Relaxed);
+ turnpike.store(0, Ordering::Relaxed);
+ i += 1;
+
+ // Wait for them to all complete.
+ sleep_count = 0;
+ while complete_count.load(Ordering::Relaxed) as usize != cpus {
+ thread::sleep(Duration::from_millis(SLEEP_MILLISECONDS));
+ sleep_count += 1;
+ if sleep_count >= MAX_SLEEPS {
+ // Enable the following block to park the thread to allow attaching a debugger.
+ if false {
+ println!(
+ "Waited {} seconds and we seem stuck. Going to sleep forever.",
+ (MAX_SLEEPS * SLEEP_MILLISECONDS) as f32 / 1000.0
+ );
+ loop {
+ thread::park();
+ }
+ } else {
+ assert!(
+ sleep_count < MAX_SLEEPS,
+ "Waited too long to complete on iteration {}, only {} are complete",
+ i,
+ complete_count.load(Ordering::Relaxed)
+ );
+ }
+ }
+ }
+ }
+
+ for t in threads {
+ t.join().unwrap();
+ }
+}
diff --git a/keystore2/selinux/src/lib.rs b/keystore2/selinux/src/lib.rs
index cc707e7..5197cf6 100644
--- a/keystore2/selinux/src/lib.rs
+++ b/keystore2/selinux/src/lib.rs
@@ -20,6 +20,13 @@
//! * selabel_lookup for the keystore2_key backend.
//! And it provides an owning wrapper around context strings `Context`.
+use anyhow::Context as AnyhowContext;
+use anyhow::{anyhow, Result};
+use lazy_static::lazy_static;
+pub use selinux::pid_t;
+use selinux::SELABEL_CTX_ANDROID_KEYSTORE2_KEY;
+use selinux::SELINUX_CB_LOG;
+use selinux_bindgen as selinux;
use std::ffi::{CStr, CString};
use std::fmt;
use std::io;
@@ -29,18 +36,18 @@
use std::ptr;
use std::sync;
-use selinux_bindgen as selinux;
-
-use anyhow::Context as AnyhowContext;
-use anyhow::{anyhow, Result};
-
-use selinux::SELABEL_CTX_ANDROID_KEYSTORE2_KEY;
-use selinux::SELINUX_CB_LOG;
-
-pub use selinux::pid_t;
-
static SELINUX_LOG_INIT: sync::Once = sync::Once::new();
+lazy_static! {
+ /// `selinux_check_access` is only thread safe if avc_init was called with lock callbacks.
+ /// However, avc_init is deprecated and not exported by androids version of libselinux.
+ /// `selinux_set_callbacks` does not allow setting lock callbacks. So the only option
+ /// that remains right now is to put a big lock around calls into libselinux.
+ /// TODO b/188079221 It should suffice to protect `selinux_check_access` but until we are
+ /// certain of that, we leave the extra locks in place
+ static ref LIB_SELINUX_LOCK: sync::Mutex<()> = Default::default();
+}
+
fn redirect_selinux_logs_to_logcat() {
// `selinux_set_callback` assigns the static lifetime function pointer
// `selinux_log_callback` to a static lifetime variable.
@@ -164,6 +171,8 @@
/// `selinux_android_keystore2_key_context_handle`.
pub fn new() -> Result<Self> {
init_logger_once();
+ let _lock = LIB_SELINUX_LOCK.lock().unwrap();
+
let handle = unsafe { selinux::selinux_android_keystore2_key_context_handle() };
if handle.is_null() {
return Err(anyhow!(Error::sys("Failed to open KeystoreKeyBackend")));
@@ -192,6 +201,8 @@
match unsafe {
// No need to initialize the logger here because it cannot run unless
// KeystoreKeyBackend::new has run.
+ let _lock = LIB_SELINUX_LOCK.lock().unwrap();
+
selinux::selabel_lookup(self.handle, &mut con, c_key.as_ptr(), Self::BACKEND_TYPE)
} {
0 => {
@@ -219,6 +230,8 @@
/// * Err(io::Error::last_os_error()) if getcon failed.
pub fn getcon() -> Result<Context> {
init_logger_once();
+ let _lock = LIB_SELINUX_LOCK.lock().unwrap();
+
let mut con: *mut c_char = ptr::null_mut();
match unsafe { selinux::getcon(&mut con) } {
0 => {
@@ -241,6 +254,8 @@
/// * Err(io::Error::last_os_error()) if getpidcon failed.
pub fn getpidcon(pid: selinux::pid_t) -> Result<Context> {
init_logger_once();
+ let _lock = LIB_SELINUX_LOCK.lock().unwrap();
+
let mut con: *mut c_char = ptr::null_mut();
match unsafe { selinux::getpidcon(pid, &mut con) } {
0 => {
@@ -267,6 +282,7 @@
/// the access check.
pub fn check_access(source: &CStr, target: &CStr, tclass: &str, perm: &str) -> Result<()> {
init_logger_once();
+
let c_tclass = CString::new(tclass).with_context(|| {
format!("check_access: Failed to convert tclass \"{}\" to CString.", tclass)
})?;
@@ -275,6 +291,8 @@
})?;
match unsafe {
+ let _lock = LIB_SELINUX_LOCK.lock().unwrap();
+
selinux::selinux_check_access(
source.as_ptr(),
target.as_ptr(),
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 4ab4258..2930162 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -46,7 +46,7 @@
use crate::impl_metadata; // This is in db_utils.rs
use crate::key_parameter::{KeyParameter, Tag};
use crate::permission::KeyPermSet;
-use crate::utils::{get_current_time_in_seconds, watchdog as wd, AID_USER_OFFSET};
+use crate::utils::{get_current_time_in_milliseconds, watchdog as wd, AID_USER_OFFSET};
use crate::{
db_utils::{self, SqlField},
gc::Gc,
@@ -737,29 +737,24 @@
}
/// Database representation of the monotonic time retrieved from the system call clock_gettime with
-/// CLOCK_MONOTONIC_RAW. Stores monotonic time as i64 in seconds.
+/// CLOCK_MONOTONIC_RAW. Stores monotonic time as i64 in milliseconds.
#[derive(Debug, Copy, Clone, Default, Eq, PartialEq, Ord, PartialOrd)]
pub struct MonotonicRawTime(i64);
impl MonotonicRawTime {
/// Constructs a new MonotonicRawTime
pub fn now() -> Self {
- Self(get_current_time_in_seconds())
+ Self(get_current_time_in_milliseconds())
}
- /// Constructs a new MonotonicRawTime from a given number of seconds.
- pub fn from_secs(val: i64) -> Self {
- Self(val)
+ /// Returns the value of MonotonicRawTime in milliseconds as i64
+ pub fn milliseconds(&self) -> i64 {
+ self.0
}
/// Returns the integer value of MonotonicRawTime as i64
pub fn seconds(&self) -> i64 {
- self.0
- }
-
- /// Returns the value of MonotonicRawTime in milli seconds as i64
- pub fn milli_seconds(&self) -> i64 {
- self.0 * 1000
+ self.0 / 1000
}
/// Like i64::checked_sub.
@@ -785,6 +780,7 @@
#[derive(Clone)]
pub struct AuthTokenEntry {
auth_token: HardwareAuthToken,
+ // Time received in milliseconds
time_received: MonotonicRawTime,
}
@@ -833,6 +829,20 @@
/// Name of the file that holds the cross-boot persistent database.
pub const PERSISTENT_DB_FILENAME: &'static str = &"persistent.sqlite";
+ /// Set write-ahead logging mode on the persistent database found in `db_root`.
+ pub fn set_wal_mode(db_root: &Path) -> Result<()> {
+ let path = Self::make_persistent_path(&db_root)?;
+ let conn =
+ Connection::open(path).context("In KeystoreDB::set_wal_mode: Failed to open DB")?;
+ let mode: String = conn
+ .pragma_update_and_check(None, "journal_mode", &"WAL", |row| row.get(0))
+ .context("In KeystoreDB::set_wal_mode: Failed to set journal_mode")?;
+ match mode.as_str() {
+ "wal" => Ok(()),
+ _ => Err(anyhow!("Unable to set WAL mode, db is still in {} mode.", mode)),
+ }
+ }
+
/// This will create a new database connection connecting the two
/// files persistent.sqlite and perboot.sqlite in the given directory.
/// It also attempts to initialize all of the tables.
@@ -841,18 +851,8 @@
pub fn new(db_root: &Path, gc: Option<Arc<Gc>>) -> Result<Self> {
let _wp = wd::watch_millis("KeystoreDB::new", 500);
- // Build the path to the sqlite file.
- let mut persistent_path = db_root.to_path_buf();
- persistent_path.push(Self::PERSISTENT_DB_FILENAME);
-
- // Now convert them to strings prefixed with "file:"
- let mut persistent_path_str = "file:".to_owned();
- persistent_path_str.push_str(&persistent_path.to_string_lossy());
-
- let conn = Self::make_connection(&persistent_path_str)?;
-
- // On busy fail Immediately. It is unlikely to succeed given a bug in sqlite.
- conn.busy_handler(None).context("In KeystoreDB::new: Failed to set busy handler.")?;
+ let persistent_path = Self::make_persistent_path(&db_root)?;
+ let conn = Self::make_connection(&persistent_path)?;
let mut db = Self { conn, gc, perboot: perboot::PERBOOT_DB.clone() };
db.with_transaction(TransactionBehavior::Immediate, |tx| {
@@ -971,6 +971,18 @@
Ok(())
}
+ fn make_persistent_path(db_root: &Path) -> Result<String> {
+ // Build the path to the sqlite file.
+ let mut persistent_path = db_root.to_path_buf();
+ persistent_path.push(Self::PERSISTENT_DB_FILENAME);
+
+ // Now convert them to strings prefixed with "file:"
+ let mut persistent_path_str = "file:".to_owned();
+ persistent_path_str.push_str(&persistent_path.to_string_lossy());
+
+ Ok(persistent_path_str)
+ }
+
fn make_connection(persistent_file: &str) -> Result<Connection> {
let conn =
Connection::open_in_memory().context("Failed to initialize SQLite connection.")?;
@@ -1413,18 +1425,6 @@
.context("In get_or_create_key_with.")
}
- /// SQLite3 seems to hold a shared mutex while running the busy handler when
- /// waiting for the database file to become available. This makes it
- /// impossible to successfully recover from a locked database when the
- /// transaction holding the device busy is in the same process on a
- /// different connection. As a result the busy handler has to time out and
- /// fail in order to make progress.
- ///
- /// Instead, we set the busy handler to None (return immediately). And catch
- /// Busy and Locked errors (the latter occur on in memory databases with
- /// shared cache, e.g., the per-boot database.) and restart the transaction
- /// after a grace period of half a millisecond.
- ///
/// Creates a transaction with the given behavior and executes f with the new transaction.
/// The transaction is committed only if f returns Ok and retried if DatabaseBusy
/// or DatabaseLocked is encountered.
@@ -2395,11 +2395,12 @@
let mut stmt = tx
.prepare(
"SELECT keyentryid, access_vector FROM persistent.grant
- WHERE grantee = ? AND id = ?;",
+ WHERE grantee = ? AND id = ? AND
+ (SELECT state FROM persistent.keyentry WHERE id = keyentryid) = ?;",
)
.context("Domain::GRANT prepare statement failed")?;
let mut rows = stmt
- .query(params![caller_uid as i64, key.nspace])
+ .query(params![caller_uid as i64, key.nspace, KeyLifeCycle::Live])
.context("Domain:Grant: query failed.")?;
let (key_id, access_vector): (i64, i32) =
db_utils::with_rows_extract_one(&mut rows, |row| {
@@ -2966,19 +2967,28 @@
/// Returns a list of KeyDescriptors in the selected domain/namespace.
/// The key descriptors will have the domain, nspace, and alias field set.
/// Domain must be APP or SELINUX, the caller must make sure of that.
- pub fn list(&mut self, domain: Domain, namespace: i64) -> Result<Vec<KeyDescriptor>> {
+ pub fn list(
+ &mut self,
+ domain: Domain,
+ namespace: i64,
+ key_type: KeyType,
+ ) -> Result<Vec<KeyDescriptor>> {
let _wp = wd::watch_millis("KeystoreDB::list", 500);
self.with_transaction(TransactionBehavior::Deferred, |tx| {
let mut stmt = tx
.prepare(
"SELECT alias FROM persistent.keyentry
- WHERE domain = ? AND namespace = ? AND alias IS NOT NULL AND state = ?;",
+ WHERE domain = ?
+ AND namespace = ?
+ AND alias IS NOT NULL
+ AND state = ?
+ AND key_type = ?;",
)
.context("In list: Failed to prepare.")?;
let mut rows = stmt
- .query(params![domain.0 as u32, namespace, KeyLifeCycle::Live])
+ .query(params![domain.0 as u32, namespace, KeyLifeCycle::Live, key_type])
.context("In list: Failed to query.")?;
let mut descriptors: Vec<KeyDescriptor> = Vec::new();
@@ -3158,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)]
@@ -3179,6 +3213,7 @@
use android_hardware_security_secureclock::aidl::android::hardware::security::secureclock::{
Timestamp::Timestamp,
};
+ use rusqlite::DatabaseName::Attached;
use rusqlite::NO_PARAMS;
use rusqlite::TransactionBehavior;
use std::cell::RefCell;
@@ -4721,7 +4756,7 @@
})
.collect();
list_o_descriptors.sort();
- let mut list_result = db.list(*domain, *namespace)?;
+ let mut list_result = db.list(*domain, *namespace, KeyType::Client)?;
list_result.sort();
assert_eq!(list_o_descriptors, list_result);
@@ -4751,7 +4786,7 @@
loaded_entries.sort_unstable();
assert_eq!(list_o_ids, loaded_entries);
}
- assert_eq!(Vec::<KeyDescriptor>::new(), db.list(Domain::SELINUX, 101)?);
+ assert_eq!(Vec::<KeyDescriptor>::new(), db.list(Domain::SELINUX, 101, KeyType::Client)?);
Ok(())
}
@@ -5201,7 +5236,7 @@
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.seconds() < last_off_body_2.seconds());
+ assert!(last_off_body_1 < last_off_body_2);
Ok(())
}
@@ -5214,11 +5249,11 @@
make_test_key_entry(&mut db, Domain::APP, 110000, TEST_ALIAS, None)?;
db.unbind_keys_for_user(2, false)?;
- assert_eq!(1, db.list(Domain::APP, 110000)?.len());
- assert_eq!(0, db.list(Domain::APP, 210000)?.len());
+ assert_eq!(1, db.list(Domain::APP, 110000, KeyType::Client)?.len());
+ assert_eq!(0, db.list(Domain::APP, 210000, KeyType::Client)?.len());
db.unbind_keys_for_user(1, true)?;
- assert_eq!(0, db.list(Domain::APP, 110000)?.len());
+ assert_eq!(0, db.list(Domain::APP, 110000, KeyType::Client)?.len());
Ok(())
}
@@ -5487,4 +5522,42 @@
assert_eq!(db.find_auth_token_entry(|_| true).unwrap().0.auth_token.mac, b"mac2".to_vec());
Ok(())
}
+
+ #[test]
+ fn test_set_wal_mode() -> Result<()> {
+ let temp_dir = TempDir::new("test_set_wal_mode")?;
+ let mut db = KeystoreDB::new(temp_dir.path(), None)?;
+ let mode: String =
+ db.conn.pragma_query_value(Some(Attached("persistent")), "journal_mode", |row| {
+ row.get(0)
+ })?;
+ assert_eq!(mode, "delete");
+ db.conn.close().expect("Close didn't work");
+
+ KeystoreDB::set_wal_mode(temp_dir.path())?;
+
+ db = KeystoreDB::new(temp_dir.path(), None)?;
+ let mode: String =
+ db.conn.pragma_query_value(Some(Attached("persistent")), "journal_mode", |row| {
+ row.get(0)
+ })?;
+ 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/enforcements.rs b/keystore2/src/enforcements.rs
index 8bf2090..29a3f0b 100644
--- a/keystore2/src/enforcements.rs
+++ b/keystore2/src/enforcements.rs
@@ -824,12 +824,12 @@
} else {
// Filter the matching auth tokens by age.
if auth_token_max_age_millis != 0 {
- let now_in_millis = MonotonicRawTime::now().milli_seconds();
+ let now_in_millis = MonotonicRawTime::now();
let result = Self::find_auth_token(|auth_token_entry: &AuthTokenEntry| {
let token_valid = now_in_millis
- .checked_sub(auth_token_entry.time_received().milli_seconds())
+ .checked_sub(&auth_token_entry.time_received())
.map_or(false, |token_age_in_millis| {
- auth_token_max_age_millis > token_age_in_millis
+ auth_token_max_age_millis > token_age_in_millis.milliseconds()
});
token_valid && auth_token_entry.satisfies(&sids, auth_type)
});
diff --git a/keystore2/src/globals.rs b/keystore2/src/globals.rs
index 8d39b22..89114a6 100644
--- a/keystore2/src/globals.rs
+++ b/keystore2/src/globals.rs
@@ -39,11 +39,12 @@
use binder::FromIBinder;
use keystore2_vintf::get_aidl_instances;
use lazy_static::lazy_static;
-use std::sync::{Arc, Mutex};
+use std::sync::{Arc, Mutex, RwLock};
use std::{cell::RefCell, sync::Once};
use std::{collections::HashMap, path::Path, path::PathBuf};
static DB_INIT: Once = Once::new();
+static DB_SET_WAL_MODE: Once = Once::new();
/// Open a connection to the Keystore 2.0 database. This is called during the initialization of
/// the thread local DB field. It should never be called directly. The first time this is called
@@ -54,11 +55,16 @@
/// is run only once, as long as the ASYNC_TASK instance is the same. So only one additional
/// database connection is created for the garbage collector worker.
pub fn create_thread_local_db() -> KeystoreDB {
- let mut db = KeystoreDB::new(
- &DB_PATH.lock().expect("Could not get the database directory."),
- Some(GC.clone()),
- )
- .expect("Failed to open database.");
+ let db_path = DB_PATH.read().expect("Could not get the database directory.");
+
+ DB_SET_WAL_MODE.call_once(|| {
+ log::info!("Setting Keystore 2.0 database to WAL mode first time since boot.");
+ KeystoreDB::set_wal_mode(&db_path)
+ .expect("In create_thread_local_db: Could not set WAL mode.");
+ });
+
+ let mut db = KeystoreDB::new(&db_path, Some(GC.clone())).expect("Failed to open database.");
+
DB_INIT.call_once(|| {
log::info!("Touching Keystore 2.0 database for this first time since boot.");
db.insert_last_off_body(MonotonicRawTime::now());
@@ -139,7 +145,7 @@
lazy_static! {
/// The path where keystore stores all its keys.
- pub static ref DB_PATH: Mutex<PathBuf> = Mutex::new(
+ pub static ref DB_PATH: RwLock<PathBuf> = RwLock::new(
Path::new("/data/misc/keystore").to_path_buf());
/// Runtime database of unwrapped super keys.
pub static ref SUPER_KEY: Arc<SuperKeyManager> = Default::default();
@@ -157,7 +163,7 @@
/// LegacyBlobLoader is initialized and exists globally.
/// The same directory used by the database is used by the LegacyBlobLoader as well.
pub static ref LEGACY_BLOB_LOADER: Arc<LegacyBlobLoader> = Arc::new(LegacyBlobLoader::new(
- &DB_PATH.lock().expect("Could not get the database path for legacy blob loader.")));
+ &DB_PATH.read().expect("Could not get the database path for legacy blob loader.")));
/// Legacy migrator. Atomically migrates legacy blobs to the database.
pub static ref LEGACY_MIGRATOR: Arc<LegacyMigrator> =
Arc::new(LegacyMigrator::new(Arc::new(Default::default())));
@@ -173,7 +179,7 @@
map_km_error(km_dev.deleteKey(&*blob))
.context("In invalidate key closure: Trying to invalidate key blob.")
}),
- KeystoreDB::new(&DB_PATH.lock().expect("Could not get the database directory."), None)
+ KeystoreDB::new(&DB_PATH.read().expect("Could not get the database directory."), None)
.expect("Failed to open database."),
SUPER_KEY.clone(),
)
diff --git a/keystore2/src/keystore2_main.rs b/keystore2/src/keystore2_main.rs
index 29330a6..53461da 100644
--- a/keystore2/src/keystore2_main.rs
+++ b/keystore2/src/keystore2_main.rs
@@ -56,7 +56,7 @@
// For the ground truth check the service startup rule for init (typically in keystore2.rc).
let id_rotation_state = if let Some(dir) = args.next() {
let db_path = Path::new(&dir);
- *keystore2::globals::DB_PATH.lock().expect("Could not lock DB_PATH.") =
+ *keystore2::globals::DB_PATH.write().expect("Could not lock DB_PATH.") =
db_path.to_path_buf();
IdRotationState::new(&db_path)
} else {
@@ -121,7 +121,7 @@
}
let vpnprofilestore = VpnProfileStore::new_native_binder(
- &keystore2::globals::DB_PATH.lock().expect("Could not get DB_PATH."),
+ &keystore2::globals::DB_PATH.read().expect("Could not get DB_PATH."),
);
binder::add_service(VPNPROFILESTORE_SERVICE_NAME, vpnprofilestore.as_binder()).unwrap_or_else(
|e| {
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,
}
},
diff --git a/keystore2/src/service.rs b/keystore2/src/service.rs
index 3ce0550..1f61729 100644
--- a/keystore2/src/service.rs
+++ b/keystore2/src/service.rs
@@ -291,7 +291,7 @@
&mut DB
.with(|db| {
let mut db = db.borrow_mut();
- db.list(k.domain, k.nspace)
+ db.list(k.domain, k.nspace, KeyType::Client)
})
.context("In list_entries: Trying to list keystore database.")?,
);
diff --git a/keystore2/src/utils.rs b/keystore2/src/utils.rs
index 10865ae..a110c64 100644
--- a/keystore2/src/utils.rs
+++ b/keystore2/src/utils.rs
@@ -185,18 +185,15 @@
parameters.into_iter().map(|p| p.into_authorization()).collect()
}
-/// This returns the current time (in seconds) as an instance of a monotonic clock, by invoking the
-/// system call since Rust does not support getting monotonic time instance as an integer.
-pub fn get_current_time_in_seconds() -> i64 {
+/// This returns the current time (in milliseconds) as an instance of a monotonic clock,
+/// by invoking the system call since Rust does not support getting monotonic time instance
+/// as an integer.
+pub fn get_current_time_in_milliseconds() -> i64 {
let mut current_time = libc::timespec { tv_sec: 0, tv_nsec: 0 };
// Following unsafe block includes one system call to get monotonic time.
// Therefore, it is not considered harmful.
unsafe { libc::clock_gettime(libc::CLOCK_MONOTONIC_RAW, &mut current_time) };
- // It is safe to unwrap here because try_from() returns std::convert::Infallible, which is
- // defined to be an error that can never happen (i.e. the result is always ok).
- // This suppresses the compiler's complaint about converting tv_sec to i64 in method
- // get_current_time_in_seconds.
- current_time.tv_sec as i64
+ current_time.tv_sec as i64 * 1000 + (current_time.tv_nsec as i64 / 1_000_000)
}
/// Converts a response code as returned by the Android Protected Confirmation HIDL compatibility
diff --git a/keystore2/vpnprofilestore/lib.rs b/keystore2/vpnprofilestore/lib.rs
index 8b3bc2b..df2731a 100644
--- a/keystore2/vpnprofilestore/lib.rs
+++ b/keystore2/vpnprofilestore/lib.rs
@@ -22,7 +22,7 @@
BinderFeatures, ExceptionCode, Result as BinderResult, Status as BinderStatus, Strong,
ThreadState,
};
-use anyhow::{Context, Result};
+use anyhow::{anyhow, Context, Result};
use keystore2::{async_task::AsyncTask, legacy_blob::LegacyBlobLoader, utils::watchdog as wd};
use rusqlite::{
params, Connection, OptionalExtension, Transaction, TransactionBehavior, NO_PARAMS,
@@ -30,25 +30,42 @@
use std::{
collections::HashSet,
path::{Path, PathBuf},
+ sync::Once,
};
+static DB_SET_WAL_MODE: Once = Once::new();
+
struct DB {
conn: Connection,
}
impl DB {
fn new(db_file: &Path) -> Result<Self> {
+ DB_SET_WAL_MODE.call_once(|| {
+ log::info!("Setting VpnProfileStore database to WAL mode first time since boot.");
+ Self::set_wal_mode(&db_file).expect("In vpnprofilestore: Could not set WAL mode.");
+ });
+
let mut db = Self {
conn: Connection::open(db_file).context("Failed to initialize SQLite connection.")?,
};
- // On busy fail Immediately. It is unlikely to succeed given a bug in sqlite.
- db.conn.busy_handler(None).context("Failed to set busy handler.")?;
-
db.init_tables().context("Trying to initialize vpnstore db.")?;
Ok(db)
}
+ fn set_wal_mode(db_file: &Path) -> Result<()> {
+ let conn = Connection::open(db_file)
+ .context("In VpnProfileStore set_wal_mode: Failed to open DB.")?;
+ let mode: String = conn
+ .pragma_update_and_check(None, "journal_mode", &"WAL", |row| row.get(0))
+ .context("In VpnProfileStore set_wal_mode: Failed to set journal_mode")?;
+ match mode.as_str() {
+ "wal" => Ok(()),
+ _ => Err(anyhow!("Unable to set WAL mode, db is still in {} mode.", mode)),
+ }
+ }
+
fn with_transaction<T, F>(&mut self, behavior: TransactionBehavior, f: F) -> Result<T>
where
F: Fn(&Transaction) -> Result<T>,
@@ -470,6 +487,9 @@
const PROFILE_COUNT: u32 = 5000u32;
const PROFILE_DB_COUNT: u32 = 5000u32;
+ let mode: String = db.conn.pragma_query_value(None, "journal_mode", |row| row.get(0))?;
+ assert_eq!(mode, "wal");
+
let mut actual_profile_count = PROFILE_COUNT;
// First insert PROFILE_COUNT profiles.
for count in 0..PROFILE_COUNT {