Revert^2 "Microdroid: Skip instance.img checks"
Microdroid no more needs Instance Image partition if Secretkeeper is
enabled.
The use of instance.img is to store package data at first boot of the
instance & MM ensures that it did not change on further boot. With
Secretkeeper based rollback protection, the auth_hash & version of each
of these packages are part of DICE Policy & Sk ensures that the secrets
are not released if the version downgrades or auth_hash changes.
Therefore, there is no longer any need for this data to be in
instance.img
Note: Since Secretkeeper is an optional HAL in Android V, we still need
to support the instance.img for cases when Secretkeeper implementation
is not available.
Security: This opens up the Sealing CDIs of a pVM to Payload with lower
security version. But all CDIs will be reset once pvmfw starts including
Instance-Id in the hidden inputs, so this is a safe change.
Trunk Flagging: If LLPVM flag is disabled, is_sk_supported() returns
false & legacy route of verification with instance img is executed.
Bug: 291306122
Test: Get an instance.img of a pVM (started with vm run-microdroid)
Test: hexdump -C img | grep for Microdroid partition UUID. It should be missing
Re-revert: The reason for failures was a different issue(b/327526008), this patch
need not be reverted.
Change-Id: I1a8b717fe88ffe4bda5278470a3d2d5ba239c404
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index 8d2c629..9a9a1f8 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -41,7 +41,7 @@
use keystore2_crypto::ZVec;
use libc::VMADDR_CID_HOST;
use log::{error, info};
-use microdroid_metadata::PayloadMetadata;
+use microdroid_metadata::{Metadata, PayloadMetadata};
use microdroid_payload_config::{ApkConfig, OsConfig, Task, TaskType, VmPayloadConfig};
use nix::sys::signal::Signal;
use payload::load_metadata;
@@ -235,17 +235,12 @@
}
}
-fn try_run_payload(
- service: &Strong<dyn IVirtualMachineService>,
- vm_payload_service_fd: OwnedFd,
-) -> Result<i32> {
- let metadata = load_metadata().context("Failed to load payload metadata")?;
- let dice = DiceDriver::new(Path::new("/dev/open-dice0"), is_strict_boot())
- .context("Failed to load DICE")?;
-
+fn verify_payload_with_instance_img(
+ metadata: &Metadata,
+ dice: &DiceDriver,
+) -> Result<MicrodroidData> {
let mut instance = InstanceDisk::new().context("Failed to load instance.img")?;
- let saved_data =
- instance.read_microdroid_data(&dice).context("Failed to read identity data")?;
+ let saved_data = instance.read_microdroid_data(dice).context("Failed to read identity data")?;
if is_strict_boot() {
// Provisioning must happen on the first boot and never again.
@@ -265,7 +260,7 @@
}
// Verify the payload before using it.
- let extracted_data = verify_payload(&metadata, saved_data.as_ref())
+ let extracted_data = verify_payload(metadata, saved_data.as_ref())
.context("Payload verification failed")
.map_err(|e| MicrodroidError::PayloadVerificationFailed(e.to_string()))?;
@@ -289,10 +284,29 @@
} else {
info!("Saving verified data.");
instance
- .write_microdroid_data(&extracted_data, &dice)
+ .write_microdroid_data(&extracted_data, dice)
.context("Failed to write identity data")?;
extracted_data
};
+ Ok(instance_data)
+}
+
+fn try_run_payload(
+ service: &Strong<dyn IVirtualMachineService>,
+ vm_payload_service_fd: OwnedFd,
+) -> Result<i32> {
+ let metadata = load_metadata().context("Failed to load payload metadata")?;
+ let dice = DiceDriver::new(Path::new("/dev/open-dice0"), is_strict_boot())
+ .context("Failed to load DICE")?;
+
+ // TODO(b/291306122): Checking with host about Secretkeeper support multiple times introduces
+ // a whole range of security vulnerability since host can give different answers. Guest should
+ // check only once and the same answer should be known to pVM Firmware and Microdroid.
+ let instance_data = if let Some(_sk) = vm_secret::is_sk_supported(service)? {
+ verify_payload(&metadata, None)?
+ } else {
+ verify_payload_with_instance_img(&metadata, &dice)?
+ };
let payload_metadata = metadata.payload.ok_or_else(|| {
MicrodroidError::PayloadInvalidConfig("No payload config in metadata".to_string())
diff --git a/microdroid_manager/src/vm_secret.rs b/microdroid_manager/src/vm_secret.rs
index 5ceedea..7b65491 100644
--- a/microdroid_manager/src/vm_secret.rs
+++ b/microdroid_manager/src/vm_secret.rs
@@ -279,9 +279,9 @@
anyhow!("{:?}", err)
}
-// Get the secretkeeper connection if supported. Host can be consulted whether the device supports
-// secretkeeper but that should be used with caution for protected VM.
-fn is_sk_supported(
+/// Get the secretkeeper connection if supported. Host can be consulted whether the device supports
+/// secretkeeper but that should be used with caution for protected VM.
+pub fn is_sk_supported(
host: &Strong<dyn IVirtualMachineService>,
) -> Result<Option<Strong<dyn ISecretkeeper>>> {
let sk = if cfg!(llpvm_changes) {