sec: constant-time admin token compare#294
Conversation
Direct == on admin token leaks length-bytes via timing channel. Replace with subtle.ConstantTimeCompare. Pre-prod hardening for multi-tenant deploy where admin token grants user-creation rights.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves security by implementing constant-time comparison for admin token validation to mitigate timing attacks. The review feedback correctly identifies that the current implementation still leaks the token length and suggests comparing SHA-256 hashes of the tokens to eliminate this side-channel.
| import ( | ||
| "bytes" | ||
| "context" | ||
| "crypto/subtle" |
There was a problem hiding this comment.
To support a more robust constant-time comparison that avoids leaking the token length, the "crypto/sha256" package is required.
| "crypto/subtle" | |
| "crypto/sha256" | |
| "crypto/subtle" |
| token := r.Header.Get("Authorization") | ||
| if token != *adminToken { | ||
| // Constant-time compare to avoid timing-attack leak of admin token bytes. | ||
| if subtle.ConstantTimeCompare([]byte(token), []byte(*adminToken)) != 1 { |
There was a problem hiding this comment.
While subtle.ConstantTimeCompare prevents byte-by-byte timing leaks, it still leaks the length of the adminToken because it returns immediately if the lengths of the two slices differ. A more robust approach is to compare the SHA-256 hashes of both tokens. Since hashes have a fixed length, this eliminates the length side-channel. For even better performance, the hash of the expected adminToken could be precomputed at startup.
| if subtle.ConstantTimeCompare([]byte(token), []byte(*adminToken)) != 1 { | |
| tokenHash := sha256.Sum256([]byte(token)) | |
| adminHash := sha256.Sum256([]byte(*adminToken)) | |
| if subtle.ConstantTimeCompare(tokenHash[:], adminHash[:]) != 1 { |
2 participants
Summary
Direct equality check on the admin token (
token != *adminToken) leaks information about the token byte-by-byte through a timing channel. Replaces it withsubtle.ConstantTimeCompare.Why
adminTokenis the master credential for/admin/*endpoints — creating users, listing them, etc. Any user created via this endpoint can connect a WhatsApp instance, so a leaked admin token compromises every tenant on the deployment.In a multi-tenant or hosted setup, a remote attacker with network access to the API can mount a timing oracle over many short HTTPS requests and recover the token character by character.
subtle.ConstantTimeCompareis the standard mitigation — it takes the same time regardless of where the strings differ.What changed
3 lines (
crypto/subtleimport + the compare line + comment).Backwards compatibility
100% — pure security hardening. Same input/output behaviour for all callers, no new env vars, no schema changes.
Verification
go build ./...passesgo vet ./...passescurl -H 'Authorization: <correct>'→ 200 (unchanged)curl -H 'Authorization: <wrong>'→ 401 (unchanged)Happy to apply the same hardening to the per-user
authalicetoken path if you'd like — that one currently hits the DB / cache, so the timing leak is much smaller, but I can rewrite it if you prefer defence-in-depth.