fix(attest): accept one vm_config, not competing copies

Signed-off-by: Kevin Wang <wy721@qq.com>

Author: kvinwang

dstack/dstack-attest/src/attestation.rs

@@ -419,23 +419,24 @@ fn find_event_payloads(runtime_events: &[RuntimeEvent], name: &str) -> Vec<Vec<u
         .collect()
 }
 
-fn decode_vm_config_with_fallback(config: &str, fallback_config: &str) -> Result<VmConfig> {
-    let config = if config.is_empty() {
-        fallback_config
-    } else {
-        config
-    };
-    let config = if config.is_empty() { "{}" } else { config };
-    let config = vm_config_json_from_config(config).unwrap_or(Cow::Borrowed(config));
-    serde_json::from_str(&config).context("Failed to parse vm config")
+/// The VM config to decode: the caller-supplied copy or the one carried in the
+/// attestation. When both are given they must be identical, so no reader can
+/// authorize on one copy and decode another.
+pub fn resolve_vm_config<'a>(external: &'a str, embedded: &'a str) -> Result<&'a str> {
+    if external.is_empty() {
+        return Ok(embedded);
+    }
+    if !embedded.is_empty() && external != embedded {
+        bail!("vm_config does not match the config carried in the attestation");
+    }
+    Ok(external)
 }
 
-fn vm_config_json_from_config(config: &str) -> Option<Cow<'_, str>> {
-    let value = serde_json::from_str::<serde_json::Value>(config).ok()?;
-    value
-        .get("vm_config")
-        .and_then(|value| value.as_str())
-        .map(|vm_config| Cow::Owned(vm_config.to_string()))
+fn decode_vm_config_with_fallback(config: &str, fallback_config: &str) -> Result<VmConfig> {
+    let config = resolve_vm_config(config, fallback_config)?;
+    // No vm config for nitro enclave
+    let config = if config.is_empty() { "{}" } else { config };
+    serde_json::from_str(config).context("Failed to parse vm config")
 }
 
 fn mr_config_document_from_value(value: &serde_json::Value) -> Result<Option<String>> {
@@ -453,16 +454,7 @@ fn mr_config_document_from_config(config: &str) -> Result<Option<String>> {
     let Ok(value) = serde_json::from_str::<serde_json::Value>(config) else {
         return Ok(None);
     };
-    if let Some(mr_config) = mr_config_document_from_value(&value)? {
-        return Ok(Some(mr_config));
-    }
-
-    let Some(vm_config) = value.get("vm_config").and_then(|value| value.as_str()) else {
-        return Ok(None);
-    };
-    let vm_config = serde_json::from_str::<serde_json::Value>(vm_config)
-        .context("Failed to parse nested vm_config for amd sev-snp mr_config")?;
-    mr_config_document_from_value(&vm_config)
+    mr_config_document_from_value(&value)
 }
 
 pub use dstack_types::TeeVariant;
@@ -1685,20 +1677,18 @@ fn decode_app_info_sev_snp(
     external_vm_config: &str,
 ) -> Result<AppInfo> {
     let parsed = crate::amd_sev_snp::parse_unverified_amd_snp_report(report)?;
+    let config = resolve_vm_config(external_vm_config, embedded_config)?;
     let mr_config_document = if let Some(mr_config) = mr_config {
         Cow::Borrowed(mr_config)
-    } else if let Some(mr_config) = mr_config_document_from_config(external_vm_config)? {
-        Cow::Owned(mr_config)
-    } else if let Some(mr_config) = mr_config_document_from_config(embedded_config)? {
+    } else if let Some(mr_config) = mr_config_document_from_config(config)? {
         Cow::Owned(mr_config)
     } else {
         bail!("amd sev-snp mr_config is missing");
     };
     let mr_config = verify_snp_mr_config_host_data(mr_config_document.as_ref(), &parsed.host_data)?;
 
     let key_provider_info = key_provider_info_from_mr_config(&mr_config)?;
-    let os_image_hash =
-        decode_vm_config_with_fallback(external_vm_config, embedded_config)?.os_image_hash;
+    let os_image_hash = decode_vm_config_with_fallback(config, "")?.os_image_hash;
     let mrs = decode_mr_sev_snp(&parsed.measurement, &parsed.host_data);
 
     Ok(AppInfo {
@@ -1983,17 +1973,8 @@ impl<T: GetDeviceId> Attestation<T> {
     }
 
     /// Decode the VM config from the external or embedded config
-    pub fn decode_vm_config<'a>(&'a self, mut config: &'a str) -> Result<VmConfig> {
-        if config.is_empty() {
-            config = &self.config;
-        }
-        if config.is_empty() {
-            // No vm config for nitro enclave
-            config = "{}";
-        }
-        let vm_config: VmConfig =
-            serde_json::from_str(config).context("Failed to parse vm config")?;
-        Ok(vm_config)
+    pub fn decode_vm_config(&self, config: &str) -> Result<VmConfig> {
+        decode_vm_config_with_fallback(config, &self.config)
     }
 
     /// Decode the app info from the platform-specific app info source.

dstack/dstack-mr/src/sev.rs

@@ -1077,38 +1077,17 @@ pub struct SnpLaunchInputs {
 /// `mr_config` document out of a VMM `vm_config` string.
 ///
 /// The fields are intentionally explicit so missing SNP launch inputs fail
-/// closed instead of falling back to TDX event-log decoding. Both the top-level
-/// shape and the legacy nested `vm_config` string shape are accepted.
+/// closed instead of falling back to TDX event-log decoding.
 pub fn parse_snp_inputs_from_vm_config(vm_config: &str) -> Result<SnpLaunchInputs> {
-    let value: serde_json::Value =
+    let parsed: SevSnpMeasurementVmConfig =
         serde_json::from_str(vm_config).context("failed to parse vm_config for amd sev-snp")?;
-    let parsed: SevSnpMeasurementVmConfig = serde_json::from_value(value.clone())
-        .context("failed to parse vm_config for amd sev-snp")?;
-    let nested = value
-        .get("vm_config")
-        .and_then(|value| value.as_str())
-        .map(|vm_config| {
-            serde_json::from_str::<SevSnpMeasurementVmConfig>(vm_config)
-                .context("failed to parse nested vm_config for amd sev-snp")
-        })
-        .transpose()?;
     let measurement_document = parsed
         .sev_snp_measurement
-        .or_else(|| {
-            nested
-                .as_ref()
-                .and_then(|nested| nested.sev_snp_measurement.clone())
-        })
-        .ok_or_else(|| anyhow::anyhow!("sev_snp_measurement is required for amd sev-snp"))?;
-    let os_image_hash = if !parsed.os_image_hash.is_empty() {
-        parsed.os_image_hash
-    } else {
-        nested
-            .as_ref()
-            .map(|nested| nested.os_image_hash.clone())
-            .filter(|hash| !hash.is_empty())
-            .ok_or_else(|| anyhow::anyhow!("os_image_hash is required for amd sev-snp"))?
-    };
+        .context("sev_snp_measurement is required for amd sev-snp")?;
+    let os_image_hash = parsed.os_image_hash;
+    if os_image_hash.is_empty() {
+        bail!("os_image_hash is required for amd sev-snp");
+    }
     let document: SnpMeasurementDocument = serde_json::from_str(&measurement_document)
         .context("invalid amd sev-snp measurement document")?;
     dstack_types::SevOsImageMeasurementDocument::new(
@@ -1122,8 +1101,7 @@ pub fn parse_snp_inputs_from_vm_config(vm_config: &str) -> Result<SnpLaunchInput
     validate_measurement_input(&input)?;
     let mr_config_document = parsed
         .mr_config
-        .or_else(|| nested.and_then(|nested| nested.mr_config))
-        .ok_or_else(|| anyhow::anyhow!("mr_config is required for amd sev-snp"))?;
+        .context("mr_config is required for amd sev-snp")?;
     MrConfigV3::from_document(&mr_config_document)
         .context("invalid amd sev-snp mr_config document")?;
     Ok(SnpLaunchInputs {

dstack/kms/src/main_service.rs

@@ -24,7 +24,7 @@ use k256::ecdsa::SigningKey;
 use moka::future::Cache;
 use ra_rpc::{CallContext, RpcCall};
 use ra_tls::{
-    attestation::{AttestationVerifier, TeeVariant, VerifiedAttestation},
+    attestation::{resolve_vm_config, AttestationVerifier, TeeVariant, VerifiedAttestation},
     cert::{CaCert, CertRequest, CertSigningRequestV1, CertSigningRequestV2, Csr},
     kdf,
 };
@@ -257,14 +257,9 @@ pub(crate) fn build_boot_info_for_attestation(
     vm_config_str: &str,
 ) -> Result<BootInfo> {
     let boot_info = if att.report.amd_snp_report().is_some() {
-        let vm_config_str = if vm_config_str.is_empty() {
-            att.config.as_str()
-        } else {
-            vm_config_str
-        };
         amd_attest::build_amd_snp_boot_info_from_verified_attestation_and_vm_config(
             att,
-            vm_config_str,
+            resolve_vm_config(vm_config_str, &att.config)?,
         )?
     } else {
         build_boot_info(att, use_boottime_mr, vm_config_str)?
@@ -1159,6 +1154,28 @@ mod tests {
         assert_eq!(boot_info.app_id, vec![0x11; 20]);
     }
 
+    #[test]
+    fn build_boot_info_for_attestation_rejects_a_vm_config_differing_from_the_attested_one() {
+        let input = valid_snp_measurement_input();
+        let measurement = compute_expected_measurement(&input).unwrap();
+        let mr_config = valid_snp_mr_config();
+        let vm_config = snp_vm_config(&input, &mr_config);
+        let attestation = verified_snp_attestation_with_config(
+            measurement,
+            [0xab; 64],
+            vm_config.clone(),
+            &mr_config,
+        );
+        let mut request: serde_json::Value = serde_json::from_str(&vm_config).unwrap();
+        request["cpu_count"] = 8.into();
+
+        let err = build_boot_info_for_attestation(&attestation, false, &request.to_string())
+            .expect_err("a vm_config differing from the attested one must be rejected");
+        assert!(format!("{err:#}").contains("does not match the config carried"));
+        build_boot_info_for_attestation(&attestation, false, &vm_config)
+            .expect("an identical copy is accepted");
+    }
+
     #[test]
     fn build_boot_info_for_attestation_accepts_self_contained_snp_input_without_config() {
         let input = valid_snp_measurement_input();

dstack/verifier/src/verification.rs

@@ -22,8 +22,8 @@ use dstack_mr::{tdx::TdxRtmr0AcpiHashes, TdxMeasurements};
 use dstack_types::{sha256sum, TdxAttestationVariant, VmConfig};
 use hex_literal::hex;
 use ra_tls::attestation::{
-    AppInfo, Attestation, AttestationQuote, AttestationVerifier, DstackVerifiedReport, NitroPcrs,
-    VerifiedAttestation, VersionedAttestation,
+    resolve_vm_config, AppInfo, Attestation, AttestationQuote, AttestationVerifier,
+    DstackVerifiedReport, NitroPcrs, VerifiedAttestation, VersionedAttestation,
 };
 use serde::{Deserialize, Serialize};
 use sha2::{Digest as _, Sha256};
@@ -764,14 +764,8 @@ impl CvmVerifier {
         attestation: &VerifiedAttestation,
         details: &mut VerificationDetails,
     ) -> Result<VmConfig> {
-        // The raw config string used for platform-specific binding: the explicit
-        // request `vm_config` when supplied, otherwise the one embedded in the
-        // attestation (mirroring `decode_vm_config`'s own fallback).
-        let raw_config = if vm_config.is_empty() {
-            attestation.config.clone()
-        } else {
-            vm_config.clone()
-        };
+        // The raw config string used for platform-specific binding.
+        let raw_config = resolve_vm_config(&vm_config, &attestation.config)?.to_string();
         let mut vm_config = attestation
             .decode_vm_config(&vm_config)
             .context("Failed to decode VM config")?;