Merge "Keystore 2.0: Rename legacy_migrator to importer."
diff --git a/identity/Android.bp b/identity/Android.bp
index 790a731..c69ead1 100644
--- a/identity/Android.bp
+++ b/identity/Android.bp
@@ -54,6 +54,7 @@
         "libkeymaster4support",
         "libkeystore-attestation-application-id",
         "android.security.authorization-ndk",
+        "android.security.remoteprovisioning-cpp",
         "libutilscallstack",
     ],
     static_libs: [
diff --git a/identity/CredentialStore.cpp b/identity/CredentialStore.cpp
index 61a9125..c5c429b 100644
--- a/identity/CredentialStore.cpp
+++ b/identity/CredentialStore.cpp
@@ -17,10 +17,15 @@
 #define LOG_TAG "credstore"
 
 #include <algorithm>
+#include <optional>
 
 #include <android-base/logging.h>
-
+#include <android/hardware/security/keymint/IRemotelyProvisionedComponent.h>
+#include <android/hardware/security/keymint/RpcHardwareInfo.h>
+#include <android/security/remoteprovisioning/IRemotelyProvisionedKeyPool.h>
+#include <android/security/remoteprovisioning/RemotelyProvisionedKey.h>
 #include <binder/IPCThreadState.h>
+#include <binder/IServiceManager.h>
 
 #include "Credential.h"
 #include "CredentialData.h"
@@ -32,6 +37,46 @@
 namespace android {
 namespace security {
 namespace identity {
+namespace {
+
+using ::android::hardware::security::keymint::IRemotelyProvisionedComponent;
+using ::android::hardware::security::keymint::RpcHardwareInfo;
+using ::android::security::remoteprovisioning::IRemotelyProvisionedKeyPool;
+using ::android::security::remoteprovisioning::RemotelyProvisionedKey;
+
+std::optional<std::string>
+getRemotelyProvisionedComponentId(const sp<IIdentityCredentialStore>& hal) {
+    auto init = [](const sp<IIdentityCredentialStore>& hal) -> std::optional<std::string> {
+        sp<IRemotelyProvisionedComponent> remotelyProvisionedComponent;
+        Status status = hal->getRemotelyProvisionedComponent(&remotelyProvisionedComponent);
+        if (!status.isOk()) {
+            LOG(ERROR) << "Error getting remotely provisioned component: " << status;
+            return std::nullopt;
+        }
+
+        RpcHardwareInfo rpcHwInfo;
+        status = remotelyProvisionedComponent->getHardwareInfo(&rpcHwInfo);
+        if (!status.isOk()) {
+            LOG(ERROR) << "Error getting remotely provisioned component hardware info: " << status;
+            return std::nullopt;
+        }
+
+        if (!rpcHwInfo.uniqueId) {
+            LOG(ERROR) << "Remotely provisioned component is missing a unique id, which is "
+                       << "required for credential key remotely provisioned attestation keys. "
+                       << "This is a bug in the vendor implementation.";
+            return std::nullopt;
+        }
+
+        // This id is required to later fetch remotely provisioned attestation keys.
+        return *rpcHwInfo.uniqueId;
+    };
+
+    static std::optional<std::string> id = init(hal);
+    return id;
+}
+
+}  // namespace
 
 CredentialStore::CredentialStore(const std::string& dataPath, sp<IIdentityCredentialStore> hal)
     : dataPath_(dataPath), hal_(hal) {}
@@ -44,6 +89,16 @@
     }
     halApiVersion_ = hal_->getInterfaceVersion();
 
+    if (hwInfo_.isRemoteKeyProvisioningSupported) {
+        keyPool_ = android::waitForService<IRemotelyProvisionedKeyPool>(
+            IRemotelyProvisionedKeyPool::descriptor);
+        if (keyPool_.get() == nullptr) {
+            LOG(ERROR) << "Error getting IRemotelyProvisionedKeyPool HAL with service name '"
+                       << IRemotelyProvisionedKeyPool::descriptor << "'";
+            return false;
+        }
+    }
+
     LOG(INFO) << "Connected to Identity Credential HAL with API version " << halApiVersion_
               << " and name '" << hwInfo_.credentialStoreName << "' authored by '"
               << hwInfo_.credentialStoreAuthorName << "' with chunk size " << hwInfo_.dataChunkSize
@@ -90,6 +145,13 @@
         return halStatusToGenericError(status);
     }
 
+    if (hwInfo_.isRemoteKeyProvisioningSupported) {
+        status = setRemotelyProvisionedAttestationKey(halWritableCredential.get());
+        if (!status.isOk()) {
+            return halStatusToGenericError(status);
+        }
+    }
+
     sp<IWritableCredential> writableCredential = new WritableCredential(
         dataPath_, credentialName, docType, false, hwInfo_, halWritableCredential);
     *_aidl_return = writableCredential;
@@ -145,6 +207,33 @@
     return Status::ok();
 }
 
+Status CredentialStore::setRemotelyProvisionedAttestationKey(
+    IWritableIdentityCredential* halWritableCredential) {
+    std::optional<std::string> rpcId = getRemotelyProvisionedComponentId(hal_);
+    if (!rpcId) {
+        return Status::fromServiceSpecificError(ERROR_GENERIC,
+                                                "Error getting remotely provisioned component id");
+    }
+
+    uid_t callingUid = android::IPCThreadState::self()->getCallingUid();
+    RemotelyProvisionedKey key;
+    Status status = keyPool_->getAttestationKey(callingUid, *rpcId, &key);
+    if (!status.isOk()) {
+        LOG(WARNING) << "Unable to fetch remotely provisioned attestation key, falling back "
+                     << "to the factory-provisioned attestation key.";
+        return Status::ok();
+    }
+
+    status = halWritableCredential->setRemotelyProvisionedAttestationKey(key.keyBlob,
+                                                                         key.encodedCertChain);
+    if (!status.isOk()) {
+        LOG(ERROR) << "Error setting remotely provisioned attestation key on credential";
+        return status;
+    }
+
+    return Status::ok();
+}
+
 }  // namespace identity
 }  // namespace security
 }  // namespace android
diff --git a/identity/CredentialStore.h b/identity/CredentialStore.h
index f2aa506..df7928e 100644
--- a/identity/CredentialStore.h
+++ b/identity/CredentialStore.h
@@ -21,8 +21,8 @@
 #include <vector>
 
 #include <android/hardware/identity/IIdentityCredentialStore.h>
-
 #include <android/security/identity/BnCredentialStore.h>
+#include <android/security/remoteprovisioning/IRemotelyProvisionedKeyPool.h>
 
 namespace android {
 namespace security {
@@ -38,6 +38,8 @@
 using ::android::hardware::identity::HardwareInformation;
 using ::android::hardware::identity::IIdentityCredentialStore;
 using ::android::hardware::identity::IPresentationSession;
+using ::android::hardware::identity::IWritableIdentityCredential;
+using ::android::security::remoteprovisioning::IRemotelyProvisionedKeyPool;
 
 class CredentialStore : public BnCredentialStore {
   public:
@@ -64,11 +66,15 @@
     Status createPresentationSession(int32_t cipherSuite, sp<ISession>* _aidl_return) override;
 
   private:
+    Status setRemotelyProvisionedAttestationKey(IWritableIdentityCredential* halWritableCredential);
+
     string dataPath_;
 
     sp<IIdentityCredentialStore> hal_;
     int halApiVersion_;
 
+    sp<IRemotelyProvisionedKeyPool> keyPool_;
+
     HardwareInformation hwInfo_;
 };
 
diff --git a/keystore2/Android.bp b/keystore2/Android.bp
index c383f4f..2027af4 100644
--- a/keystore2/Android.bp
+++ b/keystore2/Android.bp
@@ -76,15 +76,6 @@
 }
 
 rust_library {
-    name: "libkeystore2_noicu",
-    defaults: ["libkeystore2_defaults"],
-    rustlibs: [
-        "liblibsqlite3_sys_noicu",
-        "librusqlite_noicu",
-    ],
-}
-
-rust_library {
     name: "libkeystore2_test_utils",
     crate_name: "keystore2_test_utils",
     srcs: ["test_utils/lib.rs"],
diff --git a/keystore2/legacykeystore/Android.bp b/keystore2/legacykeystore/Android.bp
index d407569..505b165 100644
--- a/keystore2/legacykeystore/Android.bp
+++ b/keystore2/legacykeystore/Android.bp
@@ -47,15 +47,6 @@
     ],
 }
 
-rust_library {
-    name: "liblegacykeystore-rust_noicu",
-    defaults: ["liblegacykeystore-rust_defaults"],
-    rustlibs: [
-        "libkeystore2_noicu",
-        "librusqlite_noicu",
-    ],
-}
-
 rust_test {
     name: "legacykeystore_test",
     crate_name: "legacykeystore",
diff --git a/keystore2/src/keystore2_main.rs b/keystore2/src/keystore2_main.rs
index abab4b6..bea5f08 100644
--- a/keystore2/src/keystore2_main.rs
+++ b/keystore2/src/keystore2_main.rs
@@ -19,7 +19,9 @@
 use keystore2::maintenance::Maintenance;
 use keystore2::metrics::Metrics;
 use keystore2::metrics_store;
-use keystore2::remote_provisioning::RemoteProvisioningService;
+use keystore2::remote_provisioning::{
+    RemoteProvisioningService, RemotelyProvisionedKeyPoolService,
+};
 use keystore2::service::KeystoreService;
 use keystore2::{apc::ApcManager, shared_secret_negotiation};
 use keystore2::{authorization::AuthorizationManager, id_rotation::IdRotationState};
@@ -33,6 +35,8 @@
 static AUTHORIZATION_SERVICE_NAME: &str = "android.security.authorization";
 static METRICS_SERVICE_NAME: &str = "android.security.metrics";
 static REMOTE_PROVISIONING_SERVICE_NAME: &str = "android.security.remoteprovisioning";
+static REMOTELY_PROVISIONED_KEY_POOL_SERVICE_NAME: &str =
+    "android.security.remoteprovisioning.IRemotelyProvisionedKeyPool";
 static USER_MANAGER_SERVICE_NAME: &str = "android.security.maintenance";
 static LEGACY_KEYSTORE_SERVICE_NAME: &str = "android.security.legacykeystore";
 
@@ -145,6 +149,22 @@
         });
     }
 
+    // Even if the IRemotelyProvisionedComponent HAL is implemented, it doesn't mean that the keys
+    // may be fetched via the key pool. The HAL must be a new version that exports a unique id. If
+    // none of the HALs support this, then the key pool service is not published.
+    if let Ok(key_pool_service) = RemotelyProvisionedKeyPoolService::new_native_binder() {
+        binder::add_service(
+            REMOTELY_PROVISIONED_KEY_POOL_SERVICE_NAME,
+            key_pool_service.as_binder(),
+        )
+        .unwrap_or_else(|e| {
+            panic!(
+                "Failed to register service {} because of {:?}.",
+                REMOTELY_PROVISIONED_KEY_POOL_SERVICE_NAME, e
+            );
+        });
+    }
+
     binder::add_service(LEGACY_KEYSTORE_SERVICE_NAME, legacykeystore.as_binder()).unwrap_or_else(
         |e| {
             panic!(
diff --git a/keystore2/src/metrics_store.rs b/keystore2/src/metrics_store.rs
index b18d84c..b6f1343 100644
--- a/keystore2/src/metrics_store.rs
+++ b/keystore2/src/metrics_store.rs
@@ -649,6 +649,7 @@
 pub fn read_keystore_crash_count() -> Result<i32> {
     rustutils::system_properties::read("keystore.crash_count")
         .context("In read_keystore_crash_count: Failed read property.")?
+        .context("In read_keystore_crash_count: Property not set.")?
         .parse::<i32>()
         .map_err(std::convert::Into::into)
 }
diff --git a/keystore2/src/remote_provisioning.rs b/keystore2/src/remote_provisioning.rs
index fadd252..639fe1e 100644
--- a/keystore2/src/remote_provisioning.rs
+++ b/keystore2/src/remote_provisioning.rs
@@ -31,6 +31,7 @@
 use android_security_remoteprovisioning::aidl::android::security::remoteprovisioning::{
     AttestationPoolStatus::AttestationPoolStatus, IRemoteProvisioning::BnRemoteProvisioning,
     IRemoteProvisioning::IRemoteProvisioning,
+    IRemotelyProvisionedKeyPool::BnRemotelyProvisionedKeyPool,
     IRemotelyProvisionedKeyPool::IRemotelyProvisionedKeyPool, ImplInfo::ImplInfo,
     RemotelyProvisionedKey::RemotelyProvisionedKey,
 };
@@ -183,22 +184,6 @@
         }
     }
 
-    fn get_dev_by_unique_id(
-        &self,
-        unique_id: &str,
-    ) -> Result<(SecurityLevel, &dyn IRemotelyProvisionedComponent)> {
-        for (sec_level, dev) in &self.device_by_sec_level {
-            if dev.getHardwareInfo()?.uniqueId == Some(unique_id.to_string()) {
-                return Ok((*sec_level, dev.as_ref()));
-            }
-        }
-
-        Err(error::Error::sys()).context(format!(
-            "In get_dev_by_unique_id: Instance for requested unique id '{}' not found",
-            unique_id
-        ))
-    }
-
     /// Creates a new instance of the remote provisioning service
     pub fn new_native_binder() -> Result<Strong<dyn IRemoteProvisioning>> {
         let mut result: Self = Default::default();
@@ -421,35 +406,6 @@
             db.delete_all_attestation_keys()
         })
     }
-
-    /// Fetches a remotely provisioned certificate chain and key for the given client uid that
-    /// was provisioned using the IRemotelyProvisionedComponent with the given id. The same key
-    /// will be returned for a given caller_uid on every request. If there are no attestation keys
-    /// available, `OUT_OF_KEYS` is returned.
-    fn get_attestation_key(
-        &self,
-        db: &mut KeystoreDB,
-        caller_uid: i32,
-        irpc_id: &str,
-    ) -> Result<RemotelyProvisionedKey> {
-        log::info!("get_attestation_key(self, {}, {}", caller_uid, irpc_id);
-
-        let (sec_level, _) = self.get_dev_by_unique_id(irpc_id)?;
-        let (_, _, km_uuid) = get_keymint_device(&sec_level)?;
-
-        let cert_chain = get_rem_prov_attest_key(Domain::APP, caller_uid as u32, db, &km_uuid)
-            .context("In get_attestation_key")?;
-        match cert_chain {
-            Some(chain) => Ok(RemotelyProvisionedKey {
-                keyBlob: chain.private_key.to_vec(),
-                encodedCertChain: chain.cert_chain,
-            }),
-            // It should be impossible to get `None`, but handle it just in case as a
-            // precaution against future behavioral changes in `get_rem_prov_attest_key`.
-            None => Err(error::Error::Rc(ResponseCode::OUT_OF_KEYS))
-                .context("In get_attestation_key: No available attestation keys"),
-        }
-    }
 }
 
 /// Populates the AttestationPoolStatus parcelable with information about how many
@@ -616,9 +572,86 @@
     }
 }
 
+/// Implementation of the IRemotelyProvisionedKeyPool service.
+#[derive(Default)]
+pub struct RemotelyProvisionedKeyPoolService {
+    unique_id_to_sec_level: HashMap<String, SecurityLevel>,
+}
+
+impl RemotelyProvisionedKeyPoolService {
+    /// Fetches a remotely provisioned certificate chain and key for the given client uid that
+    /// was provisioned using the IRemotelyProvisionedComponent with the given id. The same key
+    /// will be returned for a given caller_uid on every request. If there are no attestation keys
+    /// available, `OUT_OF_KEYS` is returned.
+    fn get_attestation_key(
+        &self,
+        db: &mut KeystoreDB,
+        caller_uid: i32,
+        irpc_id: &str,
+    ) -> Result<RemotelyProvisionedKey> {
+        log::info!("get_attestation_key(self, {}, {}", caller_uid, irpc_id);
+
+        let sec_level = self
+            .unique_id_to_sec_level
+            .get(irpc_id)
+            .ok_or(Error::Rc(ResponseCode::INVALID_ARGUMENT))
+            .context(format!("In get_attestation_key: unknown irpc id '{}'", irpc_id))?;
+        let (_, _, km_uuid) = get_keymint_device(sec_level)?;
+
+        let cert_chain = get_rem_prov_attest_key(Domain::APP, caller_uid as u32, db, &km_uuid)
+            .context("In get_attestation_key")?;
+        match cert_chain {
+            Some(chain) => Ok(RemotelyProvisionedKey {
+                keyBlob: chain.private_key.to_vec(),
+                encodedCertChain: chain.cert_chain,
+            }),
+            // It should be impossible to get `None`, but handle it just in case as a
+            // precaution against future behavioral changes in `get_rem_prov_attest_key`.
+            None => Err(error::Error::Rc(ResponseCode::OUT_OF_KEYS))
+                .context("In get_attestation_key: No available attestation keys"),
+        }
+    }
+
+    /// Creates a new instance of the remotely provisioned key pool service, used for fetching
+    /// remotely provisioned attestation keys.
+    pub fn new_native_binder() -> Result<Strong<dyn IRemotelyProvisionedKeyPool>> {
+        let mut result: Self = Default::default();
+
+        let dev = get_remotely_provisioned_component(&SecurityLevel::TRUSTED_ENVIRONMENT)
+            .context("In new_native_binder: Failed to get TEE Remote Provisioner instance.")?;
+        if let Some(id) = dev.getHardwareInfo()?.uniqueId {
+            result.unique_id_to_sec_level.insert(id, SecurityLevel::TRUSTED_ENVIRONMENT);
+        }
+
+        if let Ok(dev) = get_remotely_provisioned_component(&SecurityLevel::STRONGBOX) {
+            if let Some(id) = dev.getHardwareInfo()?.uniqueId {
+                if result.unique_id_to_sec_level.contains_key(&id) {
+                    anyhow::bail!("In new_native_binder: duplicate irpc id found: '{}'", id)
+                }
+                result.unique_id_to_sec_level.insert(id, SecurityLevel::STRONGBOX);
+            }
+        }
+
+        // If none of the remotely provisioned components have unique ids, then we shouldn't
+        // bother publishing the service, as it's impossible to match keys with their backends.
+        if result.unique_id_to_sec_level.is_empty() {
+            anyhow::bail!(
+                "In new_native_binder: No remotely provisioned components have unique ids"
+            )
+        }
+
+        Ok(BnRemotelyProvisionedKeyPool::new_binder(
+            result,
+            BinderFeatures { set_requesting_sid: true, ..BinderFeatures::default() },
+        ))
+    }
+}
+
+impl binder::Interface for RemotelyProvisionedKeyPoolService {}
+
 // Implementation of IRemotelyProvisionedKeyPool. See AIDL spec at
 // :aidl/android/security/remoteprovisioning/IRemotelyProvisionedKeyPool.aidl
-impl IRemotelyProvisionedKeyPool for RemoteProvisioningService {
+impl IRemotelyProvisionedKeyPool for RemotelyProvisionedKeyPoolService {
     fn getAttestationKey(
         &self,
         caller_uid: i32,
@@ -842,10 +875,10 @@
         let mock_rpc = Box::<MockRemotelyProvisionedComponent>::default();
         mock_rpc.0.lock().unwrap().hw_info.uniqueId = Some(String::from("mallory"));
 
-        let mut service: RemoteProvisioningService = Default::default();
+        let mut service: RemotelyProvisionedKeyPoolService = Default::default();
         service
-            .device_by_sec_level
-            .insert(SecurityLevel::TRUSTED_ENVIRONMENT, Strong::new(mock_rpc));
+            .unique_id_to_sec_level
+            .insert(String::from("mallory"), SecurityLevel::TRUSTED_ENVIRONMENT);
 
         assert_eq!(
             service
@@ -867,13 +900,15 @@
 
         let mock_rpc = Box::<MockRemotelyProvisionedComponent>::default();
         let mock_values = mock_rpc.0.clone();
-        let mut service: RemoteProvisioningService = Default::default();
-        service.device_by_sec_level.insert(sec_level, Strong::new(mock_rpc));
+        let mut remote_provisioning: RemoteProvisioningService = Default::default();
+        remote_provisioning.device_by_sec_level.insert(sec_level, Strong::new(mock_rpc));
+        let mut key_pool: RemotelyProvisionedKeyPoolService = Default::default();
+        key_pool.unique_id_to_sec_level.insert(String::from(irpc_id), sec_level);
 
         mock_values.lock().unwrap().hw_info.uniqueId = Some(String::from(irpc_id));
         mock_values.lock().unwrap().private_key = vec![8, 6, 7, 5, 3, 0, 9];
         mock_values.lock().unwrap().maced_public_key = generate_maced_pubkey(0x11);
-        service.generate_key_pair(&mut db, true, sec_level).unwrap();
+        remote_provisioning.generate_key_pair(&mut db, true, sec_level).unwrap();
 
         let public_key = RemoteProvisioningService::parse_cose_mac0_for_coords(
             mock_values.lock().unwrap().maced_public_key.as_slice(),
@@ -881,7 +916,7 @@
         .unwrap();
         let batch_cert = get_fake_cert();
         let certs = &[5, 6, 7, 8];
-        assert!(service
+        assert!(remote_provisioning
             .provision_cert_chain(
                 &mut db,
                 public_key.as_slice(),
@@ -893,7 +928,7 @@
             .is_ok());
 
         // ensure we got the key we expected
-        let first_key = service
+        let first_key = key_pool
             .get_attestation_key(&mut db, caller_uid, irpc_id)
             .context("get first key")
             .unwrap();
@@ -903,7 +938,7 @@
         // ensure that multiple calls get the same key
         assert_eq!(
             first_key,
-            service
+            key_pool
                 .get_attestation_key(&mut db, caller_uid, irpc_id)
                 .context("get second key")
                 .unwrap()
@@ -911,7 +946,7 @@
 
         // no more keys for new clients
         assert_eq!(
-            service
+            key_pool
                 .get_attestation_key(&mut db, caller_uid + 1, irpc_id)
                 .unwrap_err()
                 .downcast::<error::Error>()
@@ -931,19 +966,21 @@
 
         let mock_rpc = Box::<MockRemotelyProvisionedComponent>::default();
         let mock_values = mock_rpc.0.clone();
-        let mut service: RemoteProvisioningService = Default::default();
-        service.device_by_sec_level.insert(sec_level, Strong::new(mock_rpc));
+        let mut remote_provisioning: RemoteProvisioningService = Default::default();
+        remote_provisioning.device_by_sec_level.insert(sec_level, Strong::new(mock_rpc));
+        let mut key_pool: RemotelyProvisionedKeyPoolService = Default::default();
+        key_pool.unique_id_to_sec_level.insert(String::from(irpc_id), sec_level);
 
         // generate two distinct keys and provision them with certs
         mock_values.lock().unwrap().hw_info.uniqueId = Some(String::from(irpc_id));
         mock_values.lock().unwrap().private_key = vec![3, 1, 4, 1, 5];
         mock_values.lock().unwrap().maced_public_key = generate_maced_pubkey(0x11);
-        assert!(service.generate_key_pair(&mut db, true, sec_level).is_ok());
+        assert!(remote_provisioning.generate_key_pair(&mut db, true, sec_level).is_ok());
         let public_key = RemoteProvisioningService::parse_cose_mac0_for_coords(
             mock_values.lock().unwrap().maced_public_key.as_slice(),
         )
         .unwrap();
-        assert!(service
+        assert!(remote_provisioning
             .provision_cert_chain(
                 &mut db,
                 public_key.as_slice(),
@@ -957,12 +994,12 @@
         mock_values.lock().unwrap().hw_info.uniqueId = Some(String::from(irpc_id));
         mock_values.lock().unwrap().private_key = vec![9, 0, 2, 1, 0];
         mock_values.lock().unwrap().maced_public_key = generate_maced_pubkey(0x22);
-        assert!(service.generate_key_pair(&mut db, true, sec_level).is_ok());
+        assert!(remote_provisioning.generate_key_pair(&mut db, true, sec_level).is_ok());
         let public_key = RemoteProvisioningService::parse_cose_mac0_for_coords(
             mock_values.lock().unwrap().maced_public_key.as_slice(),
         )
         .unwrap();
-        assert!(service
+        assert!(remote_provisioning
             .provision_cert_chain(
                 &mut db,
                 public_key.as_slice(),
@@ -975,11 +1012,11 @@
 
         // make sure each caller gets a distinct key
         assert_ne!(
-            service
+            key_pool
                 .get_attestation_key(&mut db, first_caller, irpc_id)
                 .context("get first key")
                 .unwrap(),
-            service
+            key_pool
                 .get_attestation_key(&mut db, second_caller, irpc_id)
                 .context("get second key")
                 .unwrap()
@@ -987,22 +1024,22 @@
 
         // repeated calls should return the same key for a given caller
         assert_eq!(
-            service
+            key_pool
                 .get_attestation_key(&mut db, first_caller, irpc_id)
                 .context("first caller a")
                 .unwrap(),
-            service
+            key_pool
                 .get_attestation_key(&mut db, first_caller, irpc_id)
                 .context("first caller b")
                 .unwrap(),
         );
 
         assert_eq!(
-            service
+            key_pool
                 .get_attestation_key(&mut db, second_caller, irpc_id)
                 .context("second caller a")
                 .unwrap(),
-            service
+            key_pool
                 .get_attestation_key(&mut db, second_caller, irpc_id)
                 .context("second caller b")
                 .unwrap()
diff --git a/keystore2/src/watchdog.rs b/keystore2/src/watchdog.rs
index 9cca171..a26b632 100644
--- a/keystore2/src/watchdog.rs
+++ b/keystore2/src/watchdog.rs
@@ -111,11 +111,44 @@
         }
         self.last_report = Instant::now();
         self.has_overdue = has_overdue;
-        log::warn!("Keystore Watchdog report:");
-        log::warn!("Overdue records:");
+        log::warn!("### Keystore Watchdog report - BEGIN ###");
+
         let now = Instant::now();
-        for (i, r) in self.records.iter() {
-            if r.deadline.saturating_duration_since(now) == Duration::new(0, 0) {
+        let mut overdue_records: Vec<(&Index, &Record)> = self
+            .records
+            .iter()
+            .filter(|(_, r)| r.deadline.saturating_duration_since(now) == Duration::new(0, 0))
+            .collect();
+
+        log::warn!("When extracting from a bug report, please include this header");
+        log::warn!("and all {} records below.", overdue_records.len());
+
+        // Watch points can be nested, i.e., a single thread may have multiple armed
+        // watch points. And the most recent on each thread (thread recent) is closest to the point
+        // where something is blocked. Furthermore, keystore2 has various critical section
+        // and common backend resources KeyMint that can only be entered serialized. So if one
+        // thread hangs, the others will soon follow suite. Thus the oldest "thread recent" watch
+        // point is most likely pointing toward the culprit.
+        // Thus, sort by start time first.
+        overdue_records.sort_unstable_by(|(_, r1), (_, r2)| r1.started.cmp(&r2.started));
+        // Then we groups all of the watch points per thread preserving the order within
+        // groups.
+        let groups = overdue_records.iter().fold(
+            HashMap::<thread::ThreadId, Vec<(&Index, &Record)>>::new(),
+            |mut acc, (i, r)| {
+                acc.entry(i.tid).or_default().push((i, r));
+                acc
+            },
+        );
+        // Put the groups back into a vector.
+        let mut groups: Vec<Vec<(&Index, &Record)>> = groups.into_iter().map(|(_, v)| v).collect();
+        // Sort the groups by start time of the most recent (.last()) of each group.
+        // It is panic safe to use unwrap() here because we never add empty vectors to
+        // the map.
+        groups.sort_by(|v1, v2| v1.last().unwrap().1.started.cmp(&v2.last().unwrap().1.started));
+
+        for g in groups.iter() {
+            for (i, r) in g.iter() {
                 match &r.callback {
                     Some(cb) => {
                         log::warn!(
@@ -139,6 +172,7 @@
                 }
             }
         }
+        log::warn!("### Keystore Watchdog report - END ###");
         true
     }
 
diff --git a/ondevice-signing/odsign_main.cpp b/ondevice-signing/odsign_main.cpp
index a324857..5c541ae 100644
--- a/ondevice-signing/odsign_main.cpp
+++ b/ondevice-signing/odsign_main.cpp
@@ -53,7 +53,6 @@
 constexpr const char* kOdrefreshPath = "/apex/com.android.art/bin/odrefresh";
 constexpr const char* kCompOsVerifyPath = "/apex/com.android.compos/bin/compos_verify_key";
 constexpr const char* kFsVerityProcPath = "/proc/sys/fs/verity";
-constexpr const char* kKvmDevicePath = "/dev/kvm";
 
 constexpr bool kForceCompilation = false;
 constexpr bool kUseCompOs = true;
@@ -145,7 +144,8 @@
 }
 
 bool compOsPresent() {
-    return access(kCompOsVerifyPath, X_OK) == 0 && access(kKvmDevicePath, F_OK) == 0;
+    // We must have the CompOS APEX
+    return access(kCompOsVerifyPath, X_OK) == 0;
 }
 
 Result<void> verifyExistingRootCert(const SigningKey& key) {