fix: an SMBIOS string turns off the guest's own attestation checks, and a host-shared size limit that is only a stat

dstack/dstack-attest/src/attestation.rs

@@ -475,34 +475,208 @@ fn choose_dstack_tee_variant(has_tdx: bool, has_sev_snp: bool) -> Result<TeeVari
     bail!("Unsupported platform: Dstack(-tdx/-amd-sev-snp)");
 }
 
-/// Detect the attestation variant exposed by the current guest environment.
-pub fn detect_tee_variant() -> Result<TeeVariant> {
-    let has_tdx = tdx_attest::is_tdx_available();
-    let has_sev_snp = std::path::Path::new("/dev/sev-guest").exists() || has_sev_snp_tsm_provider();
+/// The TEE devices this guest actually exposes.
+///
+/// Every field is a device or driver ABI the platform presents to the guest.
+/// None of them is a DMI string, which is the point: see [`resolve_tee_variant`].
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+struct TeeDevices {
+    has_tdx: bool,
+    has_sev_snp: bool,
+    has_nsm: bool,
+    has_tpm: bool,
+}
+
+impl TeeDevices {
+    fn detect() -> Self {
+        Self {
+            has_tdx: tdx_attest::is_tdx_available(),
+            has_sev_snp: std::path::Path::new("/dev/sev-guest").exists()
+                || has_sev_snp_tsm_provider(),
+            has_nsm: std::path::Path::new("/dev/nsm").exists(),
+            has_tpm: std::path::Path::new("/dev/tpmrm0").exists()
+                || std::path::Path::new("/dev/tpm0").exists(),
+        }
+    }
+}
 
-    // First, try to detect platform from DMI product name
-    let platform = Platform::detect_or_dstack();
+/// Resolve the attestation variant from the platform hint and the devices the
+/// guest can actually see.
+///
+/// `Platform::detect` reads `/sys/class/dmi/id`, i.e. SMBIOS type 1. QEMU takes
+/// those strings from host-side configuration (`vmm/src/app/qemu.rs`
+/// `configure_smbios`) and no TDX or SEV-SNP measurement covers them, so they
+/// are an untrusted host claim, not evidence.
+///
+/// That matters because the variant decides whether two guest-side checks run
+/// at all. `DstackNitroEnclave` has no TDX RTMR lane and no TPM PCR lane
+/// (`TeeVariant::has_tdx`, `TeeVariant::tpm_event_pcr_and_bank`), so
+/// `emit_runtime_event` measures nothing and still returns `Ok`; and
+/// `verify_mr_config_id` returns `Ok` for it without reading MR_CONFIG_ID at
+/// all. A host that could name the variant could therefore turn both off on
+/// hardware that supports them.
+///
+/// So the hint may only select among variants the hardware corroborates: each
+/// arm below requires the device ABI its evidence comes from, and a hint that
+/// contradicts a confidential-computing device that *is* present is refused
+/// rather than believed.
+fn resolve_tee_variant(platform: Platform, devices: TeeDevices) -> Result<TeeVariant> {
     match platform {
-        Platform::Dstack => choose_dstack_tee_variant(has_tdx, has_sev_snp),
+        Platform::Dstack => choose_dstack_tee_variant(devices.has_tdx, devices.has_sev_snp),
         Platform::Gcp => {
             // GCP platform: TDX + TPM dual mode
-            if has_tdx {
+            if devices.has_tdx {
                 return Ok(TeeVariant::DstackGcpTdx);
             }
             bail!("Unsupported platform: GCP(-tdx)");
         }
-        Platform::NitroEnclave => Ok(TeeVariant::DstackNitroEnclave),
+        Platform::NitroEnclave => {
+            if devices.has_tdx || devices.has_sev_snp {
+                bail!(
+                    "refusing the Nitro Enclave platform claim: this guest exposes a {} device,                      and the Nitro Enclave variant measures no runtime event",
+                    if devices.has_tdx { "TDX" } else { "SEV-SNP" }
+                );
+            }
+            if !devices.has_nsm {
+                bail!("unsupported platform: Nitro Enclave without /dev/nsm");
+            }
+            Ok(TeeVariant::DstackNitroEnclave)
+        }
         Platform::AwsEc2 => {
-            if std::path::Path::new("/dev/tpmrm0").exists()
-                || std::path::Path::new("/dev/tpm0").exists()
-            {
+            if devices.has_tpm {
                 return Ok(TeeVariant::DstackAwsNitroTpm);
             }
             bail!("unsupported platform: AWS EC2 without NitroTPM");
         }
     }
 }
 
+/// Detect the attestation variant exposed by the current guest environment.
+pub fn detect_tee_variant() -> Result<TeeVariant> {
+    // The DMI platform is only a hint; `resolve_tee_variant` says why.
+    resolve_tee_variant(Platform::detect_or_dstack(), TeeDevices::detect())
+}
+
+#[cfg(test)]
+mod tee_variant_resolution_tests {
+    use super::*;
+
+    const NOTHING: TeeDevices = TeeDevices {
+        has_tdx: false,
+        has_sev_snp: false,
+        has_nsm: false,
+        has_tpm: false,
+    };
+
+    #[test]
+    fn a_nitro_enclave_claim_needs_the_nsm_device() {
+        let err = resolve_tee_variant(Platform::NitroEnclave, NOTHING)
+            .expect_err("a Nitro Enclave claim with no NSM device must be refused");
+        assert!(err.to_string().contains("/dev/nsm"), "{err}");
+
+        assert_eq!(
+            resolve_tee_variant(
+                Platform::NitroEnclave,
+                TeeDevices {
+                    has_nsm: true,
+                    ..NOTHING
+                }
+            )
+            .unwrap(),
+            TeeVariant::DstackNitroEnclave
+        );
+    }
+
+    #[test]
+    fn a_nitro_enclave_claim_does_not_downgrade_tdx_or_sev_snp_hardware() {
+        // The variant this claim selects measures no runtime event and skips
+        // verify_mr_config_id, so believing it on TDX or SEV-SNP hardware would
+        // let the host turn both off with an SMBIOS string.
+        for devices in [
+            TeeDevices {
+                has_tdx: true,
+                ..NOTHING
+            },
+            TeeDevices {
+                has_sev_snp: true,
+                ..NOTHING
+            },
+            TeeDevices {
+                has_tdx: true,
+                has_nsm: true,
+                ..NOTHING
+            },
+        ] {
+            let err = resolve_tee_variant(Platform::NitroEnclave, devices)
+                .expect_err("a Nitro Enclave claim must not override present TEE hardware");
+            assert!(err.to_string().contains("refusing"), "{err}");
+        }
+    }
+
+    #[test]
+    fn hardware_backed_platforms_still_resolve() {
+        let tdx = TeeDevices {
+            has_tdx: true,
+            ..NOTHING
+        };
+        assert_eq!(
+            resolve_tee_variant(Platform::Dstack, tdx).unwrap(),
+            TeeVariant::DstackTdx
+        );
+        assert_eq!(
+            resolve_tee_variant(Platform::Gcp, tdx).unwrap(),
+            TeeVariant::DstackGcpTdx
+        );
+        assert_eq!(
+            resolve_tee_variant(
+                Platform::Dstack,
+                TeeDevices {
+                    has_sev_snp: true,
+                    ..NOTHING
+                }
+            )
+            .unwrap(),
+            TeeVariant::DstackAmdSevSnp
+        );
+        assert_eq!(
+            resolve_tee_variant(
+                Platform::AwsEc2,
+                TeeDevices {
+                    has_tpm: true,
+                    ..NOTHING
+                }
+            )
+            .unwrap(),
+            TeeVariant::DstackAwsNitroTpm
+        );
+        assert!(resolve_tee_variant(Platform::Gcp, NOTHING).is_err());
+        assert!(resolve_tee_variant(Platform::AwsEc2, NOTHING).is_err());
+        assert!(resolve_tee_variant(Platform::Dstack, NOTHING).is_err());
+    }
+
+    /// Reproduces the host-supplied-DMI path end to end.
+    ///
+    /// Passes trivially on a normal machine. To see what it guards, run it with
+    /// the SMBIOS product name a host controls bound over the real one:
+    ///
+    /// ```text
+    /// printf 'Nitro Enclave\n' > /tmp/fake_product_name
+    /// unshare -Urm sh -c 'mount --bind /tmp/fake_product_name \
+    ///     /sys/class/dmi/id/product_name && cargo test -p dstack-attest --lib \
+    ///     a_dmi_string_alone_cannot_select_the_nitro_enclave_variant'
+    /// ```
+    #[test]
+    fn a_dmi_string_alone_cannot_select_the_nitro_enclave_variant() {
+        if std::path::Path::new("/dev/nsm").exists() {
+            return;
+        }
+        assert!(
+            !matches!(detect_tee_variant(), Ok(TeeVariant::DstackNitroEnclave)),
+            "DMI alone selected DstackNitroEnclave with no NSM device present"
+        );
+    }
+}
+
 /// The content type of a quote. A CVM should only generate quotes for these types.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum QuoteContentType<'a> {

dstack/dstack-util/src/system_setup.rs

@@ -247,6 +247,51 @@ struct HostShared {
     instance_info: InstanceInfo,
 }
 
+/// Copy one host-supplied file into the guest's own copy of `.host-shared`.
+///
+/// The source lives on a mount the host backs and can change under us: 9p is
+/// the default `cvm.host_share_mode`, so the host serves every read live and
+/// decides what kind of file it is serving. Three properties follow, and each
+/// one is a guard here rather than a check on the path:
+///
+/// - `O_NOFOLLOW | O_NONBLOCK` and a regular-file check on the *open
+///   descriptor*. `Path::metadata` would describe whatever the path named a
+///   moment ago; the descriptor is what we then read. `O_NONBLOCK` is what
+///   makes the check reachable at all: opening a FIFO without it blocks until
+///   a writer appears, which is a boot that never finishes rather than one
+///   that fails.
+/// - the size limit is applied to the *copy*, not to a prior `stat`. A
+///   stat-then-copy pair is advisory against a live backing store, and against
+///   a FIFO or a character device it is not a limit at all: both stat as zero
+///   bytes and then read forever into the guest's tmpfs.
+/// - reading exactly one byte past the limit is what distinguishes "fits" from
+///   "was truncated", so an over-long file is refused instead of silently
+///   arriving as its first `max_size` bytes.
+fn copy_host_shared_file(src_path: &Path, dst_path: &Path, max_size: u64) -> Result<()> {
+    use fs::os::unix::fs::OpenOptionsExt;
+    use std::io::Read;
+
+    let src_io = fs::OpenOptions::new()
+        .read(true)
+        .custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK)
+        .open(src_path)?;
+    let file_type = src_io.metadata()?.file_type();
+    if !file_type.is_file() {
+        bail!("Source file is not a regular file: {file_type:?}");
+    }
+    let mut src_io = src_io.take(max_size.saturating_add(1));
+    let mut dst_io = fs::OpenOptions::new()
+        .write(true)
+        .create(true)
+        .truncate(true)
+        .open(dst_path)?;
+    let copied = std::io::copy(&mut src_io, &mut dst_io)?;
+    if copied > max_size {
+        bail!("Source file is too large, max size is {max_size} bytes");
+    }
+    Ok(())
+}
+
 impl HostShared {
     fn load(host_shared_dir: impl Into<HostShareDir>) -> Result<Self> {
         let host_shared_dir = host_shared_dir.into();
@@ -281,21 +326,8 @@ impl HostShared {
                 }
                 bail!("Source file {src} does not exist");
             }
-            let src_size = src_path.metadata()?.len();
-            if src_size > max_size {
-                bail!("Source file {src} is too large, max size is {max_size} bytes");
-            }
-            use fs::os::unix::fs::OpenOptionsExt;
-            let mut src_io = fs::OpenOptions::new()
-                .read(true)
-                .custom_flags(libc::O_NOFOLLOW)
-                .open(src_path)?;
-            let mut dst_io = fs::OpenOptions::new()
-                .write(true)
-                .create(true)
-                .open(dst_path)?;
-            std::io::copy(&mut src_io, &mut dst_io)?;
-            Ok(())
+            copy_host_shared_file(&src_path, &dst_path, max_size)
+                .with_context(|| format!("Failed to copy host-shared file {src}"))
         };
         info!("Mounting host-shared");
         mount_host_shared(host_shared_dir)?;
@@ -4242,3 +4274,95 @@ Endpoint = [2001:db8::1]:51822
         assert!(wireguard_endpoint_hosts("Endpoint = missing-port").is_err());
     }
 }
+
+#[cfg(test)]
+mod host_shared_copy_tests {
+    use super::*;
+    use std::os::unix::ffi::OsStrExt as _;
+
+    fn temp_dir(name: &str) -> PathBuf {
+        let dir = std::env::temp_dir().join(format!("dstack-{name}-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&dir);
+        std::fs::create_dir_all(&dir).unwrap();
+        dir
+    }
+
+    #[test]
+    fn a_regular_file_within_the_limit_is_copied() {
+        let dir = temp_dir("host-shared-ok");
+        let src = dir.join("src");
+        let dst = dir.join("dst");
+        std::fs::write(&src, b"hello").unwrap();
+        copy_host_shared_file(&src, &dst, 1024).unwrap();
+        assert_eq!(std::fs::read(&dst).unwrap(), b"hello");
+        let _ = std::fs::remove_dir_all(dir);
+    }
+
+    #[test]
+    fn a_file_over_the_limit_is_refused_rather_than_truncated() {
+        let dir = temp_dir("host-shared-big");
+        let src = dir.join("src");
+        let dst = dir.join("dst");
+        std::fs::write(&src, vec![b'a'; 64]).unwrap();
+        let err = copy_host_shared_file(&src, &dst, 16).unwrap_err();
+        assert!(err.to_string().contains("too large"), "{err}");
+        let _ = std::fs::remove_dir_all(dir);
+    }
+
+    /// A FIFO is what a 9p host share can name where a file is expected. It
+    /// stats as zero bytes, so a stat-then-copy pair reads it without any
+    /// limit, and opening it without `O_NONBLOCK` blocks until a writer shows
+    /// up -- a boot that hangs instead of one that fails.
+    #[test]
+    fn a_fifo_is_refused_without_blocking_or_reading_it() {
+        let dir = temp_dir("host-shared-fifo");
+        let src = dir.join("src");
+        let dst = dir.join("dst");
+        let path = std::ffi::CString::new(src.as_os_str().as_bytes()).unwrap();
+        // SAFETY: FFI call with a NUL-terminated path this test owns.
+        assert_eq!(unsafe { libc::mkfifo(path.as_ptr(), 0o600) }, 0);
+
+        assert_eq!(
+            std::fs::metadata(&src).unwrap().len(),
+            0,
+            "a FIFO stats as zero bytes, which is why the old size check passed it"
+        );
+
+        let (tx, rx) = std::sync::mpsc::channel();
+        let src_for_thread = src.clone();
+        let dst_for_thread = dst.clone();
+        std::thread::spawn(move || {
+            let result = copy_host_shared_file(&src_for_thread, &dst_for_thread, 32);
+            let _ = tx.send(result.is_err());
+        });
+        // A writer, so an implementation that blocks on open gets past it and
+        // then streams without a limit. Best-effort: nothing must depend on it.
+        let src_for_writer = src.clone();
+        std::thread::spawn(move || {
+            if let Ok(mut fifo) = std::fs::OpenOptions::new().write(true).open(src_for_writer) {
+                loop {
+                    if fifo.write_all(&[b'a'; 4096]).is_err() {
+                        break;
+                    }
+                }
+            }
+        });
+
+        let refused = rx.recv_timeout(std::time::Duration::from_secs(10)).expect(
+            "the copy neither finished nor failed within 10s: it is blocked on the \
+                 FIFO open, or reading it with no limit",
+        );
+        assert!(refused, "a FIFO must be refused, not read");
+        let _ = std::fs::remove_dir_all(dir);
+    }
+
+    #[test]
+    fn a_symlink_is_still_refused() {
+        let dir = temp_dir("host-shared-symlink");
+        let src = dir.join("src");
+        let dst = dir.join("dst");
+        std::os::unix::fs::symlink("/etc/hostname", &src).unwrap();
+        assert!(copy_host_shared_file(&src, &dst, 1024).is_err());
+        let _ = std::fs::remove_dir_all(dir);
+    }
+}