From b1fb53e87082ff99d2bea52d60a3fe8bdddd576e Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
 <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Mon, 9 Mar 2026 11:12:38 +0000
Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?=
 =?UTF-8?q?=20Fix=20timing=20attack=20in=20public=20key=20verification?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

**Vulnerability:**
The `consumeInitiation` and `consumeInitiationFast` functions in `src/wireguard/noise.zig` used `std.mem.eql` to verify the decrypted `initiator_static` public key against the expected `remote_static` key. `std.mem.eql` is not a constant-time comparison and terminates early on a mismatch. This created a timing oracle that an attacker could exploit to determine the expected public key byte-by-byte.

**Impact:**
While leaking a public key is generally not as severe as leaking a private key, an attacker could potentially use this to verify the identities (public keys) expected by a node without knowing them in advance, leading to topology discovery or aiding in spoofing attempts during the Noise handshake.

**Fix:**
Replaced `std.mem.eql` with `std.crypto.timing_safe.eql` for comparing the `initiator_static` and `remote_static` public keys. This ensures the comparison time is constant regardless of where the mismatch occurs, completely eliminating the timing oracle.

**Verification:**
`zig build test` passes with no regressions.

Co-authored-by: igorls <4753812+igorls@users.noreply.github.com>
---
 .jules/sentinel.md      | 5 +++++
 src/wireguard/noise.zig | 6 ++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/.jules/sentinel.md b/.jules/sentinel.md
index 7a55c59..a76b6b7 100644
--- a/.jules/sentinel.md
+++ b/.jules/sentinel.md
@@ -16,3 +16,8 @@
 **Vulnerability:** Intermediate key material (`secret` extracted from chaining key, and expanded `t*_plus` buffers) in `src/wireguard/crypto.zig`'s HKDF implementation (`kdf1`, `kdf2`, `kdf3`) was left on the stack without being zeroed. These buffers contain sensitive pseudorandom keys derived from the handshake chaining key.
 **Learning:** Helper functions often create temporary buffers on the stack which persist until overwritten. Unlike the main handshake state struct, these are ephemeral but critical. Zig does not automatically zero stack variables on return.
 **Prevention:** Always use `defer std.crypto.secureZero(u8, &var);` immediately after declaring any variable that will hold sensitive key material, especially in crypto primitives.
+
+## 2025-05-24 - [CRITICAL] Timing Attack on Public Key Verification
+**Vulnerability:** The functions `consumeInitiation` and `consumeInitiationFast` in `src/wireguard/noise.zig` used `std.mem.eql` to compare the decrypted static public key (`initiator_static`) with the expected remote public key (`remote_static`). This exposes a timing oracle, allowing an attacker to determine the expected public key byte-by-byte. While not as immediately catastrophic as leaking a private key, an attacker could abuse this to spoof initiations or gather information on network topology.
+**Learning:** Public keys used as identifiers and matched during authentication protocols must be compared using constant-time functions. `std.mem.eql` terminates early on mismatch, revealing information via timing differences.
+**Prevention:** Always use `std.crypto.timing_safe.eql` for comparing cryptographic identities and secrets during authentication flows. Update the review checklist to flag non-constant-time comparisons on keys, even public ones, during handshake validations.
diff --git a/src/wireguard/noise.zig b/src/wireguard/noise.zig
index d4c2c0b..de845a6 100644
--- a/src/wireguard/noise.zig
+++ b/src/wireguard/noise.zig
@@ -247,7 +247,8 @@ pub const Handshake = struct {
         crypto.mixHash(&hash, &msg.encrypted_static);
 
         // Verify the decrypted static matches what we expect
-        if (!std.mem.eql(u8, &initiator_static, &self.remote_static)) {
+        // Security: Use constant-time comparison for cryptographic keys to prevent timing attacks.
+        if (!std.crypto.timing_safe.eql([32]u8, initiator_static, self.remote_static)) {
             return error.UnknownPeer;
         }
 
@@ -282,7 +283,8 @@ pub const Handshake = struct {
         var hash = preamble.hash;
 
         // Verify the decrypted static matches what we expect
-        if (!std.mem.eql(u8, &preamble.initiator_static, &self.remote_static)) {
+        // Security: Use constant-time comparison for cryptographic keys to prevent timing attacks.
+        if (!std.crypto.timing_safe.eql([32]u8, preamble.initiator_static, self.remote_static)) {
             return error.UnknownPeer;
         }