refactor(guest-agent): restrict GetQuote to Intel TDX - #1107
Merged
Conversation
kvinwang
force-pushed
the
feat/getquote-tdx-only
branch
2 times, most recently
from
August 23, 2026 17:28
05664ae to
8d7dbf8
Compare
Restricting `GetQuote` to Intel TDX closes the backend path `Worker.GetAttestationForAppKey` used, so on SEV-SNP and Nitro that method can no longer answer -- its response type is `GetQuoteResponse`, and there is no quote to put in it. That leaves the external listener with no way to attest an app key at all. `Attest` is no substitute: it lives on the internal unix socket, and an external caller could not use it even if it did not, because `Attest` takes the report data from the caller and the caller does not know the app key's public key until the agent derives it. Deriving the key and building the report data is the whole reason this method exists. `AttestAppKey` is that method with a response that can carry every platform's evidence: same request, `AttestResponse` instead of `GetQuoteResponse`. Nothing is lost against the old shape -- the versioned attestation carries the report data and vm_config that `GetQuoteResponse` exposed as separate fields, and the quote and event log inside it. The old method is removed rather than kept alongside. It has shipped since v0.5.7, so this is a breaking change, but no SDK exposes it, the tree has no callers outside these tests, and on every platform but TDX it already returned an empty quote plus the field-5 attestation that never shipped. Keeping a TDX-only alias of a method that now works everywhere would cost more than it buys. `GetAttestationForAppKeyRequest` is renamed to `AttestAppKeyRequest`; message names do not appear on the wire. The report-data derivation moves to `app_key_report_data`, which also gained a bounds check: a public key encoding longer than the 64 bytes of report data used to panic on the slice copy.
`Attestation::with_report_data` patched the report data into `PlatformEvidence::Tdx` and `SevSnp` and fell through `other => other` for everything else -- including `GcpTdx`, whose quote has the identical TDX layout. The simulator uses this to answer `GetQuote` and `Attest`, so on a simulated GCP platform it served a quote still bound to the fixture's report data while echoing the caller's in the response: two different values, no error, and nothing in the tests looking. Patch `GcpTdx` the same way. The vTPM quote beside it is left alone; it commits to PCRs, not to this report data. Then drop the wildcard that hid this, here and in `strip_for_config_with_events` above it, which has the same shape and the same exposure. A wildcard in a transform like these fails open: a platform nobody thought about gets passed through unmodified and looks like it was handled. Spelling out Nitro and SEV-SNP costs two lines and turns the next such omission into a compile error. `simulator_serves_get_quote_on_gcp_tdx` now asserts the binding rather than just a non-empty quote, and fails without the fix. It also pins the other half of the contract this PR defines: GetQuote answers wherever the platform has a TDX quote, and the vTPM quote GCP's verification also binds is reachable only through Attest.
kvinwang
force-pushed
the
feat/getquote-tdx-only
branch
from
August 23, 2026 17:32
8d7dbf8 to
731aa2d
Compare
…f production `with_report_data` has one non-test caller in the tree -- the simulator -- and it is a `pub fn` on a crate that kms, gateway, verifier, guest-agent and dstack-util all depend on. What it does there is rewrite bytes 568..632 of a signed TDX quote, which leaves the signature covering the bytes that used to be there. The output is evidence-shaped and cannot pass verification. That is exactly right for a simulator whose callers skip verification, and wrong for every other consumer of this crate. That mismatch is why the missing `GcpTdx` arm went unnoticed for so long: nothing about the function's placement said "only one caller, and it is a simulator". Put it behind a `simulator` feature, `#[cfg(any(test, feature = "simulator"))]`, passed through `ra-tls` to the one crate that needs it. A per-package build -- `cargo build --release -p dstack-kms`, the form the docs prescribe -- now cannot reach it: adding a call to `kms/src/config.rs` fails with `no method named with_report_data`. `cargo build --workspace` unifies features and does still compile it in, so this is a statement of intent with teeth on the release path, not a hard wall. Adding the gate immediately found the one legitimate cfg(test) user outside the simulator, `guest-agent`'s mock platform, which now declares the feature in `[dev-dependencies]` -- so guest-agent's production build drops it too. Also splits out `with_stack_report_data`. The simulator's seeded path called the full version and then overwrote the quote with a freshly generated, properly signed one, so the quote-patching half was dead work that read as if it mattered. It now asks for the half it actually uses.
… RPC The first commit in this series stopped `generate_and_save_attestation` from discarding the attestation when the quote request fails, so a non-TDX node -- or a TDX node during a QGS outage -- now saves a record carrying only `attestation`. Nothing could read it: `CertAttestationInfo` has fields for the public key and the quote and nothing else. That made the transient-failure case worse than before, not better. `save_cert_attestation` overwrites the `latest` pointer unconditionally, so one failed quote request on a TDX node replaced a record the admin RPC could read with one it could not. Add `attestation` as field 5, additive, and map it in both the `latest` and `history` views. Documents on `quote` when it is empty and what carries the evidence instead. ct_monitor still bails on an empty quote, so it remains TDX-only; that is the follow-up this PR's description names.
The gate is `Attestation::get_tdx_quote_bytes()`, which is `Some` for `DstackGcpTdx` as well as `DstackTdx`, so GCP Confidential VMs are served. The docs said they get an error, which was wrong -- and wrong in the direction that matters: a relying party told "GCP errors here" and then handed a GCP quote has no reason to look for the vTPM quote GCP's verification binds alongside it. `GetQuoteResponse` has no field for that quote, and the raw-quote path in the verifier rebuilds its input as `AttestationQuote::DstackTdx`, so it checks the TDX half and stops. Say what actually happens instead: GetQuote needs TDX, GCP gets the TDX half only, and Attest is the answer both where GetQuote errors and where it under-reports. Updated in the proto, docs/usage.md, sdk/curl/api.md, the verifier README, and all four SDKs. Also fixes the `[Attest](#8-attest)` anchor in `sdk/curl/api.md` and `dstack/verifier/README.md` -- Attest is section 7. The verifier README has pointed at the wrong section since Sign and Verify were inserted above it.
kvinwang
force-pushed
the
feat/getquote-tdx-only
branch
from
August 24, 2026 01:02
731aa2d to
4d42fa7
Compare
`tdx_event_log()` reaches `Tdx | GcpTdx`; `tdx_event_log_mut()` reached only `Tdx`. The two drifted apart because both ended in `_ => None`, so adding `GcpTdx` to one and not the other compiled fine. The simulator fills V2 digest preimages through the mut accessor, so on a GCP attestation it silently filled nothing and returned events without the preimage `agent_rpc.proto` tells clients to verify against the digest. Drop the wildcard from all eight accessors on `PlatformEvidence` and spell the variants out. This is the same failure `with_report_data` had two commits ago; a wildcard is how a family of per-platform methods disagrees with itself without anyone noticing. `tdx_event_log_accessors_agree_on_gcp_tdx` pins the read and mut halves to the same set and fails against the old match. Also corrects the comment on the `GcpTdx` arm of `with_report_data`. It claimed the vTPM quote was left alone because it commits to PCRs rather than to the report data. That is wrong: its `qualified_data` is `sha256(tdx_quote)` (attestation.rs:1087), so patching the quote severs that binding too. The behaviour is right -- re-deriving it would need the AK to re-sign, and the patch has already invalidated the quote signature -- but the reason given was not.
The algorithm reached `get_key` verbatim, and `get_key` knows only
`ed25519` and `secp256k1`, so `AttestAppKey{algorithm:
"secp256k1_prehashed"}` came back "Unsupported algorithm" -- while
`Sign` accepts that name and has mapped it to its base key since it was
added. A client that signs prehashed had no way to attest the key it
signs with, under the name it signs with.
Prehashing is a signing mode, not a key type: the same secp256k1 key
signs both ways and has the same public key, so derive it under the base
name exactly as `Sign` does. The arm for it in the match below was
already there and simply unreachable; the PR's own prehashed sign test
sidestepped this by asking for "secp256k1".
`test_attest_app_key_accepts_secp256k1_prehashed` asserts both names
attest the same public key.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
GetQuoteanswered on every platform, but only on Intel TDX did it return what itsname promises. Elsewhere — AMD SEV-SNP, AWS Nitro, and friends — it returned an empty
quoteand an emptyevent_log, and put the real evidence in a fifth field,GetQuoteResponse.attestation, holding a SCALE/msgpack versioned attestation:So a client had to test whether
quotewas empty to learn which field carried theanswer, and one response type had two mutually exclusive shapes.
Attestalreadyreturns exactly that versioned attestation, on every platform, in one shape — the
attestationfield was a second, shape-shifting way to ask the same question.The silent-failure case is worse.
Tappd.TdxQuoteandRawQuoteshare this backendpath and copy only
quoteandevent_loginto their own response, dropping field 5.On a non-TDX platform they returned
quote: ""with no error — a caller asking forattestation evidence got an empty string back and nothing to indicate why.
Fix
GetQuoteis now Intel TDX only; every other platform gets an error namingAttestas the replacement. Field 5 is removed and reserved.
Tappd.TdxQuote/RawQuoteandWorker.GetAttestationForAppKeyshare the backend path, so they fail closed there tooinstead of returning an empty quote. GCP TDX is unaffected — it carries a real TDX quote.
Four commits, ordered so no commit leaves a regression behind:
fix(gateway): keep the attestation when the quote request fails— stands on itsown.
distributed_certbot.rstreated aget_quotefailure as fatal (return Ok(()))while tolerating an
attest()failure, so a transient QGS outage threw away theattestation it could still have had, and saved no record at all. Both are best-effort
now, and the save is skipped only when both come back empty. Without this, commit 2
would have silently stopped non-TDX gateways from persisting ACME and cert
attestations entirely.
refactor(guest-agent): restrict GetQuote to Intel TDX— the RPC change.refactor(sdk): drop the GetQuote attestation field— Rust, Python, Go, JS.docs: mark GetQuote as Intel TDX onlyWire compatibility
Field 5 never shipped. It is absent from
v0.5.7,v0.5.8andv0.5.9(
git show v0.5.9:dstack/guest-agent/rpc/proto/agent_rpc.proto), so no released clientcan be reading it. It is reserved anyway, following
vmm_rpc.proto.Verification
cargo clippy --workspace --all-features -- -D warnings --allow unused_variablescleanon this branch;
cargo fmt --all --checkclean.cargo test -p dstack-guest-agent -p dstack-guest-agent-simulator -p dstack-gateway --all-featurespasses.simulator_rejects_get_quote_on_non_tdxpins both halves of thecontract on one attestation: it swaps the TDX fixture's platform evidence for
PlatformEvidence::SevSnp, assertssimulated_quote_responsefails with an errorcontaining "Intel TDX only", then asserts
simulated_attest_responsestill succeeds.go build ./...andgo vet ./dstack/clean;tsc --noEmitclean;cargo check --all-featuresandcargo fmt --checkclean insdk/rust..attestationhits are on
AttestResponseandGpuInfoResponse, which are untouched.Follow-up (not in this PR)
ct_monitordeclares its ownQuotedPublicKeywith justpublic_keyandquote, andsends only
quote/event_log/vm_configto the verifier — it never reads theattestationfield the gateway publishes, even though/verifyaccepts it. That CTmonitoring path has therefore never worked on non-TDX, before or after this change.
This PR makes sure the evidence is at least stored and served; teaching
ct_monitorto consume it is separate work.