Revert "Make salt in Microdroid's instance img obsolete"
Revert submission 2982081
Reason for revert: There are tests failing which needs modification. Revert for immediate mitigation.
Reverted changes: /q/submissionid:2982081
Change-Id: I3e55e75e5810b06829ce66149516204e5dc81ff1
diff --git a/microdroid_manager/src/dice.rs b/microdroid_manager/src/dice.rs
index 2469325..a8b88aa 100644
--- a/microdroid_manager/src/dice.rs
+++ b/microdroid_manager/src/dice.rs
@@ -14,11 +14,11 @@
use crate::dice_driver::DiceDriver;
use crate::instance::{ApexData, ApkData};
-use crate::{is_debuggable, is_strict_boot, MicrodroidData};
+use crate::{is_debuggable, MicrodroidData};
use anyhow::{bail, Context, Result};
use ciborium::{cbor, Value};
use coset::CborSerializable;
-use diced_open_dice::{Hidden, OwnedDiceArtifacts, HIDDEN_SIZE};
+use diced_open_dice::OwnedDiceArtifacts;
use microdroid_metadata::PayloadMetadata;
use openssl::sha::{sha512, Sha512};
use std::iter::once;
@@ -53,37 +53,10 @@
let debuggable = is_debuggable()?;
// Send the details to diced
- let hidden = if cfg!(llpvm_changes) {
- hidden_input_from_instance_id()?
- } else {
- instance_data.salt.clone().try_into().unwrap()
- };
+ let hidden = 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 f42b86d..7a9f0e0 100644
--- a/microdroid_manager/src/instance.rs
+++ b/microdroid_manager/src/instance.rs
@@ -273,8 +273,6 @@
#[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/verify.rs b/microdroid_manager/src/verify.rs
index 65c32b0..445c1ae 100644
--- a/microdroid_manager/src/verify.rs
+++ b/microdroid_manager/src/verify.rs
@@ -169,14 +169,13 @@
// 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.
- 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.
+ // 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.
+ vec![0u8; 64]
} 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