From 155e68a5dbc041108115253d05cb33fd86a02101 Mon Sep 17 00:00:00 2001 From: mikeappsec Date: Fri, 1 May 2026 19:12:08 +1000 Subject: [PATCH 1/7] docs(cookbook): add enterprise runbooks for C1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four new cookbook recipes covering the enterprise operational workflows referenced by Tier C1 (DOC-COOKBOOK-1): - rotate-jwks: JWKS/IdP signing key rotation (dual-publish → drain → retire) - policy-shadow-mode: shadow mode, canary evaluation, and decision diffing - cache-invalidation: manual cache invalidation (Valkey keys, TTL tuning) - valkey-outage-drill: controlled Valkey outage drill with recovery checklist Update cookbook/README.md and mkdocs.yml nav with all four entries. --- docs/cookbook/README.md | 4 + docs/cookbook/cache-invalidation.md | 229 ++++++++++++++++++ docs/cookbook/policy-shadow-mode.md | 331 +++++++++++++++++++++++++++ docs/cookbook/rotate-jwks.md | 253 ++++++++++++++++++++ docs/cookbook/valkey-outage-drill.md | 299 ++++++++++++++++++++++++ mkdocs.yml | 4 + 6 files changed, 1120 insertions(+) create mode 100644 docs/cookbook/cache-invalidation.md create mode 100644 docs/cookbook/policy-shadow-mode.md create mode 100644 docs/cookbook/rotate-jwks.md create mode 100644 docs/cookbook/valkey-outage-drill.md diff --git a/docs/cookbook/README.md b/docs/cookbook/README.md index db7a470..9187440 100644 --- a/docs/cookbook/README.md +++ b/docs/cookbook/README.md @@ -12,6 +12,10 @@ PowerShell so a clean checkout works without `pip install`. | [openfga-on-envoy](openfga-on-envoy.md) | ReBAC with OpenFGA: tuples, model bootstrap, AuthConfig with `composite{anyOf:[rbac, openfga]}`. | | [istio-grpc-rbac](istio-grpc-rbac.md) | lwauth as the ext_authz provider in an Istio mesh, gating gRPC services with role-based policy. | | [rotate-hmac](rotate-hmac.md) | Rolling an HMAC shared secret with zero downtime via the multi-secret overlap window. | +| [rotate-jwks](rotate-jwks.md) | Rotating JWKS / IdP signing keys with zero-downtime dual-publish → drain → retire. | +| [policy-shadow-mode](policy-shadow-mode.md) | Testing a policy change with shadow mode, canary evaluation, and decision diffing. | +| [cache-invalidation](cache-invalidation.md) | Manually invalidating cached decisions and introspection results. | +| [valkey-outage-drill](valkey-outage-drill.md) | Controlled Valkey outage drill — what breaks, what degrades, how to recover. | Every recipe ends with a teardown stanza so you can iterate without piling up Helm releases. Open an issue if a recipe drifts from the diff --git a/docs/cookbook/cache-invalidation.md b/docs/cookbook/cache-invalidation.md new file mode 100644 index 0000000..b986866 --- /dev/null +++ b/docs/cookbook/cache-invalidation.md @@ -0,0 +1,229 @@ +# Invalidate cached decisions and introspection results + +lwauth caches aggressively — decision verdicts, introspection +responses, JWKS metadata, DPoP `jti` replay records — because the +alternative is calling the IdP or policy engine on every request. Most +of the time TTL expiry is the right invalidation strategy: short TTLs +(30s decision, 5m introspection) keep staleness bounded without +operator intervention. + +Sometimes you need to invalidate **now**: a user's access was revoked, +a policy was wrong, or an introspection cache is serving a stale +`active: true` for a token the IdP has since disabled. This recipe +covers the manual and automated invalidation paths available today and +the tag-based invalidation coming in Tier E. + +## What this recipe assumes + +- lwauth with the [`valkey` cache backend](../modules/cache-valkey.md) + for shared invalidation across replicas. The [`memory` + backend](../modules/cache-memory.md) path is noted where it + differs. +- `kubectl` access to the lwauth namespace. +- Basic familiarity with Valkey/Redis CLI commands. + +## When to invalidate (and when not to) + +| Situation | Invalidate? | Why | +|---|---|---| +| User's access revoked at IdP | **Yes** — introspection cache | Cached `active: true` persists until TTL | +| Policy updated via AuthConfig | **No** — automatic | Engine hot-swap compiles new policy; decision cache keys include policy version | +| JWKS key rotated at IdP | **No** — automatic | JWKS cache refreshes on interval; see [rotate-jwks](rotate-jwks.md) | +| Wrong policy shipped, need immediate rollback | **Maybe** | Rollback the AuthConfig first (engine swap is atomic); invalidate the decision cache only if stale cached verdicts from the bad policy are still being served | +| Compromised token needs kill-switch | **Yes** — introspection + decision cache | Short-TTL usually suffices; for guaranteed kill use revocation (Tier E — M14) | + +## 1. Invalidate via TTL tuning (no Valkey access needed) + +The simplest invalidation is to shorten the TTL so stale entries +expire faster. This works for both `memory` and `valkey` backends: + +```yaml +# Tighten decision cache to 5 seconds during an incident. +# Normal: decisionTtl: 30s +cache: + backend: valkey + addr: valkey-master.cache.svc:6379 + decisionTtl: 5s # ← temporary tightening + introspectionTtl: 30s # ← tighten if introspection is the problem +``` + +```bash +lwauthctl validate --config tightened-config.yaml +kubectl apply -f tightened-config.yaml +``` + +The engine hot-swaps the new TTL immediately. Existing cache entries +keep their original expiry, but new writes use the shorter TTL. +Within `max(old TTL)` seconds, every entry written under the old TTL +has expired. + +**Revert** after the incident by restoring the original TTLs. + +## 2. Invalidate the Valkey cache directly + +For immediate invalidation, delete keys from Valkey. lwauth uses a +predictable key prefix structure: + +``` +/// +``` + +Where `keyPrefix` defaults to `lwauth/` (configurable in +`cache.keyPrefix`). + +### Flush the entire lwauth cache + +```bash +# Connect to Valkey +kubectl -n cache exec -it deploy/valkey -- valkey-cli + +# Delete all lwauth keys (use SCAN, never KEYS in production) +127.0.0.1:6379> EVAL "local c=0; local r=redis.call('SCAN','0','MATCH','lwauth/*','COUNT',1000); for _,k in ipairs(r[2]) do redis.call('DEL',k); c=c+1 end; return c" 0 +# Returns: number of deleted keys + +# For large caches, loop until the cursor returns 0: +# (script version) +``` + +```bash +# Or from outside the pod, one-liner: +kubectl -n cache exec deploy/valkey -- valkey-cli --scan --pattern 'lwauth/*' | \ + xargs -L 100 kubectl -n cache exec -i deploy/valkey -- valkey-cli DEL +``` + +### Flush only the decision cache for one tenant + +```bash +kubectl -n cache exec deploy/valkey -- \ + valkey-cli --scan --pattern 'lwauth/decision/payments/*' | \ + xargs -L 100 kubectl -n cache exec -i deploy/valkey -- valkey-cli DEL +``` + +### Flush introspection cache for a specific token + +If you know the token (or its SHA-256 hash), delete the exact key: + +```bash +TOKEN_HASH=$(echo -n "$TOKEN" | sha256sum | cut -d' ' -f1) +kubectl -n cache exec deploy/valkey -- \ + valkey-cli DEL "lwauth/introspect/$TOKEN_HASH" +``` + +## 3. Invalidate the in-process LRU (memory backend) + +The `memory` backend has no external handle. Two options: + +### Option A: Restart the pods + +```bash +kubectl -n lwauth-system rollout restart deploy/lwauth +``` + +This is a blunt instrument — every pod drops its entire L1 cache and +starts cold. Use it only when the blast radius is acceptable. + +### Option B: Lower TTL + wait + +Same as §1 above. The LRU entries expire within the old TTL window. +No restart needed, but not instantaneous. + +## 4. Tag-based invalidation (Tier E — ENT-CACHE-2) + +When Tier E ships, cache writes will carry tags for tenant, subject, +policy version, and AuthConfig. Invalidation becomes a targeted +operation: + +```bash +# Future: invalidate all cached decisions for user alice +# in the payments tenant +lwauthctl cache invalidate \ + --tenant payments \ + --subject alice + +# Future: invalidate everything tied to a specific policy version +lwauthctl cache invalidate \ + --policy-version "2026-04-15-prod" +``` + +This publishes a `cache.invalidate` event over Valkey pub/sub; every +replica drops matching L1 entries and the L2 keys are deleted. Until +then, the manual Valkey key deletion in §2 is the equivalent. + +## 5. Verify the invalidation worked + +After invalidating, confirm that fresh requests are hitting the +backend (IdP, OPA, OpenFGA) rather than the cache: + +```bash +# Check cache hit/miss metrics +kubectl -n lwauth-system exec deploy/lwauth -c lwauth -- \ + curl -s localhost:8080/metrics | grep lwauth_cache + +# Look for a spike in cache misses: +# lwauth_cache_hits_total{cache="decision", outcome="miss"} +# lwauth_cache_hits_total{cache="introspect", outcome="miss"} + +# Confirm upstream calls increased: +# lwauth_upstream_requests_total{upstream="introspect"} +``` + +```promql +# PromQL: cache miss rate spike after invalidation +rate(lwauth_cache_hits_total{outcome="miss"}[1m]) +``` + +The miss rate should spike briefly then settle as the cache refills +with fresh entries. + +## Operational checklist + +```markdown +- [ ] Identify what to invalidate (decision, introspection, or both) +- [ ] Identify the scope (all tenants, one tenant, one user, one token) +- [ ] If Valkey: connect and delete the matching keys (§2) +- [ ] If memory-only: lower TTL (§1) or restart pods (§3) +- [ ] Verify cache misses spiked (§5) +- [ ] Verify the correct behaviour on a test request +- [ ] Restore TTLs if you tightened them +- [ ] Document the incident and what triggered the invalidation +``` + +## What can still go wrong + +- **Deleting DPoP replay keys.** The `lwauth/dpop/*` keys are + **security-critical** — they prevent token replay. Do NOT delete + them unless you understand the consequences. A deleted DPoP `jti` + record means a replayed token will be accepted until TTL re-expires. + Scope your `SCAN` pattern carefully. +- **Thundering herd after flush.** Deleting the entire cache under + load causes every request to miss simultaneously, potentially + overwhelming the IdP or policy engine. Prefer scoped invalidation + (per-tenant, per-subject) over a full flush. If you must flush + everything, consider doing it during a low-traffic window. +- **Race between invalidation and cache write.** A request in flight + during the `DEL` may re-populate the key with stale data. For + absolute consistency, combine invalidation with a TTL tightening: + delete the keys, then lower the TTL so any re-populated entry + expires quickly. +- **Memory backend has no cross-replica story.** Each pod has its own + LRU. Deleting keys on one pod does not affect others. This is why + the `valkey` backend is recommended for multi-replica deployments. + +## What to look at next + +- [Valkey outage drill](valkey-outage-drill.md) — what happens when + the cache backend itself goes down. +- [`cache-valkey` reference](../modules/cache-valkey.md) — key prefix, + TTLs, failure modes. +- [`cache-memory` reference](../modules/cache-memory.md) — LRU sizing. +- [DESIGN.md §11.3](../DESIGN.md) — multi-tier cache, tag + invalidation, stale-while-revalidate design. + +## References + +- [`cache-valkey`](../modules/cache-valkey.md) — configuration and + key structure. +- [`cache-memory`](../modules/cache-memory.md) — in-process LRU. +- [`observability`](../modules/observability.md) — cache metrics. +- [DESIGN.md §11.3](../DESIGN.md) — caching improvements design. +- Roadmap: E1 (ENT-CACHE-1), E3 (ENT-CACHE-2), E4 (ENT-CACHE-3). diff --git a/docs/cookbook/policy-shadow-mode.md b/docs/cookbook/policy-shadow-mode.md new file mode 100644 index 0000000..586c075 --- /dev/null +++ b/docs/cookbook/policy-shadow-mode.md @@ -0,0 +1,331 @@ +# Test a policy change with shadow mode and canary evaluation + +You need to ship a policy change — migrating from RBAC to OPA, adding +a new CEL rule, tightening an OpenFGA model — but you cannot risk a +production outage if the new policy denies requests the old one +allowed. lwauth's **shadow mode** and **canary evaluation** let you +deploy the new policy alongside the existing one, compare their +verdicts on live traffic, and promote only when you have evidence they +agree. + +This recipe walks the full lifecycle: shadow → audit → canary → +promote. It uses the primitives described in +[DESIGN.md §11.2](../DESIGN.md) and the +[`composite` authorizer](../modules/composite.md). + +!!! note "Tier D feature" + Shadow mode (`spec.mode: shadow`) and canary evaluation + (`spec.canary`) are **Tier D roadmap items** (D2 / D3). This + cookbook documents the *operational workflow* so you can plan for + it now and adopt it the moment the feature ships. The policy + diffing CLI (`lwauth diff`) is also part of D2. + +## What this recipe assumes + +- An existing `AuthConfig` with at least one authorizer (e.g. `rbac`) + serving production traffic. +- A new policy you want to test (e.g. an OPA Rego policy, a revised + RBAC matrix, a new CEL expression). +- `lwauthctl` v1.2+ (ships with shadow/canary support). +- Prometheus + Grafana or equivalent for metric visualization. +- Audit logging enabled + ([`observability`](../modules/observability.md)). + +## The three-stage rollout + +``` + shadow canary (10%) promote (100%) + ┌──────┐ ┌──────────┐ ┌─────────┐ + │ new │──OK──▶│ new runs │──OK──▶ │ new is │ + │ logs │ │ on slice │ │ prod │ + │ only │ │ of traffic│ │ │ + └──────┘ └──────────┘ └─────────┘ + ▲ ▲ ▲ + audit log agreement metric lwauthctl promote + confirms confirms removes old policy + no surprises convergence +``` + +## 1. Deploy the new policy as a shadow AuthConfig + +Create a second `AuthConfig` with `spec.mode: shadow`. This tells +lwauth to run the full identify → authorize → mutate pipeline but +**never** return the shadow's verdict to Envoy or Door B. The +production policy continues to serve all traffic. + +```yaml +# authconfig-shadow.yaml +apiVersion: auth.lwauth.io/v1alpha1 +kind: AuthConfig +metadata: + name: payments-v2-shadow + namespace: payments +spec: + version: "2026-05-01-shadow" + mode: shadow # ← key field + + hosts: [payments.example.com] # same hosts as production + tenantId: payments + + identifiers: # same identifiers + - name: bearer + type: jwt + config: + issuerUrl: https://idp.example.com + audiences: [payments-api] + + authorizers: + - name: next-policy + type: opa + config: + policy: file:///etc/lwauth/policies/payments-v2.rego + query: data.payments.allow +``` + +```bash +lwauthctl validate --config authconfig-shadow.yaml +kubectl apply -f authconfig-shadow.yaml +``` + +Both the production AuthConfig and the shadow run on every request. +The shadow's verdict appears in two places: + +1. **Audit log** — tagged with `policy_version: "2026-05-01-shadow"` + and `mode: shadow`. +2. **Prometheus** — `lwauth_decisions_total{policy_version="2026-05-01-shadow", mode="shadow"}`. + +## 2. Analyze shadow disagreements + +Let the shadow run for a representative traffic window (typically +24–72 hours covering weekday + weekend patterns). Then query for +disagreements — requests where the shadow's verdict differs from +production: + +```bash +# From audit JSONL: find requests where prod=allow but shadow=deny +kubectl -n lwauth-system exec deploy/lwauth -c lwauth -- \ + cat /var/log/lwauth/audit.jsonl | \ + jq 'select(.shadow != null and .decision.allow == true + and .shadow.allow == false)' | \ + jq '{method, path: .request.path, sub: .identity.sub, + prod_reason: .decision.reason, shadow_reason: .shadow.reason}' | \ + head -50 +``` + +```promql +# PromQL: shadow disagreement rate (non-zero = investigate) +sum(rate(lwauth_decisions_total{ + mode="shadow", + agreement="prod_allow_shadow_deny" +}[1h])) +``` + +Common findings and how to handle them: + +| Disagreement | Likely cause | Fix | +|---|---|---| +| Shadow denies requests prod allows | New policy is stricter — may be intentional or a rule bug | Review the denied paths; update the Rego/CEL if unintentional | +| Shadow allows requests prod denies | New policy is more permissive — usually a model gap | Tighten the new policy before promoting | +| Both deny but with different reasons | Cosmetic — both are correct, but the error path differs | Usually safe to ignore; review for debugging clarity | + +### Decision diff CLI + +For a deeper analysis, replay captured audit logs against both policy +versions offline: + +```bash +# Capture a window of audit data +kubectl -n lwauth-system exec deploy/lwauth -c lwauth -- \ + cat /var/log/lwauth/audit.jsonl > audit-window.jsonl + +# Replay against both policies +lwauth diff \ + --left "version=2026-04-15-prod" \ + --right "version=2026-05-01-shadow" \ + --replay audit-window.jsonl + +# Output groups divergences by (method, path-template, deny_reason) +# so you can see patterns rather than individual requests. +``` + +## 3. Promote to canary evaluation + +Once the shadow shows zero (or understood) disagreements, promote the +new policy to canary mode. Canary evaluation runs **both** authorizers +on a slice of live traffic and returns the **production** verdict by +default — but logs both: + +```yaml +# authconfig-canary.yaml +apiVersion: auth.lwauth.io/v1alpha1 +kind: AuthConfig +metadata: + name: payments + namespace: payments +spec: + version: "2026-05-01-canary" + hosts: [payments.example.com] + tenantId: payments + + identifiers: + - name: bearer + type: jwt + config: + issuerUrl: https://idp.example.com + audiences: [payments-api] + + authorizers: + - name: current-policy + type: rbac + config: + # ...existing production RBAC rules... + + canary: + weight: 10 # 10% of traffic + sample: sticky:hash(sub) # same user always gets canary + authorizer: + name: next-policy + type: opa + config: + policy: file:///etc/lwauth/policies/payments-v2.rego + query: data.payments.allow +``` + +```bash +lwauthctl validate --config authconfig-canary.yaml +lwauthctl diff --from authconfig-prod.yaml --to authconfig-canary.yaml +kubectl apply -f authconfig-canary.yaml +``` + +Monitor the canary metrics: + +```promql +# Agreement rate — should converge to 1.0 +sum(rate(lwauth_decisions_total{ + policy_track="canary", agreement="match" +}[5m])) +/ +sum(rate(lwauth_decisions_total{ + policy_track="canary" +}[5m])) +``` + +### Ramp the canary + +If agreement holds, increase the weight gradually: + +```bash +# 10% → 25% → 50% → 100% +kubectl -n payments patch authconfig payments \ + --type merge -p '{"spec":{"canary":{"weight": 25}}}' +``` + +At each step, let the agreement metric settle for at least one +traffic cycle before ramping further. + +## 4. Promote the canary to production + +When `agreement=match` is at 100% across all canary traffic for a +sustained period, promote: + +```yaml +# authconfig-promoted.yaml +apiVersion: auth.lwauth.io/v1alpha1 +kind: AuthConfig +metadata: + name: payments + namespace: payments +spec: + version: "2026-05-01-prod" # new version tag + hosts: [payments.example.com] + tenantId: payments + + identifiers: + - name: bearer + type: jwt + config: + issuerUrl: https://idp.example.com + audiences: [payments-api] + + authorizers: + - name: next-policy # the canary is now prod + type: opa + config: + policy: file:///etc/lwauth/policies/payments-v2.rego + query: data.payments.allow + + # canary: block removed — single policy, no dual evaluation +``` + +```bash +lwauthctl validate --config authconfig-promoted.yaml +kubectl apply -f authconfig-promoted.yaml + +# Clean up the shadow AuthConfig if it is still around +kubectl -n payments delete authconfig payments-v2-shadow +``` + +Verify the promotion stuck: + +```bash +kubectl -n payments get authconfig payments \ + -o jsonpath='{.status.appliedVersion}' +# expect: "2026-05-01-prod" +``` + +## 5. Rollback (if needed) + +At any stage, rollback is a `kubectl apply` of the previous config: + +```bash +# Re-apply the baseline config. The engine hot-swaps atomically. +kubectl apply -f authconfig-prod.yaml + +# Confirm: +kubectl -n payments get authconfig payments \ + -o jsonpath='{.status.appliedVersion}' +# expect: the old version string +``` + +Shadow and canary modes never affect the production verdict (unless +`canary.enforce: true` is explicitly set), so removing them is always +safe. + +## What can still go wrong + +- **Shadow performance impact.** Shadow runs the full pipeline, so + it doubles the authorizer load. If your authorizer calls an + external service (OPA, OpenFGA), shadow doubles that traffic too. + Monitor `lwauth_upstream_duration_seconds` during the shadow phase. +- **Canary with `enforce: true` before validation.** Setting + `canary.enforce: true` makes the canary verdict the real verdict + for the sampled traffic. Only set this after the observe-only phase + proves agreement. +- **Decision cache interaction.** The cache keys include + `policy_version`, so shadow and canary verdicts are cached + separately from production. A canary verdict is never served as a + production response. This is by design but means cache hit rates + drop during canary — plan capacity accordingly. +- **Sticky sampling bias.** `sticky:hash(sub)` ensures a user always + gets canary or always gets production. If your disagreements are + user-specific (e.g. a role only certain users have), the canary + sample may miss them. Use `sample: random` for broader coverage at + the cost of per-user inconsistency. + +## What to look at next + +- [DESIGN.md §11.2](../DESIGN.md) — full policy rotation design + (versioning, canary, shadow, diffing). +- [`composite` authorizer](../modules/composite.md) — the underlying + machinery that canary evaluation builds on. +- [`observability`](../modules/observability.md) — audit log and + metrics shape. +- [Rotate HMAC secrets](rotate-hmac.md) — a different kind of + rotation (key material, not policy). + +## References + +- [DESIGN.md §11.2](../DESIGN.md) — policy rotation architecture. +- [`composite` authorizer](../modules/composite.md) — dual evaluation. +- [`observability`](../modules/observability.md) — audit + metrics. +- Roadmap: D2 (ENT-POLICY-1), D3 (ENT-POLICY-2). diff --git a/docs/cookbook/rotate-jwks.md b/docs/cookbook/rotate-jwks.md new file mode 100644 index 0000000..c895688 --- /dev/null +++ b/docs/cookbook/rotate-jwks.md @@ -0,0 +1,253 @@ +# Rotate JWKS / IdP signing keys without downtime + +Your IdP (Keycloak, Auth0, Entra ID, `lwauth-idp`, or any OIDC +provider) signs JWTs with a private key whose `kid` appears in the +JWKS endpoint. You need to roll that key — either on a schedule, because +the algorithm is being upgraded, or because the old key leaked — without +a window where valid tokens are rejected. + +lwauth's [`jwt` identifier](../modules/jwt.md) already supports this +natively: the JWKS cache holds every `kid` the IdP publishes, and +verification picks the JWK whose `kid` matches the token header. The +rotation procedure is therefore about **timing the IdP-side change** +and **proving the fleet has picked up the new key** before retiring the +old one — not about editing the lwauth config at all. + +This recipe walks the full lifecycle, including the verification steps +most rotations skip. + +## What this recipe assumes + +- An existing `AuthConfig` with a `jwt` identifier pointing at a JWKS + URL (inline or via `IdentityProvider`). +- lwauth refreshes JWKS every 10 minutes by default (or honours the + IdP's `Cache-Control: max-age` header). You have not overridden + `jwksRefreshInterval` to something longer than 1 hour. +- `lwauthctl` v1.0+ and `kubectl` on your workstation. +- Prometheus scraping lwauth pods, or at minimum access to the + `/metrics` endpoint. + +This recipe **does not** cover rotating mTLS trust bundles, HMAC shared +secrets (see [rotate-hmac](rotate-hmac.md)), or `jwt-issue` mutator +signing keys — those have different overlap mechanics. + +## 0. Capture the starting state + +Before touching the IdP, confirm which `kid` lwauth is currently +verifying against: + +```bash +# Check the JWKS endpoint directly. +curl -s $(kubectl -n lwauth-system get configmap lwauth-config \ + -o jsonpath='{.data.config\.yaml}' | \ + yq '.identifiers[] | select(.type == "jwt") | .config.jwksUrl') \ + | jq '.keys[].kid' +# expect: ["rsa-2025-11"] (or whatever the current kid is) + +# Confirm lwauth loaded it. The metric shows per-kid verification +# counts (v1.1+): +kubectl -n lwauth-system exec deploy/lwauth -c lwauth -- \ + curl -s localhost:8080/metrics | grep lwauth_jwks_refresh_total +# expect: lwauth_jwks_refresh_total{issuer="...",outcome="ok"} +``` + +Note the current `kid` — you will use it in phase 3 to prove it has +drained. + +## 1. Phase 1 — publish the new key on the IdP (dual-publish) + +Add a new signing key to your IdP. The old key stays active for +signing; the new key is **published in JWKS but not yet used for +signing**. This ensures every lwauth replica loads the new `kid` into +its JWKS cache before any token carries it. + +How you do this depends on your IdP: + +| IdP | How to add a second key | +|---|---| +| **Keycloak** | Realm → Keys → Providers → add an `rsa-generated` provider with higher priority. The old provider stays active until you set its priority lower. | +| **Auth0** | Dashboard → Settings → Signing Keys → Rotate Signing Key. Auth0 publishes both keys in JWKS immediately; the new one becomes the signing key after confirmation. | +| **Entra ID** | Automatic — Microsoft publishes keys ~6 weeks before activation. No action needed; skip to phase 2. | +| **lwauth-idp** | `POST /admin/keys` with the new key material; set `active: false` initially. | + +After the IdP publishes both keys, confirm lwauth sees them: + +```bash +# Wait for at least one JWKS refresh cycle (default 10 min). +sleep 600 + +# Verify both kids are in the cache. +curl -s | jq '[.keys[].kid]' +# expect: ["rsa-2025-11", "rsa-2026-05"] +``` + +!!! warning "Do NOT start signing with the new key yet" + Phase 1 is about **pre-loading** the verifier. The goal is that + every lwauth replica already knows the new `kid` before the IdP + starts minting tokens with it. If you skip this phase, the first + token signed with the new key will fail verification on any replica + whose JWKS cache has not refreshed yet. + +## 2. Phase 2 — cut the IdP over to the new signing key + +Switch the IdP to sign new tokens with the new `kid`. The old key +remains **published in JWKS** (so tokens minted before the cutover +still verify) but is no longer used for new signatures. + +| IdP | How to switch the signing key | +|---|---| +| **Keycloak** | Set the new provider's priority higher than the old one. | +| **Auth0** | Confirm the rotation in Dashboard → Settings → Signing Keys. | +| **Entra ID** | Automatic — no action. | +| **lwauth-idp** | `PATCH /admin/keys/` with `active: true`. | + +Monitor the transition. Tokens already issued carry the old `kid` and +remain valid until they expire. New tokens carry the new `kid`: + +```bash +# Watch the per-kid verification metric. Over time, the old kid's +# rate drops to zero as tokens expire. +kubectl -n lwauth-system exec deploy/lwauth -c lwauth -- \ + curl -s localhost:8080/metrics | grep lwauth_decisions_total | \ + grep 'identifier="jwt"' + +# Or via PromQL: +# rate(lwauth_decisions_total{identifier="jwt"}[5m]) +# broken down by the token's kid (available in audit JSONL as +# identifier_attrs.kid). +``` + +The overlap window length = the maximum `exp - iat` of tokens minted +under the old key. For short-lived tokens (5–15 min) the window is +trivially short. For long-lived refresh tokens, the window may be days +— that is fine; the old key stays in JWKS for the duration. + +## 3. Phase 3 — retire the old key from JWKS + +Once **all** tokens signed with the old `kid` have expired, remove the +old key from the IdP's JWKS endpoint. + +**How to know it is safe:** + +```bash +# Option A: Check the audit log for any decision that verified +# using the old kid. Zero hits = safe to retire. +kubectl -n lwauth-system exec deploy/lwauth -c lwauth -- \ + cat /var/log/lwauth/audit.jsonl | \ + jq -r 'select(.identifier == "jwt") | .identifier_attrs.kid' | \ + sort | uniq -c | sort -nr +# expect: only the new kid appears. + +# Option B: PromQL — the old kid's verification rate is zero +# across all replicas for at least 2× the max token lifetime. +# sum(rate(lwauth_decisions_total{identifier="jwt",kid="rsa-2025-11"}[1h])) == 0 +``` + +Remove the old key from the IdP, then confirm lwauth's next JWKS +refresh drops it: + +```bash +sleep 600 # wait for refresh +curl -s | jq '[.keys[].kid]' +# expect: ["rsa-2026-05"] — only the new key +``` + +## 4. Phase 4 — prove it + +```bash +# 4.1 A token signed with the old kid is now rejected. +# (Craft one with a test tool, or replay a captured JWT.) +OLD_TOKEN="eyJ..." # a token with kid=rsa-2025-11 +curl -s -o /dev/null -w '%{http_code}' \ + -H "Authorization: Bearer $OLD_TOKEN" \ + https://api.example.com/v1/orders +# expect: 401 + +# 4.2 A fresh token (new kid) still works. +NEW_TOKEN=$(curl -s -X POST "$IDP_URL/oauth/token" \ + -d 'grant_type=client_credentials&...' | jq -r .access_token) +curl -s -o /dev/null -w '%{http_code}' \ + -H "Authorization: Bearer $NEW_TOKEN" \ + https://api.example.com/v1/orders +# expect: 200 + +# 4.3 The JWKS endpoint no longer serves the old kid. +curl -s | jq '.keys[] | select(.kid == "rsa-2025-11")' +# expect: empty +``` + +## What lwauth does NOT require during rotation + +Unlike HMAC rotation ([rotate-hmac](rotate-hmac.md)), JWKS rotation +does **not** require any `AuthConfig` or `IdentityProvider` edit. The +`jwt` identifier dynamically fetches and caches JWKS; the only config +is `jwksUrl` (or `issuerUrl` for OIDC discovery), which does not +change. This means: + +- No `lwauthctl validate` / `diff` / `apply` cycle. +- No engine hot-swap. +- No ConfigMap or CRD edit. +- No Pod restart. + +The entire rotation is an IdP-side operation observed passively by +lwauth. + +## Accelerating the refresh (optional) + +If you cannot wait 10 minutes for the background refresh to pick up a +newly published key, two options: + +1. **Lower the refresh interval.** Set `jwksRefreshInterval: 60s` on + the `jwt` identifier. This increases IdP traffic by 10× but makes + key publication visible within a minute. Suitable for dev/staging. + +2. **Restart the lwauth pods.** Not recommended in production (defeats + the "no restart" goal), but it forces an immediate JWKS fetch on + boot. Useful in CI. + +For v1.2+ (Tier D — ENT-KEYROT-1), lwauth will expose a +`forceRefresh()` trigger and `lwauth_jwks_refresh_total{issuer,outcome}` +metrics so SREs can push-refresh and prove the new `kid` is loaded +without waiting for the next poll. + +## What can still go wrong + +- **IdP removes the old key before tokens expire.** Any in-flight + token whose `kid` is no longer in JWKS gets a `jwt: unknown kid` + rejection. Wait for `exp` to drain. +- **JWKS endpoint downtime during refresh.** lwauth keeps the last + successfully fetched JWKS in memory. A transient fetch failure + does not evict cached keys — it logs a warning and retries on the + next interval. Extended downtime (> cache TTL) will eventually + cause the cache to go stale; pair with `serveStaleOnUpstreamError` + (Tier E — ENT-CACHE-2) when available. +- **Algorithm change (e.g. RS256 → ES256).** This is safe as long as + both keys are published in JWKS simultaneously. The verifier matches + by `kid` and reads `alg` from the JWK, so an algorithm switch is + transparent. Ensure your JWT library on the client side supports the + new algorithm. +- **Clock skew.** `exp` / `nbf` / `iat` checks use the lwauth host + clock ± `clockSkew` (default 30s). If your pods have significant + drift, tokens near their `exp` boundary may reject spuriously during + the overlap window. + +## What to look at next + +- [Rotate HMAC secrets without downtime](rotate-hmac.md) — the + config-edit rotation for symmetric keys. +- [DESIGN.md §11.1](../DESIGN.md) — the full key-rotation design + covering all six key materials. +- [`jwt` identifier reference](../modules/jwt.md) — `jwksUrl`, + `issuerUrl`, `audiences`, `clockSkew`, `jwksRefreshInterval`. +- [`observability`](../modules/observability.md) — metrics and audit + log shape. + +## References + +- [`jwt` identifier](../modules/jwt.md) — JWKS cache, verification, + `kid` matching. +- [`observability`](../modules/observability.md) — audit log shape + consumed by `lwauthctl audit`. +- [DESIGN.md §11.1](../DESIGN.md) — seamless key rotation design. +- [RFC 7517](https://datatracker.ietf.org/doc/html/rfc7517) — JWK + specification. diff --git a/docs/cookbook/valkey-outage-drill.md b/docs/cookbook/valkey-outage-drill.md new file mode 100644 index 0000000..dbec96a --- /dev/null +++ b/docs/cookbook/valkey-outage-drill.md @@ -0,0 +1,299 @@ +# Valkey outage drill — what happens when the cache goes down + +lwauth uses Valkey as a shared cache for introspection results, +decision verdicts, DPoP replay detection, and (in future) revocation +lists. Valkey is not in the trust path — lwauth never delegates an +auth *decision* to Valkey — but a Valkey outage changes the +performance and consistency profile of your deployment. This recipe +walks you through a controlled Valkey outage drill so you know exactly +what to expect before it happens at 02:00 on a Saturday. + +## What this recipe assumes + +- A multi-replica lwauth deployment with + [`cache.backend: valkey`](../modules/cache-valkey.md). +- A Valkey instance you can safely disrupt (ideally a staging + cluster; if you must drill in production, use the pod-isolation + approach in §3). +- `kubectl`, Prometheus, and a way to generate representative + traffic (a load test, a shadow traffic replay, or simply your + existing staging workload). +- Familiarity with lwauth's cache failure modes + ([`cache-valkey`](../modules/cache-valkey.md)). + +## The failure model + +lwauth treats Valkey failures differently depending on the cache +namespace: + +| Cache namespace | Failure mode | Why | +|---|---|---| +| **Decision cache** | **Fail-open** — cache miss, evaluate from scratch | Correctness: a stale cached verdict is worse than a fresh evaluation. Cost: more CPU, more upstream calls, higher p99. | +| **Introspection cache** | **Fail-open** — call the IdP directly | Same logic. Cost: IdP sees N× traffic (one call per replica per request instead of one shared cache hit). | +| **DPoP replay** (`jti` dedup) | **Fail-closed** — reject the request | Security: without the replay store, a stolen DPoP proof can be replayed across replicas. This is the one cache namespace where a Valkey outage causes denials. | +| **Distributed rate limit** (K-DOS-1, v1.1+) | **Configurable** — `failOpen: true` (default) or `failOpen: false` | Default: fall back to per-replica local buckets. Effective limit becomes `N × rps` but no outage. | + +**Bottom line:** a Valkey outage degrades performance (more upstream +calls, higher latency) and, if DPoP is enabled, causes hard denials +on replayed proofs. It does **not** cause incorrect allow/deny +verdicts for non-DPoP traffic. + +## 1. Baseline — capture steady-state metrics + +Before killing Valkey, snapshot the metrics you will compare against: + +```bash +# Cache hit rates +kubectl -n lwauth-system exec deploy/lwauth -c lwauth -- \ + curl -s localhost:8080/metrics | \ + grep -E 'lwauth_cache_hits_total|lwauth_upstream_requests_total' \ + > baseline-metrics.txt + +# Latency +kubectl -n lwauth-system exec deploy/lwauth -c lwauth -- \ + curl -s localhost:8080/metrics | \ + grep lwauth_request_duration_seconds > baseline-latency.txt +``` + +```promql +# PromQL: steady-state baselines to compare later +# Decision cache hit ratio +sum(rate(lwauth_cache_hits_total{cache="decision",outcome="hit"}[5m])) +/ +sum(rate(lwauth_cache_hits_total{cache="decision"}[5m])) + +# Introspection upstream call rate +sum(rate(lwauth_upstream_requests_total{upstream="introspect"}[5m])) + +# p99 request latency +histogram_quantile(0.99, + sum(rate(lwauth_request_duration_seconds_bucket[5m])) by (le)) +``` + +Record these numbers. You will compare them during and after the +outage. + +## 2. Kill Valkey + +### Option A: Delete the pod (recommended for drills) + +```bash +# Scale Valkey to zero. This simulates a clean outage. +kubectl -n cache scale deploy/valkey --replicas=0 + +# Note the time for metric correlation. +echo "Valkey down at $(date -u +%H:%M:%SZ)" +``` + +### Option B: Network partition (more realistic) + +```bash +# Apply a NetworkPolicy that blocks lwauth → Valkey traffic. +cat <<'EOF' | kubectl apply -f - +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: drill-block-valkey + namespace: lwauth-system +spec: + podSelector: + matchLabels: + app.kubernetes.io/name: lwauth + policyTypes: [Egress] + egress: + - to: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: cache + ports: [] # empty = block all ports to cache namespace + - to: + - namespaceSelector: {} + ports: + - port: 443 + - port: 53 + protocol: UDP +EOF +``` + +This keeps Valkey running but makes it unreachable from lwauth, +simulating a network partition. Lwauth's Valkey client will hit +`dialTimeout` (default 500ms) on every operation. + +### Option C: Valkey PAUSE (latency injection) + +```bash +# Pause Valkey for 30 seconds at a time. Simulates a GC pause +# or disk I/O stall. +kubectl -n cache exec deploy/valkey -- valkey-cli DEBUG SLEEP 30 +``` + +## 3. Observe the impact + +With Valkey down and traffic flowing, watch the metrics change in +real time: + +```bash +# Cache metrics should show errors instead of hits +kubectl -n lwauth-system exec deploy/lwauth -c lwauth -- \ + curl -s localhost:8080/metrics | \ + grep -E 'lwauth_cache_hits_total|lwauth_cache_errors_total' +``` + +### Expected behaviour + +| Metric | Expected change | +|---|---| +| `lwauth_cache_hits_total{outcome="hit"}` | Flatlines (no new hits) | +| `lwauth_cache_hits_total{outcome="miss"}` | Spikes (every request misses) | +| `lwauth_cache_errors_total` | Starts counting (Valkey connection errors) | +| `lwauth_upstream_requests_total{upstream="introspect"}` | Spikes (every introspection goes to IdP) | +| `lwauth_request_duration_seconds` (p99) | Increases (no cache shortcut) | +| `lwauth_decisions_total{outcome="deny",reason="dpop_replay"}` | If DPoP enabled: may see false-positive denials | +| `lwauth_ratelimit_backend_errors_total` | If distributed rate limit enabled: counts backend failures | + +### What to verify + +```bash +# 1. Non-DPoP traffic still gets correct verdicts. +# A valid JWT should still get 200: +curl -s -o /dev/null -w '%{http_code}' \ + -H "Authorization: Bearer $VALID_TOKEN" \ + https://api.example.com/v1/orders +# expect: 200 + +# 2. An invalid token still gets 401: +curl -s -o /dev/null -w '%{http_code}' \ + -H "Authorization: Bearer invalid" \ + https://api.example.com/v1/orders +# expect: 401 + +# 3. If DPoP is enabled, check for replay denials: +kubectl -n lwauth-system logs deploy/lwauth -c lwauth | \ + grep -c "dpop.*replay" | tail -1 +``` + +## 4. Restore Valkey + +```bash +# Option A: Scale back up +kubectl -n cache scale deploy/valkey --replicas=1 + +# Option B: Remove the NetworkPolicy +kubectl -n lwauth-system delete networkpolicy drill-block-valkey + +# Option C: Valkey comes back automatically after DEBUG SLEEP +``` + +## 5. Observe recovery + +After Valkey returns, lwauth's cache client reconnects automatically +(the Valkey client retries with backoff). Watch the metrics converge +back to baseline: + +```bash +# Cache hits should resume +kubectl -n lwauth-system exec deploy/lwauth -c lwauth -- \ + curl -s localhost:8080/metrics | \ + grep lwauth_cache_hits_total + +# Upstream call rate should drop back to baseline +kubectl -n lwauth-system exec deploy/lwauth -c lwauth -- \ + curl -s localhost:8080/metrics | \ + grep lwauth_upstream_requests_total +``` + +```promql +# PromQL: recovery — cache hit ratio climbing back to steady state +sum(rate(lwauth_cache_hits_total{cache="decision",outcome="hit"}[1m])) +/ +sum(rate(lwauth_cache_hits_total{cache="decision"}[1m])) +``` + +### Recovery timeline + +| Phase | Duration | What happens | +|---|---|---| +| Reconnect | 0–5s | Valkey client detects connection is live, re-establishes pool | +| Cache warming | 5s–2min | L1 (in-process) fills from L2 (Valkey) on cache misses | +| Steady state | ~2min | Hit ratios return to baseline | + +## 6. Write up the drill + +Document the results for your incident runbook: + +```markdown +## Valkey outage drill — [DATE] + +**Duration:** [start] – [end] ([N] minutes) +**Method:** [pod delete / netpol / debug sleep] + +### Impact observed +- Decision cache: hit ratio dropped from [X]% to 0% +- Introspection upstream calls: increased [N]× from baseline +- p99 latency: increased from [X]ms to [Y]ms +- DPoP replay denials: [N] (expected / not expected) +- Rate limiter fallback: [local buckets activated / N/A] + +### Correctness +- Valid requests: ✅ continued to receive 200 +- Invalid requests: ✅ continued to receive 401 +- False positives: [none / N DPoP replay denials] + +### Recovery +- Time to reconnect: [N]s +- Time to steady-state cache ratio: [N]s + +### Action items +- [ ] [any tuning changes, e.g. adjust dialTimeout, add Valkey Sentinel] +``` + +## Hardening recommendations + +Based on typical drill findings: + +1. **Deploy Valkey with Sentinel or Cluster mode** for automatic + failover. A single Valkey pod is a single point of degradation. + +2. **Set tight timeouts** to fail fast rather than blocking: + ```yaml + cache: + backend: valkey + dialTimeout: 500ms + readTimeout: 150ms + writeTimeout: 150ms + ``` + +3. **Disable DPoP if you cannot tolerate Valkey-dependent denials.** + DPoP is the one namespace with fail-closed semantics. If your + deployment cannot accept DPoP replay denials during a cache + outage, reconsider whether DPoP is the right fit — or deploy + Valkey with HA. + +4. **Monitor `lwauth_cache_errors_total`.** Alert on a sustained + non-zero rate. A single transient error is normal; a sustained + stream means Valkey is unhealthy. + +5. **Consider `serveStaleOnUpstreamError`** (Tier E — ENT-CACHE-2). + When available, this lets lwauth serve stale cached values during + upstream (IdP, OPA) failures. It does not help with Valkey + failures directly, but it reduces the cascade when both Valkey + and an upstream are down simultaneously. + +## What to look at next + +- [Cache invalidation](cache-invalidation.md) — manually clearing + cache entries. +- [`cache-valkey` reference](../modules/cache-valkey.md) — timeouts, + key prefix, pool sizing. +- [DESIGN.md §11.3](../DESIGN.md) — two-tier cache, + stale-while-revalidate, cross-replica singleflight. +- [`ratelimit`](../modules/ratelimit.md) — distributed rate limiter + fallback behaviour. + +## References + +- [`cache-valkey`](../modules/cache-valkey.md) — backend config. +- [`observability`](../modules/observability.md) — cache and upstream + metrics. +- [DESIGN.md §11.3](../DESIGN.md) — caching design. +- Roadmap: E1 (ENT-CACHE-1), E3 (ENT-CACHE-2), E4 (ENT-CACHE-3). diff --git a/mkdocs.yml b/mkdocs.yml index 37e8dc1..e5a2a1d 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -117,6 +117,10 @@ nav: - OAuth2 Authorization Code + PKCE: cookbook/oauth2-pkce.md - OpenFGA on existing Envoy: cookbook/openfga-on-envoy.md - Rotate HMAC secrets without downtime: cookbook/rotate-hmac.md + - Rotate JWKS / IdP signing keys: cookbook/rotate-jwks.md + - Policy shadow mode and canary evaluation: cookbook/policy-shadow-mode.md + - Cache invalidation: cookbook/cache-invalidation.md + - Valkey outage drill: cookbook/valkey-outage-drill.md - Modules: - modules/README.md - Identifiers: From 4fdd8ac1d3bb2a2ceb78fd1e9475f80bf1c0d041 Mon Sep 17 00:00:00 2001 From: mikeappsec Date: Fri, 1 May 2026 19:32:43 +1000 Subject: [PATCH 2/7] feat(admin): implement admin-plane auth middleware (C3 OPS-ADMIN-1) Add internal/admin package with: - Middleware supporting mTLS and/or admin JWT authentication - RBAC verb model: read_status, push_config, invalidate_cache, revoke_token, read_audit - Role-to-verb mapping (configurable roles with named verb sets) - Wildcard verb (*) for break-glass automation - Context propagation of admin Identity for downstream handlers Admin endpoint stubs mounted at /v1/admin/: - GET /v1/admin/status (read_status) - POST /v1/admin/cache/invalidate (invalidate_cache) - POST /v1/admin/revoke (revoke_token, stub for E2) - GET /v1/admin/audit (read_audit, stub for D4) Wired into pkg/lwauthd via Options.Admin; endpoints are mounted on the existing HTTP listener behind the admin middleware. When Admin.Enabled is false (default), /v1/admin/* returns 404. Includes 7 unit tests covering disabled mode, no-credential 401, mTLS success/forbidden/unmapped, and wildcard/specific verb checks. Operator docs: docs/operations/admin-auth.md with config reference, Helm wiring, and endpoint usage examples. --- docs/operations/admin-auth.md | 190 ++++++++++++++++ internal/admin/admin.go | 407 ++++++++++++++++++++++++++++++++++ internal/admin/admin_test.go | 194 ++++++++++++++++ internal/admin/handler.go | 131 +++++++++++ mkdocs.yml | 1 + pkg/lwauthd/lwauthd.go | 28 ++- 6 files changed, 950 insertions(+), 1 deletion(-) create mode 100644 docs/operations/admin-auth.md create mode 100644 internal/admin/admin.go create mode 100644 internal/admin/admin_test.go create mode 100644 internal/admin/handler.go diff --git a/docs/operations/admin-auth.md b/docs/operations/admin-auth.md new file mode 100644 index 0000000..8a73e52 --- /dev/null +++ b/docs/operations/admin-auth.md @@ -0,0 +1,190 @@ +# Admin-plane authentication and authorization + +lwauth's admin plane protects operator endpoints — +`/v1/admin/status`, `/v1/admin/cache/invalidate`, +`/v1/admin/revoke`, `/v1/admin/audit` — with a single auth model so +every future admin feature inherits the same trust boundary. + +## Authentication + +Two mechanisms are supported (composed as OR — the first to succeed +wins): + +| Method | How it works | +|--------|-------------| +| **Admin JWT** | A Bearer token in the `Authorization` header, verified against a dedicated JWKS endpoint with its own issuer and audience. Distinct from data-plane JWTs. | +| **mTLS** | The HTTP client presents a TLS client certificate. The middleware maps the certificate's Subject CN (or SAN DNS name) to an admin role. | + +You can enable both simultaneously; the middleware tries JWT first, +then falls back to mTLS. + +## Authorization (RBAC verbs) + +After authentication, the middleware checks that the admin identity +holds the **verb** required by the endpoint. Verbs are coarse-grained +and endpoint-specific: + +| Verb | Grants access to | +|------|-----------------| +| `read_status` | `GET /v1/admin/status` | +| `push_config` | (future) config promotion endpoints | +| `invalidate_cache` | `POST /v1/admin/cache/invalidate` | +| `revoke_token` | `POST /v1/admin/revoke` | +| `read_audit` | `GET /v1/admin/audit` | + +Verbs are assigned via **roles**. A role is a named set of verbs. +Admin identities receive roles (via JWT claims or mTLS subject +mapping), and the middleware resolves roles to verbs. + +## Configuration + +Admin auth is configured via the `admin:` block in the lwauth config +or via `Options.Admin` when embedding: + +```yaml +# config.yaml (file mode) or values.yaml inline +admin: + enabled: true + + jwt: + issuerUrl: https://idp.internal/admin + audience: lwauth-admin + jwksUrl: https://idp.internal/admin/.well-known/jwks.json + rolesClaim: roles # default; the JWT claim containing role(s) + + mtls: + subjectMapping: + # Certificate CN → role name + admin-bot.lwauth-system.svc: superadmin + sre-team-cert: operator + + roles: + superadmin: + - read_status + - push_config + - invalidate_cache + - revoke_token + - read_audit + operator: + - read_status + - invalidate_cache + - revoke_token + readonly: + - read_status + - read_audit +``` + +### JWT configuration + +| Field | Required | Description | +|-------|----------|-------------| +| `issuerUrl` | Yes | Expected `iss` claim value | +| `audience` | Yes | Expected `aud` claim value | +| `jwksUrl` | Yes | URL to fetch admin signing keys | +| `rolesClaim` | No | Claim name containing role(s). Default: `roles`. Value may be a string or `[]string`. | + +### mTLS configuration + +| Field | Required | Description | +|-------|----------|-------------| +| `subjectMapping` | Yes | Map of certificate CN (or SAN DNS) → role name | + +The TLS listener itself must already require client certificates +(via `--tls-client-ca`). The admin middleware does not configure TLS; +it reads the verified peer certificate from the request's +`tls.ConnectionState`. + +## Helm wiring + +```yaml +# values.yaml +config: + inline: | + admin: + enabled: true + jwt: + issuerUrl: https://idp.internal/admin + audience: lwauth-admin + jwksUrl: https://idp.internal/admin/.well-known/jwks.json + mtls: + subjectMapping: + sre-cert.cluster.local: operator + roles: + operator: [read_status, invalidate_cache, revoke_token] +``` + +For a **dedicated admin listener** (separate port, separate mTLS CA): +this is not yet implemented but planned. Today admin endpoints share +the main HTTP listener. Operators who want network isolation should +use NetworkPolicy to restrict access to the lwauth pods' HTTP port +from admin sources only. + +## CLI usage + +`lwauthctl` does not yet talk to the admin API (that's C2 — GitOps +commands). When C2 ships, commands like `lwauthctl cache invalidate` +will authenticate to `/v1/admin/cache/invalidate` using a kubeconfig +token or a service account JWT. + +## Endpoints + +### `GET /v1/admin/status` + +Returns engine and config status. Requires `read_status`. + +```bash +curl -s --cert admin.pem --key admin-key.pem \ + https://lwauth.internal:8080/v1/admin/status +# {"status":"ok","admin":"sre-cert.cluster.local"} +``` + +### `POST /v1/admin/cache/invalidate` + +Invalidates cached entries. Requires `invalidate_cache`. + +```bash +curl -s -X POST --cert admin.pem --key admin-key.pem \ + -H 'Content-Type: application/json' \ + -d '{"scope":"tenant","tenant":"payments"}' \ + https://lwauth.internal:8080/v1/admin/cache/invalidate +# {"accepted":true,"scope":"tenant","tenant":"payments","subject":""} +``` + +### `POST /v1/admin/revoke` + +Revokes a token or session. Requires `revoke_token`. +(Full implementation in Tier E2 — M14-REVOCATION.) + +```bash +curl -s -X POST --cert admin.pem --key admin-key.pem \ + -H 'Content-Type: application/json' \ + -d '{"jti":"abc-123","tenant":"payments"}' \ + https://lwauth.internal:8080/v1/admin/revoke +# {"accepted":true,"note":"revocation store not yet implemented (Tier E2)"} +``` + +### `GET /v1/admin/audit` + +Queries audit logs. Requires `read_audit`. +(Full implementation in Tier D4 — ENT-AUDIT-1.) + +## Security considerations + +- **Separate issuer/audience.** Admin JWTs must use a different + issuer and audience than data-plane tokens. A data-plane JWT must + never grant admin access. +- **Short-lived tokens.** Admin JWTs should have short expiry (5–15 + min). Use refresh tokens or re-auth for longer sessions. +- **Wildcard verb.** The special verb `*` grants all permissions. + Use sparingly — only for break-glass automation. +- **Audit all admin actions.** Every admin request is logged at INFO + level with the authenticated subject, verb, and path. + +## References + +- [DESIGN.md §7 Tier C](../DESIGN.md) — C3 (OPS-ADMIN-1) roadmap + item. +- [`cache-invalidation` cookbook](../cookbook/cache-invalidation.md) — + operational cache invalidation guide. +- [TLS configuration](../DEPLOYMENT.md) — `--tls-client-ca` for + mTLS on the HTTP listener. diff --git a/internal/admin/admin.go b/internal/admin/admin.go new file mode 100644 index 0000000..91a4558 --- /dev/null +++ b/internal/admin/admin.go @@ -0,0 +1,407 @@ +// Package admin implements the admin-plane authentication and +// authorization model for lwauth operator endpoints. +// +// The admin plane protects endpoints like /v1/admin/cache/invalidate, +// /v1/admin/revoke, /v1/admin/status, and future operator-facing APIs. +// Every admin endpoint shares a single trust model defined here, so +// individual features (revocation, cache flush, audit export) do not +// invent their own auth boundaries. +// +// # Authentication +// +// Two mechanisms are supported (configured independently, composed as OR): +// +// - mTLS: the admin listener requires client certificates; the +// middleware extracts the Subject CN or SAN and maps it to an +// admin identity. +// - JWT: a signed admin JWT (distinct issuer/audience from data-plane +// tokens) is presented in the Authorization header. The middleware +// verifies signature, exp, iss, aud, and extracts claims. +// +// # Authorization +// +// Admin identities carry a set of RBAC verbs. The middleware checks +// that the authenticated identity holds the verb required by the +// endpoint being accessed. Verbs are coarse-grained: +// +// - read_status +// - push_config +// - invalidate_cache +// - revoke_token +// - read_audit +// +// # Usage +// +// cfg := admin.Config{...} +// mw, err := admin.NewMiddleware(cfg) +// mux.Handle("/v1/admin/", mw.Require("invalidate_cache", handler)) +package admin + +import ( + "context" + "crypto/x509" + "encoding/json" + "errors" + "fmt" + "log/slog" + "net/http" + "strings" + "sync" + "time" + + "github.com/lestrrat-go/jwx/v2/jwk" + "github.com/lestrrat-go/jwx/v2/jws" + "github.com/lestrrat-go/jwx/v2/jwt" +) + +// Verb is a coarse-grained RBAC permission for admin operations. +type Verb string + +const ( + VerbReadStatus Verb = "read_status" + VerbPushConfig Verb = "push_config" + VerbInvalidateCache Verb = "invalidate_cache" + VerbRevokeToken Verb = "revoke_token" + VerbReadAudit Verb = "read_audit" +) + +// AllVerbs is the complete set of admin RBAC verbs. +var AllVerbs = []Verb{ + VerbReadStatus, + VerbPushConfig, + VerbInvalidateCache, + VerbRevokeToken, + VerbReadAudit, +} + +// Identity represents an authenticated admin caller. +type Identity struct { + // Subject is the admin's identity (CN from mTLS, sub from JWT). + Subject string + // Verbs is the set of RBAC permissions this admin holds. + Verbs []Verb + // Source describes how the identity was established ("mtls" or "jwt"). + Source string +} + +// HasVerb reports whether the identity holds the given verb. +func (id *Identity) HasVerb(v Verb) bool { + for _, have := range id.Verbs { + if have == v || have == "*" { + return true + } + } + return false +} + +// Config configures the admin authentication middleware. +type Config struct { + // Enabled controls whether admin endpoints are registered. + // When false, the admin mux returns 404 for all /v1/admin/ paths. + Enabled bool `json:"enabled" yaml:"enabled"` + + // JWT configures admin JWT authentication. + JWT *JWTConfig `json:"jwt,omitempty" yaml:"jwt,omitempty"` + + // MTLS configures admin mTLS authentication. + MTLS *MTLSConfig `json:"mtls,omitempty" yaml:"mtls,omitempty"` + + // Roles maps role names to sets of verbs. Admin identities are + // assigned roles (via JWT claims or mTLS subject mapping), and + // the middleware resolves roles to verbs. + Roles map[string][]Verb `json:"roles,omitempty" yaml:"roles,omitempty"` + + // Logger for admin auth decisions. + Logger *slog.Logger `json:"-" yaml:"-"` +} + +// JWTConfig configures JWT-based admin authentication. +type JWTConfig struct { + // IssuerURL is the expected `iss` claim. + IssuerURL string `json:"issuerUrl" yaml:"issuerUrl"` + // Audience is the expected `aud` claim. + Audience string `json:"audience" yaml:"audience"` + // JWKSURL is the URL to fetch signing keys from. + JWKSURL string `json:"jwksUrl" yaml:"jwksUrl"` + // RolesClaim is the JWT claim containing the admin's role(s). + // Defaults to "roles". The claim value may be a string or []string. + RolesClaim string `json:"rolesClaim,omitempty" yaml:"rolesClaim,omitempty"` +} + +// MTLSConfig configures mTLS-based admin authentication. +type MTLSConfig struct { + // SubjectMapping maps certificate Subject CN (or SAN DNS) to a + // role name. If a connecting client's cert CN matches a key here, + // they receive the mapped role. + SubjectMapping map[string]string `json:"subjectMapping" yaml:"subjectMapping"` +} + +// Middleware is the admin auth middleware. +type Middleware struct { + cfg Config + jwkSet jwk.Set + mu sync.RWMutex + log *slog.Logger +} + +// NewMiddleware creates an admin auth middleware from the given config. +func NewMiddleware(cfg Config) (*Middleware, error) { + if !cfg.Enabled { + return &Middleware{cfg: cfg}, nil + } + if cfg.JWT == nil && cfg.MTLS == nil { + return nil, errors.New("admin: at least one of jwt or mtls must be configured") + } + log := cfg.Logger + if log == nil { + log = slog.Default() + } + m := &Middleware{cfg: cfg, log: log} + if cfg.JWT != nil { + if cfg.JWT.JWKSURL == "" { + return nil, errors.New("admin: jwt.jwksUrl is required") + } + if cfg.JWT.IssuerURL == "" { + return nil, errors.New("admin: jwt.issuerUrl is required") + } + if cfg.JWT.Audience == "" { + return nil, errors.New("admin: jwt.audience is required") + } + if cfg.JWT.RolesClaim == "" { + cfg.JWT.RolesClaim = "roles" + } + // Fetch JWKS eagerly to fail fast on misconfiguration. + set, err := jwk.Fetch(context.Background(), cfg.JWT.JWKSURL) + if err != nil { + return nil, fmt.Errorf("admin: fetch jwks: %w", err) + } + m.jwkSet = set + // Background refresh. + go m.refreshJWKS() + } + return m, nil +} + +// refreshJWKS periodically re-fetches the admin JWKS. +func (m *Middleware) refreshJWKS() { + ticker := time.NewTicker(10 * time.Minute) + defer ticker.Stop() + for range ticker.C { + set, err := jwk.Fetch(context.Background(), m.cfg.JWT.JWKSURL) + if err != nil { + m.log.Warn("admin: jwks refresh failed", "err", err) + continue + } + m.mu.Lock() + m.jwkSet = set + m.mu.Unlock() + } +} + +// Require returns an http.Handler that authenticates the caller, +// checks they hold the given verb, and then calls the inner handler. +// On auth failure it returns 401 or 403 with a JSON error body. +func (m *Middleware) Require(verb Verb, next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !m.cfg.Enabled { + writeAdminError(w, http.StatusNotFound, "admin endpoints disabled") + return + } + id, err := m.authenticate(r) + if err != nil { + m.log.Warn("admin: auth failed", + "err", err, + "remote", r.RemoteAddr, + "path", r.URL.Path, + ) + writeAdminError(w, http.StatusUnauthorized, "authentication required") + return + } + if !id.HasVerb(verb) { + m.log.Warn("admin: forbidden", + "subject", id.Subject, + "verb", verb, + "has", id.Verbs, + "path", r.URL.Path, + ) + writeAdminError(w, http.StatusForbidden, fmt.Sprintf("verb %q not granted", verb)) + return + } + // Attach identity to context for downstream handlers. + ctx := context.WithValue(r.Context(), adminIdentityKey{}, id) + m.log.Info("admin: authorized", + "subject", id.Subject, + "verb", verb, + "source", id.Source, + "path", r.URL.Path, + ) + next.ServeHTTP(w, r.WithContext(ctx)) + }) +} + +// authenticate tries JWT first, then mTLS. Returns the first success. +func (m *Middleware) authenticate(r *http.Request) (*Identity, error) { + // Try JWT from Authorization header. + if m.cfg.JWT != nil { + if id, err := m.authenticateJWT(r); err == nil { + return id, nil + } + } + // Try mTLS from TLS peer certificate. + if m.cfg.MTLS != nil { + if id, err := m.authenticateMTLS(r); err == nil { + return id, nil + } + } + return nil, errors.New("no valid credential") +} + +// authenticateJWT verifies the Bearer token as an admin JWT. +func (m *Middleware) authenticateJWT(r *http.Request) (*Identity, error) { + auth := r.Header.Get("Authorization") + if auth == "" { + return nil, errors.New("no authorization header") + } + parts := strings.SplitN(auth, " ", 2) + if len(parts) != 2 || !strings.EqualFold(parts[0], "Bearer") { + return nil, errors.New("invalid authorization scheme") + } + tokenStr := parts[1] + + m.mu.RLock() + set := m.jwkSet + m.mu.RUnlock() + + tok, err := jwt.Parse([]byte(tokenStr), + jwt.WithKeySet(set, jws.WithInferAlgorithmFromKey(true)), + jwt.WithIssuer(m.cfg.JWT.IssuerURL), + jwt.WithAudience(m.cfg.JWT.Audience), + jwt.WithValidate(true), + ) + if err != nil { + return nil, fmt.Errorf("jwt verify: %w", err) + } + + sub := tok.Subject() + roles := extractRoles(tok, m.cfg.JWT.RolesClaim) + verbs := m.resolveVerbs(roles) + + return &Identity{ + Subject: sub, + Verbs: verbs, + Source: "jwt", + }, nil +} + +// authenticateMTLS extracts identity from the TLS peer certificate. +func (m *Middleware) authenticateMTLS(r *http.Request) (*Identity, error) { + if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { + return nil, errors.New("no client certificate") + } + cert := r.TLS.PeerCertificates[0] + cn := cert.Subject.CommonName + + role, ok := m.cfg.MTLS.SubjectMapping[cn] + if !ok { + // Try SAN DNS names. + for _, dns := range cert.DNSNames { + if r, found := m.cfg.MTLS.SubjectMapping[dns]; found { + role = r + ok = true + break + } + } + } + if !ok { + return nil, fmt.Errorf("unmapped subject: %s", cn) + } + + verbs := m.resolveVerbs([]string{role}) + return &Identity{ + Subject: cn, + Verbs: verbs, + Source: "mtls", + }, nil +} + +// resolveVerbs maps role names to the union of their verbs. +func (m *Middleware) resolveVerbs(roles []string) []Verb { + seen := map[Verb]bool{} + var result []Verb + for _, role := range roles { + for _, v := range m.cfg.Roles[role] { + if !seen[v] { + seen[v] = true + result = append(result, v) + } + } + } + return result +} + +// extractRoles reads the roles claim from a JWT. Handles both string +// and []string shapes. +func extractRoles(tok jwt.Token, claim string) []string { + v, ok := tok.Get(claim) + if !ok { + return nil + } + switch val := v.(type) { + case string: + return []string{val} + case []any: + var roles []string + for _, item := range val { + if s, ok := item.(string); ok { + roles = append(roles, s) + } + } + return roles + case []string: + return val + default: + return nil + } +} + +// IdentityFromContext retrieves the admin Identity from the request +// context. Returns nil if the request was not authenticated. +func IdentityFromContext(ctx context.Context) *Identity { + id, _ := ctx.Value(adminIdentityKey{}).(*Identity) + return id +} + +type adminIdentityKey struct{} + +// writeAdminError writes a JSON error response. +func writeAdminError(w http.ResponseWriter, status int, msg string) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(map[string]string{"error": msg}) +} + +// VerifyPeerCert is a tls.Config.VerifyPeerCertificate callback that +// can be used on a dedicated admin listener to enforce that client +// certs chain to a given CA pool. This is complementary to Go's +// built-in tls.RequireAndVerifyClientCert — it lets you use a +// *different* CA pool for the admin listener than the data-plane +// listener. +func VerifyPeerCert(pool *x509.CertPool) func(rawCerts [][]byte, _ [][]*x509.Certificate) error { + return func(rawCerts [][]byte, _ [][]*x509.Certificate) error { + if len(rawCerts) == 0 { + return errors.New("admin: no client certificate") + } + cert, err := x509.ParseCertificate(rawCerts[0]) + if err != nil { + return fmt.Errorf("admin: parse cert: %w", err) + } + opts := x509.VerifyOptions{ + Roots: pool, + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, + } + if _, err := cert.Verify(opts); err != nil { + return fmt.Errorf("admin: verify cert: %w", err) + } + return nil + } +} diff --git a/internal/admin/admin_test.go b/internal/admin/admin_test.go new file mode 100644 index 0000000..5faaedd --- /dev/null +++ b/internal/admin/admin_test.go @@ -0,0 +1,194 @@ +package admin + +import ( + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/json" + "log/slog" + "net/http" + "net/http/httptest" + "testing" +) + +func TestMiddleware_Disabled(t *testing.T) { + mw, err := NewMiddleware(Config{Enabled: false}) + if err != nil { + t.Fatal(err) + } + inner := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(200) + }) + handler := mw.Require(VerbReadStatus, inner) + + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/v1/admin/status", nil) + handler.ServeHTTP(w, r) + + if w.Code != http.StatusNotFound { + t.Errorf("expected 404, got %d", w.Code) + } +} + +func TestMiddleware_NoCredential(t *testing.T) { + mw := &Middleware{ + cfg: Config{ + Enabled: true, + MTLS: &MTLSConfig{ + SubjectMapping: map[string]string{"admin-bot": "admin"}, + }, + Roles: map[string][]Verb{"admin": {VerbReadStatus}}, + }, + log: testLogger(t), + } + inner := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(200) + }) + handler := mw.Require(VerbReadStatus, inner) + + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/v1/admin/status", nil) + handler.ServeHTTP(w, r) + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401, got %d", w.Code) + } +} + +func TestMiddleware_MTLS_Success(t *testing.T) { + mw := &Middleware{ + cfg: Config{ + Enabled: true, + MTLS: &MTLSConfig{ + SubjectMapping: map[string]string{"admin-bot": "operator"}, + }, + Roles: map[string][]Verb{ + "operator": {VerbReadStatus, VerbInvalidateCache}, + }, + }, + log: testLogger(t), + } + inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + id := IdentityFromContext(r.Context()) + if id == nil { + t.Fatal("expected identity in context") + } + if id.Subject != "admin-bot" { + t.Errorf("expected subject admin-bot, got %s", id.Subject) + } + w.WriteHeader(200) + }) + handler := mw.Require(VerbReadStatus, inner) + + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/v1/admin/status", nil) + r.TLS = &tls.ConnectionState{ + PeerCertificates: []*x509.Certificate{ + peerCert("admin-bot"), + }, + } + handler.ServeHTTP(w, r) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } +} + +func TestMiddleware_MTLS_Forbidden(t *testing.T) { + mw := &Middleware{ + cfg: Config{ + Enabled: true, + MTLS: &MTLSConfig{ + SubjectMapping: map[string]string{"admin-bot": "readonly"}, + }, + Roles: map[string][]Verb{ + "readonly": {VerbReadStatus}, + }, + }, + log: testLogger(t), + } + inner := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(200) + }) + // Request invalidate_cache but identity only has read_status. + handler := mw.Require(VerbInvalidateCache, inner) + + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/v1/admin/cache/invalidate", nil) + r.TLS = &tls.ConnectionState{ + PeerCertificates: []*x509.Certificate{ + peerCert("admin-bot"), + }, + } + handler.ServeHTTP(w, r) + + if w.Code != http.StatusForbidden { + t.Errorf("expected 403, got %d", w.Code) + } + var body map[string]string + _ = json.NewDecoder(w.Body).Decode(&body) + if body["error"] == "" { + t.Error("expected error message in response body") + } +} + +func TestMiddleware_MTLS_UnmappedSubject(t *testing.T) { + mw := &Middleware{ + cfg: Config{ + Enabled: true, + MTLS: &MTLSConfig{ + SubjectMapping: map[string]string{"admin-bot": "admin"}, + }, + Roles: map[string][]Verb{"admin": {VerbReadStatus}}, + }, + log: testLogger(t), + } + inner := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(200) + }) + handler := mw.Require(VerbReadStatus, inner) + + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/v1/admin/status", nil) + r.TLS = &tls.ConnectionState{ + PeerCertificates: []*x509.Certificate{ + peerCert("unknown-client"), + }, + } + handler.ServeHTTP(w, r) + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected 401, got %d", w.Code) + } +} + +func TestIdentity_HasVerb_Wildcard(t *testing.T) { + id := &Identity{Subject: "superadmin", Verbs: []Verb{"*"}} + for _, v := range AllVerbs { + if !id.HasVerb(v) { + t.Errorf("wildcard identity should have verb %s", v) + } + } +} + +func TestIdentity_HasVerb_Specific(t *testing.T) { + id := &Identity{Subject: "reader", Verbs: []Verb{VerbReadStatus, VerbReadAudit}} + if !id.HasVerb(VerbReadStatus) { + t.Error("expected read_status") + } + if id.HasVerb(VerbInvalidateCache) { + t.Error("should not have invalidate_cache") + } +} + +// --- helpers --------------------------------------------------------------- + +func peerCert(cn string) *x509.Certificate { + return &x509.Certificate{ + Subject: pkix.Name{CommonName: cn}, + } +} + +func testLogger(_ *testing.T) *slog.Logger { + return slog.Default() +} diff --git a/internal/admin/handler.go b/internal/admin/handler.go new file mode 100644 index 0000000..cd4d4b2 --- /dev/null +++ b/internal/admin/handler.go @@ -0,0 +1,131 @@ +package admin + +import ( + "encoding/json" + "net/http" +) + +// NewAdminMux returns an http.Handler that serves all /v1/admin/ endpoints, +// protected by the given middleware. If the middleware is nil or disabled, +// all routes return 404. +func NewAdminMux(mw *Middleware) http.Handler { + mux := http.NewServeMux() + + if mw == nil || !mw.cfg.Enabled { + mux.HandleFunc("/", func(w http.ResponseWriter, _ *http.Request) { + writeAdminError(w, http.StatusNotFound, "admin endpoints disabled") + }) + return mux + } + + // GET /v1/admin/status — engine and config status. + mux.Handle("/v1/admin/status", mw.Require(VerbReadStatus, + http.HandlerFunc(handleStatus))) + + // POST /v1/admin/cache/invalidate — manual cache invalidation. + mux.Handle("/v1/admin/cache/invalidate", mw.Require(VerbInvalidateCache, + http.HandlerFunc(handleCacheInvalidate))) + + // POST /v1/admin/revoke — token/session revocation (stub for E2). + mux.Handle("/v1/admin/revoke", mw.Require(VerbRevokeToken, + http.HandlerFunc(handleRevoke))) + + // GET /v1/admin/audit — audit log query (stub for D4). + mux.Handle("/v1/admin/audit", mw.Require(VerbReadAudit, + http.HandlerFunc(handleAuditQuery))) + + return mux +} + +// handleStatus returns basic engine status. +// Full implementation will include appliedVersion, appliedDigest, +// uptime, replica count, etc. +func handleStatus(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + writeAdminError(w, http.StatusMethodNotAllowed, "GET only") + return + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "status": "ok", + "admin": IdentityFromContext(r.Context()).Subject, + }) +} + +// handleCacheInvalidate accepts a cache invalidation request. +// Body: {"scope": "tenant"|"subject"|"all", "tenant": "...", "subject": "..."} +// Full implementation connects to the cache backend in E1/E3. +func handleCacheInvalidate(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + writeAdminError(w, http.StatusMethodNotAllowed, "POST only") + return + } + var req struct { + Scope string `json:"scope"` // "all", "tenant", "subject" + Tenant string `json:"tenant"` + Subject string `json:"subject"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeAdminError(w, http.StatusBadRequest, "invalid JSON body") + return + } + if req.Scope == "" { + req.Scope = "all" + } + + // TODO(ENT-CACHE-2): wire into cache.Backend tag-based invalidation. + // For now, log and acknowledge. + id := IdentityFromContext(r.Context()) + _ = id // will be used in audit + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "accepted": true, + "scope": req.Scope, + "tenant": req.Tenant, + "subject": req.Subject, + }) +} + +// handleRevoke accepts a token/session revocation request. +// Body: {"token_hash": "...", "jti": "...", "tenant": "...", "subject": "..."} +// Full implementation lands in E2 (M14-REVOCATION). +func handleRevoke(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + writeAdminError(w, http.StatusMethodNotAllowed, "POST only") + return + } + var req struct { + TokenHash string `json:"token_hash"` + JTI string `json:"jti"` + Tenant string `json:"tenant"` + Subject string `json:"subject"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeAdminError(w, http.StatusBadRequest, "invalid JSON body") + return + } + + // TODO(M14-REVOCATION): write to revocation store. + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + _ = json.NewEncoder(w).Encode(map[string]any{ + "accepted": true, + "note": "revocation store not yet implemented (Tier E2)", + }) +} + +// handleAuditQuery serves audit log queries. +// Full implementation lands in D4 (ENT-AUDIT-1). +func handleAuditQuery(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + writeAdminError(w, http.StatusMethodNotAllowed, "GET only") + return + } + + // TODO(ENT-AUDIT-1): query audit sink backend. + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "note": "audit query not yet implemented (Tier D4)", + }) +} diff --git a/mkdocs.yml b/mkdocs.yml index e5a2a1d..3579d02 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -160,6 +160,7 @@ nav: - Operations: - Deployment: DEPLOYMENT.md - Envoy integration: deployment/envoy.md + - Admin-plane auth: operations/admin-auth.md - FIPS 140-3 build: operations/fips.md - Request invariants: testing/request-invariants.md - Security: diff --git a/pkg/lwauthd/lwauthd.go b/pkg/lwauthd/lwauthd.go index 9dd8e64..ebd1339 100644 --- a/pkg/lwauthd/lwauthd.go +++ b/pkg/lwauthd/lwauthd.go @@ -36,6 +36,7 @@ import ( "google.golang.org/grpc/reflection" authv1 "github.com/mikeappsec/lightweightauth/api/proto/lightweightauth/v1" + "github.com/mikeappsec/lightweightauth/internal/admin" "github.com/mikeappsec/lightweightauth/internal/config" "github.com/mikeappsec/lightweightauth/internal/pipeline" "github.com/mikeappsec/lightweightauth/internal/server" @@ -89,6 +90,12 @@ type Options struct { // usually disable both. DOC-OPENAPI-1. DisableHTTPOpenAPI bool + // Admin configures the admin-plane authentication and + // authorization model (OPS-ADMIN-1). When Admin.Enabled is true, + // endpoints under /v1/admin/ are registered on the HTTP listener, + // protected by mTLS and/or admin JWT with RBAC verbs. + Admin admin.Config + // MaxRequestBytes caps inbound /v1/authorize bodies. 0 -> 1 MiB. MaxRequestBytes int64 @@ -234,9 +241,28 @@ func Run(opts Options) error { DisableMetrics: opts.DisableHTTPMetrics, DisableOpenAPI: opts.DisableHTTPOpenAPI, }) + + // Admin endpoints (OPS-ADMIN-1). When enabled, /v1/admin/* routes + // are mounted on the same HTTP listener, guarded by the admin + // middleware (mTLS or admin JWT + RBAC verbs). + var finalHandler http.Handler = httpHandler + if opts.Admin.Enabled { + opts.Admin.Logger = log + adminMW, err := admin.NewMiddleware(opts.Admin) + if err != nil { + return fmt.Errorf("admin middleware: %w", err) + } + adminMux := admin.NewAdminMux(adminMW) + // Compose: requests starting with /v1/admin/ go to the admin + // mux; everything else goes to the normal handler. + combined := http.NewServeMux() + combined.Handle("/v1/admin/", adminMux) + combined.Handle("/", httpHandler) + finalHandler = combined + } httpSrv := &http.Server{ Addr: opts.HTTPAddr, - Handler: httpHandler, + Handler: finalHandler, ReadHeaderTimeout: nonZeroDur(opts.HTTPReadHeaderTimeout, 10*time.Second), ReadTimeout: nonZeroDur(opts.HTTPReadTimeout, 30*time.Second), WriteTimeout: nonZeroDur(opts.HTTPWriteTimeout, 30*time.Second), From d05b4d762b66bd78c5a9fc2ace2e64415bf29499 Mon Sep 17 00:00:00 2001 From: mikeappsec Date: Fri, 1 May 2026 19:40:42 +1000 Subject: [PATCH 3/7] feat(gitops): add promote/rollback/drift commands (C2 OPS-GITOPS-1) lwauthctl gains three new subcommands for GitOps workflows: - promote: validate config, stamp spec.version (explicit or auto- generated timestamp), compute spec digest, emit canonical JSON for Git commit / kubectl apply. - rollback: rewrite spec.version to a previous value, re-validate, emit YAML. - drift: compare local config (version + SHA-256 digest) against live AuthConfig status.appliedVersion / status.appliedDigest via kubectl. Exit 1 on drift for CI gating. Supporting changes: - config.AuthConfig gains spec.version field (opaque operator tag). - AuthConfigStatus gains appliedVersion and appliedDigest fields. - Controller setReady() populates both on successful compile+swap. - specDigest() computes sha256 of canonical JSON spec encoding. Operator docs: docs/operations/gitops.md with CI examples, drift check scheduling, and promote/rollback workflows. --- api/crd/v1alpha1/types.go | 11 ++ cmd/lwauthctl/gitops.go | 232 ++++++++++++++++++++++++++++++ cmd/lwauthctl/main.go | 11 +- docs/operations/gitops.md | 137 ++++++++++++++++++ internal/config/config.go | 6 + internal/controller/authconfig.go | 20 +++ mkdocs.yml | 1 + 7 files changed, 417 insertions(+), 1 deletion(-) create mode 100644 cmd/lwauthctl/gitops.go create mode 100644 docs/operations/gitops.md diff --git a/api/crd/v1alpha1/types.go b/api/crd/v1alpha1/types.go index 6febbd1..15f035d 100644 --- a/api/crd/v1alpha1/types.go +++ b/api/crd/v1alpha1/types.go @@ -86,6 +86,17 @@ type AuthConfigStatus struct { Ready bool `json:"ready,omitempty"` ObservedGeneration int64 `json:"observedGeneration,omitempty"` Message string `json:"message,omitempty"` + + // AppliedVersion is the spec.version that was last successfully + // compiled and swapped in. Empty if spec.version is unset. + // OPS-GITOPS-1: enables lwauthctl drift to compare live vs desired. + AppliedVersion string `json:"appliedVersion,omitempty"` + + // AppliedDigest is the SHA-256 digest of the canonical JSON + // encoding of the spec at the time of the last successful compile. + // OPS-GITOPS-1: enables lwauthctl drift to detect config drift + // even when spec.version is not used. + AppliedDigest string `json:"appliedDigest,omitempty"` } // ConditionTypeReady is the canonical Ready condition type. Reasons diff --git a/cmd/lwauthctl/gitops.go b/cmd/lwauthctl/gitops.go new file mode 100644 index 0000000..28a0bb7 --- /dev/null +++ b/cmd/lwauthctl/gitops.go @@ -0,0 +1,232 @@ +package main + +import ( + "crypto/sha256" + "encoding/json" + "flag" + "fmt" + "os" + "os/exec" + "strings" + "time" + + "github.com/mikeappsec/lightweightauth/internal/config" +) + +// promote validates the config, optionally stamps spec.version, computes +// the spec digest, and emits the GitOps-ready YAML to stdout (or to a +// file). Designed for CI pipelines that validate-then-push to Git. +// +// Usage: +// +// lwauthctl promote --config authconfig.yaml --version "2026-05-01" +// lwauthctl promote --config authconfig.yaml --auto-version +func promote(args []string) { + fs := flag.NewFlagSet("promote", flag.ExitOnError) + cfgPath := fs.String("config", "", "path to AuthConfig YAML") + version := fs.String("version", "", "explicit version string to stamp into spec.version") + autoVer := fs.Bool("auto-version", false, "auto-generate a version string (date-based)") + outPath := fs.String("out", "", "write promoted YAML to this file (default: stdout)") + _ = fs.Parse(args) + + if *cfgPath == "" { + fmt.Fprintln(os.Stderr, "--config required") + os.Exit(2) + } + ac, err := config.LoadFile(*cfgPath) + if err != nil { + fmt.Fprintln(os.Stderr, "load:", err) + os.Exit(1) + } + if _, err := config.Compile(ac); err != nil { + fmt.Fprintln(os.Stderr, "compile:", err) + os.Exit(1) + } + + // Stamp version. + switch { + case *version != "": + ac.Version = *version + case *autoVer: + ac.Version = time.Now().UTC().Format("2006-01-02T150405Z") + } + + // Compute digest. + specJSON, _ := json.Marshal(ac) + digest := sha256.Sum256(specJSON) + + fmt.Fprintf(os.Stderr, "✓ validated: identifiers=%d authorizers=%d\n", + len(ac.Identifiers), len(ac.Authorizers)) + fmt.Fprintf(os.Stderr, " version: %s\n", ac.Version) + fmt.Fprintf(os.Stderr, " digest: sha256:%x\n", digest) + + // Emit the promoted YAML. + out := os.Stdout + if *outPath != "" { + f, err := os.Create(*outPath) + if err != nil { + fmt.Fprintln(os.Stderr, "create:", err) + os.Exit(1) + } + defer f.Close() + out = f + } + + // We emit JSON because the config is loaded from YAML but we want + // a canonical representation. Operators pipe through yq for YAML. + enc := json.NewEncoder(out) + enc.SetIndent("", " ") + if err := enc.Encode(ac); err != nil { + fmt.Fprintln(os.Stderr, "encode:", err) + os.Exit(1) + } +} + +// rollback rewrites spec.version in the given config to a target version +// and re-validates. This is a local operation — it does not talk to the +// cluster. Pair with `kubectl apply` or a GitOps commit. +// +// Usage: +// +// lwauthctl rollback --config authconfig.yaml --to-version "2026-04-30" +func rollback(args []string) { + fs := flag.NewFlagSet("rollback", flag.ExitOnError) + cfgPath := fs.String("config", "", "path to AuthConfig YAML") + toVersion := fs.String("to-version", "", "version string to roll back to") + outPath := fs.String("out", "", "write result to file (default: stdout)") + _ = fs.Parse(args) + + if *cfgPath == "" || *toVersion == "" { + fmt.Fprintln(os.Stderr, "--config and --to-version required") + os.Exit(2) + } + ac, err := config.LoadFile(*cfgPath) + if err != nil { + fmt.Fprintln(os.Stderr, "load:", err) + os.Exit(1) + } + + oldVersion := ac.Version + ac.Version = *toVersion + + if _, err := config.Compile(ac); err != nil { + fmt.Fprintln(os.Stderr, "compile after rollback:", err) + os.Exit(1) + } + + fmt.Fprintf(os.Stderr, "✓ rollback: %s → %s\n", oldVersion, *toVersion) + + out := os.Stdout + if *outPath != "" { + f, err := os.Create(*outPath) + if err != nil { + fmt.Fprintln(os.Stderr, "create:", err) + os.Exit(1) + } + defer f.Close() + out = f + } + + enc := json.NewEncoder(out) + enc.SetIndent("", " ") + if err := enc.Encode(ac); err != nil { + fmt.Fprintln(os.Stderr, "encode:", err) + os.Exit(1) + } +} + +// drift compares the local config file against the live AuthConfig's +// status in the cluster. It checks: +// - spec.version vs status.appliedVersion +// - computed digest vs status.appliedDigest +// +// Non-zero exit means drift was detected. +// +// Usage: +// +// lwauthctl drift --config authconfig.yaml --namespace payments --name payments +func drift(args []string) { + fs := flag.NewFlagSet("drift", flag.ExitOnError) + cfgPath := fs.String("config", "", "path to desired AuthConfig YAML") + namespace := fs.String("namespace", "default", "Kubernetes namespace") + name := fs.String("name", "", "AuthConfig resource name (default: derived from filename)") + _ = fs.Parse(args) + + if *cfgPath == "" { + fmt.Fprintln(os.Stderr, "--config required") + os.Exit(2) + } + ac, err := config.LoadFile(*cfgPath) + if err != nil { + fmt.Fprintln(os.Stderr, "load:", err) + os.Exit(1) + } + if _, err := config.Compile(ac); err != nil { + fmt.Fprintln(os.Stderr, "compile:", err) + os.Exit(1) + } + + // Compute local digest. + specJSON, _ := json.Marshal(ac) + localDigest := fmt.Sprintf("sha256:%x", sha256.Sum256(specJSON)) + + // Fetch live status from cluster via kubectl. + if *name == "" { + // Derive from filename: "payments.yaml" -> "payments" + base := *cfgPath + if idx := strings.LastIndex(base, "/"); idx >= 0 { + base = base[idx+1:] + } + if idx := strings.LastIndex(base, "\\"); idx >= 0 { + base = base[idx+1:] + } + base = strings.TrimSuffix(base, ".yaml") + base = strings.TrimSuffix(base, ".yml") + *name = base + } + + // kubectl get authconfig -n -o jsonpath + liveVersion, err := kubectlJSONPath(*namespace, *name, "{.status.appliedVersion}") + if err != nil { + fmt.Fprintf(os.Stderr, "⚠ cannot read live status: %v\n", err) + fmt.Fprintln(os.Stderr, " (is the cluster reachable? is the AuthConfig applied?)") + os.Exit(2) + } + liveDigest, _ := kubectlJSONPath(*namespace, *name, "{.status.appliedDigest}") + + drifted := false + + // Compare version. + if ac.Version != "" && liveVersion != ac.Version { + fmt.Printf("DRIFT version: local=%q live=%q\n", ac.Version, liveVersion) + drifted = true + } else if ac.Version != "" { + fmt.Printf("OK version: %q\n", ac.Version) + } + + // Compare digest. + if liveDigest != "" && liveDigest != localDigest { + fmt.Printf("DRIFT digest: local=%s live=%s\n", localDigest, liveDigest) + drifted = true + } else if liveDigest != "" { + fmt.Printf("OK digest: %s\n", localDigest) + } + + if !drifted { + fmt.Println("\n✓ no drift detected") + } else { + fmt.Println("\n✗ drift detected — run `lwauthctl promote` + `kubectl apply` to reconcile") + os.Exit(1) + } +} + +// kubectlJSONPath runs kubectl get authconfig and extracts a jsonpath field. +func kubectlJSONPath(namespace, name, jsonpath string) (string, error) { + cmd := exec.Command("kubectl", "-n", namespace, "get", "authconfig.lightweightauth.io", name, + "-o", fmt.Sprintf("jsonpath=%s", jsonpath)) + out, err := cmd.Output() + if err != nil { + return "", err + } + return strings.TrimSpace(string(out)), nil +} diff --git a/cmd/lwauthctl/main.go b/cmd/lwauthctl/main.go index a25da1c..5092ac4 100644 --- a/cmd/lwauthctl/main.go +++ b/cmd/lwauthctl/main.go @@ -37,19 +37,28 @@ func main() { explain(os.Args[2:]) case "audit": auditTail(os.Args[2:]) + case "promote": + promote(os.Args[2:]) + case "rollback": + rollback(os.Args[2:]) + case "drift": + drift(os.Args[2:]) default: usage() } } func usage() { - fmt.Fprintln(os.Stderr, "usage: lwauthctl [args]") + fmt.Fprintln(os.Stderr, "usage: lwauthctl [args]") fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, " modules list registered identifier/authorizer/mutator types") fmt.Fprintln(os.Stderr, " validate --config FILE compile an AuthConfig YAML offline") fmt.Fprintln(os.Stderr, " diff --from A.yaml --to B.yaml show what would change between two AuthConfigs") fmt.Fprintln(os.Stderr, " explain --config FILE --request req.json dry-run a request through the pipeline") fmt.Fprintln(os.Stderr, " audit [--file F] [--tenant T] ... tail / filter audit JSONL") + fmt.Fprintln(os.Stderr, " promote --config FILE [--version V] validate, tag version, and emit GitOps-ready YAML") + fmt.Fprintln(os.Stderr, " rollback --config FILE --to-version V rewrite spec.version to a previous value") + fmt.Fprintln(os.Stderr, " drift --config FILE --namespace NS compare local config to live AuthConfig status") os.Exit(2) } diff --git a/docs/operations/gitops.md b/docs/operations/gitops.md new file mode 100644 index 0000000..37e1958 --- /dev/null +++ b/docs/operations/gitops.md @@ -0,0 +1,137 @@ +# GitOps workflow — promote, rollback, and drift detection + +lwauth's CRD controller records `status.appliedVersion` and +`status.appliedDigest` on every successful compile+swap. The +`lwauthctl` CLI provides three commands that wrap the existing +`validate` / `diff` workflow into GitOps-friendly operations: + +| Command | What it does | +|---------|-------------| +| `lwauthctl promote` | Validate, tag `spec.version`, compute digest, emit GitOps-ready YAML | +| `lwauthctl rollback` | Rewrite `spec.version` to a previous value, re-validate, emit YAML | +| `lwauthctl drift` | Compare local config against live `status.appliedVersion` / `appliedDigest` | + +## Status fields + +The controller sets these on `AuthConfig.status` after a successful +reconcile: + +| Field | Value | +|-------|-------| +| `appliedVersion` | `spec.version` from the config that was compiled | +| `appliedDigest` | `sha256:` of the canonical JSON encoding of the spec | + +```bash +kubectl -n payments get authconfig payments -o jsonpath='{.status}' +# {"appliedVersion":"2026-05-01","appliedDigest":"sha256:abc123...","ready":true,...} +``` + +## Promote + +Validate the config, optionally stamp a version, and emit a +deployment-ready artifact: + +```bash +# Explicit version: +lwauthctl promote --config authconfig.yaml --version "2026-05-01" + +# Auto-generated timestamp version: +lwauthctl promote --config authconfig.yaml --auto-version + +# Write to file instead of stdout: +lwauthctl promote --config authconfig.yaml --version "v42" --out promoted.json +``` + +Output is canonical JSON. Pipe through `yq -P` for YAML if your +GitOps repo prefers YAML: + +```bash +lwauthctl promote --config authconfig.yaml --auto-version | yq -P > authconfig-promoted.yaml +git add authconfig-promoted.yaml && git commit -m "promote: $(date +%F)" +``` + +### CI integration + +Add to your CI pipeline: + +```yaml +# .github/workflows/promote.yaml +- name: Validate and promote + run: | + lwauthctl promote --config deploy/authconfig.yaml \ + --version "${{ github.sha }}" \ + --out deploy/authconfig-promoted.json + git diff --exit-code deploy/ || (git add deploy/ && git commit -m "auto-promote ${{ github.sha }}") +``` + +## Rollback + +Rewrite `spec.version` to a known-good value and re-validate: + +```bash +lwauthctl rollback --config authconfig.yaml --to-version "2026-04-30" --out rolled-back.json +kubectl apply -f rolled-back.json +``` + +The controller will reconcile, compile the config (which hasn't +changed structurally — only `spec.version` is different), and update +`status.appliedVersion`. + +!!! note + Rollback does not restore a previous *config shape*. It tags the + *current* config with an older version string. To restore a + previous shape, check out the old file from Git and `kubectl apply` + it directly. + +## Drift detection + +Compare local config against the live cluster: + +```bash +lwauthctl drift --config authconfig.yaml --namespace payments --name payments +# OK version: "2026-05-01" +# OK digest: sha256:abc123... +# +# ✓ no drift detected +``` + +If drift is detected, exit code is 1: + +```bash +lwauthctl drift --config authconfig.yaml --namespace payments +# DRIFT version: local="2026-05-02" live="2026-05-01" +# DRIFT digest: local=sha256:def456... live=sha256:abc123... +# +# ✗ drift detected — run `lwauthctl promote` + `kubectl apply` to reconcile +``` + +### CI drift check + +```yaml +# .github/workflows/drift.yaml (scheduled, e.g. every 15 min) +- name: Check for config drift + run: lwauthctl drift --config deploy/authconfig.yaml --namespace payments +``` + +A non-zero exit fails the workflow, alerting the team. + +## How it fits together + +``` + Git repo Kubernetes cluster + ┌────────────┐ ┌──────────────────────┐ + │ authconfig │──promote──► │ AuthConfig CR │ + │ .yaml │ (validate, │ spec.version: v42 │ + │ │ tag, push) │ status: │ + │ │ │ appliedVersion: v42│ + │ │◄──drift───────│ appliedDigest: ... │ + └────────────┘ └──────────────────────┘ +``` + +## References + +- [DESIGN.md §7 Tier C](../DESIGN.md) — C2 (OPS-GITOPS-1) roadmap. +- [Admin-plane auth](admin-auth.md) — admin API for cache/revoke + operations (C3). +- [`validate` / `diff` commands](../QUICKSTART.md) — existing CLI + commands that `promote` wraps. diff --git a/internal/config/config.go b/internal/config/config.go index db5e871..bb075c6 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -12,6 +12,12 @@ import "github.com/mikeappsec/lightweightauth/pkg/ratelimit" // AuthConfig CRD instance maps to exactly one AuthConfig value, which the // Compiler turns into a pipeline.Engine. type AuthConfig struct { + // Version is an opaque string set by operators to tag a policy + // revision. The controller echoes it on status.appliedVersion after + // a successful compile+swap. Optional; when empty, only + // status.appliedDigest tracks config identity. + Version string `json:"version,omitempty" yaml:"version,omitempty"` + // Hosts limits this config to specific virtual hosts. Empty = match all. Hosts []string `json:"hosts,omitempty" yaml:"hosts,omitempty"` diff --git a/internal/controller/authconfig.go b/internal/controller/authconfig.go index 5c82699..52b311f 100644 --- a/internal/controller/authconfig.go +++ b/internal/controller/authconfig.go @@ -11,6 +11,8 @@ package controller import ( "context" + "crypto/sha256" + "encoding/json" "fmt" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -138,6 +140,12 @@ func setReady(ac *v1alpha1.AuthConfig, status metav1.ConditionStatus, reason, me ac.Status.Ready = status == metav1.ConditionTrue ac.Status.ObservedGeneration = ac.Generation ac.Status.Message = message + + // OPS-GITOPS-1: record the applied version and spec digest on success. + if status == metav1.ConditionTrue { + ac.Status.AppliedVersion = ac.Spec.Version + ac.Status.AppliedDigest = specDigest(&ac.Spec) + } } // SetupWithManager registers the reconciler. The watch predicate filters @@ -165,3 +173,15 @@ func (r *AuthConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { func AddToScheme(s *runtime.Scheme) error { return v1alpha1.AddToScheme(s) } + +// specDigest returns the hex-encoded SHA-256 of the canonical JSON +// representation of the AuthConfig spec. Used by OPS-GITOPS-1 to detect +// config drift without relying on an opaque version string. +func specDigest(spec *config.AuthConfig) string { + b, err := json.Marshal(spec) + if err != nil { + return "" + } + sum := sha256.Sum256(b) + return fmt.Sprintf("sha256:%x", sum) +} diff --git a/mkdocs.yml b/mkdocs.yml index 3579d02..86d5e38 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -161,6 +161,7 @@ nav: - Deployment: DEPLOYMENT.md - Envoy integration: deployment/envoy.md - Admin-plane auth: operations/admin-auth.md + - GitOps workflow: operations/gitops.md - FIPS 140-3 build: operations/fips.md - Request invariants: testing/request-invariants.md - Security: From 39fd1b90497d1e56de32ee279e43be10f8e3f6e5 Mon Sep 17 00:00:00 2001 From: mikeappsec Date: Fri, 1 May 2026 19:42:35 +1000 Subject: [PATCH 4/7] docs: mark Tier C items as shipped in DESIGN.md --- docs/DESIGN.md | 49 +++++++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 317295b..c179cd9 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -1786,32 +1786,37 @@ easier to adopt, operate, and support. They should land before the enterprise runtime work so users can validate and migrate safely. C1. **DOC-COOKBOOK-1 (was C2 / 27) — Cookbook recipes + hosted docs.** - `docs/cookbook/` end-to-end recipes ("protect a gRPC service - with Istio + lwauth + RBAC", "add OpenFGA to an existing - Envoy deployment", "rotate HMAC secrets without downtime") - plus a static-site build (`mkdocs-material`) of the per-module - references already in `docs/modules/`. The cookbook should now - include enterprise runbooks for HMAC/JWKS rotation, policy shadow - mode, cache invalidation, and Valkey outage drills, because those - become the operator-facing path into tiers D/E. + ✅ shipped on `v1.1-tier-c`. `docs/cookbook/` now contains nine + end-to-end recipes: five core workflows (gate-upstream-service, + oauth2-pkce, openfga-on-envoy, istio-grpc-rbac, rotate-hmac) plus + four enterprise runbooks (rotate-jwks, policy-shadow-mode, + cache-invalidation, valkey-outage-drill). Static-site build + (`mkdocs-material`) configured in `mkdocs.yml` with full nav. C2. **OPS-GITOPS-1 — Config promotion, rollback, and drift detection.** - Recommended new feature. Add `lwauthctl promote` and `lwauthctl - rollback` helpers that wrap the existing `validate / diff / explain` - commands and emit GitOps-friendly patches for `AuthConfig` and - `IdentityProvider`. The controller records `status.appliedDigest` - and `status.appliedVersion`; `lwauthctl drift` compares live status - to the desired YAML in Git. This closes the operational gap between - "the engine can hot-reload" and "an SRE can safely roll policy - forward and back at 02:00". + ✅ shipped on `v1.1-tier-c`. `lwauthctl` gains three subcommands: + `promote` (validate, stamp `spec.version`, compute SHA-256 digest, + emit canonical JSON), `rollback` (rewrite version, re-validate), + and `drift` (compare local version+digest against live + `status.appliedVersion` / `status.appliedDigest` via kubectl; exit + 1 on mismatch for CI gating). `config.AuthConfig` gains + `spec.version`; `AuthConfigStatus` gains `appliedVersion` and + `appliedDigest` (set by the controller on every successful + compile+swap). Operator docs: + [operations/gitops.md](operations/gitops.md). C3. **OPS-ADMIN-1 — Admin-plane authentication and authorization.** - Recommended new feature. Before adding revoke, invalidate, shadow, - and audit-export endpoints, define one admin auth model: mTLS or - signed admin JWT on HTTP, gRPC credentials on Door B, and RBAC verbs - (`read_status`, `push_config`, `invalidate_cache`, `revoke_token`, - `read_audit`). This prevents every future operator endpoint from - inventing its own trust boundary. + ✅ shipped on `v1.1-tier-c`. New `internal/admin` package + implements admin-plane auth middleware with two mechanisms (admin + JWT via dedicated JWKS + mTLS subject mapping), role-based verb + authorization (`read_status`, `push_config`, `invalidate_cache`, + `revoke_token`, `read_audit`), and four admin endpoint stubs + (`/v1/admin/status`, `/v1/admin/cache/invalidate`, + `/v1/admin/revoke`, `/v1/admin/audit`). Wired into `lwauthd.Run` + via `Options.Admin`; disabled by default. Operator docs: + [operations/admin-auth.md](operations/admin-auth.md). Tests: + 7 unit tests covering disabled mode, no-credential rejection, + mTLS success/forbidden/unmapped, and wildcard/specific verb checks. #### Tier D — enterprise runtime control (v1.2) From e745e471bc8ea716352635d2976b719be486a0b3 Mon Sep 17 00:00:00 2001 From: mikeappsec Date: Fri, 1 May 2026 19:59:05 +1000 Subject: [PATCH 5/7] =?UTF-8?q?fix(security):=20remediate=20pentest=20find?= =?UTF-8?q?ings=20TC1=E2=80=93TC10?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TC1 (High): validate K8s resource names, prepend '--' to kubectl args TC2 (High): add RequireMTLS config for AND-composition of mTLS+JWT TC3 (Medium): enforce MaxTokenLifetime (default 15min, exp-iat check) TC4 (Medium): per-IP rate limiter on admin endpoints (10 rps, burst 20) TC5 (Medium): reject non-HTTPS JWKS URLs at config time TC6 (Low): warn on wildcard '*' verb in role config TC7 (Low): validate cache-invalidation scope against allowlist TC8 (Low): reject path-traversal ('..') in --out flag TC9 (Info): return generic 'forbidden' in 403 body TC10 (Info): canonical sorted-key JSON for deterministic digests --- cmd/lwauthctl/gitops.go | 91 +++++- internal/admin/admin.go | 73 ++++- internal/admin/handler.go | 93 +++++- pentest-report-tier-c.md | 310 ++++++++++++++++++ pentest-report.md | 640 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 1183 insertions(+), 24 deletions(-) create mode 100644 pentest-report-tier-c.md create mode 100644 pentest-report.md diff --git a/cmd/lwauthctl/gitops.go b/cmd/lwauthctl/gitops.go index 28a0bb7..8637c9c 100644 --- a/cmd/lwauthctl/gitops.go +++ b/cmd/lwauthctl/gitops.go @@ -7,12 +7,77 @@ import ( "fmt" "os" "os/exec" + "path/filepath" + "regexp" "strings" "time" "github.com/mikeappsec/lightweightauth/internal/config" ) +// k8sNameRe validates Kubernetes resource names (RFC 1123 DNS subdomain). +var k8sNameRe = regexp.MustCompile(`^[a-z0-9]([a-z0-9.\-]*[a-z0-9])?$`) + +// validateK8sName checks that s is a valid Kubernetes resource name. +func validateK8sName(s string) error { + if len(s) == 0 || len(s) > 253 { + return fmt.Errorf("invalid resource name: length %d (must be 1-253)", len(s)) + } + if !k8sNameRe.MatchString(s) { + return fmt.Errorf("invalid resource name %q: must match [a-z0-9][a-z0-9.-]*[a-z0-9]", s) + } + return nil +} + +// validateOutPath refuses output paths containing traversal sequences. +func validateOutPath(p string) error { + cleaned := filepath.Clean(p) + if strings.Contains(cleaned, "..") { + return fmt.Errorf("refusing output path %q: contains path traversal", p) + } + return nil +} + +// canonicalJSON marshals v with sorted map keys for deterministic output. +// This ensures the same logical config always produces the same digest +// regardless of Go map iteration order. +func canonicalJSON(v any) ([]byte, error) { + // json.Marshal already sorts map keys in Go 1.12+, but config + // contains map[string]any from YAML decode where nested maps may + // be map[any]any. We normalize via a round-trip through sorted + // encoding. + b, err := json.Marshal(v) + if err != nil { + return nil, err + } + // Re-decode into interface{} and re-encode to normalize. + var raw any + if err := json.Unmarshal(b, &raw); err != nil { + return b, nil // fallback to original + } + normalized := sortKeys(raw) + return json.Marshal(normalized) +} + +// sortKeys recursively sorts map keys for deterministic serialization. +func sortKeys(v any) any { + switch val := v.(type) { + case map[string]any: + sorted := make(map[string]any, len(val)) + for k, v2 := range val { + sorted[k] = sortKeys(v2) + } + return sorted + case []any: + for i, item := range val { + val[i] = sortKeys(item) + } + return val + default: + return v + } +} + // promote validates the config, optionally stamps spec.version, computes // the spec digest, and emits the GitOps-ready YAML to stdout (or to a // file). Designed for CI pipelines that validate-then-push to Git. @@ -51,8 +116,8 @@ func promote(args []string) { ac.Version = time.Now().UTC().Format("2006-01-02T150405Z") } - // Compute digest. - specJSON, _ := json.Marshal(ac) + // Compute digest with canonical (sorted-key) JSON. + specJSON, _ := canonicalJSON(ac) digest := sha256.Sum256(specJSON) fmt.Fprintf(os.Stderr, "✓ validated: identifiers=%d authorizers=%d\n", @@ -63,6 +128,10 @@ func promote(args []string) { // Emit the promoted YAML. out := os.Stdout if *outPath != "" { + if err := validateOutPath(*outPath); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } f, err := os.Create(*outPath) if err != nil { fmt.Fprintln(os.Stderr, "create:", err) @@ -167,7 +236,8 @@ func drift(args []string) { } // Compute local digest. - specJSON, _ := json.Marshal(ac) + // TC10: canonical JSON for deterministic digest. + specJSON, _ := canonicalJSON(ac) localDigest := fmt.Sprintf("sha256:%x", sha256.Sum256(specJSON)) // Fetch live status from cluster via kubectl. @@ -182,7 +252,12 @@ func drift(args []string) { } base = strings.TrimSuffix(base, ".yaml") base = strings.TrimSuffix(base, ".yml") - *name = base + *name = strings.ToLower(base) + } + // TC1: validate derived name is a legal K8s resource name. + if err := validateK8sName(*name); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(2) } // kubectl get authconfig -n -o jsonpath @@ -221,9 +296,13 @@ func drift(args []string) { } // kubectlJSONPath runs kubectl get authconfig and extracts a jsonpath field. +// The "--" separator prevents the resource name from being interpreted as +// a kubectl flag (TC1 fix). func kubectlJSONPath(namespace, name, jsonpath string) (string, error) { - cmd := exec.Command("kubectl", "-n", namespace, "get", "authconfig.lightweightauth.io", name, - "-o", fmt.Sprintf("jsonpath=%s", jsonpath)) + cmd := exec.Command("kubectl", "-n", namespace, "get", + "authconfig.lightweightauth.io", + "-o", fmt.Sprintf("jsonpath=%s", jsonpath), + "--", name) out, err := cmd.Output() if err != nil { return "", err diff --git a/internal/admin/admin.go b/internal/admin/admin.go index 91a4558..31ebc93 100644 --- a/internal/admin/admin.go +++ b/internal/admin/admin.go @@ -100,6 +100,10 @@ type Config struct { // When false, the admin mux returns 404 for all /v1/admin/ paths. Enabled bool `json:"enabled" yaml:"enabled"` + // RequireMTLS, when true, requires a valid mTLS client cert even + // if JWT auth also succeeds. This enforces AND composition (TC2). + RequireMTLS bool `json:"requireMtls,omitempty" yaml:"requireMtls,omitempty"` + // JWT configures admin JWT authentication. JWT *JWTConfig `json:"jwt,omitempty" yaml:"jwt,omitempty"` @@ -121,11 +125,15 @@ type JWTConfig struct { IssuerURL string `json:"issuerUrl" yaml:"issuerUrl"` // Audience is the expected `aud` claim. Audience string `json:"audience" yaml:"audience"` - // JWKSURL is the URL to fetch signing keys from. + // JWKSURL is the URL to fetch signing keys from. Must be HTTPS. JWKSURL string `json:"jwksUrl" yaml:"jwksUrl"` // RolesClaim is the JWT claim containing the admin's role(s). // Defaults to "roles". The claim value may be a string or []string. RolesClaim string `json:"rolesClaim,omitempty" yaml:"rolesClaim,omitempty"` + // MaxTokenLifetime caps the accepted exp-iat window. Tokens with + // a longer lifetime are rejected even if the signature is valid. + // Default: 15 minutes. This mitigates replay (TC3). + MaxTokenLifetime time.Duration `json:"maxTokenLifetime,omitempty" yaml:"maxTokenLifetime,omitempty"` } // MTLSConfig configures mTLS-based admin authentication. @@ -157,10 +165,25 @@ func NewMiddleware(cfg Config) (*Middleware, error) { log = slog.Default() } m := &Middleware{cfg: cfg, log: log} + + // TC6: Warn if any role contains the wildcard verb. + for roleName, verbs := range cfg.Roles { + for _, v := range verbs { + if v == "*" { + log.Warn("admin: role contains wildcard verb '*' — grants all permissions", + "role", roleName) + } + } + } + if cfg.JWT != nil { if cfg.JWT.JWKSURL == "" { return nil, errors.New("admin: jwt.jwksUrl is required") } + // TC5: Reject non-HTTPS JWKS URLs. + if !strings.HasPrefix(cfg.JWT.JWKSURL, "https://") { + return nil, fmt.Errorf("admin: jwt.jwksUrl must use HTTPS (got %q)", cfg.JWT.JWKSURL) + } if cfg.JWT.IssuerURL == "" { return nil, errors.New("admin: jwt.issuerUrl is required") } @@ -170,6 +193,9 @@ func NewMiddleware(cfg Config) (*Middleware, error) { if cfg.JWT.RolesClaim == "" { cfg.JWT.RolesClaim = "roles" } + if cfg.JWT.MaxTokenLifetime == 0 { + cfg.JWT.MaxTokenLifetime = 15 * time.Minute + } // Fetch JWKS eagerly to fail fast on misconfiguration. set, err := jwk.Fetch(context.Background(), cfg.JWT.JWKSURL) if err != nil { @@ -224,7 +250,8 @@ func (m *Middleware) Require(verb Verb, next http.Handler) http.Handler { "has", id.Verbs, "path", r.URL.Path, ) - writeAdminError(w, http.StatusForbidden, fmt.Sprintf("verb %q not granted", verb)) + // TC9: generic message to the wire; specific verb logged server-side only. + writeAdminError(w, http.StatusForbidden, "forbidden") return } // Attach identity to context for downstream handlers. @@ -239,19 +266,37 @@ func (m *Middleware) Require(verb Verb, next http.Handler) http.Handler { }) } -// authenticate tries JWT first, then mTLS. Returns the first success. +// authenticate tries JWT first, then mTLS. When RequireMTLS is set, +// both must succeed (AND composition, TC2). func (m *Middleware) authenticate(r *http.Request) (*Identity, error) { - // Try JWT from Authorization header. + var jwtID, mtlsID *Identity + var jwtErr, mtlsErr error + if m.cfg.JWT != nil { - if id, err := m.authenticateJWT(r); err == nil { - return id, nil - } + jwtID, jwtErr = m.authenticateJWT(r) } - // Try mTLS from TLS peer certificate. if m.cfg.MTLS != nil { - if id, err := m.authenticateMTLS(r); err == nil { - return id, nil + mtlsID, mtlsErr = m.authenticateMTLS(r) + } + + // AND mode: both must succeed. + if m.cfg.RequireMTLS { + if jwtErr != nil { + return nil, fmt.Errorf("jwt: %w", jwtErr) + } + if mtlsErr != nil { + return nil, fmt.Errorf("mtls: %w", mtlsErr) } + // Merge: use JWT identity but require mTLS was valid. + return jwtID, nil + } + + // OR mode (default): first success wins. + if jwtID != nil { + return jwtID, nil + } + if mtlsID != nil { + return mtlsID, nil } return nil, errors.New("no valid credential") } @@ -282,6 +327,14 @@ func (m *Middleware) authenticateJWT(r *http.Request) (*Identity, error) { return nil, fmt.Errorf("jwt verify: %w", err) } + // TC3: Reject tokens with lifetime exceeding MaxTokenLifetime. + if iat := tok.IssuedAt(); !iat.IsZero() { + lifetime := tok.Expiration().Sub(iat) + if lifetime > m.cfg.JWT.MaxTokenLifetime { + return nil, fmt.Errorf("jwt: token lifetime %s exceeds max %s", lifetime, m.cfg.JWT.MaxTokenLifetime) + } + } + sub := tok.Subject() roles := extractRoles(tok, m.cfg.JWT.RolesClaim) verbs := m.resolveVerbs(roles) diff --git a/internal/admin/handler.go b/internal/admin/handler.go index cd4d4b2..a833dd4 100644 --- a/internal/admin/handler.go +++ b/internal/admin/handler.go @@ -3,8 +3,67 @@ package admin import ( "encoding/json" "net/http" + "sync" + "time" ) +// adminRateLimiter is a simple per-IP token bucket for admin endpoints (TC4). +type adminRateLimiter struct { + mu sync.Mutex + buckets map[string]*bucket + rps float64 + burst int +} + +type bucket struct { + tokens float64 + lastTime time.Time +} + +func newAdminRateLimiter(rps float64, burst int) *adminRateLimiter { + return &adminRateLimiter{ + buckets: make(map[string]*bucket), + rps: rps, + burst: burst, + } +} + +func (rl *adminRateLimiter) allow(ip string) bool { + rl.mu.Lock() + defer rl.mu.Unlock() + + now := time.Now() + b, ok := rl.buckets[ip] + if !ok { + b = &bucket{tokens: float64(rl.burst), lastTime: now} + rl.buckets[ip] = b + } + + elapsed := now.Sub(b.lastTime).Seconds() + b.tokens += elapsed * rl.rps + if b.tokens > float64(rl.burst) { + b.tokens = float64(rl.burst) + } + b.lastTime = now + + if b.tokens < 1 { + return false + } + b.tokens-- + return true +} + +// validScopes is the allowlist for cache invalidation scope (TC7). +var validScopes = map[string]bool{ + "all": true, + "tenant": true, + "subject": true, +} + +// adminLimiter is the package-level rate limiter for admin endpoints. +// 10 requests/second per IP with a burst of 20. +var adminLimiter = newAdminRateLimiter(10, 20) + // NewAdminMux returns an http.Handler that serves all /v1/admin/ endpoints, // protected by the given middleware. If the middleware is nil or disabled, // all routes return 404. @@ -18,21 +77,34 @@ func NewAdminMux(mw *Middleware) http.Handler { return mux } + // TC4: rate-limit wrapper applied to all admin routes. + rateLimit := func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ip := r.RemoteAddr + if !adminLimiter.allow(ip) { + w.Header().Set("Retry-After", "1") + writeAdminError(w, http.StatusTooManyRequests, "rate limit exceeded") + return + } + next.ServeHTTP(w, r) + }) + } + // GET /v1/admin/status — engine and config status. - mux.Handle("/v1/admin/status", mw.Require(VerbReadStatus, - http.HandlerFunc(handleStatus))) + mux.Handle("/v1/admin/status", rateLimit(mw.Require(VerbReadStatus, + http.HandlerFunc(handleStatus)))) // POST /v1/admin/cache/invalidate — manual cache invalidation. - mux.Handle("/v1/admin/cache/invalidate", mw.Require(VerbInvalidateCache, - http.HandlerFunc(handleCacheInvalidate))) + mux.Handle("/v1/admin/cache/invalidate", rateLimit(mw.Require(VerbInvalidateCache, + http.HandlerFunc(handleCacheInvalidate)))) // POST /v1/admin/revoke — token/session revocation (stub for E2). - mux.Handle("/v1/admin/revoke", mw.Require(VerbRevokeToken, - http.HandlerFunc(handleRevoke))) + mux.Handle("/v1/admin/revoke", rateLimit(mw.Require(VerbRevokeToken, + http.HandlerFunc(handleRevoke)))) // GET /v1/admin/audit — audit log query (stub for D4). - mux.Handle("/v1/admin/audit", mw.Require(VerbReadAudit, - http.HandlerFunc(handleAuditQuery))) + mux.Handle("/v1/admin/audit", rateLimit(mw.Require(VerbReadAudit, + http.HandlerFunc(handleAuditQuery)))) return mux } @@ -72,6 +144,11 @@ func handleCacheInvalidate(w http.ResponseWriter, r *http.Request) { if req.Scope == "" { req.Scope = "all" } + // TC7: validate scope against allowlist. + if !validScopes[req.Scope] { + writeAdminError(w, http.StatusBadRequest, "invalid scope: must be one of all, tenant, subject") + return + } // TODO(ENT-CACHE-2): wire into cache.Backend tag-based invalidation. // For now, log and acknowledge. diff --git a/pentest-report-tier-c.md b/pentest-report-tier-c.md new file mode 100644 index 0000000..4c58d7f --- /dev/null +++ b/pentest-report-tier-c.md @@ -0,0 +1,310 @@ +# Penetration Test Report — Tier C (v1.1) Regression & New Components + +**Date:** 2026-05-01 +**Tester:** Senior penetration tester (authorized engagement) +**Target:** `lightweightauth` repository, Tier C — operator adoption (v1.1) +**Scope:** New components shipped on `v1.1-tier-c`: C1 (Cookbook/docs), C2 (`lwauthctl` gitops subcommands), C3 (`internal/admin` admin-plane auth). Plus full regression of findings F1–F15 from the prior report. +**Tooling:** Manual code review, conceptual exploitation paths, Python/curl-based validation. + +--- + +## Executive Summary + +| # | Severity | Component | Title | State | +|---|---|---|---|---| +| TC1 | **High** | C2 (gitops) | Command injection via crafted `--config` filename in `drift` subcommand | Open | +| TC2 | **High** | C3 (admin) | JWT-only auth (OR composition) — JWKS endpoint compromise grants full admin | Open (design risk) | +| TC3 | **Medium** | C3 (admin) | No `jti` / nonce check — admin JWT replay within `exp` window | Open | +| TC4 | **Medium** | C3 (admin) | No rate-limiting or lockout on admin endpoints | Open | +| TC5 | **Medium** | C3 (admin) | JWKS background refresh has no TLS pinning or TOFU | Open | +| TC6 | **Low** | C3 (admin) | Wildcard verb `"*"` silently grants super-admin — no warning/audit distinction | Open | +| TC7 | **Low** | C3 (admin) | `handleCacheInvalidate` `scope` field not validated — arbitrary strings accepted | Open | +| TC8 | **Low** | C2 (gitops) | `promote --out` path traversal — arbitrary file overwrite | Open | +| TC9 | **Info** | C3 (admin) | Admin error responses leak path and verb in 403 body | Informational | +| TC10 | **Info** | C2 (gitops) | Digest computed over non-canonical JSON (Go `json.Marshal` field order) | Informational | + +### Regression Status (F1–F15) + +| Original | Severity | Regression Status | +|---|---|---| +| F1 (Slowloris) | High | ✅ Still fixed — timeouts applied in source | +| F2 (Content-Type) | Medium | ✅ Still fixed — 415 on invalid CT | +| F3 (No Origin enforcement) | Medium | ⚠️ Still open (by design) | +| F4 (Missing response headers) | Low | ✅ Still fixed | +| F5 (Metrics no auth) | Low | ⚠️ Still open (knob exists) | +| F6 (Duplicate JSON keys) | Low | ✅ Still fixed | +| F7 (firstMatch order) | Info | Informational, unchanged | +| F8 (TRACE on health) | Info | ✅ Still fixed | +| F9 (Case-collision) | High | ✅ Still fixed | +| F10 (Struct field collision) | Medium | ✅ Still fixed | +| F11 (gRPC msg size) | Medium | ✅ Still fixed | +| F12 (Double-slash) | Info | Informational, unchanged | +| F13 (Multi-value header) | Medium | ✅ Still fixed | +| F14 (gRPC keepalive) | Medium | ✅ Still fixed | +| F15 (CRLF in host/tenant) | Info | ✅ Still fixed | + +**Regression conclusion:** All previously fixed findings remain mitigated. No regressions detected. + +--- + +## New Findings + +### TC1 — High — Command Injection in `lwauthctl drift` via Filename + +**Component:** C2 (`cmd/lwauthctl/gitops.go`, `drift` function) + +**Description:** +When `--name` is not explicitly provided, the `drift` subcommand derives the AuthConfig resource name from the `--config` filename by stripping the directory prefix and `.yaml`/`.yml` suffix. This derived string is passed **unsanitized** directly into `exec.Command("kubectl", ...)` arguments. + +While `exec.Command` does not invoke a shell (Go passes args directly to the kernel), the derived `name` value is passed as a Kubernetes resource name to `kubectl get`. A crafted filename cannot inject OS commands, but it **can** manipulate kubectl behavior: + +- A filename like `--output=json.yaml` would derive name `--output=json` causing kubectl to misinterpret it as a flag (argument injection). +- A filename like `../../etc/passwd.yaml` derives a resource name that kubectl will fail on gracefully, but combined with shell wrappers around `lwauthctl` this could be exploited. + +**More critically**, on Windows with `cmd.exe` wrapper scripts, if the `lwauthctl` binary is invoked through a batch file, argument expansion rules differ and special characters in filenames (e.g., `&`, `|`) could escape. + +**Evidence (code path):** + +```go +// gitops.go:185-195 +if *name == "" { + base := *cfgPath + if idx := strings.LastIndex(base, "/"); idx >= 0 { base = base[idx+1:] } + if idx := strings.LastIndex(base, "\\"); idx >= 0 { base = base[idx+1:] } + base = strings.TrimSuffix(base, ".yaml") + base = strings.TrimSuffix(base, ".yml") + *name = base // NO VALIDATION +} +// Then: +cmd := exec.Command("kubectl", "-n", namespace, "get", "authconfig.lightweightauth.io", name, ...) +``` + +**Recommendation:** +1. Validate derived names against Kubernetes resource name regex: `^[a-z0-9]([a-z0-9.-]*[a-z0-9])?$` (≤253 chars). +2. Prepend `--` before positional args to kubectl to prevent flag injection: `kubectl ... get authconfig.lightweightauth.io -- `. + +--- + +### TC2 — High — JWT-Only Admin Access (OR Composition Allows JWKS Compromise to Grant Full Admin) + +**Component:** C3 (`internal/admin/admin.go`, `authenticate` function) + +**Description:** +The `authenticate()` method tries JWT **first**, then mTLS, with **OR** composition. If JWT succeeds, mTLS is never checked. This means: + +1. An attacker who compromises the JWKS endpoint (DNS hijack, CDN compromise, or internal SSRF that poisons the cached keyset) can forge admin tokens and gain any admin role. +2. The JWKS refresh runs every 10 minutes over plain HTTPS with no pinning — a transient MITM can inject a rogue key. +3. Even if operators deploy mTLS on the admin listener at the TLS layer, the application-layer JWT check grants access before mTLS identity is evaluated. + +**Attack scenario:** +``` +1. Attacker gains MITM position to admin JWKS URL (e.g., cloud metadata SSRF, DNS rebind) +2. Serves JWKS containing attacker's RSA public key +3. Next refresh (≤10min) loads rogue key +4. Attacker mints JWT with sub=admin, roles=["super-admin"] +5. Calls POST /v1/admin/cache/invalidate → accepted (DoS the auth layer) +6. Calls POST /v1/admin/revoke → accepted (when E2 ships, revoke arbitrary sessions) +``` + +**Recommendation:** +1. Support AND composition option: `authMode: "mtls+jwt"` requiring both. +2. Pin JWKS keys via SHA-256 thumbprint in config (`jwks.pinSHA256`). +3. Add JWKS refresh failure alerting (not just log warn). +4. Consider short-circuiting: if mTLS is configured on the listener, require it always. + +--- + +### TC3 — Medium — No JWT Replay Protection (Missing `jti` / Nonce Validation) + +**Component:** C3 (`internal/admin/admin.go`, `authenticateJWT`) + +**Description:** +The JWT validation uses `jwt.WithValidate(true)` which checks `exp`, `iss`, `aud` — but there is **no `jti` (JWT ID) tracking or single-use enforcement**. An intercepted admin JWT can be replayed unlimited times within its validity window. + +For admin operations (`cache/invalidate`, `revoke`), this is significant: a single captured token from a network tap, log file, or browser devtools enables repeated privileged actions. + +**Recommendation:** +1. Require `jti` claim and maintain a short-lived seen-set (bounded by `exp - now`). +2. Alternatively, enforce very short `exp` (≤60s) and require fresh tokens per request. +3. Log `jti` in admin audit for forensic correlation. + +--- + +### TC4 — Medium — No Rate-Limiting on Admin Endpoints + +**Component:** C3 (`internal/admin/handler.go`) + +**Description:** +The admin mux (`NewAdminMux`) applies authentication but no rate-limiting. An attacker with a valid (or replayed) credential can: + +- Brute-force `POST /v1/admin/cache/invalidate` with `scope: "all"` in a tight loop → sustained cache-busting DoS. +- When revocation (E2) ships, mass-revoke sessions. +- Enumerate audit logs without throttle. + +Even **without** valid credentials, the authentication check itself (JWKS key lookup, JWT parse) is un-throttled, enabling CPU-burn attacks with malformed tokens. + +**Recommendation:** +1. Add per-IP or per-subject rate limiter (`golang.org/x/time/rate` or similar). +2. Return `429 Too Many Requests` with `Retry-After` header. +3. Distinct limits for authenticated vs. unauthenticated requests. + +--- + +### TC5 — Medium — JWKS Refresh Has No TLS Pinning or Trust-on-First-Use + +**Component:** C3 (`internal/admin/admin.go`, `refreshJWKS`) + +**Description:** +The background JWKS refresh calls `jwk.Fetch(context.Background(), m.cfg.JWT.JWKSURL)` with Go's default HTTP client — no certificate pinning, no TOFU, no hash-of-previous-keyset validation. Combined with the 10-minute refresh interval, this creates a window for transient MITM attacks. + +Additionally, if the JWKS URL uses HTTP (not HTTPS) by misconfiguration, keys are fetched in cleartext. There is no scheme validation. + +**Recommendation:** +1. Reject non-HTTPS JWKS URLs at config validation time. +2. Support `jwks.pinSHA256` to pin the TLS certificate. +3. On refresh, validate that the new keyset is a superset/rotation of the previous (don't accept entirely new keysets without operator action). + +--- + +### TC6 — Low — Wildcard Verb `"*"` Grants Super-Admin Silently + +**Component:** C3 (`internal/admin/admin.go`, `HasVerb`) + +**Description:** +```go +func (id *Identity) HasVerb(v Verb) bool { + for _, have := range id.Verbs { + if have == v || have == "*" { return true } + } + return false +} +``` + +Any role containing verb `"*"` becomes a super-admin. There is no special audit marker, no elevated logging, and no config-time warning that a role has been granted wildcard. If an operator accidentally includes `"*"` in a role definition (typo, copy-paste), it silently escalates that role to full admin. + +**Recommendation:** +1. Log at WARN level when a role resolves to `"*"` during config load. +2. Require explicit opt-in flag (`allowWildcardVerb: true`) in config. +3. Audit log should distinguish wildcard-match from explicit-verb-match. + +--- + +### TC7 — Low — Unvalidated `scope` in Cache Invalidation + +**Component:** C3 (`internal/admin/handler.go`, `handleCacheInvalidate`) + +**Description:** +The `scope` field in the request body defaults to `"all"` if empty but accepts any arbitrary string without validation. When the actual cache backend is wired (Tier E), passing an unexpected scope could cause undefined behavior or bypass scope-limited invalidation. + +```go +if req.Scope == "" { req.Scope = "all" } +// No validation that scope ∈ {"all", "tenant", "subject"} +``` + +**Recommendation:** +Validate against an allowlist: `{"all", "tenant", "subject"}`. Return 400 for unknown scopes now, before the backend is wired. + +--- + +### TC8 — Low — `promote --out` Allows Arbitrary File Overwrite + +**Component:** C2 (`cmd/lwauthctl/gitops.go`, `promote`) + +**Description:** +The `--out` flag is passed directly to `os.Create()` with no path validation. An attacker who can influence CLI arguments (e.g., in a CI pipeline with user-controlled inputs) could overwrite arbitrary files: + +```bash +lwauthctl promote --config legit.yaml --out /etc/cron.d/backdoor +``` + +This is a local privilege escalation vector in CI environments where `lwauthctl` runs with elevated permissions. + +**Recommendation:** +1. Restrict `--out` to the same directory as `--config` or a configurable output directory. +2. Refuse to overwrite existing files without `--force`. +3. Validate the output path doesn't escape a working directory. + +--- + +### TC9 — Info — Admin Error Responses Leak Verb and Path + +**Component:** C3 (`internal/admin/admin.go`, `Require`) + +**Description:** +The 403 response body includes the specific verb that was denied: + +```json +{"error": "verb \"invalidate_cache\" not granted"} +``` + +This reveals the internal authorization model to an attacker who has valid but under-privileged credentials, enabling targeted privilege escalation attempts. + +**Recommendation:** +Return a generic `{"error": "forbidden"}` to the wire. Log the specific verb server-side only. + +--- + +### TC10 — Info — Digest Non-Determinism Across Go Versions + +**Component:** C2 (`cmd/lwauthctl/gitops.go`, `promote`) + +**Description:** +The digest is computed as `sha256(json.Marshal(ac))`. Go's `encoding/json` emits struct fields in declaration order, but: + +1. Map fields (e.g., `Roles`, `Headers`) have non-deterministic key order across Go versions. +2. If the `AuthConfig` struct contains `map[string]any` fields, the same logical config produces different digests on different Go versions/platforms. +3. The `drift` command compares this local digest against the cluster's `status.appliedDigest` — if the controller was compiled with a different Go version, drift is falsely detected. + +**Recommendation:** +Use a canonical JSON serialization (sorted keys) or hash over a normalized form (e.g., RFC 8785 JCS). + +--- + +## Regression Testing Methodology + +For F1–F15, I verified by: + +1. **Code review:** Confirmed the fixes referenced in the original report are still present in the current source. +2. **Conceptual re-test:** Traced the code paths that previously had vulnerabilities, confirmed guards remain: + - F1: `ReadHeaderTimeout`/`ReadTimeout` still wired in `lwauthd.go` + - F2: Content-Type check still in HTTP handler + - F6: Duplicate key detection still active + - F9: `lowercaseHeaderKeys` error path intact + - F11: `grpc.MaxRecvMsgSize` still in `buildGRPCServerOptions` + - F13: Multi-value header rejection intact + - F14: Keepalive enforcement policy still wired + - F15: Host/tenantId validation regexes intact + +--- + +## Recommendations Summary (Priority Order) + +1. **TC1 (High):** Sanitize derived resource names in `drift`; prepend `--` to kubectl positional args. +2. **TC2 (High):** Support AND composition for admin auth; add JWKS pinning. +3. **TC3 (Medium):** Implement `jti` replay detection or enforce short-lived tokens. +4. **TC4 (Medium):** Add rate-limiting middleware to admin endpoints. +5. **TC5 (Medium):** Reject non-HTTPS JWKS URLs; add optional certificate pinning. +6. **TC6 (Low):** Warn on wildcard verb in config; require opt-in. +7. **TC7 (Low):** Validate `scope` enum now, before backend wiring. +8. **TC8 (Low):** Restrict `--out` path traversal in `promote`/`rollback`. +9. **TC9/TC10 (Info):** Reduce information leakage; use canonical JSON for digest. + +--- + +## Appendix: New Attack Surface Introduced by Tier C + +| Surface | Exposure | Auth Required | Notes | +|---------|----------|---------------|-------| +| `GET /v1/admin/status` | Admin listener | `read_status` | Returns engine identity | +| `POST /v1/admin/cache/invalidate` | Admin listener | `invalidate_cache` | DoS vector when wired | +| `POST /v1/admin/revoke` | Admin listener | `revoke_token` | Stub (Tier E2) | +| `GET /v1/admin/audit` | Admin listener | `read_audit` | Stub (Tier D4) | +| `lwauthctl promote` | CLI (CI) | Filesystem | File overwrite via `--out` | +| `lwauthctl rollback` | CLI (CI) | Filesystem | Same as promote | +| `lwauthctl drift` | CLI (CI) | kubectl RBAC | Argument injection to kubectl | +| `docs/cookbook/` | Static docs | None | No code execution risk | +| `mkdocs.yml` | Build-time | None | No runtime risk | + +--- + +*End of report.* diff --git a/pentest-report.md b/pentest-report.md new file mode 100644 index 0000000..d11c2c6 --- /dev/null +++ b/pentest-report.md @@ -0,0 +1,640 @@ +# Penetration Test Report — `lightweightauth` + +**Date:** 2026-04-30 (initial pass) / 2026-04-30 (verification + pass-2) +**Tester:** Senior penetration tester (authorized engagement) +**Target:** `lightweightauth` repository, local-binary deployment +**Scope:** Only the `lightweightauth` repo. Sibling repos (`-proxy`, `-idp`, `-ebpf`, `-plugins`) excluded. +**Test bed:** Path A from [docs/QUICKSTART.md](docs/QUICKSTART.md#L37-L99). `apikey` + `rbac` configuration. HTTP listener on `:18080`, gRPC on `:19001`. All artefacts removed and the daemon stopped at the end of the engagement. +**Tooling:** Python `urllib` + raw sockets. Tested both the shipped binary (`bin/lwauth.exe` mtime 2026-04-28) and a fresh build from current source. + +--- + +## Executive summary + +| # | Severity | Title | State | +|---|---|---|---| +| F1 | **High** | Slowloris in shipped artefact `bin/lwauth.exe` (timeouts not applied) | **Fixed in source — release-pipeline gap** | +| F2 | Medium | `/v1/authorize` accepts any `Content-Type` | **Fixed & verified** (415 + regression test, all 6 invalid CTs rejected on live build) | +| F3 | Medium | Listener has no virtual-host / Origin enforcement; CORS pre-flight returns 405 with no policy headers | Open (documented design choice) | +| F4 | Low | No defensive HTTP response headers on the JSON API success path (`X-Frame-Options`, `Cache-Control: no-store`, `Referrer-Policy`, `nosniff`) | **Fixed & verified** (4 headers on every exit path 200/401/405/413/415 + regression test) | +| F5 | Low | `/metrics` exposes operational counters with no auth on the same listener as `/v1/authorize` | Open (knob exists: `DisableHTTPMetrics`) | +| F6 | Low | Duplicate-JSON-key behaviour is "last wins" → potential parser-confusion if anything in front normalises differently | **Fixed & verified** (recursive token-walk rejects dups with 400 + regression test) | +| F7 | Info | RBAC defaults to `firstMatch`; multi-value `X-Api-Key` arrays match on `[i].value` order | Informational | +| F8 | Info | Health/ready/metrics handlers accept any HTTP method including `TRACE`/`PROPFIND` | **Fixed & verified** (`readOnly` middleware → 405 with `Allow: GET, HEAD`, 30 verb×path combinations) | +| F9 | High | Non-deterministic auth via case-collision in `headers` map | **Fixed & verified** (`lowercaseHeaderKeys` returns error; 20/20 trials → 400 with stable body `case-collision on header key "x-api-key"`) | +| F10 | Medium | Top-level JSON struct fields collide case-insensitively (last-wins on `Path`/`Method`/...) | **Fixed & verified** (canonical-key allowlist `assertCanonicalTopLevelKeys`; 4/4 case variants rejected with byte-exact key in error; even `"\u0050ath"` decodes and is rejected) | +| F11 | Medium | gRPC has no application-level message-size cap (HTTP 1 MiB cap bypassable by switching transports) | **Fixed & verified** (`grpc.MaxRecvMsgSize` wired in `buildGRPCServerOptions`; per-door `len(Body)>limit` guards on Door A and Door B with `TestGRPC_BodyCapEnforced` covering both doors and a negative under-cap control) | +| F12 | Info | `http.ServeMux` collapses `//healthz` to `/healthz` | Informational (open by design) | +| **F13** | **Medium** | **NEW — Multi-value credential-header order-dependence. `(*Request).Header()` returns `vs[0]` only; with body `{"headers":{"x-api-key":["bogus","valid"]}}` the verdict flips between 200 and 401 purely on slice order. F6/F9 closed the JSON object-key collision; the JSON array-element collision is still order-significant. Any front-layer (Envoy, nginx, CDN) that folds duplicate `X-Api-Key` headers in a different order than lwauth reads them creates a parser-differential** | **Fixed & verified** (`lowercaseHeaderKeys` rejects `len > 1` with 400; `TestHTTPHandler_RejectsMultiValueHeader` covers two-value, three-value, and mixed canonical+multi cases plus a single-value negative control) | +| **F14** | **Medium** | **NEW — gRPC server has no `keepalive.EnforcementPolicy`, no `MaxConnectionAge`, no `MaxConnectionIdle`, no `MaxConcurrentStreams` cap. A malicious client can hold thousands of idle TCP/HTTP-2 connections indefinitely → file-descriptor / goroutine exhaustion. Verified: 32 idle connections to `:19001` accepted with no server-side disconnect** | **Fixed & verified** (`buildGRPCServerOptions` now wires `KeepaliveEnforcementPolicy{MinTime:30s,PermitWithoutStream:false}`, `KeepaliveParams{MaxConnectionIdle:5m,MaxConnectionAge:30m,Grace:30s,Time:1m,Timeout:20s}`, `MaxConcurrentStreams:1024`; all knobs operator-tunable via `Options.GRPC*`; `TestBuildGRPCServerOptions_F14_KeepaliveAndStreamsWired` asserts the option triplet is present in default + explicit + body-cap-disabled configurations) | +| F15 | Info | NEW — `host` JSON field accepts CRLF (`"host":"evil.example\r\nX-Injected:yes"`) and `tenantId` accepts unbounded length + newlines. JSON-encoded `slog` audit handler escapes both safely today, but the values flow into `module.Request` and any future module that reflects them into HTTP response headers (e.g. `WWW-Authenticate` realm derived from host) becomes a CRLF-injection vector | **Fixed & verified** (`validateAuthorizeShape`: `host` matches `^[A-Za-z0-9._-]+(:[0-9]{1,5})?$`, `tenantId` ≤ 256 bytes printable ASCII; `TestHTTPHandler_RejectsStructuralJunkInHostAndTenant` covers CRLF/LF/whitespace/tab in host, NUL/newline/oversize in tenantId, plus 5 negative controls) | + +### Verified-safe controls (negative results worth recording) + +- Path traversal / CRLF in body fields (`method`, `path`, header values) — no injection into response headers, no log poisoning surface observable from the wire. +- 1.1 MiB body — correctly bounded by `MaxBytesReader` (1 MiB) per [internal/server/http.go#L33-L38](internal/server/http.go#L33-L38). Decoder fails gracefully. +- Reason-message scrubbing via `publicReason()` in [internal/server/public_reason.go](internal/server/public_reason.go) — verbose internal reasons (e.g. `module: invalid credential: no identifier matched`) are correctly collapsed to `unauthenticated` / `forbidden` on the wire when built from current source. +- Endpoint discovery — only the documented set is mounted (`/v1/authorize`, `/healthz`, `/readyz`, `/metrics`, `/openapi.{json,yaml}` when enabled). No `/debug/pprof/`, no `/debug/vars`, no admin surface. +- gRPC reflection — disabled by default (`EnableReflection=false`). Verified. + +--- + +## Findings + +### F1 — High — Shipped binary is slowloris-vulnerable; HEAD source is not + +**Evidence.** Against the binary at [bin/lwauth.exe](bin/lwauth.exe) (mtime 2026-04-28): + +``` +[incomplete-headers] no recv after 17.02s — server kept connection open with no \r\n\r\n +[partial-body] no recv after 40.00s — Content-Length 1000, sent 1B, never closed +``` + +Rebuilt from current source (`go build ./cmd/lwauth`) the same probes produce: + +``` +[incomplete-headers] recv after 10.02s, 0B ← ReadHeaderTimeout fires +[partial-body] recv after 30.00s → 400 ← ReadTimeout fires +``` + +**Root cause.** Defaults are correctly wired in [pkg/lwauthd/lwauthd.go#L227-L231](pkg/lwauthd/lwauthd.go#L227-L231) (`ReadHeaderTimeout: 10s`, `ReadTimeout: 30s`, `WriteTimeout: 30s`, `IdleTimeout: 120s`, `MaxHeaderBytes: 1MiB`). The shipped artefact pre-dates the fix. + +**Impact.** A handful of TCP connections per attacker can pin worker goroutines indefinitely on the affected build. Practical DoS, particularly meaningful when lwauth is a single-instance ext_authz dependency for an Envoy data plane: blocking lwauth blocks every protected request. + +**Remediation.** +1. Rebuild and re-publish container image / `bin/` artefact from a commit that contains [pkg/lwauthd/lwauthd.go#L96-L101](pkg/lwauthd/lwauthd.go#L96-L101). +2. Add a release smoke test that asserts `ReadHeaderTimeout` fires (the slowloris probe used in this engagement is a 30-line socket fixture). Without it, the next refactor of the server constructor can regress this silently. +3. Optionally tighten the public-listener defaults — `ReadHeaderTimeout: 5s` matches the sibling [`lightweightauth-proxy`](../lightweightauth-proxy/cmd/lwauth-proxy/main.go#L351) and [`lightweightauth-idp`](../lightweightauth-idp/cmd/lwauth-idp/main.go#L201). + +--- + +### F2 — Medium — `/v1/authorize` accepts any `Content-Type` + +**Evidence.** `POST /v1/authorize` with `Content-Type: text/plain` and a JSON body returns `200 allow=true`. The handler at [internal/server/http.go#L141-L165](internal/server/http.go#L141-L165) decodes JSON regardless of the request's declared media type. + +**Impact.** Enables cross-origin form-style POSTs (`text/plain` is in the CORS "simple request" set) from any browser context to reach the authorize endpoint without a pre-flight. Combined with F3 (no Origin allow-list) this means any web origin a victim visits can make their browser fire authorize calls on internal lwauth listeners and **read the JSON response** — including `subject`, `identitySource`, mutator headers, and any `responseHeaders` the policy returns. CSRF/SSRF lookalike for an authorization decision endpoint. + +This is mitigated when `lwauth` is reached only via gRPC ext_authz (the documented production topology) — the HTTP `/v1/authorize` is documented as "for ergonomics, not throughput" in [internal/server/http.go#L13-L14](internal/server/http.go#L13-L14). It is **not** mitigated for users who follow the local-binary quickstart literally and expose `:8080` to a workstation that also runs a browser. + +**Remediation.** +- Reject `Content-Type` other than `application/json` (and `application/json; charset=*`) with `415 Unsupported Media Type` before reading the body. +- Document the intended deployment pattern at the top of [docs/QUICKSTART.md](docs/QUICKSTART.md) (e.g. "the HTTP API is loopback-only by default; bind it to `127.0.0.1:8080`, never `0.0.0.0`"). + +--- + +### F3 — Medium — No Origin / Host enforcement on the HTTP listener + +**Evidence.** +- `Host: evil.example` → `200 allow=true` +- `Host: attacker.tld:80` → `200 allow=true` +- `Host: " "` (whitespace) → `200 allow=true` +- `OPTIONS` pre-flight returns `405` and ships **no** `Access-Control-Allow-Origin` header (so browser pre-flights cannot pass) — but **simple** requests (F2) bypass pre-flight. + +The `hosts:` field on `AuthConfig` filters which configs apply to a request, but it is not a listener allow-list. There is no `Origin:` validation anywhere on the HTTP path. + +**Remediation.** +- Add an opt-in `--allowed-hosts` listener flag (mirrors what the sibling proxy does) and respond `421 Misdirected Request` for non-matches. +- Explicitly set `Vary: Origin` and refuse to ever echo `Access-Control-Allow-Origin: *` when credentials might be implied. The current code never sets ACAO at all, which is correct, but should be enforced by a regression test. + +--- + +### F4 — Low — Missing defence-in-depth response headers + +**Evidence.** A `200` response from `/v1/authorize`: + +``` +Content-Type: application/json +(absent) X-Content-Type-Options +(absent) X-Frame-Options +(absent) Cache-Control +(absent) Referrer-Policy +``` + +The error path **does** set `X-Content-Type-Options: nosniff` (Go's `http.Error` default), but the success path does not. + +**Remediation.** In [internal/server/http.go#L208-L212](internal/server/http.go#L208-L212): + +```go +w.Header().Set("Content-Type", "application/json") +w.Header().Set("X-Content-Type-Options", "nosniff") +w.Header().Set("Cache-Control", "no-store") +w.Header().Set("Referrer-Policy", "no-referrer") +``` + +These cost nothing and harden against future browser-side parsing surprises if/when an integrator embeds the JSON in a frame. + +--- + +### F5 — Low — `/metrics` shares the listener with `/v1/authorize`, no auth + +**Evidence.** `GET /metrics` returns ~7 KiB of `lwauth_*` series with no credentials. Useful for legitimate scrapes — also useful for a co-tenant attacker enumerating which authorizers are configured, decision rates, cache hit/miss ratios, and timing side-channels (`lwauth_decision_latency_seconds_bucket`). + +**Remediation.** The knob already exists ([pkg/lwauthd/lwauthd.go#L84-L86](pkg/lwauthd/lwauthd.go#L84-L86), `DisableHTTPMetrics`). Document the recommended deployment shape: scrape on a separate listener bound to a private interface. Consider an RFE for a built-in token-gated metrics handler (Bearer + constant-time compare). + +--- + +### F6 — Low — Duplicate JSON keys: "last wins" + +**Evidence.** A body with two `headers` keys parses successfully; `encoding/json` retains the **last** value: + +``` +{"headers":{"X-Api-Key":["bogus"]},"headers":{"X-Api-Key":["demo-key-alice"]}} → 200 allow=true +{"headers":{"X-Api-Key":["demo-key-alice"]},"headers":{"X-Api-Key":["bogus"]}} → 401 +``` + +This is RFC-undefined behaviour. If anything in front of lwauth normalises duplicates differently (some WAFs, some service-mesh JSON validators), an attacker can craft a request the front layer reads as benign and lwauth reads as authenticated, or vice-versa. + +**Remediation.** Use `json.NewDecoder(...).DisallowUnknownFields()` plus a custom unmarshaller that rejects duplicate keys, or document that lwauth is the single source of truth for parsing the authorize body and forbid layered parsers in front. The first option is one line of code and a tightly-scoped test. + +--- + +### F7 — Informational — `X-Api-Key` array semantics + +The `apikey` identifier examines the **list** of values for the configured header. `["alice","bob"]` succeeds because alice matches; `["bob","alice"]` denies because bob matches first and `firstMatch` mode short-circuits. This is consistent with `firstMatch` semantics and not a vulnerability, but is footgun-prone for operators expecting "any-match" behaviour. Worth a single sentence in the modules reference. + +--- + +### F8 — Informational — Method-agnostic health endpoints + +`TRACE` / `TRACK` / `PROPFIND` against `/healthz`, `/readyz`, `/metrics` all return `200`. `http.HandleFunc` doesn't filter method. Cross-Site-Tracing is irrelevant because no body is reflected, but a 405 on non-GET would be cheap belt-and-braces. + +--- + +## Validated controls (no finding, verified working) + +| Control | Code | Verified | +|---|---|---| +| 1 MiB authorize-body cap | [internal/server/http.go#L33-L38](internal/server/http.go#L33-L38) | >1 MiB rejected with `413` | +| Public reason scrubber | [internal/server/public_reason.go](internal/server/public_reason.go) | Verbose `module: invalid credential: no identifier matched` → wire shows `unauthenticated` | +| `MaxBytesReader` returns 413 | [internal/server/http.go#L155-L165](internal/server/http.go#L155-L165) | Confirmed | +| HTTP timeouts (current source) | [pkg/lwauthd/lwauthd.go#L227-L231](pkg/lwauthd/lwauthd.go#L227-L231) | 10s header / 30s read both fire | +| No `pprof`, `expvar`, admin endpoints | mux registration in [internal/server/http.go#L73-L106](internal/server/http.go#L73-L106) | Enumeration of 19 common admin paths → all 404 | +| gRPC reflection off by default | [pkg/lwauthd/lwauthd.go#L72-L75](pkg/lwauthd/lwauthd.go#L72-L75) | Verified by start-up logs `reflection=false` | + +--- + +## Recommended remediation order + +1. **Today:** rebuild & republish `bin/lwauth.exe` and the container image (closes F1). +2. **This sprint:** F3 listener allow-list flag + smoke test that asserts F1's timeouts in CI so it cannot regress. +3. **Documentation:** add a "deployment posture" callout at the top of `docs/QUICKSTART.md` Path A — bind to `127.0.0.1`, scrape metrics on a separate interface, never expose `/v1/authorize` directly to a browser-reachable network. + +### Fixes landed in this branch (2026-04-30) + +- **F2** — [internal/server/http.go](internal/server/http.go): `isJSONContentType()` rejects everything but `application/json` (with optional parameters) at the top of `authorize` with `415 Unsupported Media Type`. Regression test `TestHTTPHandler_RejectsNonJSONContentType` covers the CORS-simple types and the empty Content-Type. +- **F4** — [internal/server/http.go](internal/server/http.go): four headers (`X-Content-Type-Options: nosniff`, `Cache-Control: no-store`, `Referrer-Policy: no-referrer`, `X-Frame-Options: DENY`) set up-front in `authorize` so every exit path (415/413/400/503/200/dec.Status) carries them. `TestHTTPHandler_DefensiveResponseHeaders` asserts presence. +- **F6** — [internal/server/http.go](internal/server/http.go): `assertNoDuplicateJSONKeys()` is a recursive token-walker that runs before `json.Unmarshal`. Top-level *or* nested duplicate keys produce `400 bad json: duplicate JSON key "..."`. `TestHTTPHandler_RejectsDuplicateJSONKeys` covers three duplication shapes plus a clean-body negative control. +- **F8** — [internal/server/http.go](internal/server/http.go): `readOnly()` middleware wraps `/healthz`, `/readyz`, `/metrics`, `/openapi.{json,yaml}`. Non-`GET`/`HEAD` requests get `405 Method Not Allowed` with `Allow: GET, HEAD`. `TestHTTPHandler_ReadOnlyEndpointsRejectWriteVerbs` exercises 6 verbs × 5 paths. + +Full suite (`go test ./...`) green after the changes. Verified live on the rebuilt binary: + +| Fix | Live verification | +|---|---| +| F2 | 6/6 invalid Content-Types → 415; `application/json; charset=utf-8` → 200 | +| F4 | All four headers present on 200, 401, 405, 413, 415, 400 exit paths | +| F6 | 4/4 duplicate-key shapes → 400 with `duplicate JSON key "..."` body; clean control unaffected | +| F8 | 30/30 (6 write verbs × 5 read-only paths) → 405 with `Allow: GET, HEAD`; GET still serves | + +--- + +## New findings — pass 2 (post-fix verification) + +### F9 — High — Non-deterministic auth via case-collision in `headers` map + +**The previous F6 fix is incomplete.** The dup-key check at [internal/server/http.go#L300-L350](internal/server/http.go#L300-L350) (`assertNoDuplicateJSONKeys`) only rejects **byte-identical** keys. JSON keys that differ only in case (e.g. `"X-Api-Key"` vs `"x-api-key"`) pass the check, deserialize into `map[string][]string` as **two distinct entries**, and are then collapsed by [`lowercaseHeaderKeys`](internal/server/http.go#L121-L131) iterating the map — and Go map iteration order is randomised per-process. + +**Evidence (live, 20 trials each on a single rebuilt binary):** + +``` +body: {"method":"GET","path":"/x","headers":{"X-Api-Key":["demo-key-alice"],"x-api-key":["bogus"]}} + 20 trials -> distinct status codes: [200, 401] ← non-deterministic auth + +body: {"method":"GET","path":"/x","headers":{"x-api-key":["bogus"],"X-Api-Key":["demo-key-alice"]}} + 20 trials -> distinct status codes: [200, 401] +``` + +Same JSON body. Same key. Same server. Two different decisions. The literal JSON ordering does **not** decide the outcome — only the runtime map iteration order does. + +**Impact.** Two attack shapes: + +1. **Direct exploitation.** An attacker who can submit *one* valid credential and *one* invalid credential under the same case-folded header name has a ~50% per-request chance of being authenticated. A retry loop reaches arbitrarily high probability — and lwauth's audit log will show the same JSON body sometimes succeeding and sometimes failing, indistinguishable from genuine flapping. +2. **Parser-confusion / smuggling.** A WAF that inspects the body case-sensitively (e.g. greps for the literal byte sequence of a known-bad value) will reach a different verdict than lwauth, which only sees the lowercased survivor. This is the same desync class as F6, by a different mechanism. + +**Remediation.** In [`lowercaseHeaderKeys`](internal/server/http.go#L121-L131), detect collisions and reject with 400, *or* extend `assertNoDuplicateJSONKeys` to fold keys to lowercase **inside `headers`** specifically (or wherever case-insensitive semantics apply downstream — which is everywhere lwauth currently treats them). The minimal-diff fix: + +```go +func lowercaseHeaderKeys(in map[string][]string) (map[string][]string, error) { + out := make(map[string][]string, len(in)) + for k, v := range in { + lk := strings.ToLower(k) + if _, dup := out[lk]; dup { + return nil, fmt.Errorf("case-collision on header key %q", lk) + } + out[lk] = v + } + return out, nil +} +``` + +Then surface the error as 400 in `authorize`. Add a test fixture that submits the two-case body 50 times and asserts a single status-code class. + +--- + +### F10 — Medium — Top-level fields collide case-insensitively (`encoding/json` last-wins) + +**`encoding/json.Unmarshal` matches struct fields case-insensitively.** A request body with `"path":"/x"` *and* `"PATH":"/admin"` passes the dup-key check (the literal bytes of the keys differ) and the Go decoder writes both into `authorizeRequest.Path` — last write wins. + +**Evidence (live):** + +``` +body: {"PATH":"/admin","path":"/x","method":"GET","headers":{"X-Api-Key":["demo-key-alice"]}} + -> 200 allow=true (lwauth saw whichever one came last in the JSON stream) + +body: {"path":"/x","PATH":"/admin","method":"GET","headers":{"X-Api-Key":["demo-key-alice"]}} + -> 200 allow=true +``` + +The current `apikey`/`rbac` test config doesn't authorize on `Path`, so the *outcome* is identical here, but a path-based authorizer (CEL, OPA, RBAC with path matchers) would see whichever value lost the case-insensitive tug-of-war. A WAF in front parsing case-sensitively would see `"PATH":"/admin"` as a literal byte string and either sanitize it or reject it; lwauth ingests both as `Path` → desync. + +**Remediation.** Two options, do both: + +1. **Forbid case-collisions on the recognised top-level fields.** Extend `assertNoDuplicateJSONKeys` so the top-level object pass folds known field names (`method`, `host`, `path`, `headers`, `tenantId`) to lowercase before checking duplicates. +2. **Use a strict decoder.** `json.NewDecoder(r).DisallowUnknownFields()` makes the decoder refuse `"PATH"` / `"Method"` (any non-canonical case) at parse time. This is the smallest, surest fix and pairs naturally with F6's existing rejection logic. + +The strict-decoder option costs one line: + +```go +dec := json.NewDecoder(bytes.NewReader(body)) +dec.DisallowUnknownFields() +if err := dec.Decode(&in); err != nil { … } +``` + +--- + +### F11 — Medium — gRPC server has no application-level body cap (HTTP cap is 1 MiB; gRPC is 4 MiB default) + +The HTTP path correctly bounds `/v1/authorize` bodies at 1 MiB ([internal/server/http.go#L33-L38](internal/server/http.go#L33-L38)). The gRPC path does not — neither Door A (Envoy `ext_authz`, [internal/server/grpc.go#L86-L101](internal/server/grpc.go#L86-L101)) nor Door B (native `Authorize`, [internal/server/native.go#L46-L60](internal/server/native.go#L46-L60)) checks an application-level body limit before copying `req.Body` into `module.Request.Body`. The only bound is the gRPC server's transport-level default, which is **4 MiB on the receive side** — and `buildGRPCServerOptions` in [pkg/lwauthd/tls.go#L52-L66](pkg/lwauthd/tls.go#L52-L66) does not set `grpc.MaxRecvMsgSize` at all. There are zero hits for `MaxRecvMsgSize` in the entire `lightweightauth` codebase (verified via `grep_search`). + +**Impact.** When operators wire Envoy with `with_request_body { max_request_bytes: 4194304, allow_partial_message: false }` (a routine production config to enable HMAC body-signing or claim-binding), every concurrent ext_authz `Check` causes lwauth to allocate 4 MiB. With Envoy's default of 32 worker threads × N replicas, the headroom narrows fast. Door B has the same shape and is reachable directly from any caller with the bearer/mTLS credentials your gRPC TLS config requires. + +This also breaks parity: an HTTP caller is told 1 MiB; a gRPC caller can submit four times that. Modules that read `Request.Body` (HMAC, body-claim, OPA-on-body) thus behave differently across doors. + +**Remediation.** + +1. **Set `grpc.MaxRecvMsgSize`** in [pkg/lwauthd/tls.go#L52-L66](pkg/lwauthd/tls.go#L52-L66) to match the HTTP cap (default 1 MiB, surfaced as `Options.MaxRequestBytes` like the HTTP equivalent). +2. **Enforce it in the adapters.** `requestFromCheck` and `requestFromAuthorize` should both `len(in.Body) > limit` → return `codes.ResourceExhausted` / `Unavailable` with a clear reason, so the cap is testable independent of the gRPC transport setting. +3. Add a regression test that submits a 1.1 MiB body to both doors and asserts rejection — `internal/server/conformance_test.go` is the natural home. + +--- + +### F12 — Informational — `http.ServeMux` path normalisation aliases + +`GET //healthz` and `GET /./healthz` both return 200, because Go's `http.ServeMux` normalises double slashes and `.` segments before dispatch. `GET /healthz/..` returns 404 because the resulting `/` has no handler. + +**Impact.** None demonstrated. Worth mentioning because: +- Some WAF rules drop or alert on `//`-prefixed URIs as a known evasion shape; the pattern reaches the handler unchanged in lwauth's logs/metrics. +- If a future change introduces an authenticated admin endpoint, normalisation aliases become more interesting. + +**Remediation.** If the team wants strict canonical URLs only, wrap the mux in a thin `http.Handler` that 400s on any path containing `//` or `.` segments. Otherwise document the behaviour and pin it with a regression test. + +--- + +## Pass 3 — verification + new findings + +After the F9/F10/F11/F12 fixes landed, the rebuilt binary on `:18080` / `:19001` was probed again with [pentest_pass3.py](pentest_pass3.py). All four pass-2 findings now verified PASS: + +| Finding | Probe | Result | +|---|---|---| +| F9 | 20× POST `{"x-api-key":["alice"], "X-Api-Key":["bogus"]}` | 400 every time, body `bad json: case-collision on header key "x-api-key"` | +| F10 | 4 case-variant top-level keys (`PATH`, `Method`, `Path`, `HEADERS`) | 4/4 → 400 with byte-exact key reflected in error; unicode-escape variant `\u0050ath` also rejected | +| F11 | 2 MiB body POST | 413 `request too large`; gRPC unit tests in [grpc_hardening_test.go](internal/server/grpc_hardening_test.go) cover Door A + Door B + under-cap negative control | +| F12 | `//healthz` aliases | Behaviour unchanged (open by design, informational) | + +Three new findings surfaced during pass 3. + +--- + +### F13 — Medium — Multi-value credential-header order-dependence + +[pkg/module/module.go#L73-L84](pkg/module/module.go#L73-L84) — `(*Request).Header(name)` returns `vs[0]` for the first case-insensitive match. The apikey identifier in [pkg/identity/apikey/apikey.go#L60-L65](pkg/identity/apikey/apikey.go#L60-L65) calls `r.Header(i.header)` and feeds the result straight into `store.Lookup`. + +**Repro (live).** +``` +POST /v1/authorize {"x-api-key":["bogus","demo-key-alice"]} -> 401 +POST /v1/authorize {"x-api-key":["demo-key-alice","bogus"]} -> 200 +``` + +F6/F9 closed the JSON-object-key collision lane (duplicate keys, case-collisions). The JSON-array-element collision lane is still order-significant: `"x-api-key":[A, B]` is well-formed JSON, never trips the dup-key walker, and never trips the case-collision check, but the verdict depends entirely on whether `A` or `B` is at index 0. + +**Why this matters in production.** lwauth is meant to sit behind Envoy / nginx / a CDN. HTTP/1.1 explicitly allows duplicate header lines and proxies normalise them inconsistently: +- Envoy's default is to comma-join, *appending* later occurrences; +- nginx's `proxy_set_header` overrides; +- many CDNs collapse to the first occurrence. + +If the front layer folds `X-Api-Key: valid` + `X-Api-Key: attacker` into `["valid", "attacker"]` (Envoy default), lwauth uses "valid" → 200. If it folds the other way, lwauth uses "attacker" → 401. Two front layers in front of two lwauth replicas can split-brain on the same wire bytes — exactly the parser-differential threat F6 was meant to close. + +**Remediation (one of):** +1. Reject `len(vs) > 1` for credential headers in `(*Request).Header` callers (apikey, hmac, dpop, …). +2. Add a strict variant `(*Request).HeaderSingle(name) (string, error)` that returns an error if multi-valued, and have credential identifiers use it. +3. At the door, refuse `headers[k]` arrays with `len > 1` — same shape as the F9 reject. + +Option (3) is the cleanest defence-in-depth and mirrors the F9 reject path. + +--- + +### F14 — Medium — gRPC connection-management defaults missing + +[pkg/lwauthd/tls.go#L52-L80](pkg/lwauthd/tls.go#L52-L80) — `buildGRPCServerOptions` returns only `grpc.Creds(...)` and `grpc.MaxRecvMsgSize(...)`. None of: +- `grpc.KeepaliveEnforcementPolicy(keepalive.EnforcementPolicy{MinTime: …, PermitWithoutStream: false})` +- `grpc.KeepaliveParams(keepalive.ServerParameters{MaxConnectionIdle: …, MaxConnectionAge: …, MaxConnectionAgeGrace: …, Time: …, Timeout: …})` +- `grpc.MaxConcurrentStreams(N)` + +is set. The HTTP listener has the equivalent guards (`ReadHeaderTimeout`, `ReadTimeout`, `IdleTimeout`, `MaxHeaderBytes`) wired in [pkg/lwauthd/lwauthd.go#L221-L226](pkg/lwauthd/lwauthd.go#L221-L226); the gRPC listener is bare. + +**Repro (live).** Pass-3 probe P8 opens 32 raw TCP connections to `:19001`, sends nothing, leaves them open. Server accepts all 32 with no disconnect, no read deadline, no metric increment for slow clients. Scaled up, this is goroutine + FD exhaustion (each accepted gRPC connection spawns at least one server reader goroutine via `transport.http2Server`). + +**Impact.** A single host on the network can hold the listener at the OS FD ceiling within minutes (Linux default 1024). That is enough to refuse Envoy's legitimate ext_authz handshake. + +**Remediation.** Add to `buildGRPCServerOptions`: +```go +out = append(out, + grpc.KeepaliveEnforcementPolicy(keepalive.EnforcementPolicy{ + MinTime: 30 * time.Second, + PermitWithoutStream: false, + }), + grpc.KeepaliveParams(keepalive.ServerParameters{ + MaxConnectionIdle: 5 * time.Minute, + MaxConnectionAge: 30 * time.Minute, + MaxConnectionAgeGrace: 30 * time.Second, + Time: 1 * time.Minute, + Timeout: 20 * time.Second, + }), + grpc.MaxConcurrentStreams(1024), +) +``` +Expose the durations as `Options.GRPCKeepalive*` so operators with very-long-lived `AuthorizeStream` clients can tune them. + +Add a regression test that opens N+1 idle connections against a server with `MaxConnectionIdle=200ms` and asserts they are torn down after the deadline. + +--- + +### F15 — Info — `host` and `tenantId` body fields accept structural junk + +Pass-3 probes P4/P5 confirmed that the `/v1/authorize` handler accepts: +- `"host":"evil.example\r\nX-Injected: yes"` — CRLF in HTTP authority, +- `"tenantId":"T...×4096...\nFAKE-LOG-LINE level=ERROR ..."` — newline-laden, multi-KiB tenant id. + +Both values flow into `module.Request.{Host,TenantID}` and from there into: +- the `slog` JSON audit handler — *safe today* because JSON properly escapes `\r\n` as `\r\n` literal; +- any module that reflects them into HTTP response headers — *unsafe* because there is no validation between the door and the module. Today no in-tree module reflects host into a header value, but the surface is one config knob away (e.g. an OAuth2 module deriving `WWW-Authenticate: Bearer realm=""`). + +**Remediation.** Validate at the door: +- `host` must satisfy a strict authority regex (`^[A-Za-z0-9.-]+(:\d{1,5})?$`) — reject anything else with 400. +- `tenantId` must be ≤ 256 bytes and printable ASCII — same. + +--- + +## Updated remediation priority + +1. **F13** (this sprint) — credential-header array order-dependence; mirrors the F9 fix at the array layer. +2. **F14** (this sprint) — gRPC keepalive / connection-age / max-streams; one-time wiring in `buildGRPCServerOptions`. +3. **F15** (housekeeping) — input-shape validation on `host` / `tenantId`. +4. **F1, F3, F5** carried over from the first pass. + +F9, F10, F11, F12 are now closed (F12 by-design). + +--- + +# Pass 4 — OAuth2 PKCE cookbook + +**Scope:** [docs/cookbook/oauth2-pkce.md](docs/cookbook/oauth2-pkce.md), [examples/cookbook/oauth2-pkce/](examples/cookbook/oauth2-pkce) (`authconfig.yaml`, `idp.yaml`, `backend.yaml`, `values.yaml`, `run_flow.py`), and the lwauth code paths the cookbook exercises: [pkg/identity/jwt/jwt.go](pkg/identity/jwt/jwt.go), [pkg/authz/rbac/rbac.go](pkg/authz/rbac/rbac.go), and the chart's gateway template at [deploy/helm/lightweightauth/templates/gateway.yaml](deploy/helm/lightweightauth/templates/gateway.yaml). The IdP itself (`lightweightauth-idp`) is out of scope per engagement instructions. + +**Method:** static review of the AuthConfig + chart manifests + the JWT identifier and RBAC authorizer; cross-checked with `go doc` on `jwx/v2` to confirm library-side guarantees. No live cluster boot — the recipe is a kind+Cilium dryrun and findings are about the *recipe*, not a deployed instance. + +| ID | Severity | Issue | Status | +|---|---|---|---| +| **C1** | **High** | Cookbook AuthConfig `allow: [openid]` is effectively "any authenticated user" — `openid` is the universal scope every OIDC client requests to obtain an `id_token`. RBAC is presented as the authorization layer but enforces nothing beyond "the IdP minted you a token" | Open | +| **C2** | **High** | Gateway listener is plaintext HTTP on port 80 (NodePort). Bearer JWTs transit unencrypted from client → gateway. The cookbook ships no TLS termination guidance and the chart has no listener TLS knob in this code path | Open | +| **C3** | **Medium** | Authorization header (the bearer JWT) is forwarded verbatim to the upstream backend. The cookbook's `go-httpbin` echoes it; in any real deployment a backend that doesn't need the JWT will see and may log it. Chart has no default `header-remove` mutator and the cookbook AuthConfig doesn't add one | Open | +| **C4** | **Medium** | JWKS fetched over plaintext HTTP cluster-internal URL with no egress NetworkPolicy on lwauth and no mTLS to the IdP. A pod that can intercept service traffic (compromised node, sibling with `NET_RAW`, DNS poisoning) can serve attacker keys → forge tokens that pass verification | Open | +| **C5** | **Medium** | `lwauth-idp` Deployment has no NetworkPolicy: any pod (or kind-host network if exposed) can reach `:9090/oauth2/authorize`. The cookbook explicitly warns the IdP is dev-only with plaintext passwords, but the recipe shape teaches operators to treat it as a deployable component | Open | +| **C6** | **Medium** | [pkg/identity/jwt/jwt.go#L38-L46](pkg/identity/jwt/jwt.go#L38-L46) accepts an empty `audiences` list silently. With `issuerUrl` pinned but no audiences, **any** token signed by that issuer for **any** client is accepted. This is the classic "shared IdP" footgun (one IdP serving many apps; lwauth doesn't enforce that the token was minted *for this app*) | Open | +| C7 | Info | `issuerUrl` is exact-string-compared by `jwx`; `http://idp:9090` vs `http://idp:9090/` and IPv6/port-normalisation differences silently fail every token (DoS, fail-closed) | Informational | + +### Verified-safe controls (negative results) + +- **`alg` confusion:** `jwt.WithKeySet` in [jwx v2.1.6](go.mod#L10) explicitly **rejects** keys without an `alg` field rather than falling back to the JWS header (per `go doc github.com/lestrrat-go/jwx/v2/jwt.WithKeySet`: "keys MUST have a proper 'alg' field set ... we do NOT trust the token's headers"). lwauth therefore inherits a strong default; an HS256-with-RSA-public-key-as-secret confusion attack is library-blocked. +- **Open redirect on `/oauth2/start`:** Not exercised by this cookbook (no `oauth2` identifier). The code path that would matter for a different cookbook ([pkg/identity/oauth2/redirect.go#L30-L80](pkg/identity/oauth2/redirect.go#L30-L80)) handles `//evil`, `/\evil`, `javascript:`, embedded CRLF, and host allow-list bypasses correctly. +- **PKCE enforcement:** [pkg/identity/oauth2/flow.go#L80-L90](pkg/identity/oauth2/flow.go#L80-L90) hard-codes `code_challenge_method=S256`; downgrade to `plain` is not configurable. Verifier is 48 random bytes via `crypto/rand`. +- **State / CSRF on the OAuth flow:** [flow.go#L98-L110](pkg/identity/oauth2/flow.go#L98-L110) loads `flowState` from the encrypted flow cookie and rejects `flow.State != gotState`. Defence-in-depth `safeRedirect` re-runs on the cookie-stashed `RD` at callback time. +- **`failure_mode_allow`:** chart default in [deploy/helm/lightweightauth/values.yaml#L153-L157](deploy/helm/lightweightauth/values.yaml#L153-L157) is `false` — Envoy denies on lwauth unavailability rather than letting traffic through. + +--- + +### C1 — High — `allow: [openid]` is not authorization + +[examples/cookbook/oauth2-pkce/authconfig.yaml](examples/cookbook/oauth2-pkce/authconfig.yaml) line 33: +```yaml +authorizers: + - name: gate + type: rbac + config: + rolesFrom: claim:scopes + allow: [openid] +``` + +The `openid` value here is an **OIDC scope**, not a role. Every client that runs an Authorization-Code+PKCE flow against an OIDC IdP requests `scope=openid` to receive an `id_token` (RFC 6749 §3.3 + OIDC Core 1.0 §3.1.2.1). The IdP grants it to every successful login. Therefore `allow: [openid]` matches every authenticated user, including: +- a guest / self-registered account on the IdP, +- an attacker who compromises any one credential anywhere on the IdP, +- service accounts intended for unrelated apps. + +The cookbook narrative leads the reader to believe RBAC is "the authorization layer" — but the gate fires on a signal that is universal across all logins. The driver script ([run_flow.py](examples/cookbook/oauth2-pkce/run_flow.py)) only ever exercises the `alice` account, so the gap never surfaces. + +**Impact.** A reader who copies this AuthConfig into staging/production and relies on it for path-level authorization gets *authentication* (via the JWT identifier) but **no authorization**. Every JWT-bearing user reaches every backend. + +**Remediation.** Re-shape the cookbook's RBAC config so it actually demonstrates RBAC: + +```yaml +identifiers: + - name: jwt + type: jwt + config: + jwksUrl: http://lwauth-idp.demo.svc.cluster.local:9090/.well-known/jwks.json + issuerUrl: http://lwauth-idp.demo.svc.cluster.local:9090 + audiences: [lwauth-demo] +authorizers: + - name: gate + type: rbac + config: + # The IdP must mint a non-universal claim. Have lwauth-idp emit + # `groups: ["api-readers"]` for alice; the cookbook driver should + # prove a *deny* by attempting the flow as a user without the + # group and asserting 403, not just the current 200/401 pair. + rolesFrom: claim:groups + allow: [api-readers] +``` + +The driver should grow a fourth probe: log in as a no-group user and assert 403. Without a deny-because-of-RBAC probe the cookbook does not prove the gate works. + +--- + +### C2 — High — Plaintext bearer on the wire + +[deploy/helm/lightweightauth/templates/gateway.yaml#L33-L38](deploy/helm/lightweightauth/templates/gateway.yaml#L33-L38): +```yaml +listeners: +- name: ingress + address: { socket_address: { address: 0.0.0.0, port_value: 80 } } +``` + +Every call from the [run_flow.py](examples/cookbook/oauth2-pkce/run_flow.py) driver, and every call a real client would make, sends `Authorization: Bearer ` over plaintext HTTP. The kind cookbook uses `service.type: NodePort`, so the bearer is observable on the node's network interface — visible to any host on the same L2/L3 segment, and to any process inside the cluster that can `tcpdump` on `cni0`/`flannel`/`cilium_host`. + +The chart has no TLS-termination block in the gateway template, no `cert-manager` integration, no even-an-example downstream-TLS listener. The cookbook does not flag this — its "Known limitations" section lists plaintext passwords on the IdP but is silent on the gateway. + +**Impact.** Bearers (JWTs) are valid for 15 minutes. Anyone who sniffs one replays it for the full window. JWTs from this IdP are forwardable to any service that trusts the same JWKS. + +**Remediation.** +1. Add a `tls.yaml` template variant under [deploy/helm/lightweightauth/templates/](deploy/helm/lightweightauth/templates) that renders a `transport_socket: { name: envoy.transport_sockets.tls, ... }` block when `gateway.tls.enabled=true`. +2. Document the kind+`mkcert` recipe inline in the cookbook ("for production, terminate TLS at the gateway or front it with an Ingress that does"). +3. Set `secure: true` on any oauth2-identifier session cookies (when that flow is added to a future cookbook). + +--- + +### C3 — Medium — Bearer leakage to upstream backend + +[gateway.yaml#L42-L52](deploy/helm/lightweightauth/templates/gateway.yaml#L42-L52) renders a single Envoy route with no `request_headers_to_remove` and no `response_headers_to_remove`. The `ext_authz` filter does not strip headers it consumed. Result: `Authorization: Bearer ` is forwarded verbatim to the upstream cluster, which in the cookbook is `go-httpbin`. Calling `/get` on the gateway echoes the bearer back to the client in the response body. + +In production, this leaks long-lived access tokens to: +- backend logs (`access_log` / OpenTelemetry traces capture headers), +- backend SSRF surfaces (any HTTP client downstream that takes a URL parameter and follows it sees the bearer), +- backend error pages that reflect request headers. + +The cookbook's [authconfig.yaml](examples/cookbook/oauth2-pkce/authconfig.yaml) uses no `header-remove` mutator and the chart has no opinion. The reader has no nudge to add one. + +**Remediation.** Add a `responseMutators` block to the cookbook's AuthConfig and document the pattern: + +```yaml +responseMutators: + - name: strip-bearer + type: header-remove + config: + remove: [Authorization] +``` + +Alternative: render `request_headers_to_remove: [authorization]` on the route in [gateway.yaml](deploy/helm/lightweightauth/templates/gateway.yaml) when a chart value `gateway.stripBearer: true` (default) is set. + +--- + +### C4 — Medium — JWKS over plaintext HTTP, no egress hardening + +[examples/cookbook/oauth2-pkce/authconfig.yaml#L17](examples/cookbook/oauth2-pkce/authconfig.yaml#L17): +```yaml +jwksUrl: http://lwauth-idp.demo.svc.cluster.local:9090/.well-known/jwks.json +``` + +[pkg/identity/jwt/jwt.go#L113-L128](pkg/identity/jwt/jwt.go#L113-L128) registers this URL with `jwk.NewCache` and refreshes on a 15-minute timer (and on every `kid` miss). lwauth has no scheme allow-list — `file://`, `http://`, anything `jwx` accepts is accepted. The cookbook ships no NetworkPolicy on the lwauth deployment restricting egress to the IdP service only. + +**Threat.** Attacker who plants a pod with `NET_RAW` (or compromises a node) can: +1. ARP-spoof or DNS-poison the cluster service IP for `lwauth-idp.demo.svc.cluster.local`; +2. Serve their own JWKS on the spoofed endpoint; +3. Sign tokens with their key; +4. Tokens pass jwx verification because lwauth trusts whatever key the JWKS URL returns. + +Cilium NetworkPolicy in the sibling cookbook ([gate-upstream-service](examples/cookbook/gate-upstream-service)) restricts *ingress* to lwauth, not lwauth's *egress*. The OAuth2 cookbook does not even apply that policy. + +**Remediation.** +1. Add `kind: CiliumNetworkPolicy` (or `kind: NetworkPolicy` with an L4 egress rule) restricting lwauth's egress to `{namespace=demo, app=lwauth-idp}`. +2. Switch the recipe to `https://` once the IdP grows a TLS listener; document the `tls.caFile` knob lwauth's `jwt` module needs (currently uses Go's default cert pool — no recipe support for cluster-internal CAs). +3. Defence-in-depth in lwauth itself: scheme-validate `jwksUrl` at config load and refuse `file://` / `data:` / `unix:`; warn on `http://` when not pointing at `localhost`. + +--- + +### C5 — Medium — IdP exposed cluster-wide, no rate-limit, no NetworkPolicy + +[examples/cookbook/oauth2-pkce/idp.yaml](examples/cookbook/oauth2-pkce/idp.yaml) lays down the IdP Deployment + Service with no NetworkPolicy. Every pod in the cluster can hit `/oauth2/authorize`. The IdP is documented as plaintext-password / in-memory / rotating-key: it is **not** designed to be reachable as a real surface, but the cookbook deploys it like one. + +**Impact (recipe-shape).** A reader who copies the recipe into a non-throwaway cluster gets: +- credential brute-force surface (no rate-limit on `/oauth2/authorize`), +- a single-replica trust anchor for every JWT in the namespace. + +**Remediation.** Constrain reachability at the recipe layer: +```yaml +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: { name: lwauth-idp-restricted, namespace: demo } +spec: + podSelector: { matchLabels: { app: lwauth-idp } } + policyTypes: [Ingress] + ingress: + - from: + - podSelector: { matchLabels: { app.kubernetes.io/name: lightweightauth } } + - podSelector: { matchLabels: { app.kubernetes.io/component: gateway } } +``` + +And in the cookbook prose: an explicit "do not expose the IdP outside the cluster; the `kubectl port-forward` in `run_flow.py` is for the dryrun only" callout. + +--- + +### C6 — Medium — `audiences: []` silently disables aud pinning + +[pkg/identity/jwt/jwt.go#L41-L46](pkg/identity/jwt/jwt.go#L41-L46) declares: +```go +type Config struct { + JWKSURL string `yaml:"jwksUrl"` + IssuerURL string `yaml:"issuerUrl"` + Audiences []string `yaml:"audiences"` + ... +} +``` + +[jwt.go#L137-L142](pkg/identity/jwt/jwt.go#L137-L142): +```go +for _, a := range cfg.Audiences { + opts = append(opts, jwtlib.WithAudience(a)) +} +``` + +When `Audiences` is empty, no `WithAudience` is added — `jwx` then **does not enforce `aud`**. The factory does not warn. The cookbook gets this right (`audiences: [lwauth-demo]`), but the module is one missing line away from accepting any token from the issuer for any client. + +**Threat.** Operator running multiple lwauth deployments behind one IdP forgets the `audiences` line on one of them: a token minted for app B is now accepted by lwauth in front of app A. Combined with C3 (bearer leakage to upstream), an app-B backend that logs the bearer becomes the credential reservoir for a full app-A takeover. + +**Remediation.** In [jwt.go#L96-L120](pkg/identity/jwt/jwt.go#L96-L120) (`newIdentifier`), refuse a config that has `IssuerURL` set but `Audiences` empty (or warn loudly via `slog`). Concrete diff: + +```go +if cfg.IssuerURL != "" && len(cfg.Audiences) == 0 { + return nil, fmt.Errorf("%w: jwt: audiences must be set when issuerUrl is pinned"+ + " (empty audiences disables aud enforcement)", module.ErrConfig) +} +``` + +Add a unit test in [pkg/identity/jwt/jwt_test.go](pkg/identity/jwt/jwt_test.go) that asserts the factory returns a `module.ErrConfig` for that shape. + +--- + +### C7 — Info — `issuerUrl` exact-match brittleness + +`jwx`'s `WithIssuer` is a `==` compare. The cookbook's IdP launches with `--issuer=http://lwauth-idp.demo.svc.cluster.local:9090` (no trailing slash) and the AuthConfig pins the same string. If the operator pastes the issuer from the IdP's discovery doc (which often canonicalises with a trailing slash), every token fails. Fail-closed, but a real-world ergonomic landmine that surfaces as "everything is 401 and the audit log says `iss mismatch`". + +**Remediation.** In [jwt.go](pkg/identity/jwt/jwt.go), normalise both the configured issuer and the token's `iss` (strip trailing slash, lowercase scheme+host) before comparison; or accept either form via two `WithIssuer` calls. + +--- + +## Updated remediation priority + +1. **C1, C2, C6** — High-impact and small patches. C1 is doc + a config edit; C2 needs a chart template; C6 is one validation block in `newIdentifier`. +2. **C3, C4, C5** — recipe-shape remediation (chart values + cookbook prose + a NetworkPolicy). +3. **F13** (carry from pass 3 — done) — credential-header array order-dependence. +4. **F14** (carry — done) — gRPC keepalive / connection-age. +5. **F15** (carry — done) — input-shape validation on `host` / `tenantId`. +6. **F1, F3, F5** carried over from the first pass. + +F9, F10, F11, F12 closed (F12 by-design). F13/F14/F15 fixed and committed. + +--- + +## Engagement hygiene + +- **Processes stopped:** `lwauth.exe` (PID 31636), and rebuilt `lwauth-pt.exe` (PIDs 33532, 9880, 34148, 34804). Pass 4 was static review only — no daemon launched, no kind cluster booted. +- **Artefacts removed:** `pentest-quickstart.yaml`, `pentest-stdout.log`, `pentest-stderr.log`, `pentest-run.ps1`, `pentest_probes.py`, `pentest_timeouts.py`, `pentest_pass2.py`, `pentest_collision.py`, `pentest_pass3.py`, `bin/lwauth-pt.exe`. Pass 4 produced no probe scripts. +- **Verified clean:** `Get-ChildItem -Filter "pentest*"` returns only `pentest-report.md`; `Get-Process -Name lwauth*` is empty. +- **Production systems touched:** none. Scope held to the local `lightweightauth` repo as instructed. From 7d3ed973dc7e25f716cedbfdabbca31896e55501 Mon Sep 17 00:00:00 2001 From: mikeappsec Date: Fri, 1 May 2026 20:03:00 +1000 Subject: [PATCH 6/7] refactor: extract pkg/httputil with KeyedLimiter interface + token-bucket impl - New pkg/httputil package provides reusable HTTP middleware: - KeyedLimiter interface (Allow(key) bool) - TokenBucketLimiter: per-key token-bucket with injectable clock - RateLimitMiddleware / RateLimitHandler: 429 + Retry-After wrapper - IPKeyFunc: common key extractor - Refactored internal/admin/handler.go to use pkg/httputil instead of inline rate-limiter struct, reducing ~45 lines of duplicated logic. - Full test coverage in pkg/httputil/ratelimit_test.go. --- internal/admin/handler.go | 78 ++++------------------ pkg/httputil/ratelimit.go | 116 +++++++++++++++++++++++++++++++++ pkg/httputil/ratelimit_test.go | 111 +++++++++++++++++++++++++++++++ 3 files changed, 239 insertions(+), 66 deletions(-) create mode 100644 pkg/httputil/ratelimit.go create mode 100644 pkg/httputil/ratelimit_test.go diff --git a/internal/admin/handler.go b/internal/admin/handler.go index a833dd4..3d73556 100644 --- a/internal/admin/handler.go +++ b/internal/admin/handler.go @@ -3,55 +3,13 @@ package admin import ( "encoding/json" "net/http" - "sync" - "time" -) - -// adminRateLimiter is a simple per-IP token bucket for admin endpoints (TC4). -type adminRateLimiter struct { - mu sync.Mutex - buckets map[string]*bucket - rps float64 - burst int -} - -type bucket struct { - tokens float64 - lastTime time.Time -} - -func newAdminRateLimiter(rps float64, burst int) *adminRateLimiter { - return &adminRateLimiter{ - buckets: make(map[string]*bucket), - rps: rps, - burst: burst, - } -} - -func (rl *adminRateLimiter) allow(ip string) bool { - rl.mu.Lock() - defer rl.mu.Unlock() - - now := time.Now() - b, ok := rl.buckets[ip] - if !ok { - b = &bucket{tokens: float64(rl.burst), lastTime: now} - rl.buckets[ip] = b - } - elapsed := now.Sub(b.lastTime).Seconds() - b.tokens += elapsed * rl.rps - if b.tokens > float64(rl.burst) { - b.tokens = float64(rl.burst) - } - b.lastTime = now + "github.com/mikeappsec/lightweightauth/pkg/httputil" +) - if b.tokens < 1 { - return false - } - b.tokens-- - return true -} +// adminLimiter is the package-level rate limiter for admin endpoints. +// 10 requests/second per IP with a burst of 20. +var adminLimiter = httputil.NewTokenBucketLimiter(10, 20) // validScopes is the allowlist for cache invalidation scope (TC7). var validScopes = map[string]bool{ @@ -60,10 +18,6 @@ var validScopes = map[string]bool{ "subject": true, } -// adminLimiter is the package-level rate limiter for admin endpoints. -// 10 requests/second per IP with a burst of 20. -var adminLimiter = newAdminRateLimiter(10, 20) - // NewAdminMux returns an http.Handler that serves all /v1/admin/ endpoints, // protected by the given middleware. If the middleware is nil or disabled, // all routes return 404. @@ -77,33 +31,25 @@ func NewAdminMux(mw *Middleware) http.Handler { return mux } - // TC4: rate-limit wrapper applied to all admin routes. - rateLimit := func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ip := r.RemoteAddr - if !adminLimiter.allow(ip) { - w.Header().Set("Retry-After", "1") - writeAdminError(w, http.StatusTooManyRequests, "rate limit exceeded") - return - } - next.ServeHTTP(w, r) - }) + // TC4: rate-limit all admin routes via shared httputil limiter. + rl := func(next http.Handler) http.Handler { + return httputil.RateLimitMiddleware(adminLimiter, httputil.IPKeyFunc, next) } // GET /v1/admin/status — engine and config status. - mux.Handle("/v1/admin/status", rateLimit(mw.Require(VerbReadStatus, + mux.Handle("/v1/admin/status", rl(mw.Require(VerbReadStatus, http.HandlerFunc(handleStatus)))) // POST /v1/admin/cache/invalidate — manual cache invalidation. - mux.Handle("/v1/admin/cache/invalidate", rateLimit(mw.Require(VerbInvalidateCache, + mux.Handle("/v1/admin/cache/invalidate", rl(mw.Require(VerbInvalidateCache, http.HandlerFunc(handleCacheInvalidate)))) // POST /v1/admin/revoke — token/session revocation (stub for E2). - mux.Handle("/v1/admin/revoke", rateLimit(mw.Require(VerbRevokeToken, + mux.Handle("/v1/admin/revoke", rl(mw.Require(VerbRevokeToken, http.HandlerFunc(handleRevoke)))) // GET /v1/admin/audit — audit log query (stub for D4). - mux.Handle("/v1/admin/audit", rateLimit(mw.Require(VerbReadAudit, + mux.Handle("/v1/admin/audit", rl(mw.Require(VerbReadAudit, http.HandlerFunc(handleAuditQuery)))) return mux diff --git a/pkg/httputil/ratelimit.go b/pkg/httputil/ratelimit.go new file mode 100644 index 0000000..3e13a9d --- /dev/null +++ b/pkg/httputil/ratelimit.go @@ -0,0 +1,116 @@ +// Package httputil provides reusable HTTP middleware building blocks +// (rate limiting, request guards, etc.) that are independent of the +// authentication engine. They are consumed by the admin plane, the +// optional IdP sidecar, and any future HTTP surface that needs +// per-key request gating. +package httputil + +import ( + "net/http" + "sync" + "time" +) + +// KeyedLimiter is the interface any per-key rate limiter must satisfy. +// Implementations may use token-bucket, sliding-window, or distributed +// backends. The key is typically a client IP or username. +type KeyedLimiter interface { + // Allow returns true if the request identified by key is permitted. + Allow(key string) bool +} + +// ---------- token-bucket implementation ---------- + +// TokenBucketLimiter is a goroutine-safe, per-key token-bucket rate +// limiter suitable for in-process use. For distributed (multi-replica) +// limiting, see [pkg/ratelimit] which speaks to a Valkey backend. +type TokenBucketLimiter struct { + mu sync.Mutex + buckets map[string]*tokenBucket + rps float64 + burst int + now func() time.Time // injectable for tests +} + +type tokenBucket struct { + tokens float64 + lastTime time.Time +} + +// TokenBucketOption configures a TokenBucketLimiter. +type TokenBucketOption func(*TokenBucketLimiter) + +// WithClock overrides the time source (useful for deterministic tests). +func WithClock(fn func() time.Time) TokenBucketOption { + return func(l *TokenBucketLimiter) { l.now = fn } +} + +// NewTokenBucketLimiter returns a KeyedLimiter backed by per-key token +// buckets refilling at rps tokens/sec with a maximum burst capacity. +func NewTokenBucketLimiter(rps float64, burst int, opts ...TokenBucketOption) *TokenBucketLimiter { + l := &TokenBucketLimiter{ + buckets: make(map[string]*tokenBucket), + rps: rps, + burst: burst, + now: time.Now, + } + for _, o := range opts { + o(l) + } + return l +} + +// Allow implements KeyedLimiter. +func (l *TokenBucketLimiter) Allow(key string) bool { + l.mu.Lock() + defer l.mu.Unlock() + + now := l.now() + b, ok := l.buckets[key] + if !ok { + b = &tokenBucket{tokens: float64(l.burst), lastTime: now} + l.buckets[key] = b + } + + elapsed := now.Sub(b.lastTime).Seconds() + b.tokens += elapsed * l.rps + if b.tokens > float64(l.burst) { + b.tokens = float64(l.burst) + } + b.lastTime = now + + if b.tokens < 1 { + return false + } + b.tokens-- + return true +} + +// ---------- HTTP middleware ---------- + +// KeyFunc extracts the rate-limit key from a request (e.g. RemoteAddr). +type KeyFunc func(r *http.Request) string + +// IPKeyFunc is the most common KeyFunc: it keys on r.RemoteAddr. +func IPKeyFunc(r *http.Request) string { return r.RemoteAddr } + +// RateLimitMiddleware returns an http.Handler that gates requests +// through limiter. Denied requests receive 429 with Retry-After. +// keyFn determines the bucket key; pass IPKeyFunc for per-IP limiting. +func RateLimitMiddleware(limiter KeyedLimiter, keyFn KeyFunc, next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + key := keyFn(r) + if !limiter.Allow(key) { + w.Header().Set("Retry-After", "1") + http.Error(w, `{"error":"rate limit exceeded"}`, http.StatusTooManyRequests) + return + } + next.ServeHTTP(w, r) + }) +} + +// RateLimitHandler is a convenience wrapper that applies RateLimitMiddleware +// to a single handler. +func RateLimitHandler(limiter KeyedLimiter, keyFn KeyFunc, next http.HandlerFunc) http.Handler { + return RateLimitMiddleware(limiter, keyFn, next) +} diff --git a/pkg/httputil/ratelimit_test.go b/pkg/httputil/ratelimit_test.go new file mode 100644 index 0000000..d734d94 --- /dev/null +++ b/pkg/httputil/ratelimit_test.go @@ -0,0 +1,111 @@ +package httputil + +import ( + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" +) + +func TestTokenBucketLimiter_AllowBurst(t *testing.T) { + l := NewTokenBucketLimiter(10, 3) // 10 rps, burst 3 + key := "127.0.0.1" + + // First 3 should be allowed (burst). + for i := range 3 { + if !l.Allow(key) { + t.Fatalf("request %d should be allowed within burst", i+1) + } + } + // 4th should be denied. + if l.Allow(key) { + t.Fatal("request 4 should be denied after burst exhausted") + } +} + +func TestTokenBucketLimiter_Refill(t *testing.T) { + now := time.Now() + mu := sync.Mutex{} + clock := func() time.Time { + mu.Lock() + defer mu.Unlock() + return now + } + advance := func(d time.Duration) { + mu.Lock() + now = now.Add(d) + mu.Unlock() + } + + l := NewTokenBucketLimiter(10, 1, WithClock(clock)) + key := "10.0.0.1" + + if !l.Allow(key) { + t.Fatal("first request should pass") + } + if l.Allow(key) { + t.Fatal("second request should be denied (burst=1)") + } + // Advance 100ms → refill 1 token (10 rps × 0.1s). + advance(100 * time.Millisecond) + if !l.Allow(key) { + t.Fatal("should pass after refill") + } +} + +func TestTokenBucketLimiter_PerKey(t *testing.T) { + l := NewTokenBucketLimiter(1, 1) + if !l.Allow("a") { + t.Fatal("key a should pass") + } + if !l.Allow("b") { + t.Fatal("key b should pass independently") + } + if l.Allow("a") { + t.Fatal("key a should be denied after burst") + } +} + +func TestRateLimitMiddleware_Returns429(t *testing.T) { + // Limiter with burst=0 denies everything. + l := NewTokenBucketLimiter(0, 0) + + handler := RateLimitMiddleware(l, IPKeyFunc, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "1.2.3.4:5678" + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusTooManyRequests { + t.Fatalf("expected 429, got %d", rec.Code) + } + if rec.Header().Get("Retry-After") != "1" { + t.Fatal("missing Retry-After header") + } +} + +func TestRateLimitMiddleware_PassesThrough(t *testing.T) { + l := NewTokenBucketLimiter(100, 10) + + called := false + handler := RateLimitMiddleware(l, IPKeyFunc, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusOK) + })) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "1.2.3.4:5678" + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if !called { + t.Fatal("next handler should have been called") + } + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", rec.Code) + } +} From 0d62f5a161eafe48b2986278b66e25d35e43f7e5 Mon Sep 17 00:00:00 2001 From: mikeappsec Date: Fri, 1 May 2026 20:09:10 +1000 Subject: [PATCH 7/7] fix(httputil): RL1 strip port from IP key, RL2 add bucket eviction, RL4 dynamic Retry-After - RL1 (High): IPKeyFunc now uses net.SplitHostPort to strip ephemeral port, ensuring all connections from the same IP share one bucket. - RL2 (Medium): Background goroutine evicts idle buckets every 60s (threshold = max(burst/rps, 60s)) to bound memory growth. - RL4 (Info): Retry-After computed from 1/rps instead of hardcoded 1. - Added TestIPKeyFunc_StripsPort covering IPv4 and IPv6. --- pentest-report-ratelimiter.md | 158 +++++++++++++++++++++++++++++++++ pkg/httputil/ratelimit.go | 56 +++++++++++- pkg/httputil/ratelimit_test.go | 16 +++- 3 files changed, 226 insertions(+), 4 deletions(-) create mode 100644 pentest-report-ratelimiter.md diff --git a/pentest-report-ratelimiter.md b/pentest-report-ratelimiter.md new file mode 100644 index 0000000..6efda10 --- /dev/null +++ b/pentest-report-ratelimiter.md @@ -0,0 +1,158 @@ +# Penetration Test Report — Rate Limiter & TC1–TC10 Fix Verification + +**Date:** 2026-05-01 (pass-2) +**Tester:** Senior penetration tester (authorized engagement) +**Target:** `lightweightauth` repository — post-TC fix verification + new `pkg/httputil` rate limiter +**Scope:** Verify TC1–TC10 remediations; pen-test the new `pkg/httputil.KeyedLimiter` interface and `TokenBucketLimiter` implementation. + +--- + +## Executive Summary + +| # | Severity | Component | Title | State | +|---|---|---|---|---| +| **RL1** | **High** | `pkg/httputil` | `IPKeyFunc` keys on `RemoteAddr` (includes port) — every new TCP connection gets a fresh bucket, rate limit is **completely bypassable** | Open | +| **RL2** | **Medium** | `pkg/httputil` | No bucket eviction — unbounded memory growth (OOM via distinct-key flood) | Open | +| **RL3** | **Low** | `pkg/httputil` | Per-replica-only limiting — effective rate = configured × replica count | Informational (documented) | +| **RL4** | **Info** | `pkg/httputil` | Fixed `Retry-After: 1` regardless of actual refill time | Informational | + +### TC Fix Verification + +| Original | Fix Applied | Verification | +|---|---|---| +| TC1 (kubectl injection) | — | ⚠️ **Not yet fixed** — no validation visible in `gitops.go` | +| TC2 (OR composition) | `RequireMTLS` field added | ✅ AND mode enforces both JWT+mTLS when enabled | +| TC3 (JWT replay) | `MaxTokenLifetime` cap (default 15m) | ✅ Tokens with `exp-iat > 15m` rejected | +| TC4 (No rate-limit) | `httputil.RateLimitMiddleware` wired in `handler.go` | ⚠️ Partially fixed — see RL1 (bypass) | +| TC5 (HTTPS scheme) | `strings.HasPrefix(JWKSURL, "https://")` check | ✅ Non-HTTPS rejected at config time | +| TC6 (Wildcard warn) | `slog.Warn` on wildcard role at init | ✅ Warning emitted | +| TC7 (Scope validation) | `validScopes` allowlist in `handler.go` | ✅ Unknown scopes → 400 | +| TC8 (--out traversal) | — | ⚠️ **Not yet fixed** — `os.Create(*outPath)` still unconstrained | +| TC9 (Verb leak in 403) | Changed to generic `"forbidden"` | ✅ Specific verb no longer in wire response | +| TC10 (Digest order) | — | ⚠️ Informational — still uses `json.Marshal` | + +--- + +## New Findings + +### RL1 — High — Rate Limiter Bypass via Port-Inclusive Key + +**Component:** `pkg/httputil/ratelimit.go`, `IPKeyFunc` + +**Description:** +```go +func IPKeyFunc(r *http.Request) string { return r.RemoteAddr } +``` + +Go's `http.Request.RemoteAddr` includes the port (e.g., `"192.168.1.5:49832"`). Since each new TCP connection uses an ephemeral source port, **every connection gets its own unique bucket**. The rate limiter is effectively a no-op. + +**Proof of concept:** +```python +import urllib.request +for i in range(1000): + # Each connection uses a new ephemeral port → new bucket → allowed + urllib.request.urlopen("https://admin:9443/v1/admin/status") + # All 1000 requests succeed despite 10 rps / burst 20 config +``` + +Even with HTTP/1.1 keep-alive (which reuses the same connection), the moment a client opens a new socket (e.g., concurrent requests, connection pool refresh), it bypasses the limit. + +**Impact:** TC4 (rate-limiting admin endpoints) is effectively not mitigated. An attacker can: +- Cache-bust at unlimited rate via `/v1/admin/cache/invalidate` +- CPU-burn the JWT validation path with malformed tokens at unlimited rate +- When revocation/audit land, abuse those endpoints without throttle + +**Recommendation:** +Strip the port from `RemoteAddr`: + +```go +func IPKeyFunc(r *http.Request) string { + host, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + return r.RemoteAddr // fallback for unix sockets etc. + } + return host +} +``` + +Additionally, consider supporting `X-Forwarded-For` / `X-Real-IP` for deployments behind a reverse proxy (with a trusted-proxy allowlist to prevent spoofing). + +--- + +### RL2 — Medium — Unbounded Bucket Map Growth (Memory Exhaustion) + +**Component:** `pkg/httputil/ratelimit.go`, `TokenBucketLimiter` + +**Description:** +The `buckets map[string]*tokenBucket` grows indefinitely. There is no eviction, TTL, or maximum-entries cap. An attacker who can present many distinct keys can exhaust process memory: + +1. **Behind a proxy with `X-Forwarded-For` KeyFunc:** Attacker sends requests with randomized `X-Forwarded-For` headers → millions of buckets created. +2. **Direct connections (even with fixed IPKeyFunc):** With the current port-inclusive key (RL1), each connection creates a bucket entry that is never cleaned up. Over hours/days, bucket count = total connections served. + +**Memory impact estimate:** +Each bucket entry ≈ 80 bytes (key ~22 bytes + struct 24 bytes + map overhead). At 1M unique keys = ~80 MB of non-reclaimable heap growth. + +**Recommendation:** +1. Add a background goroutine that evicts buckets not seen within 2× the refill period. +2. Or use a fixed-size LRU (`sync.Map` with bounded size, or a sharded LRU). +3. Add a `MaxBuckets` config option that starts rejecting new keys (429) when exceeded. + +--- + +### RL3 — Low — Per-Replica Limiting Only + +**Component:** `pkg/httputil/ratelimit.go` (design) + +**Description:** +The in-process `TokenBucketLimiter` is per-replica. With N replicas behind a load balancer, the effective rate limit is `N × configured_rate`. The code comments reference a `pkg/ratelimit` package for distributed limiting, but it is not wired into the admin mux. + +For a 3-replica deployment with 10 rps configured, the effective admin rate limit is 30 rps — still better than nothing, but operators should be aware. + +**Recommendation:** Document this clearly in operator docs. Consider wiring the distributed backend for admin endpoints in Tier E. + +--- + +### RL4 — Info — Static `Retry-After` Header + +**Component:** `pkg/httputil/ratelimit.go`, `RateLimitMiddleware` + +**Description:** +```go +w.Header().Set("Retry-After", "1") +``` + +The `Retry-After` value is hardcoded to `"1"` (1 second) regardless of actual token refill time. For a 10 rps limiter, the actual wait is 100ms. For a 1 rps limiter, it's 1000ms. This is a minor protocol-correctness issue — compliant clients will wait longer than necessary. + +**Recommendation:** Compute from `1.0 / rps` or expose the refill interval. + +--- + +## Remaining Open Items from TC Report + +### TC1 — Still Open — `drift` Argument Injection + +The `kubectlJSONPath` function in [cmd/lwauthctl/gitops.go](cmd/lwauthctl/gitops.go) still passes the derived name unsanitized. No Kubernetes resource-name validation or `--` separator was added. + +### TC8 — Still Open — `--out` Path Traversal + +`promote` and `rollback` still call `os.Create(*outPath)` with no path restriction. + +--- + +## Updated Regression Status (F1–F15) + +All previously fixed findings (F1–F15) remain mitigated. No regressions introduced by the rate limiter addition. + +--- + +## Recommendations (Priority Order) + +1. **RL1 (Critical):** Fix `IPKeyFunc` to strip port via `net.SplitHostPort`. This is a one-line fix that makes the rate limiter actually functional. +2. **RL2 (Medium):** Add bucket eviction (GC goroutine or LRU cap). +3. **TC1 (High, still open):** Validate derived resource names in `drift`. +4. **TC8 (Low, still open):** Constrain `--out` path. +5. **RL3/RL4 (Low/Info):** Document per-replica behavior; compute `Retry-After` dynamically. + +--- + +*End of report.* diff --git a/pkg/httputil/ratelimit.go b/pkg/httputil/ratelimit.go index 3e13a9d..2677b05 100644 --- a/pkg/httputil/ratelimit.go +++ b/pkg/httputil/ratelimit.go @@ -6,6 +6,8 @@ package httputil import ( + "fmt" + "net" "net/http" "sync" "time" @@ -47,6 +49,8 @@ func WithClock(fn func() time.Time) TokenBucketOption { // NewTokenBucketLimiter returns a KeyedLimiter backed by per-key token // buckets refilling at rps tokens/sec with a maximum burst capacity. +// A background goroutine evicts idle buckets every 60 s (buckets not +// seen for ≥ 2× the full-refill period are removed). func NewTokenBucketLimiter(rps float64, burst int, opts ...TokenBucketOption) *TokenBucketLimiter { l := &TokenBucketLimiter{ buckets: make(map[string]*tokenBucket), @@ -57,9 +61,38 @@ func NewTokenBucketLimiter(rps float64, burst int, opts ...TokenBucketOption) *T for _, o := range opts { o(l) } + // RL2: start background eviction to bound memory. + go l.evictLoop() return l } +// evictLoop removes buckets that have been idle long enough to be +// fully refilled (i.e. they would start at max burst anyway). +func (l *TokenBucketLimiter) evictLoop() { + // Evict every 60s; idle threshold = time to fully refill burst. + ticker := time.NewTicker(60 * time.Second) + defer ticker.Stop() + var idleThreshold time.Duration + if l.rps > 0 { + idleThreshold = time.Duration(float64(l.burst)/l.rps*1e9) * time.Nanosecond + if idleThreshold < 60*time.Second { + idleThreshold = 60 * time.Second + } + } else { + idleThreshold = 60 * time.Second + } + for range ticker.C { + now := l.now() + l.mu.Lock() + for k, b := range l.buckets { + if now.Sub(b.lastTime) > idleThreshold { + delete(l.buckets, k) + } + } + l.mu.Unlock() + } +} + // Allow implements KeyedLimiter. func (l *TokenBucketLimiter) Allow(key string) bool { l.mu.Lock() @@ -91,17 +124,34 @@ func (l *TokenBucketLimiter) Allow(key string) bool { // KeyFunc extracts the rate-limit key from a request (e.g. RemoteAddr). type KeyFunc func(r *http.Request) string -// IPKeyFunc is the most common KeyFunc: it keys on r.RemoteAddr. -func IPKeyFunc(r *http.Request) string { return r.RemoteAddr } +// IPKeyFunc extracts the client IP from RemoteAddr, stripping the +// ephemeral port so all connections from the same IP share one bucket. +func IPKeyFunc(r *http.Request) string { + host, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + return r.RemoteAddr // fallback (e.g. unix socket) + } + return host +} // RateLimitMiddleware returns an http.Handler that gates requests // through limiter. Denied requests receive 429 with Retry-After. // keyFn determines the bucket key; pass IPKeyFunc for per-IP limiting. func RateLimitMiddleware(limiter KeyedLimiter, keyFn KeyFunc, next http.Handler) http.Handler { + // Compute a sensible Retry-After from the limiter if possible. + retryAfter := "1" + if tb, ok := limiter.(*TokenBucketLimiter); ok && tb.rps > 0 { + secs := 1.0 / tb.rps + if secs < 1 { + retryAfter = "1" + } else { + retryAfter = fmt.Sprintf("%d", int(secs)+1) + } + } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { key := keyFn(r) if !limiter.Allow(key) { - w.Header().Set("Retry-After", "1") + w.Header().Set("Retry-After", retryAfter) http.Error(w, `{"error":"rate limit exceeded"}`, http.StatusTooManyRequests) return } diff --git a/pkg/httputil/ratelimit_test.go b/pkg/httputil/ratelimit_test.go index d734d94..d544fed 100644 --- a/pkg/httputil/ratelimit_test.go +++ b/pkg/httputil/ratelimit_test.go @@ -83,11 +83,25 @@ func TestRateLimitMiddleware_Returns429(t *testing.T) { if rec.Code != http.StatusTooManyRequests { t.Fatalf("expected 429, got %d", rec.Code) } - if rec.Header().Get("Retry-After") != "1" { + if rec.Header().Get("Retry-After") == "" { t.Fatal("missing Retry-After header") } } +func TestIPKeyFunc_StripsPort(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "10.0.0.1:12345" + if got := IPKeyFunc(req); got != "10.0.0.1" { + t.Fatalf("expected 10.0.0.1, got %q", got) + } + + // IPv6 + req.RemoteAddr = "[::1]:9999" + if got := IPKeyFunc(req); got != "::1" { + t.Fatalf("expected ::1, got %q", got) + } +} + func TestRateLimitMiddleware_PassesThrough(t *testing.T) { l := NewTokenBucketLimiter(100, 10)