Merge "virtmgr: fail fast on misconfigured device"
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachine.java b/javalib/src/android/system/virtualmachine/VirtualMachine.java
index e319aa3..b57cb5e 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachine.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachine.java
@@ -192,9 +192,6 @@
/** Name of the idsig files for extra APKs. */
private static final String EXTRA_IDSIG_FILE_PREFIX = "extra_idsig_";
- /** Name of the virtualization service. */
- private static final String SERVICE_NAME = "android.system.virtualizationservice";
-
/** Size of the instance image. 10 MB. */
private static final long INSTANCE_FILE_SIZE = 10 * 1024 * 1024;
diff --git a/javalib/src/android/system/virtualmachine/VirtualizationService.java b/javalib/src/android/system/virtualmachine/VirtualizationService.java
index 75e359f..c3f2ba3 100644
--- a/javalib/src/android/system/virtualmachine/VirtualizationService.java
+++ b/javalib/src/android/system/virtualmachine/VirtualizationService.java
@@ -39,7 +39,7 @@
* Client FD for UDS connection to virtmgr's RpcBinder server. Closing it
* will make virtmgr shut down.
*/
- private ParcelFileDescriptor mClientFd;
+ private final ParcelFileDescriptor mClientFd;
private static native int nativeSpawn();
@@ -86,7 +86,7 @@
VirtualizationService service = (sInstance == null) ? null : sInstance.get();
if (service == null || !service.isOk()) {
service = new VirtualizationService();
- sInstance = new WeakReference(service);
+ sInstance = new WeakReference<>(service);
}
return service;
}
diff --git a/microdroid/Android.bp b/microdroid/Android.bp
index ecaadf8..b46f112 100644
--- a/microdroid/Android.bp
+++ b/microdroid/Android.bp
@@ -538,6 +538,46 @@
srcs: ["bootconfig.normal"],
}
+// python -c "import hashlib; print(hashlib.sha256(b'initrd_normal').hexdigest())"
+initrd_normal_salt = "8041a07d54ac82290f6d90bac1fa8d7fdbc4db974d101d60faf294749d1ebaf8"
+
+avb_gen_vbmeta_image {
+ name: "microdroid_initrd_normal_hashdesc",
+ src: ":microdroid_initrd_normal",
+ partition_name: "vbmeta_initrd_normal",
+ salt: initrd_normal_salt,
+ enabled: false,
+ arch: {
+ // Microdroid kernel is only available in these architectures.
+ arm64: {
+ enabled: true,
+ },
+ x86_64: {
+ enabled: true,
+ },
+ },
+}
+
+// python -c "import hashlib; print(hashlib.sha256(b'initrd_debug').hexdigest())"
+initrd_debug_salt = "8ab9dc9cb7e6456700ff6ef18c6b4c3acc24c5fa5381b829563f8d7a415d869a"
+
+avb_gen_vbmeta_image {
+ name: "microdroid_initrd_debug_hashdesc",
+ src: ":microdroid_initrd_debuggable",
+ partition_name: "vbmeta_initrd_debug",
+ salt: initrd_debug_salt,
+ enabled: false,
+ arch: {
+ // Microdroid kernel is only available in these architectures.
+ arm64: {
+ enabled: true,
+ },
+ x86_64: {
+ enabled: true,
+ },
+ },
+}
+
avb_add_hash_footer {
name: "microdroid_kernel_signed",
src: "empty_kernel",
@@ -556,11 +596,9 @@
enabled: true,
},
},
- props: [
- {
- name: "trusted_ramdisk",
- file: ":microdroid_initrd_hashes",
- },
+ include_descriptors_from_images: [
+ ":microdroid_initrd_normal_hashdesc",
+ ":microdroid_initrd_debug_hashdesc",
],
}
diff --git a/microdroid/initrd/Android.bp b/microdroid/initrd/Android.bp
index d05ea86..4531583 100644
--- a/microdroid/initrd/Android.bp
+++ b/microdroid/initrd/Android.bp
@@ -122,28 +122,3 @@
},
filename: "microdroid_initrd_normal.img",
}
-
-genrule {
- name: "microdroid_initrd_normal.sha256",
- srcs: [":microdroid_initrd_normal"],
- cmd: "cat $(in) | sha256sum | cut -d' ' -f1 > $(out)",
- out: ["hash"],
-}
-
-genrule {
- name: "microdroid_initrd_debuggable.sha256",
- srcs: [":microdroid_initrd_debuggable"],
- cmd: "cat $(in) | sha256sum | cut -d' ' -f1 > $(out)",
- out: ["hash"],
-}
-
-genrule {
- name: "microdroid_initrd_hashes",
- srcs: [
- ":microdroid_initrd_normal.sha256",
- ":microdroid_initrd_debuggable.sha256",
- ],
- // join the hashes with commas
- cmd: "cat $(in) | tr '\n' ',' > $(out) && truncate -s -1 $(out)",
- out: ["output"],
-}
diff --git a/pvmfw/avb/Android.bp b/pvmfw/avb/Android.bp
index 3026d20..cbec235 100644
--- a/pvmfw/avb/Android.bp
+++ b/pvmfw/avb/Android.bp
@@ -32,6 +32,8 @@
":avb_testkey_rsa2048_pub_bin",
":avb_testkey_rsa4096_pub_bin",
":microdroid_kernel_signed",
+ ":microdroid_initrd_normal",
+ ":test_image_with_one_hashdesc",
":unsigned_test_image",
],
rustlibs: [
@@ -56,3 +58,11 @@
out: ["unsigned_test.img"],
cmd: "$(location avbtool) generate_test_image --image_size 16384 --output $(out)",
}
+
+avb_add_hash_footer {
+ name: "test_image_with_one_hashdesc",
+ src: ":unsigned_test_image",
+ partition_name: "bootloader",
+ private_key: ":pvmfw_sign_key",
+ salt: "1111",
+}
diff --git a/pvmfw/avb/src/verify.rs b/pvmfw/avb/src/verify.rs
index f01c6b8..a465b12 100644
--- a/pvmfw/avb/src/verify.rs
+++ b/pvmfw/avb/src/verify.rs
@@ -25,6 +25,8 @@
slice,
};
+static NULL_BYTE: &[u8] = b"\0";
+
/// Error code from AVB image verification.
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum AvbImageVerifyError {
@@ -354,6 +356,7 @@
struct Payload<'a> {
kernel: &'a [u8],
+ initrd: Option<&'a [u8]>,
trusted_public_key: &'a [u8],
}
@@ -370,10 +373,10 @@
impl<'a> Payload<'a> {
const KERNEL_PARTITION_NAME: &[u8] = b"bootloader\0";
+ const INITRD_NORMAL_PARTITION_NAME: &[u8] = b"initrd_normal\0";
+ const INITRD_DEBUG_PARTITION_NAME: &[u8] = b"initrd_debug\0";
- fn kernel_partition_name(&self) -> &CStr {
- CStr::from_bytes_with_nul(Self::KERNEL_PARTITION_NAME).unwrap()
- }
+ const MAX_NUM_OF_HASH_DESCRIPTORS: usize = 3;
fn get_partition(&self, partition_name: *const c_char) -> Result<&[u8], AvbIOError> {
is_not_null(partition_name)?;
@@ -381,51 +384,70 @@
let partition_name = unsafe { CStr::from_ptr(partition_name) };
match partition_name.to_bytes_with_nul() {
Self::KERNEL_PARTITION_NAME => Ok(self.kernel),
+ Self::INITRD_NORMAL_PARTITION_NAME | Self::INITRD_DEBUG_PARTITION_NAME => {
+ self.initrd.ok_or(AvbIOError::NoSuchPartition)
+ }
_ => Err(AvbIOError::NoSuchPartition),
}
}
+
+ fn verify_partitions(&mut self, partition_names: &[&CStr]) -> Result<(), AvbImageVerifyError> {
+ if partition_names.len() > Self::MAX_NUM_OF_HASH_DESCRIPTORS {
+ return Err(AvbImageVerifyError::InvalidArgument);
+ }
+ let mut requested_partitions = [ptr::null(); Self::MAX_NUM_OF_HASH_DESCRIPTORS + 1];
+ partition_names
+ .iter()
+ .enumerate()
+ .for_each(|(i, name)| requested_partitions[i] = name.as_ptr());
+
+ let mut avb_ops = AvbOps {
+ user_data: self as *mut _ as *mut c_void,
+ ab_ops: ptr::null_mut(),
+ atx_ops: ptr::null_mut(),
+ read_from_partition: Some(read_from_partition),
+ get_preloaded_partition: Some(get_preloaded_partition),
+ write_to_partition: None,
+ validate_vbmeta_public_key: None,
+ read_rollback_index: Some(read_rollback_index),
+ write_rollback_index: None,
+ read_is_device_unlocked: Some(read_is_device_unlocked),
+ get_unique_guid_for_partition: Some(get_unique_guid_for_partition),
+ get_size_of_partition: Some(get_size_of_partition),
+ read_persistent_value: None,
+ write_persistent_value: None,
+ validate_public_key_for_partition: Some(validate_public_key_for_partition),
+ };
+ let ab_suffix = CStr::from_bytes_with_nul(NULL_BYTE).unwrap();
+ let out_data = ptr::null_mut();
+ // SAFETY: It is safe to call `avb_slot_verify()` as the pointer arguments (`ops`,
+ // `requested_partitions` and `ab_suffix`) passed to the method are all valid and
+ // initialized. The last argument `out_data` is allowed to be null so that nothing
+ // will be written to it.
+ let result = unsafe {
+ avb_slot_verify(
+ &mut avb_ops,
+ requested_partitions.as_ptr(),
+ ab_suffix.as_ptr(),
+ AvbSlotVerifyFlags::AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION,
+ AvbHashtreeErrorMode::AVB_HASHTREE_ERROR_MODE_RESTART_AND_INVALIDATE,
+ out_data,
+ )
+ };
+ to_avb_verify_result(result)
+ }
}
/// Verifies the payload (signed kernel + initrd) against the trusted public key.
-pub fn verify_payload(kernel: &[u8], trusted_public_key: &[u8]) -> Result<(), AvbImageVerifyError> {
- let mut payload = Payload { kernel, trusted_public_key };
- let mut avb_ops = AvbOps {
- user_data: &mut payload as *mut _ as *mut c_void,
- ab_ops: ptr::null_mut(),
- atx_ops: ptr::null_mut(),
- read_from_partition: Some(read_from_partition),
- get_preloaded_partition: Some(get_preloaded_partition),
- write_to_partition: None,
- validate_vbmeta_public_key: None,
- read_rollback_index: Some(read_rollback_index),
- write_rollback_index: None,
- read_is_device_unlocked: Some(read_is_device_unlocked),
- get_unique_guid_for_partition: Some(get_unique_guid_for_partition),
- get_size_of_partition: Some(get_size_of_partition),
- read_persistent_value: None,
- write_persistent_value: None,
- validate_public_key_for_partition: Some(validate_public_key_for_partition),
- };
- // NULL is needed to mark the end of the array.
- let requested_partitions: [*const c_char; 2] =
- [payload.kernel_partition_name().as_ptr(), ptr::null()];
- let ab_suffix = CStr::from_bytes_with_nul(b"\0").unwrap();
-
- // SAFETY: It is safe to call `avb_slot_verify()` as the pointer arguments (`ops`,
- // `requested_partitions` and `ab_suffix`) passed to the method are all valid and
- // initialized. The last argument `out_data` is allowed to be null so that nothing
- // will be written to it.
- let result = unsafe {
- avb_slot_verify(
- &mut avb_ops,
- requested_partitions.as_ptr(),
- ab_suffix.as_ptr(),
- AvbSlotVerifyFlags::AVB_SLOT_VERIFY_FLAGS_NO_VBMETA_PARTITION,
- AvbHashtreeErrorMode::AVB_HASHTREE_ERROR_MODE_RESTART_AND_INVALIDATE,
- /*out_data=*/ ptr::null_mut(),
- )
- };
- to_avb_verify_result(result)
+pub fn verify_payload(
+ kernel: &[u8],
+ initrd: Option<&[u8]>,
+ trusted_public_key: &[u8],
+) -> Result<(), AvbImageVerifyError> {
+ let mut payload = Payload { kernel, initrd, trusted_public_key };
+ let kernel = CStr::from_bytes_with_nul(Payload::KERNEL_PARTITION_NAME).unwrap();
+ let requested_partitions = [kernel];
+ payload.verify_partitions(&requested_partitions)
}
#[cfg(test)]
@@ -435,6 +457,11 @@
use avb_bindgen::AvbFooter;
use std::{fs, mem::size_of};
+ const MICRODROID_KERNEL_IMG_PATH: &str = "microdroid_kernel";
+ const INITRD_NORMAL_IMG_PATH: &str = "microdroid_initrd_normal.img";
+ const TEST_IMG_WITH_ONE_HASHDESC_PATH: &str = "test_image_with_one_hashdesc.img";
+ const UNSIGNED_TEST_IMG_PATH: &str = "unsigned_test.img";
+
const PUBLIC_KEY_RSA2048_PATH: &str = "data/testkey_rsa2048_pub.bin";
const PUBLIC_KEY_RSA4096_PATH: &str = "data/testkey_rsa4096_pub.bin";
const RANDOM_FOOTER_POS: usize = 30;
@@ -442,18 +469,32 @@
/// This test uses the Microdroid payload compiled on the fly to check that
/// the latest payload can be verified successfully.
#[test]
- fn latest_valid_payload_is_verified_successfully() -> Result<()> {
+ fn latest_valid_payload_passes_verification() -> Result<()> {
let kernel = load_latest_signed_kernel()?;
+ let initrd_normal = fs::read(INITRD_NORMAL_IMG_PATH)?;
let public_key = fs::read(PUBLIC_KEY_RSA4096_PATH)?;
- assert_eq!(Ok(()), verify_payload(&kernel, &public_key));
+ assert_eq!(Ok(()), verify_payload(&kernel, Some(&initrd_normal[..]), &public_key));
Ok(())
}
#[test]
+ fn payload_expecting_no_initrd_passes_verification_with_no_initrd() -> Result<()> {
+ let kernel = fs::read(TEST_IMG_WITH_ONE_HASHDESC_PATH)?;
+ let public_key = fs::read(PUBLIC_KEY_RSA4096_PATH)?;
+
+ assert_eq!(Ok(()), verify_payload(&kernel, None, &public_key));
+ Ok(())
+ }
+
+ // TODO(b/256148034): Test that kernel with two hashdesc and no initrd fails verification.
+ // e.g. payload_expecting_initrd_fails_verification_with_no_initrd
+
+ #[test]
fn payload_with_empty_public_key_fails_verification() -> Result<()> {
assert_payload_verification_fails(
&load_latest_signed_kernel()?,
+ &load_latest_initrd_normal()?,
/*trusted_public_key=*/ &[0u8; 0],
AvbImageVerifyError::PublicKeyRejected,
)
@@ -463,6 +504,7 @@
fn payload_with_an_invalid_public_key_fails_verification() -> Result<()> {
assert_payload_verification_fails(
&load_latest_signed_kernel()?,
+ &load_latest_initrd_normal()?,
/*trusted_public_key=*/ &[0u8; 512],
AvbImageVerifyError::PublicKeyRejected,
)
@@ -472,6 +514,7 @@
fn payload_with_a_different_valid_public_key_fails_verification() -> Result<()> {
assert_payload_verification_fails(
&load_latest_signed_kernel()?,
+ &load_latest_initrd_normal()?,
&fs::read(PUBLIC_KEY_RSA2048_PATH)?,
AvbImageVerifyError::PublicKeyRejected,
)
@@ -480,7 +523,8 @@
#[test]
fn unsigned_kernel_fails_verification() -> Result<()> {
assert_payload_verification_fails(
- &fs::read("unsigned_test.img")?,
+ &fs::read(UNSIGNED_TEST_IMG_PATH)?,
+ &load_latest_initrd_normal()?,
&fs::read(PUBLIC_KEY_RSA4096_PATH)?,
AvbImageVerifyError::Io,
)
@@ -493,6 +537,7 @@
assert_payload_verification_fails(
&kernel,
+ &load_latest_initrd_normal()?,
&fs::read(PUBLIC_KEY_RSA4096_PATH)?,
AvbImageVerifyError::Verification,
)
@@ -506,6 +551,7 @@
assert_payload_verification_fails(
&kernel,
+ &load_latest_initrd_normal()?,
&fs::read(PUBLIC_KEY_RSA4096_PATH)?,
AvbImageVerifyError::InvalidMetadata,
)
@@ -513,14 +559,19 @@
fn assert_payload_verification_fails(
kernel: &[u8],
+ initrd: &[u8],
trusted_public_key: &[u8],
expected_error: AvbImageVerifyError,
) -> Result<()> {
- assert_eq!(Err(expected_error), verify_payload(kernel, trusted_public_key));
+ assert_eq!(Err(expected_error), verify_payload(kernel, Some(initrd), trusted_public_key));
Ok(())
}
fn load_latest_signed_kernel() -> Result<Vec<u8>> {
- Ok(fs::read("microdroid_kernel")?)
+ Ok(fs::read(MICRODROID_KERNEL_IMG_PATH)?)
+ }
+
+ fn load_latest_initrd_normal() -> Result<Vec<u8>> {
+ Ok(fs::read(INITRD_NORMAL_IMG_PATH)?)
}
}
diff --git a/pvmfw/src/main.rs b/pvmfw/src/main.rs
index b0177bf..9b14644 100644
--- a/pvmfw/src/main.rs
+++ b/pvmfw/src/main.rs
@@ -72,7 +72,7 @@
let mut pci_root = unsafe { pci_info.make_pci_root() };
find_virtio_devices(&mut pci_root).map_err(handle_pci_error)?;
- verify_payload(signed_kernel, PUBLIC_KEY).map_err(|e| {
+ verify_payload(signed_kernel, ramdisk, PUBLIC_KEY).map_err(|e| {
error!("Failed to verify the payload: {e}");
RebootReason::PayloadVerificationError
})?;
diff --git a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
index 43d822b..d8e74f7 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -394,37 +394,43 @@
assertThat(e).hasMessageThat().contains("setProtectedVm must be called");
}
+ private VirtualMachineConfig.Builder newBaselineBuilder() {
+ return newVmConfigBuilder().setPayloadBinaryName("binary.so").setApkPath("/apk/path");
+ }
+
@Test
@CddTest(requirements = {"9.17/C-1-1"})
public void compatibleConfigTests() throws Exception {
int maxCpus = Runtime.getRuntime().availableProcessors();
- VirtualMachineConfig.Builder builder =
- newVmConfigBuilder().setPayloadBinaryName("binary.so").setApkPath("/apk/path");
- VirtualMachineConfig baseline = builder.build();
+ VirtualMachineConfig baseline = newBaselineBuilder().build();
// A config must be compatible with itself
- assertConfigCompatible(baseline, builder).isTrue();
+ assertConfigCompatible(baseline, newBaselineBuilder()).isTrue();
// Changes that must always be compatible
- assertConfigCompatible(baseline, builder.setMemoryMib(99)).isTrue();
+ assertConfigCompatible(baseline, newBaselineBuilder().setMemoryMib(99)).isTrue();
if (maxCpus > 1) {
- assertConfigCompatible(baseline, builder.setNumCpus(2)).isTrue();
+ assertConfigCompatible(baseline, newBaselineBuilder().setNumCpus(2)).isTrue();
}
// Changes that must be incompatible, since they must change the VM identity.
- assertConfigCompatible(baseline, builder.setDebugLevel(DEBUG_LEVEL_FULL)).isFalse();
- assertConfigCompatible(baseline, builder.setPayloadBinaryName("different")).isFalse();
+ assertConfigCompatible(baseline, newBaselineBuilder().setDebugLevel(DEBUG_LEVEL_FULL))
+ .isFalse();
+ assertConfigCompatible(baseline, newBaselineBuilder().setPayloadBinaryName("different"))
+ .isFalse();
int capabilities = getVirtualMachineManager().getCapabilities();
if ((capabilities & CAPABILITY_PROTECTED_VM) != 0
&& (capabilities & CAPABILITY_NON_PROTECTED_VM) != 0) {
- assertConfigCompatible(baseline, builder.setProtectedVm(!isProtectedVm())).isFalse();
+ assertConfigCompatible(baseline, newBaselineBuilder().setProtectedVm(!isProtectedVm()))
+ .isFalse();
}
// Changes that are currently incompatible for ease of implementation, but this might change
// in the future.
- assertConfigCompatible(baseline, builder.setApkPath("/different")).isFalse();
- assertConfigCompatible(baseline, builder.setEncryptedStorageKib(100)).isFalse();
+ assertConfigCompatible(baseline, newBaselineBuilder().setApkPath("/different")).isFalse();
+ assertConfigCompatible(baseline, newBaselineBuilder().setEncryptedStorageKib(100))
+ .isFalse();
}
private BooleanSubject assertConfigCompatible(