v0.1.11 - Fix named path-role display and allow fixed paths without range/set

2026-08-04 20:53:12 -04:00
parent 86a97aee01
commit f0e90e0ea6
32 changed files with 2516 additions and 42 deletions
@@ -0,0 +1,406 @@
+# Plan: Named path-roles + path-template whitelist in the wizard
+
+## Goal
+
+Let the user define **named roles bound to a derivation path template** in the
+interactive wizard. The client then selects a key by **role name** (not by raw
+path), and optionally by an **index within the role's allowed range**. The
+derivation path stays hidden on the signer side — the role name acts as an
+access token: if the client doesn't know the name, they can't get the key.
+
+Example wizard session:
+
+```
+Define a named path role? [y/N] y
+  Role name: myrole
+  Purpose [nostr]: nostr
+  Curve  [secp256k1]: secp256k1
+  Path template: m/44'/1237'/0-3/1/0
+  Default index: 1   (optional — press Enter to require explicit index)
+
+  Role 'myrole' registered: purpose=nostr curve=secp256k1 path=m/44'/1237'/0-3/1/0 (index 0..3, default 1).
+Define another? [y/N] n
+```
+
+The purpose + curve combination must be valid per `crypto_alg_from_role()`
+(see [`src/key_store.c`](src/key_store.c) / [`src/enforcement.c`](src/enforcement.c)).
+The wizard validates the combination and re-prompts on invalid input. Valid
+combinations:
+
+| Purpose   | Curve          | Algorithm      | Typical path prefix        |
+|-----------|----------------|----------------|----------------------------|
+| nostr     | secp256k1      | secp256k1      | m/44'/1237'/...            |
+| bitcoin   | secp256k1      | secp256k1      | m/84'/0'/... / m/86'/...   |
+| ssh       | ed25519        | ed25519        | m/44'/102001'/...          |
+| age       | x25519         | x25519         | m/44'/102002'/...          |
+| fips      | secp256k1      | secp256k1      | (FIPS mode)                |
+| pq-sig    | ml-dsa-65      | ml-dsa-65      | m/44'/102003'/...          |
+| pq-sig    | slh-dsa-128s   | slh-dsa-128s   | m/44'/102004'/...          |
+| pq-kem    | ml-kem-768     | ml-kem-768     | m/44'/102005'/...          |
+
+The curve determines which `derive_*` function runs
+([`derive_for_role`](src/key_store.c:1004)). The path template is passed
+verbatim to `crypto_derive_seed_from_mnemonic` for all curves except
+`secp256k1`+`nostr`, which uses the NIP-06 helper when the path matches the
+NIP-06 form and the new `nostr_derive_keys_from_path` helper otherwise.
+
+Client requests:
+
+```json
+{"id":"1","method":"nostr_get_public_key","params":[{},{"role":"myrole"}]}
+```
+→ derives `m/44'/1237'/1/1/0` (default index 1) and returns the pubkey.
+
+```json
+{"id":"2","method":"nostr_get_public_key","params":[{},{"role":"myrole","index":2}]}
+```
+→ derives `m/44'/1237'/2/1/0` (index 2, within allowed range 0-3).
+
+```json
+{"id":"3","method":"nostr_get_public_key","params":[{},{"role":"myrole","index":5}]}
+```
+→ `2003 index_out_of_range` (5 is outside 0-3).
+
+```json
+{"id":"4","method":"nostr_get_public_key","params":[{},{"role":"unknown"}]}
+```
+→ `1002 unknown_role` (name not registered).
+
+## Why this design
+
+The user's insight: a **role name is a password**. The client never sees the
+derivation path; they only know the role name the operator gave them. This:
+
+1. **Hides the path** from the client — they can't enumerate or guess paths.
+2. **Acts as access control** — must know the name to get the key.
+3. **Enforces a range** — the server only derives paths within the template's
+   range, so even a knowing client can't escape to `m/44'/1237'/99/1/0`.
+4. **Is backward compatible** — existing `nostr_index` and `role_path`
+   selectors still work; named path-roles are an additive feature.
+
+## Root cause recap (3 compounding defects this plan fixes)
+
+1. No code path registers `SELECTOR_ROLE_PATH` roles at runtime — only
+   `SELECTOR_NOSTR_INDEX` roles are created
+   ([`role_table_register_nostr_index`](src/role_table.c:805),
+   [`setup_default_role`](src/main.c:1708)).
+2. [`crypto_derive_all`](src/key_store.c:1054) / [`crypto_derive_one`](src/key_store.c:1102)
+   explicitly skip roles where `selector_type != SELECTOR_NOSTR_INDEX`.
+3. [`derive_secp256k1`](src/key_store.c:699) builds the path from `role->nostr_index`,
+   ignoring `role->role_path` entirely. The other derive_* functions
+   (ed25519, x25519, ml_dsa_65, slh_dsa_128s, ml_kem_768) do the same via
+   `snprintf(..., "m/44'/10200X'/%d'/0'/0'", role->nostr_index)`.
+
+The "auto approve all" setting ([`g_prompt_always_allow`](src/server.c:953)) only
+bypasses the approval prompt — it never runs because the 1002 hard selector error
+fires first at [`server.c:2074`](src/server.c:2074) /
+[`dispatcher.c:1784`](src/dispatcher.c:1784).
+
+## Design
+
+### New: path-template role entry
+
+Extend `role_entry_t` (in `src/role_table.c` and mirrored decls) with two
+fields:
+
+```c
+/* In role_entry_t, added after role_path[]: */
+int  path_range_lo;    /* for SELECTOR_ROLE_PATH roles: inclusive lower bound
+                          for the %d placeholder in role_path; -1 = no range
+                          (single fixed path) */
+int  path_range_hi;    /* inclusive upper bound; == path_range_lo for single */
+int  path_default_index; /* default index to use when client sends {"role":...}
+                            without "index"; -1 = require explicit index */
+```
+
+A path-template role stores its template in `role_path` with a `%d`-style
+placeholder segment, e.g. `role_path = "m/44'/1237'/%d/1/0"`,
+`path_range_lo = 0`, `path_range_hi = 3`, `path_default_index = 1`.
+
+### Path-template data model for the whitelist
+
+(Kept from the previous plan — the whitelist is the underlying mechanism the
+wizard uses to validate, but the user-facing UX is the named-role prompt.)
+
+```c
+#define PATH_WHITELIST_MAX_TEMPLATES 16
+#define PATH_TEMPLATE_MAX_LEN 128
+
+typedef struct {
+    char template[PATH_TEMPLATE_MAX_LEN];  /* "m/44'/1237'/%d/1/0" */
+    int  range_lo;
+    int  range_hi;
+} path_template_t;
+
+typedef struct {
+    int  active;
+    int  count;
+    path_template_t templates[PATH_WHITELIST_MAX_TEMPLATES];
+} path_whitelist_t;
+```
+
+Add `path_whitelist_t path_whitelist;` to `server_ctx_t`.
+
+### Spec syntax (for `--allow-index` CLI flag and raw whitelist input)
+
+Each comma-separated token may be:
+
+- `all`  → no restriction
+- `0-3` / `0,1,3`  → existing integer `nostr_index` syntax (backward compat)
+- `m/44'/1237'/0-3/0/0`  → path template, range 0..3
+- `m/44'/1237'/0-3/1/0`  → path template, range 0..3 (the user's case)
+- `m/44'/1237'/0-3/0/0,m/44'/1237'/0-3/1/0`  → multiple templates
+
+A token containing `/` is a path template; the first segment matching
+`^[0-9]+(-[0-9]+)?$` is the range placeholder.
+
+### Named-role wizard syntax (primary UX)
+
+The wizard prompt offers two modes:
+
+1. **Quick mode** (existing): enter a whitelist spec as above. Roles are
+   auto-registered on demand when a client sends a matching `role_path`.
+2. **Named mode** (new): define named roles bound to path templates. The
+   client uses `{"role":"name"}` (optionally with `"index":N`).
+
+## Implementation steps
+
+### Step 1 — Extend `role_entry_t` with path-range fields
+
+Files: `src/role_table.c` (definition), and every .c with headerless decls
+mirroring `role_entry_t` (search for `selector_type` field to find all copies).
+Add `path_range_lo`, `path_range_hi`, `path_default_index` after `role_path[]`.
+
+### Step 2 — Add `path_whitelist_t` struct + field to `server_ctx_t`
+
+Files: `src/server.c` (definition + field), `src/main.c` (headerless decls
+mirror), and any other .c declaring `server_ctx_t` (search for
+`index_whitelist_active`). Add constants `PATH_WHITELIST_MAX_TEMPLATES`,
+`PATH_TEMPLATE_MAX_LEN`.
+
+### Step 3 — Implement `server_set_path_whitelist()` parser in `src/server.c`
+
+```c
+int server_set_path_whitelist(server_ctx_t *ctx, const char *spec);
+```
+
+Unified parser: integer tokens → existing bitmap; path-template tokens →
+`path_whitelist.templates[]`. `"all"` clears both. Returns 0 / -1.
+
+Keep `server_set_index_whitelist` as a thin wrapper (backward compat).
+
+### Step 4 — Implement `server_path_whitelist_allows()` in `src/server.c`
+
+```c
+int server_path_whitelist_allows(const server_ctx_t *ctx, const char *role_path);
+```
+
+Iterate templates, format each candidate with the range, `strcmp`. Return 1/0.
+
+### Step 5 — Add `role_table_register_role_path()` helper in `src/role_table.c`
+
+```c
+int role_table_register_role_path(role_table_t *table, const char *path,
+                                  role_purpose_t purpose, role_curve_t curve,
+                                  int range_lo, int range_hi, int default_index);
+```
+
+- `purpose` and `curve` are caller-supplied (from the wizard prompt), not
+  hardcoded. The caller must validate the combination via
+  `crypto_alg_from_role(curve, purpose) != CRYPTO_ALG_UNKNOWN` before calling.
+- Idempotent via `role_table_find_by_path` (compare template + range).
+- Sets `selector_type = SELECTOR_ROLE_PATH`, copies `path` (with `%d`)
+  into `role_path`, sets `purpose`/`curve`/`purpose_str`/`curve_str` from the
+  enum + string forms, sets the range fields, `derived = 0`.
+- Add the prototype to the headerless-decls block in every .c that includes
+  role_table decls.
+
+### Step 6 — Make `derive_secp256k1` honor `role_path` in `src/key_store.c`
+
+- When `role->selector_type == SELECTOR_ROLE_PATH`:
+  - If `role_path` contains `%d`, the caller must have already resolved the
+    concrete path (see Step 8 — the server formats `role_path` with the
+    chosen index before calling `crypto_derive_one`). So `derive_secp256k1`
+    just uses `role->role_path` directly as the full BIP-32 path.
+  - Call `crypto_derive_seed_from_mnemonic(phrase, role->role_path, seed, 32)`
+    then derive secp256k1 priv/pub from that seed.
+  - Add helper `nostr_derive_keys_from_path(const char *mnemonic, const char *path,
+    unsigned char *priv, unsigned char *pub)` (or inline using the existing
+    BIP-32 seed→key derivation that `nostr_derive_keys_from_mnemonic` uses).
+- When `SELECTOR_NOSTR_INDEX`, keep existing behavior.
+- Apply the same `SELECTOR_ROLE_PATH` branch to the other derive_* functions.
+
+### Step 7 — Remove the `SELECTOR_NOSTR_INDEX`-only guards in `src/key_store.c`
+
+- [`crypto_derive_all`](src/key_store.c:1054): allow `SELECTOR_ROLE_PATH`.
+- [`crypto_derive_one`](src/key_store.c:1102): allow `SELECTOR_ROLE_PATH`.
+
+### Step 8 — Wire named path-roles + whitelist into `src/server.c` request handling
+
+In the selector-resolution block ([`server.c:2028-2066`](src/server.c:2028)):
+
+**Case A — client sends `{"role":"myrole"}` (named path-role):**
+- `selector_resolve` finds the role by name (already works for registered roles).
+- If the role is a path-template role (`SELECTOR_ROLE_PATH` with `%d`):
+  - Read optional `"index"` from the request options.
+  - If no `index` and `path_default_index >= 0` → use `path_default_index`.
+  - If no `index` and `path_default_index < 0` → `2004 index_required`.
+  - Validate `index` is in `[path_range_lo, path_range_hi]` → else `2003 index_out_of_range`.
+  - Format the concrete path: `snprintf(concrete, ..., role_path, index)`.
+  - Set `pending_derivation = 1` if the role isn't derived yet, with the
+    concrete path stored for `crypto_derive_one`.
+- If the role is a `nostr_index` role → existing behavior.
+
+**Case B — client sends `{"role_path":"m/44'/1237'/1/1/0"}` (raw path):**
+- If `server_path_whitelist_allows(ctx, role_path)` → set
+  `pending_derivation = 1`, synthesize role name, `purpose=nostr`,
+  `curve=secp256k1`.
+- Else → `2003 path_not_allowed`.
+
+**Case C — client sends `{"nostr_index":N}`:** existing behavior unchanged.
+
+In the `if (pchk == POLICY_ALLOW && pending_derivation)` block
+([`server.c:2106`](src/server.c:2106)):
+- For named path-roles: the role already exists in the table; just call
+  `crypto_derive_one` with the concrete path (temporarily set
+  `role->role_path` to the concrete path, or pass the path via a side channel).
+- For raw `role_path`: `role_table_register_role_path` (no `%d`, fixed path)
+  → `crypto_derive_one`.
+
+### Step 9 — Add the named-role wizard prompt in `src/main.c`
+
+New function `prompt_named_path_roles(role_table_t *role_table)`:
+
+```
+Define a named path role? [y/N] y
+  Role name: myrole
+  Purpose [nostr]: nostr
+  Curve  [secp256k1]: secp256k1
+  Path template (use 0-3 for a range, or a single number): m/44'/1237'/0-3/1/0
+  Default index [1]: 1
+  Role 'myrole' registered: purpose=nostr curve=secp256k1 path=m/44'/1237'/0-3/1/0 (index 0..3, default 1).
+Define another? [y/N] n
+```
+
+- **Purpose** prompt: default `nostr`; accept any of
+  `nostr|bitcoin|ssh|age|fips|pq-sig|pq-kem`; parse via
+  `role_purpose_from_str()`.
+- **Curve** prompt: default `secp256k1`; accept any of
+  `secp256k1|ed25519|x25519|ml-dsa-65|slh-dsa-128s|ml-kem-768`; parse via
+  `role_curve_from_str()`.
+- **Validate** the purpose+curve combination:
+  `crypto_alg_from_role(curve, purpose) != CRYPTO_ALG_UNKNOWN`; re-prompt on
+  invalid combo (e.g. `nostr`+`ed25519` is invalid).
+- Parse the path template: find the range segment, extract `range_lo`/`range_hi`,
+  store template with `%d`.
+- Call `role_table_register_role_path(table, template, purpose, curve,
+  range_lo, range_hi, default_index)`.
+- Loop until user declines.
+- Call this after [`setup_default_role`](src/main.c:1708) and before
+  `crypto_derive_all` (so named roles are pre-derived at startup using their
+  default index).
+
+Also update [`prompt_index_whitelist()`](src/main.c:2088) to mention that
+named path-roles bypass the raw-path whitelist (they're explicitly registered).
+
+### Step 10 — Update `--allow-index` flag + wizard text in `src/main.c`
+
+- Update `--allow-index` help ([`main.c:1109`](src/main.c:1109)) to mention
+  path templates.
+- Update call sites at [`main.c:2902`](src/main.c:2902) /
+  [`main.c:2945`](src/main.c:2945) / [`main.c:2973`](src/main.c:2973) to call
+  `server_set_path_whitelist`.
+
+### Step 11 — (Optional) Also handle `role_path` in `src/dispatcher.c`
+
+[`dispatcher.c:1778-1791`](src/dispatcher.c:1778) returns 1002 on
+`SELECTOR_ERR_NOT_FOUND`. **Decision**: scope to `server.c` only for now;
+stdio/qrexec still returns 1002 for unknown `role_path` (future work). Named
+roles registered at startup work everywhere because they're in the role table
+before any request arrives.
+
+### Step 12 — Tests
+
+- [`tests/test_role_table.c`](tests/test_role_table.c): test
+  `role_table_register_role_path` (idempotent, range fields stored).
+- [`tests/test_integration.c`](tests/test_integration.c) or new
+  `tests/test_path_whitelist.c`:
+  - Parse `m/44'/1237'/0-3/0/0` → assert `server_path_whitelist_allows` returns
+    1 for `m/44'/1237'/2/0/0` and 0 for `m/44'/1237'/5/0/0`.
+  - Parse `m/44'/1237'/0-3/1/0` → assert allows `m/44'/1237'/1/1/0` (the user's
+    exact case), denies `m/44'/1237'/1/0/0`.
+  - End-to-end (named role): register `myrole` with template
+    `m/44'/1237'/%d/1/0`, range 0-3, default 1. Send
+    `{"role":"myrole"}` → assert pubkey for `m/44'/1237'/1/1/0`.
+    Send `{"role":"myrole","index":2}` → assert pubkey for
+    `m/44'/1237'/2/1/0`. Send `{"role":"myrole","index":5}` → assert
+    `2003 index_out_of_range`.
+  - End-to-end (raw path): start server with
+    `--allow-index "m/44'/1237'/0-3/1/0"`, send
+    `{"role_path":"m/44'/1237'/1/1/0"}` → assert valid pubkey.
+    Send `{"role_path":"m/44'/1237'/1/0/0"}` → assert `2003 path_not_allowed`.
+  - Backward compat: `--allow-index "0-3"` still works for `nostr_index`.
+
+### Step 13 — Docs
+
+- [`README.md`](README.md) §4.6: document named path-roles, the `"index"`
+  option, and the `2003`/`2004` error codes.
+- [`README.md`](README.md) §3 (wizard): document the named-role prompt.
+- [`api.md`](api.md): add error codes `2003 path_not_allowed` /
+  `2003 index_out_of_range` / `2004 index_required`.
+- [`README.md`](README.md) error table: add the new codes.
+
+## New error codes
+
+| Code  | Message              | Meaning                                              |
+|-------|----------------------|------------------------------------------------------|
+| 2003  | `path_not_allowed`   | `role_path` not on the path whitelist.               |
+| 2003  | `index_out_of_range` | `index` outside the named role's `[lo,hi]` range.    |
+| 2004  | `index_required`     | Named path-role has no default index and none given. |
+
+(2003 is reused for both path-not-allowed and index-out-of-range since they're
+both "whitelist range" violations; the message distinguishes them. If you
+prefer distinct codes, use 2005 for `index_out_of_range`.)
+
+## Open questions / decisions
+
+- **Placeholder detection**: first path segment matching `^[0-9]+(-[0-9]+)?$`
+  is the range. No explicit `X` char needed.
+- **Default purpose/curve**: `nostr` / `secp256k1` for now. Inferring from path
+  prefix is future work.
+- **Flag name**: keep `--allow-index` for backward compat; path syntax accepted
+  by the same flag.
+- **Pre-derivation**: named roles with a default index are pre-derived at
+  startup (in `crypto_derive_all`); roles without a default are derived on
+  first request.
+- **dispatcher.c scope**: stdio/qrexec gets named roles (they're in the table
+  at startup) but not raw-path auto-registration (future work).
+- **Distinct error codes for 2003**: decision pending — reuse 2003 with
+  different messages, or split into 2003/2005.
+
+## Mermaid: request flow after implementation
+
+```mermaid
+flowchart TD
+  A[Client request] --> B{selector type?}
+  B -- role name --> C[role_table_find_by_name]
+  C --> D{found?}
+  D -- no --> E[1002 unknown_role]
+  D -- yes --> F{is path-template role?}
+  F -- no, nostr_index --> G[existing nostr_index path]
+  F -- yes --> H{index in options?}
+  H -- yes --> I{index in range lo..hi?}
+  H -- no --> J{default_index set?}
+  J -- no --> K[2004 index_required]
+  J -- yes --> I
+  I -- no --> L[2003 index_out_of_range]
+  I -- yes --> M[format concrete path with index]
+  M --> N[derive + execute verb]
+  G --> N
+  B -- role_path --> O[server_path_whitelist_allows]
+  O -- no --> P[2003 path_not_allowed]
+  O -- yes --> Q[auto-register + derive]
+  Q --> N
+  B -- nostr_index --> R[existing index whitelist check]
+  R --> N
+```