From 4279903a7c2462739f4a604eb3ccba5871b0e531 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Wed, 21 Oct 2026 23:34:27 -0700 Subject: [PATCH 1/4] guest-agent: scope-gated external API The external listener answered anyone who could route to the CVM. It now requires a scope per route, granted by an Ed25519 JWT or by the host's `guest_agent_api` sys-config. Nothing is anonymous by default. This is an exposure control, not a confidentiality one: `public_logs`, `public_sysinfo` and `public_tcbinfo` still decide what may leave the TEE and are still measured. A scope can only narrow what they allow. - `dstack-api-auth::jwt`: a fixed-algorithm subset of JWT. The header is a constant, so `none`, HMAC and key-carrying headers cannot be expressed. Tokens are bound to the app id and capped at an hour. - The gateway's token is minted inside the CVM from the app root key and carried on `RegisterCvmRequest`, so registration needs no new distribution channel. - Paths are checked after percent-decoding and segment normalisation, since the router matches on that rather than on the raw string. - The dashboard is filtered block by block: identity, sysinfo, containers, logs and TCB each need their own scope. - The systemd watchdog probes the internal socket, which cannot depend on anonymous access being configured. Upgrade order matters: an old gateway sees 401s from a new guest and drops it from rotation after `failure_threshold` polls. See docs/guest-api-auth.md. Signed-off-by: Kevin Wang --- CHANGELOG.md | 1 + README.md | 1 + docs/guest-api-auth.md | 144 +++++++ docs/guest-api-v1.md | 5 + docs/security/cvm-boundaries.md | 11 +- docs/security/security-best-practices.md | 2 +- docs/usage.md | 2 +- dstack/Cargo.lock | 7 +- dstack/crates/api-auth/Cargo.toml | 3 + dstack/crates/api-auth/src/jwt.rs | 388 ++++++++++++++++++ dstack/crates/api-auth/src/lib.rs | 43 +- dstack/dstack-types/src/lib.rs | 34 ++ dstack/dstack-util/Cargo.toml | 1 + dstack/dstack-util/src/system_setup.rs | 55 +++ dstack/gateway/rpc/proto/gateway_rpc.proto | 10 + dstack/gateway/src/kv/import.rs | 1 + dstack/gateway/src/kv/mod.rs | 15 + dstack/gateway/src/main_service.rs | 18 + ...ateway__main_service__tests__config-2.snap | 3 +- ..._gateway__main_service__tests__config.snap | 1 + dstack/gateway/src/main_service/tests.rs | 13 + dstack/gateway/src/models.rs | 10 + dstack/gateway/src/proxy/health_check.rs | 39 +- dstack/gateway/src/proxy/port_policy.rs | 23 +- dstack/guest-agent/Cargo.toml | 3 +- dstack/guest-agent/src/api_auth.rs | 335 +++++++++++++++ dstack/guest-agent/src/config.rs | 13 +- dstack/guest-agent/src/http_routes.rs | 37 +- dstack/guest-agent/src/lib.rs | 1 + dstack/guest-agent/src/models.rs | 36 +- dstack/guest-agent/src/rpc_service.rs | 26 +- dstack/guest-agent/src/server.rs | 40 +- dstack/guest-agent/templates/dashboard.html | 18 +- dstack/vmm/src/app.rs | 5 + dstack/vmm/src/config.rs | 55 +++ dstack/vmm/vmm.toml | 12 + 36 files changed, 1337 insertions(+), 74 deletions(-) create mode 100644 docs/guest-api-auth.md create mode 100644 dstack/crates/api-auth/src/jwt.rs create mode 100644 dstack/guest-agent/src/api_auth.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f5c18bc8..3b2a07773 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - guest: a fresh CVM with a verity volume could take the volume for its data disk and try to LUKS-format it: volumes labelled their data partition `dstack-data`, the data disk's label. `dstack verity` now labels it `dstack-volume-data`, and the guest skips disks that carry a `dstack-volume` partition, so volumes built by older releases are safe too ### Changed +- guest-agent: the external listener (port 8090) is no longer anonymous. Every route needs a scope, granted by a signed Ed25519 JWT or by `[cvm.guest_agent_api].anonymous_scopes` in sys-config; the gateway gets its token minted inside the CVM from the app root key. `public_logs` / `public_sysinfo` / `public_tcbinfo` are unchanged and still decide what may leave the TEE. The dashboard is filtered block by block by scope. See `docs/guest-api-auth.md`. An old gateway sees 401s and drops new guests from rotation until it is upgraded - vmm: `qmp_socket` now defaults to `true`. Each CVM's QEMU exposes `qmp.sock` in its work dir, so the host can change disk options or compact a bloated `hda.img` (`blockdev-mirror`) without stopping the CVM. Applies to CVMs started after the change; set `qmp_socket = false` to opt out ## [0.6.1] - 2026-10-12 diff --git a/README.md b/README.md index 27b55eedf..3c4bbfbfe 100644 --- a/README.md +++ b/README.md @@ -161,6 +161,7 @@ Apps communicate with the guest agent via HTTP over `/var/run/dstack.sock`. Use **Reference** - [Guest Agent API v1](./docs/guest-api-v1.md) - Key derivation, signature chains, and the versioned guest API - [Guest Agent API v0](./docs/guest-api-v0.md) - The frozen legacy surface, for clients that still speak the unversioned API +- [Guest Agent API access control](./docs/guest-api-auth.md) - Scopes, tokens, and anonymous access on port 8090 - [App Compose Format](./docs/normalized-app-compose.md) - Compose file specification - [Intel TDX Attestation](./docs/attestation-tdx.md) - Measurement and runtime-event verification - [Native TEE Interfaces](./docs/native-tee-interfaces.md) - Advanced compatibility with Linux TEE devices and configfs-tsm diff --git a/docs/guest-api-auth.md b/docs/guest-api-auth.md new file mode 100644 index 000000000..2ec51495b --- /dev/null +++ b/docs/guest-api-auth.md @@ -0,0 +1,144 @@ +# Guest Agent API access control + +The guest agent's external listener (port 8090) is reachable by anyone who can +route to the CVM. It refuses callers it cannot identify: every request names a +scope, and a scope is granted either by a signed token or by the host's +configuration. + +This is an exposure control, not a confidentiality one. It decides who may +*ask*. Whether the answer may leave the TEE is decided by the app's measured +`public_logs` / `public_sysinfo` / `public_tcbinfo` switches, and a token can +only narrow what those allow. See +[the two gates](#the-two-gates) below. + +## Scopes + +| Scope | Grants | +|---|---| +| `guest:version` | `Worker.Version` on either pRPC surface | +| `guest:health` | `Worker.Health` (`/prpc/v1`) | +| `guest:info` | `Worker.Info` on either surface, and the identity block on the dashboard | +| `guest:attest-app-key` | the frozen `Worker.GetAttestationForAppKey` | +| `guest:dashboard` | the dashboard page itself | +| `guest:containers` | the container table on the dashboard | +| `guest:logs` | `/logs/`, when the app sets `public_logs` | +| `guest:metrics` | `/metrics`, and the sysinfo block on the dashboard | + +`/prpc`, `/prpc/v0` and `/prpc/v1` are matched after percent-decoding and +segment normalisation, so `/prpc/Worker.Info`, `/prpc/v0/Info` and +`/%70rpc/Info` all need `guest:info`. There is no spelling that reaches a +handler with a weaker check. + +A route the vocabulary does not name needs no scope: nothing is mounted there, +and the router's 404 is the answer. + +## Default: nothing anonymous + +With no configuration, every scope above requires a token. The gateway still +works, because it is issued one internally; the dashboard, `/metrics` and the +log views stop answering to the public. + +The app's own SDKs are unaffected: they use the internal Unix socket, which is +unauthenticated and reachable only by the application. + +## Configuring anonymous access + +Anonymous access is a host decision, made in sys-config. The VMM writes it from +`[cvm.guest_agent_api]`: + +```toml +[cvm.guest_agent_api] +anonymous_scopes = ["guest:dashboard", "guest:version", "guest:health"] +``` + +The same block in the guest's `/dstack/.host-shared/.sys-config.json`: + +```json +{ + "guest_agent_api": { + "anonymous_scopes": ["guest:dashboard", "guest:version", "guest:health"] + } +} +``` + +Listing `guest:dashboard` without `guest:containers`, `guest:info`, `guest:logs` +or `guest:metrics` gives an anonymous visitor the page with every block hidden. +Scopes are independent: each one adds exactly its own block. + +An unknown name is a boot error rather than a permission that silently never +matches. + +## External issuers + +Other than its own token, the agent accepts tokens from issuers the host names: + +```toml +[[cvm.guest_agent_api.external_issuers]] +name = "ops" +public_keys = ["2f8d...c1"] # hex Ed25519 public keys, 32 bytes each +``` + +`name` is the exact `iss` claim a token must carry; `public_keys` is a list so a +key can be rotated by publishing the new one beside the old, waiting out the +longest token lifetime (one hour), then dropping the old one. + +Tokens must be Ed25519. `alg: none`, HMAC, and headers carrying key material +(`jku`, `x5u`, `jwk`) are not rejected by policy -- the header is a constant, +so they cannot be expressed. + +### Where the internal issuer comes from + +The agent and the gateway both hold the app root key inside the TEE. The token +the gateway presents is minted in the guest's own setup (`dstack-util`) and +carried on registration, so it can be bound to the app id and expire: + +1. Both derive the same Ed25519 key: + `derive_app_key(app_root_key, "dstack:guest-api:issuer:v1", Ed25519)`. +2. `dstack-util` signs a token with `iss = "dstack:guest-api:"`, + `aud = `, a 10-minute lifetime, and the `guest:health`, `guest:info` + scopes. +3. Registration carries it to the gateway, which presents it on the v1 `Health` + poll and on the lazy `Info` fetch that fills in a port policy the CVM did not + report. + +Nothing else gets a token: the gateway has no business on the dashboard or in +the logs. + +The host cannot mint one: the signing key is derived from the app root key, which +is released to the guest by KMS only after the boot measurement is verified. + +## The two gates + +Two independent checks decide what a caller sees, and both must pass: + +| Gate | Source | Measured | Answers | +|---|---|---|---| +| `public_logs`, `public_sysinfo`, `public_tcbinfo` | app-compose | yes | may this data leave the TEE at all? | +| scopes | sys-config | no | may this caller ask? | + +They combine with `&&`. A token cannot reveal logs the app kept private -- with +`public_logs = false` the route is not even mounted -- and an anonymous caller +cannot see a block it was not granted, however public the app made it. + +Scope configuration is deliberately unmeasured. Host-supplied sys-config is not +part of the attestation, so a host could grant a caller more than the operator +intended. That grants nothing beyond what anonymous 8090 access already granted +before this change: the data is still gated by the measured switches, which a +host cannot alter without changing the app id. + +## Compatibility + +A guest with this change answers 401 to an old gateway that presents no token. The +gateway counts that as unreachable and drops the instance after +`failure_threshold` consecutive polls, so a mixed cluster rebalances away from +new guests until the gateway is upgraded. + +During the rollout, either upgrade the gateway first, or leave the host open: + +```toml +[cvm.guest_agent_api] +anonymous_scopes = ["guest:version", "guest:health", "guest:info"] +``` + +`guest:info` is needed by gateways that did not report a port policy and fetch +it lazily. diff --git a/docs/guest-api-v1.md b/docs/guest-api-v1.md index c870ae00b..652b8fff5 100644 --- a/docs/guest-api-v1.md +++ b/docs/guest-api-v1.md @@ -458,6 +458,11 @@ The v1 `Worker` has three methods, served at `/prpc/v1`. Nothing here returns key material, and no caller chooses what gets signed or attested. +Every method also needs a scope, granted by a signed token or by the host's +configuration. With no configuration the listener answers only token holders; +see [Guest Agent API access control](./guest-api-auth.md) for the vocabulary and +for how the gateway authenticates its health polls. + ### There is no v1 AttestAppKey The frozen surface has `Worker.GetAttestationForAppKey`, which attests a key the diff --git a/docs/security/cvm-boundaries.md b/docs/security/cvm-boundaries.md index df369899b..7a8a02ce8 100644 --- a/docs/security/cvm-boundaries.md +++ b/docs/security/cvm-boundaries.md @@ -180,7 +180,16 @@ Full specification: [host_api.proto](../../dstack/host-api/proto/host_api.proto) ### HTTP-based Public Guest API Service -The dstack-guest-agent runs an HTTP server on port 8090 inside the CVM. This port is publicly accessible, allowing external clients to view basic CVM information. +The dstack-guest-agent runs an HTTP server on port 8090 inside the CVM. By +default it refuses callers it cannot identify: every route needs a scope, granted +by a signed token or by the host's `guest_agent_api` sys-config. Anonymous access +is an explicit host choice. [guest-api-auth.md](../guest-api-auth.md) covers the +scope vocabulary and the token issuers. + +Anonymous access is an exposure decision, not a confidentiality one. Whether the +data may leave the TEE at all is still decided by the app's measured +`public_logs` / `public_sysinfo` / `public_tcbinfo` switches, which a scope +cannot widen. Since dstack 0.6.0 the listener serves two API surfaces, selected by URL path alone: the frozen v0.5.11 `Worker` service at `/prpc` (equivalently diff --git a/docs/security/security-best-practices.md b/docs/security/security-best-practices.md index 8fd9d28f9..093657198 100644 --- a/docs/security/security-best-practices.md +++ b/docs/security/security-best-practices.md @@ -146,7 +146,7 @@ Example app-compose.json: ## Don't expose unexpected ports -In dstack CVM, dstack-guest-agent listens on port 8090, allowing public access to basic CVM information. +In dstack CVM, dstack-guest-agent listens on port 8090. It is not anonymous by default; see [guest-api-auth.md](../guest-api-auth.md) for what the host can grant anonymously. In docker-compose.yaml, all declared ports will be exposed to the public internet. Do not expose unnecessary ports. diff --git a/docs/usage.md b/docs/usage.md index 85702f2c9..46e962acf 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -17,7 +17,7 @@ Open the dstack-vmm webpage [http://localhost:9080](http://localhost:9080) (chan After the container is deployed, it should take some time to start the CVM and the containers. Time would vary depending on your workload. - **[Logs]**: Click this button to view the CVM logs and monitor container startup progress -- **[Dashboard]**: Once the container is running, click this button to view container information and logs. (Note: This button is only visible when dstack-gateway is enabled. If disabled, you'll need to add a port mapping to port 8090 to access the CVM dashboard) +- **[Dashboard]**: Once the container is running, click this button to view container information and logs. (Note: This button is only visible when dstack-gateway is enabled. If disabled, you'll need to add a port mapping to port 8090 and allow the dashboard scopes in `[cvm.guest_agent_api]`; see [Guest Agent API access control](./guest-api-auth.md))
Guest Agent Dashboard diff --git a/dstack/Cargo.lock b/dstack/Cargo.lock index 31a97f389..fcfd0385a 100644 --- a/dstack/Cargo.lock +++ b/dstack/Cargo.lock @@ -1825,7 +1825,10 @@ dependencies = [ "anyhow", "base64 0.22.1", "bcrypt", + "ed25519-dalek", "rocket", + "serde", + "serde_json", "sha2 0.10.9", "subtle", ] @@ -2024,6 +2027,7 @@ dependencies = [ "clap", "cmd_lib", "default-net", + "dstack-api-auth", "dstack-attest", "dstack-build-info", "dstack-guest-agent-rpc", @@ -2035,6 +2039,7 @@ dependencies = [ "guest-api", "hex", "host-api", + "http-client", "k256", "libc", "listenfd", @@ -2046,7 +2051,6 @@ dependencies = [ "ra-tls", "rand 0.8.6", "rcgen 0.13.2", - "reqwest", "ring", "rinja", "rocket", @@ -2313,6 +2317,7 @@ dependencies = [ "cmd_lib", "curve25519-dalek", "dcap-qvl", + "dstack-api-auth", "dstack-attest", "dstack-gateway-rpc", "dstack-kms-rpc", diff --git a/dstack/crates/api-auth/Cargo.toml b/dstack/crates/api-auth/Cargo.toml index c3df2919d..47967d4f9 100644 --- a/dstack/crates/api-auth/Cargo.toml +++ b/dstack/crates/api-auth/Cargo.toml @@ -13,6 +13,9 @@ license.workspace = true anyhow.workspace = true base64.workspace = true bcrypt.workspace = true +ed25519-dalek.workspace = true rocket.workspace = true +serde.workspace = true +serde_json = { workspace = true, features = ["std"] } sha2.workspace = true subtle.workspace = true diff --git a/dstack/crates/api-auth/src/jwt.rs b/dstack/crates/api-auth/src/jwt.rs new file mode 100644 index 000000000..361e868c8 --- /dev/null +++ b/dstack/crates/api-auth/src/jwt.rs @@ -0,0 +1,388 @@ +// SPDX-FileCopyrightText: © 2026 Phala Network +// +// SPDX-License-Identifier: Apache-2.0 + +//! Ed25519 JWT tokens for the guest agent's external API. +//! +//! Deliberately a small subset: one algorithm, a fixed claim set, and no +//! key-discovery header. Everything a general-purpose JWT library would let a +//! caller negotiate is something an attacker would like to negotiate too -- +//! `none`, an HMAC verified with a public key as the secret, or a `jku`/`x5u`/ +//! `jwk` header naming a key the verifier never configured. Here those are not +//! rejected by policy but impossible to express. + +use std::collections::BTreeSet; + +use anyhow::{bail, Context, Result}; +use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _}; +use ed25519_dalek::{Signature, Signer, SigningKey, VerifyingKey}; +use serde::{Deserialize, Serialize}; + +/// Longest lifetime a token may claim between `iat` and `exp`. +/// +/// There is no revocation list: a token is trusted until it expires, so the +/// lifetime is the whole of the exposure control. The gateway re-registers +/// every three minutes and mints a fresh token each time, so this costs it +/// nothing. +pub const MAX_TOKEN_LIFETIME_SECS: u64 = 3600; + +/// Tolerance for clock skew in either direction. +pub const CLOCK_LEEWAY_SECS: u64 = 30; + +/// The base64url header every token carries. +/// +/// A constant, not a serialized struct: a header this module writes is a +/// header it can also verify exactly. +const HEADER: &str = "eyJhbGciOiJFZERTQSIsInR5cCI6IkpXVCJ9"; + +/// The scope names the guest agent understands. +/// +/// A vocabulary rather than free-form strings, so a typo in `anonymous_scopes` +/// is a startup error instead of a permission that silently never matches. +pub mod scope { + /// `Worker.Version` on either pRPC surface. + pub const VERSION: &str = "guest:version"; + /// `Worker.Health` on the v1 surface. + pub const HEALTH: &str = "guest:health"; + /// `Worker.Info` on either pRPC surface. + pub const INFO: &str = "guest:info"; + /// `Worker.GetAttestationForAppKey` on the frozen surface. + pub const ATTEST_APP_KEY: &str = "guest:attest-app-key"; + /// The HTML dashboard itself. + pub const DASHBOARD: &str = "guest:dashboard"; + /// The container table on the dashboard. + pub const CONTAINERS: &str = "guest:containers"; + /// `/logs/`. + pub const LOGS: &str = "guest:logs"; + /// `/metrics`, and the sysinfo block on the dashboard. + pub const METRICS: &str = "guest:metrics"; + + /// Every name above, in the order a dashboard shows them. + pub const ALL: &[&str] = &[ + VERSION, + HEALTH, + INFO, + ATTEST_APP_KEY, + DASHBOARD, + CONTAINERS, + LOGS, + METRICS, + ]; + + pub fn is_known(name: &str) -> bool { + ALL.contains(&name) + } +} + +/// A set of granted scopes. +/// +/// Space-separated on the wire, because that is what RFC 8693 and every +/// OAuth-adjacent tool produces, and a collection here because the only +/// operation is membership. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct ScopeSet(BTreeSet); + +impl ScopeSet { + /// Parse the space-separated `scope` claim. + pub fn parse(scope: &str) -> Self { + Self(scope.split_whitespace().map(str::to_owned).collect()) + } + + pub fn from_names(names: impl IntoIterator>) -> Self { + Self(names.into_iter().map(Into::into).collect()) + } + + pub fn contains(&self, scope: &str) -> bool { + self.0.contains(scope) + } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + /// Everything in both sets. Used to add the anonymous baseline to a + /// caller's own grants. + pub fn union(&self, other: &Self) -> Self { + Self(self.0.union(&other.0).cloned().collect()) + } +} + +/// The claims this module issues and accepts. +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct Claims { + /// Who minted the token. + pub iss: String, + /// Which application the token is for, hex-encoded. + pub aud: String, + pub iat: u64, + pub exp: u64, + /// Space-separated [`scope`] names. + #[serde(default, skip_serializing_if = "String::is_empty")] + pub scope: String, +} + +impl Claims { + pub fn scopes(&self) -> ScopeSet { + ScopeSet::parse(&self.scope) + } +} + +/// The domain the internal issuer key is derived under. +/// +/// Part of the wire contract between the agent (which derives the public half +/// to verify with) and the application (which derives the private half to +/// sign with). Changing it invalidates every token in flight, and a host and +/// guest on either side of the change would stop trusting each other, so it +/// never changes. +pub const INTERNAL_ISSUER_DOMAIN: &str = "dstack:guest-api:issuer:v1"; + +/// The `iss` an internally minted token carries. +/// +/// Bound to the app id so a token minted for one application cannot be +/// replayed against another: `aud` alone would already stop that, but naming +/// the application in the issuer too keeps the two checks independent. +pub fn internal_issuer(app_id_hex: &str) -> String { + format!("dstack:guest-api:{app_id_hex}") +} + +/// Mint a token. `signing_key` is the raw 32-byte Ed25519 seed. +pub fn sign(signing_key: &[u8; 32], claims: &Claims) -> Result { + let payload = serde_json::to_vec(claims).context("failed to serialize claims")?; + let signing_input = format!("{HEADER}.{}", URL_SAFE_NO_PAD.encode(payload)); + let signature = SigningKey::from_bytes(signing_key).sign(signing_input.as_bytes()); + Ok(format!( + "{signing_input}.{}", + URL_SAFE_NO_PAD.encode(signature.to_bytes()) + )) +} + +/// The public keys one issuer may sign with. +/// +/// A list so a key can be rotated without a flag day: publish the new key +/// alongside the old, wait out [`MAX_TOKEN_LIFETIME_SECS`], drop the old one. +#[derive(Clone, Debug)] +pub struct IssuerKeys { + /// The exact `iss` this issuer's tokens must carry. + pub name: String, + pub keys: Vec, +} + +/// Verifies tokens and reports what they grant. +#[derive(Clone)] +pub struct Verifier { + internal: Option, + external: Vec, +} + +impl Verifier { + /// `internal` is absent only when the app root key could not produce one; + /// an agent in that state still verifies external issuers. + pub fn new(internal: Option, external: Vec) -> Self { + Self { internal, external } + } + + pub fn is_empty(&self) -> bool { + self.internal.is_none() && self.external.is_empty() + } + + /// Verify `token` for the application whose id is `app_id_hex`. + /// + /// `now` is seconds since the UNIX epoch, passed in rather than read here + /// so the expiry rules can be tested. + pub fn verify(&self, token: &str, app_id_hex: &str, now: u64) -> Result { + let mut parts = token.split('.'); + let (Some(header), Some(encoded_payload), Some(signature), None) = + (parts.next(), parts.next(), parts.next(), parts.next()) + else { + bail!("token is not a JWT"); + }; + if header != HEADER { + // Covers `alg: none`, HMAC, and any header carrying key material. + bail!("unsupported JWT header"); + } + let payload = URL_SAFE_NO_PAD + .decode(encoded_payload) + .context("payload is not base64url")?; + let claims: Claims = serde_json::from_slice(&payload).context("malformed claims")?; + if claims.aud != app_id_hex { + bail!("token was issued for a different application"); + } + if claims.exp < now.saturating_sub(CLOCK_LEEWAY_SECS) { + bail!("token has expired"); + } + if claims.exp.saturating_sub(claims.iat) > MAX_TOKEN_LIFETIME_SECS { + bail!("token lifetime exceeds {MAX_TOKEN_LIFETIME_SECS}s"); + } + if claims.iat > now + CLOCK_LEEWAY_SECS { + bail!("token was issued in the future"); + } + let signature = URL_SAFE_NO_PAD + .decode(signature) + .context("signature is not base64url")?; + let signature = + Signature::from_slice(&signature).context("signature has the wrong length")?; + let signing_input = format!("{header}.{encoded_payload}"); + + let keys = self.keys_for(&claims.iss, app_id_hex); + if keys.is_empty() { + bail!("unknown token issuer"); + } + if !keys.iter().any(|key| { + key.verify_strict(signing_input.as_bytes(), &signature) + .is_ok() + }) { + bail!("token signature does not verify"); + } + Ok(claims.scopes()) + } + + fn keys_for(&self, iss: &str, app_id_hex: &str) -> Vec<&VerifyingKey> { + if iss == internal_issuer(app_id_hex) { + return self.internal.iter().collect(); + } + self.external + .iter() + .find(|issuer| issuer.name == iss) + .map(|issuer| issuer.keys.iter().collect()) + .unwrap_or_default() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const APP_ID: &str = "00112233445566778899aabbccddeeff00112233"; + + fn signing_key(seed: u8) -> [u8; 32] { + [seed; 32] + } + + fn mint(seed: u8, iss: &str, aud: &str, iat: u64, exp: u64, scope: &str) -> String { + sign( + &signing_key(seed), + &Claims { + iss: iss.to_owned(), + aud: aud.to_owned(), + iat, + exp, + scope: scope.to_owned(), + }, + ) + .unwrap() + } + + fn verifier(seed: u8) -> Verifier { + Verifier::new( + Some(SigningKey::from_bytes(&signing_key(seed)).verifying_key()), + vec![], + ) + } + + #[test] + fn round_trips_scopes() { + let token = mint( + 1, + &internal_issuer(APP_ID), + APP_ID, + 1000, + 1300, + "guest:info guest:logs", + ); + let scopes = verifier(1).verify(&token, APP_ID, 1100).unwrap(); + assert!(scopes.contains(scope::INFO)); + assert!(scopes.contains(scope::LOGS)); + assert!(!scopes.contains(scope::METRICS)); + } + + #[test] + fn rejects_a_token_for_another_application() { + let token = mint( + 1, + &internal_issuer(APP_ID), + APP_ID, + 1000, + 1300, + "guest:info", + ); + let other = "ffeeddccbbaa99887766554433221100ffeeddcc"; + assert!(verifier(1).verify(&token, other, 1100).is_err()); + } + + #[test] + fn rejects_alg_none_and_foreign_signatures() { + for token in [ + "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0.eyJpc3MiOiJ4In0.".to_owned(), + mint( + 2, + &internal_issuer(APP_ID), + APP_ID, + 1000, + 1300, + "guest:info", + ), + ] { + assert!(verifier(1).verify(&token, APP_ID, 1100).is_err()); + } + } + + #[test] + fn rejects_expired_overlong_and_future_tokens() { + let expired = mint( + 1, + &internal_issuer(APP_ID), + APP_ID, + 1000, + 1100, + "guest:info", + ); + assert!(verifier(1).verify(&expired, APP_ID, 1200).is_err()); + + let overlong = mint( + 1, + &internal_issuer(APP_ID), + APP_ID, + 1000, + 1000 + 7200, + "guest:info", + ); + assert!(verifier(1).verify(&overlong, APP_ID, 1100).is_err()); + + let future = mint( + 1, + &internal_issuer(APP_ID), + APP_ID, + 5000, + 5300, + "guest:info", + ); + assert!(verifier(1).verify(&future, APP_ID, 1000).is_err()); + } + + #[test] + fn accepts_an_external_issuer_by_name() { + let verifier = Verifier::new( + None, + vec![IssuerKeys { + name: "ops".into(), + keys: vec![SigningKey::from_bytes(&signing_key(9)).verifying_key()], + }], + ); + let token = mint(9, "ops", APP_ID, 1000, 1300, "guest:containers"); + assert!(verifier + .verify(&token, APP_ID, 1100) + .unwrap() + .contains(scope::CONTAINERS)); + + // The same key under the internal issuer name is not an internal token. + let impostor = mint( + 9, + &internal_issuer(APP_ID), + APP_ID, + 1000, + 1300, + "guest:info", + ); + assert!(verifier.verify(&impostor, APP_ID, 1100).is_err()); + } +} diff --git a/dstack/crates/api-auth/src/lib.rs b/dstack/crates/api-auth/src/lib.rs index c52f7bcb3..3932de22a 100644 --- a/dstack/crates/api-auth/src/lib.rs +++ b/dstack/crates/api-auth/src/lib.rs @@ -5,8 +5,10 @@ //! Shared HTTP API authentication for dstack services. //! //! Supports bearer/shared-secret tokens, HTTP Basic credentials backed by an -//! Apache htpasswd file, and an optional GET-only query token for compatibility -//! with browser dashboard links. +//! Apache htpasswd file, an optional GET-only query token for compatibility +//! with browser dashboard links, and Ed25519 JWT scope grants (see [`jwt`]). + +pub mod jwt; use std::{collections::HashMap, path::Path, sync::Arc}; @@ -21,7 +23,9 @@ use rocket::{ use sha2::{Digest, Sha256}; use subtle::{Choice, ConstantTimeEq}; -const UNAUTH_URI: &str = "/__dstack_api_auth_unauthorized"; +/// Where a denied request is rewritten to, so every refusal is answered by one +/// responder regardless of which route it was headed for. +pub const UNAUTHORIZED_PATH: &str = "/__dstack_api_auth_unauthorized"; #[derive(Debug)] struct Htpasswd { @@ -252,11 +256,11 @@ impl Fairing for HttpAuthFairing { async fn on_request(&self, req: &mut Request<'_>, _: &mut Data<'_>) { req.local_cache(|| RequestRealm(self.config.realm.clone())); - if req.uri().path() == UNAUTH_URI { + if req.uri().path() == UNAUTHORIZED_PATH { return; } if !self.authorized(req) { - if let Ok(uri) = Origin::parse_owned(UNAUTH_URI.to_owned()) { + if let Ok(uri) = Origin::parse_owned(UNAUTHORIZED_PATH.to_owned()) { req.set_uri(uri); } } else if self.config.allow_get_query_token { @@ -267,22 +271,37 @@ impl Fairing for HttpAuthFairing { } } -struct Unauthorized; +/// What a 401 asks the caller for. +/// +/// A Basic challenge tells a bearer-token client to do something it cannot do, +/// so a fairing denying on a token denies with `Bearer` instead. +#[derive(Clone, Copy, Default, PartialEq, Eq)] +pub enum Challenge { + #[default] + Basic, + Bearer, +} + +/// The responder every denied request lands on. +pub struct Unauthorized; impl<'r> Responder<'r, 'static> for Unauthorized { fn respond_to(self, req: &'r Request<'_>) -> rocket::response::Result<'static> { - let realm = &req.local_cache(|| RequestRealm("dstack API".into())).0; + let value = match req.local_cache(Challenge::default) { + Challenge::Basic => { + let realm = &req.local_cache(|| RequestRealm("dstack API".into())).0; + format!("Basic realm=\"{realm}\"") + } + Challenge::Bearer => "Bearer".to_owned(), + }; Response::build() .status(Status::Unauthorized) - .header(Header::new( - "WWW-Authenticate", - format!("Basic realm=\"{realm}\""), - )) + .header(Header::new("WWW-Authenticate", value)) .ok() } } -struct RequestRealm(String); +pub(crate) struct RequestRealm(String); #[rocket::get("/__dstack_api_auth_unauthorized")] fn unauth_get() -> Unauthorized { diff --git a/dstack/dstack-types/src/lib.rs b/dstack/dstack-types/src/lib.rs index a0ce437e2..ccf2ed02f 100644 --- a/dstack/dstack-types/src/lib.rs +++ b/dstack/dstack-types/src/lib.rs @@ -1254,10 +1254,44 @@ pub struct SysConfig { /// string. #[serde(default, skip_serializing_if = "Option::is_none")] pub mr_config: Option, + /// Access control for the guest agent's external listener. Absent means + /// the code defaults: nothing anonymous, no external issuers. + /// + /// Host-supplied, and deliberately not measured: it decides who may *ask* + /// the agent, never whether data may leave the TEE. Confidentiality stays + /// with the measured `public_logs`/`public_sysinfo`/`public_tcbinfo` + /// switches in app-compose, which a token here cannot widen. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub guest_agent_api: Option, // JSON serialized VmConfig pub vm_config: String, } +#[derive(Deserialize, Serialize, Debug, Clone, Default, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub struct GuestAgentApiConfig { + /// Scopes granted to a caller that presents no token. + /// + /// Empty by default: an app that wants its dashboard on the internet has + /// to say so. This is an exposure choice, not a trust one -- the anonymous + /// caller is the same party a public 8090 already served. + #[serde(default)] + pub anonymous_scopes: Vec, + /// Issuers other than the app's own root key. + #[serde(default)] + pub external_issuers: Vec, +} + +#[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub struct ExternalJwtIssuer { + /// The exact `iss` claim tokens from this issuer must carry. + pub name: String, + /// Hex-encoded Ed25519 public keys. More than one so a key can be rotated + /// without a flag day. + pub public_keys: Vec, +} + #[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)] #[serde(deny_unknown_fields)] pub struct GatewayClusterConfig { diff --git a/dstack/dstack-util/Cargo.toml b/dstack/dstack-util/Cargo.toml index a38f02f41..1f1f747a0 100644 --- a/dstack/dstack-util/Cargo.toml +++ b/dstack/dstack-util/Cargo.toml @@ -47,6 +47,7 @@ cmd_lib.workspace = true toml.workspace = true dcap-qvl.workspace = true k256 = { workspace = true, features = ["ecdsa"] } +dstack-api-auth.workspace = true dstack-types.workspace = true rand.workspace = true regorus.workspace = true diff --git a/dstack/dstack-util/src/system_setup.rs b/dstack/dstack-util/src/system_setup.rs index 114e8d37e..bc2b22759 100644 --- a/dstack/dstack-util/src/system_setup.rs +++ b/dstack/dstack-util/src/system_setup.rs @@ -56,6 +56,7 @@ use crate::{ }; use cert_client::{validate_kms_rpc_cert, CertRequestClient}; use cmd_lib::run_fun as cmd; +use dstack_api_auth::jwt; use dstack_gateway_rpc::{ gateway_client::GatewayClient, PortAttrs as RpcPortAttrs, PortPolicy as RpcPortPolicy, RegisterCvmRequest, RegisterCvmResponse, WireGuardPeer, @@ -493,6 +494,50 @@ struct GatewayContext<'a> { keys: &'a AppKeys, } +/// Scopes the gateway needs on the guest agent's external listener: the v1 +/// `Health` poll, and the frozen `Info` call that fills in a port policy the +/// CVM did not report. Nothing else -- the gateway has no business on the +/// dashboard or in the logs. +const GATEWAY_AGENT_SCOPES: &[&str] = &[jwt::scope::HEALTH, jwt::scope::INFO]; + +/// Mint the token the gateway presents to this CVM's guest agent. +/// +/// Derived from the app root key, so it needs no distribution: the agent holds +/// the public half, this holds the private one, and the gateway only carries +/// the result. `None` when either side is not ready yet -- an app id that only +/// the boot attestation can supply, or an app root key that does not parse -- +/// in which case registration carries no token and the agent answers +/// anonymously, exactly as it did before this existed. +/// +/// One token for both calls the gateway makes, because both are the gateway +/// doing its own job: the v1 `Health` poll, and the frozen `Info` that fills in +/// a port policy the CVM did not report. Scopes are still a whitelist -- the +/// gateway gets nothing on the dashboard or in the logs. +fn gateway_agent_token(app_id: &[u8], app_root_key: &[u8]) -> Option { + let secret = ra_tls::api_v1::derive_app_key( + app_root_key, + jwt::INTERNAL_ISSUER_DOMAIN, + ra_tls::api_v1::KeyAlgorithm::Ed25519, + ) + .ok()?; + let app_id = hex::encode(app_id); + // Long enough to survive a few failed refreshes: a token that expired + // during a gateway outage would turn "we could not re-register" into "we + // are locked out until the outage ends". + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .ok()? + .as_secs(); + let claims = jwt::Claims { + iss: jwt::internal_issuer(&app_id), + aud: app_id, + iat: now, + exp: now + 600, + scope: GATEWAY_AGENT_SCOPES.join(" "), + }; + jwt::sign(&secret, &claims).ok() +} + impl<'a> GatewayContext<'a> { fn new(shared: &'a HostShared, keys: &'a AppKeys) -> Self { Self { shared, keys } @@ -532,6 +577,14 @@ impl<'a> GatewayContext<'a> { key_store: &GatewayKeyStore, gateway_app_id: &str, ) -> Result { + let agent_token = + gateway_agent_token(&self.shared.instance_info.app_id, &self.keys.k256_key); + if agent_token.is_none() { + // Not fatal: the CVM registers without one and the agent answers + // however the host configured it. Say so once per registration + // rather than failing a boot over it. + info!("no guest agent token available; registering without one"); + } let port_policy = RpcPortPolicy { ports: self .shared @@ -558,6 +611,7 @@ impl<'a> GatewayContext<'a> { client_public_key: key_store.wg_pk.clone(), port_policy: Some(port_policy.clone()), health_check, + agent_token: agent_token.clone(), }) .await .context("Failed to register CVM"); @@ -581,6 +635,7 @@ impl<'a> GatewayContext<'a> { client_public_key: key_store.wg_pk.clone(), port_policy: Some(port_policy), health_check, + agent_token, }) .await .context("Failed to register CVM") diff --git a/dstack/gateway/rpc/proto/gateway_rpc.proto b/dstack/gateway/rpc/proto/gateway_rpc.proto index 6a23982ca..6a889673f 100644 --- a/dstack/gateway/rpc/proto/gateway_rpc.proto +++ b/dstack/gateway/rpc/proto/gateway_rpc.proto @@ -31,6 +31,16 @@ message RegisterCvmRequest { // gateway falls back to fetching app-compose via Info()) from "reported // empty" (new CVM with no special port behaviour). optional PortPolicy port_policy = 2; + // Ed25519 JWT the CVM minted for its own guest agent, so the gateway can + // call the agent's external listener now that it refuses anonymous callers + // by default. Bound to this CVM's app id and expiring within the hour; the + // CVM re-registers every three minutes, so a stale copy never outlives its + // usefulness. + // + // Empty from a CVM that predates the field, or one whose app identity is + // not known yet. The gateway then calls without a token and gets whatever + // the host left open -- possibly nothing, which is the intended default. + optional string agent_token = 4; } // PortPolicy carries the gateway-relevant per-port configuration declared by diff --git a/dstack/gateway/src/kv/import.rs b/dstack/gateway/src/kv/import.rs index 593715379..f18d62bc2 100644 --- a/dstack/gateway/src/kv/import.rs +++ b/dstack/gateway/src/kv/import.rs @@ -282,6 +282,7 @@ mod tests { port_policy: None, port_policy_hash: String::new(), health_check: None, + agent_token: None, } } diff --git a/dstack/gateway/src/kv/mod.rs b/dstack/gateway/src/kv/mod.rs index 81486a859..5238d3676 100644 --- a/dstack/gateway/src/kv/mod.rs +++ b/dstack/gateway/src/kv/mod.rs @@ -130,6 +130,16 @@ pub struct InstanceRecord { /// a rollback and forward again. #[serde(default)] pub health_check: Option, + /// Token this CVM minted for the gateway, so the agent's external listener + /// can answer an operator-run caller. `None` for a CVM that predates the + /// field, or one whose app id is not yet known. + /// + /// Per-instance rather than per-app: the token is bound to the instance's + /// app id and expires within the hour, and the CVM re-registers every + /// three minutes anyway. `Option` for the same reason as `health_check`: + /// a pre-field record has to read as "not known", not "empty token". + #[serde(default)] + pub agent_token: Option, } /// What an operator has said about an instance's ports, under @@ -193,6 +203,7 @@ impl From<&InstanceInfo> for InstanceRecord { // `Health` is this node's own observation, and a shared one would // outlive the instance that earned it. health_check: Some(info.health_check()), + agent_token: info.agent_token.clone(), } } } @@ -2729,6 +2740,7 @@ mod value_encoding_tests { port_policy: None, port_policy_hash: String::new(), health_check: None, + agent_token: None, }, ) .expect("sync should succeed"); @@ -3003,6 +3015,7 @@ mod corruption_tests { port_policy: None, port_policy_hash: String::new(), health_check: None, + agent_token: None, }, ) .expect("seed"); @@ -3473,6 +3486,7 @@ mod corruption_tests { port_policy: None, port_policy_hash: String::new(), health_check: None, + agent_token: None, }, ) .expect("sync should succeed"); @@ -3659,6 +3673,7 @@ mod tombstone_gc_tests { port_policy: None, port_policy_hash: String::new(), health_check: None, + agent_token: None, }, ) .expect("seed"); diff --git a/dstack/gateway/src/main_service.rs b/dstack/gateway/src/main_service.rs index 70cd0cec2..5c77116c7 100644 --- a/dstack/gateway/src/main_service.rs +++ b/dstack/gateway/src/main_service.rs @@ -1053,6 +1053,7 @@ fn instance_info_from_record( // cached selection on a record that never claimed the app opted out. health: data.health_check.unwrap_or(known_gated).into(), connections: Default::default(), + agent_token: data.agent_token, }; (info, unreadable) } @@ -1968,6 +1969,7 @@ impl ProxyState { let ReportedCapabilities { port_policy, health_check, + agent_token, } = reported; if id.is_empty() { bail!("instance_id is empty (no_instance_id is set?)"); @@ -2064,6 +2066,11 @@ impl ProxyState { if port_policy.is_some() { existing.port_policy = port_policy.clone(); } + // Same rule: a registration that supplies no token must not erase + // one, and one that does replaces it wholesale. + if agent_token.is_some() { + existing.agent_token = agent_token.clone(); + } existing.clone() }); if let Some(existing) = edited { @@ -2115,6 +2122,7 @@ impl ProxyState { ready: None, health: health_check.unwrap_or(false).into(), connections: Default::default(), + agent_token, }, }; self.add_instance(host_info.clone()); @@ -2126,6 +2134,15 @@ impl ProxyState { self.state.instances.get(instance_id).map(|i| i.ip) } + /// The bearer token an instance's guest agent external listener accepts, + /// if the CVM supplied one at registration. + pub(crate) fn instance_agent_token(&self, instance_id: &str) -> Option { + self.state + .instances + .get(instance_id) + .and_then(|info| info.agent_token.clone()) + } + /// Lookup the effective port_policy for an instance: admin override wins, /// otherwise fall back to the instance-reported policy. `None` means /// neither is set — caller fails closed and may schedule a lazy fetch. @@ -2819,6 +2836,7 @@ impl GatewayRpc for RpcHandler { port_policy, // A real CVM always states its intent, either way. health_check: Some(request.health_check), + agent_token: request.agent_token.filter(|token| !token.is_empty()), }, ) .await diff --git a/dstack/gateway/src/main_service/snapshots/dstack_gateway__main_service__tests__config-2.snap b/dstack/gateway/src/main_service/snapshots/dstack_gateway__main_service__tests__config-2.snap index f126f03ee..eebb0353e 100644 --- a/dstack/gateway/src/main_service/snapshots/dstack_gateway__main_service__tests__config-2.snap +++ b/dstack/gateway/src/main_service/snapshots/dstack_gateway__main_service__tests__config-2.snap @@ -1,6 +1,6 @@ --- source: gateway/src/main_service/tests.rs -assertion_line: 71 +assertion_line: 86 expression: info1 --- InstanceInfo { @@ -20,5 +20,6 @@ InstanceInfo { gated: false, state: Ungated, }, + agent_token: None, connections: 0, } diff --git a/dstack/gateway/src/main_service/snapshots/dstack_gateway__main_service__tests__config.snap b/dstack/gateway/src/main_service/snapshots/dstack_gateway__main_service__tests__config.snap index 221032272..9aaa3dcd4 100644 --- a/dstack/gateway/src/main_service/snapshots/dstack_gateway__main_service__tests__config.snap +++ b/dstack/gateway/src/main_service/snapshots/dstack_gateway__main_service__tests__config.snap @@ -20,5 +20,6 @@ InstanceInfo { gated: false, state: Ungated, }, + agent_token: None, connections: 0, } diff --git a/dstack/gateway/src/main_service/tests.rs b/dstack/gateway/src/main_service/tests.rs index d2f6efc5e..e33fd0749 100644 --- a/dstack/gateway/src/main_service/tests.rs +++ b/dstack/gateway/src/main_service/tests.rs @@ -535,6 +535,7 @@ fn register_instances(proxy: &mut ProxyState, app: &str, count: usize, health_ch ReportedCapabilities { port_policy: Some(policy(false, &[])), health_check: Some(health_check), + agent_token: None, }, ) .unwrap(); @@ -556,6 +557,7 @@ fn observe(proxy: &mut ProxyState, instance_id: &str, state: HealthState) { id: info.id, ip: info.ip, public_key: info.public_key, + agent_token: info.agent_token, }, state, ); @@ -827,6 +829,7 @@ async fn a_published_record_carries_the_declaration_and_not_the_gate() { ReportedCapabilities { port_policy: Some(policy(false, &[])), health_check: Some(true), + agent_token: None, }, ) .unwrap(); @@ -1051,6 +1054,7 @@ async fn both_operator_overrides_survive_the_ip_rebuild_path() { ReportedCapabilities { port_policy: Some(policy(false, &[])), health_check: Some(false), + agent_token: None, }, ) .unwrap(); @@ -1189,6 +1193,7 @@ async fn a_capability_change_arriving_through_kv_invalidates_the_cached_selectio port_policy: existing.port_policy.clone(), port_policy_hash: existing.port_policy_hash.clone(), health_check: Some(true), + agent_token: None, } }; state @@ -1529,6 +1534,7 @@ fn re_register( ReportedCapabilities { port_policy: Some(policy(false, &[])), health_check: Some(health_check), + agent_token: None, }, ) .unwrap(); @@ -1715,6 +1721,7 @@ async fn a_reboot_arriving_through_kv_resets_health_on_this_node_too() { port_policy: existing.port_policy.clone(), port_policy_hash: existing.port_policy_hash.clone(), health_check: Some(true), + agent_token: None, } }; state @@ -1761,6 +1768,7 @@ async fn a_record_written_without_the_capability_field_does_not_clear_it() { port_policy_hash: existing.port_policy_hash.clone(), // The older build does not know the field exists. health_check: None, + agent_token: None, } }; state @@ -1852,6 +1860,7 @@ async fn a_verdict_polled_before_a_reboot_is_not_applied_after_it() { id: "race-app-0".to_string(), ip: proxy.state.instances["race-app-0"].ip, public_key: proxy.state.instances["race-app-0"].public_key.clone(), + agent_token: None, }; // The CVM reboots and re-registers mid-round. @@ -1884,6 +1893,7 @@ async fn a_verdict_polled_from_a_stale_address_is_not_applied() { id: "addr-app-0".to_string(), ip: "10.99.99.99".parse().unwrap(), public_key: proxy.state.instances["addr-app-0"].public_key.clone(), + agent_token: None, }; observe_from(&mut proxy, &polled, HealthState::Healthy); @@ -1913,6 +1923,7 @@ async fn a_registration_that_reports_no_capability_leaves_it_alone() { ReportedCapabilities { port_policy: Some(policy(false, &[])), health_check: None, + agent_token: None, }, ) .unwrap(); @@ -1979,6 +1990,7 @@ async fn disabling_polling_keeps_both_old_and_new_instances_eligible() { ReportedCapabilities { port_policy: Some(policy(false, &[])), health_check: Some(has_endpoint), + agent_token: None, }, ) .unwrap(); @@ -2135,6 +2147,7 @@ fn sync_from_peer_at( port_policy: None, port_policy_hash: String::new(), health_check: Some(false), + agent_token: None, }, ) .unwrap(); diff --git a/dstack/gateway/src/models.rs b/dstack/gateway/src/models.rs index b41a7ba93..abaa9663a 100644 --- a/dstack/gateway/src/models.rs +++ b/dstack/gateway/src/models.rs @@ -72,6 +72,10 @@ pub struct InstanceInfo { /// [`Health`]'s own state is not, so there is no way to build one whose /// declaration and verdict disagree. pub(crate) health: Health, + /// Bearer token for the instance's guest agent external listener, minted + /// by the CVM at registration. Absent for a legacy CVM, which has no such + /// listener to authenticate against. + pub agent_token: Option, pub connections: Arc, } @@ -93,6 +97,11 @@ pub struct ReportedCapabilities { /// instance already declared in place. `Some(false)` is an app that did not /// opt in, or an image that predates the field; the gateway polls neither. pub health_check: Option, + /// Bearer token for the agent's external listener. `None` leaves whatever + /// the instance already carried in place, so a registration without one + /// (a legacy CVM, or one that could not mint one yet) does not erase a + /// token a previous registration supplied. + pub agent_token: Option, } impl From> for ReportedCapabilities { @@ -102,6 +111,7 @@ impl From> for ReportedCapabilities { Self { port_policy, health_check: None, + agent_token: None, } } } diff --git a/dstack/gateway/src/proxy/health_check.rs b/dstack/gateway/src/proxy/health_check.rs index c0ea83c7b..754e3c50f 100644 --- a/dstack/gateway/src/proxy/health_check.rs +++ b/dstack/gateway/src/proxy/health_check.rs @@ -95,6 +95,10 @@ pub(crate) struct Target { pub id: String, pub ip: Ipv4Addr, pub public_key: String, + /// The token the CVM supplied at registration. `None` for a CVM that + /// predates the field; the poll then carries no credentials and gets + /// whatever the host left anonymous. + pub agent_token: Option, } /// Start the polling loop. Called once at startup. @@ -161,7 +165,7 @@ async fn poll_round( let timeout = config.timeout; let mut polls = futures::stream::iter(targets) .map(|target| async move { - let result = poll_instance(target.ip, agent_port, timeout).await; + let result = poll_instance(&target, agent_port, timeout).await; (target, result) }) .buffer_unordered(config.concurrency.max(1)); @@ -243,6 +247,7 @@ pub(crate) fn select_targets(state: &Proxy) -> Vec { id: instance.id.clone(), ip: instance.ip, public_key: instance.public_key.clone(), + agent_token: instance.agent_token.clone(), }) .collect() } @@ -300,8 +305,8 @@ fn apply_hysteresis( /// A failure to get an answer comes back as [`PollResult::Unreachable`] rather /// than as a verdict; [`apply_hysteresis`] decides when enough of them in a row /// amount to one. -async fn poll_instance(ip: Ipv4Addr, agent_port: u16, timeout: Duration) -> PollResult { - let client = WorkerV1Client::new(prober_transport(ip, agent_port)); +async fn poll_instance(target: &Target, agent_port: u16, timeout: Duration) -> PollResult { + let client = WorkerV1Client::new(prober_transport(target, agent_port)); let response = match tokio::time::timeout(timeout, client.health(Default::default())).await { Err(_) => { return PollResult::unreachable(format!("health poll timed out after {timeout:?}")) @@ -331,7 +336,7 @@ async fn poll_instance(ip: Ipv4Addr, agent_port: u16, timeout: Duration) -> Poll /// one. A probe riding a pooled connection would report healthy for an instance /// no real request can reach. Only the connection is unshared; the client /// itself is process-wide. See `http_client::ConnectionReuse`. -fn prober_transport(ip: Ipv4Addr, agent_port: u16) -> http_client::prpc::PrpcClient { +fn prober_transport(target: &Target, agent_port: u16) -> http_client::prpc::PrpcClient { // `/prpc/v1`, not `/prpc`: `Health` is a v1 `Worker` method. // // No released agent is affected. `Health` never shipped in a release, and a @@ -345,8 +350,12 @@ fn prober_transport(ip: Ipv4Addr, agent_port: u16) -> http_client::prpc::PrpcCli // Instance-id routing keeps working and a restart on a current build fixes // it. Not worth a compatibility probe on every poll for a build nobody was // asked to run. - let url = format!("http://{ip}:{agent_port}/prpc/v1"); - super::guest_agent_client(url, ConnectionReuse::Fresh) + let url = format!("http://{}:{agent_port}/prpc/v1", target.ip); + let client = super::guest_agent_client(url, ConnectionReuse::Fresh); + match &target.agent_token { + Some(token) => client.with_bearer_token(token), + None => client, + } } /// Longest reason kept from an agent's answer. @@ -512,7 +521,7 @@ mod tests { let (tx, rx) = tokio::sync::oneshot::channel(); let port = fake_agent_capturing(Some(br#"{"healthy":true}"#.to_vec()), Some(tx)).await; - let _ = poll_instance(Ipv4Addr::LOCALHOST, port, Duration::from_secs(5)).await; + let _ = poll_instance(&local_target(), port, Duration::from_secs(5)).await; let request_line = rx.await.expect("the poller sent no request"); assert!( @@ -523,7 +532,7 @@ mod tests { async fn poll_fake(answer: Option>) -> PollResult { let port = fake_agent(answer).await; - poll_instance(Ipv4Addr::LOCALHOST, port, Duration::from_secs(5)).await + poll_instance(&local_target(), port, Duration::from_secs(5)).await } #[tokio::test] @@ -553,7 +562,7 @@ mod tests { #[tokio::test] async fn an_agent_that_accepts_and_then_says_nothing_times_out() { let port = fake_agent(None).await; - let result = poll_instance(Ipv4Addr::LOCALHOST, port, Duration::from_millis(100)).await; + let result = poll_instance(&local_target(), port, Duration::from_millis(100)).await; let PollResult::Unreachable(reason) = result else { panic!("a silent agent must not produce a verdict, got {result:?}"); }; @@ -569,7 +578,7 @@ mod tests { .expect("bind"); listener.local_addr().expect("addr").port() }; - let result = poll_instance(Ipv4Addr::LOCALHOST, port, Duration::from_secs(5)).await; + let result = poll_instance(&local_target(), port, Duration::from_secs(5)).await; assert!( matches!(result, PollResult::Unreachable(_)), "a refused connection must not read as an agent saying no, got {result:?}" @@ -635,7 +644,7 @@ mod tests { #[test] fn the_probe_does_not_ride_a_connection_someone_else_opened() { assert_eq!( - prober_transport(Ipv4Addr::new(10, 0, 0, 2), 8090).connection_reuse(), + prober_transport(&target(), 8090).connection_reuse(), ConnectionReuse::Fresh, "a probe over a pooled connection cannot see an agent that refuses new ones" ); @@ -656,6 +665,14 @@ mod tests { id: "inst-1".to_string(), ip: Ipv4Addr::new(10, 0, 0, 2), public_key: "key-1".to_string(), + agent_token: None, + } + } + + fn local_target() -> Target { + Target { + ip: Ipv4Addr::LOCALHOST, + ..target() } } diff --git a/dstack/gateway/src/proxy/port_policy.rs b/dstack/gateway/src/proxy/port_policy.rs index bc8ce05d1..a8573afdb 100644 --- a/dstack/gateway/src/proxy/port_policy.rs +++ b/dstack/gateway/src/proxy/port_policy.rs @@ -217,28 +217,41 @@ async fn fetch_with_retry(state: &Proxy, instance_id: &str) { } async fn fetch_and_store(state: &Proxy, instance_id: &str) -> Result<(), FetchError> { - let (ip, agent_port) = { + let (ip, agent_port, agent_token) = { let guard = state.lock(); let ip = guard .instance_ip(instance_id) // Instance was recycled — never coming back under this id. .ok_or_else(|| FetchError::Permanent(anyhow::anyhow!("unknown instance")))?; - (ip, guard.config.proxy.agent_port) + ( + ip, + guard.config.proxy.agent_port, + guard.instance_agent_token(instance_id), + ) }; - let policy = fetch_port_policy(ip, agent_port).await?; + let policy = fetch_port_policy(ip, agent_port, agent_token.as_deref()).await?; state .lock() .update_instance_port_policy(instance_id, policy); Ok(()) } -async fn fetch_port_policy(ip: Ipv4Addr, agent_port: u16) -> Result { +async fn fetch_port_policy( + ip: Ipv4Addr, + agent_port: u16, + agent_token: Option<&str>, +) -> Result { let url = format!("http://{ip}:{agent_port}/prpc"); // Same untrusted peer as the health poller; see `guest_agent_client`. // Pooled, unlike the poller: this fills a cache rather than deciding // anything, it fires once per instance instead of on a timer, and a // connection that died while idle costs one retry of a lazy fetch. - let client = DstackGuestClient::new(super::guest_agent_client(url, ConnectionReuse::Pooled)); + let transport = super::guest_agent_client(url, ConnectionReuse::Pooled); + let transport = match agent_token { + Some(token) => transport.with_bearer_token(token), + None => transport, + }; + let client = DstackGuestClient::new(transport); // Network/RPC errors here are transient: agent might still be coming up. let info = client .info() diff --git a/dstack/guest-agent/Cargo.toml b/dstack/guest-agent/Cargo.toml index 9b028a6cc..c0cff94d2 100644 --- a/dstack/guest-agent/Cargo.toml +++ b/dstack/guest-agent/Cargo.toml @@ -40,11 +40,12 @@ sysinfo.workspace = true default-net.workspace = true rocket-vsock-listener.workspace = true sd-notify.workspace = true -reqwest.workspace = true cmd_lib.workspace = true figment = { workspace = true, features = ["json", "toml"] } load_config.workspace = true k256 = { workspace = true, features = ["ecdsa"] } +dstack-api-auth.workspace = true +http-client.workspace = true dstack-types.workspace = true sha3.workspace = true strip-ansi-escapes.workspace = true diff --git a/dstack/guest-agent/src/api_auth.rs b/dstack/guest-agent/src/api_auth.rs new file mode 100644 index 000000000..8dc31db4a --- /dev/null +++ b/dstack/guest-agent/src/api_auth.rs @@ -0,0 +1,335 @@ +// SPDX-FileCopyrightText: © 2026 Phala Network +// +// SPDX-License-Identifier: Apache-2.0 + +//! Access control for the guest agent's external listener. +//! +//! Every request that reaches `[external]` names a scope, and the caller has +//! to be granted it: either by an Ed25519 JWT, or -- when the host configured +//! it -- by being anonymous. Nothing is granted by default. +//! +//! This is deliberately not a trust boundary. The measured `public_*` switches +//! in app-compose still decide whether confidential data (logs, sysinfo, TCB +//! info) may leave the TEE at all; a scope can only narrow that, never widen +//! it. What scopes decide is who may *ask* -- which the host configures, and +//! which therefore cannot be part of the measurement. + +use std::{ + sync::{Arc, OnceLock}, + time::{SystemTime, UNIX_EPOCH}, +}; + +use anyhow::{Context, Result}; +use dstack_api_auth::jwt::{self, IssuerKeys, ScopeSet, Verifier}; +use dstack_types::{GuestAgentApiConfig, SysConfig}; +use ed25519_dalek::VerifyingKey; +use rocket::{ + fairing::{Fairing, Info, Kind}, + http::uri::Origin, + request::{FromRequest, Outcome}, + Data, Request, +}; +use tracing::warn; + +/// What a caller may do on the external listener. +#[derive(Clone)] +pub struct GuestApiAuth { + verifier: Verifier, + anonymous: ScopeSet, + /// The app id every token must be bound to. Not known until the boot + /// attestation has been decoded: until then anonymous access still works, + /// but no token can be checked, so none is accepted. + app_id: Arc>, +} + +impl GuestApiAuth { + /// Build from sys-config, given the app root key (if it parsed) and the + /// app id. + /// + /// `anonymous_scopes` is validated eagerly: a name outside the vocabulary + /// never matches anything, so a typo would otherwise read as "the host + /// asked for a permission that does not exist" only when someone could + /// not reach the dashboard. + pub fn new(config: Option<&GuestAgentApiConfig>, app_root_key: Option<&[u8]>) -> Result { + let config = config.cloned().unwrap_or_default(); + let anonymous = ScopeSet::from_names(config.anonymous_scopes.iter().cloned()); + for name in config.anonymous_scopes { + if !jwt::scope::is_known(&name) { + anyhow::bail!("guest_agent_api: unknown anonymous scope {name:?}"); + } + } + + let internal = match app_root_key { + Some(root) => { + let secret = ra_tls::api_v1::derive_app_key( + root, + jwt::INTERNAL_ISSUER_DOMAIN, + ra_tls::api_v1::KeyAlgorithm::Ed25519, + ) + .context("failed to derive the internal issuer key")?; + Some(ed25519_dalek::SigningKey::from_bytes(&secret).verifying_key()) + } + None => None, + }; + + let mut external = Vec::with_capacity(config.external_issuers.len()); + for issuer in config.external_issuers { + let mut keys = Vec::with_capacity(issuer.public_keys.len()); + for encoded in &issuer.public_keys { + let bytes = hex::decode(encoded) + .with_context(|| format!("issuer {:?}: public key is not hex", issuer.name))?; + let Ok(bytes): Result<[u8; 32], _> = bytes.try_into() else { + anyhow::bail!("issuer {:?}: public key must be 32 bytes", issuer.name); + }; + keys.push(VerifyingKey::from_bytes(&bytes).with_context(|| { + format!( + "issuer {:?}: public key is not a valid Ed25519 key", + issuer.name + ) + })?); + } + if keys.is_empty() { + warn!("issuer {:?} has no public keys", issuer.name); + } + external.push(IssuerKeys { + name: issuer.name, + keys, + }); + } + + Ok(Self { + verifier: Verifier::new(internal, external), + anonymous, + app_id: Arc::default(), + }) + } + + /// Build from a sys-config file's contents. + pub fn from_sys_config(sys_config: &SysConfig, app_root_key: Option<&[u8]>) -> Result { + Self::new(sys_config.guest_agent_api.as_ref(), app_root_key) + } + + /// Bind tokens to `app_id`. Called once the boot attestation has been + /// decoded; a second call is ignored, since the value cannot change. + pub fn bind_app_id(&self, app_id: String) { + let _ = self.app_id.set(app_id); + } + + /// The scopes a request carries, or `Err` when it presented a token that + /// did not verify. + /// + /// A rejected token never falls back to anonymous: a caller that meant to + /// authenticate and got it wrong should see a refusal, not whatever the + /// host happened to leave open. + fn resolve(&self, token: Option<&str>) -> Result { + let Some(token) = token else { + return Ok(self.anonymous.clone()); + }; + let Some(app_id) = self.app_id.get().map(String::as_str) else { + anyhow::bail!("no token can be accepted until the app identity is known"); + }; + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + let granted = self + .verifier + .verify(token, app_id, now) + .context("rejected bearer token")?; + Ok(self.anonymous.union(&granted)) + } +} + +/// The scopes in force for one request, published by [`ScopeFairing`]. +/// +/// A request guard rather than a fairing-only side effect because the +/// dashboard renders *parts* of itself by scope: it needs the set, not a +/// yes/no. +pub struct EffectiveScopes(pub ScopeSet); + +impl EffectiveScopes { + pub fn allows(&self, scope: &str) -> bool { + self.0.contains(scope) + } +} + +#[rocket::async_trait] +impl<'r> FromRequest<'r> for EffectiveScopes { + type Error = (); + + async fn from_request(request: &'r Request<'_>) -> Outcome { + match request.local_cache(|| None::) { + Some(scopes) => Outcome::Success(EffectiveScopes(scopes.clone())), + None => Outcome::Forward(rocket::http::Status::Forbidden), + } + } +} + +/// The scope a request to the external listener needs, or `None` when the +/// request names no scope at all (an unknown route, which the router answers). +/// +/// `/prpc` and `/prpc/v0` are the same frozen surface, and the frozen mounts +/// trim the `Worker.` prefix, so `/prpc/Worker.Info`, `/prpc/v0/Info` and +/// `/prpc/Info` are one route with three spellings. Normalising here is what +/// keeps a caller from picking the spelling whose check was forgotten. +/// +/// The segments come from Rocket's own parser rather than from the raw path, +/// because that is what the router matches on: it percent-decodes and drops +/// empty segments, so `/%70rpc/Info` and `/prpc//Info` reach the same handler +/// as `/prpc/Info`. Comparing raw strings would let those spellings through +/// unchecked. +pub fn required_scope(path: &str) -> Option<&'static str> { + let origin = Origin::parse(path).ok()?; + let segments: Vec<&str> = origin.path().segments().filter(|s| !s.is_empty()).collect(); + match segments.as_slice() { + [] => Some(jwt::scope::DASHBOARD), + ["metrics"] => Some(jwt::scope::METRICS), + ["logs", ..] => Some(jwt::scope::LOGS), + _ => None, + } + .or_else(|| { + let method = match segments.as_slice() { + ["prpc", method] | ["prpc", "v0" | "v1", method] => *method, + _ => return None, + }; + match method.strip_prefix("Worker.").unwrap_or(method) { + "Version" => Some(jwt::scope::VERSION), + "Health" => Some(jwt::scope::HEALTH), + "Info" => Some(jwt::scope::INFO), + "GetAttestationForAppKey" => Some(jwt::scope::ATTEST_APP_KEY), + _ => None, + } + }) +} + +/// Refuses a request whose caller lacks the scope it names. +pub struct ScopeFairing { + auth: GuestApiAuth, +} + +impl ScopeFairing { + pub fn new(auth: GuestApiAuth) -> Self { + Self { auth } + } +} + +#[rocket::async_trait] +impl Fairing for ScopeFairing { + fn info(&self) -> Info { + Info { + name: "guest API scope enforcement", + kind: Kind::Request, + } + } + + async fn on_request(&self, req: &mut Request<'_>, _: &mut Data<'_>) { + let path = req.uri().path().as_str(); + let token = req + .headers() + .get_one("Authorization") + .and_then(|value| value.strip_prefix("Bearer ")) + .map(str::trim); + let scopes = match self.auth.resolve(token) { + Ok(scopes) => scopes, + Err(err) => { + warn!("{err:#}"); + deny(req); + return; + } + }; + let denied = required_scope(path).is_some_and(|scope| !scopes.contains(scope)); + if denied { + deny(req); + return; + } + req.local_cache(|| Some(scopes)); + } +} + +fn deny(req: &mut Request<'_>) { + // `Bearer`, not the shared-token `Basic` default: the way to get in here is + // to present a token, and a client told to send Basic would send the wrong + // thing. + req.local_cache(|| dstack_api_auth::Challenge::Bearer); + if let Ok(uri) = Origin::parse_owned(dstack_api_auth::UNAUTHORIZED_PATH.to_owned()) { + req.set_uri(uri); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn every_spelling_of_a_route_needs_the_same_scope() { + for path in [ + "/prpc/Info", + "/prpc/v0/Info", + "/prpc/Worker.Info", + "/prpc/v0/Worker.Info", + ] { + assert_eq!(required_scope(path), Some(jwt::scope::INFO), "{path}"); + } + // The frozen surface has no `Health`, but requiring the scope for that + // spelling too is what keeps the check keyed on the method name rather + // than on which mount happened to answer. + for path in ["/prpc/v1/Health", "/prpc/Health", "/prpc/Worker.Health"] { + assert_eq!(required_scope(path), Some(jwt::scope::HEALTH), "{path}"); + } + assert_eq!(required_scope("/"), Some(jwt::scope::DASHBOARD)); + assert_eq!(required_scope("/metrics"), Some(jwt::scope::METRICS)); + assert_eq!(required_scope("/logs/app-1"), Some(jwt::scope::LOGS)); + assert_eq!( + required_scope("/prpc/GetAttestationForAppKey"), + Some(jwt::scope::ATTEST_APP_KEY) + ); + assert_eq!(required_scope("/nope"), None); + + // The router matches on decoded, non-empty segments, so these reach + // the same handler and must not be able to slip past the check. + for path in ["/%70rpc/Info", "/prpc//Info", "/prpc/v0//Worker.Info"] { + assert_eq!(required_scope(path), Some(jwt::scope::INFO), "{path}"); + } + assert_eq!(required_scope("/%6detrics"), Some(jwt::scope::METRICS)); + } + + fn auth(anonymous: &[&str]) -> GuestApiAuth { + GuestApiAuth::new( + Some(&GuestAgentApiConfig { + anonymous_scopes: anonymous.iter().map(|s| s.to_string()).collect(), + external_issuers: vec![], + }), + Some(&[7u8; 32]), + ) + .unwrap() + } + + #[test] + fn anonymous_grants_only_what_was_configured() { + assert!(auth(&[]).resolve(None).unwrap().is_empty()); + let scopes = auth(&["guest:version"]).resolve(None).unwrap(); + assert!(scopes.contains(jwt::scope::VERSION)); + assert!(!scopes.contains(jwt::scope::INFO)); + } + + #[test] + fn a_bad_token_is_refused_rather_than_downgraded_to_anonymous() { + assert!(auth(&["guest:dashboard"]) + .resolve(Some("not-a-token")) + .is_err()); + } + + #[test] + fn an_unknown_anonymous_scope_fails_at_startup() { + let err = GuestApiAuth::new( + Some(&GuestAgentApiConfig { + anonymous_scopes: vec!["guest:everything".into()], + external_issuers: vec![], + }), + Some(&[7u8; 32]), + ) + .err() + .expect("should reject"); + assert!(err.to_string().contains("guest:everything"), "{err}"); + } +} diff --git a/dstack/guest-agent/src/config.rs b/dstack/guest-agent/src/config.rs index fb816f2f1..1c090c0ea 100644 --- a/dstack/guest-agent/src/config.rs +++ b/dstack/guest-agent/src/config.rs @@ -23,9 +23,16 @@ pub fn load_config_figment_with_default( load_config("dstack", default_config, config_file, true) } -#[derive(Debug, Clone, Copy, Deserialize)] -pub struct BindAddr { - pub port: u16, +/// Where one listener binds. +/// +/// The external listener states a port; the internal ones state a unix socket +/// path. Deserialized as a whole rather than per field so a typo in either is +/// a startup error instead of a listener that silently binds elsewhere. +#[derive(Debug, Clone, Deserialize)] +pub struct SocketAddr { + pub address: String, + #[serde(default)] + pub port: Option, } #[derive(Debug, Clone)] diff --git a/dstack/guest-agent/src/http_routes.rs b/dstack/guest-agent/src/http_routes.rs index 833cd0bd3..4a09aac90 100644 --- a/dstack/guest-agent/src/http_routes.rs +++ b/dstack/guest-agent/src/http_routes.rs @@ -4,11 +4,13 @@ use std::time::Duration; +use crate::api_auth::EffectiveScopes; use crate::config::Config; use crate::guest_api_service::{list_containers, GuestApiHandler}; use crate::rpc_service::{AppState, ExternalRpcHandler}; use anyhow::Result; use docker_logs::parse_duration; +use dstack_api_auth::jwt; use dstack_guest_agent_rpc::v0::{worker_server::WorkerRpc, AppInfo}; use guest_api::guest_api_server::GuestApiRpc; use ra_rpc::{CallContext, RpcCall}; @@ -28,11 +30,26 @@ pub fn external_routes(config: &Config) -> Vec { routes } +/// The dashboard, filtered by what this caller was granted. +/// +/// Access to the page is `guest:dashboard` (enforced by the fairing); what it +/// *shows* is narrowed block by block. Two independent gates: the app's +/// measured `public_*` switches decide whether confidential data may leave the +/// TEE at all, and the caller's scopes decide whether this caller may see it. +/// A scope can only hide a block, never reveal one the app kept private. #[get("/")] -async fn index(state: &State) -> Result, String> { +async fn index( + state: &State, + scopes: EffectiveScopes, +) -> Result, String> { let public_logs = state.config().app_compose.public_logs; let public_sysinfo = state.config().app_compose.public_sysinfo; let public_tcbinfo = state.config().app_compose.public_tcbinfo; + let show_identity = scopes.allows(jwt::scope::INFO); + let show_sysinfo = public_sysinfo && scopes.allows(jwt::scope::METRICS); + let show_containers = scopes.allows(jwt::scope::CONTAINERS); + let show_logs = public_logs && scopes.allows(jwt::scope::LOGS); + let show_tcb = public_tcbinfo && show_identity; let context = CallContext::builder().state(&**state).build(); let handler = ExternalRpcHandler::construct(context.clone()) .map_err(|e| format!("Failed to construct RPC handler: {}", e))?; @@ -58,7 +75,7 @@ async fn index(state: &State) -> Result, String> { // Only collected when the page will actually render it. Collecting // otherwise would scan every process and disk, and spawn a GPU collector, // on guests that keep their sysinfo private. - let (system_info, gpu_info) = if public_sysinfo { + let (system_info, gpu_info) = if show_sysinfo { let handler = GuestApiHandler::construct(context) .map_err(|e| format!("Failed to construct RPC handler: {}", e))?; ( @@ -69,7 +86,13 @@ async fn index(state: &State) -> Result, String> { Default::default() }; - let containers = list_containers().await.unwrap_or_default().containers; + // Skipped, not merely hidden: the listing walks every container on a + // page an anonymous caller may be allowed to reach. + let containers = if show_containers { + list_containers().await.unwrap_or_default().containers + } else { + Vec::new() + }; let model = crate::models::Dashboard { app_name, app_id, @@ -79,9 +102,11 @@ async fn index(state: &State) -> Result, String> { tcb_info, containers, system_info, - public_sysinfo, - public_logs, - public_tcbinfo, + show_identity, + show_sysinfo, + show_containers, + show_logs, + show_tcb, cloud_vendor, cloud_product, gpu_info, diff --git a/dstack/guest-agent/src/lib.rs b/dstack/guest-agent/src/lib.rs index 7c70e192a..d419cabf8 100644 --- a/dstack/guest-agent/src/lib.rs +++ b/dstack/guest-agent/src/lib.rs @@ -5,6 +5,7 @@ pub const CARGO_PKG_VERSION: &str = env!("CARGO_PKG_VERSION"); pub const GIT_REV: &str = dstack_build_info::git_revision!(); +mod api_auth; pub mod backend; pub mod config; mod container_health; diff --git a/dstack/guest-agent/src/models.rs b/dstack/guest-agent/src/models.rs index c8719105b..89f9624cc 100644 --- a/dstack/guest-agent/src/models.rs +++ b/dstack/guest-agent/src/models.rs @@ -116,12 +116,26 @@ pub struct Dashboard { pub tcb_info: String, pub containers: Vec, pub system_info: SystemInfo, - pub public_sysinfo: bool, - pub public_logs: bool, - pub public_tcbinfo: bool, + /// The host's sysinfo and GPU collection may be shown: published by the + /// app (`public_sysinfo`) *and* granted to this caller. + pub show_sysinfo: bool, + /// The TCB document may be shown: published by the app + /// (`public_tcbinfo`) *and* granted to this caller. + pub show_tcb: bool, pub cloud_vendor: String, pub cloud_product: String, pub gpu_info: GpuInfoResponse, + /// Whether the caller may see the identity block at all. The page itself + /// already needs `guest:dashboard`; this narrows what it shows, block by + /// block, so a token for one kind of data does not carry the rest. + pub show_identity: bool, + /// Whether the container table is shown. Also decides whether the + /// container list is collected: a scope that hides a block should not pay + /// for producing it. + pub show_containers: bool, + /// Whether the logs column is shown. Requires `public_logs` as well -- a + /// scope cannot unlock logs the app did not publish. + pub show_logs: bool, } #[derive(Template)] @@ -253,11 +267,13 @@ mod tests { tcb_info: String::new(), containers: vec![], system_info: Default::default(), - public_sysinfo: true, - public_logs: false, - public_tcbinfo: false, + show_sysinfo: true, + show_tcb: false, cloud_vendor: String::new(), cloud_product: String::new(), + show_identity: true, + show_containers: true, + show_logs: true, gpu_info: GpuInfoResponse { gpus: vec![GpuDevice { power_usage_mw: Some(70_123), @@ -404,11 +420,13 @@ mod tests { tcb_info: String::new(), containers: vec![], system_info: Default::default(), - public_sysinfo: true, - public_logs: false, - public_tcbinfo: false, + show_sysinfo: true, + show_tcb: false, cloud_vendor: String::new(), cloud_product: String::new(), + show_identity: true, + show_containers: true, + show_logs: true, gpu_info, } .render() diff --git a/dstack/guest-agent/src/rpc_service.rs b/dstack/guest-agent/src/rpc_service.rs index 6ae1a8aa7..a180c0c5a 100644 --- a/dstack/guest-agent/src/rpc_service.rs +++ b/dstack/guest-agent/src/rpc_service.rs @@ -123,6 +123,8 @@ struct AppStateInner { /// VM is running. cloud_vendor: String, cloud_product: String, + /// Access control for the external listener. See `api_auth`. + api_auth: crate::api_auth::GuestApiAuth, } /// The identity fields v1 `Info` reports, decoded once. @@ -258,7 +260,7 @@ impl AppState { .context("Failed to parse VM config")?; let collateral_urls = sys_config.collateral_urls(); let gpu_attestor = crate::gpu_attest::GpuAttestor::new(); - let vm_config = sys_config.vm_config; + let vm_config = sys_config.vm_config.clone(); // Same trust anchor decision as dstack-util: never host-supplied, and // development roots only when this guest published them itself. let verifier = Arc::new(default_verifier(&collateral_urls)?); @@ -279,6 +281,11 @@ impl AppState { config.app_compose.runner.clone(), ) }); + // Built before the state so a bad `guest_agent_api` block is a boot + // error: an unknown scope name would otherwise silently grant nothing. + let api_auth = + crate::api_auth::GuestApiAuth::from_sys_config(&sys_config, Some(&keys.k256_key)) + .context("Failed to set up guest API access control")?; // Parsed once, and non-fatally: an unusable app root key is reported by // the RPCs that need one, not by refusing to start. let app_root_signing_key = match SigningKey::from_slice(&keys.k256_key) { @@ -305,6 +312,7 @@ impl AppState { attest_lock: Arc::default(), cloud_vendor: read_dmi_file("sys_vendor"), cloud_product: read_dmi_file("product_name"), + api_auth, }), }; // Decode identity now so no request has to. Non-fatal: a platform that @@ -327,6 +335,11 @@ impl AppState { &self.inner.config } + /// Access control for the external listener. + pub(crate) fn api_auth(&self) -> &crate::api_auth::GuestApiAuth { + &self.inner.api_auth + } + pub(crate) fn health(&self) -> Option<&crate::health::HealthMonitor> { self.inner.health.as_deref() } @@ -405,6 +418,16 @@ impl AppState { self.inner .attest(decode_identity) .await + // External tokens are bound to the app id, which only the + // boot attestation can supply. Binding here rather than at + // startup covers the retry path too: until it lands the + // listener serves anonymous callers and refuses every + // token. + .inspect(|identity| { + self.inner + .api_auth + .bind_app_id(hex::encode(&identity.app_id)); + }) .inspect_err(|_| { *last_failure.lock().or_panic("lock should never fail") = Some(Instant::now()); @@ -1463,6 +1486,7 @@ pNs85uhOZE8z2jr8Pg== // against v0 `get_info` compares like with like. cloud_vendor: read_dmi_file("sys_vendor"), cloud_product: read_dmi_file("product_name"), + api_auth: crate::api_auth::GuestApiAuth::new(None, Some(&DUMMY_K256_KEY)).unwrap(), }; ( diff --git a/dstack/guest-agent/src/server.rs b/dstack/guest-agent/src/server.rs index 0021f4f9b..7a02500cd 100644 --- a/dstack/guest-agent/src/server.rs +++ b/dstack/guest-agent/src/server.rs @@ -4,7 +4,8 @@ use std::{future::pending, os::unix::net::UnixListener as StdUnixListener, time::Duration}; -use crate::config::BindAddr; +use crate::api_auth::ScopeFairing; +use crate::config::SocketAddr; use crate::guest_api_service::GuestApiHandler; use crate::http_routes; use crate::rpc_service::{AppState, ExternalRpcHandler, InternalRpcHandler, InternalRpcHandlerV0}; @@ -177,8 +178,13 @@ async fn run_internal( } async fn run_external(state: AppState, figment: Figment) -> Result<()> { + let scopes = ScopeFairing::new(state.api_auth().clone()); let rocket = mount_external(rocket::custom(figment)) .mount("/", http_routes::external_routes(state.config())) + // Mounted first so a denied request has somewhere to land that is not + // itself behind the check. + .mount("/", dstack_api_auth::routes()) + .attach(scopes) .attach(AdHoc::on_response("Add app version header", |_req, res| { Box::pin(async move { res.set_raw_header("X-App-Version", app_version()); @@ -220,7 +226,14 @@ async fn run_guest_api(state: AppState, figment: Figment) -> Result<()> { Ok(()) } -async fn run_watchdog(port: u16) { +/// Probe the internal socket, not the external listener. +/// +/// The external listener now refuses anonymous callers by default, so probing +/// it would report the agent dead whenever the host configured no anonymous +/// scope. The unix socket is the surface that cannot depend on that: it is +/// local-only, unauthenticated, and always mounted. It also answers with the +/// same `Version` the port probe asked for. +async fn run_watchdog(socket_path: String) { let mut watchdog_usec = 0; let enabled = sd_notify::watchdog_enabled(false, &mut watchdog_usec); if !enabled { @@ -237,19 +250,17 @@ async fn run_watchdog(port: u16) { info!("Watchdog enabled, interval={watchdog_usec}us, heartbeat={heatbeat_interval:?}"); let mut interval = tokio::time::interval(heatbeat_interval); - let probe_url = format!("http://localhost:{port}/prpc/Worker.Version"); loop { interval.tick().await; - let client = reqwest::Client::new(); - match client.get(&probe_url).send().await { - Ok(response) if response.status().is_success() => { + match http_client::http_request("GET", &socket_path, "/Version", &[]).await { + Ok((status, _)) if (200..300).contains(&status) => { if let Err(err) = sd_notify(false, &[NotifyState::Watchdog]) { error!("Failed to notify systemd: {err}"); } } - Ok(response) => { - error!("Health check failed with status: {}", response.status()); + Ok((status, _)) => { + error!("Health check failed with status: {status}"); } Err(err) => { error!("Health check request failed: {err:?}"); @@ -262,11 +273,12 @@ pub async fn run(state: AppState, figment: Figment, watchdog: bool) -> Result<() let internal_v0_figment = figment.clone().select("internal-v0"); let internal_figment = figment.clone().select("internal"); let external_figment = figment.clone().select("external"); - let bind_addr = if watchdog { + let watchdog_socket = if watchdog { Some( - external_figment - .extract::() - .context("Failed to extract bind address")?, + internal_figment + .extract::() + .context("Failed to extract the internal socket address")? + .address, ) } else { None @@ -288,8 +300,8 @@ pub async fn run(state: AppState, figment: Figment, watchdog: bool) -> Result<() _ = async { let _ = tappd_ready_rx.await; let _ = sock_ready_rx.await; - if let Some(bind_addr) = bind_addr { - run_watchdog(bind_addr.port).await; + if let Some(socket_path) = watchdog_socket { + run_watchdog(socket_path).await; } else { pending::<()>().await; } diff --git a/dstack/guest-agent/templates/dashboard.html b/dstack/guest-agent/templates/dashboard.html index ee973bb90..29e19d94e 100644 --- a/dstack/guest-agent/templates/dashboard.html +++ b/dstack/guest-agent/templates/dashboard.html @@ -158,9 +158,11 @@ + {% if show_identity || show_sysinfo %}

Node Information

+ {% if show_identity %}
App Name
{{app_name}}
@@ -187,7 +189,8 @@

Node Information

{{cloud_vendor}} ({{cloud_product}})
{% endif %} - {% if public_sysinfo %} + {% endif %} + {% if show_sysinfo %}
Operating System
{{system_info.os_name}} {{system_info.os_version}}
@@ -225,8 +228,9 @@

Node Information

{% endif %}
+ {% endif %} - {% if public_sysinfo %} + {% if show_sysinfo %}

GPUs

{% if !gpu_info.error.is_empty() %}
{{ gpu_info.error }}
@@ -333,6 +337,7 @@

GPUs

{% endif %} {% endif %} + {% if show_containers %}

Deployed Containers

@@ -340,7 +345,7 @@

Deployed Containers

- {% if public_logs %} + {% if show_logs %} {% endif %} @@ -351,7 +356,7 @@

Deployed Containers

- {% if public_logs %} + {% if show_logs %}
Name StatusLogs
{{name|cname}} {{container.status}} View Logs @@ -362,12 +367,11 @@

Deployed Containers

+ {% endif %} + {% if show_tcb %}

TCB Info

- {% if public_tcbinfo %} - {% else %} -

TCB info is not public

{% endif %} diff --git a/dstack/vmm/src/app.rs b/dstack/vmm/src/app.rs index b43710e7b..7be178e7d 100644 --- a/dstack/vmm/src/app.rs +++ b/dstack/vmm/src/app.rs @@ -2078,6 +2078,11 @@ pub(crate) fn make_sys_config( "host_api_url": format!("vsock://2:{}/api", cfg.host_api.port), "vm_config": serde_json::to_string(&vm_config)?, }); + // Omitted when unconfigured so an existing node's sys-config does not + // change shape just because this setting exists. + if cfg.cvm.guest_agent_api != Default::default() { + sys_config["guest_agent_api"] = serde_json::to_value(&cfg.cvm.guest_agent_api)?; + } // No attestation trust anchor is ever written here. Simulated deployments // receive only the development seed through `.tee-simulator.json`; the // in-guest simulator derives the matching roots itself and publishes them diff --git a/dstack/vmm/src/config.rs b/dstack/vmm/src/config.rs index 7d97a5ef5..6bacd0838 100644 --- a/dstack/vmm/src/config.rs +++ b/dstack/vmm/src/config.rs @@ -308,6 +308,29 @@ where ) } +/// Check the block before it is handed to guests: the guest rejects the same +/// things at boot, and an operator would rather learn about a typo at `vmm` +/// startup than from a CVM that will not come up. +fn validate_guest_agent_api(config: &dstack_types::GuestAgentApiConfig) -> Result<()> { + for name in &config.anonymous_scopes { + anyhow::ensure!( + dstack_api_auth::jwt::scope::is_known(name), + "unknown anonymous scope {name:?}" + ); + } + for issuer in &config.external_issuers { + anyhow::ensure!(!issuer.name.is_empty(), "an external issuer needs a name"); + for key in &issuer.public_keys { + anyhow::ensure!( + hex::decode(key).is_ok_and(|bytes| bytes.len() == 32), + "issuer {:?}: public key must be 32 bytes of hex", + issuer.name + ); + } + } + Ok(()) +} + impl CvmConfig { /// The effective TEE platform: the configured one, or host auto-detection /// when left unset (`platform` omitted / `auto`). @@ -496,6 +519,13 @@ pub struct CvmConfig { #[serde(default)] pub instance_id: String, + /// Who may call the guest agent's external API (port 8090). Handed to the + /// guest in sys-config, so it is a host decision and not part of the + /// measurement; the measured `public_*` switches in app-compose still + /// decide whether confidential data may be exposed at all. + #[serde(default)] + pub guest_agent_api: dstack_types::GuestAgentApiConfig, + /// Host sharing mode. (9p, vhd, vvfat) pub host_share_mode: String, @@ -828,6 +858,8 @@ impl Config { .context("Invalid host_api configuration")?; self.netd.validate()?; + validate_guest_agent_api(&self.cvm.guest_agent_api) + .context("Invalid cvm.guest_agent_api configuration")?; anyhow::ensure!(self.cvm.cid_start >= 3, "cvm.cid_start must be at least 3"); anyhow::ensure!( @@ -1462,6 +1494,29 @@ mod tests { assert!(err.contains("set `qemu_version` in vmm.toml"), "{err}"); } + #[test] + fn guest_agent_api_rejects_what_the_guest_would_reject_at_boot() { + let mut config = dstack_types::GuestAgentApiConfig::default(); + config.anonymous_scopes = vec!["guest:dashboard".into()]; + assert!(validate_guest_agent_api(&config).is_ok()); + + config.anonymous_scopes = vec!["guest:everything".into()]; + assert!(validate_guest_agent_api(&config) + .unwrap_err() + .to_string() + .contains("guest:everything")); + + config.anonymous_scopes.clear(); + config.external_issuers = vec![dstack_types::ExternalJwtIssuer { + name: "ops".into(), + public_keys: vec!["not-hex".into()], + }]; + assert!(validate_guest_agent_api(&config) + .unwrap_err() + .to_string() + .contains("ops")); + } + #[test] fn auto_restart_config_rejects_hot_loop_and_inverted_backoff() { let mut config = AutoRestartConfig { diff --git a/dstack/vmm/vmm.toml b/dstack/vmm/vmm.toml index f040d60f6..680dbc8fb 100644 --- a/dstack/vmm/vmm.toml +++ b/dstack/vmm/vmm.toml @@ -109,6 +109,18 @@ tdx_attestation_variant = "auto" host_share_mode = "9p" +# Access control for the guest agent's external listener (port 8090). Absent +# means nothing is anonymous: only holders of a token (the gateway, minted +# inside the CVM) can call it. This is an exposure choice, not a +# confidentiality one -- what may leave the TEE is still decided by the +# measured public_logs / public_sysinfo / public_tcbinfo switches in +# app-compose. See docs/guest-api-auth.md. +# [cvm.guest_agent_api] +# anonymous_scopes = ["guest:dashboard", "guest:version", "guest:health"] +# [[cvm.guest_agent_api.external_issuers]] +# name = "ops" +# public_keys = ["<32-byte ed25519 public key, hex>"] + # Default preallocation for a new CVM data disk. This is a host storage # policy -- the guest cannot see it and it does not affect measurements -- and # a deploy request may pick another mode per VM. From 23a6ed629519c1b9e508978e6da3591377713d42 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Thu, 22 Oct 2026 00:54:48 -0700 Subject: [PATCH 2/4] guest-agent: fail closed on unknown external routes `/prpc/Worker.Worker.Info` reached `Info` with no token: the scope check stripped one `Worker.` prefix and saw an unknown method, which it let through, while ra-rpc stripped every leading prefix with `trim_start_matches` and dispatched it. ra-rpc now strips exactly one, and a path the scope table does not name is refused instead of passed on. Tokens were refused for the life of the process if the startup identity decode failed: the app id was only bound when `identity()` succeeded, and nothing on the token path retried it. The fairing now asks `identity()` itself whenever a token is presented, which also drops the `OnceLock`. Also: - The gateway token carries only `guest:health`. A CVM that mints one always reports its port policy, so the lazy `Info` fetch never runs with it; the `port_policy.rs` plumbing is reverted. - The systemd watchdog probes the external listener again and counts a 401 as alive, instead of moving to the internal socket. - The bearer scheme is matched case-insensitively. - Docs: the CVM is one trust domain, so any process with `dstack.sock` can derive the issuer key through `GetKey`; fix who holds the app root key. Signed-off-by: Kevin Wang --- docs/guest-api-auth.md | 46 ++-- dstack/Cargo.lock | 2 +- dstack/crates/api-auth/src/jwt.rs | 16 +- dstack/crates/api-auth/src/lib.rs | 2 +- dstack/dstack-util/src/system_setup.rs | 57 ++--- dstack/gateway/rpc/proto/gateway_rpc.proto | 6 +- dstack/gateway/src/kv/mod.rs | 5 +- dstack/gateway/src/main_service.rs | 9 - dstack/gateway/src/models.rs | 4 +- dstack/gateway/src/proxy/port_policy.rs | 23 +- dstack/guest-agent/Cargo.toml | 2 +- dstack/guest-agent/src/api_auth.rs | 261 ++++++++++----------- dstack/guest-agent/src/config.rs | 13 +- dstack/guest-agent/src/rpc_service.rs | 32 +-- dstack/guest-agent/src/server.rs | 114 +++++++-- dstack/ra-rpc/src/rocket_helper.rs | 4 +- 16 files changed, 302 insertions(+), 294 deletions(-) diff --git a/docs/guest-api-auth.md b/docs/guest-api-auth.md index 2ec51495b..c6c745186 100644 --- a/docs/guest-api-auth.md +++ b/docs/guest-api-auth.md @@ -26,11 +26,10 @@ only narrow what those allow. See `/prpc`, `/prpc/v0` and `/prpc/v1` are matched after percent-decoding and segment normalisation, so `/prpc/Worker.Info`, `/prpc/v0/Info` and -`/%70rpc/Info` all need `guest:info`. There is no spelling that reaches a -handler with a weaker check. +`/%70rpc/Info` all need `guest:info`. -A route the vocabulary does not name needs no scope: nothing is mounted there, -and the router's 404 is the answer. +The check fails closed: a path the table does not name is answered with 401, +not passed to the router. ## Default: nothing anonymous @@ -88,24 +87,32 @@ so they cannot be expressed. ### Where the internal issuer comes from -The agent and the gateway both hold the app root key inside the TEE. The token -the gateway presents is minted in the guest's own setup (`dstack-util`) and -carried on registration, so it can be bound to the app id and expire: +The gateway does not hold the app root key; the CVM mints its token and hands +it over on registration, bound to the app id and short-lived: -1. Both derive the same Ed25519 key: - `derive_app_key(app_root_key, "dstack:guest-api:issuer:v1", Ed25519)`. +1. The agent and `dstack-util`, both inside the CVM, derive the same Ed25519 + key: `derive_app_key(app_root_key, "dstack:guest-api:issuer:v1", Ed25519)`. 2. `dstack-util` signs a token with `iss = "dstack:guest-api:"`, - `aud = `, a 10-minute lifetime, and the `guest:health`, `guest:info` - scopes. + `aud = `, a 10-minute lifetime, and only the `guest:health` scope. 3. Registration carries it to the gateway, which presents it on the v1 `Health` - poll and on the lazy `Info` fetch that fills in a port policy the CVM did not - report. + poll. -Nothing else gets a token: the gateway has no business on the dashboard or in -the logs. +The host cannot mint an internal token: the signing key is derived from the app +root key, which KMS releases only after the boot measurement is verified. The +host can still name its own [external issuer](#external-issuers) and grant +itself any scope; that is the same exposure as listing the scope in +`anonymous_scopes`, and still bounded by the measured switches. -The host cannot mint one: the signing key is derived from the app root key, which -is released to the guest by KMS only after the boot measurement is verified. +### Trust inside the CVM + +The CVM is one trust domain; the agent does not sandbox the processes in it. The +issuer key is an ordinary v1 application key, so any process that can reach +`dstack.sock` can obtain it with +`GetKey(domain = "dstack:guest-api:issuer:v1", algorithm = ed25519)` and mint a +token with any scope. That is by design -- the same process can already read +the data those scopes expose -- and it is also how an app issues tokens of its +own. Do not mount `dstack.sock` into a container you would not trust with +that. ## The two gates @@ -137,8 +144,5 @@ During the rollout, either upgrade the gateway first, or leave the host open: ```toml [cvm.guest_agent_api] -anonymous_scopes = ["guest:version", "guest:health", "guest:info"] +anonymous_scopes = ["guest:version", "guest:health"] ``` - -`guest:info` is needed by gateways that did not report a port policy and fetch -it lazily. diff --git a/dstack/Cargo.lock b/dstack/Cargo.lock index fcfd0385a..06397dbdd 100644 --- a/dstack/Cargo.lock +++ b/dstack/Cargo.lock @@ -2039,7 +2039,6 @@ dependencies = [ "guest-api", "hex", "host-api", - "http-client", "k256", "libc", "listenfd", @@ -2051,6 +2050,7 @@ dependencies = [ "ra-tls", "rand 0.8.6", "rcgen 0.13.2", + "reqwest", "ring", "rinja", "rocket", diff --git a/dstack/crates/api-auth/src/jwt.rs b/dstack/crates/api-auth/src/jwt.rs index 361e868c8..6f1359981 100644 --- a/dstack/crates/api-auth/src/jwt.rs +++ b/dstack/crates/api-auth/src/jwt.rs @@ -170,21 +170,15 @@ pub struct IssuerKeys { /// Verifies tokens and reports what they grant. #[derive(Clone)] pub struct Verifier { - internal: Option, + internal: VerifyingKey, external: Vec, } impl Verifier { - /// `internal` is absent only when the app root key could not produce one; - /// an agent in that state still verifies external issuers. - pub fn new(internal: Option, external: Vec) -> Self { + pub fn new(internal: VerifyingKey, external: Vec) -> Self { Self { internal, external } } - pub fn is_empty(&self) -> bool { - self.internal.is_none() && self.external.is_empty() - } - /// Verify `token` for the application whose id is `app_id_hex`. /// /// `now` is seconds since the UNIX epoch, passed in rather than read here @@ -238,7 +232,7 @@ impl Verifier { fn keys_for(&self, iss: &str, app_id_hex: &str) -> Vec<&VerifyingKey> { if iss == internal_issuer(app_id_hex) { - return self.internal.iter().collect(); + return vec![&self.internal]; } self.external .iter() @@ -274,7 +268,7 @@ mod tests { fn verifier(seed: u8) -> Verifier { Verifier::new( - Some(SigningKey::from_bytes(&signing_key(seed)).verifying_key()), + SigningKey::from_bytes(&signing_key(seed)).verifying_key(), vec![], ) } @@ -362,7 +356,7 @@ mod tests { #[test] fn accepts_an_external_issuer_by_name() { let verifier = Verifier::new( - None, + SigningKey::from_bytes(&signing_key(1)).verifying_key(), vec![IssuerKeys { name: "ops".into(), keys: vec![SigningKey::from_bytes(&signing_key(9)).verifying_key()], diff --git a/dstack/crates/api-auth/src/lib.rs b/dstack/crates/api-auth/src/lib.rs index 3932de22a..f53deab2b 100644 --- a/dstack/crates/api-auth/src/lib.rs +++ b/dstack/crates/api-auth/src/lib.rs @@ -301,7 +301,7 @@ impl<'r> Responder<'r, 'static> for Unauthorized { } } -pub(crate) struct RequestRealm(String); +struct RequestRealm(String); #[rocket::get("/__dstack_api_auth_unauthorized")] fn unauth_get() -> Unauthorized { diff --git a/dstack/dstack-util/src/system_setup.rs b/dstack/dstack-util/src/system_setup.rs index bc2b22759..86f1cc5f2 100644 --- a/dstack/dstack-util/src/system_setup.rs +++ b/dstack/dstack-util/src/system_setup.rs @@ -494,48 +494,35 @@ struct GatewayContext<'a> { keys: &'a AppKeys, } -/// Scopes the gateway needs on the guest agent's external listener: the v1 -/// `Health` poll, and the frozen `Info` call that fills in a port policy the -/// CVM did not report. Nothing else -- the gateway has no business on the -/// dashboard or in the logs. -const GATEWAY_AGENT_SCOPES: &[&str] = &[jwt::scope::HEALTH, jwt::scope::INFO]; - -/// Mint the token the gateway presents to this CVM's guest agent. -/// -/// Derived from the app root key, so it needs no distribution: the agent holds -/// the public half, this holds the private one, and the gateway only carries -/// the result. `None` when either side is not ready yet -- an app id that only -/// the boot attestation can supply, or an app root key that does not parse -- -/// in which case registration carries no token and the agent answers -/// anonymously, exactly as it did before this existed. -/// -/// One token for both calls the gateway makes, because both are the gateway -/// doing its own job: the v1 `Health` poll, and the frozen `Info` that fills in -/// a port policy the CVM did not report. Scopes are still a whitelist -- the -/// gateway gets nothing on the dashboard or in the logs. -fn gateway_agent_token(app_id: &[u8], app_root_key: &[u8]) -> Option { +/// The gateway only polls `Health`. Port policy reaches it on registration, so +/// it never needs `Info` from a CVM that can mint this token. +const GATEWAY_AGENT_SCOPES: &[&str] = &[jwt::scope::HEALTH]; + +/// Lifetime of the gateway's token. Registration refreshes every three +/// minutes; this survives a few failed refreshes so a gateway outage does not +/// also lock the gateway out of the agent. +const GATEWAY_AGENT_TOKEN_LIFETIME_SECS: u64 = 600; + +/// Mint the token the gateway presents to this CVM's guest agent, signed with +/// the key the agent derives the same way from the app root key. +fn gateway_agent_token(app_id: &[u8], app_root_key: &[u8]) -> Result { let secret = ra_tls::api_v1::derive_app_key( app_root_key, jwt::INTERNAL_ISSUER_DOMAIN, ra_tls::api_v1::KeyAlgorithm::Ed25519, - ) - .ok()?; + )?; let app_id = hex::encode(app_id); - // Long enough to survive a few failed refreshes: a token that expired - // during a gateway outage would turn "we could not re-register" into "we - // are locked out until the outage ends". let now = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .ok()? + .duration_since(std::time::UNIX_EPOCH)? .as_secs(); let claims = jwt::Claims { iss: jwt::internal_issuer(&app_id), aud: app_id, iat: now, - exp: now + 600, + exp: now + GATEWAY_AGENT_TOKEN_LIFETIME_SECS, scope: GATEWAY_AGENT_SCOPES.join(" "), }; - jwt::sign(&secret, &claims).ok() + jwt::sign(&secret, &claims) } impl<'a> GatewayContext<'a> { @@ -577,14 +564,12 @@ impl<'a> GatewayContext<'a> { key_store: &GatewayKeyStore, gateway_app_id: &str, ) -> Result { + // Not fatal: without a token the gateway's health poll is refused, which + // costs routing but should not cost the boot. let agent_token = - gateway_agent_token(&self.shared.instance_info.app_id, &self.keys.k256_key); - if agent_token.is_none() { - // Not fatal: the CVM registers without one and the agent answers - // however the host configured it. Say so once per registration - // rather than failing a boot over it. - info!("no guest agent token available; registering without one"); - } + gateway_agent_token(&self.shared.instance_info.app_id, &self.keys.k256_key) + .inspect_err(|err| warn!("failed to mint the guest agent token: {err:#}")) + .ok(); let port_policy = RpcPortPolicy { ports: self .shared diff --git a/dstack/gateway/rpc/proto/gateway_rpc.proto b/dstack/gateway/rpc/proto/gateway_rpc.proto index 6a889673f..04ad4c795 100644 --- a/dstack/gateway/rpc/proto/gateway_rpc.proto +++ b/dstack/gateway/rpc/proto/gateway_rpc.proto @@ -37,9 +37,9 @@ message RegisterCvmRequest { // CVM re-registers every three minutes, so a stale copy never outlives its // usefulness. // - // Empty from a CVM that predates the field, or one whose app identity is - // not known yet. The gateway then calls without a token and gets whatever - // the host left open -- possibly nothing, which is the intended default. + // Empty from a CVM that predates the field, or one that could not mint it. + // The gateway then calls without a token and gets whatever the host left + // open -- possibly nothing, which is the intended default. optional string agent_token = 4; } diff --git a/dstack/gateway/src/kv/mod.rs b/dstack/gateway/src/kv/mod.rs index 5238d3676..915baee5e 100644 --- a/dstack/gateway/src/kv/mod.rs +++ b/dstack/gateway/src/kv/mod.rs @@ -130,9 +130,8 @@ pub struct InstanceRecord { /// a rollback and forward again. #[serde(default)] pub health_check: Option, - /// Token this CVM minted for the gateway, so the agent's external listener - /// can answer an operator-run caller. `None` for a CVM that predates the - /// field, or one whose app id is not yet known. + /// Token this CVM minted for the gateway's `Health` poll. `None` for a CVM + /// that predates the field, or one that could not mint it. /// /// Per-instance rather than per-app: the token is bound to the instance's /// app id and expires within the hour, and the CVM re-registers every diff --git a/dstack/gateway/src/main_service.rs b/dstack/gateway/src/main_service.rs index 5c77116c7..7c931a430 100644 --- a/dstack/gateway/src/main_service.rs +++ b/dstack/gateway/src/main_service.rs @@ -2134,15 +2134,6 @@ impl ProxyState { self.state.instances.get(instance_id).map(|i| i.ip) } - /// The bearer token an instance's guest agent external listener accepts, - /// if the CVM supplied one at registration. - pub(crate) fn instance_agent_token(&self, instance_id: &str) -> Option { - self.state - .instances - .get(instance_id) - .and_then(|info| info.agent_token.clone()) - } - /// Lookup the effective port_policy for an instance: admin override wins, /// otherwise fall back to the instance-reported policy. `None` means /// neither is set — caller fails closed and may schedule a lazy fetch. diff --git a/dstack/gateway/src/models.rs b/dstack/gateway/src/models.rs index abaa9663a..6ee4e11d4 100644 --- a/dstack/gateway/src/models.rs +++ b/dstack/gateway/src/models.rs @@ -73,8 +73,8 @@ pub struct InstanceInfo { /// declaration and verdict disagree. pub(crate) health: Health, /// Bearer token for the instance's guest agent external listener, minted - /// by the CVM at registration. Absent for a legacy CVM, which has no such - /// listener to authenticate against. + /// by the CVM at registration. Absent for a legacy CVM, whose listener + /// needs none. pub agent_token: Option, pub connections: Arc, } diff --git a/dstack/gateway/src/proxy/port_policy.rs b/dstack/gateway/src/proxy/port_policy.rs index a8573afdb..bc8ce05d1 100644 --- a/dstack/gateway/src/proxy/port_policy.rs +++ b/dstack/gateway/src/proxy/port_policy.rs @@ -217,41 +217,28 @@ async fn fetch_with_retry(state: &Proxy, instance_id: &str) { } async fn fetch_and_store(state: &Proxy, instance_id: &str) -> Result<(), FetchError> { - let (ip, agent_port, agent_token) = { + let (ip, agent_port) = { let guard = state.lock(); let ip = guard .instance_ip(instance_id) // Instance was recycled — never coming back under this id. .ok_or_else(|| FetchError::Permanent(anyhow::anyhow!("unknown instance")))?; - ( - ip, - guard.config.proxy.agent_port, - guard.instance_agent_token(instance_id), - ) + (ip, guard.config.proxy.agent_port) }; - let policy = fetch_port_policy(ip, agent_port, agent_token.as_deref()).await?; + let policy = fetch_port_policy(ip, agent_port).await?; state .lock() .update_instance_port_policy(instance_id, policy); Ok(()) } -async fn fetch_port_policy( - ip: Ipv4Addr, - agent_port: u16, - agent_token: Option<&str>, -) -> Result { +async fn fetch_port_policy(ip: Ipv4Addr, agent_port: u16) -> Result { let url = format!("http://{ip}:{agent_port}/prpc"); // Same untrusted peer as the health poller; see `guest_agent_client`. // Pooled, unlike the poller: this fills a cache rather than deciding // anything, it fires once per instance instead of on a timer, and a // connection that died while idle costs one retry of a lazy fetch. - let transport = super::guest_agent_client(url, ConnectionReuse::Pooled); - let transport = match agent_token { - Some(token) => transport.with_bearer_token(token), - None => transport, - }; - let client = DstackGuestClient::new(transport); + let client = DstackGuestClient::new(super::guest_agent_client(url, ConnectionReuse::Pooled)); // Network/RPC errors here are transient: agent might still be coming up. let info = client .info() diff --git a/dstack/guest-agent/Cargo.toml b/dstack/guest-agent/Cargo.toml index c0cff94d2..4d81c6172 100644 --- a/dstack/guest-agent/Cargo.toml +++ b/dstack/guest-agent/Cargo.toml @@ -40,12 +40,12 @@ sysinfo.workspace = true default-net.workspace = true rocket-vsock-listener.workspace = true sd-notify.workspace = true +reqwest.workspace = true cmd_lib.workspace = true figment = { workspace = true, features = ["json", "toml"] } load_config.workspace = true k256 = { workspace = true, features = ["ecdsa"] } dstack-api-auth.workspace = true -http-client.workspace = true dstack-types.workspace = true sha3.workspace = true strip-ansi-escapes.workspace = true diff --git a/dstack/guest-agent/src/api_auth.rs b/dstack/guest-agent/src/api_auth.rs index 8dc31db4a..907f6c0f4 100644 --- a/dstack/guest-agent/src/api_auth.rs +++ b/dstack/guest-agent/src/api_auth.rs @@ -14,14 +14,11 @@ //! it. What scopes decide is who may *ask* -- which the host configures, and //! which therefore cannot be part of the measurement. -use std::{ - sync::{Arc, OnceLock}, - time::{SystemTime, UNIX_EPOCH}, -}; +use std::time::{SystemTime, UNIX_EPOCH}; use anyhow::{Context, Result}; use dstack_api_auth::jwt::{self, IssuerKeys, ScopeSet, Verifier}; -use dstack_types::{GuestAgentApiConfig, SysConfig}; +use dstack_types::GuestAgentApiConfig; use ed25519_dalek::VerifyingKey; use rocket::{ fairing::{Fairing, Info, Kind}, @@ -31,103 +28,77 @@ use rocket::{ }; use tracing::warn; +use crate::rpc_service::AppState; + /// What a caller may do on the external listener. #[derive(Clone)] pub struct GuestApiAuth { verifier: Verifier, anonymous: ScopeSet, - /// The app id every token must be bound to. Not known until the boot - /// attestation has been decoded: until then anonymous access still works, - /// but no token can be checked, so none is accepted. - app_id: Arc>, } impl GuestApiAuth { - /// Build from sys-config, given the app root key (if it parsed) and the - /// app id. + /// Build from sys-config's `guest_agent_api` block and the app root key. /// - /// `anonymous_scopes` is validated eagerly: a name outside the vocabulary - /// never matches anything, so a typo would otherwise read as "the host - /// asked for a permission that does not exist" only when someone could - /// not reach the dashboard. - pub fn new(config: Option<&GuestAgentApiConfig>, app_root_key: Option<&[u8]>) -> Result { + /// `anonymous_scopes` is validated eagerly so a typo is a boot error rather + /// than a permission that silently never matches. + pub fn new(config: Option<&GuestAgentApiConfig>, app_root_key: &[u8]) -> Result { let config = config.cloned().unwrap_or_default(); - let anonymous = ScopeSet::from_names(config.anonymous_scopes.iter().cloned()); - for name in config.anonymous_scopes { - if !jwt::scope::is_known(&name) { - anyhow::bail!("guest_agent_api: unknown anonymous scope {name:?}"); - } + if let Some(name) = config + .anonymous_scopes + .iter() + .find(|name| !jwt::scope::is_known(name)) + { + anyhow::bail!("guest_agent_api: unknown anonymous scope {name:?}"); } + let anonymous = ScopeSet::from_names(config.anonymous_scopes); - let internal = match app_root_key { - Some(root) => { - let secret = ra_tls::api_v1::derive_app_key( - root, - jwt::INTERNAL_ISSUER_DOMAIN, - ra_tls::api_v1::KeyAlgorithm::Ed25519, - ) - .context("failed to derive the internal issuer key")?; - Some(ed25519_dalek::SigningKey::from_bytes(&secret).verifying_key()) - } - None => None, - }; + let secret = ra_tls::api_v1::derive_app_key( + app_root_key, + jwt::INTERNAL_ISSUER_DOMAIN, + ra_tls::api_v1::KeyAlgorithm::Ed25519, + ) + .context("failed to derive the internal issuer key")?; + let internal = ed25519_dalek::SigningKey::from_bytes(&secret).verifying_key(); - let mut external = Vec::with_capacity(config.external_issuers.len()); - for issuer in config.external_issuers { - let mut keys = Vec::with_capacity(issuer.public_keys.len()); - for encoded in &issuer.public_keys { - let bytes = hex::decode(encoded) - .with_context(|| format!("issuer {:?}: public key is not hex", issuer.name))?; - let Ok(bytes): Result<[u8; 32], _> = bytes.try_into() else { - anyhow::bail!("issuer {:?}: public key must be 32 bytes", issuer.name); - }; - keys.push(VerifyingKey::from_bytes(&bytes).with_context(|| { - format!( - "issuer {:?}: public key is not a valid Ed25519 key", - issuer.name - ) - })?); - } - if keys.is_empty() { - warn!("issuer {:?} has no public keys", issuer.name); - } - external.push(IssuerKeys { - name: issuer.name, - keys, - }); - } + let external = config + .external_issuers + .into_iter() + .map(|issuer| { + let keys = issuer + .public_keys + .iter() + .map(|encoded| parse_public_key(encoded)) + .collect::>>() + .with_context(|| format!("issuer {:?}", issuer.name))?; + if keys.is_empty() { + warn!("issuer {:?} has no public keys", issuer.name); + } + Ok(IssuerKeys { + name: issuer.name, + keys, + }) + }) + .collect::>()?; Ok(Self { verifier: Verifier::new(internal, external), anonymous, - app_id: Arc::default(), }) } - /// Build from a sys-config file's contents. - pub fn from_sys_config(sys_config: &SysConfig, app_root_key: Option<&[u8]>) -> Result { - Self::new(sys_config.guest_agent_api.as_ref(), app_root_key) - } - - /// Bind tokens to `app_id`. Called once the boot attestation has been - /// decoded; a second call is ignored, since the value cannot change. - pub fn bind_app_id(&self, app_id: String) { - let _ = self.app_id.set(app_id); - } - /// The scopes a request carries, or `Err` when it presented a token that /// did not verify. /// /// A rejected token never falls back to anonymous: a caller that meant to /// authenticate and got it wrong should see a refusal, not whatever the - /// host happened to leave open. - fn resolve(&self, token: Option<&str>) -> Result { + /// host happened to leave open. `app_id` is `None` while the app identity + /// is unavailable, in which case no token can be checked. + fn resolve(&self, token: Option<&str>, app_id: Option<&str>) -> Result { let Some(token) = token else { return Ok(self.anonymous.clone()); }; - let Some(app_id) = self.app_id.get().map(String::as_str) else { - anyhow::bail!("no token can be accepted until the app identity is known"); - }; + let app_id = app_id.context("no token can be accepted until the app identity is known")?; let now = SystemTime::now() .duration_since(UNIX_EPOCH) .map(|d| d.as_secs()) @@ -140,11 +111,19 @@ impl GuestApiAuth { } } +fn parse_public_key(encoded: &str) -> Result { + let bytes: [u8; 32] = hex::decode(encoded) + .context("public key is not hex")? + .try_into() + .ok() + .context("public key must be 32 bytes")?; + VerifyingKey::from_bytes(&bytes).context("public key is not a valid Ed25519 key") +} + /// The scopes in force for one request, published by [`ScopeFairing`]. /// -/// A request guard rather than a fairing-only side effect because the -/// dashboard renders *parts* of itself by scope: it needs the set, not a -/// yes/no. +/// A request guard because the dashboard renders parts of itself by scope: it +/// needs the set, not a yes/no. pub struct EffectiveScopes(pub ScopeSet); impl EffectiveScopes { @@ -165,54 +144,58 @@ impl<'r> FromRequest<'r> for EffectiveScopes { } } -/// The scope a request to the external listener needs, or `None` when the -/// request names no scope at all (an unknown route, which the router answers). +/// The scope a request to the external listener needs, or `None` for a path +/// that names no known route. `None` is refused: the check fails closed, so a +/// spelling this function does not recognise can never reach a handler. /// -/// `/prpc` and `/prpc/v0` are the same frozen surface, and the frozen mounts -/// trim the `Worker.` prefix, so `/prpc/Worker.Info`, `/prpc/v0/Info` and -/// `/prpc/Info` are one route with three spellings. Normalising here is what -/// keeps a caller from picking the spelling whose check was forgotten. +/// Segments come from Rocket's own parser, because that is what the router +/// matches on: it percent-decodes and drops empty segments, so `/%70rpc/Info` +/// and `/prpc//Info` reach the same handler as `/prpc/Info`. /// -/// The segments come from Rocket's own parser rather than from the raw path, -/// because that is what the router matches on: it percent-decodes and drops -/// empty segments, so `/%70rpc/Info` and `/prpc//Info` reach the same handler -/// as `/prpc/Info`. Comparing raw strings would let those spellings through -/// unchecked. +/// The frozen mounts (`/prpc`, `/prpc/v0`) strip one `Worker.` prefix before +/// dispatch, exactly as here; `/prpc/v1` strips nothing. pub fn required_scope(path: &str) -> Option<&'static str> { let origin = Origin::parse(path).ok()?; let segments: Vec<&str> = origin.path().segments().filter(|s| !s.is_empty()).collect(); - match segments.as_slice() { - [] => Some(jwt::scope::DASHBOARD), - ["metrics"] => Some(jwt::scope::METRICS), - ["logs", ..] => Some(jwt::scope::LOGS), + let method = match segments.as_slice() { + [] => return Some(jwt::scope::DASHBOARD), + ["metrics"] => return Some(jwt::scope::METRICS), + ["logs", _] => return Some(jwt::scope::LOGS), + ["prpc", "v1", method] => *method, + ["prpc", "v0", method] | ["prpc", method] => { + method.strip_prefix("Worker.").unwrap_or(method) + } + _ => return None, + }; + match method { + "Version" => Some(jwt::scope::VERSION), + "Health" => Some(jwt::scope::HEALTH), + "Info" => Some(jwt::scope::INFO), + "GetAttestationForAppKey" => Some(jwt::scope::ATTEST_APP_KEY), _ => None, } - .or_else(|| { - let method = match segments.as_slice() { - ["prpc", method] | ["prpc", "v0" | "v1", method] => *method, - _ => return None, - }; - match method.strip_prefix("Worker.").unwrap_or(method) { - "Version" => Some(jwt::scope::VERSION), - "Health" => Some(jwt::scope::HEALTH), - "Info" => Some(jwt::scope::INFO), - "GetAttestationForAppKey" => Some(jwt::scope::ATTEST_APP_KEY), - _ => None, - } - }) } -/// Refuses a request whose caller lacks the scope it names. +/// Refuses a request whose caller lacks the scope its route needs. pub struct ScopeFairing { - auth: GuestApiAuth, + state: AppState, } impl ScopeFairing { - pub fn new(auth: GuestApiAuth) -> Self { - Self { auth } + pub fn new(state: AppState) -> Self { + Self { state } } } +/// The token from an `Authorization: Bearer` header. The scheme is +/// case-insensitive (RFC 9110 §11.1); any other scheme is no token at all. +fn bearer_token<'r>(req: &'r Request<'_>) -> Option<&'r str> { + let (scheme, token) = req.headers().get_one("Authorization")?.split_once(' ')?; + scheme + .eq_ignore_ascii_case("bearer") + .then_some(token.trim()) +} + #[rocket::async_trait] impl Fairing for ScopeFairing { fn info(&self) -> Info { @@ -223,33 +206,37 @@ impl Fairing for ScopeFairing { } async fn on_request(&self, req: &mut Request<'_>, _: &mut Data<'_>) { - let path = req.uri().path().as_str(); - let token = req - .headers() - .get_one("Authorization") - .and_then(|value| value.strip_prefix("Bearer ")) - .map(str::trim); - let scopes = match self.auth.resolve(token) { + let token = bearer_token(req); + // Only a token needs the app id. Going through `identity()` rather + // than a value bound at startup is what lets a boot whose first decode + // failed recover: the retry is driven by the callers that need it. + let app_id = match token { + Some(_) => self + .state + .identity() + .await + .ok() + .map(|identity| hex::encode(&identity.app_id)), + None => None, + }; + let scopes = match self.state.api_auth().resolve(token, app_id.as_deref()) { Ok(scopes) => scopes, Err(err) => { warn!("{err:#}"); - deny(req); - return; + return deny(req); } }; - let denied = required_scope(path).is_some_and(|scope| !scopes.contains(scope)); - if denied { - deny(req); - return; + match required_scope(req.uri().path().as_str()) { + Some(scope) if scopes.contains(scope) => { + req.local_cache(|| Some(scopes)); + } + _ => deny(req), } - req.local_cache(|| Some(scopes)); } } fn deny(req: &mut Request<'_>) { - // `Bearer`, not the shared-token `Basic` default: the way to get in here is - // to present a token, and a client told to send Basic would send the wrong - // thing. + // `Bearer`, not the shared-token `Basic` default: the way in is a token. req.local_cache(|| dstack_api_auth::Challenge::Bearer); if let Ok(uri) = Origin::parse_owned(dstack_api_auth::UNAUTHORIZED_PATH.to_owned()) { req.set_uri(uri); @@ -283,7 +270,17 @@ mod tests { required_scope("/prpc/GetAttestationForAppKey"), Some(jwt::scope::ATTEST_APP_KEY) ); - assert_eq!(required_scope("/nope"), None); + // Fails closed: anything not named here is refused, including a + // second service prefix the dispatcher would not strip. + for path in [ + "/nope", + "/prpc/Worker.Worker.Info", + "/prpc/v0/Worker.Worker.Info", + "/prpc/v1/Worker.Health", + "/prpc/GetQuote", + ] { + assert_eq!(required_scope(path), None, "{path}"); + } // The router matches on decoded, non-empty segments, so these reach // the same handler and must not be able to slip past the check. @@ -299,15 +296,15 @@ mod tests { anonymous_scopes: anonymous.iter().map(|s| s.to_string()).collect(), external_issuers: vec![], }), - Some(&[7u8; 32]), + &[7u8; 32], ) .unwrap() } #[test] fn anonymous_grants_only_what_was_configured() { - assert!(auth(&[]).resolve(None).unwrap().is_empty()); - let scopes = auth(&["guest:version"]).resolve(None).unwrap(); + assert!(auth(&[]).resolve(None, None).unwrap().is_empty()); + let scopes = auth(&["guest:version"]).resolve(None, None).unwrap(); assert!(scopes.contains(jwt::scope::VERSION)); assert!(!scopes.contains(jwt::scope::INFO)); } @@ -315,7 +312,7 @@ mod tests { #[test] fn a_bad_token_is_refused_rather_than_downgraded_to_anonymous() { assert!(auth(&["guest:dashboard"]) - .resolve(Some("not-a-token")) + .resolve(Some("not-a-token"), Some("00")) .is_err()); } @@ -326,7 +323,7 @@ mod tests { anonymous_scopes: vec!["guest:everything".into()], external_issuers: vec![], }), - Some(&[7u8; 32]), + &[7u8; 32], ) .err() .expect("should reject"); diff --git a/dstack/guest-agent/src/config.rs b/dstack/guest-agent/src/config.rs index 1c090c0ea..fb816f2f1 100644 --- a/dstack/guest-agent/src/config.rs +++ b/dstack/guest-agent/src/config.rs @@ -23,16 +23,9 @@ pub fn load_config_figment_with_default( load_config("dstack", default_config, config_file, true) } -/// Where one listener binds. -/// -/// The external listener states a port; the internal ones state a unix socket -/// path. Deserialized as a whole rather than per field so a typo in either is -/// a startup error instead of a listener that silently binds elsewhere. -#[derive(Debug, Clone, Deserialize)] -pub struct SocketAddr { - pub address: String, - #[serde(default)] - pub port: Option, +#[derive(Debug, Clone, Copy, Deserialize)] +pub struct BindAddr { + pub port: u16, } #[derive(Debug, Clone)] diff --git a/dstack/guest-agent/src/rpc_service.rs b/dstack/guest-agent/src/rpc_service.rs index a180c0c5a..a6dcd8e5f 100644 --- a/dstack/guest-agent/src/rpc_service.rs +++ b/dstack/guest-agent/src/rpc_service.rs @@ -281,11 +281,11 @@ impl AppState { config.app_compose.runner.clone(), ) }); - // Built before the state so a bad `guest_agent_api` block is a boot - // error: an unknown scope name would otherwise silently grant nothing. + // A bad `guest_agent_api` block is a boot error: an unknown scope name + // would otherwise silently grant nothing. let api_auth = - crate::api_auth::GuestApiAuth::from_sys_config(&sys_config, Some(&keys.k256_key)) - .context("Failed to set up guest API access control")?; + crate::api_auth::GuestApiAuth::new(sys_config.guest_agent_api.as_ref(), &keys.k256_key) + .context("failed to set up guest API access control")?; // Parsed once, and non-fatally: an unusable app root key is reported by // the RPCs that need one, not by refusing to start. let app_root_signing_key = match SigningKey::from_slice(&keys.k256_key) { @@ -418,16 +418,6 @@ impl AppState { self.inner .attest(decode_identity) .await - // External tokens are bound to the app id, which only the - // boot attestation can supply. Binding here rather than at - // startup covers the retry path too: until it lands the - // listener serves anonymous callers and refuses every - // token. - .inspect(|identity| { - self.inner - .api_auth - .bind_app_id(hex::encode(&identity.app_id)); - }) .inspect_err(|_| { *last_failure.lock().or_panic("lock should never fail") = Some(Instant::now()); @@ -1126,6 +1116,12 @@ pub(crate) mod tests { use std::io::Write; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; + pub(crate) const DUMMY_K256_KEY: [u8; 32] = [ + 0x1A, 0x2B, 0x3C, 0x4D, 0x5E, 0x6F, 0x7A, 0x8B, 0x9C, 0x0D, 0x1E, 0x2F, 0x3A, 0x4B, 0x5C, + 0x6D, 0x7E, 0x8F, 0x9A, 0x0B, 0x1C, 0x2D, 0x3E, 0x4F, 0x5A, 0x6B, 0x7C, 0x8D, 0x9E, 0x0F, + 0x1A, 0x2B, + ]; + fn extract_pubkey_from_report_data(report_data: &[u8], prefix: &str) -> Result> { let end = report_data .iter() @@ -1352,12 +1348,6 @@ pNs85uhOZE8z2jr8Pg== -----END CERTIFICATE----- "#; - const DUMMY_K256_KEY: [u8; 32] = [ - 0x1A, 0x2B, 0x3C, 0x4D, 0x5E, 0x6F, 0x7A, 0x8B, 0x9C, 0x0D, 0x1E, 0x2F, 0x3A, 0x4B, - 0x5C, 0x6D, 0x7E, 0x8F, 0x9A, 0x0B, 0x1C, 0x2D, 0x3E, 0x4F, 0x5A, 0x6B, 0x7C, 0x8D, - 0x9E, 0x0F, 0x1A, 0x2B, - ]; - let dummy_keys = AppKeys { disk_crypt_key: Vec::new(), env_crypt_key: Vec::new(), @@ -1486,7 +1476,7 @@ pNs85uhOZE8z2jr8Pg== // against v0 `get_info` compares like with like. cloud_vendor: read_dmi_file("sys_vendor"), cloud_product: read_dmi_file("product_name"), - api_auth: crate::api_auth::GuestApiAuth::new(None, Some(&DUMMY_K256_KEY)).unwrap(), + api_auth: crate::api_auth::GuestApiAuth::new(None, &DUMMY_K256_KEY).unwrap(), }; ( diff --git a/dstack/guest-agent/src/server.rs b/dstack/guest-agent/src/server.rs index 7a02500cd..4590cc82f 100644 --- a/dstack/guest-agent/src/server.rs +++ b/dstack/guest-agent/src/server.rs @@ -5,7 +5,7 @@ use std::{future::pending, os::unix::net::UnixListener as StdUnixListener, time::Duration}; use crate::api_auth::ScopeFairing; -use crate::config::SocketAddr; +use crate::config::BindAddr; use crate::guest_api_service::GuestApiHandler; use crate::http_routes; use crate::rpc_service::{AppState, ExternalRpcHandler, InternalRpcHandler, InternalRpcHandlerV0}; @@ -178,13 +178,11 @@ async fn run_internal( } async fn run_external(state: AppState, figment: Figment) -> Result<()> { - let scopes = ScopeFairing::new(state.api_auth().clone()); let rocket = mount_external(rocket::custom(figment)) .mount("/", http_routes::external_routes(state.config())) - // Mounted first so a denied request has somewhere to land that is not - // itself behind the check. + // Where `ScopeFairing` rewrites a denied request. .mount("/", dstack_api_auth::routes()) - .attach(scopes) + .attach(ScopeFairing::new(state.clone())) .attach(AdHoc::on_response("Add app version header", |_req, res| { Box::pin(async move { res.set_raw_header("X-App-Version", app_version()); @@ -226,14 +224,7 @@ async fn run_guest_api(state: AppState, figment: Figment) -> Result<()> { Ok(()) } -/// Probe the internal socket, not the external listener. -/// -/// The external listener now refuses anonymous callers by default, so probing -/// it would report the agent dead whenever the host configured no anonymous -/// scope. The unix socket is the surface that cannot depend on that: it is -/// local-only, unauthenticated, and always mounted. It also answers with the -/// same `Version` the port probe asked for. -async fn run_watchdog(socket_path: String) { +async fn run_watchdog(port: u16) { let mut watchdog_usec = 0; let enabled = sd_notify::watchdog_enabled(false, &mut watchdog_usec); if !enabled { @@ -250,17 +241,24 @@ async fn run_watchdog(socket_path: String) { info!("Watchdog enabled, interval={watchdog_usec}us, heartbeat={heatbeat_interval:?}"); let mut interval = tokio::time::interval(heatbeat_interval); + let probe_url = format!("http://localhost:{port}/prpc/Worker.Version"); loop { interval.tick().await; - match http_client::http_request("GET", &socket_path, "/Version", &[]).await { - Ok((status, _)) if (200..300).contains(&status) => { + let client = reqwest::Client::new(); + match client.get(&probe_url).send().await { + // 401 is the listener answering a caller with no scope, which is + // the default; it proves the listener is up as well as a 200. + Ok(response) + if response.status().is_success() + || response.status() == reqwest::StatusCode::UNAUTHORIZED => + { if let Err(err) = sd_notify(false, &[NotifyState::Watchdog]) { error!("Failed to notify systemd: {err}"); } } - Ok((status, _)) => { - error!("Health check failed with status: {status}"); + Ok(response) => { + error!("Health check failed with status: {}", response.status()); } Err(err) => { error!("Health check request failed: {err:?}"); @@ -273,12 +271,11 @@ pub async fn run(state: AppState, figment: Figment, watchdog: bool) -> Result<() let internal_v0_figment = figment.clone().select("internal-v0"); let internal_figment = figment.clone().select("internal"); let external_figment = figment.clone().select("external"); - let watchdog_socket = if watchdog { + let bind_addr = if watchdog { Some( - internal_figment - .extract::() - .context("Failed to extract the internal socket address")? - .address, + external_figment + .extract::() + .context("Failed to extract bind address")?, ) } else { None @@ -300,8 +297,8 @@ pub async fn run(state: AppState, figment: Figment, watchdog: bool) -> Result<() _ = async { let _ = tappd_ready_rx.await; let _ = sock_ready_rx.await; - if let Some(socket_path) = watchdog_socket { - run_watchdog(socket_path).await; + if let Some(bind_addr) = bind_addr { + run_watchdog(bind_addr.port).await; } else { pending::<()>().await; } @@ -437,4 +434,73 @@ mod tests { let (status, _) = get(&pre_v1, "/EmitEvent").await; assert_eq!(status, 400); } + + /// With nothing anonymous, no spelling of a route reaches a handler + /// without a token. + #[tokio::test] + async fn the_external_listener_refuses_every_spelling_without_a_token() { + let (state, _guard) = setup_test_state().await; + let client = Client::tracked( + mount_external(rocket::build()) + .mount("/", dstack_api_auth::routes()) + .attach(ScopeFairing::new(state.clone())) + .manage(state), + ) + .await + .expect("rocket failed to ignite"); + for path in [ + "/prpc/Info", + "/prpc/Worker.Worker.Info", + "/prpc/v0/Worker.Worker.GetAttestationForAppKey", + "/%70rpc/Version", + "/prpc/v1/Health", + "/nope", + ] { + let (status, _) = get(&client, path).await; + assert_eq!(status, 401, "{path}"); + } + } + + /// A token from the app's own issuer opens exactly its scopes. + #[tokio::test] + async fn an_internal_token_grants_only_its_own_scopes() { + use dstack_api_auth::jwt; + + let (state, _guard) = setup_test_state().await; + let app_id = hex::encode(&state.identity().await.unwrap().app_id); + let secret = ra_tls::api_v1::derive_app_key( + &crate::rpc_service::tests::DUMMY_K256_KEY, + jwt::INTERNAL_ISSUER_DOMAIN, + ra_tls::api_v1::KeyAlgorithm::Ed25519, + ) + .unwrap(); + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs(); + let token = jwt::sign( + &secret, + &jwt::Claims { + iss: jwt::internal_issuer(&app_id), + aud: app_id, + iat: now, + exp: now + 60, + scope: jwt::scope::INFO.into(), + }, + ) + .unwrap(); + let client = Client::tracked( + mount_external(rocket::build()) + .mount("/", dstack_api_auth::routes()) + .attach(ScopeFairing::new(state.clone())) + .manage(state), + ) + .await + .expect("rocket failed to ignite"); + let auth = rocket::http::Header::new("Authorization", format!("bearer {token}")); + for (path, expected) in [("/prpc/Info", 200), ("/prpc/Version", 401)] { + let response = client.get(path).header(auth.clone()).dispatch().await; + assert_eq!(response.status().code, expected, "{path}"); + } + } } diff --git a/dstack/ra-rpc/src/rocket_helper.rs b/dstack/ra-rpc/src/rocket_helper.rs index b17dce88c..b3b93d6a9 100644 --- a/dstack/ra-rpc/src/rocket_helper.rs +++ b/dstack/ra-rpc/src/rocket_helper.rs @@ -527,7 +527,9 @@ pub async fn handle_prpc_impl>( method_trim_prefix, data, } = args; - let method = method.trim_start_matches(method_trim_prefix.unwrap_or_default()); + let method = method + .strip_prefix(method_trim_prefix.unwrap_or_default()) + .unwrap_or(method); let is_json = request.wants_json(); let info = request .certificate From 476cd6b202c32f1e2c24815abce3f2ae06a701d2 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Thu, 22 Oct 2026 19:02:41 -0700 Subject: [PATCH 3/4] guest-agent: refuse a malformed token before paying for identity The scope fairing resolved the app id before looking at the token, so any string in an Authorization header could drive a boot attestation and queue behind attest_lock. Split `jwt::decode` out of `Verifier::verify` so the fairing reads the header and claims first, and only then asks for the app id. `verify_decoded` checks the signature before the claims: a claim error from an unsigned token describes a guess at the caller's intent, not the token. Also: - the dashboard skips `Worker.Info` unless a block would render it, so an anonymous page view no longer costs an attestation; - the watchdog builds one HTTP client instead of one per heartbeat; - `EffectiveScopes` answers 401, not 403, when the fairing published nothing; - the gateway drops an `agent_token` longer than 4 KiB with a warning: it is self-declared, persisted and synced, and must not be sized by the CVM. Co-Authored-By: Claude Code Signed-off-by: Kevin Wang --- dstack/crates/api-auth/src/jwt.rs | 125 +++++++++++++++++++------- dstack/gateway/src/main_service.rs | 30 ++++++- dstack/guest-agent/src/api_auth.rs | 53 +++++++++-- dstack/guest-agent/src/http_routes.rs | 21 +++-- dstack/guest-agent/src/models.rs | 53 +++++++++++ dstack/guest-agent/src/server.rs | 8 +- dstack/ra-rpc/tests/status_codes.rs | 11 +++ 7 files changed, 253 insertions(+), 48 deletions(-) diff --git a/dstack/crates/api-auth/src/jwt.rs b/dstack/crates/api-auth/src/jwt.rs index 6f1359981..855af3150 100644 --- a/dstack/crates/api-auth/src/jwt.rs +++ b/dstack/crates/api-auth/src/jwt.rs @@ -145,6 +145,48 @@ pub fn internal_issuer(app_id_hex: &str) -> String { format!("dstack:guest-api:{app_id_hex}") } +/// A token whose shape and claims have been read, with the signature still +/// unchecked. +/// +/// Split out from verification so a caller can refuse a malformed token before +/// it does anything expensive. The guest agent needs exactly that: checking a +/// token it was handed must not cost a boot attestation. +#[derive(Clone, Debug)] +pub struct Decoded { + signing_input: String, + signature: Vec, + pub claims: Claims, +} + +/// Read a token's header and claims without verifying it. +/// +/// Everything here is cheap and local. The result says nothing about whether +/// the token was signed by anyone; [`Verifier::verify_decoded`] answers that. +pub fn decode(token: &str) -> Result { + let mut parts = token.split('.'); + let (Some(header), Some(encoded_payload), Some(encoded_signature), None) = + (parts.next(), parts.next(), parts.next(), parts.next()) + else { + bail!("token is not a JWT"); + }; + if header != HEADER { + // Covers `alg: none`, HMAC, and any header carrying key material. + bail!("unsupported JWT header"); + } + let payload = URL_SAFE_NO_PAD + .decode(encoded_payload) + .context("payload is not base64url")?; + let claims = serde_json::from_slice(&payload).context("malformed claims")?; + let signature = URL_SAFE_NO_PAD + .decode(encoded_signature) + .context("signature is not base64url")?; + Ok(Decoded { + signing_input: format!("{header}.{encoded_payload}"), + signature, + claims, + }) +} + /// Mint a token. `signing_key` is the raw 32-byte Ed25519 seed. pub fn sign(signing_key: &[u8; 32], claims: &Claims) -> Result { let payload = serde_json::to_vec(claims).context("failed to serialize claims")?; @@ -184,20 +226,37 @@ impl Verifier { /// `now` is seconds since the UNIX epoch, passed in rather than read here /// so the expiry rules can be tested. pub fn verify(&self, token: &str, app_id_hex: &str, now: u64) -> Result { - let mut parts = token.split('.'); - let (Some(header), Some(encoded_payload), Some(signature), None) = - (parts.next(), parts.next(), parts.next(), parts.next()) - else { - bail!("token is not a JWT"); - }; - if header != HEADER { - // Covers `alg: none`, HMAC, and any header carrying key material. - bail!("unsupported JWT header"); + self.verify_decoded(&decode(token)?, app_id_hex, now) + } + + /// Check the signature and claims of a token [`decode`] already read. + /// + /// Separate from [`Verifier::verify`] so a caller can pay for [`decode`] + /// alone -- it needs no key material and no app id -- and refuse the token + /// before doing anything that costs. + /// + /// The signature is checked before the claims: a claim error from an + /// unsigned token is a guess about a caller's intent, not a fact. + pub fn verify_decoded(&self, token: &Decoded, app_id_hex: &str, now: u64) -> Result { + let Decoded { + signing_input, + signature, + claims, + } = token; + let signature = + Signature::from_slice(signature).context("signature has the wrong length")?; + + let keys = self.keys_for(&claims.iss, app_id_hex); + if keys.is_empty() { + bail!("unknown token issuer"); + } + if !keys.iter().any(|key| { + key.verify_strict(signing_input.as_bytes(), &signature) + .is_ok() + }) { + bail!("token signature does not verify"); } - let payload = URL_SAFE_NO_PAD - .decode(encoded_payload) - .context("payload is not base64url")?; - let claims: Claims = serde_json::from_slice(&payload).context("malformed claims")?; + if claims.aud != app_id_hex { bail!("token was issued for a different application"); } @@ -210,23 +269,6 @@ impl Verifier { if claims.iat > now + CLOCK_LEEWAY_SECS { bail!("token was issued in the future"); } - let signature = URL_SAFE_NO_PAD - .decode(signature) - .context("signature is not base64url")?; - let signature = - Signature::from_slice(&signature).context("signature has the wrong length")?; - let signing_input = format!("{header}.{encoded_payload}"); - - let keys = self.keys_for(&claims.iss, app_id_hex); - if keys.is_empty() { - bail!("unknown token issuer"); - } - if !keys.iter().any(|key| { - key.verify_strict(signing_input.as_bytes(), &signature) - .is_ok() - }) { - bail!("token signature does not verify"); - } Ok(claims.scopes()) } @@ -353,6 +395,29 @@ mod tests { assert!(verifier(1).verify(&future, APP_ID, 1000).is_err()); } + #[test] + fn decoding_refuses_a_shape_no_token_could_have() { + for token in [ + "not-a-token", + "a.b.c.d", + "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0.eyJpc3MiOiJ4In0.", + "eyJhbGciOiJFZERTQSIsInR5cCI6IkpXVCJ9.!!!.c2ln", + ] { + assert!(decode(token).is_err(), "{token}"); + } + // A well-formed token decodes without any key: that is what lets the + // caller refuse it before doing work. + let token = mint( + 1, + &internal_issuer(APP_ID), + APP_ID, + 1000, + 1300, + "guest:info", + ); + assert_eq!(decode(&token).unwrap().claims.aud, APP_ID); + } + #[test] fn accepts_an_external_issuer_by_name() { let verifier = Verifier::new( diff --git a/dstack/gateway/src/main_service.rs b/dstack/gateway/src/main_service.rs index 7c931a430..1761fe84e 100644 --- a/dstack/gateway/src/main_service.rs +++ b/dstack/gateway/src/main_service.rs @@ -2757,6 +2757,33 @@ fn gateway_nodes(kv_store: &KvStore, exclude_down: bool) -> Vec .collect() } +/// Longest `agent_token` a registration may carry. +/// +/// The token is a short Ed25519 JWT -- a few hundred bytes. The field is +/// self-declared, persisted, synced to every peer and sent back out as a +/// request header, so it is bounded here rather than trusted. +const MAX_AGENT_TOKEN_BYTES: usize = 4096; + +/// The `agent_token` a registration may store, or `None`. +/// +/// Absent and empty both mean "this CVM has no token", which is what a CVM +/// that predates the field or could not mint one reports. A value past +/// [`MAX_AGENT_TOKEN_BYTES`] is not a token at all: it is dropped with a +/// warning rather than stored, so one CVM cannot inflate every record. +fn validated_agent_token(instance_id: &str, token: Option) -> Option { + match token { + Some(token) if token.len() > MAX_AGENT_TOKEN_BYTES => { + warn!( + "[{instance_id}] ignoring an agent token of {} bytes (limit {MAX_AGENT_TOKEN_BYTES})", + token.len() + ); + None + } + Some(token) if token.is_empty() => None, + token => token, + } +} + pub struct RpcHandler { remote_app_id: Option>, remote_app_info: Option, @@ -2817,6 +2844,7 @@ impl GatewayRpc for RpcHandler { }) }) .transpose()?; + let agent_token = validated_agent_token(&instance_id, request.agent_token); self.state .register_cvm_async( app_id, @@ -2827,7 +2855,7 @@ impl GatewayRpc for RpcHandler { port_policy, // A real CVM always states its intent, either way. health_check: Some(request.health_check), - agent_token: request.agent_token.filter(|token| !token.is_empty()), + agent_token, }, ) .await diff --git a/dstack/guest-agent/src/api_auth.rs b/dstack/guest-agent/src/api_auth.rs index 907f6c0f4..f247a5a22 100644 --- a/dstack/guest-agent/src/api_auth.rs +++ b/dstack/guest-agent/src/api_auth.rs @@ -6,7 +6,7 @@ //! //! Every request that reaches `[external]` names a scope, and the caller has //! to be granted it: either by an Ed25519 JWT, or -- when the host configured -//! it -- by being anonymous. Nothing is granted by default. +//! it -- by being anonymous. Nothing is granted without one or the other. //! //! This is deliberately not a trust boundary. The measured `public_*` switches //! in app-compose still decide whether confidential data (logs, sysinfo, TCB @@ -17,7 +17,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use anyhow::{Context, Result}; -use dstack_api_auth::jwt::{self, IssuerKeys, ScopeSet, Verifier}; +use dstack_api_auth::jwt::{self, Decoded, IssuerKeys, ScopeSet, Verifier}; use dstack_types::GuestAgentApiConfig; use ed25519_dalek::VerifyingKey; use rocket::{ @@ -94,7 +94,7 @@ impl GuestApiAuth { /// authenticate and got it wrong should see a refusal, not whatever the /// host happened to leave open. `app_id` is `None` while the app identity /// is unavailable, in which case no token can be checked. - fn resolve(&self, token: Option<&str>, app_id: Option<&str>) -> Result { + fn resolve(&self, token: Option<&Decoded>, app_id: Option<&str>) -> Result { let Some(token) = token else { return Ok(self.anonymous.clone()); }; @@ -105,7 +105,7 @@ impl GuestApiAuth { .unwrap_or(0); let granted = self .verifier - .verify(token, app_id, now) + .verify_decoded(token, app_id, now) .context("rejected bearer token")?; Ok(self.anonymous.union(&granted)) } @@ -139,7 +139,11 @@ impl<'r> FromRequest<'r> for EffectiveScopes { async fn from_request(request: &'r Request<'_>) -> Outcome { match request.local_cache(|| None::) { Some(scopes) => Outcome::Success(EffectiveScopes(scopes.clone())), - None => Outcome::Forward(rocket::http::Status::Forbidden), + // Unreachable: `ScopeFairing` either publishes a set or rewrites + // the request to the 401 route. Kept as a 401 rather than a 403 so + // a request that somehow got past the fairing still looks like + // every other refusal. + None => Outcome::Error((rocket::http::Status::Unauthorized, ())), } } } @@ -206,7 +210,17 @@ impl Fairing for ScopeFairing { } async fn on_request(&self, req: &mut Request<'_>, _: &mut Data<'_>) { - let token = bearer_token(req); + // Read the token's shape first. It is the only cheap step, and it is + // the one an unauthenticated caller can drive; the app id lookup below + // can cost a boot attestation, so nothing that arrives here gets that + // far unless it is at least a well-formed Ed25519 token. + let token = match bearer_token(req).map(jwt::decode).transpose() { + Ok(token) => token, + Err(err) => { + tracing::debug!("{err:#}"); + return deny(req); + } + }; // Only a token needs the app id. Going through `identity()` rather // than a value bound at startup is what lets a boot whose first decode // failed recover: the retry is driven by the callers that need it. @@ -219,10 +233,16 @@ impl Fairing for ScopeFairing { .map(|identity| hex::encode(&identity.app_id)), None => None, }; - let scopes = match self.state.api_auth().resolve(token, app_id.as_deref()) { + let scopes = match self + .state + .api_auth() + .resolve(token.as_ref(), app_id.as_deref()) + { Ok(scopes) => scopes, Err(err) => { - warn!("{err:#}"); + // Every caller on the internet reaches this. A warning per + // attempt is a log the public gets to size. + tracing::debug!("{err:#}"); return deny(req); } }; @@ -301,6 +321,13 @@ mod tests { .unwrap() } + /// Stands in for a token that is well-formed but not signed by anything + /// this agent trusts. + fn decoded(claims: jwt::Claims) -> Decoded { + let token = jwt::sign(&[1u8; 32], &claims).unwrap(); + jwt::decode(&token).unwrap() + } + #[test] fn anonymous_grants_only_what_was_configured() { assert!(auth(&[]).resolve(None, None).unwrap().is_empty()); @@ -311,8 +338,16 @@ mod tests { #[test] fn a_bad_token_is_refused_rather_than_downgraded_to_anonymous() { + let claims = jwt::Claims { + iss: jwt::internal_issuer("00"), + aud: "00".into(), + iat: 0, + exp: u64::MAX / 2, + scope: jwt::scope::DASHBOARD.into(), + }; + // Signed by the wrong key, so it decodes and fails to verify. assert!(auth(&["guest:dashboard"]) - .resolve(Some("not-a-token"), Some("00")) + .resolve(Some(&decoded(claims)), Some("00")) .is_err()); } diff --git a/dstack/guest-agent/src/http_routes.rs b/dstack/guest-agent/src/http_routes.rs index 4a09aac90..dc59c674a 100644 --- a/dstack/guest-agent/src/http_routes.rs +++ b/dstack/guest-agent/src/http_routes.rs @@ -51,8 +51,11 @@ async fn index( let show_logs = public_logs && scopes.allows(jwt::scope::LOGS); let show_tcb = public_tcbinfo && show_identity; let context = CallContext::builder().state(&**state).build(); - let handler = ExternalRpcHandler::construct(context.clone()) - .map_err(|e| format!("Failed to construct RPC handler: {}", e))?; + // `Info` costs a boot attestation (and may start a demo-cert request), so + // it is only asked for when the identity block -- and with it the TCB + // block, which requires `guest:info` too -- will be rendered. + // `guest:dashboard` is the one scope an anonymous visitor is most likely + // to be given, and on its own it renders every block hidden. let AppInfo { app_name, app_id, @@ -67,10 +70,16 @@ async fn index( vm_config: _, cloud_vendor, cloud_product, - } = handler - .info() - .await - .map_err(|e| format!("Failed to get worker info: {}", e))?; + } = if show_identity || show_tcb { + let handler = ExternalRpcHandler::construct(context.clone()) + .map_err(|e| format!("Failed to construct RPC handler: {}", e))?; + handler + .info() + .await + .map_err(|e| format!("Failed to get worker info: {}", e))? + } else { + Default::default() + }; // Only collected when the page will actually render it. Collecting // otherwise would scan every process and disk, and spawn a GPU collector, diff --git a/dstack/guest-agent/src/models.rs b/dstack/guest-agent/src/models.rs index 89f9624cc..7b86f599c 100644 --- a/dstack/guest-agent/src/models.rs +++ b/dstack/guest-agent/src/models.rs @@ -410,6 +410,59 @@ mod tests { assert!(body.contains("system_load_average_1m 0.40"), "{body}"); } + /// A caller granted only `guest:dashboard` gets the page and nothing on + /// it. Each block is guarded independently, so a missing guard shows up as + /// data in an otherwise empty document rather than as a stray heading. + #[test] + fn a_dashboard_granted_no_block_renders_none_of_them() { + let (app_id, instance_id, device_id) = ([0xd3u8; 32], [0xe5u8; 32], [0xf7u8; 32]); + let html = Dashboard { + app_name: "secret-app".into(), + app_id: app_id.to_vec(), + instance_id: instance_id.to_vec(), + device_id: device_id.to_vec(), + key_provider_info: "secret-kms".into(), + tcb_info: "secret-tcb".into(), + containers: vec![Container { + names: vec!["secret-container".into()], + status: "running".into(), + ..Default::default() + }], + system_info: SystemInfo { + os_name: "secret-os".into(), + cpu_model: "secret-cpu".into(), + ..Default::default() + }, + show_identity: false, + show_sysinfo: false, + show_containers: false, + show_logs: false, + show_tcb: false, + cloud_vendor: "secret-vendor".into(), + cloud_product: "secret-product".into(), + gpu_info: Default::default(), + } + .render() + .expect("render"); + for hidden in [ + "secret-app".to_owned(), + hex::encode(app_id), + hex::encode(instance_id), + hex::encode(device_id), + "secret-kms".into(), + "secret-tcb".into(), + "secret-container".into(), + "secret-os".into(), + "secret-cpu".into(), + "secret-vendor".into(), + "Deployed Containers".into(), + "TCB Info".into(), + "Node Information".into(), + ] { + assert!(!html.contains(&hidden), "{hidden} leaked into {html}"); + } + } + fn dashboard_with(gpu_info: GpuInfoResponse) -> String { Dashboard { app_name: String::new(), diff --git a/dstack/guest-agent/src/server.rs b/dstack/guest-agent/src/server.rs index 4590cc82f..77fb6214a 100644 --- a/dstack/guest-agent/src/server.rs +++ b/dstack/guest-agent/src/server.rs @@ -242,13 +242,17 @@ async fn run_watchdog(port: u16) { let mut interval = tokio::time::interval(heatbeat_interval); let probe_url = format!("http://localhost:{port}/prpc/Worker.Version"); + // Built once: the loop runs every few seconds for the process's lifetime, + // and a fresh client per tick leaks a connection pool each time. + let client = reqwest::Client::new(); loop { interval.tick().await; - let client = reqwest::Client::new(); match client.get(&probe_url).send().await { // 401 is the listener answering a caller with no scope, which is - // the default; it proves the listener is up as well as a 200. + // what happens when the host removes `guest:version` from + // `anonymous_scopes`; it proves the listener is up as well as a + // 200 does. Ok(response) if response.status().is_success() || response.status() == reqwest::StatusCode::UNAUTHORIZED => diff --git a/dstack/ra-rpc/tests/status_codes.rs b/dstack/ra-rpc/tests/status_codes.rs index 1ce44f467..80e12f74c 100644 --- a/dstack/ra-rpc/tests/status_codes.rs +++ b/dstack/ra-rpc/tests/status_codes.rs @@ -143,6 +143,17 @@ async fn an_unknown_method_is_reported_as_not_found() { assert!(body.contains("Service not found: NoSuchMethod"), "{body}"); } +#[tokio::test] +async fn a_repeated_service_prefix_is_not_stripped() { + // Exactly one prefix goes, so a caller cannot spell `Echo` in a path the + // routes do not name -- a front-end that keys a check on the route would + // otherwise let the request through. + let client = client().await; + let (status, body) = post(&client, "Demo.Demo.Echo", r#"{"text":"hello"}"#).await; + assert_eq!(status, 404, "{body}"); + assert!(body.contains("Service not found: Demo.Echo"), "{body}"); +} + #[tokio::test] async fn a_malformed_body_is_reported_as_a_bad_request() { let client = client().await; From 2d75cd1956d1bdcf208fedce4929d24672d83704 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Thu, 22 Oct 2026 19:02:43 -0700 Subject: [PATCH 4/4] guest-agent: document where anonymous scopes are configured There is no built-in anonymous default, so the sample configs are the only place a host learns what to open. Fill in `[cvm.guest_agent_api]` in the shipped `vmm.toml`, in the `dstackup install` renderer, and in the two dev/test stacks with what an unconfigured 8090 used to serve: the dashboard plus the identity, version and health it publishes. Co-Authored-By: Claude Code Signed-off-by: Kevin Wang --- CHANGELOG.md | 2 +- docs/guest-api-auth.md | 50 +++++++++++++++---- dstack/crates/dstack-cli-core/src/config.rs | 6 +++ dstack/dstack-types/src/lib.rs | 7 +-- dstack/vmm/vmm.toml | 29 ++++++----- .../scripts/render-config.sh | 3 ++ tools/dev-stack.sh | 4 ++ 7 files changed, 75 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b2a07773..d7c01b43a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - guest: a fresh CVM with a verity volume could take the volume for its data disk and try to LUKS-format it: volumes labelled their data partition `dstack-data`, the data disk's label. `dstack verity` now labels it `dstack-volume-data`, and the guest skips disks that carry a `dstack-volume` partition, so volumes built by older releases are safe too ### Changed -- guest-agent: the external listener (port 8090) is no longer anonymous. Every route needs a scope, granted by a signed Ed25519 JWT or by `[cvm.guest_agent_api].anonymous_scopes` in sys-config; the gateway gets its token minted inside the CVM from the app root key. `public_logs` / `public_sysinfo` / `public_tcbinfo` are unchanged and still decide what may leave the TEE. The dashboard is filtered block by block by scope. See `docs/guest-api-auth.md`. An old gateway sees 401s and drops new guests from rotation until it is upgraded +- guest-agent: the external listener (port 8090) is no longer anonymous. Every route needs a scope, granted by a signed Ed25519 JWT or by `[cvm.guest_agent_api].anonymous_scopes` in sys-config. There is no built-in anonymous default; the sample `vmm.toml` opens `guest:dashboard`, `guest:info`, `guest:version` and `guest:health`, which is what an unconfigured 8090 used to serve, while containers, logs, metrics and app-key attestation stay behind a token. The gateway gets its token minted inside the CVM from the app root key. `public_logs` / `public_sysinfo` / `public_tcbinfo` are unchanged and still decide what may leave the TEE. The dashboard is filtered block by block by scope. See `docs/guest-api-auth.md`. An old gateway sees 401s and drops new guests from rotation until it is upgraded - vmm: `qmp_socket` now defaults to `true`. Each CVM's QEMU exposes `qmp.sock` in its work dir, so the host can change disk options or compact a bloated `hda.img` (`blockdev-mirror`) without stopping the CVM. Applies to CVMs started after the change; set `qmp_socket = false` to opt out ## [0.6.1] - 2026-10-12 diff --git a/docs/guest-api-auth.md b/docs/guest-api-auth.md index c6c745186..3a665f5b4 100644 --- a/docs/guest-api-auth.md +++ b/docs/guest-api-auth.md @@ -33,13 +33,29 @@ not passed to the router. ## Default: nothing anonymous -With no configuration, every scope above requires a token. The gateway still -works, because it is issued one internally; the dashboard, `/metrics` and the -log views stop answering to the public. - -The app's own SDKs are unaffected: they use the internal Unix socket, which is +With no `[cvm.guest_agent_api]` block, an anonymous caller is granted nothing: +the dashboard, its identity block, `/metrics` and the log views all need a +token. The gateway still works, because it is issued one internally. The app's +own SDKs are unaffected: they use the internal Unix socket, which is unauthenticated and reachable only by the application. +The sample `vmm.toml` ships the block filled in with what an unconfigured 8090 +used to serve, so a host that copies the sample keeps the page it had: + +| Scope | Effect | +|---|---| +| `guest:version` | `Worker.Version` answers | +| `guest:health` | `Worker.Health` answers | +| `guest:info` | the identity block renders | +| `guest:dashboard` | the page loads at all | + +Everything else is about the app's *contents* and stays behind a token even in +that sample: the container table, `/metrics`, the log views and +`Worker.GetAttestationForAppKey`. + +The dashboard does not fetch `Info` unless a block would show it, so an +anonymous visitor granted only `guest:dashboard` costs the TEE no attestation. + ## Configuring anonymous access Anonymous access is a host decision, made in sys-config. The VMM writes it from @@ -47,7 +63,7 @@ Anonymous access is a host decision, made in sys-config. The VMM writes it from ```toml [cvm.guest_agent_api] -anonymous_scopes = ["guest:dashboard", "guest:version", "guest:health"] +anonymous_scopes = ["guest:dashboard", "guest:version", "guest:health", "guest:info"] ``` The same block in the guest's `/dstack/.host-shared/.sys-config.json`: @@ -55,11 +71,15 @@ The same block in the guest's `/dstack/.host-shared/.sys-config.json`: ```json { "guest_agent_api": { - "anonymous_scopes": ["guest:dashboard", "guest:version", "guest:health"] + "anonymous_scopes": ["guest:dashboard", "guest:version", "guest:health", "guest:info"] } } ``` +The list is whole, not additive, and there is no built-in default to fall back +on: omitting `anonymous_scopes`, or leaving the list empty, closes the listener +to anonymous callers. Removing the block entirely does the same. + Listing `guest:dashboard` without `guest:containers`, `guest:info`, `guest:logs` or `guest:metrics` gives an anonymous visitor the page with every block hidden. Scopes are independent: each one adds exactly its own block. @@ -92,6 +112,9 @@ it over on registration, bound to the app id and short-lived: 1. The agent and `dstack-util`, both inside the CVM, derive the same Ed25519 key: `derive_app_key(app_root_key, "dstack:guest-api:issuer:v1", Ed25519)`. + That domain string is final: it is a wire contract between `dstack-util` and + the agent, which are upgraded together but may briefly disagree, so it is + never versioned again. 2. `dstack-util` signs a token with `iss = "dstack:guest-api:"`, `aud = `, a 10-minute lifetime, and only the `guest:health` scope. 3. Registration carries it to the gateway, which presents it on the v1 `Health` @@ -135,8 +158,8 @@ host cannot alter without changing the app id. ## Compatibility -A guest with this change answers 401 to an old gateway that presents no token. The -gateway counts that as unreachable and drops the instance after +A guest with this change answers 401 to an old gateway that presents no token. +The gateway counts that as unreachable and drops the instance after `failure_threshold` consecutive polls, so a mixed cluster rebalances away from new guests until the gateway is upgraded. @@ -144,5 +167,12 @@ During the rollout, either upgrade the gateway first, or leave the host open: ```toml [cvm.guest_agent_api] -anonymous_scopes = ["guest:version", "guest:health"] +anonymous_scopes = ["guest:version", "guest:health", "guest:info", "guest:dashboard"] ``` + +The reverse direction is quieter. A new guest registers with an old gateway +node that does not store `agent_token`; when that record syncs to an upgraded +node, the missing token means the upgraded node's health probe is refused too. +The instance drops out of rotation while `order_by_stickiness` keeps routing +user traffic to it. Upgrade every gateway node in the cluster before rolling +guests, not just the one the VMM talks to. diff --git a/dstack/crates/dstack-cli-core/src/config.rs b/dstack/crates/dstack-cli-core/src/config.rs index 2618bc6d9..0f885265e 100644 --- a/dstack/crates/dstack-cli-core/src/config.rs +++ b/dstack/crates/dstack-cli-core/src/config.rs @@ -348,6 +348,12 @@ qemu_hotplug_off = true host_share_mode = "{host_share_mode}" qgs_port = 4050 +# What an unconfigured 8090 serves: the dashboard, and the identity, version +# and health it publishes. Containers, logs, metrics and app-key attestation +# need a token. See docs/guest-api-auth.md. +[cvm.guest_agent_api] +anonymous_scopes = ["guest:dashboard", "guest:version", "guest:health", "guest:info"] + [cvm.product] sys_vendor = "dstack" product_name = "dstack" diff --git a/dstack/dstack-types/src/lib.rs b/dstack/dstack-types/src/lib.rs index ccf2ed02f..68369be18 100644 --- a/dstack/dstack-types/src/lib.rs +++ b/dstack/dstack-types/src/lib.rs @@ -1255,7 +1255,7 @@ pub struct SysConfig { #[serde(default, skip_serializing_if = "Option::is_none")] pub mr_config: Option, /// Access control for the guest agent's external listener. Absent means - /// the code defaults: nothing anonymous, no external issuers. + /// nothing anonymous and no external issuers. /// /// Host-supplied, and deliberately not measured: it decides who may *ask* /// the agent, never whether data may leave the TEE. Confidentiality stays @@ -1273,8 +1273,9 @@ pub struct GuestAgentApiConfig { /// Scopes granted to a caller that presents no token. /// /// Empty by default: an app that wants its dashboard on the internet has - /// to say so. This is an exposure choice, not a trust one -- the anonymous - /// caller is the same party a public 8090 already served. + /// to say so, in `vmm.toml` or in the guest's sys-config. What a host is + /// expected to open is spelled out in the sample `vmm.toml` rather than + /// here, so the shipped default stays visible where an operator sets it. #[serde(default)] pub anonymous_scopes: Vec, /// Issuers other than the app's own root key. diff --git a/dstack/vmm/vmm.toml b/dstack/vmm/vmm.toml index 680dbc8fb..6db4441f6 100644 --- a/dstack/vmm/vmm.toml +++ b/dstack/vmm/vmm.toml @@ -109,18 +109,6 @@ tdx_attestation_variant = "auto" host_share_mode = "9p" -# Access control for the guest agent's external listener (port 8090). Absent -# means nothing is anonymous: only holders of a token (the gateway, minted -# inside the CVM) can call it. This is an exposure choice, not a -# confidentiality one -- what may leave the TEE is still decided by the -# measured public_logs / public_sysinfo / public_tcbinfo switches in -# app-compose. See docs/guest-api-auth.md. -# [cvm.guest_agent_api] -# anonymous_scopes = ["guest:dashboard", "guest:version", "guest:health"] -# [[cvm.guest_agent_api.external_issuers]] -# name = "ops" -# public_keys = ["<32-byte ed25519 public key, hex>"] - # Default preallocation for a new CVM data disk. This is a host storage # policy -- the guest cannot see it and it does not affect measurements -- and # a deploy request may pick another mode per VM. @@ -317,6 +305,23 @@ sanitize_on_attach = false # Maximum time for all reset GPUs to become continuously VFIO-ready. sbr_timeout_ms = 10000 +# Access control for the guest agent's external listener (port 8090). Every +# route needs a scope, granted by a signed Ed25519 JWT or by the anonymous list +# below. This is an exposure choice, not a confidentiality one -- what may leave +# the TEE is still decided by the measured public_logs / public_sysinfo / +# public_tcbinfo switches in app-compose. See docs/guest-api-auth.md. +# +# The list is whole, not additive, and there is no built-in anonymous default: +# comment the whole block out to refuse anonymous callers entirely. The set +# below is what an unconfigured 8090 used to serve -- the dashboard, and the +# identity, version and health it publishes. Containers, logs, metrics and +# app-key attestation need a token either way. +[cvm.guest_agent_api] +anonymous_scopes = ["guest:dashboard", "guest:version", "guest:health", "guest:info"] +# [[cvm.guest_agent_api.external_issuers]] +# name = "ops" +# public_keys = ["<32-byte ed25519 public key, hex>"] + [gateway] base_domain = "localhost" port = 8082 diff --git a/test-suites/full-stack-compose/scripts/render-config.sh b/test-suites/full-stack-compose/scripts/render-config.sh index 3a585d6ca..5f1c4c946 100755 --- a/test-suites/full-stack-compose/scripts/render-config.sh +++ b/test-suites/full-stack-compose/scripts/render-config.sh @@ -216,6 +216,9 @@ exclude = [] include = [] allow_attach_all = false +[cvm.guest_agent_api] +anonymous_scopes = ["guest:dashboard", "guest:version", "guest:health", "guest:info"] + [gateway] base_domain = "${BASE_DOMAIN}" port = ${GATEWAY1_PROXY_HOST_PORT} diff --git a/tools/dev-stack.sh b/tools/dev-stack.sh index cb43aa8cf..9884c88bf 100755 --- a/tools/dev-stack.sh +++ b/tools/dev-stack.sh @@ -261,6 +261,10 @@ kms_urls = ["https://$KMS_DOMAIN:$KMS_RPC_LISTEN_PORT"] gateway_urls = ["https://$GATEWAY_DOMAIN:$GATEWAY_RPC_LISTEN_PORT"] cid_start = $VMM_CID_POOL_START cid_pool_size = $VMM_CID_POOL_SIZE + +[cvm.guest_agent_api] +anonymous_scopes = ["guest:dashboard", "guest:version", "guest:health", "guest:info"] + [cvm.port_mapping] enabled = $VMM_PORT_MAPPING_ENABLED address = "127.0.0.1"