mirror of
https://github.com/Such-Software/smirk-backend-core.git
synced 2026-10-17 07:21:32 +02:00
fix(auth): canonicalize verified Nostr pubkey + harden the link/login grade
Adversarial review (Identity Phase 1 flow) findings: - Canonicalize the pubkey returned by verify_nip98 / verify_signed_action to lowercase hex. BIP-340 hex-decode is case-insensitive, so an UPPER-case-hex pubkey is a fully self-consistent, verifying event — but stored/looked-up verbatim it would let ONE key bind to two accounts via case variance (bypassing the UNIQUE index + 409) and yield non-canonical NIP-05 output. Now every downstream store/compare/lookup is on the canonical form. +KAT. - nostr_link records a login event on success: a session-authed link (including a rotation that REPLACES an existing npub) is a security-relevant binding change and must not be silent. - Tighten NIP98_LOGIN_MAX_AGE_SECS 60 -> 30s: a captured login token is a bearer credential within the window; match it to the state-change grade.
This commit is contained in:
+11
-3
@@ -61,9 +61,10 @@ use crate::AppState;
|
||||
|
||||
// ── shared constants ────────────────────────────────────────────────────────
|
||||
|
||||
/// Replay window for a NIP-98 LOGIN token (seconds). Wide enough for client
|
||||
/// clock skew, tight enough to bound replay.
|
||||
const NIP98_LOGIN_MAX_AGE_SECS: i64 = 60;
|
||||
/// Replay window for a NIP-98 LOGIN token (seconds). A captured login token is a
|
||||
/// bearer credential within this window, so keep it tight — matched to the
|
||||
/// state-change grade below. Still ample for real client clock skew.
|
||||
const NIP98_LOGIN_MAX_AGE_SECS: i64 = 30;
|
||||
|
||||
/// Replay window for a NIP-98 STATE-CHANGE (signed action) token (seconds).
|
||||
/// Deliberately tighter than login.
|
||||
@@ -1393,6 +1394,13 @@ pub async fn nostr_link(
|
||||
|
||||
// 4. Persist. UNIQUE collision -> 409 CONFLICT (handled in set_nostr_pubkey).
|
||||
state.db.set_nostr_pubkey(user_id, &pubkey).await?;
|
||||
// Observability: linking (incl. a rotation that REPLACES an existing npub) is a
|
||||
// security-relevant binding change, so record it in the login history — a
|
||||
// session-authed rebind should never be silent.
|
||||
let _ = state
|
||||
.db
|
||||
.record_login_event(Some(user_id), "btc", Platform::Nostr.as_str(), None, None)
|
||||
.await;
|
||||
info!(user_id = %user_id, "linked Nostr identity");
|
||||
Ok(Json(NostrLinkResponse {
|
||||
nostr_pubkey: pubkey,
|
||||
|
||||
@@ -150,7 +150,11 @@ pub fn verify_nip98(
|
||||
if !exactly_one(&ev, "method")?.eq_ignore_ascii_case(expected_method) {
|
||||
return Err(Nip98Error::MethodMismatch);
|
||||
}
|
||||
Ok(ev.pubkey)
|
||||
// Canonicalize: BIP-340 hex-decode is case-insensitive, so an UPPER-case-hex
|
||||
// pubkey verifies just fine. Return lowercase so storage / the UNIQUE index /
|
||||
// login lookup / NIP-05 hosting are all canonical — otherwise the same key
|
||||
// could bind to two accounts via case variance (defeating the 409).
|
||||
Ok(ev.pubkey.to_lowercase())
|
||||
}
|
||||
|
||||
/// Canonical request descriptor bound into a signed action. Covers method,
|
||||
@@ -222,7 +226,9 @@ pub fn verify_signed_action(
|
||||
}
|
||||
}
|
||||
|
||||
Ok(ev.pubkey)
|
||||
// Canonicalize the verified key to lowercase hex (see verify_nip98) so every
|
||||
// downstream store/compare is on the canonical form.
|
||||
Ok(ev.pubkey.to_lowercase())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -347,6 +353,54 @@ mod tests {
|
||||
]
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn canonicalizes_uppercase_pubkey() {
|
||||
// An UPPER-case-hex pubkey is self-consistent (the id commits to it, and
|
||||
// BIP-340 hex-decode is case-insensitive), so it verifies — but the
|
||||
// returned key MUST be canonical lowercase so it can't bind twice.
|
||||
use k256::schnorr::SigningKey;
|
||||
let sk = SigningKey::from_bytes(&[7u8; 32]).expect("valid scalar");
|
||||
let pk_upper = hex::encode(sk.verifying_key().to_bytes()).to_uppercase();
|
||||
let p = payload_hash();
|
||||
let tags = action_tags("admin_action", "nonce-up", &p);
|
||||
let created_at = 1000;
|
||||
let serial = serde_json::to_string(&serde_json::json!([
|
||||
0, pk_upper, created_at, NIP98_KIND, tags, ""
|
||||
]))
|
||||
.unwrap();
|
||||
let id = hex::encode(Sha256::digest(serial.as_bytes()));
|
||||
let sig = sk
|
||||
.sign_raw(&hex::decode(&id).unwrap(), &[0u8; 32])
|
||||
.expect("sign");
|
||||
let ev = serde_json::json!({
|
||||
"id": id, "pubkey": pk_upper, "created_at": created_at,
|
||||
"kind": NIP98_KIND, "tags": tags, "content": "", "sig": hex::encode(sig.to_bytes())
|
||||
});
|
||||
let header = format!(
|
||||
"Nostr {}",
|
||||
STANDARD.encode(serde_json::to_vec(&ev).unwrap())
|
||||
);
|
||||
let got = verify_signed_action(
|
||||
&header,
|
||||
AURL,
|
||||
"POST",
|
||||
"admin_action",
|
||||
"nonce-up",
|
||||
&p,
|
||||
None,
|
||||
None,
|
||||
created_at,
|
||||
30,
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
got,
|
||||
pk_upper.to_lowercase(),
|
||||
"returned key is canonical lowercase"
|
||||
);
|
||||
assert_ne!(got, pk_upper, "not the verbatim uppercase input");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn signed_action_verifies() {
|
||||
let p = payload_hash();
|
||||
|
||||
Reference in New Issue
Block a user