More lenient config descriptor parsing am: 8f7db050c0 am: d6bafbe61a
Original change: https://android-review.googlesource.com/c/platform/packages/modules/Virtualization/+/3081483
Change-Id: I8bf6fb2504070f76a54b9886fb56a6e100cf5e49
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
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 })
}