From 9466449f840c8ff445fcb247a5a2246ad59ae801 Mon Sep 17 00:00:00 2001 From: "SREDevOps.org Agent" Date: Tue, 3 Nov 2026 01:03:06 -0300 Subject: [PATCH] fix(exposure): check SSO RBAC and NetworkPolicies in preflight (#47) The preflight is the documented gate before real SSO traffic, but it did not check two things the dsh-demo install got wrong: - The gateway Role must grant `patch` on extensions.agents.x-k8s.io/sandboxclaims. The SSO landing path rotates the claim's token-hash annotation whenever the browser has no valid gateway session (first visit, expired or foreign cookie, gateway restart), so a missing verb makes every such sign-in answer 503. Verify with `kubectl auth can-i --as`; when a read-only kubeconfig cannot impersonate, fall back to reading the rules of the Role (or ClusterRole) the RoleBinding points at. - The variant's NetworkPolicies must be applied. Without them any Pod in the cluster can reach dsh-gateway:8080 and spoof X-Forwarded-User, so the exposure is demo-only. Fail the preflight while either policy from 75-networkpolicies.yaml is missing. Also fix a false positive in the cookie-secret check: oauth2-proxy accepts 16, 24 or 32 raw bytes, or URL-safe unpadded base64 that decodes to one of those lengths (pkg/encryption.SecretBytes). The documented `secrets.token_urlsafe(32)` value is 43 base64 characters that decode to 32 bytes, so it is valid and must not be reported as 43 raw bytes. The docs that enumerate the preflight checks are updated to match, and 75-networkpolicies.yaml notes that the preflight enforces it. Verified with shellcheck, yamllint, make check and the gateway test suite (150 passed); the new checks were exercised read-only against the live dsh-demo cluster. Entire-Checkpoint: 01M6FNY2WTC4GMAJ8BAHABN88M Co-authored-by: SREDevOpsAgents --- SECURITY.md | 3 +- examples/dsh-agents-as-a-service/README.md | 8 +- .../docs/architecture.md | 5 +- .../docs/optional-hardening.md | 6 +- .../manifests/exposure/README.md | 6 +- .../gateway-api-sso/75-networkpolicies.yaml | 1 + .../scripts/preflight-sso.sh | 111 ++++++++++++++++-- 7 files changed, 117 insertions(+), 23 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 0c4efd8..e147cd4 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -35,7 +35,8 @@ disclosure once a fix is available. exposure variant's policy is required, not optional: an exposure without it is demo-only. Run `examples/dsh-agents-as-a-service/scripts/preflight-sso.sh` after applying the variant; it checks the Secrets, gateway env, TLS - reference, leftover placeholders and accidental second paths. + reference, leftover placeholders, accidental second paths, the gateway + Role's `patch` verb on `sandboxclaims` and the variant NetworkPolicies. - **Insecure-cookie guard.** TLS terminates outside the gateway. Set COOKIE_SECURE=true for HTTPS and remove ALLOW_INSECURE_COOKIES; the gateway refuses to start with COOKIE_SECURE=false unless ALLOW_INSECURE_COOKIES=1 is diff --git a/examples/dsh-agents-as-a-service/README.md b/examples/dsh-agents-as-a-service/README.md index 48546c8..a5b66ff 100644 --- a/examples/dsh-agents-as-a-service/README.md +++ b/examples/dsh-agents-as-a-service/README.md @@ -518,8 +518,10 @@ front of **oauth2-proxy**, which authenticates the browser against GitHub reports **before** pointing real traffic at the Gateway. It verifies the Secrets (including a valid oauth2-proxy cookie secret length), the gateway's `AUTH_PROXY_HEADER`/`COOKIE_SECURE`, that the Gateway's TLS certificate - reference resolves, and that no `your-github-org`/`dsh-demo.example.com` - placeholder was left applied. Full verification steps are in the + reference resolves, that the gateway ServiceAccount may `patch` + `sandboxclaims` (SSO token rotation), that the variant's NetworkPolicies are + applied, and that no `your-github-org`/`dsh-demo.example.com` placeholder was + left applied. Full verification steps are in the [exposure README](manifests/exposure/README.md). > **Exposure without the variant NetworkPolicy is demo-only.** The SSO @@ -599,7 +601,7 @@ applied by the base step. | Every browser is logged out after a gateway rollout/restart | Expected: sessions are opaque ids in gateway memory and a restart drops them. Sign in again; a production deployment needs shared session state or per-request re-authentication. The DSH sandboxes and their data are unaffected | | WebSocket closes with `1011` for a busy client | `MAX_IN_FLIGHT_PER_USER` (per-user connection cap) or an upstream/handshake failure closed the socket; check the gateway logs to tell them apart | | WebSocket closes after a very large message from DSH | A message received from DSH exceeded `WS_MAX_MESSAGE_BYTES` (16 MiB), so the relay closed it instead of buffering; raise the limit only if DSH legitimately sends larger frames. Browser-to-DSH frames are bounded by uvicorn's own limit | -| `scripts/preflight-sso.sh` exits non-zero | It names the failing item: a missing Secret/key, a bad oauth2-proxy cookie-secret length, missing `AUTH_PROXY_HEADER`/`COOKIE_SECURE`, a dangling Gateway TLS reference, a leftover `your-github-org`/`dsh-demo.example.com` placeholder, or a second path to the gateway (an Ingress). Fix all items before exposing the SSO variant | +| `scripts/preflight-sso.sh` exits non-zero | It names the failing item: a missing Secret/key, a bad oauth2-proxy cookie-secret length, missing `AUTH_PROXY_HEADER`/`COOKIE_SECURE`, a dangling Gateway TLS reference, a leftover `your-github-org`/`dsh-demo.example.com` placeholder, a second path to the gateway (an Ingress), a gateway Role without `patch` on `sandboxclaims`, or a missing SSO NetworkPolicy. Fix all items before exposing the SSO variant | | `409` on signup | User already has a claim; use `POST /login` with the existing token or delete the user first | | Cookie set but browser keeps getting `401` | `COOKIE_SECURE=true` while serving over plain HTTP (secure cookies are not sent); use HTTPS or set `false` **and** `ALLOW_INSECURE_COOKIES=1` for local demos | | WebSocket closes immediately (`4401`) | Missing/expired gateway cookie, or the upgrade's `Origin` does not match `Host` (CSWSH check); DSH rejecting `Host` instead surfaces as close `1011` — add the host to `DSH_TRUSTED_HOSTS` | diff --git a/examples/dsh-agents-as-a-service/docs/architecture.md b/examples/dsh-agents-as-a-service/docs/architecture.md index 38a63ed..45bc341 100644 --- a/examples/dsh-agents-as-a-service/docs/architecture.md +++ b/examples/dsh-agents-as-a-service/docs/architecture.md @@ -395,7 +395,8 @@ Notes and limits: - **SSO is trust-on-hop.** `AUTH_PROXY_HEADER` makes the gateway trust the proxy-asserted identity; the SSO variant must remain the only path to the Service, and its NetworkPolicy is required, not optional. Run [`scripts/preflight-sso.sh`](../scripts/preflight-sso.sh) after - applying it. + applying it: it fails when the Role does not grant `patch` on `sandboxclaims` (SSO token + rotation) or when the variant's NetworkPolicies are missing. ## Failure modes @@ -412,7 +413,7 @@ Notes and limits: | Per-user concurrency cap hit | HTTP `503` + `Retry-After`, or WebSocket close `1011` | `MAX_IN_FLIGHT_PER_USER` protects the single gateway from one busy tenant; reduce concurrency or raise the cap deliberately. These are per replica | | Oversized WebSocket message from DSH | Connection closes | `WS_MAX_MESSAGE_BYTES` (16 MiB) is enforced on upstream messages; raise it only if DSH legitimately needs larger frames | | Session expiry / revocation | `401` (HTTP) or close `4401` (WebSocket) | Expected after logout, user delete, `/recreate`, `COOKIE_MAX_AGE` expiry or a gateway restart; sign in again | -| SSO variant applied without its NetworkPolicy (or with an Ingress) | Gateway accepts spoofed `X-Forwarded-User` | Demo-only configuration; keep `75-networkpolicies.yaml`, remove the second path, and run `scripts/preflight-sso.sh` | +| SSO variant applied without its NetworkPolicy (or with an Ingress) | Gateway accepts spoofed `X-Forwarded-User` | Demo-only configuration; keep `75-networkpolicies.yaml`, remove the second path, and run `scripts/preflight-sso.sh`, which fails when either policy is missing | | Browser aborts a request (tab reload, back navigation) during a slow wake or handshake | No response reaches the client; nothing appears in the access log | If the abort lands before the response starts, the body read raises `ClientDisconnect` and the app answers `499` (`CLIENT_CLOSED_REQUEST`), which cannot reach the departed peer. Nothing reached DSH, no ERROR traceback is logged, and the in-flight slot is released. A request already streaming ends with its upstream status. | | DSH auth cookie expired mid-session | Proxy `401` once, then recovers | Gateway refreshes the handshake once | | Upstream pod name changed after resume | Cached cookie keyed by old pod UID is useless | Cache is invalidated on suspend; next request re-handshakes | diff --git a/examples/dsh-agents-as-a-service/docs/optional-hardening.md b/examples/dsh-agents-as-a-service/docs/optional-hardening.md index 7ad8137..9742727 100644 --- a/examples/dsh-agents-as-a-service/docs/optional-hardening.md +++ b/examples/dsh-agents-as-a-service/docs/optional-hardening.md @@ -338,8 +338,10 @@ The SSO policy is what turns the trust boundary in the The `dsh-gateway` Service must still stay ClusterIP; do not apply both exposure variants. After applying the SSO variant, run [`scripts/preflight-sso.sh`](../scripts/preflight-sso.sh): it checks the -gateway's SSO env, the Secrets, the TLS reference, leftover placeholders and -accidental second paths (such as an Ingress). +gateway's SSO env, the Secrets, the TLS reference, leftover placeholders, +accidental second paths (such as an Ingress), the gateway Role's `patch` verb on +`sandboxclaims` (SSO token rotation) and that both SSO NetworkPolicies are +applied. Verify the negative case from a sandbox pod (it is not the proxy): diff --git a/examples/dsh-agents-as-a-service/manifests/exposure/README.md b/examples/dsh-agents-as-a-service/manifests/exposure/README.md index e3f1537..4dec732 100644 --- a/examples/dsh-agents-as-a-service/manifests/exposure/README.md +++ b/examples/dsh-agents-as-a-service/manifests/exposure/README.md @@ -139,13 +139,15 @@ kubectl apply -f manifests/exposure/gateway-api-sso/ # 5. Preflight: fails loudly on missing Secrets, a bad oauth2-proxy cookie # secret, missing gateway SSO env, a dangling TLS reference, leftover -# placeholders, or a second path to the gateway. +# placeholders, a second path to the gateway, a Role without `patch` on +# sandboxclaims, or missing SSO NetworkPolicies. ./scripts/preflight-sso.sh ``` The preflight is read-only and exits non-zero on any failure, naming the exact fix. Run it again after rotating a Secret or changing the public host; it also -verifies that the oauth2-proxy cookie secret decodes to 16, 24 or 32 bytes. +verifies that the oauth2-proxy cookie secret is a valid AES key length (16, 24 +or 32 raw bytes, or URL-safe unpadded base64 that decodes to one of those). ### SSO trust boundary diff --git a/examples/dsh-agents-as-a-service/manifests/exposure/gateway-api-sso/75-networkpolicies.yaml b/examples/dsh-agents-as-a-service/manifests/exposure/gateway-api-sso/75-networkpolicies.yaml index 42ac27b..0546499 100644 --- a/examples/dsh-agents-as-a-service/manifests/exposure/gateway-api-sso/75-networkpolicies.yaml +++ b/examples/dsh-agents-as-a-service/manifests/exposure/gateway-api-sso/75-networkpolicies.yaml @@ -1,5 +1,6 @@ # REQUIRED with the SSO exposure variant: enforce the trust boundary at the # network layer. Applying the SSO variant without this file is demo-only. +# `scripts/preflight-sso.sh` fails while either policy below is missing. # # The chain is Traefik (Gateway API) -> oauth2-proxy -> dsh-gateway. These # policies make it the ONLY chain: diff --git a/examples/dsh-agents-as-a-service/scripts/preflight-sso.sh b/examples/dsh-agents-as-a-service/scripts/preflight-sso.sh index 084ff9e..b64556e 100755 --- a/examples/dsh-agents-as-a-service/scripts/preflight-sso.sh +++ b/examples/dsh-agents-as-a-service/scripts/preflight-sso.sh @@ -20,7 +20,14 @@ # applied oauth2-proxy args, Gateway listeners or HTTPRoute hostnames; # 6. (warning) no Ingress in the namespace creates a second path to # dsh-gateway. With SSO enabled the Service must be reachable only through -# oauth2-proxy. +# oauth2-proxy; +# 7. the gateway ServiceAccount may `patch` sandboxclaims. The SSO landing +# path rotates the token-hash annotation whenever the browser has no valid +# gateway session (first visit, expired/foreign cookie, gateway restart), +# so without that verb every such sign-in answers 503; +# 8. the SSO variant's required NetworkPolicies are applied. Without them any +# Pod in the cluster can reach dsh-gateway:8080 and spoof the identity +# header, so the exposure is demo-only. # # Exit status: 0 when every check passes (warnings allowed), 1 on any failure. # Every failure prints the fix. This script only reads cluster state. @@ -35,6 +42,9 @@ # OAUTH_SECRET oauth2-proxy Secret name (default: oauth2-proxy) # TLS_SECRET TLS Secret name the Gateway should reference (default: # dsh-demo-tls) +# GATEWAY_ROLE Role/RoleBinding name that grants the gateway access +# (default: dsh-gateway; only used when impersonation is +# unavailable and the permission must be read statically) set -euo pipefail cd "$(dirname "$0")/.." @@ -43,6 +53,7 @@ GATEWAY_DEPLOY="${GATEWAY_DEPLOY:-dsh-gateway}" OAUTH_DEPLOY="${OAUTH_DEPLOY:-oauth2-proxy}" OAUTH_SECRET="${OAUTH_SECRET:-oauth2-proxy}" TLS_SECRET="${TLS_SECRET:-dsh-demo-tls}" +GATEWAY_ROLE="${GATEWAY_ROLE:-dsh-gateway}" FAILED=0 pass() { printf 'PASS %s\n' "$*"; } @@ -83,21 +94,38 @@ check_secret_key "$OAUTH_SECRET" client-secret \ check_secret_key "$OAUTH_SECRET" cookie-secret \ "Create it from manifests/exposure/gateway-api-sso/73-oauth2-proxy-secret.yaml.example." -# The cookie secret must be URL-safe unpadded base64 of 16, 24 or 32 bytes; -# oauth2-proxy refuses anything else (the classic error is "44 bytes"). +# oauth2-proxy accepts the cookie secret either as raw bytes of length 16, 24 +# or 32, or as URL-safe unpadded base64 that decodes to one of those lengths +# (pkg/encryption.SecretBytes). The documented generator, +# `python3 -c 'import secrets; print(secrets.token_urlsafe(32))'`, produces the +# second form: 43 base64 characters that decode to 32 bytes. if COOKIE_B64=$(kubectl -n "$NS" get secret "$OAUTH_SECRET" \ -o jsonpath='{.data.cookie-secret}' 2>/dev/null); then if [ -n "$COOKIE_B64" ]; then - if ! COOKIE_LEN=$(printf '%s' "$COOKIE_B64" | base64 -d 2>/dev/null | wc -c); then - fail "Secret $OAUTH_SECRET key 'cookie-secret' is not valid base64. Generate it with: python3 -c 'import secrets; print(secrets.token_urlsafe(32))'" - else - case "$COOKIE_LEN" in - 16 | 24 | 32) pass "oauth2-proxy cookie-secret decodes to $COOKIE_LEN bytes" ;; - *) - fail "oauth2-proxy cookie-secret decodes to $COOKIE_LEN bytes; it must be 16, 24 or 32. Generate a URL-safe unpadded value with: python3 -c 'import secrets; print(secrets.token_urlsafe(32))'" - ;; - esac - fi + COOKIE_RAW=$(printf '%s' "$COOKIE_B64" | base64 -d 2>/dev/null || true) + COOKIE_LEN=${#COOKIE_RAW} + COOKIE_FORM=raw + case "$COOKIE_LEN" in + 16 | 24 | 32) ;; + *) + # Mirror SecretBytes: only a strictly URL-safe base64 string is decoded; + # anything else is used as raw bytes (and is then the wrong length). + COOKIE_FORM=base64 + case "$COOKIE_RAW" in + "" | *[!A-Za-z0-9_=-]*) COOKIE_LEN=0 ;; + *) + COOKIE_LEN=$(printf '%s' "$COOKIE_RAW" | tr -- '-_' '+/' | tr -d '=\n' \ + | base64 -d 2>/dev/null | wc -c || true) + ;; + esac + ;; + esac + case "$COOKIE_LEN" in + 16 | 24 | 32) pass "oauth2-proxy cookie-secret is $COOKIE_FORM with a $COOKIE_LEN-byte key" ;; + *) + fail "oauth2-proxy cookie-secret is not a valid length ($COOKIE_LEN bytes). It must be 16, 24 or 32 raw bytes, or URL-safe unpadded base64 that decodes to one of those lengths. Generate it with: python3 -c 'import secrets; print(secrets.token_urlsafe(32))'" + ;; + esac fi fi @@ -139,6 +167,51 @@ if env_is_set ALLOW_INSECURE_COOKIES; then warn "ALLOW_INSECURE_COOKIES is still set on deploy/$GATEWAY_DEPLOY. It is ignored while COOKIE_SECURE=true but is a footgun: remove it with 'kubectl -n $NS set env deploy/$GATEWAY_DEPLOY ALLOW_INSECURE_COOKIES-'." fi +echo +echo "== Gateway RBAC (SSO token rotation)" +# The SSO landing path patches the token-hash annotation on the user's claim +# whenever the browser has no valid gateway session: first visit, an expired or +# foreign cookie, or a gateway restart (sessions live in the gateway's memory). +# Without `patch` every such GET / answers 503 and the user cannot sign in. +GATEWAY_SA=$(kubectl -n "$NS" get deployment "$GATEWAY_DEPLOY" \ + -o jsonpath='{.spec.template.spec.serviceAccountName}' 2>/dev/null || true) +GATEWAY_SA=${GATEWAY_SA:-default} +CAN_I=$(kubectl auth can-i patch sandboxclaims.extensions.agents.x-k8s.io \ + -n "$NS" --as="system:serviceaccount:$NS:$GATEWAY_SA" 2>&1 || true) + +# read_gateway_rules: one "apiGroups|resources|verbs" line per rule of the +# Role (or ClusterRole) the gateway RoleBinding points at. Used only when +# `kubectl auth can-i --as` is unavailable (a read-only kubeconfig cannot +# impersonate); it is a static check, not an authoritative one. +read_gateway_rules() { + local rules_jsonpath kind name + rules_jsonpath='{range .rules[*]}{range .apiGroups[*]}{@}{","}{end}{"|"}{range .resources[*]}{@}{","}{end}{"|"}{range .verbs[*]}{@}{","}{end}{"\n"}{end}' + kind=$(kubectl -n "$NS" get rolebinding "$GATEWAY_ROLE" \ + -o jsonpath='{.roleRef.kind}' 2>/dev/null || true) + name=$(kubectl -n "$NS" get rolebinding "$GATEWAY_ROLE" \ + -o jsonpath='{.roleRef.name}' 2>/dev/null || true) + case "$kind" in + Role) kubectl -n "$NS" get role "$name" -o jsonpath="$rules_jsonpath" 2>/dev/null || true ;; + ClusterRole) kubectl get clusterrole "$name" -o jsonpath="$rules_jsonpath" 2>/dev/null || true ;; + esac +} + +if [ "$CAN_I" = "yes" ]; then + pass "ServiceAccount $GATEWAY_SA can patch sandboxclaims (token rotation)" +elif [ "$CAN_I" = "no" ]; then + fail "ServiceAccount $GATEWAY_SA cannot patch sandboxclaims: SSO sign-in answers 503 whenever the browser has no valid gateway session (after a gateway restart, cookie expiry or in a new browser). Fix: grant 'patch' on sandboxclaims to Role $GATEWAY_ROLE (the shipped Role in manifests/50-gateway.yaml has it; re-applying just the Role/RoleBinding is enough). RBAC is evaluated per request; no gateway restart is needed." +else + RULES=$(read_gateway_rules) + if printf '%s\n' "$RULES" | awk -F'|' \ + '$1 ~ /(^|,)extensions\.agents\.x-k8s\.io(,|$)/ && $2 ~ /(^|,)sandboxclaims(,|$)/ && $3 ~ /(^|,)patch(,|$)/ { found = 1 } END { exit found ? 0 : 1 }'; then + pass "the gateway Role grants patch on sandboxclaims (static check; impersonation unavailable)" + elif [ -n "$RULES" ]; then + fail "Role $GATEWAY_ROLE does not grant 'patch' on sandboxclaims: SSO sign-in answers 503 whenever the browser has no valid gateway session. Fix: grant that verb to the Role (the shipped Role in manifests/50-gateway.yaml has it; re-applying just the Role/RoleBinding is enough). RBAC is evaluated per request; no gateway restart is needed." + else + warn "could not verify that ServiceAccount $GATEWAY_SA may patch sandboxclaims ($CAN_I). Check manually that Role $GATEWAY_ROLE grants 'patch' on extensions.agents.x-k8s.io/sandboxclaims." + fi +fi + echo echo "== SSO objects and placeholders" if kubectl -n "$NS" get deployment "$OAUTH_DEPLOY" >/dev/null 2>&1; then @@ -226,6 +299,18 @@ if kubectl -n "$NS" get ingress >/dev/null 2>&1; then fi fi +# The SSO variant's policies are part of the variant: without them the +# gateway Service is reachable by any Pod, which can spoof the identity header. +echo +echo "== SSO NetworkPolicies (required with this variant)" +for POLICY in dsh-gateway-ingress oauth2-proxy-ingress; do + if kubectl -n "$NS" get networkpolicy "$POLICY" >/dev/null 2>&1; then + pass "NetworkPolicy $POLICY exists" + else + fail "NetworkPolicy $POLICY is missing: any Pod in the cluster can reach dsh-gateway:8080 and spoof X-Forwarded-User, so the SSO identity is not trustworthy. Fix: kubectl apply -f manifests/exposure/gateway-api-sso/75-networkpolicies.yaml" + fi +done + echo if [ "$FAILED" -ne 0 ]; then echo "preflight-sso: FAILED - fix the items above before exposing the gateway." >&2