From b2a03fcbfeb8b0de55d130e05d605ac6e69e0e02 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 3 Mar 2026 11:12:52 +0000 Subject: [PATCH] fix: use constant-time comparison for static public keys Replaced `std.mem.eql` with `std.crypto.timing_safe.eql` in the Noise handshake (`consumeInitiation` and `consumeInitiationFast`) when comparing the decrypted initiator static public key with the expected remote peer static public key. This mitigates a timing side-channel that could potentially leak information about expected public keys within the cryptographic state machine. 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..35ad3c2 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. + +## 2026-10-25 - [CRITICAL] Timing Attack on Static Public Key Comparison +**Vulnerability:** In `src/wireguard/noise.zig`, the `consumeInitiation` and `consumeInitiationFast` functions used `std.mem.eql` to verify the decrypted initiator's static public key against the expected remote peer's static public key. `std.mem.eql` terminates early on the first mismatched byte, exposing a timing oracle. An attacker could potentially use this side-channel to forge identities or glean information about the expected public keys within the Noise protocol state machine. +**Learning:** Even though public keys are ostensibly public information, within the context of a cryptographic protocol state machine where they form the basis for establishing trust, identity matching must be treated as a cryptographic check requiring constant-time execution to prevent information leakage. +**Prevention:** Always use `std.crypto.timing_safe.eql` (specifically with the `[32]u8` type signature in Zig 0.15) when comparing public keys and other cryptographic identities for authorization or verification inside the Noise handshake. diff --git a/src/wireguard/noise.zig b/src/wireguard/noise.zig index d4c2c0b..45ff661 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: Must use constant-time comparison to prevent timing attacks on public key identity validation. + 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: Must use constant-time comparison to prevent timing attacks on public key identity validation. + if (!std.crypto.timing_safe.eql([32]u8, preamble.initiator_static, self.remote_static)) { return error.UnknownPeer; }