Merge changes from topic "udroid-instance-img" into main
* changes:
Revert^2 "Make salt in Microdroid's instance img obsolete"
Revert^2 "Microdroid: Skip instance.img checks"
diff --git a/microdroid_manager/src/dice.rs b/microdroid_manager/src/dice.rs
index 7f65159..cecf413 100644
--- a/microdroid_manager/src/dice.rs
+++ b/microdroid_manager/src/dice.rs
@@ -13,12 +13,12 @@
// limitations under the License.
use crate::instance::{ApexData, ApkData};
-use crate::{is_debuggable, MicrodroidData};
+use crate::{is_debuggable, is_strict_boot, MicrodroidData};
use anyhow::{bail, Context, Result};
use ciborium::{cbor, Value};
use coset::CborSerializable;
use dice_driver::DiceDriver;
-use diced_open_dice::OwnedDiceArtifacts;
+use diced_open_dice::{Hidden, OwnedDiceArtifacts, HIDDEN_SIZE};
use microdroid_metadata::PayloadMetadata;
use openssl::sha::{sha512, Sha512};
use std::iter::once;
@@ -53,10 +53,37 @@
let debuggable = is_debuggable()?;
// Send the details to diced
- let hidden = instance_data.salt.clone().try_into().unwrap();
+ let hidden = if cfg!(llpvm_changes) {
+ hidden_input_from_instance_id()?
+ } else {
+ instance_data.salt.clone().try_into().unwrap()
+ };
dice.derive(code_hash, &config_descriptor, authority_hash, debuggable, hidden)
}
+// Get the "Hidden input" for DICE derivation.
+// This provides differentiation of secrets for different VM instances with same payload.
+fn hidden_input_from_instance_id() -> Result<Hidden> {
+ // For protected VM: this is all 0s, pvmfw ensures differentiation is added early in secrets.
+ // For non-protected VM: this is derived from instance_id of the VM instance.
+ let hidden_input = if !is_strict_boot() {
+ if let Some(id) = super::get_instance_id()? {
+ sha512(&id)
+ } else {
+ // TODO(b/325094712): Absence of instance_id occurs due to missing DT in some
+ // x86_64 test devices (such as Cuttlefish). From security perspective, this is
+ // acceptable for non-protected VM.
+ log::warn!(
+ "Instance Id missing, this may lead to 2 non protected VMs having same secrets"
+ );
+ [0u8; HIDDEN_SIZE]
+ }
+ } else {
+ [0u8; HIDDEN_SIZE]
+ };
+ Ok(hidden_input)
+}
+
struct Subcomponent {
name: String,
version: u64,
diff --git a/microdroid_manager/src/instance.rs b/microdroid_manager/src/instance.rs
index 888c451..2d39cd8 100644
--- a/microdroid_manager/src/instance.rs
+++ b/microdroid_manager/src/instance.rs
@@ -273,6 +273,8 @@
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
pub struct MicrodroidData {
+ // `salt` is obsolete, it was used as a differentiator for non-protected VM instances running
+ // same payload. Instance-id (present in DT) is used for that now.
pub salt: Vec<u8>, // Should be [u8; 64] but that isn't serializable.
pub apk_data: ApkData,
pub extra_apks_data: Vec<ApkData>,
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/verify.rs b/microdroid_manager/src/verify.rs
index 445c1ae..65c32b0 100644
--- a/microdroid_manager/src/verify.rs
+++ b/microdroid_manager/src/verify.rs
@@ -169,13 +169,14 @@
// verified is consistent with the root hash) or because we have the saved APK data which will
// be checked as identical to the data we have verified.
- // Use the salt from a verified instance, or generate a salt for a new instance.
- let salt = if let Some(saved_data) = saved_data {
- saved_data.salt.clone()
- } else if is_strict_boot() {
- // No need to add more entropy as a previous stage must have used a new, random salt.
+ let salt = if cfg!(llpvm_changes) || is_strict_boot() {
+ // Salt is obsolete with llpvm_changes.
vec![0u8; 64]
+ } else if let Some(saved_data) = saved_data {
+ // Use the salt from a verified instance.
+ saved_data.salt.clone()
} else {
+ // Generate a salt for a new instance.
let mut salt = vec![0u8; 64];
salt.as_mut_slice().try_fill(&mut rand::thread_rng())?;
salt
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) {