diff --git a/CHANGELOG.md b/CHANGELOG.md index 1869a86b..d6e8e1a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,29 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## 1.4.5 - UNRELEASED +## 1.5.0 - UNRELEASED + +### Added + +- An optional token service exchanges a client's credential for a registry-signed bearer token at `GET /token`, so a short-lived CI credential no longer has to outlive the push it starts. +- `auth.oidc..required_claims` rejects a token that does not carry the claims listed, before any access policy runs. +- `auth.oidc..server_ca_bundle` trusts a private CA for that provider's discovery and JWKS fetches, so an issuer such as a kube-apiserver needs no host-wide trust. + +### Changed + +- **Breaking:** OIDC providers are no longer typed. `provider = "github"` and `provider = "generic"` are gone; a provider is now an issuer plus how its tokens are validated, so every entry takes the same options. A GitHub Actions entry spells out the issuer it used to get for free; see [Upgrade Angos](doc/how-to/upgrade.md). +- **Breaking:** `identity.oidc.provider_type` is removed from access policies and the denial audit log. It only ever distinguished the two built-in provider types; `identity.oidc.provider_name`, the entry's own name, tells providers apart. +- Cached JWKS and discovery documents are keyed by issuer alone rather than by issuer and provider type, so two entries trusting one issuer share a fetch. Existing entries are refetched once on upgrade. + +### Fixed + +- Content pushed to a namespace no `[repository]` entry matches can now be pulled back: retrieval required a configured repository while every other route did not, so such a namespace was writable, listable, and unreadable. +- A JWKS key angos cannot turn into a decoding key now reports the provider unavailable, as the fetch and parse before it already did, instead of surfacing as an internal error. +- A basic-auth username matching an OIDC provider name is refused at startup instead of locking that user out, since a Basic credential naming a provider is read as that provider's token. +- An authorization webhook now receives the caller's OIDC provider and subject, so it can decide per user and its decision cache no longer serves one answer to every OIDC caller performing the same action. +- An upstream token response that omits `expires_in` is now cached for the 60 seconds the spec defines as its default rather than an hour, so angos stops sending a token long after its issuer stopped honouring it. + +## 1.4.5 ### Fixed diff --git a/Cargo.lock b/Cargo.lock index b318c281..a863ff54 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,25 +4,25 @@ version = 4 [[package]] name = "aho-corasick" -version = "1.1.4" +version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ddd31a130427c27518df266943a5308ed92d4b226cc639f5a8f1002816174301" +checksum = "c982642fa9e8606056828ee9a8505737230110bb1099153c79efe865c59d12ba" dependencies = [ "memchr", ] [[package]] name = "android_system_properties" -version = "0.1.5" +version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "819e7219dbd41043ac279b19830f2efc897156490d7fd6ea916720117ee66311" +checksum = "ae221649c9976a6f6c56ae1facf410f3ddb33cc661c4b7b61020a912d4237fbc" dependencies = [ "libc", ] [[package]] name = "angos" -version = "1.4.5" +version = "1.5.0" dependencies = [ "angos-backoff", "angos-s3-client", @@ -71,7 +71,7 @@ dependencies = [ "sha2", "smallvec", "tempfile", - "thiserror 2.0.19", + "thiserror 2.0.20", "tokio", "tokio-rustls", "tokio-util", @@ -130,7 +130,7 @@ dependencies = [ "serde", "serde_json", "tempfile", - "thiserror 2.0.19", + "thiserror 2.0.20", "tokio", "tokio-util", "tracing", @@ -154,7 +154,7 @@ dependencies = [ "serde", "serde_json", "sha2", - "thiserror 2.0.19", + "thiserror 2.0.20", "tokio", "tokio-util", "tracing", @@ -254,7 +254,7 @@ dependencies = [ "nom", "num-traits", "rusticata-macros", - "thiserror 2.0.19", + "thiserror 2.0.20", "time", ] @@ -304,9 +304,9 @@ dependencies = [ [[package]] name = "async-trait" -version = "0.1.91" +version = "0.1.92" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae36dc4177970ef04fde5178d3e2429882def40e57a451f919c098f72baa6cec" +checksum = "82f6aeea286b8eb4dd3431a1be1b59d290ace00f5bfd8e2a159bc2a05e2c1667" dependencies = [ "proc-macro2", "quote", @@ -327,9 +327,9 @@ checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" [[package]] name = "aws-lc-rs" -version = "1.17.3" +version = "1.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "00bdb5da18dac48ca2cc7cd4a98e533e8635a58e2361d13a1a4ee3888e0d72f1" +checksum = "ce2b2dcc879c3bae0d371e77c99f2238400ef24ec001394befa67b6e543add9e" dependencies = [ "aws-lc-sys", "untrusted 0.7.1", @@ -338,9 +338,9 @@ dependencies = [ [[package]] name = "aws-lc-sys" -version = "0.43.0" +version = "0.44.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43103168cc76fe62678a375e722fc9cb3a0146159ac5828bc4f0dfd755c2224c" +checksum = "f09fae7be8bb3174e05c6afdb34199e6dc0c7c04ba9fa237b1967adfbde27483" dependencies = [ "cc", "cmake", @@ -468,9 +468,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.4.0" +version = "1.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5add81bb678e6cb321aff7fa0dc7689ad82b112dbc032cea19f91d6b8e3582b9" +checksum = "5d262e149917187838d5b42777c8253bcb64500067342904e7d429499a6f277e" dependencies = [ "find-msvc-tools", "jobserver", @@ -584,7 +584,7 @@ dependencies = [ "serde", "serde_json", "sha2", - "thiserror 2.0.19", + "thiserror 2.0.20", "tokio", ] @@ -872,9 +872,9 @@ dependencies = [ [[package]] name = "find-msvc-tools" -version = "0.1.9" +version = "0.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5baebc0774151f905a1a2cc41989300b1e6fbb29aff0ceffa1064fdd3088d582" +checksum = "26b73573e6edcd2af0cdf47bd6cb58f0b3839491263c314eaad1ccf24430e1de" [[package]] name = "fnv" @@ -1425,7 +1425,7 @@ dependencies = [ "jni-sys", "log", "simd_cesu8", - "thiserror 2.0.19", + "thiserror 2.0.20", "walkdir", "windows-link", ] @@ -1474,9 +1474,9 @@ dependencies = [ [[package]] name = "js-sys" -version = "0.3.103" +version = "0.3.104" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53b44bfcdb3f8d5837a46dae1ca9660a837176eee74a28b229bc626816589102" +checksum = "0e0c1080212aad755ea003d18543e8768dd432c48819efd73a7bf1e39b7a5a3a" dependencies = [ "cfg-if", "futures-util", @@ -1503,9 +1503,9 @@ dependencies = [ [[package]] name = "kqueue" -version = "1.2.0" +version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "273c0752728918e0ac4976f2b275b6fefb9ecd400585dec929419f3844cd87b5" +checksum = "8d763e5b24120b4ddf50de6c92308156765aabfbbccebf401da7cff2d70a41ea" dependencies = [ "kqueue-sys", "libc", @@ -1755,7 +1755,7 @@ dependencies = [ "futures-sink", "js-sys", "pin-project-lite", - "thiserror 2.0.19", + "thiserror 2.0.20", "tracing", ] @@ -1785,7 +1785,7 @@ dependencies = [ "opentelemetry_sdk", "prost", "reqwest", - "thiserror 2.0.19", + "thiserror 2.0.20", "tokio", "tonic", "tonic-types", @@ -1817,7 +1817,7 @@ dependencies = [ "percent-encoding", "portable-atomic", "rand 0.9.5", - "thiserror 2.0.19", + "thiserror 2.0.20", "tokio", "tokio-stream", ] @@ -2026,7 +2026,7 @@ dependencies = [ "memchr", "parking_lot", "protobuf", - "thiserror 2.0.19", + "thiserror 2.0.20", ] [[package]] @@ -2104,7 +2104,7 @@ dependencies = [ "rustc-hash", "rustls", "socket2", - "thiserror 2.0.19", + "thiserror 2.0.20", "tokio", "tracing", "web-time", @@ -2127,7 +2127,7 @@ dependencies = [ "rustls", "rustls-pki-types", "slab", - "thiserror 2.0.19", + "thiserror 2.0.20", "tinyvec", "tracing", "web-time", @@ -2297,9 +2297,9 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.4.16" +version = "0.4.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8fcfdb36bda0c880c5931cdc7a2bcdc8ba4556847b9d912bca70bc94708711ad" +checksum = "ad8553b9b26413251cbf30e620595c7a41b3887f03da04579c0e6b0d6a06b4b2" dependencies = [ "aho-corasick", "memchr", @@ -2785,7 +2785,7 @@ checksum = "0d585997b0ac10be3c5ee635f1bab02d512760d14b7c468801ac8a01d9ae5f1d" dependencies = [ "num-bigint", "num-traits", - "thiserror 2.0.19", + "thiserror 2.0.20", "time", ] @@ -2899,11 +2899,11 @@ dependencies = [ [[package]] name = "thiserror" -version = "2.0.19" +version = "2.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "09a43598840e33d5b0331f38c5e30d13bb11c11210a4b58f0d9b18a5a5eefcd9" +checksum = "ec86235f5fcc2a73650310756d2ac5b138a5780bbbdfae3eeccec992c435ba4f" dependencies = [ - "thiserror-impl 2.0.19", + "thiserror-impl 2.0.20", ] [[package]] @@ -2919,9 +2919,9 @@ dependencies = [ [[package]] name = "thiserror-impl" -version = "2.0.19" +version = "2.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43cbfe0cf76104d42a574802844187e84a305e531ed54455f11fbde0f10541cd" +checksum = "bc04cd3e1236dd4a98afca4569f2deb3f120e5422a4023be2cb683f8486292af" dependencies = [ "proc-macro2", "quote", @@ -3404,9 +3404,9 @@ dependencies = [ [[package]] name = "wasm-bindgen" -version = "0.2.126" +version = "0.2.127" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4b067c0c11094aef6b7a801c1e34a26affafdf3d051dba08456b868789aaf9a4" +checksum = "1b70935747edd64d89de3efa29d73789b806c15798f8e7dca4d8ac356b50ce70" dependencies = [ "cfg-if", "once_cell", @@ -3417,9 +3417,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-futures" -version = "0.4.76" +version = "0.4.77" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c62df1340f32221cb9c54d6a27b030e3dba64361d4a95bed55f9aacb44da291d" +checksum = "6b7777d5cc23d0e91404e53ce2d5e8ec7acae3026b16233dba62cd3246457950" dependencies = [ "js-sys", "wasm-bindgen", @@ -3427,9 +3427,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro" -version = "0.2.126" +version = "0.2.127" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "167ce5e579f6bcf889c4f7175a8a5a585de84e8ff93976ce393efa5f2837aab1" +checksum = "77775f8f3f7217702089053b94958f8f54061a3f663417df76e19cbdcca29bc1" dependencies = [ "quote", "wasm-bindgen-macro-support", @@ -3437,9 +3437,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro-support" -version = "0.2.126" +version = "0.2.127" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f3997c7839262f4ef12cf90b818d6340c18e80f263f1a94bf157d0ec4420380e" +checksum = "e11d33f857dc2fb11b8bc75aee111aa9cbeb12cd9f25efd3d4c2a3dd4e235284" dependencies = [ "bumpalo", "proc-macro2", @@ -3450,9 +3450,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-shared" -version = "0.2.126" +version = "0.2.127" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc1b4cb0cc549fcf58d7dfc081778139b3d283a081644e833e84682ad71cea24" +checksum = "7ef64dbcc55df09c7e5a46182d181c2cfa3e925f3da937ea764728b4bbb9dcbf" dependencies = [ "unicode-ident", ] @@ -3472,9 +3472,9 @@ dependencies = [ [[package]] name = "web-sys" -version = "0.3.103" +version = "0.3.104" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8622dcb61c0bcc9fffa6938bed81210af2da9a7e4a1a834b2e37a59b6dfb6141" +checksum = "c435338968042f4f59a557f690a253676d47ce13ceb55d70100e7facf6620a30" dependencies = [ "js-sys", "wasm-bindgen", @@ -3798,7 +3798,7 @@ dependencies = [ "oid-registry", "ring", "rusticata-macros", - "thiserror 2.0.19", + "thiserror 2.0.20", "time", ] @@ -3843,18 +3843,18 @@ dependencies = [ [[package]] name = "zerocopy" -version = "0.8.55" +version = "0.8.56" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b5a105cd7b140f6eeec8acff2ea38135d3cab283ada58540f629fe51e46696eb" +checksum = "556764e583adb45a9f8d413c2a147fa7e8d821e48e12b14fd560b607998b75eb" dependencies = [ "zerocopy-derive", ] [[package]] name = "zerocopy-derive" -version = "0.8.55" +version = "0.8.56" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0fe976fb70c78cd64cccfe3a6fc142244e8a77b70959b30faf9d0ac37ee228eb" +checksum = "f2ab42fc20575779bd240faa45f94a74256f755c0fa9e89f0ede20d91d0cdfc1" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index 7ff8ba26..eac7bf81 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,7 +46,7 @@ wiremock = "0.6.5" [package] name = "angos" -version = "1.4.5" +version = "1.5.0" edition = "2024" [profile.release] diff --git a/README.md b/README.md index 6d4caefa..612aaa09 100644 --- a/README.md +++ b/README.md @@ -74,7 +74,8 @@ The complete documentation index lives in [doc/index.md](doc/index.md). - [Deploy on Kubernetes](doc/how-to/deploy-kubernetes.md) - [Configure mTLS](doc/how-to/configure-mtls.md) - [Configure GitHub Actions OIDC](doc/how-to/configure-github-actions-oidc.md) -- [Configure Generic OIDC](doc/how-to/configure-generic-oidc.md) +- [Push from GitHub Actions](doc/how-to/push-from-github-actions.md) +- [Configure OIDC](doc/how-to/configure-generic-oidc.md) - [Set Up Access Control](doc/how-to/set-up-access-control.md) - [Configure Retention Policies](doc/how-to/configure-retention-policies.md) - [Protect Tags with Immutability](doc/how-to/protect-tags-immutability.md) diff --git a/config.example.toml b/config.example.toml index 7ee1e119..7d842324 100644 --- a/config.example.toml +++ b/config.example.toml @@ -126,14 +126,20 @@ password = "$argon2id$v=19$m=16,t=2,p=1$MTIzNDU2Nzg$lurg6dYCXXrJP3zaFwu35w" # te # OIDC provider configuration # [auth.oidc.github] -# provider = "github" # issuer = "https://token.actions.githubusercontent.com" +# jwks_uri = "https://token.actions.githubusercontent.com/.well-known/jwks" +# required_claims = ["repository", "actor"] # reject a token that does not carry them # # [auth.oidc.generic] -# provider = "generic" # issuer = "https://auth.example.com" # jwks_uri = "https://auth.example.com/.well-known/jwks.json" # discovered from the issuer when omitted +# Token service -- exchange a short-lived credential for a registry-signed token +# [auth.token_service] +# secret_key = "base64 of 32 random bytes, from `openssl rand -base64 32`" +# realm = "https://registry.example.com/token" # derived from the request Host when omitted +# ttl_secs = 3600 + # Event webhooks -- notify external systems on registry operations # Define webhooks with [event_webhook.], then reference them in global or repository config. diff --git a/doc/explanation/authentication-authorization.md b/doc/explanation/authentication-authorization.md index 822dc6e5..f5c2c598 100644 --- a/doc/explanation/authentication-authorization.md +++ b/doc/explanation/authentication-authorization.md @@ -31,8 +31,9 @@ sequenceDiagram Registry->>Auth: Authenticate Note over Auth: 1. Check mTLS certificate - Note over Auth: 2. Validate OIDC token - Note over Auth: 3. Verify Basic Auth + Note over Auth: 2. Validate registry token + Note over Auth: 3. Validate OIDC token + Note over Auth: 4. Verify Basic Auth Auth-->>Registry: Identity (or anonymous) Registry->>Registry: Authorize request @@ -42,8 +43,12 @@ sequenceDiagram ### Processing Order 1. **mTLS**: Extract certificate information if present -2. **OIDC**: Validate Bearer tokens or OIDC-as-Basic-Auth -3. **Basic Auth**: Validate username/password from configuration +2. **Registry token**: Validate a token this registry issued +3. **OIDC**: Validate Bearer tokens or OIDC-as-Basic-Auth +4. **Basic Auth**: Validate username/password from configuration + +A valid registry token stops the chain: the OIDC middlewares claim any bearer +header and reject what they cannot validate, so they must not see one. ### Credential Handling @@ -52,6 +57,7 @@ Each method handles missing vs invalid credentials differently: | Method | No Credentials | Invalid Credentials | |------------|----------------|---------------------| | mTLS | Continue | **TLS handshake fails** | +| Registry token | Continue | **Reject immediately** | | OIDC | Continue | **Reject immediately** | | Basic Auth | Continue | **Reject immediately** | @@ -95,13 +101,16 @@ Tokens are validated by: 2. Issuer claim matching 3. Audience claim (if configured) 4. Time-based claims (exp, nbf) +5. Presence of every claim listed in `required_claims` + +Providers differ only by configuration; there is no provider type to select. ```toml [auth.oidc.github-actions] -provider = "github" +issuer = "https://token.actions.githubusercontent.com" +required_claims = ["repository", "actor"] [auth.oidc.corporate] -provider = "generic" issuer = "https://auth.example.com" ``` @@ -113,7 +122,6 @@ Authorization schemes are case-insensitive; for example, `bearer` and `Bearer` a **Identity fields:** - `identity.oidc.provider_name` -- `identity.oidc.provider_type` - `identity.oidc.claims["claim_name"]` ### Basic Auth (Password) @@ -130,6 +138,14 @@ password = "$argon2id$v=19$m=19456,t=2,p=1$..." # Argon2 hash - `identity.id` (e.g., "alice") - `identity.username` +### Registry Token + +With [`auth.token_service`](../reference/configuration.md#token-service-authtoken_service) configured, `GET /token` exchanges whatever credential authenticated the request for a registry-signed bearer token, advertised through the `WWW-Authenticate` header of a 401. OCI clients drive this on their own. It exists so a credential that expires faster than a push takes, such as a GitHub Actions OIDC token, only has to be valid when the push starts. + +The token carries `identity.id`, `identity.username` and `identity.oidc`, so policies decide exactly as they did for the original credential. It never carries `identity.certificate` or `identity.client_ip`: both are read from the live request, which keeps a certificate-bound identity from becoming a replayable bearer credential and lets mTLS compose with a token. + +The identity is frozen, the permissions are not: policies, repository rules and webhooks are still evaluated per request against live configuration. The reverse also holds, and is the cost of the feature: a token outlives the credential it was minted from and cannot be revoked before `ttl_secs` elapses. Rotating `secret_key`, or removing the OIDC provider a token names, invalidates outstanding tokens. + --- ## Authorization Flow @@ -207,7 +223,6 @@ identity = { oidc: { // OIDC info (null if not OIDC) provider_name: "github-actions", - provider_type: "GitHub Actions", claims: { "repository": "org/repo", "ref": "refs/heads/main", @@ -346,6 +361,8 @@ OIDC tokens are cryptographically verified: Bad OIDC tokens return 401. If Angos cannot reach or parse the configured provider discovery or JWKS endpoint, it returns 503 because the provider is temporarily unavailable rather than treating the credential as rejected. +Registry tokens are HMAC-signed and their algorithm is pinned, so a token whose header claims another algorithm is treated as another scheme's bearer and left to the OIDC middlewares rather than verified with the signing key. + ### Password Storage - Argon2id hashing diff --git a/doc/explanation/security-model.md b/doc/explanation/security-model.md index ee30df46..b89c6b3a 100644 --- a/doc/explanation/security-model.md +++ b/doc/explanation/security-model.md @@ -162,6 +162,16 @@ OIDC tokens are fully verified: - Expiration enforced - Clock skew tolerance configurable +### Registry Tokens + +Tokens the token service issues are HMAC-signed with a key of at least 32 bytes and their algorithm is pinned, so a token claiming another algorithm is never verified with the signing key. They carry the identity but never the client certificate or IP, which keeps a certificate-bound identity from becoming a replayable bearer credential. + +A token cannot be revoked before it expires: `ttl_secs`, capped at a day, is the window a stolen one stays usable. Nor can it be exchanged for a fresh one at `/token`, so that window never extends itself past the credential the token was minted from. Rotating `secret_key` or removing the OIDC provider a token names invalidates outstanding tokens. Authorization is unaffected, since policies are evaluated per request against live configuration rather than frozen into the token. + +The token is not scope-bound either. Where the registry v2 model narrows a token to one repository and a set of actions through an `access` claim, angos puts the identity in the token, so a stolen one reaches everything that identity reaches. What bounds it is the per-request policy evaluation above: the token grants no more than the credential it replaced, just over a wider surface than a scoped token would. + +That spec's claim set (`iss`, `sub`, `aud`, `nbf`, `jti`, `access`) and its `kid` header exist so a registry can verify tokens minted by a separate authorization server. Angos is both issuer and verifier, so its token is opaque to clients and carries only the identity it restores. + ### TLS Configuration Server TLS with modern defaults: diff --git a/doc/how-to/configure-generic-oidc.md b/doc/how-to/configure-generic-oidc.md index c99eba17..9506ba09 100644 --- a/doc/how-to/configure-generic-oidc.md +++ b/doc/how-to/configure-generic-oidc.md @@ -1,10 +1,10 @@ --- displayed_sidebar: howto sidebar_position: 5 -title: "Generic OIDC" +title: "OIDC" --- -# Configure Generic OIDC +# Configure OIDC Set up Angos to accept tokens from any OIDC-compliant identity provider (Google, Okta, Auth0, Keycloak, etc.). @@ -19,11 +19,12 @@ Set up Angos to accept tokens from any OIDC-compliant identity provider (Google, ### Step 1: Add OIDC Provider -Add a generic provider to `config.toml`: +Add a provider to `config.toml`. A provider is an issuer plus how its tokens are +validated, so every provider takes the same options; the issuer is what tells +them apart. ```toml [auth.oidc.my-provider] -provider = "generic" issuer = "https://auth.example.com" ``` @@ -33,15 +34,19 @@ The registry automatically discovers the JWKS endpoint from the issuer's `.well- ```toml [auth.oidc.my-provider] -provider = "generic" issuer = "https://auth.example.com" required_audience = "my-registry" # Validate audience claim +required_claims = ["email"] # Reject a token that does not carry them jwks_uri = "https://auth.example.com/.well-known/jwks.json" # Override discovery jwks_refresh_interval = 3600 # Refresh keys hourly (default) clock_skew_tolerance = 60 # Allow 60s clock drift (default) allowed_algorithms = ["RS256"] # Restrict accepted JWT algorithms (default) +server_ca_bundle = "/certs/ca.pem" # Trust a private CA for this issuer ``` +`required_claims` checks presence only. To test a claim's *value*, use an access +policy rule, which sees the whole claim map. + ### Step 3: Add Access Policy ```toml @@ -60,7 +65,6 @@ rules = [ ```toml [auth.oidc.google] -provider = "generic" issuer = "https://accounts.google.com" required_audience = "your-client-id.apps.googleusercontent.com" ``` @@ -69,7 +73,6 @@ required_audience = "your-client-id.apps.googleusercontent.com" ```toml [auth.oidc.okta] -provider = "generic" issuer = "https://your-org.okta.com" required_audience = "your-client-id" ``` @@ -78,7 +81,6 @@ required_audience = "your-client-id" ```toml [auth.oidc.auth0] -provider = "generic" issuer = "https://your-tenant.auth0.com/" required_audience = "your-api-identifier" ``` @@ -87,7 +89,6 @@ required_audience = "your-api-identifier" ```toml [auth.oidc.keycloak] -provider = "generic" issuer = "https://keycloak.example.com/realms/myrealm" required_audience = "registry-client" ``` @@ -96,11 +97,22 @@ required_audience = "registry-client" ```toml [auth.oidc.azure] -provider = "generic" issuer = "https://login.microsoftonline.com/your-tenant-id/v2.0" required_audience = "api://your-app-id" ``` +### Kubernetes API Server + +The cluster CA signs the issuer, so point `server_ca_bundle` at it rather than +trusting that CA for every outbound connection. + +```toml +[auth.oidc.kube] +issuer = "https://kubernetes.default.svc" +server_ca_bundle = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" +required_audience = "angos" +``` + --- ## Multiple Providers @@ -114,15 +126,14 @@ that actually signed it. ```toml [auth.oidc.github-actions] -provider = "github" +issuer = "https://token.actions.githubusercontent.com" +required_claims = ["repository", "actor"] [auth.oidc.corporate] -provider = "generic" issuer = "https://auth.corp.example.com" required_audience = "registry" [auth.oidc.cloud] -provider = "generic" issuer = "https://accounts.google.com" ``` diff --git a/doc/how-to/configure-github-actions-oidc.md b/doc/how-to/configure-github-actions-oidc.md index 907d7d67..5cdadfc9 100644 --- a/doc/how-to/configure-github-actions-oidc.md +++ b/doc/how-to/configure-github-actions-oidc.md @@ -17,15 +17,20 @@ Set up Angos to accept GitHub Actions OIDC tokens for passwordless authenticatio ### Step 1: Add OIDC Provider -Add the GitHub provider to `config.toml`: +Add GitHub's issuer to `config.toml`: ```toml [auth.oidc.github-actions] -provider = "github" +issuer = "https://token.actions.githubusercontent.com" +jwks_uri = "https://token.actions.githubusercontent.com/.well-known/jwks" +required_claims = ["repository", "actor"] ``` -That's it for basic configuration. The registry automatically uses GitHub's default issuer and JWKS endpoints. -OIDC tokens must use an allowed JWT signing algorithm; the default allowlist is `["RS256"]`. +`jwks_uri` is optional: leave it out and the registry discovers it from the +issuer's `.well-known/openid-configuration`. `required_claims` rejects a token +that does not carry the claims a workflow token always has, before any access +policy runs. OIDC tokens must use an allowed JWT signing algorithm; the default +allowlist is `["RS256"]`. ### Step 2: Add Access Policy @@ -96,6 +101,38 @@ jobs: The username must match the provider name (`github-actions` in this example). +For Kaniko, Buildx and the rest of the workflow side, see +[Push from GitHub Actions](push-from-github-actions.md). + +--- + +## Long-Running Pushes + +A GitHub Actions OIDC token is valid for about ten minutes and that lifetime cannot be extended. The registry checks it on every request, so a push still running when the token expires fails part-way through. + +Enable the [token service](../reference/configuration.md#token-service-authtoken_service) to decouple the two. The exchange is reactive rather than up front: the client's first request is refused with a 401 carrying the challenge, the client follows it to `/token`, and it uses the token it gets back for the rest of the push. Under a deny-by-default policy that refused request is the `GET /v2/` ping, so the OIDC token only has to be valid at the start of the push instead of for its whole duration: + +```toml +[auth.token_service] +secret_key = "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" # openssl rand -base64 32 +ttl_secs = 3600 +``` + +Under a default-deny policy, allow the exchange: + +```toml +[global.access_policy] +default = "deny" +rules = [ + "request.action == 'get-token' && identity.oidc != null", + "identity.oidc != null && identity.oidc.claims['repository'].startsWith('myorg/')", +] +``` + +No workflow change is needed, and this is not Docker-specific: the exchange is the registry v2 bearer-token flow that every OCI client implements, so Docker, Podman, Buildah, Skopeo, containerd, BuildKit, Kaniko, crane and ORAS all discover the endpoint from the registry's `WWW-Authenticate` header on their own. Fetch the OIDC token close to the push rather than at the start of the job, so the ten minutes covers the build as well. + +A token that expires mid-push is answered with a fresh challenge and the client exchanges again, but only while the credential it started from is still valid, which here is the same ten minutes. Size `ttl_secs` to the longest push you expect rather than to the maximum allowed: a registry token cannot be revoked before it expires. A client whose credentials do not expire, such as basic auth, recovers from any expiry and is fine with a short one. + --- ## Policy Examples @@ -180,7 +217,8 @@ server_private_key = "/tls/server.key" root_dir = "/data" [auth.oidc.github-actions] -provider = "github" +issuer = "https://token.actions.githubusercontent.com" +required_claims = ["repository", "actor"] # Production: only main branch from specific repos [repository."production".access_policy] @@ -292,5 +330,5 @@ The log only includes the provider name/type and the `sub`/`iss` claims; the ful ## Next Steps -- [Configure Generic OIDC](configure-generic-oidc.md) for other identity providers +- [Configure OIDC](configure-generic-oidc.md) for other identity providers - [Set Up Access Control](set-up-access-control.md) for comprehensive policies diff --git a/doc/how-to/configure-webhook-authorization.md b/doc/how-to/configure-webhook-authorization.md index 575b909a..c48df7b2 100644 --- a/doc/how-to/configure-webhook-authorization.md +++ b/doc/how-to/configure-webhook-authorization.md @@ -155,12 +155,18 @@ The registry sends GET requests with headers containing request context. ### Identity (when authenticated) -| Header | Description | -|-----------------------------|----------------------------| -| `X-Registry-Username` | Basic auth or OIDC subject | -| `X-Registry-Identity-ID` | Identity identifier | -| `X-Registry-Certificate-CN` | Certificate Common Name | -| `X-Registry-Certificate-O` | Certificate Organization | +| Header | Description | +|-----------------------------|----------------------------------------| +| `X-Registry-Username` | Basic auth username | +| `X-Registry-Identity-ID` | Identity identifier | +| `X-Registry-Certificate-CN` | Certificate Common Name | +| `X-Registry-Certificate-O` | Certificate Organization | +| `X-Registry-OIDC-Provider` | Name of the `auth.oidc` entry | +| `X-Registry-OIDC-Subject` | The token's `sub` claim, if it has one | + +An OIDC caller carries the two OIDC headers and no username, so a webhook +deciding per user must read `X-Registry-OIDC-Subject`. Every header here enters +the decision cache key, so two subjects never share one cached answer. --- diff --git a/doc/how-to/push-from-github-actions.md b/doc/how-to/push-from-github-actions.md new file mode 100644 index 00000000..4e0075f2 --- /dev/null +++ b/doc/how-to/push-from-github-actions.md @@ -0,0 +1,206 @@ +--- +displayed_sidebar: howto +sidebar_position: 5 +title: "Push from GitHub Actions" +--- + +# Push from GitHub Actions Without Stored Credentials + +Push images from a workflow with no registry password anywhere: no repository +secret, no service account, nothing to rotate. GitHub mints a short-lived OIDC +token per job and Angos validates it against GitHub's public keys. + +This page covers the workflow side. For the registry side, see +[Configure GitHub Actions OIDC](configure-github-actions-oidc.md). + +## Prerequisites + +A provider entry trusting GitHub, whose key is the username clients will send: + +```toml +[auth.oidc.github-actions] +issuer = "https://token.actions.githubusercontent.com" +required_claims = ["repository", "actor"] +``` + +`required_claims` rejects a token missing either claim before any policy runs, so +a rule reading `claims["repository"]` can never evaluate against an absent value. + +Optionally add the token service, so the credential only has to be valid when a +push starts rather than for its whole duration: + +```toml +[auth.token_service] +realm = "https://registry.example.com/token" +ttl_secs = 3600 +secret_key = "..." # openssl rand -base64 32 +``` + +Keep `secret_key` in a second `-c` file so rotating it never touches the file +your deployment tooling renders. Both files are watched, so the rotation applies +without a restart. + +## How a Client Authenticates + +Angos reads an OIDC token from either of two places, so any client that speaks +HTTP basic auth can present one: + +| Form | Username | Password | +|------|----------|----------| +| `Authorization: Bearer ` | | | +| `Authorization: Basic ` | the `[auth.oidc.]` key | the token | + +The second form is why `docker login`, Kaniko, Buildah and anything else reading +a `config.json` work unchanged. The username is not a user: it names the provider +entry that should validate the password. + +## Step 1: Grant the Job an OIDC Token + +```yaml +permissions: + contents: read + id-token: write # without this the token endpoint is not injected +``` + +## Step 2: Request the Token + +```yaml +- name: Get OIDC token + id: oidc + run: | + TOKEN=$(curl -sSf -H "Authorization: bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \ + "$ACTIONS_ID_TOKEN_REQUEST_URL&audience=https://registry.example.com" \ + | jq -r '.value') + echo "::add-mask::$TOKEN" + echo "token=$TOKEN" >> "$GITHUB_OUTPUT" +``` + +`audience` names the container registry, not the GitHub repository, so `required_audience` can pin +one value for every workflow that pushes. It is public: a token minted for +another service is refused, nothing is kept secret. + +`::add-mask::` keeps the token out of the job log. Request it close to the push: +it is valid for about ten minutes, which has to cover everything up to the +registry's first request. + +## Step 3: Push + +### Kaniko + +Kaniko takes credentials only through a Docker config file, so write one: + +```yaml +- name: Build and push + env: + REGISTRY_PASSWORD: ${{ steps.oidc.outputs.token }} + run: | + install -m600 /dev/null kaniko-secret.json + jq -n --arg auth "$(printf 'github-actions:%s' "$REGISTRY_PASSWORD" | base64 -w0)" \ + '{auths: {"registry.example.com": {auth: $auth}}}' > kaniko-secret.json + + docker run -i --rm -v "$PWD:/workspace" \ + -v "$PWD/kaniko-secret.json":/kaniko/.docker/config.json:ro \ + gcr.io/kaniko-project/executor:v1.18.0 \ + --dockerfile=Dockerfile \ + --destination=registry.example.com/myorg/myapp:"$IMAGE_TAG" +``` + +`install -m600` creates the file unreadable by other users before anything is +written to it, and `jq --arg` passes the token as an argument rather than +interpolating it into a command line other processes could read. + +### Docker + +```yaml +- name: Log in + run: | + echo "${{ steps.oidc.outputs.token }}" | \ + docker login registry.example.com --username github-actions --password-stdin + +- name: Push + run: docker push registry.example.com/myorg/myapp:"$IMAGE_TAG" +``` + +### Buildx + +`docker/login-action` writes the same config file, so it needs no special +handling: + +```yaml +- uses: docker/login-action@v3 + with: + registry: registry.example.com + username: github-actions + password: ${{ steps.oidc.outputs.token }} + +- uses: docker/build-push-action@v6 + with: + push: true + tags: registry.example.com/myorg/myapp:${{ github.sha }} +``` + +`build-push-action` builds and pushes in one step, so a long build eats into the +token's ten minutes before the push starts. Either build first with `push: false` +and push in a later step, or enable the +[token service](../reference/configuration.md#token-service-authtoken_service) so +the credential only has to be valid when the push begins. + +## Restricting Who May Push + +The token's claims reach the access policy, so a rule can name the repository, +the branch, or the workflow that produced it. Policies apply in two layers: the +global one gates every request, and the repository one then decides what that +caller may do in its namespaces. + +```toml +[global.access_policy] +default = "deny" +rules = [ + "request.action in ['healthz', 'readyz', 'metrics']", + "identity.username != null", + "identity.oidc != null", +] + +[repository."myorg/website".access_policy] +default = "deny" +rules = [ + 'identity.id == "admin"', + '''identity.oidc != null && + identity.oidc.claims["repository"] == "myorg/website-frontend"''', +] +``` + +**A namespace with no matching `[repository]` entry is governed by the global +policy alone.** In the example above that means any valid GitHub Actions token, +from any repository on GitHub, may push to a namespace you have not configured. +Either configure every namespace you serve, or make the global rules stand on +their own, for example by naming the permitted repositories there too: + +```toml +rules = [ + "request.action in ['healthz', 'readyz', 'metrics']", + "identity.username != null", + '''identity.oidc != null && + identity.oidc.claims["repository"].startsWith("myorg/")''', +] +``` + +More examples in [Configure GitHub Actions OIDC](configure-github-actions-oidc.md#policy-examples). + +## Troubleshooting + +| Symptom | Cause | +|---------|-------| +| `ACTIONS_ID_TOKEN_REQUEST_URL` is empty | the job is missing `id-token: write` | +| `401` on the first push request | the username does not match an `[auth.oidc.]` key | +| `401` part-way through a push | the token expired mid-push; see the token service above | +| `403` after a successful login | the token validated but no policy rule admits its claims | + +Run the registry with `RUST_LOG=angos::auth=debug` to see which provider accepted +or rejected a token, and `RUST_LOG=angos::policy=debug` to see how rules evaluated +against its claims. + +## Next Steps + +- [Configure GitHub Actions OIDC](configure-github-actions-oidc.md) for the registry side +- [Set Up Access Control](set-up-access-control.md) for policy syntax diff --git a/doc/how-to/set-up-access-control.md b/doc/how-to/set-up-access-control.md index 3c760644..c8bb04ed 100644 --- a/doc/how-to/set-up-access-control.md +++ b/doc/how-to/set-up-access-control.md @@ -430,6 +430,7 @@ RUST_LOG=angos::policy=debug \ | `delete-job` | Extension: delete a pending/failed job | | `ui-asset` | UI static assets | | `ui-config` | UI configuration | +| `get-token` | Exchange a credential for a token | - [CEL Expressions Reference](../reference/cel-expressions.md) - All variables and functions - [Configuration Reference](../reference/configuration.md) - Policy configuration options diff --git a/doc/how-to/upgrade.md b/doc/how-to/upgrade.md index 4c5354b8..4b5fa95f 100644 --- a/doc/how-to/upgrade.md +++ b/doc/how-to/upgrade.md @@ -400,3 +400,59 @@ angos migrate ``` Run it once after upgrading. The command is idempotent and leaves links that already carry a `media_type` untouched; native angos pushes have always stored it, so a registry that never imported a raw `distribution` layout needs no action. + +--- + +## 1.4.5 → 1.5.0 + +### OIDC Providers Are No Longer Typed (Breaking Change) + +#### What Changed + +`auth.oidc..provider` is gone. A provider is an issuer plus how its tokens are validated, so every entry takes the same options and nothing selects between provider types. + +**Who is affected:** every deployment with an `[auth.oidc.*]` table, and any access policy reading `identity.oidc.provider_type`. + +The `provider` key is now an unknown field, which TOML ignores rather than rejects. An entry that relied on the GitHub defaults therefore fails to load with `missing field 'issuer'` instead of naming `provider` as the cause. + +#### Migrate a GitHub Actions Provider + +**Before:** + +```toml +[auth.oidc.github-actions] +provider = "github" +``` + +**After:** + +```toml +[auth.oidc.github-actions] +issuer = "https://token.actions.githubusercontent.com" +jwks_uri = "https://token.actions.githubusercontent.com/.well-known/jwks" +required_claims = ["repository", "actor"] +``` + +`jwks_uri` is optional; without it the registry discovers the endpoint from the issuer. `required_claims` preserves the repository/actor check the GitHub provider performed on every token; drop it only if you want tokens missing those claims to reach your access policy. + +#### Migrate a Generic Provider + +Delete the `provider = "generic"` line. Nothing else changes. + +### `identity.oidc.provider_type` Is Removed (Breaking Change) + +It only ever held `"GitHub Actions"` or `"Generic OIDC"`, a distinction that no longer exists. Rewrite any policy rule using it to test `identity.oidc.provider_name`, which is the name of the `[auth.oidc.]` entry that authenticated the token: + +```toml +# Before +'identity.oidc != null && identity.oidc.provider_type == "GitHub Actions"' + +# After +'identity.oidc != null && identity.oidc.provider_name == "github-actions"' +``` + +The field is also gone from the denial audit log, and from the payload of registry tokens issued by `auth.token_service`. Tokens minted before the upgrade stay valid: the extra field is ignored when they are validated. + +### Cached JWKS Refetched Once + +JWKS and discovery documents are now cached by issuer alone rather than by issuer and provider type. Existing cache entries are not read after the upgrade, so each issuer is fetched once more than usual on the first requests. No action is required. diff --git a/doc/index.md b/doc/index.md index 0749085c..c85d0e0d 100644 --- a/doc/index.md +++ b/doc/index.md @@ -21,7 +21,8 @@ Step-by-step instructions for specific tasks: ### Authentication - [Configure mTLS](how-to/configure-mtls.md) - [Configure GitHub Actions OIDC](how-to/configure-github-actions-oidc.md) -- [Configure Generic OIDC](how-to/configure-generic-oidc.md) +- [Push from GitHub Actions](how-to/push-from-github-actions.md) +- [Configure OIDC](how-to/configure-generic-oidc.md) ### Policies - [Set Up Access Control](how-to/set-up-access-control.md) diff --git a/doc/reference/api-endpoints.md b/doc/reference/api-endpoints.md index 798dfb41..e307315e 100644 --- a/doc/reference/api-endpoints.md +++ b/doc/reference/api-endpoints.md @@ -514,7 +514,26 @@ Returns UI configuration. ## Authentication -Every route passes through the access policy, including `/healthz` and `/readyz` (actions `healthz` and `readyz`). A default-deny policy must allow those actions or health and readiness probes fail. +Every route passes through the access policy, including `/healthz`, `/readyz` and `/token` (actions `healthz`, `readyz` and `get-token`). A default-deny policy must allow those actions or health probes and token exchange fail. + +### Token Service + +``` +GET /token +``` + +Exchanges the credential that authenticated the request for a registry-signed bearer token, so a client holding a short-lived credential can keep working after it expires. Returns `404` unless [`auth.token_service`](configuration.md#token-service-authtoken_service) is configured. + +The endpoint is advertised in the `WWW-Authenticate` header of a `401`, and OCI clients follow it on their own. A request with no credentials gets a token carrying no identity, which the access policy then evaluates as anonymous. + +**Success Response:** +```json +{"token":"","expires_in":3600} +``` + +Present it as `Authorization: Bearer ` on subsequent requests. The token declares its own type, `angos+jwt`, which is what tells it apart from a provider's bearer on the same header. The token carries the identity, never the client certificate or IP: those are read from the live request, so an mTLS client keeps its certificate identity while using a token. + +The response is `Cache-Control: no-store`, and presenting a registry token here is refused with a `401`: renewing one would let a token outlive the credential it was minted from for as long as a client kept asking, and `ttl_secs` would bound nothing. ### Methods @@ -528,6 +547,14 @@ Authorization: Basic base64(username:password) Authorization: Bearer ``` +**Bearer Token (registry):** +``` +Authorization: Bearer +``` + +Both bearers share the header. Angos tells them apart by the type each declares, +so a token it did not issue is left for the OIDC providers to validate. + **OIDC via Basic Auth (Docker compatibility):** ``` Authorization: Basic base64(provider-name:jwt-token) @@ -550,7 +577,8 @@ Present a client certificate during TLS handshake. 1. Client makes unauthenticated request 2. Server returns `401 Unauthorized` with `WWW-Authenticate` header -3. Client retries with credentials +3. Client retries with credentials, or, when the header names a `Bearer` realm, + exchanges them at that realm for a registry token and retries with it 4. Server validates and processes request --- diff --git a/doc/reference/cel-expressions.md b/doc/reference/cel-expressions.md index 335b07d5..f06bad1f 100644 --- a/doc/reference/cel-expressions.md +++ b/doc/reference/cel-expressions.md @@ -39,7 +39,6 @@ Available when client authenticates with OIDC token. **Always check for null bef |-------------------------------|---------|----------------------------------------------------| | `identity.oidc` | object? | OIDC context (null if not OIDC) | | `identity.oidc.provider_name` | string | Configured provider name | -| `identity.oidc.provider_type` | string | Provider type ("GitHub Actions" or "Generic OIDC") | | `identity.oidc.claims` | map | JWT claims (access with bracket notation) | **GitHub Actions Claims:** @@ -57,7 +56,7 @@ Available when client authenticates with OIDC token. **Always check for null bef | `repository_owner` | Repository owner | | `repository_visibility` | Repository visibility (public/private) | -**Generic OIDC Claims:** +**Standard OIDC Claims:** | Claim | Description | |----------|---------------------------------| @@ -112,6 +111,7 @@ Optional `request` fields are **omitted** when unset (only `request.action` is a | `list-tags` | List tags | | `ui-asset` | UI static assets | | `ui-config` | UI configuration | +| `get-token` | Registry token exchange | | `list-repositories` | Extension: list repositories | | `list-namespaces` | Extension: list namespaces | | `list-revisions` | Extension: list revisions | diff --git a/doc/reference/configuration.md b/doc/reference/configuration.md index 5297b585..24f4b407 100644 --- a/doc/reference/configuration.md +++ b/doc/reference/configuration.md @@ -419,31 +419,21 @@ ttl = 10 Password hashes are validated when the configuration is parsed. An invalid Argon2 hash causes the server to fail to start with a clear error. Use `angos argon` to generate a valid hash. -Usernames must be unique across all `auth.identity` entries; a duplicate causes the server to fail to start. +Usernames must be unique across all `auth.identity` entries, and none may match +an `auth.oidc` provider name: a Basic credential naming a provider is read as +that provider's token. Either collision causes the server to fail to start. ### OIDC (`auth.oidc.`) -#### GitHub Provider - -| Option | Type | Default | Description | -|-------------------------|--------|------------------------------------------------------------------|---------------------------------| -| `provider` | string | required | Must be `"github"` | -| `issuer` | string | `"https://token.actions.githubusercontent.com"` | Issuer URL | -| `jwks_uri` | string | `"https://token.actions.githubusercontent.com/.well-known/jwks"` | JWKS URI | -| `jwks_refresh_interval` | u64 | `3600` | JWKS refresh interval (seconds) | -| `required_audience` | string | - | Required audience claim | -| `clock_skew_tolerance` | u64 | `60` | Clock skew tolerance (seconds) | -| `allowed_algorithms` | array | `["RS256"]` | Allowed JWT signing algorithms | -| `http_request_timeout_secs` | u64 | `30` | Timeout for a JWKS or discovery HTTP fetch (seconds) | -| `jwks_refresh_timeout_secs` | u64 | `5` | Timeout for the forced JWKS refetch on key rotation (seconds) | - -#### Generic Provider +Every provider takes the same options: a provider is an issuer plus how its +tokens are validated, so there is no provider type to select. | Option | Type | Default | Description | |-------------------------|--------|------------|----------------------------------------------| -| `provider` | string | required | Must be `"generic"` | | `issuer` | string | required | OIDC issuer URL | | `jwks_uri` | string | - | Custom JWKS URI (auto-discovered if not set) | +| `server_ca_bundle` | string | - | PEM CA bundle trusted for this provider's HTTPS fetches | +| `required_claims` | array | `[]` | Claims a token must carry; a missing or null one is rejected | | `jwks_refresh_interval` | u64 | `3600` | JWKS refresh interval (seconds) | | `required_audience` | string | - | Required audience claim | | `clock_skew_tolerance` | u64 | `60` | Clock skew tolerance (seconds) | @@ -451,8 +441,68 @@ Usernames must be unique across all `auth.identity` entries; a duplicate causes | `http_request_timeout_secs` | u64 | `30` | Timeout for a JWKS or discovery HTTP fetch (seconds) | | `jwks_refresh_timeout_secs` | u64 | `5` | Timeout for the forced JWKS refetch on key rotation (seconds) | +GitHub Actions, for example, is one such entry: + +```toml +[auth.oidc.github-actions] +issuer = "https://token.actions.githubusercontent.com" +jwks_uri = "https://token.actions.githubusercontent.com/.well-known/jwks" +required_claims = ["repository", "actor"] +``` + +Set `server_ca_bundle` for an issuer whose certificate the system roots do not +cover, such as a kube-apiserver signed by the cluster CA. It applies to the +discovery and JWKS fetches for that provider alone; other providers keep the +system roots. + +`required_claims` checks presence only. Predicates over claim *values* belong in +the access policy, which sees the whole claim map. + `allowed_algorithms` accepts JWT algorithm names such as `"RS256"`, `"RS384"`, `"RS512"`, `"ES256"`, and `"ES384"`. Angos rejects tokens whose header claims an algorithm outside the provider allowlist before signature verification to prevent algorithm-confusion attacks. +### Token Service (`auth.token_service`) + +Issues registry-signed bearer tokens at `GET /token`, so a client holding a +short-lived credential can exchange it once and keep pushing after that +credential expires. Present the section to enable it. + +| Option | Type | Default | Description | +|--------------|--------|----------|-----------------------------------------------------------------| +| `secret_key` | string | required | Base64 HMAC signing key, at least 32 bytes decoded | +| `realm` | string | - | Absolute token URL advertised to clients, path must end with `/token` | +| `ttl_secs` | u64 | `3600` | Token lifetime in seconds, at most `86400` | + +With the section present, a `401` carries `WWW-Authenticate: Bearer` instead of +`Basic`, for every client rather than only OIDC ones. Left unset, the challenge +is built from each request's own `Host`, which is what a registry serving +several hostnames wants; behind a TLS-terminating proxy, list the proxy in +`global.trusted_proxies` so its `X-Forwarded-Proto` decides the scheme. Set +`realm` when anything in front of the registry caches responses, so the +challenge cannot follow a `Host` a client chose, and when a proxy strips a path +prefix, so the advertised URL is the prefixed one clients must call. + +Generate `secret_key` with `openssl rand -base64 32`: it is decoded before use, +so its strength is the randomness of those bytes and not the length of a +passphrase. Rotating it invalidates every outstanding token; clients recover by +fetching a new one. An issued token freezes the identity it was minted from but +not its permissions: access policies are still evaluated per request. A token +cannot otherwise be revoked before it expires, so `ttl_secs` is the window a +stolen one stays usable. Removing or renaming an `auth.oidc` entry invalidates +outstanding tokens minted from that provider. The section reloads without a +restart, `secret_key` included, so rotating the key during an incident costs no +downtime. + +`GET /token` is subject to the access policy like any other route. Under +`default = "deny"`, add a rule for it: + +```toml +[global.access_policy] +default = "deny" +rules = [ + "request.action == 'get-token' && identity.oidc != null", +] +``` + ### Webhooks (`auth.webhook.`) | Option | Type | Default | Description | @@ -636,7 +686,8 @@ username = "admin" password = "$argon2id$v=19$m=19456,t=2,p=1$..." [auth.oidc.github-actions] -provider = "github" +issuer = "https://token.actions.githubusercontent.com" +required_claims = ["repository", "actor"] [global.access_policy] default = "deny" diff --git a/doc/reference/metrics.md b/doc/reference/metrics.md index 984812f4..6fdbb089 100644 --- a/doc/reference/metrics.md +++ b/doc/reference/metrics.md @@ -106,6 +106,7 @@ The `route` label uses action names from the OCI Distribution API: | `get-referrers` | Get referrers | | `ui-asset` | UI static files | | `ui-config` | UI configuration | +| `get-token` | Token service | | `list-repositories` | Extension API | | `list-namespaces` | Extension API | | `list-revisions` | Extension API | @@ -129,7 +130,7 @@ Total number of authentication attempts. | Counter | `method`, `result` | **Labels:** -- `method`: `basic`, `mtls`, `oidc` +- `method`: `basic`, `mtls`, `oidc`, `token` - `result`: `success`, `failed` **Example:** diff --git a/src/auth/authenticator.rs b/src/auth/authenticator.rs index bef72dd4..c630da90 100644 --- a/src/auth/authenticator.rs +++ b/src/auth/authenticator.rs @@ -8,11 +8,12 @@ use tracing::{Span, debug, info, instrument, warn}; use crate::{ auth::Error, auth::{ - AuthMiddleware, AuthResult, BasicAuthValidator, MtlsValidator, OidcValidator, basic_auth, - oidc, webhook, + AuthMiddleware, AuthResult, BasicAuthValidator, MtlsValidator, OidcValidator, + TokenValidator, basic_auth, oidc, token_service, webhook, }, cache::Cache, configuration::Configuration, + http_client::apply_tls_files, identity::{AuthMethod, ClientIdentity}, metrics_provider::metrics_provider, }; @@ -25,27 +26,16 @@ pub struct AuthConfig { pub oidc: HashMap, #[serde(default)] pub webhook: HashMap, + #[serde(default)] + pub token_service: Option, } type OidcValidators = Vec<(String, Arc)>; -/// Returns the strongest method that succeeded, using first-wins priority: mTLS > OIDC > Basic. -fn select_auth_method(mtls: bool, oidc: bool, basic: bool) -> AuthMethod { - if mtls { - return AuthMethod::Mtls; - } - if oidc { - return AuthMethod::Oidc; - } - if basic { - return AuthMethod::Basic; - } - AuthMethod::Anonymous -} - /// Coordinates all authentication methods and handles the authentication chain pub struct Authenticator { mtls_validator: MtlsValidator, + token_validator: Option, oidc_validators: OidcValidators, basic_auth_validator: BasicAuthValidator, } @@ -53,20 +43,25 @@ pub struct Authenticator { impl Authenticator { pub fn new(config: &Configuration, cache: &Arc) -> Result { let auth_config = &config.auth; - // No client-level timeout: each OIDC fetch carries a per-request - // timeout from its provider config (`http_request_timeout_secs`, - // `jwks_refresh_timeout_secs`). - let oidc_client = - Arc::new(Client::builder().build().map_err(|e| { - Error::Initialization(format!("Failed to create HTTP client: {e}")) - })?); + reject_provider_name_collision(auth_config)?; let mtls_validator = MtlsValidator::new(); - let oidc_validators = Self::build_oidc_validators(auth_config, &oidc_client, cache); + let oidc_validators = Self::build_oidc_validators(auth_config, cache)?; let basic_auth_validator = BasicAuthValidator::new(&auth_config.identity)?; + let provider_names: Vec = oidc_validators + .iter() + .map(|(name, _)| name.clone()) + .collect(); + let token_validator = auth_config + .token_service + .as_ref() + .map(|config| TokenValidator::new(config, &provider_names)) + .transpose()?; + Ok(Self { mtls_validator, + token_validator, oidc_validators, basic_auth_validator, }) @@ -74,26 +69,29 @@ impl Authenticator { fn build_oidc_validators( auth_config: &AuthConfig, - client: &Arc, cache: &Arc, - ) -> OidcValidators { + ) -> Result { let mut validators = Vec::with_capacity(auth_config.oidc.len()); for (name, oidc_config) in &auth_config.oidc { let validator = OidcValidator::new( name.clone(), oidc_config, - Arc::clone(client), + build_oidc_client(name, oidc_config)?, Arc::clone(cache), ); validators.push((name.clone(), Arc::new(validator) as Arc)); } validators.sort_by(|a, b| a.0.cmp(&b.0)); - validators + Ok(validators) } - /// Authentication order: mTLS → OIDC → Basic Auth + /// Authentication order: mTLS → Registry token → OIDC → Basic Auth + /// + /// A registry token short-circuits OIDC and Basic: the OIDC middlewares claim + /// any bearer header, so letting them run would reject the token they cannot + /// validate. #[instrument(skip(self, parts), fields(auth_method = tracing::field::Empty))] pub async fn authenticate_request( &self, @@ -102,22 +100,36 @@ impl Authenticator { ) -> Result { let mut identity = ClientIdentity::new(remote_address); - let mtls_ok = self.try_mtls_authentication(parts, &mut identity).await; - let oidc_ok = self.try_oidc_authentication(parts, &mut identity).await?; - let basic_ok = if oidc_ok { - false + let mtls = self.try_mtls_authentication(parts, &mut identity).await; + let token = self.try_token_authentication(parts, &mut identity).await?; + let oidc = if token.is_none() { + self.try_oidc_authentication(parts, &mut identity).await? } else { + None + }; + let basic = if token.is_none() && oidc.is_none() { self.try_basic_authentication(parts, &mut identity).await? + } else { + None }; - identity.auth_method = select_auth_method(mtls_ok, oidc_ok, basic_ok); + // First-wins priority, strongest first: a request can satisfy several + // methods at once and the identity states one answer. + identity.auth_method = mtls + .or(token) + .or(oidc) + .or(basic) + .unwrap_or(AuthMethod::Anonymous); Span::current().record("auth_method", identity.auth_method.as_str()); Ok(identity) } - /// Attempts mTLS authentication. Returns `true` if a valid certificate was extracted. /// Errors are logged and suppressed: mTLS is non-fatal so other methods can follow. - async fn try_mtls_authentication(&self, parts: &Parts, identity: &mut ClientIdentity) -> bool { + async fn try_mtls_authentication( + &self, + parts: &Parts, + identity: &mut ClientIdentity, + ) -> Option { match self.mtls_validator.authenticate(parts, identity).await { Ok(AuthResult::Authenticated) => { debug!("mTLS authentication extracted certificate info"); @@ -128,7 +140,7 @@ impl Authenticator { .auth_attempts .with_label_values(&["mtls", "success"]) .inc(); - return true; + return Some(AuthMethod::Mtls); } } Ok(AuthResult::NoCredentials) => {} @@ -140,11 +152,43 @@ impl Authenticator { .inc(); } } - false + None + } + + /// Returns `Err` when the bearer is one of ours but no longer valid; a bearer + /// belonging to another scheme is left for the OIDC middlewares. + async fn try_token_authentication( + &self, + parts: &Parts, + identity: &mut ClientIdentity, + ) -> Result, Error> { + let Some(token_validator) = &self.token_validator else { + return Ok(None); + }; + + match token_validator.authenticate(parts, identity).await { + Ok(AuthResult::Authenticated) => { + debug!("Registry token authentication succeeded"); + metrics_provider() + .auth_attempts + .with_label_values(&["token", "success"]) + .inc(); + Ok(Some(AuthMethod::Token)) + } + Ok(AuthResult::NoCredentials) => Ok(None), + Err(e) => { + info!("Registry token validation failed: {e}"); + metrics_provider() + .auth_attempts + .with_label_values(&["token", "failed"]) + .inc(); + Err(e) + } + } } - /// Tries each OIDC provider in sorted order, returning `true` on first success. - /// A failure from one provider does not prevent subsequent providers from being tried. + /// Providers are tried in sorted order, and a failure from one does not stop + /// the next from being tried. /// If no provider succeeds and at least one returned an error, the first error is returned. /// First rather than last so that deterministic sort order also makes error reporting deterministic. /// @@ -155,7 +199,7 @@ impl Authenticator { &self, parts: &Parts, identity: &mut ClientIdentity, - ) -> Result { + ) -> Result, Error> { let mut first_error: Option = None; for (provider_name, validator) in &self.oidc_validators { match validator.authenticate(parts, identity).await { @@ -165,7 +209,7 @@ impl Authenticator { .auth_attempts .with_label_values(&["oidc", "success"]) .inc(); - return Ok(true); + return Ok(Some(AuthMethod::Oidc)); } Ok(AuthResult::NoCredentials) => {} Err(e) => { @@ -184,17 +228,16 @@ impl Authenticator { .inc(); Err(e) } - None => Ok(false), + None => Ok(None), } } - /// Attempts basic auth authentication, returning `true` on success. /// Returns `Err` if credentials were presented but invalid. async fn try_basic_authentication( &self, parts: &Parts, identity: &mut ClientIdentity, - ) -> Result { + ) -> Result, Error> { match self .basic_auth_validator .authenticate(parts, identity) @@ -206,9 +249,9 @@ impl Authenticator { .auth_attempts .with_label_values(&["basic", "success"]) .inc(); - Ok(true) + Ok(Some(AuthMethod::Basic)) } - Ok(AuthResult::NoCredentials) => Ok(false), + Ok(AuthResult::NoCredentials) => Ok(None), Err(e) => { warn!("Basic auth validation failed: {e}"); metrics_provider() @@ -221,25 +264,72 @@ impl Authenticator { } } +/// One client per provider: a CA bundle is baked into a client when it is built, +/// so a provider trusting its own issuer cannot share one with the others. +fn build_oidc_client(name: &str, config: &oidc::Config) -> Result, Error> { + let initialization_error = |e: String| { + Error::Initialization(format!( + "Failed to create HTTP client for auth.oidc.{name}: {e}" + )) + }; + + // No client-level timeout: each fetch carries a per-request timeout from the + // provider config (`http_request_timeout_secs`, `jwks_refresh_timeout_secs`). + apply_tls_files( + Client::builder(), + config.server_ca_bundle.as_deref(), + None, + None, + ) + .map_err(initialization_error)? + .build() + .map(Arc::new) + .map_err(|e| initialization_error(e.to_string())) +} + +/// A Basic credential whose username names a provider is read as that provider's +/// token, and the validation failure ends the chain before basic auth runs, so +/// the user could never authenticate. Refused at startup rather than at runtime. +fn reject_provider_name_collision(auth_config: &AuthConfig) -> Result<(), Error> { + let collision = auth_config + .identity + .values() + .find(|identity| auth_config.oidc.contains_key(&identity.username)); + + match collision { + Some(identity) => Err(Error::Initialization(format!( + "basic-auth username '{}' is also an OIDC provider name", + identity.username + ))), + None => Ok(()), + } +} + #[cfg(test)] mod tests { + use std::fs; + use argon2::{ Algorithm, Argon2, Params, PasswordHasher, Version, password_hash::{SaltString, rand_core::OsRng}, }; use async_trait::async_trait; + use tempfile::tempdir; use super::*; use crate::{ - auth::PeerCertificate, + auth::{PeerCertificate, TokenIssuer, oidc::validator::tests::make_token}, cache, configuration::Configuration, identity::OidcClaims, metrics_provider, + secret::Secret, test_fixtures::{ configuration::{load_config, minimal_config}, mtls::cert_der, - requests::{empty_parts, parts_with_basic_auth}, + oidc::KID, + requests::{empty_parts, parts_with_authorization, parts_with_basic_auth}, + webhook::ca_bundle_pem, }, }; @@ -248,10 +338,6 @@ mod tests { minimal_config() } - fn test_http_client() -> Arc { - Arc::new(Client::new()) - } - #[test] fn test_auth_config_empty() { let config = create_minimal_config(); @@ -279,7 +365,7 @@ mod tests { let config = load_config( r#" [auth.oidc.github] - provider = "github" + issuer = "https://token.actions.githubusercontent.com" "#, ); @@ -328,13 +414,39 @@ mod tests { assert!(authenticator.is_ok()); } + /// The Basic username field selects the provider, so the two names cannot + /// both be honoured and the collision is a configuration mistake. + #[test] + fn a_basic_username_may_not_name_an_oidc_provider() { + let config = load_config( + r#" + [auth.identity.ci] + username = "github-actions" + password = "$argon2id$v=19$m=19456,t=2,p=1$test" + + [auth.oidc.github-actions] + issuer = "https://token.actions.githubusercontent.com" + "#, + ); + + let cache = cache::Config::Memory.to_backend().unwrap(); + + let Err(error) = Authenticator::new(&config, &cache) else { + panic!("a colliding name must be refused at startup"); + }; + + assert!( + matches!(&error, Error::Initialization(msg) if msg.contains("github-actions")), + "got: {error:?}" + ); + } + #[test] fn test_build_oidc_validators_empty() { let auth_config = AuthConfig::default(); let cache = cache::Config::Memory.to_backend().unwrap(); - let validators = - Authenticator::build_oidc_validators(&auth_config, &test_http_client(), &cache); + let validators = Authenticator::build_oidc_validators(&auth_config, &cache).unwrap(); assert!(validators.is_empty()); } @@ -344,14 +456,13 @@ mod tests { let config = load_config( r#" [auth.oidc.github] - provider = "github" + issuer = "https://token.actions.githubusercontent.com" "#, ); let cache = cache::Config::Memory.to_backend().unwrap(); - let validators = - Authenticator::build_oidc_validators(&config.auth, &test_http_client(), &cache); + let validators = Authenticator::build_oidc_validators(&config.auth, &cache).unwrap(); assert_eq!(validators.len(), 1); assert_eq!(validators[0].0, "github"); @@ -362,20 +473,65 @@ mod tests { let config = load_config( r#" [auth.oidc.custom] - provider = "generic" issuer = "https://auth.example.com" "#, ); let cache = cache::Config::Memory.to_backend().unwrap(); - let validators = - Authenticator::build_oidc_validators(&config.auth, &test_http_client(), &cache); + let validators = Authenticator::build_oidc_validators(&config.auth, &cache).unwrap(); assert_eq!(validators.len(), 1); assert_eq!(validators[0].0, "custom"); } + /// An issuer whose certificate the system roots do not cover, such as a + /// kube-apiserver, is reachable only through its own CA bundle. + #[test] + fn a_provider_may_trust_its_own_ca_bundle() { + let bundle = tempdir().unwrap(); + let bundle_path = bundle.path().join("ca.pem"); + fs::write(&bundle_path, ca_bundle_pem()).unwrap(); + + let config = load_config(&format!( + r#" + [auth.oidc.kube] + issuer = "https://kubernetes.default.svc" + server_ca_bundle = "{}" + "#, + bundle_path.display() + )); + + let cache = cache::Config::Memory.to_backend().unwrap(); + + assert_eq!( + config.auth.oidc["kube"].server_ca_bundle.as_deref(), + Some(bundle_path.as_path()) + ); + assert!(Authenticator::build_oidc_validators(&config.auth, &cache).is_ok()); + } + + #[test] + fn a_ca_bundle_that_does_not_load_is_refused_at_startup() { + let config = load_config( + r#" + [auth.oidc.kube] + issuer = "https://kubernetes.default.svc" + server_ca_bundle = "/nonexistent/ca.pem" + "#, + ); + + let cache = cache::Config::Memory.to_backend().unwrap(); + + let Err(error) = Authenticator::build_oidc_validators(&config.auth, &cache) else { + panic!("an unreadable CA bundle must be refused"); + }; + assert!( + matches!(&error, Error::Initialization(msg) if msg.contains("auth.oidc.kube")), + "got: {error:?}" + ); + } + #[tokio::test] async fn test_authenticate_request_no_credentials() { let config = create_minimal_config(); @@ -447,40 +603,6 @@ mod tests { assert!(matches!(result, Err(Error::Unauthorized(_)))); } - #[test] - fn select_auth_method_returns_mtls_when_only_mtls_succeeds() { - assert_eq!(select_auth_method(true, false, false), AuthMethod::Mtls); - } - - #[test] - fn select_auth_method_keeps_mtls_when_basic_also_succeeds() { - // Bug: basic-auth success used to overwrite mtls. Must not. - assert_eq!(select_auth_method(true, false, true), AuthMethod::Mtls); - } - - #[test] - fn select_auth_method_keeps_mtls_when_oidc_also_succeeds() { - assert_eq!(select_auth_method(true, true, false), AuthMethod::Mtls); - } - - #[test] - fn select_auth_method_returns_oidc_when_no_mtls() { - assert_eq!(select_auth_method(false, true, false), AuthMethod::Oidc); - } - - #[test] - fn select_auth_method_returns_basic_when_no_mtls_no_oidc() { - assert_eq!(select_auth_method(false, false, true), AuthMethod::Basic); - } - - #[test] - fn select_auth_method_returns_anonymous_when_nothing_succeeded() { - assert_eq!( - select_auth_method(false, false, false), - AuthMethod::Anonymous - ); - } - #[tokio::test] async fn test_authenticate_request_preserves_client_ip() { let config = create_minimal_config(); @@ -506,18 +628,16 @@ mod tests { let config = load_config( r#" [auth.oidc.github] - provider = "github" + issuer = "https://token.actions.githubusercontent.com" [auth.oidc.custom] - provider = "generic" issuer = "https://auth.example.com" "#, ); let cache = cache::Config::Memory.to_backend().unwrap(); - let validators = - Authenticator::build_oidc_validators(&config.auth, &test_http_client(), &cache); + let validators = Authenticator::build_oidc_validators(&config.auth, &cache).unwrap(); assert_eq!(validators.len(), 2); assert_eq!(validators[0].0, "custom"); @@ -550,7 +670,6 @@ mod tests { MockOutcome::Authenticated => { identity.oidc = Some(OidcClaims { provider_name: "mock".to_string(), - provider_type: "Mock".to_string(), claims: HashMap::new(), }); Ok(AuthResult::Authenticated) @@ -574,6 +693,7 @@ mod tests { Authenticator { mtls_validator: MtlsValidator::new(), + token_validator: None, oidc_validators, basic_auth_validator: BasicAuthValidator::new(&HashMap::new()).unwrap(), } @@ -590,7 +710,7 @@ mod tests { .try_oidc_authentication(&parts, &mut identity) .await; - assert!(!result.unwrap()); + assert!(result.unwrap().is_none()); assert!(identity.oidc.is_none()); } @@ -611,7 +731,7 @@ mod tests { .try_oidc_authentication(&parts, &mut identity) .await; - assert!(result.unwrap()); + assert_eq!(result.unwrap(), Some(AuthMethod::Oidc)); assert!(identity.oidc.is_some()); } @@ -651,7 +771,7 @@ mod tests { .try_oidc_authentication(&parts, &mut identity) .await; - assert!(!result.unwrap()); + assert!(result.unwrap().is_none()); assert!(identity.oidc.is_none()); } @@ -659,6 +779,23 @@ mod tests { // Helpers shared by method-tracking integration tests below. // --------------------------------------------------------------------------- + /// Built through `load_config` because `basic_auth::PasswordHash` is not + /// publicly constructible: its only path is deserialisation. + fn admin_basic_auth_validator() -> BasicAuthValidator { + let salt = SaltString::generate(OsRng); + let argon = Argon2::new(Algorithm::Argon2id, Version::V0x13, Params::default()); + let password_hash = argon.hash_password(b"secret", &salt).unwrap().to_string(); + let config = load_config(&format!( + r#" + [auth.identity.admin] + username = "admin" + password = "{password_hash}" + "#, + )); + + BasicAuthValidator::new(&config.auth.identity).unwrap() + } + fn make_authenticator_with_cert_and_mocks( validators: Vec<(&'static str, MockOutcome)>, ) -> (Authenticator, PeerCertificate) { @@ -667,13 +804,96 @@ mod tests { (authenticator, peer_cert) } + /// The two halves the token service splits into, built from one config so + /// the issued token is the one the chain's validator accepts. + fn make_authenticator_with_token_service( + validators: Vec<(&'static str, MockOutcome)>, + ) -> (Authenticator, TokenIssuer) { + let config = token_service::Config { + secret_key: Secret::new(vec![7; 32].into()), + realm: None, + ttl_secs: 3600, + }; + let authenticator = Authenticator { + token_validator: Some(TokenValidator::new(&config, &["mock".to_string()]).unwrap()), + ..make_authenticator_with_mocks(validators) + }; + + (authenticator, TokenIssuer::new(&config).unwrap()) + } + + // --------------------------------------------------------------------------- + // Registry token tests. + // --------------------------------------------------------------------------- + + /// The OIDC middlewares claim any bearer header and fail the request when they + /// cannot validate it, so without the short-circuit no reissued token works. + #[tokio::test] + async fn a_valid_token_skips_oidc_and_basic() { + metrics_provider::init_for_tests(); + let (authenticator, issuer) = make_authenticator_with_token_service(vec![( + "mock", + MockOutcome::Fail("must not be reached".to_string()), + )]); + let issued_from = ClientIdentity { + username: Some("ci-bot".to_string()), + ..Default::default() + }; + let (token, _) = issuer.issue(&issued_from).unwrap(); + + let parts = parts_with_authorization(&format!("Bearer {token}")); + let identity = authenticator + .authenticate_request(&parts, None) + .await + .unwrap(); + + assert_eq!(identity.auth_method, AuthMethod::Token); + assert_eq!(identity.username.as_deref(), Some("ci-bot")); + } + + #[tokio::test] + async fn a_bearer_that_is_not_ours_still_reaches_oidc() { + metrics_provider::init_for_tests(); + let (authenticator, _) = + make_authenticator_with_token_service(vec![("mock", MockOutcome::Authenticated)]); + + let token = make_token(&HashMap::new(), KID); + let parts = parts_with_authorization(&format!("Bearer {token}")); + let identity = authenticator + .authenticate_request(&parts, None) + .await + .unwrap(); + + assert_eq!(identity.auth_method, AuthMethod::Oidc); + } + + #[tokio::test] + async fn mtls_outranks_a_registry_token() { + metrics_provider::init_for_tests(); + let (authenticator, issuer) = + make_authenticator_with_token_service(vec![("mock", MockOutcome::NoCredentials)]); + let (token, _) = issuer.issue(&ClientIdentity::default()).unwrap(); + + let mut parts = parts_with_authorization(&format!("Bearer {token}")); + parts + .extensions + .insert(PeerCertificate(Arc::new(cert_der()))); + + let identity = authenticator + .authenticate_request(&parts, None) + .await + .unwrap(); + + assert_eq!(identity.auth_method, AuthMethod::Mtls); + } + // --------------------------------------------------------------------------- // Method-tracking integration tests. // --------------------------------------------------------------------------- /// mTLS succeeds + all OIDC providers return `NoCredentials`. /// The identity must carry certificate info and no OIDC claims. - /// `select_auth_method(true, false, false)` is `Mtls`; certificate not downgraded. + /// The reported method is `Mtls`; certificate not downgraded. #[tokio::test] async fn method_tracking_mtls_success_oidc_no_credentials_preserves_cert() { metrics_provider::init_for_tests(); @@ -701,11 +921,47 @@ mod tests { identity.oidc.is_none(), "oidc claims must not be set when no OIDC provider had credentials" ); + assert_eq!(identity.auth_method, AuthMethod::Mtls); + } + + /// mTLS and OIDC both succeed. The identity carries both credentials and + /// reports the stronger one. + #[tokio::test] + async fn mtls_outranks_a_successful_oidc_provider() { + metrics_provider::init_for_tests(); + let (authenticator, peer_cert) = + make_authenticator_with_cert_and_mocks(vec![("provider", MockOutcome::Authenticated)]); + + let mut parts = empty_parts(); + parts.extensions.insert(peer_cert); + + let identity = authenticator + .authenticate_request(&parts, None) + .await + .unwrap(); + + assert!(identity.oidc.is_some()); + assert_eq!(identity.auth_method, AuthMethod::Mtls); + } + + #[tokio::test] + async fn a_request_with_no_credentials_is_anonymous() { + metrics_provider::init_for_tests(); + let authenticator = make_authenticator_with_mocks(vec![ + ("alpha", MockOutcome::NoCredentials), + ("beta", MockOutcome::NoCredentials), + ]); + + let identity = authenticator + .authenticate_request(&empty_parts(), None) + .await + .unwrap(); + + assert_eq!(identity.auth_method, AuthMethod::Anonymous); } /// mTLS has no certificate (`NoCredentials`) + one OIDC provider succeeds. /// The identity must carry OIDC claims and no certificate info. - /// `select_auth_method(false, true, false)` is `Oidc`. #[tokio::test] async fn method_tracking_no_mtls_oidc_success_sets_oidc_identity() { metrics_provider::init_for_tests(); @@ -730,14 +986,15 @@ mod tests { && identity.certificate.organizations.is_empty(), "certificate info must be empty when no mTLS cert was presented" ); + assert_eq!(identity.auth_method, AuthMethod::Oidc); } /// mTLS succeeds + OIDC provider A fails, provider B also fails. /// The chain propagates the first OIDC error via `?`, so `authenticate_request` /// returns `Err`. The test verifies the error is the one from the - /// alphabetically-first provider ("alpha"). The method-label computation - /// (`select_auth_method`) is never reached in this path, which is correct - /// behaviour: an explicit OIDC credential rejection overrides mTLS success. + /// alphabetically-first provider ("alpha"). The method label is never + /// computed in this path, which is correct behaviour: an explicit OIDC + /// credential rejection overrides mTLS success. #[tokio::test] async fn method_tracking_mtls_success_oidc_all_fail_returns_oidc_error() { metrics_provider::init_for_tests(); @@ -760,7 +1017,7 @@ mod tests { /// When OIDC succeeds, basic auth is skipped entirely. /// Even if valid basic-auth credentials are present in the request, the - /// OIDC success short-circuits the basic-auth path (`if oidc_ok { false }`). + /// OIDC success short-circuits the basic-auth path. /// The identity carries OIDC claims; username is None (basic never ran). #[tokio::test] async fn method_tracking_oidc_success_skips_basic_auth() { @@ -791,6 +1048,7 @@ mod tests { let authenticator = Authenticator { mtls_validator: MtlsValidator::new(), + token_validator: None, oidc_validators, basic_auth_validator, }; @@ -813,5 +1071,49 @@ mod tests { identity.username.is_none(), "basic auth must be skipped when OIDC already succeeded; username must be None" ); + assert_eq!(identity.auth_method, AuthMethod::Oidc); + } + + #[tokio::test] + async fn basic_auth_is_reported_when_nothing_stronger_succeeds() { + metrics_provider::init_for_tests(); + + let authenticator = Authenticator { + basic_auth_validator: admin_basic_auth_validator(), + ..make_authenticator_with_mocks(vec![("provider", MockOutcome::NoCredentials)]) + }; + + let parts = parts_with_basic_auth("admin", "secret"); + let identity = authenticator + .authenticate_request(&parts, None) + .await + .unwrap(); + + assert_eq!(identity.username.as_deref(), Some("admin")); + assert_eq!(identity.auth_method, AuthMethod::Basic); + } + + /// Bug: a basic-auth success used to overwrite mTLS. Must not. + #[tokio::test] + async fn mtls_outranks_successful_basic_auth() { + metrics_provider::init_for_tests(); + + let authenticator = Authenticator { + basic_auth_validator: admin_basic_auth_validator(), + ..make_authenticator_with_mocks(vec![("provider", MockOutcome::NoCredentials)]) + }; + + let mut parts = parts_with_basic_auth("admin", "secret"); + parts + .extensions + .insert(PeerCertificate(Arc::new(cert_der()))); + + let identity = authenticator + .authenticate_request(&parts, None) + .await + .unwrap(); + + assert_eq!(identity.auth_method, AuthMethod::Mtls); + assert!(!identity.certificate.organizations.is_empty()); } } diff --git a/src/auth/authorizer/mod.rs b/src/auth/authorizer/mod.rs index 09e06707..d97205e9 100644 --- a/src/auth/authorizer/mod.rs +++ b/src/auth/authorizer/mod.rs @@ -60,7 +60,6 @@ struct AuditIdentity<'a> { certificate_organizations: &'a [String], certificate_common_names: &'a [String], oidc_provider_name: Option<&'a str>, - oidc_provider_type: Option<&'a str>, } // Debug is this projection's sole consumer; the manual impl (not a derive) @@ -75,7 +74,6 @@ impl fmt::Debug for AuditIdentity<'_> { .field("certificate_organizations", &self.certificate_organizations) .field("certificate_common_names", &self.certificate_common_names) .field("oidc_provider_name", &self.oidc_provider_name) - .field("oidc_provider_type", &self.oidc_provider_type) .finish() } } @@ -95,10 +93,6 @@ impl<'a> From<&'a ClientIdentity> for AuditIdentity<'a> { .oidc .as_ref() .map(|oidc| oidc.provider_name.as_str()), - oidc_provider_type: identity - .oidc - .as_ref() - .map(|oidc| oidc.provider_type.as_str()), } } } diff --git a/src/auth/authorizer/tests.rs b/src/auth/authorizer/tests.rs index d72fcdd5..ed868bd6 100644 --- a/src/auth/authorizer/tests.rs +++ b/src/auth/authorizer/tests.rs @@ -445,7 +445,6 @@ fn log_denial_uses_audit_identity_without_oidc_claims() { }, oidc: Some(OidcClaims { provider_name: "github-actions".to_string(), - provider_type: "GitHub Actions".to_string(), claims: HashMap::from([ ("sub".to_string(), json!("repo:private/repo:ref:main")), ("email".to_string(), json!("person@example.com")), @@ -454,6 +453,7 @@ fn log_denial_uses_audit_identity_without_oidc_claims() { }), client_ip: Some("192.0.2.10".to_string()), auth_method: AuthMethod::Mtls, + from_registry_token: false, }; tracing::subscriber::with_default(subscriber, || log_denial("test reason", &identity)); @@ -467,7 +467,6 @@ fn log_denial_uses_audit_identity_without_oidc_claims() { assert!(logs.contains("BuildOrg"), "logs were: {logs}"); assert!(logs.contains("build-cert"), "logs were: {logs}"); assert!(logs.contains("github-actions"), "logs were: {logs}"); - assert!(logs.contains("GitHub Actions"), "logs were: {logs}"); assert!(!logs.contains("person@example.com"), "logs were: {logs}"); assert!(!logs.contains("repo:private/repo"), "logs were: {logs}"); assert!(!logs.contains("internal-secret"), "logs were: {logs}"); diff --git a/src/auth/mod.rs b/src/auth/mod.rs index 3cac26da..3789cfa2 100644 --- a/src/auth/mod.rs +++ b/src/auth/mod.rs @@ -5,6 +5,7 @@ pub mod basic_auth; mod error; pub mod mtls; pub mod oidc; +pub mod token_service; pub mod webhook; use async_trait::async_trait; @@ -16,6 +17,7 @@ use hyper::http::request::Parts; pub use mtls::{MtlsValidator, PeerCertificate}; pub use oidc::OidcValidator; use sha2::{Digest as Sha2Digest, Sha256}; +pub use token_service::{TokenIssuer, TokenValidator}; use crate::identity::ClientIdentity; diff --git a/src/auth/mtls.rs b/src/auth/mtls.rs index 66d1fb5c..3c2a4b81 100644 --- a/src/auth/mtls.rs +++ b/src/auth/mtls.rs @@ -67,14 +67,21 @@ impl AuthMiddleware for MtlsValidator { return Ok(AuthResult::NoCredentials); }; - let (_, cert) = X509Certificate::from_der(&peer_cert.0).map_err(|e| { - error!( - error = ?e, - certificate_len = peer_cert.0.len(), - "Failed to parse client certificate" - ); - Error::Unauthorized("Invalid certificate".to_string()) - })?; + // The TLS layer already accepted this certificate, so a parse failure is + // two parsers disagreeing rather than a bad credential. Carrying on + // without certificate identity leaves the refusal to a policy that needs + // one, which is where it belongs. + let cert = match X509Certificate::from_der(&peer_cert.0) { + Ok((_, cert)) => cert, + Err(e) => { + error!( + error = ?e, + certificate_len = peer_cert.0.len(), + "Failed to parse client certificate" + ); + return Ok(AuthResult::NoCredentials); + } + }; debug!("Extracting identity from client certificate"); let cert_info = Self::extract_certificate_identity(&cert); @@ -88,26 +95,21 @@ impl AuthMiddleware for MtlsValidator { pub mod tests { use std::sync::Arc; - use hyper::StatusCode; use tracing::Level; use super::*; - use crate::{ - command::server::Error as ServerError, - test_fixtures::{ - logging::LogCapture, - mtls::{cert_der, minimal_cert_der}, - requests::empty_parts, - }, + use crate::test_fixtures::{ + logging::LogCapture, + mtls::{cert_der, minimal_cert_der}, + requests::empty_parts, }; - /// The parse error is logged server-side while the client only sees the - /// generic "Invalid certificate". This must remain the ONLY test driving - /// the certificate-parse-error branch: `tracing` caches callsite interest - /// process-globally, so a second test under a non-capturing subscriber - /// could cache the `error!` as disabled and make the log assertion flaky. + /// This must remain the ONLY test driving the parse-error branch: `tracing` caches callsite + /// interest process-globally, so a second test under a non-capturing + /// subscriber could cache the `error!` as disabled and make the log + /// assertion flaky. #[test] - fn malformed_certificate_is_logged_but_not_leaked_to_client() { + fn malformed_certificate_is_logged_and_yields_no_identity() { let capture = LogCapture::default(); let subscriber = tracing_subscriber::fmt() .with_max_level(Level::DEBUG) @@ -115,7 +117,7 @@ pub mod tests { .with_ansi(false) .finish(); - let result = tracing::subscriber::with_default(subscriber, || { + let (result, identity) = tracing::subscriber::with_default(subscriber, || { let runtime = tokio::runtime::Builder::new_current_thread() .build() .unwrap(); @@ -126,25 +128,14 @@ pub mod tests { .extensions .insert(PeerCertificate(Arc::new(vec![0u8; 100]))); let mut identity = ClientIdentity::new(None); - validator.authenticate(&parts, &mut identity).await + let result = validator.authenticate(&parts, &mut identity).await; + (result, identity) }) }); - match result.unwrap_err() { - Error::Unauthorized(msg) => { - assert_eq!(msg, "Invalid certificate"); - let error = ServerError::from(Error::Unauthorized(msg)); - assert_eq!(error.status_code(), StatusCode::UNAUTHORIZED); - let body = error.as_json(None).to_string(); - assert!(body.contains("Invalid certificate")); - assert!( - !body.contains("UnexpectedTag"), - "raw parse error leaked: {body}" - ); - assert!(!body.contains("Malformed client certificate")); - } - err => panic!("expected Unauthorized, got {err:?}"), - } + assert!(matches!(result.unwrap(), AuthResult::NoCredentials)); + assert!(identity.certificate.common_names.is_empty()); + assert!(identity.certificate.organizations.is_empty()); let logs = capture.contents(); assert!( diff --git a/src/auth/oidc/jwk.rs b/src/auth/oidc/jwk.rs index bc72ea3d..a4434d71 100644 --- a/src/auth/oidc/jwk.rs +++ b/src/auth/oidc/jwk.rs @@ -34,17 +34,22 @@ impl Jwk { } } + /// A key the provider served but angos cannot use is the provider's failure, + /// not the caller's credential, so it reports the provider unavailable like + /// the JWKS fetch and parse that preceded it. pub fn to_decoding_key(&self) -> Result { match self { Jwk::Rsa { n, e, alg, kid, .. } => { debug!("Creating RSA DecodingKey from JWK with alg={alg:?}, kid={kid:?}"); - DecodingKey::from_rsa_components(n, e) - .map_err(|e| Error::Initialization(format!("Failed to create RSA key: {e}"))) + DecodingKey::from_rsa_components(n, e).map_err(|e| { + Error::ProviderUnavailable(format!("Failed to create RSA key: {e}")) + }) } Jwk::Ec { x, y, alg, kid, .. } => { debug!("Creating EC DecodingKey from JWK with alg={alg:?}, kid={kid:?}"); - DecodingKey::from_ec_components(x, y) - .map_err(|e| Error::Initialization(format!("Failed to create EC key: {e}"))) + DecodingKey::from_ec_components(x, y).map_err(|e| { + Error::ProviderUnavailable(format!("Failed to create EC key: {e}")) + }) } } } @@ -89,4 +94,37 @@ mod tests { let jwk: Result = serde_json::from_str(unsupported_json); assert!(jwk.is_err()); } + + /// The key material is fetched per request, so an unusable one must not be + /// reported as a startup failure and answered with a 500. + #[test] + fn an_unusable_key_reports_the_provider_unavailable() { + let undecodable = "not base64".to_string(); + let keys = [ + Jwk::Rsa { + key_use: None, + kid: None, + alg: None, + n: undecodable.clone(), + e: "AQAB".to_string(), + }, + Jwk::Ec { + key_use: None, + kid: None, + alg: None, + x: undecodable.clone(), + y: undecodable, + }, + ]; + + for jwk in keys { + let Err(error) = jwk.to_decoding_key() else { + panic!("a JWK angos cannot decode must not yield a key: {jwk:?}"); + }; + assert!( + matches!(error, Error::ProviderUnavailable(_)), + "got: {error:?}" + ); + } + } } diff --git a/src/auth/oidc/mod.rs b/src/auth/oidc/mod.rs index e7adfcab..7a6dafad 100644 --- a/src/auth/oidc/mod.rs +++ b/src/auth/oidc/mod.rs @@ -1,13 +1,12 @@ pub mod jwk; -pub mod provider; pub mod validator; -use std::sync::Arc; +use std::{collections::HashMap, path::PathBuf, sync::Arc}; use async_trait::async_trait; use hyper::http::request::Parts; +use jsonwebtoken::Algorithm; pub use jwk::Jwk; -pub use provider::OidcProvider; use reqwest::Client; use serde::Deserialize; use tracing::debug; @@ -16,31 +15,90 @@ use crate::{ auth::{ AuthMiddleware, AuthResult, Error, authorization::{basic_credentials, bearer_token}, - oidc::provider::{BaseConfig, generic, github}, }, cache::Cache, identity::{ClientIdentity, OidcClaims}, }; +/// An OIDC provider: the issuer to trust, and how tokens it signed are validated. +/// +/// Providers differ only by configuration, so there is one type rather than one +/// per vendor. What a vendor "is" reduces to its issuer, its JWKS location, and +/// the claims its tokens are expected to carry. #[derive(Clone, Debug, Deserialize)] -#[serde(tag = "provider", rename_all = "lowercase")] -pub enum Config { - Generic(BaseConfig), - GitHub(github::ProviderConfig), +pub struct Config { + pub issuer: String, + /// CA bundle trusted for the discovery and JWKS fetches, for an issuer whose + /// certificate the system roots do not cover, such as a kube-apiserver. + #[serde(default)] + pub server_ca_bundle: Option, + /// Discovered from the issuer's `.well-known/openid-configuration` when omitted. + #[serde(default)] + pub jwks_uri: Option, + /// Claims a token must carry beyond the ones JWT validation itself checks. + /// Presence only: predicates over claim *values* belong in the access + /// policy, which sees the whole claim map. + #[serde(default)] + pub required_claims: Vec, + #[serde(default = "Config::default_jwks_refresh_interval")] + pub jwks_refresh_interval: u64, + #[serde(default)] + pub required_audience: Option, + #[serde(default = "Config::default_clock_skew_tolerance")] + pub clock_skew_tolerance: u64, + #[serde(default = "Config::default_allowed_algorithms")] + pub allowed_algorithms: Vec, + /// Timeout for an OIDC HTTP fetch (JWKS or discovery document). + #[serde(default = "Config::default_http_request_timeout_secs")] + pub http_request_timeout_secs: u64, + /// Timeout for the forced JWKS refetch triggered when a cached JWKS is + /// missing the token's key id (a rotated signing key). + #[serde(default = "Config::default_jwks_refresh_timeout_secs")] + pub jwks_refresh_timeout_secs: u64, } impl Config { - pub fn to_backend(&self) -> Arc { - match self { - Config::Generic(config) => Arc::new(generic::Provider::new(config.clone())), - Config::GitHub(config) => Arc::new(github::Provider::new(config.clone())), + fn default_jwks_refresh_interval() -> u64 { + 3600 + } + + fn default_clock_skew_tolerance() -> u64 { + 60 + } + + fn default_allowed_algorithms() -> Vec { + vec![Algorithm::RS256] + } + + fn default_http_request_timeout_secs() -> u64 { + 30 + } + + fn default_jwks_refresh_timeout_secs() -> u64 { + 5 + } + + /// Rejects a token missing any claim the provider requires. A claim present + /// but null counts as missing: a null carries no more identity than an + /// absent key, and a policy reading it would see the same nothing. + pub(crate) fn verify_required_claims( + &self, + claims: &HashMap, + ) -> Result<(), Error> { + for name in &self.required_claims { + if claims.get(name).is_none_or(serde_json::Value::is_null) { + return Err(Error::Unauthorized(format!( + "token is missing required claim '{name}'" + ))); + } } + Ok(()) } } pub struct OidcValidator { provider_name: String, - provider: Arc, + config: Config, client: Arc, cache: Arc, } @@ -48,15 +106,13 @@ pub struct OidcValidator { impl OidcValidator { pub fn new( provider_name: String, - provider_config: &Config, + config: &Config, client: Arc, cache: Arc, ) -> Self { - let provider = provider_config.to_backend(); - Self { provider_name, - provider, + config: config.clone(), client, cache, } @@ -65,7 +121,7 @@ impl OidcValidator { pub async fn validate_token(&self, token: &str) -> Result { validator::validate_oidc_token( &self.provider_name, - &*self.provider, + &self.config, token, &self.client, self.cache.as_ref(), @@ -91,8 +147,8 @@ impl AuthMiddleware for OidcValidator { let subject = claims.claims.get("sub").and_then(|v| v.as_str()); let issuer = claims.claims.get("iss").and_then(|v| v.as_str()); debug!( - "OIDC token validated for provider '{}' (type='{}', sub={:?}, iss={:?})", - claims.provider_name, claims.provider_type, subject, issuer + "OIDC token validated for provider '{}' (sub={:?}, iss={:?})", + claims.provider_name, subject, issuer ); identity.oidc = Some(claims); Ok(AuthResult::Authenticated) @@ -140,10 +196,7 @@ mod tests { use super::*; use crate::{ - auth::oidc::{ - provider::github::tests::default_github_config, - validator::tests::{build_test_provider_config, make_token, valid_claims}, - }, + auth::oidc::validator::tests::{build_test_provider_config, make_token, valid_claims}, cache, identity::ClientIdentity, test_fixtures::{ @@ -154,10 +207,10 @@ mod tests { }; fn build_config(issuer: &str) -> Config { - Config::Generic(BaseConfig { + Config { required_audience: None, ..build_test_provider_config(issuer) - }) + } } fn make_test_token(issuer: &str) -> String { @@ -172,66 +225,69 @@ mod tests { } #[test] - fn test_config_deserialize_generic() { + fn test_config_deserialize_minimal() { let toml = r#" - provider = "generic" issuer = "https://auth.example.com" - jwks_uri = "https://auth.example.com/jwks" "#; let config: Config = toml::from_str(toml).unwrap(); - match config { - Config::Generic(cfg) => { - assert_eq!(cfg.issuer, "https://auth.example.com"); - assert_eq!( - cfg.jwks_uri, - Some("https://auth.example.com/jwks".to_string()) - ); - } - Config::GitHub(_) => panic!("Expected Generic config"), - } + assert_eq!(config.issuer, "https://auth.example.com"); + assert!(config.jwks_uri.is_none()); + assert!(config.required_claims.is_empty()); + assert_eq!(config.jwks_refresh_interval, 3600); + assert!(config.required_audience.is_none()); + assert_eq!(config.clock_skew_tolerance, 60); + assert_eq!(config.allowed_algorithms, vec![Algorithm::RS256]); + assert_eq!(config.http_request_timeout_secs, 30); + assert_eq!(config.jwks_refresh_timeout_secs, 5); } #[test] - fn test_config_deserialize_github() { + fn test_config_deserialize_full() { let toml = r#" - provider = "github" - issuer = "https://token.actions.githubusercontent.com" + issuer = "https://auth.example.com" + jwks_uri = "https://auth.example.com/jwks" + required_claims = ["repository", "actor"] + jwks_refresh_interval = 7200 + required_audience = "my-app" + clock_skew_tolerance = 120 + allowed_algorithms = ["RS256", "ES256"] "#; let config: Config = toml::from_str(toml).unwrap(); - match config { - Config::GitHub(cfg) => { - assert_eq!(cfg.issuer, "https://token.actions.githubusercontent.com"); - } - Config::Generic(_) => panic!("Expected GitHub config"), - } - } - - #[test] - fn test_config_to_backend_generic() { - let config = build_config("https://auth.example.com"); - - let provider = config.to_backend(); - assert_eq!(provider.base_config().issuer, "https://auth.example.com"); - assert_eq!(provider.name(), "Generic OIDC"); + assert_eq!( + config.jwks_uri, + Some("https://auth.example.com/jwks".to_string()) + ); + assert_eq!(config.required_claims, vec!["repository", "actor"]); + assert_eq!(config.jwks_refresh_interval, 7200); + assert_eq!(config.required_audience, Some("my-app".to_string())); + assert_eq!(config.clock_skew_tolerance, 120); + assert_eq!( + config.allowed_algorithms, + vec![Algorithm::RS256, Algorithm::ES256] + ); } + /// GitHub Actions used to be its own provider type. Its entire definition + /// is now a config entry, so this pins that the shape still loads without + /// a dedicated variant. #[test] - fn test_config_to_backend_github() { - let config = Config::GitHub(default_github_config()); + fn test_config_deserialize_github_actions_shape() { + let toml = r#" + issuer = "https://token.actions.githubusercontent.com" + jwks_uri = "https://token.actions.githubusercontent.com/.well-known/jwks" + required_claims = ["repository", "actor"] + "#; - let provider = config.to_backend(); - assert_eq!( - provider.base_config().issuer, - "https://token.actions.githubusercontent.com" - ); - assert_eq!(provider.name(), "GitHub Actions"); + let config: Config = toml::from_str(toml).unwrap(); + assert_eq!(config.issuer, "https://token.actions.githubusercontent.com"); + assert_eq!(config.required_claims, vec!["repository", "actor"]); } #[test] - fn test_oidc_validator_new_generic() { - let config = Config::Generic(build_test_provider_config("https://auth.example.com")); + fn test_oidc_validator_new() { + let config = build_test_provider_config("https://auth.example.com"); let cache = cache::Config::Memory.to_backend().unwrap(); let client = test_http_client(); @@ -239,28 +295,10 @@ mod tests { OidcValidator::new("test-provider".to_string(), &config, client.clone(), cache); assert_eq!(validator.provider_name, "test-provider"); - assert_eq!( - validator.provider.base_config().issuer, - "https://auth.example.com" - ); + assert_eq!(validator.config.issuer, "https://auth.example.com"); assert!(Arc::ptr_eq(&validator.client, &client)); } - #[test] - fn test_oidc_validator_new_github() { - let config = Config::GitHub(default_github_config()); - - let cache = cache::Config::Memory.to_backend().unwrap(); - let validator = - OidcValidator::new("github".to_string(), &config, test_http_client(), cache); - - assert_eq!(validator.provider_name, "github"); - assert_eq!( - validator.provider.base_config().issuer, - "https://token.actions.githubusercontent.com" - ); - } - #[tokio::test] async fn test_validate_token_success() { let mock_server = MockServer::start().await; @@ -282,7 +320,6 @@ mod tests { assert!(result.is_ok()); let oidc_claims = result.unwrap(); assert_eq!(oidc_claims.provider_name, "test-provider"); - assert_eq!(oidc_claims.provider_type, "Generic OIDC"); assert_eq!(oidc_claims.claims.get("sub").unwrap(), "test-user"); } @@ -452,7 +489,6 @@ mod tests { let oidc_claims = identity.oidc.unwrap(); assert_eq!(oidc_claims.provider_name, "my-provider"); - assert_eq!(oidc_claims.provider_type, "Generic OIDC"); assert_eq!(oidc_claims.claims.get("sub").unwrap(), "user-123"); assert_eq!(oidc_claims.claims.get("email").unwrap(), "user@example.com"); assert_eq!(identity.client_ip, Some("192.168.1.1".to_string())); diff --git a/src/auth/oidc/provider/generic.rs b/src/auth/oidc/provider/generic.rs deleted file mode 100644 index 6c7135bf..00000000 --- a/src/auth/oidc/provider/generic.rs +++ /dev/null @@ -1,69 +0,0 @@ -use crate::auth::oidc::provider::{BaseConfig, OidcProvider}; - -pub struct Provider { - base: BaseConfig, -} - -impl Provider { - pub fn new(base: BaseConfig) -> Self { - Self { base } - } -} - -impl OidcProvider for Provider { - fn base_config(&self) -> &BaseConfig { - &self.base - } - - fn name(&self) -> &'static str { - "Generic OIDC" - } -} - -#[cfg(test)] -mod tests { - use jsonwebtoken::Algorithm; - - use super::*; - - #[test] - fn test_config_deserialize_minimal() { - let toml = r#" - issuer = "https://example.com" - "#; - - let config: BaseConfig = toml::from_str(toml).unwrap(); - assert_eq!(config.issuer, "https://example.com"); - assert!(config.jwks_uri.is_none()); - assert_eq!(config.jwks_refresh_interval, 3600); - assert!(config.required_audience.is_none()); - assert_eq!(config.clock_skew_tolerance, 60); - assert_eq!(config.allowed_algorithms, vec![Algorithm::RS256]); - } - - #[test] - fn test_config_deserialize_full() { - let toml = r#" - issuer = "https://auth.example.com" - jwks_uri = "https://auth.example.com/jwks" - jwks_refresh_interval = 7200 - required_audience = "my-app" - clock_skew_tolerance = 120 - allowed_algorithms = ["RS256", "ES256"] - "#; - - let config: BaseConfig = toml::from_str(toml).unwrap(); - assert_eq!(config.issuer, "https://auth.example.com"); - assert_eq!( - config.jwks_uri, - Some("https://auth.example.com/jwks".to_string()) - ); - assert_eq!(config.jwks_refresh_interval, 7200); - assert_eq!(config.required_audience, Some("my-app".to_string())); - assert_eq!(config.clock_skew_tolerance, 120); - assert_eq!( - config.allowed_algorithms, - vec![Algorithm::RS256, Algorithm::ES256] - ); - } -} diff --git a/src/auth/oidc/provider/github.rs b/src/auth/oidc/provider/github.rs deleted file mode 100644 index 6eece633..00000000 --- a/src/auth/oidc/provider/github.rs +++ /dev/null @@ -1,245 +0,0 @@ -use std::collections::HashMap; - -use jsonwebtoken::Algorithm; -use serde::{Deserialize, Serialize}; - -use crate::{ - auth::Error, - auth::oidc::provider::{BaseConfig, OidcProvider}, -}; - -#[derive(Clone, Debug, Deserialize, Serialize)] -pub struct ProviderConfig { - #[serde(default = "default_github_issuer")] - pub issuer: String, - #[serde(default = "default_github_jwks_uri")] - pub jwks_uri: String, - #[serde(default = "BaseConfig::default_jwks_refresh_interval")] - pub jwks_refresh_interval: u64, - #[serde(default)] - pub required_audience: Option, - #[serde(default = "BaseConfig::default_clock_skew_tolerance")] - pub clock_skew_tolerance: u64, - #[serde(default = "BaseConfig::default_allowed_algorithms")] - pub allowed_algorithms: Vec, - #[serde(default = "BaseConfig::default_http_request_timeout_secs")] - pub http_request_timeout_secs: u64, - #[serde(default = "BaseConfig::default_jwks_refresh_timeout_secs")] - pub jwks_refresh_timeout_secs: u64, -} - -fn default_github_issuer() -> String { - "https://token.actions.githubusercontent.com".to_string() -} - -fn default_github_jwks_uri() -> String { - "https://token.actions.githubusercontent.com/.well-known/jwks".to_string() -} - -pub struct Provider { - base: BaseConfig, -} - -impl Provider { - pub fn new(config: ProviderConfig) -> Self { - Self { - base: BaseConfig { - issuer: config.issuer, - jwks_uri: Some(config.jwks_uri), - jwks_refresh_interval: config.jwks_refresh_interval, - required_audience: config.required_audience, - clock_skew_tolerance: config.clock_skew_tolerance, - allowed_algorithms: config.allowed_algorithms, - http_request_timeout_secs: config.http_request_timeout_secs, - jwks_refresh_timeout_secs: config.jwks_refresh_timeout_secs, - }, - } - } -} - -impl OidcProvider for Provider { - fn base_config(&self) -> &BaseConfig { - &self.base - } - - fn name(&self) -> &'static str { - "GitHub Actions" - } - - fn validate_provider_claims( - &self, - claims: &HashMap, - ) -> Result<(), Error> { - if !claims.contains_key("repository") { - let msg = "Missing repository claim in GitHub token".to_string(); - return Err(Error::Unauthorized(msg)); - } - if !claims.contains_key("actor") { - let msg = "Missing actor claim in GitHub token".to_string(); - return Err(Error::Unauthorized(msg)); - } - Ok(()) - } -} - -#[cfg(test)] -pub mod tests { - use super::*; - use crate::auth::Error; - - pub fn default_github_config() -> ProviderConfig { - ProviderConfig { - issuer: default_github_issuer(), - jwks_uri: default_github_jwks_uri(), - jwks_refresh_interval: BaseConfig::default_jwks_refresh_interval(), - required_audience: None, - clock_skew_tolerance: BaseConfig::default_clock_skew_tolerance(), - allowed_algorithms: BaseConfig::default_allowed_algorithms(), - http_request_timeout_secs: BaseConfig::default_http_request_timeout_secs(), - jwks_refresh_timeout_secs: BaseConfig::default_jwks_refresh_timeout_secs(), - } - } - - #[test] - fn test_config_deserialize_minimal() { - let toml = r#" - issuer = "https://token.actions.githubusercontent.com" - jwks_uri = "https://token.actions.githubusercontent.com/.well-known/jwks" - "#; - - let config: ProviderConfig = toml::from_str(toml).unwrap(); - assert_eq!(config.issuer, "https://token.actions.githubusercontent.com"); - assert_eq!( - config.jwks_uri, - "https://token.actions.githubusercontent.com/.well-known/jwks" - ); - assert_eq!(config.jwks_refresh_interval, 3600); - assert!(config.required_audience.is_none()); - assert_eq!(config.clock_skew_tolerance, 60); - assert_eq!(config.allowed_algorithms, vec![Algorithm::RS256]); - } - - #[test] - fn test_config_deserialize_with_defaults() { - let toml = r""; - - let config: ProviderConfig = toml::from_str(toml).unwrap(); - assert_eq!(config.issuer, "https://token.actions.githubusercontent.com"); - assert_eq!( - config.jwks_uri, - "https://token.actions.githubusercontent.com/.well-known/jwks" - ); - assert_eq!(config.jwks_refresh_interval, 3600); - assert_eq!(config.clock_skew_tolerance, 60); - assert_eq!(config.allowed_algorithms, vec![Algorithm::RS256]); - } - - #[test] - fn test_config_deserialize_partial_override() { - // Setting only one of issuer/jwks_uri in TOML must override that field - // while the unspecified field still takes its per-field serde default. - let custom_issuer: ProviderConfig = toml::from_str( - r#" - issuer = "https://custom.example.com" - "#, - ) - .unwrap(); - assert_eq!(custom_issuer.issuer, "https://custom.example.com"); - assert_eq!( - custom_issuer.jwks_uri, - "https://token.actions.githubusercontent.com/.well-known/jwks" - ); - - let custom_jwks: ProviderConfig = toml::from_str( - r#" - jwks_uri = "https://custom.example.com/.well-known/jwks" - "#, - ) - .unwrap(); - assert_eq!( - custom_jwks.issuer, - "https://token.actions.githubusercontent.com" - ); - assert_eq!( - custom_jwks.jwks_uri, - "https://custom.example.com/.well-known/jwks" - ); - } - - #[test] - fn test_config_deserialize_full() { - let toml = r#" - issuer = "https://custom.github.com" - jwks_uri = "https://custom.github.com/jwks" - jwks_refresh_interval = 7200 - required_audience = "my-app" - clock_skew_tolerance = 120 - allowed_algorithms = ["RS256", "ES256"] - "#; - - let config: ProviderConfig = toml::from_str(toml).unwrap(); - assert_eq!(config.issuer, "https://custom.github.com"); - assert_eq!(config.jwks_uri, "https://custom.github.com/jwks"); - assert_eq!(config.jwks_refresh_interval, 7200); - assert_eq!(config.required_audience, Some("my-app".to_string())); - assert_eq!(config.clock_skew_tolerance, 120); - assert_eq!( - config.allowed_algorithms, - vec![Algorithm::RS256, Algorithm::ES256] - ); - } - - #[test] - fn test_validate_provider_claims_success() { - let provider = Provider::new(default_github_config()); - - let mut claims = HashMap::new(); - claims.insert("repository".to_string(), serde_json::json!("org/repo")); - claims.insert("actor".to_string(), serde_json::json!("user")); - claims.insert("extra".to_string(), serde_json::json!("data")); - - assert!(provider.validate_provider_claims(&claims).is_ok()); - } - - #[test] - fn test_validate_provider_claims_missing_repository() { - let provider = Provider::new(default_github_config()); - - let mut claims = HashMap::new(); - claims.insert("actor".to_string(), serde_json::json!("user")); - - let result = provider.validate_provider_claims(&claims); - assert!(result.is_err()); - match result.unwrap_err() { - Error::Unauthorized(msg) => { - assert!(msg.contains("repository")); - } - err => panic!("Expected Unauthorized error, got {err:?}"), - } - } - - #[test] - fn test_validate_provider_claims_missing_actor() { - let provider = Provider::new(default_github_config()); - - let mut claims = HashMap::new(); - claims.insert("repository".to_string(), serde_json::json!("org/repo")); - - let result = provider.validate_provider_claims(&claims); - assert!(result.is_err()); - match result.unwrap_err() { - Error::Unauthorized(msg) => { - assert!(msg.contains("actor")); - } - err => panic!("Expected Unauthorized error, got {err:?}"), - } - } - - #[test] - fn test_validate_provider_claims_empty() { - let provider = Provider::new(default_github_config()); - let claims = HashMap::new(); - let result = provider.validate_provider_claims(&claims); - assert!(matches!(&result, Err(Error::Unauthorized(_)))); - } -} diff --git a/src/auth/oidc/provider/mod.rs b/src/auth/oidc/provider/mod.rs deleted file mode 100644 index 59328822..00000000 --- a/src/auth/oidc/provider/mod.rs +++ /dev/null @@ -1,70 +0,0 @@ -pub mod generic; -pub mod github; - -use std::collections::HashMap; - -use jsonwebtoken::Algorithm; -use serde::{Deserialize, Serialize}; - -use crate::auth::Error; - -/// Shared OIDC provider configuration. The generic provider deserializes into -/// it directly; the GitHub provider builds it from its own defaults. -#[derive(Clone, Debug, Deserialize, Serialize)] -pub struct BaseConfig { - pub issuer: String, - #[serde(default)] - pub jwks_uri: Option, - #[serde(default = "BaseConfig::default_jwks_refresh_interval")] - pub jwks_refresh_interval: u64, - #[serde(default)] - pub required_audience: Option, - #[serde(default = "BaseConfig::default_clock_skew_tolerance")] - pub clock_skew_tolerance: u64, - #[serde(default = "BaseConfig::default_allowed_algorithms")] - pub allowed_algorithms: Vec, - /// Timeout for an OIDC HTTP fetch (JWKS or discovery document). - #[serde(default = "BaseConfig::default_http_request_timeout_secs")] - pub http_request_timeout_secs: u64, - /// Timeout for the forced JWKS refetch triggered when a cached JWKS is - /// missing the token's key id (a rotated signing key). - #[serde(default = "BaseConfig::default_jwks_refresh_timeout_secs")] - pub jwks_refresh_timeout_secs: u64, -} - -impl BaseConfig { - pub fn default_jwks_refresh_interval() -> u64 { - 3600 - } - - pub fn default_clock_skew_tolerance() -> u64 { - 60 - } - - pub fn default_allowed_algorithms() -> Vec { - vec![Algorithm::RS256] - } - - pub fn default_http_request_timeout_secs() -> u64 { - 30 - } - - pub fn default_jwks_refresh_timeout_secs() -> u64 { - 5 - } -} - -/// An OIDC provider: its shared [`BaseConfig`] plus provider-specific claim -/// validation. Field access goes through [`OidcProvider::base_config`]. -pub trait OidcProvider: Send + Sync { - fn base_config(&self) -> &BaseConfig; - - fn name(&self) -> &'static str; - - fn validate_provider_claims( - &self, - _claims: &HashMap, - ) -> Result<(), Error> { - Ok(()) - } -} diff --git a/src/auth/oidc/validator/mod.rs b/src/auth/oidc/validator/mod.rs index 8a89ed8c..d0a83305 100644 --- a/src/auth/oidc/validator/mod.rs +++ b/src/auth/oidc/validator/mod.rs @@ -9,7 +9,7 @@ use tracing::{debug, info, warn}; use crate::{ auth::Error, auth::{ - oidc::{Jwk, OidcProvider}, + oidc::{Config, Jwk}, sha256_hex, }, cache::Cache, @@ -45,7 +45,7 @@ struct CachedJsonRequest<'a> { pub async fn validate_oidc_token( provider_name: &str, - provider: &dyn OidcProvider, + provider: &Config, token: &str, client: &Client, cache: &Cache, @@ -70,7 +70,7 @@ fn verify_jwt_with_header( header: &Header, jwks: &Jwks, provider_name: &str, - provider: &dyn OidcProvider, + provider: &Config, ) -> Result { debug!( "JWT header: alg={:?}, kid={:?}, typ={:?}", @@ -114,37 +114,40 @@ fn verify_jwt_with_header( }, )?; - provider.validate_provider_claims(&token_data.claims)?; + provider.verify_required_claims(&token_data.claims)?; - debug!("{} provider: Token validated successfully", provider.name()); + debug!( + "Token validated successfully for issuer {}", + provider.issuer + ); Ok(OidcClaims { provider_name: provider_name.to_string(), - provider_type: provider.name().to_string(), claims: token_data.claims, }) } -fn verify_allowed_algorithm(provider: &dyn OidcProvider, alg: Algorithm) -> Result<(), Error> { - if provider.base_config().allowed_algorithms.contains(&alg) { +fn verify_allowed_algorithm(provider: &Config, alg: Algorithm) -> Result<(), Error> { + if provider.allowed_algorithms.contains(&alg) { return Ok(()); } Err(Error::Unauthorized(format!( - "algorithm {alg:?} not allowed for provider {}", - provider.name() + "algorithm {alg:?} not allowed for issuer {}", + provider.issuer ))) } -fn build_validation(provider: &dyn OidcProvider, alg: Algorithm) -> Validation { - let base = provider.base_config(); +fn build_validation(provider: &Config, alg: Algorithm) -> Validation { let mut validation = Validation::new(alg); - validation.algorithms.clone_from(&base.allowed_algorithms); - validation.set_issuer(&[base.issuer.as_str()]); - if let Some(aud) = &base.required_audience { + validation + .algorithms + .clone_from(&provider.allowed_algorithms); + validation.set_issuer(&[provider.issuer.as_str()]); + if let Some(aud) = &provider.required_audience { validation.set_audience(&[aud.as_str()]); } else { validation.validate_aud = false; } - validation.leeway = base.clock_skew_tolerance; + validation.leeway = provider.clock_skew_tolerance; validation.validate_exp = true; validation.validate_nbf = true; validation @@ -192,12 +195,12 @@ where } async fn get_jwks_url( - provider: &dyn OidcProvider, + provider: &Config, client: &Client, cache: &Cache, fetch_timeout: Option, ) -> Result { - if let Some(uri) = provider.base_config().jwks_uri.as_deref() { + if let Some(uri) = provider.jwks_uri.as_deref() { return Ok(uri.to_string()); } let oidc_config = @@ -212,22 +215,22 @@ async fn get_jwks_url( /// request. The cost is that a key rotation can take this long to be picked up. const JWKS_REFRESH_COOLDOWN_SECS: u64 = 60; -fn jwks_refresh_cooldown_key(provider: &dyn OidcProvider) -> String { - let provider_name = provider.name(); - let issuer_hash = sha256_hex(&provider.base_config().issuer); - format!("oidc:{provider_name}:jwks-refresh:{issuer_hash}") +/// The keys below name the issuer, not the config entry: two entries trusting +/// one issuer describe the same signing keys, so they share the cached document +/// rather than each fetching their own. +fn jwks_refresh_cooldown_key(provider: &Config) -> String { + let issuer_hash = sha256_hex(&provider.issuer); + format!("oidc:jwks-refresh:{issuer_hash}") } -fn jwks_cache_key(provider: &dyn OidcProvider) -> String { - let provider_name = provider.name(); - let issuer_hash = sha256_hex(&provider.base_config().issuer); - format!("oidc:{provider_name}:jwks:{issuer_hash}") +fn jwks_cache_key(provider: &Config) -> String { + let issuer_hash = sha256_hex(&provider.issuer); + format!("oidc:jwks:{issuer_hash}") } -fn oidc_configuration_cache_key(provider: &dyn OidcProvider) -> String { - let provider_name = provider.name(); - let issuer_hash = sha256_hex(&provider.base_config().issuer); - format!("oidc:{provider_name}:config:{issuer_hash}") +fn oidc_configuration_cache_key(provider: &Config) -> String { + let issuer_hash = sha256_hex(&provider.issuer); + format!("oidc:config:{issuer_hash}") } async fn fetch_cached_json( @@ -277,11 +280,11 @@ where /// Load the provider's JWKS, preferring the cache. `from_cache` on the result /// tells the caller whether it may still be stale for a just-rotated key. async fn fetch_jwks( - provider: &dyn OidcProvider, + provider: &Config, client: &Client, cache: &Cache, ) -> Result, Error> { - let timeout = Duration::from_secs(provider.base_config().http_request_timeout_secs); + let timeout = Duration::from_secs(provider.http_request_timeout_secs); let cache_key = jwks_cache_key(provider); let jwks_url = get_jwks_url(provider, client, cache, Some(timeout)).await?; let fetched = fetch_cached_json::( @@ -290,7 +293,7 @@ async fn fetch_jwks( cache, cache_key: &cache_key, url: &jwks_url, - ttl: provider.base_config().jwks_refresh_interval, + ttl: provider.jwks_refresh_interval, read_cache: true, fetch_timeout: Some(timeout), }, @@ -310,7 +313,7 @@ async fn fetch_jwks( /// The marker is claimed before fetching, so a burst of unknown-kid requests /// costs one outbound fetch rather than one each. async fn refresh_jwks_rate_limited( - provider: &dyn OidcProvider, + provider: &Config, client: &Client, cache: &Cache, ) -> Result>, Error> { @@ -323,8 +326,8 @@ async fn refresh_jwks_rate_limited( .is_some() { debug!( - "Skipping JWKS refresh for provider {}: one already ran within the cooldown", - provider.name() + "Skipping JWKS refresh for issuer {}: one already ran within the cooldown", + provider.issuer ); return Ok(None); } @@ -332,18 +335,18 @@ async fn refresh_jwks_rate_limited( .store_value(&cooldown_key, "1", JWKS_REFRESH_COOLDOWN_SECS) .await; - info!("Refreshing JWKS for provider {}", provider.name()); + info!("Refreshing JWKS for issuer {}", provider.issuer); refresh_jwks(provider, client, cache).await.map(Some) } /// Force a fresh JWKS fetch, bypassing the cache under a short timeout. Used /// when a cached JWKS is missing the token's key id (a rotated signing key). async fn refresh_jwks( - provider: &dyn OidcProvider, + provider: &Config, client: &Client, cache: &Cache, ) -> Result, Error> { - let timeout = Duration::from_secs(provider.base_config().jwks_refresh_timeout_secs); + let timeout = Duration::from_secs(provider.jwks_refresh_timeout_secs); let cache_key = jwks_cache_key(provider); let jwks_url = get_jwks_url(provider, client, cache, Some(timeout)).await?; let fetched = fetch_cached_json::( @@ -352,7 +355,7 @@ async fn refresh_jwks( cache, cache_key: &cache_key, url: &jwks_url, - ttl: provider.base_config().jwks_refresh_interval, + ttl: provider.jwks_refresh_interval, read_cache: false, fetch_timeout: Some(timeout), }, @@ -366,7 +369,7 @@ async fn refresh_jwks( #[cfg(test)] async fn fetch_oidc_configuration( - provider: &dyn OidcProvider, + provider: &Config, client: &Client, cache: &Cache, ) -> Result { @@ -374,23 +377,20 @@ async fn fetch_oidc_configuration( } async fn fetch_oidc_configuration_with_timeout( - provider: &dyn OidcProvider, + provider: &Config, client: &Client, cache: &Cache, fetch_timeout: Option, ) -> Result { let cache_key = oidc_configuration_cache_key(provider); - let config_url = format!( - "{}/.well-known/openid-configuration", - provider.base_config().issuer - ); + let config_url = format!("{}/.well-known/openid-configuration", provider.issuer); let fetched = fetch_cached_json::( CachedJsonRequest { client, cache, cache_key: &cache_key, url: &config_url, - ttl: provider.base_config().jwks_refresh_interval, + ttl: provider.jwks_refresh_interval, read_cache: true, fetch_timeout, }, @@ -405,10 +405,10 @@ async fn fetch_oidc_configuration_with_timeout( } fn validate_oidc_configuration( - provider: &dyn OidcProvider, + provider: &Config, config: &OpenIdConfiguration, ) -> Result<(), Error> { - let expected_issuer = &provider.base_config().issuer; + let expected_issuer = &provider.issuer; if &config.issuer != expected_issuer { return Err(Error::Unauthorized(format!( "OIDC configuration issuer mismatch: expected {expected_issuer}, got {}", diff --git a/src/auth/oidc/validator/tests.rs b/src/auth/oidc/validator/tests.rs index 476d449e..6c993034 100644 --- a/src/auth/oidc/validator/tests.rs +++ b/src/auth/oidc/validator/tests.rs @@ -11,8 +11,7 @@ use wiremock::{ use crate::{ auth::Error, auth::oidc::{ - Jwk, OidcProvider, - provider::{BaseConfig, generic::Provider}, + Config, Jwk, validator::{ Jwks, OpenIdConfiguration, fetch_jwks, fetch_oidc_configuration, jwks_cache_key, oidc_configuration_cache_key, validate_oidc_token, verify_allowed_algorithm, @@ -27,10 +26,12 @@ use crate::{ }, }; -pub fn build_test_provider_config(uri: &str) -> BaseConfig { - BaseConfig { +pub fn build_test_provider_config(uri: &str) -> Config { + Config { + server_ca_bundle: None, issuer: uri.to_string(), jwks_uri: Some(format!("{uri}/.well-known/jwks")), + required_claims: Vec::new(), jwks_refresh_interval: 3600, required_audience: Some("test-audience".to_string()), clock_skew_tolerance: 60, @@ -44,7 +45,7 @@ fn verify_jwt( token: &str, jwks: &Jwks, provider_name: &str, - provider: &dyn OidcProvider, + provider: &Config, ) -> Result { let header = decode_header(token) .map_err(|e| Error::Unauthorized(format!("Failed to decode JWT header: {e}")))?; @@ -68,12 +69,11 @@ async fn test_fetch_jwks_with_explicit_uri() { mount_jwks(&mock_server, jwks_response).await; - let config = BaseConfig { + let provider = Config { required_audience: None, ..build_test_provider_config(&mock_server.uri()) }; - let provider = Provider::new(config); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -112,13 +112,12 @@ async fn test_fetch_jwks_with_discovery() { mount_jwks(&mock_server, jwks_response).await; - let config = BaseConfig { + let provider = Config { jwks_uri: None, required_audience: None, ..build_test_provider_config(&mock_server.uri()) }; - let provider = Provider::new(config); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -151,12 +150,11 @@ async fn test_fetch_jwks_uses_cache() { .mount(&mock_server) .await; - let config = BaseConfig { + let provider = Config { required_audience: None, ..build_test_provider_config(&mock_server.uri()) }; - let provider = Provider::new(config); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -178,12 +176,11 @@ async fn test_fetch_jwks_http_error() { .mount(&mock_server) .await; - let config = BaseConfig { + let provider = Config { required_audience: None, ..build_test_provider_config(&mock_server.uri()) }; - let provider = Provider::new(config); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -212,13 +209,12 @@ async fn test_fetch_oidc_configuration_success() { .mount(&mock_server) .await; - let config = BaseConfig { + let provider = Config { jwks_uri: None, required_audience: None, ..build_test_provider_config(&mock_server.uri()) }; - let provider = Provider::new(config); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -249,13 +245,12 @@ async fn test_fetch_oidc_configuration_uses_cache() { .mount(&mock_server) .await; - let config = BaseConfig { + let provider = Config { jwks_uri: None, required_audience: None, ..build_test_provider_config(&mock_server.uri()) }; - let provider = Provider::new(config); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -281,13 +276,12 @@ async fn test_fetch_oidc_configuration_issuer_mismatch() { .mount(&mock_server) .await; - let config = BaseConfig { + let provider = Config { jwks_uri: None, required_audience: None, ..build_test_provider_config(&mock_server.uri()) }; - let provider = Provider::new(config); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -318,13 +312,12 @@ async fn test_fetch_oidc_configuration_http_error() { .mount(&mock_server) .await; - let config = BaseConfig { + let provider = Config { jwks_uri: None, required_audience: None, ..build_test_provider_config(&mock_server.uri()) }; - let provider = Provider::new(config); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -343,12 +336,11 @@ async fn test_fetch_jwks_network_error_returns_provider_unavailable() { let url = format!("http://{}", listener.local_addr().unwrap()); drop(listener); - let config = BaseConfig { + let provider = Config { required_audience: None, ..build_test_provider_config(&url) }; - let provider = Provider::new(config); let client = Client::builder() .timeout(Duration::from_millis(200)) .build() @@ -375,7 +367,7 @@ async fn test_validate_oidc_token_success() { let token = make_token(&claims, KID); - let provider = Provider::new(build_test_provider_config(&mock_server.uri())); + let provider = build_test_provider_config(&mock_server.uri()); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -385,7 +377,6 @@ async fn test_validate_oidc_token_success() { assert!(result.is_ok()); let oidc_claims = result.unwrap(); assert_eq!(oidc_claims.provider_name, "test-provider"); - assert_eq!(oidc_claims.provider_type, "Generic OIDC"); assert_eq!(oidc_claims.claims.get("sub").unwrap(), "test-user"); } @@ -400,7 +391,7 @@ async fn test_validate_oidc_token_refreshes_jwks_once_when_cached_kid_is_missing .mount(&mock_server) .await; - let provider = Provider::new(build_test_provider_config(&mock_server.uri())); + let provider = build_test_provider_config(&mock_server.uri()); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); let stale_jwks = Jwks { @@ -450,7 +441,7 @@ async fn unknown_kids_cost_one_jwks_fetch_per_cooldown_not_one_per_request() { .mount(&mock_server) .await; - let provider = Provider::new(build_test_provider_config(&mock_server.uri())); + let provider = build_test_provider_config(&mock_server.uri()); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); let stale_jwks = Jwks { @@ -501,7 +492,7 @@ async fn test_validate_oidc_token_returns_unauthorized_when_refreshed_jwks_still .mount(&mock_server) .await; - let provider = Provider::new(build_test_provider_config(&mock_server.uri())); + let provider = build_test_provider_config(&mock_server.uri()); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); let stale_jwks = Jwks { keys: Vec::new() }; @@ -538,7 +529,7 @@ async fn test_validate_oidc_token_invalid_signature() { EncodingKey::from_ec_pem(alt_private_key_pem().as_bytes()).expect("alt key must parse"); let token = encode(&header, &claims, &alt_key).unwrap(); - let provider = Provider::new(build_test_provider_config(&mock_server.uri())); + let provider = build_test_provider_config(&mock_server.uri()); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -560,12 +551,11 @@ async fn test_validate_oidc_token_rejects_disallowed_algorithm_before_jwks_fetch let claims = valid_claims(&mock_server.uri(), "test-audience"); let token = make_token(&claims, KID); - let config = BaseConfig { + let provider = Config { allowed_algorithms: vec![Algorithm::RS256], ..build_test_provider_config(&mock_server.uri()) }; - let provider = Provider::new(config); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -596,7 +586,7 @@ async fn test_validate_oidc_token_expired() { let token = make_token(&claims, KID); - let provider = Provider::new(build_test_provider_config(&mock_server.uri())); + let provider = build_test_provider_config(&mock_server.uri()); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -622,7 +612,7 @@ async fn test_validate_oidc_token_wrong_issuer() { let token = make_token(&claims, KID); - let provider = Provider::new(build_test_provider_config(&mock_server.uri())); + let provider = build_test_provider_config(&mock_server.uri()); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -642,7 +632,7 @@ async fn test_validate_oidc_token_wrong_audience() { let token = make_token(&claims, KID); - let provider = Provider::new(build_test_provider_config(&mock_server.uri())); + let provider = build_test_provider_config(&mock_server.uri()); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -664,7 +654,7 @@ async fn test_validate_oidc_token_missing_kid() { let header = Header::new(Algorithm::ES256); let token = encode(&header, &claims, &encoding_key()).unwrap(); - let provider = Provider::new(build_test_provider_config(&mock_server.uri())); + let provider = build_test_provider_config(&mock_server.uri()); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -691,12 +681,11 @@ async fn test_validate_oidc_token_no_audience_validation() { let token = make_token(&claims, KID); - let config = BaseConfig { + let provider = Config { required_audience: None, ..build_test_provider_config(&mock_server.uri()) }; - let provider = Provider::new(config); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -713,12 +702,11 @@ async fn test_validate_oidc_token_invalid_jwt_format() { let jwks_response = json!({ "keys": [] }); mount_jwks(&mock_server, jwks_response).await; - let config = BaseConfig { + let provider = Config { required_audience: None, ..build_test_provider_config(&mock_server.uri()) }; - let provider = Provider::new(config); let client = Client::new(); let cache = cache::Config::Memory.to_backend().unwrap(); @@ -775,51 +763,20 @@ pub fn valid_claims(issuer: &str, audience: &str) -> HashMap, -} - -impl TestProvider { - fn new(issuer: &str, audience: Option<&str>) -> Self { - Self { - base: BaseConfig { - issuer: issuer.to_string(), - jwks_uri: None, - jwks_refresh_interval: 3600, - required_audience: audience.map(str::to_string), - clock_skew_tolerance: 0, - allowed_algorithms: vec![Algorithm::ES256], - http_request_timeout_secs: 30, - jwks_refresh_timeout_secs: 5, - }, - claim_error: None, - } - } - - fn with_claim_error(mut self, msg: &str) -> Self { - self.claim_error = Some(msg.to_string()); - self - } -} - -impl OidcProvider for TestProvider { - fn base_config(&self) -> &BaseConfig { - &self.base - } - - fn name(&self) -> &'static str { - "Test" - } - - fn validate_provider_claims( - &self, - _claims: &HashMap, - ) -> Result<(), Error> { - match &self.claim_error { - Some(msg) => Err(Error::Unauthorized(msg.clone())), - None => Ok(()), - } +/// A provider with no JWKS URI and no clock skew, for the tests that verify a +/// token against a JWKS they hold rather than one they fetch. +fn test_provider(issuer: &str, audience: Option<&str>) -> Config { + Config { + server_ca_bundle: None, + issuer: issuer.to_string(), + jwks_uri: None, + required_claims: Vec::new(), + jwks_refresh_interval: 3600, + required_audience: audience.map(str::to_string), + clock_skew_tolerance: 0, + allowed_algorithms: vec![Algorithm::ES256], + http_request_timeout_secs: 30, + jwks_refresh_timeout_secs: 5, } } @@ -827,7 +784,7 @@ impl OidcProvider for TestProvider { fn verify_jwt_accepts_valid_token() { let issuer = "https://issuer.example.com"; let audience = "my-audience"; - let provider = TestProvider::new(issuer, Some(audience)); + let provider = test_provider(issuer, Some(audience)); let jwks = test_jwks(); let claims = valid_claims(issuer, audience); let token = make_token(&claims, KID); @@ -837,7 +794,6 @@ fn verify_jwt_accepts_valid_token() { assert!(result.is_ok(), "expected Ok, got {result:?}"); let oidc = result.unwrap(); assert_eq!(oidc.provider_name, "test-provider"); - assert_eq!(oidc.provider_type, "Test"); assert_eq!( oidc.claims.get("sub").and_then(|v| v.as_str()), Some("unit-test-subject") @@ -847,7 +803,7 @@ fn verify_jwt_accepts_valid_token() { #[test] fn verify_jwt_rejects_unknown_kid() { let issuer = "https://issuer.example.com"; - let provider = TestProvider::new(issuer, None); + let provider = test_provider(issuer, None); let jwks = test_jwks(); let claims = valid_claims(issuer, "any"); let token = make_token(&claims, "unknown-kid-that-is-not-in-jwks"); @@ -864,7 +820,7 @@ fn verify_jwt_rejects_unknown_kid() { fn verify_jwt_rejects_expired_token() { let issuer = "https://issuer.example.com"; // clock_skew = 0 so even a 1-second-old exp is rejected - let provider = TestProvider::new(issuer, None); + let provider = test_provider(issuer, None); let jwks = test_jwks(); let mut claims = HashMap::new(); @@ -890,7 +846,7 @@ fn verify_jwt_rejects_expired_token() { #[test] fn verify_jwt_rejects_wrong_issuer() { - let provider = TestProvider::new("https://expected-issuer.example.com", None); + let provider = test_provider("https://expected-issuer.example.com", None); let jwks = test_jwks(); let claims = valid_claims("https://wrong-issuer.example.com", "any"); let token = make_token(&claims, KID); @@ -903,7 +859,7 @@ fn verify_jwt_rejects_wrong_issuer() { #[test] fn verify_jwt_rejects_wrong_audience() { let issuer = "https://issuer.example.com"; - let provider = TestProvider::new(issuer, Some("required-audience")); + let provider = test_provider(issuer, Some("required-audience")); let jwks = test_jwks(); let claims = valid_claims(issuer, "wrong-audience"); let token = make_token(&claims, KID); @@ -917,7 +873,7 @@ fn verify_jwt_rejects_wrong_audience() { fn verify_jwt_skips_audience_when_provider_has_none() { let issuer = "https://issuer.example.com"; // required_audience = None → validate_aud is disabled - let provider = TestProvider::new(issuer, None); + let provider = test_provider(issuer, None); let jwks = test_jwks(); // Token has an audience claim, but the provider doesn't require a specific one let claims = valid_claims(issuer, "any-audience-value"); @@ -934,7 +890,7 @@ fn verify_jwt_skips_audience_when_provider_has_none() { #[test] fn verify_jwt_rejects_invalid_signature() { let issuer = "https://issuer.example.com"; - let provider = TestProvider::new(issuer, None); + let provider = test_provider(issuer, None); let jwks = test_jwks(); // contains public key for private_key_pem() // Sign with the alt key: kid matches, but signature won't verify against JWKS. @@ -959,7 +915,7 @@ fn verify_jwt_rejects_invalid_signature() { #[test] fn verify_jwt_rejects_malformed_header() { - let provider = TestProvider::new("https://issuer.example.com", None); + let provider = test_provider("https://issuer.example.com", None); let jwks = test_jwks(); let result = verify_jwt("not-a-valid-jwt", &jwks, "test-provider", &provider); @@ -973,18 +929,64 @@ fn verify_jwt_rejects_malformed_header() { } } +/// `required_claims` is what replaced the GitHub provider's hard-coded +/// repository/actor check, so the rejection it used to perform is pinned here. #[test] -fn verify_jwt_propagates_provider_claim_validation_error() { +fn verify_jwt_rejects_token_missing_a_required_claim() { let issuer = "https://issuer.example.com"; - let provider = TestProvider::new(issuer, None).with_claim_error("custom claim check failed"); + let provider = Config { + required_claims: vec!["repository".to_string(), "actor".to_string()], + ..test_provider(issuer, None) + }; let jwks = test_jwks(); - let claims = valid_claims(issuer, "any"); + let mut claims = valid_claims(issuer, "any"); + claims.insert("repository".to_string(), json!("myorg/myapp")); + let token = make_token(&claims, KID); + + let result = verify_jwt(&token, &jwks, "test-provider", &provider); + + match result.unwrap_err() { + Error::Unauthorized(msg) => assert_eq!(msg, "token is missing required claim 'actor'"), + e => panic!("expected Unauthorized, got {e:?}"), + } +} + +#[test] +fn verify_jwt_accepts_token_carrying_every_required_claim() { + let issuer = "https://issuer.example.com"; + let provider = Config { + required_claims: vec!["repository".to_string(), "actor".to_string()], + ..test_provider(issuer, None) + }; + let jwks = test_jwks(); + let mut claims = valid_claims(issuer, "any"); + claims.insert("repository".to_string(), json!("myorg/myapp")); + claims.insert("actor".to_string(), json!("octocat")); + let token = make_token(&claims, KID); + + let result = verify_jwt(&token, &jwks, "test-provider", &provider); + + assert!(result.is_ok(), "expected Ok, got {result:?}"); +} + +/// A null claim carries no more identity than an absent one, so it must not +/// satisfy the requirement just by having the key present. +#[test] +fn verify_jwt_rejects_a_required_claim_present_but_null() { + let issuer = "https://issuer.example.com"; + let provider = Config { + required_claims: vec!["repository".to_string()], + ..test_provider(issuer, None) + }; + let jwks = test_jwks(); + let mut claims = valid_claims(issuer, "any"); + claims.insert("repository".to_string(), json!(null)); let token = make_token(&claims, KID); let result = verify_jwt(&token, &jwks, "test-provider", &provider); match result.unwrap_err() { - Error::Unauthorized(msg) => assert_eq!(msg, "custom claim check failed"), + Error::Unauthorized(msg) => assert_eq!(msg, "token is missing required claim 'repository'"), e => panic!("expected Unauthorized, got {e:?}"), } } @@ -995,7 +997,7 @@ fn verify_jwt_propagates_provider_claim_validation_error() { fn verify_jwt_rejects_future_nbf() { let issuer = "https://issuer.example.com"; // clock_skew = 0 so a future nbf is not tolerated - let provider = TestProvider::new(issuer, None); + let provider = test_provider(issuer, None); let jwks = test_jwks(); let mut claims = HashMap::new(); @@ -1024,7 +1026,7 @@ fn verify_jwt_rejects_future_nbf() { fn verify_jwt_selects_correct_key_from_multi_key_jwks() { let issuer = "https://issuer.example.com"; let audience = "my-audience"; - let provider = TestProvider::new(issuer, Some(audience)); + let provider = test_provider(issuer, Some(audience)); // Add a second EC key with a different kid as a decoy. // The x/y values below are from the JWK.rs test; they form a valid @@ -1074,7 +1076,7 @@ fn verify_jwt_selects_correct_key_from_multi_key_jwks() { #[test] fn verify_jwt_preserves_custom_claims() { let issuer = "https://token.actions.githubusercontent.com"; - let provider = TestProvider::new(issuer, None); + let provider = test_provider(issuer, None); let jwks = test_jwks(); let mut claims = valid_claims(issuer, "any"); diff --git a/src/auth/token_service.rs b/src/auth/token_service.rs new file mode 100644 index 00000000..42a1456c --- /dev/null +++ b/src/auth/token_service.rs @@ -0,0 +1,595 @@ +//! Registry-issued bearer tokens, the OCI distribution token service. +//! +//! Clients whose credential is short-lived (a GitHub Actions OIDC token lives +//! about ten minutes) exchange it once at `/token` for a registry-signed token +//! that outlives a long push. The token freezes the identity; authorization is +//! still evaluated per request against live configuration. +//! +//! Issuing and validating are separate types built from one configuration: the +//! validator joins the authentication chain, the issuer serves the endpoint. + +use std::{ + collections::HashSet, + time::{SystemTime, UNIX_EPOCH}, +}; + +use async_trait::async_trait; +use hyper::{header::HeaderValue, http::request::Parts}; +use jsonwebtoken::{ + Algorithm, DecodingKey, EncodingKey, Header, Validation, decode, decode_header, encode, +}; +use serde::{Deserialize, Serialize}; +use url::Url; + +use crate::{ + auth::{AuthMiddleware, AuthResult, Error, authorization::bearer_token}, + configuration::Base64String, + identity::{ClientIdentity, OidcClaims}, + secret::Secret, +}; + +/// Shortest HMAC key accepted, in decoded bytes, matching the HS256 hash output +/// RFC 7518 requires. Every issued token is a public value handed to a CI job, +/// so a short key is brute-forceable offline. +const MIN_SECRET_LEN: usize = 32; + +/// Longest token lifetime accepted. An issued token cannot be revoked before it +/// expires, so its lifetime is the window a stolen one stays usable. +const MAX_TTL_SECS: u64 = 86400; + +/// The path the registry serves the token service on. A configured realm must +/// end with it, or clients follow the challenge to a 404. +const TOKEN_PATH: &str = "/token"; + +const ALGORITHM: Algorithm = Algorithm::HS256; + +/// The type a registry token declares in its own JOSE header. RFC 8725 asks for +/// explicit typing so one application's JWTs cannot be taken for another's, +/// which is what tells our bearer apart from a provider's. +const TOKEN_TYPE: &str = "angos+jwt"; + +#[derive(Clone, Debug, Deserialize)] +pub struct Config { + pub secret_key: Secret, + /// Absolute URL clients fetch tokens from. Derived from the request's `Host` + /// when unset, which is what a registry reachable under several hostnames + /// wants. + #[serde(default)] + pub realm: Option, + #[serde(default = "default_ttl_secs")] + pub ttl_secs: u64, +} + +fn default_ttl_secs() -> u64 { + 3600 +} + +impl Config { + /// Base64-ness is the type's business; this is the length HS256 needs, and + /// both halves take the key from here so it has one enforcement point. + fn signing_key(&self) -> Result<&[u8], Error> { + let key = self.secret_key.expose().as_bytes(); + if key.len() < MIN_SECRET_LEN { + return Err(Error::Initialization(format!( + "auth.token_service.secret_key must decode to at least {MIN_SECRET_LEN} bytes" + ))); + } + Ok(key) + } +} + +/// The payload of an issued token. `oidc` stays nested rather than flattened: +/// a provider claim named `exp` would otherwise collide with this one. +#[derive(Debug, Deserialize, Serialize)] +struct TokenClaims { + exp: u64, + #[serde(default, skip_serializing_if = "Option::is_none")] + id: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + username: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + oidc: Option, +} + +pub struct TokenIssuer { + encoding: EncodingKey, + ttl_secs: u64, + configured_challenge: Option, +} + +impl TokenIssuer { + pub fn new(config: &Config) -> Result { + let key = config.signing_key()?; + if !(1..=MAX_TTL_SECS).contains(&config.ttl_secs) { + return Err(Error::Initialization(format!( + "auth.token_service.ttl_secs must be between 1 and {MAX_TTL_SECS}" + ))); + } + + Ok(Self { + encoding: EncodingKey::from_secret(key), + ttl_secs: config.ttl_secs, + configured_challenge: config.realm.as_deref().map(build_challenge).transpose()?, + }) + } + + /// Signs `identity` into a token, returning it with its lifetime in seconds. + /// + /// The certificate and client IP are left out on purpose: both are re-derived + /// from the live request, so a certificate-bound identity never becomes a + /// replayable bearer credential. + pub fn issue(&self, identity: &ClientIdentity) -> Result<(String, u64), Error> { + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_err(|e| Error::Execution(format!("System clock is before the Unix epoch: {e}")))? + .as_secs(); + + let claims = TokenClaims { + exp: now.saturating_add(self.ttl_secs), + id: identity.id.clone(), + username: identity.username.clone(), + oidc: identity.oidc.clone(), + }; + + let mut header = Header::new(ALGORITHM); + header.typ = Some(TOKEN_TYPE.to_string()); + + let token = encode(&header, &claims, &self.encoding) + .map_err(|e| Error::Execution(format!("Failed to sign token: {e}")))?; + Ok((token, self.ttl_secs)) + } + + /// The `WWW-Authenticate` value pointing clients at the token endpoint, using + /// the configured realm when there is one and the request's own host otherwise. + pub fn challenge(&self, scheme: &str, host: &str) -> Option { + if let Some(challenge) = &self.configured_challenge { + return Some(challenge.clone()); + } + build_challenge(&format!("{scheme}://{host}{TOKEN_PATH}")).ok() + } +} + +fn build_challenge(realm: &str) -> Result { + let url = Url::parse(realm).map_err(|e| { + Error::Initialization(format!("auth.token_service.realm is not a URL: {e}")) + })?; + if !matches!(url.scheme(), "http" | "https") { + return Err(Error::Initialization( + "auth.token_service.realm must be an http or https URL".to_string(), + )); + } + // A suffix rather than the whole path, so a registry behind a proxy that + // strips a prefix can advertise the prefixed URL its clients must call. + if !url.path().ends_with(TOKEN_PATH) { + return Err(Error::Initialization(format!( + "auth.token_service.realm must have a path ending in {TOKEN_PATH}" + ))); + } + + // Rebuilt from the parsed parts rather than echoed: a configured realm may + // carry userinfo, a query or a fragment, none of which belong in a header + // every client receives. + let host = url + .host_str() + .ok_or_else(|| Error::Initialization("auth.token_service.realm has no host".to_string()))?; + let service = match url.port() { + Some(port) => format!("{host}:{port}"), + None => host.to_string(), + }; + let realm = format!("{}://{service}{}", url.scheme(), url.path()); + + HeaderValue::from_str(&format!(r#"Bearer realm="{realm}",service="{service}""#)) + .map_err(|e| Error::Initialization(format!("auth.token_service.realm is unusable: {e}"))) +} + +pub struct TokenValidator { + decoding: DecodingKey, + validation: Validation, + oidc_providers: HashSet, +} + +impl TokenValidator { + /// `oidc_providers` are the currently configured provider names. A token + /// naming one that has since been removed or renamed is refused, which is + /// the only way an operator can invalidate tokens before they expire. + pub fn new(config: &Config, oidc_providers: &[String]) -> Result { + let mut validation = Validation::new(ALGORITHM); + validation.validate_aud = false; + + Ok(Self { + decoding: DecodingKey::from_secret(config.signing_key()?), + validation, + oidc_providers: oidc_providers.iter().cloned().collect(), + }) + } +} + +#[async_trait] +impl AuthMiddleware for TokenValidator { + async fn authenticate( + &self, + parts: &Parts, + identity: &mut ClientIdentity, + ) -> Result { + // A bearer typed as anything else belongs to an OIDC provider and must + // reach those middlewares untouched. + let Some(token) = bearer_token(&parts.headers) else { + return Ok(AuthResult::NoCredentials); + }; + if !matches!(decode_header(&token), Ok(header) if header.typ.as_deref() == Some(TOKEN_TYPE)) + { + return Ok(AuthResult::NoCredentials); + } + + let claims = decode::(&token, &self.decoding, &self.validation) + .map_err(|e| Error::Unauthorized(format!("Registry token rejected: {e}")))? + .claims; + + if let Some(oidc) = &claims.oidc + && !self.oidc_providers.contains(&oidc.provider_name) + { + return Err(Error::Unauthorized(format!( + "Registry token names OIDC provider '{}', which is no longer configured", + oidc.provider_name + ))); + } + + identity.id = claims.id; + identity.username = claims.username; + identity.oidc = claims.oidc; + identity.from_registry_token = true; + Ok(AuthResult::Authenticated) + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use serde_json::json; + + use super::*; + use crate::{ + auth::oidc::validator::tests::{encoding_key, make_token}, + identity::{ClientCertificate, OidcClaims}, + test_fixtures::{oidc::KID, requests::parts_with_authorization}, + }; + + const SECRET: [u8; 32] = [7; 32]; + const PROVIDER: &str = "github-actions"; + + fn config() -> Config { + Config { + secret_key: Secret::new(SECRET.to_vec().into()), + realm: None, + ttl_secs: default_ttl_secs(), + } + } + + /// The header a forger must reproduce: our declared type, any algorithm. + fn our_header(algorithm: Algorithm) -> Header { + let mut header = Header::new(algorithm); + header.typ = Some(TOKEN_TYPE.to_string()); + header + } + + fn issuer() -> TokenIssuer { + TokenIssuer::new(&config()).expect("valid issuer config") + } + + fn validator() -> TokenValidator { + TokenValidator::new(&config(), &[PROVIDER.to_string()]).expect("valid validator config") + } + + /// Every JSON shape a provider can put in a claim, so the round trip is + /// pinned on type fidelity and not just on the keys surviving. + fn oidc_claims() -> OidcClaims { + let mut claims = HashMap::new(); + claims.insert("repository".to_string(), json!("myorg/myapp")); + claims.insert("run_number".to_string(), json!(4_812_u64)); + claims.insert("offset".to_string(), json!(-17_i64)); + claims.insert("large".to_string(), json!(i64::MAX as u64 - 1)); + claims.insert("ratio".to_string(), json!(0.25_f64)); + claims.insert("email_verified".to_string(), json!(true)); + claims.insert("missing".to_string(), json!(null)); + claims.insert("groups".to_string(), json!(["admins", "devs"])); + claims.insert("nested".to_string(), json!({"a": {"b": [1, 2]}})); + + OidcClaims { + provider_name: PROVIDER.to_string(), + claims, + } + } + + fn oidc_identity() -> ClientIdentity { + ClientIdentity { + id: Some("ci".to_string()), + username: Some("ci-bot".to_string()), + oidc: Some(oidc_claims()), + ..Default::default() + } + } + + async fn reissue(identity: &ClientIdentity) -> ClientIdentity { + let (token, _) = issuer().issue(identity).expect("issuing must succeed"); + let parts = parts_with_authorization(&format!("Bearer {token}")); + let mut restored = ClientIdentity::default(); + let result = validator() + .authenticate(&parts, &mut restored) + .await + .expect("a freshly issued token must validate"); + assert!(matches!(result, AuthResult::Authenticated)); + restored + } + + /// The CEL policy context is built by serializing `ClientIdentity`, so this + /// equality is the policy contract: a reissued identity must decide every + /// rule exactly as the original did. + #[tokio::test] + async fn identity_survives_a_token_round_trip_unchanged() { + let original = oidc_identity(); + let restored = reissue(&original).await; + + assert_eq!( + serde_json::to_value(&restored).unwrap(), + serde_json::to_value(&original).unwrap() + ); + } + + #[tokio::test] + async fn certificate_and_client_ip_never_ride_in_the_token() { + let mut issued_from = oidc_identity(); + issued_from.certificate = ClientCertificate { + organizations: vec!["IssuerOrg".to_string()], + common_names: vec!["issuer-cn".to_string()], + }; + issued_from.client_ip = Some("10.0.0.1".to_string()); + + let (token, _) = issuer().issue(&issued_from).expect("issuing must succeed"); + let parts = parts_with_authorization(&format!("Bearer {token}")); + + // What the live request established must survive validation untouched. + let mut identity = ClientIdentity { + certificate: ClientCertificate { + organizations: vec!["LiveOrg".to_string()], + common_names: vec!["live-cn".to_string()], + }, + client_ip: Some("192.0.2.7".to_string()), + ..Default::default() + }; + validator() + .authenticate(&parts, &mut identity) + .await + .unwrap(); + + assert_eq!(identity.certificate.organizations, vec!["LiveOrg"]); + assert_eq!(identity.certificate.common_names, vec!["live-cn"]); + assert_eq!(identity.client_ip.as_deref(), Some("192.0.2.7")); + } + + #[tokio::test] + async fn anonymous_identity_reissues_as_anonymous() { + let restored = reissue(&ClientIdentity::default()).await; + + assert!(restored.id.is_none()); + assert!(restored.username.is_none()); + assert!(restored.oidc.is_none()); + } + + #[tokio::test] + async fn rejects_an_expired_token() { + // Validation::new leaves a 60s leeway, so the token must be older than that. + let claims = TokenClaims { + exp: 1_000_000, + id: None, + username: Some("ci-bot".to_string()), + oidc: None, + }; + let token = encode(&our_header(ALGORITHM), &claims, &issuer().encoding).unwrap(); + let parts = parts_with_authorization(&format!("Bearer {token}")); + + let error = validator() + .authenticate(&parts, &mut ClientIdentity::default()) + .await + .expect_err("an expired token must be refused"); + assert!(matches!(error, Error::Unauthorized(_))); + } + + #[tokio::test] + async fn rejects_a_token_signed_with_another_secret() { + let other = Config { + secret_key: Secret::new(vec![9; 32].into()), + ..config() + }; + let (token, _) = TokenIssuer::new(&other) + .unwrap() + .issue(&oidc_identity()) + .unwrap(); + let parts = parts_with_authorization(&format!("Bearer {token}")); + + let error = validator() + .authenticate(&parts, &mut ClientIdentity::default()) + .await + .expect_err("a token from another key must be refused"); + assert!(matches!(error, Error::Unauthorized(_))); + } + + #[tokio::test] + async fn rejects_a_token_naming_a_removed_provider() { + let (token, _) = issuer().issue(&oidc_identity()).unwrap(); + let parts = parts_with_authorization(&format!("Bearer {token}")); + + let validator = TokenValidator::new(&config(), &[]).unwrap(); + let error = validator + .authenticate(&parts, &mut ClientIdentity::default()) + .await + .expect_err("a token naming an unconfigured provider must be refused"); + assert!(matches!(error, Error::Unauthorized(_))); + } + + /// A provider's own bearer must fall through rather than error, or the OIDC + /// middlewares never get to see it. + #[tokio::test] + async fn a_bearer_typed_as_another_scheme_yields_no_credentials() { + let mut claims = HashMap::new(); + claims.insert("sub".to_string(), json!("someone")); + let token = make_token(&claims, KID); + let parts = parts_with_authorization(&format!("Bearer {token}")); + + let result = validator() + .authenticate(&parts, &mut ClientIdentity::default()) + .await + .expect("another scheme's bearer must not fail the request"); + assert!(matches!(result, AuthResult::NoCredentials)); + } + + /// The type marks a token as ours, so refusing a forgery that claims the type + /// rests on the algorithm `Validation` pins at verification. + #[tokio::test] + async fn a_token_typed_as_ours_but_signed_otherwise_is_refused() { + let mut claims = HashMap::new(); + claims.insert("exp".to_string(), json!(9_999_999_999_u64)); + let token = encode(&our_header(Algorithm::ES256), &claims, &encoding_key()).unwrap(); + let parts = parts_with_authorization(&format!("Bearer {token}")); + + let error = validator() + .authenticate(&parts, &mut ClientIdentity::default()) + .await + .expect_err("only the token service's own algorithm may verify"); + assert!(matches!(error, Error::Unauthorized(_)), "got: {error:?}"); + } + + /// `alg=none` has no `Algorithm` to decode into, so such a token is dropped + /// before its type is read and can never authenticate. + #[tokio::test] + async fn an_unsigned_token_never_authenticates() { + let token = concat!( + "eyJ0eXAiOiJhbmdvcy10b2tlbitqd3QiLCJhbGciOiJub25lIn0", + ".eyJleHAiOjk5OTk5OTk5OTl9." + ); + let parts = parts_with_authorization(&format!("Bearer {token}")); + + let result = validator() + .authenticate(&parts, &mut ClientIdentity::default()) + .await; + assert!( + !matches!(result, Ok(AuthResult::Authenticated)), + "got: {result:?}" + ); + } + + /// Both halves take the key from the same place, so neither can be built + /// with a weaker one than the other. + #[test] + fn neither_half_accepts_a_key_under_32_bytes() { + let config = Config { + secret_key: Secret::new(vec![7; 31].into()), + ..config() + }; + + assert!(TokenIssuer::new(&config).is_err()); + assert!(TokenValidator::new(&config, &[]).is_err()); + } + + #[test] + fn rejects_a_ttl_outside_the_allowed_range() { + for ttl_secs in [0, MAX_TTL_SECS + 1] { + let config = Config { + ttl_secs, + ..config() + }; + assert!(TokenIssuer::new(&config).is_err()); + } + } + + #[test] + fn rejects_a_realm_that_is_not_an_absolute_token_url() { + for realm in [ + "/token", + "registry.example.com/token", + "ftp://registry.example.com/token", + "https://registry.example.com/tokens", + "https://registry.example.com/token/exchange", + ] { + let config = Config { + realm: Some(realm.to_string()), + ..config() + }; + assert!( + TokenIssuer::new(&config).is_err(), + "{realm} must be refused" + ); + } + } + + #[test] + fn the_configured_realm_wins_over_the_request_host() { + let config = Config { + realm: Some("https://registry.example.com/token".to_string()), + ..config() + }; + let issuer = TokenIssuer::new(&config).unwrap(); + + assert_eq!( + issuer.challenge("http", "other.example.com").unwrap(), + r#"Bearer realm="https://registry.example.com/token",service="registry.example.com""# + ); + } + + #[test] + fn derives_the_challenge_from_the_request_host_when_no_realm_is_set() { + let issuer = issuer(); + + assert_eq!( + issuer.challenge("https", "registry.example.com").unwrap(), + r#"Bearer realm="https://registry.example.com/token",service="registry.example.com""# + ); + assert!(issuer.challenge("https", "not a host").is_none()); + } + + /// A proxy that strips a path prefix before forwarding needs the prefixed + /// URL advertised, or clients follow the challenge to a 404. + #[test] + fn a_prefixed_realm_is_advertised_as_configured() { + let config = Config { + realm: Some("https://registry.example.com/registry/token".to_string()), + ..config() + }; + let issuer = TokenIssuer::new(&config).unwrap(); + + assert_eq!( + issuer.challenge("https", "other.example.com").unwrap(), + r#"Bearer realm="https://registry.example.com/registry/token",service="registry.example.com""# + ); + } + + /// The challenge reaches every anonymous client, so anything the realm URL + /// carries beyond scheme, authority and path must be dropped. + #[test] + fn the_challenge_drops_realm_userinfo_and_query() { + let config = Config { + realm: Some("https://bot:hunter2@registry.example.com/token?x=1#f".to_string()), + ..config() + }; + let issuer = TokenIssuer::new(&config).unwrap(); + + assert_eq!( + issuer.challenge("https", "other.example.com").unwrap(), + r#"Bearer realm="https://registry.example.com/token",service="registry.example.com""# + ); + } + + /// A client sends `service` back verbatim, so dropping the port would name a + /// service the registry is not reachable at. + #[test] + fn the_service_carries_the_realm_port() { + let issuer = issuer(); + + assert_eq!( + issuer + .challenge("https", "registry.example.com:8443") + .unwrap(), + r#"Bearer realm="https://registry.example.com:8443/token",service="registry.example.com:8443""# + ); + } +} diff --git a/src/auth/webhook/headers.rs b/src/auth/webhook/headers.rs index 4eae4617..5e906efc 100644 --- a/src/auth/webhook/headers.rs +++ b/src/auth/webhook/headers.rs @@ -4,6 +4,7 @@ use hyper::{ header::{HeaderName, HeaderValue}, http::{HeaderMap, request::Parts}, }; +use serde_json::Value; use crate::{ auth::Error, @@ -24,6 +25,8 @@ static X_REGISTRY_USERNAME: &str = "X-Registry-Username"; static X_REGISTRY_IDENTITY_ID: &str = "X-Registry-Identity-ID"; static X_REGISTRY_CERTIFICATE_CN: &str = "X-Registry-Certificate-CN"; static X_REGISTRY_CERTIFICATE_O: &str = "X-Registry-Certificate-O"; +static X_REGISTRY_OIDC_PROVIDER: &str = "X-Registry-OIDC-Provider"; +static X_REGISTRY_OIDC_SUBJECT: &str = "X-Registry-OIDC-Subject"; pub fn build_header_name(name: &str) -> Result { match HeaderName::from_str(name) { @@ -100,6 +103,18 @@ pub fn build_headers( for org in &identity.certificate.organizations { headers.append(X_REGISTRY_CERTIFICATE_O, build_header_value(org)?); } + // Without these an OIDC caller reaches the webhook anonymous, and since the + // cache key digests exactly these headers, every OIDC user would share one + // decision. + if let Some(oidc) = &identity.oidc { + headers.insert( + X_REGISTRY_OIDC_PROVIDER, + build_header_value(&oidc.provider_name)?, + ); + if let Some(subject) = oidc.claims.get("sub").and_then(Value::as_str) { + headers.insert(X_REGISTRY_OIDC_SUBJECT, build_header_value(subject)?); + } + } // Operator-selected client headers, forwarded verbatim. A repeated header // carries every one of its values, which the cache key then covers. @@ -137,16 +152,17 @@ pub fn build_cache_key(name: &str, headers: &HeaderMap) -> String { #[cfg(test)] mod tests { - use std::str::FromStr; + use std::{collections::HashMap, str::FromStr}; use hyper::{ Request, http::{HeaderMap, HeaderName, HeaderValue, request::Parts}, }; + use serde_json::json; use super::{build_cache_key, build_headers}; use crate::{ - identity::{Action, ClientIdentity}, + identity::{Action, ClientIdentity, OidcClaims}, oci::{Namespace, Reference, Tag}, }; @@ -160,6 +176,15 @@ mod tests { id } + fn identity_with_oidc(provider_name: &str, subject: &str) -> ClientIdentity { + let mut id = ClientIdentity::new(None); + id.oidc = Some(OidcClaims { + provider_name: provider_name.to_string(), + claims: HashMap::from([("sub".to_string(), json!(subject))]), + }); + id + } + /// A bare GET `Parts` carrying the given request headers. fn parts_with_headers(headers: &[(&str, &str)]) -> Parts { let mut builder = Request::builder(); @@ -370,6 +395,61 @@ mod tests { ); } + #[test] + fn an_oidc_identity_reaches_the_webhook() { + let headers = build_headers( + &[], + &Action::ApiVersion, + &identity_with_oidc("github-actions", "repo:myorg/myapp:ref:refs/heads/main"), + &parts_with_headers(&[]), + ) + .unwrap(); + + assert_eq!( + headers.get("X-Registry-OIDC-Provider").unwrap(), + "github-actions" + ); + assert_eq!( + headers.get("X-Registry-OIDC-Subject").unwrap(), + "repo:myorg/myapp:ref:refs/heads/main" + ); + } + + #[test] + fn two_oidc_subjects_do_not_share_one_cached_decision() { + let action = Action::ApiVersion; + assert_ne!( + simple_key("wh", &action, &identity_with_oidc("gh", "alice")), + simple_key("wh", &action, &identity_with_oidc("gh", "bob")) + ); + assert_ne!( + simple_key("wh", &action, &identity_with_oidc("gh", "alice")), + simple_key("wh", &action, &identity_with_oidc("okta", "alice")), + "one subject name must not span two providers" + ); + } + + /// A provider is free to omit `sub`, which must leave the caller identified + /// by provider alone rather than failing the request. + #[test] + fn an_oidc_identity_without_a_subject_still_names_its_provider() { + let mut identity = identity_with_oidc("gh", "alice"); + if let Some(oidc) = identity.oidc.as_mut() { + oidc.claims.remove("sub"); + } + + let headers = build_headers( + &[], + &Action::ApiVersion, + &identity, + &parts_with_headers(&[]), + ) + .unwrap(); + + assert_eq!(headers.get("X-Registry-OIDC-Provider").unwrap(), "gh"); + assert!(headers.get("X-Registry-OIDC-Subject").is_none()); + } + #[test] fn key_is_independent_of_header_insertion_order() { let build = |pairs: &[(&str, &str)]| { diff --git a/src/command/server/handlers/blob.rs b/src/command/server/handlers/blob.rs index f31e2020..b27095a6 100644 --- a/src/command/server/handlers/blob.rs +++ b/src/command/server/handlers/blob.rs @@ -60,7 +60,10 @@ pub async fn handle_head_blob( digest: &Digest, ) -> Result, Error> { let mime_types = RequestHeaders::new(&parts.headers).accepted_content_types(); - let repository = context.registry.get_repository_for_namespace(namespace)?; + let repository = context + .registry + .get_repository_for_namespace(namespace) + .ok(); let response = context .registry .head_blob(repository, &mime_types, namespace, digest) diff --git a/src/command/server/handlers/manifest.rs b/src/command/server/handlers/manifest.rs index c344bbd0..bd3821db 100644 --- a/src/command/server/handlers/manifest.rs +++ b/src/command/server/handlers/manifest.rs @@ -74,7 +74,10 @@ pub async fn handle_head_manifest( ) -> Result, Error> { let mime_types = RequestHeaders::new(&parts.headers).accepted_content_types(); let is_tag_immutable = context.is_reference_immutable(namespace, &reference); - let repository = context.registry.get_repository_for_namespace(namespace)?; + let repository = context + .registry + .get_repository_for_namespace(namespace) + .ok(); let response = context .registry .head_manifest( @@ -443,7 +446,7 @@ mod tests { let kept_digest = context .registry .get_manifest( - repo, + Some(repo), std::slice::from_ref(&media_range()), &namespace, tag(), @@ -495,7 +498,7 @@ mod tests { let after = context .registry .get_manifest( - repo, + Some(repo), std::slice::from_ref(&media_range()), &namespace, tag(), diff --git a/src/command/server/handlers/mod.rs b/src/command/server/handlers/mod.rs index 1a2a9838..ffac8bfc 100644 --- a/src/command/server/handlers/mod.rs +++ b/src/command/server/handlers/mod.rs @@ -17,6 +17,7 @@ pub mod blob; pub mod content_discovery; pub mod ext; pub mod manifest; +pub mod token; pub mod upload; pub mod version; diff --git a/src/command/server/handlers/token.rs b/src/command/server/handlers/token.rs new file mode 100644 index 00000000..01a24837 --- /dev/null +++ b/src/command/server/handlers/token.rs @@ -0,0 +1,50 @@ +use hyper::{ + Response, StatusCode, + header::{CACHE_CONTROL, HeaderValue}, +}; +use serde::Serialize; + +use crate::{ + command::server::{ + ServerContext, error::Error, handlers::json_response, response_body::ResponseBody, + }, + identity::ClientIdentity, +}; + +/// Exchanges the credential that authenticated this request for a registry-issued +/// token. The response field names are the ones OCI clients read. +/// +/// A registry token is not such a credential: renewing one would let a token +/// outlive the credential it was minted from forever, and `ttl_secs` would bound +/// nothing. +pub fn handle_get_token( + context: &ServerContext, + identity: &ClientIdentity, +) -> Result, Error> { + #[derive(Serialize)] + struct TokenResponse { + token: String, + expires_in: u64, + } + + let Some(token_issuer) = context.token_issuer() else { + return Err(Error::NotFound( + "No token service is configured".to_string(), + )); + }; + + if identity.from_registry_token { + return Err(Error::Unauthorized( + "A registry token cannot be exchanged for another".to_string(), + )); + } + + let (token, expires_in) = token_issuer.issue(identity)?; + let mut response = json_response(StatusCode::OK, &TokenResponse { token, expires_in })?; + // The body is a bearer credential, so no shared cache may store it and hand + // it to the next client asking for one. + response + .headers_mut() + .insert(CACHE_CONTROL, HeaderValue::from_static("no-store")); + Ok(response) +} diff --git a/src/command/server/http_server/connection.rs b/src/command/server/http_server/connection.rs index e0bd1358..a8c1380c 100644 --- a/src/command/server/http_server/connection.rs +++ b/src/command/server/http_server/connection.rs @@ -106,12 +106,23 @@ async fn handle_request( let route_action = action.as_ref().map_or("unknown", Action::action_name); let trace_id = current_trace_id(&Span::current()); + // Captured before the request moves into the dispatch future, since the realm + // may be derived from the request's own host. A denial on the token endpoint + // itself is not challenged: the client would be sent to fetch a token from + // the endpoint that just refused it. + let challenge_origin = (!matches!(action, Some(Action::Token))) + .then(|| context.challenge_origin(&request)) + .flatten(); let dispatch: DispatchFuture = Box::pin(dispatch_request(Arc::clone(&context), request, action)); let response = match dispatch.await { Ok(response) => response, - Err(error) => error_to_response(&error, trace_id.as_ref()), + Err(error) => { + let challenge = + challenge_origin.and_then(|(scheme, host)| context.bearer_challenge(scheme, &host)); + error_to_response(&error, trace_id.as_ref(), challenge) + } }; let elapsed = elapsed_ms(start_time); diff --git a/src/command/server/http_server/dispatch.rs b/src/command/server/http_server/dispatch.rs index da719032..e780d2da 100644 --- a/src/command/server/http_server/dispatch.rs +++ b/src/command/server/http_server/dispatch.rs @@ -57,6 +57,7 @@ async fn dispatch_route<'a>( Action::UiAsset { path } if context.enable_ui => ui::serve_asset(&path), Action::UiConfig if context.enable_ui => handle_ui_config(context), Action::UiAsset { .. } | Action::UiConfig => handle_unknown_route(parts), + Action::Token => handlers::token::handle_get_token(context, identity), Action::ApiVersion => Ok(handlers::version::handle_get_api_version()?), Action::StartUpload { namespace, digest } => { handlers::upload::handle_start_upload(context, &namespace, digest).await diff --git a/src/command/server/http_server/error_response.rs b/src/command/server/http_server/error_response.rs index 1bc5c616..7092e8a0 100644 --- a/src/command/server/http_server/error_response.rs +++ b/src/command/server/http_server/error_response.rs @@ -9,7 +9,15 @@ use crate::command::server::{error::Error, response_body::ResponseBody}; const BASIC_AUTH_CHALLENGE: &str = r#"Basic realm="Angos", charset="UTF-8""#; -pub fn error_to_response(error: &Error, request_id: Option<&String>) -> Response { +/// `challenge` is the token service's bearer challenge when one is configured. +/// Only our own denial is challenged: a 401 relayed from a pull-through upstream +/// is that registry's refusal, and answering it with our realm buys the client a +/// token round trip that changes nothing. +pub fn error_to_response( + error: &Error, + request_id: Option<&String>, + challenge: Option, +) -> Response { let body = Bytes::from(error.as_json(request_id).to_string()); let mut response = Response::builder() @@ -22,7 +30,7 @@ pub fn error_to_response(error: &Error, request_id: Option<&String>) -> Response if matches!(error, Error::Unauthorized(_)) { response.headers_mut().insert( WWW_AUTHENTICATE, - HeaderValue::from_static(BASIC_AUTH_CHALLENGE), + challenge.unwrap_or(HeaderValue::from_static(BASIC_AUTH_CHALLENGE)), ); } diff --git a/src/command/server/http_server/tests.rs b/src/command/server/http_server/tests.rs index bc223aec..61f6477d 100644 --- a/src/command/server/http_server/tests.rs +++ b/src/command/server/http_server/tests.rs @@ -6,7 +6,7 @@ use base64::{Engine, prelude::BASE64_STANDARD}; use http_body_util::BodyExt; use hyper::{ Method, Request, StatusCode, - header::{AUTHORIZATION, CONTENT_TYPE, WWW_AUTHENTICATE}, + header::{AUTHORIZATION, CACHE_CONTROL, CONTENT_TYPE, HeaderValue, WWW_AUTHENTICATE}, }; use opentelemetry::trace::TracerProvider; use opentelemetry_sdk::trace::{Sampler, SdkTracerProvider}; @@ -18,7 +18,10 @@ use crate::{ command::server::{ ServerContext, error::Error, - handlers::{content_discovery::handle_list_catalog, ext::handle_list_repositories}, + handlers::{ + content_discovery::handle_list_catalog, ext::handle_list_repositories, + token::handle_get_token, + }, http_server::{ connection::{current_trace_id, inject_peer_certificate}, dispatch::{authenticate_and_authorize, handle_unknown_route}, @@ -43,7 +46,7 @@ fn test_error_to_response_unauthorized_with_request_id() { let error = Error::Unauthorized("Invalid credentials".to_string()); let request_id = Some("req-123".to_string()); - let response = error_to_response(&error, request_id.as_ref()); + let response = error_to_response(&error, request_id.as_ref(), None); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); assert_eq!( @@ -62,7 +65,7 @@ async fn test_error_to_response_from_registry_error() { let error: Error = registry_error.into(); let request_id = Some("req-blob".to_string()); - let response = error_to_response(&error, request_id.as_ref()); + let response = error_to_response(&error, request_id.as_ref(), None); assert_eq!(response.status(), StatusCode::NOT_FOUND); assert_eq!( @@ -88,7 +91,7 @@ fn test_error_to_response_custom_error() { }; let request_id = Some("req-custom".to_string()); - let response = error_to_response(&error, request_id.as_ref()); + let response = error_to_response(&error, request_id.as_ref(), None); assert_eq!(response.status(), StatusCode::BAD_GATEWAY); assert_eq!( @@ -98,6 +101,32 @@ fn test_error_to_response_custom_error() { assert!(response.headers().get(WWW_AUTHENTICATE).is_none()); } +/// A 401 relayed from a pull-through upstream is that registry's refusal, so +/// answering it with our own realm costs the client a token round trip that +/// cannot change the outcome. +#[test] +fn only_our_own_denial_carries_the_bearer_challenge() { + let challenge = HeaderValue::from_static( + r#"Bearer realm="https://registry.example.com/token",service="registry.example.com""#, + ); + let upstream = Error::Custom { + status_code: StatusCode::UNAUTHORIZED, + code: "UNAUTHORIZED".to_string(), + msg: Some("upstream refused".to_string()), + }; + + let relayed = error_to_response(&upstream, None, Some(challenge.clone())); + assert_eq!(relayed.status(), StatusCode::UNAUTHORIZED); + assert!(relayed.headers().get(WWW_AUTHENTICATE).is_none()); + + let ours = error_to_response( + &Error::Unauthorized("no credentials".to_string()), + None, + Some(challenge.clone()), + ); + assert_eq!(ours.headers().get(WWW_AUTHENTICATE), Some(&challenge)); +} + #[test] fn test_handle_healthz_success() { let result = handle_healthz(); @@ -210,7 +239,7 @@ fn test_error_to_response_all_error_types() { ]; for (error, expected_status, should_have_www_authenticate) in errors { - let response = error_to_response(&error, None); + let response = error_to_response(&error, None, None); assert_eq!(response.status(), expected_status); assert_eq!( @@ -233,7 +262,7 @@ async fn test_error_to_response_body_contains_error_message() { let error = Error::BadRequest("Invalid manifest format".to_string()); let request_id = Some("req-manifest".to_string()); - let response = error_to_response(&error, request_id.as_ref()); + let response = error_to_response(&error, request_id.as_ref(), None); let (_, body) = response.into_parts(); let body_bytes = match body { @@ -250,7 +279,7 @@ fn test_error_to_response_with_empty_message() { let error = Error::Internal(String::new()); let request_id = None; - let response = error_to_response(&error, request_id.as_ref()); + let response = error_to_response(&error, request_id.as_ref(), None); assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); assert_eq!( @@ -333,7 +362,7 @@ async fn bad_basic_auth_returns_http_401() { let error = authenticate_and_authorize(&context, &Action::ApiVersion, &parts) .await .unwrap_err(); - let response = error_to_response(&error, None); + let response = error_to_response(&error, None, None); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); assert_eq!( @@ -342,6 +371,78 @@ async fn bad_basic_auth_returns_http_401() { ); } +/// The token endpoint is a route like any other. It grants nothing on its own, +/// but an operator who wants to refuse issuance must be able to say so. +#[tokio::test] +async fn the_token_endpoint_is_gated_by_the_access_policy() { + let config = load_config( + r#" + [global.access_policy] + default = "deny" + rules = [] + + [auth.token_service] + secret_key = "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" + "#, + ); + let context = create_test_server_context_from_config(&config).await; + let request = Request::builder().uri("/token").body(()).unwrap(); + let (parts, ()) = request.into_parts(); + + let error = authenticate_and_authorize(&context, &Action::Token, &parts) + .await + .unwrap_err(); + + assert_eq!( + error_to_response(&error, None, None).status(), + StatusCode::UNAUTHORIZED + ); +} + +async fn token_service_context() -> ServerContext { + let config = load_config( + r#" + [global.access_policy] + default = "allow" + rules = [] + + [auth.token_service] + secret_key = "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" + "#, + ); + create_test_server_context_from_config(&config).await +} + +/// Renewal would let a token outlive the credential it was minted from for as +/// long as the client keeps asking, so `ttl_secs` would bound nothing. +#[tokio::test] +async fn a_registry_token_cannot_be_exchanged_for_another() { + let context = token_service_context().await; + let identity = ClientIdentity { + username: Some("ci-bot".to_string()), + from_registry_token: true, + ..ClientIdentity::default() + }; + + let Err(error) = handle_get_token(&context, &identity) else { + panic!("a registry token must not be renewable"); + }; + + assert!(matches!(error, Error::Unauthorized(_))); +} + +/// The body is a bearer credential, and an anonymous exchange is a plain 200 +/// JSON GET that a shared cache would otherwise be free to store. +#[tokio::test] +async fn an_issued_token_is_never_cached() { + let context = token_service_context().await; + + let response = handle_get_token(&context, &ClientIdentity::default()).unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(response.headers().get(CACHE_CONTROL).unwrap(), "no-store"); +} + async fn create_test_context_with_allow_policy() -> ServerContext { create_test_server_context_with(TestConfigOptions { access_policy: Some(AccessPolicyConfig { diff --git a/src/command/server/router/mod.rs b/src/command/server/router/mod.rs index 1459c7cf..7ae56d12 100644 --- a/src/command/server/router/mod.rs +++ b/src/command/server/router/mod.rs @@ -29,6 +29,9 @@ pub fn parse(method: &Method, uri: &Uri) -> Option { "/readyz" if method == Method::GET => return Some(Action::Readyz), "/metrics" if method == Method::GET => return Some(Action::Metrics), "/_ui/config" if method == Method::GET => return Some(Action::UiConfig), + // Matched for every method: guarded by `if method == GET` a HEAD would + // fall through to the UI-asset arm below and answer with `index.html`. + "/token" => return (method == Method::GET).then_some(Action::Token), // HEAD as well as GET: the version check is the OCI conformance probe, // and without this it falls through to the UI-asset arm below and // answers with `index.html`. diff --git a/src/command/server/router/tests.rs b/src/command/server/router/tests.rs index 6b3ab412..c65a9e38 100644 --- a/src/command/server/router/tests.rs +++ b/src/command/server/router/tests.rs @@ -16,6 +16,16 @@ fn test_parse_metrics() { assert!(matches!(route, Some(Action::Metrics))); } +/// Without its own arm the token endpoint reaches the UI-asset arm and answers +/// `index.html` with a 200. +#[test] +fn test_parse_token() { + let uri: Uri = "/token".parse().unwrap(); + assert!(matches!(parse(&Method::GET, &uri), Some(Action::Token))); + assert!(parse(&Method::POST, &uri).is_none()); + assert!(parse(&Method::HEAD, &uri).is_none()); +} + #[test] fn test_parse_api_version() { let method = Method::GET; diff --git a/src/command/server/server_context/mod.rs b/src/command/server/server_context/mod.rs index bef49abe..e5517c0c 100644 --- a/src/command/server/server_context/mod.rs +++ b/src/command/server/server_context/mod.rs @@ -3,15 +3,19 @@ use std::{ sync::Arc, }; -use hyper::{header::HeaderMap, http::request::Parts}; +use hyper::{ + Request, + header::{HOST, HeaderMap, HeaderValue}, + http::{request::Parts, uri::Authority}, +}; use tracing::instrument; use crate::{ - auth::{Authenticator, Authorizer}, + auth::{Authenticator, Authorizer, TokenIssuer}, cache::Cache, command::server::error::Error, configuration::{Configuration, TrustedProxy}, - identity::{Action, ClientIdentity}, + identity::{Action, ClientIdentity, RequestScheme}, oci::{Namespace, Reference}, registry::{BlobMount, Registry}, }; @@ -19,6 +23,7 @@ use crate::{ pub struct ServerContext { authenticator: Arc, authorizer: Arc, + token_issuer: Option, trusted_proxies: Vec, pub registry: Arc, pub enable_ui: bool, @@ -35,10 +40,17 @@ impl ServerContext { ) -> Result { let authenticator = Arc::new(Authenticator::new(config, cache)?); let authorizer = Arc::new(Authorizer::new(config, cache)?); + let token_issuer = config + .auth + .token_service + .as_ref() + .map(TokenIssuer::new) + .transpose()?; Ok(Self { authenticator, authorizer, + token_issuer, trusted_proxies: config.global.trusted_proxies.clone(), registry, enable_ui: config.ui.enabled, @@ -51,6 +63,58 @@ impl ServerContext { self.registry.has_event_dispatcher() } + pub fn token_issuer(&self) -> Option<&TokenIssuer> { + self.token_issuer.as_ref() + } + + /// The scheme and host a bearer challenge is derived from, or `None` when no + /// token service is configured. + /// + /// Taken before the request is dispatched, since the realm may need the + /// request's own host; the header itself is built on the denial path, so a + /// served request does not pay for a challenge it discards. + pub fn challenge_origin(&self, request: &Request) -> Option<(&'static str, String)> { + self.token_issuer.as_ref()?; + let host = request + .headers() + .get(HOST) + .and_then(|host| host.to_str().ok()) + .or_else(|| request.uri().authority().map(Authority::as_str))?; + + Some((self.request_scheme(request), host.to_string())) + } + + /// The `WWW-Authenticate` challenge pointing clients at the token endpoint, + /// from an origin [`Self::challenge_origin`] captured. + pub fn bearer_challenge(&self, scheme: &str, host: &str) -> Option { + self.token_issuer.as_ref()?.challenge(scheme, host) + } + + fn is_trusted_proxy(&self, peer: IpAddr) -> bool { + self.trusted_proxies.iter().any(|p| p.contains(peer)) + } + + /// The scheme the client used, which behind a TLS-terminating proxy is not + /// the scheme this server was reached on. Only a trusted peer's + /// `X-Forwarded-Proto` is believed. + fn request_scheme(&self, request: &Request) -> &'static str { + let peer = request.extensions().get::(); + if peer.is_some_and(|peer| self.is_trusted_proxy(peer.ip())) + && let Some(proto) = request.headers().get("X-Forwarded-Proto") + && let Ok(proto) = proto.to_str() + && proto.trim().eq_ignore_ascii_case("https") + { + return RequestScheme::Https.as_str(); + } + + request + .extensions() + .get::() + .copied() + .unwrap_or(RequestScheme::Http) + .as_str() + } + #[instrument(skip(self, parts))] pub async fn authenticate_request( &self, @@ -62,7 +126,7 @@ impl ServerContext { .authenticate_request(parts, remote_address) .await?; if let Some(peer) = remote_address - && self.trusted_proxies.iter().any(|p| p.contains(peer.ip())) + && self.is_trusted_proxy(peer.ip()) && let Some(client_ip) = resolve_forwarded_ip(&parts.headers, &self.trusted_proxies) { identity.client_ip = Some(client_ip); diff --git a/src/command/server/server_context/tests.rs b/src/command/server/server_context/tests.rs index abe63290..da7fd2dd 100644 --- a/src/command/server/server_context/tests.rs +++ b/src/command/server/server_context/tests.rs @@ -1,4 +1,6 @@ -use std::{collections::HashMap, path::PathBuf, str::FromStr, sync::Arc, time::Duration}; +use std::{ + collections::HashMap, net::SocketAddr, path::PathBuf, str::FromStr, sync::Arc, time::Duration, +}; use tempfile::TempDir; use argon2::{ @@ -6,7 +8,10 @@ use argon2::{ password_hash::{SaltString, rand_core::OsRng}, }; use base64::Engine; -use hyper::{Request, header::HeaderMap}; +use hyper::{ + Request, + header::{HOST, HeaderMap, HeaderValue}, +}; use uuid::Uuid; use wiremock::{Mock, MockServer, ResponseTemplate, matchers::method}; @@ -18,7 +23,7 @@ use crate::{ command::server::server_context::{ServerContext, resolve_forwarded_ip}, configuration::{Configuration, TrustedProxy}, event_webhook::{config::EventWebhookConfig, dispatcher::EventDispatcher, event::Event}, - identity::{Action, ClientIdentity}, + identity::{Action, ClientIdentity, RequestScheme}, metrics_provider, oci::{Digest, Namespace, Reference, Tag}, policy::AccessPolicyConfig, @@ -812,3 +817,95 @@ async fn dispatch_events_all_success_returns_ok() { let requests = mock_server.received_requests().await.unwrap(); assert_eq!(requests.len(), 2); } + +/// `global` is appended to the minimal config's `[global]` table, so it must come +/// before the token service's own table header. +async fn token_service_context(global: &str, token_service: &str) -> ServerContext { + let config = load_config(&format!( + r#"{global} + [auth.token_service] + secret_key = "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" + {token_service}"# + )); + create_test_server_context_from_config(&config).await +} + +fn challenge_request(scheme: RequestScheme, headers: &[(&str, &str)]) -> Request<()> { + let mut builder = Request::builder() + .uri("/v2/") + .header(HOST, "registry.example.com"); + for (name, value) in headers { + builder = builder.header(*name, *value); + } + + let mut request = builder.body(()).unwrap(); + request.extensions_mut().insert(scheme); + request + .extensions_mut() + .insert("10.0.0.1:9999".parse::().unwrap()); + request +} + +/// The two halves the connection handler calls, one before dispatch and one on +/// the denial path. +fn challenge_for(context: &ServerContext, request: &Request<()>) -> Option { + let (scheme, host) = context.challenge_origin(request)?; + context.bearer_challenge(scheme, &host) +} + +#[tokio::test] +async fn no_bearer_challenge_without_a_token_service() { + let context = create_test_server_context().await; + + assert!(challenge_for(&context, &challenge_request(RequestScheme::Https, &[])).is_none()); +} + +#[tokio::test] +async fn the_bearer_challenge_falls_back_to_the_request_host() { + let context = token_service_context("", "").await; + + assert_eq!( + challenge_for(&context, &challenge_request(RequestScheme::Https, &[])).unwrap(), + r#"Bearer realm="https://registry.example.com/token",service="registry.example.com""# + ); +} + +#[tokio::test] +async fn a_configured_realm_wins_over_the_request_host() { + let context = token_service_context("", r#"realm = "https://public.example.com/token""#).await; + + assert_eq!( + challenge_for(&context, &challenge_request(RequestScheme::Https, &[])).unwrap(), + r#"Bearer realm="https://public.example.com/token",service="public.example.com""# + ); +} + +/// A TLS-terminating proxy serves angos over plaintext, so without honouring its +/// `X-Forwarded-Proto` the challenge would send credentials to an http realm. +#[tokio::test] +async fn a_trusted_proxy_decides_the_realm_scheme() { + let context = token_service_context(r#"trusted_proxies = ["10.0.0.0/8"]"#, "").await; + + assert_eq!( + challenge_for( + &context, + &challenge_request(RequestScheme::Http, &[("X-Forwarded-Proto", "https")]) + ) + .unwrap(), + r#"Bearer realm="https://registry.example.com/token",service="registry.example.com""# + ); +} + +#[tokio::test] +async fn an_untrusted_peer_cannot_change_the_realm_scheme() { + let context = token_service_context("", "").await; + + assert_eq!( + challenge_for( + &context, + &challenge_request(RequestScheme::Http, &[("X-Forwarded-Proto", "https")]) + ) + .unwrap(), + r#"Bearer realm="http://registry.example.com/token",service="registry.example.com""# + ); +} diff --git a/src/configuration/base64_string.rs b/src/configuration/base64_string.rs new file mode 100644 index 00000000..3fd4c0a7 --- /dev/null +++ b/src/configuration/base64_string.rs @@ -0,0 +1,60 @@ +use base64::{Engine, prelude::BASE64_STANDARD}; +use serde::{Deserialize, de}; +use zeroize::Zeroize; + +/// Bytes written in configuration as base64, decoded when the file is parsed. +/// +/// Key material belongs here rather than in a plain string: the strength is then +/// the randomness of the decoded bytes instead of whatever entropy a passphrase +/// happens to carry. It has no `Debug`, so it cannot reach a log by accident; +/// wrap it in [`Secret`](crate::secret::Secret) to have it zeroized on drop. +#[derive(Clone, Zeroize)] +pub struct Base64String(Vec); + +impl Base64String { + pub fn as_bytes(&self) -> &[u8] { + &self.0 + } +} + +impl From> for Base64String { + fn from(bytes: Vec) -> Self { + Self(bytes) + } +} + +impl<'de> Deserialize<'de> for Base64String { + fn deserialize>(deserializer: D) -> Result { + let encoded = String::deserialize(deserializer)?; + BASE64_STANDARD + .decode(&encoded) + .map(Self) + .map_err(|e| de::Error::custom(format!("invalid base64: {e}"))) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn parse(encoded: &str) -> Result { + #[derive(Deserialize)] + struct Holder { + value: Base64String, + } + toml::from_str::(&format!("value = \"{encoded}\"")).map(|holder| holder.value) + } + + #[test] + fn decodes_base64_with_and_without_padding() { + assert_eq!(parse("aGk=").unwrap().as_bytes(), b"hi"); + assert_eq!(parse("aGl0").unwrap().as_bytes(), b"hit"); + assert_eq!(parse("").unwrap().as_bytes(), b""); + } + + #[test] + fn rejects_text_that_is_not_base64() { + assert!(parse("not base64!").is_err()); + assert!(parse("aGk").is_err()); + } +} diff --git a/src/configuration/mod.rs b/src/configuration/mod.rs index e364bf86..1e6e26b0 100644 --- a/src/configuration/mod.rs +++ b/src/configuration/mod.rs @@ -8,6 +8,7 @@ use toml::{ use angos_tx_engine::lock::LockStrategy; +pub mod base64_string; mod error; pub mod global; pub mod listeners; @@ -20,6 +21,7 @@ pub mod trusted_proxy; mod ui; pub mod watcher; +pub use base64_string::Base64String; pub use error::Error; /// Deserialize a positive integer into a `NonZero` type, naming `field` in the diff --git a/src/configuration/tests/integration.rs b/src/configuration/tests/integration.rs index 9c4b0218..ede62d13 100644 --- a/src/configuration/tests/integration.rs +++ b/src/configuration/tests/integration.rs @@ -3,7 +3,6 @@ use std::{num::NonZeroUsize, path::PathBuf}; use angos_tx_engine::lock::LockStrategy; use crate::{ - auth::oidc, cache, configuration::listeners::ClientAuth, configuration::{ @@ -115,7 +114,6 @@ fn test_auth_section() { password = "$argon2id$v=19$m=19456,t=2,p=1$9pxWwg0VtZzDXno/25417Q$e+cuKy9VisJVxec/EEuKvvfIIIOy5yDGRzYKiuDLjx0" [auth.oidc.generic] - provider = "generic" issuer = "https://example.com" discovery_url = "https://example.com/.well-known/openid-configuration" "#; @@ -124,10 +122,7 @@ fn test_auth_section() { assert_eq!(config.auth.identity.len(), 1); assert_eq!(config.auth.identity["user1"].username, "bob"); assert_eq!(config.auth.oidc.len(), 1); - assert!(matches!( - config.auth.oidc.get("generic"), - Some(oidc::Config::Generic(_)) - )); + assert_eq!(config.auth.oidc["generic"].issuer, "https://example.com"); } #[test] diff --git a/src/identity/action/mod.rs b/src/identity/action/mod.rs index cb6f1aa6..2e7b0c56 100644 --- a/src/identity/action/mod.rs +++ b/src/identity/action/mod.rs @@ -52,6 +52,8 @@ pub enum Action { }, #[serde(rename = "ui-config")] UiConfig, + #[serde(rename = "get-token")] + Token, Healthz, Readyz, Metrics, @@ -285,6 +287,7 @@ impl Action { match self { Action::UiAsset { .. } => "ui-asset", Action::UiConfig => "ui-config", + Action::Token => "get-token", Action::Healthz => "healthz", Action::Readyz => "readyz", Action::Metrics => "metrics", @@ -319,6 +322,7 @@ impl Action { match self { Action::UiAsset { .. } | Action::UiConfig + | Action::Token | Action::Healthz | Action::Readyz | Action::Metrics diff --git a/src/identity/action/tests.rs b/src/identity/action/tests.rs index d5320e9a..fe3826f6 100644 --- a/src/identity/action/tests.rs +++ b/src/identity/action/tests.rs @@ -35,6 +35,7 @@ fn test_action_serialization_cel_compatibility() { }, ), ("ui-config", Action::UiConfig), + ("get-token", Action::Token), ("healthz", Action::Healthz), ("readyz", Action::Readyz), ("metrics", Action::Metrics), @@ -219,6 +220,7 @@ fn assert_action_variant_covered(action: &Action) { match action { Action::UiAsset { .. } | Action::UiConfig + | Action::Token | Action::Healthz | Action::Readyz | Action::Metrics diff --git a/src/identity/auth_method.rs b/src/identity/auth_method.rs index 25675bfd..49388579 100644 --- a/src/identity/auth_method.rs +++ b/src/identity/auth_method.rs @@ -6,6 +6,7 @@ #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub enum AuthMethod { Mtls, + Token, Oidc, Basic, /// No credential authenticated the request. @@ -18,6 +19,7 @@ impl AuthMethod { pub fn as_str(self) -> &'static str { match self { Self::Mtls => "mtls", + Self::Token => "token", Self::Oidc => "oidc", Self::Basic => "basic", Self::Anonymous => "anonymous", @@ -34,6 +36,7 @@ mod tests { #[test] fn every_method_keeps_its_logged_label() { assert_eq!(AuthMethod::Mtls.as_str(), "mtls"); + assert_eq!(AuthMethod::Token.as_str(), "token"); assert_eq!(AuthMethod::Oidc.as_str(), "oidc"); assert_eq!(AuthMethod::Basic.as_str(), "basic"); assert_eq!(AuthMethod::Anonymous.as_str(), "anonymous"); diff --git a/src/identity/client_identity.rs b/src/identity/client_identity.rs index ffe9191e..b3edd455 100644 --- a/src/identity/client_identity.rs +++ b/src/identity/client_identity.rs @@ -1,6 +1,6 @@ use std::{collections::HashMap, net::SocketAddr}; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use crate::identity::AuthMethod; @@ -19,6 +19,11 @@ pub struct ClientIdentity { /// policies see the credential fields, not the label. #[serde(skip)] pub auth_method: AuthMethod, + /// Set when a registry token supplied this identity. `auth_method` cannot + /// answer that on its own: a stronger method outranks the token in the + /// label while the token still filled the identity fields. + #[serde(skip)] + pub from_registry_token: bool, } impl ClientIdentity { @@ -43,10 +48,14 @@ pub struct ClientCertificate { /// All claims from the token are exposed as-is to allow maximum flexibility /// in policy expressions. Standard claims like sub, iss, aud are available /// along with any custom claims from the OIDC provider. -#[derive(Clone, Debug, Default, Serialize)] +/// +/// `Deserialize` exists so the token service can restore these claims from a +/// registry-issued token. It is deliberately absent from `ClientIdentity` and +/// `ClientCertificate`: certificate and client IP must come from the live +/// request, never from a token. +#[derive(Clone, Debug, Default, Deserialize, Serialize)] pub struct OidcClaims { pub provider_name: String, - pub provider_type: String, pub claims: HashMap, } diff --git a/src/registry/blob.rs b/src/registry/blob.rs index 9990a467..e5730975 100644 --- a/src/registry/blob.rs +++ b/src/registry/blob.rs @@ -195,14 +195,17 @@ impl Registry { #[instrument(skip(repository))] pub async fn head_blob( &self, - repository: &Repository, + repository: Option<&Repository>, accepted_types: &[MediaRange], namespace: &Namespace, digest: &Digest, ) -> Result { let has_access = self.blob_ownership().can_read(namespace, digest).await?; + // A namespace no `[repository]` entry matches has no upstream, so it + // serves what it owns and nothing else. + let upstream = repository.filter(|repository| repository.is_pull_through()); - if !repository.is_pull_through() && !has_access { + if upstream.is_none() && !has_access { return Err(Error::BlobUnknown); } @@ -217,19 +220,18 @@ impl Registry { // Mirror GET: a genuine miss on a pull-through repo re-heads // upstream; every other error (a transient or internal fault) // propagates instead of masquerading as a 404. - Err(Error::BlobUnknown) if repository.is_pull_through() => {} + Err(Error::BlobUnknown) if upstream.is_some() => {} Err(error) => return Err(error), } } - if repository.is_pull_through() { - let (digest, size) = repository - .head_blob(accepted_types, namespace, digest) - .await?; - Ok(HeadBlobResponse { digest, size }) - } else { - Err(Error::BlobUnknown) - } + let Some(repository) = upstream else { + return Err(Error::BlobUnknown); + }; + let (digest, size) = repository + .head_blob(accepted_types, namespace, digest) + .await?; + Ok(HeadBlobResponse { digest, size }) } /// Serve the blob locally when `has_access`, else fall back to the @@ -238,21 +240,23 @@ impl Registry { /// for the blob-index read twice. pub async fn get_blob_with_access( &self, - repository: &Repository, + repository: Option<&Repository>, accepted_types: &[MediaRange], namespace: &Namespace, digest: &Digest, range: Option, has_access: bool, ) -> Result { + let upstream = repository.filter(|repository| repository.is_pull_through()); + if has_access { match self.get_local_blob(digest, range).await { Ok(response) => return Ok(response), // Owned but the bytes are gone: a pull-through repo re-fetches. - Err(Error::BlobUnknown) if repository.is_pull_through() => {} + Err(Error::BlobUnknown) if upstream.is_some() => {} Err(error) => return Err(error), } - } else if !repository.is_pull_through() { + } else if upstream.is_none() { return Err(Error::BlobUnknown); } @@ -261,6 +265,9 @@ impl Registry { return Err(Error::RangeNotSatisfiable); } + let Some(repository) = upstream else { + return Err(Error::BlobUnknown); + }; let (total_length, client_stream) = repository .get_blob(accepted_types, namespace, digest) .await?; @@ -379,15 +386,15 @@ impl Registry { range: Option, allow_redirect: bool, ) -> Result { - let repository = self.get_repository_for_namespace(namespace)?; + let repository = self.get_repository_for_namespace(namespace).ok(); let has_access = self.blob_ownership().can_read(namespace, digest).await?; - if !repository.is_pull_through() && !has_access { + if !repository.is_some_and(Repository::is_pull_through) && !has_access { return Err(Error::BlobUnknown); } - let repository_name = repository.name.to_string(); + let repository_name = self.repository_name_for(namespace); let response = if range.is_none() && allow_redirect && self.enable_blob_redirect @@ -495,7 +502,7 @@ mod tests { let (digest, repository) = create_test_blob(registry, namespace, content).await; let response = registry - .head_blob(&repository, &[], namespace, &digest) + .head_blob(Some(&repository), &[], namespace, &digest) .await .unwrap(); @@ -550,7 +557,7 @@ mod tests { // A transient fault must surface (500), not masquerade as a missing blob // (404) the way GET already avoids. let result = registry - .head_blob(repository, &[], namespace, &digest) + .head_blob(Some(repository), &[], namespace, &digest) .await; assert!( matches!(result, Err(Error::Internal(_))), @@ -599,7 +606,7 @@ mod tests { let repository = registry.get_repository_for_namespace(namespace).unwrap(); let head_result = registry - .head_blob(repository, &[], namespace, &digest) + .head_blob(Some(repository), &[], namespace, &digest) .await; assert!(matches!(head_result, Err(Error::BlobUnknown))); @@ -1339,7 +1346,7 @@ mod tests { let (digest, repository) = create_test_blob(registry, namespace, content).await; let head_response = registry - .head_blob(&repository, &[], namespace, &digest) + .head_blob(Some(&repository), &[], namespace, &digest) .await .unwrap(); assert_eq!(head_response.digest, digest); diff --git a/src/registry/manifest/mod.rs b/src/registry/manifest/mod.rs index 36728d12..a7e7c3df 100644 --- a/src/registry/manifest/mod.rs +++ b/src/registry/manifest/mod.rs @@ -97,7 +97,7 @@ impl Registry { #[instrument(skip(repository))] pub async fn head_manifest( &self, - repository: &Repository, + repository: Option<&Repository>, accepted_types: &[MediaRange], namespace: &Namespace, reference: Reference, @@ -108,7 +108,7 @@ impl Registry { .serveable_local( namespace, &reference, - repository.is_pull_through(), + repository.is_some_and(Repository::is_pull_through), local, async |meta| { self.needs_upstream_pull_manifest( @@ -198,7 +198,7 @@ impl Registry { #[instrument(skip(repository))] pub async fn get_manifest( &self, - repository: &Repository, + repository: Option<&Repository>, accepted_types: &[MediaRange], namespace: &Namespace, reference: Reference, @@ -209,7 +209,7 @@ impl Registry { .serveable_local( namespace, &reference, - repository.is_pull_through(), + repository.is_some_and(Repository::is_pull_through), local, async |body| { self.needs_upstream_pull_manifest( @@ -228,6 +228,9 @@ impl Registry { return Ok(manifest); } + let Some(repository) = repository else { + return Err(Error::ManifestUnknown); + }; let fetched = repository .get_manifest(accepted_types, namespace, &reference) .await?; @@ -317,17 +320,18 @@ impl Registry { async fn needs_upstream_pull_manifest( &self, - repository: &Repository, + repository: Option<&Repository>, accepted_types: &[MediaRange], namespace: &Namespace, reference: &Reference, is_tag_immutable: bool, local_digest: &Digest, ) -> Result { - if !repository.is_pull_through() - || !matches!(reference, Reference::Tag(_)) - || is_tag_immutable - { + let upstream = repository.filter(|repository| repository.is_pull_through()); + let Some(repository) = upstream else { + return Ok(false); + }; + if !matches!(reference, Reference::Tag(_)) || is_tag_immutable { return Ok(false); } @@ -728,8 +732,8 @@ impl Registry { is_tag_immutable: bool, allow_redirect: bool, ) -> Result { - let repository = self.get_repository_for_namespace(namespace)?; - let repository_name = repository.name.to_string(); + let repository = self.get_repository_for_namespace(namespace).ok(); + let repository_name = self.repository_name_for(namespace); let event_reference = reference.clone(); let response = self @@ -757,14 +761,14 @@ impl Registry { async fn resolve_get_manifest_response( &self, - repository: &Repository, + repository: Option<&Repository>, namespace: &Namespace, reference: Reference, mime_types: &[MediaRange], is_tag_immutable: bool, allow_redirect: bool, ) -> Result { - let redirect_is_authoritative = !repository.is_pull_through() + let redirect_is_authoritative = !repository.is_some_and(Repository::is_pull_through) || matches!(reference, Reference::Digest(_)) || is_tag_immutable; diff --git a/src/registry/manifest/tests.rs b/src/registry/manifest/tests.rs index 5acafe20..8c63b5d3 100644 --- a/src/registry/manifest/tests.rs +++ b/src/registry/manifest/tests.rs @@ -198,6 +198,41 @@ async fn create_test_manifest_with_subject( ) } +/// `[repository]` entries configure namespaces rather than admitting them, so a +/// namespace none of them match is pushable and pullable like any other. +#[tokio::test] +async fn an_unconfigured_namespace_round_trips() { + for_each_backend(async |test_case| { + let registry = test_case.registry(); + let namespace = &Namespace::new("never/configured").unwrap(); + assert!(registry.get_repository_for_namespace(namespace).is_err()); + + let (content, media_type) = create_test_manifest(registry, namespace).await; + let tag = Reference::Tag(Tag::new("latest").unwrap()); + registry + .put_manifest(namespace, &tag, Some(&media_type), &content) + .await + .unwrap(); + + let stored = registry + .resolve_get_manifest( + None, + namespace, + tag, + &[MediaRange::from(media_type)], + false, + false, + ) + .await + .unwrap(); + + assert!( + matches!(stored, GetManifestResponse::Body { content: served, .. } if served == content) + ); + }) + .await; +} + #[tokio::test] async fn test_put_manifest() { for_each_backend(async |test_case| { @@ -218,7 +253,7 @@ async fn test_put_manifest() { let stored_manifest = registry .get_manifest( - registry.get_repository_for_namespace(namespace).unwrap(), + registry.get_repository_for_namespace(namespace).ok(), &[MediaRange::from(media_type.clone())], namespace, Reference::Tag(Tag::new(tag).unwrap()), @@ -288,7 +323,7 @@ async fn accept_put_manifest_by_sha512_digest_with_tag_params_creates_tags() { for tag in ["1.2.3", "latest"] { let head = registry .head_manifest( - repository, + Some(repository), &[MediaRange::from(media_type.clone())], &namespace, Reference::Tag(Tag::new(tag).unwrap()), @@ -340,7 +375,7 @@ async fn accept_put_manifest_by_tag_ignores_tag_params() { let repository = registry.get_repository_for_namespace(&namespace).unwrap(); let ignored = registry .head_manifest( - repository, + Some(repository), &[MediaRange::from(media_type.clone())], &namespace, Reference::Tag(Tag::new("ignored").unwrap()), @@ -714,7 +749,7 @@ async fn permissive_push_does_not_grant_read_of_unowned_child_manifest() { let repository = permissive.get_repository_for_namespace(&attacker).unwrap(); let outcome = permissive .get_manifest( - repository, + Some(repository), &[], &attacker, Reference::Digest(child_digest.clone()), @@ -783,7 +818,7 @@ async fn permissive_push_of_owned_references_yields_a_pullable_manifest() { let repository = permissive.get_repository_for_namespace(&namespace).unwrap(); permissive .get_manifest( - repository, + Some(repository), &[], &namespace, Reference::Tag(Tag::new("latest").unwrap()), @@ -859,7 +894,7 @@ async fn pull_through_computes_the_digest_when_the_upstream_omits_the_header() { let manifest = case .registry() .get_manifest( - &repository, + Some(&repository), &[MediaRange::from(MediaType::docker_manifest())], &namespace, Reference::Tag(Tag::new("latest").unwrap()), @@ -892,7 +927,7 @@ async fn pull_through_recomputes_under_the_requested_digest_algorithm() { let manifest = case .registry() .get_manifest( - &repository, + Some(&repository), &[MediaRange::from(MediaType::docker_manifest())], &namespace, Reference::Digest(requested.clone()), @@ -945,7 +980,13 @@ async fn a_backend_fault_is_not_reported_as_a_missing_manifest() { let repository = registry.get_repository_for_namespace(&namespace).unwrap(); let error = registry - .get_manifest(repository, &[], &namespace, Reference::Tag(tag), false) + .get_manifest( + Some(repository), + &[], + &namespace, + Reference::Tag(tag), + false, + ) .await .err() .expect("a failing metadata store must not read as a successful lookup"); @@ -976,7 +1017,7 @@ async fn test_get_manifest() { let manifest = registry .get_manifest( - registry.get_repository_for_namespace(namespace).unwrap(), + registry.get_repository_for_namespace(namespace).ok(), &[MediaRange::from(media_type.clone())], namespace, Reference::Tag(Tag::new(tag).unwrap()), @@ -991,7 +1032,7 @@ async fn test_get_manifest() { let manifest = registry .get_manifest( - registry.get_repository_for_namespace(namespace).unwrap(), + registry.get_repository_for_namespace(namespace).ok(), &[MediaRange::from(media_type.clone())], namespace, Reference::Digest(response.digest.clone()), @@ -1027,7 +1068,7 @@ async fn test_head_manifest() { let manifest = registry .head_manifest( - registry.get_repository_for_namespace(namespace).unwrap(), + registry.get_repository_for_namespace(namespace).ok(), &[MediaRange::from(media_type.clone())], namespace, Reference::Tag(Tag::new(tag).unwrap()), @@ -1042,7 +1083,7 @@ async fn test_head_manifest() { let manifest = registry .head_manifest( - registry.get_repository_for_namespace(namespace).unwrap(), + registry.get_repository_for_namespace(namespace).ok(), &[MediaRange::from(media_type.clone())], namespace, Reference::Digest(response.digest.clone()), @@ -1089,7 +1130,7 @@ async fn test_delete_manifest() { assert!( registry .get_manifest( - registry.get_repository_for_namespace(namespace).unwrap(), + registry.get_repository_for_namespace(namespace).ok(), &[MediaRange::from(media_type.clone())], namespace, Reference::Tag(Tag::new(tag).unwrap()), @@ -1112,7 +1153,7 @@ async fn test_delete_manifest() { assert!( registry .get_manifest( - registry.get_repository_for_namespace(namespace).unwrap(), + registry.get_repository_for_namespace(namespace).ok(), &[MediaRange::from(media_type.clone())], namespace, Reference::Digest(response.digest.clone()), @@ -1714,7 +1755,7 @@ async fn test_handle_put_manifest() { .expect("get repository failed"); let stored_manifest = registry .get_manifest( - repository, + Some(repository), &[MediaRange::from(media_type.clone())], namespace, Reference::Tag(Tag::new(tag).unwrap()), @@ -1772,7 +1813,7 @@ async fn test_delete_manifest_by_digest_removes_multiple_tags() { assert!( registry .get_manifest( - repository, + Some(repository), &[MediaRange::from(media_type.clone())], namespace, Reference::Tag(Tag::new("latest").unwrap()), @@ -1785,7 +1826,7 @@ async fn test_delete_manifest_by_digest_removes_multiple_tags() { assert!( registry .get_manifest( - repository, + Some(repository), &[MediaRange::from(media_type.clone())], namespace, Reference::Tag(Tag::new("v1.0").unwrap()), @@ -1798,7 +1839,7 @@ async fn test_delete_manifest_by_digest_removes_multiple_tags() { assert!( registry .get_manifest( - repository, + Some(repository), &[MediaRange::from(media_type.clone())], namespace, Reference::Digest(response.digest.clone()), @@ -1865,7 +1906,7 @@ async fn test_delete_manifest_by_digest_preserves_unrelated_tags() { assert!( registry .get_manifest( - repository, + Some(repository), &[MediaRange::from(media_type_a.clone())], namespace, Reference::Tag(Tag::new("v1.0").unwrap()), @@ -1878,7 +1919,7 @@ async fn test_delete_manifest_by_digest_preserves_unrelated_tags() { assert!( registry .get_manifest( - repository, + Some(repository), &[MediaRange::from(media_type_a.clone())], namespace, Reference::Tag(Tag::new("v1.1").unwrap()), @@ -1890,7 +1931,7 @@ async fn test_delete_manifest_by_digest_preserves_unrelated_tags() { let manifest_b = registry .get_manifest( - repository, + Some(repository), &[MediaRange::from(media_type_b.clone())], namespace, Reference::Tag(Tag::new("v2.0").unwrap()), @@ -1963,7 +2004,7 @@ async fn test_delete_manifest_with_many_tags() { assert!( registry .get_manifest( - repository, + Some(repository), &[MediaRange::from(media_type_a.clone())], namespace, Reference::Tag(Tag::new(&format!("tag-{i}")).unwrap()), @@ -1979,7 +2020,7 @@ async fn test_delete_manifest_with_many_tags() { assert!( registry .get_manifest( - repository, + Some(repository), &[MediaRange::from(media_type_b.clone())], namespace, Reference::Tag(Tag::new(&format!("other-{i}")).unwrap()), diff --git a/src/registry/test_utils.rs b/src/registry/test_utils.rs index 6843c438..69e2571f 100644 --- a/src/registry/test_utils.rs +++ b/src/registry/test_utils.rs @@ -275,7 +275,7 @@ pub async fn get_blob( .await?; registry .get_blob_with_access( - repository, + Some(repository), accepted_types, namespace, digest, diff --git a/src/registry_client/auth.rs b/src/registry_client/auth.rs index 546d9838..dbb84799 100644 --- a/src/registry_client/auth.rs +++ b/src/registry_client/auth.rs @@ -73,7 +73,7 @@ struct BearerToken { impl BearerToken { fn default_expires_in() -> u64 { - 3600 + 60 } fn token(&self) -> Result { @@ -231,14 +231,19 @@ impl RegistryClient { #[cfg(test)] mod tests { + use serde_json::from_str; use url::Url; - use crate::registry_client::{ - Error, - auth::{ - BearerToken, authority_for_cache_key, parse_bearer_challenge, token_cache_key, - token_index_cache_key, + use crate::{ + auth::{TokenIssuer, token_service::Config as TokenServiceConfig}, + registry_client::{ + Error, + auth::{ + BearerToken, authority_for_cache_key, parse_bearer_challenge, token_cache_key, + token_index_cache_key, + }, }, + secret::Secret, }; #[test] @@ -287,6 +292,41 @@ mod tests { assert!(matches!(result.unwrap_err(), Error::Internal(_))); } + /// Closes the loop on angos challenging angos: the server's own challenge + /// format must satisfy this parser, which needs the literal `Bearer ` prefix, + /// quoted values and an absolute realm. + #[test] + fn the_servers_own_challenge_parses_back() { + let config = TokenServiceConfig { + secret_key: Secret::new(vec![7; 32].into()), + realm: None, + ttl_secs: 3600, + }; + let issuer = TokenIssuer::new(&config).unwrap(); + let challenge = issuer.challenge("https", "registry.example.com").unwrap(); + + let parsed = parse_bearer_challenge(challenge.to_str().unwrap()).unwrap(); + + assert_eq!(parsed.realm, "https://registry.example.com/token"); + // The path the router serves, so a client following the challenge lands + // on the token endpoint rather than the UI. + assert_eq!(parsed.token_url().unwrap().path(), "/token"); + assert_eq!(parsed.param("service"), Some("registry.example.com")); + assert_eq!( + parsed.token_url().unwrap().query(), + Some("service=registry.example.com") + ); + } + + /// The spec's default when an upstream omits `expires_in`. Caching longer + /// than that keeps a token past the lifetime its issuer granted. + #[test] + fn an_omitted_expires_in_defaults_to_sixty_seconds() { + let bearer: BearerToken = from_str(r#"{"token":"t"}"#).unwrap(); + + assert_eq!(bearer.expires_in, 60); + } + #[test] fn authority_for_cache_key_returns_host() { let url = Url::parse("https://registry.example.com/v2/").unwrap();