diff --git a/nostr_core/NSIGNER_INTEGRATION.md b/nostr_core/NSIGNER_INTEGRATION.md index 50176ec8..52aabc35 100644 --- a/nostr_core/NSIGNER_INTEGRATION.md +++ b/nostr_core/NSIGNER_INTEGRATION.md @@ -49,13 +49,50 @@ nostr_signer_t* nostr_signer_nsigner_unix(const char* socket_name, const char* r nostr_signer_t* nostr_signer_nsigner_serial(const char* device_path, const char* role, int timeout_ms); nostr_signer_t* nostr_signer_nsigner_tcp(const char* host, int port, const char* role, int timeout_ms); nostr_signer_t* nostr_signer_nsigner_fds(int read_fd, int write_fd, const char* role, int timeout_ms); +nostr_signer_t* nostr_signer_nsigner_qrexec(const char* target_qube, const char* service_name, const char* role, int timeout_ms); /* TCP mode requires auth envelope per n_signer; set this before making calls. */ int nostr_signer_nsigner_set_auth(nostr_signer_t* signer, const unsigned char auth_privkey[32], const char* label); +/* Set the full BIP-44 role_path. n_signer requires BOTH role and role_path + * together for nostr_* verbs; set the role via the constructor and the path + * here. */ +int nostr_signer_nsigner_set_role_path(nostr_signer_t* signer, const char* role_path); +/* Compatibility shim: expands NIP-06 index N into role="main" + + * role_path="m/44'/1237'/N'/0/0" and stores N as the derive index. */ +int nostr_signer_nsigner_set_nostr_index(nostr_signer_t* signer, int nostr_index); #endif ``` +### 2.1.1 Key selector model + +n_signer's `nostr_*` verbs select a key via a **combined** `role` + `role_path` +selector — both fields are required together. The remote backend emits: + +```json +{"role":"","role_path":"m/44'/1237'/N'/0/0"} +``` + +as the trailing options object of every `nostr_*` request. Omitting either +field is rejected by n_signer with `2008 role_required` / `2009 path_required`. +The bare `{"nostr_index":N}` selector is **removed** on the n_signer side +(error `2006 nostr_index_deprecated`); this library never emits it. + +Two ways to set the selector: + +1. **`nostr_signer_nsigner_set_role_path(signer, path)`** — explicit. Pass the + role name to the constructor and the full path here. This is the canonical + path for new code. +2. **`nostr_signer_nsigner_set_nostr_index(signer, N)`** — convenience. Expands + `N` into `role="main"` + `role_path="m/44'/1237'/N'/0/0"` and also stores `N` + as the algorithm `index` used by `nostr_signer_derive_hmac`. The signer must + have a `main` role whose template matches the expanded path. + +For the algorithm-based `derive` verb (used by `nostr_signer_derive_hmac`), the +backend emits `{"algorithm":"secp256k1","index":N}`. The index comes from the +last `set_nostr_index` call; if unset, the request is sent without `index` and +n_signer rejects it with `missing_index`. + ### 2.2 Transport constructors + framed I/O + discovery (`nostr_core/nsigner_transport.h`) ```c @@ -132,7 +169,7 @@ The remote signer backend uses n_signer's role-based Nostr methods: These wire names are distinct from both the public `nostr_signer_*` C API and the unprefixed NIP-46 method names. In particular, n_signer's unprefixed `get_public_key` is algorithm-based; this library uses `nostr_get_public_key` because remote signer -factories select keys by `role` or `nostr_index`. +factories select keys by `role` + `role_path` (see §2.1.1). ### Auth and framing notes diff --git a/nostr_core/nostr_signer.c b/nostr_core/nostr_signer.c index 3bddbfe2..ed67942f 100644 --- a/nostr_core/nostr_signer.c +++ b/nostr_core/nostr_signer.c @@ -1,5 +1,6 @@ #include "nostr_signer.h" +#include #include #include #include @@ -30,8 +31,10 @@ struct nostr_signer { struct { nsigner_client_t* client; char role[64]; - int nostr_index; /* -1 = not set; when set, overrides role */ - int has_nostr_index; /* 1 if nostr_index is set */ + char role_path[128]; /* full BIP-44 path; required with role for nostr_* verbs */ + int has_role_path; /* 1 if role_path was set explicitly */ + int derive_index; /* algorithm index for the derive verb; -1 = not set */ + int has_derive_index; /* 1 if derive_index was set */ } remote; #endif } u; @@ -306,7 +309,10 @@ static int signer_local_derive_hmac(nostr_signer_t* signer, #if defined(NOSTR_ENABLE_NSIGNER_CLIENT) -static cJSON* signer_remote_params_with_selector(cJSON* params, const char* role, int has_nostr_index, int nostr_index) { +static cJSON* signer_remote_params_with_selector(cJSON* params, + const char* role, + const char* role_path, + int has_role_path) { cJSON* selector; if (params == NULL) { @@ -316,19 +322,9 @@ static cJSON* signer_remote_params_with_selector(cJSON* params, const char* role } } - /* nostr_index takes precedence over role when set */ - if (has_nostr_index) { - selector = cJSON_CreateObject(); - if (selector == NULL) { - cJSON_Delete(params); - return NULL; - } - cJSON_AddNumberToObject(selector, "nostr_index", nostr_index); - cJSON_AddItemToArray(params, selector); - return params; - } - - if (role == NULL || role[0] == '\0') { + /* n_signer requires BOTH role and role_path together for nostr_* verbs. + * If neither is set, emit nothing (caller handles the error). */ + if ((role == NULL || role[0] == '\0') && !has_role_path) { return params; } @@ -338,7 +334,12 @@ static cJSON* signer_remote_params_with_selector(cJSON* params, const char* role return NULL; } - cJSON_AddStringToObject(selector, "role", role); + if (role != NULL && role[0] != '\0') { + cJSON_AddStringToObject(selector, "role", role); + } + if (has_role_path && role_path != NULL && role_path[0] != '\0') { + cJSON_AddStringToObject(selector, "role_path", role_path); + } cJSON_AddItemToArray(params, selector); return params; } @@ -354,8 +355,8 @@ static int signer_remote_get_public_key(nostr_signer_t* signer, char out_pubkey_ } params = signer_remote_params_with_selector(NULL, signer->u.remote.role, - signer->u.remote.has_nostr_index, - signer->u.remote.nostr_index); + signer->u.remote.role_path, + signer->u.remote.has_role_path); if (params == NULL) { return NOSTR_ERROR_MEMORY_FAILED; } @@ -406,8 +407,8 @@ static int signer_remote_sign_event(nostr_signer_t* signer, const cJSON* unsigne free(event_json); params = signer_remote_params_with_selector(params, signer->u.remote.role, - signer->u.remote.has_nostr_index, - signer->u.remote.nostr_index); + signer->u.remote.role_path, + signer->u.remote.has_role_path); if (params == NULL) { return NOSTR_ERROR_MEMORY_FAILED; } @@ -466,8 +467,8 @@ static int signer_remote_encrypt_decrypt(nostr_signer_t* signer, cJSON_AddItemToArray(params, cJSON_CreateString(in)); params = signer_remote_params_with_selector(params, signer->u.remote.role, - signer->u.remote.has_nostr_index, - signer->u.remote.nostr_index); + signer->u.remote.role_path, + signer->u.remote.has_role_path); if (params == NULL) { return NOSTR_ERROR_MEMORY_FAILED; } @@ -546,22 +547,20 @@ static int signer_remote_derive_hmac(nostr_signer_t* signer, } cJSON_AddItemToArray(params, cJSON_CreateString(data)); - /* Build the options object with algorithm:"secp256k1" and the selector - * (role or nostr_index). The derive verb requires index, so when - * has_nostr_index is set we include it; otherwise we include role (the - * nsigner derive handler requires index, so callers using the remote - * backend must set nostr_index via nostr_signer_nsigner_set_nostr_index - * before calling derive_hmac). */ + /* Build the options object with algorithm:"secp256k1" and the index. + * The derive verb is algorithm-based and requires an explicit index + * (no default). The index is set via nostr_signer_nsigner_set_nostr_index + * (which stores it in derive_index) or directly via a future + * set_derive_index helper. Without an index the signer rejects the + * request with missing_index. */ opts = cJSON_CreateObject(); if (opts == NULL) { cJSON_Delete(params); return NOSTR_ERROR_MEMORY_FAILED; } cJSON_AddStringToObject(opts, "algorithm", "secp256k1"); - if (signer->u.remote.has_nostr_index) { - cJSON_AddNumberToObject(opts, "index", signer->u.remote.nostr_index); - } else if (signer->u.remote.role[0] != '\0') { - cJSON_AddStringToObject(opts, "role", signer->u.remote.role); + if (signer->u.remote.has_derive_index) { + cJSON_AddNumberToObject(opts, "index", signer->u.remote.derive_index); } cJSON_AddItemToArray(params, opts); @@ -627,6 +626,7 @@ void nostr_signer_free(nostr_signer_t* signer) { signer->u.remote.client = NULL; } memset(signer->u.remote.role, 0, sizeof(signer->u.remote.role)); + memset(signer->u.remote.role_path, 0, sizeof(signer->u.remote.role_path)); } #endif @@ -855,6 +855,32 @@ nostr_signer_t* nostr_signer_nsigner_qrexec(const char* target_qube, const char* return nostr_signer_nsigner_from_transport(transport, role); } +int nostr_signer_nsigner_set_role_path(nostr_signer_t* signer, const char* role_path) { + if (signer == NULL) { + return NOSTR_ERROR_INVALID_INPUT; + } + + if (signer->backend != NOSTR_SIGNER_BACKEND_NSIGNER_REMOTE || signer->u.remote.client == NULL) { + return NOSTR_ERROR_INVALID_INPUT; + } + + if (role_path == NULL || role_path[0] == '\0') { + /* Clear the role_path selector */ + signer->u.remote.has_role_path = 0; + signer->u.remote.role_path[0] = '\0'; + } else { + size_t n = strlen(role_path); + if (n >= sizeof(signer->u.remote.role_path)) { + return NOSTR_ERROR_INVALID_INPUT; + } + memcpy(signer->u.remote.role_path, role_path, n); + signer->u.remote.role_path[n] = '\0'; + signer->u.remote.has_role_path = 1; + } + + return NOSTR_SUCCESS; +} + int nostr_signer_nsigner_set_nostr_index(nostr_signer_t* signer, int nostr_index) { if (signer == NULL) { return NOSTR_ERROR_INVALID_INPUT; @@ -865,14 +891,30 @@ int nostr_signer_nsigner_set_nostr_index(nostr_signer_t* signer, int nostr_index } if (nostr_index < 0) { - /* Clear the nostr_index selector, fall back to role */ - signer->u.remote.has_nostr_index = 0; - signer->u.remote.nostr_index = -1; + /* Clear the selector */ + signer->u.remote.has_role_path = 0; + signer->u.remote.role_path[0] = '\0'; + signer->u.remote.has_derive_index = 0; + signer->u.remote.derive_index = -1; } else { - /* Set nostr_index; clear role to avoid ambiguous selector */ - signer->u.remote.has_nostr_index = 1; - signer->u.remote.nostr_index = nostr_index; - memset(signer->u.remote.role, 0, sizeof(signer->u.remote.role)); + /* Expand the NIP-06 index into role="main" + the full derivation path. + * n_signer no longer accepts a bare {"nostr_index":N}; it requires + * {"role":"main","role_path":"m/44'/1237'/N'/0/0"}. We also store N as + * the algorithm derive index so nostr_signer_derive_hmac works. */ + const char* main_role = "main"; + size_t rlen = strlen(main_role); + if (rlen >= sizeof(signer->u.remote.role)) { + return NOSTR_ERROR_INVALID_INPUT; + } + memcpy(signer->u.remote.role, main_role, rlen); + signer->u.remote.role[rlen] = '\0'; + + snprintf(signer->u.remote.role_path, sizeof(signer->u.remote.role_path), + "m/44'/1237'/%d'/0/0", nostr_index); + signer->u.remote.has_role_path = 1; + + signer->u.remote.derive_index = nostr_index; + signer->u.remote.has_derive_index = 1; } return NOSTR_SUCCESS; diff --git a/nostr_core/nostr_signer.h b/nostr_core/nostr_signer.h index a7937b13..6ced519c 100644 --- a/nostr_core/nostr_signer.h +++ b/nostr_core/nostr_signer.h @@ -58,10 +58,20 @@ int nostr_signer_nsigner_set_auth(nostr_signer_t* signer, const unsigned char auth_privkey[32], const char* label); /* - * Set the nostr_index selector for the remote nsigner backend. When set, - * requests use {"nostr_index":N} instead of {"role":"..."}. This is mutually - * exclusive with the role parameter (setting index clears role). - * Returns NOSTR_SUCCESS or an error code. + * Set the role_path selector for the remote nsigner backend. n_signer now + * requires BOTH "role" and "role_path" together for nostr_* verbs; set the + * role via the constructor (or keep the constructor default) and set the full + * BIP-44 derivation path here. Returns NOSTR_SUCCESS or an error code. + */ +int nostr_signer_nsigner_set_role_path(nostr_signer_t* signer, const char* role_path); +/* + * Compatibility shim: expands the NIP-06 index N into role="main" and + * role_path="m/44'/1237'/N'/0/0" and stores N as the algorithm derive index. + * n_signer no longer accepts a bare {"nostr_index":N} selector, so this is + * the supported way to select a key by NIP-06 account index. The signer must + * have a "main" role registered whose template matches the expanded path. + * Pass a negative value to clear the selector. Returns NOSTR_SUCCESS or an + * error code. */ int nostr_signer_nsigner_set_nostr_index(nostr_signer_t* signer, int nostr_index); #endif diff --git a/plans/n_signer_selector_rewrite.md b/plans/n_signer_selector_rewrite.md new file mode 100644 index 00000000..8618c00f --- /dev/null +++ b/plans/n_signer_selector_rewrite.md @@ -0,0 +1,73 @@ +# Plan: n_signer Selector Rewrite for nostr_core_lib — COMPLETED + +> **Status: Done.** The remote signer backend now emits the combined +> `role` + `role_path` selector required by current n_signer. The bare +> `nostr_index` selector is no longer emitted. `make` builds clean and the +> nsigner client test passes. + +## Context + +n_signer removed the `nostr_index` selector and `index`-on-nostr-verbs +([`n_signer/plans/role_path_authorization.md`](../../n_signer/plans/role_path_authorization.md)). +The only accepted selector for `nostr_*` verbs is now +`{"role":"","role_path":""}` sent together. Either field +alone is rejected (`2008 role_required` / `2009 path_required`); a bare +`nostr_index` is rejected with `2006 nostr_index_deprecated`. + +`nostr_core_lib`'s remote signer backend was emitting `{"nostr_index":N}` or +`{"role":"..."}` alone — both now invalid. + +## Changes made + +### `nostr_core/nostr_signer.h` +- Documented the new `nostr_signer_nsigner_set_role_path` setter. +- Repurposed `nostr_signer_nsigner_set_nostr_index` as a compatibility shim + that expands N into `role="main"` + `role_path="m/44'/1237'/N'/0/0"`. + +### `nostr_core/nostr_signer.c` +- Replaced the `remote` struct fields `nostr_index`/`has_nostr_index` with + `role_path[128]`/`has_role_path` plus `derive_index`/`has_derive_index` + (the latter feeds the algorithm-based `derive` verb). +- Rewrote `signer_remote_params_with_selector` to emit both `role` and + `role_path` in the options object. +- Updated all three call sites (`get_public_key`, `sign_event`, + `encrypt_decrypt`) to the new signature. +- Rewrote `signer_remote_derive_hmac` to emit `{"algorithm":"secp256k1", + "index":N}` using `derive_index` (set by the `set_nostr_index` shim). +- Added `nostr_signer_nsigner_set_role_path`. +- Rewrote `nostr_signer_nsigner_set_nostr_index` to expand the NIP-06 + template instead of storing a bare index. +- Added `` for `snprintf`. +- `nostr_signer_free` now also zeroes `role_path`. + +### `tests/nsigner_client_test.c` +- The mock-server fds test now sends a `{"role":"main", + "role_path":"m/44'/1237'/0'/0/0"}` selector and the child asserts both + fields are present in the framed request. + +### Docs +- `NSIGNER_INTEGRATION.md` §2.1 and new §2.1.1 document the selector model. +- `plans/n_signer_verb_migration.md` and `plans/nsigner_integration_plan.md` + updated with notes pointing here. + +## Verification + +- `make` builds clean. +- `nsigner_client_test` passes (mock transport round-trip with the new + selector). +- `grep -rn 'nostr_index' nostr_core/nostr_signer.c` returns no bare-selector + emission (only the compatibility-shim function name and its doc comment). + +## Downstream consumers still needing updates + +These repos link `nostr_core_lib` and call `nostr_signer_nsigner_*`; the +`set_nostr_index` shim keeps them functional, but their UIs still expose only +an "index" input and should be updated to collect role + path explicitly: + +- `sovereign_browser` — `src/login_dialog.c`, `src/agent_login.c`, + `src/key_store.c` (uses `set_nostr_index`; works via shim, UI needs role+path). +- `nostr_terminal` — has its own hand-rolled `nsigner_client.c` that still + emits `{"nostr_index":N}` directly (NOT via this lib); must be rewritten + independently. +- `laantungir_website` — raw JS JSON-RPC, emits `{"nostr_index":N}`; must be + rewritten independently. diff --git a/plans/n_signer_verb_migration.md b/plans/n_signer_verb_migration.md index 66163f3e..9b5873f1 100644 --- a/plans/n_signer_verb_migration.md +++ b/plans/n_signer_verb_migration.md @@ -20,7 +20,13 @@ The Nostr protocol verbs gained a `nostr_` prefix. The algorithm-based verbs (`s | `nip44_decrypt` | `nostr_nip44_decrypt` | | `get_public_key` (role-based) | `nostr_get_public_key` | -**Important distinction:** `get_public_key` is now **algorithm-based** (takes `algorithm`+`index`). The role-based Nostr pubkey verb is `nostr_get_public_key` (takes `nostr_index`/`role`). Since `nostr_core_lib`'s remote signer always uses role-based selectors (`nostr_index`/`role`), the correct replacement is `nostr_get_public_key`. +**Important distinction:** `get_public_key` is now **algorithm-based** (takes `algorithm`+`index`). The role-based Nostr pubkey verb is `nostr_get_public_key` (takes `role`+`role_path`). Since `nostr_core_lib`'s remote signer always uses role-based selectors, the correct replacement is `nostr_get_public_key`. + +> **Selector update (post-migration):** n_signer subsequently removed the +> `nostr_index` and `index` selectors for `nostr_*` verbs entirely. The only +> accepted selector is now `{"role":"","role_path":""}` sent +> together. `nostr_core_lib` was updated to emit both fields; see +> `NSIGNER_INTEGRATION.md` §2.1.1 and `plans/n_signer_selector_rewrite.md`. ## What does NOT change @@ -43,7 +49,7 @@ This is the only file that calls n_signer verbs via `nsigner_client_call()`. All | 498 | `"nip44_encrypt"` | `"nostr_nip44_encrypt"` | | 505 | `"nip44_decrypt"` | `"nostr_nip44_decrypt"` | -These are simple string-literal replacements. The `signer_remote_params_with_selector()` function (line 289) builds `{"nostr_index": N}` or `{"role": "..."}` selectors — these are still valid for the `nostr_*` verbs, so no selector changes needed. +These are simple string-literal replacements. The `signer_remote_params_with_selector()` function (line 289) builds `{"nostr_index": N}` or `{"role": "..."}` selectors — **these were subsequently invalidated** by n_signer's selector rewrite (the `nostr_index` selector is removed and `role` alone is rejected). That rewrite was handled in a follow-up; see `plans/n_signer_selector_rewrite.md`. **Response format note:** `nostr_get_public_key` returns a plain 64-hex-char string by default (same as the old role-based `get_public_key`). The existing parsing at line 348 (`strlen(result->valuestring) != 64`) continues to work. If the caller ever passes `{"format":"structured"}`, the response would be a JSON object string — but the current code doesn't request structured format, so no change needed. diff --git a/plans/nsigner_integration_plan.md b/plans/nsigner_integration_plan.md index df4edae8..1acbcb79 100644 --- a/plans/nsigner_integration_plan.md +++ b/plans/nsigner_integration_plan.md @@ -9,9 +9,16 @@ > `nostr_get_public_key`, `nostr_sign_event`, `nostr_nip04_encrypt`, > `nostr_nip04_decrypt`, `nostr_nip44_encrypt`, and `nostr_nip44_decrypt`. > The unprefixed `get_public_key` method is algorithm-based and is not used by the -> role/`nostr_index` selectors in this library. Public `nostr_signer_*` C API names +> role/`role_path` selectors in this library. Public `nostr_signer_*` C API names > and standard NIP-46 method names are unchanged. > +> **Selector update:** n_signer removed the bare `nostr_index` selector and +> `index`-on-nostr-verbs. The only accepted selector for `nostr_*` verbs is now +> `{"role":"","role_path":""}` sent together. This library +> emits both fields; `nostr_signer_nsigner_set_nostr_index` is retained as a +> compatibility shim that expands N into `role="main"` + +> `role_path="m/44'/1237'/N'/0/0"`. See `NSIGNER_INTEGRATION.md` §2.1.1. +> > This plan originally described pulling the **caller-side** signer-integration glue out of per-project implementations and into `nostr_core_lib`, > so any project that already links the library can sign **locally**, via a **running > n_signer process**, or via a **USB hardware signer** — with the same code. diff --git a/tests/nsigner_client_test.c b/tests/nsigner_client_test.c index 7bef0fc1..483662c8 100644 --- a/tests/nsigner_client_test.c +++ b/tests/nsigner_client_test.c @@ -255,6 +255,12 @@ static int test_fds_transport_round_trip_via_client(void) { free(req); _exit(6); } + /* The new n_signer wire contract requires both role and role_path. */ + if (strstr(req, "\"role\":\"main\"") == NULL || + strstr(req, "\"role_path\":\"m/44'/1237'/0'/0/0\"") == NULL) { + free(req); + _exit(9); + } free(req); out_hdr[0] = (unsigned char)((res_len >> 24) & 0xFFU); @@ -293,6 +299,15 @@ static int test_fds_transport_round_trip_via_client(void) { if (params == NULL) { goto cleanup; } + { + cJSON* selector = cJSON_CreateObject(); + if (selector == NULL) { + goto cleanup; + } + cJSON_AddStringToObject(selector, "role", "main"); + cJSON_AddStringToObject(selector, "role_path", "m/44'/1237'/0'/0/0"); + cJSON_AddItemToArray(params, selector); + } if (nsigner_client_call(client, "nostr_get_public_key", params, &result) != NOSTR_SUCCESS) { params = NULL;