fix(pairing): never reject a decrypted NIP-46 payload on shape
RawRequest still dropped real Amber connect requests after aedde8f:
tonight's re-scan (20:11) hit the lenient-params build and the payload
parsed no better. The strict derive rejected non-string ids, missing
ids, object-shaped params, and double-encoded request strings. Replace
the derived Deserialize with a coercion-based one: any valid JSON
deserializes, every scalar becomes text, a JSON-string-wrapped object
is unwrapped, and only truly non-JSON reaches the log path — now
dumping the exact payload plus the real decrypt error (HMAC vs padding
vs wrong key) instead of a generic 'wrong conversation key'.
This commit is contained in:
parent
aedde8ffb8
commit
188b2eb61d
1 changed files with 129 additions and 27 deletions
|
|
@ -613,15 +613,21 @@ impl Nip46ClientSigner {
|
||||||
eprintln!("[nip46 pairing] could not derive a conversation with this author");
|
eprintln!("[nip46 pairing] could not derive a conversation with this author");
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
let Ok(plain) = nip44_decrypt(&conv, &event.content) else {
|
let plain = match nip44_decrypt(&conv, &event.content) {
|
||||||
eprintln!("[nip46 pairing] payload did not decrypt (wrong conversation key)");
|
Ok(p) => p,
|
||||||
continue;
|
Err(e) => {
|
||||||
|
// Surface the REAL failure (HMAC vs wrong key vs invalid
|
||||||
|
// padding): a malformed NIP-44 frame looks identical to a
|
||||||
|
// wrong conversation key otherwise.
|
||||||
|
eprintln!(
|
||||||
|
"[nip46 pairing] payload did not decrypt: {e} (frame {} chars b64)",
|
||||||
|
event.content.len()
|
||||||
|
);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
};
|
};
|
||||||
let Ok(request) = serde_json::from_str::<RawRequest>(&plain) else {
|
let Ok(request) = serde_json::from_str::<RawRequest>(&plain) else {
|
||||||
eprintln!(
|
eprintln!("[nip46 pairing] decrypted payload is not a NIP-46 request: {plain}");
|
||||||
"[nip46 pairing] decrypted payload is not a NIP-46 request (len {})",
|
|
||||||
plain.len()
|
|
||||||
);
|
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
if request.method != "connect" {
|
if request.method != "connect" {
|
||||||
|
|
@ -1855,32 +1861,70 @@ impl Signer for Nip46ClientSigner {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Minimal decrypted NIP-46 request.
|
/// Minimal decrypted NIP-46 request. Deserialization is maximally lenient:
|
||||||
#[derive(Debug, Deserialize)]
|
/// real signers deviate from the spec'd shape (`id` numeric, `params` an
|
||||||
|
/// object instead of an array, raw JSON grants embedded in params), and a
|
||||||
|
/// strict parse silently dropped the pairing handshake. Every scalar is
|
||||||
|
/// coerced to text instead of rejecting the request.
|
||||||
|
#[derive(Debug)]
|
||||||
struct RawRequest {
|
struct RawRequest {
|
||||||
id: String,
|
id: String,
|
||||||
method: String,
|
method: String,
|
||||||
#[serde(default, deserialize_with = "lenient_params")]
|
|
||||||
params: Vec<String>,
|
params: Vec<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// NIP-46 params are conventionally strings, but real signers embed raw JSON
|
impl<'de> serde::Deserialize<'de> for RawRequest {
|
||||||
/// objects (Amber sends its requested_perms grant object as `connect`
|
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
|
||||||
/// params[1]). A strict `Vec<String>` would reject the entire request and
|
where
|
||||||
/// silently drop the handshake, so non-string params are coerced to their
|
D: serde::Deserializer<'de>,
|
||||||
/// JSON text instead.
|
{
|
||||||
fn lenient_params<'de, D>(deserializer: D) -> Result<Vec<String>, D::Error>
|
fn to_text(v: &serde_json::Value) -> String {
|
||||||
where
|
match v {
|
||||||
D: serde::Deserializer<'de>,
|
serde_json::Value::String(s) => s.clone(),
|
||||||
{
|
other => other.to_string(),
|
||||||
let values: Vec<serde_json::Value> = Vec::deserialize(deserializer)?;
|
}
|
||||||
Ok(values
|
}
|
||||||
.into_iter()
|
let value = serde_json::Value::deserialize(deserializer)?;
|
||||||
.map(|v| match v {
|
let mut obj = match value {
|
||||||
serde_json::Value::String(s) => s,
|
serde_json::Value::Object(map) => map,
|
||||||
other => other.to_string(),
|
// Double-encoded payload: a JSON string wrapping the request
|
||||||
})
|
// object. Unwrap one level and retry.
|
||||||
.collect())
|
serde_json::Value::String(ref s) => {
|
||||||
|
match serde_json::from_str::<serde_json::Value>(s) {
|
||||||
|
Ok(serde_json::Value::Object(map)) => map,
|
||||||
|
_ => {
|
||||||
|
return Ok(RawRequest {
|
||||||
|
id: String::new(),
|
||||||
|
method: String::new(),
|
||||||
|
params: vec![s.clone()],
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Any other non-object is not a JSON-RPC request; surface it as
|
||||||
|
// an empty method (the caller logs and ignores it) rather than
|
||||||
|
// failing, so the raw text still reaches the diagnostic log.
|
||||||
|
other => {
|
||||||
|
return Ok(RawRequest {
|
||||||
|
id: String::new(),
|
||||||
|
method: String::new(),
|
||||||
|
params: vec![to_text(&other)],
|
||||||
|
});
|
||||||
|
}
|
||||||
|
};
|
||||||
|
let id = obj.remove("id").map(|v| to_text(&v)).unwrap_or_default();
|
||||||
|
let method = obj
|
||||||
|
.remove("method")
|
||||||
|
.map(|v| to_text(&v))
|
||||||
|
.unwrap_or_default();
|
||||||
|
let params = match obj.remove("params") {
|
||||||
|
Some(serde_json::Value::Array(arr)) => arr.iter().map(to_text).collect(),
|
||||||
|
Some(serde_json::Value::Null) | None => Vec::new(),
|
||||||
|
// `params` shaped as an object: treat it as a single param.
|
||||||
|
Some(other) => vec![to_text(&other)],
|
||||||
|
};
|
||||||
|
Ok(RawRequest { id, method, params })
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn response_ok(id: &str, result: String) -> String {
|
fn response_ok(id: &str, result: String) -> String {
|
||||||
|
|
@ -1976,6 +2020,64 @@ mod raw_request_tests {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn coerces_numeric_ids() {
|
||||||
|
// Some signers use numeric request ids; a strict String id used to
|
||||||
|
// reject the whole request and silently drop the handshake.
|
||||||
|
let req: RawRequest =
|
||||||
|
serde_json::from_str(r#"{"id":42,"method":"connect","params":[]}"#).unwrap();
|
||||||
|
assert_eq!(req.id, "42");
|
||||||
|
let req: RawRequest =
|
||||||
|
serde_json::from_str(r#"{"id":1789267093,"method":"connect","params":["aa","bb"]}"#)
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(req.id, "1789267093");
|
||||||
|
assert_eq!(req.method, "connect");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn missing_id_defaults_to_empty() {
|
||||||
|
let req: RawRequest = serde_json::from_str(r#"{"method":"connect"}"#).unwrap();
|
||||||
|
assert_eq!(req.id, "");
|
||||||
|
assert_eq!(req.method, "connect");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn unwraps_double_encoded_request() {
|
||||||
|
// A signer that JSON-stringifies the whole request object.
|
||||||
|
let inner = r#"{"id":"9","method":"connect","params":["aa"]}"#;
|
||||||
|
let raw = serde_json::Value::String(inner.to_string()).to_string();
|
||||||
|
let req: RawRequest = serde_json::from_str(&raw).expect("must parse");
|
||||||
|
assert_eq!(req.id, "9");
|
||||||
|
assert_eq!(req.method, "connect");
|
||||||
|
assert_eq!(req.params, vec!["aa".to_string()]);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn accepts_object_shaped_params() {
|
||||||
|
// `params` given as a bare object rather than an array.
|
||||||
|
let req: RawRequest =
|
||||||
|
serde_json::from_str(r#"{"id":"x","method":"connect","params":{"sign_event":[0]}}"#)
|
||||||
|
.expect("must parse");
|
||||||
|
assert_eq!(req.params.len(), 1);
|
||||||
|
assert!(req.params[0].starts_with('{'));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn never_fails_on_valid_json() {
|
||||||
|
// Whatever a signer sends that is valid JSON must deserialize; the
|
||||||
|
// worst case is an empty method the caller logs and ignores.
|
||||||
|
for raw in [
|
||||||
|
r#"[1,2,3]"#,
|
||||||
|
r#""just a string""#,
|
||||||
|
r#"42"#,
|
||||||
|
r#"null"#,
|
||||||
|
r#"{"no":"fields"}"#,
|
||||||
|
] {
|
||||||
|
let req: RawRequest = serde_json::from_str(raw).expect("must not fail");
|
||||||
|
assert!(raw.contains("fields") || req.method.is_empty() || req.method == "connect");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn parses_string_params_unchanged() {
|
fn parses_string_params_unchanged() {
|
||||||
let req: RawRequest =
|
let req: RawRequest =
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue