[avb] Collect hash descriptors when verification succeeds
This cl uses `avb_descriptor_foreach()` to collect all the
hash descriptors once the verification succeeds and checks
that there's no unknown, duplicated or non-hash descriptor
in the vbmeta.
We will extract the partition digest from the collected
hash descriptors later.
Bug: 256148034
Bug: 265897559
Test: m pvmfw_img && atest libpvmfw_avb.integration_test
Change-Id: Ifa0a91f1e4384007e58d99585d72cdee81bd8dbc
diff --git a/libs/avb/Android.bp b/libs/avb/Android.bp
index 436f672..1d257bc 100644
--- a/libs/avb/Android.bp
+++ b/libs/avb/Android.bp
@@ -14,6 +14,7 @@
"--size_t-is-usize",
"--default-enum-style rust",
"--allowlist-function=.*",
+ "--allowlist-var=AVB.*",
"--use-core",
"--raw-line=#![no_std]",
"--ctypes-prefix=core::ffi",
diff --git a/pvmfw/avb/Android.bp b/pvmfw/avb/Android.bp
index 837f747..fb950b7 100644
--- a/pvmfw/avb/Android.bp
+++ b/pvmfw/avb/Android.bp
@@ -9,6 +9,7 @@
prefer_rlib: true,
rustlibs: [
"libavb_bindgen",
+ "libtinyvec_nostd",
],
whole_static_libs: [
"libavb",
@@ -37,6 +38,7 @@
":microdroid_initrd_debuggable",
":test_image_with_one_hashdesc",
":test_image_with_non_initrd_hashdesc",
+ ":test_image_with_initrd_and_non_initrd_desc",
":test_image_with_prop_desc",
":unsigned_test_image",
],
@@ -78,18 +80,40 @@
src: ":unsigned_test_image",
partition_name: "boot",
private_key: ":pvmfw_sign_key",
- salt: "1111",
+ salt: "3322",
include_descriptors_from_images: [
":test_non_initrd_hashdesc",
],
}
avb_add_hash_footer {
+ name: "test_image_with_initrd_and_non_initrd_desc",
+ src: ":unsigned_test_image",
+ partition_name: "boot",
+ private_key: ":pvmfw_sign_key",
+ salt: "3241",
+ include_descriptors_from_images: [
+ ":microdroid_initrd_normal_hashdesc",
+ ":test_non_initrd_hashdesc",
+ ],
+ enabled: false,
+ arch: {
+ // microdroid_initrd_normal_hashdesc is only available in these architectures.
+ arm64: {
+ enabled: true,
+ },
+ x86_64: {
+ enabled: true,
+ },
+ },
+}
+
+avb_add_hash_footer {
name: "test_image_with_prop_desc",
src: ":unsigned_test_image",
partition_name: "boot",
private_key: ":pvmfw_sign_key",
- salt: "1111",
+ salt: "2134",
props: [
{
name: "mock_prop",
diff --git a/pvmfw/avb/src/descriptor.rs b/pvmfw/avb/src/descriptor.rs
new file mode 100644
index 0000000..b0598de
--- /dev/null
+++ b/pvmfw/avb/src/descriptor.rs
@@ -0,0 +1,204 @@
+// Copyright 2023, The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Structs and functions relating to the descriptors.
+
+#![warn(unsafe_op_in_unsafe_fn)]
+
+use crate::error::{AvbIOError, AvbSlotVerifyError};
+use crate::partition::PartitionName;
+use crate::utils::{self, is_not_null, to_nonnull, to_usize, usize_checked_add};
+use avb_bindgen::{
+ avb_descriptor_foreach, avb_hash_descriptor_validate_and_byteswap, AvbDescriptor,
+ AvbHashDescriptor, AvbVBMetaData, AVB_SHA256_DIGEST_SIZE,
+};
+use core::{
+ ffi::c_void,
+ mem::{size_of, MaybeUninit},
+ ops::Range,
+ slice,
+};
+use tinyvec::ArrayVec;
+
+const DIGEST_SIZE: usize = AVB_SHA256_DIGEST_SIZE as usize;
+
+/// `HashDescriptors` can have maximum one `HashDescriptor` per known partition.
+#[derive(Default)]
+pub(crate) struct HashDescriptors(
+ ArrayVec<[HashDescriptor; PartitionName::NUM_OF_KNOWN_PARTITIONS]>,
+);
+
+impl HashDescriptors {
+ /// Builds `HashDescriptors` from `AvbVBMetaData`.
+ /// Returns an error if the given `AvbVBMetaData` contains non-hash descriptor, hash
+ /// descriptor of unknown `PartitionName` or duplicated hash descriptors.
+ ///
+ /// # Safety
+ ///
+ /// Behavior is undefined if any of the following conditions are violated:
+ /// * `vbmeta.vbmeta_data` must be non-null and points to a valid VBMeta.
+ /// * `vbmeta.vbmeta_data` must be valid for reading `vbmeta.vbmeta_size` bytes.
+ pub(crate) unsafe fn from_vbmeta(vbmeta: AvbVBMetaData) -> Result<Self, AvbSlotVerifyError> {
+ is_not_null(vbmeta.vbmeta_data).map_err(|_| AvbSlotVerifyError::Io)?;
+ let mut descriptors = Self::default();
+ // SAFETY: It is safe as the raw pointer `vbmeta.vbmeta_data` is a non-null pointer and
+ // points to a valid VBMeta structure.
+ if !unsafe {
+ avb_descriptor_foreach(
+ vbmeta.vbmeta_data,
+ vbmeta.vbmeta_size,
+ Some(check_and_save_descriptor),
+ &mut descriptors as *mut _ as *mut c_void,
+ )
+ } {
+ return Err(AvbSlotVerifyError::InvalidMetadata);
+ }
+ Ok(descriptors)
+ }
+
+ fn push(&mut self, descriptor: HashDescriptor) -> utils::Result<()> {
+ if self.0.iter().any(|d| d.partition_name_eq(&descriptor)) {
+ return Err(AvbIOError::Io);
+ }
+ self.0.push(descriptor);
+ Ok(())
+ }
+
+ pub(crate) fn len(&self) -> usize {
+ self.0.len()
+ }
+
+ /// Finds the `HashDescriptor` for the given `PartitionName`.
+ /// Throws an error if no corresponding descriptor found.
+ pub(crate) fn find(
+ &self,
+ partition_name: PartitionName,
+ ) -> Result<&HashDescriptor, AvbSlotVerifyError> {
+ self.0
+ .iter()
+ .find(|d| d.partition_name == partition_name)
+ .ok_or(AvbSlotVerifyError::InvalidMetadata)
+ }
+}
+
+/// # Safety
+///
+/// Behavior is undefined if any of the following conditions are violated:
+/// * The `descriptor` pointer must be non-null and points to a valid `AvbDescriptor` struct.
+/// * The `user_data` pointer must be non-null and points to a valid `HashDescriptors` struct.
+unsafe extern "C" fn check_and_save_descriptor(
+ descriptor: *const AvbDescriptor,
+ user_data: *mut c_void,
+) -> bool {
+ // SAFETY: It is safe because the caller must ensure that the `descriptor` pointer and
+ // the `user_data` are non-null and valid.
+ unsafe { try_check_and_save_descriptor(descriptor, user_data).is_ok() }
+}
+
+/// # Safety
+///
+/// Behavior is undefined if any of the following conditions are violated:
+/// * The `descriptor` pointer must be non-null and points to a valid `AvbDescriptor` struct.
+/// * The `user_data` pointer must be non-null and points to a valid `HashDescriptors` struct.
+unsafe fn try_check_and_save_descriptor(
+ descriptor: *const AvbDescriptor,
+ user_data: *mut c_void,
+) -> utils::Result<()> {
+ is_not_null(descriptor)?;
+ // SAFETY: It is safe because the caller ensures that `descriptor` is a non-null pointer
+ // pointing to a valid struct.
+ let desc = unsafe { AvbHashDescriptorWrap::from_descriptor_ptr(descriptor)? };
+ // SAFETY: It is safe because the caller ensures that `descriptor` is a non-null pointer
+ // pointing to a valid struct.
+ let data = unsafe { slice::from_raw_parts(descriptor as *const u8, desc.len()?) };
+ let mut descriptors = to_nonnull(user_data as *mut HashDescriptors)?;
+ // SAFETY: It is safe because the caller ensures that `user_data` is a non-null pointer
+ // pointing to a valid struct.
+ let descriptors = unsafe { descriptors.as_mut() };
+ descriptors.push(HashDescriptor::new(&desc, data)?)
+}
+
+#[derive(Default)]
+pub(crate) struct HashDescriptor {
+ partition_name: PartitionName,
+ /// TODO(b/265897559): Pass this digest to DICE.
+ #[allow(dead_code)]
+ pub(crate) digest: [u8; DIGEST_SIZE],
+}
+
+impl HashDescriptor {
+ fn new(desc: &AvbHashDescriptorWrap, data: &[u8]) -> utils::Result<Self> {
+ let partition_name = data
+ .get(desc.partition_name_range()?)
+ .ok_or(AvbIOError::RangeOutsidePartition)?
+ .try_into()?;
+ let partition_digest =
+ data.get(desc.digest_range()?).ok_or(AvbIOError::RangeOutsidePartition)?;
+ let mut digest = [0u8; DIGEST_SIZE];
+ digest.copy_from_slice(partition_digest);
+ Ok(Self { partition_name, digest })
+ }
+
+ fn partition_name_eq(&self, other: &HashDescriptor) -> bool {
+ self.partition_name == other.partition_name
+ }
+}
+
+/// `AvbHashDescriptor` contains the metadata for the given descriptor.
+struct AvbHashDescriptorWrap(AvbHashDescriptor);
+
+impl AvbHashDescriptorWrap {
+ /// # Safety
+ ///
+ /// Behavior is undefined if any of the following conditions are violated:
+ /// * The `descriptor` pointer must be non-null and point to a valid `AvbDescriptor`.
+ unsafe fn from_descriptor_ptr(descriptor: *const AvbDescriptor) -> utils::Result<Self> {
+ is_not_null(descriptor)?;
+ // SAFETY: It is safe as the raw pointer `descriptor` is non-null and points to
+ // a valid `AvbDescriptor`.
+ let desc = unsafe {
+ let mut desc = MaybeUninit::uninit();
+ if !avb_hash_descriptor_validate_and_byteswap(
+ descriptor as *const AvbHashDescriptor,
+ desc.as_mut_ptr(),
+ ) {
+ return Err(AvbIOError::Io);
+ }
+ desc.assume_init()
+ };
+ Ok(Self(desc))
+ }
+
+ fn len(&self) -> utils::Result<usize> {
+ usize_checked_add(
+ size_of::<AvbDescriptor>(),
+ to_usize(self.0.parent_descriptor.num_bytes_following)?,
+ )
+ }
+
+ fn partition_name_end(&self) -> utils::Result<usize> {
+ usize_checked_add(size_of::<AvbHashDescriptor>(), to_usize(self.0.partition_name_len)?)
+ }
+
+ fn partition_name_range(&self) -> utils::Result<Range<usize>> {
+ let start = size_of::<AvbHashDescriptor>();
+ Ok(start..(self.partition_name_end()?))
+ }
+
+ fn digest_range(&self) -> utils::Result<Range<usize>> {
+ let start = usize_checked_add(self.partition_name_end()?, to_usize(self.0.salt_len)?)?;
+ let end = usize_checked_add(start, to_usize(self.0.digest_len)?)?;
+ Ok(start..end)
+ }
+}
diff --git a/pvmfw/avb/src/lib.rs b/pvmfw/avb/src/lib.rs
index a1e3ee0..065eca5 100644
--- a/pvmfw/avb/src/lib.rs
+++ b/pvmfw/avb/src/lib.rs
@@ -18,6 +18,7 @@
// For usize.checked_add_signed(isize), available in Rust 1.66.0
#![feature(mixed_integer_ops)]
+mod descriptor;
mod error;
mod ops;
mod partition;
diff --git a/pvmfw/avb/src/partition.rs b/pvmfw/avb/src/partition.rs
index 82d89f8..10a5084 100644
--- a/pvmfw/avb/src/partition.rs
+++ b/pvmfw/avb/src/partition.rs
@@ -25,7 +25,16 @@
InitrdDebug,
}
+/// This is needed to build the default `HashDescriptor`.
+impl Default for PartitionName {
+ fn default() -> Self {
+ Self::Kernel
+ }
+}
+
impl PartitionName {
+ pub(crate) const NUM_OF_KNOWN_PARTITIONS: usize = 3;
+
const KERNEL_PARTITION_NAME: &[u8] = b"boot\0";
const INITRD_NORMAL_PARTITION_NAME: &[u8] = b"initrd_normal\0";
const INITRD_DEBUG_PARTITION_NAME: &[u8] = b"initrd_debug\0";
diff --git a/pvmfw/avb/src/utils.rs b/pvmfw/avb/src/utils.rs
index 1b35c22..a24d61f 100644
--- a/pvmfw/avb/src/utils.rs
+++ b/pvmfw/avb/src/utils.rs
@@ -22,7 +22,7 @@
pub(crate) fn write<T>(ptr: *mut T, value: T) -> Result<()> {
let ptr = to_nonnull(ptr)?;
- // SAFETY: It is safe as the raw pointer `ptr` is a nonnull pointer.
+ // SAFETY: It is safe as the raw pointer `ptr` is a non-null pointer.
unsafe {
*ptr.as_ptr() = value;
}
@@ -31,7 +31,7 @@
pub(crate) fn as_ref<'a, T>(ptr: *mut T) -> Result<&'a T> {
let ptr = to_nonnull(ptr)?;
- // SAFETY: It is safe as the raw pointer `ptr` is a nonnull pointer.
+ // SAFETY: It is safe as the raw pointer `ptr` is a non-null pointer.
unsafe { Ok(ptr.as_ref()) }
}
diff --git a/pvmfw/avb/src/verify.rs b/pvmfw/avb/src/verify.rs
index a062061..14b0e7e 100644
--- a/pvmfw/avb/src/verify.rs
+++ b/pvmfw/avb/src/verify.rs
@@ -14,19 +14,12 @@
//! This module handles the pvmfw payload verification.
-use crate::error::{AvbIOError, AvbSlotVerifyError};
+use crate::descriptor::HashDescriptors;
+use crate::error::AvbSlotVerifyError;
use crate::ops::{Ops, Payload};
use crate::partition::PartitionName;
-use crate::utils::{is_not_null, to_usize, usize_checked_add, write};
-use avb_bindgen::{
- avb_descriptor_foreach, avb_hash_descriptor_validate_and_byteswap, AvbDescriptor,
- AvbHashDescriptor, AvbPartitionData, AvbVBMetaData,
-};
-use core::{
- ffi::{c_char, c_void},
- mem::{size_of, MaybeUninit},
- slice,
-};
+use avb_bindgen::{AvbPartitionData, AvbVBMetaData};
+use core::ffi::c_char;
/// This enum corresponds to the `DebugLevel` in `VirtualMachineConfig`.
#[derive(Clone, Debug, PartialEq, Eq)]
@@ -37,104 +30,6 @@
Full,
}
-extern "C" fn search_initrd_hash_descriptor(
- descriptor: *const AvbDescriptor,
- user_data: *mut c_void,
-) -> bool {
- try_search_initrd_hash_descriptor(descriptor, user_data).is_ok()
-}
-
-fn try_search_initrd_hash_descriptor(
- descriptor: *const AvbDescriptor,
- user_data: *mut c_void,
-) -> Result<(), AvbIOError> {
- let hash_desc = AvbHashDescriptorRef::try_from(descriptor)?;
- if matches!(
- hash_desc.partition_name()?.try_into(),
- Ok(PartitionName::InitrdDebug) | Ok(PartitionName::InitrdNormal),
- ) {
- write(user_data as *mut bool, true)?;
- }
- Ok(())
-}
-
-/// `hash_desc` only contains the metadata like fields length and flags of the descriptor.
-/// The data itself is contained in `ptr`.
-struct AvbHashDescriptorRef {
- hash_desc: AvbHashDescriptor,
- ptr: *const AvbDescriptor,
-}
-
-impl TryFrom<*const AvbDescriptor> for AvbHashDescriptorRef {
- type Error = AvbIOError;
-
- fn try_from(descriptor: *const AvbDescriptor) -> Result<Self, Self::Error> {
- is_not_null(descriptor)?;
- // SAFETY: It is safe as the raw pointer `descriptor` is a nonnull pointer and
- // we have validated that it is of hash descriptor type.
- let hash_desc = unsafe {
- let mut desc = MaybeUninit::uninit();
- if !avb_hash_descriptor_validate_and_byteswap(
- descriptor as *const AvbHashDescriptor,
- desc.as_mut_ptr(),
- ) {
- return Err(AvbIOError::Io);
- }
- desc.assume_init()
- };
- Ok(Self { hash_desc, ptr: descriptor })
- }
-}
-
-impl AvbHashDescriptorRef {
- fn check_is_in_range(&self, index: usize) -> Result<(), AvbIOError> {
- let parent_desc = self.hash_desc.parent_descriptor;
- let total_len = usize_checked_add(
- size_of::<AvbDescriptor>(),
- to_usize(parent_desc.num_bytes_following)?,
- )?;
- if index <= total_len {
- Ok(())
- } else {
- Err(AvbIOError::Io)
- }
- }
-
- /// Returns the non null-terminated partition name.
- fn partition_name(&self) -> Result<&[u8], AvbIOError> {
- let partition_name_offset = size_of::<AvbHashDescriptor>();
- let partition_name_len = to_usize(self.hash_desc.partition_name_len)?;
- self.check_is_in_range(usize_checked_add(partition_name_offset, partition_name_len)?)?;
- let desc = self.ptr as *const u8;
- // SAFETY: The descriptor has been validated as nonnull and the partition name is
- // contained within the image.
- unsafe { Ok(slice::from_raw_parts(desc.add(partition_name_offset), partition_name_len)) }
- }
-}
-
-fn verify_vbmeta_has_no_initrd_descriptor(
- vbmeta_image: &AvbVBMetaData,
-) -> Result<(), AvbSlotVerifyError> {
- is_not_null(vbmeta_image.vbmeta_data).map_err(|_| AvbSlotVerifyError::Io)?;
- let mut has_unexpected_descriptor = false;
- // SAFETY: It is safe as the raw pointer `vbmeta_image.vbmeta_data` is a nonnull pointer.
- if !unsafe {
- avb_descriptor_foreach(
- vbmeta_image.vbmeta_data,
- vbmeta_image.vbmeta_size,
- Some(search_initrd_hash_descriptor),
- &mut has_unexpected_descriptor as *mut _ as *mut c_void,
- )
- } {
- return Err(AvbSlotVerifyError::InvalidMetadata);
- }
- if has_unexpected_descriptor {
- Err(AvbSlotVerifyError::InvalidMetadata)
- } else {
- Ok(())
- }
-}
-
fn verify_only_one_vbmeta_exists(
vbmeta_images: &[AvbVBMetaData],
) -> Result<(), AvbSlotVerifyError> {
@@ -154,6 +49,16 @@
}
}
+fn verify_vbmeta_has_only_one_hash_descriptor(
+ hash_descriptors: &HashDescriptors,
+) -> Result<(), AvbSlotVerifyError> {
+ if hash_descriptors.len() == 1 {
+ Ok(())
+ } else {
+ Err(AvbSlotVerifyError::InvalidMetadata)
+ }
+}
+
fn verify_loaded_partition_has_expected_length(
loaded_partitions: &[AvbPartitionData],
partition_name: PartitionName,
@@ -191,13 +96,17 @@
verify_only_one_vbmeta_exists(vbmeta_images)?;
let vbmeta_image = vbmeta_images[0];
verify_vbmeta_is_from_kernel_partition(&vbmeta_image)?;
+ // SAFETY: It is safe because the `vbmeta_image` is collected from `AvbSlotVerifyData`,
+ // which is returned by `avb_slot_verify()` when the verification succeeds. It is
+ // guaranteed by libavb to be non-null and to point to a valid VBMeta structure.
+ let hash_descriptors = unsafe { HashDescriptors::from_vbmeta(vbmeta_image)? };
+ // TODO(b/265897559): Pass the digest in kernel descriptor to DICE.
+ let _kernel_descriptor = hash_descriptors.find(PartitionName::Kernel)?;
if initrd.is_none() {
- verify_vbmeta_has_no_initrd_descriptor(&vbmeta_image)?;
+ verify_vbmeta_has_only_one_hash_descriptor(&hash_descriptors)?;
return Ok(DebugLevel::None);
}
- // TODO(b/256148034): Check the vbmeta doesn't have hash descriptors other than
- // boot, initrd_normal, initrd_debug.
let initrd = initrd.unwrap();
let (debug_level, initrd_verify_result, initrd_partition_name) =
diff --git a/pvmfw/avb/tests/api_test.rs b/pvmfw/avb/tests/api_test.rs
index b5300f9..4f00f1e 100644
--- a/pvmfw/avb/tests/api_test.rs
+++ b/pvmfw/avb/tests/api_test.rs
@@ -25,6 +25,8 @@
const TEST_IMG_WITH_ONE_HASHDESC_PATH: &str = "test_image_with_one_hashdesc.img";
const TEST_IMG_WITH_PROP_DESC_PATH: &str = "test_image_with_prop_desc.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 =
+ "test_image_with_initrd_and_non_initrd_desc.img";
const UNSIGNED_TEST_IMG_PATH: &str = "unsigned_test.img";
const RANDOM_FOOTER_POS: usize = 30;
@@ -62,12 +64,22 @@
}
#[test]
-fn payload_with_non_initrd_descriptor_passes_verification_with_no_initrd() -> Result<()> {
+fn payload_with_non_initrd_descriptor_fails_verification_with_no_initrd() -> Result<()> {
assert_payload_verification_eq(
&fs::read(TEST_IMG_WITH_NON_INITRD_HASHDESC_PATH)?,
/*initrd=*/ None,
&load_trusted_public_key()?,
- Ok(DebugLevel::None),
+ Err(AvbSlotVerifyError::InvalidMetadata),
+ )
+}
+
+#[test]
+fn payload_with_non_initrd_descriptor_fails_verification_with_initrd() -> Result<()> {
+ assert_payload_verification_with_initrd_eq(
+ &fs::read(TEST_IMG_WITH_INITRD_AND_NON_INITRD_DESC_PATH)?,
+ &load_latest_initrd_normal()?,
+ &load_trusted_public_key()?,
+ Err(AvbSlotVerifyError::InvalidMetadata),
)
}