fix(verifier): a DebugSwap guest verifies, and four more ways host bytes reach a measurement unchecked

dstack/dstack-attest/src/attestation.rs

@@ -1657,7 +1657,7 @@ fn decode_app_info_sev_snp(
     embedded_config: &str,
     external_vm_config: &str,
 ) -> Result<AppInfo> {
-    let parsed = crate::amd_sev_snp::parse_amd_snp_report(report)?;
+    let parsed = crate::amd_sev_snp::parse_unverified_amd_snp_report(report)?;
     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)? {

dstack/dstack-attest/src/v1.rs

@@ -7,7 +7,6 @@ use cc_eventlog::{
     tdx::{self, TDX_ACPI_DATA_EVENT_PAYLOAD},
     RuntimeEvent, TdxEvent,
 };
-use dstack_types::mr_config::MrConfigV3;
 use serde::{Deserialize, Serialize};
 use tpm_types::TpmQuote;
 
@@ -155,11 +154,6 @@ impl PlatformEvidence {
         }
     }
 
-    pub fn sev_snp_mr_config(&self) -> Option<MrConfigV3> {
-        self.sev_snp_mr_config_document()
-            .and_then(|document| MrConfigV3::from_document(document).ok())
-    }
-
     pub fn tdx_event_log_mut(&mut self) -> Option<&mut Vec<TdxEvent>> {
         match self {
             Self::Tdx { event_log, .. } | Self::GcpTdx { event_log, .. } => Some(event_log),

dstack/dstack-mr/src/main.rs

@@ -134,11 +134,8 @@ fn inspect_measurement(kind: &str, path: &Path) -> Result<Value> {
             .map_err(anyhow::Error::msg),
         "gcp" => dstack_types::GcpOsImageMeasurement::cbor_json_value_from_slice(&cbor)
             .map_err(anyhow::Error::msg),
-        "aws" => {
-            let measurement = dstack_types::AwsOsImageMeasurement::from_cbor_slice(&cbor)
-                .map_err(anyhow::Error::msg)?;
-            serde_json::to_value(measurement).context("failed to convert AWS measurement to JSON")
-        }
+        "aws" => dstack_types::AwsOsImageMeasurement::cbor_json_value_from_slice(&cbor)
+            .map_err(anyhow::Error::msg),
         other => bail!("unknown measurement kind {other:?}; expected tdx, snp, gcp, or aws"),
     }
 }

dstack/dstack-mr/src/sev.rs

@@ -34,6 +34,37 @@ pub const MAX_OVMF_METADATA_PAGES: u64 = 16_777_216;
 // VMSA page GPA: (u64)(-1) page-aligned, bits >51 cleared.
 const VMSA_GPA: u64 = 0x0000_FFFF_FFFF_F000;
 
+/// `SNPActive`, bit 0 of `SEV_FEATURES` (AMD64 APM vol. 2, VMCB SEV_FEATURES;
+/// the same bit layout the SEV-SNP ABI's `GUEST_FEATURES` field in
+/// `SNP_LAUNCH_START` carries). Set on every SNP guest.
+pub const SEV_FEATURE_SNP_ACTIVE: u64 = 1 << 0;
+
+/// `SEV_FEATURES` bits a dstack launch may carry.
+///
+/// `guest_features` is the only launch parameter that is neither pinned by
+/// `SevOsImageMeasurement` nor otherwise constrained: it rides in the
+/// host-written `SnpMeasurementDocument` alongside `vcpus`/`vcpu_type`, outside
+/// the CBOR that `os_image_hash` commits to, and lands verbatim in both VMSA
+/// pages at offset 0x3B0. Because the expected launch digest is *recomputed*
+/// from the declared value, a host that really booted the guest with extra
+/// feature bits and declares them gets a matching digest and a key release —
+/// the bits themselves were never policy-checked. The dangerous direction is
+/// DebugSwap (bit 5), which swaps the guest's debug registers on VMEXIT and so
+/// exposes guest state to the hypervisor; clearing RestrictedInjection (bit 3),
+/// SecureTSC (bit 9) or VmsaRegProt (bit 14) likewise removes a guest-side
+/// protection the operator may believe is on.
+///
+/// dstack has exactly one launch path, and it sets no feature property:
+/// `sev-snp-guest,id=sev0,policy=0x30000,...` in `dstack/vmm/src/app/qemu.rs`,
+/// which leaves KVM to start the guest with `SNPActive` alone, and the VMM
+/// writes the matching `guest_features: 1` into the measurement document
+/// (`dstack/vmm/src/app.rs`). `1` is therefore the only value dstack has ever
+/// produced — the captured real-hardware vector in this module's tests carries
+/// it too — so it is the only value accepted. Widening this is a one-line
+/// change once a launch path actually sets another bit; accepting bits nothing
+/// emits would only ever admit a guest dstack did not configure.
+pub const ALLOWED_GUEST_FEATURES: u64 = SEV_FEATURE_SNP_ACTIVE;
+
 #[derive(Debug, Clone, PartialEq, Eq, serde::Deserialize, serde::Serialize)]
 #[serde(deny_unknown_fields)]
 pub struct OvmfSectionParam {
@@ -111,8 +142,16 @@ where
 
 /// Validate a `MeasurementInput` for shape/bounds before recomputation.
 pub fn validate_measurement_input(input: &MeasurementInput) -> Result<()> {
-    if input.guest_features == 0 {
-        bail!("guest_features must be non-zero");
+    if input.guest_features & SEV_FEATURE_SNP_ACTIVE == 0 {
+        bail!("guest_features must set SNPActive (bit 0)");
+    }
+    let unexpected_features = input.guest_features & !ALLOWED_GUEST_FEATURES;
+    if unexpected_features != 0 {
+        bail!(
+            "guest_features {:#x} carries unsupported SEV_FEATURES bits {unexpected_features:#x}; \
+             dstack launches amd sev-snp guests with SNPActive only ({ALLOWED_GUEST_FEATURES:#x})",
+            input.guest_features
+        );
     }
 
     rootfs_hash_from_cmdline(Some(&input.base_cmdline))?;
@@ -836,16 +875,33 @@ fn file_sha256(path: &Path) -> Result<Vec<u8>> {
     Ok(Sha256::digest(data).to_vec())
 }
 
+/// Read the rootfs identity the measured kernel command line commits to.
+///
+/// A duplicated `dstack.rootfs_hash=` is rejected rather than resolved. The
+/// Linux command line has no "first wins" rule -- the dstack initramfs that
+/// mounts the rootfs reads the last occurrence -- so returning the first, as
+/// this did, would have answered with an identity the guest did not use. Taking
+/// the last would agree with the initramfs but would still accept a measured
+/// command line asserting two different rootfs identities, which is not a shape
+/// any dstack image produces and not one a reader can disambiguate. Callers use
+/// this as a validation gate on a command line an untrusted host supplies, so
+/// it fails closed.
 pub fn rootfs_hash_from_cmdline(cmdline: Option<&str>) -> Result<String> {
-    let rootfs_hash = cmdline
-        .unwrap_or_default()
-        .split_whitespace()
-        .find_map(|param| param.strip_prefix("dstack.rootfs_hash="))
-        .map(ToString::to_string)
-        .context("dstack.rootfs_hash is required in amd sev-snp measured cmdline")?;
+    let mut rootfs_hash = None;
+    for param in cmdline.unwrap_or_default().split_whitespace() {
+        let Some(value) = param.strip_prefix("dstack.rootfs_hash=") else {
+            continue;
+        };
+        if rootfs_hash.is_some() {
+            bail!("dstack.rootfs_hash appears more than once in the measured cmdline");
+        }
+        rootfs_hash = Some(value);
+    }
+    let rootfs_hash =
+        rootfs_hash.context("dstack.rootfs_hash is required in amd sev-snp measured cmdline")?;
     Ok(hex::encode(decode_required_hex(
         "dstack.rootfs_hash",
-        &rootfs_hash,
+        rootfs_hash,
         32,
     )?))
 }
@@ -1527,6 +1583,103 @@ mod tests {
         assert_eq!(binding.mr_config.app_id, mr_config.app_id);
     }
 
+    /// A host that really boots the guest with DebugSwap and declares it is not
+    /// tampering: every field is internally consistent, the recomputed launch
+    /// digest matches the hardware `MEASUREMENT`, and `host_data` matches the
+    /// mr_config. The only thing standing between that guest and a key release
+    /// is a policy check on `guest_features` itself.
+    /// Bit positions from the AMD64 APM `SEV_FEATURES` table, which the SEV-SNP
+    /// ABI `GUEST_FEATURES` field in `SNP_LAUNCH_START` mirrors.
+    const SNP_ACTIVE: u64 = 1 << 0;
+    const DEBUG_SWAP: u64 = 1 << 5;
+
+    /// `find_map` took the first `dstack.rootfs_hash=`, while the initramfs
+    /// that actually mounts the rootfs honours the last one. The function is
+    /// `pub` and reads authoritative, so a caller that trusted it would have
+    /// been told a different rootfs identity than the guest used.
+    #[test]
+    fn a_duplicated_rootfs_hash_is_not_silently_resolved() {
+        let first = hex_of(0x11, 32);
+        let last = hex_of(0x22, 32);
+        let cmdline = format!(
+            "console=ttyS0 dstack.rootfs_hash={first} init=/init dstack.rootfs_hash={last}"
+        );
+        let err = match rootfs_hash_from_cmdline(Some(&cmdline)) {
+            Ok(hash) => panic!("a duplicated rootfs hash resolved to {hash}"),
+            Err(err) => err.to_string(),
+        };
+        assert!(
+            err.contains("dstack.rootfs_hash appears more than once"),
+            "unexpected error: {err}"
+        );
+
+        // A single occurrence is unchanged.
+        let cmdline = format!("console=ttyS0 dstack.rootfs_hash={first}");
+        assert_eq!(
+            rootfs_hash_from_cmdline(Some(&cmdline)).expect("single occurrence"),
+            first
+        );
+    }
+
+    #[test]
+    fn verify_sev_launch_rejects_a_consistent_debugswap_guest() {
+        let mut input = valid_input();
+        input.guest_features = SNP_ACTIVE | DEBUG_SWAP;
+        let mr_config = synthetic_mr_config();
+        let host_data = MrConfigV3::snp_host_data_from_document(&mr_config.to_canonical_json());
+        let measurement = compute_expected_measurement(&input).expect("measurement");
+        let vm_config = synthetic_vm_config(&input, &mr_config);
+
+        let err = match verify_sev_launch(&measurement, &host_data, &vm_config) {
+            Ok(binding) => panic!(
+                "a DebugSwap launch verified; bound os_image_hash {}",
+                hex::encode(binding.os_image_hash)
+            ),
+            Err(err) => err.to_string(),
+        };
+        assert!(
+            err.contains("unsupported SEV_FEATURES bits 0x20"),
+            "unexpected error: {err}"
+        );
+    }
+
+    #[test]
+    fn rejects_guest_features_outside_the_launch_allowlist() {
+        let cases: [(&str, u64); 4] = [
+            ("RestrictedInjection", SNP_ACTIVE | (1 << 3)),
+            ("DebugSwap", SNP_ACTIVE | (1 << 5)),
+            ("SecureTSC", SNP_ACTIVE | (1 << 9)),
+            ("reserved bit 63", SNP_ACTIVE | (1 << 63)),
+        ];
+        for (name, features) in cases {
+            let mut input = valid_input();
+            input.guest_features = features;
+            let err = validate_measurement_input(&input)
+                .expect_err("feature bit outside the allowlist must not be accepted")
+                .to_string();
+            assert!(
+                err.contains("unsupported SEV_FEATURES bits"),
+                "{name}: unexpected error: {err}"
+            );
+        }
+
+        // SNPActive missing is still rejected, now by name.
+        let mut input = valid_input();
+        input.guest_features = 0;
+        let err = validate_measurement_input(&input)
+            .expect_err("guest_features 0 must not be accepted")
+            .to_string();
+        assert!(
+            err.contains("must set SNPActive"),
+            "unexpected error: {err}"
+        );
+
+        // The one value dstack's launch path produces stays accepted.
+        let mut input = valid_input();
+        input.guest_features = SNP_ACTIVE;
+        validate_measurement_input(&input).expect("SNPActive-only launch is accepted");
+    }
+
     #[test]
     fn verify_sev_launch_rejects_forged_measurement() {
         let (_input, _mr, measurement, host_data, vm_config) = honest_case();
@@ -1579,7 +1732,12 @@ mod tests {
             ("vcpu_type", |i| {
                 i.vcpu_type = Some("epyc-milan".to_string())
             }),
-            ("guest_features", |i| i.guest_features = 3),
+            // guest_features is not in this list: the SEV_FEATURES allowlist
+            // now rejects every value but SNPActive before the measurement is
+            // ever recomputed, so there is no tampered value left to reach the
+            // measurement comparison. See
+            // `rejects_guest_features_outside_the_launch_allowlist` and
+            // `verify_sev_launch_rejects_a_consistent_debugswap_guest`.
         ];
         for (name, mutate) in cases {
             let mut tampered = input.clone();