Keystore 2.0: Fix shared secret negotiation for Keymaster 4.x
The km_compat legacy wrapper would only cache the first shared secret
participant and then return this participant regardless of which
security level was requested. As a result only one Keymaster instance
would take part in the shared secret negotiation.
This patch adds a per security level cache for ISharedSecret instances
to km_compat. It filters Keymaster instances in Keystore 2.0 to only
include the highest version of each HIDL Keymaster security level.
Bug: 190539964
Test: See b/190539964
Merged-In: I0b73da88d3e1b6900cfb332c1befc704eca59cc5
Change-Id: I0b73da88d3e1b6900cfb332c1befc704eca59cc5
diff --git a/keystore2/src/km_compat/km_compat.cpp b/keystore2/src/km_compat/km_compat.cpp
index 19576aa..f6f8bfe 100644
--- a/keystore2/src/km_compat/km_compat.cpp
+++ b/keystore2/src/km_compat/km_compat.cpp
@@ -1395,8 +1395,7 @@
if (!device) {
return ScopedAStatus::fromStatus(STATUS_NAME_NOT_FOUND);
}
- bool inserted = false;
- std::tie(i, inserted) = mDeviceCache.insert({in_securityLevel, std::move(device)});
+ i = mDeviceCache.insert(i, {in_securityLevel, std::move(device)});
}
*_aidl_return = i->second;
return ScopedAStatus::ok();
@@ -1404,14 +1403,15 @@
ScopedAStatus KeystoreCompatService::getSharedSecret(KeyMintSecurityLevel in_securityLevel,
std::shared_ptr<ISharedSecret>* _aidl_return) {
- if (!mSharedSecret) {
+ auto i = mSharedSecretCache.find(in_securityLevel);
+ if (i == mSharedSecretCache.end()) {
auto secret = SharedSecret::createSharedSecret(in_securityLevel);
if (!secret) {
return ScopedAStatus::fromStatus(STATUS_NAME_NOT_FOUND);
}
- mSharedSecret = std::move(secret);
+ i = mSharedSecretCache.insert(i, {in_securityLevel, std::move(secret)});
}
- *_aidl_return = mSharedSecret;
+ *_aidl_return = i->second;
return ScopedAStatus::ok();
}
diff --git a/keystore2/src/km_compat/km_compat.h b/keystore2/src/km_compat/km_compat.h
index 09c9157..2d892da 100644
--- a/keystore2/src/km_compat/km_compat.h
+++ b/keystore2/src/km_compat/km_compat.h
@@ -197,7 +197,7 @@
class KeystoreCompatService : public BnKeystoreCompatService {
private:
std::unordered_map<KeyMintSecurityLevel, std::shared_ptr<IKeyMintDevice>> mDeviceCache;
- std::shared_ptr<ISharedSecret> mSharedSecret;
+ std::unordered_map<KeyMintSecurityLevel, std::shared_ptr<ISharedSecret>> mSharedSecretCache;
std::shared_ptr<ISecureClock> mSecureClock;
public:
diff --git a/keystore2/src/shared_secret_negotiation.rs b/keystore2/src/shared_secret_negotiation.rs
index fb55f33..c29eaf9 100644
--- a/keystore2/src/shared_secret_negotiation.rs
+++ b/keystore2/src/shared_secret_negotiation.rs
@@ -109,7 +109,11 @@
/// Lists participants.
fn list_participants() -> Result<Vec<SharedSecretParticipant>> {
- Ok([(4, 0), (4, 1)]
+ // 4.1 implementation always also register as 4.0. So only the highest version of each
+ // "default" and "strongbox" makes the cut.
+ let mut legacy_default_found: bool = false;
+ let mut legacy_strongbox_found: bool = false;
+ Ok([(4, 1), (4, 0)]
.iter()
.map(|(ma, mi)| {
get_hidl_instances(KEYMASTER_PACKAGE_NAME, *ma, *mi, KEYMASTER_INTERFACE_NAME)
@@ -119,7 +123,24 @@
instances
.into_iter()
.filter_map(|name| {
- filter_map_legacy_km_instances(name.to_string(), (*ma, *mi))
+ filter_map_legacy_km_instances(name.to_string(), (*ma, *mi)).and_then(
+ |sp| {
+ if let SharedSecretParticipant::Hidl {
+ is_strongbox: true,
+ ..
+ } = &sp
+ {
+ if !legacy_strongbox_found {
+ legacy_strongbox_found = true;
+ return Some(sp);
+ }
+ } else if !legacy_default_found {
+ legacy_default_found = true;
+ return Some(sp);
+ }
+ None
+ },
+ )
})
.collect::<Vec<SharedSecretParticipant>>()
})