Merge "[vm_attestation] Add more detail about the RKP VM marker" into main
diff --git a/android/vm/Android.bp b/android/vm/Android.bp
index c1d9b6b..ba8b416 100644
--- a/android/vm/Android.bp
+++ b/android/vm/Android.bp
@@ -16,6 +16,7 @@
"libbinder_rs",
"libclap",
"libenv_logger",
+ "libcfg_if",
"libglob",
"libhypervisor_props",
"liblibc",
diff --git a/android/vm/src/main.rs b/android/vm/src/main.rs
index f2c2fa4..609bbdf 100644
--- a/android/vm/src/main.rs
+++ b/android/vm/src/main.rs
@@ -75,14 +75,14 @@
}
impl CommonConfig {
- #[cfg(network)]
fn network_supported(&self) -> bool {
- self.network_supported
- }
-
- #[cfg(not(network))]
- fn network_supported(&self) -> bool {
- false
+ cfg_if::cfg_if! {
+ if #[cfg(network)] {
+ self.network_supported
+ } else {
+ false
+ }
+ }
}
}
@@ -117,14 +117,14 @@
}
impl DebugConfig {
- #[cfg(debuggable_vms_improvements)]
fn enable_earlycon(&self) -> bool {
- self.enable_earlycon
- }
-
- #[cfg(not(debuggable_vms_improvements))]
- fn enable_earlycon(&self) -> bool {
- false
+ cfg_if::cfg_if! {
+ if #[cfg(debuggable_vms_improvements)] {
+ self.enable_earlycon
+ } else {
+ false
+ }
+ }
}
}
@@ -158,34 +158,34 @@
}
impl MicrodroidConfig {
- #[cfg(vendor_modules)]
fn vendor(&self) -> Option<&PathBuf> {
- self.vendor.as_ref()
+ cfg_if::cfg_if! {
+ if #[cfg(vendor_modules)] {
+ self.vendor.as_ref()
+ } else {
+ None
+ }
+ }
}
- #[cfg(not(vendor_modules))]
- fn vendor(&self) -> Option<&PathBuf> {
- None
- }
-
- #[cfg(vendor_modules)]
fn gki(&self) -> Option<&str> {
- self.gki.as_deref()
+ cfg_if::cfg_if! {
+ if #[cfg(vendor_modules)] {
+ self.gki.as_deref()
+ } else {
+ None
+ }
+ }
}
- #[cfg(not(vendor_modules))]
- fn gki(&self) -> Option<&str> {
- None
- }
-
- #[cfg(device_assignment)]
fn devices(&self) -> &[PathBuf] {
- &self.devices
- }
-
- #[cfg(not(device_assignment))]
- fn devices(&self) -> &[PathBuf] {
- &[]
+ cfg_if::cfg_if! {
+ if #[cfg(device_assignment)] {
+ &self.devices
+ } else {
+ &[]
+ }
+ }
}
}
@@ -236,35 +236,36 @@
}
impl RunAppConfig {
- #[cfg(multi_tenant)]
fn extra_apks(&self) -> &[PathBuf] {
- &self.extra_apks
+ cfg_if::cfg_if! {
+ if #[cfg(multi_tenant)] {
+ &self.extra_apks
+ } else {
+ &[]
+ }
+ }
}
- #[cfg(not(multi_tenant))]
- fn extra_apks(&self) -> &[PathBuf] {
- &[]
- }
-
- #[cfg(llpvm_changes)]
fn instance_id(&self) -> Result<PathBuf, Error> {
- Ok(self.instance_id.clone())
+ cfg_if::cfg_if! {
+ if #[cfg(llpvm_changes)] {
+ Ok(self.instance_id.clone())
+ } else {
+ Err(anyhow!("LLPVM feature is disabled, --instance_id flag not supported"))
+ }
+ }
}
- #[cfg(not(llpvm_changes))]
- fn instance_id(&self) -> Result<PathBuf, Error> {
- Err(anyhow!("LLPVM feature is disabled, --instance_id flag not supported"))
- }
-
- #[cfg(llpvm_changes)]
fn set_instance_id(&mut self, instance_id_file: PathBuf) -> Result<(), Error> {
- self.instance_id = instance_id_file;
- Ok(())
- }
-
- #[cfg(not(llpvm_changes))]
- fn set_instance_id(&mut self, _: PathBuf) -> Result<(), Error> {
- Err(anyhow!("LLPVM feature is disabled, --instance_id flag not supported"))
+ cfg_if::cfg_if! {
+ if #[cfg(llpvm_changes)] {
+ self.instance_id = instance_id_file;
+ Ok(())
+ } else {
+ let _ = instance_id_file;
+ Err(anyhow!("LLPVM feature is disabled, --instance_id flag not supported"))
+ }
+ }
}
}
diff --git a/docs/vm_remote_attestation.md b/docs/vm_remote_attestation.md
index 6a18031..ee20591 100644
--- a/docs/vm_remote_attestation.md
+++ b/docs/vm_remote_attestation.md
@@ -51,10 +51,12 @@
Once the RKP VM is successfully attested, it acts as a trusted platform to
attest pVMs. Leveraging its trusted status, the RKP VM validates the integrity
-of each pVM's DICE chain by comparing it against its own DICE chain. This
-validation process ensures that the pVMs are running in the expected VM
-environment and certifies the payload executed within each pVM. Currently, only
-Microdroid VMs are supported.
+of each [pVM DICE chain][pvm-dice-chain] by comparing it against its own DICE
+chain. This validation process ensures that the pVMs are running in the expected
+VM environment and certifies the payload executed within each pVM. Currently,
+only Microdroid VMs are supported.
+
+[pvm-dice-chain]: ./pvm_dice_chain.md
## API
diff --git a/guest/microdroid_manager/src/main.rs b/guest/microdroid_manager/src/main.rs
index 7352a2c..8186e9d 100644
--- a/guest/microdroid_manager/src/main.rs
+++ b/guest/microdroid_manager/src/main.rs
@@ -654,7 +654,7 @@
if requested {
let status = Command::new("/system/bin/kexec_load").status()?;
if !status.success() {
- return Err(anyhow!("Failed to load crashkernel: {:?}", status));
+ return Err(anyhow!("Failed to load crashkernel: {status}"));
}
info!("ramdump is loaded: debuggable={debuggable}, ramdump={ramdump}");
}
diff --git a/guest/pvmfw/Android.bp b/guest/pvmfw/Android.bp
index 144e81e..b502af6 100644
--- a/guest/pvmfw/Android.bp
+++ b/guest/pvmfw/Android.bp
@@ -13,7 +13,6 @@
rustlibs: [
"libaarch64_paging",
"libbssl_avf_nostd",
- "libbssl_sys_nostd",
"libcbor_util_nostd",
"libciborium_nostd",
"libciborium_io_nostd",
diff --git a/guest/pvmfw/src/entry.rs b/guest/pvmfw/src/entry.rs
index 8f9340b..945ad6a 100644
--- a/guest/pvmfw/src/entry.rs
+++ b/guest/pvmfw/src/entry.rs
@@ -17,7 +17,6 @@
use crate::config;
use crate::fdt;
use crate::memory;
-use bssl_sys::CRYPTO_library_init;
use core::arch::asm;
use core::mem::{drop, size_of};
use core::num::NonZeroUsize;
@@ -216,12 +215,6 @@
// - only access non-pvmfw memory once (and while) it has been mapped
log::set_max_level(LevelFilter::Info);
- // TODO(https://crbug.com/boringssl/35): Remove this init when BoringSSL can handle this
- // internally.
- // SAFETY: Configures the internal state of the library - may be called multiple times.
- unsafe {
- CRYPTO_library_init();
- }
let page_table = memory::init_page_table().map_err(|e| {
error!("Failed to set up the dynamic page tables: {e}");
diff --git a/guest/rialto/Android.bp b/guest/rialto/Android.bp
index b26a1c4..4c18bf9 100644
--- a/guest/rialto/Android.bp
+++ b/guest/rialto/Android.bp
@@ -10,7 +10,6 @@
rustlibs: [
"libaarch64_paging",
"libbssl_avf_nostd",
- "libbssl_sys_nostd",
"libciborium_io_nostd",
"libciborium_nostd",
"libcstr",
diff --git a/guest/rialto/src/main.rs b/guest/rialto/src/main.rs
index 7de8718..9265775 100644
--- a/guest/rialto/src/main.rs
+++ b/guest/rialto/src/main.rs
@@ -28,7 +28,6 @@
use crate::error::{Error, Result};
use crate::fdt::{read_dice_range_from, read_is_strict_boot, read_vendor_hashtree_root_digest};
use alloc::boxed::Box;
-use bssl_sys::CRYPTO_library_init;
use ciborium_io::Write;
use core::num::NonZeroUsize;
use core::slice;
@@ -133,12 +132,6 @@
})?;
}
- // Initializes the crypto library before any crypto operations and after the heap is
- // initialized.
- // SAFETY: It is safe to call this function multiple times and concurrently.
- unsafe {
- CRYPTO_library_init();
- }
let bcc_handover: Box<dyn DiceArtifacts> = match vm_type(fdt)? {
VmType::ProtectedVm => {
let dice_range = read_dice_range_from(fdt)?;
@@ -228,6 +221,28 @@
}
}
+/// Flushes data caches over the provided address range.
+///
+/// # Safety
+///
+/// The provided address and size must be to an address range that is valid for read and write
+/// (typically on the stack, .bss, .data, or provided BCC) from a single allocation
+/// (e.g. stack array).
+#[no_mangle]
+unsafe extern "C" fn DiceClearMemory(
+ _ctx: *mut core::ffi::c_void,
+ size: usize,
+ addr: *mut core::ffi::c_void,
+) {
+ use core::slice;
+ use vmbase::memory::flushed_zeroize;
+
+ // SAFETY: We require our caller to provide a valid range within a single object. The open-dice
+ // always calls this on individual stack-allocated arrays which ensures that.
+ let region = unsafe { slice::from_raw_parts_mut(addr as *mut u8, size) };
+ flushed_zeroize(region)
+}
+
generate_image_header!();
main!(main);
configure_heap!(SIZE_128KB * 2);
diff --git a/libs/dice/open_dice/Android.bp b/libs/dice/open_dice/Android.bp
index efe350f..d1129fb 100644
--- a/libs/dice/open_dice/Android.bp
+++ b/libs/dice/open_dice/Android.bp
@@ -22,7 +22,6 @@
"alloc",
],
whole_static_libs: [
- "libopen_dice_cbor",
"libcrypto_baremetal",
],
visibility: [
@@ -55,6 +54,7 @@
"//packages/modules/Virtualization:__subpackages__",
"//system/authgraph/tests:__subpackages__",
"//system/secretkeeper/client:__subpackages__",
+ "//system/software_defined_vehicle:__subpackages__",
],
apex_available: [
"//apex_available:platform",
diff --git a/libs/dice/sample_inputs/tests/api_test.rs b/libs/dice/sample_inputs/tests/api_test.rs
index 0823f16..d713168 100644
--- a/libs/dice/sample_inputs/tests/api_test.rs
+++ b/libs/dice/sample_inputs/tests/api_test.rs
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#![cfg_attr(not(feature = "std"), no_std)]
+
use anyhow::Result;
use diced_open_dice::{derive_cdi_leaf_priv, sign, DiceArtifacts};
use diced_sample_inputs::make_sample_bcc_and_cdis;
@@ -144,3 +146,21 @@
let public_key = chain.leaf().subject_public_key();
public_key.verify(&signature, MESSAGE)
}
+
+/// Flushes data caches over the provided address range in open-dice.
+///
+/// # Safety
+///
+/// The provided address and size must be to an address range that is valid for read and write
+/// (typically on the stack, .bss, .data, or provided BCC) from a single allocation
+/// (e.g. stack array).
+#[cfg(not(feature = "std"))]
+#[no_mangle]
+unsafe extern "C" fn DiceClearMemory(
+ _ctx: *mut core::ffi::c_void,
+ size: usize,
+ addr: *mut core::ffi::c_void,
+) {
+ // SAFETY: The caller ensures that the address and size are valid for write.
+ unsafe { core::ptr::write_bytes(addr as *mut u8, 0, size) };
+}
diff --git a/libs/libinherited_fd/Android.bp b/libs/libinherited_fd/Android.bp
new file mode 100644
index 0000000..28ec2e5
--- /dev/null
+++ b/libs/libinherited_fd/Android.bp
@@ -0,0 +1,44 @@
+package {
+ default_applicable_licenses: ["Android-Apache-2.0"],
+}
+
+rust_defaults {
+ name: "libinherited_fd.defaults",
+ crate_name: "inherited_fd",
+ srcs: ["src/lib.rs"],
+ edition: "2021",
+ rustlibs: [
+ "libnix",
+ "libonce_cell",
+ "libthiserror",
+ ],
+}
+
+rust_library {
+ name: "libinherited_fd",
+ defaults: ["libinherited_fd.defaults"],
+ apex_available: [
+ "com.android.compos",
+ "com.android.virt",
+ ],
+}
+
+rust_test {
+ name: "libinherited_fd.test",
+ defaults: ["libinherited_fd.defaults"],
+ rustlibs: [
+ "libanyhow",
+ "libtempfile",
+ ],
+ host_supported: true,
+ test_suites: ["general-tests"],
+ test_options: {
+ unit_test: true,
+ },
+ // this is to run each test function in a separate process.
+ // note that they still run in parallel.
+ flags: [
+ "-C panic=abort",
+ "-Z panic_abort_tests",
+ ],
+}
diff --git a/libs/libinherited_fd/src/lib.rs b/libs/libinherited_fd/src/lib.rs
new file mode 100644
index 0000000..f5e2d6b
--- /dev/null
+++ b/libs/libinherited_fd/src/lib.rs
@@ -0,0 +1,270 @@
+// Copyright 2024, 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.
+
+//! Library for safely obtaining `OwnedFd` for inherited file descriptors.
+
+use nix::fcntl::{fcntl, FdFlag, F_SETFD};
+use nix::libc;
+use std::collections::HashMap;
+use std::fs::canonicalize;
+use std::fs::read_dir;
+use std::os::fd::FromRawFd;
+use std::os::fd::OwnedFd;
+use std::os::fd::RawFd;
+use std::sync::Mutex;
+use std::sync::OnceLock;
+use thiserror::Error;
+
+/// Errors that can occur while taking an ownership of `RawFd`
+#[derive(Debug, PartialEq, Error)]
+pub enum Error {
+ /// init_once() not called
+ #[error("init_once() not called")]
+ NotInitialized,
+
+ /// Ownership already taken
+ #[error("Ownership of FD {0} is already taken")]
+ OwnershipTaken(RawFd),
+
+ /// Not an inherited file descriptor
+ #[error("FD {0} is either invalid file descriptor or not an inherited one")]
+ FileDescriptorNotInherited(RawFd),
+
+ /// Failed to set CLOEXEC
+ #[error("Failed to set CLOEXEC on FD {0}")]
+ FailCloseOnExec(RawFd),
+}
+
+static INHERITED_FDS: OnceLock<Mutex<HashMap<RawFd, Option<OwnedFd>>>> = OnceLock::new();
+
+/// Take ownership of all open file descriptors in this process, which later can be obtained by
+/// calling `take_fd_ownership`.
+///
+/// # Safety
+/// This function has to be called very early in the program before the ownership of any file
+/// descriptors (except stdin/out/err) is taken.
+pub unsafe fn init_once() -> Result<(), std::io::Error> {
+ let mut fds = HashMap::new();
+
+ let fd_path = canonicalize("/proc/self/fd")?;
+
+ for entry in read_dir(&fd_path)? {
+ let entry = entry?;
+
+ // Files in /prod/self/fd are guaranteed to be numbers. So parsing is always successful.
+ let file_name = entry.file_name();
+ let raw_fd = file_name.to_str().unwrap().parse::<RawFd>().unwrap();
+
+ // We don't take ownership of the stdio FDs as the Rust runtime owns them.
+ if [libc::STDIN_FILENO, libc::STDOUT_FILENO, libc::STDERR_FILENO].contains(&raw_fd) {
+ continue;
+ }
+
+ // Exceptional case: /proc/self/fd/* may be a dir fd created by read_dir just above. Since
+ // the file descriptor is owned by read_dir (and thus closed by it), we shouldn't take
+ // ownership to it.
+ if entry.path().read_link()? == fd_path {
+ continue;
+ }
+
+ // SAFETY: /proc/self/fd/* are file descriptors that are open. If `init_once()` was called
+ // at the very beginning of the program execution (as requested by the safety requirement
+ // of this function), this is the first time to claim the ownership of these file
+ // descriptors.
+ let owned_fd = unsafe { OwnedFd::from_raw_fd(raw_fd) };
+ fds.insert(raw_fd, Some(owned_fd));
+ }
+
+ INHERITED_FDS
+ .set(Mutex::new(fds))
+ .or(Err(std::io::Error::other("Inherited fds were already initialized")))
+}
+
+/// Take the ownership of the given `RawFd` and returns `OwnedFd` for it. The returned FD is set
+/// CLOEXEC. `Error` is returned when the ownership was already taken (by a prior call to this
+/// function with the same `RawFd`) or `RawFd` is not an inherited file descriptor.
+pub fn take_fd_ownership(raw_fd: RawFd) -> Result<OwnedFd, Error> {
+ let mut fds = INHERITED_FDS.get().ok_or(Error::NotInitialized)?.lock().unwrap();
+
+ if let Some(value) = fds.get_mut(&raw_fd) {
+ if let Some(owned_fd) = value.take() {
+ fcntl(raw_fd, F_SETFD(FdFlag::FD_CLOEXEC)).or(Err(Error::FailCloseOnExec(raw_fd)))?;
+ Ok(owned_fd)
+ } else {
+ Err(Error::OwnershipTaken(raw_fd))
+ }
+ } else {
+ Err(Error::FileDescriptorNotInherited(raw_fd))
+ }
+}
+
+#[cfg(test)]
+mod test {
+ use super::*;
+ use anyhow::Result;
+ use nix::fcntl::{fcntl, FdFlag, F_GETFD, F_SETFD};
+ use nix::unistd::close;
+ use std::os::fd::{AsRawFd, IntoRawFd};
+ use tempfile::tempfile;
+
+ struct Fixture {
+ fds: Vec<RawFd>,
+ }
+
+ impl Fixture {
+ fn setup(num_fds: usize) -> Result<Self> {
+ let mut fds = Vec::new();
+ for _ in 0..num_fds {
+ fds.push(tempfile()?.into_raw_fd());
+ }
+ Ok(Fixture { fds })
+ }
+
+ fn open_new_file(&mut self) -> Result<RawFd> {
+ let raw_fd = tempfile()?.into_raw_fd();
+ self.fds.push(raw_fd);
+ Ok(raw_fd)
+ }
+ }
+
+ impl Drop for Fixture {
+ fn drop(&mut self) {
+ self.fds.iter().for_each(|fd| {
+ let _ = close(*fd);
+ });
+ }
+ }
+
+ fn is_fd_opened(raw_fd: RawFd) -> bool {
+ fcntl(raw_fd, F_GETFD).is_ok()
+ }
+
+ #[test]
+ fn happy_case() -> Result<()> {
+ let fixture = Fixture::setup(2)?;
+ let f0 = fixture.fds[0];
+ let f1 = fixture.fds[1];
+
+ // SAFETY: assume files opened by Fixture are inherited ones
+ unsafe {
+ init_once()?;
+ }
+
+ let f0_owned = take_fd_ownership(f0)?;
+ let f1_owned = take_fd_ownership(f1)?;
+ assert_eq!(f0, f0_owned.as_raw_fd());
+ assert_eq!(f1, f1_owned.as_raw_fd());
+
+ drop(f0_owned);
+ drop(f1_owned);
+ assert!(!is_fd_opened(f0));
+ assert!(!is_fd_opened(f1));
+ Ok(())
+ }
+
+ #[test]
+ fn access_non_inherited_fd() -> Result<()> {
+ let mut fixture = Fixture::setup(2)?;
+
+ // SAFETY: assume files opened by Fixture are inherited ones
+ unsafe {
+ init_once()?;
+ }
+
+ let f = fixture.open_new_file()?;
+ assert_eq!(Some(Error::FileDescriptorNotInherited(f)), take_fd_ownership(f).err());
+ Ok(())
+ }
+
+ #[test]
+ fn call_init_once_multiple_times() -> Result<()> {
+ let _ = Fixture::setup(2)?;
+
+ // SAFETY: assume files opened by Fixture are inherited ones
+ unsafe {
+ init_once()?;
+ }
+
+ // SAFETY: for testing
+ let res = unsafe { init_once() };
+ assert!(res.is_err());
+ Ok(())
+ }
+
+ #[test]
+ fn access_without_init_once() -> Result<()> {
+ let fixture = Fixture::setup(2)?;
+
+ let f = fixture.fds[0];
+ assert_eq!(Some(Error::NotInitialized), take_fd_ownership(f).err());
+ Ok(())
+ }
+
+ #[test]
+ fn double_ownership() -> Result<()> {
+ let fixture = Fixture::setup(2)?;
+ let f = fixture.fds[0];
+
+ // SAFETY: assume files opened by Fixture are inherited ones
+ unsafe {
+ init_once()?;
+ }
+
+ let f_owned = take_fd_ownership(f)?;
+ let f_double_owned = take_fd_ownership(f);
+ assert_eq!(Some(Error::OwnershipTaken(f)), f_double_owned.err());
+
+ // just to highlight that f_owned is kept alive when the second call to take_fd_ownership
+ // is made.
+ drop(f_owned);
+ Ok(())
+ }
+
+ #[test]
+ fn take_drop_retake() -> Result<()> {
+ let fixture = Fixture::setup(2)?;
+ let f = fixture.fds[0];
+
+ // SAFETY: assume files opened by Fixture are inherited ones
+ unsafe {
+ init_once()?;
+ }
+
+ let f_owned = take_fd_ownership(f)?;
+ drop(f_owned);
+
+ let f_double_owned = take_fd_ownership(f);
+ assert_eq!(Some(Error::OwnershipTaken(f)), f_double_owned.err());
+ Ok(())
+ }
+
+ #[test]
+ fn cloexec() -> Result<()> {
+ let fixture = Fixture::setup(2)?;
+ let f = fixture.fds[0];
+
+ // SAFETY: assume files opened by Fixture are inherited ones
+ unsafe {
+ init_once()?;
+ }
+
+ // Intentionally cleaar cloexec to see if it is set by take_fd_ownership
+ fcntl(f, F_SETFD(FdFlag::empty()))?;
+
+ let f_owned = take_fd_ownership(f)?;
+ let flags = fcntl(f_owned.as_raw_fd(), F_GETFD)?;
+ assert_eq!(flags, FdFlag::FD_CLOEXEC.bits());
+ Ok(())
+ }
+}