diff options
author | David Pursell <dpursell@google.com> | 2023-11-27 18:55:28 +0000 |
---|---|---|
committer | Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> | 2023-11-27 18:55:28 +0000 |
commit | 8419735d87d8a5d05d526325d37a2b94a751f82b (patch) | |
tree | 5ade7a39ddb153d4332953acfeba26f10c8d3c8d | |
parent | 2ef0cbc1c12cede83682bb7882a80962893661f0 (diff) | |
parent | d495b9cfc30a4cac3fdbb40c73716b2615f0e6c3 (diff) | |
download | avb-8419735d87d8a5d05d526325d37a2b94a751f82b.tar.gz |
libavb_rs: fix default UUID for `boot` partition am: d495b9cfc3
Original change: https://android-review.googlesource.com/c/platform/external/avb/+/2844140
Change-Id: Ib20d23ea20eee4dc9ab84f7b50e45bf5d2bd265b
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
-rw-r--r-- | rust/Android.bp | 10 | ||||
-rw-r--r-- | rust/src/ops.rs | 24 | ||||
-rw-r--r-- | rust/tests/verify_tests.rs | 68 |
3 files changed, 85 insertions, 17 deletions
diff --git a/rust/Android.bp b/rust/Android.bp index b4d20e3..61fe0db 100644 --- a/rust/Android.bp +++ b/rust/Android.bp @@ -181,6 +181,7 @@ rust_defaults { ":avb_testkey_rsa4096_pub_bin", ":avbrs_test_image", ":avbrs_test_image_with_vbmeta_footer", + ":avbrs_test_image_with_vbmeta_footer_for_boot", ":avbrs_test_vbmeta", ":avbrs_test_vbmeta_2_parts", ":avbrs_test_vbmeta_persistent_digest", @@ -289,3 +290,12 @@ avb_add_hash_footer { private_key: ":avb_testkey_rsa4096", salt: "A000", } + +// Combined test image + signed vbmeta footer for "boot". +avb_add_hash_footer { + name: "avbrs_test_image_with_vbmeta_footer_for_boot", + src: ":avbrs_test_image", + partition_name: "boot", + private_key: ":avb_testkey_rsa4096", + salt: "A001", +} diff --git a/rust/src/ops.rs b/rust/src/ops.rs index 5846b17..f613fb5 100644 --- a/rust/src/ops.rs +++ b/rust/src/ops.rs @@ -755,6 +755,12 @@ unsafe fn try_get_unique_guid_for_partition( let buffer = unsafe { slice::from_raw_parts_mut(guid_buf as *mut u8, guid_buf_size) }; // Initialize the output buffer to the empty string. + // + // When the `uuid` feature is not selected, the user doesn't need commandline GUIDs but libavb + // may still attempt to inject the `vmbeta` or `boot` partition GUIDs into the commandline, + // depending on the verification settings. In order to satisfy libavb's requirements we must: + // * write a nul-terminated string to avoid undefined behavior (empty string is sufficient) + // * return `Ok(())` or verification will fail if buffer.is_empty() { return Err(IoError::Oom); } @@ -783,25 +789,9 @@ unsafe fn try_get_unique_guid_for_partition( return Err(IoError::Oom); } buffer[..guid_bytes.len()].copy_from_slice(guid_bytes); - Ok(()) } - #[cfg(not(feature = "uuid"))] - { - // The user doesn't need this feature, but libavb may still attempt to inject the vbmeta - // partition GUID into the commandline, depending on the verification flags. In this case - // the function needs to return success or verification will fail, but leaving the buffer - // as the empty string is sufficient since nobody will be reading the value. - // - // We restrict this to only "vbmeta*" partitions because if we return success for all - // partitions, libavb will also try to inject a "system" partition GUID into the - // commandline, which the user doesn't need. - partition - .to_bytes() - .starts_with(b"vbmeta") - .then_some(()) - .ok_or(IoError::NoSuchPartition) - } + Ok(()) } /// Wraps a callback to convert the given `Result<>` to raw `AvbIOResult` for libavb. diff --git a/rust/tests/verify_tests.rs b/rust/tests/verify_tests.rs index 0482ae7..416e0ec 100644 --- a/rust/tests/verify_tests.rs +++ b/rust/tests/verify_tests.rs @@ -29,6 +29,8 @@ const TEST_VBMETA_PATH: &str = "test_vbmeta.img"; const TEST_VBMETA_2_PARTITIONS_PATH: &str = "test_vbmeta_2_parts.img"; const TEST_VBMETA_PERSISTENT_DIGEST_PATH: &str = "test_vbmeta_persistent_digest.img"; const TEST_IMAGE_WITH_VBMETA_FOOTER_PATH: &str = "avbrs_test_image_with_vbmeta_footer.img"; +const TEST_IMAGE_WITH_VBMETA_FOOTER_FOR_BOOT_PATH: &str = + "avbrs_test_image_with_vbmeta_footer_for_boot.img"; const TEST_PUBLIC_KEY_PATH: &str = "data/testkey_rsa4096_pub.bin"; const TEST_PARTITION_NAME: &str = "test_part"; const TEST_PARTITION_SLOT_C_NAME: &str = "test_part_c"; @@ -87,6 +89,34 @@ fn verify_two_images_one_vbmeta<'a>( ) } +/// Initializes a `TestOps` object such that verification will succeed on the `boot` partition with +/// a combined image + vbmeta. +fn test_ops_boot_partition() -> TestOps { + let mut ops = test_ops_one_image_one_vbmeta(); + ops.partitions.clear(); + ops.add_partition( + "boot", + fs::read(TEST_IMAGE_WITH_VBMETA_FOOTER_FOR_BOOT_PATH).unwrap(), + ); + ops +} + +/// Calls `slot_verify()` using standard args for `test_ops_boot_partition()` setup. +fn verify_boot_partition<'a>( + ops: &'a mut TestOps, +) -> Result<SlotVerifyData<'a>, SlotVerifyError<'a>> { + slot_verify( + ops, + &[&CString::new("boot").unwrap()], + None, + // libavb has some special-case handling to automatically detect a combined image + vbmeta + // in the `boot` partition; don't pass the `AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION` flag + // so we can test this behavior. + SlotVerifyFlags::AVB_SLOT_VERIFY_FLAGS_NONE, + HashtreeErrorMode::AVB_HASHTREE_ERROR_MODE_EIO, + ) +} + /// Initializes a `TestOps` object such that verification will succeed on /// `TEST_PARTITION_PERSISTENT_DIGEST_NAME`. fn test_ops_persistent_digest(image: Vec<u8>) -> TestOps { @@ -276,6 +306,27 @@ fn combined_image_vbmeta_partition_passes_verification() { assert_eq!(partition_data.data(), fs::read(TEST_IMAGE_PATH).unwrap()); } +// Validate the custom behavior if the combined image + vbmeta live in the `boot` partition. +#[test] +fn vbmeta_with_boot_partition_passes_verification() { + let mut ops = test_ops_boot_partition(); + + let result = verify_boot_partition(&mut ops); + + let data = result.unwrap(); + + // Vbmeta should indicate that it came from `boot`. + assert_eq!(data.vbmeta_data().len(), 1); + let vbmeta_data = &data.vbmeta_data()[0]; + assert_eq!(vbmeta_data.partition_name().to_str().unwrap(), "boot"); + + // Partition should indicate that it came from `boot`, but only contain the image contents. + assert_eq!(data.partition_data().len(), 1); + let partition_data = &data.partition_data()[0]; + assert_eq!(partition_data.partition_name().to_str().unwrap(), "boot"); + assert_eq!(partition_data.data(), fs::read(TEST_IMAGE_PATH).unwrap()); +} + #[test] fn persistent_digest_verification_updates_persistent_value() { // With persistent digests, the image hash isn't stored in the descriptor, but is instead @@ -311,6 +362,23 @@ fn successful_verification_substitutes_partition_guid() { .contains("androidboot.vbmeta.device=PARTUUID=01234567-89ab-cdef-0123-456789abcdef")); } +#[cfg(feature = "uuid")] +#[test] +fn successful_verification_substitutes_boot_partition_guid() { + let mut ops = test_ops_boot_partition(); + ops.partitions.get_mut("boot").unwrap().uuid = uuid!("01234567-89ab-cdef-0123-456789abcdef"); + + let result = verify_boot_partition(&mut ops); + + let data = result.unwrap(); + // In this case libavb substitutes the `boot` partition GUID in for `vbmeta`. + assert!(data + .cmdline() + .to_str() + .unwrap() + .contains("androidboot.vbmeta.device=PARTUUID=01234567-89ab-cdef-0123-456789abcdef")); +} + #[test] fn corrupted_image_fails_verification() { let mut ops = test_ops_one_image_one_vbmeta(); |