pvmfw: Don't panic on extra VBMeta properties
Remove the assumption that the image should contain a single property
descriptor (the descriptors should NOT be re-used after pvmfw, and extra
descriptors are safe to ignore) and update libpvmfw_avb.integration_test
to prevent a verification error.
Bug: 339779843
Bug: 339782511
Bug: 378673494
Test: atest libpvmfw_avb.integration_test
Change-Id: Iacf7efc2278354eda10e50a2a1f2122815dedab3
diff --git a/guest/pvmfw/avb/Android.bp b/guest/pvmfw/avb/Android.bp
index a1ee626..10c7841 100644
--- a/guest/pvmfw/avb/Android.bp
+++ b/guest/pvmfw/avb/Android.bp
@@ -37,10 +37,8 @@
":test_image_with_one_hashdesc",
":test_image_with_non_initrd_hashdesc",
":test_image_with_initrd_and_non_initrd_desc",
- ":test_image_with_prop_desc",
":test_image_with_service_vm_prop",
":test_image_with_unknown_vm_type_prop",
- ":test_image_with_multiple_props",
":test_image_with_duplicated_capability",
":test_image_with_rollback_index_5",
":test_image_with_multiple_capabilities",
@@ -117,20 +115,6 @@
}
avb_add_hash_footer {
- name: "test_image_with_prop_desc",
- src: ":unsigned_test_image",
- partition_name: "boot",
- private_key: ":pvmfw_sign_key",
- salt: "2134",
- props: [
- {
- name: "mock_prop",
- value: "3333",
- },
- ],
-}
-
-avb_add_hash_footer {
name: "test_image_with_service_vm_prop",
src: ":unsigned_test_image",
partition_name: "boot",
@@ -159,24 +143,6 @@
}
avb_add_hash_footer {
- name: "test_image_with_multiple_props",
- src: ":unsigned_test_image",
- partition_name: "boot",
- private_key: ":pvmfw_sign_key",
- salt: "2133",
- props: [
- {
- name: "com.android.virt.cap",
- value: "remote_attest",
- },
- {
- name: "another_vm_type",
- value: "foo_vm",
- },
- ],
-}
-
-avb_add_hash_footer {
name: "test_image_with_duplicated_capability",
src: ":unsigned_test_image",
partition_name: "boot",
diff --git a/guest/pvmfw/avb/src/verify.rs b/guest/pvmfw/avb/src/verify.rs
index c85d886..2a7eed2 100644
--- a/guest/pvmfw/avb/src/verify.rs
+++ b/guest/pvmfw/avb/src/verify.rs
@@ -17,11 +17,10 @@
use crate::ops::{Ops, Payload};
use crate::partition::PartitionName;
use crate::PvmfwVerifyError;
-use alloc::vec;
use alloc::vec::Vec;
use avb::{
- Descriptor, DescriptorError, DescriptorResult, HashDescriptor, PartitionData,
- PropertyDescriptor, SlotVerifyError, SlotVerifyNoDataResult, VbmetaData,
+ Descriptor, DescriptorError, DescriptorResult, HashDescriptor, PartitionData, SlotVerifyError,
+ SlotVerifyNoDataResult, VbmetaData,
};
// We use this for the rollback_index field if SlotVerifyData has empty rollback_indexes
@@ -93,14 +92,14 @@
/// Returns the capabilities indicated in `descriptor`, or error if the descriptor has
/// unexpected contents.
- fn get_capabilities(descriptor: &PropertyDescriptor) -> Result<Vec<Self>, PvmfwVerifyError> {
- if descriptor.key != Self::KEY {
- return Err(PvmfwVerifyError::UnknownVbmetaProperty);
- }
+ fn get_capabilities(vbmeta_data: &VbmetaData) -> Result<Vec<Self>, PvmfwVerifyError> {
+ let Some(value) = vbmeta_data.get_property_value(Self::KEY) else {
+ return Ok(Vec::new());
+ };
let mut res = Vec::new();
- for v in descriptor.value.split(|b| *b == Self::SEPARATOR) {
+ for v in value.split(|b| *b == Self::SEPARATOR) {
let cap = match v {
Self::REMOTE_ATTEST => Self::RemoteAttest,
Self::TRUSTY_SECURITY_VM => Self::TrustySecurityVm,
@@ -155,30 +154,6 @@
}
}
-/// Verifies that the vbmeta contains at most one property descriptor and it indicates the
-/// vm type is service VM.
-fn verify_property_and_get_capabilities(
- descriptors: &[Descriptor],
-) -> Result<Vec<Capability>, PvmfwVerifyError> {
- let mut iter = descriptors.iter().filter_map(|d| match d {
- Descriptor::Property(p) => Some(p),
- _ => None,
- });
-
- let descriptor = match iter.next() {
- // No property descriptors -> no capabilities.
- None => return Ok(vec![]),
- Some(d) => d,
- };
-
- // Multiple property descriptors -> error.
- if iter.next().is_some() {
- return Err(DescriptorError::InvalidContents.into());
- }
-
- Capability::get_capabilities(descriptor)
-}
-
/// Hash descriptors extracted from a vbmeta image.
///
/// We always have a kernel hash descriptor and may have initrd normal or debug descriptors.
@@ -280,7 +255,7 @@
verify_vbmeta_is_from_kernel_partition(vbmeta_image)?;
let descriptors = vbmeta_image.descriptors()?;
let hash_descriptors = HashDescriptors::get(&descriptors)?;
- let capabilities = verify_property_and_get_capabilities(&descriptors)?;
+ let capabilities = Capability::get_capabilities(vbmeta_image)?;
let page_size = None; // TODO(ptosi): Read from payload.
if initrd.is_none() {
diff --git a/guest/pvmfw/avb/tests/api_test.rs b/guest/pvmfw/avb/tests/api_test.rs
index 5bf777a..1a4e247 100644
--- a/guest/pvmfw/avb/tests/api_test.rs
+++ b/guest/pvmfw/avb/tests/api_test.rs
@@ -29,10 +29,8 @@
const TEST_IMG_WITH_ONE_HASHDESC_PATH: &str = "test_image_with_one_hashdesc.img";
const TEST_IMG_WITH_ROLLBACK_INDEX_5: &str = "test_image_with_rollback_index_5.img";
-const TEST_IMG_WITH_PROP_DESC_PATH: &str = "test_image_with_prop_desc.img";
const TEST_IMG_WITH_SERVICE_VM_PROP_PATH: &str = "test_image_with_service_vm_prop.img";
const TEST_IMG_WITH_UNKNOWN_VM_TYPE_PROP_PATH: &str = "test_image_with_unknown_vm_type_prop.img";
-const TEST_IMG_WITH_MULTIPLE_PROPS_PATH: &str = "test_image_with_multiple_props.img";
const TEST_IMG_WITH_DUPLICATED_CAP_PATH: &str = "test_image_with_duplicated_capability.img";
const TEST_IMG_WITH_NON_INITRD_HASHDESC_PATH: &str = "test_image_with_non_initrd_hashdesc.img";
const TEST_IMG_WITH_INITRD_AND_NON_INITRD_DESC_PATH: &str =
@@ -159,16 +157,6 @@
}
#[test]
-fn payload_with_multiple_props_fails_verification_with_no_initrd() -> Result<()> {
- assert_payload_verification_fails(
- &fs::read(TEST_IMG_WITH_MULTIPLE_PROPS_PATH)?,
- /* initrd= */ None,
- &load_trusted_public_key()?,
- PvmfwVerifyError::InvalidDescriptors(DescriptorError::InvalidContents),
- )
-}
-
-#[test]
fn payload_with_duplicated_capability_fails_verification_with_no_initrd() -> Result<()> {
assert_payload_verification_fails(
&fs::read(TEST_IMG_WITH_DUPLICATED_CAP_PATH)?,
@@ -179,16 +167,6 @@
}
#[test]
-fn payload_with_prop_descriptor_fails_verification_with_no_initrd() -> Result<()> {
- assert_payload_verification_fails(
- &fs::read(TEST_IMG_WITH_PROP_DESC_PATH)?,
- /* initrd= */ None,
- &load_trusted_public_key()?,
- PvmfwVerifyError::UnknownVbmetaProperty,
- )
-}
-
-#[test]
fn payload_expecting_initrd_fails_verification_with_no_initrd() -> Result<()> {
assert_payload_verification_fails(
&load_latest_signed_kernel()?,