Merge "pVMs to use Sk for rollback protected secrets" into main
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index 86284a5..0d67632 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -45,8 +45,6 @@
 use microdroid_metadata::PayloadMetadata;
 use microdroid_payload_config::{ApkConfig, OsConfig, Task, TaskType, VmPayloadConfig};
 use nix::sys::signal::Signal;
-use openssl::hkdf::hkdf;
-use openssl::md::Md;
 use payload::load_metadata;
 use rpcbinder::RpcSession;
 use rustutils::sockets::android_get_control_socket;
@@ -73,6 +71,8 @@
 const AVF_DEBUG_POLICY_RAMDUMP: &str = "/proc/device-tree/avf/guest/common/ramdump";
 const DEBUG_MICRODROID_NO_VERIFIED_BOOT: &str =
     "/proc/device-tree/virtualization/guest/debug-microdroid,no-verified-boot";
+const SECRETKEEPER_KEY: &str = "/proc/device-tree/avf/secretkeeper_public_key";
+const INSTANCE_ID_PATH: &str = "/proc/device-tree/avf/untrusted/instance-id";
 
 const ENCRYPTEDSTORE_BIN: &str = "/system/bin/encryptedstore";
 const ZIPFUSE_BIN: &str = "/system/bin/zipfuse";
@@ -145,6 +145,23 @@
     Ok(())
 }
 
+/// The (host allocated) instance_id can be found at node /avf/untrusted/ in the device tree.
+fn get_instance_id() -> Result<Option<[u8; ID_SIZE]>> {
+    let path = Path::new(INSTANCE_ID_PATH);
+    let instance_id = if path.exists() {
+        Some(
+            fs::read(path)?
+                .try_into()
+                .map_err(|x: Vec<_>| anyhow!("Expected {ID_SIZE} bytes, found {:?}", x.len()))?,
+        )
+    } else {
+        // TODO(b/325094712): x86 support for Device tree in nested guest is limited/broken/
+        // untested. So instance_id will not be present in cuttlefish.
+        None
+    };
+    Ok(instance_id)
+}
+
 fn main() -> Result<()> {
     // If debuggable, print full backtrace to console log with stdio_to_kmsg
     if is_debuggable()? {
@@ -284,19 +301,8 @@
     // To minimize the exposure to untrusted data, derive dice profile as soon as possible.
     info!("DICE derivation for payload");
     let dice_artifacts = dice_derivation(dice, &instance_data, &payload_metadata)?;
-    // TODO(b/291213394): This will be the Id for non-pVM only, instance_data.salt is all 0
-    // for protected VM, implement a mechanism for pVM!
-    let mut vm_id = [0u8; ID_SIZE];
-    hkdf(
-        &mut vm_id,
-        Md::sha256(),
-        &instance_data.salt,
-        /* salt */ b"",
-        /* info */ b"VM_ID",
-    )
-    .context("hkdf failed")?;
     let vm_secret =
-        VmSecret::new(vm_id, dice_artifacts, service).context("Failed to create VM secrets")?;
+        VmSecret::new(dice_artifacts, service).context("Failed to create VM secrets")?;
 
     if cfg!(dice_changes) {
         // Now that the DICE derivation is done, it's ok to allow payload code to run.
diff --git a/microdroid_manager/src/vm_secret.rs b/microdroid_manager/src/vm_secret.rs
index 7df7cc9..4ead211 100644
--- a/microdroid_manager/src/vm_secret.rs
+++ b/microdroid_manager/src/vm_secret.rs
@@ -19,7 +19,7 @@
 use android_hardware_security_secretkeeper::aidl::android::hardware::security::secretkeeper::ISecretkeeper::ISecretkeeper;
 use secretkeeper_comm::data_types::request::Request;
 use binder::{Strong};
-use coset::CborSerializable;
+use coset::{CoseKey, CborSerializable, CborOrdering};
 use dice_policy_builder::{CertIndex, ConstraintSpec, ConstraintType, policy_for_dice_chain, MissingAction, WILDCARD_FULL_ARRAY};
 use diced_open_dice::{DiceArtifacts, OwnedDiceArtifacts};
 use keystore2_crypto::ZVec;
@@ -34,6 +34,7 @@
 use secretkeeper_comm::data_types::request_response_impl::{
     StoreSecretRequest, GetSecretResponse, GetSecretRequest};
 use secretkeeper_comm::data_types::error::SecretkeeperError;
+use std::fs;
 use zeroize::Zeroizing;
 
 const ENCRYPTEDSTORE_KEY_IDENTIFIER: &str = "encryptedstore_key";
@@ -58,11 +59,6 @@
     0x55, 0xF8, 0x08, 0x23, 0x81, 0x5F, 0xF5, 0x16, 0x20, 0x3E, 0xBE, 0xBA, 0xB7, 0xA8, 0x43, 0x92,
 ];
 
-const SKP_SECRET_NP_VM: [u8; SECRET_SIZE] = [
-    0xA9, 0x89, 0x97, 0xFE, 0xAE, 0x97, 0x55, 0x4B, 0x32, 0x35, 0xF0, 0xE8, 0x93, 0xDA, 0xEA, 0x24,
-    0x06, 0xAC, 0x36, 0x8B, 0x3C, 0x95, 0x50, 0x16, 0x67, 0x71, 0x65, 0x26, 0xEB, 0xD0, 0xC3, 0x98,
-];
-
 pub enum VmSecret {
     // V2 secrets are derived from 2 independently secured secrets:
     //      1. Secretkeeper protected secrets (skp secret).
@@ -81,9 +77,16 @@
     V1 { dice_artifacts: OwnedDiceArtifacts },
 }
 
+// For supporting V2 secrets, guest expects the public key to be present in the Linux device tree.
+fn get_secretkeeper_identity() -> Result<CoseKey> {
+    let key = fs::read(super::SECRETKEEPER_KEY)?;
+    let mut key = CoseKey::from_slice(&key)?;
+    key.canonicalize(CborOrdering::Lexicographic);
+    Ok(key)
+}
+
 impl VmSecret {
     pub fn new(
-        id: [u8; ID_SIZE],
         dice_artifacts: OwnedDiceArtifacts,
         vm_service: &Strong<dyn IVirtualMachineService>,
     ) -> Result<Self> {
@@ -93,29 +96,27 @@
             // Use V1 secrets if Secretkeeper is not supported.
             return Ok(Self::V1 { dice_artifacts });
         };
+
         let explicit_dice =
             OwnedDiceArtifactsWithExplicitKey::from_owned_artifacts(dice_artifacts)?;
-        let explicit_dice_chain = explicit_dice
-            .explicit_key_dice_chain()
-            .ok_or(anyhow!("Missing explicit dice chain, this is unusual"))?;
-        let policy = sealing_policy(explicit_dice_chain).map_err(anyhow_err)?;
-
-        // Start a new session with Secretkeeper!
-        let mut session = SkSession::new(sk_service, &explicit_dice, None)?;
+        // For pVM, skp_secret are stored in Secretkeeper. For non-protected it is all 0s.
         let mut skp_secret = Zeroizing::new([0u8; SECRET_SIZE]);
         if super::is_strict_boot() {
+            let mut session =
+                SkSession::new(sk_service, &explicit_dice, Some(get_secretkeeper_identity()?))?;
+            let id = super::get_instance_id()?.ok_or(anyhow!("Missing instance_id"))?;
+            let explicit_dice_chain = explicit_dice
+                .explicit_key_dice_chain()
+                .ok_or(anyhow!("Missing explicit dice chain, this is unusual"))?;
+            let policy = sealing_policy(explicit_dice_chain).map_err(anyhow_err)?;
             if super::is_new_instance() {
+                // New instance -> create a secret & store in Secretkeeper.
                 *skp_secret = rand::random();
                 store_secret(&mut session, id, skp_secret.clone(), policy)?;
             } else {
                 // Subsequent run of the pVM -> get the secret stored in Secretkeeper.
                 *skp_secret = get_secret(&mut session, id, Some(policy))?;
             }
-        } else {
-            // TODO(b/291213394): Non protected VM don't need to use Secretkeeper, remove this
-            // once we have sufficient testing on protected VM.
-            store_secret(&mut session, id, SKP_SECRET_NP_VM.into(), policy)?;
-            *skp_secret = get_secret(&mut session, id, None)?;
         }
         Ok(Self::V2 {
             dice_artifacts: explicit_dice,
@@ -273,17 +274,13 @@
     host: &Strong<dyn IVirtualMachineService>,
 ) -> Result<Option<Strong<dyn ISecretkeeper>>> {
     let sk = if cfg!(llpvm_changes) {
-        if super::is_strict_boot() {
-            // TODO: For protected VM check for Secretkeeper authentication data in device tree.
-            None
-        } else {
-            // For non-protected VM, believe what host claims.
-            host.getSecretkeeper()
-                // TODO rename this error!
-                .map_err(|e| {
-                    super::MicrodroidError::FailedToConnectToVirtualizationService(e.to_string())
-                })?
-        }
+        host.getSecretkeeper()
+            // TODO rename this error!
+            .map_err(|e| {
+                super::MicrodroidError::FailedToConnectToVirtualizationService(format!(
+                    "Failed to get Secretkeeper: {e:?}"
+                ))
+            })?
     } else {
         // LLPVM flag is disabled
         None