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.

## 2023-10-27 - [CRITICAL] Timing Attack on Remote 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`. `std.mem.eql` terminates early on the first mismatch, creating a timing oracle. This could allow an attacker to perform a timing attack to determine the expected remote static key during the handshake processing.
**Learning:** It is easy to accidentally use `std.mem.eql` for comparing public keys or decrypted public keys when validating expected peers, especially when the key itself is public but the expected value is part of the secret handshake state.
**Prevention:** Always use `std.crypto.timing_safe.eql` for any comparison involving cryptographic keys, even public keys, when the comparison occurs within a cryptographic protocol or handshake where timing could leak information about the internal state or expected peer.
Comment on lines +20 to +23
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

The new Sentinel entry is dated 2023-10-27 but is appended after 2026 entries, which breaks the apparent chronological ordering of this log and makes it harder to scan. Consider either using the actual discovery/fix date for this entry, or moving it to the correct position in the file based on date order.

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: Must use constant-time comparison to prevent timing attacks
// from revealing the expected remote static 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: Must use constant-time comparison to prevent timing attacks
// from revealing the expected remote static key
if (!std.crypto.timing_safe.eql([32]u8, preamble.initiator_static, self.remote_static)) {
return error.UnknownPeer;
}

Expand Down