Sk Maintenance: payload's rollback protected data
Microdroid now allows payload to (indirectly) store a Secretkeeper
entry. This needs to be plugged into the maintenance database that the
VS keeps. This allows deletion of such entries when the app is
uninstalled.
Test: Builds
Test: logcat -e "Claiming Secretkeeper entry"
Bug: 389631490
Change-Id: I412cd2b0a2ee6f5eabeea700b6b88e830a5f20d9
diff --git a/android/virtmgr/src/aidl.rs b/android/virtmgr/src/aidl.rs
index 1a263bd..5327635 100644
--- a/android/virtmgr/src/aidl.rs
+++ b/android/virtmgr/src/aidl.rs
@@ -2210,6 +2210,10 @@
fn requestAttestation(&self, csr: &[u8], test_mode: bool) -> binder::Result<Vec<Certificate>> {
GLOBAL_SERVICE.requestAttestation(csr, get_calling_uid() as i32, test_mode)
}
+
+ fn claimSecretkeeperEntry(&self, id: &[u8; 64]) -> binder::Result<()> {
+ GLOBAL_SERVICE.claimSecretkeeperEntry(id)
+ }
}
fn is_secretkeeper_supported() -> bool {
diff --git a/android/virtualizationservice/aidl/android/system/virtualizationservice_internal/IVirtualizationServiceInternal.aidl b/android/virtualizationservice/aidl/android/system/virtualizationservice_internal/IVirtualizationServiceInternal.aidl
index 0da7755..4f549cb 100644
--- a/android/virtualizationservice/aidl/android/system/virtualizationservice_internal/IVirtualizationServiceInternal.aidl
+++ b/android/virtualizationservice/aidl/android/system/virtualizationservice_internal/IVirtualizationServiceInternal.aidl
@@ -134,4 +134,10 @@
* @param file descriptor of the TAP network interface.
*/
void deleteTapInterface(in ParcelFileDescriptor tapFd);
+
+ /**
+ * Account the caller for the corresponding Secretkeeper entry.
+ * @param id Identifier for the secret held in Secretkeeper for the caller
+ */
+ void claimSecretkeeperEntry(in byte[64] id);
}
diff --git a/android/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl b/android/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
index 662c8f1..7fe11e9 100644
--- a/android/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
+++ b/android/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
@@ -63,4 +63,10 @@
* that Secretkeeper is supported from Linux device tree before calling this.
*/
ISecretkeeper getSecretkeeper();
+
+ /**
+ * Account the caller for the corresponding Secretkeeper entry.
+ * @param id Identifier for the secret held in Secretkeeper for the caller
+ */
+ void claimSecretkeeperEntry(in byte[64] id);
}
diff --git a/android/virtualizationservice/src/aidl.rs b/android/virtualizationservice/src/aidl.rs
index f4e64e3..62cede8 100644
--- a/android/virtualizationservice/src/aidl.rs
+++ b/android/virtualizationservice/src/aidl.rs
@@ -196,6 +196,28 @@
service
}
+
+ // Attempt to update the sk_state maintenance database. Errors are ignored - calling app
+ // can not really do much to fix the errors & letting AVF VMs run irrespective of such internal
+ // error is acceptable.
+ fn try_updating_sk_state(&self, id: &[u8; 64]) {
+ let state = &mut *self.state.lock().unwrap();
+ if let Some(sk_state) = &mut state.sk_state {
+ let uid = get_calling_uid();
+ let user_id = multiuser_get_user_id(uid);
+ let app_id = multiuser_get_app_id(uid);
+ info!(
+ "Recording possible new owner of Secretkeeper entry={:?}:
+ (user_id={user_id}, app_id={app_id},)",
+ hex::encode(id)
+ );
+ if let Err(e) = sk_state.add_id(id, user_id, app_id) {
+ error!("Failed to update the Secretkeeper entry owner: {e:?}");
+ }
+ } else {
+ info!("ignoring update of Secretkeeper entry as no ISecretkeeper");
+ }
+ }
}
impl Interface for VirtualizationServiceInternal {}
@@ -467,16 +489,7 @@
.or_service_specific_exception(-1)?;
let uid = get_calling_uid();
info!("Allocated a VM's instance_id: {:?}..., for uid: {:?}", &hex::encode(id)[..8], uid);
- let state = &mut *self.state.lock().unwrap();
- if let Some(sk_state) = &mut state.sk_state {
- let user_id = multiuser_get_user_id(uid);
- let app_id = multiuser_get_app_id(uid);
- info!("Recording possible existence of state for (user_id={user_id}, app_id={app_id})");
- if let Err(e) = sk_state.add_id(&id, user_id, app_id) {
- error!("Failed to record the instance_id: {e:?}");
- }
- }
-
+ self.try_updating_sk_state(&id);
Ok(id)
}
@@ -500,24 +513,8 @@
}
fn claimVmInstance(&self, instance_id: &[u8; 64]) -> binder::Result<()> {
- let state = &mut *self.state.lock().unwrap();
- if let Some(sk_state) = &mut state.sk_state {
- let uid = get_calling_uid();
- info!(
- "Claiming a VM's instance_id: {:?}, for uid: {:?}",
- hex::encode(instance_id),
- uid
- );
-
- let user_id = multiuser_get_user_id(uid);
- let app_id = multiuser_get_app_id(uid);
- info!("Recording possible new owner of state for (user_id={user_id}, app_id={app_id})");
- if let Err(e) = sk_state.add_id(instance_id, user_id, app_id) {
- error!("Failed to update the instance_id owner: {e:?}");
- }
- } else {
- info!("ignoring claimVmInstance() as no ISecretkeeper");
- }
+ info!("Claiming a VM's instance_id: {:?}", hex::encode(instance_id));
+ self.try_updating_sk_state(instance_id);
Ok(())
}
@@ -559,6 +556,12 @@
NETWORK_SERVICE.deleteTapInterface(tap_fd)
}
+
+ fn claimSecretkeeperEntry(&self, id: &[u8; 64]) -> binder::Result<()> {
+ info!("Claiming Secretkeeper entry: {:?}", hex::encode(id));
+ self.try_updating_sk_state(id);
+ Ok(())
+ }
}
impl IVirtualizationMaintenance for VirtualizationServiceInternal {
diff --git a/guest/microdroid_manager/src/vm_secret.rs b/guest/microdroid_manager/src/vm_secret.rs
index 6331074..f031859 100644
--- a/guest/microdroid_manager/src/vm_secret.rs
+++ b/guest/microdroid_manager/src/vm_secret.rs
@@ -77,6 +77,7 @@
dice_artifacts: OwnedDiceArtifactsWithExplicitKey,
skp_secret: ZVec,
secretkeeper_session: SkVmSession,
+ virtual_machine_service: Strong<dyn IVirtualMachineService>,
},
// V1 secrets are not protected against rollback of boot images.
// They are reliable only if rollback of images was prevented by verified boot ie,
@@ -131,6 +132,7 @@
dice_artifacts: explicit_dice,
skp_secret: ZVec::try_from(skp_secret.to_vec())?,
secretkeeper_session: session,
+ virtual_machine_service: vm_service.clone(),
})
}
@@ -180,10 +182,20 @@
pub fn write_payload_data_rp(&self, data: &[u8; SECRET_SIZE]) -> Result<()> {
let data = Zeroizing::new(*data);
- let Self::V2 { instance_id, secretkeeper_session, .. } = self else {
+ let Self::V2 { instance_id, secretkeeper_session, virtual_machine_service, .. } = self
+ else {
return Err(anyhow!("Rollback protected data is not available with V1 secrets"));
};
let payload_id = sha::sha512(instance_id);
+ // Claim the Secretkeeper entry - this pings AVF host to account this Secretkeeper entry
+ // correctly.
+ virtual_machine_service.claimSecretkeeperEntry(&payload_id).map_err(|e| {
+ // TODO rename this error!
+ super::MicrodroidError::FailedToConnectToVirtualizationService(format!(
+ "Failed to claim Secretkeeper entry: {e:?}"
+ ))
+ })?;
+
if let Err(e) = secretkeeper_session.store_secret(payload_id, data.clone()) {
log::info!("Secretkeeper store failed with {e:?}. Refreshing connection & retrying!");
secretkeeper_session.refresh()?;