diff --git a/docs/approval-engine-provisioning-request.yaml b/docs/approval-engine-provisioning-request.yaml index 71308fa..8337f76 100644 --- a/docs/approval-engine-provisioning-request.yaml +++ b/docs/approval-engine-provisioning-request.yaml @@ -39,10 +39,29 @@ human_registration: mfa_required: true client_type: public verification: - - Validate the new signature against deployed JWKS and all exact claim bindings. - - Reject wrong secrets, operator consume, PEP lifecycle scopes, and human consume. + # The first two lines are now one runnable command per client; see + # docs/native-authentication.md, "Verifying a live registration". It writes + # nothing and prints no value, so it is safe to run against production. + - command: | + keycape verify-client -issuer https://kc.coulomb.social + -client-id secrets-engine-approval -audience approval-engine + -scope "approval:read approval:consume" + -secret-env KEYCAPE_SECRETS_ENGINE_APPROVAL_CLIENT_SECRET + -expect-subject service:secrets-engine -expect-tenant tenant:platform + -expect-roles secrets-engine + -deny-scope "approval:approve approval:revoke approval:supersede" + - command: | + keycape verify-client -issuer https://kc.coulomb.social + -client-id approval-engine-operator -audience approval-engine + -scope "approval:create approval:read approval:approve approval:revoke approval:supersede approval:observe approval:emit" + -secret-env KEYCAPE_APPROVAL_ENGINE_OPERATOR_CLIENT_SECRET + -expect-subject service:approval-engine-operator -expect-tenant tenant:platform + -expect-roles approval-operator + -deny-scope "approval:consume" - Check KeyCape and consumer readiness without emitting secrets or tokens. - Preserve existing registrations and signing key; record versions and image digest. + - Human consume denial is approval-engine's to verify at its resource; KeyCape + proves only that the human client is never issued a consume grant. blockers: - Admit exact custody paths, field delivery, consumer identities and lifecycle authority. - Resolve attended first-provision authority through the custody owner. diff --git a/docs/native-authentication.md b/docs/native-authentication.md index 1362690..e714df2 100644 --- a/docs/native-authentication.md +++ b/docs/native-authentication.md @@ -85,3 +85,50 @@ provider update, not a wrapper around a raw KV read. The reviewed sequence is: No general rotation command is shipped until that cross-owner transaction has an admitted execution and rollback contract. Calling secrets-engine's KV rotation alone would leave the provider and consumers inconsistent. + +Steps 1-3 are custody's and are not automated here. Step 4, and the denial +checks step 5 depends on, are KeyCape's and now have a command. + +## Verifying a live registration + +`keycape verify-client` proves a deployed registration behaves as its contract +says, without writing a token file or printing any value. It is the evidence for +a rollout (KEY-WP-0013-T02) and for step 4 of the rotation sequence above. + +``` +keycape verify-client \ + -issuer https://kc.coulomb.social \ + -client-id secrets-engine-approval \ + -audience approval-engine \ + -scope "approval:read approval:consume" \ + -secret-env KEYCAPE_SECRETS_ENGINE_APPROVAL_CLIENT_SECRET \ + -expect-subject service:secrets-engine \ + -expect-tenant tenant:platform \ + -expect-roles secrets-engine \ + -deny-scope "approval:approve approval:revoke" +``` + +It checks, and prints one `PASS`/`FAIL` line per check: + +- discovery resolves over HTTPS and every endpoint shares the issuer origin; +- the `client_credentials` exchange succeeds and its access token verifies RS256 + against the discovered JWKS, with exact issuer, audience and validity window; +- `principal_type` is `service`, and `sub`, `tenant` and `roles` match what the + registration declares — exact comparison, no alias; +- the token carries **no scope that was not requested**, which the caller + commands do not check: they prove every requested scope was granted, not that + nothing extra came back; +- every `-deny-scope` is refused. A success there is the failure. + +Add `-previous-secret-env` after a rotation to require that the predecessor is +refused. It also fails if the predecessor and current values are identical, +which means no rotation occurred. + +Every check runs before the command reports, so one failure does not hide the +rest; the exit status is non-zero if any failed. Failures name the **claim**, +never the observed value — running this against production must not turn a +verification into a disclosure. Nothing is written to disk. + +The command needs the client secret in the named environment variable, so it +runs wherever custody already delivers that value. It never reads OpenBao or +Kubernetes itself. diff --git a/src/cmd/keycape/main.go b/src/cmd/keycape/main.go index 65bff12..da21c33 100644 --- a/src/cmd/keycape/main.go +++ b/src/cmd/keycape/main.go @@ -36,7 +36,7 @@ import ( const version = "0.1.0" func main() { - if len(os.Args) > 1 && (os.Args[1] == "login" || os.Args[1] == "service-token") { + if len(os.Args) > 1 && (os.Args[1] == "login" || os.Args[1] == "service-token" || os.Args[1] == "verify-client") { if err := authclient.Run(context.Background(), os.Args[1:], os.Stderr); err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) diff --git a/src/internal/authclient/cli.go b/src/internal/authclient/cli.go index 79e3f11..2b0417e 100644 --- a/src/internal/authclient/cli.go +++ b/src/internal/authclient/cli.go @@ -22,9 +22,12 @@ import ( // Run executes a caller command. Output contains only instructions or status. func Run(ctx context.Context, args []string, stderr io.Writer) error { if len(args) == 0 { - return errors.New("expected login or service-token") + return errors.New("expected login, service-token or verify-client") } mode := args[0] + if mode == "verify-client" { + return verifyClient(ctx, args[1:], stderr) + } if mode != "login" && mode != "service-token" { return errors.New("unknown authentication command") } diff --git a/src/internal/authclient/verify.go b/src/internal/authclient/verify.go new file mode 100644 index 0000000..d912efd --- /dev/null +++ b/src/internal/authclient/verify.go @@ -0,0 +1,231 @@ +package authclient + +import ( + "context" + "errors" + "flag" + "fmt" + "io" + "net/url" + "os" + "sort" + "strings" + "time" +) + +// verifyClient proves a live service registration without disclosing anything. +// +// It exists because two blocked tasks need the same evidence and neither can +// produce it by hand safely. KEY-WP-0013-T02 must "prove live JWKS verification +// and denied excess scopes without logging values" once custody materializes the +// approval client secrets, and KEY-WP-0014-T04 step 4 must verify a rotated +// secret, reject its predecessor and deny excess scope before a rotation is +// declared complete. The custody owner performs the writes; this is the check +// they can run afterwards. +// +// Nothing is written to disk and no token, secret or claim value is ever +// printed. A mismatch reports the claim name only: an operator running this +// against production must not turn a verification into a disclosure. +func verifyClient(ctx context.Context, args []string, stderr io.Writer) error { + fs := flag.NewFlagSet("verify-client", flag.ContinueOnError) + fs.SetOutput(stderr) + issuer := fs.String("issuer", "", "HTTPS issuer") + id := fs.String("client-id", "", "registered client ID") + audience := fs.String("audience", "", "expected access-token audience (defaults to client ID)") + scope := fs.String("scope", "", "space-separated scopes the registration grants") + secretEnv := fs.String("secret-env", "", "environment variable holding the current client secret") + previousEnv := fs.String("previous-secret-env", "", "environment variable holding the predecessor secret, which must be rejected") + denyScope := fs.String("deny-scope", "", "space-separated scopes the registration must refuse") + expectSubject := fs.String("expect-subject", "", "exact required sub claim") + expectTenant := fs.String("expect-tenant", "", "exact required tenant claim") + expectRoles := fs.String("expect-roles", "", "space-separated roles that must all be present") + if err := fs.Parse(args); err != nil { + return err + } + if fs.NArg() != 0 || *id == "" || *secretEnv == "" || strings.TrimSpace(*scope) == "" { + return errors.New("client-id, secret-env and scope are required; positional arguments are not accepted") + } + if *audience == "" { + *audience = *id + } + secret := os.Getenv(*secretEnv) + if secret == "" { + return fmt.Errorf("environment variable %s is empty", *secretEnv) + } + c, err := New(*issuer) + if err != nil { + return err + } + return runVerify(ctx, c, verifyOptions{ + ClientID: *id, + Secret: secret, + Previous: os.Getenv(*previousEnv), + PreviousNamed: *previousEnv != "", + Audience: *audience, + Scope: *scope, + DenyScope: *denyScope, + Subject: *expectSubject, + Tenant: *expectTenant, + Roles: *expectRoles, + }, stderr) +} + +// verifyOptions is the resolved, secret-bearing input to a verification run. +// Secrets arrive as values here and never leave this package. +type verifyOptions struct { + ClientID string + Secret string + Previous string + PreviousNamed bool + Audience string + Scope string + DenyScope string + Subject string + Tenant string + Roles string +} + +func runVerify(ctx context.Context, c *Client, o verifyOptions, stderr io.Writer) error { + ctx, cancel := context.WithTimeout(ctx, 2*time.Minute) + defer cancel() + + report := &checks{out: stderr} + d, err := c.Discover(ctx) + report.record("discovery and endpoint origin", err) + if err != nil { + return report.result() + } + + // The positive exchange. Exchange already verifies RS256 against the + // discovered JWKS and binds issuer, audience, expiry and requested scope. + form := url.Values{"grant_type": {"client_credentials"}, "scope": {o.Scope}} + tokens, err := c.Exchange(ctx, d, form, o.ClientID, o.Secret, o.Audience, "") + report.record("exchange and JWKS signature for granted scopes", err) + if err == nil { + claims, verr := c.Verify(ctx, d, tokens.AccessToken, o.Audience, "") + report.record("access-token claim bindings", verr) + if verr == nil { + report.record("principal_type is service", exactClaim(claims, "principal_type", "service")) + if o.Subject != "" { + report.record("sub matches the registration", exactClaim(claims, "sub", o.Subject)) + } + if o.Tenant != "" { + report.record("tenant matches the registration", exactClaim(claims, "tenant", o.Tenant)) + } + if strings.TrimSpace(o.Roles) != "" { + report.record("roles present", hasAllRoles(claims, o.Roles)) + } + report.record("granted scope carries no excess grant", noExcessScope(claims, o.Scope)) + } + } + + // Excess-scope denial. A success here is the failure: the registration + // handed out a grant it must not have. + for _, s := range strings.Fields(o.DenyScope) { + excess := url.Values{"grant_type": {"client_credentials"}, "scope": {s}} + _, err := c.Exchange(ctx, d, excess, o.ClientID, o.Secret, o.Audience, "") + report.record("excess scope refused: "+s, mustFail(err)) + } + + // Predecessor rejection, for a rotation. Only meaningful after step 3 of the + // rotation sequence has replaced the value in both custodians. + if o.PreviousNamed { + if o.Previous == "" { + report.record("predecessor secret refused", errors.New("predecessor environment variable is empty")) + } else if o.Previous == o.Secret { + report.record("predecessor secret refused", errors.New("predecessor and current secret are identical; no rotation occurred")) + } else { + _, err := c.Exchange(ctx, d, form, o.ClientID, o.Previous, o.Audience, "") + report.record("predecessor secret refused", mustFail(err)) + } + } + return report.result() +} + +// checks accumulates outcomes so every check runs and the operator sees the +// whole picture, rather than stopping at the first failure and hiding the rest. +type checks struct { + out io.Writer + failed int + total int +} + +func (c *checks) record(name string, err error) { + c.total++ + if err != nil { + c.failed++ + fmt.Fprintf(c.out, "FAIL %s: %v\n", name, err) + return + } + fmt.Fprintf(c.out, "PASS %s\n", name) +} + +func (c *checks) result() error { + if c.failed > 0 { + return fmt.Errorf("%d of %d checks failed", c.failed, c.total) + } + fmt.Fprintf(c.out, "\nAll %d checks passed. No token, secret or claim value was written or printed.\n", c.total) + return nil +} + +// exactClaim reports the claim name only. The observed value stays unprinted so +// running this against production cannot leak a subject or tenant. +func exactClaim(claims map[string]any, name, want string) error { + got, ok := claims[name].(string) + if !ok { + return fmt.Errorf("claim %q is absent or not a string", name) + } + if got != want { + return fmt.Errorf("claim %q does not equal the expected value (exact match, no alias)", name) + } + return nil +} + +func hasAllRoles(claims map[string]any, want string) error { + raw, _ := claims["roles"].([]any) + present := map[string]bool{} + for _, r := range raw { + if s, ok := r.(string); ok { + present[s] = true + } + } + var missing []string + for _, r := range strings.Fields(want) { + if !present[r] { + missing = append(missing, r) + } + } + if len(missing) > 0 { + sort.Strings(missing) + return fmt.Errorf("roles absent: %s", strings.Join(missing, " ")) + } + return nil +} + +// noExcessScope catches the direction Exchange does not check: Exchange proves +// every requested scope was granted, not that nothing extra came back. +func noExcessScope(claims map[string]any, requested string) error { + granted, _ := claims["scope"].(string) + want := map[string]bool{} + for _, s := range strings.Fields(requested) { + want[s] = true + } + var extra []string + for _, s := range strings.Fields(granted) { + if !want[s] { + extra = append(extra, s) + } + } + if len(extra) > 0 { + sort.Strings(extra) + return fmt.Errorf("token carries scopes that were not requested: %s", strings.Join(extra, " ")) + } + return nil +} + +func mustFail(err error) error { + if err == nil { + return errors.New("the issuer accepted a request it must have refused") + } + return nil +} diff --git a/src/internal/authclient/verify_test.go b/src/internal/authclient/verify_test.go new file mode 100644 index 0000000..b7eda28 --- /dev/null +++ b/src/internal/authclient/verify_test.go @@ -0,0 +1,162 @@ +package authclient + +import ( + "bytes" + "context" + "strings" + "testing" + + "keycape/internal/domain" +) + +// The verification command is the evidence two blocked tasks will rest on +// (KEY-WP-0013-T02 rollout proof, KEY-WP-0014-T04 rotation step 4), so the +// tests that matter are the ones proving it FAILS when the issuer misbehaves. +// A checker that always passes is worse than no checker. + +func verifyOpts() verifyOptions { + return verifyOptions{ + ClientID: "service:consumer", + Secret: "special+%: secret", + Audience: "approval-engine", + Scope: "approval:read", + DenyScope: "approval:consume approval:approve", + Subject: "service:test", + Tenant: "tenant:test", + } +} + +func TestVerifyClientPassesOnCorrectRegistration(t *testing.T) { + c, _, _ := provider(t) + var out bytes.Buffer + if err := runVerify(context.Background(), c, verifyOpts(), &out); err != nil { + t.Fatalf("correct registration failed verification: %v\n%s", err, out.String()) + } + for _, want := range []string{ + "PASS discovery and endpoint origin", + "PASS exchange and JWKS signature for granted scopes", + "PASS principal_type is service", + "PASS sub matches the registration", + "PASS tenant matches the registration", + "PASS excess scope refused: approval:consume", + "PASS excess scope refused: approval:approve", + } { + if !strings.Contains(out.String(), want) { + t.Errorf("missing check %q in:\n%s", want, out.String()) + } + } +} + +// The whole point of the command: it must not print what it verified. An +// operator runs this against production, so a leaked subject, tenant or token +// would turn a verification into a disclosure. +func TestVerifyClientNeverPrintsSecretsOrTokens(t *testing.T) { + c, _, _ := provider(t) + o := verifyOpts() + o.Tenant = "tenant:wrong" // force a mismatch so the failure path is covered too + var out bytes.Buffer + if err := runVerify(context.Background(), c, o, &out); err == nil { + t.Fatal("wrong tenant accepted") + } + text := out.String() + for _, forbidden := range []string{"special+%: secret", "eyJ", "tenant:test", "service:test"} { + if strings.Contains(text, forbidden) { + t.Errorf("output disclosed %q:\n%s", forbidden, text) + } + } + if !strings.Contains(text, `FAIL tenant matches the registration`) { + t.Errorf("mismatch not reported by claim name:\n%s", text) + } +} + +func TestVerifyClientFailsWhenExcessScopeIsGranted(t *testing.T) { + c, _, h := provider(t) + // A registration that hands out the grant it must refuse. + h.ClientConfig["service:consumer"].AllowedScopes = []string{"approval:read", "approval:consume"} + var out bytes.Buffer + err := runVerify(context.Background(), c, verifyOpts(), &out) + if err == nil { + t.Fatal("over-broad registration passed verification") + } + if !strings.Contains(out.String(), "FAIL excess scope refused: approval:consume") { + t.Errorf("over-broad grant not reported:\n%s", out.String()) + } + if !strings.Contains(out.String(), "PASS excess scope refused: approval:approve") { + t.Errorf("unrelated denial should still pass:\n%s", out.String()) + } +} + +func TestVerifyClientRejectsUnrotatedAndAcceptedPredecessor(t *testing.T) { + c, _, h := provider(t) + + // Rotation that never happened: predecessor equals current. + o := verifyOpts() + o.PreviousNamed, o.Previous = true, o.Secret + var same bytes.Buffer + if err := runVerify(context.Background(), c, o, &same); err == nil { + t.Fatal("identical predecessor accepted as a rotation") + } + if !strings.Contains(same.String(), "no rotation occurred") { + t.Errorf("identical secret not diagnosed:\n%s", same.String()) + } + + // A real predecessor that the issuer still honours: the dangerous case, an + // old secret left valid after rotation. + o.Previous = "previous secret" + h.ClientConfig["service:consumer"].ClientSecret = "previous secret" + var stale bytes.Buffer + if err := runVerify(context.Background(), c, o, &stale); err == nil { + t.Fatal("issuer honouring the predecessor passed verification") + } + if !strings.Contains(stale.String(), "FAIL predecessor secret refused") { + t.Errorf("live predecessor not reported:\n%s", stale.String()) + } + + // The rotated state: predecessor differs and is refused. + h.ClientConfig["service:consumer"].ClientSecret = o.Secret + var good bytes.Buffer + if err := runVerify(context.Background(), c, o, &good); err != nil { + t.Fatalf("rotated registration failed: %v\n%s", err, good.String()) + } + if !strings.Contains(good.String(), "PASS predecessor secret refused") { + t.Errorf("predecessor rejection not confirmed:\n%s", good.String()) + } +} + +func TestVerifyClientRequiresCompleteArguments(t *testing.T) { + for _, args := range [][]string{ + {"-client-id", "x", "-scope", "a"}, // no secret-env + {"-secret-env", "X", "-scope", "a"}, // no client-id + {"-client-id", "x", "-secret-env", "X"}, // no scope + {"-client-id", "x", "-secret-env", "X", "-scope", "a", "extra"}, // positional + } { + if err := verifyClient(context.Background(), args, &bytes.Buffer{}); err == nil { + t.Errorf("incomplete arguments accepted: %v", args) + } + } +} + +// noExcessScope is the check Exchange does not perform: Exchange proves every +// requested scope was granted, never that nothing extra came back. +func TestNoExcessScopeCatchesUnrequestedGrants(t *testing.T) { + if err := noExcessScope(map[string]any{"scope": "approval:read"}, "approval:read"); err != nil { + t.Errorf("exact grant rejected: %v", err) + } + err := noExcessScope(map[string]any{"scope": "approval:read approval:consume"}, "approval:read") + if err == nil || !strings.Contains(err.Error(), "approval:consume") { + t.Errorf("unrequested grant not caught: %v", err) + } +} + +func TestHasAllRolesNamesWhatIsMissing(t *testing.T) { + claims := map[string]any{"roles": []any{"secrets-engine"}} + if err := hasAllRoles(claims, "secrets-engine"); err != nil { + t.Errorf("present role rejected: %v", err) + } + err := hasAllRoles(claims, "secrets-engine approval-operator") + if err == nil || !strings.Contains(err.Error(), "approval-operator") { + t.Errorf("missing role not named: %v", err) + } +} + +var _ = domain.Client{} diff --git a/workplans/KEY-WP-0013-approval-engine-resource-audience.md b/workplans/KEY-WP-0013-approval-engine-resource-audience.md index 3b163b1..fc11d83 100644 --- a/workplans/KEY-WP-0013-approval-engine-resource-audience.md +++ b/workplans/KEY-WP-0013-approval-engine-resource-audience.md @@ -85,6 +85,34 @@ railiance-platform and approval-engine respectively, and live provisioning proof cannot begin until both land. Still no secret read and no production resource changed. +2026-09-08, second pass. Custody and the human callback are still owed, but one +part of this task was ours all along and was not built: the task requires proving +"live JWKS verification and denied excess scopes without logging values", and +there was no way to do that except by hand, against production, at the moment +custody lands — which is the worst time to be improvising a check. + +Shipped `keycape verify-client` (`src/internal/authclient/verify.go`, documented +in `docs/native-authentication.md`). Per registration it verifies discovery +origin, the `client_credentials` exchange and its RS256 signature against the +deployed JWKS, exact `sub`/`tenant`/`roles`/`principal_type`, that every +`-deny-scope` is refused, and that the token carries no scope that was not +requested — the last being a gap the caller commands do not cover, since they +prove requested scopes were granted and not that nothing extra came back. +Failures name the claim, never the observed value, and nothing is written to +disk, so it is safe to run against production. The exact invocation for each of +the two clients is now recorded in the `verification:` block of +`docs/approval-engine-provisioning-request.yaml`, so admission hands back a +command rather than a description. + +Tests: `src/internal/authclient/verify_test.go` covers the passing case, an +over-broad registration, a live predecessor secret, an identical "rotation" and +output non-disclosure. Verified with teeth — neutering `mustFail` makes the suite +fail rather than pass silently. + +Task stays `wait`. What is owed from elsewhere is unchanged and unreduced: the +two secret values through an admitted custody path, and the exact human +`client_id` and callback URI. Nothing here provisions anything. + ## Reconcile tenant vocabularies across approval layers ```task diff --git a/workplans/KEY-WP-0014-native-credential-lane-handoff.md b/workplans/KEY-WP-0014-native-credential-lane-handoff.md index cad1c16..c5307cf 100644 --- a/workplans/KEY-WP-0014-native-credential-lane-handoff.md +++ b/workplans/KEY-WP-0014-native-credential-lane-handoff.md @@ -105,3 +105,34 @@ The KEY-WP-0009 handoff ops-warden reported missing has also now been delivered Task remains `wait` — no route was changed and no rotation command ships until ops-warden answers (1) and an execution/rollback authority is admitted for (2). + +2026-09-08, second pass — read ops-warden's own catalog rather than waiting for a +reply, and it changes the shape of both questions. + +`registry/routing/catalog.yaml`, lane `key-cape-oidc-login`: `intended_owner` was +already corrected to key-cape, and `verified: asked-and-waiting` since +2026-08-28. **ops-warden is waiting on us, not the other way round.** The lane is +also `warden_executes: false` with a `wiki_ref` into `CredentialRouting.md` — a +pointer, per their ADR-0001, not a code path with programmatic consumers. So the +consumer-contract risk behind question (1) is smaller than assumed: adding +`keycape login` alongside retires nothing, and the acceptance we owe them is a +statement of ownership, not a cutover. + +Lane `rapp-qonto-keycape-client`: `rotation.owner: key-cape`, +`automatable: true`, and `blocked_on` reads "still no key-cape-native +exchange/rotation command", `verified: source-read` 2026-08-28. **That blocker is +stale.** `keycape service-token` (T03) shipped 2026-09-05 and is exactly the +native `client_secret_basic` exchange they record as absent. It does not clear +the lane, and should be narrowed rather than closed. + +Shipped here: `keycape verify-client`, which is rotation step 4 and the denial +checks step 5 rests on — new exchange verified against deployed JWKS, predecessor +refused, excess scope refused, no value printed or written. See +`docs/native-authentication.md`, "Verifying a live registration". The identical +predecessor case is treated as a failure, because an unchanged secret is a +rotation that did not happen. + +Steps 1-3 remain custody's and are deliberately not automated. Task stays `wait` +on exactly two answers: which option ops-warden takes for the login proxy, and +who executes the successor generation and CAS update under what authority. No +route changed.