diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0012e058..488165d0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -109,16 +109,19 @@ jobs: # 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. + # token acts as an account the stub says trusts this repository. + # AUTH_ISSUER names an issuer that mints nothing. AUTH_AUDIENCE is the + # wrong-audience token's audience: if the platform path ever checked + # the person audiences instead of GitHub's own, test_writes.py's + # audience tests would fail. The person route runs on a second worker; + # see "Start wrangler dev". 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://auth.example.invalid" - echo "AUTH_AUDIENCE=source-data-proxy-ci" + echo "AUTH_AUDIENCE=not-the-data-proxy" echo 'PLATFORM_ISSUERS={"https://token.actions.githubusercontent.com": ["source-data-proxy-ci"]}' echo "SOURCE_API_URL=http://localhost:9000" } > .dev.vars @@ -148,6 +151,10 @@ jobs: fi echo "::add-mask::$token" echo "CI_WRITE_ID_TOKEN=$token" >> "$GITHUB_ENV" + # The stub trusts exactly this token's subject, as source.coop + # matches a trust (see tests/stub_api.py). + sub=$(python3 -c 'import base64,json,sys; p=sys.argv[1].split(".")[1]; print(json.loads(base64.urlsafe_b64decode(p + "=" * (-len(p) % 4)))["sub"])' "$token") + echo "CI_TRUSTED_SUBJECT=$sub" >> "$GITHUB_ENV" # 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). @@ -175,8 +182,15 @@ jobs: # may appear later in the config are untouched. sed -i '/^\[build\]/,/^\[/ s/^command = .*/command = "echo skip"/' wrangler.toml wrangler dev --port 8787 > /tmp/wrangler.log 2>&1 & + # A second worker whose person issuer is GitHub, so CI's token + # exercises the person route (tests/test_person_route.py). Its + # PLATFORM_ISSUERS still names GitHub, which the proxy drops at load. + wrangler dev --port 8788 --inspector-port 9230 --persist-to /tmp/wrangler-person \ + --var AUTH_ISSUER:https://token.actions.githubusercontent.com \ + --var AUTH_AUDIENCE:source-data-proxy-ci > /tmp/wrangler-person.log 2>&1 & curl --retry 120 --retry-delay 1 --retry-connrefused --silent --fail http://localhost:8787/ > /dev/null - echo "Server ready" + curl --retry 120 --retry-delay 1 --retry-connrefused --silent --fail http://localhost:8788/ > /dev/null + echo "Servers ready" - name: Run integration tests # Contract tests are excluded: they hit the live prod API, so they run # on a schedule instead (.github/workflows/contract.yml) — a prod @@ -186,6 +200,8 @@ jobs: # var went missing (renamed, mint plumbing broken), the tests fail # loudly instead of skipping green (see tests/test_writes.py). CI_EXPECT_OIDC: ${{ github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository }} + # Unset on fork PRs, which have no token to exchange there. + PERSON_PROXY_URL: ${{ env.CI_WRITE_ID_TOKEN && 'http://localhost:8788' || '' }} run: uvx --with requests --with boto3 pytest tests/ -v --ignore=tests/test_contract.py - name: Dump server logs # Both servers run in the background, so their output isn't captured @@ -201,6 +217,9 @@ jobs: echo "::group::wrangler dev" cat /tmp/wrangler.log || true echo "::endgroup::" + echo "::group::wrangler dev (person issuer)" + cat /tmp/wrangler-person.log || true + echo "::endgroup::" audit: name: Security Audit diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index a8e24df2..f7564546 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -62,9 +62,10 @@ jobs: # 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 != '' + # account. Until both are set no token is minted and the copy-source + # authz test skips; the rest of the suite is unaffected. (A token with + # no trust account would name the stub's, which staging lacks.) + if: vars.FEDERATION_TEST_AUDIENCE != '' && vars.FEDERATION_TEST_TRUST_ACCOUNT != '' run: | set -euo pipefail token=$(curl -sSf -H "Authorization: bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \ diff --git a/README.md b/README.md index 6c4a0227..6b20a90f 100644 --- a/README.md +++ b/README.md @@ -111,7 +111,7 @@ 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` | 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) | +| `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 = person-token exchange at `/.sts` is disabled (returns 501); API keys and platform tokens still exchange | | `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 | diff --git a/src/config.rs b/src/config.rs index cbc1e8ed..8e77f0b5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -97,16 +97,23 @@ fn build_config(env: &Env) -> AppConfig { if auth_audiences.is_empty() { // Fail closed: without an audience restriction, an ID token minted for // ANY OAuth client of AUTH_ISSUER could be exchanged for a user's - // credentials, so /.sts is disabled entirely (returns 501) until set. - tracing::warn!("AUTH_AUDIENCE not set: /.sts token exchange is disabled (returns 501)"); + // credentials, so its exchange is disabled (returns 501) until set. + tracing::warn!( + "AUTH_AUDIENCE not set: person-token exchange at /.sts is disabled (returns 501)" + ); } // Platform identity providers (GitHub Actions, say), each with its own // audiences. Unset trusts none. - let platform_issuers = env + let mut platform_issuers = env .var("PLATFORM_ISSUERS") .map(|v| crate::platform::parse_issuers(&v.to_string())) .unwrap_or_default(); + // The platform path claims its issuers' tokens ahead of the STS route, so + // an entry for the person issuer would refuse every person exchange. + if platform_issuers.remove(&auth_issuer).is_some() { + tracing::error!(issuer = %auth_issuer, "PLATFORM_ISSUERS names AUTH_ISSUER; ignoring that entry"); + } // Ceiling for client-requested DurationSeconds on /.sts. Unset → 3600 (1h), // matching multistore's own default so behavior is unchanged until raised. diff --git a/src/lib.rs b/src/lib.rs index 6a835f11..9d6f8b28 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -171,23 +171,6 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result Result { tracing::warn!(%request_id, error = %e, "API key exchange failed"); - build_sts_error_response(&e) + sts_refusal(&e, request_id) } }, ) @@ -559,9 +560,14 @@ async fn api_key_exchange( /// `InvalidIdentityToken`, with the request id in the message. fn key_refusal(message: &str, request_id: &str) -> (u16, String) { - build_sts_error_response(&ProxyError::InvalidOidcToken(with_request_id( - message, request_id, - ))) + sts_refusal(&ProxyError::InvalidOidcToken(message.into()), request_id) +} + +/// `build_sts_error_response`, with the request id in the message. +fn sts_refusal(e: &ProxyError, request_id: &str) -> (u16, String) { + let (status, xml) = build_sts_error_response(e); + let message_end = format!("{}", with_request_id("", request_id)); + (status, xml.replacen("", &message_end, 1)) } /// `message` with the request id, if there is one: SDKs show a user the @@ -688,9 +694,12 @@ async fn platform_exchange( api_auth: &ApiAuth, request_id: &str, ) -> Option<(u16, String)> { - let sts = try_parse_sts_request(parts.query.as_deref()) + let mut sts = try_parse_sts_request(parts.query.as_deref()) .or_else(|| try_parse_sts_request(parts.form_body.as_deref()))? .ok()?; + // SDKs send a token file's contents as-is, and `jq -r … > file` ends it in + // a newline, which the signature segment's base64 decode rejects. + sts.web_identity_token = sts.web_identity_token.trim().to_string(); let (header, claims) = platform::unverified(&sts.web_identity_token)?; let issuer = claims.get("iss")?.as_str()?; let audiences = config.platform_issuers.get(issuer)?; @@ -740,7 +749,7 @@ async fn exchange_platform_token( } = token; let failed = |e: ProxyError| { tracing::warn!(%request_id, %issuer, error = %e, "platform token exchange failed"); - build_sts_error_response(&e) + sts_refusal(&e, request_id) }; let not_authorized = || { let message = "Not authorized to perform sts:AssumeRoleWithWebIdentity"; diff --git a/src/source_api/cache.rs b/src/source_api/cache.rs index 9c764cef..a06d8b07 100644 --- a/src/source_api/cache.rs +++ b/src/source_api/cache.rs @@ -57,10 +57,9 @@ 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. +/// A refusal from the trusts route: long enough that replaying one token its +/// account does not trust costs about one lookup per 10 seconds per data +/// center, and short enough that a trust just added works within seconds. const REFUSED_TRUST_CACHE_SECS: u32 = 10; // ── Public cache functions ───────────────────────────────────────── @@ -220,7 +219,7 @@ pub async fn get_or_fetch_key_standing( /// 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 (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. +/// this caches as `{"trusted":false}` for `REFUSED_TRUST_CACHE_SECS`. pub async fn get_or_fetch_trust( api_base_url: &str, account: &str, @@ -241,11 +240,6 @@ 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 = cached_fetch::( &cache_key, @@ -260,17 +254,21 @@ pub async fn get_or_fetch_trust( .await; let trusted = match answer { Ok(answer) => answer.trusted, - Err(ProxyError::AccessDenied) => false, + Err(ProxyError::AccessDenied) => { + let cache = worker::Cache::default(); + let refused = r#"{"trusted":false}"#; + cache_put(&cache, &cache_key, refused, REFUSED_TRUST_CACHE_SECS).await; + false + } Err(e) => return Err(e), }; - if !trusted { - // A 200 saying no was cached like any 200: drop it, so a no is held - // for `REFUSED_TRUST_CACHE_SECS` whichever way the route said it. - let _ = cache.delete(cache_key.as_str(), false).await; - cache_put(&cache, &refused_key, "{}", REFUSED_TRUST_CACHE_SECS).await; - return Err(ProxyError::AccessDenied); + // ponytail: a 200 saying no (which the route never sends) is held for + // TRUST_CACHE_SECS like any 200; still refused, only slower to flip to yes. + if trusted { + Ok(()) + } else { + Err(ProxyError::AccessDenied) } - Ok(()) } /// The trusts route's answer. Its status already says yes (200) or no (403); diff --git a/tests/stub_api.py b/tests/stub_api.py index 6b362e19..457a22f7 100644 --- a/tests/stub_api.py +++ b/tests/stub_api.py @@ -158,12 +158,15 @@ def _hash(key): # ── 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. +# account itself. Only TRUST_ACCOUNT trusts anyone, and only the exact subject +# of the token CI minted for this run (CI_TRUSTED_SUBJECT, set by ci.yml), as +# source.coop matches a trust exactly. SAYS_NO_ACCOUNT answers no with a 200, +# which the real route never does, so the proxy's read of the body is pinned. +# A counter per account lets test_platform_trust.py prove the proxy caches. TRUST_ACCOUNT = "ci-tests--github-ci" +SAYS_NO_ACCOUNT = "ci-tests--says-no-with-200" TRUSTED_ISSUER = "https://token.actions.githubusercontent.com" -TRUSTED_SUBJECT_PREFIX = "repo:source-cooperative/data.source.coop:" +TRUSTED_SUBJECT = os.environ.get("CI_TRUSTED_SUBJECT") TRUST_EXCHANGE_COUNTS = {} # Who the proxy said it was asking as, per product path, so a test can check @@ -212,10 +215,12 @@ def _trust_exchange(self, account): except (ValueError, KeyError, TypeError): return self._send(400, b"{}") TRUST_EXCHANGE_COUNTS[account] = TRUST_EXCHANGE_COUNTS.get(account, 0) + 1 + if account == SAYS_NO_ACCOUNT: + return self._send(200, b'{"trusted": false}') trusted = ( account == TRUST_ACCOUNT and issuer == TRUSTED_ISSUER - and subject.startswith(TRUSTED_SUBJECT_PREFIX) + and subject == TRUSTED_SUBJECT ) self._send(200 if trusted else 403, json.dumps({"trusted": trusted}).encode()) diff --git a/tests/test_person_route.py b/tests/test_person_route.py new file mode 100644 index 00000000..f5c03c07 --- /dev/null +++ b/tests/test_person_route.py @@ -0,0 +1,62 @@ +"""The person issuer's STS route, with a validly signed token. + +CI's main worker has no person issuer that mints anything. A second worker +(PERSON_PROXY_URL in ci.yml) names GitHub Actions as AUTH_ISSUER instead, so +CI's GitHub token is a person token there: it exchanges at `_default` and acts +as its own subject. That worker's PLATFORM_ISSUERS also names GitHub, which +the proxy drops at load, so these tests also pin that the person route wins. +""" + +import os +import xml.etree.ElementTree as ET + +import pytest +import requests + +from test_writes import ID_TOKEN, WRONG_AUD_TOKEN + +PERSON_PROXY_URL = os.environ.get("PERSON_PROXY_URL") + +pytestmark = pytest.mark.skipif( + not PERSON_PROXY_URL, reason="person-issuer worker not configured (set PERSON_PROXY_URL)" +) + + +def exchange(token): + params = {"Action": "AssumeRoleWithWebIdentity", "RoleArn": "_default", "WebIdentityToken": token} + return requests.post(f"{PERSON_PROXY_URL}/.sts", data=params) + + +def test_a_person_token_gets_credentials_that_sign(): + assert ID_TOKEN, "PERSON_PROXY_URL is set but CI_WRITE_ID_TOKEN is not" + import boto3 + from botocore.config import Config + + resp = exchange(ID_TOKEN) + assert resp.status_code == 200, resp.text[:300] + fields = {el.tag.rpartition("}")[2]: el.text for el in ET.fromstring(resp.text).iter()} + client = boto3.client( + "s3", + endpoint_url=PERSON_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-scoped list runs SigV4 verification, which unseals the token. + client.list_objects_v2(Bucket="cholmes", Prefix="admin-boundaries/", MaxKeys=1) + + +def test_a_person_token_for_another_audience_is_refused(): + assert WRONG_AUD_TOKEN, "PERSON_PROXY_URL is set but CI_WRONG_AUDIENCE_TOKEN is not" + resp = exchange(WRONG_AUD_TOKEN) + assert resp.status_code == 400, resp.text[:300] + assert "InvalidIdentityToken" in resp.text + + +def test_a_person_token_with_a_tampered_signature_is_refused(): + assert ID_TOKEN, "PERSON_PROXY_URL is set but CI_WRITE_ID_TOKEN is not" + tampered = ID_TOKEN[:-1] + ("A" if ID_TOKEN[-1] != "A" else "B") + resp = exchange(tampered) + assert resp.status_code == 400, resp.text[:300] diff --git a/tests/test_platform_trust.py b/tests/test_platform_trust.py index 46f1aa88..d53fb230 100644 --- a/tests/test_platform_trust.py +++ b/tests/test_platform_trust.py @@ -16,7 +16,7 @@ import pytest import requests -from stub_api import TRUST_ACCOUNT, TRUSTED_ISSUER, WRITE_ACCOUNT +from stub_api import SAYS_NO_ACCOUNT, TRUST_ACCOUNT, TRUSTED_ISSUER, WRITE_ACCOUNT from test_writes import ID_TOKEN, PROXY_URL, needs_token STUB_URL = "http://localhost:9000" @@ -83,7 +83,9 @@ 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" + fields = sts_fields(resp) + assert fields["Code"] == "InvalidIdentityToken" + assert fields["Message"].endswith(f"(request id {RAY})") assert trust_lookups(TRUST_ACCOUNT) == before @@ -115,6 +117,20 @@ def test_a_trusted_workflow_gets_credentials_that_act_as_the_account(): assert subjects[f"/api/v1/products/{WRITE_ACCOUNT}/{product}"] == TRUST_ACCOUNT +@needs_token +def test_a_token_read_from_a_file_may_end_in_a_newline(): + """SDKs send AWS_WEB_IDENTITY_TOKEN_FILE's contents untrimmed.""" + resp = exchange(ID_TOKEN + "\n", as_account(TRUST_ACCOUNT)) + assert resp.status_code == 200, resp.text[:300] + + +@needs_token +def test_a_200_that_says_no_is_still_a_refusal(): + resp = exchange(ID_TOKEN, as_account(SAYS_NO_ACCOUNT)) + assert resp.status_code == 403, resp.text[:300] + assert sts_fields(resp)["Code"] == "AccessDenied" + + @needs_token def test_an_account_that_does_not_trust_the_workflow_refuses_it(): untrusting = as_account("ci-tests--someone-else") diff --git a/wrangler.preview.toml b/wrangler.preview.toml index 4262d9be..23810e61 100644 --- a/wrangler.preview.toml +++ b/wrangler.preview.toml @@ -18,7 +18,7 @@ OIDC_PROVIDER_KID = "data-proxy-1" # Preview is staging-equivalent (shares staging analytics + log stream), so # trust the staging Ory issuer and accept the staging frontend + CLI client_ids. -# AUTH_AUDIENCE must be set or /.sts fail-closes with 501. +# AUTH_AUDIENCE must be set or person-token exchange at /.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 diff --git a/wrangler.toml b/wrangler.toml index ce6b814d..564e0382 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -45,7 +45,7 @@ STS_MAX_SESSION_DURATION_SECS = "43200" # Required vars (to enable /.sts token exchange): # 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). +# it matches any. Unset = person-token exchange at /.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.