Use constant-time comparison for API key check#34
Merged
Conversation
Go's == on strings short-circuits at the first differing byte, so an attacker who can time many requests could recover the configured API key byte by byte. Hash both the provided and configured keys with SHA-256 and compare the digests with subtle.ConstantTimeCompare, which also hides the configured key's length. Closes #18 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017mkTn2GYT2E8mCtD5gVYy5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
withAPIKeymiddleware compared the client-supplied API key against the configured key with plain string==, which short-circuits at the first differing byte. An attacker able to time many requests could recover the key byte by byte (timing side channel), which is practical on a LAN — the primary deployment target.Both keys are now hashed with SHA-256 and the digests compared with
crypto/subtle.ConstantTimeCompare, so the comparison runs in constant time over a fixed length and neither key bytes nor the configured key's length leak through response timing. The early return for an empty configured key (auth disabled) is unchanged.Related Issue
Closes #18
Checklist
go test ./...passes locally) — existingTestAPIKey_RequiredWhenConfiguredandTestAPIKey_NotRequiredByDefaultcover the acceptance criteria and pass unchangedgo vet ./...andgolangci-lint runare cleandocs/API.md), configuration (README.md,packaging/pimonitor.example.yaml), or installation/packaging (packaging/install.sh, systemd units) — no user-facing behavior change, no docs needed/api/v1/...response shapes, or a new API version was introduced instead