fix(pairing): accept the NIP-46 connect *response* shape Amber actually sends
For a client-initiated nostrconnect:// scan, NIP-46 says the signer
sends a connect RESPONSE event — {"id",…,"result":"<secret>"} — not a
connect request: the secret echo IS the handshake, nothing to answer.
run_pairing_task only recognized an inbound {"method":"connect"}
*request*; a bare {"result":…} fell through as 'not a NIP-46 request'
(method empty) or 'pre-handshake ignored', so Amber's approval was
silently dropped and no profile row was ever created.
Now the pairing loop verifies the echoed secret (or 'ack') directly and
proceeds to identity adoption; a signer-sent error fails fast with the
signer's message. Wrong-secret responses are ignored as spoofing, same
as before. The legacy request shape keeps working.
e2e: run_fake_scanner takes a connect_shape; new
nip46_qr_pairing_connect_response_shape pins the Amber shape end-to-end
(fails on the old parser, green on the new one).
This commit is contained in:
parent
a28d76e7d0
commit
edd4e565fb
2 changed files with 74 additions and 9 deletions
|
|
@ -683,6 +683,44 @@ impl Nip46ClientSigner {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
let shaped: serde_json::Value =
|
||||||
|
serde_json::from_str(&plain).unwrap_or(serde_json::Value::Null);
|
||||||
|
// Per NIP-46, for a client-initiated `nostrconnect://` the signer
|
||||||
|
// sends a connect *response* — `{"id":…,"result":"<secret>"}` —
|
||||||
|
// not a `connect` request: there is nothing to answer, the secret
|
||||||
|
// echo IS the handshake. (Spec: "the _remote-signer_ … then sends
|
||||||
|
// `connect` *response* event to the `client-pubkey`"; result is
|
||||||
|
// `"ack"` OR the secret.) The legacy request shape is still
|
||||||
|
// handled below for signers that send `{"method":"connect"}`.
|
||||||
|
if shaped.get("method").is_none()
|
||||||
|
&& (shaped.get("result").is_some() || shaped.get("error").is_some())
|
||||||
|
{
|
||||||
|
if shaped.get("error").is_some() && !shaped["error"].is_null() {
|
||||||
|
let msg = shaped["error"]
|
||||||
|
.as_str()
|
||||||
|
.map(|s| s.to_string())
|
||||||
|
.unwrap_or_else(|| shaped["error"].to_string());
|
||||||
|
return Err(format!("The signer rejected the connection: {msg}"));
|
||||||
|
}
|
||||||
|
let result = shaped["result"].as_str().unwrap_or("");
|
||||||
|
if result != secret && result != "ack" {
|
||||||
|
eprintln!(
|
||||||
|
"[nip46 pairing] connect response echoed the wrong secret — ignoring"
|
||||||
|
);
|
||||||
|
if live_capture {
|
||||||
|
pairing_trace("connect response echoed the wrong secret — ignoring");
|
||||||
|
}
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
eprintln!("[nip46 pairing] connect response accepted (secret echo verified)");
|
||||||
|
if live_capture {
|
||||||
|
pairing_trace("connect response accepted (secret echo verified)");
|
||||||
|
}
|
||||||
|
// Tear down the pairing subscription BEFORE handing over:
|
||||||
|
// from here on the demux loop owns the conversation.
|
||||||
|
client.unsubscribe(subscription.id()).await.ok();
|
||||||
|
break Ok::<_, String>((event.pubkey, conv, None));
|
||||||
|
}
|
||||||
let Ok(request) = serde_json::from_str::<RawRequest>(&plain) else {
|
let Ok(request) = serde_json::from_str::<RawRequest>(&plain) else {
|
||||||
eprintln!("[nip46 pairing] decrypted payload is not a NIP-46 request: {plain}");
|
eprintln!("[nip46 pairing] decrypted payload is not a NIP-46 request: {plain}");
|
||||||
if live_capture {
|
if live_capture {
|
||||||
|
|
@ -700,8 +738,8 @@ impl Nip46ClientSigner {
|
||||||
}
|
}
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
// NIP-46: the client answers the signer's `connect` with the
|
// Legacy path: a signer that sends an actual `connect` *request*
|
||||||
// secret as the result; the signer verifies the echo.
|
// gets the secret as the answer, and it verifies the echo.
|
||||||
let response = response_ok(&request.id, secret.clone());
|
let response = response_ok(&request.id, secret.clone());
|
||||||
// Tear down the pairing subscription BEFORE answering: from here
|
// Tear down the pairing subscription BEFORE answering: from here
|
||||||
// on the demux loop owns the conversation. If both subscriptions
|
// on the demux loop owns the conversation. If both subscriptions
|
||||||
|
|
|
||||||
|
|
@ -476,6 +476,7 @@ async fn run_fake_scanner(
|
||||||
client_pk: PublicKey,
|
client_pk: PublicKey,
|
||||||
expected_secret: String,
|
expected_secret: String,
|
||||||
identity: Keys,
|
identity: Keys,
|
||||||
|
connect_shape: &'static str,
|
||||||
) {
|
) {
|
||||||
let (mut ws, _) = tokio_tungstenite::connect_async(&relay_url)
|
let (mut ws, _) = tokio_tungstenite::connect_async(&relay_url)
|
||||||
.await
|
.await
|
||||||
|
|
@ -491,12 +492,21 @@ async fn run_fake_scanner(
|
||||||
let conversation = ConversationKey::derive(identity.secret_key(), &client_pk).unwrap();
|
let conversation = ConversationKey::derive(identity.secret_key(), &client_pk).unwrap();
|
||||||
|
|
||||||
// The scanned URI tells us who to contact and what secret to echo.
|
// The scanned URI tells us who to contact and what secret to echo.
|
||||||
let connect_req = json!({
|
// Two shapes:
|
||||||
"id": "pair-1",
|
// - "request": {"id","method":"connect","params":[secret]} — what the
|
||||||
"method": "connect",
|
// e2e originally simulated; the client answers with the secret.
|
||||||
"params": [expected_secret.clone()],
|
// - "response": {"id","result":secret} — what NIP-46 actually specifies
|
||||||
});
|
// for nostrconnect:// ("the _remote-signer_ … sends `connect`
|
||||||
let content = nip44_enc(&conversation, &connect_req.to_string());
|
// *response* event"), and what Amber sends. No answer expected.
|
||||||
|
let connect_msg = match connect_shape {
|
||||||
|
"response" => json!({ "id": "pair-1", "result": expected_secret.clone() }),
|
||||||
|
_ => json!({
|
||||||
|
"id": "pair-1",
|
||||||
|
"method": "connect",
|
||||||
|
"params": [expected_secret.clone()],
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
let content = nip44_enc(&conversation, &connect_msg.to_string());
|
||||||
let out = EventBuilder::new(Kind::NostrConnect, content)
|
let out = EventBuilder::new(Kind::NostrConnect, content)
|
||||||
.tags([Tag::parse(["p", client_pk.to_hex().as_str()]).unwrap()])
|
.tags([Tag::parse(["p", client_pk.to_hex().as_str()]).unwrap()])
|
||||||
.finalize(&identity)
|
.finalize(&identity)
|
||||||
|
|
@ -602,12 +612,28 @@ async fn run_fake_scanner(
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
|
||||||
async fn nip46_qr_pairing_handshake_and_sign() {
|
async fn nip46_qr_pairing_handshake_and_sign() {
|
||||||
|
run_qr_pairing("request").await;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The shape NIP-46 actually specifies for a `nostrconnect://` scan — and
|
||||||
|
/// what Amber sends — is a connect *response* (`{"id","result":"<secret>"}`),
|
||||||
|
/// not a `connect` request. Pairing must complete on that shape too.
|
||||||
|
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
|
||||||
|
async fn nip46_qr_pairing_connect_response_shape() {
|
||||||
|
run_qr_pairing("response").await;
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn run_qr_pairing(connect_shape: &'static str) {
|
||||||
let relay_url = start_relay().await;
|
let relay_url = start_relay().await;
|
||||||
|
|
||||||
// Isolated vault + pairing relay, serialized with the other e2e test.
|
// Isolated vault + pairing relay, serialized with the other e2e test.
|
||||||
let app = {
|
let app = {
|
||||||
let _guard = VAULT_ENV_LOCK.lock().unwrap();
|
let _guard = VAULT_ENV_LOCK.lock().unwrap();
|
||||||
let tmp = std::env::temp_dir().join(format!("keynectr-e2e-pair-{}", std::process::id()));
|
let tmp = std::env::temp_dir().join(format!(
|
||||||
|
"keynectr-e2e-pair-{}-{}",
|
||||||
|
std::process::id(),
|
||||||
|
connect_shape
|
||||||
|
));
|
||||||
std::fs::create_dir_all(&tmp).unwrap();
|
std::fs::create_dir_all(&tmp).unwrap();
|
||||||
std::fs::write(
|
std::fs::write(
|
||||||
tmp.join("profiles_vault.json"),
|
tmp.join("profiles_vault.json"),
|
||||||
|
|
@ -662,6 +688,7 @@ async fn nip46_qr_pairing_handshake_and_sign() {
|
||||||
client_pk,
|
client_pk,
|
||||||
secret.clone(),
|
secret.clone(),
|
||||||
identity.clone(),
|
identity.clone(),
|
||||||
|
connect_shape,
|
||||||
));
|
));
|
||||||
|
|
||||||
// Wait for scan -> secret echo -> identity adoption.
|
// Wait for scan -> secret echo -> identity adoption.
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue