From 1181b5938a73acea80843fd34208fe8b0d22816c Mon Sep 17 00:00:00 2001
From: jwinterm <johnwmurphy@gmail.com>
Date: Thu, 2 Jul 2026 05:43:20 -0400
Subject: [PATCH] fix(auth): canonicalize verified Nostr pubkey + harden the
 link/login grade
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 src/api/auth.rs          | 14 +++++++---
 src/core/crypto/nip98.rs | 58 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/src/api/auth.rs b/src/api/auth.rs
index 2c4495b..50044a1 100644
--- a/src/api/auth.rs
+++ b/src/api/auth.rs
@@ -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,
diff --git a/src/core/crypto/nip98.rs b/src/core/crypto/nip98.rs
index a81cd56..4ce7d61 100644
--- a/src/core/crypto/nip98.rs
+++ b/src/core/crypto/nip98.rs
@@ -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();