diff --git a/LLM.md b/LLM.md index 681d19f4e..554266f5c 100644 --- a/LLM.md +++ b/LLM.md @@ -3,6 +3,29 @@ **Project**: Lux Key Management Service (KMS) **Organization**: Lux Network +## v1.12.5 — deleted the env-var fetch route (secrets are never process env) + +- **Hole (HIGH, CRITICAL on live `lux-kms-go`):** `GET /v1/kms/secrets/{name}` was + registered **unwrapped** — no auth middleware — and did `os.Getenv(name)`, returning + the raw value. Any caller reaching `:8080` past the network boundary (NetworkPolicy + gap, port-forward, pod compromise, SSRF) could `curl .../v1/kms/secrets/KMS_MASTER_KEY_B64` + with **no Authorization header** and read the root REK that protects every per-secret + DEK — plus `MPC_TOKEN`, `KMS_ENCRYPTION_KEY_B64`, and (in the `k8s/` variant) the S3 + backup keys. On the live `lux-kms-go/kms` statefulset `KMS_MASTER_KEY_B64` IS populated + (verified: 32-byte key present in `kms-secrets`), so the leak was **live master-key + exposure gated only by the network** → CRITICAL. +- **Fix: deleted the route** (it had **zero callers** — the kms-operator reads via the + org-scoped path/ZAP, the `kms` Go client uses ZAP; process env was never a secret + source). The three org-scoped secret routes were extracted into `registerSecretRoutes` + (mirrors `registerKMSRoutes`/`registerOIDCRoutes`/`registerWebUI`) so the exact exposed + route set is testable. Secrets now flow **only** through `/v1/kms/orgs/{org}/secrets/*` + (JWT-gated, `requireOrgJWT`) and the ZAP wire. Regression `TestSecretRoutes_NoEnvVarLeak` + fails if any route ever echoes process env again (unauth → 404, admin → 404, canary + value never in body). Stale `kms.go` doc comment referencing the route corrected. +- Contrast: the Hanzo white-label fork (`hanzo/kms`) KEPT its `/v1/kms/secrets/{name}` + because it has real SDK consumers, and gated it (admin-only + `safeEnvName` + audit, + test `TestRed2_EnvVarReadRequiresAdmin`). Lux has no such consumers → delete, don't gate. + ## 2026-07-15 — KMS↔MPC wire fix (v1.12.3) + authorizer-coupling caveat - **v1.12.3** (wire-fix commit `7105376`) realigns the KMS↔MPC ZAP signing wire to the mpcd diff --git a/VERSION b/VERSION index 1cac385c6..e0a6b34fb 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.11.0 +1.12.5 diff --git a/cmd/kms/env_leak_test.go b/cmd/kms/env_leak_test.go new file mode 100644 index 000000000..4caab30ea --- /dev/null +++ b/cmd/kms/env_leak_test.go @@ -0,0 +1,154 @@ +// Regression coverage for the deleted env-var fetch route. +// +// A prior `GET /v1/kms/secrets/{name}` handler read os.Getenv(name) and +// returned it UNWRAPPED — registered with no auth middleware — so anyone who +// reached :8080 past the network boundary (NetworkPolicy gap, port-forward, +// pod compromise, SSRF) could exfiltrate the KMS process environment: the +// KMS_MASTER_KEY_B64 root REK that protects every per-secret DEK, MPC_TOKEN, +// the S3 backup keys, IAM/OIDC client secrets. On the live lux-kms-go pod +// KMS_MASTER_KEY_B64 is populated, so the leak was CRITICAL, gated only by the +// network. +// +// The route is gone. Secrets are served ONLY through the org-scoped, +// JWT-gated routes registerSecretRoutes installs (and the ZAP wire). These +// tests pin that invariant against the REAL production registrar: no path +// echoes process env, with or without a valid admin bearer. If anyone +// re-introduces such a route, the leak assertion fails. +package main + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/luxfi/kms/pkg/store" +) + +// A sentinel that must never appear in any HTTP response body. Installed into +// the process env under the exact name the deleted route would have echoed. +const leakSentinel = "MASTER-KEY-LEAK-CANARY-c1a9f0e7-do-not-return" + +// TestSecretRoutes_NoEnvVarLeak drives the production secret surface and proves +// the vestigial env-fetch path leaks nothing — the fix for the HIGH/CRITICAL +// os.Getenv hole. Fail-closed: unauthenticated AND authenticated-admin both get +// no env echo. +func TestSecretRoutes_NoEnvVarLeak(t *testing.T) { + // Populate the crown-jewel env var the deleted route echoed. registerSecretRoutes + // reads no env at all; the value is here only so a canary would surface if it did. + t.Setenv("KMS_MASTER_KEY_B64", leakSentinel) + + auth, bearer, cleanup := newTestKeyAuth(t, roleKMSAdmin) + defer cleanup() + secStore := newTestSecretStore(t) + + // The SAME registrar main() calls — this is the real route set, not a mock. + mux := http.NewServeMux() + registerSecretRoutes(mux, auth, secStore) + + srv := httptest.NewServer(mux) + defer srv.Close() + + // The vestigial env-fetch path, probed the way an attacker on :8080 would. + url := srv.URL + "/v1/kms/secrets/KMS_MASTER_KEY_B64" + + cases := []struct { + name string + bearer string + }{ + {"unauthenticated", ""}, + {"authenticated kms-admin", bearer}, // even a valid admin gets no env echo + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, url, nil) + if err != nil { + t.Fatalf("build req: %v", err) + } + if c.bearer != "" { + req.Header.Set("Authorization", "Bearer "+c.bearer) + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("do: %v", err) + } + defer resp.Body.Close() + body, _ := io.ReadAll(resp.Body) + + // Load-bearing assertion: the process-env value is never returned, + // regardless of status code. + if strings.Contains(string(body), leakSentinel) { + t.Fatalf("LEAK: response body contained the process-env master key (status=%d)", resp.StatusCode) + } + + // No handler serves this path anymore → 404. A 200 would mean an + // env-echoing route came back. + if resp.StatusCode != http.StatusNotFound { + t.Fatalf("deleted env route: got status %d, want 404 (no handler)", resp.StatusCode) + } + }) + } +} + +// TestSecretRoutes_OrgScopedReadStillWorks proves the extraction into +// registerSecretRoutes didn't break the real read path, and that a read returns +// the STORED value — never process env. Guards against a "delete broke the good +// path" regression. +func TestSecretRoutes_OrgScopedReadStillWorks(t *testing.T) { + auth, bearer, cleanup := newTestKeyAuth(t, roleKMSAdmin) + defer cleanup() + secStore := newTestSecretStore(t) + + if err := secStore.Put(&store.Secret{ + Name: "API_KEY", Path: "svc", Env: "prod", Ciphertext: []byte("stored-value-ok"), + }); err != nil { + t.Fatalf("seed: %v", err) + } + + mux := http.NewServeMux() + registerSecretRoutes(mux, auth, secStore) + srv := httptest.NewServer(mux) + defer srv.Close() + + req, _ := http.NewRequest(http.MethodGet, srv.URL+"/v1/kms/orgs/operator-org/secrets/svc/API_KEY?env=prod", nil) + req.Header.Set("Authorization", "Bearer "+bearer) + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("do: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("org read: got %d want 200", resp.StatusCode) + } + var m map[string]map[string]string + if err := json.NewDecoder(resp.Body).Decode(&m); err != nil { + t.Fatalf("decode: %v", err) + } + if got := m["secret"]["value"]; got != "stored-value-ok" { + t.Fatalf("org read value: got %q want stored-value-ok", got) + } +} + +// TestSecretRoutes_UnauthOrgReadRejected confirms the surviving org-scoped read +// still fails closed without a bearer — the extraction preserved requireOrgJWT. +func TestSecretRoutes_UnauthOrgReadRejected(t *testing.T) { + auth, _, cleanup := newTestKeyAuth(t, roleKMSAdmin) + defer cleanup() + secStore := newTestSecretStore(t) + + mux := http.NewServeMux() + registerSecretRoutes(mux, auth, secStore) + srv := httptest.NewServer(mux) + defer srv.Close() + + resp, err := http.Get(srv.URL + "/v1/kms/orgs/operator-org/secrets/svc/API_KEY?env=prod") + if err != nil { + t.Fatalf("get: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusUnauthorized { + t.Fatalf("unauth org read: got %d want 401", resp.StatusCode) + } +} diff --git a/cmd/kms/main.go b/cmd/kms/main.go index 5ba7f6833..cd0b0ab88 100644 --- a/cmd/kms/main.go +++ b/cmd/kms/main.go @@ -247,71 +247,14 @@ func main() { expectedIss := envOr("KMS_EXPECTED_ISSUER", iamEndpoint) auth := newOrgJWTAuth(jwksFrom, expectedIss) - // GET /v1/kms/orgs/{org}/secrets/{path...}/{name} - // Matches the ATS kmsclient.Get() URL pattern. - mux.HandleFunc("GET /v1/kms/orgs/{org}/secrets/{rest...}", auth.requireOrgJWT(func(w http.ResponseWriter, r *http.Request) { - rest := r.PathValue("rest") - idx := strings.LastIndex(rest, "/") - if idx < 0 { - writeJSON(w, http.StatusBadRequest, map[string]any{"message": "path and name required"}) - return - } - path, name := rest[:idx], rest[idx+1:] - env := r.URL.Query().Get("env") - if env == "" { - env = "default" - } - sec, err := secStore.Get(path, name, env) - if err != nil { - writeJSON(w, http.StatusNotFound, map[string]any{"message": "not found"}) - return - } - writeJSON(w, http.StatusOK, map[string]any{ - "secret": map[string]any{"value": string(sec.Ciphertext)}, - }) - })) - - // POST /v1/kms/orgs/{org}/secrets — create a secret. The handler is a - // named func (putSecretHandler) so the env-required contract is unit - // testable without standing up the full server. - mux.HandleFunc("POST /v1/kms/orgs/{org}/secrets", auth.requireOrgJWT(putSecretHandler(secStore))) - - // DELETE /v1/kms/orgs/{org}/secrets/{rest...}/{name} - mux.HandleFunc("DELETE /v1/kms/orgs/{org}/secrets/{rest...}", auth.requireOrgJWT(func(w http.ResponseWriter, r *http.Request) { - rest := r.PathValue("rest") - idx := strings.LastIndex(rest, "/") - if idx < 0 { - writeJSON(w, http.StatusBadRequest, map[string]any{"message": "path and name required"}) - return - } - path, name := rest[:idx], rest[idx+1:] - env := r.URL.Query().Get("env") - if env == "" { - env = "default" - } - if err := secStore.Delete(path, name, env); err != nil { - writeJSON(w, http.StatusNotFound, map[string]any{"message": "not found"}) - return - } - writeJSON(w, http.StatusOK, map[string]any{"ok": true}) - })) - - // Legacy: env-backed secret fetch. - mux.HandleFunc("GET /v1/kms/secrets/{name}", func(w http.ResponseWriter, r *http.Request) { - name := r.PathValue("name") - if name == "" { - writeJSON(w, http.StatusBadRequest, map[string]any{"message": "secret name required"}) - return - } - val := os.Getenv(name) - if val == "" { - writeJSON(w, http.StatusNotFound, map[string]any{"message": "not found"}) - return - } - writeJSON(w, http.StatusOK, map[string]any{ - "secret": map[string]any{"secretKey": name, "secretValue": val}, - }) - }) + // Secret CRUD surface — org-scoped, JWT-gated, ZapDB-backed. Extracted + // into one named registrar (mirrors registerKMSRoutes / registerOIDCRoutes + // / registerWebUI) so the exact route set the server exposes is testable + // in isolation. There is deliberately NO env-var fetch route: the KMS + // process environment (KMS_MASTER_KEY_B64 root REK, MPC_TOKEN, S3 keys) is + // never reachable over HTTP. Secrets flow ONLY through these org-scoped + // routes and the ZAP wire. Regression: TestSecretRoutes_NoEnvVarLeak. + registerSecretRoutes(mux, auth, secStore) // MPC key management (only when MPC_VAULT_ID is set). // @@ -479,6 +422,78 @@ func main() { srv.Shutdown(ctx) } +// registerSecretRoutes installs the org-scoped secret CRUD surface — the ONLY +// HTTP way to read or write secrets: +// +// GET /v1/kms/orgs/{org}/secrets/{path...}/{name} read a secret value +// POST /v1/kms/orgs/{org}/secrets create/upsert (env required) +// DELETE /v1/kms/orgs/{org}/secrets/{path...}/{name} delete a secret +// +// Every route is wrapped in auth.requireOrgJWT: an IAM-signed bearer whose +// owner/name/tag resolves to {org} (or carries the kms-admin role) is +// mandatory, verified against IAM's JWKS. The values live in ZapDB (encrypted +// at rest) and are also reachable over the ZAP wire. +// +// There is intentionally no `GET /v1/kms/secrets/{name}` env-var route. Such a +// route once read os.Getenv of an attacker-supplied name and returned it +// UNWRAPPED — no auth middleware — leaking the KMS process environment +// (KMS_MASTER_KEY_B64 root REK, MPC_TOKEN, S3 backup keys) to anyone who +// reached :8080 past the network boundary (NetworkPolicy gap, port-forward, +// pod compromise, SSRF). It was deleted, not gated: it had zero callers (the +// kms-operator and in-cluster clients read via the org-scoped path or ZAP) and +// process env is never a secret-fetch source. Regression that keeps it gone: +// TestSecretRoutes_NoEnvVarLeak. +func registerSecretRoutes(mux *http.ServeMux, auth *orgJWTAuth, secStore *store.SecretStore) { + // GET /v1/kms/orgs/{org}/secrets/{path...}/{name} + // Matches the ATS kmsclient.Get() URL pattern. + mux.HandleFunc("GET /v1/kms/orgs/{org}/secrets/{rest...}", auth.requireOrgJWT(func(w http.ResponseWriter, r *http.Request) { + rest := r.PathValue("rest") + idx := strings.LastIndex(rest, "/") + if idx < 0 { + writeJSON(w, http.StatusBadRequest, map[string]any{"message": "path and name required"}) + return + } + path, name := rest[:idx], rest[idx+1:] + env := r.URL.Query().Get("env") + if env == "" { + env = "default" + } + sec, err := secStore.Get(path, name, env) + if err != nil { + writeJSON(w, http.StatusNotFound, map[string]any{"message": "not found"}) + return + } + writeJSON(w, http.StatusOK, map[string]any{ + "secret": map[string]any{"value": string(sec.Ciphertext)}, + }) + })) + + // POST /v1/kms/orgs/{org}/secrets — create a secret. The handler is a + // named func (putSecretHandler) so the env-required contract is unit + // testable without standing up the full server. + mux.HandleFunc("POST /v1/kms/orgs/{org}/secrets", auth.requireOrgJWT(putSecretHandler(secStore))) + + // DELETE /v1/kms/orgs/{org}/secrets/{rest...}/{name} + mux.HandleFunc("DELETE /v1/kms/orgs/{org}/secrets/{rest...}", auth.requireOrgJWT(func(w http.ResponseWriter, r *http.Request) { + rest := r.PathValue("rest") + idx := strings.LastIndex(rest, "/") + if idx < 0 { + writeJSON(w, http.StatusBadRequest, map[string]any{"message": "path and name required"}) + return + } + path, name := rest[:idx], rest[idx+1:] + env := r.URL.Query().Get("env") + if env == "" { + env = "default" + } + if err := secStore.Delete(path, name, env); err != nil { + writeJSON(w, http.StatusNotFound, map[string]any{"message": "not found"}) + return + } + writeJSON(w, http.StatusOK, map[string]any{"ok": true}) + })) +} + // putSecretHandler serves POST /v1/kms/orgs/{org}/secrets (create/upsert). // // env is a first-class component of the storage key diff --git a/kms.go b/kms.go index e6bc1eed9..7c75d19a9 100644 --- a/kms.go +++ b/kms.go @@ -1,9 +1,9 @@ // Package kms is the canonical Go client for KMS. // // Transport: native luxfi/zap binary protocol on port 9999. There is no -// HTTP fallback in this package — services that need cross-cluster reach -// can shell out to /v1/kms/secrets/{name} via curl, but in-cluster Go is -// always ZAP. +// HTTP fallback in this package — services that need cross-cluster reach use +// the org-scoped HTTP surface (GET /v1/kms/orgs/{org}/secrets/{path}/{name} +// with an IAM bearer), but in-cluster Go is always ZAP. // // Defaults read from environment: //