Merge "MM: Check if Secretkeeper is supported from DT" into main
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index 9a9a1f8..2386bd4 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -72,6 +72,7 @@
"/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 DEFER_ROLLBACK_PROTECTION: &str = "/proc/device-tree/avf/untrusted/defer-rollback-protection";
const ENCRYPTEDSTORE_BIN: &str = "/system/bin/encryptedstore";
const ZIPFUSE_BIN: &str = "/system/bin/zipfuse";
@@ -161,6 +162,10 @@
Ok(instance_id)
}
+fn should_defer_rollback_protection() -> bool {
+ Path::new(DEFER_ROLLBACK_PROTECTION).exists()
+}
+
fn main() -> Result<()> {
// If debuggable, print full backtrace to console log with stdio_to_kmsg
if is_debuggable()? {
@@ -299,10 +304,10 @@
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)? {
+ // Microdroid skips checking payload against instance image iff the device supports
+ // secretkeeper. In that case Microdroid use VmSecret::V2, which provide protection against
+ // rollback of boot images and packages.
+ let instance_data = if should_defer_rollback_protection() {
verify_payload(&metadata, None)?
} else {
verify_payload_with_instance_img(&metadata, &dice)?
diff --git a/microdroid_manager/src/vm_secret.rs b/microdroid_manager/src/vm_secret.rs
index 7b65491..ed8ab1d 100644
--- a/microdroid_manager/src/vm_secret.rs
+++ b/microdroid_manager/src/vm_secret.rs
@@ -91,28 +91,19 @@
vm_service: &Strong<dyn IVirtualMachineService>,
) -> Result<Self> {
ensure!(dice_artifacts.bcc().is_some(), "Dice chain missing");
-
- let Some(sk_service) =
- is_sk_supported(vm_service).context("Failed to check if Secretkeeper is supported")?
- else {
- // Use V1 secrets if Secretkeeper is not supported.
+ if !crate::should_defer_rollback_protection() {
return Ok(Self::V1 { dice_artifacts });
- };
+ }
let explicit_dice = OwnedDiceArtifactsWithExplicitKey::from_owned_artifacts(dice_artifacts)
.context("Failed to get Dice artifacts in explicit key format")?;
// 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().context("Failed to get secretkeeper identity")?),
- )
- .context("Failed to setup a Secretkeeper session")?;
- let id = super::get_instance_id()
- .context("Failed to get instance-id")?
- .ok_or(anyhow!("Missing instance-id"))?;
+ let sk_service = get_secretkeeper_service(vm_service)?;
+ 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"))?;
@@ -279,22 +270,15 @@
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.
-pub fn is_sk_supported(
+fn get_secretkeeper_service(
host: &Strong<dyn IVirtualMachineService>,
-) -> Result<Option<Strong<dyn ISecretkeeper>>> {
- let sk = if cfg!(llpvm_changes) {
- host.getSecretkeeper()
- // TODO rename this error!
- .map_err(|e| {
- super::MicrodroidError::FailedToConnectToVirtualizationService(format!(
- "Failed to get Secretkeeper: {e:?}"
- ))
- })?
- } else {
- // LLPVM flag is disabled
- None
- };
- Ok(sk)
+) -> Result<Strong<dyn ISecretkeeper>> {
+ Ok(host
+ .getSecretkeeper()
+ // TODO rename this error!
+ .map_err(|e| {
+ super::MicrodroidError::FailedToConnectToVirtualizationService(format!(
+ "Failed to get Secretkeeper: {e:?}"
+ ))
+ })?)
}
diff --git a/virtualizationmanager/src/aidl.rs b/virtualizationmanager/src/aidl.rs
index 22bea58..6be219e 100644
--- a/virtualizationmanager/src/aidl.rs
+++ b/virtualizationmanager/src/aidl.rs
@@ -49,7 +49,7 @@
use android_system_virtualmachineservice::aidl::android::system::virtualmachineservice::IVirtualMachineService::{
BnVirtualMachineService, IVirtualMachineService,
};
-use android_hardware_security_secretkeeper::aidl::android::hardware::security::secretkeeper::ISecretkeeper::ISecretkeeper;
+use android_hardware_security_secretkeeper::aidl::android::hardware::security::secretkeeper::ISecretkeeper::{BnSecretkeeper, ISecretkeeper};
use android_hardware_security_secretkeeper::aidl::android::hardware::security::secretkeeper::SecretId::SecretId;
use android_hardware_security_authgraph::aidl::android::hardware::security::authgraph::{
Arc::Arc as AuthgraphArc, IAuthGraphKeyExchange::IAuthGraphKeyExchange,
@@ -1501,11 +1501,12 @@
}
}
- fn getSecretkeeper(&self) -> binder::Result<Option<Strong<dyn ISecretkeeper>>> {
- // TODO(b/327526008): Session establishment wth secretkeeper is failing.
- // Re-enable this when fixed.
- let _sk_supported = is_secretkeeper_supported();
- Ok(None)
+ fn getSecretkeeper(&self) -> binder::Result<Strong<dyn ISecretkeeper>> {
+ if !is_secretkeeper_supported() {
+ return Err(StatusCode::NAME_NOT_FOUND)?;
+ }
+ let sk = binder::wait_for_interface(SECRETKEEPER_IDENTIFIER)?;
+ Ok(BnSecretkeeper::new_binder(SecretkeeperProxy(sk), BinderFeatures::default()))
}
fn requestAttestation(&self, csr: &[u8], test_mode: bool) -> binder::Result<Vec<Certificate>> {
@@ -1514,8 +1515,11 @@
}
fn is_secretkeeper_supported() -> bool {
- binder::is_declared(SECRETKEEPER_IDENTIFIER)
- .expect("Could not check for declared Secretkeeper interface")
+ // TODO(b/327526008): Session establishment wth secretkeeper is failing.
+ // Re-enable this when fixed.
+ let _sk_supported = binder::is_declared(SECRETKEEPER_IDENTIFIER)
+ .expect("Could not check for declared Secretkeeper interface");
+ false
}
impl VirtualMachineService {
diff --git a/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl b/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
index 6806a5c..662c8f1 100644
--- a/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
+++ b/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
@@ -58,9 +58,9 @@
Certificate[] requestAttestation(in byte[] csr, in boolean testMode);
/**
- * Request connection to Secretkeeper. This is used by pVM to store Anti-Rollback protected
- * secrets. Note that the return value is nullable to reflect that Secretkeeper HAL may not be
- * present.
+ * Request connection to Secretkeeper. This is used by pVM to store rollback protected secrets.
+ * Note that this returns error if Secretkeeper is not supported on device. Guest should check
+ * that Secretkeeper is supported from Linux device tree before calling this.
*/
- @nullable ISecretkeeper getSecretkeeper();
+ ISecretkeeper getSecretkeeper();
}