chore(gateway)!: remove insecure_skip_attestation - #1148
Merged
Conversation
kvinwang
force-pushed
the
chore/gateway-drop-skip-attestation
branch
2 times, most recently
from
August 27, 2026 06:02
a48f88c to
d9d51e9
Compare
`core.debug.insecure_skip_attestation` turned off the two checks that make a gateway cluster mean anything: the app_id a node presents on its RPC certificate, and the peer identity behind every WaveKV sync and push. With it set, any client that could reach the sync routes could insert entries that replicate to every gateway in the cluster. It existed because the integration suites could not run without it. They can now -- they bring up a guest agent simulator and verify quotes through the normal production path -- so nothing sets it, and it is deleted rather than left as a setting a copied config could carry into production. The full-stack suite's forbidden-settings audit loses the pattern for it and keeps the other four; there is no longer a setting to catch. BREAKING CHANGE: a deployment that set `core.debug.insecure_skip_attestation = true` now attests its peers. The config struct does not deny unknown fields, so the line is ignored rather than rejected -- the gateway starts, and peers that could not present a verifiable app_id stop being accepted. Remove the line and make sure every node can attest. There is no replacement: a gateway that cannot attest its peers is not a cluster it is safe to join.
Removing `insecure_skip_attestation` left `my_app_id` as `Option<Vec<u8>>` even though `main` now always produces a value. The type still permitted `None`, and one consumer failed open on it: `HttpsClientConfig.cert_validator` was built with `my_app_id.map(...)`, and `HttpsClient::new` treated a `None` validator as "standard verification" -- plain CA-path validation with no app id check on every outgoing WaveKV sync connection. That was the outbound half of the switch that was just deleted, still reachable by construction. Made the identity non-optional the whole way down: - `Proxy::my_app_id` and `ProxyOptions::my_app_id` are `Vec<u8>`; `main` binds the guest agent lookup directly, so a host that cannot answer it fails to start instead of starting without an identity. - `HttpsClientConfig::cert_validator` is `Arc<dyn CertValidator>`, and `HttpsClient::new` lost its validator-less branch. Chain verification against the configured CA still runs first; the validator is an identity check on top of it, never a replacement. - `authorize_peer` takes `&[u8]`. `a_gateway_without_an_app_id_authorizes_nobody` is deleted: it asserted that a `None` self-identity rejects everyone, which the type system now guarantees. The `https_client` transport tests mint an app-id leaf and a matching validator instead of opting out, so they exercise the real verifier path. Also records in `security-model.md` why this switch was removed rather than listed as a development mode: the "auditable, not production-safe" argument covers a switch whose trust decision is still made and measured, not one that decides whether attestation happens at all. 307 passed / 0 failed; clippy and fmt clean.
…t PKI `app_id_server_cert` was the last hand-rolled CA in the gateway: raw `rcgen` for the CA half, `CertRequest` for the leaf, then three `fs::write` calls. `ra_tls::test_pki::write_mtls_pki` does all of it, so this drops to a builder chain and the two DER accessors. Same certificates as before -- a `127.0.0.1` server-auth leaf carrying the requested app id, under a CA the client's root store accepts. Also spells out in the doc comment that the returned config validates against that same app id, which is the setup `a_peer_from_another_app_cannot_complete_the_handshake` deliberately breaks by overriding `cert_validator`. `IsCa::Ca` and `CertificateParams::new` now appear zero times in the gateway. 307 passed / 0 failed; clippy and fmt clean.
cert_validator is an Arc<dyn CertValidator>, not an Option, but both its own doc comment and HttpsClientConfig's still described it as optional.
kvinwang
force-pushed
the
chore/gateway-drop-skip-attestation
branch
from
August 27, 2026 07:25
d9d51e9 to
d5457be
Compare
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.
Stacked on #1147. One commit, 12 files, +15 / -32.
Problem
core.debug.insecure_skip_attestationturned off the two checks that make agateway cluster mean anything:
main.rsskipped askingthe guest agent for it entirely, leaving
my_app_idasNone);ensure_from_gateway.With it set, any client that could reach the sync routes could insert entries
that replicate to every gateway in the cluster.
It existed for one reason: the integration suites could not run without it.
That reason is gone — #1147 gives them a guest agent simulator and verifies
quotes through the normal production path — so nothing sets it any more.
Leaving it in place would leave a setting that a copied config can carry into
production, in a component whose whole job is to be the cluster's trust
boundary.
docs/security/security-model.mdargues that dstack's developmentswitches are acceptable because they are visible in attestation measurements
or public contract state. That argument does not extend to this one: it is
what decides whether attestation happens at all.
Fix
Deleted: the field, the three code paths that read it, the two config files
and two documents that mention it, and the pattern for it in the full-stack
suite's forbidden-settings audit. The audit keeps the other four patterns;
there is simply no longer a setting to catch.
One stale comment goes with it —
authorize_peer's doc referred to "theattestation bypass" as the half of
verify_gateway_peera test cannot reach.Now only Rocket's certificate guard is.
Breaking change
A deployment that set
core.debug.insecure_skip_attestation = truenowattests its peers. The config struct does not use
deny_unknown_fields, sothe leftover line is ignored rather than rejected. What happens next
depends on why it was set:
that cannot present a verifiable app_id stop being accepted.
the gateway no longer starts at all.
mainnow asks the guest agent forthis node's app_id unconditionally and fails with
Failed to get app info.This is deliberate:
my_app_idis what every peer check compares against, soa gateway that cannot learn its own identity must not come up without one.
Either way: remove the line and make sure every node can attest.
There is no replacement, deliberately: a gateway that cannot attest its peers
is not a cluster it is safe to join.
Verification
cargo test -p dstack-gateway --all-features: 308 passed / 0 failed, on thisbranch and on its base, so the deletion changes no test outcome.
The three integration suites were run on this exact tree — this is the state
they were verified against in #1147:
cluster/e2e/proxy-e2e/grep -rn insecure_skip_attestationover the repo: 0 hits.Follow-up: the identity is now non-optional (cda064e)
Review found that the deletion left
my_app_idasOption<Vec<u8>>, and thatone consumer still failed open on
None:cert_validatorwas built withmy_app_id.map(...), andHttpsClient::newtreated aNonevalidator as"standard verification" — plain CA-path validation with no app_id check on
every outgoing WaveKV sync connection. That was the outbound half of the very
switch being removed, still reachable by construction.
Tightened the whole way down:
Proxy/ProxyOptionscarryVec<u8>,HttpsClientConfig::cert_validatorisArc<dyn CertValidator>andHttpsClient::newlost its validator-less branch, andauthorize_peertakes&[u8]. Chain verification against the CA still runs first — the validator isan identity check on top of it, not a replacement.
a_gateway_without_an_app_id_authorizes_nobodyis deleted; it asserted whatthe type system now guarantees. The
https_clienttransport tests mint anapp-id leaf and a matching validator rather than opting out, so they now
exercise the real verifier path.
cargo test -p dstack-gateway --all-features: 307 passed / 0 failed (308 minusthe deleted test).
cargo clippy --all-targetsandcargo fmt --check: clean.