Skip to content

use constant-time comparison for auth token validation#7730

Merged
jh-block merged 2 commits intoblock:mainfrom
extrasmall0:fix/constant-time-auth-comparison
Mar 11, 2026
Merged

use constant-time comparison for auth token validation#7730
jh-block merged 2 commits intoblock:mainfrom
extrasmall0:fix/constant-time-auth-comparison

Conversation

@extrasmall0
Copy link
Contributor

The auth middleware in auth.rs uses == for comparing the X-Secret-Key header against the stored secret. Rust's default string comparison short-circuits on the first differing byte, which leaks timing information that could let an attacker recover the key incrementally.

This swaps the comparison for subtle::ConstantTimeEq::ct_eq, which always compares every byte regardless of where they differ. subtle is already in the dependency tree (pulled in transitively), so this just adds it as a direct dep for goose-server.

The explicit key.len() == state.len() guard before ct_eq is technically redundant (ct_eq on unequal-length slices returns 0), but makes the intent clearer and avoids doing the full comparison when lengths obviously differ.

Closes #7710

Signed-off-by: Extra Small <littleshuai.bot@gmail.com>
@extrasmall0 extrasmall0 force-pushed the fix/constant-time-auth-comparison branch from 1ad2e2e to 08cc403 Compare March 8, 2026 16:04
@DOsinga DOsinga self-assigned this Mar 9, 2026
@jh-block jh-block self-assigned this Mar 10, 2026
@jh-block
Copy link
Collaborator

Doesn't the explicit length guard let an attacker know whether their key is the right length or not based on the timing? I don't think it makes the intent clearer and doing the full computation every time is rather the point of constant time comparison. Happy to merge if that is removed.

@DOsinga DOsinga removed their assignment Mar 10, 2026
@extrasmall0
Copy link
Contributor Author

Good point — the length check does leak that info through timing. Removed it in 99e1d11, ct_eq handles different lengths fine on its own.

Signed-off-by: Extra Small <littleshuai.bot@gmail.com>
@extrasmall0 extrasmall0 force-pushed the fix/constant-time-auth-comparison branch from 99e1d11 to eea6a07 Compare March 11, 2026 07:19
@jh-block jh-block added this pull request to the merge queue Mar 11, 2026
Merged via the queue into block:main with commit a28c306 Mar 11, 2026
20 checks passed
lifeizhou-ap added a commit that referenced this pull request Mar 11, 2026
* main:
  use pnpm for Desktop Electron App (#7679)
  use constant-time comparison for auth token validation (#7730)
@extrasmall0
Copy link
Contributor Author

Thanks for the quick review and merge @jh-block!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: Timing Attack Vulnerability in Authentication Middleware

3 participants