More lenient config descriptor parsing
Some DICE nodes simply have a hash in the config descriptor field; so
if parsing as the expected CBOR fails, we should ignore that.
Some nodes also have no name field, so we need to cope with that too.
And the SubComponents key in the config descriptor map is not
reserved, so it may appear in any DICE node with arbitrary content. So
we defer parsing the subcomponents until we've figured out which node
describes the Microdroid payload.
Bug: 338745127
Test: atest VmAttestationTestApp
Change-Id: I70169fc77170e2286d93246b079d3036d544b776
diff --git a/service_vm/requests/src/client_vm.rs b/service_vm/requests/src/client_vm.rs
index 3b5d68a..00effd7 100644
--- a/service_vm/requests/src/client_vm.rs
+++ b/service_vm/requests/src/client_vm.rs
@@ -68,6 +68,7 @@
cose_sign.verify_signature(ATTESTATION_KEY_SIGNATURE_INDEX, aad, |signature, message| {
ecdsa_verify(&ec_public_key, signature, message)
})?;
+
let subject_public_key_info = PKey::try_from(ec_public_key)?.subject_public_key_info()?;
// Builds the TBSCertificate.
@@ -80,12 +81,9 @@
rand_bytes(&mut serial_number)?;
let subject = Name::encode_from_string("CN=Android Protected Virtual Machine Key")?;
let rkp_cert = Certificate::from_der(¶ms.remotely_provisioned_cert)?;
+ let vm_components = client_vm_dice_chain.microdroid_payload_components()?;
let vm_components =
- if let Some(components) = client_vm_dice_chain.microdroid_payload_components() {
- components.iter().map(cert::VmComponent::new).collect::<der::Result<Vec<_>>>()?
- } else {
- Vec::new()
- };
+ vm_components.iter().map(cert::VmComponent::new).collect::<der::Result<Vec<_>>>()?;
info!("The client VM DICE chain validation succeeded. Beginning to generate the certificate.");
let attestation_ext = cert::AttestationExtension::new(
diff --git a/service_vm/requests/src/dice.rs b/service_vm/requests/src/dice.rs
index ec05f66..f4da7ef 100644
--- a/service_vm/requests/src/dice.rs
+++ b/service_vm/requests/src/dice.rs
@@ -15,6 +15,7 @@
//! This module contains functions related to DICE.
use alloc::string::String;
+use alloc::vec;
use alloc::vec::Vec;
use bssl_avf::{ed25519_verify, Digester, EcKey};
use cbor_util::{
@@ -112,11 +113,10 @@
) -> Result<Self> {
let microdroid_payload_name =
&dice_entry_payloads[dice_entry_payloads.len() - 1].config_descriptor.component_name;
- if MICRODROID_PAYLOAD_COMPONENT_NAME != microdroid_payload_name {
+ if Some(MICRODROID_PAYLOAD_COMPONENT_NAME) != microdroid_payload_name.as_deref() {
error!(
"The last entry in the client VM DICE chain must describe the Microdroid \
- payload. Got '{}'",
- microdroid_payload_name
+ payload. Got '{microdroid_payload_name:?}'"
);
return Err(RequestProcessingError::InvalidDiceChain);
}
@@ -125,11 +125,10 @@
let index = dice_entry_payloads.len() - 2;
let vendor_partition_name =
&dice_entry_payloads[index].config_descriptor.component_name;
- if VENDOR_PARTITION_COMPONENT_NAME != vendor_partition_name {
+ if Some(VENDOR_PARTITION_COMPONENT_NAME) != vendor_partition_name.as_deref() {
error!(
"The vendor partition entry in the client VM DICE chain must describe the \
- vendor partition. Got '{}'",
- vendor_partition_name,
+ vendor partition. Got '{vendor_partition_name:?}'"
);
return Err(RequestProcessingError::InvalidDiceChain);
}
@@ -139,11 +138,10 @@
};
let kernel_name = &dice_entry_payloads[kernel_index].config_descriptor.component_name;
- if KERNEL_COMPONENT_NAME != kernel_name {
+ if Some(KERNEL_COMPONENT_NAME) != kernel_name.as_deref() {
error!(
"The microdroid kernel entry in the client VM DICE chain must describe the \
- Microdroid kernel. Got '{}'",
- kernel_name,
+ Microdroid kernel. Got '{kernel_name:?}'"
);
return Err(RequestProcessingError::InvalidDiceChain);
}
@@ -164,8 +162,8 @@
&self.payloads[self.payloads.len() - 1]
}
- pub(crate) fn microdroid_payload_components(&self) -> Option<&Vec<SubComponent>> {
- self.microdroid_payload().config_descriptor.sub_components.as_ref()
+ pub(crate) fn microdroid_payload_components(&self) -> Result<Vec<SubComponent>> {
+ self.microdroid_payload().config_descriptor.sub_components()
}
/// Returns true if all payloads in the DICE chain are in normal mode.
@@ -354,15 +352,21 @@
///
/// hardware/interfaces/security/rkp/aidl/android/hardware/security/keymint/
/// generateCertificateRequestV2.cddl
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Default)]
pub(crate) struct ConfigDescriptor {
- component_name: String,
- sub_components: Option<Vec<SubComponent>>,
+ component_name: Option<String>,
+ sub_components: Option<Value>,
}
impl ConfigDescriptor {
fn from_slice(data: &[u8]) -> Result<Self> {
- let value = Value::from_slice(data)?;
+ let value = Value::from_slice(data);
+ let Ok(value) = value else {
+ // Some DICE implementations store a hash in the config descriptor. So we just
+ // skip anything that doesn't parse correctly.
+ info!("Ignoring malformed config descriptor");
+ return Ok(Default::default());
+ };
let entries = value_to_map(value, "ConfigDescriptor")?;
let mut builder = ConfigDescriptorBuilder::default();
for (key, value) in entries.into_iter() {
@@ -373,24 +377,31 @@
builder.component_name(name)?;
}
CONFIG_DESC_SUB_COMPONENTS => {
- let sub_components = value_to_array(value, "ConfigDescriptor sub_components")?;
- let sub_components = sub_components
- .into_iter()
- .map(SubComponent::try_from)
- .collect::<Result<Vec<_>>>()?;
- builder.sub_components(sub_components)?
+ // If this is the Microdroid payload node then these are the subcomponents. But
+ // for any other node it could be anything - this isn't a reserved key. So defer
+ // decoding until we know which node is which.
+ builder.sub_components(value)?
}
_ => {}
}
}
builder.build()
}
+
+ /// Attempt to decode any Microdroid sub-components that were present in this config descriptor.
+ fn sub_components(&self) -> Result<Vec<SubComponent>> {
+ let Some(value) = &self.sub_components else {
+ return Ok(vec![]);
+ };
+ let sub_components = value_to_array(value.clone(), "ConfigDescriptor sub_components")?;
+ sub_components.into_iter().map(SubComponent::try_from).collect()
+ }
}
#[derive(Debug, Clone, Default)]
struct ConfigDescriptorBuilder {
component_name: OnceCell<String>,
- sub_components: OnceCell<Vec<SubComponent>>,
+ sub_components: OnceCell<Value>,
}
impl ConfigDescriptorBuilder {
@@ -398,13 +409,12 @@
set_once(&self.component_name, component_name, "ConfigDescriptor component_name")
}
- fn sub_components(&mut self, sub_components: Vec<SubComponent>) -> Result<()> {
+ fn sub_components(&mut self, sub_components: Value) -> Result<()> {
set_once(&self.sub_components, sub_components, "ConfigDescriptor sub_components")
}
fn build(mut self) -> Result<ConfigDescriptor> {
- let component_name =
- take_value(&mut self.component_name, "ConfigDescriptor component_name")?;
+ let component_name = self.component_name.take();
let sub_components = self.sub_components.take();
Ok(ConfigDescriptor { component_name, sub_components })
}