avi/Keynctr
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(nip46): restored sessions no longer demand a connect secret re-echo

Browse source 
Live Amber sign-in (Sep 25) paired fine, but every restart died with
'the signer did not echo the connection secret': the restore re-sends
connect with the ORIGINAL pairing secret, and an already-approved
signer legitimately answers 'true' without re-echoing — NIP-46 reserves
the echo for proving possession during the INITIAL pairing, and Amber
proved it once. Requiring it on re-dial made session restore fail
100% against real Amber (the e2e missed it because its fake answered a
plain ack with no secret in play).

ConnectUri gains a 'restore' flag (set only by reactivate_saved_sessions).
Fresh handshakes still fail closed on a wrong echo. The restore path's
anti-spoofing is expected_identity in adopt_identity — a peer answering
as any other account is refused, and only the real key holder can
decrypt traffic on the stored conversation key. Log now says
'secret validation: SKIPPED (restored session)' and proceeds.

e2e: run_fake_amber now mirrors Amber's ack shapes (plain ack on first
pairing; 'true' when the client re-presents a secret) and the restore
test seeds a pairing secret so it exercises exactly the live failure —
it fails without the fix and passes with it.

cargo test 216 unit + 5 e2e green; clippy --all-targets 0 warnings;
fmt clean; release rebuilt.
This commit is contained in:
Avi 2026-09-25 16:19:19 -05:00
commit 01ce5de417
2 changed files with 63 additions and 5 deletions
Download patch file Download diff file Expand all files Collapse all files

43
src/signer/nip46_client.rs
View file

@ -112,6 +112,15 @@ struct ConnectUri {
relays: Vec<RelayUrl>,
secret: Option<String>,
permissions: Option<Nip46Permissions>,
/// True when this session was rebuilt from a saved vault row
/// (`reactivate_saved_sessions`) rather than dialed from a fresh URI.
/// A restored session re-sends `connect` with the ORIGINAL pairing
/// secret, but an already-approved signer answers `true` without
/// re-echoing it (NIP-46 reserves the echo for proving possession
/// during the initial pairing) — so the echo check is skipped and the
/// anti-spoofing is `expected_identity` in `adopt_identity` instead:
/// answering as any other account fails closed.
restore: bool,
}
/// The NIP-46 client signer.
@ -274,6 +283,7 @@ impl Nip46ClientSigner {
relays,
secret,
permissions,
restore: false,
})
}
@ -630,6 +640,11 @@ impl Nip46ClientSigner {
relays,
secret: connect_secret,
permissions: None,
// Restored vault session: already-approved signers answer the
// re-`connect` with `true` instead of re-echoing the pairing
// secret; the handshake skips the echo and binds identity via
// `expected_identity` instead.
restore: true,
};
let signer = self.clone();
let task = tokio::spawn(async move {
@ -1082,6 +1097,7 @@ impl Nip46ClientSigner {
relays,
secret: Some(secret),
permissions: connection.permissions.clone(),
restore: false,
};
let paired = {
let mut inner = self.inner.lock().await;
@ -1565,8 +1581,13 @@ impl Nip46ClientSigner {
let handshake = self.clone();
let peer = uri.peer;
let expected_secret = uri.secret.clone();
let restore = uri.restore;
tokio::spawn(async move {
if let Err(e) = handshake.clone().run_handshake(peer, expected_secret).await {
if let Err(e) = handshake
.clone()
.run_handshake(peer, expected_secret, restore)
.await
{
handshake.fail(e);
}
});
@ -1998,6 +2019,7 @@ impl Nip46ClientSigner {
self,
peer: PublicKey,
expected_secret: Option<String>,
restore: bool,
) -> Result<(), String> {
// Resolve the connect secret ON-DEMAND from the vault — the single
// source of truth. Fail-closed: if the vault cannot produce the
@ -2053,16 +2075,31 @@ impl Nip46ClientSigner {
// Anti-spoofing: per NIP-46, a signer answering a nostrconnect://
// (secret-carrying) handshake must echo the secret back. A mismatch
// means something other than the intended signer answered.
//
// RESTORED sessions are exempt from the echo: the secret they resend
// is the ORIGINAL pairing secret, and a signer that already approved
// this client answers `true` instead of re-echoing (an echo would
// re-prove possession that the initial pairing proved once). Real
// Amber does exactly this, so requiring the echo made every restart
// fail with "did not echo the connection secret". The restore path's
// anti-spoofing is `expected_identity`, enforced in adopt_identity:
// anything that answers as another account is refused, and only the
// key holder can decrypt our traffic on the stored conversation key.
if let Some(expected) = &expected_secret {
let trimmed = ack.trim();
if trimmed != expected.as_str() {
if trimmed == expected.as_str() {
eprintln!("[NIP46] secret validation: PASS (connect ack echo matched)");
} else if restore {
eprintln!(
"[NIP46] secret validation: SKIPPED (restored session; ack={trimmed}) — identity will be re-proved"
);
} else {
eprintln!("[NIP46] secret validation: FAIL (connect ack echo did not match)");
return Err(
"The signer did not echo the connection secret; refusing the connection."
.to_string(),
);
}
eprintln!("[NIP46] secret validation: PASS (connect ack echo matched)");
}
self.adopt_identity(peer).await

23
tests/nip46_e2e.rs
View file

@ -284,10 +284,19 @@ async fn run_fake_amber(relay_url: String, comms: Keys, identity: Keys, approval
let response: Value = match method {
"connect" => {
// Simulate a human tapping "approve" in Amber.
// Simulate a human tapping "approve" in Amber. Amber's
// ack SHAPE depends on the connection state: a first-time
// pairing (no secret in params) gets a plain ack; an
// ALREADY-APPROVED connection re-dialing with the stored
// secret gets `true` WITHOUT a secret echo (live Amber,
// Sep 25 — the client must not demand an echo there).
tokio::time::sleep(approval_delay).await;
if req["params"].as_array().is_some_and(|p| p.len() > 1) {
json!({"id": id, "result": true})
} else {
json!({"id": id, "result": "ack"})
}
}
"get_public_key" => json!({"id": id, "result": identity.public_key().to_hex()}),
"sign_event" => {
let unsigned_json = req["params"].get(0).and_then(|v| v.as_str());
@ -1189,6 +1198,18 @@ async fn nip46_session_restore_redials_and_refuses_wrong_identity() {
// (The old instance's listener task is left running on purpose — the
// live process exiting is modeled by the new instance, not by
// `disconnect()`, which revokes and wipes the stored key.)
//
// Seed a pairing secret at the identity ref first: a QR pairing stores
// one, and the restore re-sends it — real Amber then answers `true`
// WITHOUT echoing (already-approved connection), which the fake models.
// Without the restore-mode skip this handshake fails "did not echo the
// connection secret" (the exact live Sep 25 failure).
{
let mut g = app.lock().await;
keynectr::vault::store_connection_secret(&mut g.vault, None, &ref_id, "restore-secret-123")
.unwrap();
g.save_vault().unwrap();
}
let signer2 = Nip46ClientSigner::new(app.clone());
let restored = signer2
.reactivate_saved_sessions()