Commit fe29adc
authored
feat(kmsClient): ECDSA-attested decrypt, environment presets, and creator-bound ECDSA /secrets (#109)
## Overview
This PR makes the `kms-client` CLI usable with ECDSA attestation, adds a
named-environment shortcut so connection flags don't have to be retyped,
and — most importantly — fixes a security gap in how operators authorize
ECDSA `/secrets` requests.
It bundles three related changes (each developed spec → plan → TDD, docs
under `docs/superpowers/`):
1. **Client: ECDSA-attested `decrypt`** (`cmd/kmsClient`)
2. **Client: `--environment` connection presets** (`cmd/kmsClient`)
3. **Server: bind ECDSA `/secrets` to the app creator + drop the release
requirement** (`pkg/node`)
> Note: the branch name (`feat/kmsclient-ecdsa-attestation`) predates
changes 2 and 3 — the scope grew during review/testing.
---
## 1. ECDSA-attested decrypt (client)
The CLI's `decrypt` previously only used the unauthenticated `/app/sign`
endpoint. It now optionally authenticates with ECDSA challenge-response
attestation against the `/secrets` endpoint.
New flags on `decrypt`:
- `--attestation` — `""` (default, legacy `/app/sign`) or `ecdsa`. Any
other value is a usage error.
- `--ecdsa-private-key` — hex-encoded secp256k1 key (optional `0x`
prefix). Takes priority over the file flag.
- `--ecdsa-private-key-file` — path to a file holding the hex key.
When `--attestation ecdsa` is set, the CLI loads the key, generates an
ephemeral RSA transit keypair, calls the library's existing
`RetrieveSecretsWithOptions` to recover the app private key, then
decrypts the supplied ciphertext via `crypto.DecryptForApp`. When unset,
behavior is byte-for-byte unchanged. This is a thin wiring layer — no
library (`pkg/clients/kmsClient`) changes.
```bash
kms-client --avs-address 0x.. decrypt \
--app-id 0x<appAddr> --encrypted-data enc.hex \
--attestation ecdsa --ecdsa-private-key 0x<creatorKey>
```
## 2. `--environment` connection presets (client)
New global flag `--environment` / `-e` fills `--avs-address` and
`--operator-set-id` from a named preset, so they don't need to be passed
every invocation.
- First registered environment: `sepolia` →
`avs-address=0x47c9806e7DC4e6fE9a0a2399831F32d06DaE5730`,
`operator-set-id=0` (sourced from the operator deployment charts).
- **Precedence:** explicit flag > preset > built-in default.
`--avs-address` lost its hard `Required: true` and is now required only
when no preset supplies it.
- **The RPC URL is deliberately NOT part of any preset** — production
RPC URLs embed API-key credentials and must not be committed. Users
still pass `--rpc-url`.
- Resolution happens in a pure, unit-tested helper (`resolveConnection`)
and runs at the very top of `createClient`, so a config error (e.g.
missing avs-address) fails fast without needing a reachable RPC.
```bash
kms-client --environment sepolia --rpc-url https://my-sepolia-rpc/... \
get-pubkey --app-id 0x<appAddr>
```
## 3. Server: ECDSA `/secrets` binds to the app creator (security fix)
**The gap:** ECDSA attestation authenticated *nothing app-specific*. The
operator verified the request signature against the **client-supplied**
public key, but never tied that key to the app. Because an `appID` is an
app contract address with no private key, "prove you control a key" was
satisfied by any freshly generated keypair — so anyone could request any
app's key material over the ECDSA path. (In practice it failed for
unrelated reasons: ECDSA sets `ImageDigest="ecdsa:unverified"`, which
never matches a real release, so every ECDSA request against a real app
404'd or 403'd.)
**The fix** (`pkg/node/handlers.go`, ECDSA path of
`handleSecretsRequest`):
- **Owner binding** — derive the signer address from the verified
`claims.PublicKey` and require it to equal the app's on-chain creator
(`GetAppCreator(appID)`, already on the contract-caller interface). The
supplied ECDSA key must be the EOA that deployed/created the app. The
`appID` must be a valid contract address.
- **No release requirement for ECDSA** — ECDSA is a lightweight testing
method. It no longer fetches-and-requires an on-chain release, and skips
the image-digest / registry / container-policy checks (all meaningless
for ECDSA). Env is **best-effort**: if a release exists, its
`encrypted_env`/`public_env` are returned; otherwise empty env is
returned alongside the recovered key. The partial signature is always
returned on success.
- **All other methods unchanged** — gcp / intel / tpm / eigenx-snp keep
the full release + digest + registry + container-policy enforcement
(guarded by a regression test).
HTTP statuses on the ECDSA path: non-address appID → 400; unparseable
pubkey → 400; `GetAppCreator` failure → 502; signer ≠ creator → 403;
success → 200.
`pkg/attestation/ecdsa.go` is intentionally untouched — it already
surfaces the verified public key.
---
## Testing
- **`cmd/kmsClient`** — `TestLoadECDSAKey` (8 cases: file/flag
precedence, `0x` prefix, whitespace, errors) and `TestResolveConnection`
/ `TestSupportedEnvironmentsString` (preset fill, explicit override,
unknown env, missing-avs error).
- **`pkg/contractCaller`** — `TestTestableStubGetAppCreator` for the new
configurable-creator test hook.
- **`pkg/node`** — 6 new ECDSA `/secrets` subtests: owner+env,
owner+no-release (→ 200 empty env, the no-404 fix),
owner+empty-env-release, wrong-signer (403), non-address appID (400),
bad pubkey (400) — plus a `NonECDSAStillRequiresRelease` regression
guard proving non-ECDSA enforcement is intact.
All suites green locally: `pkg/node`, `pkg/contractCaller`,
`cmd/kmsClient` all `ok`. The full `pkg/node` suite (including the
anvil-backed persistence tests) passes.
## Docs
`cmd/kmsClient/README.md` documents the attestation flags, the
`--environment` preset (incl. the RPC-secret rationale), and the ECDSA
security model — notably that the key must be the app creator's, and
that the attested path no longer needs a release. Design specs and
implementation plans live under `docs/superpowers/`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)20 files changed
Lines changed: 3037 additions & 134 deletions
File tree
- .github/workflows
- cmd/kmsClient
- docs/superpowers
- plans
- specs
- internal
- testData
- tests/integration
- pkg
- contractCaller
- node
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
68 | 68 | | |
69 | 69 | | |
70 | 70 | | |
71 | | - | |
| 71 | + | |
72 | 72 | | |
73 | 73 | | |
74 | 74 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
55 | 107 | | |
56 | 108 | | |
57 | 109 | | |
58 | 110 | | |
59 | 111 | | |
60 | 112 | | |
61 | 113 | | |
62 | | - | |
| 114 | + | |
63 | 115 | | |
64 | 116 | | |
65 | | - | |
| 117 | + | |
66 | 118 | | |
67 | 119 | | |
68 | 120 | | |
| |||
110 | 162 | | |
111 | 163 | | |
112 | 164 | | |
| 165 | + | |
113 | 166 | | |
114 | | - | |
| 167 | + | |
115 | 168 | | |
116 | 169 | | |
117 | | - | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
0 commit comments