Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -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-05-25 - [CRITICAL] Timing Attack on Initiator Static Key Verification
**Vulnerability:** The functions `consumeInitiation` and `consumeInitiationFast` in `src/wireguard/noise.zig` used `std.mem.eql` to compare the decrypted `initiator_static` key against `self.remote_static`. This exposed a timing oracle during the Handshake Initiation phase, potentially allowing an attacker to deduce the expected static key via a timing side-channel.
**Learning:** Checking the equality of sensitive values, including cryptographic public keys, should always be done in constant time to prevent leaking information about the expected state. Standard library memory equality checks terminate early.
**Prevention:** Always use `std.crypto.timing_safe.eql` to compare cryptographic key materials such as decrypted identity keys, shared secrets, signatures, and MACs.
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor inconsistency: The earlier entry on line 6 refers to the API as std.crypto.utils.timingSafeEql, while this new entry on line 23 refers to it as std.crypto.timing_safe.eql. The correct API (as used in the actual code at line 634 of noise.zig) is std.crypto.timing_safe.eql. The earlier entry (line 6) appears to be the one that's wrong, but since this new entry will be read alongside it, it may be worth noting the discrepancy or correcting line 6 in this PR for consistency.

Copilot uses AI. Check for mistakes.
8 changes: 6 additions & 2 deletions src/wireguard/noise.zig
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,9 @@ 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 side-channels that could leak information about the expected public key.
if (!std.crypto.timing_safe.eql([32]u8, initiator_static, self.remote_static)) {
return error.UnknownPeer;
}

Expand Down Expand Up @@ -282,7 +284,9 @@ 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 side-channels that could leak information about the expected public key.
if (!std.crypto.timing_safe.eql([32]u8, preamble.initiator_static, self.remote_static)) {
return error.UnknownPeer;
}

Expand Down