Snap for 11914274 from 7904777563f45d3f1b30254ff2dffcc8d1dc4c12 to 24Q3-release
Change-Id: I9d891b53b6a8c8f8b9a2c550bea7b64917b4c3d2
diff --git a/pvmfw/Android.bp b/pvmfw/Android.bp
index 769a955..144e81e 100644
--- a/pvmfw/Android.bp
+++ b/pvmfw/Android.bp
@@ -117,9 +117,10 @@
rustlibs: [
"libcbor_util",
"libciborium",
- "libdiced_open_dice_nostd",
+ "libdiced_open_dice",
"libpvmfw_avb_nostd",
"libzerocopy_nostd",
+ "libhex",
],
}
@@ -320,15 +321,22 @@
installable: false,
}
-prebuilt_etc {
+filegroup {
name: "pvmfw_embedded_key",
- src: ":avb_testkey_rsa4096_pub_bin",
- installable: false,
+ srcs: [":avb_testkey_rsa4096"],
+}
+
+genrule {
+ name: "pvmfw_embedded_key_pub_bin",
+ tools: ["avbtool"],
+ srcs: [":pvmfw_embedded_key"],
+ out: ["pvmfw_embedded_key_pub.bin"],
+ cmd: "$(location avbtool) extract_public_key --key $(in) --output $(out)",
}
genrule {
name: "pvmfw_embedded_key_rs",
- srcs: [":pvmfw_embedded_key"],
+ srcs: [":pvmfw_embedded_key_pub_bin"],
out: ["lib.rs"],
cmd: "(" +
" echo '#![no_std]';" +
diff --git a/pvmfw/src/dice.rs b/pvmfw/src/dice.rs
index aaf2691..da19931 100644
--- a/pvmfw/src/dice.rs
+++ b/pvmfw/src/dice.rs
@@ -71,6 +71,7 @@
Ok(hash(&digests)?)
}
+#[derive(Clone)]
pub struct PartialInputs {
pub code_hash: Hash,
pub auth_hash: Hash,
@@ -96,6 +97,7 @@
current_bcc_handover: &[u8],
salt: &[u8; HIDDEN_SIZE],
instance_hash: Option<Hash>,
+ deferred_rollback_protection: bool,
next_bcc: &mut [u8],
) -> Result<()> {
let config = self
@@ -107,16 +109,23 @@
Config::Descriptor(&config),
self.auth_hash,
self.mode,
- self.make_hidden(salt)?,
+ self.make_hidden(salt, deferred_rollback_protection)?,
);
let _ = bcc_handover_main_flow(current_bcc_handover, &dice_inputs, next_bcc)?;
Ok(())
}
- fn make_hidden(&self, salt: &[u8; HIDDEN_SIZE]) -> Result<[u8; HIDDEN_SIZE]> {
+ fn make_hidden(
+ &self,
+ salt: &[u8; HIDDEN_SIZE],
+ deferred_rollback_protection: bool,
+ ) -> diced_open_dice::Result<[u8; HIDDEN_SIZE]> {
// We want to make sure we get a different sealing CDI for:
// - VMs with different salt values
// - An RKP VM and any other VM (regardless of salt)
+ // - depending on whether rollback protection has been deferred to payload. This ensures the
+ // adversary cannot leak the secrets by using old images & setting
+ // `deferred_rollback_protection` to true.
// The hidden input for DICE affects the sealing CDI (but the values in the config
// descriptor do not).
// Since the hidden input has to be a fixed size, create it as a hash of the values we
@@ -126,10 +135,16 @@
struct HiddenInput {
rkp_vm_marker: bool,
salt: [u8; HIDDEN_SIZE],
+ deferred_rollback_protection: bool,
}
- // TODO(b/291213394): Include `defer_rollback_protection` flag in the Hidden Input to
- // differentiate the secrets in both cases.
- Ok(hash(HiddenInput { rkp_vm_marker: self.rkp_vm_marker, salt: *salt }.as_bytes())?)
+ hash(
+ HiddenInput {
+ rkp_vm_marker: self.rkp_vm_marker,
+ salt: *salt,
+ deferred_rollback_protection,
+ }
+ .as_bytes(),
+ )
}
fn generate_config_descriptor(&self, instance_hash: Option<Hash>) -> Result<Vec<u8>> {
@@ -176,9 +191,20 @@
#[cfg(test)]
mod tests {
- use super::*;
+ use crate::{
+ Hash, PartialInputs, COMPONENT_NAME_KEY, INSTANCE_HASH_KEY, RKP_VM_MARKER_KEY,
+ SECURITY_VERSION_KEY,
+ };
use ciborium::Value;
+ use diced_open_dice::DiceArtifacts;
+ use diced_open_dice::DiceMode;
+ use diced_open_dice::HIDDEN_SIZE;
+ use pvmfw_avb::Capability;
+ use pvmfw_avb::DebugLevel;
+ use pvmfw_avb::Digest;
+ use pvmfw_avb::VerifiedBootData;
use std::collections::HashMap;
+ use std::mem::size_of;
use std::vec;
const COMPONENT_VERSION_KEY: i64 = -70003;
@@ -284,4 +310,67 @@
.map(|(k, v)| ((k.into_integer().unwrap().try_into().unwrap()), v))
.collect()
}
+
+ #[test]
+ fn changing_deferred_rpb_changes_secrets() {
+ let vb_data = VerifiedBootData { debug_level: DebugLevel::Full, ..BASE_VB_DATA };
+ let inputs = PartialInputs::new(&vb_data).unwrap();
+ let mut buffer_without_defer = [0; 4096];
+ let mut buffer_with_defer = [0; 4096];
+ let mut buffer_without_defer_retry = [0; 4096];
+
+ let sample_dice_input: &[u8] = &[
+ 0xa3, // CDI attest
+ 0x01, 0x58, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // CDI seal
+ 0x02, 0x58, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // DICE chain
+ 0x03, 0x82, 0xa6, 0x01, 0x02, 0x03, 0x27, 0x04, 0x02, 0x20, 0x01, 0x21, 0x40, 0x22,
+ 0x40, 0x84, 0x40, 0xa0, 0x40, 0x40,
+ // 8-bytes of trailing data that aren't part of the DICE chain.
+ 0x84, 0x41, 0x55, 0xa0, 0x42, 0x11, 0x22, 0x40,
+ ];
+
+ inputs
+ .clone()
+ .write_next_bcc(
+ sample_dice_input,
+ &[0u8; HIDDEN_SIZE],
+ Some([0u8; 64]),
+ false,
+ &mut buffer_without_defer,
+ )
+ .unwrap();
+ let bcc_handover1 = diced_open_dice::bcc_handover_parse(&buffer_without_defer).unwrap();
+
+ inputs
+ .clone()
+ .write_next_bcc(
+ sample_dice_input,
+ &[0u8; HIDDEN_SIZE],
+ Some([0u8; 64]),
+ true,
+ &mut buffer_with_defer,
+ )
+ .unwrap();
+ let bcc_handover2 = diced_open_dice::bcc_handover_parse(&buffer_with_defer).unwrap();
+
+ inputs
+ .clone()
+ .write_next_bcc(
+ sample_dice_input,
+ &[0u8; HIDDEN_SIZE],
+ Some([0u8; 64]),
+ false,
+ &mut buffer_without_defer_retry,
+ )
+ .unwrap();
+ let bcc_handover3 =
+ diced_open_dice::bcc_handover_parse(&buffer_without_defer_retry).unwrap();
+
+ assert_ne!(bcc_handover1.cdi_seal(), bcc_handover2.cdi_seal());
+ assert_eq!(bcc_handover1.cdi_seal(), bcc_handover3.cdi_seal());
+ }
}
diff --git a/pvmfw/src/main.rs b/pvmfw/src/main.rs
index 5893907..299d1c0 100644
--- a/pvmfw/src/main.rs
+++ b/pvmfw/src/main.rs
@@ -144,9 +144,9 @@
})?;
let instance_hash = if cfg!(llpvm_changes) { Some(salt_from_instance_id(fdt)?) } else { None };
- let (new_instance, salt) = if should_defer_rollback_protection(fdt)?
- && verified_boot_data.has_capability(Capability::SecretkeeperProtection)
- {
+ let defer_rollback_protection = should_defer_rollback_protection(fdt)?
+ && verified_boot_data.has_capability(Capability::SecretkeeperProtection);
+ let (new_instance, salt) = if defer_rollback_protection {
info!("Guest OS is capable of Secretkeeper protection, deferring rollback protection");
// rollback_index of the image is used as security_version and is expected to be > 0 to
// discourage implicit allocation.
@@ -201,12 +201,18 @@
Cow::Owned(truncated_bcc_handover)
};
- dice_inputs.write_next_bcc(new_bcc_handover.as_ref(), &salt, instance_hash, next_bcc).map_err(
- |e| {
+ dice_inputs
+ .write_next_bcc(
+ new_bcc_handover.as_ref(),
+ &salt,
+ instance_hash,
+ defer_rollback_protection,
+ next_bcc,
+ )
+ .map_err(|e| {
error!("Failed to derive next-stage DICE secrets: {e:?}");
RebootReason::SecretDerivationError
- },
- )?;
+ })?;
flush(next_bcc);
let kaslr_seed = u64::from_ne_bytes(rand::random_array().map_err(|e| {
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 5e71245..8fe4167 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -346,7 +346,7 @@
))
.with_log();
}
- if !remotely_provisioned_component_service_exists()? {
+ if !is_remote_provisioning_hal_declared()? {
return Err(Status::new_exception_str(
ExceptionCode::UNSUPPORTED_OPERATION,
Some("AVF remotely provisioned component service is not declared"),
@@ -403,7 +403,7 @@
}
fn isRemoteAttestationSupported(&self) -> binder::Result<bool> {
- remotely_provisioned_component_service_exists()
+ is_remote_provisioning_hal_declared()
}
fn getAssignableDevices(&self) -> binder::Result<Vec<AssignableDevice>> {
@@ -862,7 +862,9 @@
Ok(())
}
-fn remotely_provisioned_component_service_exists() -> binder::Result<bool> {
+/// Returns true if the AVF remotely provisioned component service is declared in the
+/// VINTF manifest.
+pub(crate) fn is_remote_provisioning_hal_declared() -> binder::Result<bool> {
Ok(binder::is_declared(REMOTELY_PROVISIONED_COMPONENT_SERVICE_NAME)?)
}
diff --git a/virtualizationservice/src/main.rs b/virtualizationservice/src/main.rs
index 8acfdd3..55245f6 100644
--- a/virtualizationservice/src/main.rs
+++ b/virtualizationservice/src/main.rs
@@ -20,7 +20,10 @@
mod remote_provisioning;
mod rkpvm;
-use crate::aidl::{remove_temporary_dir, VirtualizationServiceInternal, TEMPORARY_DIRECTORY};
+use crate::aidl::{
+ is_remote_provisioning_hal_declared, remove_temporary_dir, VirtualizationServiceInternal,
+ TEMPORARY_DIRECTORY,
+};
use android_logger::{Config, FilterBuilder};
use android_system_virtualizationmaintenance::aidl::android::system::virtualizationmaintenance;
use android_system_virtualizationservice_internal::aidl::android::system::virtualizationservice_internal;
@@ -81,7 +84,7 @@
BnVirtualizationServiceInternal::new_binder(service.clone(), BinderFeatures::default());
register(INTERNAL_SERVICE_NAME, internal_service)?;
- if cfg!(remote_attestation) {
+ if is_remote_provisioning_hal_declared().unwrap_or(false) {
// The IRemotelyProvisionedComponent service is only supposed to be triggered by rkpd for
// RKP VM attestation.
let remote_provisioning_service = remote_provisioning::new_binder();