Review oidc-service-client 0.1 against KeyCape's client field rules
Answers net-kingdom's NK-WP-0056-T14 review request (message dc359034). Field rules confirmed: the rendered record passes keycape check-config, and the token shape was observed on a local synthetic run (assurance is a fixed aal1/client_secret block). One correction: an excess scope returns invalid_profile_usage (HTTP 400, feature scope), not invalid_scope. Notes on the bind-env restart and its Secret precondition under Recreate, and on the rotation window. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Assistant: claude-code Assistant-Model: opus Assistant-Process: 5502@bnt-lap001 Assistant-Session: 68b3b02f-5d0f-4e71-8421-796f62e50196
This commit is contained in:
parent
a140a4ab05
commit
655d8d31b3
1 changed files with 146 additions and 0 deletions
146
docs/oidc-service-client-type-review.md
Normal file
146
docs/oidc-service-client-type-review.md
Normal file
|
|
@ -0,0 +1,146 @@
|
|||
# Review: SecurityCaseType oidc-service-client 0.1
|
||||
|
||||
Reviewed 2026-10-31 by key-cape, the owner of the client field rules, at net-kingdom's
|
||||
request (NK-WP-0056-T14, message `dc359034`, hub decision `f8044f81`). Sources:
|
||||
net-kingdom `security-case-types/oidc-service-client/type.yaml`, its README, the
|
||||
example case `examples/nk-bind-executor-approval.yaml` and the renderer
|
||||
`sso-mfa/k8s/keycape/keycape-client.py` (`service_client_from_params`), all at
|
||||
net-kingdom `5d5f15d`. Compared with KeyCape `src/internal/config/validate.go`,
|
||||
`src/internal/config/config.go`, `src/internal/domain/model.go`,
|
||||
`src/internal/server/oidc/token.go` and `src/cmd/keycape/{main,checkconfig}.go`
|
||||
at key-cape `a140a4a`.
|
||||
|
||||
**Verdict: the field rules are correct; one correction to the type (R3) and two
|
||||
notes for its attended steps (R4, R5).** No finding blocks KeyCape from starting.
|
||||
|
||||
## How this was checked (no live system)
|
||||
|
||||
- The renderer turned the example case into a client record. That record went into
|
||||
a copy of `config/dev-config.yaml` as the only client. A locally built `keycape
|
||||
check-config` accepted it (`valid: true`, `required_env:
|
||||
[KEYCAPE_NK_BIND_EXECUTOR_APPROVAL_CLIENT_SECRET]`). With `--env-names` missing
|
||||
that name, check-config refused it.
|
||||
- The renderer reproduces `config/service-clients.example.yaml`'s
|
||||
`secrets-engine-approval` record exactly from the equivalent parameters.
|
||||
- A local KeyCape ran on the dev config with a synthetic secret in the env
|
||||
variable. Three client_credentials exchanges were made against it. The results
|
||||
are in R2 and R3.
|
||||
- The same binary, started without the env variable, exited with `client
|
||||
"nk-bind-executor-approval" secret environment variable ... is empty` / `failed to
|
||||
build client registry`.
|
||||
|
||||
## Findings
|
||||
|
||||
**R1: the rendered client record matches KeyCape's model and passes check-config.**
|
||||
(confirmed)
|
||||
For the example case the renderer emits `clientId`, `displayName`, `audience`,
|
||||
`allowedScopes` (a list), `grantTypes: [client_credentials]`, `clientType:
|
||||
confidential`, `secretRef: env:KEYCAPE_NK_BIND_EXECUTOR_APPROVAL_CLIENT_SECRET`,
|
||||
`serviceSubject`, `tenant`, `roles` (a list, omitted when empty) and `tokenLifetime`.
|
||||
Every key is a field of `config.ClientConfig`. KeyCape requires these for
|
||||
client_credentials: confidential, an `env:` secretRef, a non-empty `serviceSubject`
|
||||
and `tenant`, and a lifetime between 1m and 1h. Redirect URIs are not required. The
|
||||
type is stricter than KeyCape in each respect, which is allowed:
|
||||
- resource scopes only;
|
||||
- `service:` and `tenant:` prefixes;
|
||||
- one audience token without whitespace;
|
||||
- 5m, 10m or 15m only.
|
||||
The env name maps `-` to `_` and admits no `_` in `client_id`, so no two ids share an
|
||||
env name. The pattern `service:[a-z0-9-]+` does not fit two existing clients
|
||||
(`secrets-engine-requester` uses `secrets-engine`, and `codex-railiance-platform` uses
|
||||
`service:codex:railiance-platform`). That only matters if those clients are adopted
|
||||
later. The README already limits the fit claim to `secrets-engine-approval`.
|
||||
|
||||
**R2: shape of a client_credentials token.** (confirmed; this answers the
|
||||
assurance question)
|
||||
Observed from the local exchange for the example case:
|
||||
|
||||
```json
|
||||
{
|
||||
"iss": "<issuer>", "sub": "service:nk-bind-executor", "aud": "approval-engine",
|
||||
"iat": 1793479725, "exp": 1793480625,
|
||||
"tenant": "tenant:platform", "tenant_source": "registration",
|
||||
"principal_type": "service", "groups": [], "roles": ["bounded-executor"],
|
||||
"scope": "approval:create approval:read approval:consume",
|
||||
"assurance": {"level": "aal1", "methods": ["client_secret"], "mfa": false,
|
||||
"source": "key-cape", "at": 1793479725}
|
||||
}
|
||||
```
|
||||
|
||||
- `aud` is a single string: the registered `audience`, or the `client_id` when no
|
||||
audience is registered. The request cannot select it.
|
||||
- `scope` is a space-separated string. With no `scope` parameter the token carries
|
||||
all registered scopes; with one, exactly the requested subset.
|
||||
- `exp - iat` and the response `expires_in` both equal the client's `tokenLifetime`
|
||||
(900 for 15m). No refresh token is issued.
|
||||
- `assurance` is a fixed literal for service tokens:
|
||||
- `level` is always `aal1` and `methods` always `["client_secret"]`;
|
||||
- `mfa` is always false;
|
||||
- `at` is the mint time, since a service has no separate authentication event.
|
||||
No configuration raises it. approval-engine's `identity_from_claims` needs only an
|
||||
object here, so the token is admissible. Any approval-engine rule that requires
|
||||
aal2 will, correctly, never be met by a service token. A consumer must use
|
||||
`principal_type: service`, not the assurance level, to tell service tokens from
|
||||
human ones.
|
||||
- `jti` appears only when token status is enabled. `tenant_roles` appears only when
|
||||
tenant-engine is configured and answers for the tenant. Neither is guaranteed.
|
||||
|
||||
**R3: an excess scope returns `invalid_profile_usage`, not `invalid_scope`.**
|
||||
(correction to the type)
|
||||
KeyCape refuses any requested scope outside `allowedScopes` with HTTP 400 and the
|
||||
body `{"error":"invalid_profile_usage","description":"requested scope is not
|
||||
allowed","feature":"scope"}` (`token.go`, `serveClientCredentials`; the response was
|
||||
observed for `scope=approval:approve`). The verification step `excess-scope-refused`
|
||||
("KeyCape returns invalid_scope") and the README's threat model ("KeyCape refuses an
|
||||
excess scope (`invalid_scope`)") name the wrong code. A literal check against them
|
||||
would fail on a correct refusal. Fix: expect HTTP 400 with `error:
|
||||
invalid_profile_usage` and `feature: scope`, and correct the README. The refusal
|
||||
itself is real, and the whole request fails: no token is issued for the allowed
|
||||
subset. KeyCape follows its profile error taxonomy here rather than RFC 6749's
|
||||
`invalid_scope`. Changing that is a separate key-cape decision and is not needed for
|
||||
this type.
|
||||
|
||||
**R4: bind-env restarts KeyCape on its own, and its Secret must exist first.**
|
||||
(note for the attended steps)
|
||||
The type's order and its stated reason are correct. KeyCape resolves every `env:`
|
||||
secretRef at start and exits if one is empty (`buildClientRegistryWith`). Running
|
||||
register-client before bind-env therefore takes the only issuer down for every
|
||||
client. The renderer guards against that: `keycape-client.py --check-config` passes
|
||||
the Deployment's env names to `keycape check-config --env-names`, which refuses a
|
||||
registry naming an unbound variable. Two points are missing from the attended
|
||||
justification:
|
||||
- Patching the Deployment env is itself a rollout. The Deployment has one replica
|
||||
and `strategy: Recreate`, so bind-env and register-client each cause a restart that
|
||||
drops in-flight logins for every client. That is two restarts per case unless both
|
||||
changes go in one rollout.
|
||||
- The env entry references Secret `sso/keycape-<client_id>-client` with no
|
||||
`optional`. If deliver-secret is not Ready when bind-env runs, the new pod fails
|
||||
with CreateContainerConfigError. Under Recreate the old pod is already gone, so
|
||||
sign-in fails for every client. deliver-secret's ready_check must pass immediately
|
||||
before bind-env in the same session, not just at some point beforehand.
|
||||
|
||||
**R5: rotation breaks the consumer until KeyCape restarts.** (note on Known
|
||||
limits)
|
||||
The README says that a rotation also restarts KeyCape. The order matters:
|
||||
- The consumer reads the new value from OpenBao on its next call.
|
||||
- KeyCape keeps comparing against the value in its environment at start, until both
|
||||
of these happen: ESO refreshes `sso/keycape-<client_id>-client`, and KeyCape
|
||||
restarts.
|
||||
- In between, every exchange by the consumer fails with 401 (`invalid client
|
||||
authentication`).
|
||||
Rotation should therefore be: mint, wait for the ESO refresh to report Ready, restart
|
||||
KeyCape, and only then expect the consumer's exchanges to succeed. The window will
|
||||
show up as consumer errors. KeyCape has no dual-secret overlap.
|
||||
|
||||
## Not a finding
|
||||
|
||||
- Identity scopes (`openid`, `profile`, `email`, `groups`) are accepted by KeyCape on
|
||||
a client_credentials client, but they add nothing to the token. Excluding them in
|
||||
the type is correct and costs nothing.
|
||||
- `service-token-claims` checks sub, aud, tenant, scope and lifetime, and R2 confirms
|
||||
each. Adding `principal_type: service` to the expected claims would be cheap and
|
||||
matches what approval-engine checks.
|
||||
- The client secret format is opaque to KeyCape. KeyCape form-decodes both halves of
|
||||
`client_secret_basic`, so a base64url secret needs no escaping.
|
||||
- Rollback order (registration removed before the env binding) mirrors the apply
|
||||
order correctly. Removing the env first would hit the same start-up exit as in R4.
|
||||
Loading…
Add table
Add a link
Reference in a new issue