From 670fd6cff390d9c162ab168a45222907dedb4867 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Fri, 25 Sep 2026 13:35:03 -0700 Subject: [PATCH 1/7] feat(sts): exchange opaque API keys at /.sts by hash lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-013 as revised (#234): a service account's API key is an opaque `sck_` secret that source.coop stores as a SHA-256 hash. When `/.sts` receives one as `WebIdentityToken`, the proxy trims and format-checks it locally, hashes it, and asks `POST {SOURCE_API_URL}/api/v1/service-account-keys/exchanges` whether it is active and for which account, authenticated as itself with the sentinel subject `urn:source:data-proxy`. The answer is cached for 60 seconds, inactive answers included; an API failure fails closed with a 500 and caches nothing. It then mints credentials under the `_default` role for the account the API names, through the STS crate's minting and sealing. A key is accepted only from a POST form body. One in the query string is refused before any lookup, with a message saying why, because Cloudflare logs request URLs. Every refusal of the key itself reads "API key was not accepted (request id …)" with the id also in `x-amzn-requestid`; the reason is logged once under that id. Attempts are rate-limited per client IP by a new `KEY_EXCHANGE_LIMIT` ratelimit binding in every wrangler config; a deployment without it logs an error and does not refuse traffic. `ApiAuth` gains `authorization_header_as_self` and refuses the sentinel as an on-behalf-of subject; `cached_fetch` takes a method, an optional JSON body and an `ApiCaller`. `sts::default_role` is factored out so the key exchange mints under the same role. Supersedes #233: nothing signs a key, so `/.keys`, self-verification against the proxy's own JWKS, the `api_key` role and the multistore pin are gone. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd --- Cargo.toml | 4 + README.md | 10 ++ src/keys.rs | 82 +++++++++++++++++ src/lib.rs | 198 ++++++++++++++++++++++++++++++++++++++++ src/source_api/auth.rs | 60 +++++++++++- src/source_api/cache.rs | 81 +++++++++++++--- src/sts.rs | 30 ++++-- tests/keys.rs | 106 +++++++++++++++++++++ tests/stub_api.py | 47 ++++++++++ tests/test_api_keys.py | 133 +++++++++++++++++++++++++++ wrangler.preview.toml | 7 ++ wrangler.toml | 16 ++++ 12 files changed, 746 insertions(+), 28 deletions(-) create mode 100644 src/keys.rs create mode 100644 tests/keys.rs create mode 100644 tests/test_api_keys.py diff --git a/Cargo.toml b/Cargo.toml index 472e6b39..e5f80da3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,10 @@ path = "tests/object_path.rs" name = "fixtures" path = "tests/fixtures.rs" +[[test]] +name = "keys" +path = "tests/keys.rs" + [dependencies] # Multistore multistore = { version = "0.7.2", features = ["azure", "gcp"] } diff --git a/README.md b/README.md index 2699e72e..381912bf 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,16 @@ Set in `wrangler.toml` or via the Cloudflare dashboard: | `OIDC_PROVIDER_KID` | `data-proxy-1` | Key ID for the active signing key | | `OIDC_PROVIDER_KID_PREVIOUS` | — | Key ID for the previous key (during rotation) | +### Bindings + +| Binding | Kind | Description | +| -------------------- | ----------- | -------------------------------------------------------------------------------------------------------------------------------------------- | +| `KEY_EXCHANGE_LIMIT` | `ratelimit` | Per-client-IP limit on API-key exchanges at `/.sts` (ADR-013). Declared under `[[unsafe.bindings]]` in every `wrangler*.toml`; a deployment without it logs an error and exchanges without a limit | + +### API keys + +A service account's API key (ADR-013) is an opaque `sck_` secret that source.coop stores as a hash. It is presented at `/.sts` as `WebIdentityToken`, from a POST form body only — a key in the URL is refused, because the URL is logged. The proxy trims and format-checks it, hashes it, and asks `POST {SOURCE_API_URL}/api/v1/service-account-keys/exchanges` for its standing as itself (subject `urn:source:data-proxy`), caching the answer for 60 seconds; then it mints credentials for the account the API names, exactly as it would for an ID token. Every refusal of the key reads `API key was not accepted (request id …)`; the reason is in the log under that id. + ### Secrets **GitHub environment secrets are the source of truth.** The deploy workflow diff --git a/src/keys.rs b/src/keys.rs new file mode 100644 index 00000000..7e06074f --- /dev/null +++ b/src/keys.rs @@ -0,0 +1,82 @@ +//! API keys (ADR-013): opaque secrets a service account presents at `/.sts` +//! in place of an OIDC token. Nothing here verifies a signature — there is +//! none. The key's standing lives in source.coop, keyed by the key's SHA-256, +//! and this module is the wasm-free half of the exchange: recognising a key, +//! hashing it, and sealing credentials for the account the API names. + +use multistore::error::ProxyError; +use multistore::types::{RoleConfig, TemporaryCredentials}; +use multistore_sts::sts::mint_temporary_credentials; +use multistore_sts::TokenKey; +use serde::Deserialize; +use sha2::{Digest, Sha256}; + +/// Every key starts with this, followed by 32 random bytes in base64url: +/// a fixed 47 characters, the pattern secret scanners are given. +pub const API_KEY_PREFIX: &str = "sck_"; +const API_KEY_LEN: usize = 47; + +/// The token, trimmed, if it has exactly a key's shape; `None` for anything +/// else — a JWT, a truncated key, the wrong case — so the JWT path or a local +/// refusal takes it without a lookup. Whitespace is trimmed first because +/// every hand-made token file ends in a newline, and some SDKs send it. +pub fn parse_api_key(token: &str) -> Option<&str> { + let key = token.trim(); + (key.len() == API_KEY_LEN + && key.starts_with(API_KEY_PREFIX) + && key[API_KEY_PREFIX.len()..] + .bytes() + .all(|b| b.is_ascii_alphanumeric() || b == b'-' || b == b'_')) + .then_some(key) +} + +/// Whether the token so much as looks like a key — the prefix alone. Used to +/// refuse a key sent where it would be logged, before checking anything else. +pub fn looks_like_api_key(token: &str) -> bool { + token.trim_start().starts_with(API_KEY_PREFIX) +} + +/// Hex SHA-256 of a key: the record's key in source.coop, and all the proxy +/// ever sends of it. +pub fn key_hash(key: &str) -> String { + Sha256::digest(key.as_bytes()) + .iter() + .map(|b| format!("{b:02x}")) + .collect() +} + +/// The Source API's answer for a presented key +/// (`POST /api/v1/service-account-keys/exchanges`): whether it may be +/// exchanged and, if so, for whom. Unknown, revoked, expired and disabled all +/// come back inactive and unnamed, so nothing distinguishes them here. +#[derive(Debug, Clone, Deserialize)] +pub struct KeyStanding { + pub active: bool, + #[serde(default)] + pub account_id: Option, + #[serde(default)] + pub key_id: Option, +} + +/// Credentials for an account the API has vouched for, sealed the way the +/// STS route seals every session, for the duration the client asked for +/// within the role's cap. The floor and default are AWS's and multistore's. +pub fn credentials_for( + role: &RoleConfig, + account_id: &str, + duration_seconds: Option, + token_key: &TokenKey, +) -> Result { + let duration = duration_seconds + .unwrap_or(3600) + .clamp(900, role.max_session_duration_secs); + let mut creds = mint_temporary_credentials( + role, + account_id, + duration, + "STSPRXY", + &serde_json::json!({}), + ); + creds.session_token = token_key.seal(&creds)?; + Ok(creds) +} diff --git a/src/lib.rs b/src/lib.rs index e9f8d42e..d3199bb0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,19 +12,23 @@ mod authz; mod backend_auth; mod config; mod handlers; +mod keys; mod location; mod object_path; mod pagination; mod source_api; mod sts; +use crate::config::AppConfig; use crate::source_api::{ApiAuth, SourceCoopRegistry}; use analytics::log_analytics; use handlers::{AccountListHandler, IndexHandler}; use multistore::api::response::ErrorResponse; +use multistore::error::ProxyError; use multistore::proxy::{GatewayResponse, ProxyGateway}; use multistore::route_handler::{ProxyResult, RequestInfo}; use multistore::router::Router; +use multistore::types::TemporaryCredentials; use multistore_cf_workers::{ collect_js_body, GatewayResponseExt, NoopCredentialRegistry, RequestParts, WorkerBackend, WorkerSubscriber, @@ -35,6 +39,7 @@ use multistore_oidc_provider::{HttpExchange, OidcCredentialProvider, OidcProvide use multistore_path_mapping::{MappedRegistry, PathMapping}; use multistore_sts::jwks::JwksCache; use multistore_sts::route_handler::StsRouterExt; +use multistore_sts::{build_sts_error_response, build_sts_response, try_parse_sts_request}; use object_path::{extract_path_segments, is_keyless_write, mapped_copy_source}; use std::sync::OnceLock; use sts::StsCredentialRegistry; @@ -231,6 +236,16 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result(headers: &'a http::HeaderMap, name: &str) -> &'a st .unwrap_or("") } +// ── API keys ──────────────────────────────────────────────────────── + +/// A response answered before the gateway, with the CORS headers and the +/// request id every gateway response carries — as `x-request-id`, and as +/// `x-amzn-requestid`, the header AWS SDKs read it from. +fn finish((status, xml): (u16, String), request_id: &str) -> web_sys::Response { + let response = + add_cors(GatewayResponse::Response(ProxyResult::xml(status, xml)).into_web_sys()); + if !request_id.is_empty() { + let _ = response.headers().set("x-request-id", request_id); + let _ = response.headers().set("x-amzn-requestid", request_id); + } + response +} + +/// The rate-limiter binding for API-key exchanges, keyed by client IP. +const KEY_EXCHANGE_LIMIT: &str = "KEY_EXCHANGE_LIMIT"; + +/// The API-key exchange, if this request is one: `None` when it is not an +/// `AssumeRoleWithWebIdentity` carrying an `sck_` key, so the STS route takes +/// it. A key is accepted from the form body only — Cloudflare logs the URL — +/// and the refusal for one in the query string says so, because that is the +/// one mistake a user can fix. +async fn api_key_exchange( + config: &AppConfig, + parts: &RequestParts, + env: &Env, + api_auth: &ApiAuth, + request_id: &str, +) -> Option<(u16, String)> { + if let Some(parsed) = try_parse_sts_request(parts.query.as_deref()) { + let is_key = parsed + .as_ref() + .is_ok_and(|sts| keys::looks_like_api_key(&sts.web_identity_token)); + if !is_key { + return None; // a token in the query string is the STS route's + } + tracing::warn!(%request_id, reason = "query_string", "API key exchange refused"); + return Some(key_refusal( + "API key must be sent in the request body, not the URL", + request_id, + )); + } + let sts = try_parse_sts_request(parts.form_body.as_deref())?.ok()?; + if !keys::looks_like_api_key(&sts.web_identity_token) { + return None; + } + + // Every attempt costs a lookup for a distinct key, so the flood to bound is + // distinct junk keys from one place. Legitimate exchanges are rare — once + // per session — so even a cluster behind one NAT stays well under the limit. + let client_ip = header_str(&parts.headers, "cf-connecting-ip"); + if !within_rate_limit(env, client_ip).await { + tracing::warn!(%request_id, reason = "rate_limited", "API key exchange refused"); + return Some(( + 429, + sts_error_xml( + "Throttling", + "too many API key exchanges from this address; retry later", + ), + )); + } + + Some( + match exchange_api_key(config, &sts, api_auth, request_id).await { + Ok(creds) => build_sts_response(&creds), + // One answer for every refusal of the key itself — unknown, revoked, + // expired, disabled, malformed. `exchange_api_key` has logged why. + Err(ProxyError::InvalidOidcToken(_)) => { + key_refusal("API key was not accepted", request_id) + } + // A bad role is about the request, not the key; anything else is the + // API being unreachable, which fails closed as a 500 the SDK retries. + Err(e) => { + tracing::warn!(%request_id, error = %e, "API key exchange failed"); + build_sts_error_response(&e) + } + }, + ) +} + +/// `InvalidIdentityToken` with the request id in the message: SDKs show a +/// user the message and nothing else, and the id is what finds the log line. +fn key_refusal(message: &str, request_id: &str) -> (u16, String) { + let message = if request_id.is_empty() { + message.to_string() + } else { + format!("{message} (request id {request_id})") + }; + build_sts_error_response(&ProxyError::InvalidOidcToken(message)) +} + +/// Hash the key, ask source.coop for its standing, and mint for the account +/// it names. A refused key is logged here, once, and returned as +/// `InvalidOidcToken`. The proxy knows only "malformed" or "inactive": which +/// of unknown, revoked, expired or disabled is in source.coop's log under the +/// same request id. +async fn exchange_api_key( + config: &AppConfig, + sts: &multistore_sts::request::StsRequest, + api_auth: &ApiAuth, + request_id: &str, +) -> Result { + let Some(key) = keys::parse_api_key(&sts.web_identity_token) else { + tracing::warn!(%request_id, reason = "malformed", "API key exchange refused"); + return Err(ProxyError::InvalidOidcToken("malformed".into())); + }; + if !sts::is_default_role(&sts.role_arn) { + return Err(ProxyError::RoleNotFound(sts.role_arn.clone())); + } + let key_hash = keys::key_hash(key); + let standing = source_api::cache::get_or_fetch_key_standing( + &config.api_base_url, + &key_hash, + api_auth, + request_id, + ) + .await + .map_err(|e| match e { + // The route answers 200 for any well-formed hash; a 404 means the API + // does not serve it, which is a deployment mismatch, not an unknown key. + ProxyError::BucketNotFound(_) => { + ProxyError::Internal("key standing route not found".into()) + } + e => e, + })?; + let key_id = standing.key_id.as_deref().unwrap_or(""); + let hash_prefix = &key_hash[..8]; + let account_id = match (standing.active, standing.account_id) { + (true, Some(account_id)) => account_id, + _ => { + tracing::warn!(%request_id, key_id, hash_prefix, "API key is not active"); + return Err(ProxyError::InvalidOidcToken("inactive".into())); + } + }; + let role = sts::default_role( + config.auth_issuer.clone(), + config.auth_audiences.clone(), + config.sts_max_session_duration_secs, + ); + let creds = keys::credentials_for( + &role, + &account_id, + sts.duration_seconds, + &config.session_token_key, + )?; + tracing::info!(%request_id, key_id, %account_id, "API key exchanged"); + Ok(creds) +} + +/// Whether `client_ip` may make another exchange attempt now. A missing +/// binding is a deployment error, logged as such; it does not refuse traffic. +async fn within_rate_limit(env: &Env, client_ip: &str) -> bool { + let key = if client_ip.is_empty() { + "unknown" + } else { + client_ip + }; + match env.rate_limiter(KEY_EXCHANGE_LIMIT) { + Ok(limiter) => match limiter.limit(key.to_string()).await { + Ok(outcome) => outcome.success, + Err(e) => { + tracing::warn!("rate limiter call failed: {e}"); + true + } + }, + Err(_) => { + tracing::error!( + "{KEY_EXCHANGE_LIMIT} binding is not configured; API-key exchanges are unlimited" + ); + true + } + } +} + +/// An STS-shaped error body for a status `build_sts_error_response` has no +/// variant for. +fn sts_error_xml(code: &str, message: &str) -> String { + format!( + "\n{code}{message}" + ) +} + // ── CORS ──────────────────────────────────────────────────────────── fn add_cors(resp: web_sys::Response) -> web_sys::Response { diff --git a/src/source_api/auth.rs b/src/source_api/auth.rs index f803c622..41da850c 100644 --- a/src/source_api/auth.rs +++ b/src/source_api/auth.rs @@ -1,5 +1,29 @@ use multistore_oidc_provider::jwt::JwtSigner; +/// The subject the proxy signs with when it calls the API as itself rather +/// than on behalf of an account (ADR-013, amending ADR-005). Only the API-key +/// standing lookup accepts it: the API-key exchange happens before anything +/// names an account. A URN, so no account id can ever equal it, and refused +/// as an on-behalf-of subject so no request can claim it. +pub(crate) const PROXY_SELF_SUBJECT: &str = "urn:source:data-proxy"; + +/// Who an API request is made as. +#[derive(Clone, Copy, Debug)] +pub(crate) enum ApiCaller<'a> { + /// No credentials: the API answers as it would any stranger. + Anonymous, + /// On behalf of an account the proxy has authenticated. + Account(&'a str), + /// The proxy itself. + Proxy, +} + +impl<'a> From> for ApiCaller<'a> { + fn from(subject: Option<&'a str>) -> Self { + subject.map_or(ApiCaller::Anonymous, ApiCaller::Account) + } +} + /// How the proxy authenticates to the Source Cooperative API. #[derive(Clone)] pub(crate) struct ApiAuth { @@ -20,12 +44,38 @@ impl ApiAuth { /// Build the `Authorization` header value for an API request on behalf of /// `subject`. /// - /// Returns `None` if signing fails. The signing key is parsed and validated - /// once at startup (`JwtSigner::from_pem`, which panics on a bad key), so a - /// runtime failure here is very unlikely. When it does happen the error is - /// logged and the caller falls through to an unauthenticated request, which - /// the API surfaces as `AccessDenied` (403) rather than a 500. + /// Returns `None` if signing fails, or if `subject` is the proxy's own + /// sentinel — that is `authorization_header_as_self`'s to sign, never a + /// caller's to claim. The signing key is parsed and validated once at + /// startup (`JwtSigner::from_pem`, which panics on a bad key), so a + /// runtime signing failure is very unlikely. When it does happen the + /// error is logged and the caller falls through to an unauthenticated + /// request, which the API surfaces as `AccessDenied` (403) rather than a + /// 500. pub fn authorization_header(&self, subject: &str) -> Option { + if subject == PROXY_SELF_SUBJECT { + tracing::error!("refusing to sign an on-behalf-of assertion as the proxy itself"); + return None; + } + self.sign(subject) + } + + /// The `Authorization` header value for a request the proxy makes as + /// itself. See `PROXY_SELF_SUBJECT`. + pub fn authorization_header_as_self(&self) -> Option { + self.sign(PROXY_SELF_SUBJECT) + } + + /// The header for `caller`, or `None` when the request goes out anonymous. + pub fn authorization_header_for(&self, caller: ApiCaller<'_>) -> Option { + match caller { + ApiCaller::Anonymous => None, + ApiCaller::Account(subject) => self.authorization_header(subject), + ApiCaller::Proxy => self.authorization_header_as_self(), + } + } + + fn sign(&self, subject: &str) -> Option { match self.signer.sign(subject, &self.issuer, &self.audience, &[]) { Ok(token) => Some(format!("Bearer {}", token)), Err(e) => { diff --git a/src/source_api/cache.rs b/src/source_api/cache.rs index a3499843..5cb7743a 100644 --- a/src/source_api/cache.rs +++ b/src/source_api/cache.rs @@ -3,7 +3,9 @@ //! Each public function caches one API call type with its own TTL. //! Adjust the `*_CACHE_SECS` constants to tune per-datatype expiry. +use super::auth::ApiCaller; use super::types::{DataConnection, SourceProduct, SourceProductList}; +use crate::keys::KeyStanding; use multistore::error::ProxyError; use percent_encoding::{utf8_percent_encode, AsciiSet, NON_ALPHANUMERIC}; @@ -44,6 +46,12 @@ const PRODUCT_LIST_CACHE_SECS: u32 = 60; // 1 minute /// so a revoked grant should stop taking effect quickly. const PERMISSIONS_CACHE_SECS: u32 = 60; // 1 minute +/// A presented API key's standing (`/service-account-keys/exchanges`). The +/// permissions TTL, for the same reason: it gates access, so a revoked key +/// should stop being exchangeable quickly (ADR-013). Inactive answers are +/// cached too — an unknown key costs one lookup a minute, not one a request. +const KEY_STANDING_CACHE_SECS: u32 = 60; // 1 minute + // ── Public cache functions ───────────────────────────────────────── /// Fetch a single product's metadata, cached for `PRODUCT_CACHE_SECS`. @@ -65,10 +73,12 @@ pub async fn get_or_fetch_product( cached_fetch( &cache_key, &api_url, + "GET", + None, PRODUCT_CACHE_SECS, api_auth, request_id, - subject, + subject.into(), ) .await } @@ -94,10 +104,12 @@ pub async fn get_or_fetch_permissions( cached_fetch( &cache_key, &api_url, + "GET", + None, PERMISSIONS_CACHE_SECS, api_auth, request_id, - Some(subject), + ApiCaller::Account(subject), ) .await } @@ -125,10 +137,12 @@ pub async fn get_or_fetch_data_connection( cached_fetch( &cache_key, &api_url, + "GET", + None, DATA_CONNECTION_CACHE_SECS, api_auth, request_id, - subject, + subject.into(), ) .await } @@ -150,10 +164,42 @@ pub async fn get_or_fetch_product_list( cached_fetch( &cache_key, &api_url, + "GET", + None, PRODUCT_LIST_CACHE_SECS, api_auth, request_id, - subject, + subject.into(), + ) + .await +} + +/// A presented API key's standing, cached for `KEY_STANDING_CACHE_SECS`, +/// looked up by the key's hash — the key itself never leaves the proxy. +/// Asked as the proxy itself: nothing names an account before the answer. +/// A POST, because the API records the use as it answers; the route answers +/// 200 for any well-formed hash, inactive ones included, so the cache holds +/// every answer alike. +pub async fn get_or_fetch_key_standing( + api_base_url: &str, + key_hash: &str, + api_auth: &crate::ApiAuth, + request_id: &str, +) -> Result { + let api_url = format!("{}/api/v1/service-account-keys/exchanges", api_base_url); + // The Cache API keys on URLs; the base URL scopes the entry to this + // environment's API, and the hash is the only thing that varies. + let cache_key = format!("{api_url}?key_hash={key_hash}"); + let body = serde_json::json!({ "key_hash": key_hash }).to_string(); + cached_fetch( + &cache_key, + &api_url, + "POST", + Some(&body), + KEY_STANDING_CACHE_SECS, + api_auth, + request_id, + ApiCaller::Proxy, ) .await } @@ -181,15 +227,18 @@ fn cache_key_with_subject(api_url: &str, subject: Option<&str>) -> String { } /// Generic cache-or-fetch: check the Cache API, return cached JSON on hit, -/// otherwise fetch from `api_url`, store in cache with the given TTL, and -/// return the deserialized result. +/// otherwise fetch from `api_url` with `method` (and a JSON `body`, if any), +/// store in cache with the given TTL, and return the deserialized result. +#[allow(clippy::too_many_arguments)] async fn cached_fetch( cache_key: &str, api_url: &str, + method: &str, + body: Option<&str>, ttl_secs: u32, api_auth: &crate::ApiAuth, request_id: &str, - subject: Option<&str>, + caller: ApiCaller<'_>, ) -> Result { let span = tracing::info_span!( "cached_fetch", @@ -219,17 +268,21 @@ async fn cached_fetch( // ── Cache miss — fetch from API ──────────────────────────── span.record("cache_hit", false); let init = web_sys::RequestInit::new(); - init.set_method("GET"); + init.set_method(method); let req_headers = web_sys::Headers::new() .map_err(|e| ProxyError::Internal(format!("headers build failed: {:?}", e)))?; // Only authenticate to the API when we have an identified caller. // Anonymous proxy requests hit the API without credentials. - if let Some(subj) = subject { - if let Some(auth_value) = api_auth.authorization_header(subj) { - req_headers - .set("Authorization", &auth_value) - .map_err(|e| ProxyError::Internal(format!("header set failed: {:?}", e)))?; - } + if let Some(auth_value) = api_auth.authorization_header_for(caller) { + req_headers + .set("Authorization", &auth_value) + .map_err(|e| ProxyError::Internal(format!("header set failed: {:?}", e)))?; + } + if let Some(body) = body { + req_headers + .set("content-type", "application/json") + .map_err(|e| ProxyError::Internal(format!("header set failed: {:?}", e)))?; + init.set_body(&wasm_bindgen::JsValue::from_str(body)); } if !request_id.is_empty() { let _ = req_headers.set("x-request-id", request_id); diff --git a/src/sts.rs b/src/sts.rs index da49bc1e..4290aa9d 100644 --- a/src/sts.rs +++ b/src/sts.rs @@ -36,19 +36,31 @@ impl StsCredentialRegistry { max_session_duration_secs: u64, ) -> Self { Self { - default_role: RoleConfig { - role_id: "_default".to_string(), - name: "Default".to_string(), - trusted_oidc_issuers: vec![oidc_issuer], - required_audiences, - subject_conditions: vec![], - allowed_scopes: vec![], // unlimited - max_session_duration_secs, - }, + default_role: default_role(oidc_issuer, required_audiences, max_session_duration_secs), } } } +/// The `_default` role: trusts `oidc_issuer` for tokens minted for one of +/// `required_audiences`, with no scope restriction. Shared with the API-key +/// exchange, which mints under the same role once the API has named the +/// account (`keys::credentials_for`). +pub(crate) fn default_role( + oidc_issuer: String, + required_audiences: Vec, + max_session_duration_secs: u64, +) -> RoleConfig { + RoleConfig { + role_id: "_default".to_string(), + name: "Default".to_string(), + trusted_oidc_issuers: vec![oidc_issuer], + required_audiences, + subject_conditions: vec![], + allowed_scopes: vec![], // unlimited + max_session_duration_secs, + } +} + impl CredentialRegistry for StsCredentialRegistry { async fn get_credential( &self, diff --git a/tests/keys.rs b/tests/keys.rs new file mode 100644 index 00000000..7543dd88 --- /dev/null +++ b/tests/keys.rs @@ -0,0 +1,106 @@ +//! Native unit tests for the wasm-free half of the API-key exchange (`keys`), +//! included via `#[path]` like `tests/sts.rs`. The Cache API and the lookup +//! itself are wasm-only and are covered by `tests/test_api_keys.py`. + +#[path = "../src/keys.rs"] +mod keys; +#[path = "../src/sts.rs"] +mod sts; + +use keys::*; +use multistore_sts::TokenKey; + +const KEY: &str = "sck_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + +// ── recognising a key ────────────────────────────────────────────── + +#[test] +fn a_well_formed_key_is_a_key() { + assert_eq!(parse_api_key(KEY), Some(KEY)); + let mixed = "sck_Ab-_09Ab-_09Ab-_09Ab-_09Ab-_09Ab-_09Ab-_09A"; + assert_eq!(mixed.len(), 47); + assert_eq!(parse_api_key(mixed), Some(mixed)); +} + +#[test] +fn surrounding_whitespace_is_trimmed() { + // Every hand-made token file ends in a newline; some SDKs send it. + assert_eq!(parse_api_key(&format!("{KEY}\n")), Some(KEY)); + assert_eq!(parse_api_key(&format!("{KEY}\r\n")), Some(KEY)); + assert_eq!(parse_api_key(&format!(" {KEY} ")), Some(KEY)); +} + +#[test] +fn anything_else_is_not_a_key() { + assert_eq!(parse_api_key(&KEY[..46]), None, "too short"); + assert_eq!(parse_api_key(&format!("{KEY}a")), None, "too long"); + assert_eq!( + parse_api_key(&KEY.replace("sck_", "SCK_")), + None, + "wrong case" + ); + assert_eq!(parse_api_key(&KEY.replace('a', "+")), None, "not base64url"); + assert_eq!( + parse_api_key("eyJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJ4In0.sig"), + None, + "a JWT" + ); + assert_eq!(parse_api_key(""), None); +} + +#[test] +fn the_prefix_alone_marks_a_key_for_refusal() { + assert!(looks_like_api_key("sck_anything")); + assert!(looks_like_api_key(" sck_anything")); + assert!(!looks_like_api_key("eyJ.a.b")); + assert!(!looks_like_api_key("SCK_anything")); +} + +// ── hashing ──────────────────────────────────────────────────────── + +#[test] +fn the_hash_is_hex_sha256_of_the_key() { + // Computed independently: sha256("sck_" + "a" * 43). + assert_eq!( + key_hash(KEY), + "079124300599a6ace561d0554a60dccf90edb04d486b55062bba42ff230d4f5f" + ); + assert_eq!(key_hash(KEY).len(), 64); +} + +// ── minting ──────────────────────────────────────────────────────── + +fn token_key() -> TokenKey { + TokenKey::from_base64("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=").unwrap() +} + +fn role(cap: u64) -> multistore::types::RoleConfig { + sts::default_role("https://auth.example.test".into(), vec!["aud".into()], cap) +} + +#[test] +fn credentials_are_sealed_for_the_account_within_floor_and_cap() { + let key = token_key(); + let creds = credentials_for(&role(43_200), "acme--nightly-sync", None, &key).unwrap(); + assert_eq!(creds.source_identity, "acme--nightly-sync"); + assert_eq!(creds.assumed_role_id, "_default"); + assert!(creds.access_key_id.starts_with("STSPRXY")); + // Sealed: the session token unseals to these credentials. + let unsealed = key.unseal(&creds.session_token).unwrap().unwrap(); + assert_eq!(unsealed.source_identity, "acme--nightly-sync"); + + let now = chrono_now(); + let default = credentials_for(&role(43_200), "a", None, &key).unwrap(); + assert!((default.expiration.timestamp() - now - 3600).abs() <= 2); + let floored = credentials_for(&role(43_200), "a", Some(1), &key).unwrap(); + assert!((floored.expiration.timestamp() - now - 900).abs() <= 2); + let capped = credentials_for(&role(3_600), "a", Some(86_400), &key).unwrap(); + assert!((capped.expiration.timestamp() - now - 3600).abs() <= 2); +} + +fn chrono_now() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() as i64 +} diff --git a/tests/stub_api.py b/tests/stub_api.py index 994fcbf9..ce08b3bc 100644 --- a/tests/stub_api.py +++ b/tests/stub_api.py @@ -21,6 +21,7 @@ SOURCE_API_URL in .dev.vars. """ +import hashlib import json import os from http.server import BaseHTTPRequestHandler, HTTPServer @@ -112,9 +113,55 @@ def _fixture(name): ) +# ── API keys ─────────────────────────────────────────────────────── +# Opaque keys the proxy resolves by SHA-256 at POST +# /api/v1/service-account-keys/exchanges (ADR-013), as itself. The stub keys +# its answers on the hash of each constant, so test_api_keys.py presents the +# key and never the hash — exactly what the proxy is meant to send. A counter +# per hash lets the tests prove the proxy's 60s standing cache is doing its +# job: the second exchange of a key must not reach here. +LIVE_KEY = "sck_" + "L" * 43 +REVOKED_KEY = "sck_" + "R" * 43 +UNKNOWN_KEY = "sck_" + "U" * 43 +ERR_500_KEY = "sck_" + "E" * 43 +KEY_ACCOUNT = "ci-tests--nightly-sync" + + +def _hash(key): + return hashlib.sha256(key.encode()).hexdigest() + + +KEY_STANDINGS = { + _hash(LIVE_KEY): (200, {"account_id": KEY_ACCOUNT, "key_id": "k-live", "active": True}), + _hash(REVOKED_KEY): (200, {"active": False}), + _hash(ERR_500_KEY): (500, {}), +} +KEY_EXCHANGE_COUNTS = {} + + class Handler(BaseHTTPRequestHandler): + def do_POST(self): + path = self.path.split("?")[0] + if path != "/api/v1/service-account-keys/exchanges": + return self._send(404, b"{}") + # The proxy authenticates as itself; the stub cannot verify the + # signature, but a missing header would mean the proxy sent nothing. + if not self.headers.get("Authorization", "").startswith("Bearer "): + return self._send(401, b"{}") + length = int(self.headers.get("content-length") or 0) + try: + key_hash = json.loads(self.rfile.read(length))["key_hash"] + except (ValueError, KeyError, TypeError): + return self._send(400, b"{}") + KEY_EXCHANGE_COUNTS[key_hash] = KEY_EXCHANGE_COUNTS.get(key_hash, 0) + 1 + status, body = KEY_STANDINGS.get(key_hash, (200, {"active": False})) + self._send(status, json.dumps(body).encode()) + def do_GET(self): path = self.path.split("?")[0] + # Test-only: how many times each key's standing was asked for. + if path == "/_stub/key-exchange-counts": + return self._send(200, json.dumps(KEY_EXCHANGE_COUNTS).encode()) if path == f"/api/v1/products/{WRITE_ACCOUNT}/{ERR_500_PRODUCT}": return self._send(500, b"{}") if path == f"/api/v1/products/{WRITE_ACCOUNT}/{ERR_BAD_JSON_PRODUCT}": diff --git a/tests/test_api_keys.py b/tests/test_api_keys.py new file mode 100644 index 00000000..05170499 --- /dev/null +++ b/tests/test_api_keys.py @@ -0,0 +1,133 @@ +"""API-key exchange at /.sts (ADR-013), against the stub Source API. + +A key is opaque: the proxy hashes it and asks the stub for its standing, as +itself, then mints credentials for the account the stub names. These tests +pin the parts that only run in the worker — the form-body-only rule, the +uniform refusal, the 60s standing cache, and fail-closed on an API error — +by counting how often each key's standing reaches the stub. +""" + +import re +import xml.etree.ElementTree as ET + +import requests + +from stub_api import ERR_500_KEY, KEY_ACCOUNT, LIVE_KEY, REVOKED_KEY, UNKNOWN_KEY, _hash + +PROXY_URL = "http://localhost:8787" +STUB_URL = "http://localhost:9000" + +# The worker takes its request id from `cf-ray`, which Cloudflare sets on +# every real request and `wrangler dev` does not; the tests supply one. +RAY = "ci-ray-0001" +REQUEST_ID = re.compile(r"\(request id [^)]+\)") + + +def exchange(key, *, in_query=False, role="arn:aws:iam::000000000000:role/_default"): + params = {"Action": "AssumeRoleWithWebIdentity", "RoleArn": role, "WebIdentityToken": key} + headers = {"cf-ray": RAY} + if in_query: + return requests.post(f"{PROXY_URL}/.sts", params=params, headers=headers) + return requests.post(f"{PROXY_URL}/.sts", data=params, headers=headers) + + +def lookups(key): + return requests.get(f"{STUB_URL}/_stub/key-exchange-counts").json().get(_hash(key), 0) + + +def sts_fields(resp): + return {el.tag.rpartition("}")[2]: el.text for el in ET.fromstring(resp.text).iter()} + + +def test_a_live_key_is_exchanged_for_credentials_of_its_account(): + resp = exchange(LIVE_KEY) + assert resp.status_code == 200, resp.text[:300] + fields = sts_fields(resp) + assert fields["AccessKeyId"].startswith("STSPRXY") + assert fields["SessionToken"] + assert fields["Expiration"] + # The account lives inside the sealed session token; the response names + # only the role. That the token was sealed for KEY_ACCOUNT is pinned by + # tests/keys.rs, which can unseal it. + assert fields["AssumedRoleId"] == "_default" + + +def test_a_stock_sdk_acquires_credentials_from_a_token_file_holding_the_key(tmp_path, monkeypatch): + """The point of the design: an unmodified SDK, configured by environment + alone, with the key saved to a file the way a user saves it — trailing + newline and all.""" + import boto3 + + token_file = tmp_path / "key" + token_file.write_text(LIVE_KEY + "\n") + for var in ("AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY", "AWS_SESSION_TOKEN", "AWS_PROFILE"): + monkeypatch.delenv(var, raising=False) + monkeypatch.setenv("AWS_CONFIG_FILE", str(tmp_path / "config")) + monkeypatch.setenv("AWS_SHARED_CREDENTIALS_FILE", str(tmp_path / "credentials")) + monkeypatch.setenv("AWS_ROLE_ARN", "arn:aws:iam::000000000000:role/_default") + monkeypatch.setenv("AWS_WEB_IDENTITY_TOKEN_FILE", str(token_file)) + monkeypatch.setenv("AWS_ENDPOINT_URL_STS", f"{PROXY_URL}/.sts") + monkeypatch.setenv("AWS_REGION", "us-east-1") + + creds = boto3.Session().get_credentials().get_frozen_credentials() + assert creds.access_key.startswith("STSPRXY") + assert creds.token + + +def test_the_standing_is_cached_so_a_second_exchange_never_reaches_the_api(): + exchange(LIVE_KEY) + before = lookups(LIVE_KEY) + assert exchange(LIVE_KEY).status_code == 200 + assert lookups(LIVE_KEY) == before, "second exchange within the TTL asked the API again" + + +def test_a_trailing_newline_in_the_token_file_is_harmless(): + assert exchange(LIVE_KEY + "\n").status_code == 200 + + +def test_unknown_and_revoked_keys_are_refused_alike_with_a_request_id(): + answers = {name: exchange(key) for name, key in [("unknown", UNKNOWN_KEY), ("revoked", REVOKED_KEY)]} + for name, resp in answers.items(): + assert resp.status_code == 400, name + fields = sts_fields(resp) + assert fields["Code"] == "InvalidIdentityToken", name + assert fields["Message"] == f"API key was not accepted (request id {RAY})", name + assert resp.headers.get("x-amzn-requestid") == RAY, name + # Nothing in the body says which it was. + assert REQUEST_ID.sub("", answers["unknown"].text) == REQUEST_ID.sub("", answers["revoked"].text) + # Refusals are cached too: an unknown key costs one lookup a minute. + before = lookups(UNKNOWN_KEY) + exchange(UNKNOWN_KEY) + assert lookups(UNKNOWN_KEY) == before + + +def test_a_key_in_the_query_string_is_refused_before_any_lookup(): + before = lookups(LIVE_KEY) + resp = exchange(LIVE_KEY, in_query=True) + assert resp.status_code == 400 + assert "request body" in sts_fields(resp)["Message"] + assert lookups(LIVE_KEY) == before + + +def test_a_malformed_key_is_refused_locally(): + before = sum(requests.get(f"{STUB_URL}/_stub/key-exchange-counts").json().values()) + for bad in ["sck_tooshort", "sck_" + "x" * 44, "SCK_" + "L" * 43]: + resp = exchange(bad) + assert resp.status_code == 400, bad + assert sts_fields(resp)["Code"] == "InvalidIdentityToken", bad + assert sum(requests.get(f"{STUB_URL}/_stub/key-exchange-counts").json().values()) == before + + +def test_a_wrong_role_is_reported_as_such(): + resp = exchange(LIVE_KEY, role="arn:aws:iam::000000000000:role/nope") + assert resp.status_code == 400 + assert sts_fields(resp)["Code"] == "MalformedPolicyDocument" + + +def test_an_api_failure_fails_closed_and_is_not_cached(): + first = exchange(ERR_500_KEY) + assert first.status_code == 500 + assert sts_fields(first)["Code"] == "InternalError" + before = lookups(ERR_500_KEY) + exchange(ERR_500_KEY) + assert lookups(ERR_500_KEY) == before + 1, "a failed lookup was cached" diff --git a/wrangler.preview.toml b/wrangler.preview.toml index 0f13699f..323e9f6f 100644 --- a/wrangler.preview.toml +++ b/wrangler.preview.toml @@ -47,3 +47,10 @@ dataset = "source_data_proxy_staging" [[services]] binding = "PUBLIC_LOG_STREAM" service = "public-log-stream-staging" + +# API-key exchange rate limit, per client IP; see wrangler.toml. +[[unsafe.bindings]] +name = "KEY_EXCHANGE_LIMIT" +type = "ratelimit" +namespace_id = "1003" +simple = { limit = 100, period = 60 } diff --git a/wrangler.toml b/wrangler.toml index 14b970db..6b10f65d 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -67,6 +67,16 @@ dataset = "source_data_proxy_production" binding = "PUBLIC_LOG_STREAM" service = "public-log-stream" +# API-key exchanges at /.sts, per client IP. Every attempt costs the Source +# API one lookup for a distinct key, so this bounds a flood of junk keys from +# one place; a legitimate client exchanges about once a session, so even a +# cluster behind one NAT stays far under it. See src/lib.rs `api_key_exchange`. +[[unsafe.bindings]] +name = "KEY_EXCHANGE_LIMIT" +type = "ratelimit" +namespace_id = "1001" +simple = { limit = 100, period = 60 } + [env.staging] routes = [ {pattern = "data.staging.coolnewgeo.com/*", zone_name = "coolnewgeo.com"}, @@ -90,6 +100,12 @@ dataset = "source_data_proxy_staging" binding = "PUBLIC_LOG_STREAM" service = "public-log-stream-staging" +[[env.staging.unsafe.bindings]] +name = "KEY_EXCHANGE_LIMIT" +type = "ratelimit" +namespace_id = "1002" +simple = { limit = 100, period = 60 } + [env.staging.observability] enabled = true head_sampling_rate = 1 From bb7525bc2483a8221437078517318ec1d3181473 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Fri, 25 Sep 2026 14:03:24 -0700 Subject: [PATCH 2/7] feat(sts): serve the FullAccess and ReadOnly roles `/.sts` now resolves `RoleArn` to one of three hardcoded Roles, for ID tokens and API keys alike: `FullAccess`, the unlimited Role that `_default` has been; `ReadOnly`, whose sealed ceiling allows only reads; and `_default`, kept as an alias of `FullAccess` because deployed clients name it. Each is accepted bare or as the `role/` resource of an ARN of any partition and account, since SDKs insist on an ARN. Any other name is `RoleNotFound`, never a fallback to a default. ReadOnly's ceiling rides in the session token's `allowed_scopes` as one scope over every product (`*`) that lists the read actions. multistore's own scope check never runs on this gateway, so the registry enforces it: `get_bucket` checks the ceiling before anything is fetched and refuses with the same `AccessDenied` as every other refusal (ADR-011). The ceiling only subtracts, so a write it allows still needs the account's write permission. No scopes means no ceiling, which keeps every `_default` session already issued working unchanged. source.coop's GitHub integration snippet already names `role/FullAccess`; this is what makes that name resolve. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd --- README.md | 12 ++++ src/authz.rs | 28 ++++++++-- src/lib.rs | 16 +++--- src/source_api/registry.rs | 19 ++++++- src/sts.rs | 111 ++++++++++++++++++++++--------------- tests/authz.rs | 78 ++++++++++++++++++++++++-- tests/keys.rs | 32 +++++++++-- tests/sts.rs | 84 ++++++++++++++++++---------- tests/test_api_keys.py | 50 ++++++++++++++++- 9 files changed, 327 insertions(+), 103 deletions(-) diff --git a/README.md b/README.md index 381912bf..3a81ae8e 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,18 @@ Set in `wrangler.toml` or via the Cloudflare dashboard: A service account's API key (ADR-013) is an opaque `sck_` secret that source.coop stores as a hash. It is presented at `/.sts` as `WebIdentityToken`, from a POST form body only — a key in the URL is refused, because the URL is logged. The proxy trims and format-checks it, hashes it, and asks `POST {SOURCE_API_URL}/api/v1/service-account-keys/exchanges` for its standing as itself (subject `urn:source:data-proxy`), caching the answer for 60 seconds; then it mints credentials for the account the API names, exactly as it would for an ID token. Every refusal of the key reads `API key was not accepted (request id …)`; the reason is in the log under that id. +### Roles + +Every exchange at `/.sts`, of an ID token or an API key, names a Role in `RoleArn`, either bare or as the resource of an ARN of any partition and account (`arn:aws:iam::000000000000:role/ReadOnly`), since AWS SDKs insist on an ARN. The Roles are hardcoded (ADR-014): + +| Role | Credentials may | +| ------------ | -------------------------------------------------------- | +| `FullAccess` | do everything the account's memberships allow | +| `ReadOnly` | do the same, except write | +| `_default` | do what `FullAccess` does; the name existing clients use | + +Any other name is refused with `MalformedPolicyDocument`, never mapped to a default. A Role only subtracts: its ceiling is sealed into the session token and checked locally before the account's own permissions are looked up (ADR-011), and a request it refuses gets the same `AccessDenied` as any other refusal. + ### Secrets **GitHub environment secrets are the source of truth.** The deploy workflow diff --git a/src/authz.rs b/src/authz.rs index 54cef0e0..b812c7a1 100644 --- a/src/authz.rs +++ b/src/authz.rs @@ -1,14 +1,15 @@ -//! Authorization for product backends: write-action classification and the -//! authorization → federation decision ([`decide_backend_auth`]). Kept wasm-free -//! so both can be unit-tested natively (see `tests/authz.rs`), despite the -//! crate's `[lib] test = false`. +//! Authorization for product backends: write-action classification, the Role +//! ceiling ([`ceiling_permits`]) and the authorization → federation decision +//! ([`decide_backend_auth`]). Kept wasm-free so all three can be unit-tested +//! natively (see `tests/authz.rs`), despite the crate's `[lib] test = false`. use std::collections::HashMap; use multistore::error::ProxyError; -use multistore::types::Action; +use multistore::types::{AccessScope, Action}; use crate::backend_auth::{apply_backend_auth, BackendAuth}; +use crate::sts::ALL_PRODUCTS; /// Whether an S3 action mutates the backend. Reads (GET/HEAD/LIST) are served /// without a write check; everything else is a write and must be authorized. @@ -24,6 +25,23 @@ pub(crate) fn is_write_action(action: Action) -> bool { ) } +/// Whether the Role ceiling sealed into a session (ADR-011) allows `action`. +/// Checked before anything is fetched, and it only subtracts: what it allows +/// still needs the account's own permissions. +/// +/// No scopes means no ceiling: `FullAccess` and `_default` seal none. Otherwise +/// a scope must name every product ([`ALL_PRODUCTS`]) with no prefix and list +/// the action. The proxy seals nothing narrower, so a narrower scope is refused +/// rather than guessed at. +pub(crate) fn ceiling_permits(scopes: &[AccessScope], action: Action) -> bool { + scopes.is_empty() + || scopes.iter().any(|scope| { + scope.bucket == ALL_PRODUCTS + && scope.prefixes.is_empty() + && scope.actions.contains(&action) + }) +} + /// Authorize a resolved product's request and, only on success, translate the /// connection's backend authentication into multistore `backend_options`. This /// is the single authorization → federation seam: `resolve_product` performs the diff --git a/src/lib.rs b/src/lib.rs index d3199bb0..948a012f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -574,9 +574,14 @@ async fn exchange_api_key( tracing::warn!(%request_id, reason = "malformed", "API key exchange refused"); return Err(ProxyError::InvalidOidcToken("malformed".into())); }; - if !sts::is_default_role(&sts.role_arn) { + let Some(role) = sts::role( + &sts.role_arn, + config.auth_issuer.clone(), + config.auth_audiences.clone(), + config.sts_max_session_duration_secs, + ) else { return Err(ProxyError::RoleNotFound(sts.role_arn.clone())); - } + }; let key_hash = keys::key_hash(key); let standing = source_api::cache::get_or_fetch_key_standing( &config.api_base_url, @@ -602,18 +607,13 @@ async fn exchange_api_key( return Err(ProxyError::InvalidOidcToken("inactive".into())); } }; - let role = sts::default_role( - config.auth_issuer.clone(), - config.auth_audiences.clone(), - config.sts_max_session_duration_secs, - ); let creds = keys::credentials_for( &role, &account_id, sts.duration_seconds, &config.session_token_key, )?; - tracing::info!(%request_id, key_id, %account_id, "API key exchanged"); + tracing::info!(%request_id, key_id, %account_id, role = %role.role_id, "API key exchanged"); Ok(creds) } diff --git a/src/source_api/registry.rs b/src/source_api/registry.rs index 063d2d17..f7e93dca 100644 --- a/src/source_api/registry.rs +++ b/src/source_api/registry.rs @@ -5,7 +5,7 @@ use multistore::error::ProxyError; use multistore::registry::{BucketRegistry, ResolvedBucket}; use multistore::types::{Action, BucketConfig, ResolvedIdentity, S3Operation}; -use crate::authz::{decide_backend_auth, is_write_action}; +use crate::authz::{ceiling_permits, decide_backend_auth, is_write_action}; /// Registry that resolves Source Cooperative products to multistore `BucketConfig`s /// by calling the Source Cooperative API. @@ -56,7 +56,22 @@ impl BucketRegistry for SourceCoopRegistry { .ok_or_else(|| ProxyError::BucketNotFound(name.to_string()))?; let subject = match identity { - ResolvedIdentity::Authenticated(auth) => Some(auth.principal_name.as_str()), + ResolvedIdentity::Authenticated(auth) => { + // The Role ceiling goes first and is local (ADR-011): a session + // whose Role does not allow the action is refused before + // anything is fetched, with the AccessDenied every other + // refusal gets, so the answer says nothing about the product. + // Only this log line records why. + if !ceiling_permits(&auth.allowed_scopes, operation.action()) { + tracing::info!( + principal = %auth.principal_name, + action = ?operation.action(), + "refused by the Role ceiling" + ); + return Err(ProxyError::AccessDenied); + } + Some(auth.principal_name.as_str()) + } ResolvedIdentity::Anonymous => None, }; diff --git a/src/sts.rs b/src/sts.rs index 4290aa9d..58e895b3 100644 --- a/src/sts.rs +++ b/src/sts.rs @@ -1,24 +1,30 @@ //! STS credential registry for token exchange. //! -//! Provides a hardcoded `_default` role that trusts the Source Cooperative auth -//! provider, enabling clients to exchange OIDC tokens for temporary S3-style credentials. +//! Serves the hardcoded Roles (ADR-014): `FullAccess`, everything the caller's +//! memberships allow, and `ReadOnly`, the same with writing removed. `_default` +//! is `FullAccess` under the name existing clients already use. There is no +//! lookup: account-owned Roles (ADR-010) are deferred, and a Role only ever +//! subtracts from the account's own permissions, so any caller may name either. use multistore::error::ProxyError; use multistore::registry::CredentialRegistry; -use multistore::types::{RoleConfig, StoredCredential}; +use multistore::types::{AccessScope, Action, RoleConfig, StoredCredential}; -/// Credential registry that serves a single hardcoded `_default` role. -/// -/// The default role trusts the Source Cooperative auth provider with no scope -/// restrictions, so any user holding a token for one of the configured -/// audiences (`required_audiences`) can obtain temporary credentials. +/// The bucket a Role's scope names to cover every product. Only the proxy's +/// registry reads scopes — multistore's own scope check never runs on this +/// gateway — so the wildcard means what `authz::ceiling_permits` says it does. +pub(crate) const ALL_PRODUCTS: &str = "*"; + +/// Credential registry that serves the hardcoded Roles. #[derive(Clone)] pub struct StsCredentialRegistry { - default_role: RoleConfig, + oidc_issuer: String, + required_audiences: Vec, + max_session_duration_secs: u64, } impl StsCredentialRegistry { - /// Create a new registry whose `_default` role trusts the given auth issuer. + /// Create a new registry whose Roles trust the given auth issuer. /// /// `required_audiences` restricts token exchange to subject tokens minted /// for one of these OAuth clients (the `aud` claim); a token is accepted if @@ -36,29 +42,62 @@ impl StsCredentialRegistry { max_session_duration_secs: u64, ) -> Self { Self { - default_role: default_role(oidc_issuer, required_audiences, max_session_duration_secs), + oidc_issuer, + required_audiences, + max_session_duration_secs, } } } -/// The `_default` role: trusts `oidc_issuer` for tokens minted for one of -/// `required_audiences`, with no scope restriction. Shared with the API-key -/// exchange, which mints under the same role once the API has named the -/// account (`keys::credentials_for`). -pub(crate) fn default_role( +/// The Role `role_arn` names, trusting `oidc_issuer` for tokens minted for one +/// of `required_audiences`; `None` for a name the proxy does not serve. Never a +/// fallback: a workload that asks for a Role it cannot have fails at exchange +/// rather than receiving different access than it asked for. Shared with the +/// API-key exchange, which mints under the named Role once the API has named +/// the account (`keys::credentials_for`). +pub(crate) fn role( + role_arn: &str, oidc_issuer: String, required_audiences: Vec, max_session_duration_secs: u64, -) -> RoleConfig { - RoleConfig { - role_id: "_default".to_string(), - name: "Default".to_string(), +) -> Option { + let name = role_name(role_arn)?; + let allowed_scopes = match name { + // No scopes, no ceiling: the account's permissions are the only limit. + "FullAccess" | "_default" => vec![], + // Sealed into the session; `authz::ceiling_permits` enforces it. + "ReadOnly" => vec![AccessScope { + bucket: ALL_PRODUCTS.to_string(), + prefixes: vec![], + actions: vec![Action::GetObject, Action::HeadObject, Action::ListBucket], + }], + _ => return None, + }; + Some(RoleConfig { + role_id: name.to_string(), + name: name.to_string(), trusted_oidc_issuers: vec![oidc_issuer], required_audiences, subject_conditions: vec![], - allowed_scopes: vec![], // unlimited + allowed_scopes, max_session_duration_secs, + }) +} + +/// The Role name in `role_arn`: a bare name, or the `role/` resource of +/// an ARN of any partition and account, such as +/// `arn:aws:iam::000000000000:role/ReadOnly`. +/// +/// The ARN form exists because AWS SDKs validate `RoleArn` client-side (ARN +/// shape, 20-character minimum) before the request is ever sent, so a bare name +/// can't reach the server from standard tooling (see +/// source-cooperative/data.source.coop#184). The partition and account carry no +/// meaning for the Role itself, so they are ignored rather than validated. +fn role_name(role_arn: &str) -> Option<&str> { + if !role_arn.starts_with("arn:") { + return Some(role_arn); } + role_arn.splitn(6, ':').nth(5)?.strip_prefix("role/") } impl CredentialRegistry for StsCredentialRegistry { @@ -71,29 +110,11 @@ impl CredentialRegistry for StsCredentialRegistry { } async fn get_role(&self, role_id: &str) -> Result, ProxyError> { - // TODO: Eventually look up roles via the Source Cooperative API so that - // individual repositories can define custom roles with fine-grained - // scope and subject restrictions (e.g. per-repo CI/CD access). - // For now, only the hardcoded `_default` role is supported. - if is_default_role(role_id) { - Ok(Some(self.default_role.clone())) - } else { - Ok(None) - } + Ok(role( + role_id, + self.oidc_issuer.clone(), + self.required_audiences.clone(), + self.max_session_duration_secs, + )) } } - -/// Whether `role_id` names the `_default` role — literally, or via an -/// ARN-shaped alias whose resource is `role/_default` (any partition/account, -/// e.g. `arn:aws:iam::000000000000:role/_default`). -/// -/// The alias exists because AWS SDKs validate `RoleArn` client-side (ARN shape, -/// 20-character minimum) before the request is ever sent, so a bare `_default` -/// can't reach the server from standard tooling. Accepting the alias keeps -/// `/.sts` a drop-in `AssumeRoleWithWebIdentity` target for unmodified SDKs -/// (see source-cooperative/data.source.coop#184). Same role, same trust model — -/// only the name is longer; the partition/account portion is ignored rather -/// than validated because it carries no meaning here. -pub(crate) fn is_default_role(role_id: &str) -> bool { - role_id == "_default" || (role_id.starts_with("arn:") && role_id.ends_with(":role/_default")) -} diff --git a/tests/authz.rs b/tests/authz.rs index 9f9e2df1..1aaaae28 100644 --- a/tests/authz.rs +++ b/tests/authz.rs @@ -2,19 +2,21 @@ //! (the lib itself is `cdylib` with `test = false`). Mirrors the pattern in //! `tests/backend_auth.rs`. //! -//! `authz` references `crate::backend_auth`, so that module is pulled in here too -//! (under the test crate root) so the `crate::` path resolves the same way it -//! does in the lib build. +//! `authz` references `crate::backend_auth` and `crate::sts`, so those modules +//! are pulled in here too (under the test crate root) so the `crate::` paths +//! resolve the same way they do in the lib build. #[path = "../src/authz.rs"] mod authz; #[path = "../src/backend_auth.rs"] mod backend_auth; +#[path = "../src/sts.rs"] +mod sts; -use authz::{decide_backend_auth, is_write_action}; +use authz::{ceiling_permits, decide_backend_auth, is_write_action}; use backend_auth::BackendAuth; use multistore::error::ProxyError; -use multistore::types::Action; +use multistore::types::{AccessScope, Action}; use std::collections::HashMap; #[test] @@ -38,6 +40,72 @@ fn mutations_are_writes() { } } +// ── ceiling_permits: the Role ceiling (ADR-011) ───────────────────────────── + +const EVERY_ACTION: [Action; 10] = [ + Action::GetObject, + Action::GetObjectVersion, + Action::HeadObject, + Action::PutObject, + Action::ListBucket, + Action::CreateMultipartUpload, + Action::UploadPart, + Action::CompleteMultipartUpload, + Action::AbortMultipartUpload, + Action::DeleteObject, +]; + +/// The scopes a Role seals into every session it mints. +fn sealed(role: &str) -> Vec { + sts::role(role, "https://auth.example.test".into(), vec![], 3600) + .unwrap() + .allowed_scopes +} + +/// The ceiling and the write gate must agree on what a read is, or ReadOnly +/// either refuses a read or lets a write through. +#[test] +fn read_only_allows_exactly_the_reads() { + let read_only = sealed("ReadOnly"); + for action in EVERY_ACTION { + assert_eq!( + ceiling_permits(&read_only, action), + !is_write_action(action), + "{action:?}" + ); + } +} + +#[test] +fn full_access_and_its_alias_have_no_ceiling() { + for role in ["FullAccess", "_default"] { + let scopes = sealed(role); + for action in EVERY_ACTION { + assert!(ceiling_permits(&scopes, action), "{role} {action:?}"); + } + } +} + +/// Nothing the proxy mints is narrower than every product, so a scope that is +/// must not be read as if it were. +#[test] +fn a_scope_narrower_than_every_product_permits_nothing() { + for scope in [ + AccessScope { + bucket: "acme:data".into(), + prefixes: vec![], + actions: vec![Action::GetObject], + }, + AccessScope { + bucket: "*".into(), + prefixes: vec!["public/".into()], + actions: vec![Action::GetObject], + }, + ] { + assert!(!ceiling_permits(&[scope], Action::GetObject)); + } +} + // ── decide_backend_auth: authorization → federation ordering (#142) ───────── // // The invariant under test: an unauthorized request must be denied *before* any diff --git a/tests/keys.rs b/tests/keys.rs index 7543dd88..21df6af5 100644 --- a/tests/keys.rs +++ b/tests/keys.rs @@ -74,14 +74,21 @@ fn token_key() -> TokenKey { TokenKey::from_base64("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=").unwrap() } -fn role(cap: u64) -> multistore::types::RoleConfig { - sts::default_role("https://auth.example.test".into(), vec!["aud".into()], cap) +fn role(name: &str, cap: u64) -> multistore::types::RoleConfig { + sts::role( + name, + "https://auth.example.test".into(), + vec!["aud".into()], + cap, + ) + .unwrap() } #[test] fn credentials_are_sealed_for_the_account_within_floor_and_cap() { let key = token_key(); - let creds = credentials_for(&role(43_200), "acme--nightly-sync", None, &key).unwrap(); + let creds = + credentials_for(&role("_default", 43_200), "acme--nightly-sync", None, &key).unwrap(); assert_eq!(creds.source_identity, "acme--nightly-sync"); assert_eq!(creds.assumed_role_id, "_default"); assert!(creds.access_key_id.starts_with("STSPRXY")); @@ -90,14 +97,27 @@ fn credentials_are_sealed_for_the_account_within_floor_and_cap() { assert_eq!(unsealed.source_identity, "acme--nightly-sync"); let now = chrono_now(); - let default = credentials_for(&role(43_200), "a", None, &key).unwrap(); + let default = credentials_for(&role("_default", 43_200), "a", None, &key).unwrap(); assert!((default.expiration.timestamp() - now - 3600).abs() <= 2); - let floored = credentials_for(&role(43_200), "a", Some(1), &key).unwrap(); + let floored = credentials_for(&role("_default", 43_200), "a", Some(1), &key).unwrap(); assert!((floored.expiration.timestamp() - now - 900).abs() <= 2); - let capped = credentials_for(&role(3_600), "a", Some(86_400), &key).unwrap(); + let capped = credentials_for(&role("_default", 3_600), "a", Some(86_400), &key).unwrap(); assert!((capped.expiration.timestamp() - now - 3600).abs() <= 2); } +#[test] +fn credentials_carry_the_named_roles_ceiling() { + let key = token_key(); + let read_only = role("ReadOnly", 3_600); + let creds = credentials_for(&read_only, "acme--nightly-sync", None, &key).unwrap(); + let unsealed = key.unseal(&creds.session_token).unwrap().unwrap(); + assert_eq!(unsealed.assumed_role_id, "ReadOnly"); + assert_eq!( + serde_json::to_value(&unsealed.allowed_scopes).unwrap(), + serde_json::to_value(&read_only.allowed_scopes).unwrap() + ); +} + fn chrono_now() -> i64 { std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) diff --git a/tests/sts.rs b/tests/sts.rs index 182fa7ad..7077edaa 100644 --- a/tests/sts.rs +++ b/tests/sts.rs @@ -1,44 +1,68 @@ -//! Native unit tests for the `_default` role alias in `sts`, included via -//! `#[path]` (the lib itself is `cdylib` with `test = false`). Mirrors the -//! pattern in `tests/backend_auth.rs`. +//! Native unit tests for Role lookup in `sts`, included via `#[path]` (the lib +//! itself is `cdylib` with `test = false`). Mirrors the pattern in +//! `tests/backend_auth.rs`. What each Role's ceiling allows is pinned in +//! `tests/authz.rs`, next to the check that enforces it. #[path = "../src/sts.rs"] mod sts; -use sts::is_default_role; - -#[test] -fn literal_default_accepted() { - assert!(is_default_role("_default")); +/// The Role `role_arn` resolves to, by id. +fn named(role_arn: &str) -> Option { + sts::role( + role_arn, + "https://auth.example.test".into(), + vec!["aud".into()], + 3600, + ) + .map(|role| role.role_id) } #[test] -fn arn_alias_accepted_for_any_partition_and_account() { - assert!(is_default_role("arn:aws:iam::000000000000:role/_default")); - assert!(is_default_role("arn:aws:iam::123456789012:role/_default")); - assert!(is_default_role( - "arn:aws-us-gov:iam::123456789012:role/_default" - )); +fn each_role_is_served_by_its_bare_name() { + for name in ["FullAccess", "ReadOnly", "_default"] { + assert_eq!(named(name).as_deref(), Some(name)); + } } #[test] -fn other_roles_rejected() { - assert!(!is_default_role("")); - assert!(!is_default_role("_other")); - assert!(!is_default_role("default")); - assert!(!is_default_role("arn:aws:iam::123456789012:role/other")); +fn arn_forms_are_accepted_for_any_partition_and_account() { + for arn in [ + "arn:aws:iam::000000000000:role/ReadOnly", + "arn:aws:iam::123456789012:role/ReadOnly", + "arn:aws-us-gov:iam::123456789012:role/ReadOnly", + // A service account's id, as source.coop's GitHub snippet names it. + "arn:aws:iam::acme--nightly-sync:role/ReadOnly", + ] { + assert_eq!(named(arn).as_deref(), Some("ReadOnly"), "{arn}"); + } + assert_eq!( + named("arn:aws:iam::acme--nightly-sync:role/FullAccess").as_deref(), + Some("FullAccess") + ); + // Deployed client configuration uses this one. + assert_eq!( + named("arn:aws:iam::000000000000:role/_default").as_deref(), + Some("_default") + ); } #[test] -fn alias_requires_arn_prefix_and_exact_resource() { - // No arn: prefix. - assert!(!is_default_role("role/_default")); - // Pathed resource is not the `_default` role. - assert!(!is_default_role( - "arn:aws:iam::123456789012:role/team/_default" - )); - // `_default` as a suffix of another role name. - assert!(!is_default_role( - "arn:aws:iam::123456789012:role/not_default" - )); +fn unknown_names_are_refused_not_defaulted() { + for arn in [ + "", + "default", + "readonly", + "Admin", + "arn:aws:iam::123456789012:role/other", + // No arn: prefix. + "role/_default", + // A pathed resource is not the Role. + "arn:aws:iam::123456789012:role/team/_default", + // A Role's name as the suffix of another. + "arn:aws:iam::123456789012:role/not_default", + // Not a role resource. + "arn:aws:iam::123456789012:user/ReadOnly", + ] { + assert_eq!(named(arn), None, "{arn}"); + } } diff --git a/tests/test_api_keys.py b/tests/test_api_keys.py index 05170499..93daf167 100644 --- a/tests/test_api_keys.py +++ b/tests/test_api_keys.py @@ -4,15 +4,26 @@ itself, then mints credentials for the account the stub names. These tests pin the parts that only run in the worker — the form-body-only rule, the uniform refusal, the 60s standing cache, and fail-closed on an API error — -by counting how often each key's standing reaches the stub. +by counting how often each key's standing reaches the stub, and the ceiling of +the Role a key is exchanged for. """ import re +import uuid import xml.etree.ElementTree as ET +import pytest import requests -from stub_api import ERR_500_KEY, KEY_ACCOUNT, LIVE_KEY, REVOKED_KEY, UNKNOWN_KEY, _hash +from stub_api import ( + ERR_500_KEY, + KEY_ACCOUNT, + LIVE_KEY, + REVOKED_KEY, + UNKNOWN_KEY, + WRITE_ACCOUNT, + _hash, +) PROXY_URL = "http://localhost:8787" STUB_URL = "http://localhost:9000" @@ -124,6 +135,41 @@ def test_a_wrong_role_is_reported_as_such(): assert sts_fields(resp)["Code"] == "MalformedPolicyDocument" +def test_read_only_refuses_a_write_before_anything_is_looked_up(): + """ReadOnly's ceiling is checked locally, ahead of every lookup, so it + refuses a write with AccessDenied even for a product the API has never + heard of. FullAccess, and ReadOnly reading, get past it to the product + lookup, which finds no such product. (A write that reached the upstream + would fail closed in CI anyway, so the lookup is where to tell them apart.)""" + import boto3 + from botocore.config import Config + from botocore.exceptions import ClientError + + def client(role): + fields = sts_fields(exchange(LIVE_KEY, role=f"arn:aws:iam::000000000000:role/{role}")) + return boto3.client( + "s3", + endpoint_url=PROXY_URL, + aws_access_key_id=fields["AccessKeyId"], + aws_secret_access_key=fields["SecretAccessKey"], + aws_session_token=fields["SessionToken"], + region_name="us-east-1", + config=Config(s3={"addressing_style": "path"}), + ) + + def error_code(call): + with pytest.raises(ClientError) as exc: + call() + return exc.value.response["Error"]["Code"] + + key = f"no-such-product-{uuid.uuid4().hex}/x.txt" + read_only, full_access = client("ReadOnly"), client("FullAccess") + put = {"Bucket": WRITE_ACCOUNT, "Key": key, "Body": b"x"} + assert error_code(lambda: read_only.put_object(**put)) == "AccessDenied" + assert error_code(lambda: full_access.put_object(**put)) == "NoSuchBucket" + assert error_code(lambda: read_only.get_object(Bucket=WRITE_ACCOUNT, Key=key)) == "NoSuchBucket" + + def test_an_api_failure_fails_closed_and_is_not_cached(): first = exchange(ERR_500_KEY) assert first.status_code == 500 From 1b6a91ec4ce82dc7b0956a852ec846b3a9dc04ce Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Fri, 25 Sep 2026 14:15:52 -0700 Subject: [PATCH 3/7] docs(adrs): record the FullAccess and ReadOnly roles ADR-004 described a single built-in `_default` Role, and ADR-001 said the sealed `allowed_scopes` was always empty and consulted nowhere, with empty meaning deny-all wherever scopes are evaluated. Both are dated by the hardcoded `FullAccess` and `ReadOnly` Roles: ADR-004 gains a note pointing to ADR-014, and ADR-001 now says the bucket registry enforces the ceiling and reads an empty one as no ceiling. ADR-011's status records that its step 2 and denial semantics are implemented for `ReadOnly`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd --- adrs/001-s3-credentials.md | 6 +++--- adrs/004-sts.md | 5 ++++- adrs/011-role-ceiling-authorization.md | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/adrs/001-s3-credentials.md b/adrs/001-s3-credentials.md index 1523ca11..0078274c 100644 --- a/adrs/001-s3-credentials.md +++ b/adrs/001-s3-credentials.md @@ -57,16 +57,16 @@ The sealed payload carries: | `access_key_id` | The identifier the caller signs with | | `secret_access_key` | The signing secret, recovered by unsealing | | `expiration` | Enforced at unseal time; an expired token fails closed | -| `assumed_role_id` | The Role assumed at exchange time (currently always `_default`) | +| `assumed_role_id` | The Role assumed at exchange time: `_default`, `FullAccess` or `ReadOnly` (ADR-004) | | `source_identity` | The original OIDC `sub` — the caller's Ory identity | -| `allowed_scopes` | Scope ceiling sealed at mint time — currently empty, and not consulted on this path (see below) | +| `allowed_scopes` | The Role's ceiling, sealed at mint time: empty for `FullAccess` and `_default`, reads of every product for `ReadOnly` (see below) | | `session_token` | A discarded random placeholder. The credential set is sealed *before* this field is overwritten with the sealed blob, so the value inside the envelope is not the token itself | Key properties of this design: - **Verification is fully stateless.** The proxy decrypts the token on each request and recovers the `SecretAccessKey` directly. No database lookup, no key derivation, and no asymmetric verification on the request hot path — which matters on Workers, where in-memory state does not persist across invocations. - **The token is opaque to the caller.** Unlike a JWT, a client cannot read the sealed payload. Scope and identity metadata are not disclosed to whoever holds the credential. -- **`allowed_scopes` is sealed but not enforced on this path.** Its only consumer, `multistore::auth::authorize`, has no call site in the pinned crate; the gateway delegates authorization to the bucket registry instead (ADR-005). Where scopes *are* evaluated, an empty vec means **deny-all**, not unlimited — which is why the registry overrides `authorize_key` rather than inheriting the default. The effective behaviour is "no ceiling", but by bypass rather than by an empty-means-unlimited rule. ADR-011 is where this field would become load-bearing, and wiring it up is part of that work rather than a given. +- **`allowed_scopes` is enforced by the bucket registry, not by multistore.** multistore's own consumer, `multistore::auth::authorize`, has no call site in the pinned crate; the gateway delegates authorization to the registry instead (ADR-005), which checks the ceiling before any lookup (ADR-011, #236). The registry reads an empty vec as **no ceiling**, the reverse of `authorize`, where empty means deny-all — which is also why the registry overrides `authorize_key` rather than inheriting the default. The only non-empty ceiling is `ReadOnly`'s: every product (`*`), read actions only. - **`source_identity` preserves the original subject**, which is what the proxy presents to the policy store (see ADR-005). - **Authenticated encryption.** GCM provides integrity as well as confidentiality: a tampered token fails to decrypt rather than decoding into attacker-chosen values. diff --git a/adrs/004-sts.md b/adrs/004-sts.md index 4c689a27..6540d8e5 100644 --- a/adrs/004-sts.md +++ b/adrs/004-sts.md @@ -5,7 +5,7 @@ **RFC:** RFC-001 §7 **Depends on:** ADR-001 **Implementation:** `src/sts.rs`, `src/lib.rs`, `src/config.rs`; `source.coop:src/lib/actions/proxy-credentials.ts` -**Implemented by:** #116 (initial `/.sts` exchange), #163 (multiple accepted audiences), #165 (configurable max session duration), #185 (ARN-shaped `_default` alias), #196 (form-encoded POST bodies, wiring [multistore#112](https://github.com/developmentseed/multistore/pull/112)) · source.coop#283 (OIDC auth), source.coop#391 (in-browser uploads via the proxy), source.coop#402 (mid-upload credential refresh) +**Implemented by:** #116 (initial `/.sts` exchange), #163 (multiple accepted audiences), #165 (configurable max session duration), #185 (ARN-shaped `_default` alias), #196 (form-encoded POST bodies, wiring [multistore#112](https://github.com/developmentseed/multistore/pull/112)), #236 (`FullAccess` and `ReadOnly` Roles) · source.coop#283 (OIDC auth), source.coop#391 (in-browser uploads via the proxy), source.coop#402 (mid-upload credential refresh) --- @@ -73,6 +73,9 @@ A single built-in Role, `_default`, is served from a hardcoded registry: `RoleArn` is accepted either literally as `_default` or as an ARN-shaped alias whose resource is `role/_default` (e.g. `arn:aws:iam::000000000000:role/_default`, any partition or account ID). The alias exists because AWS SDKs validate `RoleArn` client-side — ARN shape, 20-character minimum — before the request is ever sent, so a bare `_default` cannot reach the server from unmodified tooling. The partition and account portions carry no meaning here and are ignored rather than validated. +> [!NOTE] +> Two more hardcoded Roles are served alongside it (ADR-014, #236): `FullAccess`, of which `_default` is now an alias, and `ReadOnly`, whose read-only ceiling is sealed into the session and enforced by the bucket registry (ADR-011). All three take the same ARN form. + ### Trust Model — Issuer and Audience **Issuer.** `AUTH_ISSUER` names the single trusted OIDC issuer: Source Cooperative's Ory-based auth system (`https://auth.source.coop`, or the staging equivalent). A token from any other issuer is rejected before any network call. diff --git a/adrs/011-role-ceiling-authorization.md b/adrs/011-role-ceiling-authorization.md index 7a021647..1486e4a8 100644 --- a/adrs/011-role-ceiling-authorization.md +++ b/adrs/011-role-ceiling-authorization.md @@ -1,6 +1,6 @@ # ADR-011: Role-Ceiling Authorization -**Status:** Proposed — not implemented +**Status:** Proposed — implemented in part (#236: step 2 and the denial semantics, for the hardcoded `ReadOnly` Role's action ceiling) **Date:** 2026-08-09 **RFC:** RFC-001 §8 **Depends on:** ADR-005, ADR-010 From 4552c466f5d3c7546dd4ebac8f444bd532ba8840 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Fri, 25 Sep 2026 14:29:55 -0700 Subject: [PATCH 4/7] feat(sts): let platform IdP tokens act as accounts that trust them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A token from a platform issuer, GitHub Actions to begin with, now acts at `/.sts` as the account its `RoleArn` names (`arn:aws:iam:::role/FullAccess`), and only if that account trusts the token's issuer and subject (ADR-014). The proxy reads the token's issuer unverified to route it, checks the Role and that `RoleArn` names an account, then verifies the token against the issuer's JWKS with that issuer's own audiences and a required `exp`, since multistore checks `exp` only when the claim is present. Only then does it ask `POST /api/v1/accounts/{account}/trusts/exchanges` with the verified issuer and subject, as the account. A yes is cached for 60 seconds per account, issuer and subject. A no, the route's 403, is not cached, so a trust just added works on the next attempt. The credentials' principal is the account, never the token's subject, and every refusal of the trust reads `AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity (request id …)`. Platform issuers are configured in `PLATFORM_ISSUERS`, a JSON object from issuer to the audiences its tokens must carry, so that one issuer's audience never admits another's token (ADR-009); an issuer with no audience is refused. `AUTH_ISSUER` and `AUTH_AUDIENCE` still configure the person issuer, whose tokens act as their own subject as before. Production trusts GitHub with the proxy's origin as the audience, which is how source.coop's workflow snippet mints the token; staging and previews use staging's origin. CI now configures GitHub as a platform issuer, as production does, and the stub answers the trusts route: yes for this repository's workflows on one account, no otherwise. The credentialed write tests name that account. `aws-actions/configure-aws-credentials` still fails after a successful exchange, on `GetCallerIdentity`, until developmentseed/multistore#126 lands. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd --- .github/workflows/ci.yml | 20 +++-- .github/workflows/staging.yml | 15 ++-- Cargo.lock | 1 + Cargo.toml | 3 + README.md | 11 ++- src/config.rs | 17 ++++- src/lib.rs | 140 +++++++++++++++++++++++++++++++--- src/platform.rs | 89 +++++++++++++++++++++ src/source_api/cache.rs | 56 ++++++++++++++ src/sts.rs | 10 +++ tests/platform.rs | 95 +++++++++++++++++++++++ tests/sts.rs | 19 +++++ tests/stub_api.py | 57 ++++++++++++++ tests/test_federation.py | 20 +++-- tests/test_platform_trust.py | 118 ++++++++++++++++++++++++++++ tests/test_writes.py | 28 ++++--- wrangler.preview.toml | 4 + wrangler.toml | 9 +++ 18 files changed, 660 insertions(+), 52 deletions(-) create mode 100644 src/platform.rs create mode 100644 tests/platform.rs create mode 100644 tests/test_platform_trust.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 61b6f174..90b0265e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,7 +85,7 @@ jobs: permissions: contents: read # Mint a GitHub OIDC token as the write tests' caller identity — the - # worker verifies it against GitHub's JWKS (AUTH_ISSUER below). + # worker verifies it against GitHub's JWKS (PLATFORM_ISSUERS below). id-token: write steps: - uses: actions/checkout@v4 @@ -106,16 +106,20 @@ jobs: # dependency in this job can't mint federation assertions any AWS role # trusts. Federated end-to-end coverage lives in the deployed-environment # smoke tests (tests/test_federation.py, wired into staging.yml) instead. - # Inbound callers authenticate with GitHub Actions OIDC tokens: - # AUTH_ISSUER points at GitHub and AUTH_AUDIENCE matches the audience - # requested in "Mint caller identity token" below. + # Inbound callers authenticate with GitHub Actions OIDC tokens. GitHub + # is a platform issuer, as in production: PLATFORM_ISSUERS names the + # audience requested in "Mint caller identity token" below, and a + # token acts as an account the stub says trusts this repository. No + # person-issuer token exists in CI; AUTH_ISSUER names an issuer that + # mints nothing, and AUTH_AUDIENCE keeps /.sts from answering 501. run: | { openssl genpkey -algorithm RSA -out /tmp/oidc.pem -pkeyopt rsa_keygen_bits:2048 printf 'OIDC_PROVIDER_KEY="%s"\n' "$(cat /tmp/oidc.pem)" echo "SESSION_TOKEN_KEY=$(openssl rand -base64 32)" - echo "AUTH_ISSUER=https://token.actions.githubusercontent.com" + echo "AUTH_ISSUER=https://auth.example.invalid" echo "AUTH_AUDIENCE=source-data-proxy-ci" + echo 'PLATFORM_ISSUERS={"https://token.actions.githubusercontent.com": ["source-data-proxy-ci"]}' echo "SOURCE_API_URL=http://localhost:9000" } > .dev.vars - name: Mint caller identity token (GitHub OIDC) @@ -144,9 +148,9 @@ jobs: fi echo "::add-mask::$token" echo "CI_WRITE_ID_TOKEN=$token" >> "$GITHUB_ENV" - # A second, validly-signed token whose audience mismatches - # AUTH_AUDIENCE: test_writes.py asserts the /.sts aud gate rejects - # it (signature checks alone would let it through). + # A second, validly-signed token whose audience is not GitHub's in + # PLATFORM_ISSUERS: test_writes.py asserts the /.sts aud gate + # rejects it (signature checks alone would let it through). wrong=$(curl -sSf -H "Authorization: bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \ "$ACTIONS_ID_TOKEN_REQUEST_URL&audience=not-the-data-proxy" | jq -r '.value') if [ -z "$wrong" ] || [ "$wrong" = "null" ]; then diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index 835fbc6f..a8e24df2 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -58,14 +58,12 @@ jobs: # A GitHub Actions OIDC token, minted per run — short-lived by design # and never stored, unlike a token parked in a repo secret. # - # Dormant until Source registers GitHub as a valid IdP, so that products - # can accept writes from GitHub Actions. Two things must land first: - # the deployment's AUTH_ISSUER must accept GitHub's issuer (today it is - # a single Ory URL — src/config.rs reads AUTH_ISSUER as one String, - # unlike the comma-separated AUTH_AUDIENCE, so this needs a code change - # too), and the audience Source expects must be set as - # FEDERATION_TEST_AUDIENCE. Until then no token is minted and the - # copy-source authz test skips; the rest of the suite is unaffected. + # Dormant until FEDERATION_TEST_AUDIENCE is set to the staging proxy's + # origin, the audience staging's PLATFORM_ISSUERS accepts for GitHub, + # and FEDERATION_TEST_TRUST_ACCOUNT to a staging service account that + # trusts this repository's workflows (ADR-014): the token acts as that + # account. Until then no token is minted and the copy-source authz + # test skips; the rest of the suite is unaffected. if: vars.FEDERATION_TEST_AUDIENCE != '' run: | set -euo pipefail @@ -88,6 +86,7 @@ jobs: FEDERATION_WRITE_PRODUCT: ${{ vars.FEDERATION_WRITE_PRODUCT }} # Set by the mint step above, and only when it runs. CI_WRITE_ID_TOKEN: ${{ env.CI_WRITE_ID_TOKEN }} + CI_TRUST_ACCOUNT: ${{ vars.FEDERATION_TEST_TRUST_ACCOUNT }} # boto3: the copy-source authz test signs SigV4 through the AWS SDK # rather than hand-rolling requests. run: uvx --with requests --with boto3 pytest tests/test_federation.py -v diff --git a/Cargo.lock b/Cargo.lock index b70dd727..9aad4fa2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1891,6 +1891,7 @@ dependencies = [ name = "source-data-proxy" version = "2.3.4" dependencies = [ + "base64", "console_error_panic_hook", "getrandom 0.4.3", "hmac", diff --git a/Cargo.toml b/Cargo.toml index e5f80da3..f687b363 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,6 +57,9 @@ percent-encoding = "2" hmac = "0.12" sha2 = "0.10" +# Reading a platform IdP's token before verifying it (issuer, key id) +base64 = "0.22" + # Tracing tracing = "0.1" diff --git a/README.md b/README.md index 3a81ae8e..9232d39d 100644 --- a/README.md +++ b/README.md @@ -110,8 +110,9 @@ Set in `wrangler.toml` or via the Cloudflare dashboard: | ---------------------------- | --------------------------- | ---------------------------------------------------------------------------------------------------------------------------------- | | `SOURCE_API_URL` | `https://source.coop` | Source Cooperative API base URL | | `LOG_LEVEL` | `WARN` | Tracing level (`TRACE`, `DEBUG`, `INFO`, `WARN`, `ERROR`) | -| `AUTH_ISSUER` | `https://auth.source.coop` | OIDC issuer trusted for `/.sts` token exchange | +| `AUTH_ISSUER` | `https://auth.source.coop` | The person issuer trusted for `/.sts` token exchange; its tokens act as their own subject | | `AUTH_AUDIENCE` | — | Comma-separated OAuth client ID(s) that `/.sts` subject tokens must be issued to (`aud` claim); a token is accepted if it matches any. Unset = `/.sts` token exchange is disabled (returns 501) | +| `PLATFORM_ISSUERS` | — | JSON object from each platform issuer URL to the audiences its tokens must carry, such as `{"https://token.actions.githubusercontent.com": ["https://data.source.coop"]}`. An issuer with no audience is refused. Unset = no platform issuer is trusted | | `OIDC_PROVIDER_ISSUER` | `https://data.source.coop` | Issuer URL for minted JWTs and OIDC discovery | | `OIDC_PROVIDER_KID` | `data-proxy-1` | Key ID for the active signing key | | `OIDC_PROVIDER_KID_PREVIOUS` | — | Key ID for the previous key (during rotation) | @@ -128,7 +129,7 @@ A service account's API key (ADR-013) is an opaque `sck_` secret that source.coo ### Roles -Every exchange at `/.sts`, of an ID token or an API key, names a Role in `RoleArn`, either bare or as the resource of an ARN of any partition and account (`arn:aws:iam::000000000000:role/ReadOnly`), since AWS SDKs insist on an ARN. The Roles are hardcoded (ADR-014): +Every exchange at `/.sts`, of an ID token or an API key, names a Role in `RoleArn`, either bare or as the resource of an ARN of any partition and account (`arn:aws:iam::000000000000:role/ReadOnly`), since AWS SDKs insist on an ARN. The account matters only to a platform token (below). The Roles are hardcoded (ADR-014): | Role | Credentials may | | ------------ | -------------------------------------------------------- | @@ -138,6 +139,12 @@ Every exchange at `/.sts`, of an ID token or an API key, names a Role in `RoleAr Any other name is refused with `MalformedPolicyDocument`, never mapped to a default. A Role only subtracts: its ceiling is sealed into the session token and checked locally before the account's own permissions are looked up (ADR-011), and a request it refuses gets the same `AccessDenied` as any other refusal. +### Platform identity providers + +A token from a platform issuer in `PLATFORM_ISSUERS`, such as GitHub Actions, says which workload is calling but not which account it may act as. At `/.sts` it acts as the account in `RoleArn`, `arn:aws:iam:::role/FullAccess`, and only if that account trusts the token's issuer and subject (ADR-014). The proxy verifies the token against the issuer's JWKS, with that issuer's own audiences and a required `exp`, then asks `POST {SOURCE_API_URL}/api/v1/accounts/{account}/trusts/exchanges` with `{"issuer", "subject"}`, as the account. A yes is cached for 60 seconds per account, issuer and subject, and the credentials' principal is the account, never the token's subject. Any other answer reads `AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity (request id …)`. A token from `AUTH_ISSUER` still acts as its own subject and ignores the account in `RoleArn`. + +`aws-actions/configure-aws-credentials` fails after the exchange succeeds: it checks the credentials it exports with `GetCallerIdentity`, which the proxy cannot answer until developmentseed/multistore#126 lands. Until then a workflow saves its token to a file and lets an AWS SDK exchange it, with `AWS_WEB_IDENTITY_TOKEN_FILE`, `AWS_ROLE_ARN`, `AWS_ENDPOINT_URL_STS=/.sts`, `AWS_ENDPOINT_URL_S3=` and `AWS_REGION`. + ### Secrets **GitHub environment secrets are the source of truth.** The deploy workflow diff --git a/src/config.rs b/src/config.rs index 5b1cf663..cbc1e8ed 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,5 +1,6 @@ //! Process-wide configuration parsed once from Worker env vars + secrets. +use std::collections::HashMap; use std::sync::OnceLock; use multistore_oidc_provider::jwt::JwtSigner; @@ -100,6 +101,13 @@ fn build_config(env: &Env) -> AppConfig { tracing::warn!("AUTH_AUDIENCE not set: /.sts token exchange is disabled (returns 501)"); } + // Platform identity providers (GitHub Actions, say), each with its own + // audiences. Unset trusts none. + let platform_issuers = env + .var("PLATFORM_ISSUERS") + .map(|v| crate::platform::parse_issuers(&v.to_string())) + .unwrap_or_default(); + // Ceiling for client-requested DurationSeconds on /.sts. Unset → 3600 (1h), // matching multistore's own default so behavior is unchanged until raised. let sts_max_session_duration_secs = match env.var("STS_MAX_SESSION_DURATION_SECS") { @@ -141,6 +149,7 @@ fn build_config(env: &Env) -> AppConfig { session_token_key, auth_issuer, auth_audiences, + platform_issuers, sts_max_session_duration_secs, ip_hash_salt, } @@ -151,13 +160,19 @@ pub struct AppConfig { pub oidc: OidcConfig, /// AES key for sealing/unsealing STS session tokens. pub session_token_key: TokenKey, - /// OIDC issuer URL for the Source Cooperative auth provider (e.g. `https://auth.source.coop`). + /// OIDC issuer URL for the Source Cooperative auth provider (e.g. + /// `https://auth.source.coop`): the person issuer, whose tokens say who the + /// caller is. pub auth_issuer: String, /// OAuth client IDs that subject tokens presented to `/.sts` may be issued /// to (the `aud` claim); a token is accepted if it matches any. Parsed from /// the comma-separated `AUTH_AUDIENCE`. Empty disables `/.sts` entirely /// (returns 501) rather than accepting any audience. pub auth_audiences: Vec, + /// Platform issuers and the audiences each one's tokens must carry, from + /// the JSON object in `PLATFORM_ISSUERS`. A platform token acts as the + /// account `RoleArn` names, if that account trusts it (ADR-014). + pub platform_issuers: HashMap>, /// Ceiling for client-requested STS session length (`DurationSeconds`), /// in seconds. From `STS_MAX_SESSION_DURATION_SECS`; defaults to 3600 (1h). pub sts_max_session_duration_secs: u64, diff --git a/src/lib.rs b/src/lib.rs index 948a012f..eab40117 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,6 +16,7 @@ mod keys; mod location; mod object_path; mod pagination; +mod platform; mod source_api; mod sts; @@ -236,14 +237,19 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result (u16, String) { - let message = if request_id.is_empty() { + build_sts_error_response(&ProxyError::InvalidOidcToken(with_request_id( + message, request_id, + ))) +} + +/// `message` with the request id, if there is one: SDKs show a user the +/// message and nothing else, and the id is what finds the log line. +fn with_request_id(message: &str, request_id: &str) -> String { + if request_id.is_empty() { message.to_string() } else { format!("{message} (request id {request_id})") - }; - build_sts_error_response(&ProxyError::InvalidOidcToken(message)) + } } /// Hash the key, ask source.coop for its standing, and mint for the account @@ -642,14 +654,122 @@ async fn within_rate_limit(env: &Env, client_ip: &str) -> bool { } } -/// An STS-shaped error body for a status `build_sts_error_response` has no -/// variant for. +/// An STS-shaped error body with a code or message `build_sts_error_response` +/// does not produce. fn sts_error_xml(code: &str, message: &str) -> String { format!( "\n{code}{message}" ) } +// ── Platform identity providers ───────────────────────────────────── + +/// The exchange of a platform issuer's token, if this request carries one: +/// `None` for any other token, which the STS route takes. Parameters come from +/// the query string or the form body, never both, as at the STS route. Every +/// refusal of the account's trust reads the same, whether the account does not +/// exist or does not trust the token. +async fn platform_exchange( + config: &AppConfig, + parts: &RequestParts, + api_auth: &ApiAuth, + request_id: &str, +) -> Option<(u16, String)> { + let sts = try_parse_sts_request(parts.query.as_deref()) + .or_else(|| try_parse_sts_request(parts.form_body.as_deref()))? + .ok()?; + let (header, claims) = platform::unverified(&sts.web_identity_token)?; + let issuer = claims.get("iss")?.as_str()?; + let audiences = config.platform_issuers.get(issuer)?; + Some( + match exchange_platform_token( + config, &sts, &header, issuer, audiences, api_auth, request_id, + ) + .await + { + Ok(creds) => build_sts_response(&creds), + // `exchange_platform_token` has logged who asked to act as whom. + Err(ProxyError::AccessDenied) => ( + 403, + sts_error_xml( + "AccessDenied", + &with_request_id( + "Not authorized to perform sts:AssumeRoleWithWebIdentity", + request_id, + ), + ), + ), + Err(e) => { + tracing::warn!(%request_id, %issuer, error = %e, "platform token exchange failed"); + build_sts_error_response(&e) + } + }, + ) +} + +/// Verify a platform issuer's token, then mint for the account `RoleArn` +/// names if that account trusts the token's issuer and subject (ADR-014). The +/// credentials act as the account, never as the token's subject. Everything +/// local comes first, so a token that fails it costs the Source API nothing. +async fn exchange_platform_token( + config: &AppConfig, + sts: &multistore_sts::request::StsRequest, + header: &serde_json::Value, + issuer: &str, + audiences: &[String], + api_auth: &ApiAuth, + request_id: &str, +) -> Result { + let role = sts::role( + &sts.role_arn, + issuer.to_string(), + audiences.to_vec(), + config.sts_max_session_duration_secs, + ) + .ok_or_else(|| ProxyError::RoleNotFound(sts.role_arn.clone()))?; + // No angle brackets in the message: the STS error body carries it unescaped. + let account = sts::account(&sts.role_arn).ok_or_else(|| { + ProxyError::InvalidRequest( + "RoleArn must name the account to act as: arn:aws:iam::ACCOUNT:role/ROLE".into(), + ) + })?; + let subject = platform::verify( + &sts.web_identity_token, + header, + issuer, + &role, + &jwks_cache(), + ) + .await?; + source_api::cache::get_or_fetch_trust( + &config.api_base_url, + account, + issuer, + &subject, + api_auth, + request_id, + ) + .await + .map_err(|e| match e { + ProxyError::AccessDenied => { + tracing::warn!(%request_id, %issuer, %subject, %account, "account does not trust the token"); + e + } + // The route answers for any account; a 404 means the API does not + // serve it, which is a deployment mismatch, not a refusal. + ProxyError::BucketNotFound(_) => ProxyError::Internal("trusts route not found".into()), + e => e, + })?; + let creds = keys::credentials_for( + &role, + account, + sts.duration_seconds, + &config.session_token_key, + )?; + tracing::info!(%request_id, %issuer, %subject, %account, role = %role.role_id, "platform token exchanged"); + Ok(creds) +} + // ── CORS ──────────────────────────────────────────────────────────── fn add_cors(resp: web_sys::Response) -> web_sys::Response { diff --git a/src/platform.rs b/src/platform.rs new file mode 100644 index 00000000..27180a8c --- /dev/null +++ b/src/platform.rs @@ -0,0 +1,89 @@ +//! Platform identity providers at `/.sts` (ADR-009, ADR-014): GitHub Actions +//! and the like, whose tokens say which workload is calling but not which +//! account it may act as. The account is the one `RoleArn` names, and only if +//! that account trusts the token's issuer and subject, which the Source API +//! answers. This module is the wasm-free half: which issuers are platform +//! issuers, reading a token before it is verified, and verifying it. + +use std::collections::HashMap; + +use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use base64::Engine; +use multistore::error::ProxyError; +use multistore::types::RoleConfig; +use multistore_sts::jwks::{find_key, verify_token}; +use multistore_sts::JwksCache; +use serde_json::Value; + +/// The platform issuers in `PLATFORM_ISSUERS`, a JSON object from each issuer +/// URL to the audiences its tokens must carry. The audiences are per issuer so +/// that one issuer's audience never admits another's token (ADR-009). An +/// issuer with no audience is left out, as the person issuer is disabled +/// without one: a token minted for any other service could be exchanged here. +/// A value that does not parse trusts no platform issuer. +pub fn parse_issuers(json: &str) -> HashMap> { + let issuers: HashMap> = match serde_json::from_str(json) { + Ok(issuers) => issuers, + Err(e) => { + tracing::error!( + "PLATFORM_ISSUERS is not an object of issuer to audiences ({e}); trusting none" + ); + return HashMap::new(); + } + }; + issuers + .into_iter() + .filter(|(issuer, audiences)| { + if audiences.is_empty() { + tracing::error!(%issuer, "platform issuer has no audience; refusing its tokens"); + } + !audiences.is_empty() + }) + .collect() +} + +/// A token's header and claims, read without verifying anything: enough to +/// route it to its issuer and find the key it names. `None` if it is not a +/// JWT, an API key for one. +pub fn unverified(token: &str) -> Option<(Value, Value)> { + let mut segments = token.split('.').map(|segment| { + let json = URL_SAFE_NO_PAD.decode(segment).ok()?; + serde_json::from_slice::(&json).ok() + }); + Some((segments.next()??, segments.next()??)) +} + +/// Verify a platform issuer's token as the STS route verifies the person +/// issuer's (signature against the issuer's published keys, issuer, the +/// audiences `role` requires, `exp` and `nbf`) and return its subject. +pub async fn verify( + token: &str, + header: &Value, + issuer: &str, + role: &RoleConfig, + jwks: &JwksCache, +) -> Result { + let kid = header + .get("kid") + .and_then(Value::as_str) + .ok_or_else(|| ProxyError::InvalidOidcToken("JWT missing kid".into()))?; + let keys = jwks.get_or_fetch(issuer).await?; + let claims = verify_token(token, find_key(&keys, kid)?, issuer, role)?; + subject(&claims).map(str::to_string) +} + +/// The subject of verified `claims`, which must also carry an expiry: +/// multistore checks `exp` only when it is present, and a third-party token +/// with none would be replayable for good (ADR-004). +pub fn subject(claims: &Value) -> Result<&str, ProxyError> { + if claims.get("exp").and_then(Value::as_i64).is_none() { + return Err(ProxyError::InvalidOidcToken( + "token has no exp claim".into(), + )); + } + claims + .get("sub") + .and_then(Value::as_str) + .filter(|sub| !sub.is_empty()) + .ok_or_else(|| ProxyError::InvalidOidcToken("token has no sub claim".into())) +} diff --git a/src/source_api/cache.rs b/src/source_api/cache.rs index 5cb7743a..0e952257 100644 --- a/src/source_api/cache.rs +++ b/src/source_api/cache.rs @@ -52,6 +52,11 @@ const PERMISSIONS_CACHE_SECS: u32 = 60; // 1 minute /// cached too — an unknown key costs one lookup a minute, not one a request. const KEY_STANDING_CACHE_SECS: u32 = 60; // 1 minute +/// Whether an account trusts a platform token's issuer and subject +/// (`/accounts/{id}/trusts/exchanges`). The permissions TTL, for the same +/// reason: a trust that is removed should stop minting quickly (ADR-014). +const TRUST_CACHE_SECS: u32 = 60; // 1 minute + // ── Public cache functions ───────────────────────────────────────── /// Fetch a single product's metadata, cached for `PRODUCT_CACHE_SECS`. @@ -204,6 +209,57 @@ pub async fn get_or_fetch_key_standing( .await } +/// Whether `account` trusts `issuer`'s `subject` to act as it: `Ok` if so, +/// `AccessDenied` if not, the way a role's own trust policy decides an +/// assume-role call. Asked as the account itself. Only a yes is cached, for +/// `TRUST_CACHE_SECS`: the route says no with a 403, and no 403 is cached, so +/// a trust just added works on the next attempt. +pub async fn get_or_fetch_trust( + api_base_url: &str, + account: &str, + issuer: &str, + subject: &str, + api_auth: &crate::ApiAuth, + request_id: &str, +) -> Result<(), ProxyError> { + let api_url = format!( + "{}/api/v1/accounts/{}/trusts/exchanges", + api_base_url, + utf8_percent_encode(account, PATH_SEGMENT), + ); + // The Cache API keys on URLs: the account is in the path, and the issuer + // and subject vary with it. + let cache_key = format!( + "{api_url}?issuer={}&subject={}", + utf8_percent_encode(issuer, PATH_SEGMENT), + utf8_percent_encode(subject, PATH_SEGMENT), + ); + let body = serde_json::json!({ "issuer": issuer, "subject": subject }).to_string(); + let answer: TrustAnswer = cached_fetch( + &cache_key, + &api_url, + "POST", + Some(&body), + TRUST_CACHE_SECS, + api_auth, + request_id, + ApiCaller::Account(account), + ) + .await?; + if answer.trusted { + Ok(()) + } else { + Err(ProxyError::AccessDenied) + } +} + +/// The trusts route's answer. Its status already says yes (200) or no (403); +/// the body is read too, so that a 200 saying no mints nothing. +#[derive(serde::Deserialize)] +struct TrustAnswer { + trusted: bool, +} + // ── Internal helpers ────────────────────────────────────────────── /// Build a cache key that includes the caller's identity so that diff --git a/src/sts.rs b/src/sts.rs index 58e895b3..dc85ca2a 100644 --- a/src/sts.rs +++ b/src/sts.rs @@ -100,6 +100,16 @@ fn role_name(role_arn: &str) -> Option<&str> { role_arn.splitn(6, ':').nth(5)?.strip_prefix("role/") } +/// The account segment of an ARN-form `role_arn` +/// (`arn:aws:iam:::role/`): the account a platform IdP's token +/// asks to act as (ADR-014). `None` for a bare name or an empty account. +pub(crate) fn account(role_arn: &str) -> Option<&str> { + match role_arn.splitn(6, ':').collect::>()[..] { + ["arn", _, _, _, account, _] if !account.is_empty() => Some(account), + _ => None, + } +} + impl CredentialRegistry for StsCredentialRegistry { async fn get_credential( &self, diff --git a/tests/platform.rs b/tests/platform.rs new file mode 100644 index 00000000..c1c3f305 --- /dev/null +++ b/tests/platform.rs @@ -0,0 +1,95 @@ +//! Native unit tests for the wasm-free half of platform-IdP exchanges +//! (`platform`), included via `#[path]` like `tests/keys.rs`. Verification +//! against a real issuer's keys and the trust lookup run in the worker and are +//! covered by `tests/test_platform_trust.py`. + +#[path = "../src/platform.rs"] +mod platform; + +use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use base64::Engine; +use serde_json::json; + +const GITHUB: &str = "https://token.actions.githubusercontent.com"; + +// ── configuration ────────────────────────────────────────────────── + +#[test] +fn each_issuer_keeps_its_own_audiences() { + let issuers = platform::parse_issuers( + r#"{"https://token.actions.githubusercontent.com": ["https://data.source.coop"], + "https://gitlab.com": ["a", "b"]}"#, + ); + assert_eq!(issuers[GITHUB], ["https://data.source.coop"]); + assert_eq!(issuers["https://gitlab.com"], ["a", "b"]); +} + +#[test] +fn an_issuer_without_an_audience_is_not_trusted() { + let issuers = platform::parse_issuers(r#"{"https://token.actions.githubusercontent.com": []}"#); + assert!(issuers.is_empty()); +} + +#[test] +fn a_value_that_does_not_parse_trusts_no_issuer() { + for value in [ + "", + GITHUB, + r#"["https://token.actions.githubusercontent.com"]"#, + ] { + assert!(platform::parse_issuers(value).is_empty(), "{value}"); + } +} + +// ── reading a token before verifying it ──────────────────────────── + +fn segment(value: serde_json::Value) -> String { + URL_SAFE_NO_PAD.encode(value.to_string()) +} + +#[test] +fn a_jwt_is_read_without_verifying_it() { + let token = format!( + "{}.{}.not-a-signature", + segment(json!({"alg": "RS256", "kid": "k1"})), + segment(json!({"iss": GITHUB, "sub": "repo:o/r:ref:refs/heads/main"})), + ); + let (header, claims) = platform::unverified(&token).unwrap(); + assert_eq!(header["kid"], "k1"); + assert_eq!(claims["iss"], GITHUB); +} + +#[test] +fn anything_else_is_not_read() { + let claims_not_json = format!("{}.not-json.sig", segment(json!({"alg": "RS256"}))); + for token in [ + "", + "not-a-jwt", + "sck_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + claims_not_json.as_str(), + ] { + assert!(platform::unverified(token).is_none(), "{token}"); + } +} + +// ── claims a platform token must carry ───────────────────────────── + +#[test] +fn a_verified_token_names_its_subject() { + let claims = json!({"sub": "repo:o/r:ref:refs/heads/main", "exp": 1_900_000_000}); + assert_eq!( + platform::subject(&claims).unwrap(), + "repo:o/r:ref:refs/heads/main" + ); +} + +#[test] +fn a_token_without_an_expiry_or_a_subject_is_refused() { + for claims in [ + json!({"sub": "repo:o/r:ref:refs/heads/main"}), + json!({"exp": 1_900_000_000}), + json!({"sub": "", "exp": 1_900_000_000}), + ] { + assert!(platform::subject(&claims).is_err(), "{claims}"); + } +} diff --git a/tests/sts.rs b/tests/sts.rs index 7077edaa..1dab3f71 100644 --- a/tests/sts.rs +++ b/tests/sts.rs @@ -66,3 +66,22 @@ fn unknown_names_are_refused_not_defaulted() { assert_eq!(named(arn), None, "{arn}"); } } + +#[test] +fn the_account_is_the_arns_account_segment() { + assert_eq!( + sts::account("arn:aws:iam::acme--nightly-sync:role/FullAccess"), + Some("acme--nightly-sync") + ); + assert_eq!( + sts::account("arn:aws:iam::000000000000:role/_default"), + Some("000000000000") + ); + for role_arn in [ + "FullAccess", + "arn:aws:iam:::role/FullAccess", + "arn:aws:iam::acme", + ] { + assert_eq!(sts::account(role_arn), None, "{role_arn}"); + } +} diff --git a/tests/stub_api.py b/tests/stub_api.py index ce08b3bc..2a672573 100644 --- a/tests/stub_api.py +++ b/tests/stub_api.py @@ -21,11 +21,14 @@ SOURCE_API_URL in .dev.vars. """ +import base64 import hashlib import json import os +import re from http.server import BaseHTTPRequestHandler, HTTPServer from pathlib import Path +from urllib.parse import unquote PORT = 9000 @@ -139,9 +142,37 @@ def _hash(key): KEY_EXCHANGE_COUNTS = {} +# ── Account trusts ───────────────────────────────────────────────── +# Whether an account trusts a platform token's issuer and subject, at POST +# /api/v1/accounts/{account}/trusts/exchanges (ADR-014). The proxy asks as the +# account itself. Only TRUST_ACCOUNT trusts anyone: GitHub Actions workflows +# in this repository, whatever event minted the token. A counter per account +# lets test_platform_trust.py prove the proxy caches a yes. +TRUST_ACCOUNT = "ci-tests--github-ci" +TRUSTED_ISSUER = "https://token.actions.githubusercontent.com" +TRUSTED_SUBJECT_PREFIX = "repo:source-cooperative/data.source.coop:" +TRUST_EXCHANGE_COUNTS = {} + +# Who the proxy said it was asking as, per product path, so a test can check +# which principal a session carries: see test_platform_trust.py. +PRODUCT_LOOKUP_SUBJECTS = {} + + +def _bearer_subject(authorization): + """The `sub` of the proxy's assertion, unverified: the stub has no key.""" + try: + payload = authorization.removeprefix("Bearer ").split(".")[1] + return json.loads(base64.urlsafe_b64decode(payload + "=" * (-len(payload) % 4)))["sub"] + except (IndexError, ValueError, KeyError): + return None + + class Handler(BaseHTTPRequestHandler): def do_POST(self): path = self.path.split("?")[0] + trust = re.fullmatch(r"/api/v1/accounts/([^/]+)/trusts/exchanges", path) + if trust: + return self._trust_exchange(unquote(trust.group(1))) if path != "/api/v1/service-account-keys/exchanges": return self._send(404, b"{}") # The proxy authenticates as itself; the stub cannot verify the @@ -157,11 +188,37 @@ def do_POST(self): status, body = KEY_STANDINGS.get(key_hash, (200, {"active": False})) self._send(status, json.dumps(body).encode()) + def _trust_exchange(self, account): + # Anyone but the account itself is refused, as the real route does. + if _bearer_subject(self.headers.get("Authorization", "")) != account: + return self._send(401, b'{"error": "Unauthorized"}') + length = int(self.headers.get("content-length") or 0) + try: + body = json.loads(self.rfile.read(length)) + issuer, subject = body["issuer"], body["subject"] + except (ValueError, KeyError, TypeError): + return self._send(400, b"{}") + TRUST_EXCHANGE_COUNTS[account] = TRUST_EXCHANGE_COUNTS.get(account, 0) + 1 + trusted = ( + account == TRUST_ACCOUNT + and issuer == TRUSTED_ISSUER + and subject.startswith(TRUSTED_SUBJECT_PREFIX) + ) + self._send(200 if trusted else 403, json.dumps({"trusted": trusted}).encode()) + def do_GET(self): path = self.path.split("?")[0] # Test-only: how many times each key's standing was asked for. if path == "/_stub/key-exchange-counts": return self._send(200, json.dumps(KEY_EXCHANGE_COUNTS).encode()) + # Test-only: how many times each account's trust was asked for. + if path == "/_stub/trust-exchange-counts": + return self._send(200, json.dumps(TRUST_EXCHANGE_COUNTS).encode()) + # Test-only: the subject each product was last looked up as. + if path == "/_stub/product-lookup-subjects": + return self._send(200, json.dumps(PRODUCT_LOOKUP_SUBJECTS).encode()) + if path.startswith("/api/v1/products/") and self.headers.get("Authorization"): + PRODUCT_LOOKUP_SUBJECTS[path] = _bearer_subject(self.headers["Authorization"]) if path == f"/api/v1/products/{WRITE_ACCOUNT}/{ERR_500_PRODUCT}": return self._send(500, b"{}") if path == f"/api/v1/products/{WRITE_ACCOUNT}/{ERR_BAD_JSON_PRODUCT}": diff --git a/tests/test_federation.py b/tests/test_federation.py index fff5ad26..9d2b5d45 100644 --- a/tests/test_federation.py +++ b/tests/test_federation.py @@ -31,17 +31,15 @@ OIDC token, minted per run by staging.yml (short- lived by design, never stored) once FEDERATION_TEST_AUDIENCE is set. - - Dormant until Source registers GitHub as a valid - IdP so products can accept writes from GitHub - Actions. That needs the deployment's AUTH_ISSUER to - accept GitHub's issuer — today it is a single Ory - URL, and ``src/config.rs`` reads AUTH_ISSUER as one - String (unlike the comma-separated AUTH_AUDIENCE), - so it is a code change as well as config — and the - audience Source expects to be set as - FEDERATION_TEST_AUDIENCE. Until then this test - skips. The caller must also hold write on + CI_TRUST_ACCOUNT the account that token acts as: a service account + that trusts this repository's workflows (ADR-014). + + Dormant until FEDERATION_TEST_AUDIENCE is set to + the staging proxy's origin, the audience its + PLATFORM_ISSUERS accepts for GitHub, and a staging + service account that trusts this repository is set + as FEDERATION_TEST_TRUST_ACCOUNT. Until then this + test skips. That account must also hold write on FEDERATION_WRITE_PRODUCT """ diff --git a/tests/test_platform_trust.py b/tests/test_platform_trust.py new file mode 100644 index 00000000..9974965d --- /dev/null +++ b/tests/test_platform_trust.py @@ -0,0 +1,118 @@ +"""Platform-IdP tokens at /.sts (ADR-014), against the stub Source API. + +CI trusts GitHub Actions as a platform issuer (PLATFORM_ISSUERS in ci.yml). A +verified GitHub token acts as the account its RoleArn names, and only if the +stub's trusts route says that account trusts the token's issuer and subject: +TRUST_ACCOUNT trusts this repository's workflows, and no other account trusts +anything. The tests that need a real token run where CI mints one (see +test_writes.py); the others pin what is refused before any trust lookup. +""" + +import base64 +import json +import uuid +import xml.etree.ElementTree as ET + +import pytest +import requests + +from stub_api import TRUST_ACCOUNT, TRUSTED_ISSUER, WRITE_ACCOUNT +from test_writes import ID_TOKEN, PROXY_URL, needs_token + +STUB_URL = "http://localhost:9000" +# The worker takes its request id from `cf-ray`, which `wrangler dev` does not +# set; the tests supply one. +RAY = "ci-ray-trust" + + +def exchange(token, role_arn): + params = {"Action": "AssumeRoleWithWebIdentity", "RoleArn": role_arn, "WebIdentityToken": token} + return requests.post(f"{PROXY_URL}/.sts", data=params, headers={"cf-ray": RAY}) + + +def as_account(account): + return f"arn:aws:iam::{account}:role/FullAccess" + + +def sts_fields(resp): + return {el.tag.rpartition("}")[2]: el.text for el in ET.fromstring(resp.text).iter()} + + +def trust_lookups(account): + return requests.get(f"{STUB_URL}/_stub/trust-exchange-counts").json().get(account, 0) + + +def forged(): + """A token that says it is GitHub's, signed by no one.""" + + def segment(value): + return base64.urlsafe_b64encode(json.dumps(value).encode()).rstrip(b"=").decode() + + claims = { + "iss": TRUSTED_ISSUER, + "sub": "repo:source-cooperative/data.source.coop:ref:refs/heads/main", + "aud": "source-data-proxy-ci", + "exp": 4_000_000_000, + } + return ".".join([segment({"alg": "RS256", "kid": "forged"}), segment(claims), segment("x")]) + + +def test_a_platform_token_must_name_the_account_it_acts_as(): + resp = exchange(forged(), "arn:aws:iam:::role/FullAccess") + assert resp.status_code == 400 + assert "RoleArn must name the account" in sts_fields(resp)["Message"] + + +def test_a_forged_token_is_refused_before_any_trust_lookup(): + before = trust_lookups(TRUST_ACCOUNT) + resp = exchange(forged(), as_account(TRUST_ACCOUNT)) + assert resp.status_code == 400 + assert sts_fields(resp)["Code"] == "InvalidIdentityToken" + assert trust_lookups(TRUST_ACCOUNT) == before + + +@needs_token +def test_a_trusted_workflow_gets_credentials_that_act_as_the_account(): + """The session's principal is the account, not the token's subject: the + stub records who the proxy asked about a product as.""" + import boto3 + from botocore.config import Config + from botocore.exceptions import ClientError + + resp = exchange(ID_TOKEN, as_account(TRUST_ACCOUNT)) + assert resp.status_code == 200, resp.text[:300] + fields = sts_fields(resp) + client = boto3.client( + "s3", + endpoint_url=PROXY_URL, + aws_access_key_id=fields["AccessKeyId"], + aws_secret_access_key=fields["SecretAccessKey"], + aws_session_token=fields["SessionToken"], + region_name="us-east-1", + config=Config(s3={"addressing_style": "path"}), + ) + # A product the stub has never heard of, so the lookup is not cached. + product = f"principal-probe-{uuid.uuid4().hex}" + with pytest.raises(ClientError): + client.get_object(Bucket=WRITE_ACCOUNT, Key=f"{product}/x") + subjects = requests.get(f"{STUB_URL}/_stub/product-lookup-subjects").json() + assert subjects[f"/api/v1/products/{WRITE_ACCOUNT}/{product}"] == TRUST_ACCOUNT + + +@needs_token +def test_an_account_that_does_not_trust_the_workflow_refuses_it(): + resp = exchange(ID_TOKEN, as_account("ci-tests--someone-else")) + assert resp.status_code == 403 + fields = sts_fields(resp) + assert fields["Code"] == "AccessDenied" + assert fields["Message"] == ( + f"Not authorized to perform sts:AssumeRoleWithWebIdentity (request id {RAY})" + ) + + +@needs_token +def test_a_trusted_answer_is_cached(): + exchange(ID_TOKEN, as_account(TRUST_ACCOUNT)) + before = trust_lookups(TRUST_ACCOUNT) + assert exchange(ID_TOKEN, as_account(TRUST_ACCOUNT)).status_code == 200 + assert trust_lookups(TRUST_ACCOUNT) == before, "second exchange within the TTL asked the API again" diff --git a/tests/test_writes.py b/tests/test_writes.py index 0122ec3f..5d5da408 100644 --- a/tests/test_writes.py +++ b/tests/test_writes.py @@ -3,15 +3,15 @@ Data requests to the proxy are SigV4-only (Bearer JWTs are rejected), so an authenticated write follows the real client flow end-to-end: - 1. Obtain an OIDC identity token whose `aud` is in the worker's AUTH_AUDIENCE. - In CI this is a GitHub Actions OIDC token (AUTH_ISSUER = - https://token.actions.githubusercontent.com); the proxy verifies it via - OIDC discovery against GitHub's JWKS. - 2. Exchange it at POST /.sts (AssumeRoleWithWebIdentity, RoleArn=_default) - for temporary credentials whose SessionToken is sealed under - SESSION_TOKEN_KEY. + 1. Obtain a GitHub Actions OIDC token with an audience the worker accepts + for GitHub, a platform issuer (PLATFORM_ISSUERS in ci.yml). The proxy + verifies it via OIDC discovery against GitHub's JWKS. + 2. Exchange it at POST /.sts (AssumeRoleWithWebIdentity) with a RoleArn + naming TRUST_ACCOUNT, which the stub says trusts this repository's + workflows, for temporary credentials whose SessionToken is sealed under + SESSION_TOKEN_KEY (ADR-014). 3. SigV4-sign S3 requests with those credentials; the proxy unseals the - token, verifies the signature, and recovers the subject (the JWT's `sub`). + token, verifies the signature, and recovers the principal: the account. Two tiers, so the suite degrades gracefully: @@ -33,11 +33,15 @@ import pytest import requests +from stub_api import TRUST_ACCOUNT as STUB_TRUST_ACCOUNT from stub_api import WRITE_ACCOUNT, WRITE_PRODUCT PROXY_URL = os.environ.get("PROXY_URL", "http://localhost:8787") ID_TOKEN = os.environ.get("CI_WRITE_ID_TOKEN") WRONG_AUD_TOKEN = os.environ.get("CI_WRONG_AUDIENCE_TOKEN") +# The account the caller's token acts as: the stub's, or against a deployed +# proxy (staging.yml) a service account there that trusts this repository. +TRUST_ACCOUNT = os.environ.get("CI_TRUST_ACCOUNT") or STUB_TRUST_ACCOUNT # When CI declares a token must exist (same-repo runs export CI_EXPECT_OIDC), # a missing token means the mint->env plumbing broke: run the tests and fail @@ -60,7 +64,7 @@ def sts_exchange(token, *, form_body=False): request body, with no query string — instead of in the query string.""" params = { "Action": "AssumeRoleWithWebIdentity", - "RoleArn": "_default", + "RoleArn": f"arn:aws:iam::{TRUST_ACCOUNT}:role/FullAccess", "WebIdentityToken": token, } if form_body: @@ -274,9 +278,9 @@ def test_sts_rejects_tampered_signature(): @needs_wrong_aud_token def test_sts_rejects_wrong_audience(): - """A validly-signed token whose aud isn't in AUTH_AUDIENCE must be - rejected — this is the gate that keeps other GitHub OIDC consumers' - tokens from minting credentials here.""" + """A validly-signed token whose aud isn't one PLATFORM_ISSUERS lists for + GitHub must be rejected — this is the gate that keeps other GitHub OIDC + consumers' tokens from minting credentials here.""" # Presence assert, not just the skipif: with the token missing, # sts_exchange(None) sends no WebIdentityToken and the 4xx assertion # below would pass vacuously — testing nothing. diff --git a/wrangler.preview.toml b/wrangler.preview.toml index 323e9f6f..31b52248 100644 --- a/wrangler.preview.toml +++ b/wrangler.preview.toml @@ -21,6 +21,10 @@ OIDC_PROVIDER_KID = "data-proxy-1" # AUTH_AUDIENCE must be set or /.sts fail-closes with 501. AUTH_ISSUER = "https://auth.staging.source.coop" AUTH_AUDIENCE = "1123dfa8-469f-44fe-b9f4-9b76f06fd325,a79c9537-be78-454a-9ea1-b96a1be811cc" # staging frontend + source-coop-cli client_ids +# Staging's platform issuers too: a GitHub Actions workflow calling a preview +# mints its token for the staging proxy's origin, since a preview's own +# hostname differs per PR. +PLATFORM_ISSUERS = '{"https://token.actions.githubusercontent.com": ["https://data.staging.source.coop"]}' [build] command = "cargo install -q worker-build@0.7.5 && worker-build --release" diff --git a/wrangler.toml b/wrangler.toml index 6b10f65d..6b3d3a9d 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -21,6 +21,11 @@ AUTH_ISSUER = "https://auth.source.coop" LOG_LEVEL = "WARN" OIDC_PROVIDER_ISSUER = "https://data.source.coop" OIDC_PROVIDER_KID = "data-proxy-1" +# Platform identity providers, each with the audiences its tokens must carry. +# A platform token acts as the account RoleArn names, if that account trusts +# its issuer and subject (ADR-014). GitHub Actions workflows mint their token +# for the proxy's own origin, as source.coop's workflow snippet does. +PLATFORM_ISSUERS = '{"https://token.actions.githubusercontent.com": ["https://data.source.coop"]}' SOURCE_API_URL = "https://source.coop" # Ceiling for client-requested /.sts DurationSeconds (seconds). 43200 = 12h, # 86400 = 24h. Unset → 3600 (1h). Clients must request DurationSeconds to use @@ -41,6 +46,9 @@ STS_MAX_SESSION_DURATION_SECS = "43200" # AUTH_AUDIENCE - comma-separated OAuth client_id(s) that /.sts subject tokens # must be issued to (the `aud` claim). A token is accepted if # it matches any. Unset = /.sts is disabled (returns 501). +# Optional vars: +# PLATFORM_ISSUERS - JSON object from platform issuer URL to the audiences its +# tokens must carry. Unset = no platform issuer is trusted. # TODO: Set different sampling rates for prod vs staging [observability] enabled = true @@ -89,6 +97,7 @@ AUTH_AUDIENCE = "1123dfa8-469f-44fe-b9f4-9b76f06fd325,a79c9537-be78-454a-9ea1-b9 AUTH_ISSUER = "https://auth.staging.source.coop" OIDC_PROVIDER_ISSUER = "https://data.staging.source.coop" OIDC_PROVIDER_KID = "data-proxy-1" +PLATFORM_ISSUERS = '{"https://token.actions.githubusercontent.com": ["https://data.staging.source.coop"]}' SOURCE_API_URL = "https://staging.source.coop" STS_MAX_SESSION_DURATION_SECS = "43200" # 12h ceiling for client-requested /.sts DurationSeconds From e103b6f1092a0732d076b2ee4b9f7b4d9ce0a8fd Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Fri, 25 Sep 2026 14:32:32 -0700 Subject: [PATCH 5/7] docs(adrs): record platform issuers and their trust path ADR-009's migration planned to add platform issuers to `_default` and warned that a CI token would then act as its subject's whole account. ADR-014's trust path replaces that: a platform token acts only as an account that trusts its issuer and subject. ADR-009 gains a note saying so, and its status now reads implemented in part. ADR-004 now notes platform issuers under its trust model, and its `exp` warning says platform tokens must carry the claim, which the proxy checks itself. ADR-001's `source_identity` now says it is the account an API key or a trusted platform token names, never a platform token's subject. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd --- adrs/001-s3-credentials.md | 4 ++-- adrs/004-sts.md | 7 +++++-- adrs/009-platform-idps.md | 5 ++++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/adrs/001-s3-credentials.md b/adrs/001-s3-credentials.md index 0078274c..dbe8702c 100644 --- a/adrs/001-s3-credentials.md +++ b/adrs/001-s3-credentials.md @@ -58,7 +58,7 @@ The sealed payload carries: | `secret_access_key` | The signing secret, recovered by unsealing | | `expiration` | Enforced at unseal time; an expired token fails closed | | `assumed_role_id` | The Role assumed at exchange time: `_default`, `FullAccess` or `ReadOnly` (ADR-004) | -| `source_identity` | The original OIDC `sub` — the caller's Ory identity | +| `source_identity` | Who the credentials act as: an Ory ID token's `sub`, or the account an API key or a trusted platform token names (ADR-013, ADR-014) | | `allowed_scopes` | The Role's ceiling, sealed at mint time: empty for `FullAccess` and `_default`, reads of every product for `ReadOnly` (see below) | | `session_token` | A discarded random placeholder. The credential set is sealed *before* this field is overwritten with the sealed blob, so the value inside the envelope is not the token itself | @@ -67,7 +67,7 @@ Key properties of this design: - **Verification is fully stateless.** The proxy decrypts the token on each request and recovers the `SecretAccessKey` directly. No database lookup, no key derivation, and no asymmetric verification on the request hot path — which matters on Workers, where in-memory state does not persist across invocations. - **The token is opaque to the caller.** Unlike a JWT, a client cannot read the sealed payload. Scope and identity metadata are not disclosed to whoever holds the credential. - **`allowed_scopes` is enforced by the bucket registry, not by multistore.** multistore's own consumer, `multistore::auth::authorize`, has no call site in the pinned crate; the gateway delegates authorization to the registry instead (ADR-005), which checks the ceiling before any lookup (ADR-011, #236). The registry reads an empty vec as **no ceiling**, the reverse of `authorize`, where empty means deny-all — which is also why the registry overrides `authorize_key` rather than inheriting the default. The only non-empty ceiling is `ReadOnly`'s: every product (`*`), read actions only. -- **`source_identity` preserves the original subject**, which is what the proxy presents to the policy store (see ADR-005). +- **`source_identity` is the principal**, which is what the proxy presents to the policy store (see ADR-005): the original subject for an Ory ID token, never a platform token's subject. - **Authenticated encryption.** GCM provides integrity as well as confidentiality: a tampered token fails to decrypt rather than decoding into attacker-chosen values. ### SigV4 Verification Flow diff --git a/adrs/004-sts.md b/adrs/004-sts.md index 6540d8e5..11a0b06e 100644 --- a/adrs/004-sts.md +++ b/adrs/004-sts.md @@ -5,7 +5,7 @@ **RFC:** RFC-001 §7 **Depends on:** ADR-001 **Implementation:** `src/sts.rs`, `src/lib.rs`, `src/config.rs`; `source.coop:src/lib/actions/proxy-credentials.ts` -**Implemented by:** #116 (initial `/.sts` exchange), #163 (multiple accepted audiences), #165 (configurable max session duration), #185 (ARN-shaped `_default` alias), #196 (form-encoded POST bodies, wiring [multistore#112](https://github.com/developmentseed/multistore/pull/112)), #236 (`FullAccess` and `ReadOnly` Roles) · source.coop#283 (OIDC auth), source.coop#391 (in-browser uploads via the proxy), source.coop#402 (mid-upload credential refresh) +**Implemented by:** #116 (initial `/.sts` exchange), #163 (multiple accepted audiences), #165 (configurable max session duration), #185 (ARN-shaped `_default` alias), #196 (form-encoded POST bodies, wiring [multistore#112](https://github.com/developmentseed/multistore/pull/112)), #236 (`FullAccess` and `ReadOnly` Roles), #237 (platform issuers) · source.coop#283 (OIDC auth), source.coop#391 (in-browser uploads via the proxy), source.coop#402 (mid-upload credential refresh) --- @@ -80,6 +80,9 @@ A single built-in Role, `_default`, is served from a hardcoded registry: **Issuer.** `AUTH_ISSUER` names the single trusted OIDC issuer: Source Cooperative's Ory-based auth system (`https://auth.source.coop`, or the staging equivalent). A token from any other issuer is rejected before any network call. +> [!NOTE] +> Platform issuers (ADR-009) are now trusted alongside it, each with its own audiences in `PLATFORM_ISSUERS`. Their tokens take a separate path ahead of the STS route and act as the account `RoleArn` names, if that account trusts the token's issuer and subject (ADR-014, #237). + **Audience.** `AUTH_AUDIENCE` is a comma-separated allowlist of OAuth client IDs; a token is accepted if its `aud` matches any entry. Production lists the web frontend and `source-coop-cli`. **The audience restriction is load-bearing and the endpoint fails closed without it.** Without it, an ID token that a user granted to *any* third-party OAuth client registered with the issuer could be exchanged for that user's proxy credentials. When `AUTH_AUDIENCE` is unset the STS route is never mounted and `/.sts` returns `501 NotImplemented`, rather than being served unrestricted. @@ -113,7 +116,7 @@ flowchart TD ``` > [!WARNING] -> **`exp` is only checked when the claim is present.** Upstream, both time claims are guarded by `if let Some(..)`, so a token carrying no `exp` is accepted and never expires. A missing `aud`, by contrast, is fail-closed. This is harmless with a single trusted issuer that always sets `exp` (Ory does), but it becomes a real exposure the moment ADR-009 admits issuers we do not control — it should be fixed upstream before then. +> **`exp` is only checked when the claim is present.** Upstream, both time claims are guarded by `if let Some(..)`, so a token carrying no `exp` is accepted and never expires. A missing `aud`, by contrast, is fail-closed. This is harmless with a single trusted issuer that always sets `exp` (Ory does), but it becomes a real exposure the moment ADR-009 admits issuers we do not control — it should be fixed upstream before then. #237 admits them and requires `exp` on their tokens itself; the upstream fix, developmentseed/multistore#146, is not yet in a release. Steps 2 and 3 reject **before any network call**: the issuer is matched and the algorithm pinned prior to fetching JWKS, so a token from an untrusted issuer costs nothing to refuse. diff --git a/adrs/009-platform-idps.md b/adrs/009-platform-idps.md index e76ec13a..ece4d4d4 100644 --- a/adrs/009-platform-idps.md +++ b/adrs/009-platform-idps.md @@ -1,6 +1,6 @@ # ADR-009: Multi-Issuer Platform Identity Providers -**Status:** Proposed — not implemented +**Status:** Proposed — implemented in part (#237: platform issuers with per-issuer audiences, whose tokens act as an account that trusts them, per ADR-014) **Date:** 2026-08-09 **RFC:** RFC-001 §7 **Depends on:** ADR-004 @@ -61,6 +61,9 @@ ADR-004's rule — an issuer with no audience restriction disables exchange rath ### Migration +> [!NOTE] +> Superseded in part by ADR-014, as implemented in #237. Platform issuers are not added to `_default`: `AUTH_ISSUER` stays the one person issuer, and `PLATFORM_ISSUERS` maps each platform issuer to its own audiences (step 2). A platform token acts only as the account `RoleArn` names, and only if that account trusts the token's issuer and subject, so the note below no longer applies: a CI token reaches the memberships of one account that trusts it, not those of whoever its subject might map to. + 1. Parse `AUTH_ISSUER` as a comma-separated list, mirroring `AUTH_AUDIENCE`; a single value remains valid, so existing deployments are unaffected. 2. Move the issuer→audience mapping into a structured variable, since a flat pair of lists cannot express per-issuer requirements. 3. Populate `trusted_oidc_issuers` on the `_default` Role from the parsed list. From db891122e732fa0119e26329a3362bada9562ae8 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Fri, 25 Sep 2026 14:38:28 -0700 Subject: [PATCH 6/7] docs(sts): say exactly which trust answers are cached `get_or_fetch_trust` said only a yes is cached. `cached_fetch` caches every 200, so a `200 {"trusted": false}`, which the route does not send today, would be cached too, and still refused. The comments now say that the route's yes is a 200 cached like any other and its no is a 403 that never is. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd --- src/source_api/cache.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/source_api/cache.rs b/src/source_api/cache.rs index 0e952257..2410b9fa 100644 --- a/src/source_api/cache.rs +++ b/src/source_api/cache.rs @@ -211,9 +211,9 @@ pub async fn get_or_fetch_key_standing( /// Whether `account` trusts `issuer`'s `subject` to act as it: `Ok` if so, /// `AccessDenied` if not, the way a role's own trust policy decides an -/// assume-role call. Asked as the account itself. Only a yes is cached, for -/// `TRUST_CACHE_SECS`: the route says no with a 403, and no 403 is cached, so -/// a trust just added works on the next attempt. +/// assume-role call. Asked as the account itself. The route says yes with a +/// 200, cached for `TRUST_CACHE_SECS` like every 200, and no with a 403, which +/// is never cached, so a trust just added works on the next attempt. pub async fn get_or_fetch_trust( api_base_url: &str, account: &str, @@ -254,7 +254,8 @@ pub async fn get_or_fetch_trust( } /// The trusts route's answer. Its status already says yes (200) or no (403); -/// the body is read too, so that a 200 saying no mints nothing. +/// the body is read too, so that a 200 saying no, cached like any 200, still +/// mints nothing. #[derive(serde::Deserialize)] struct TrustAnswer { trusted: bool, From ba94fbc8046e3484b5b471b9e813c2b06eee25c6 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Fri, 25 Sep 2026 14:51:47 -0700 Subject: [PATCH 7/7] fix(sts): name only service accounts and bound replayed platform tokens A platform token's `RoleArn` must now name a service account, `{owner}--{name}` as source.coop's `SERVICE_ACCOUNT_ID_REGEX` defines it (82 characters at most). Anything else is refused as an untrusting account is, before the token is verified and before the proxy signs anything as it. The minted principal is that segment as given, and source.coop resolves a principal as an Ory identity first; an Ory id fits the person and organisation handle grammar, so without this, a trust on such an account would let platform credentials act as a person. source.coop writes trusts only to service accounts today, and ADR-014 makes trusts a service account's, so this is defense in depth. Anyone can mint a GitHub token for the proxy's audience in their own workflow, and a refusal was not cached, so replaying one valid token cost a trusts lookup per request. Two bounds follow. The per-address rate limit from the API-key exchange now also covers platform exchanges, after everything local and before the trusts lookup; the binding is renamed `STS_EXCHANGE_LIMIT`, since it now bounds every exchange that costs a Source API call, and stays at 100 a minute. And a refusal from the trusts route is cached for 10 seconds under its own key, so a replay flood from many addresses costs about one lookup per 10 seconds per account, issuer and subject, while a trust just added still works within seconds. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd --- README.md | 4 +- src/lib.rs | 127 ++++++++++++++++++++++------------- src/source_api/cache.rs | 37 +++++++--- src/sts.rs | 21 ++++++ tests/sts.rs | 41 +++++++++++ tests/test_platform_trust.py | 23 ++++++- wrangler.preview.toml | 5 +- wrangler.toml | 13 ++-- 8 files changed, 203 insertions(+), 68 deletions(-) diff --git a/README.md b/README.md index 9232d39d..564bf865 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,7 @@ Set in `wrangler.toml` or via the Cloudflare dashboard: | Binding | Kind | Description | | -------------------- | ----------- | -------------------------------------------------------------------------------------------------------------------------------------------- | -| `KEY_EXCHANGE_LIMIT` | `ratelimit` | Per-client-IP limit on API-key exchanges at `/.sts` (ADR-013). Declared under `[[unsafe.bindings]]` in every `wrangler*.toml`; a deployment without it logs an error and exchanges without a limit | +| `STS_EXCHANGE_LIMIT` | `ratelimit` | Per-client-IP limit on `/.sts` exchanges that cost a Source API call: API keys (ADR-013) and platform tokens (ADR-014). Declared under `[[unsafe.bindings]]` in every `wrangler*.toml`; a deployment without it logs an error and exchanges without a limit | ### API keys @@ -141,7 +141,7 @@ Any other name is refused with `MalformedPolicyDocument`, never mapped to a defa ### Platform identity providers -A token from a platform issuer in `PLATFORM_ISSUERS`, such as GitHub Actions, says which workload is calling but not which account it may act as. At `/.sts` it acts as the account in `RoleArn`, `arn:aws:iam:::role/FullAccess`, and only if that account trusts the token's issuer and subject (ADR-014). The proxy verifies the token against the issuer's JWKS, with that issuer's own audiences and a required `exp`, then asks `POST {SOURCE_API_URL}/api/v1/accounts/{account}/trusts/exchanges` with `{"issuer", "subject"}`, as the account. A yes is cached for 60 seconds per account, issuer and subject, and the credentials' principal is the account, never the token's subject. Any other answer reads `AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity (request id …)`. A token from `AUTH_ISSUER` still acts as its own subject and ignores the account in `RoleArn`. +A token from a platform issuer in `PLATFORM_ISSUERS`, such as GitHub Actions, says which workload is calling but not which account it may act as. At `/.sts` it acts as the service account in `RoleArn`, `arn:aws:iam::--:role/FullAccess`, and only if that account trusts the token's issuer and subject (ADR-014); an account that is not a service account is refused before anything else. The proxy verifies the token against the issuer's JWKS, with that issuer's own audiences and a required `exp`, then, within `STS_EXCHANGE_LIMIT`, asks `POST {SOURCE_API_URL}/api/v1/accounts/{account}/trusts/exchanges` with `{"issuer", "subject"}`, as the account. Per account, issuer and subject, a yes is cached for 60 seconds and a no for 10, and the credentials' principal is the account, never the token's subject. Every refusal reads `AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity (request id …)`. A token from `AUTH_ISSUER` still acts as its own subject and ignores the account in `RoleArn`. `aws-actions/configure-aws-credentials` fails after the exchange succeeds: it checks the credentials it exports with `GetCallerIdentity`, which the proxy cannot answer until developmentseed/multistore#126 lands. Until then a workflow saves its token to a file and lets an AWS SDK exchange it, with `AWS_WEB_IDENTITY_TOKEN_FILE`, `AWS_ROLE_ARN`, `AWS_ENDPOINT_URL_STS=/.sts`, `AWS_ENDPOINT_URL_S3=` and `AWS_REGION`. diff --git a/src/lib.rs b/src/lib.rs index eab40117..6735e40a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -247,7 +247,8 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result web_sys::Response { response } -/// The rate-limiter binding for API-key exchanges, keyed by client IP. -const KEY_EXCHANGE_LIMIT: &str = "KEY_EXCHANGE_LIMIT"; +/// The rate-limiter binding for `/.sts` exchanges that cost a Source API call, +/// of API keys and platform tokens alike, keyed by client IP. +const STS_EXCHANGE_LIMIT: &str = "STS_EXCHANGE_LIMIT"; /// The API-key exchange, if this request is one: `None` when it is not an /// `AssumeRoleWithWebIdentity` carrying an `sck_` key, so the STS route takes @@ -527,13 +529,7 @@ async fn api_key_exchange( let client_ip = header_str(&parts.headers, "cf-connecting-ip"); if !within_rate_limit(env, client_ip).await { tracing::warn!(%request_id, reason = "rate_limited", "API key exchange refused"); - return Some(( - 429, - sts_error_xml( - "Throttling", - "too many API key exchanges from this address; retry later", - ), - )); + return Some(throttled()); } Some( @@ -637,7 +633,7 @@ async fn within_rate_limit(env: &Env, client_ip: &str) -> bool { } else { client_ip }; - match env.rate_limiter(KEY_EXCHANGE_LIMIT) { + match env.rate_limiter(STS_EXCHANGE_LIMIT) { Ok(limiter) => match limiter.limit(key.to_string()).await { Ok(outcome) => outcome.success, Err(e) => { @@ -647,13 +643,24 @@ async fn within_rate_limit(env: &Env, client_ip: &str) -> bool { }, Err(_) => { tracing::error!( - "{KEY_EXCHANGE_LIMIT} binding is not configured; API-key exchanges are unlimited" + "{STS_EXCHANGE_LIMIT} binding is not configured; exchanges are unlimited" ); true } } } +/// The answer to an exchange over `STS_EXCHANGE_LIMIT`, which SDKs back off on. +fn throttled() -> (u16, String) { + ( + 429, + sts_error_xml( + "Throttling", + "too many exchanges from this address; retry later", + ), + ) +} + /// An STS-shaped error body with a code or message `build_sts_error_response` /// does not produce. fn sts_error_xml(code: &str, message: &str) -> String { @@ -666,12 +673,11 @@ fn sts_error_xml(code: &str, message: &str) -> String { /// The exchange of a platform issuer's token, if this request carries one: /// `None` for any other token, which the STS route takes. Parameters come from -/// the query string or the form body, never both, as at the STS route. Every -/// refusal of the account's trust reads the same, whether the account does not -/// exist or does not trust the token. +/// the query string or the form body, never both, as at the STS route. async fn platform_exchange( config: &AppConfig, parts: &RequestParts, + env: &Env, api_auth: &ApiAuth, request_id: &str, ) -> Option<(u16, String)> { @@ -681,58 +687,70 @@ async fn platform_exchange( let (header, claims) = platform::unverified(&sts.web_identity_token)?; let issuer = claims.get("iss")?.as_str()?; let audiences = config.platform_issuers.get(issuer)?; + let client_ip = header_str(&parts.headers, "cf-connecting-ip"); Some( match exchange_platform_token( - config, &sts, &header, issuer, audiences, api_auth, request_id, + config, env, client_ip, &sts, &header, issuer, audiences, api_auth, request_id, ) .await { Ok(creds) => build_sts_response(&creds), - // `exchange_platform_token` has logged who asked to act as whom. - Err(ProxyError::AccessDenied) => ( - 403, - sts_error_xml( - "AccessDenied", - &with_request_id( - "Not authorized to perform sts:AssumeRoleWithWebIdentity", - request_id, - ), - ), - ), - Err(e) => { - tracing::warn!(%request_id, %issuer, error = %e, "platform token exchange failed"); - build_sts_error_response(&e) - } + Err(response) => response, }, ) } -/// Verify a platform issuer's token, then mint for the account `RoleArn` -/// names if that account trusts the token's issuer and subject (ADR-014). The -/// credentials act as the account, never as the token's subject. Everything -/// local comes first, so a token that fails it costs the Source API nothing. +/// Verify a platform issuer's token, then mint for the service account +/// `RoleArn` names if that account trusts the token's issuer and subject +/// (ADR-014). The credentials act as the account, never as the token's +/// subject. Everything local comes first, so a token that fails it costs the +/// Source API nothing; what does cost a call is rate-limited per address. +/// Every refusal of the account's trust reads the same, whatever the reason. +#[allow(clippy::too_many_arguments)] async fn exchange_platform_token( config: &AppConfig, + env: &Env, + client_ip: &str, sts: &multistore_sts::request::StsRequest, header: &serde_json::Value, issuer: &str, audiences: &[String], api_auth: &ApiAuth, request_id: &str, -) -> Result { +) -> Result { + let failed = |e: ProxyError| { + tracing::warn!(%request_id, %issuer, error = %e, "platform token exchange failed"); + build_sts_error_response(&e) + }; + let not_authorized = || { + let message = "Not authorized to perform sts:AssumeRoleWithWebIdentity"; + ( + 403, + sts_error_xml("AccessDenied", &with_request_id(message, request_id)), + ) + }; + let role = sts::role( &sts.role_arn, issuer.to_string(), audiences.to_vec(), config.sts_max_session_duration_secs, ) - .ok_or_else(|| ProxyError::RoleNotFound(sts.role_arn.clone()))?; + .ok_or_else(|| failed(ProxyError::RoleNotFound(sts.role_arn.clone())))?; // No angle brackets in the message: the STS error body carries it unescaped. let account = sts::account(&sts.role_arn).ok_or_else(|| { - ProxyError::InvalidRequest( + failed(ProxyError::InvalidRequest( "RoleArn must name the account to act as: arn:aws:iam::ACCOUNT:role/ROLE".into(), - ) + )) })?; + // Only a service account trusts subjects (ADR-014), and the credentials' + // principal is this segment as given, which source.coop tries as an Ory + // identity first. So anything else is refused before the token is + // verified or anything is signed as it. + if !sts::is_service_account_id(account) { + tracing::warn!(%request_id, %issuer, %account, "RoleArn names no service account"); + return Err(not_authorized()); + } let subject = platform::verify( &sts.web_identity_token, header, @@ -740,8 +758,15 @@ async fn exchange_platform_token( &role, &jwks_cache(), ) - .await?; - source_api::cache::get_or_fetch_trust( + .await + .map_err(failed)?; + // Anyone can mint a token for this audience in their own workflow, and + // every exchange from here on may cost the Source API a call. + if !within_rate_limit(env, client_ip).await { + tracing::warn!(%request_id, %issuer, reason = "rate_limited", "platform token exchange refused"); + return Err(throttled()); + } + match source_api::cache::get_or_fetch_trust( &config.api_base_url, account, issuer, @@ -750,22 +775,28 @@ async fn exchange_platform_token( request_id, ) .await - .map_err(|e| match e { - ProxyError::AccessDenied => { + { + Ok(()) => {} + Err(ProxyError::AccessDenied) => { tracing::warn!(%request_id, %issuer, %subject, %account, "account does not trust the token"); - e + return Err(not_authorized()); } // The route answers for any account; a 404 means the API does not // serve it, which is a deployment mismatch, not a refusal. - ProxyError::BucketNotFound(_) => ProxyError::Internal("trusts route not found".into()), - e => e, - })?; + Err(ProxyError::BucketNotFound(_)) => { + return Err(failed(ProxyError::Internal( + "trusts route not found".into(), + ))) + } + Err(e) => return Err(failed(e)), + } let creds = keys::credentials_for( &role, account, sts.duration_seconds, &config.session_token_key, - )?; + ) + .map_err(failed)?; tracing::info!(%request_id, %issuer, %subject, %account, role = %role.role_id, "platform token exchanged"); Ok(creds) } diff --git a/src/source_api/cache.rs b/src/source_api/cache.rs index 2410b9fa..bb4f3175 100644 --- a/src/source_api/cache.rs +++ b/src/source_api/cache.rs @@ -57,6 +57,12 @@ const KEY_STANDING_CACHE_SECS: u32 = 60; // 1 minute /// reason: a trust that is removed should stop minting quickly (ADR-014). const TRUST_CACHE_SECS: u32 = 60; // 1 minute +/// A refusal from the trusts route, cached under its own key: long enough that +/// replaying one token its account does not trust costs about one lookup per +/// 10 seconds, however many addresses it comes from, and short enough that a +/// trust just added works within seconds. +const REFUSED_TRUST_CACHE_SECS: u32 = 10; + // ── Public cache functions ───────────────────────────────────────── /// Fetch a single product's metadata, cached for `PRODUCT_CACHE_SECS`. @@ -212,8 +218,9 @@ pub async fn get_or_fetch_key_standing( /// Whether `account` trusts `issuer`'s `subject` to act as it: `Ok` if so, /// `AccessDenied` if not, the way a role's own trust policy decides an /// assume-role call. Asked as the account itself. The route says yes with a -/// 200, cached for `TRUST_CACHE_SECS` like every 200, and no with a 403, which -/// is never cached, so a trust just added works on the next attempt. +/// 200, cached for `TRUST_CACHE_SECS` like every 200, and no with a 403 (a 401 +/// for an account it cannot resolve), which `cached_fetch` leaves uncached and +/// this caches for `REFUSED_TRUST_CACHE_SECS` under a key of its own. pub async fn get_or_fetch_trust( api_base_url: &str, account: &str, @@ -234,8 +241,13 @@ pub async fn get_or_fetch_trust( utf8_percent_encode(issuer, PATH_SEGMENT), utf8_percent_encode(subject, PATH_SEGMENT), ); + let refused_key = format!("{cache_key}&refused"); + let cache = worker::Cache::default(); + if matches!(cache.get(&refused_key, false).await, Ok(Some(_))) { + return Err(ProxyError::AccessDenied); + } let body = serde_json::json!({ "issuer": issuer, "subject": subject }).to_string(); - let answer: TrustAnswer = cached_fetch( + let answer = cached_fetch::( &cache_key, &api_url, "POST", @@ -245,8 +257,11 @@ pub async fn get_or_fetch_trust( request_id, ApiCaller::Account(account), ) - .await?; - if answer.trusted { + .await; + if matches!(answer, Err(ProxyError::AccessDenied)) { + cache_put(&cache, &refused_key, "{}", REFUSED_TRUST_CACHE_SECS).await; + } + if answer?.trusted { Ok(()) } else { Err(ProxyError::AccessDenied) @@ -379,16 +394,20 @@ async fn cached_fetch( let result: T = serde_json::from_str(&text) .map_err(|e| ProxyError::Internal(format!("JSON parse failed: {} for {}", e, api_url)))?; - // ── Store in cache ───────────────────────────────────────── + cache_put(&cache, cache_key, &text, ttl_secs).await; + Ok(result) +} + +/// Store `text` under `cache_key` for `ttl_secs`. A failed put costs only a +/// later lookup, so it is logged rather than returned. +async fn cache_put(cache: &worker::Cache, cache_key: &str, text: &str, ttl_secs: u32) { let headers = worker::Headers::new(); let _ = headers.set("content-type", "application/json"); let _ = headers.set("cache-control", &format!("max-age={}", ttl_secs)); - if let Ok(cache_resp) = worker::Response::ok(&text) { + if let Ok(cache_resp) = worker::Response::ok(text) { let cache_resp = cache_resp.with_headers(headers); if let Err(e) = cache.put(cache_key, cache_resp).await { tracing::warn!("cache put failed: {}", e); } } - - Ok(result) } diff --git a/src/sts.rs b/src/sts.rs index dc85ca2a..db2f080a 100644 --- a/src/sts.rs +++ b/src/sts.rs @@ -110,6 +110,27 @@ pub(crate) fn account(role_arn: &str) -> Option<&str> { } } +/// Whether `id` is a service account's id, `{owner}--{name}`: source.coop's +/// `SERVICE_ACCOUNT_ID_REGEX` and its 82-character limit. Each half is at least +/// two of `a-z`, `0-9` and inner single hyphens, so the one `--` is the +/// separator, and no person's or organisation's handle, nor an Ory identity +/// id, can match. +pub(crate) fn is_service_account_id(id: &str) -> bool { + let half = |part: &str| { + part.len() >= 2 + && part + .bytes() + .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'-') + && !part.starts_with('-') + && !part.ends_with('-') + && !part.contains("--") + }; + id.len() <= 82 + && id + .split_once("--") + .is_some_and(|(owner, name)| half(owner) && half(name)) +} + impl CredentialRegistry for StsCredentialRegistry { async fn get_credential( &self, diff --git a/tests/sts.rs b/tests/sts.rs index 1dab3f71..e97bcedc 100644 --- a/tests/sts.rs +++ b/tests/sts.rs @@ -85,3 +85,44 @@ fn the_account_is_the_arns_account_segment() { assert_eq!(sts::account(role_arn), None, "{role_arn}"); } } + +#[test] +fn a_service_account_id_is_owner_dash_dash_name() { + let longest = format!("{}--{}", "a".repeat(40), "b".repeat(40)); + for id in [ + "acme--nightly-sync", + "ab--cd", + "my-org-1--a1-b2", + longest.as_str(), + ] { + assert!(sts::is_service_account_id(id), "{id}"); + } +} + +#[test] +fn nothing_else_is_a_service_account_id() { + let too_long = format!("{}--{}", "a".repeat(40), "b".repeat(41)); + for id in [ + "", + // A person's or organisation's handle. + "alice", + "my-org", + // An Ory identity id, which fits the handle grammar. + "2c5b4f0e-8a3b-4e2d-9a1f-3c4d5e6f7a8b", + "000000000000", + "Acme--sync", + "acme--sync_1", + "acme--", + "--sync", + "a--sync", + "acme--s", + "acme---sync", + "acme--sync--x", + "-acme--sync", + "acme--sync-", + "acme---", + too_long.as_str(), + ] { + assert!(!sts::is_service_account_id(id), "{id}"); + } +} diff --git a/tests/test_platform_trust.py b/tests/test_platform_trust.py index 9974965d..46f1aa88 100644 --- a/tests/test_platform_trust.py +++ b/tests/test_platform_trust.py @@ -63,6 +63,22 @@ def test_a_platform_token_must_name_the_account_it_acts_as(): assert "RoleArn must name the account" in sts_fields(resp)["Message"] +@pytest.mark.parametrize( + "account", + ["alice", "2c5b4f0e-8a3b-4e2d-9a1f-3c4d5e6f7a8b", "000000000000"], + ids=["person", "ory-identity-id", "placeholder"], +) +def test_only_a_service_account_can_be_named(account): + """Refused as an account that does not trust the token is, and before the + token is verified: this one is forged, and still no lookup happens.""" + resp = exchange(forged(), as_account(account)) + assert resp.status_code == 403 + assert sts_fields(resp)["Message"] == ( + f"Not authorized to perform sts:AssumeRoleWithWebIdentity (request id {RAY})" + ) + assert trust_lookups(account) == 0 + + def test_a_forged_token_is_refused_before_any_trust_lookup(): before = trust_lookups(TRUST_ACCOUNT) resp = exchange(forged(), as_account(TRUST_ACCOUNT)) @@ -101,13 +117,18 @@ def test_a_trusted_workflow_gets_credentials_that_act_as_the_account(): @needs_token def test_an_account_that_does_not_trust_the_workflow_refuses_it(): - resp = exchange(ID_TOKEN, as_account("ci-tests--someone-else")) + untrusting = as_account("ci-tests--someone-else") + resp = exchange(ID_TOKEN, untrusting) assert resp.status_code == 403 fields = sts_fields(resp) assert fields["Code"] == "AccessDenied" assert fields["Message"] == ( f"Not authorized to perform sts:AssumeRoleWithWebIdentity (request id {RAY})" ) + # The refusal is cached briefly, so replaying the token costs no lookup. + before = trust_lookups("ci-tests--someone-else") + assert exchange(ID_TOKEN, untrusting).status_code == 403 + assert trust_lookups("ci-tests--someone-else") == before @needs_token diff --git a/wrangler.preview.toml b/wrangler.preview.toml index 31b52248..f3c45c51 100644 --- a/wrangler.preview.toml +++ b/wrangler.preview.toml @@ -52,9 +52,10 @@ dataset = "source_data_proxy_staging" binding = "PUBLIC_LOG_STREAM" service = "public-log-stream-staging" -# API-key exchange rate limit, per client IP; see wrangler.toml. +# Limit on /.sts exchanges that cost a Source API call, per client IP; see +# wrangler.toml. [[unsafe.bindings]] -name = "KEY_EXCHANGE_LIMIT" +name = "STS_EXCHANGE_LIMIT" type = "ratelimit" namespace_id = "1003" simple = { limit = 100, period = 60 } diff --git a/wrangler.toml b/wrangler.toml index 6b3d3a9d..16970621 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -75,12 +75,13 @@ dataset = "source_data_proxy_production" binding = "PUBLIC_LOG_STREAM" service = "public-log-stream" -# API-key exchanges at /.sts, per client IP. Every attempt costs the Source -# API one lookup for a distinct key, so this bounds a flood of junk keys from -# one place; a legitimate client exchanges about once a session, so even a -# cluster behind one NAT stays far under it. See src/lib.rs `api_key_exchange`. +# Exchanges at /.sts that cost a Source API call (API keys and platform +# tokens), per client IP. It bounds a flood of junk keys, or of one replayed +# platform token, from one place; a legitimate client exchanges about once a +# session or job, so even a cluster behind one NAT stays far under it. See +# src/lib.rs `within_rate_limit`. [[unsafe.bindings]] -name = "KEY_EXCHANGE_LIMIT" +name = "STS_EXCHANGE_LIMIT" type = "ratelimit" namespace_id = "1001" simple = { limit = 100, period = 60 } @@ -110,7 +111,7 @@ binding = "PUBLIC_LOG_STREAM" service = "public-log-stream-staging" [[env.staging.unsafe.bindings]] -name = "KEY_EXCHANGE_LIMIT" +name = "STS_EXCHANGE_LIMIT" type = "ratelimit" namespace_id = "1002" simple = { limit = 100, period = 60 }