Add BCC checking
Check whether any stage in the received BCC is marked as debug. If
not, refuse to apply any debug policy we receive. (The bootloader
shouldn't pass one in this case, this is just to make sure we catch
any mistake here.)
In passing fix the lifetime of the config descriptor buffer
(b/280617929).
Bug: 275424867
Test: atest MicrodroidTests
Change-Id: I507fedee9e21e8cbda60044a4e0324e0d6530b00
diff --git a/pvmfw/src/bcc.rs b/pvmfw/src/bcc.rs
new file mode 100644
index 0000000..c58ead1
--- /dev/null
+++ b/pvmfw/src/bcc.rs
@@ -0,0 +1,189 @@
+// 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::Vec;
+use ciborium::value::Value;
+use core::fmt;
+use diced_open_dice::DiceMode;
+use log::trace;
+
+type Result<T> = core::result::Result<T, BccError>;
+
+pub enum BccError {
+ CborDecodeError(ciborium::de::Error<ciborium_io::EndOfFile>),
+ 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::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"),
+ }
+ }
+}
+
+/// 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)
+}
diff --git a/pvmfw/src/dice.rs b/pvmfw/src/dice.rs
index bad3453..e588acb 100644
--- a/pvmfw/src/dice.rs
+++ b/pvmfw/src/dice.rs
@@ -60,13 +60,13 @@
pub fn into_input_values(
self,
salt: &[u8; HIDDEN_SIZE],
+ config_descriptor_buffer: &mut [u8],
) -> diced_open_dice::Result<InputValues> {
- let mut config_descriptor_buffer = [0; 128];
let config_descriptor_size = bcc_format_config_descriptor(
Some(cstr!("vm_entry")),
None, // component_version
false, // resettable
- &mut config_descriptor_buffer,
+ config_descriptor_buffer,
)?;
let config = &config_descriptor_buffer[..config_descriptor_size];
diff --git a/pvmfw/src/main.rs b/pvmfw/src/main.rs
index 96b707e..abe6a25 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_main_flow, 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,10 +124,13 @@
})?;
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:?}");
- RebootReason::InternalError
- })?;
+ let mut config_descriptor_buffer = [0; 128];
+ let dice_inputs =
+ dice_inputs.into_input_values(&salt, &mut config_descriptor_buffer).map_err(|e| {
+ error!("Failed to generate DICE inputs: {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