Add BCC truncating
"Truncate" the received BCC by removing the entire chain we receive
and peforming a non-DICE derivation on the CDIs. This is to ensure
that we don't provide access to a UDS-rooted BCC, since that might be
what we received. This needs to be removed once we have a reliable way
to distinguish a VM BCC from a non-VM one.
Fixed a test whose assumption is no longer true.
Bug: 266172411
Test: atest ComposHostTestCases (this validates the CompOS BCC)
Test: atest MicrodroidTests
Change-Id: I288f4ed8e108c81ab46f8ce2c94a9336855422c8
diff --git a/pvmfw/src/bcc.rs b/pvmfw/src/bcc.rs
index c58ead1..f56e62b 100644
--- a/pvmfw/src/bcc.rs
+++ b/pvmfw/src/bcc.rs
@@ -16,16 +16,20 @@
// 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 diced_open_dice::DiceMode;
+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,
@@ -35,6 +39,8 @@
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}")
@@ -44,6 +50,39 @@
}
}
+/// 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,
@@ -187,3 +226,10 @@
}
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/main.rs b/pvmfw/src/main.rs
index abe6a25..21521da 100644
--- a/pvmfw/src/main.rs
+++ b/pvmfw/src/main.rs
@@ -131,10 +131,21 @@
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
+ // 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(truncated_bcc_handover.as_slice(), &dice_inputs, 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 38bc485..8303791 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);
}
}