Merge "Delete some old files"
diff --git a/encryptedstore/src/main.rs b/encryptedstore/src/main.rs
index 888485b..2f54534 100644
--- a/encryptedstore/src/main.rs
+++ b/encryptedstore/src/main.rs
@@ -63,7 +63,7 @@
fn encryptedstore_init(blkdevice: &Path, key: &str, mountpoint: &Path) -> Result<()> {
ensure!(
- std::fs::metadata(&blkdevice)
+ std::fs::metadata(blkdevice)
.context(format!("Failed to get metadata of {:?}", blkdevice))?
.file_type()
.is_block_device(),
diff --git a/libs/libfdt/src/lib.rs b/libs/libfdt/src/lib.rs
index 7c72fab..c6d6c2c 100644
--- a/libs/libfdt/src/lib.rs
+++ b/libs/libfdt/src/lib.rs
@@ -508,6 +508,19 @@
fdt_err_expect_zero(ret)
}
+ /// Applies a DT overlay on the base DT.
+ ///
+ /// # Safety
+ ///
+ /// On failure, the library corrupts the DT and overlay so both must be discarded.
+ pub unsafe fn apply_overlay<'a>(&'a mut self, overlay: &'a mut Fdt) -> Result<&'a mut Self> {
+ fdt_err_expect_zero(libfdt_bindgen::fdt_overlay_apply(
+ self.as_mut_ptr(),
+ overlay.as_mut_ptr(),
+ ))?;
+ Ok(self)
+ }
+
/// Return an iterator of memory banks specified the "/memory" node.
///
/// NOTE: This does not support individual "/memory@XXXX" banks.
diff --git a/microdroid/init.rc b/microdroid/init.rc
index bc42791..d2c9c41 100644
--- a/microdroid/init.rc
+++ b/microdroid/init.rc
@@ -98,7 +98,7 @@
mount rootfs rootfs / remount bind ro nodev
# TODO(b/185767624): change the hard-coded size?
- mount tmpfs tmpfs /data noatime nosuid nodev rw size=128M
+ mount tmpfs tmpfs /data noatime nosuid nodev noexec rw size=128M
# We chown/chmod /data again so because mount is run as root + defaults
chown system system /data
diff --git a/pvmfw/README.md b/pvmfw/README.md
index e5ba88b..f46c718 100644
--- a/pvmfw/README.md
+++ b/pvmfw/README.md
@@ -1,12 +1,107 @@
# Protected Virtual Machine Firmware
-## Configuration Data Format
+In the context of the [Android Virtualization Framework][AVF], a hypervisor
+(_e.g._ [pKVM]) enforces full memory isolation between its virtual machines
+(VMs) and the host. As a result, the host is only allowed to access memory that
+has been explicitly shared back by a VM. Such _protected VMs_ (“pVMs”) are
+therefore able to manipulate secrets without being at risk of an attacker
+stealing them by compromising the Android host.
-pvmfw will expect a [header] to have been appended to its loaded binary image
-at the next 4KiB boundary. It describes the configuration data entries that
-pvmfw will use and, being loaded by the pvmfw loader, is necessarily trusted.
+As pVMs are started dynamically by a _virtual machine manager_ (“VMM”) running
+as a host process and as pVMs must not trust the host (see [_Why
+AVF?_][why-avf]), the virtual machine it configures can't be trusted either.
+Furthermore, even though the isolation mentioned above allows pVMs to protect
+their secrets from the host, it does not help with provisioning them during
+boot. In particular, the threat model would prohibit the host from ever having
+access to those secrets, preventing the VMM from passing them to the pVM.
-The layout of the configuration data is as follows:
+To address these concerns the hypervisor securely loads the pVM firmware
+(“pvmfw”) in the pVM from a protected memory region (this prevents the host or
+any pVM from tampering with it), setting it as the entry point of the virtual
+machine. As a result, pvmfw becomes the very first code that gets executed in
+the pVM, allowing it to validate the environment and abort the boot sequence if
+necessary. This process takes place whenever the VMM places a VM in protected
+mode and can’t be prevented by the host.
+
+Given the threat model, pvmfw is not allowed to trust the devices or device
+layout provided by the virtual platform it is running on as those are configured
+by the VMM. Instead, it performs all the necessary checks to ensure that the pVM
+was set up as expected. For functional purposes, the interface with the
+hypervisor, although trusted, is also validated.
+
+Once it has been determined that the platform can be trusted, pvmfw derives
+unique secrets for the guest through the [_Boot Certificate Chain_][BCC]
+("BCC", see [Open Profile for DICE][open-dice]) that can be used to prove the
+identity of the pVM to local and remote actors. If any operation or check fails,
+or in case of a missing prerequisite, pvmfw will abort the boot process of the
+pVM, effectively preventing non-compliant pVMs and/or guests from running.
+Otherwise, it hands over the pVM to the guest kernel by jumping to its first
+instruction, similarly to a bootloader.
+
+pvmfw currently only supports AArch64.
+
+[AVF]: https://source.android.com/docs/core/virtualization
+[why-avf]: https://source.android.com/docs/core/virtualization/whyavf
+[BCC]: https://pigweed.googlesource.com/open-dice/+/master/src/android/README.md
+[pKVM]: https://source.android.com/docs/core/virtualization/architecture#hypervisor
+[open-dice]: https://pigweed.googlesource.com/open-dice/+/refs/heads/main/docs/specification.md
+
+## Integration
+
+### pvmfw Loading
+
+When running pKVM, the physical memory from which the hypervisor loads pvmfw
+into guest address space is not initially populated by the hypervisor itself.
+Instead, it receives a pre-loaded memory region from a trusted pvmfw loader and
+only then becomes responsible for protecting it. As a result, the hypervisor is
+kept generic (beyond AVF) and small as it is not expected (nor necessary) for it
+to know how to interpret or obtain the content of that region.
+
+#### Android Bootloader (ABL) Support
+
+Starting in Android T, the `PRODUCT_BUILD_PVMFW_IMAGE` build variable controls
+the generation of `pvmfw.img`, a new [ABL partition][ABL-part] containing the
+pvmfw binary and following the internal format of the [`boot`][boot-img]
+partition, intended to be verified and loaded by ABL on AVF-compatible devices.
+
+To support pKVM, ABL is expected to describe the region using a reserved memory
+device tree node where both address and size have been properly aligned to the
+page size used by the hypervisor. For example, the following node describes a
+region of size `0x40000` at address `0x80000000`:
+```
+reserved-memory {
+ ...
+ pkvm_guest_firmware {
+ compatible = "linux,pkvm-guest-firmware-memory";
+ reg = <0x0 0x80000000 0x40000>;
+ no-map;
+ }
+}
+```
+
+[ABL-part]: https://source.android.com/docs/core/architecture/bootloader/partitions
+[boot-img]: https://source.android.com/docs/core/architecture/bootloader/boot-image-header
+
+### Configuration Data
+
+As part of the process of loading pvmfw, the loader (typically the Android
+Bootloader, "ABL") is expected to pass device-specific pvmfw configuration data
+by appending it to the pvmfw binary and including it in the region passed to the
+hypervisor. As a result, the hypervisor will give the same protection to this
+data as it does to pvmfw and will transparently load it in guest memory, making
+it available to pvmfw at runtime. This enables pvmfw to be kept device-agnostic,
+simplifying its adoption and distribution as a centralized signed binary, while
+also being able to support device-specific details.
+
+The configuration data will be read by pvmfw at the next 4KiB boundary from the
+end of its loaded binary. Even if the pvmfw is position-independent, it will be
+expected for it to also have been loaded at a 4-KiB boundary. As a result, the
+location of the configuration data is implicitly passed to pvmfw and known to it
+at build time.
+
+#### Configuration Data Format
+
+The configuration data is described using the following [header]:
```
+===============================+
@@ -64,9 +159,62 @@
The header format itself is agnostic of the internal format of the individual
blos it refers to. In version 1.0, it describes two blobs:
-- entry 0 must point to a valid [BCC Handover]
+- entry 0 must point to a valid BCC Handover (see below)
- entry 1 may point to a [DTBO] to be applied to the pVM device tree
[header]: src/config.rs
-[BCC Handover]: https://pigweed.googlesource.com/open-dice/+/825e3beb6c6efcd8c35506d818c18d1e73b9834a/src/android/bcc.c#260
[DTBO]: https://android.googlesource.com/platform/external/dtc/+/refs/heads/master/Documentation/dt-object-internal.txt
+
+#### Virtual Platform Boot Certificate Chain Handover
+
+The format of the BCC entry mentioned above, compatible with the
+[`BccHandover`][BccHandover] defined by the Open Profile for DICE reference
+implementation, is described by the following [CDDL][CDDL]:
+```
+PvmfwBccHandover = {
+ 1 : bstr .size 32, ; CDI_Attest
+ 2 : bstr .size 32, ; CDI_Seal
+ 3 : Bcc, ; Certificate chain
+}
+```
+
+and contains the _Compound Device Identifiers_ ("CDIs"), used to derive the
+next-stage secret, and a certificate chain, intended for pVM attestation. Note
+that it differs from the `BccHandover` defined by the specification in that its
+`Bcc` field is mandatory (while optional in the original).
+
+The handover expected by pvmfw can be generated as follows:
+
+- by passing a `BccHandover` received from a previous boot stage (_e.g._ Trusted
+ Firmware, ROM bootloader, ...) to
+ [`BccHandoverMainFlow`][BccHandoverMainFlow];
+
+- by generating a `BccHandover` (as an example, see [Trusty][Trusty-BCC]) with
+ both CDIs set to an arbitrary constant value and no `Bcc`, and pass it to
+ `BccHandoverMainFlow`, which will both derive the pvmfw CDIs and start a
+ valid certificate chain, making the pvmfw loader the root of the BCC.
+
+The recommended DICE inputs at this stage are:
+
+- **Code**: hash of the pvmfw image, hypervisor (`boot.img`), and other target
+ code relevant to the secure execution of pvmfw (_e.g._ `vendor_boot.img`)
+- **Configuration Data**: any extra input relevant to pvmfw security
+- **Authority Data**: must cover all the public keys used to sign and verify the
+ code contributing to the **Code** input
+- **Mode Decision**: Set according to the [specification][dice-mode]. In
+ particular, should only be `Normal` if secure boot is being properly enforced
+ (_e.g._ locked device in [Android Verified Boot][AVB])
+- **Hidden Inputs**: Factory Reset Secret (FRS, stored in a tamper evident
+ storage and changes during every factory reset) or similar that changes as
+ part of the device lifecycle (_e.g._ reset)
+
+The resulting `BccHandover` is then used by pvmfw in a similar way to derive
+another [DICE layer][Layering], passed to the guest through a `/reserved-memory`
+device tree node marked as [`compatible=”google,open-dice”`][dice-dt].
+
+[AVB]: https://source.android.com/docs/security/features/verifiedboot/boot-flow
+[BccHandover]: https://pigweed.googlesource.com/open-dice/+/825e3beb6c/src/android/bcc.c#260
+[CDDL]: https://datatracker.ietf.org/doc/rfc8610
+[dice-dt]: https://www.kernel.org/doc/Documentation/devicetree/bindings/reserved-memory/google%2Copen-dice.yaml
+[Layering]: https://pigweed.googlesource.com/open-dice/+/refs/heads/main/docs/specification.md#layering-details
+[Trusty-BCC]: https://android.googlesource.com/trusty/lib/+/1696be0a8f3a7103/lib/hwbcc/common/swbcc.c#554
diff --git a/pvmfw/avb/src/utils.rs b/pvmfw/avb/src/utils.rs
index bb27497..1b35c22 100644
--- a/pvmfw/avb/src/utils.rs
+++ b/pvmfw/avb/src/utils.rs
@@ -16,8 +16,11 @@
use crate::error::AvbIOError;
use core::ptr::NonNull;
+use core::result;
-pub(crate) fn write<T>(ptr: *mut T, value: T) -> Result<(), AvbIOError> {
+pub(crate) type Result<T> = result::Result<T, AvbIOError>;
+
+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.
unsafe {
@@ -26,17 +29,17 @@
Ok(())
}
-pub(crate) fn as_ref<'a, T>(ptr: *mut T) -> Result<&'a T, AvbIOError> {
+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.
unsafe { Ok(ptr.as_ref()) }
}
-pub(crate) fn to_nonnull<T>(ptr: *mut T) -> Result<NonNull<T>, AvbIOError> {
+pub(crate) fn to_nonnull<T>(ptr: *mut T) -> Result<NonNull<T>> {
NonNull::new(ptr).ok_or(AvbIOError::NoSuchValue)
}
-pub(crate) fn is_not_null<T>(ptr: *const T) -> Result<(), AvbIOError> {
+pub(crate) fn is_not_null<T>(ptr: *const T) -> Result<()> {
if ptr.is_null() {
Err(AvbIOError::NoSuchValue)
} else {
@@ -44,10 +47,10 @@
}
}
-pub(crate) fn to_usize<T: TryInto<usize>>(num: T) -> Result<usize, AvbIOError> {
+pub(crate) fn to_usize<T: TryInto<usize>>(num: T) -> Result<usize> {
num.try_into().map_err(|_| AvbIOError::InvalidValueSize)
}
-pub(crate) fn usize_checked_add(x: usize, y: usize) -> Result<usize, AvbIOError> {
+pub(crate) fn usize_checked_add(x: usize, y: usize) -> Result<usize> {
x.checked_add(y).ok_or(AvbIOError::InvalidValueSize)
}
diff --git a/pvmfw/avb/tests/utils.rs b/pvmfw/avb/tests/utils.rs
index 0d9657e..0a2eac6 100644
--- a/pvmfw/avb/tests/utils.rs
+++ b/pvmfw/avb/tests/utils.rs
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-//! Utility methods used by API tests.
+//! Utility functions used by API tests.
use anyhow::Result;
use avb_bindgen::{
diff --git a/pvmfw/src/entry.rs b/pvmfw/src/entry.rs
index bfcb423..4f30902 100644
--- a/pvmfw/src/entry.rs
+++ b/pvmfw/src/entry.rs
@@ -178,6 +178,37 @@
}
}
+/// Applies the debug policy device tree overlay to the pVM DT.
+///
+/// # Safety
+///
+/// When an error is returned by this function, the input `Fdt` should be discarded as it may have
+/// have been partially corrupted during the overlay application process.
+unsafe fn apply_debug_policy(
+ fdt: &mut libfdt::Fdt,
+ debug_policy: &mut [u8],
+) -> Result<(), RebootReason> {
+ let overlay = libfdt::Fdt::from_mut_slice(debug_policy).map_err(|e| {
+ error!("Failed to load the debug policy overlay: {e}");
+ RebootReason::InvalidConfig
+ })?;
+
+ fdt.unpack().map_err(|e| {
+ error!("Failed to unpack DT for debug policy: {e}");
+ RebootReason::InternalError
+ })?;
+
+ let fdt = fdt.apply_overlay(overlay).map_err(|e| {
+ error!("Failed to apply the debug policy overlay: {e}");
+ RebootReason::InvalidConfig
+ })?;
+
+ fdt.pack().map_err(|e| {
+ error!("Failed to re-pack DT after debug policy: {e}");
+ RebootReason::InternalError
+ })
+}
+
/// Sets up the environment for main() and wraps its result for start().
///
/// Provide the abstractions necessary for start() to abort the pVM boot and for main() to run with
@@ -252,6 +283,11 @@
helpers::flushed_zeroize(bcc_slice);
helpers::flush(slices.fdt.as_slice());
+ if let Some(debug_policy) = appended.get_debug_policy() {
+ // SAFETY - As we `?` the result, there is no risk of re-using a bad `slices.fdt`.
+ unsafe { apply_debug_policy(slices.fdt, debug_policy) }?;
+ }
+
info!("Expecting a bug making MMIO_GUARD_UNMAP return NOT_SUPPORTED on success");
memory.mmio_unmap_all().map_err(|e| {
error!("Failed to unshare MMIO ranges: {e}");
diff --git a/tests/aidl/com/android/microdroid/testservice/ITestService.aidl b/tests/aidl/com/android/microdroid/testservice/ITestService.aidl
index 7ee1f01..aa5b32d 100644
--- a/tests/aidl/com/android/microdroid/testservice/ITestService.aidl
+++ b/tests/aidl/com/android/microdroid/testservice/ITestService.aidl
@@ -55,4 +55,7 @@
/* get the content of the specified file. */
String readFromFile(String path);
+
+ /** Returns flags for the given mountPoint. */
+ int getMountFlags(String mountPoint);
}
diff --git a/tests/testapk/Android.bp b/tests/testapk/Android.bp
index e3c9961..9eda246 100644
--- a/tests/testapk/Android.bp
+++ b/tests/testapk/Android.bp
@@ -47,6 +47,7 @@
static_libs: [
"com.android.microdroid.testservice-ndk",
"libbase",
+ "libfstab",
"libfsverity_digests_proto_cc",
"liblog",
"libprotobuf-cpp-lite-ndk",
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 e1a2e40..aba6f65 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -24,6 +24,7 @@
import static android.system.virtualmachine.VirtualMachineManager.CAPABILITY_PROTECTED_VM;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.common.truth.TruthJUnit.assume;
import static org.junit.Assert.assertThrows;
@@ -1400,6 +1401,34 @@
assertThat(checkVmOutputIsRedirectedToLogcat(false)).isFalse();
}
+ // Take from bionic/libs/kernel/uapi/linux/mounth.h.
+ private static final int MS_NOEXEC = 8;
+
+ @Test
+ public void dataIsMountedWithNoExec() throws Exception {
+ assumeSupportedKernel();
+
+ final VirtualMachineConfig vmConfig =
+ newVmConfigBuilder()
+ .setPayloadBinaryName("MicrodroidTestNativeLib.so")
+ .setMemoryMib(minMemoryRequired())
+ .setDebugLevel(DEBUG_LEVEL_FULL)
+ .build();
+ final VirtualMachine vm = forceCreateNewVirtualMachine("test_vm_data_mount", vmConfig);
+
+ final TestResults testResults =
+ runVmTestService(
+ vm,
+ (ts, tr) -> {
+ tr.mountFlags = ts.getMountFlags("/data");
+ });
+
+ assertThat(testResults.mException).isNull();
+ assertWithMessage("/data should be mounted with MS_NOEXEC")
+ .that(testResults.mountFlags & MS_NOEXEC)
+ .isEqualTo(MS_NOEXEC);
+ }
+
private void assertFileContentsAreEqualInTwoVms(String fileName, String vmName1, String vmName2)
throws IOException {
File file1 = getVmFile(vmName1, fileName);
@@ -1456,6 +1485,7 @@
String mEncryptedStoragePath;
String[] mEffectiveCapabilities;
String mFileContent;
+ int mountFlags;
}
private TestResults runVmTestService(VirtualMachine vm, RunTestsAgainstTestService testsToRun)
diff --git a/tests/testapk/src/native/testbinary.cpp b/tests/testapk/src/native/testbinary.cpp
index 4ba502a..f5f6783 100644
--- a/tests/testapk/src/native/testbinary.cpp
+++ b/tests/testapk/src/native/testbinary.cpp
@@ -21,6 +21,7 @@
#include <android-base/scopeguard.h>
#include <android/log.h>
#include <fcntl.h>
+#include <fstab/fstab.h>
#include <fsverity_digests.pb.h>
#include <linux/vm_sockets.h>
#include <stdint.h>
@@ -31,6 +32,7 @@
#include <vm_main.h>
#include <vm_payload_restricted.h>
+#include <cstdint>
#include <string>
#include <thread>
@@ -40,6 +42,10 @@
using android::base::make_scope_guard;
using android::base::Result;
using android::base::unique_fd;
+using android::fs_mgr::Fstab;
+using android::fs_mgr::FstabEntry;
+using android::fs_mgr::GetEntryForMountPoint;
+using android::fs_mgr::ReadFstabFromFile;
using aidl::com::android::microdroid::testservice::BnTestService;
using ndk::ScopedAStatus;
@@ -255,6 +261,22 @@
}
return ScopedAStatus::ok();
}
+
+ ScopedAStatus getMountFlags(const std::string& mount_point, int32_t* out) override {
+ Fstab fstab;
+ if (!ReadFstabFromFile("/proc/mounts", &fstab)) {
+ return ScopedAStatus::fromExceptionCodeWithMessage(EX_SERVICE_SPECIFIC,
+ "Failed to read /proc/mounts");
+ }
+ FstabEntry* entry = GetEntryForMountPoint(&fstab, mount_point);
+ if (entry == nullptr) {
+ std::string msg = mount_point + " not found in /proc/mounts";
+ return ScopedAStatus::fromExceptionCodeWithMessage(EX_SERVICE_SPECIFIC,
+ msg.c_str());
+ }
+ *out = entry->flags;
+ return ScopedAStatus::ok();
+ }
};
auto testService = ndk::SharedRefBase::make<TestService>();