verifier: performance findings — a quadratic pre-signature replay, five parses of one blob, and two binaries named dstack-mr

[kvinwang]

A read of verifier, dstack-mr, dstack-attest, cc-eventlog, tpm-qvl and qemu-acpi, looking for cost rather than correctness. Readability findings from the same sweep are collected separately.

Baseline for "input size": the verifier sets no limits in dstack-verifier.toml, so Rocket's default 1 MiB JSON body applies to POST /verify. MAX_ATTESTATION_BYTES is 10 MiB, which is the operative ceiling on the RA-TLS/KMS path where the attestation rides in a certificate.

Already fixed elsewhere and not repeated below: the unbounded AIA walk (#1238), the zero-length TDVF GUID entry hang and the unbounded compute_mrtd page loop (#1236), the dead --debug RTMR diff and the attacker-steerable cache key and the divergent sha256sum.txt grammars (#1251).

Two findings are going out as PRs rather than living here, because they are defects rather than slowness: the quadratic TPM event-log replay described first below, and dstack-mr diagnose computing a different RTMR0 than the verifier.

The expensive one: O(pcrs × events) replay, before any signature is checked

tpm-qvl/src/verify.rs:317-334, called from verify_quote_with_ca:127 — before extract_ak_public_key_from_cert (:134), before verify_signature_with_key (:148), before verify_ak_chain_with_collateral (:163). Nothing upstream requires the PCR indices in quote.pcr_values to be distinct.

for pcr in pcr_values {
    let pcr_events: Vec<&TpmEvent> = event_log.iter().filter(|e| e.pcr_index == pcr.index).collect();
    for event in &pcr_events {
        let mut hasher = Sha256::new();
        hasher.update(&replayed_pcr);
        hasher.update(&event.digest);
        replayed_pcr = hasher.finalize().to_vec();   // heap alloc per extend
    }

Both gates in front of it are attacker-computable: attested_pcr_indices == provided_pcr_indices (the TPMS_ATTEST pcr_selections list is unsigned at this point and pcr_select_count is a u32), and pcr_digest == compute_pcr_digest(pcr_values) (a plain SHA-256 the attacker computes themselves). Making every pcr_values entry identical makes every replay succeed, so the loop runs to completion rather than bailing early.

~5 000 PcrValue (≈105 B of JSON each) plus ~5 000 TpmEvent (≈95 B each) fit inside 1 MiB and yield 25 million SHA-256 extends plus 25 million 32-byte heap allocations. openssl speed on this host reports 3.4 M 64-byte SHA-256 ops/s → ~8–12 s of one core, unauthenticated, before a signature is checked. The 10 MiB RA-TLS ceiling makes it ~100× worse.

Reachable on POST /verify via AttestationQuote::DstackGcpTdx: attestation.rs:1075 verifies the TDX quote first, then hands the separately supplied, entirely attacker-controlled tpm_quote to fetch_and_verify. A replayed public GCP attestation satisfies the first step.

Fix (going out as a PR): reject duplicate PCR indices up front — TPM semantics already forbid them — bucket the event log by pcr_index in one pass, and hoist replayed_pcr to a [u8; 32]. The duplicate check alone kills the quadratic.

Two binaries are both named dstack-mr, and documented commands do not run

dstack-mr/Cargo.toml:18 declares [[bin]] name = "dstack-mr" (measure-os, tdx-measurement-cbor, snp-measurement-cbor, inspect-measurement) and dstack-mr/cli/Cargo.toml:12 declares a second [[bin]] name = "dstack-mr" (measure, diagnose). The command sets do not overlap.

$ cargo build --release -p dstack-mr -p dstack-mr-cli --bins   # succeeds, no warning
$ ./target/release/dstack-mr measure-os /tmp
error: unrecognized subcommand 'measure-os'

$ cargo run --manifest-path dstack/Cargo.toml --bin dstack-mr -- --help
error: `cargo run` can run at most one executable, but multiple were specified
help: available targets:
    bin `dstack-mr` in package `dstack-mr`
    bin `dstack-mr` in package `dstack-mr-cli`

That last invocation is verbatim docs/attestation-tdx.md:75. docs/rc-testing-runbook.md:272 and verifier/fixtures/tdx-lite.README.md:32 document tdx-measurement-cbor / snp-measurement-cbor, which the surviving target/release/dstack-mr does not have. Half the documented measurement-reproduction workflow does not run, and which half depends on link order. Rename one bin, or fold the two clap subcommands into the other package's dispatcher.

Repeated parsing and copying

The SNP vm_config document is parsed five times per request, once via a full Value deep clone — dstack-mr/src/sev.rs:1026-1035:

let value: serde_json::Value = serde_json::from_str(vm_config)?;                 // parse 1
let parsed: SevSnpMeasurementVmConfig = serde_json::from_value(value.clone())?;  // deep clone of the whole tree
let nested = value.get("vm_config")...map(|s| serde_json::from_str::<..>(s))     // parse 2

value.clone() deep-copies every node of a caller-controlled document to feed a three-field struct; SevSnpMeasurementVmConfig::deserialize(&value) does the same work with no copy. On one SNP POST /verify the same string is additionally parsed by vm_config_json_from_config (attestation.rs:421, a full Value parse plus a nested re-parse) and mr_config_document_from_config (:440, another pair) — five from_str passes over the same ≤1 MiB blob. The verifier adds two string copies on top (verification.rs:802 passes .clone() by value, and :880 clones it again into raw_config).

TcgEventLog::to_cc_event_log clones the entire boot event log to throw it away — cc-eventlog/src/tcg.rs:311. TryFrom<TcgEvent> consumes by value, so every event including its multi-KB EV_EFI_* payload is deep-copied — and both callers own the log and drop it on the next line. A real CCEL is ~100 KB across ~60 events. into_cc_event_log(self) is a three-line change.

patch_kernel copies the whole kernel to rewrite ~20 header bytes — dstack-mr/src/kernel.rs:137, kernel_data.to_vec(), where every byte it writes lives below offset 0x238. One 13 MiB allocation and memcpy per uncached measurement, perhaps 3–5 % on top of the ~40 ms SHA-384 pass. Splitting authenticode_sha384_hash to take (patched_header, rest) touches the function that computes RTMR1[0], so it needs a golden-vector proof first.

Dead code carrying real weight

tdx_measurements_for_image_dir_without_rtmr0 and ..._with_acpi_hashes have zero callers — dstack-mr/src/tdx.rs:467, :559. rg matches only the definitions. ~180 lines, ~90 % identical to each other, and they hold two of the five copies of the VmConfig → Machine translation — so every new measured VmConfig field has to be added to code nobody calls. Tdvf::rtmr0 (tdvf.rs:407) is explicitly #[allow(dead_code)] in the same file.

#![allow(dead_code)] on all of cc-eventlog/src/tcg.rs hides the TcgIMR trait with no implementors, two unreferenced event types, three format constants and ~40 EV_* constants of which two are used — roughly 120 of the file's 447 lines.

Unbounded and uncached

No spawn_blocking anywhere in the verifier — rg spawn_blocking dstack/verifier → no hits, while the gateway, VMM, guest-agent, tpm-qvl and cached-cell all use it. On a Rocket worker, POST /verify synchronously does: read_to_string(sha256sum.txt), fs::read of firmware + kernel + initrd, SHA-384 over kernel and initrd, the MRTD page walk, ACPI generation, cache read/write, and flate2 + tar extraction of a downloaded image. guest-agent/src/rpc_service.rs:386 even has a comment explaining when the hop is not worth it — the reasoning was never applied here. Wants a measurement on a real image first.

The measurement cache still has no eviction. #1251 fixed the attacker-steerable key; store_measurements_in_cache still writes measurements/<key>.json with no size cap, no LRU, no TTL, and no deletion anywhere.

TDX-lite recomputes the ACPI tables per request with no cache — verification.rs:1069. The legacy path caches its much more expensive result on disk; the lite path caches nothing, though the output depends only on the 13-field MachineConfig. Measured with crates/qemu-acpi/examples/dump, 3 runs each including ~2–3 ms of process start:

cpu_count	per run	tables.bin
1	4 ms	128 KiB
256	3 ms	128 KiB
1024	5 ms	256 KiB
2048	8 ms	384 KiB
4096	12 ms	640 KiB

~10 ms of CPU and ~640 KiB at the maximum cpu_count MachineConfig::validate allows, per unauthenticated request, scaling linearly. Not alarming alone; the odd part is that the cheap-to-cache path is the uncached one.

Smaller repeated work

where	what
sev.rs:1053 / :1104	validate_measurement_input runs twice per SNP request — two full passes resolving section types and accumulating the page budget
sev.rs:249 / :762	Gctx::update_vmsa_page re-hashes a byte-identical AP VMSA page once per vCPU — 511 redundant SHA-384 passes over 4 KiB at MAX_VCPUS, ~1 ms. Three lines to hoist
sev-snp-qvl/src/lib.rs:494-501, :528-542	the SNP report is length-checked and parsed twice, once per candidate product; and ark/ask/vcek are parsed, then discarded when external_root is set, then re-parsed by verify_x509_chain
tdvf.rs:42-47	five EFI-variable digests are GUID-parsed and hashed per call, in the same vec![] as two hex! literals — two styles for one class of constant, three lines apart
attestation.rs:2138	Attestation stores the runtime events twice — runtime_events is a filtered deep copy of quote.event_log and both are kept, so every runtime event is in memory twice and the two must stay in sync by convention
verification.rs:518	prune_unlisted_image_files linear-scans listed_files inside the per-directory-entry loop — O(entries × lines), nothing at ~10 files, but it is the pattern #33 called out

Checked and found clean

qemu-acpi — MachineConfig::validate bounds every input that drives generation, so build is linear in a bounded quantity (confirmed empirically above); aml_encode::package's fixpoint loop converges in ≤2 iterations and encode_pkg_length rejects >256 MiB packages; QemuVersion::compatibility's clamp-up/reject-down asymmetry is documented with its reasoning and both edges are tested. cc-eventlog::codecs::VecOf does the right things — MAX_LEN before allocating, with_capacity(len.min(1024)) so a length field cannot size the allocation. The V2 preimage handling checks presence, hex validity, digest agreement and canonical-form equality, with a test pinning exactly that. dstack-mr::sev's OVMF parsing uses checked_sub / data.get(..) / binrw asserts throughout and bounds the section count at decode time. dstack-types' measurement-document helpers are consistent across TDX/SNP/GCP/AWS with golden CBOR vectors pinning the encodings os_image_hash commits to. The verifier's image download and extraction carry comments that correctly explain why, and store_measurements_in_cache writes via NamedTempFile + sync_all + persist with a concurrency test. policy_tcb_fields is an exhaustive match with a comment per arm and a seven-row table plus a fail-closed test — the model the rest of the file should follow. The verifier's config validation covers precedence, each failure mode and deny_unknown_fields. And dstack-mr::kernel's high-memory kernel-hash tests pin exactly the property the lite path's memory_size restriction depends on, including both boundary values.

[kvinwang]

Cold verifier work is implemented in follow-up PR #1277: same-image downloads are coalesced (baseline 16 concurrent downloads -> fixed 1), extraction/hash/measurement work is offloaded and bounded, and publication is lifecycle-safe.