From 9603578477ca9e38ae43a0343412ce5c0a04dc26 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 10:29:50 +0000 Subject: [PATCH 1/8] fix(stack-kms): only trust a CipherStash-hosted ZeroKMS from a token claim StackKms took the ZeroKMS endpoint from the access token's `services` claim, which nothing verifies, and pinned the first value it saw for the connection's lifetime. Any https or http host was accepted. A token from a cookie-backed TokenStore is attacker-written, so its author could redirect key operations (and, on a shared client, every later caller's bearer token) to a host of their choosing. A claimed endpoint must now be https, on the default port, on `..viturhosted.net` for a region cts-common knows (InvalidEndpoint::NotHosted otherwise). Every later token must name the endpoint the first one did (InvalidEndpoint::Mismatch otherwise), so a token is never sent to an endpoint other than its own and cannot move the pin. Whether the endpoint was configured explicitly is decided once at connect, not per call, so a concurrent first caller cannot race it. Explicit configuration (`with_base_url`, `CS_ZEROKMS_HOST`) is unchanged and may still name any usable endpoint, plain http included. Refs CIP-5152 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012Jmn7o2Eouw3CQpnBNGrwC --- Cargo.lock | 1 + languages/golang/encrypt/credentials.go | 3 +- languages/golang/encrypt/guest/Cargo.lock | 1 + packages/eql/Cargo.lock | 1 + packages/stack-encrypt/fuzz/Cargo.lock | 1 + packages/stack-kms/Cargo.toml | 2 + packages/stack-kms/fuzz/Cargo.lock | 1 + packages/stack-kms/src/builder.rs | 5 +- packages/stack-kms/src/client.rs | 276 +++++++++++++++++++++- packages/stack-kms/src/connection/http.rs | 7 +- packages/stack-kms/src/endpoint.rs | 177 +++++++++++++- packages/stack-kms/src/errors.rs | 5 + 12 files changed, 461 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6964dbbf09..0824d92cb0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3371,6 +3371,7 @@ dependencies = [ "base16ct", "base64ct", "blake3", + "cts-common", "futures", "lazy_static", "miette", diff --git a/languages/golang/encrypt/credentials.go b/languages/golang/encrypt/credentials.go index 50caf30024..c758f46386 100644 --- a/languages/golang/encrypt/credentials.go +++ b/languages/golang/encrypt/credentials.go @@ -404,7 +404,8 @@ func clientKeyFromProfile(ctx context.Context, profile *auth.ProfileStore, noPro // zerokmsEndpoint is the endpoint NewClient pins, in stack-kms's order: the // explicit URL, else the first of CS_ZEROKMS_HOST and CS_VITUR_HOST that is -// set, else none (the token's services claim decides on first use). A set +// set, else none (the token's services claim decides on first use, and may +// only name a CipherStash-hosted ZeroKMS: https://..viturhosted.net). A set // variable that is not a usable endpoint is an error, not a fall-through: // falling through would send key operations somewhere the operator did not // configure. The guest validates the URL in full; this check exists to name diff --git a/languages/golang/encrypt/guest/Cargo.lock b/languages/golang/encrypt/guest/Cargo.lock index 7f542b6ded..6bf4f812db 100644 --- a/languages/golang/encrypt/guest/Cargo.lock +++ b/languages/golang/encrypt/guest/Cargo.lock @@ -2094,6 +2094,7 @@ dependencies = [ "base16ct", "base64ct", "blake3", + "cts-common", "futures", "miette", "opaque-debug", diff --git a/packages/eql/Cargo.lock b/packages/eql/Cargo.lock index 5c90c00475..f11b03237c 100644 --- a/packages/eql/Cargo.lock +++ b/packages/eql/Cargo.lock @@ -4159,6 +4159,7 @@ dependencies = [ "base16ct", "base64ct", "blake3", + "cts-common 0.43.0", "futures", "miette", "opaque-debug", diff --git a/packages/stack-encrypt/fuzz/Cargo.lock b/packages/stack-encrypt/fuzz/Cargo.lock index 1663a68989..ad55ba0c62 100644 --- a/packages/stack-encrypt/fuzz/Cargo.lock +++ b/packages/stack-encrypt/fuzz/Cargo.lock @@ -2026,6 +2026,7 @@ dependencies = [ "base16ct", "base64ct", "blake3", + "cts-common", "futures", "miette", "opaque-debug", diff --git a/packages/stack-kms/Cargo.toml b/packages/stack-kms/Cargo.toml index f1687fd494..5fb6e495b9 100644 --- a/packages/stack-kms/Cargo.toml +++ b/packages/stack-kms/Cargo.toml @@ -44,6 +44,8 @@ recipher = { workspace = true } # `http` feature rides on this crate's `http` feature rather than arriving by # default. stack-auth = { workspace = true } +# `ZeroKmsServiceDiscovery`: which hosts a token's `services` claim may name. +cts-common = { workspace = true } zerokms-protocol = { workspace = true } vitaminc = { workspace = true, features = ["protected", "random"] } diff --git a/packages/stack-kms/fuzz/Cargo.lock b/packages/stack-kms/fuzz/Cargo.lock index a18c2eba9f..73f49856b7 100644 --- a/packages/stack-kms/fuzz/Cargo.lock +++ b/packages/stack-kms/fuzz/Cargo.lock @@ -1969,6 +1969,7 @@ dependencies = [ "base16ct", "base64ct", "blake3", + "cts-common", "futures", "miette", "opaque-debug", diff --git a/packages/stack-kms/src/builder.rs b/packages/stack-kms/src/builder.rs index 7d1bca2583..e1a93bd3e5 100644 --- a/packages/stack-kms/src/builder.rs +++ b/packages/stack-kms/src/builder.rs @@ -77,7 +77,10 @@ impl stack_auth::ErrorPayload for StackKmsBuilderError { /// 1. Explicit [`ZeroKmsEndpoint`] via [`with_base_url`](Self::with_base_url) /// 2. `CS_ZEROKMS_HOST` (or legacy `CS_VITUR_HOST`) environment variable — /// the first one that is *set* is used, and an invalid value is an error -/// 3. Automatically from the token's `services` claim +/// 3. Automatically from the token's `services` claim. Only a +/// CipherStash-hosted ZeroKMS (`https://..viturhosted.net`) +/// is accepted there, and every later token must name the same one; any +/// other endpoint must be configured by 1 or 2 /// /// # Example /// diff --git a/packages/stack-kms/src/client.rs b/packages/stack-kms/src/client.rs index 6b8d0bc6dc..7bea0d5316 100644 --- a/packages/stack-kms/src/client.rs +++ b/packages/stack-kms/src/client.rs @@ -1,4 +1,5 @@ use std::borrow::Cow; +use std::sync::OnceLock; use uuid::Uuid; use zerokms_protocol::{ GenerateKeyRequest, GenerateKeySpec, GeneratedKey, IdentifiedBy, Keyset, LoadKeysetRequest, @@ -13,6 +14,7 @@ use vitaminc::random::{Generatable, SafeRand}; #[cfg(feature = "http")] use crate::connection::HttpConnection; use crate::connection::ZeroKMSConnection; +use crate::endpoint::{InvalidEndpoint, ZeroKmsEndpoint}; use crate::errors::{Error, GenerateKeyError, LoadKeysetError, RetrieveKeyError}; use crate::futures::map_async_chunked; use crate::key::{ClientKey, DataKey, DataKeyWithTag, IndexKey}; @@ -410,6 +412,9 @@ pub struct StackKms { client: Client, credentials: C, client_key: ClientKey, + /// `None` when the connection was given its endpoint at init; otherwise + /// the endpoint the first token's `services` claim named, once known. + discovered: Option>, } /// `StackKms` owns the transport [`Client`], a [`stack_auth`] credential @@ -421,6 +426,9 @@ pub struct StackKms { client: Client, credentials: C, client_key: ClientKey, + /// `None` when the connection was given its endpoint at init; otherwise + /// the endpoint the first token's `services` claim named, once known. + discovered: Option>, } impl StackKms @@ -442,33 +450,60 @@ where /// /// The connection is initialised from `opts`'s connection options; the /// ZeroKMS endpoint is taken from the access token's `services` claim on - /// first use unless the connection already knows one. + /// first use unless the connection already knows one. A claimed endpoint + /// must be CipherStash-hosted, and every later token must name the same + /// one; whether the connection already knows one is decided here, once. pub fn connect( opts: ClientOpts, credentials: C, client_key: ClientKey, ) -> Result { - let client = Client::init_opts(opts).map_err(|e| Error::ConnectionInit(Box::new(e)))?; + let client: Client = + Client::init_opts(opts).map_err(|e| Error::ConnectionInit(Box::new(e)))?; + // Decided once, here: asking the connection on every call would race a + // concurrent first caller that has just pinned it from a claim. + let discovered = (!client.connection().has_base_url()).then(OnceLock::new); Ok(Self { client, credentials, client_key, + discovered, }) } - /// Fetch a token from the credentials provider and ensure the ZeroKMS base - /// URL has been resolved on the connection (from the token's `services` - /// claim). The URL is only resolved once; subsequent calls skip resolution. + /// Fetch a token from the credentials provider and make sure the + /// connection sends it to the ZeroKMS endpoint the token is meant for. + /// + /// An endpoint configured on the connection (`with_base_url`, + /// `CS_ZEROKMS_HOST`, or a transport that needs none) is the operator's + /// choice and is used as it is. Otherwise the endpoint comes from the + /// token's `services` claim, which nothing here verifies — a + /// [`TokenStore`](stack_auth::TokenStore) can return a token anyone wrote + /// — so: + /// + /// * the claim may only name a CipherStash-hosted ZeroKMS + /// ([`ZeroKmsEndpoint::from_services_claim`]), and + /// * every later token must name the endpoint the first one did. A token + /// naming another is refused rather than sent to the pinned endpoint, + /// and it cannot move the pin. async fn get_token(&self) -> Result { let token = (&self.credentials).get_token().await?; - if !self.client.connection().has_base_url() { - let endpoint = crate::endpoint::ZeroKmsEndpoint::try_from(token.zerokms_url()?)?; + // Configured explicitly: the claim is not consulted. + let Some(discovered) = &self.discovered else { + return Ok(token); + }; + let claimed = ZeroKmsEndpoint::from_services_claim(token.zerokms_url()?)?; + let pinned = discovered.get_or_init(|| { tracing::debug!( target: "stack_kms", - %endpoint, + endpoint = %claimed, "resolved ZeroKMS endpoint from the token's services claim" ); - self.client.connection().ensure_base_url(endpoint); + self.client.connection().ensure_base_url(claimed.clone()); + claimed.clone() + }); + if *pinned != claimed { + return Err(InvalidEndpoint::Mismatch.into()); } Ok(token) } @@ -708,6 +743,229 @@ mod tests { } } + /// Where a token's `services` claim may send key operations (CIP-5152). + /// The claim is unverified, so it may only name a CipherStash-hosted + /// ZeroKMS, and it cannot move an endpoint already in use. + mod endpoint_from_the_services_claim { + use super::*; + use base64ct::{Base64UrlUnpadded, Encoding}; + use std::collections::VecDeque; + use std::sync::Mutex; + + const SYDNEY: &str = "https://ap-southeast-2.aws.viturhosted.net/"; + const VIRGINIA: &str = "https://us-east-1.aws.viturhosted.net/"; + + /// An unsigned JWT naming `zerokms` — the shape a poisoned + /// `TokenStore` can hand back, since nothing verifies the signature. + fn jwt(zerokms: Option<&str>) -> String { + let mut claims = serde_json::json!({ + "iss": "https://ap-southeast-2.aws.cts.cipherstashmanaged.net/", + "sub": "CS|test-user", + "workspace": "ZVATKW3VHMFG27DY", + }); + if let Some(url) = zerokms { + claims["services"] = serde_json::json!({ "zerokms": url }); + } + let part = + |v: serde_json::Value| Base64UrlUnpadded::encode_string(v.to_string().as_bytes()); + format!( + "{}.{}.c2ln", + part(serde_json::json!({ "alg": "HS256", "typ": "JWT" })), + part(claims) + ) + } + + /// Hands out the given tokens in order. + struct Tokens(Mutex>); + + impl Tokens { + fn new(tokens: [String; N]) -> Self { + Self(Mutex::new(tokens.into())) + } + } + + impl AuthStrategy for &Tokens { + async fn get_token(self) -> Result { + let token = self + .0 + .lock() + .unwrap() + .pop_front() + .expect("a token per call"); + Ok(stack_auth::ServiceToken::new(stack_auth::SecretToken::new( + token, + ))) + } + } + + /// A transport that, like `HttpConnection`, discovers its endpoint + /// and records what it was told. It never sends anything: + /// `get_token` is the code under test. + #[derive(Default)] + struct Discovering { + pinned: Mutex>, + calls: Mutex, + } + + impl crate::connection::ZeroKMSConnectionInit for Discovering { + type ConnectionOpts = Option; + type Error = std::convert::Infallible; + + fn init(pinned: Self::ConnectionOpts) -> Result { + Ok(Self { + pinned: Mutex::new(pinned), + calls: Mutex::new(0), + }) + } + } + + impl ZeroKMSConnection for Discovering { + fn ensure_base_url(&self, url: ZeroKmsEndpoint) { + *self.calls.lock().unwrap() += 1; + self.pinned.lock().unwrap().get_or_insert(url); + } + + fn has_base_url(&self) -> bool { + self.pinned.lock().unwrap().is_some() + } + + async fn send( + &self, + _request: Request, + _access_token: &str, + ) -> Result { + unreachable!("these tests stop at get_token") + } + } + + fn kms(configured: Option<&str>, tokens: Tokens) -> StackKms { + let configured = configured.map(|s| s.parse().unwrap()); + StackKms::connect(ClientOpts::new(configured), tokens, random_client_key()) + .expect("connect") + } + + fn pinned(kms: &StackKms) -> Option { + let pinned = kms.client.connection().pinned.lock().unwrap(); + pinned.as_ref().map(ToString::to_string) + } + + #[tokio::test] + async fn a_hosted_endpoint_is_used() { + let kms = kms(None, Tokens::new([jwt(Some(SYDNEY))])); + kms.get_token() + .await + .expect("a hosted endpoint is accepted"); + assert_eq!(pinned(&kms).as_deref(), Some(SYDNEY)); + } + + #[tokio::test] + async fn a_host_outside_cipherstash_is_refused_and_not_pinned() { + let kms = kms(None, Tokens::new([jwt(Some("https://evil.example/"))])); + let err = kms.get_token().await.expect_err("an attacker's host"); + assert!( + matches!(err, Error::InvalidEndpoint(InvalidEndpoint::NotHosted(_))), + "{err:?}" + ); + assert_eq!(pinned(&kms), None); + } + + #[tokio::test] + async fn plain_http_is_refused() { + let kms = kms( + None, + Tokens::new([jwt(Some("http://ap-southeast-2.aws.viturhosted.net/"))]), + ); + let err = kms.get_token().await.expect_err("http"); + assert!( + matches!(err, Error::InvalidEndpoint(InvalidEndpoint::NotHosted(_))), + "{err:?}" + ); + assert_eq!(pinned(&kms), None); + } + + #[tokio::test] + async fn a_refused_first_token_does_not_stop_a_good_one() { + let kms = kms( + None, + Tokens::new([jwt(Some("https://evil.example/")), jwt(Some(SYDNEY))]), + ); + kms.get_token().await.expect_err("an attacker's host"); + kms.get_token().await.expect("the next caller's token"); + assert_eq!(pinned(&kms).as_deref(), Some(SYDNEY)); + } + + #[tokio::test] + async fn a_later_token_naming_another_endpoint_is_refused() { + let kms = kms( + None, + Tokens::new([jwt(Some(SYDNEY)), jwt(Some(VIRGINIA)), jwt(Some(SYDNEY))]), + ); + kms.get_token().await.expect("first token pins Sydney"); + let err = kms + .get_token() + .await + .expect_err("a token for another region must not go to Sydney"); + assert!( + matches!(err, Error::InvalidEndpoint(InvalidEndpoint::Mismatch)), + "{err:?}" + ); + kms.get_token().await.expect("Sydney tokens still work"); + assert_eq!(pinned(&kms).as_deref(), Some(SYDNEY)); + assert_eq!( + *kms.client.connection().calls.lock().unwrap(), + 1, + "the endpoint is handed to the connection once" + ); + } + + #[tokio::test] + async fn a_later_token_naming_a_foreign_host_is_refused() { + let kms = kms( + None, + Tokens::new([jwt(Some(SYDNEY)), jwt(Some("https://evil.example/"))]), + ); + kms.get_token().await.expect("first token pins Sydney"); + let err = kms.get_token().await.expect_err("foreign host"); + assert!( + matches!(err, Error::InvalidEndpoint(InvalidEndpoint::NotHosted(_))), + "{err:?}" + ); + } + + #[tokio::test] + async fn a_later_token_without_the_claim_is_refused() { + let kms = kms(None, Tokens::new([jwt(Some(SYDNEY)), jwt(None)])); + kms.get_token().await.expect("first token pins Sydney"); + let err = kms.get_token().await.expect_err("no services claim"); + assert!(matches!(err, Error::Auth(_)), "{err:?}"); + } + + #[tokio::test] + async fn a_configured_endpoint_ignores_the_claim() { + // `CS_ZEROKMS_HOST` / `with_base_url` is the operator's choice: + // any host, `http` included, and the claim is never consulted. + let kms = kms( + Some("http://localhost:3002"), + Tokens::new([jwt(Some("https://evil.example/")), jwt(None)]), + ); + kms.get_token().await.expect("claim ignored"); + kms.get_token().await.expect("claim ignored"); + assert_eq!(pinned(&kms).as_deref(), Some("http://localhost:3002/")); + assert_eq!(*kms.client.connection().calls.lock().unwrap(), 0); + } + + #[tokio::test] + async fn a_transport_that_needs_no_endpoint_ignores_the_claim() { + let kms = StackKms::<_, TestConnection>::connect( + ClientOpts::new(TestConnectionBuilder::new()), + stack_auth::StaticTokenStrategy::new(jwt(Some("https://evil.example/"))), + random_client_key(), + ) + .expect("connect"); + kms.get_token().await.expect("claim ignored"); + } + } + mod count_mismatch { use super::*; diff --git a/packages/stack-kms/src/connection/http.rs b/packages/stack-kms/src/connection/http.rs index 809e0c9325..831cae9e56 100644 --- a/packages/stack-kms/src/connection/http.rs +++ b/packages/stack-kms/src/connection/http.rs @@ -51,9 +51,10 @@ pub struct HttpConnectionOpts { } impl HttpConnectionOpts { - /// Options for a connection to `base_url`, or — when `None` — to whatever - /// endpoint the access token's `services` claim names (resolved on first - /// use via [`HttpConnection::ensure_base_url`]). + /// Options for a connection to `base_url`, or — when `None` — to the + /// CipherStash-hosted endpoint the access token's `services` claim names + /// (resolved on first use via [`HttpConnection::ensure_base_url`]). An + /// explicit `base_url` may be any usable endpoint; a claimed one may not. pub fn new(base_url: Option) -> Self { Self { base_url, diff --git a/packages/stack-kms/src/endpoint.rs b/packages/stack-kms/src/endpoint.rs index 57f44be8f5..6278775701 100644 --- a/packages/stack-kms/src/endpoint.rs +++ b/packages/stack-kms/src/endpoint.rs @@ -9,8 +9,10 @@ use url::Url; /// No message repeats the URL it refused: [`NoHost`](Self::NoHost) and /// [`QueryOrFragment`](Self::QueryOrFragment) are checked before /// [`Userinfo`](Self::Userinfo), so the URL they hold can still carry -/// credentials, and a query string can carry anything. The URL stays in the -/// variant for a caller in this process. +/// credentials, and a query string can carry anything. A +/// [`NotHosted`](Self::NotHosted) URL came from an unverified token claim, so +/// it can carry whatever its author chose. The URL stays in the variant for a +/// caller in this process. #[derive(Debug, Error, PartialEq, Eq, miette::Diagnostic)] pub enum InvalidEndpoint { /// The value did not parse as a URL at all. @@ -42,6 +44,31 @@ pub enum InvalidEndpoint { help("Remove the user and password from the URL: ZeroKMS takes the credential as a bearer token.") )] Userinfo, + + /// A token's `services` claim named an endpoint that is not a + /// CipherStash-hosted ZeroKMS. The claim is read from a token whose + /// signature this client does not verify, so it may only choose between + /// CipherStash's own regional endpoints; anything else must be configured + /// explicitly by the operator. + #[error( + "the token's services claim may only name a CipherStash-hosted ZeroKMS endpoint (`https://..viturhosted.net`)" + )] + #[diagnostic( + code(stack_kms::endpoint_not_hosted), + help("To use any other ZeroKMS endpoint, configure it explicitly (`CS_ZEROKMS_HOST` or `with_base_url`).") + )] + NotHosted(String), + + /// A token's `services` claim named a different ZeroKMS endpoint from the + /// one this client resolved from an earlier token. The client refuses to + /// send the token anywhere but the endpoint it already uses, and refuses + /// to move to the new one. + #[error("the token names a different ZeroKMS endpoint from the one this client already uses")] + #[diagnostic( + code(stack_kms::endpoint_mismatch), + help("A client resolves its ZeroKMS endpoint from the first token it holds. Use a separate client for each region's credentials, or configure the endpoint explicitly (`CS_ZEROKMS_HOST`).") + )] + Mismatch, } impl stack_auth::ErrorPayload for InvalidEndpoint { @@ -50,9 +77,12 @@ impl stack_auth::ErrorPayload for InvalidEndpoint { Self::Scheme(scheme) => { stack_auth::diagnostic::payload([("scheme", scheme.as_str().into())]) } - Self::Parse(_) | Self::NoHost(_) | Self::QueryOrFragment(_) | Self::Userinfo => { - serde_json::Map::new() - } + Self::Parse(_) + | Self::NoHost(_) + | Self::QueryOrFragment(_) + | Self::Userinfo + | Self::NotHosted(_) + | Self::Mismatch => serde_json::Map::new(), } } } @@ -108,6 +138,40 @@ impl ZeroKmsEndpoint { Ok(Self(url)) } + /// Validate an endpoint named by an access token's `services` claim. + /// + /// The claim comes from a token whose signature this crate never checks, + /// and a [`TokenStore`](stack_auth::TokenStore) can hand back a token + /// someone else wrote. So on top of [`new`](Self::new)'s hygiene, a + /// claimed endpoint must be a CipherStash-hosted ZeroKMS: + /// + /// * scheme `https`, on the default port + /// * host `..viturhosted.net`, for a region CipherStash + /// runs (`cts_common::Region`) — the form CTS issues + /// + /// Any other endpoint (a gateway, a local ZeroKMS, plain `http`) is + /// accepted only from explicit configuration: `CS_ZEROKMS_HOST` or the + /// builder's `with_base_url`. + /// + /// # Errors + /// + /// Whatever [`new`](Self::new) refuses, and + /// [`InvalidEndpoint::NotHosted`] for a usable URL on any other host. + pub(crate) fn from_services_claim(url: Url) -> Result { + let endpoint = Self::new(url)?; + let url = endpoint.as_url(); + let hosted = url.scheme() == "https" + && url.port().is_none() + && url.host_str().is_some_and(|host| { + cts_common::ZeroKmsServiceDiscovery::region_from_host_fqdn(host).is_ok() + }); + if hosted { + Ok(endpoint) + } else { + Err(InvalidEndpoint::NotHosted(endpoint.0.into())) + } + } + /// The URL for a request path (a `ViturRequest::ENDPOINT`), appended to /// the base. Infallible: the base is known to be http(s) with a /// slash-terminated path, and the endpoint paths are relative constants. @@ -295,5 +359,108 @@ mod tests { Err(InvalidEndpoint::Parse(_)) )); } + + #[test] + fn nothing_with_a_message_that_repeats_a_claimed_url() { + let err = claimed("https://marker.example/").unwrap_err(); + assert!(!err.to_string().contains("marker"), "{err}"); + } + } + + fn claimed(s: &str) -> Result { + ZeroKmsEndpoint::from_services_claim(Url::parse(s).unwrap()) + } + + /// The endpoint a token's `services` claim may name: only CipherStash's + /// own regional ZeroKMS, whatever the explicit-config path accepts. + mod from_services_claim { + use super::*; + + #[test] + fn accepts_every_region_cipherstash_runs() { + for region in cts_common::Region::all() { + let s = format!("https://{}.viturhosted.net/", region.identifier()); + assert_eq!(claimed(&s).map(|e| e.to_string()), Ok(s.clone()), "{s}"); + } + } + + #[test] + fn accepts_a_hosted_endpoint_without_a_trailing_slash() { + assert_eq!( + claimed("https://ap-southeast-2.aws.viturhosted.net") + .unwrap() + .as_str(), + "https://ap-southeast-2.aws.viturhosted.net/" + ); + } + + #[test] + fn accepts_an_explicit_default_port() { + // `Url` drops `:443` on https, so it is the same endpoint. + assert!(claimed("https://us-east-1.aws.viturhosted.net:443/").is_ok()); + } + + #[test] + fn rejects_hosts_outside_cipherstash() { + for s in [ + "https://evil.example/", + "https://zerokms.example.com/", + "https://localhost:3002/", + "https://127.0.0.1/", + "https://viturhosted.net/", + "https://aws.viturhosted.net/", + "https://us-east-1.aws.viturhosted.net.evil.example/", + "https://evil.us-east-1.aws.viturhosted.net/", + "https://us-east-1.aws.viturhosted.net.", + "https://us-east-1-aws.viturhosted.net/", + "https://xviturhosted.net/", + ] { + assert!( + matches!(claimed(s), Err(InvalidEndpoint::NotHosted(_))), + "{s}: {:?}", + claimed(s) + ); + } + } + + #[test] + fn rejects_a_region_cipherstash_does_not_run() { + assert!(matches!( + claimed("https://mars-north-1.aws.viturhosted.net/"), + Err(InvalidEndpoint::NotHosted(_)) + )); + assert!(matches!( + claimed("https://us-east-1.gcp.viturhosted.net/"), + Err(InvalidEndpoint::NotHosted(_)) + )); + } + + #[test] + fn rejects_plain_http_to_a_hosted_name() { + assert!(matches!( + claimed("http://us-east-1.aws.viturhosted.net/"), + Err(InvalidEndpoint::NotHosted(_)) + )); + } + + #[test] + fn rejects_a_non_default_port() { + assert!(matches!( + claimed("https://us-east-1.aws.viturhosted.net:8443/"), + Err(InvalidEndpoint::NotHosted(_)) + )); + } + + #[test] + fn still_applies_the_hygiene_rules_first() { + assert!(matches!( + claimed("https://user:pw@us-east-1.aws.viturhosted.net/"), + Err(InvalidEndpoint::Userinfo) + )); + assert!(matches!( + claimed("https://us-east-1.aws.viturhosted.net/?q=1"), + Err(InvalidEndpoint::QueryOrFragment(_)) + )); + } } } diff --git a/packages/stack-kms/src/errors.rs b/packages/stack-kms/src/errors.rs index 9287e6f18e..4260cbf8cb 100644 --- a/packages/stack-kms/src/errors.rs +++ b/packages/stack-kms/src/errors.rs @@ -349,6 +349,10 @@ mod codes { InvalidEndpoint::QueryOrFragment("https://x/?q".into()) }, InvalidEndpoint::Userinfo => InvalidEndpoint::Userinfo, + InvalidEndpoint::NotHosted(_) => { + InvalidEndpoint::NotHosted("https://evil.example/".into()) + }, + InvalidEndpoint::Mismatch => InvalidEndpoint::Mismatch, ])); errors.extend(boxed(variants![ KeyProviderError::NotConfigured(_) => KeyProviderError::NotConfigured("unset".into()), @@ -539,6 +543,7 @@ mod codes { for error in [ InvalidEndpoint::NoHost("marker://user:pass@".into()), InvalidEndpoint::QueryOrFragment("https://x/?marker".into()), + InvalidEndpoint::NotHosted("https://marker.example/".into()), ] { assert!(!error.to_string().contains("marker"), "{error}"); } From d2ed81cc8843b7f42fea36a2193eaa2f0f13fbee Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 10:34:19 +0000 Subject: [PATCH 2/8] fix(stack-kms)!: refuse a claimed ZeroKMS endpoint with a path Review follow-up to the services-claim allowlist. A claimed endpoint must now sit at the host's root path, so a token cannot aim the client at another service under a path prefix on a CipherStash host. The outer `Error::InvalidEndpoint` help now fits the not-hosted and mismatch cases too. BREAKING CHANGE: `InvalidEndpoint` gains the `NotHosted` and `Mismatch` variants. A token's `services` claim may now only name a CipherStash-hosted ZeroKMS (`https://..viturhosted.net/`), and every later token must name the same endpoint as the first. A setup whose tokens name any other endpoint (a local or staging ZeroKMS) must set `CS_ZEROKMS_HOST` or `with_base_url`, which still accept any usable endpoint. Refs CIP-5152 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012Jmn7o2Eouw3CQpnBNGrwC --- packages/stack-kms/src/endpoint.rs | 25 ++++++++++++++++++++++--- packages/stack-kms/src/errors.rs | 5 +++-- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/stack-kms/src/endpoint.rs b/packages/stack-kms/src/endpoint.rs index 6278775701..dfc0b9f516 100644 --- a/packages/stack-kms/src/endpoint.rs +++ b/packages/stack-kms/src/endpoint.rs @@ -51,7 +51,7 @@ pub enum InvalidEndpoint { /// CipherStash's own regional endpoints; anything else must be configured /// explicitly by the operator. #[error( - "the token's services claim may only name a CipherStash-hosted ZeroKMS endpoint (`https://..viturhosted.net`)" + "the token's services claim may only name a CipherStash-hosted ZeroKMS endpoint (`https://..viturhosted.net/`)" )] #[diagnostic( code(stack_kms::endpoint_not_hosted), @@ -145,9 +145,11 @@ impl ZeroKmsEndpoint { /// someone else wrote. So on top of [`new`](Self::new)'s hygiene, a /// claimed endpoint must be a CipherStash-hosted ZeroKMS: /// - /// * scheme `https`, on the default port + /// * scheme `https`, on the default port, at the root path /// * host `..viturhosted.net`, for a region CipherStash - /// runs (`cts_common::Region`) — the form CTS issues + /// runs (`cts_common::Region`) — the form CTS issues. A region newer + /// than this build's `cts-common` is refused, as its workspace CRN + /// already is by stack-auth. /// /// Any other endpoint (a gateway, a local ZeroKMS, plain `http`) is /// accepted only from explicit configuration: `CS_ZEROKMS_HOST` or the @@ -162,6 +164,7 @@ impl ZeroKmsEndpoint { let url = endpoint.as_url(); let hosted = url.scheme() == "https" && url.port().is_none() + && url.path() == "/" && url.host_str().is_some_and(|host| { cts_common::ZeroKmsServiceDiscovery::region_from_host_fqdn(host).is_ok() }); @@ -443,6 +446,22 @@ mod tests { )); } + #[test] + fn rejects_a_path_on_a_hosted_name() { + // The claim chooses the host's ZeroKMS, not some other service + // under a path prefix on it. + for s in [ + "https://us-east-1.aws.viturhosted.net/other/service", + "https://us-east-1.aws.viturhosted.net/other/service/", + ] { + assert!( + matches!(claimed(s), Err(InvalidEndpoint::NotHosted(_))), + "{s}: {:?}", + claimed(s) + ); + } + } + #[test] fn rejects_a_non_default_port() { assert!(matches!( diff --git a/packages/stack-kms/src/errors.rs b/packages/stack-kms/src/errors.rs index 4260cbf8cb..5b32faa6a1 100644 --- a/packages/stack-kms/src/errors.rs +++ b/packages/stack-kms/src/errors.rs @@ -741,11 +741,12 @@ pub enum Error { #[diagnostic(code(stack_kms::connection_init))] ConnectionInit(#[source] Box), - /// The ZeroKMS endpoint named by the token's `services` claim is unusable. + /// The ZeroKMS endpoint named by the token's `services` claim is unusable, + /// is not CipherStash-hosted, or is not the one this client already uses. #[error("Invalid ZeroKMS endpoint in the token's services claim: {0}")] #[diagnostic( code(stack_kms::invalid_endpoint), - help("Configure the ZeroKMS endpoint explicitly (`CS_ZEROKMS_HOST`), or use a token whose services claim names a usable one.") + help("Configure the ZeroKMS endpoint explicitly (`CS_ZEROKMS_HOST`), or use tokens whose services claim names the CipherStash-hosted endpoint for this client's region.") )] InvalidEndpoint( #[from] From 55d64cd3b126d455de4ab0a1459972baa22e85bb Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 10 Nov 2026 01:57:26 +0000 Subject: [PATCH 3/8] fix(stack-kms): accept the zerokms.cipherstashmanaged.net ZeroKMS hosts CTS also issues ZeroKMS endpoints of the form `https://..zerokms.cipherstashmanaged.net/`, which cts-common's ZeroKmsServiceDiscovery does not know: it matches only `..viturhosted.net`. The services-claim allowlist built on it would have refused those tokens. stack-kms now matches the host itself against both hosted domains, requiring exactly `.` before the domain and a region cts_common::Region knows. The scheme, port and path rules are unchanged. Refs CIP-5152 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012Jmn7o2Eouw3CQpnBNGrwC --- languages/golang/encrypt/credentials.go | 3 +- packages/stack-kms/Cargo.toml | 2 +- packages/stack-kms/src/builder.rs | 3 +- packages/stack-kms/src/client.rs | 2 +- packages/stack-kms/src/endpoint.rs | 55 ++++++++++++++++++++----- 5 files changed, 51 insertions(+), 14 deletions(-) diff --git a/languages/golang/encrypt/credentials.go b/languages/golang/encrypt/credentials.go index c758f46386..4051a9a099 100644 --- a/languages/golang/encrypt/credentials.go +++ b/languages/golang/encrypt/credentials.go @@ -405,7 +405,8 @@ func clientKeyFromProfile(ctx context.Context, profile *auth.ProfileStore, noPro // zerokmsEndpoint is the endpoint NewClient pins, in stack-kms's order: the // explicit URL, else the first of CS_ZEROKMS_HOST and CS_VITUR_HOST that is // set, else none (the token's services claim decides on first use, and may -// only name a CipherStash-hosted ZeroKMS: https://..viturhosted.net). A set +// only name a CipherStash-hosted ZeroKMS: https://..viturhosted.net +// or https://..zerokms.cipherstashmanaged.net). A set // variable that is not a usable endpoint is an error, not a fall-through: // falling through would send key operations somewhere the operator did not // configure. The guest validates the URL in full; this check exists to name diff --git a/packages/stack-kms/Cargo.toml b/packages/stack-kms/Cargo.toml index 5fb6e495b9..d9269de3ce 100644 --- a/packages/stack-kms/Cargo.toml +++ b/packages/stack-kms/Cargo.toml @@ -44,7 +44,7 @@ recipher = { workspace = true } # `http` feature rides on this crate's `http` feature rather than arriving by # default. stack-auth = { workspace = true } -# `ZeroKmsServiceDiscovery`: which hosts a token's `services` claim may name. +# `Region`: which regions a token's `services` claim may name a ZeroKMS in. cts-common = { workspace = true } zerokms-protocol = { workspace = true } diff --git a/packages/stack-kms/src/builder.rs b/packages/stack-kms/src/builder.rs index e1a93bd3e5..713fcf9180 100644 --- a/packages/stack-kms/src/builder.rs +++ b/packages/stack-kms/src/builder.rs @@ -78,7 +78,8 @@ impl stack_auth::ErrorPayload for StackKmsBuilderError { /// 2. `CS_ZEROKMS_HOST` (or legacy `CS_VITUR_HOST`) environment variable — /// the first one that is *set* is used, and an invalid value is an error /// 3. Automatically from the token's `services` claim. Only a -/// CipherStash-hosted ZeroKMS (`https://..viturhosted.net`) +/// CipherStash-hosted ZeroKMS (`https://..viturhosted.net` +/// or `https://..zerokms.cipherstashmanaged.net`) /// is accepted there, and every later token must name the same one; any /// other endpoint must be configured by 1 or 2 /// diff --git a/packages/stack-kms/src/client.rs b/packages/stack-kms/src/client.rs index 7bea0d5316..93dec1672e 100644 --- a/packages/stack-kms/src/client.rs +++ b/packages/stack-kms/src/client.rs @@ -753,7 +753,7 @@ mod tests { use std::sync::Mutex; const SYDNEY: &str = "https://ap-southeast-2.aws.viturhosted.net/"; - const VIRGINIA: &str = "https://us-east-1.aws.viturhosted.net/"; + const VIRGINIA: &str = "https://us-east-1.aws.zerokms.cipherstashmanaged.net/"; /// An unsigned JWT naming `zerokms` — the shape a poisoned /// `TokenStore` can hand back, since nothing verifies the signature. diff --git a/packages/stack-kms/src/endpoint.rs b/packages/stack-kms/src/endpoint.rs index dfc0b9f516..b4ac43fc96 100644 --- a/packages/stack-kms/src/endpoint.rs +++ b/packages/stack-kms/src/endpoint.rs @@ -51,7 +51,7 @@ pub enum InvalidEndpoint { /// CipherStash's own regional endpoints; anything else must be configured /// explicitly by the operator. #[error( - "the token's services claim may only name a CipherStash-hosted ZeroKMS endpoint (`https://..viturhosted.net/`)" + "the token's services claim may only name a CipherStash-hosted ZeroKMS endpoint (`https://..viturhosted.net/` or `https://..zerokms.cipherstashmanaged.net/`)" )] #[diagnostic( code(stack_kms::endpoint_not_hosted), @@ -87,6 +87,24 @@ impl stack_auth::ErrorPayload for InvalidEndpoint { } } +/// The domains CipherStash serves ZeroKMS from. A hosted ZeroKMS is +/// `..`; `cts_common` knows only the first, so +/// the list lives here. +const HOSTED_ZEROKMS_DOMAINS: &[&str] = &["viturhosted.net", "zerokms.cipherstashmanaged.net"]; + +/// Whether `host` is `..` for a hosted domain and a +/// region CipherStash runs. `host` comes from a parsed [`Url`], so it is +/// already lowercase and IDNA-normalised. +fn is_hosted_zerokms(host: &str) -> bool { + HOSTED_ZEROKMS_DOMAINS.iter().any(|domain| { + host.strip_suffix(domain) + .and_then(|prefix| prefix.strip_suffix('.')) + // Exactly two labels: `.`. + .filter(|region| region.matches('.').count() == 1) + .is_some_and(|region| cts_common::Region::new(region).is_ok()) + }) +} + /// A validated ZeroKMS base URL. /// /// Construction is the one place URL hygiene happens, so everything downstream @@ -146,8 +164,9 @@ impl ZeroKmsEndpoint { /// claimed endpoint must be a CipherStash-hosted ZeroKMS: /// /// * scheme `https`, on the default port, at the root path - /// * host `..viturhosted.net`, for a region CipherStash - /// runs (`cts_common::Region`) — the form CTS issues. A region newer + /// * host `..viturhosted.net` or + /// `..zerokms.cipherstashmanaged.net`, for a region + /// CipherStash runs (`cts_common::Region`) — the forms CTS issues. A region newer /// than this build's `cts-common` is refused, as its workspace CRN /// already is by stack-auth. /// @@ -165,9 +184,7 @@ impl ZeroKmsEndpoint { let hosted = url.scheme() == "https" && url.port().is_none() && url.path() == "/" - && url.host_str().is_some_and(|host| { - cts_common::ZeroKmsServiceDiscovery::region_from_host_fqdn(host).is_ok() - }); + && url.host_str().is_some_and(is_hosted_zerokms); if hosted { Ok(endpoint) } else { @@ -380,13 +397,20 @@ mod tests { use super::*; #[test] - fn accepts_every_region_cipherstash_runs() { - for region in cts_common::Region::all() { - let s = format!("https://{}.viturhosted.net/", region.identifier()); - assert_eq!(claimed(&s).map(|e| e.to_string()), Ok(s.clone()), "{s}"); + fn accepts_every_region_cipherstash_runs_on_every_hosted_domain() { + for domain in HOSTED_ZEROKMS_DOMAINS { + for region in cts_common::Region::all() { + let s = format!("https://{}.{domain}/", region.identifier()); + assert_eq!(claimed(&s).map(|e| e.to_string()), Ok(s.clone()), "{s}"); + } } } + #[test] + fn accepts_the_cipherstashmanaged_form() { + assert!(claimed("https://ap-southeast-2.aws.zerokms.cipherstashmanaged.net/").is_ok()); + } + #[test] fn accepts_a_hosted_endpoint_without_a_trailing_slash() { assert_eq!( @@ -417,6 +441,13 @@ mod tests { "https://us-east-1.aws.viturhosted.net.", "https://us-east-1-aws.viturhosted.net/", "https://xviturhosted.net/", + "https://zerokms.cipherstashmanaged.net/", + "https://aws.zerokms.cipherstashmanaged.net/", + "https://us-east-1.aws.cts.cipherstashmanaged.net/", + "https://us-east-1.aws.cipherstashmanaged.net/", + "https://evil.us-east-1.aws.zerokms.cipherstashmanaged.net/", + "https://us-east-1.aws.zerokms.cipherstashmanaged.net.evil.example/", + "https://us-east-1.aws.xzerokms.cipherstashmanaged.net/", ] { assert!( matches!(claimed(s), Err(InvalidEndpoint::NotHosted(_))), @@ -436,6 +467,10 @@ mod tests { claimed("https://us-east-1.gcp.viturhosted.net/"), Err(InvalidEndpoint::NotHosted(_)) )); + assert!(matches!( + claimed("https://mars-north-1.aws.zerokms.cipherstashmanaged.net/"), + Err(InvalidEndpoint::NotHosted(_)) + )); } #[test] From 97ce53438ab41914ec66c6c5ac10babce5632ea8 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Wed, 11 Nov 2026 11:46:05 +1100 Subject: [PATCH 4/8] fix(stack-kms): pin a claimed ZeroKMS endpoint by region, not URL CipherStash serves each region's ZeroKMS under both viturhosted.net and zerokms.cipherstashmanaged.net while it moves between them; the viturhosted.net name is weighted between the Lambda and ECS platforms. CTS issues viturhosted.net in the services claim today (cts-common's ZeroKmsServiceDiscovery). Comparing later claims to the pin by exact URL meant that when CTS switches domain, every long-lived client would reject every refreshed token with InvalidEndpoint::Mismatch until it restarted. A claimed endpoint now carries the region its host names. A later token for the same region on either hosted domain is accepted and still sent to the pinned endpoint; only a different region is a mismatch. Adds a contract test that every endpoint cts-common's discovery builds is accepted, so a cts-common bump that moves ZeroKMS to an unlisted domain fails at bump time rather than in production. --- packages/stack-kms/src/builder.rs | 4 +- packages/stack-kms/src/client.rs | 82 ++++++++++++++++++++--- packages/stack-kms/src/endpoint.rs | 100 +++++++++++++++++++++++------ 3 files changed, 153 insertions(+), 33 deletions(-) diff --git a/packages/stack-kms/src/builder.rs b/packages/stack-kms/src/builder.rs index 713fcf9180..3a02930089 100644 --- a/packages/stack-kms/src/builder.rs +++ b/packages/stack-kms/src/builder.rs @@ -80,8 +80,8 @@ impl stack_auth::ErrorPayload for StackKmsBuilderError { /// 3. Automatically from the token's `services` claim. Only a /// CipherStash-hosted ZeroKMS (`https://..viturhosted.net` /// or `https://..zerokms.cipherstashmanaged.net`) -/// is accepted there, and every later token must name the same one; any -/// other endpoint must be configured by 1 or 2 +/// is accepted there, and every later token must name the same region +/// (on either domain); any other endpoint must be configured by 1 or 2 /// /// # Example /// diff --git a/packages/stack-kms/src/client.rs b/packages/stack-kms/src/client.rs index 93dec1672e..6bee9e20f7 100644 --- a/packages/stack-kms/src/client.rs +++ b/packages/stack-kms/src/client.rs @@ -14,7 +14,7 @@ use vitaminc::random::{Generatable, SafeRand}; #[cfg(feature = "http")] use crate::connection::HttpConnection; use crate::connection::ZeroKMSConnection; -use crate::endpoint::{InvalidEndpoint, ZeroKmsEndpoint}; +use crate::endpoint::{HostedEndpoint, InvalidEndpoint, ZeroKmsEndpoint}; use crate::errors::{Error, GenerateKeyError, LoadKeysetError, RetrieveKeyError}; use crate::futures::map_async_chunked; use crate::key::{ClientKey, DataKey, DataKeyWithTag, IndexKey}; @@ -414,7 +414,7 @@ pub struct StackKms { client_key: ClientKey, /// `None` when the connection was given its endpoint at init; otherwise /// the endpoint the first token's `services` claim named, once known. - discovered: Option>, + discovered: Option>, } /// `StackKms` owns the transport [`Client`], a [`stack_auth`] credential @@ -428,7 +428,7 @@ pub struct StackKms { client_key: ClientKey, /// `None` when the connection was given its endpoint at init; otherwise /// the endpoint the first token's `services` claim named, once known. - discovered: Option>, + discovered: Option>, } impl StackKms @@ -452,7 +452,7 @@ where /// ZeroKMS endpoint is taken from the access token's `services` claim on /// first use unless the connection already knows one. A claimed endpoint /// must be CipherStash-hosted, and every later token must name the same - /// one; whether the connection already knows one is decided here, once. + /// region; whether the connection already knows one is decided here, once. pub fn connect( opts: ClientOpts, credentials: C, @@ -483,9 +483,13 @@ where /// /// * the claim may only name a CipherStash-hosted ZeroKMS /// ([`ZeroKmsEndpoint::from_services_claim`]), and - /// * every later token must name the endpoint the first one did. A token - /// naming another is refused rather than sent to the pinned endpoint, - /// and it cannot move the pin. + /// * every later token must name the same region as the first one did. + /// A token for another region is refused rather than sent to the + /// pinned endpoint, and it cannot move the pin. A token naming the + /// same region on the other hosted domain is accepted and still sent + /// to the pinned endpoint: both domains serve that region's ZeroKMS + /// while CipherStash moves between them, and refusing would fail + /// every call on a long-lived client when CTS switches domain. async fn get_token(&self) -> Result { let token = (&self.credentials).get_token().await?; // Configured explicitly: the claim is not consulted. @@ -496,13 +500,15 @@ where let pinned = discovered.get_or_init(|| { tracing::debug!( target: "stack_kms", - endpoint = %claimed, + endpoint = %claimed.endpoint, "resolved ZeroKMS endpoint from the token's services claim" ); - self.client.connection().ensure_base_url(claimed.clone()); + self.client + .connection() + .ensure_base_url(claimed.endpoint.clone()); claimed.clone() }); - if *pinned != claimed { + if pinned.region != claimed.region { return Err(InvalidEndpoint::Mismatch.into()); } Ok(token) @@ -918,6 +924,62 @@ mod tests { ); } + /// CipherStash serves each region's ZeroKMS under both hosted domains + /// while it moves from `viturhosted.net` to + /// `zerokms.cipherstashmanaged.net`. When CTS starts issuing the new + /// name, a long-lived client pinned to the old one must keep working: + /// refusing would fail every call until the process restarted. + #[tokio::test] + async fn a_later_token_for_the_same_region_on_the_other_hosted_domain_is_accepted() { + const SYDNEY_NEW: &str = "https://ap-southeast-2.aws.zerokms.cipherstashmanaged.net/"; + let kms = kms( + None, + Tokens::new([jwt(Some(SYDNEY)), jwt(Some(SYDNEY_NEW)), jwt(Some(SYDNEY))]), + ); + kms.get_token().await.expect("first token pins Sydney"); + kms.get_token() + .await + .expect("same region on the new domain is the same ZeroKMS"); + kms.get_token().await.expect("and back again"); + assert_eq!( + pinned(&kms).as_deref(), + Some(SYDNEY), + "the endpoint in use does not move" + ); + assert_eq!(*kms.client.connection().calls.lock().unwrap(), 1); + } + + #[tokio::test] + async fn the_domain_move_works_in_both_directions() { + const VIRGINIA_OLD: &str = "https://us-east-1.aws.viturhosted.net/"; + let kms = kms( + None, + Tokens::new([jwt(Some(VIRGINIA)), jwt(Some(VIRGINIA_OLD))]), + ); + kms.get_token().await.expect("first token pins Virginia"); + kms.get_token() + .await + .expect("same region on the old domain is the same ZeroKMS"); + assert_eq!(pinned(&kms).as_deref(), Some(VIRGINIA)); + } + + #[tokio::test] + async fn another_region_is_refused_on_either_domain() { + for other in [ + "https://us-east-1.aws.viturhosted.net/", + "https://us-east-1.aws.zerokms.cipherstashmanaged.net/", + ] { + let kms = kms(None, Tokens::new([jwt(Some(SYDNEY)), jwt(Some(other))])); + kms.get_token().await.expect("first token pins Sydney"); + let err = kms.get_token().await.expect_err(other); + assert!( + matches!(err, Error::InvalidEndpoint(InvalidEndpoint::Mismatch)), + "{other}: {err:?}" + ); + assert_eq!(pinned(&kms).as_deref(), Some(SYDNEY)); + } + } + #[tokio::test] async fn a_later_token_naming_a_foreign_host_is_refused() { let kms = kms( diff --git a/packages/stack-kms/src/endpoint.rs b/packages/stack-kms/src/endpoint.rs index b4ac43fc96..61392c6e0b 100644 --- a/packages/stack-kms/src/endpoint.rs +++ b/packages/stack-kms/src/endpoint.rs @@ -59,11 +59,14 @@ pub enum InvalidEndpoint { )] NotHosted(String), - /// A token's `services` claim named a different ZeroKMS endpoint from the - /// one this client resolved from an earlier token. The client refuses to - /// send the token anywhere but the endpoint it already uses, and refuses - /// to move to the new one. - #[error("the token names a different ZeroKMS endpoint from the one this client already uses")] + /// A token's `services` claim named a ZeroKMS in a different region from + /// the one this client resolved from an earlier token. The client refuses + /// to send the token anywhere but the endpoint it already uses, and + /// refuses to move to the new one. The same region on the other hosted + /// domain is not a mismatch: both domains serve that region's ZeroKMS. + #[error( + "the token names a ZeroKMS in a different region from the one this client already uses" + )] #[diagnostic( code(stack_kms::endpoint_mismatch), help("A client resolves its ZeroKMS endpoint from the first token it holds. Use a separate client for each region's credentials, or configure the endpoint explicitly (`CS_ZEROKMS_HOST`).") @@ -92,19 +95,34 @@ impl stack_auth::ErrorPayload for InvalidEndpoint { /// the list lives here. const HOSTED_ZEROKMS_DOMAINS: &[&str] = &["viturhosted.net", "zerokms.cipherstashmanaged.net"]; -/// Whether `host` is `..` for a hosted domain and a -/// region CipherStash runs. `host` comes from a parsed [`Url`], so it is -/// already lowercase and IDNA-normalised. -fn is_hosted_zerokms(host: &str) -> bool { - HOSTED_ZEROKMS_DOMAINS.iter().any(|domain| { +/// The region of `host` when it is `..` for a +/// hosted domain and a region CipherStash runs. `host` comes from a parsed +/// [`Url`], so it is already lowercase and IDNA-normalised. +fn hosted_zerokms_region(host: &str) -> Option { + HOSTED_ZEROKMS_DOMAINS.iter().find_map(|domain| { host.strip_suffix(domain) .and_then(|prefix| prefix.strip_suffix('.')) // Exactly two labels: `.`. .filter(|region| region.matches('.').count() == 1) - .is_some_and(|region| cts_common::Region::new(region).is_ok()) + .and_then(|region| cts_common::Region::new(region).ok()) }) } +/// A CipherStash-hosted ZeroKMS endpoint named by a token's `services` +/// claim, with the region its host names. +/// +/// Each region's ZeroKMS is served under every domain in +/// `HOSTED_ZEROKMS_DOMAINS` while CipherStash moves from `viturhosted.net` +/// to `zerokms.cipherstashmanaged.net`, so two claims for the same region +/// name the same service. Compare claims by [`region`](Self::region), not +/// by URL: a client pinned to one domain must keep working when CTS starts +/// issuing the other. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct HostedEndpoint { + pub(crate) endpoint: ZeroKmsEndpoint, + pub(crate) region: cts_common::Region, +} + /// A validated ZeroKMS base URL. /// /// Construction is the one place URL hygiene happens, so everything downstream @@ -178,17 +196,15 @@ impl ZeroKmsEndpoint { /// /// Whatever [`new`](Self::new) refuses, and /// [`InvalidEndpoint::NotHosted`] for a usable URL on any other host. - pub(crate) fn from_services_claim(url: Url) -> Result { + pub(crate) fn from_services_claim(url: Url) -> Result { let endpoint = Self::new(url)?; let url = endpoint.as_url(); - let hosted = url.scheme() == "https" - && url.port().is_none() - && url.path() == "/" - && url.host_str().is_some_and(is_hosted_zerokms); - if hosted { - Ok(endpoint) - } else { - Err(InvalidEndpoint::NotHosted(endpoint.0.into())) + let region = (url.scheme() == "https" && url.port().is_none() && url.path() == "/") + .then(|| url.host_str().and_then(hosted_zerokms_region)) + .flatten(); + match region { + Some(region) => Ok(HostedEndpoint { endpoint, region }), + None => Err(InvalidEndpoint::NotHosted(endpoint.0.into())), } } @@ -388,7 +404,7 @@ mod tests { } fn claimed(s: &str) -> Result { - ZeroKmsEndpoint::from_services_claim(Url::parse(s).unwrap()) + ZeroKmsEndpoint::from_services_claim(Url::parse(s).unwrap()).map(|h| h.endpoint) } /// The endpoint a token's `services` claim may name: only CipherStash's @@ -406,6 +422,48 @@ mod tests { } } + /// The contract with CTS: whatever `cts-common` builds for the + /// `services` claim must be accepted. If a `cts-common` bump moves + /// ZeroKMS to a domain this crate does not list, this fails at bump + /// time instead of every customer's client failing in production. + #[test] + fn accepts_every_endpoint_cts_issues() { + use cts_common::{ServiceDiscovery, ZeroKmsServiceDiscovery}; + for region in cts_common::Region::all() { + let issued = ZeroKmsServiceDiscovery::endpoint(region).unwrap(); + assert_eq!( + ZeroKmsEndpoint::from_services_claim(issued.clone()) + .map(|h| (h.endpoint.to_string(), h.region)), + Ok((issued.to_string(), region)), + "{issued}" + ); + } + } + + #[test] + fn rejects_the_cts_hosts_on_both_domains() { + for s in [ + "https://us-east-1.aws.auth.viturhosted.net/", + "https://us-east-1.aws.cts.cipherstashmanaged.net/", + ] { + assert!( + matches!(claimed(s), Err(InvalidEndpoint::NotHosted(_))), + "{s}: {:?}", + claimed(s) + ); + } + } + + #[test] + fn names_the_same_region_on_both_hosted_domains() { + let parse = |s: &str| ZeroKmsEndpoint::from_services_claim(Url::parse(s).unwrap()); + let old = parse("https://ap-southeast-2.aws.viturhosted.net/").unwrap(); + let new = parse("https://ap-southeast-2.aws.zerokms.cipherstashmanaged.net/").unwrap(); + assert_ne!(old.endpoint, new.endpoint); + assert_eq!(old.region, new.region); + assert_eq!(old.region.identifier(), "ap-southeast-2.aws"); + } + #[test] fn accepts_the_cipherstashmanaged_form() { assert!(claimed("https://ap-southeast-2.aws.zerokms.cipherstashmanaged.net/").is_ok()); From f9c592c96107209fd7e4c685dc66e5552b3fc4ed Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Wed, 11 Nov 2026 13:32:03 +1100 Subject: [PATCH 5/8] fix(stack-kms)!: share hosted endpoint validation and follow domain migrations Reject untrusted ZeroKMS destinations during device provisioning as well as key operations, so a planted profile token cannot redirect bearer credentials. Share the validated claim accessor and pin the region while allowing trusted domain migrations without restarting clients. Publish the breaking KMS contract with matching dependent crate versions and document later-token validation and custom transport requirements. Refs #1546 --- .../validate-hosted-zerokms-discovery.md | 9 + Cargo.lock | 10 +- Cargo.toml | 4 +- languages/golang/auth/guest/Cargo.lock | 4 +- languages/golang/encrypt/guest/Cargo.lock | 10 +- languages/golang/encrypt/guest/src/host.rs | 6 +- languages/golang/encrypt/guest/src/status.rs | 4 +- packages/eql/Cargo.lock | 16 +- packages/eql/crates/eql-bindings/Cargo.toml | 2 +- packages/stack-auth/Cargo.toml | 2 +- packages/stack-auth/fuzz/Cargo.lock | 4 +- packages/stack-auth/src/device_client.rs | 103 +++++++++- packages/stack-auth/src/hosted_zerokms.rs | 81 ++++++++ packages/stack-auth/src/lib.rs | 2 + packages/stack-auth/src/service_token.rs | 94 ++++++++- packages/stack-encrypt-derive/Cargo.toml | 2 +- packages/stack-encrypt/Cargo.toml | 6 +- packages/stack-encrypt/fuzz/Cargo.lock | 10 +- packages/stack-kms/Cargo.toml | 2 +- packages/stack-kms/fuzz/Cargo.lock | 6 +- packages/stack-kms/src/client.rs | 182 +++++++++++++++--- packages/stack-kms/src/connection.rs | 10 +- packages/stack-kms/src/connection/http.rs | 103 +++++++--- packages/stack-kms/src/endpoint.rs | 44 ++--- packages/stack-kms/src/lib.rs | 21 ++ packages/stack-profile/Cargo.toml | 2 +- skills/stash-zerokms/SKILL.md | 13 ++ 27 files changed, 604 insertions(+), 148 deletions(-) create mode 100644 .changeset/validate-hosted-zerokms-discovery.md create mode 100644 packages/stack-auth/src/hosted_zerokms.rs diff --git a/.changeset/validate-hosted-zerokms-discovery.md b/.changeset/validate-hosted-zerokms-discovery.md new file mode 100644 index 0000000000..a94bc1821a --- /dev/null +++ b/.changeset/validate-hosted-zerokms-discovery.md @@ -0,0 +1,9 @@ +--- +'@cipherstash/auth': patch +'stash': patch +'@cipherstash/eql': minor +--- + +Validate token-discovered ZeroKMS destinations before sending bearer credentials, including device provisioning during login. Discovery accepts only HTTPS endpoints for supported regions under the CipherStash ZeroKMS domains. Every later token must carry a valid hosted claim for the original region; missing, non-hosted, or different-region claims are refused. Same-region domain migrations update the destination used by long-lived clients. Explicit KMS configuration remains authoritative; staging and regions unknown to the SDK require explicit configuration or an SDK upgrade. + +The Rust releases are `stack-auth`/`stack-profile` 0.43.1, `stack-kms` 0.2.0, and `stack-encrypt`/`stack-encrypt-derive` 0.3.0. The pending EQL Rust encryption integration uses `stack-encrypt` 0.3.0. Custom `ZeroKMSConnection` transports that opt into discovery must update discovered URLs on later `ensure_base_url` calls while preserving URLs supplied at initialization. The CLI's bundled agent guidance describes these discovery requirements. diff --git a/Cargo.lock b/Cargo.lock index 0824d92cb0..77c7178d71 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3237,7 +3237,7 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "stack-auth" -version = "0.43.0" +version = "0.43.1" dependencies = [ "aquamarine", "axum", @@ -3314,7 +3314,7 @@ dependencies = [ [[package]] name = "stack-encrypt" -version = "0.2.0" +version = "0.3.0" dependencies = [ "base64ct", "cllw-ore", @@ -3340,7 +3340,7 @@ dependencies = [ [[package]] name = "stack-encrypt-derive" -version = "0.2.0" +version = "0.3.0" dependencies = [ "proc-macro2", "quote", @@ -3365,7 +3365,7 @@ dependencies = [ [[package]] name = "stack-kms" -version = "0.1.0" +version = "0.2.0" dependencies = [ "async-mutex", "base16ct", @@ -3401,7 +3401,7 @@ dependencies = [ [[package]] name = "stack-profile" -version = "0.43.0" +version = "0.43.1" dependencies = [ "dirs", "gethostname", diff --git a/Cargo.toml b/Cargo.toml index b1ccc99399..8ba7be0013 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,8 +69,8 @@ incremental = true # but cannot subtract them, and `stack-kms` / `stack-encrypt` need stack-auth # without `http`. Consumers that want the default transport re-enable it with # `features = ["http"]`. -stack-auth = { path = "./packages/stack-auth", version = "0.43.0", default-features = false } -stack-profile = { path = "./packages/stack-profile", version = "0.43.0" } +stack-auth = { path = "./packages/stack-auth", version = "0.43.1", default-features = false } +stack-profile = { path = "./packages/stack-profile", version = "0.43.1" } # Suite crates, from crates.io; Cargo.lock holds the exact versions. # cts-common and zerokms-protocol take caret requirements because the diff --git a/languages/golang/auth/guest/Cargo.lock b/languages/golang/auth/guest/Cargo.lock index 970b1cd538..e539f70ed1 100644 --- a/languages/golang/auth/guest/Cargo.lock +++ b/languages/golang/auth/guest/Cargo.lock @@ -1780,7 +1780,7 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "stack-auth" -version = "0.43.0" +version = "0.43.1" dependencies = [ "aquamarine", "base64", @@ -1838,7 +1838,7 @@ dependencies = [ [[package]] name = "stack-profile" -version = "0.43.0" +version = "0.43.1" dependencies = [ "dirs", "gethostname", diff --git a/languages/golang/encrypt/guest/Cargo.lock b/languages/golang/encrypt/guest/Cargo.lock index 6bf4f812db..04da6a04e6 100644 --- a/languages/golang/encrypt/guest/Cargo.lock +++ b/languages/golang/encrypt/guest/Cargo.lock @@ -1997,7 +1997,7 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "stack-auth" -version = "0.43.0" +version = "0.43.1" dependencies = [ "aquamarine", "base64", @@ -2024,7 +2024,7 @@ dependencies = [ [[package]] name = "stack-encrypt" -version = "0.2.0" +version = "0.3.0" dependencies = [ "base64ct", "cllw-ore", @@ -2046,7 +2046,7 @@ dependencies = [ [[package]] name = "stack-encrypt-derive" -version = "0.2.0" +version = "0.3.0" dependencies = [ "proc-macro2", "quote", @@ -2089,7 +2089,7 @@ dependencies = [ [[package]] name = "stack-kms" -version = "0.1.0" +version = "0.2.0" dependencies = [ "base16ct", "base64ct", @@ -2118,7 +2118,7 @@ dependencies = [ [[package]] name = "stack-profile" -version = "0.43.0" +version = "0.43.1" dependencies = [ "dirs", "gethostname", diff --git a/languages/golang/encrypt/guest/src/host.rs b/languages/golang/encrypt/guest/src/host.rs index 7c7564f5f5..3f1da2ab9d 100644 --- a/languages/golang/encrypt/guest/src/host.rs +++ b/languages/golang/encrypt/guest/src/host.rs @@ -54,9 +54,10 @@ extern "C" { /// The endpoint is pinned at init when the cipher config named one; /// otherwise it is discovered by `StackKms` from the access token's /// `services` claim through [`ensure_base_url`](Self::ensure_base_url) — -/// the first value wins, per the trait's contract. +/// later validated claims may update the domain within the pinned region. pub struct WasiHostConnection { base: Mutex>, + explicitly_configured: bool, } impl WasiHostConnection { @@ -76,6 +77,7 @@ impl ZeroKMSConnectionInit for WasiHostConnection { fn init(opts: Self::ConnectionOpts) -> Result { Ok(Self { + explicitly_configured: opts.is_some(), base: Mutex::new(opts), }) } @@ -84,7 +86,7 @@ impl ZeroKMSConnectionInit for WasiHostConnection { impl ZeroKMSConnection for WasiHostConnection { fn ensure_base_url(&self, url: ZeroKmsEndpoint) { let mut base = self.base(); - if base.is_none() { + if !self.explicitly_configured { *base = Some(url); } } diff --git a/languages/golang/encrypt/guest/src/status.rs b/languages/golang/encrypt/guest/src/status.rs index 8425272e43..296648ee58 100644 --- a/languages/golang/encrypt/guest/src/status.rs +++ b/languages/golang/encrypt/guest/src/status.rs @@ -179,8 +179,8 @@ pub fn status_for_dynamic(error: &stack_encrypt::dynamic::Error) -> u32 { } } -// These matches are deliberately exhaustive — no `_` arms. None of the -// stack-kms error enums is `#[non_exhaustive]`, so exhaustiveness is free +// These matches are deliberately exhaustive — no `_` arms. The outer KMS +// error and request error enums matched here are exhaustive, so this is free // compiler coverage: `GenerateKeyError` already grew `Unauthorized` / // `Forbidden` out of the shared `From` pattern, and a // variant added tomorrow must be classified here before this crate builds, diff --git a/packages/eql/Cargo.lock b/packages/eql/Cargo.lock index f11b03237c..04c84a5bc8 100644 --- a/packages/eql/Cargo.lock +++ b/packages/eql/Cargo.lock @@ -4097,7 +4097,7 @@ dependencies = [ [[package]] name = "stack-auth" -version = "0.43.0" +version = "0.43.1" dependencies = [ "aquamarine", "base64 0.22.1", @@ -4109,7 +4109,7 @@ dependencies = [ "serde_json", "serde_urlencoded", "sha2 0.10.9", - "stack-profile 0.43.0", + "stack-profile 0.43.1", "thiserror 1.0.69", "tokio", "tracing", @@ -4124,7 +4124,7 @@ dependencies = [ [[package]] name = "stack-encrypt" -version = "0.2.0" +version = "0.3.0" dependencies = [ "base64ct", "cllw-ore 0.5.0", @@ -4145,7 +4145,7 @@ dependencies = [ [[package]] name = "stack-encrypt-derive" -version = "0.2.0" +version = "0.3.0" dependencies = [ "proc-macro2", "quote", @@ -4154,7 +4154,7 @@ dependencies = [ [[package]] name = "stack-kms" -version = "0.1.0" +version = "0.2.0" dependencies = [ "base16ct", "base64ct", @@ -4169,8 +4169,8 @@ dependencies = [ "serde_json", "serdect", "sha2 0.10.9", - "stack-auth 0.43.0", - "stack-profile 0.43.0", + "stack-auth 0.43.1", + "stack-profile 0.43.1", "thiserror 1.0.69", "tracing", "url", @@ -4197,7 +4197,7 @@ dependencies = [ [[package]] name = "stack-profile" -version = "0.43.0" +version = "0.43.1" dependencies = [ "dirs", "gethostname", diff --git a/packages/eql/crates/eql-bindings/Cargo.toml b/packages/eql/crates/eql-bindings/Cargo.toml index 602ff78514..e3e8aca4fc 100644 --- a/packages/eql/crates/eql-bindings/Cargo.toml +++ b/packages/eql/crates/eql-bindings/Cargo.toml @@ -43,7 +43,7 @@ schemars = "1" # from the PACKAGED crate against the registry stack-encrypt instead: # `cargo publish --dry-run --all-features` in test-eql.yml at PR time, and # `publish_all_features` in ../../release-plz.toml at release time. -stack-encrypt = { path = "../../../stack-encrypt", version = "0.2.0", optional = true, default-features = false } +stack-encrypt = { path = "../../../stack-encrypt", version = "0.3.0", optional = true, default-features = false } # The vitaminc line stack-encrypt is built on. `PrfValue` is named from this # crate directly (stack-encrypt re-exports only `IntoPrfContext` and # `PrfContext`), and two vitaminc lines in one graph do not interoperate, so diff --git a/packages/stack-auth/Cargo.toml b/packages/stack-auth/Cargo.toml index 3fae08b092..ab7d14f933 100644 --- a/packages/stack-auth/Cargo.toml +++ b/packages/stack-auth/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "stack-auth" description = "Authentication library for CipherStash services" -version = "0.43.0" +version = "0.43.1" edition.workspace = true authors.workspace = true repository.workspace = true diff --git a/packages/stack-auth/fuzz/Cargo.lock b/packages/stack-auth/fuzz/Cargo.lock index 99bbb9c36a..4e9df84138 100644 --- a/packages/stack-auth/fuzz/Cargo.lock +++ b/packages/stack-auth/fuzz/Cargo.lock @@ -2594,7 +2594,7 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "stack-auth" -version = "0.43.0" +version = "0.43.1" dependencies = [ "aquamarine", "base64", @@ -2634,7 +2634,7 @@ dependencies = [ [[package]] name = "stack-profile" -version = "0.43.0" +version = "0.43.1" dependencies = [ "dirs", "gethostname", diff --git a/packages/stack-auth/src/device_client.rs b/packages/stack-auth/src/device_client.rs index 7490c1cf66..97fac32bc4 100644 --- a/packages/stack-auth/src/device_client.rs +++ b/packages/stack-auth/src/device_client.rs @@ -11,7 +11,7 @@ use zerokms_protocol::{CreateClientRequest, CreateClientResponse, ViturKeyMateri use crate::error::RequestError; use crate::transport::{self, ReqwestTransport}; -use crate::{ensure_trailing_slash, ServiceToken, Token}; +use crate::{ServiceToken, Token}; // --------------------------------------------------------------------------- // Secret key file (output) @@ -96,7 +96,20 @@ impl ErrorPayload for DeviceClientError { /// /// If the secret key already exists on disk, or the server returns 409 /// (conflict), this is a no-op. +/// +/// # Errors +/// +/// Returns [`DeviceClientError::Auth`] before sending credentials when the +/// token lacks a valid CipherStash-hosted ZeroKMS claim. Profile, transport, +/// response and persistence failures are reported by the other variants. pub async fn bind_client_device(store: &ProfileStore) -> Result<(), DeviceClientError> { + bind_client_device_with_transport(store, &transport::share(ReqwestTransport::default())).await +} + +async fn bind_client_device_with_transport( + store: &ProfileStore, + transport: &transport::SharedTransport, +) -> Result<(), DeviceClientError> { let ws_store = store.current_workspace_store()?; if ws_store.exists(SECRET_KEY_FILENAME) { @@ -106,7 +119,7 @@ pub async fn bind_client_device(store: &ProfileStore) -> Result<(), DeviceClient let token: Token = ws_store.load_profile()?; let service_token = ServiceToken::new(token.access_token().clone()); - let zerokms_url = ensure_trailing_slash(service_token.zerokms_url()?); + let zerokms_url = service_token.hosted_zerokms_endpoint()?.as_url(); // DeviceIdentity is NOT workspace-scoped, so this reads from the root. let identity = DeviceIdentity::load_or_create(store)?; @@ -124,7 +137,7 @@ pub async fn bind_client_device(store: &ProfileStore) -> Result<(), DeviceClient serde_json::to_vec(&request).map_err(|e| RequestError(Box::new(e)))?, ); let response = transport::post( - &transport::share(ReqwestTransport::default()), + transport, url, "application/json", // The shared transport adds `content-type` and the crate's @@ -267,6 +280,63 @@ mod tests { server } + // Exercise hosted-claim validation normally, but carry the request to a + // per-test local server through the transport seam. + struct LocalTransport(url::Url); + + impl crate::transport::HttpTransport for LocalTransport { + async fn send( + &self, + request: crate::transport::HttpRequest, + ) -> Result { + use crate::transport::HttpRequest; + assert_eq!( + request.url().as_str(), + "https://us-east-1.aws.viturhosted.net/create-client", + "provisioning must use the validated hosted destination" + ); + assert_eq!(request.method(), "POST", "provisioning uses POST"); + let local = HttpRequest::new( + "POST", + self.0.join(request.url().path()).unwrap(), + request.headers().to_vec(), + request.body().to_vec(), + ); + ReqwestTransport::default().send(local).await + } + } + + mod given_a_profile_token_naming_an_untrusted_endpoint { + use super::*; + + #[tokio::test] + async fn refuses_provisioning_before_sending_the_bearer() { + let dir = TempDir::new().unwrap(); + let store = ProfileStore::new(dir.path()); + let mut mocks = MockSet::new(); + mocks.mock(|when, then| { + when.post().path("/create-client"); + then.json(client_response_json()); + }); + let server = start_server(mocks).await; + save_test_token(&store, &make_test_jwt(server.url("/"))); + + let result = bind_client_device(&store).await; + + assert!( + matches!(result, Err(DeviceClientError::Auth(_))), + "an untrusted services claim must be refused before provisioning; got {result:?}" + ); + assert!( + !store + .workspace_store(TEST_WORKSPACE_ID) + .unwrap() + .exists(SECRET_KEY_FILENAME), + "a refused endpoint must not create a device credential" + ); + } + } + #[tokio::test] async fn provisions_and_saves_secret_key() { let dir = TempDir::new().unwrap(); @@ -289,10 +359,15 @@ mod tests { }); let server = start_server(mocks).await; - let jwt = make_test_jwt(server.url("/")); + let jwt = make_test_jwt("https://us-east-1.aws.viturhosted.net/"); save_test_token(&store, &jwt); - bind_client_device(&store).await.unwrap(); + bind_client_device_with_transport( + &store, + &transport::share(LocalTransport(server.url("/"))), + ) + .await + .unwrap(); let ws_store = store.workspace_store(TEST_WORKSPACE_ID).unwrap(); let saved: serde_json::Value = ws_store.load(SECRET_KEY_FILENAME).unwrap(); @@ -339,10 +414,15 @@ mod tests { }); let server = start_server(mocks).await; - let jwt = make_test_jwt(server.url("/")); + let jwt = make_test_jwt("https://us-east-1.aws.viturhosted.net/"); save_test_token(&store, &jwt); - bind_client_device(&store).await.unwrap(); + bind_client_device_with_transport( + &store, + &transport::share(LocalTransport(server.url("/"))), + ) + .await + .unwrap(); let ws_store = store.workspace_store(TEST_WORKSPACE_ID).unwrap(); assert!( @@ -364,10 +444,15 @@ mod tests { }); let server = start_server(mocks).await; - let jwt = make_test_jwt(server.url("/")); + let jwt = make_test_jwt("https://us-east-1.aws.viturhosted.net/"); save_test_token(&store, &jwt); - let err = bind_client_device(&store).await.unwrap_err(); + let err = bind_client_device_with_transport( + &store, + &transport::share(LocalTransport(server.url("/"))), + ) + .await + .unwrap_err(); assert!( matches!(err, DeviceClientError::Server { status: 500, .. }), "expected Server error, got: {err:?}" diff --git a/packages/stack-auth/src/hosted_zerokms.rs b/packages/stack-auth/src/hosted_zerokms.rs new file mode 100644 index 0000000000..bec882cab6 --- /dev/null +++ b/packages/stack-auth/src/hosted_zerokms.rs @@ -0,0 +1,81 @@ +//! The trust boundary for ZeroKMS endpoints read from unverified claims. + +use cts_common::{Region, ZeroKmsServiceDiscovery}; +use url::Url; + +/// A ZeroKMS claim URL outside the supported CipherStash-hosted service. +/// +/// The message never includes the untrusted URL. +#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] +#[error("the token must name a supported CipherStash-hosted ZeroKMS endpoint")] +pub struct InvalidHostedZeroKmsEndpoint; + +/// A validated CipherStash-hosted ZeroKMS endpoint and its region. +/// +/// Claims may select HTTPS on the default port at the root path of +/// `..viturhosted.net` or +/// `..zerokms.cipherstashmanaged.net`. The region must be +/// known to this build's `cts-common`; future regions and staging hosts +/// require an SDK upgrade or an explicitly configured KMS endpoint. +/// +/// This validates the destination, not the token's signature or authority. +/// +/// ``` +/// use stack_auth::HostedZeroKmsEndpoint; +/// let endpoint = HostedZeroKmsEndpoint::new( +/// "https://us-east-1.aws.zerokms.cipherstashmanaged.net/".parse()?, +/// )?; +/// assert_eq!(endpoint.region().identifier(), "us-east-1.aws"); +/// # Ok::<(), Box>(()) +/// ``` +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct HostedZeroKmsEndpoint { + url: Url, + region: Region, +} + +impl HostedZeroKmsEndpoint { + /// Validate an endpoint from an unverified service claim. + /// + /// # Errors + /// + /// Returns [`InvalidHostedZeroKmsEndpoint`] for any other destination, + /// including userinfo, query strings and fragments. + pub fn new(url: Url) -> Result { + if url.scheme() != "https" + || url.port().is_some() + || url.path() != "/" + || !url.username().is_empty() + || url.password().is_some() + || url.query().is_some() + || url.fragment().is_some() + { + return Err(InvalidHostedZeroKmsEndpoint); + } + let host = url.host_str().ok_or(InvalidHostedZeroKmsEndpoint)?; + if !host.ends_with(".viturhosted.net") && !host.ends_with(".zerokms.cipherstashmanaged.net") + { + return Err(InvalidHostedZeroKmsEndpoint); + } + // cts-common owns region extraction but currently knows only the + // legacy domain. Normalize the one additional trusted suffix before + // delegating, retaining its exact-label and region checks. + let legacy_host = host + .strip_suffix(".zerokms.cipherstashmanaged.net") + .map(|prefix| format!("{prefix}.viturhosted.net")); + let region = + ZeroKmsServiceDiscovery::region_from_host_fqdn(legacy_host.as_deref().unwrap_or(host)) + .map_err(|_| InvalidHostedZeroKmsEndpoint)?; + Ok(Self { url, region }) + } + + /// The validated HTTPS URL. + pub fn as_url(&self) -> &Url { + &self.url + } + + /// The service's region, independent of its hosted domain. + pub fn region(&self) -> Region { + self.region + } +} diff --git a/packages/stack-auth/src/lib.rs b/packages/stack-auth/src/lib.rs index 11adc7bb85..f591fac38a 100644 --- a/packages/stack-auth/src/lib.rs +++ b/packages/stack-auth/src/lib.rs @@ -56,6 +56,7 @@ mod access_key; mod auth_strategy_fn; mod clock; mod error; +mod hosted_zerokms; mod service_token; mod token; mod token_store; @@ -116,6 +117,7 @@ pub use access_key_strategy::{AccessKeyStrategy, AccessKeyStrategyBuilder}; pub use auth_strategy_fn::AuthStrategyFn; pub use auto_strategy::{AutoStrategy, AutoStrategyBuilder}; pub use device_session_strategy::{DeviceSessionStrategy, DeviceSessionStrategyBuilder}; +pub use hosted_zerokms::{HostedZeroKmsEndpoint, InvalidHostedZeroKmsEndpoint}; pub use oidc_federation_strategy::{OidcFederationStrategy, OidcFederationStrategyBuilder}; pub use oidc_refresher::{OidcProvider, OidcProviderFn}; pub use service_token::ServiceToken; diff --git a/packages/stack-auth/src/service_token.rs b/packages/stack-auth/src/service_token.rs index a822d227ac..30d711d5d0 100644 --- a/packages/stack-auth/src/service_token.rs +++ b/packages/stack-auth/src/service_token.rs @@ -4,15 +4,16 @@ use url::Url; use vitaminc::protected::OpaqueDebug; use zeroize::ZeroizeOnDrop; -use crate::{AuthError, SecretToken}; +use crate::{AuthError, HostedZeroKmsEndpoint, SecretToken}; /// A CipherStash service token returned by an [`AuthStrategy`](crate::AuthStrategy). /// /// Wraps a bearer credential ([`SecretToken`]) together with eagerly decoded /// JWT claims that are used for service discovery. The JWT is decoded (but -/// **not** signature-verified) using [`cts_common::claims::ClientClaims`], so -/// only CipherStash-issued service tokens (from CTS or the access-key exchange) -/// will have their claims resolved. +/// **not** signature-verified) using [`cts_common::claims::ClientClaims`]. +/// Any token matching that schema can have its claims resolved, including +/// one planted in a profile store. Hosted endpoint validation constrains +/// where such a token can send credentials; it does not authenticate it. /// /// # Decoded claims /// @@ -20,6 +21,8 @@ use crate::{AuthError, SecretToken}; /// * `workspace_id()` — the workspace identifier from the token. /// * `issuer()` — the `iss` URL, i.e. the CTS host for this workspace. /// * `zerokms_url()` — the ZeroKMS endpoint from the `services` claim. +/// * `hosted_zerokms_endpoint()` — the validated hosted destination to use +/// before sending credentials to a claim-selected endpoint. /// /// For non-JWT tokens (e.g. static test tokens) or JWTs that don't match /// the CipherStash claims schema, these methods return @@ -42,6 +45,7 @@ struct DecodedClaims { workspace: WorkspaceId, issuer: Url, services: Services, + hosted_zerokms: Result, } impl ServiceToken { @@ -152,6 +156,8 @@ impl ServiceToken { /// /// CTS-issued JWTs include a `services` claim containing a map of service /// type to endpoint URL. This method looks up the `zerokms` entry. + /// The returned URL is untrusted. Use [`hosted_zerokms_endpoint`](Self::hosted_zerokms_endpoint) + /// before sending a bearer credential to a destination chosen by a claim. /// /// # Errors /// @@ -168,6 +174,26 @@ impl ServiceToken { }) } + /// Return the validated CipherStash-hosted ZeroKMS destination. + /// + /// Validation is cached when this token is decoded. Both device + /// provisioning and KMS discovery use this accessor before sending a + /// bearer credential. It does not verify the token's signature. + /// + /// # Errors + /// + /// Returns [`AuthError::InvalidToken`] when claims cannot be decoded, + /// ZeroKMS is missing, or the endpoint is not a supported hosted service. + pub fn hosted_zerokms_endpoint(&self) -> Result<&HostedZeroKmsEndpoint, AuthError> { + let decoded = self.decoded.as_ref().map_err(|reason| { + AuthError::InvalidToken(crate::error::InvalidToken(reason.clone())) + })?; + decoded + .hosted_zerokms + .as_ref() + .map_err(|reason| AuthError::InvalidToken(crate::error::InvalidToken((*reason).into()))) + } + /// Attempt to decode the JWT claims from the token string. /// /// NOTE: This does not verify the token signature or validate any claims, @@ -179,11 +205,21 @@ impl ServiceToken { .parse() .map_err(|e| format!("iss claim is not a valid URL: {e}"))?; + let hosted_zerokms = claims + .services + .get(ServiceType::ZeroKms) + .ok_or("Token does not include a ZeroKMS endpoint in the services claim") + .and_then(|url| { + HostedZeroKmsEndpoint::new(url.clone()).map_err(|_| { + "Token does not name a supported CipherStash-hosted ZeroKMS endpoint" + }) + }); Ok(DecodedClaims { subject: claims.sub, workspace: claims.workspace, issuer, services: claims.services, + hosted_zerokms, }) } } @@ -317,6 +353,56 @@ mod tests { ); } + mod given_a_hosted_zerokms_claim { + use super::*; + + #[test] + fn returns_the_same_cached_validated_endpoint() { + let jwt = make_jwt( + "https://cts.example.com/", + services_with_zerokms("https://us-east-1.aws.zerokms.cipherstashmanaged.net/"), + ); + let token = ServiceToken::new(SecretToken::new(jwt)); + let first = token.hosted_zerokms_endpoint().unwrap(); + let again = token.hosted_zerokms_endpoint().unwrap(); + assert!( + std::ptr::eq(first, again), + "validation is cached on the decoded token" + ); + assert_eq!( + first.region().identifier(), + "us-east-1.aws", + "the validated endpoint carries its region" + ); + } + } + + mod given_an_untrusted_zerokms_claim { + use super::*; + + #[test] + fn refuses_the_validated_accessor_without_exposing_the_claim() { + for url in [ + "https://marker.example/", + "http://us-east-1.aws.viturhosted.net/", + "https://marker@us-east-1.aws.viturhosted.net/", + "https://us-east-1.aws.viturhosted.net/?marker=secret", + ] { + let jwt = make_jwt("https://cts.example.com/", services_with_zerokms(url)); + let token = ServiceToken::new(SecretToken::new(jwt)); + let error = token.hosted_zerokms_endpoint().unwrap_err(); + assert!( + matches!(error, AuthError::InvalidToken(_)), + "untrusted endpoints are invalid tokens" + ); + assert!( + !error.to_string().contains("marker"), + "errors must not expose claim content" + ); + } + } + } + #[test] fn zerokms_url_errors_when_services_claim_missing() { let jwt = make_jwt("https://cts.example.com/", None); diff --git a/packages/stack-encrypt-derive/Cargo.toml b/packages/stack-encrypt-derive/Cargo.toml index 223cd01bca..0e90b04d01 100644 --- a/packages/stack-encrypt-derive/Cargo.toml +++ b/packages/stack-encrypt-derive/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "stack-encrypt-derive" description = "Derive macros for stack-encrypt's target-directed encryption" -version = "0.2.0" +version = "0.3.0" edition.workspace = true authors.workspace = true repository.workspace = true diff --git a/packages/stack-encrypt/Cargo.toml b/packages/stack-encrypt/Cargo.toml index d62c87f477..579888f0ef 100644 --- a/packages/stack-encrypt/Cargo.toml +++ b/packages/stack-encrypt/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "stack-encrypt" description = "Encrypt Rust values under per-value ZeroKMS data keys via the vitaminc cipher traits" -version = "0.2.0" +version = "0.3.0" edition.workspace = true authors.workspace = true repository.workspace = true @@ -17,12 +17,12 @@ license-file = "LICENSE" # `StackCipher::new()` fall back to the client key `stash auth login` writes # to the profile directory (`secretkey.json`) when CS_CLIENT_ID / # CS_CLIENT_KEY are not set — the same place `AutoStrategy` finds the token. -stack-kms = { path = "../stack-kms", version = "0.1.0", default-features = false, features = ["profile"] } +stack-kms = { path = "../stack-kms", version = "0.2.0", default-features = false, features = ["profile"] } # `StackCipher::new()` builds a ZeroKMS client from the environment, so its # return type names the auto-detected auth strategy. Only needed with `http`. stack-auth = { workspace = true, optional = true } # `#[derive(EncryptFrom)]` / `#[derive(DecryptInto)]`, re-exported from `target`. -stack-encrypt-derive = { path = "../stack-encrypt-derive", version = "0.2.0" } +stack-encrypt-derive = { path = "../stack-encrypt-derive", version = "0.3.0" } # The workspace vitaminc (0.5.1): one canonical context encoding shared by # the AEAD and PRF derivations (`IntoContext`, cipherstash/vitaminc#339), diff --git a/packages/stack-encrypt/fuzz/Cargo.lock b/packages/stack-encrypt/fuzz/Cargo.lock index ad55ba0c62..a02bb9ce53 100644 --- a/packages/stack-encrypt/fuzz/Cargo.lock +++ b/packages/stack-encrypt/fuzz/Cargo.lock @@ -1952,7 +1952,7 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "stack-auth" -version = "0.43.0" +version = "0.43.1" dependencies = [ "aquamarine", "base64", @@ -1979,7 +1979,7 @@ dependencies = [ [[package]] name = "stack-encrypt" -version = "0.2.0" +version = "0.3.0" dependencies = [ "base64ct", "cllw-ore", @@ -2001,7 +2001,7 @@ dependencies = [ [[package]] name = "stack-encrypt-derive" -version = "0.2.0" +version = "0.3.0" dependencies = [ "proc-macro2", "quote", @@ -2021,7 +2021,7 @@ dependencies = [ [[package]] name = "stack-kms" -version = "0.1.0" +version = "0.2.0" dependencies = [ "base16ct", "base64ct", @@ -2050,7 +2050,7 @@ dependencies = [ [[package]] name = "stack-profile" -version = "0.43.0" +version = "0.43.1" dependencies = [ "dirs", "gethostname", diff --git a/packages/stack-kms/Cargo.toml b/packages/stack-kms/Cargo.toml index d9269de3ce..cd2a63a36a 100644 --- a/packages/stack-kms/Cargo.toml +++ b/packages/stack-kms/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "stack-kms" description = "Standalone client for ZeroKMS key generation and retrieval" -version = "0.1.0" +version = "0.2.0" edition.workspace = true authors.workspace = true repository.workspace = true diff --git a/packages/stack-kms/fuzz/Cargo.lock b/packages/stack-kms/fuzz/Cargo.lock index 73f49856b7..718d6678ec 100644 --- a/packages/stack-kms/fuzz/Cargo.lock +++ b/packages/stack-kms/fuzz/Cargo.lock @@ -1937,7 +1937,7 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "stack-auth" -version = "0.43.0" +version = "0.43.1" dependencies = [ "aquamarine", "base64", @@ -1964,7 +1964,7 @@ dependencies = [ [[package]] name = "stack-kms" -version = "0.1.0" +version = "0.2.0" dependencies = [ "base16ct", "base64ct", @@ -2001,7 +2001,7 @@ dependencies = [ [[package]] name = "stack-profile" -version = "0.43.0" +version = "0.43.1" dependencies = [ "dirs", "gethostname", diff --git a/packages/stack-kms/src/client.rs b/packages/stack-kms/src/client.rs index 6bee9e20f7..cc548579b8 100644 --- a/packages/stack-kms/src/client.rs +++ b/packages/stack-kms/src/client.rs @@ -402,7 +402,12 @@ impl Client { /// `StackKms` owns the transport [`Client`], a [`stack_auth`] credential /// provider, and a [`ClientKey`]. Each operation fetches a fresh access token /// (refreshing as needed), resolves the ZeroKMS endpoint from the token's -/// `services` claim on first use, and delegates to the low-level client. +/// `services` claim, and delegates to the low-level client. Discovery accepts +/// only CipherStash-hosted HTTPS endpoints in regions known to this build. +/// Every token must carry a valid claim for the first token's region; missing, +/// untrusted, or different-region claims are refused before a request is sent. +/// A later token may move that region's endpoint between the supported hosted +/// domains. Explicitly configured endpoints bypass claim-based discovery. /// /// Build one with [`StackKmsBuilder`](crate::StackKmsBuilder) (the default /// HTTP transport, `http` feature), or with [`connect`](Self::connect) over @@ -413,22 +418,25 @@ pub struct StackKms { credentials: C, client_key: ClientKey, /// `None` when the connection was given its endpoint at init; otherwise - /// the endpoint the first token's `services` claim named, once known. - discovered: Option>, + /// the region the first token's `services` claim named, once known. + discovered: Option>, } /// `StackKms` owns the transport [`Client`], a [`stack_auth`] credential /// provider, and a [`ClientKey`]. Without the `http` feature there is no /// default connection: build one with [`connect`](Self::connect) over the /// host's [`ZeroKMSConnection`]. +/// Discovery requires every token to name a supported CipherStash-hosted +/// HTTPS endpoint in the first token's region. Later tokens can change its +/// hosted domain; explicitly configured endpoints bypass discovery. #[cfg(not(feature = "http"))] pub struct StackKms { client: Client, credentials: C, client_key: ClientKey, /// `None` when the connection was given its endpoint at init; otherwise - /// the endpoint the first token's `services` claim named, once known. - discovered: Option>, + /// the region the first token's `services` claim named, once known. + discovered: Option>, } impl StackKms @@ -452,7 +460,8 @@ where /// ZeroKMS endpoint is taken from the access token's `services` claim on /// first use unless the connection already knows one. A claimed endpoint /// must be CipherStash-hosted, and every later token must name the same - /// region; whether the connection already knows one is decided here, once. + /// region, while its hosted domain may change. Whether the connection + /// already knows one is decided here, once. pub fn connect( opts: ClientOpts, credentials: C, @@ -482,35 +491,33 @@ where /// — so: /// /// * the claim may only name a CipherStash-hosted ZeroKMS - /// ([`ZeroKmsEndpoint::from_services_claim`]), and + /// ([`stack_auth::ServiceToken::hosted_zerokms_endpoint`]), and /// * every later token must name the same region as the first one did. /// A token for another region is refused rather than sent to the - /// pinned endpoint, and it cannot move the pin. A token naming the - /// same region on the other hosted domain is accepted and still sent - /// to the pinned endpoint: both domains serve that region's ZeroKMS - /// while CipherStash moves between them, and refusing would fail - /// every call on a long-lived client when CTS switches domain. + /// pinned region. A token naming the same region on the other hosted + /// domain updates the connection, so it can keep working when the old + /// domain is retired. Missing or untrusted claims are refused even + /// after the region has been pinned. async fn get_token(&self) -> Result { let token = (&self.credentials).get_token().await?; // Configured explicitly: the claim is not consulted. let Some(discovered) = &self.discovered else { return Ok(token); }; - let claimed = ZeroKmsEndpoint::from_services_claim(token.zerokms_url()?)?; - let pinned = discovered.get_or_init(|| { - tracing::debug!( - target: "stack_kms", - endpoint = %claimed.endpoint, - "resolved ZeroKMS endpoint from the token's services claim" - ); - self.client - .connection() - .ensure_base_url(claimed.endpoint.clone()); - claimed.clone() - }); - if pinned.region != claimed.region { + let claimed = match token.hosted_zerokms_endpoint() { + Ok(hosted) => HostedEndpoint { + endpoint: hosted.into(), + region: hosted.region(), + }, + // Retain the KMS endpoint error classification for malformed or + // untrusted claims; successful discovery uses the shared cache. + Err(_) => ZeroKmsEndpoint::from_services_claim(token.zerokms_url()?)?, + }; + let region = discovered.get_or_init(|| claimed.region); + if *region != claimed.region { return Err(InvalidEndpoint::Mismatch.into()); } + self.client.connection().ensure_base_url(claimed.endpoint); Ok(token) } @@ -751,7 +758,7 @@ mod tests { /// Where a token's `services` claim may send key operations (CIP-5152). /// The claim is unverified, so it may only name a CipherStash-hosted - /// ZeroKMS, and it cannot move an endpoint already in use. + /// ZeroKMS, and it cannot change a region already in use. mod endpoint_from_the_services_claim { use super::*; use base64ct::{Base64UrlUnpadded, Encoding}; @@ -828,7 +835,7 @@ mod tests { impl ZeroKMSConnection for Discovering { fn ensure_base_url(&self, url: ZeroKmsEndpoint) { *self.calls.lock().unwrap() += 1; - self.pinned.lock().unwrap().get_or_insert(url); + *self.pinned.lock().unwrap() = Some(url); } fn has_base_url(&self) -> bool { @@ -919,8 +926,8 @@ mod tests { assert_eq!(pinned(&kms).as_deref(), Some(SYDNEY)); assert_eq!( *kms.client.connection().calls.lock().unwrap(), - 1, - "the endpoint is handed to the connection once" + 2, + "only the two valid same-region tokens update the connection" ); } @@ -944,9 +951,9 @@ mod tests { assert_eq!( pinned(&kms).as_deref(), Some(SYDNEY), - "the endpoint in use does not move" + "the endpoint follows each same-region claim" ); - assert_eq!(*kms.client.connection().calls.lock().unwrap(), 1); + assert_eq!(*kms.client.connection().calls.lock().unwrap(), 3); } #[tokio::test] @@ -960,7 +967,33 @@ mod tests { kms.get_token() .await .expect("same region on the old domain is the same ZeroKMS"); - assert_eq!(pinned(&kms).as_deref(), Some(VIRGINIA)); + assert_eq!(pinned(&kms).as_deref(), Some(VIRGINIA_OLD)); + } + + mod given_a_same_region_hosted_domain_migration { + use super::*; + + #[tokio::test] + async fn subsequent_requests_use_the_new_domain() { + const SYDNEY_NEW: &str = + "https://ap-southeast-2.aws.zerokms.cipherstashmanaged.net/"; + let kms = kms( + None, + Tokens::new([jwt(Some(SYDNEY)), jwt(Some(SYDNEY_NEW))]), + ); + kms.get_token() + .await + .expect("the first token discovers Sydney"); + kms.get_token() + .await + .expect("the refreshed token names the same region"); + + assert_eq!( + pinned(&kms).as_deref(), + Some(SYDNEY_NEW), + "the connection must follow the trusted same-region domain migration before the old domain retires" + ); + } } #[tokio::test] @@ -994,6 +1027,91 @@ mod tests { ); } + mod stress_tests { + use super::*; + use std::sync::Arc; + use tokio::sync::Barrier; + + struct ConcurrentTokens { + tokens: Tokens, + barrier: Barrier, + } + + impl AuthStrategy for &ConcurrentTokens { + async fn get_token( + self, + ) -> Result { + let token = (&self.tokens).get_token().await?; + self.barrier.wait().await; + Ok(token) + } + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] + async fn concurrent_discovery_keeps_one_region() { + const CALLERS: usize = 32; + let tokens = std::array::from_fn::<_, CALLERS, _>(|i| { + jwt(Some(if i % 2 == 0 { SYDNEY } else { VIRGINIA })) + }); + let kms = Arc::new( + StackKms::<_, Discovering>::connect( + ClientOpts::new(None), + ConcurrentTokens { + tokens: Tokens::new(tokens), + barrier: Barrier::new(CALLERS), + }, + random_client_key(), + ) + .unwrap(), + ); + let mut callers = Vec::new(); + for _ in 0..CALLERS { + let kms = Arc::clone(&kms); + callers.push(tokio::spawn(async move { kms.get_token().await })); + } + let mut accepted_region = None; + let mut accepted = 0; + for caller in callers { + match caller.await.unwrap() { + Ok(token) => { + let region = token.hosted_zerokms_endpoint().unwrap().region(); + assert_eq!( + *accepted_region.get_or_insert(region), + region, + "only one region may win discovery" + ); + accepted += 1; + } + Err(error) => assert!( + matches!(error, Error::InvalidEndpoint(InvalidEndpoint::Mismatch)), + "other regions must fail before transport: {error:?}" + ), + } + } + assert_eq!( + accepted, + CALLERS / 2, + "every token for the winning region is accepted" + ); + let endpoint = kms + .client + .connection() + .pinned + .lock() + .unwrap() + .clone() + .unwrap(); + let region = stack_auth::HostedZeroKmsEndpoint::new(endpoint.as_url().clone()) + .unwrap() + .region(); + assert_eq!( + Some(region), + accepted_region, + "the transport uses the winning region" + ); + } + } + #[tokio::test] async fn a_later_token_without_the_claim_is_refused() { let kms = kms(None, Tokens::new([jwt(Some(SYDNEY)), jwt(None)])); diff --git a/packages/stack-kms/src/connection.rs b/packages/stack-kms/src/connection.rs index 90e2842c8b..e6c5b5ce28 100644 --- a/packages/stack-kms/src/connection.rs +++ b/packages/stack-kms/src/connection.rs @@ -48,18 +48,20 @@ pub trait ZeroKMSConnection: ZeroKMSConnectionInit { access_token: &str, ) -> impl Future> + crate::MaybeSend; - /// Record the ZeroKMS endpoint if none is known yet. + /// Update the discovered ZeroKMS endpoint, preserving explicit configuration. /// /// [`StackKms`](crate::StackKms) calls this with the endpoint named by the - /// access token's `services` claim the first time it holds a token. + /// access token's `services` claim after validating its hosted domain + /// and checking that its region matches the first discovered region. /// /// Endpoint discovery is `StackKms` policy, not something every transport /// needs: the default is a no-op, paired with a `has_base_url` of `true`, /// which is correct for a transport that does not build URLs from a base /// (it dispatches on the request's endpoint name) or that was pinned at /// init. A transport that *does* want discovery must override **both**, - /// and its `ensure_base_url` must keep the first value it is given — an - /// endpoint pinned at init must not be overridden by a later token. + /// and its `ensure_base_url` must accept later discovered URLs so a + /// hosted domain can migrate. An endpoint configured at init must stay + /// fixed. Snapshot the URL for each request before awaiting network I/O. fn ensure_base_url(&self, _url: ZeroKmsEndpoint) {} /// Whether an endpoint is known, either from init or from a previous diff --git a/packages/stack-kms/src/connection/http.rs b/packages/stack-kms/src/connection/http.rs index 831cae9e56..e65369b52b 100644 --- a/packages/stack-kms/src/connection/http.rs +++ b/packages/stack-kms/src/connection/http.rs @@ -11,7 +11,7 @@ use reqwest::header::HeaderMap; use serde_json::to_vec; #[cfg(not(target_arch = "wasm32"))] use std::time::Duration; -use std::{collections::HashMap, sync::OnceLock}; +use std::{collections::HashMap, sync::Mutex}; use thiserror::Error; use zerokms_protocol::{ViturRequest, ViturRequestError}; @@ -126,7 +126,8 @@ impl HttpConnectionOpts { } pub struct HttpConnection { - base_url: OnceLock, + base_url: Mutex>, + explicitly_configured: bool, client: reqwest::Client, } @@ -181,24 +182,31 @@ impl ZeroKMSConnectionInit for HttpConnection { let client = builder.build()?; - let base_url = OnceLock::new(); - if let Some(url) = opts.base_url { - // Pre-fill when an explicit URL was provided at build time. - let _ = base_url.set(url); - } - - Ok(Self { base_url, client }) + let explicitly_configured = opts.base_url.is_some(); + let base_url = Mutex::new(opts.base_url); + Ok(Self { + base_url, + explicitly_configured, + client, + }) } } impl ZeroKMSConnection for HttpConnection { fn ensure_base_url(&self, url: ZeroKmsEndpoint) { - // OnceLock::set returns Err if already set — that's fine, we keep the first value. - let _ = self.base_url.set(url); + if !self.explicitly_configured { + *self + .base_url + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = Some(url); + } } fn has_base_url(&self) -> bool { - self.base_url.get().is_some() + self.base_url + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .is_some() } async fn send( @@ -209,17 +217,24 @@ impl ZeroKMSConnection for HttpConnection { let body = to_vec(&request) .map_err(|e| ViturRequestError::prepare("Failed to serialize request", e))?; - let base_url = self.base_url.get().ok_or_else(|| { - // A missing base URL is a client-side configuration problem (the - // token carried no ZeroKMS `services` claim and none was set - // explicitly), not an authentication failure — classify it as a - // request-preparation error so callers don't mistake it for a 401 - // and trigger a token refresh/reauth loop. - ViturRequestError::prepare( - "ZeroKMS base URL was not resolved from the token's services claim", - BaseUrlUnresolved, - ) - })?; + // Snapshot the destination before any await. A concurrent refresh may + // update the same region's domain without changing this request's URL. + let base_url = self + .base_url + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone() + .ok_or_else(|| { + // A missing base URL is a client-side configuration problem (the + // token carried no ZeroKMS `services` claim and none was set + // explicitly), not an authentication failure — classify it as a + // request-preparation error so callers don't mistake it for a 401 + // and trigger a token refresh/reauth loop. + ViturRequestError::prepare( + "ZeroKMS base URL was not resolved from the token's services claim", + BaseUrlUnresolved, + ) + })?; let url = base_url.request_url(Request::ENDPOINT); @@ -285,16 +300,43 @@ mod base_url_tests { c.ensure_base_url(endpoint("https://a.example")); assert!(c.has_base_url()); - assert_eq!(c.base_url.get().unwrap().as_str(), "https://a.example/"); + assert_eq!( + c.base_url.lock().unwrap().as_ref().unwrap().as_str(), + "https://a.example/" + ); } #[test] - fn the_first_ensured_url_wins() { + fn the_latest_discovered_url_is_used() { let c = conn(None); c.ensure_base_url(endpoint("https://first.example")); c.ensure_base_url(endpoint("https://second.example")); - assert_eq!(c.base_url.get().unwrap().as_str(), "https://first.example/"); + assert_eq!( + c.base_url.lock().unwrap().as_ref().unwrap().as_str(), + "https://second.example/", + "discovery follows the latest validated claim" + ); + } + + #[test] + fn a_discovered_endpoint_follows_a_hosted_domain_migration() { + let c = conn(None); + c.ensure_base_url(endpoint("https://us-east-1.aws.viturhosted.net/")); + c.ensure_base_url(endpoint( + "https://us-east-1.aws.zerokms.cipherstashmanaged.net/", + )); + assert_eq!( + c.base_url + .lock() + .unwrap() + .as_ref() + .unwrap() + .request_url("retrieve-data-key") + .as_str(), + "https://us-east-1.aws.zerokms.cipherstashmanaged.net/retrieve-data-key", + "request URLs must follow a validated domain migration" + ); } #[test] @@ -304,7 +346,10 @@ mod base_url_tests { c.ensure_base_url(endpoint("https://other.example")); - assert_eq!(c.base_url.get().unwrap().as_str(), "https://init.example/"); + assert_eq!( + c.base_url.lock().unwrap().as_ref().unwrap().as_str(), + "https://init.example/" + ); } #[test] @@ -316,7 +361,9 @@ mod base_url_tests { assert_eq!( c.base_url - .get() + .lock() + .unwrap() + .as_ref() .unwrap() .request_url(zerokms_protocol::RetrieveKeyRequest::ENDPOINT) .as_str(), diff --git a/packages/stack-kms/src/endpoint.rs b/packages/stack-kms/src/endpoint.rs index 61392c6e0b..bf3e3bb8e1 100644 --- a/packages/stack-kms/src/endpoint.rs +++ b/packages/stack-kms/src/endpoint.rs @@ -14,6 +14,7 @@ use url::Url; /// it can carry whatever its author chose. The URL stays in the variant for a /// caller in this process. #[derive(Debug, Error, PartialEq, Eq, miette::Diagnostic)] +#[non_exhaustive] pub enum InvalidEndpoint { /// The value did not parse as a URL at all. #[error("not a valid URL: {0}")] @@ -90,29 +91,11 @@ impl stack_auth::ErrorPayload for InvalidEndpoint { } } -/// The domains CipherStash serves ZeroKMS from. A hosted ZeroKMS is -/// `..`; `cts_common` knows only the first, so -/// the list lives here. -const HOSTED_ZEROKMS_DOMAINS: &[&str] = &["viturhosted.net", "zerokms.cipherstashmanaged.net"]; - -/// The region of `host` when it is `..` for a -/// hosted domain and a region CipherStash runs. `host` comes from a parsed -/// [`Url`], so it is already lowercase and IDNA-normalised. -fn hosted_zerokms_region(host: &str) -> Option { - HOSTED_ZEROKMS_DOMAINS.iter().find_map(|domain| { - host.strip_suffix(domain) - .and_then(|prefix| prefix.strip_suffix('.')) - // Exactly two labels: `.`. - .filter(|region| region.matches('.').count() == 1) - .and_then(|region| cts_common::Region::new(region).ok()) - }) -} - /// A CipherStash-hosted ZeroKMS endpoint named by a token's `services` /// claim, with the region its host names. /// -/// Each region's ZeroKMS is served under every domain in -/// `HOSTED_ZEROKMS_DOMAINS` while CipherStash moves from `viturhosted.net` +/// Each region's ZeroKMS is served under both supported domains +/// while CipherStash moves from `viturhosted.net` /// to `zerokms.cipherstashmanaged.net`, so two claims for the same region /// name the same service. Compare claims by [`region`](Self::region), not /// by URL: a client pinned to one domain must keep working when CTS starts @@ -152,6 +135,13 @@ pub(crate) struct HostedEndpoint { #[derive(Debug, Clone, PartialEq, Eq)] pub struct ZeroKmsEndpoint(Url); +impl From<&stack_auth::HostedZeroKmsEndpoint> for ZeroKmsEndpoint { + fn from(endpoint: &stack_auth::HostedZeroKmsEndpoint) -> Self { + // The shared type has already enforced stricter rules than `new`. + Self(endpoint.as_url().clone()) + } +} + impl ZeroKmsEndpoint { /// Validate and normalise a parsed [`Url`]. pub fn new(mut url: Url) -> Result { @@ -199,12 +189,12 @@ impl ZeroKmsEndpoint { pub(crate) fn from_services_claim(url: Url) -> Result { let endpoint = Self::new(url)?; let url = endpoint.as_url(); - let region = (url.scheme() == "https" && url.port().is_none() && url.path() == "/") - .then(|| url.host_str().and_then(hosted_zerokms_region)) - .flatten(); - match region { - Some(region) => Ok(HostedEndpoint { endpoint, region }), - None => Err(InvalidEndpoint::NotHosted(endpoint.0.into())), + match stack_auth::HostedZeroKmsEndpoint::new(url.clone()) { + Ok(hosted) => Ok(HostedEndpoint { + endpoint, + region: hosted.region(), + }), + Err(_) => Err(InvalidEndpoint::NotHosted(endpoint.0.into())), } } @@ -414,7 +404,7 @@ mod tests { #[test] fn accepts_every_region_cipherstash_runs_on_every_hosted_domain() { - for domain in HOSTED_ZEROKMS_DOMAINS { + for domain in ["viturhosted.net", "zerokms.cipherstashmanaged.net"] { for region in cts_common::Region::all() { let s = format!("https://{}.{domain}/", region.identifier()); assert_eq!(claimed(&s).map(|e| e.to_string()), Ok(s.clone()), "{s}"); diff --git a/packages/stack-kms/src/lib.rs b/packages/stack-kms/src/lib.rs index ee484f39e1..3e5bf3c40f 100644 --- a/packages/stack-kms/src/lib.rs +++ b/packages/stack-kms/src/lib.rs @@ -15,6 +15,27 @@ //! Encryption/decryption, keyset and client management, and config save/load //! all remain in `cipherstash-client` for now. //! +//! # Upgrading to 0.2 +//! +//! Endpoint discovery now requires a valid CipherStash-hosted ZeroKMS claim +//! on **every** token, including tokens fetched after initial discovery. +//! Missing claims, non-hosted URLs and a change of region fail before sending +//! credentials. Supported domains are `viturhosted.net` and +//! `zerokms.cipherstashmanaged.net`; HTTPS, the default port, a root path and +//! a region known to this build's `cts-common` are required. Future regions +//! require an SDK upgrade. Gateways, staging and local services require an +//! explicit endpoint through the builder or `CS_ZEROKMS_HOST`. +//! +//! The first discovered **region** remains fixed, while subsequent valid +//! tokens can change its hosted domain. Custom discovery transports must +//! update their [`ZeroKMSConnection::ensure_base_url`] implementation to +//! replace discovered URLs and preserve explicitly configured URLs. Snapshot +//! each request's URL before awaiting transport work. [`InvalidEndpoint`] +//! has new `NotHosted` and `Mismatch` variants and is now non-exhaustive; +//! downstream matches must include a fallback arm. `stack-encrypt` exposes +//! this version through its `kms` re-export +//! starting in 0.3. +//! //! # Quick start //! // The quick start goes through `StackKmsBuilder`, which configures the default diff --git a/packages/stack-profile/Cargo.toml b/packages/stack-profile/Cargo.toml index b2b57354e2..09ee4c9c84 100644 --- a/packages/stack-profile/Cargo.toml +++ b/packages/stack-profile/Cargo.toml @@ -2,7 +2,7 @@ name = "stack-profile" description = "Centralised ~/.cipherstash profile file management" license-file = "LICENSE" -version = "0.43.0" +version = "0.43.1" edition.workspace = true authors.workspace = true repository.workspace = true diff --git a/skills/stash-zerokms/SKILL.md b/skills/stash-zerokms/SKILL.md index dfee7d5f6d..9d2953c60f 100644 --- a/skills/stash-zerokms/SKILL.md +++ b/skills/stash-zerokms/SKILL.md @@ -28,6 +28,19 @@ where ZeroKMS is from the token itself. You never configure a ZeroKMS URL, which is also why the `CS_*_HOST` override variables are debug-only and must not appear in CI or examples. +Rust `stack-kms` 0.2 (also re-exported by `stack-encrypt` 0.3) validates the +destination on every token, including refreshes: HTTPS at the root path on +the default port, under `viturhosted.net` or +`zerokms.cipherstashmanaged.net`, in a region known to that SDK build. Keep +one client per region. Later tokens may move its hosted domain, but a missing, +untrusted, or different-region claim fails before sending credentials. For +a newly introduced region, upgrade the SDK. For an intentionally configured +gateway, staging service or local service, supply an explicit Rust KMS +endpoint; normal hosted deployments use discovery. Custom Rust transports +must let `ensure_base_url` update discovered URLs while preserving explicit +configuration. Device provisioning also validates hosted claims; editing a +profile token is not an endpoint configuration mechanism. + ## The model in one paragraph Every value is encrypted **under a keyset**. A **client** (an application From ffb5768223b9a57ab5e9e8900e4677df38fea95b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 02:44:16 +0000 Subject: [PATCH 6/8] fix(stack-kms)!: pin a claimed ZeroKMS region only after ZeroKMS accepts a token The services claim is unverified, and StackKms pinned the region it named before any request was made. A forged first token naming another hosted region therefore fixed the client to that region: ZeroKMS there refused the forged token, and every real token after it failed with a region mismatch until the client was rebuilt. The endpoint now travels with each request instead of being stored on the connection, and each token goes to the endpoint its own claim names. The region is pinned when ZeroKMS first answers a request without error; from then on a token for another region is refused before it is sent. Routing by the token's own claim also follows a same-region domain migration without a shared mutable URL, so concurrent first callers holding tokens for different regions can never send one token to the other's region. A claimed URL that is refused is no longer kept in the error either. NotHosted held the URL and the derived Debug printed it, so `?err` logged whatever path or query the token's author chose. NotHosted now holds nothing, and a claim refused for a query, fragment or missing host maps to it rather than to the variants that keep the URL. BREAKING CHANGE: `ZeroKMSConnection::send` takes the endpoint to use as a third argument, `ensure_base_url` is removed, the low-level `Client` key methods take that endpoint after the access token, and `InvalidEndpoint::NotHosted` is a unit variant. Refs #1546 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012Jmn7o2Eouw3CQpnBNGrwC --- .../validate-hosted-zerokms-discovery.md | 4 +- languages/golang/encrypt/guest/src/host.rs | 40 +- packages/stack-kms/src/builder.rs | 5 +- packages/stack-kms/src/client.rs | 576 ++++++++++++------ .../stack-kms/src/client/test_connection.rs | 6 +- packages/stack-kms/src/connection.rs | 43 +- packages/stack-kms/src/connection/http.rs | 129 ++-- packages/stack-kms/src/endpoint.rs | 65 +- packages/stack-kms/src/errors.rs | 5 +- packages/stack-kms/src/lib.rs | 19 +- skills/stash-zerokms/SKILL.md | 9 +- 11 files changed, 528 insertions(+), 373 deletions(-) diff --git a/.changeset/validate-hosted-zerokms-discovery.md b/.changeset/validate-hosted-zerokms-discovery.md index a94bc1821a..1987b8168f 100644 --- a/.changeset/validate-hosted-zerokms-discovery.md +++ b/.changeset/validate-hosted-zerokms-discovery.md @@ -4,6 +4,6 @@ '@cipherstash/eql': minor --- -Validate token-discovered ZeroKMS destinations before sending bearer credentials, including device provisioning during login. Discovery accepts only HTTPS endpoints for supported regions under the CipherStash ZeroKMS domains. Every later token must carry a valid hosted claim for the original region; missing, non-hosted, or different-region claims are refused. Same-region domain migrations update the destination used by long-lived clients. Explicit KMS configuration remains authoritative; staging and regions unknown to the SDK require explicit configuration or an SDK upgrade. +Validate token-discovered ZeroKMS destinations before sending bearer credentials, including device provisioning during login. Discovery accepts only HTTPS endpoints for supported regions under the CipherStash ZeroKMS domains. Once ZeroKMS has accepted a token, every later token must carry a valid hosted claim for that region; missing, non-hosted, or different-region claims are refused. A token ZeroKMS refuses does not fix the region. Same-region domain migrations update the destination used by long-lived clients. Explicit KMS configuration remains authoritative; staging and regions unknown to the SDK require explicit configuration or an SDK upgrade. -The Rust releases are `stack-auth`/`stack-profile` 0.43.1, `stack-kms` 0.2.0, and `stack-encrypt`/`stack-encrypt-derive` 0.3.0. The pending EQL Rust encryption integration uses `stack-encrypt` 0.3.0. Custom `ZeroKMSConnection` transports that opt into discovery must update discovered URLs on later `ensure_base_url` calls while preserving URLs supplied at initialization. The CLI's bundled agent guidance describes these discovery requirements. +The Rust releases are `stack-auth`/`stack-profile` 0.43.1, `stack-kms` 0.2.0, and `stack-encrypt`/`stack-encrypt-derive` 0.3.0. The pending EQL Rust encryption integration uses `stack-encrypt` 0.3.0. Custom `ZeroKMSConnection` transports receive the discovered endpoint as a third `send` argument, and use it only when they were not configured with one; `ensure_base_url` is removed. The CLI's bundled agent guidance describes these discovery requirements. diff --git a/languages/golang/encrypt/guest/src/host.rs b/languages/golang/encrypt/guest/src/host.rs index 3f1da2ab9d..6e4c16316c 100644 --- a/languages/golang/encrypt/guest/src/host.rs +++ b/languages/golang/encrypt/guest/src/host.rs @@ -29,7 +29,6 @@ //! are reclaimed via the registry (which the ABI's `se_dealloc` also wipes). use std::convert::Infallible; -use std::sync::Mutex; use stack_auth::{AuthError, AuthStrategy, CustomError, SecretToken, ServiceToken}; use stack_guest_abi::buffers; @@ -51,24 +50,11 @@ extern "C" { /// import. The host call is synchronous from the guest's perspective, so /// `send` resolves immediately — `block_on` in the ABI layer never parks. /// -/// The endpoint is pinned at init when the cipher config named one; -/// otherwise it is discovered by `StackKms` from the access token's -/// `services` claim through [`ensure_base_url`](Self::ensure_base_url) — -/// later validated claims may update the domain within the pinned region. +/// The endpoint is fixed at init when the cipher config named one; +/// otherwise each request goes to the endpoint `StackKms` resolved from the +/// access token's `services` claim and passes to [`send`](Self::send). pub struct WasiHostConnection { - base: Mutex>, - explicitly_configured: bool, -} - -impl WasiHostConnection { - fn base(&self) -> std::sync::MutexGuard<'_, Option> { - // Wasm is single-threaded: a poisoned lock can only mean a previous - // panic already aborted the instance, so this is unreachable — - // recover rather than add a second panic path. - self.base - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - } + base: Option, } impl ZeroKMSConnectionInit for WasiHostConnection { @@ -76,29 +62,20 @@ impl ZeroKMSConnectionInit for WasiHostConnection { type Error = Infallible; fn init(opts: Self::ConnectionOpts) -> Result { - Ok(Self { - explicitly_configured: opts.is_some(), - base: Mutex::new(opts), - }) + Ok(Self { base: opts }) } } impl ZeroKMSConnection for WasiHostConnection { - fn ensure_base_url(&self, url: ZeroKmsEndpoint) { - let mut base = self.base(); - if !self.explicitly_configured { - *base = Some(url); - } - } - fn has_base_url(&self) -> bool { - self.base().is_some() + self.base.is_some() } async fn send( &self, request: Request, access_token: &str, + endpoint: Option<&ZeroKmsEndpoint>, ) -> Result { // Defence in depth, shared verbatim with `HttpConnection`: // `StackKms::get_token` resolves the endpoint (or fails with @@ -109,8 +86,9 @@ impl ZeroKMSConnection for WasiHostConnection { // caller from mistaking a missing endpoint for a 401 and refreshing // in a loop. let url = self - .base() + .base .as_ref() + .or(endpoint) .map(|base| base.request_url(Request::ENDPOINT)) .ok_or_else(|| { ViturRequestError::prepare( diff --git a/packages/stack-kms/src/builder.rs b/packages/stack-kms/src/builder.rs index 3a02930089..b1f7d932d9 100644 --- a/packages/stack-kms/src/builder.rs +++ b/packages/stack-kms/src/builder.rs @@ -80,8 +80,9 @@ impl stack_auth::ErrorPayload for StackKmsBuilderError { /// 3. Automatically from the token's `services` claim. Only a /// CipherStash-hosted ZeroKMS (`https://..viturhosted.net` /// or `https://..zerokms.cipherstashmanaged.net`) -/// is accepted there, and every later token must name the same region -/// (on either domain); any other endpoint must be configured by 1 or 2 +/// is accepted there. Once ZeroKMS has accepted a token at that +/// endpoint, every later token must name the same region (on either +/// domain); any other endpoint must be configured by 1 or 2 /// /// # Example /// diff --git a/packages/stack-kms/src/client.rs b/packages/stack-kms/src/client.rs index cc548579b8..16312385a0 100644 --- a/packages/stack-kms/src/client.rs +++ b/packages/stack-kms/src/client.rs @@ -110,8 +110,10 @@ impl ClientOpts { /// /// The client is generic over the transport [`ZeroKMSConnection`]; the default /// [`HttpConnection`] talks to a real ZeroKMS endpoint. Each method takes an -/// access token directly — see [`StackKms`] for the high-level wrapper that -/// fetches and refreshes tokens via [`stack_auth`]. +/// access token directly, and the endpoint to send it to when the connection +/// has none of its own (see [`ZeroKMSConnection::send`]) — see [`StackKms`] +/// for the high-level wrapper that fetches and refreshes tokens via +/// [`stack_auth`] and resolves that endpoint. #[cfg(feature = "http")] pub struct Client { connection: C, @@ -167,6 +169,7 @@ impl Client { operation: &'static str, specs: &'a [Spec], access_token: &str, + endpoint: Option<&ZeroKmsEndpoint>, make_request: impl Fn(&'a [Spec]) -> Req + Sync, response_keys: impl Fn(Req::Response) -> Vec + Sync, map_key: impl Fn(&'a Spec, Item) -> Result + Sync, @@ -184,7 +187,7 @@ impl Client { let keys = self .connection - .send(make_request(chunk), access_token) + .send(make_request(chunk), access_token, endpoint) .await .map(&response_keys) .map_err(E::from)?; @@ -226,6 +229,7 @@ impl Client { key: &ClientKey, keyset_id: Option, access_token: &str, + endpoint: Option<&ZeroKmsEndpoint>, unverified_context: Option<&UnverifiedContext>, ) -> Result, RetrieveKeyError> { tracing::trace!(target: "stack_kms::retrieve_keys", "preparing payloads"); @@ -241,6 +245,7 @@ impl Client { "retrieve_keys", &keys, access_token, + endpoint, |keys| RetrieveKeyRequest { keys: keys.into(), keyset_id: keyset_id.map(Into::into), @@ -265,6 +270,7 @@ impl Client { client_key: &ClientKey, keyset_id: Option, access_token: &str, + endpoint: Option<&ZeroKmsEndpoint>, unverified_context: Option>, ) -> Result { tracing::trace!(target: "stack_kms::retrieve_keys_fallible", "preparing payloads"); @@ -280,6 +286,7 @@ impl Client { "retrieve_keys_fallible", &keys, access_token, + endpoint, |keys| RetrieveKeyRequestFallible { keys: keys.into(), keyset_id: keyset_id.map(Into::into), @@ -312,6 +319,7 @@ impl Client { client_key: &ClientKey, keyset_id: Option, access_token: &str, + endpoint: Option<&ZeroKmsEndpoint>, ) -> Result<(Keyset, IndexKey), LoadKeysetError> { let req = LoadKeysetRequest { client_id: client_key.key_id, @@ -321,7 +329,7 @@ impl Client { let LoadKeysetResponse { keyset, partial_index_key, - } = self.connection.send(req, access_token).await?; + } = self.connection.send(req, access_token, endpoint).await?; let index_key = IndexKey::from_key_material(client_key, &partial_index_key.key_material)?; @@ -335,6 +343,7 @@ impl Client { client_key: &ClientKey, keyset_id: Option, access_token: &str, + endpoint: Option<&ZeroKmsEndpoint>, unverified_context: Option>, ) -> Result, GenerateKeyError> { let keys = { @@ -369,6 +378,7 @@ impl Client { "generate_keys", &keys, access_token, + endpoint, |keys| GenerateKeyRequest { keys: keys.into(), keyset_id: keyset_id.map(Into::into), @@ -404,10 +414,12 @@ impl Client { /// (refreshing as needed), resolves the ZeroKMS endpoint from the token's /// `services` claim, and delegates to the low-level client. Discovery accepts /// only CipherStash-hosted HTTPS endpoints in regions known to this build. -/// Every token must carry a valid claim for the first token's region; missing, -/// untrusted, or different-region claims are refused before a request is sent. -/// A later token may move that region's endpoint between the supported hosted -/// domains. Explicitly configured endpoints bypass claim-based discovery. +/// Once ZeroKMS has accepted a token, every later token must carry a valid +/// claim for that token's region; missing, untrusted, or different-region +/// claims are refused before a request is sent. Each token goes to the +/// endpoint it claims, so a later token may move that region's endpoint +/// between the supported hosted domains. Explicitly configured endpoints +/// bypass claim-based discovery. /// /// Build one with [`StackKmsBuilder`](crate::StackKmsBuilder) (the default /// HTTP transport, `http` feature), or with [`connect`](Self::connect) over @@ -418,7 +430,8 @@ pub struct StackKms { credentials: C, client_key: ClientKey, /// `None` when the connection was given its endpoint at init; otherwise - /// the region the first token's `services` claim named, once known. + /// the region of the first claimed endpoint ZeroKMS accepted a token at, + /// once it has. discovered: Option>, } @@ -427,15 +440,17 @@ pub struct StackKms { /// default connection: build one with [`connect`](Self::connect) over the /// host's [`ZeroKMSConnection`]. /// Discovery requires every token to name a supported CipherStash-hosted -/// HTTPS endpoint in the first token's region. Later tokens can change its -/// hosted domain; explicitly configured endpoints bypass discovery. +/// HTTPS endpoint, in the region of the first one ZeroKMS accepted. Later +/// tokens can change its hosted domain; explicitly configured endpoints +/// bypass discovery. #[cfg(not(feature = "http"))] pub struct StackKms { client: Client, credentials: C, client_key: ClientKey, /// `None` when the connection was given its endpoint at init; otherwise - /// the region the first token's `services` claim named, once known. + /// the region of the first claimed endpoint ZeroKMS accepted a token at, + /// once it has. discovered: Option>, } @@ -457,11 +472,11 @@ where )] /// /// The connection is initialised from `opts`'s connection options; the - /// ZeroKMS endpoint is taken from the access token's `services` claim on - /// first use unless the connection already knows one. A claimed endpoint - /// must be CipherStash-hosted, and every later token must name the same - /// region, while its hosted domain may change. Whether the connection - /// already knows one is decided here, once. + /// ZeroKMS endpoint is taken from each access token's `services` claim + /// unless the connection already knows one. A claimed endpoint must be + /// CipherStash-hosted, and once ZeroKMS has accepted a token, every later + /// token must name the same region, while its hosted domain may change. + /// Whether the connection already knows one is decided here, once. pub fn connect( opts: ClientOpts, credentials: C, @@ -469,8 +484,7 @@ where ) -> Result { let client: Client = Client::init_opts(opts).map_err(|e| Error::ConnectionInit(Box::new(e)))?; - // Decided once, here: asking the connection on every call would race a - // concurrent first caller that has just pinned it from a claim. + // Decided once, here, from what the connection was built with. let discovered = (!client.connection().has_base_url()).then(OnceLock::new); Ok(Self { client, @@ -480,29 +494,35 @@ where }) } - /// Fetch a token from the credentials provider and make sure the - /// connection sends it to the ZeroKMS endpoint the token is meant for. + /// Fetch a token from the credentials provider, and the ZeroKMS endpoint + /// to send it to. /// /// An endpoint configured on the connection (`with_base_url`, /// `CS_ZEROKMS_HOST`, or a transport that needs none) is the operator's - /// choice and is used as it is. Otherwise the endpoint comes from the - /// token's `services` claim, which nothing here verifies — a - /// [`TokenStore`](stack_auth::TokenStore) can return a token anyone wrote - /// — so: + /// choice: the connection uses it, so this returns no endpoint. Otherwise + /// the endpoint comes from the token's `services` claim, which nothing + /// here verifies — a [`TokenStore`](stack_auth::TokenStore) can return a + /// token anyone wrote — so: /// /// * the claim may only name a CipherStash-hosted ZeroKMS - /// ([`stack_auth::ServiceToken::hosted_zerokms_endpoint`]), and - /// * every later token must name the same region as the first one did. - /// A token for another region is refused rather than sent to the - /// pinned region. A token naming the same region on the other hosted - /// domain updates the connection, so it can keep working when the old - /// domain is retired. Missing or untrusted claims are refused even - /// after the region has been pinned. - async fn get_token(&self) -> Result { + /// ([`stack_auth::ServiceToken::hosted_zerokms_endpoint`]), and is + /// checked on every token, and + /// * once ZeroKMS has accepted a token at a claimed endpoint (see + /// [`settle`](Self::settle)), every later token must name that + /// endpoint's region. A token for another region is refused before it + /// is sent anywhere. A token naming the same region on the other hosted + /// domain goes to the endpoint it names, so a long-lived client keeps + /// working when the old domain is retired. + /// + /// Each token goes to the endpoint its own claim names, never to one + /// another token named. Until ZeroKMS has accepted a token nothing is + /// pinned: a forged token naming another hosted region is refused by + /// ZeroKMS there and leaves the client free for the real one. + async fn get_token(&self) -> Result<(stack_auth::ServiceToken, Option), Error> { let token = (&self.credentials).get_token().await?; // Configured explicitly: the claim is not consulted. let Some(discovered) = &self.discovered else { - return Ok(token); + return Ok((token, None)); }; let claimed = match token.hosted_zerokms_endpoint() { Ok(hosted) => HostedEndpoint { @@ -513,12 +533,33 @@ where // untrusted claims; successful discovery uses the shared cache. Err(_) => ZeroKmsEndpoint::from_services_claim(token.zerokms_url()?)?, }; - let region = discovered.get_or_init(|| claimed.region); - if *region != claimed.region { + if discovered + .get() + .is_some_and(|region| *region != claimed.region) + { return Err(InvalidEndpoint::Mismatch.into()); } - self.client.connection().ensure_base_url(claimed.endpoint); - Ok(token) + Ok((token, Some(claimed))) + } + + /// Pin a claimed endpoint's region once ZeroKMS has accepted a token sent + /// there, and pass the operation's result through. + /// + /// A response other than an error means ZeroKMS authenticated the token, + /// so the claim that routed it is one ZeroKMS stands behind. The first + /// such region wins; a concurrent first call that lost the race to a + /// different region is refused as a mismatch on its next token. + fn settle(&self, result: Result, endpoint: Option) -> Result { + if let (Ok(_), Some(discovered), Some(endpoint)) = (&result, &self.discovered, endpoint) { + if discovered.set(endpoint.region).is_ok() { + tracing::debug!( + target: "stack_kms", + endpoint = %endpoint.endpoint, + "pinned the ZeroKMS region from the token's services claim" + ); + } + } + result } /// The [`ClientKey`] this client uses to derive data keys. @@ -533,18 +574,20 @@ where keyset_id: Option, unverified_context: Option>, ) -> Result, Error> { - let token = self.get_token().await?; + let (token, endpoint) = self.get_token().await?; - self.client + let result = self + .client .generate_keys( payloads, &self.client_key, keyset_id, token.as_str(), + endpoint.as_ref().map(|e| &e.endpoint), unverified_context, ) - .await - .map_err(Error::from) + .await; + self.settle(result, endpoint).map_err(Error::from) } /// Retrieve multiple data keys for an iterator of [`RetrieveKeyPayload`]. @@ -554,18 +597,20 @@ where keyset_id: Option, unverified_context: Option<&UnverifiedContext>, ) -> Result, Error> { - let token = self.get_token().await?; + let (token, endpoint) = self.get_token().await?; - self.client + let result = self + .client .retrieve_keys( payloads, &self.client_key, keyset_id, token.as_str(), + endpoint.as_ref().map(|e| &e.endpoint), unverified_context, ) - .await - .map_err(Error::from) + .await; + self.settle(result, endpoint).map_err(Error::from) } /// Load a keyset and derive its [`IndexKey`] — the deterministic per-keyset @@ -576,12 +621,18 @@ where &self, keyset_id: Option, ) -> Result<(Keyset, IndexKey), Error> { - let token = self.get_token().await?; + let (token, endpoint) = self.get_token().await?; - let (keyset, index_key) = self + let result = self .client - .load_keyset(&self.client_key, keyset_id, token.as_str()) - .await?; + .load_keyset( + &self.client_key, + keyset_id, + token.as_str(), + endpoint.as_ref().map(|e| &e.endpoint), + ) + .await; + let (keyset, index_key) = self.settle(result, endpoint)?; tracing::debug!(target: "stack_kms::load_keyset", "loaded keyset: [{}]({})", keyset.id, keyset.name); @@ -596,19 +647,21 @@ where keyset_id: Option, unverified_context: Option>, ) -> Result { - let token = self.get_token().await?; + let (token, endpoint) = self.get_token().await?; tracing::debug!(target: "stack_kms::retrieve_keys_fallible", "got token, retrieving keys"); - self.client + let result = self + .client .retrieve_keys_fallible( payloads, &self.client_key, keyset_id, token.as_str(), + endpoint.as_ref().map(|e| &e.endpoint), unverified_context, ) - .await - .map_err(Error::from) + .await; + self.settle(result, endpoint).map_err(Error::from) } } @@ -756,9 +809,9 @@ mod tests { } } - /// Where a token's `services` claim may send key operations (CIP-5152). + /// Where a token's `services` claim may send key operations. /// The claim is unverified, so it may only name a CipherStash-hosted - /// ZeroKMS, and it cannot change a region already in use. + /// ZeroKMS, and it cannot change a region ZeroKMS has accepted a token in. mod endpoint_from_the_services_claim { use super::*; use base64ct::{Base64UrlUnpadded, Encoding}; @@ -767,6 +820,8 @@ mod tests { const SYDNEY: &str = "https://ap-southeast-2.aws.viturhosted.net/"; const VIRGINIA: &str = "https://us-east-1.aws.zerokms.cipherstashmanaged.net/"; + const SYD: &str = "ap-southeast-2.aws"; + const IAD: &str = "us-east-1.aws"; /// An unsigned JWT naming `zerokms` — the shape a poisoned /// `TokenStore` can hand back, since nothing verifies the signature. @@ -811,74 +866,125 @@ mod tests { } } - /// A transport that, like `HttpConnection`, discovers its endpoint - /// and records what it was told. It never sends anything: - /// `get_token` is the code under test. - #[derive(Default)] + /// A transport that, like `HttpConnection`, sends to the endpoint it + /// was configured with or else the one `StackKms` passes, and records + /// where each request went. ZeroKMS's answer to each request is + /// scripted: `true` accepts the token, `false` refuses it. struct Discovering { - pinned: Mutex>, - calls: Mutex, + configured: Option, + answers: Mutex>, + /// Where each request went, and the token it carried. + sent: Mutex>, } impl crate::connection::ZeroKMSConnectionInit for Discovering { - type ConnectionOpts = Option; + type ConnectionOpts = (Option, Vec); type Error = std::convert::Infallible; - fn init(pinned: Self::ConnectionOpts) -> Result { + fn init((configured, answers): Self::ConnectionOpts) -> Result { Ok(Self { - pinned: Mutex::new(pinned), - calls: Mutex::new(0), + configured, + answers: Mutex::new(answers.into()), + sent: Mutex::new(Vec::new()), }) } } - impl ZeroKMSConnection for Discovering { - fn ensure_base_url(&self, url: ZeroKmsEndpoint) { - *self.calls.lock().unwrap() += 1; - *self.pinned.lock().unwrap() = Some(url); - } + #[derive(Debug, thiserror::Error)] + #[error("401 Unauthorized")] + struct Refused; + impl ZeroKMSConnection for Discovering { fn has_base_url(&self) -> bool { - self.pinned.lock().unwrap().is_some() + self.configured.is_some() } async fn send( &self, _request: Request, - _access_token: &str, + access_token: &str, + endpoint: Option<&ZeroKmsEndpoint>, ) -> Result { - unreachable!("these tests stop at get_token") + let to = self + .configured + .as_ref() + .or(endpoint) + .expect("an endpoint for every request"); + self.sent + .lock() + .unwrap() + .push((to.to_string(), access_token.to_owned())); + let accepted = self + .answers + .lock() + .unwrap() + .pop_front() + .expect("an answer per request"); + if !accepted { + return Err(ViturRequestError::response("refused", Refused)); + } + let response = GenerateKeyResponse { + keys: vec![generated_key(vec![1])], + }; + Ok(serde_json::from_value(serde_json::to_value(response).unwrap()).unwrap()) } } - fn kms(configured: Option<&str>, tokens: Tokens) -> StackKms { + fn kms( + configured: Option<&str>, + tokens: Tokens, + answers: impl Into>, + ) -> StackKms { let configured = configured.map(|s| s.parse().unwrap()); - StackKms::connect(ClientOpts::new(configured), tokens, random_client_key()) - .expect("connect") + StackKms::connect( + ClientOpts::new((configured, answers.into())), + tokens, + random_client_key(), + ) + .expect("connect") } + /// One key operation, as a caller makes it. + async fn call(kms: &StackKms) -> Result<(), Error> { + kms.generate_keys( + vec![GenerateKeyPayload::new("a", Cow::Owned(vec![]))], + None, + None, + ) + .await + .map(|_| ()) + } + + /// The region pinned so far, as `.`. fn pinned(kms: &StackKms) -> Option { - let pinned = kms.client.connection().pinned.lock().unwrap(); - pinned.as_ref().map(ToString::to_string) + kms.discovered + .as_ref() + .and_then(OnceLock::get) + .map(|region| region.identifier()) + } + + fn sent_to(kms: &StackKms) -> Vec { + let sent = kms.client.connection().sent.lock().unwrap(); + sent.iter().map(|(to, _)| to.clone()).collect() } #[tokio::test] - async fn a_hosted_endpoint_is_used() { - let kms = kms(None, Tokens::new([jwt(Some(SYDNEY))])); - kms.get_token() - .await - .expect("a hosted endpoint is accepted"); - assert_eq!(pinned(&kms).as_deref(), Some(SYDNEY)); + async fn a_hosted_endpoint_is_used_and_pinned_once_zerokms_accepts_the_token() { + let kms = kms(None, Tokens::new([jwt(Some(SYDNEY))]), [true]); + call(&kms).await.expect("a hosted endpoint is accepted"); + assert_eq!(sent_to(&kms), [SYDNEY]); + assert_eq!(pinned(&kms).as_deref(), Some(SYD)); } #[tokio::test] async fn a_host_outside_cipherstash_is_refused_and_not_pinned() { - let kms = kms(None, Tokens::new([jwt(Some("https://evil.example/"))])); - let err = kms.get_token().await.expect_err("an attacker's host"); + let kms = kms(None, Tokens::new([jwt(Some("https://evil.example/"))]), []); + let err = call(&kms).await.expect_err("an attacker's host"); assert!( - matches!(err, Error::InvalidEndpoint(InvalidEndpoint::NotHosted(_))), + matches!(err, Error::InvalidEndpoint(InvalidEndpoint::NotHosted)), "{err:?}" ); + assert!(sent_to(&kms).is_empty(), "nothing is sent"); assert_eq!(pinned(&kms), None); } @@ -887,12 +993,14 @@ mod tests { let kms = kms( None, Tokens::new([jwt(Some("http://ap-southeast-2.aws.viturhosted.net/"))]), + [], ); - let err = kms.get_token().await.expect_err("http"); + let err = call(&kms).await.expect_err("http"); assert!( - matches!(err, Error::InvalidEndpoint(InvalidEndpoint::NotHosted(_))), + matches!(err, Error::InvalidEndpoint(InvalidEndpoint::NotHosted)), "{err:?}" ); + assert!(sent_to(&kms).is_empty(), "nothing is sent"); assert_eq!(pinned(&kms), None); } @@ -901,34 +1009,57 @@ mod tests { let kms = kms( None, Tokens::new([jwt(Some("https://evil.example/")), jwt(Some(SYDNEY))]), + [true], + ); + call(&kms).await.expect_err("an attacker's host"); + call(&kms).await.expect("the next caller's token"); + assert_eq!(pinned(&kms).as_deref(), Some(SYD)); + } + + /// A forged token can name any hosted region. ZeroKMS there refuses + /// it, so it must not decide where the client's real tokens go: the + /// endpoint is pinned only once ZeroKMS has accepted a token. + #[tokio::test] + async fn a_token_zerokms_refuses_does_not_pin_its_region() { + let kms = kms( + None, + Tokens::new([jwt(Some(VIRGINIA)), jwt(Some(SYDNEY)), jwt(Some(SYDNEY))]), + [false, true, true], ); - kms.get_token().await.expect_err("an attacker's host"); - kms.get_token().await.expect("the next caller's token"); - assert_eq!(pinned(&kms).as_deref(), Some(SYDNEY)); + let err = call(&kms) + .await + .expect_err("ZeroKMS refuses the forged token"); + assert!(!matches!(err, Error::InvalidEndpoint(_)), "{err:?}"); + assert_eq!(pinned(&kms), None, "a refused token pins nothing"); + + call(&kms).await.expect("the real token works"); + call(&kms).await.expect("and keeps working"); + assert_eq!(sent_to(&kms), [VIRGINIA, SYDNEY, SYDNEY]); + assert_eq!(pinned(&kms).as_deref(), Some(SYD)); } #[tokio::test] - async fn a_later_token_naming_another_endpoint_is_refused() { + async fn a_later_token_naming_another_region_is_refused() { let kms = kms( None, Tokens::new([jwt(Some(SYDNEY)), jwt(Some(VIRGINIA)), jwt(Some(SYDNEY))]), + [true, true], ); - kms.get_token().await.expect("first token pins Sydney"); - let err = kms - .get_token() + call(&kms).await.expect("first token pins Sydney"); + let err = call(&kms) .await .expect_err("a token for another region must not go to Sydney"); assert!( matches!(err, Error::InvalidEndpoint(InvalidEndpoint::Mismatch)), "{err:?}" ); - kms.get_token().await.expect("Sydney tokens still work"); - assert_eq!(pinned(&kms).as_deref(), Some(SYDNEY)); + call(&kms).await.expect("Sydney tokens still work"); assert_eq!( - *kms.client.connection().calls.lock().unwrap(), - 2, - "only the two valid same-region tokens update the connection" + sent_to(&kms), + [SYDNEY, SYDNEY], + "Virginia's token went nowhere" ); + assert_eq!(pinned(&kms).as_deref(), Some(SYD)); } /// CipherStash serves each region's ZeroKMS under both hosted domains @@ -942,18 +1073,19 @@ mod tests { let kms = kms( None, Tokens::new([jwt(Some(SYDNEY)), jwt(Some(SYDNEY_NEW)), jwt(Some(SYDNEY))]), + [true, true, true], ); - kms.get_token().await.expect("first token pins Sydney"); - kms.get_token() + call(&kms).await.expect("first token pins Sydney"); + call(&kms) .await .expect("same region on the new domain is the same ZeroKMS"); - kms.get_token().await.expect("and back again"); + call(&kms).await.expect("and back again"); assert_eq!( - pinned(&kms).as_deref(), - Some(SYDNEY), - "the endpoint follows each same-region claim" + sent_to(&kms), + [SYDNEY, SYDNEY_NEW, SYDNEY], + "each token goes to the endpoint it names" ); - assert_eq!(*kms.client.connection().calls.lock().unwrap(), 3); + assert_eq!(pinned(&kms).as_deref(), Some(SYD)); } #[tokio::test] @@ -962,12 +1094,14 @@ mod tests { let kms = kms( None, Tokens::new([jwt(Some(VIRGINIA)), jwt(Some(VIRGINIA_OLD))]), + [true, true], ); - kms.get_token().await.expect("first token pins Virginia"); - kms.get_token() + call(&kms).await.expect("first token pins Virginia"); + call(&kms) .await .expect("same region on the old domain is the same ZeroKMS"); - assert_eq!(pinned(&kms).as_deref(), Some(VIRGINIA_OLD)); + assert_eq!(sent_to(&kms), [VIRGINIA, VIRGINIA_OLD]); + assert_eq!(pinned(&kms).as_deref(), Some(IAD)); } mod given_a_same_region_hosted_domain_migration { @@ -979,54 +1113,27 @@ mod tests { "https://ap-southeast-2.aws.zerokms.cipherstashmanaged.net/"; let kms = kms( None, - Tokens::new([jwt(Some(SYDNEY)), jwt(Some(SYDNEY_NEW))]), + Tokens::new([ + jwt(Some(SYDNEY)), + jwt(Some(SYDNEY_NEW)), + jwt(Some(SYDNEY_NEW)), + ]), + [true, true, true], ); - kms.get_token() - .await - .expect("the first token discovers Sydney"); - kms.get_token() + call(&kms).await.expect("the first token discovers Sydney"); + call(&kms) .await .expect("the refreshed token names the same region"); + call(&kms).await.expect("and the next"); assert_eq!( - pinned(&kms).as_deref(), - Some(SYDNEY_NEW), - "the connection must follow the trusted same-region domain migration before the old domain retires" - ); - } - } - - #[tokio::test] - async fn another_region_is_refused_on_either_domain() { - for other in [ - "https://us-east-1.aws.viturhosted.net/", - "https://us-east-1.aws.zerokms.cipherstashmanaged.net/", - ] { - let kms = kms(None, Tokens::new([jwt(Some(SYDNEY)), jwt(Some(other))])); - kms.get_token().await.expect("first token pins Sydney"); - let err = kms.get_token().await.expect_err(other); - assert!( - matches!(err, Error::InvalidEndpoint(InvalidEndpoint::Mismatch)), - "{other}: {err:?}" + sent_to(&kms), + [SYDNEY, SYDNEY_NEW, SYDNEY_NEW], + "requests must follow the trusted same-region domain migration before the old domain retires" ); - assert_eq!(pinned(&kms).as_deref(), Some(SYDNEY)); } } - #[tokio::test] - async fn a_later_token_naming_a_foreign_host_is_refused() { - let kms = kms( - None, - Tokens::new([jwt(Some(SYDNEY)), jwt(Some("https://evil.example/"))]), - ); - kms.get_token().await.expect("first token pins Sydney"); - let err = kms.get_token().await.expect_err("foreign host"); - assert!( - matches!(err, Error::InvalidEndpoint(InvalidEndpoint::NotHosted(_))), - "{err:?}" - ); - } - mod stress_tests { use super::*; use std::sync::Arc; @@ -1047,6 +1154,9 @@ mod tests { } } + /// Concurrent first callers holding tokens for two regions: each + /// token goes only to the region it names, ZeroKMS accepting them + /// pins exactly one region, and the other is refused from then on. #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn concurrent_discovery_keeps_one_region() { const CALLERS: usize = 32; @@ -1055,7 +1165,7 @@ mod tests { }); let kms = Arc::new( StackKms::<_, Discovering>::connect( - ClientOpts::new(None), + ClientOpts::new((None, vec![true; CALLERS])), ConcurrentTokens { tokens: Tokens::new(tokens), barrier: Barrier::new(CALLERS), @@ -1067,56 +1177,120 @@ mod tests { let mut callers = Vec::new(); for _ in 0..CALLERS { let kms = Arc::clone(&kms); - callers.push(tokio::spawn(async move { kms.get_token().await })); + callers.push(tokio::spawn(async move { + kms.generate_keys( + vec![GenerateKeyPayload::new("a", Cow::Owned(vec![]))], + None, + None, + ) + .await + })); } - let mut accepted_region = None; let mut accepted = 0; for caller in callers { match caller.await.unwrap() { - Ok(token) => { - let region = token.hosted_zerokms_endpoint().unwrap().region(); - assert_eq!( - *accepted_region.get_or_insert(region), - region, - "only one region may win discovery" - ); - accepted += 1; - } + Ok(_) => accepted += 1, + // A caller that reached the check after another + // region was pinned is refused before sending. Err(error) => assert!( matches!(error, Error::InvalidEndpoint(InvalidEndpoint::Mismatch)), - "other regions must fail before transport: {error:?}" + "{error:?}" ), } } + + let sent = kms.client.connection().sent.lock().unwrap().clone(); + assert_eq!(sent.len(), accepted, "only refused callers send nothing"); + for (to, token) in &sent { + let named = + stack_auth::ServiceToken::new(stack_auth::SecretToken::new(token.clone())) + .hosted_zerokms_endpoint() + .unwrap() + .as_url() + .to_string(); + assert_eq!(to, &named, "a token goes only where it names"); + } + + let won = kms + .discovered + .as_ref() + .and_then(OnceLock::get) + .copied() + .expect("one region is pinned"); + let won_url = if won.identifier() == SYD { + SYDNEY + } else { + VIRGINIA + }; assert_eq!( - accepted, + sent.iter().filter(|(to, _)| to == won_url).count(), CALLERS / 2, - "every token for the winning region is accepted" + "every token for the pinned region is accepted" ); - let endpoint = kms - .client - .connection() - .pinned - .lock() - .unwrap() - .clone() - .unwrap(); - let region = stack_auth::HostedZeroKmsEndpoint::new(endpoint.as_url().clone()) - .unwrap() - .region(); - assert_eq!( - Some(region), - accepted_region, - "the transport uses the winning region" + let loser = if won.identifier() == SYD { + VIRGINIA + } else { + SYDNEY + }; + let late = StackKms { + client: Client::init_opts(ClientOpts::new((None, vec![]))).unwrap(), + credentials: Tokens::new([jwt(Some(loser))]), + client_key: random_client_key(), + discovered: Some(OnceLock::from(won)), + }; + let err = call(&late) + .await + .expect_err("the other region is refused once one is pinned"); + assert!( + matches!(err, Error::InvalidEndpoint(InvalidEndpoint::Mismatch)), + "{err:?}" ); + assert!(sent_to(&late).is_empty(), "before anything is sent"); } } + #[tokio::test] + async fn another_region_is_refused_on_either_domain() { + for other in [ + "https://us-east-1.aws.viturhosted.net/", + "https://us-east-1.aws.zerokms.cipherstashmanaged.net/", + ] { + let kms = kms( + None, + Tokens::new([jwt(Some(SYDNEY)), jwt(Some(other))]), + [true], + ); + call(&kms).await.expect("first token pins Sydney"); + let err = call(&kms).await.expect_err(other); + assert!( + matches!(err, Error::InvalidEndpoint(InvalidEndpoint::Mismatch)), + "{other}: {err:?}" + ); + assert_eq!(sent_to(&kms), [SYDNEY]); + } + } + + #[tokio::test] + async fn a_later_token_naming_a_foreign_host_is_refused() { + let kms = kms( + None, + Tokens::new([jwt(Some(SYDNEY)), jwt(Some("https://evil.example/"))]), + [true], + ); + call(&kms).await.expect("first token pins Sydney"); + let err = call(&kms).await.expect_err("foreign host"); + assert!( + matches!(err, Error::InvalidEndpoint(InvalidEndpoint::NotHosted)), + "{err:?}" + ); + assert_eq!(sent_to(&kms), [SYDNEY]); + } + #[tokio::test] async fn a_later_token_without_the_claim_is_refused() { - let kms = kms(None, Tokens::new([jwt(Some(SYDNEY)), jwt(None)])); - kms.get_token().await.expect("first token pins Sydney"); - let err = kms.get_token().await.expect_err("no services claim"); + let kms = kms(None, Tokens::new([jwt(Some(SYDNEY)), jwt(None)]), [true]); + call(&kms).await.expect("first token pins Sydney"); + let err = call(&kms).await.expect_err("no services claim"); assert!(matches!(err, Error::Auth(_)), "{err:?}"); } @@ -1127,11 +1301,15 @@ mod tests { let kms = kms( Some("http://localhost:3002"), Tokens::new([jwt(Some("https://evil.example/")), jwt(None)]), + [true, true], ); - kms.get_token().await.expect("claim ignored"); - kms.get_token().await.expect("claim ignored"); - assert_eq!(pinned(&kms).as_deref(), Some("http://localhost:3002/")); - assert_eq!(*kms.client.connection().calls.lock().unwrap(), 0); + call(&kms).await.expect("claim ignored"); + call(&kms).await.expect("claim ignored"); + assert_eq!( + sent_to(&kms), + ["http://localhost:3002/", "http://localhost:3002/"] + ); + assert_eq!(pinned(&kms), None, "nothing is discovered"); } #[tokio::test] @@ -1142,7 +1320,8 @@ mod tests { random_client_key(), ) .expect("connect"); - kms.get_token().await.expect("claim ignored"); + let (_, endpoint) = kms.get_token().await.expect("claim ignored"); + assert_eq!(endpoint, None); } } @@ -1169,6 +1348,7 @@ mod tests { None, "token", None, + None, ) .await .expect_err("count mismatch must be an error"); @@ -1201,6 +1381,7 @@ mod tests { None, "token", None, + None, ) .await .expect_err("count mismatch must be an error"); @@ -1233,6 +1414,7 @@ mod tests { None, "token", None, + None, ) .await .expect_err("count mismatch must be an error"); @@ -1274,6 +1456,7 @@ mod tests { None, "token", None, + None, ) .await .expect_err("a failed request must surface"); @@ -1300,6 +1483,7 @@ mod tests { None, "token", None, + None, ) .await .expect_err("a failed request must surface"); @@ -1357,6 +1541,7 @@ mod tests { None, "token", None, + None, ) .await .expect("generate_keys should succeed"); @@ -1406,7 +1591,7 @@ mod tests { ]; let keys = client - .generate_keys(payloads, &client_key, None, "token", None) + .generate_keys(payloads, &client_key, None, "token", None, None) .await .expect("generate_keys should succeed"); @@ -1431,7 +1616,7 @@ mod tests { let payloads = vec![RetrieveKeyPayload::new(iv, "a", &[1, 2, 3])]; let keys = client - .retrieve_keys(payloads, &client_key, None, "token", None) + .retrieve_keys(payloads, &client_key, None, "token", None, None) .await .expect("retrieve_keys should succeed"); @@ -1464,6 +1649,7 @@ mod tests { None, "token", None, + None, ) .await .expect("generate should succeed"); @@ -1487,6 +1673,7 @@ mod tests { None, "token", None, + None, ) .await .expect("retrieve should succeed"); @@ -1518,11 +1705,11 @@ mod tests { }; let (loaded_a, key_a) = keyset(shared_bytes.clone()) - .load_keyset(&client_key, None, "token") + .load_keyset(&client_key, None, "token", None) .await .expect("load_keyset should succeed"); let (_, key_b) = keyset(shared_bytes) - .load_keyset(&client_key, Some(keyset_id.into()), "token") + .load_keyset(&client_key, Some(keyset_id.into()), "token", None) .await .expect("load_keyset should succeed"); @@ -1533,7 +1720,7 @@ mod tests { // Different key material derives a different index key. let (_, key_c) = keyset(vec![8u8; 528]) - .load_keyset(&client_key, None, "token") + .load_keyset(&client_key, None, "token", None) .await .expect("load_keyset should succeed"); assert_ne!(key_a.key(), key_c.key()); @@ -1569,6 +1756,7 @@ mod tests { None, "token", None, + None, ) .await .expect("batch call itself should succeed"); diff --git a/packages/stack-kms/src/client/test_connection.rs b/packages/stack-kms/src/client/test_connection.rs index 17560b4719..7312ceb9a7 100644 --- a/packages/stack-kms/src/client/test_connection.rs +++ b/packages/stack-kms/src/client/test_connection.rs @@ -5,6 +5,7 @@ use async_mutex::Mutex; use zerokms_protocol::{ViturRequest, ViturRequestError}; use crate::connection::{ZeroKMSConnection, ZeroKMSConnectionInit}; +use crate::endpoint::ZeroKmsEndpoint; type EffectHandlers = Vec<(String, Box)>; type RequestHandlers = Vec<(String, Result)>; @@ -93,12 +94,13 @@ impl ZeroKMSConnectionInit for TestConnection { impl ZeroKMSConnection for TestConnection { // The stub has no URL to resolve — it dispatches on the request's endpoint - // name — so the trait's defaults (no-op `ensure_base_url`, `has_base_url` - // of `true`) are exactly right here. + // name — so the trait's default `has_base_url` of `true` is exactly + // right here, and no endpoint is ever passed to it. async fn send( &self, request: Request, _access_token: &str, + _endpoint: Option<&ZeroKmsEndpoint>, ) -> Result { let endpoint = Request::ENDPOINT; diff --git a/packages/stack-kms/src/connection.rs b/packages/stack-kms/src/connection.rs index e6c5b5ce28..613e4a7859 100644 --- a/packages/stack-kms/src/connection.rs +++ b/packages/stack-kms/src/connection.rs @@ -42,35 +42,36 @@ pub trait ZeroKMSConnectionInit { /// unbounded on wasm32 — reqwest's fetch-backed response futures aren't /// `Send`, and edge runtimes are single-threaded anyway. pub trait ZeroKMSConnection: ZeroKMSConnectionInit { + /// Send `request` with `access_token` as its bearer credential. + /// + /// `endpoint` is the ZeroKMS [`StackKms`](crate::StackKms) resolved from + /// the token's `services` claim, and is `Some` only while + /// [`has_base_url`](Self::has_base_url) is `false`. A connection with an + /// endpoint of its own uses that and ignores this one; a connection with + /// neither cannot build a request URL and should fail with a *prepare* + /// error ([`BaseUrlUnresolved`]), so a caller does not mistake it for a + /// 401 and refresh in a loop. + /// + /// The endpoint travels with each request rather than being stored on the + /// connection: `StackKms` keeps one only after ZeroKMS has accepted a + /// token sent there, so a token it refuses cannot fix the endpoint every + /// later request goes to. fn send( &self, request: Request, access_token: &str, + endpoint: Option<&ZeroKmsEndpoint>, ) -> impl Future> + crate::MaybeSend; - /// Update the discovered ZeroKMS endpoint, preserving explicit configuration. + /// Whether the connection was given its ZeroKMS endpoint at init. /// - /// [`StackKms`](crate::StackKms) calls this with the endpoint named by the - /// access token's `services` claim after validating its hosted domain - /// and checking that its region matches the first discovered region. - /// - /// Endpoint discovery is `StackKms` policy, not something every transport - /// needs: the default is a no-op, paired with a `has_base_url` of `true`, - /// which is correct for a transport that does not build URLs from a base - /// (it dispatches on the request's endpoint name) or that was pinned at - /// init. A transport that *does* want discovery must override **both**, - /// and its `ensure_base_url` must accept later discovered URLs so a - /// hosted domain can migrate. An endpoint configured at init must stay - /// fixed. Snapshot the URL for each request before awaiting network I/O. - fn ensure_base_url(&self, _url: ZeroKmsEndpoint) {} - - /// Whether an endpoint is known, either from init or from a previous - /// [`ensure_base_url`](Self::ensure_base_url). While this is `false`, - /// [`send`](Self::send) cannot build a request URL. + /// While this is `false`, [`StackKms`](crate::StackKms) resolves the + /// endpoint from each access token's `services` claim and passes it to + /// [`send`](Self::send). It is asked once, when the client is built. /// - /// Defaults to `true`: a transport that needs no base URL always has - /// everything it needs, so `StackKms` never tries to resolve one from a - /// token. Override alongside [`ensure_base_url`](Self::ensure_base_url). + /// Defaults to `true`, which is correct for a transport that does not + /// build URLs from a base (it dispatches on the request's endpoint name) + /// or that was configured with one: `StackKms` never consults the claim. fn has_base_url(&self) -> bool { true } diff --git a/packages/stack-kms/src/connection/http.rs b/packages/stack-kms/src/connection/http.rs index e65369b52b..cd14148f76 100644 --- a/packages/stack-kms/src/connection/http.rs +++ b/packages/stack-kms/src/connection/http.rs @@ -9,9 +9,9 @@ use crate::endpoint::ZeroKmsEndpoint; use crate::user_agent::get_user_agent; use reqwest::header::HeaderMap; use serde_json::to_vec; +use std::collections::HashMap; #[cfg(not(target_arch = "wasm32"))] use std::time::Duration; -use std::{collections::HashMap, sync::Mutex}; use thiserror::Error; use zerokms_protocol::{ViturRequest, ViturRequestError}; @@ -52,8 +52,8 @@ pub struct HttpConnectionOpts { impl HttpConnectionOpts { /// Options for a connection to `base_url`, or — when `None` — to the - /// CipherStash-hosted endpoint the access token's `services` claim names - /// (resolved on first use via [`HttpConnection::ensure_base_url`]). An + /// CipherStash-hosted endpoint the access token's `services` claim names, + /// which [`StackKms`](crate::StackKms) passes with each request. An /// explicit `base_url` may be any usable endpoint; a claimed one may not. pub fn new(base_url: Option) -> Self { Self { @@ -126,8 +126,9 @@ impl HttpConnectionOpts { } pub struct HttpConnection { - base_url: Mutex>, - explicitly_configured: bool, + /// The endpoint configured at init, if any. Without one, each request + /// goes to the endpoint `StackKms` passes to `send`. + base_url: Option, client: reqwest::Client, } @@ -182,59 +183,49 @@ impl ZeroKMSConnectionInit for HttpConnection { let client = builder.build()?; - let explicitly_configured = opts.base_url.is_some(); - let base_url = Mutex::new(opts.base_url); Ok(Self { - base_url, - explicitly_configured, + base_url: opts.base_url, client, }) } } -impl ZeroKMSConnection for HttpConnection { - fn ensure_base_url(&self, url: ZeroKmsEndpoint) { - if !self.explicitly_configured { - *self - .base_url - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) = Some(url); - } +impl HttpConnection { + /// Where a request goes: the endpoint configured at init, or else the one + /// the caller resolved for this request. + fn request_base<'a>( + &'a self, + endpoint: Option<&'a ZeroKmsEndpoint>, + ) -> Option<&'a ZeroKmsEndpoint> { + self.base_url.as_ref().or(endpoint) } +} +impl ZeroKMSConnection for HttpConnection { fn has_base_url(&self) -> bool { - self.base_url - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .is_some() + self.base_url.is_some() } async fn send( &self, request: Request, access_token: &str, + endpoint: Option<&ZeroKmsEndpoint>, ) -> Result { let body = to_vec(&request) .map_err(|e| ViturRequestError::prepare("Failed to serialize request", e))?; - // Snapshot the destination before any await. A concurrent refresh may - // update the same region's domain without changing this request's URL. - let base_url = self - .base_url - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .clone() - .ok_or_else(|| { - // A missing base URL is a client-side configuration problem (the - // token carried no ZeroKMS `services` claim and none was set - // explicitly), not an authentication failure — classify it as a - // request-preparation error so callers don't mistake it for a 401 - // and trigger a token refresh/reauth loop. - ViturRequestError::prepare( - "ZeroKMS base URL was not resolved from the token's services claim", - BaseUrlUnresolved, - ) - })?; + let base_url = self.request_base(endpoint).ok_or_else(|| { + // A missing base URL is a client-side configuration problem (the + // token carried no ZeroKMS `services` claim and none was set + // explicitly), not an authentication failure — classify it as a + // request-preparation error so callers don't mistake it for a 401 + // and trigger a token refresh/reauth loop. + ViturRequestError::prepare( + "ZeroKMS base URL was not resolved from the token's services claim", + BaseUrlUnresolved, + ) + })?; let url = base_url.request_url(Request::ENDPOINT); @@ -293,62 +284,27 @@ mod base_url_tests { } #[test] - fn is_unset_until_ensured() { + fn without_a_configured_url_a_request_goes_where_the_caller_says() { let c = conn(None); assert!(!c.has_base_url()); - c.ensure_base_url(endpoint("https://a.example")); - - assert!(c.has_base_url()); - assert_eq!( - c.base_url.lock().unwrap().as_ref().unwrap().as_str(), - "https://a.example/" - ); - } - - #[test] - fn the_latest_discovered_url_is_used() { - let c = conn(None); - c.ensure_base_url(endpoint("https://first.example")); - c.ensure_base_url(endpoint("https://second.example")); - + let passed = endpoint("https://a.example"); assert_eq!( - c.base_url.lock().unwrap().as_ref().unwrap().as_str(), - "https://second.example/", - "discovery follows the latest validated claim" + c.request_base(Some(&passed)).map(ZeroKmsEndpoint::as_str), + Some("https://a.example/") ); + assert_eq!(c.request_base(None), None); } #[test] - fn a_discovered_endpoint_follows_a_hosted_domain_migration() { - let c = conn(None); - c.ensure_base_url(endpoint("https://us-east-1.aws.viturhosted.net/")); - c.ensure_base_url(endpoint( - "https://us-east-1.aws.zerokms.cipherstashmanaged.net/", - )); - assert_eq!( - c.base_url - .lock() - .unwrap() - .as_ref() - .unwrap() - .request_url("retrieve-data-key") - .as_str(), - "https://us-east-1.aws.zerokms.cipherstashmanaged.net/retrieve-data-key", - "request URLs must follow a validated domain migration" - ); - } - - #[test] - fn a_url_given_at_init_is_kept_over_a_later_ensure() { + fn a_url_given_at_init_wins_over_the_callers() { let c = conn(Some(endpoint("https://init.example"))); assert!(c.has_base_url()); - c.ensure_base_url(endpoint("https://other.example")); - + let passed = endpoint("https://other.example"); assert_eq!( - c.base_url.lock().unwrap().as_ref().unwrap().as_str(), - "https://init.example/" + c.request_base(Some(&passed)).map(ZeroKmsEndpoint::as_str), + Some("https://init.example/") ); } @@ -360,10 +316,7 @@ mod base_url_tests { let c = conn(Some(endpoint("https://gateway.example/zerokms"))); assert_eq!( - c.base_url - .lock() - .unwrap() - .as_ref() + c.request_base(None) .unwrap() .request_url(zerokms_protocol::RetrieveKeyRequest::ENDPOINT) .as_str(), @@ -383,7 +336,7 @@ mod base_url_tests { unverified_context: Default::default(), }; - let err = c.send(req, "token").await.unwrap_err(); + let err = c.send(req, "token", None).await.unwrap_err(); assert!( matches!(err.kind, ViturRequestErrorKind::PrepareRequest), diff --git a/packages/stack-kms/src/endpoint.rs b/packages/stack-kms/src/endpoint.rs index bf3e3bb8e1..169178c0eb 100644 --- a/packages/stack-kms/src/endpoint.rs +++ b/packages/stack-kms/src/endpoint.rs @@ -9,10 +9,11 @@ use url::Url; /// No message repeats the URL it refused: [`NoHost`](Self::NoHost) and /// [`QueryOrFragment`](Self::QueryOrFragment) are checked before /// [`Userinfo`](Self::Userinfo), so the URL they hold can still carry -/// credentials, and a query string can carry anything. A -/// [`NotHosted`](Self::NotHosted) URL came from an unverified token claim, so -/// it can carry whatever its author chose. The URL stays in the variant for a -/// caller in this process. +/// credentials, and a query string can carry anything. The URL stays in +/// those variants for a caller in this process. A URL refused from a token's +/// `services` claim is not kept anywhere, not even for `Debug`: the claim is +/// unverified, so it can carry whatever its author chose, and the error is +/// [`NotHosted`](Self::NotHosted), which holds nothing. #[derive(Debug, Error, PartialEq, Eq, miette::Diagnostic)] #[non_exhaustive] pub enum InvalidEndpoint { @@ -58,7 +59,7 @@ pub enum InvalidEndpoint { code(stack_kms::endpoint_not_hosted), help("To use any other ZeroKMS endpoint, configure it explicitly (`CS_ZEROKMS_HOST` or `with_base_url`).") )] - NotHosted(String), + NotHosted, /// A token's `services` claim named a ZeroKMS in a different region from /// the one this client resolved from an earlier token. The client refuses @@ -85,7 +86,7 @@ impl stack_auth::ErrorPayload for InvalidEndpoint { | Self::NoHost(_) | Self::QueryOrFragment(_) | Self::Userinfo - | Self::NotHosted(_) + | Self::NotHosted | Self::Mismatch => serde_json::Map::new(), } } @@ -187,14 +188,20 @@ impl ZeroKmsEndpoint { /// Whatever [`new`](Self::new) refuses, and /// [`InvalidEndpoint::NotHosted`] for a usable URL on any other host. pub(crate) fn from_services_claim(url: Url) -> Result { - let endpoint = Self::new(url)?; + let endpoint = Self::new(url).map_err(|e| match e { + // These two keep the URL, and a claimed URL must not be kept. + InvalidEndpoint::NoHost(_) | InvalidEndpoint::QueryOrFragment(_) => { + InvalidEndpoint::NotHosted + } + other => other, + })?; let url = endpoint.as_url(); match stack_auth::HostedZeroKmsEndpoint::new(url.clone()) { Ok(hosted) => Ok(HostedEndpoint { endpoint, region: hosted.region(), }), - Err(_) => Err(InvalidEndpoint::NotHosted(endpoint.0.into())), + Err(_) => Err(InvalidEndpoint::NotHosted), } } @@ -437,7 +444,7 @@ mod tests { "https://us-east-1.aws.cts.cipherstashmanaged.net/", ] { assert!( - matches!(claimed(s), Err(InvalidEndpoint::NotHosted(_))), + matches!(claimed(s), Err(InvalidEndpoint::NotHosted)), "{s}: {:?}", claimed(s) ); @@ -498,7 +505,7 @@ mod tests { "https://us-east-1.aws.xzerokms.cipherstashmanaged.net/", ] { assert!( - matches!(claimed(s), Err(InvalidEndpoint::NotHosted(_))), + matches!(claimed(s), Err(InvalidEndpoint::NotHosted)), "{s}: {:?}", claimed(s) ); @@ -509,15 +516,15 @@ mod tests { fn rejects_a_region_cipherstash_does_not_run() { assert!(matches!( claimed("https://mars-north-1.aws.viturhosted.net/"), - Err(InvalidEndpoint::NotHosted(_)) + Err(InvalidEndpoint::NotHosted) )); assert!(matches!( claimed("https://us-east-1.gcp.viturhosted.net/"), - Err(InvalidEndpoint::NotHosted(_)) + Err(InvalidEndpoint::NotHosted) )); assert!(matches!( claimed("https://mars-north-1.aws.zerokms.cipherstashmanaged.net/"), - Err(InvalidEndpoint::NotHosted(_)) + Err(InvalidEndpoint::NotHosted) )); } @@ -525,7 +532,7 @@ mod tests { fn rejects_plain_http_to_a_hosted_name() { assert!(matches!( claimed("http://us-east-1.aws.viturhosted.net/"), - Err(InvalidEndpoint::NotHosted(_)) + Err(InvalidEndpoint::NotHosted) )); } @@ -538,7 +545,7 @@ mod tests { "https://us-east-1.aws.viturhosted.net/other/service/", ] { assert!( - matches!(claimed(s), Err(InvalidEndpoint::NotHosted(_))), + matches!(claimed(s), Err(InvalidEndpoint::NotHosted)), "{s}: {:?}", claimed(s) ); @@ -549,7 +556,7 @@ mod tests { fn rejects_a_non_default_port() { assert!(matches!( claimed("https://us-east-1.aws.viturhosted.net:8443/"), - Err(InvalidEndpoint::NotHosted(_)) + Err(InvalidEndpoint::NotHosted) )); } @@ -561,8 +568,32 @@ mod tests { )); assert!(matches!( claimed("https://us-east-1.aws.viturhosted.net/?q=1"), - Err(InvalidEndpoint::QueryOrFragment(_)) + Err(InvalidEndpoint::NotHosted) )); } + + /// The claim is unverified and its URL may carry anything, so a + /// refused one is not kept, not even where `?err` would print it. + #[test] + fn a_refused_claim_is_not_kept_for_debug_or_display() { + for s in [ + "https://marker-host.example/marker-path", + "https://us-east-1.aws.viturhosted.net/marker-path", + "https://us-east-1.aws.viturhosted.net/?marker-query", + "https://us-east-1.aws.viturhosted.net/#marker-fragment", + ] { + let err = claimed(s).unwrap_err(); + assert_eq!(err, InvalidEndpoint::NotHosted, "{s}"); + let outer = crate::Error::from(claimed(s).unwrap_err()); + for shown in [ + format!("{err}"), + format!("{err:?}"), + format!("{outer}"), + format!("{outer:?}"), + ] { + assert!(!shown.contains("marker"), "{s}: {shown}"); + } + } + } } } diff --git a/packages/stack-kms/src/errors.rs b/packages/stack-kms/src/errors.rs index 5b32faa6a1..afd1edd6ab 100644 --- a/packages/stack-kms/src/errors.rs +++ b/packages/stack-kms/src/errors.rs @@ -349,9 +349,7 @@ mod codes { InvalidEndpoint::QueryOrFragment("https://x/?q".into()) }, InvalidEndpoint::Userinfo => InvalidEndpoint::Userinfo, - InvalidEndpoint::NotHosted(_) => { - InvalidEndpoint::NotHosted("https://evil.example/".into()) - }, + InvalidEndpoint::NotHosted => InvalidEndpoint::NotHosted, InvalidEndpoint::Mismatch => InvalidEndpoint::Mismatch, ])); errors.extend(boxed(variants![ @@ -543,7 +541,6 @@ mod codes { for error in [ InvalidEndpoint::NoHost("marker://user:pass@".into()), InvalidEndpoint::QueryOrFragment("https://x/?marker".into()), - InvalidEndpoint::NotHosted("https://marker.example/".into()), ] { assert!(!error.to_string().contains("marker"), "{error}"); } diff --git a/packages/stack-kms/src/lib.rs b/packages/stack-kms/src/lib.rs index 3e5bf3c40f..910a4bd9b0 100644 --- a/packages/stack-kms/src/lib.rs +++ b/packages/stack-kms/src/lib.rs @@ -19,19 +19,22 @@ //! //! Endpoint discovery now requires a valid CipherStash-hosted ZeroKMS claim //! on **every** token, including tokens fetched after initial discovery. -//! Missing claims, non-hosted URLs and a change of region fail before sending -//! credentials. Supported domains are `viturhosted.net` and +//! Missing claims and non-hosted URLs fail before sending credentials, and so +//! does a change of region once ZeroKMS has accepted a token. Supported domains are `viturhosted.net` and //! `zerokms.cipherstashmanaged.net`; HTTPS, the default port, a root path and //! a region known to this build's `cts-common` are required. Future regions //! require an SDK upgrade. Gateways, staging and local services require an //! explicit endpoint through the builder or `CS_ZEROKMS_HOST`. //! -//! The first discovered **region** remains fixed, while subsequent valid -//! tokens can change its hosted domain. Custom discovery transports must -//! update their [`ZeroKMSConnection::ensure_base_url`] implementation to -//! replace discovered URLs and preserve explicitly configured URLs. Snapshot -//! each request's URL before awaiting transport work. [`InvalidEndpoint`] -//! has new `NotHosted` and `Mismatch` variants and is now non-exhaustive; +//! The **region** of the first claimed endpoint ZeroKMS accepts a token at +//! remains fixed; a token ZeroKMS refuses pins nothing. Each token is sent to +//! the endpoint its own claim names, so later valid tokens can change the +//! region's hosted domain. The endpoint now travels with each request: +//! [`ZeroKMSConnection::send`] takes it as a third argument, used only when +//! the connection has no endpoint of its own; `ensure_base_url` is gone, and +//! the low-level [`Client`] key methods take the endpoint after the access +//! token. [`InvalidEndpoint`] has new `NotHosted` and `Mismatch` variants and +//! is now non-exhaustive; `NotHosted` keeps no part of the refused URL; //! downstream matches must include a fallback arm. `stack-encrypt` exposes //! this version through its `kms` re-export //! starting in 0.3. diff --git a/skills/stash-zerokms/SKILL.md b/skills/stash-zerokms/SKILL.md index 9d2953c60f..1346603da7 100644 --- a/skills/stash-zerokms/SKILL.md +++ b/skills/stash-zerokms/SKILL.md @@ -32,13 +32,14 @@ Rust `stack-kms` 0.2 (also re-exported by `stack-encrypt` 0.3) validates the destination on every token, including refreshes: HTTPS at the root path on the default port, under `viturhosted.net` or `zerokms.cipherstashmanaged.net`, in a region known to that SDK build. Keep -one client per region. Later tokens may move its hosted domain, but a missing, -untrusted, or different-region claim fails before sending credentials. For +one client per region. Once ZeroKMS accepts a token, later tokens may move +its hosted domain, but a missing, untrusted, or different-region claim fails +before sending credentials. For a newly introduced region, upgrade the SDK. For an intentionally configured gateway, staging service or local service, supply an explicit Rust KMS endpoint; normal hosted deployments use discovery. Custom Rust transports -must let `ensure_base_url` update discovered URLs while preserving explicit -configuration. Device provisioning also validates hosted claims; editing a +receive the discovered endpoint with each `send` call and use it only when +they were not configured with one. Device provisioning also validates hosted claims; editing a profile token is not an endpoint configuration mechanism. ## The model in one paragraph From cf1cf67bbf77ed58544afd0ed6d3aa01c80ff548 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 02:47:10 +0000 Subject: [PATCH 7/8] test(auth): expect provisioning to refuse a non-hosted ZeroKMS claim bindClientDevice now sends the bearer only to a CipherStash-hosted ZeroKMS, so the binding can no longer reach the local mock server the success, 409 and server-error cases pointed the token's claim at. Those tests failed with INVALID_TOKEN, and the server-error one passed only because refusal is also a failure. Replace them with one test of what the binding must now do: refuse with INVALID_TOKEN before any request reaches the server, and write no secret key. The success and 409 paths stay covered by stack-auth's Rust tests, which route a hosted destination to a local server through the transport seam the binding does not expose. Refs #1546 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012Jmn7o2Eouw3CQpnBNGrwC --- .../auth/__tests__/helpers/mock-cts-server.ts | 7 ++++ .../__tests__/provision-device-client.test.ts | 39 +++++-------------- 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/languages/typescript/packages/auth/__tests__/helpers/mock-cts-server.ts b/languages/typescript/packages/auth/__tests__/helpers/mock-cts-server.ts index 652daf64be..4a6e3459eb 100644 --- a/languages/typescript/packages/auth/__tests__/helpers/mock-cts-server.ts +++ b/languages/typescript/packages/auth/__tests__/helpers/mock-cts-server.ts @@ -17,6 +17,7 @@ export class MockCtsServer { #server: Server #routes = new Map() #baseUrl = '' + #requestCount = 0 private constructor(server: Server) { this.#server = server @@ -26,6 +27,7 @@ export class MockCtsServer { static async start(): Promise { const mock = new MockCtsServer( createServer((req, res) => { + mock.#requestCount += 1 const handler = mock.#routes.get(`${req.method} ${req.url}`) if (!handler) { res.writeHead(404, { 'content-type': 'application/json' }) @@ -205,6 +207,11 @@ export class MockCtsServer { })) } + /** How many requests reached the server, matched or not. */ + get requestCount(): number { + return this.#requestCount + } + clearMocks(): void { this.#routes.clear() } diff --git a/languages/typescript/packages/auth/__tests__/provision-device-client.test.ts b/languages/typescript/packages/auth/__tests__/provision-device-client.test.ts index a3cd438fac..7b48dde5c0 100644 --- a/languages/typescript/packages/auth/__tests__/provision-device-client.test.ts +++ b/languages/typescript/packages/auth/__tests__/provision-device-client.test.ts @@ -48,19 +48,21 @@ describe('provision device client (TypeScript / vitest)', () => { await server.close() }) - it('creates secretkey.json on successful provisioning', async () => { + // A profile token's ZeroKMS claim is unverified, so provisioning only + // sends the bearer to a CipherStash-hosted ZeroKMS. This mock server is not + // one, so the binding cannot reach it: the success and 409 paths are + // covered by stack-auth's Rust tests through its transport seam, and here + // the binding must refuse before any request leaves. + it('refuses a token naming a ZeroKMS outside CipherStash, before sending it', async () => { server.mockCreateClientEndpoint() saveTestToken(profileDir, server.baseUrl) const r = await bindClientDevice() - if (r.failure) { - expect.unreachable(`bindClientDevice failed: ${r.failure.type}`) - } - const raw = readFileSync(join(workspaceDir(), 'secretkey.json'), 'utf-8') - const secretKey = JSON.parse(raw) - expect(secretKey.client_id).toBe('00000000-0000-0000-0000-000000000001') - expect(secretKey.client_key).toBe('dGVzdC1rZXktbWF0ZXJpYWw=') + expect(r.failure?.type).toBe('INVALID_TOKEN') + expect(r.failure?.error).toBeInstanceOf(Error) + expect(server.requestCount).toBe(0) + expect(existsSync(join(workspaceDir(), 'secretkey.json'))).toBe(false) }) it('is a no-op when secretkey.json already exists', async () => { @@ -84,27 +86,6 @@ describe('provision device client (TypeScript / vitest)', () => { expect(secretKey.client_id).toBe('existing-id') }) - it('is a no-op on 409 conflict', async () => { - server.mockCreateClientConflict() - saveTestToken(profileDir, server.baseUrl) - - const r = await bindClientDevice() - if (r.failure) { - expect.unreachable(`bindClientDevice failed: ${r.failure.type}`) - } - - expect(existsSync(join(workspaceDir(), 'secretkey.json'))).toBe(false) - }) - - it('fails on server error', async () => { - // No mock endpoint — server will return an error for unmatched route. - saveTestToken(profileDir, server.baseUrl) - - const r = await bindClientDevice() - expect(r.failure).toBeTruthy() - expect(r.failure?.error).toBeInstanceOf(Error) - }) - it('fails with STORE_ERROR when auth token is missing', async () => { // No token saved — should fail trying to load auth.json const r = await bindClientDevice() From e76ae3df8cdbd9e19b0c1e735d638dc59a46c4fa Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 02:59:30 +0000 Subject: [PATCH 8/8] fix(stack-kms): keep a refused claim's scheme out of the error too A claim such as `marker:///` has a host, so it passed the mapping to NotHosted and came back as Scheme("marker"): a string the claim's author chose, in Display, Debug and the error payload. That breaks the rule the previous commit stated, that a refused claim is kept nowhere. Scheme now maps to NotHosted on the claim path as well, and the redaction test covers it. Also rewrap the 0.2 upgrade notes in lib.rs. Refs #1546 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012Jmn7o2Eouw3CQpnBNGrwC --- packages/stack-kms/src/endpoint.rs | 10 ++++++---- packages/stack-kms/src/lib.rs | 7 ++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/stack-kms/src/endpoint.rs b/packages/stack-kms/src/endpoint.rs index 169178c0eb..8dad3927ad 100644 --- a/packages/stack-kms/src/endpoint.rs +++ b/packages/stack-kms/src/endpoint.rs @@ -189,10 +189,11 @@ impl ZeroKmsEndpoint { /// [`InvalidEndpoint::NotHosted`] for a usable URL on any other host. pub(crate) fn from_services_claim(url: Url) -> Result { let endpoint = Self::new(url).map_err(|e| match e { - // These two keep the URL, and a claimed URL must not be kept. - InvalidEndpoint::NoHost(_) | InvalidEndpoint::QueryOrFragment(_) => { - InvalidEndpoint::NotHosted - } + // These keep the URL or part of it (`Scheme` keeps the scheme, + // which the claim's author chose), and a claim must not be kept. + InvalidEndpoint::NoHost(_) + | InvalidEndpoint::QueryOrFragment(_) + | InvalidEndpoint::Scheme(_) => InvalidEndpoint::NotHosted, other => other, })?; let url = endpoint.as_url(); @@ -581,6 +582,7 @@ mod tests { "https://us-east-1.aws.viturhosted.net/marker-path", "https://us-east-1.aws.viturhosted.net/?marker-query", "https://us-east-1.aws.viturhosted.net/#marker-fragment", + "marker://us-east-1.aws.viturhosted.net/", ] { let err = claimed(s).unwrap_err(); assert_eq!(err, InvalidEndpoint::NotHosted, "{s}"); diff --git a/packages/stack-kms/src/lib.rs b/packages/stack-kms/src/lib.rs index 910a4bd9b0..2c50e3b225 100644 --- a/packages/stack-kms/src/lib.rs +++ b/packages/stack-kms/src/lib.rs @@ -20,9 +20,10 @@ //! Endpoint discovery now requires a valid CipherStash-hosted ZeroKMS claim //! on **every** token, including tokens fetched after initial discovery. //! Missing claims and non-hosted URLs fail before sending credentials, and so -//! does a change of region once ZeroKMS has accepted a token. Supported domains are `viturhosted.net` and -//! `zerokms.cipherstashmanaged.net`; HTTPS, the default port, a root path and -//! a region known to this build's `cts-common` are required. Future regions +//! does a change of region once ZeroKMS has accepted a token. Supported +//! domains are `viturhosted.net` and `zerokms.cipherstashmanaged.net`; HTTPS, +//! the default port, a root path and a region known to this build's +//! `cts-common` are required. Future regions //! require an SDK upgrade. Gateways, staging and local services require an //! explicit endpoint through the builder or `CS_ZEROKMS_HOST`. //!