Merge "pvmfw: Improve memory sharing with the host"
diff --git a/pvmfw/README.md b/pvmfw/README.md
index 1eb7286..4e93648 100644
--- a/pvmfw/README.md
+++ b/pvmfw/README.md
@@ -197,20 +197,16 @@
that it differs from the `BccHandover` defined by the specification in that its
`Bcc` field is mandatory (while optional in the original).
-Ideally devices that fully implement DICE should provide a certificate rooted at
-the Unique Device Secret (UDS) in a boot stage preceding the pvmfw loader
-(typically ABL), in such a way that it would receive a valid `BccHandover`, that
-can be passed to [`BccHandoverMainFlow`][BccHandoverMainFlow] along with the
-inputs described below.
+Devices that fully implement DICE should provide a certificate rooted at the
+Unique Device Secret (UDS) in a boot stage preceding the pvmfw loader (typically
+ABL), in such a way that it would receive a valid `BccHandover`, that can be
+passed to [`BccHandoverMainFlow`][BccHandoverMainFlow] along with the inputs
+described below.
-However, there is a limitation in Android 14 that means that a UDS-rooted DICE
-chain must not be used for pvmfw. A non-UDS rooted DICE chain is recommended for
-Android 14.
-
-As an intermediate step towards supporting DICE throughout the software stack of
-the device, incomplete implementations may root the BCC at the pvmfw loader,
-using an arbitrary constant as initial CDI. The pvmfw loader can easily do so
-by:
+Otherwise, as an intermediate step towards supporting DICE throughout the
+software stack of the device, incomplete implementations may root the BCC at the
+pvmfw loader, using an arbitrary constant as initial CDI. The pvmfw loader can
+easily do so by:
1. Building a BCC-less `BccHandover` using CBOR operations
([example][Trusty-BCC]) and containing the constant CDIs
diff --git a/pvmfw/src/bcc.rs b/pvmfw/src/bcc.rs
new file mode 100644
index 0000000..f56e62b
--- /dev/null
+++ b/pvmfw/src/bcc.rs
@@ -0,0 +1,235 @@
+// 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.
+
+//! Code to inspect/manipulate the BCC (DICE Chain) we receive from our loader (the hypervisor).
+
+// TODO(b/279910232): Unify this, somehow, with the similar but different code in hwtrust.
+
+use alloc::vec;
+use alloc::vec::Vec;
+use ciborium::value::Value;
+use core::fmt;
+use core::mem::size_of;
+use diced_open_dice::{BccHandover, Cdi, DiceArtifacts, DiceMode};
+use log::trace;
+
+type Result<T> = core::result::Result<T, BccError>;
+
+pub enum BccError {
+ CborDecodeError(ciborium::de::Error<ciborium_io::EndOfFile>),
+ CborEncodeError(ciborium::ser::Error<core::convert::Infallible>),
+ DiceError(diced_open_dice::DiceError),
+ ExtraneousBytes,
+ MalformedBcc(&'static str),
+ MissingBcc,
+}
+
+impl fmt::Display for BccError {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ match self {
+ Self::CborDecodeError(e) => write!(f, "Error parsing BCC CBOR: {e:?}"),
+ Self::CborEncodeError(e) => write!(f, "Error encoding BCC CBOR: {e:?}"),
+ Self::DiceError(e) => write!(f, "Dice error: {e:?}"),
+ Self::ExtraneousBytes => write!(f, "Unexpected trailing data in BCC"),
+ Self::MalformedBcc(s) => {
+ write!(f, "BCC does not have the expected CBOR structure: {s}")
+ }
+ Self::MissingBcc => write!(f, "Missing BCC"),
+ }
+ }
+}
+
+/// Return a new CBOR encoded BccHandover that is based on the incoming CDIs but does not chain
+/// from the received BCC.
+pub fn truncate(bcc_handover: BccHandover) -> Result<Vec<u8>> {
+ // Note: The strings here are deliberately different from those used in a normal DICE handover
+ // because we want this to not be equivalent to any valid DICE derivation.
+ let cdi_seal = taint_cdi(bcc_handover.cdi_seal(), "TaintCdiSeal")?;
+ let cdi_attest = taint_cdi(bcc_handover.cdi_attest(), "TaintCdiAttest")?;
+
+ // BccHandover = {
+ // 1 : bstr .size 32, ; CDI_Attest
+ // 2 : bstr .size 32, ; CDI_Seal
+ // ? 3 : Bcc, ; Certificate chain
+ // }
+ let bcc_handover: Vec<(Value, Value)> =
+ vec![(1.into(), cdi_attest.as_slice().into()), (2.into(), cdi_seal.as_slice().into())];
+ value_to_bytes(&bcc_handover.into())
+}
+
+fn taint_cdi(cdi: &Cdi, info: &str) -> Result<Cdi> {
+ // An arbitrary value generated randomly.
+ const SALT: [u8; 64] = [
+ 0xdc, 0x0d, 0xe7, 0x40, 0x47, 0x9d, 0x71, 0xb8, 0x69, 0xd0, 0x71, 0x85, 0x27, 0x47, 0xf5,
+ 0x65, 0x7f, 0x16, 0xfa, 0x59, 0x23, 0x19, 0x6a, 0x6b, 0x77, 0x41, 0x01, 0x45, 0x90, 0x3b,
+ 0xfa, 0x68, 0xad, 0xe5, 0x26, 0x31, 0x5b, 0x40, 0x85, 0x71, 0x97, 0x12, 0xbd, 0x0b, 0x38,
+ 0x5c, 0x98, 0xf3, 0x0e, 0xe1, 0x7c, 0x82, 0x23, 0xa4, 0x38, 0x38, 0x85, 0x84, 0x85, 0x0d,
+ 0x02, 0x90, 0x60, 0xd3,
+ ];
+ let mut result = [0u8; size_of::<Cdi>()];
+ diced_open_dice::kdf(cdi.as_slice(), &SALT, info.as_bytes(), result.as_mut_slice())
+ .map_err(BccError::DiceError)?;
+ Ok(result)
+}
+
+/// Represents a (partially) decoded BCC DICE chain.
+pub struct Bcc {
+ is_debug_mode: bool,
+}
+
+impl Bcc {
+ /// Returns whether any node in the received DICE chain is marked as debug (and hence is not
+ /// secure).
+ pub fn new(received_bcc: Option<&[u8]>) -> Result<Bcc> {
+ let received_bcc = received_bcc.unwrap_or(&[]);
+ if received_bcc.is_empty() {
+ return Err(BccError::MissingBcc);
+ }
+
+ // We don't attempt to fully validate the BCC (e.g. we don't check the signatures) - we
+ // have to trust our loader. But if it's invalid CBOR or otherwise clearly ill-formed,
+ // something is very wrong, so we fail.
+ let bcc_cbor = value_from_bytes(received_bcc)?;
+
+ // Bcc = [
+ // PubKeyEd25519 / PubKeyECDSA256, // DK_pub
+ // + BccEntry, // Root -> leaf (KM_pub)
+ // ]
+ let bcc = match bcc_cbor {
+ Value::Array(v) if v.len() >= 2 => v,
+ _ => return Err(BccError::MalformedBcc("Invalid top level value")),
+ };
+ // Decode all the entries to make sure they are well-formed.
+ let entries: Vec<_> = bcc.into_iter().skip(1).map(BccEntry::new).collect();
+
+ let is_debug_mode = is_any_entry_debug_mode(entries.as_slice())?;
+ Ok(Self { is_debug_mode })
+ }
+
+ pub fn is_debug_mode(&self) -> bool {
+ self.is_debug_mode
+ }
+}
+
+fn is_any_entry_debug_mode(entries: &[BccEntry]) -> Result<bool> {
+ // Check if any entry in the chain is marked as Debug mode, which means the device is not
+ // secure. (Normal means it is a secure boot, for that stage at least; we ignore recovery
+ // & not configured /invalid values, since it's not clear what they would mean in this
+ // context.)
+ for entry in entries {
+ if entry.payload()?.is_debug_mode()? {
+ return Ok(true);
+ }
+ }
+ Ok(false)
+}
+
+#[repr(transparent)]
+struct BccEntry(Value);
+
+#[repr(transparent)]
+struct BccPayload(Value);
+
+impl BccEntry {
+ pub fn new(entry: Value) -> Self {
+ Self(entry)
+ }
+
+ pub fn payload(&self) -> Result<BccPayload> {
+ // BccEntry = [ // COSE_Sign1 (untagged)
+ // protected : bstr .cbor {
+ // 1 : AlgorithmEdDSA / AlgorithmES256, // Algorithm
+ // },
+ // unprotected: {},
+ // payload: bstr .cbor BccPayload,
+ // signature: bstr // PureEd25519(SigningKey, bstr .cbor BccEntryInput) /
+ // // ECDSA(SigningKey, bstr .cbor BccEntryInput)
+ // // See RFC 8032 for details of how to encode the signature value for Ed25519.
+ // ]
+ let payload =
+ self.payload_bytes().ok_or(BccError::MalformedBcc("Invalid payload in BccEntry"))?;
+ let payload = value_from_bytes(payload)?;
+ trace!("Bcc payload: {payload:?}");
+ Ok(BccPayload(payload))
+ }
+
+ fn payload_bytes(&self) -> Option<&Vec<u8>> {
+ let entry = self.0.as_array()?;
+ if entry.len() != 4 {
+ return None;
+ };
+ entry[2].as_bytes()
+ }
+}
+
+const KEY_MODE: i32 = -4670551;
+const MODE_DEBUG: u8 = DiceMode::kDiceModeDebug as u8;
+
+impl BccPayload {
+ pub fn is_debug_mode(&self) -> Result<bool> {
+ // BccPayload = { // CWT
+ // ...
+ // ? -4670551 : bstr, // Mode
+ // ...
+ // }
+
+ let Some(value) = self.value_from_key(KEY_MODE) else { return Ok(false) };
+
+ // Mode is supposed to be encoded as a 1-byte bstr, but some implementations instead
+ // encode it as an integer. Accept either. See b/273552826.
+ // If Mode is omitted, it should be treated as if it was Unknown, according to the Open
+ // Profile for DICE spec.
+ let mode = if let Some(bytes) = value.as_bytes() {
+ if bytes.len() != 1 {
+ return Err(BccError::MalformedBcc("Invalid mode bstr"));
+ }
+ bytes[0].into()
+ } else {
+ value.as_integer().ok_or(BccError::MalformedBcc("Invalid type for mode"))?
+ };
+ Ok(mode == MODE_DEBUG.into())
+ }
+
+ fn value_from_key(&self, key: i32) -> Option<&Value> {
+ // BccPayload is just a map; we only use integral keys, but in general it's legitimate
+ // for other things to be present, or for the key we care about not to be present.
+ // Ciborium represents the map as a Vec, preserving order (and allowing duplicate keys,
+ // which we ignore) but preventing fast lookup.
+ let payload = self.0.as_map()?;
+ for (k, v) in payload {
+ if k.as_integer() == Some(key.into()) {
+ return Some(v);
+ }
+ }
+ None
+ }
+}
+
+/// Decodes the provided binary CBOR-encoded value and returns a
+/// ciborium::Value struct wrapped in Result.
+fn value_from_bytes(mut bytes: &[u8]) -> Result<Value> {
+ let value = ciborium::de::from_reader(&mut bytes).map_err(BccError::CborDecodeError)?;
+ // Ciborium tries to read one Value, but doesn't care if there is trailing data after it. We do.
+ if !bytes.is_empty() {
+ return Err(BccError::ExtraneousBytes);
+ }
+ Ok(value)
+}
+
+/// Encodes a ciborium::Value into bytes.
+fn value_to_bytes(value: &Value) -> Result<Vec<u8>> {
+ let mut bytes: Vec<u8> = Vec::new();
+ ciborium::ser::into_writer(&value, &mut bytes).map_err(BccError::CborEncodeError)?;
+ Ok(bytes)
+}
diff --git a/pvmfw/src/dice.rs b/pvmfw/src/dice.rs
index bad3453..4e303ac 100644
--- a/pvmfw/src/dice.rs
+++ b/pvmfw/src/dice.rs
@@ -22,7 +22,8 @@
use core::slice;
use diced_open_dice::{
- bcc_format_config_descriptor, hash, Config, DiceMode, Hash, InputValues, HIDDEN_SIZE,
+ bcc_format_config_descriptor, bcc_handover_main_flow, hash, Config, DiceMode, Hash,
+ InputValues, HIDDEN_SIZE,
};
use pvmfw_avb::{DebugLevel, Digest, VerifiedBootData};
@@ -57,10 +58,12 @@
Ok(Self { code_hash, auth_hash, mode })
}
- pub fn into_input_values(
+ pub fn write_next_bcc(
self,
+ current_bcc_handover: &[u8],
salt: &[u8; HIDDEN_SIZE],
- ) -> diced_open_dice::Result<InputValues> {
+ next_bcc: &mut [u8],
+ ) -> diced_open_dice::Result<()> {
let mut config_descriptor_buffer = [0; 128];
let config_descriptor_size = bcc_format_config_descriptor(
Some(cstr!("vm_entry")),
@@ -70,13 +73,15 @@
)?;
let config = &config_descriptor_buffer[..config_descriptor_size];
- Ok(InputValues::new(
+ let dice_inputs = InputValues::new(
self.code_hash,
Config::Descriptor(config),
self.auth_hash,
self.mode,
*salt,
- ))
+ );
+ let _ = bcc_handover_main_flow(current_bcc_handover, &dice_inputs, next_bcc)?;
+ Ok(())
}
}
diff --git a/pvmfw/src/main.rs b/pvmfw/src/main.rs
index 96b707e..3c0acc7 100644
--- a/pvmfw/src/main.rs
+++ b/pvmfw/src/main.rs
@@ -19,6 +19,7 @@
extern crate alloc;
+mod bcc;
mod bootargs;
mod config;
mod crypto;
@@ -36,10 +37,7 @@
mod rand;
mod virtio;
-use alloc::boxed::Box;
-use alloc::string::ToString;
-use core::ops::Range;
-
+use crate::bcc::Bcc;
use crate::dice::PartialInputs;
use crate::entry::RebootReason;
use crate::fdt::modify_for_next_stage;
@@ -48,38 +46,24 @@
use crate::instance::get_or_generate_instance_salt;
use crate::memory::MemoryTracker;
use crate::virtio::pci;
-use ciborium::{de::from_reader, value::Value};
-use diced_open_dice::bcc_handover_main_flow;
-use diced_open_dice::bcc_handover_parse;
-use diced_open_dice::DiceArtifacts;
+use alloc::boxed::Box;
+use core::ops::Range;
+use diced_open_dice::{bcc_handover_parse, DiceArtifacts};
use fdtpci::{PciError, PciInfo};
use libfdt::Fdt;
-use log::{debug, error, info, trace};
+use log::{debug, error, info, trace, warn};
use pvmfw_avb::verify_payload;
use pvmfw_avb::DebugLevel;
use pvmfw_embedded_key::PUBLIC_KEY;
const NEXT_BCC_SIZE: usize = GUEST_PAGE_SIZE;
-type CiboriumError = ciborium::de::Error<ciborium_io::EndOfFile>;
-
-/// Decodes the provided binary CBOR-encoded value and returns a
-/// ciborium::Value struct wrapped in Result.
-fn value_from_bytes(mut bytes: &[u8]) -> Result<Value, CiboriumError> {
- let value = from_reader(&mut bytes)?;
- // Ciborium tries to read one Value, but doesn't care if there is trailing data. We do.
- if !bytes.is_empty() {
- return Err(CiboriumError::Semantic(Some(0), "unexpected trailing data".to_string()));
- }
- Ok(value)
-}
-
fn main(
fdt: &mut Fdt,
signed_kernel: &[u8],
ramdisk: Option<&[u8]>,
current_bcc_handover: &[u8],
- debug_policy: Option<&mut [u8]>,
+ mut debug_policy: Option<&mut [u8]>,
memory: &mut MemoryTracker,
) -> Result<Range<usize>, RebootReason> {
info!("pVM firmware");
@@ -91,22 +75,25 @@
} else {
debug!("Ramdisk: None");
}
+
let bcc_handover = bcc_handover_parse(current_bcc_handover).map_err(|e| {
error!("Invalid BCC Handover: {e:?}");
RebootReason::InvalidBcc
})?;
trace!("BCC: {bcc_handover:x?}");
- // Minimal BCC verification - check the BCC exists & is valid CBOR.
- // TODO(alanstokes): Do something more useful.
- if let Some(bytes) = bcc_handover.bcc() {
- let _ = value_from_bytes(bytes).map_err(|e| {
- error!("Invalid BCC: {e:?}");
- RebootReason::InvalidBcc
- })?;
- } else {
- error!("Missing BCC");
- return Err(RebootReason::InvalidBcc);
+ let cdi_seal = bcc_handover.cdi_seal();
+
+ let bcc = Bcc::new(bcc_handover.bcc()).map_err(|e| {
+ error!("{e}");
+ RebootReason::InvalidBcc
+ })?;
+
+ // The bootloader should never pass us a debug policy when the boot is secure (the bootloader
+ // is locked). If it gets it wrong, disregard it & log it, to avoid it causing problems.
+ if debug_policy.is_some() && !bcc.is_debug_mode() {
+ warn!("Ignoring debug policy, BCC does not indicate Debug mode");
+ debug_policy = None;
}
// Set up PCI bus for VirtIO devices.
@@ -130,7 +117,6 @@
error!("Failed to compute partial DICE inputs: {e:?}");
RebootReason::InternalError
})?;
- let cdi_seal = DiceArtifacts::cdi_seal(&bcc_handover);
let (new_instance, salt) = get_or_generate_instance_salt(&mut pci_root, &dice_inputs, cdi_seal)
.map_err(|e| {
error!("Failed to get instance.img salt: {e}");
@@ -138,14 +124,22 @@
})?;
trace!("Got salt from instance.img: {salt:x?}");
- let dice_inputs = dice_inputs.into_input_values(&salt).map_err(|e| {
- error!("Failed to generate DICE inputs: {e:?}");
+ // It is possible that the DICE chain we were given is rooted in the UDS. We do not want to give
+ // such a chain to the payload, or even the associated CDIs. So remove the entire chain we
+ // were given and taint the CDIs. Note that the resulting CDIs are still deterministically
+ // derived from those we received, so will vary iff they do.
+ // TODO(b/280405545): Remove this post Android 14.
+ let truncated_bcc_handover = bcc::truncate(bcc_handover).map_err(|e| {
+ error!("{e}");
RebootReason::InternalError
})?;
- let _ = bcc_handover_main_flow(current_bcc_handover, &dice_inputs, next_bcc).map_err(|e| {
- error!("Failed to derive next-stage DICE secrets: {e:?}");
- RebootReason::SecretDerivationError
- })?;
+
+ dice_inputs.write_next_bcc(truncated_bcc_handover.as_slice(), &salt, next_bcc).map_err(
+ |e| {
+ error!("Failed to derive next-stage DICE secrets: {e:?}");
+ RebootReason::SecretDerivationError
+ },
+ )?;
flush(next_bcc);
let strict_boot = true;
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 bee424d..0ddafeb 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -24,16 +24,16 @@
import static android.system.virtualmachine.VirtualMachineConfig.DEBUG_LEVEL_NONE;
import static android.system.virtualmachine.VirtualMachineManager.CAPABILITY_NON_PROTECTED_VM;
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 java.nio.file.StandardCopyOption.REPLACE_EXISTING;
-import static org.junit.Assume.assumeTrue;
+
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
-import com.google.common.base.Strings;
-import com.google.common.truth.BooleanSubject;
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
import android.app.Instrumentation;
import android.app.UiAutomation;
@@ -67,6 +67,9 @@
import com.android.microdroid.testservice.ITestService;
import com.android.microdroid.testservice.IVmCallback;
+import com.google.common.base.Strings;
+import com.google.common.truth.BooleanSubject;
+
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
@@ -1084,9 +1087,9 @@
List<DataItem> rootArrayItems = ((Array) dataItems.get(0)).getDataItems();
assertThat(rootArrayItems.size()).isAtLeast(2); // Public key and one certificate
if (mProtectedVm) {
- // When a true DICE chain is created, microdroid expects entries for: u-boot,
- // u-boot-env, microdroid, app payload and the service process.
- assertThat(rootArrayItems.size()).isAtLeast(5);
+ // pvmfw truncates the DICE chain it gets, so we expect exactly entries for: public key,
+ // Microdroid and app payload.
+ assertThat(rootArrayItems.size()).isEqualTo(3);
}
}