fix: use constant-time comparison for HMAC challenge ID verification

.changelog/constant-time-hmac.md

@@ -0,0 +1,5 @@
+---
+mpp: patch
+---
+
+Fixed a timing side-channel in HMAC challenge ID verification by replacing non-constant-time string comparison with `constant_time_eq`. Added an `ast-grep` lint rule to prevent future regressions.

.github/workflows/ci.yml

@@ -38,6 +38,17 @@ jobs:
           path: "."
           post-comment: false
           github-token: ${{ secrets.GITHUB_TOKEN }}
+      - name: Install ast-grep
+        uses: jaxxstorm/action-install-gh-release@v1
+        with:
+          repo: ast-grep/ast-grep
+          tag: "0.37.0"
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      - name: Run ast-grep lint
+        run: sg scan -c sgconfig.yml src/
+      - name: Run ast-grep tests
+        run: sg test -c sgconfig.yml
 
   test:
     name: Test

ast-grep-tests/__snapshots__/no-hmac-equality-compare-snapshot.yml

@@ -0,0 +1,45 @@
+id: no-hmac-equality-compare
+snapshots:
+  ? |
+    if credential.challenge.id != expected_id {
+        return Err("mismatch");
+    }
+  : labels:
+    - source: credential.challenge.id != expected_id
+      style: primary
+      start: 3
+      end: 41
+  ? |
+    if credential.challenge.id == expected_id {
+        return Ok(());
+    }
+  : labels:
+    - source: credential.challenge.id == expected_id
+      style: primary
+      start: 3
+      end: 41
+  ? |
+    if expected_id != credential.challenge.id {
+        return Err("mismatch");
+    }
+  : labels:
+    - source: expected_id != credential.challenge.id
+      style: primary
+      start: 3
+      end: 41
+  ? |
+    if expected_id == credential.challenge.id {
+        return Ok(());
+    }
+  : labels:
+    - source: expected_id == credential.challenge.id
+      style: primary
+      start: 3
+      end: 41
+  ? |
+    let matches = self.id == expected_id;
+  : labels:
+    - source: self.id == expected_id
+      style: primary
+      start: 14
+      end: 36

ast-grep-tests/no-hmac-equality-compare-test.yml

@@ -0,0 +1,32 @@
+id: no-hmac-equality-compare
+valid:
+  - |
+    if !constant_time_eq(&credential.challenge.id, &expected_id) {
+        return Err("mismatch");
+    }
+  - |
+    constant_time_eq(&self.id, &expected_id)
+  - |
+    let expected_id = compute_challenge_id(secret, realm, method, intent, request, None, None, None);
+  - |
+    let is_valid = hmac_verify(&id, &expected_id);
+
+invalid:
+  - |
+    if credential.challenge.id != expected_id {
+        return Err("mismatch");
+    }
+  - |
+    if credential.challenge.id == expected_id {
+        return Ok(());
+    }
+  - |
+    if expected_id != credential.challenge.id {
+        return Err("mismatch");
+    }
+  - |
+    if expected_id == credential.challenge.id {
+        return Ok(());
+    }
+  - |
+    let matches = self.id == expected_id;

rules/no-hmac-equality-compare.yml

@@ -0,0 +1,22 @@
+id: no-hmac-equality-compare
+message: "Use constant_time_eq() for HMAC comparison, not == or !=. Non-constant-time comparison leaks timing information."
+severity: error
+language: rust
+note: |
+  HMAC challenge IDs must be compared using constant_time_eq() to prevent
+  timing side-channel attacks. Using == or != leaks information about how
+  many leading bytes match, which can allow an attacker to forge valid HMACs.
+
+  Replace:
+    if credential.challenge.id != expected_id { ... }
+  With:
+    if !constant_time_eq(&credential.challenge.id, &expected_id) { ... }
+
+  To disable this rule:
+  - Line: add `// ast-grep-ignore: no-hmac-equality-compare` with justification
+rule:
+  any:
+    - pattern: $A == expected_id
+    - pattern: $A != expected_id
+    - pattern: expected_id == $A
+    - pattern: expected_id != $A

sgconfig.yml

@@ -0,0 +1,5 @@
+ruleDirs:
+  - rules
+
+testConfigs:
+  - testDir: ast-grep-tests

src/protocol/core/challenge.rs

@@ -481,7 +481,7 @@ pub fn compute_challenge_id(
 }
 
 /// Constant-time string comparison to prevent timing attacks.
-fn constant_time_eq(a: &str, b: &str) -> bool {
+pub(crate) fn constant_time_eq(a: &str, b: &str) -> bool {
     if a.len() != b.len() {
         return false;
     }

src/protocol/core/mod.rs

@@ -71,6 +71,8 @@ pub mod headers;
 pub mod types;
 
 // Re-export all public types
+#[cfg(feature = "server")]
+pub(crate) use challenge::constant_time_eq;
 pub use challenge::{
     compute_challenge_id, extract_tx_hash, ChallengeEcho, PaymentChallenge, PaymentCredential,
     PaymentPayload, Receipt,

src/server/mpp.rs

@@ -230,7 +230,7 @@ where
             credential.challenge.opaque.as_ref().map(|o| o.raw()),
         );
 
-        if credential.challenge.id != expected_id {
+        if !crate::protocol::core::constant_time_eq(&credential.challenge.id, &expected_id) {
             return Err(VerificationError::with_code(
                 "Challenge ID mismatch - not issued by this server",
                 crate::protocol::traits::ErrorCode::CredentialMismatch,