Merge "Add an option to allow/disallow degenerate DICE chains" into main
diff --git a/keystore-engine/Android.bp b/keystore-engine/Android.bp
index 7fbfe53..d763445 100644
--- a/keystore-engine/Android.bp
+++ b/keystore-engine/Android.bp
@@ -27,7 +27,8 @@
name: "libkeystore-engine",
defaults: [
- "keystore2_use_latest_aidl_ndk_shared",
+ "keymint_use_latest_hal_aidl_ndk_static",
+ "keystore2_use_latest_aidl_ndk_static",
],
srcs: [
"android_engine.cpp",
@@ -41,7 +42,6 @@
],
shared_libs: [
- "android.system.keystore2-V4-ndk",
"libbinder_ndk",
"libcrypto",
"libcutils",
diff --git a/keystore2/Android.bp b/keystore2/Android.bp
index c482e84..5726078 100644
--- a/keystore2/Android.bp
+++ b/keystore2/Android.bp
@@ -56,7 +56,6 @@
"libkeystore2_hal_names_rust",
"libkeystore2_km_compat",
"libkeystore2_selinux",
- "liblazy_static",
"liblibc",
"liblog_rust",
"libmessage_macro",
diff --git a/keystore2/apc_compat/apc_compat.cpp b/keystore2/apc_compat/apc_compat.cpp
index ffe7595..0ff2fc5 100644
--- a/keystore2/apc_compat/apc_compat.cpp
+++ b/keystore2/apc_compat/apc_compat.cpp
@@ -26,6 +26,7 @@
#include <android/binder_manager.h>
#include <memory>
+#include <set>
#include <string>
#include <thread>
#include <vector>
@@ -214,6 +215,14 @@
return nullptr;
}
+ class DeathRecipientCookie {
+ public:
+ DeathRecipientCookie(std::weak_ptr<ConfuiAidlCompatSession> session)
+ : mAidlSession(session) {}
+ DeathRecipientCookie() = delete;
+ std::weak_ptr<ConfuiAidlCompatSession> mAidlSession;
+ };
+
uint32_t promptUserConfirmation(ApcCompatCallback callback, const char* prompt_text,
const uint8_t* extra_data, size_t extra_data_size,
const char* locale, ApcCompatUiOptions ui_options) {
@@ -234,12 +243,19 @@
if (!aidlService_) {
return APC_COMPAT_ERROR_SYSTEM_ERROR;
}
- auto linkRet =
- AIBinder_linkToDeath(aidlService_->asBinder().get(), death_recipient_.get(), this);
- if (linkRet != STATUS_OK) {
- LOG(ERROR) << "Communication error: promptUserConfirmation: "
- "Trying to register death recipient: ";
- return APC_COMPAT_ERROR_SYSTEM_ERROR;
+
+ {
+ auto cookieLock = std::lock_guard(deathRecipientCookie_lock_);
+ void* cookie = new DeathRecipientCookie(this->ref<ConfuiAidlCompatSession>());
+ auto linkRet = AIBinder_linkToDeath(aidlService_->asBinder().get(),
+ death_recipient_.get(), cookie);
+ if (linkRet != STATUS_OK) {
+ LOG(ERROR) << "Communication error: promptUserConfirmation: "
+ "Trying to register death recipient: ";
+ delete static_cast<DeathRecipientCookie*>(cookie);
+ return APC_COMPAT_ERROR_SYSTEM_ERROR;
+ }
+ deathRecipientCookie_.insert(cookie);
}
auto rc = aidlService_->promptUserConfirmation(ref<ConfuiAidlCompatSession>(), aidl_prompt,
@@ -275,8 +291,21 @@
if (callback.result != nullptr) {
if (aidlService_) {
- AIBinder_unlinkToDeath(aidlService_->asBinder().get(), death_recipient_.get(),
- this);
+ // unlink all of the registered death recipients in case there
+ // were multiple calls to promptUserConfirmation before a call
+ // to finalize
+ std::set<void*> cookiesToUnlink;
+ {
+ auto cookieLock = std::lock_guard(deathRecipientCookie_lock_);
+ cookiesToUnlink = deathRecipientCookie_;
+ deathRecipientCookie_.clear();
+ }
+
+ // Unlink these outside of the lock
+ for (void* cookie : cookiesToUnlink) {
+ AIBinder_unlinkToDeath(aidlService_->asBinder().get(), death_recipient_.get(),
+ cookie);
+ }
}
CompatSessionCB::finalize(responseCode2Compat(responseCode), callback, dataConfirmed,
confirmationToken);
@@ -293,17 +322,46 @@
void serviceDied() {
aidlService_.reset();
aidlService_ = nullptr;
+ {
+ std::lock_guard lock(deathRecipientCookie_lock_);
+ deathRecipientCookie_.clear();
+ }
finalize(AidlConfirmationUI::SYSTEM_ERROR, {}, {});
}
- static void binderDiedCallbackAidl(void* ptr) {
- LOG(ERROR) << __func__ << " : ConfuiAidlCompatSession Service died.";
- auto aidlSession = static_cast<ConfuiAidlCompatSession*>(ptr);
- if (aidlSession == nullptr) {
- LOG(ERROR) << __func__ << ": Null ConfuiAidlCompatSession HAL died.";
- return;
+ void serviceUnlinked(void* cookie) {
+ {
+ std::lock_guard lock(deathRecipientCookie_lock_);
+ deathRecipientCookie_.erase(cookie);
}
- aidlSession->serviceDied();
+ }
+
+ static void binderDiedCallbackAidl(void* ptr) {
+ auto aidlSessionCookie = static_cast<ConfuiAidlCompatSession::DeathRecipientCookie*>(ptr);
+ if (aidlSessionCookie == nullptr) {
+ LOG(ERROR) << __func__ << ": Null cookie for binderDiedCallbackAidl when HAL died!";
+ return;
+ } else if (auto aidlSession = aidlSessionCookie->mAidlSession.lock();
+ aidlSession != nullptr) {
+ LOG(WARNING) << __func__ << " : Notififying ConfuiAidlCompatSession Service died.";
+ aidlSession->serviceDied();
+ } else {
+ LOG(ERROR) << __func__
+ << " : ConfuiAidlCompatSession Service died but object is no longer around "
+ "to be able to notify.";
+ }
+ }
+
+ static void binderUnlinkedCallbackAidl(void* ptr) {
+ auto aidlSessionCookie = static_cast<ConfuiAidlCompatSession::DeathRecipientCookie*>(ptr);
+ if (aidlSessionCookie == nullptr) {
+ LOG(ERROR) << __func__ << ": Null cookie!";
+ return;
+ } else if (auto aidlSession = aidlSessionCookie->mAidlSession.lock();
+ aidlSession != nullptr) {
+ aidlSession->serviceUnlinked(ptr);
+ }
+ delete aidlSessionCookie;
}
int getReturnCode(const ::ndk::ScopedAStatus& result) {
@@ -343,6 +401,7 @@
: aidlService_(service), callback_{nullptr, nullptr} {
death_recipient_ = ::ndk::ScopedAIBinder_DeathRecipient(
AIBinder_DeathRecipient_new(binderDiedCallbackAidl));
+ AIBinder_DeathRecipient_setOnUnlinked(death_recipient_.get(), binderUnlinkedCallbackAidl);
}
virtual ~ConfuiAidlCompatSession() = default;
@@ -351,6 +410,8 @@
private:
std::shared_ptr<AidlConfirmationUI> aidlService_;
+ std::mutex deathRecipientCookie_lock_;
+ std::set<void*> deathRecipientCookie_;
// The callback_lock_ protects the callback_ field against concurrent modification.
// IMPORTANT: It must never be held while calling the call back.
diff --git a/keystore2/selinux/Android.bp b/keystore2/selinux/Android.bp
index 254f95e..8e644e6 100644
--- a/keystore2/selinux/Android.bp
+++ b/keystore2/selinux/Android.bp
@@ -34,7 +34,6 @@
rustlibs: [
"libanyhow",
- "liblazy_static",
"liblog_rust",
"libselinux_bindgen",
"libthiserror",
@@ -57,7 +56,6 @@
rustlibs: [
"libandroid_logger",
"libanyhow",
- "liblazy_static",
"liblog_rust",
"libselinux_bindgen",
"libthiserror",
@@ -77,7 +75,6 @@
"libandroid_logger",
"libanyhow",
"libkeystore2_selinux",
- "liblazy_static",
"liblog_rust",
"libnix",
"libnum_cpus",
diff --git a/keystore2/selinux/src/lib.rs b/keystore2/selinux/src/lib.rs
index 695e029..d57a99a 100644
--- a/keystore2/selinux/src/lib.rs
+++ b/keystore2/selinux/src/lib.rs
@@ -18,6 +18,7 @@
//! * getcon
//! * selinux_check_access
//! * selabel_lookup for the keystore2_key backend.
+//!
//! And it provides an owning wrapper around context strings `Context`.
// TODO(b/290018030): Remove this and add proper safety comments.
@@ -25,7 +26,6 @@
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;
@@ -41,15 +41,13 @@
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();
-}
+/// `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 LIB_SELINUX_LOCK: sync::Mutex<()> = sync::Mutex::new(());
fn redirect_selinux_logs_to_logcat() {
// `selinux_set_callback` assigns the static lifetime function pointer
diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs
index 9ce6506..8165c54 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -70,12 +70,11 @@
};
use anyhow::{anyhow, Context, Result};
use keystore2_flags;
-use std::{convert::TryFrom, convert::TryInto, ops::Deref, time::SystemTimeError};
+use std::{convert::TryFrom, convert::TryInto, ops::Deref, sync::LazyLock, time::SystemTimeError};
use utils as db_utils;
use utils::SqlField;
use keystore2_crypto::ZVec;
-use lazy_static::lazy_static;
use log::error;
#[cfg(not(test))]
use rand::prelude::random;
@@ -529,9 +528,7 @@
}
}
-lazy_static! {
- static ref KEY_ID_LOCK: KeyIdLockDb = KeyIdLockDb::new();
-}
+static KEY_ID_LOCK: LazyLock<KeyIdLockDb> = LazyLock::new(KeyIdLockDb::new);
struct KeyIdLockDb {
locked_keys: Mutex<HashSet<i64>>,
@@ -1902,6 +1899,7 @@
/// `access_vector`.
/// * Domain::KEY_ID: The keyentry table is queried for the owning `domain` and
/// `namespace`.
+ ///
/// In each case the information returned is sufficient to perform the access
/// check and the key id can be used to load further key artifacts.
fn load_access_tuple(
diff --git a/keystore2/src/database/perboot.rs b/keystore2/src/database/perboot.rs
index 4727015..a1890a6 100644
--- a/keystore2/src/database/perboot.rs
+++ b/keystore2/src/database/perboot.rs
@@ -19,9 +19,9 @@
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::Arc;
+use std::sync::LazyLock;
use std::sync::RwLock;
#[derive(PartialEq, PartialOrd, Ord, Eq, Hash)]
@@ -70,11 +70,9 @@
auth_tokens: RwLock<HashSet<AuthTokenEntryWrap>>,
}
-lazy_static! {
- /// The global instance of the perboot DB. Located here rather than in globals
- /// in order to restrict access to the database module.
- pub static ref PERBOOT_DB: Arc<PerbootDB> = Arc::new(PerbootDB::new());
-}
+/// The global instance of the perboot DB. Located here rather than in globals
+/// in order to restrict access to the database module.
+pub static PERBOOT_DB: LazyLock<Arc<PerbootDB>> = LazyLock::new(|| Arc::new(PerbootDB::new()));
impl PerbootDB {
/// Construct a new perboot database. Currently just uses default values.
diff --git a/keystore2/src/globals.rs b/keystore2/src/globals.rs
index bde83fd..39d6f9c 100644
--- a/keystore2/src/globals.rs
+++ b/keystore2/src/globals.rs
@@ -46,8 +46,7 @@
use anyhow::{Context, Result};
use binder::FromIBinder;
use binder::{get_declared_instances, is_declared};
-use lazy_static::lazy_static;
-use std::sync::{Arc, Mutex, RwLock};
+use std::sync::{Arc, LazyLock, Mutex, RwLock};
use std::{cell::RefCell, sync::Once};
use std::{collections::HashMap, path::Path, path::PathBuf};
@@ -139,32 +138,36 @@
}
}
-lazy_static! {
- /// The path where keystore stores all its keys.
- 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<RwLock<SuperKeyManager>> = Default::default();
- /// Map of KeyMint devices.
- static ref KEY_MINT_DEVICES: Mutex<DevicesMap<dyn IKeyMintDevice>> = Default::default();
- /// Timestamp service.
- static ref TIME_STAMP_DEVICE: Mutex<Option<Strong<dyn ISecureClock>>> = Default::default();
- /// A single on-demand worker thread that handles deferred tasks with two different
- /// priorities.
- pub static ref ASYNC_TASK: Arc<AsyncTask> = Default::default();
- /// Singleton for enforcements.
- pub static ref ENFORCEMENTS: Enforcements = Default::default();
- /// 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.read().expect("Could not determine database path for legacy blob loader")));
- /// Legacy migrator. Atomically migrates legacy blobs to the database.
- pub static ref LEGACY_IMPORTER: Arc<LegacyImporter> =
- Arc::new(LegacyImporter::new(Arc::new(Default::default())));
- /// Background thread which handles logging via statsd and logd
- pub static ref LOGS_HANDLER: Arc<AsyncTask> = Default::default();
+/// The path where keystore stores all its keys.
+pub static DB_PATH: LazyLock<RwLock<PathBuf>> =
+ LazyLock::new(|| RwLock::new(Path::new("/data/misc/keystore").to_path_buf()));
+/// Runtime database of unwrapped super keys.
+pub static SUPER_KEY: LazyLock<Arc<RwLock<SuperKeyManager>>> = LazyLock::new(Default::default);
+/// Map of KeyMint devices.
+static KEY_MINT_DEVICES: LazyLock<Mutex<DevicesMap<dyn IKeyMintDevice>>> =
+ LazyLock::new(Default::default);
+/// Timestamp service.
+static TIME_STAMP_DEVICE: Mutex<Option<Strong<dyn ISecureClock>>> = Mutex::new(None);
+/// A single on-demand worker thread that handles deferred tasks with two different
+/// priorities.
+pub static ASYNC_TASK: LazyLock<Arc<AsyncTask>> = LazyLock::new(Default::default);
+/// Singleton for enforcements.
+pub static ENFORCEMENTS: LazyLock<Enforcements> = LazyLock::new(Default::default);
+/// LegacyBlobLoader is initialized and exists globally.
+/// The same directory used by the database is used by the LegacyBlobLoader as well.
+pub static LEGACY_BLOB_LOADER: LazyLock<Arc<LegacyBlobLoader>> = LazyLock::new(|| {
+ Arc::new(LegacyBlobLoader::new(
+ &DB_PATH.read().expect("Could not determine database path for legacy blob loader"),
+ ))
+});
+/// Legacy migrator. Atomically migrates legacy blobs to the database.
+pub static LEGACY_IMPORTER: LazyLock<Arc<LegacyImporter>> =
+ LazyLock::new(|| Arc::new(LegacyImporter::new(Arc::new(Default::default()))));
+/// Background thread which handles logging via statsd and logd
+pub static LOGS_HANDLER: LazyLock<Arc<AsyncTask>> = LazyLock::new(Default::default);
- static ref GC: Arc<Gc> = Arc::new(Gc::new_init_with(ASYNC_TASK.clone(), || {
+static GC: LazyLock<Arc<Gc>> = LazyLock::new(|| {
+ Arc::new(Gc::new_init_with(ASYNC_TASK.clone(), || {
(
Box::new(|uuid, blob| {
let km_dev = get_keymint_dev_by_uuid(uuid).map(|(dev, _)| dev)?;
@@ -172,12 +175,15 @@
map_km_error(km_dev.deleteKey(blob))
.context(ks_err!("Trying to invalidate key blob."))
}),
- KeystoreDB::new(&DB_PATH.read().expect("Could not determine database path for GC"), None)
- .expect("Failed to open database"),
+ KeystoreDB::new(
+ &DB_PATH.read().expect("Could not determine database path for GC"),
+ None,
+ )
+ .expect("Failed to open database"),
SUPER_KEY.clone(),
)
- }));
-}
+ }))
+});
/// Determine the service name for a KeyMint device of the given security level
/// gotten by binder service from the device and determining what services
diff --git a/keystore2/src/legacy_blob.rs b/keystore2/src/legacy_blob.rs
index c27a050..e05e686 100644
--- a/keystore2/src/legacy_blob.rs
+++ b/keystore2/src/legacy_blob.rs
@@ -961,7 +961,7 @@
fn make_user_path_name(&self, user_id: u32) -> PathBuf {
let mut path = self.path.clone();
- path.push(&format!("user_{}", user_id));
+ path.push(format!("user_{}", user_id));
path
}
diff --git a/keystore2/src/metrics_store.rs b/keystore2/src/metrics_store.rs
index 5a76d04..895374c 100644
--- a/keystore2/src/metrics_store.rs
+++ b/keystore2/src/metrics_store.rs
@@ -44,18 +44,15 @@
SecurityLevel::SecurityLevel as MetricsSecurityLevel, Storage::Storage as MetricsStorage,
};
use anyhow::{anyhow, Context, Result};
-use lazy_static::lazy_static;
use std::collections::HashMap;
-use std::sync::Mutex;
+use std::sync::{LazyLock, Mutex};
// Note: Crash events are recorded at keystore restarts, based on the assumption that keystore only
// gets restarted after a crash, during a boot cycle.
const KEYSTORE_CRASH_COUNT_PROPERTY: &str = "keystore.crash_count";
-lazy_static! {
- /// Singleton for MetricsStore.
- pub static ref METRICS_STORE: MetricsStore = Default::default();
-}
+/// Singleton for MetricsStore.
+pub static METRICS_STORE: LazyLock<MetricsStore> = LazyLock::new(Default::default);
/// MetricsStore stores the <atom object, count> as <key, value> in the inner hash map,
/// indexed by the atom id, in the outer hash map.
diff --git a/keystore2/src/operation.rs b/keystore2/src/operation.rs
index 27f8ef6..9ae8ccf 100644
--- a/keystore2/src/operation.rs
+++ b/keystore2/src/operation.rs
@@ -31,6 +31,7 @@
//! * `abort` is called.
//! * The operation gets dropped.
//! * The operation gets pruned.
+//!
//! `Operation` has an `Outcome` member. While the outcome is `Outcome::Unknown`,
//! the operation is active and in a good state. Any of the above conditions may
//! change the outcome to one of the defined outcomes Success, Abort, Dropped,
diff --git a/keystore2/src/permission.rs b/keystore2/src/permission.rs
index d79445b..7bf17b5 100644
--- a/keystore2/src/permission.rs
+++ b/keystore2/src/permission.rs
@@ -26,11 +26,11 @@
};
use anyhow::Context as AnyhowContext;
use keystore2_selinux as selinux;
-use lazy_static::lazy_static;
use selinux::{implement_class, Backend, ClassPermission};
use std::cmp::PartialEq;
use std::convert::From;
use std::ffi::CStr;
+use std::sync::LazyLock;
// Replace getcon with a mock in the test situation
#[cfg(not(test))]
@@ -41,12 +41,10 @@
#[cfg(test)]
mod tests;
-lazy_static! {
- // Panicking here is allowed because keystore cannot function without this backend
- // and it would happen early and indicate a gross misconfiguration of the device.
- static ref KEYSTORE2_KEY_LABEL_BACKEND: selinux::KeystoreKeyBackend =
- selinux::KeystoreKeyBackend::new().unwrap();
-}
+// Panicking here is allowed because keystore cannot function without this backend
+// and it would happen early and indicate a gross misconfiguration of the device.
+static KEYSTORE2_KEY_LABEL_BACKEND: LazyLock<selinux::KeystoreKeyBackend> =
+ LazyLock::new(|| selinux::KeystoreKeyBackend::new().unwrap());
fn lookup_keystore2_key_context(namespace: i64) -> anyhow::Result<selinux::Context> {
KEYSTORE2_KEY_LABEL_BACKEND.lookup(&namespace.to_string())
diff --git a/keystore2/src/watchdog_helper.rs b/keystore2/src/watchdog_helper.rs
index 1072ac0..63383aa 100644
--- a/keystore2/src/watchdog_helper.rs
+++ b/keystore2/src/watchdog_helper.rs
@@ -17,8 +17,7 @@
/// This module provides helpers for simplified use of the watchdog module.
#[cfg(feature = "watchdog")]
pub mod watchdog {
- use lazy_static::lazy_static;
- use std::sync::Arc;
+ use std::sync::{Arc, LazyLock};
use std::time::Duration;
pub use watchdog_rs::WatchPoint;
use watchdog_rs::Watchdog;
@@ -28,10 +27,8 @@
const DEFAULT_TIMEOUT: Duration = Duration::from_millis(DEFAULT_TIMEOUT_MS);
- lazy_static! {
- /// A Watchdog thread, that can be used to create watch points.
- static ref WD: Arc<Watchdog> = Watchdog::new(Duration::from_secs(10));
- }
+ /// A Watchdog thread, that can be used to create watch points.
+ static WD: LazyLock<Arc<Watchdog>> = LazyLock::new(|| Watchdog::new(Duration::from_secs(10)));
/// Sets a watch point with `id` and a timeout of `millis` milliseconds.
pub fn watch_millis(id: &'static str, millis: u64) -> Option<WatchPoint> {
diff --git a/keystore2/test_utils/Android.bp b/keystore2/test_utils/Android.bp
index 4c7c18a..d0b5540 100644
--- a/keystore2/test_utils/Android.bp
+++ b/keystore2/test_utils/Android.bp
@@ -42,15 +42,15 @@
"libthiserror",
],
static_libs: [
+ "libcppbor",
+ "libkeymaster_portable",
+ "libkeymint_support",
"libkeystore-engine",
"libkeystore2_ffi_test_utils",
],
shared_libs: [
- "android.system.keystore2-V4-ndk",
"libbase",
"libcrypto",
- "libkeymaster_portable",
- "libkeymint_support",
],
}
@@ -59,6 +59,12 @@
crate_name: "keystore2_test_utils",
srcs: ["lib.rs"],
defaults: ["libkeystore2_test_utils_defaults"],
+ static_libs: [
+ // Also include static_libs for the NDK variants so that they are available
+ // for dependencies.
+ "android.system.keystore2-V4-ndk",
+ "android.hardware.security.keymint-V3-ndk",
+ ],
}
rust_test {
@@ -75,20 +81,22 @@
name: "libkeystore2_ffi_test_utils",
srcs: ["ffi_test_utils.cpp"],
defaults: [
- "keymint_use_latest_hal_aidl_ndk_shared",
- "keystore2_use_latest_aidl_ndk_shared",
+ "keymint_use_latest_hal_aidl_ndk_static",
+ "keystore2_use_latest_aidl_ndk_static",
],
generated_headers: [
"cxx-bridge-header",
"libkeystore2_ffi_test_utils_bridge_header",
],
generated_sources: ["libkeystore2_ffi_test_utils_bridge_code"],
- static_libs: ["libkeystore-engine"],
+ static_libs: [
+ "libkeymaster_portable",
+ "libkeymint_support",
+ "libkeystore-engine",
+ ],
shared_libs: [
"libbase",
"libcrypto",
- "libkeymaster_portable",
- "libkeymint_support",
],
}
diff --git a/keystore2/test_utils/key_generations.rs b/keystore2/test_utils/key_generations.rs
index 258c68f..e2f0b3e 100644
--- a/keystore2/test_utils/key_generations.rs
+++ b/keystore2/test_utils/key_generations.rs
@@ -466,14 +466,19 @@
return true;
}
+ // Don't check these parameters if the underlying device is a Keymaster implementation.
if sl.is_keymaster() {
- // `Tag::USAGE_COUNT_LIMIT` was added in KeyMint 1.0, so don't check for it if the
- // underlying device is a Keymaster implementation.
- if matches!(key_param.tag, Tag::USAGE_COUNT_LIMIT) {
+ if matches!(
+ key_param.tag,
+ // `Tag::USAGE_COUNT_LIMIT` was added in KeyMint 1.0.
+ Tag::USAGE_COUNT_LIMIT |
+ // Keymaster implementations may not consistently include `Tag::VENDOR_PATCHLEVEL`
+ // in generated key characteristics.
+ Tag::VENDOR_PATCHLEVEL
+ ) {
return true;
}
- // `KeyPurpose::ATTEST_KEY` was added in KeyMint 1.0, so don't check for it if the
- // underlying device is a Keymaster implementation.
+ // `KeyPurpose::ATTEST_KEY` was added in KeyMint 1.0.
if key_param.tag == Tag::PURPOSE
&& key_param.value == KeyParameterValue::KeyPurpose(KeyPurpose::ATTEST_KEY)
{
diff --git a/keystore2/test_utils/run_as.rs b/keystore2/test_utils/run_as.rs
index d39d069..2cd9fec 100644
--- a/keystore2/test_utils/run_as.rs
+++ b/keystore2/test_utils/run_as.rs
@@ -357,9 +357,12 @@
// Safety: run_as must be called from a single threaded process.
// This device test is run as a separate single threaded process.
unsafe {
- run_as(selinux::getcon().unwrap().to_str().unwrap(), getuid(), getgid(), || {
- panic!("Closure must panic.")
- })
+ run_as::<_, ()>(
+ selinux::getcon().unwrap().to_str().unwrap(),
+ getuid(),
+ getgid(),
+ || panic!("Closure must panic."),
+ )
};
}
diff --git a/keystore2/tests/Android.bp b/keystore2/tests/Android.bp
index ca9f5e3..dbef46c 100644
--- a/keystore2/tests/Android.bp
+++ b/keystore2/tests/Android.bp
@@ -24,11 +24,16 @@
rust_test {
name: "keystore2_client_tests",
- compile_multilib: "first",
defaults: [
"keymint_use_latest_hal_aidl_rust",
"keystore2_use_latest_aidl_rust",
],
+ static_libs: [
+ // Also include static_libs for the NDK variants so that they are available
+ // for dependencies.
+ "android.system.keystore2-V4-ndk",
+ "android.hardware.security.keymint-V3-ndk",
+ ],
srcs: ["keystore2_client_tests.rs"],
test_suites: [
"general-tests",
diff --git a/keystore2/tests/AndroidTest.xml b/keystore2/tests/AndroidTest.xml
index dde18a9..7db36f7 100644
--- a/keystore2/tests/AndroidTest.xml
+++ b/keystore2/tests/AndroidTest.xml
@@ -14,7 +14,6 @@
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_3des_key_tests.rs b/keystore2/tests/keystore2_client_3des_key_tests.rs
index 29f1617..4cb81d1 100644
--- a/keystore2/tests/keystore2_client_3des_key_tests.rs
+++ b/keystore2/tests/keystore2_client_3des_key_tests.rs
@@ -72,6 +72,7 @@
/// Generate 3DES keys with various block modes and paddings.
/// - Block Modes: ECB, CBC
/// - Padding Modes: NONE, PKCS7
+///
/// Test should generate keys and perform operation successfully.
#[test]
fn keystore2_3des_ecb_cbc_generate_key_success() {
diff --git a/keystore2/tests/keystore2_client_aes_key_tests.rs b/keystore2/tests/keystore2_client_aes_key_tests.rs
index 9f85c38..3c5fda5 100644
--- a/keystore2/tests/keystore2_client_aes_key_tests.rs
+++ b/keystore2/tests/keystore2_client_aes_key_tests.rs
@@ -75,6 +75,7 @@
/// Generate AES keys with various block modes and paddings.
/// - Block Modes: ECB, CBC
/// - Padding Modes: NONE, PKCS7
+///
/// Test should generate keys and perform operation successfully.
#[test]
fn keystore2_aes_ecb_cbc_generate_key() {
@@ -106,6 +107,7 @@
/// Generate AES keys with -
/// - Block Modes: `CTR, GCM`
/// - Padding Modes: `NONE`
+///
/// Test should generate keys and perform operation successfully.
#[test]
fn keystore2_aes_ctr_gcm_generate_key_success() {
@@ -133,6 +135,7 @@
/// Generate AES keys with -
/// - Block Modes: `CTR, GCM`
/// - Padding Modes: `PKCS7`
+///
/// Try to create an operation using generated keys, test should fail to create an operation
/// with an error code `INCOMPATIBLE_PADDING_MODE`.
#[test]
@@ -248,7 +251,7 @@
}
/// Try to create an operation using AES key with multiple padding modes. Test should fail to create
-/// an operation with `UNSUPPORTED_PADDING_MODE` error code.
+/// an operation.
#[test]
fn keystore2_aes_key_op_fails_multi_padding_modes() {
let sl = SecLevel::tee();
@@ -292,7 +295,12 @@
false,
));
assert!(result.is_err());
- assert_eq!(Error::Km(ErrorCode::UNSUPPORTED_PADDING_MODE), result.unwrap_err());
+ assert!(matches!(
+ result.unwrap_err(),
+ Error::Km(ErrorCode::INCOMPATIBLE_PADDING_MODE)
+ | Error::Km(ErrorCode::UNSUPPORTED_PADDING_MODE)
+ | Error::Km(ErrorCode::INVALID_ARGUMENT)
+ ));
}
/// Generate a AES-ECB key with unpadded mode. Try to create an operation using generated key
diff --git a/keystore2/tests/keystore2_client_attest_key_tests.rs b/keystore2/tests/keystore2_client_attest_key_tests.rs
index 7597ba1..f723d02 100644
--- a/keystore2/tests/keystore2_client_attest_key_tests.rs
+++ b/keystore2/tests/keystore2_client_attest_key_tests.rs
@@ -463,59 +463,6 @@
assert_eq!(Error::Rc(ResponseCode::INVALID_ARGUMENT), result.unwrap_err());
}
-/// Generate RSA attestation key and try to use it as attestation key while generating symmetric
-/// key. Test should generate symmetric key successfully. Verify that generated symmetric key
-/// should not have attestation record or certificate.
-#[test]
-fn keystore2_attest_symmetric_key_fail_sys_error() {
- skip_test_if_no_app_attest_key_feature!();
-
- let sl = SecLevel::tee();
- let att_challenge: &[u8] = b"foo";
-
- // Create attestation key.
- let Some(attestation_key_metadata) = key_generations::map_ks_error(
- key_generations::generate_attestation_key(&sl, Algorithm::RSA, att_challenge),
- )
- .unwrap() else {
- return;
- };
-
- let mut cert_chain: Vec<u8> = Vec::new();
- cert_chain.extend(attestation_key_metadata.certificate.as_ref().unwrap());
- cert_chain.extend(attestation_key_metadata.certificateChain.as_ref().unwrap());
- validate_certchain(&cert_chain).expect("Error while validating cert chain.");
-
- // Generate symmetric key with above generated key as attestation key.
- let gen_params = authorizations::AuthSetBuilder::new()
- .no_auth_required()
- .algorithm(Algorithm::AES)
- .purpose(KeyPurpose::ENCRYPT)
- .purpose(KeyPurpose::DECRYPT)
- .key_size(128)
- .padding_mode(PaddingMode::NONE)
- .block_mode(BlockMode::ECB)
- .attestation_challenge(att_challenge.to_vec());
-
- let alias = format!("ks_test_sym_key_attest_{}", getuid());
- let aes_key_metadata = sl
- .binder
- .generateKey(
- &KeyDescriptor { domain: Domain::APP, nspace: -1, alias: Some(alias), blob: None },
- Some(&attestation_key_metadata.key),
- &gen_params,
- 0,
- b"entropy",
- )
- .unwrap();
-
- // Should not have public certificate.
- assert!(aes_key_metadata.certificate.is_none());
-
- // Should not have an attestation record.
- assert!(aes_key_metadata.certificateChain.is_none());
-}
-
fn get_attestation_ids(keystore2: &binder::Strong<dyn IKeystoreService>) -> Vec<(Tag, Vec<u8>)> {
let attest_ids = vec![
(Tag::ATTESTATION_ID_BRAND, "brand"),
@@ -615,6 +562,8 @@
#[test]
fn keystore2_attest_key_fails_with_invalid_attestation_id() {
skip_test_if_no_device_id_attestation_feature!();
+ skip_device_id_attestation_tests!();
+ skip_test_if_no_app_attest_key_feature!();
let sl = SecLevel::tee();
diff --git a/keystore2/tests/keystore2_client_operation_tests.rs b/keystore2/tests/keystore2_client_operation_tests.rs
index 95888a6..5f640ef 100644
--- a/keystore2/tests/keystore2_client_operation_tests.rs
+++ b/keystore2/tests/keystore2_client_operation_tests.rs
@@ -28,6 +28,10 @@
};
use nix::unistd::{getuid, Gid, Uid};
use rustutils::users::AID_USER_OFFSET;
+use std::sync::{
+ atomic::{AtomicBool, Ordering},
+ Arc,
+};
use std::thread;
use std::thread::JoinHandle;
@@ -376,6 +380,7 @@
/// - untrusted_app
/// - system_server
/// - priv_app
+///
/// `PERMISSION_DENIED` error response is expected.
#[test]
fn keystore2_forced_op_perm_denied_test() {
@@ -460,3 +465,120 @@
assert!(result1 || result2);
}
+
+/// Create an operation and use it for performing sign operation. After completing the operation
+/// try to abort the operation. Test should fail to abort already finalized operation with error
+/// code `INVALID_OPERATION_HANDLE`.
+#[test]
+fn keystore2_abort_finalized_op_fail_test() {
+ let op_response = create_signing_operation(
+ ForcedOp(false),
+ KeyPurpose::SIGN,
+ Digest::SHA_2_256,
+ Domain::APP,
+ -1,
+ Some("ks_op_abort_fail_test_key".to_string()),
+ )
+ .unwrap();
+
+ let op: binder::Strong<dyn IKeystoreOperation> = op_response.iOperation.unwrap();
+ perform_sample_sign_operation(&op).unwrap();
+ let result = key_generations::map_ks_error(op.abort());
+ assert!(result.is_err());
+ assert_eq!(Error::Km(ErrorCode::INVALID_OPERATION_HANDLE), result.unwrap_err());
+}
+
+/// Create an operation and use it for performing sign operation. Before finishing the operation
+/// try to abort the operation. Test should successfully abort the operation. After aborting try to
+/// use the operation handle, test should fail to use already aborted operation handle with error
+/// code `INVALID_OPERATION_HANDLE`.
+#[test]
+fn keystore2_op_abort_success_test() {
+ let op_response = create_signing_operation(
+ ForcedOp(false),
+ KeyPurpose::SIGN,
+ Digest::SHA_2_256,
+ Domain::APP,
+ -1,
+ Some("ks_op_abort_success_key".to_string()),
+ )
+ .unwrap();
+
+ let op: binder::Strong<dyn IKeystoreOperation> = op_response.iOperation.unwrap();
+ op.update(b"my message").unwrap();
+ let result = key_generations::map_ks_error(op.abort());
+ assert!(result.is_ok());
+
+ // Try to use the op handle after abort.
+ let result = key_generations::map_ks_error(op.finish(None, None));
+ assert!(result.is_err());
+ assert_eq!(Error::Km(ErrorCode::INVALID_OPERATION_HANDLE), result.unwrap_err());
+}
+
+/// Executes an operation in a thread. Performs an `update` operation repeatedly till the user
+/// interrupts it or encounters any error other than `OPERATION_BUSY`.
+/// Return `false` in case of any error other than `OPERATION_BUSY`, otherwise it returns true.
+fn perform_abort_op_busy_in_thread(
+ op: binder::Strong<dyn IKeystoreOperation>,
+ should_exit_clone: Arc<AtomicBool>,
+) -> JoinHandle<bool> {
+ thread::spawn(move || {
+ loop {
+ if should_exit_clone.load(Ordering::Relaxed) {
+ // Caller requested to exit the thread.
+ return true;
+ }
+
+ match key_generations::map_ks_error(op.update(b"my message")) {
+ Ok(_) => continue,
+ Err(Error::Rc(ResponseCode::OPERATION_BUSY)) => continue,
+ Err(_) => return false,
+ }
+ }
+ })
+}
+
+/// Create an operation and try to use same operation handle in multiple threads to perform
+/// operations. Test tries to abort the operation and expects `abort` call to fail with the error
+/// response `OPERATION_BUSY` as multiple threads try to access the same operation handle
+/// simultaneously. Test tries to simulate `OPERATION_BUSY` error response from `abort` api.
+#[test]
+fn keystore2_op_abort_fails_with_operation_busy_error_test() {
+ loop {
+ let op_response = create_signing_operation(
+ ForcedOp(false),
+ KeyPurpose::SIGN,
+ Digest::SHA_2_256,
+ Domain::APP,
+ -1,
+ Some("op_abort_busy_alias_test_key".to_string()),
+ )
+ .unwrap();
+ let op: binder::Strong<dyn IKeystoreOperation> = op_response.iOperation.unwrap();
+
+ let should_exit = Arc::new(AtomicBool::new(false));
+
+ let update_t_handle1 = perform_abort_op_busy_in_thread(op.clone(), should_exit.clone());
+ let update_t_handle2 = perform_abort_op_busy_in_thread(op.clone(), should_exit.clone());
+
+ // Attempt to abort the operation and anticipate an 'OPERATION_BUSY' error, as multiple
+ // threads are concurrently accessing the same operation handle.
+ let result = match op.abort() {
+ Ok(_) => 0, // Operation successfully aborted.
+ Err(e) => e.service_specific_error(),
+ };
+
+ // Notify threads to stop performing `update` operation.
+ should_exit.store(true, Ordering::Relaxed);
+
+ let _update_op_result = update_t_handle1.join().unwrap();
+ let _update_op_result2 = update_t_handle2.join().unwrap();
+
+ if result == ResponseCode::OPERATION_BUSY.0 {
+ // The abort call failed with an OPERATION_BUSY error, as anticipated, due to multiple
+ // threads competing for access to the same operation handle.
+ return;
+ }
+ assert_eq!(result, 0);
+ }
+}
diff --git a/keystore2/tests/legacy_blobs/Android.bp b/keystore2/tests/legacy_blobs/Android.bp
index 0f310f5..92d1307 100644
--- a/keystore2/tests/legacy_blobs/Android.bp
+++ b/keystore2/tests/legacy_blobs/Android.bp
@@ -38,7 +38,6 @@
"libkeystore2_crypto_rust",
"libkeystore2_test_utils",
"libkeystore2_with_test_utils",
- "liblazy_static",
"liblibc",
"libnix",
"librustutils",