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; }