Your HMAC webhook check calls subtle.ConstantTimeCompare — what still leaks timing?
answer
- the comparison is not the whole path
- look at what happens before it runs
- a missing header returns fastest of all
- an unknown sender costs a different lookup
- one exit, one status, one message
basics
~20 sThe comparison is only one step. Whether the signature header is present, whether it decodes, whether the sender's key was found, and how early the middleware returns all take measurably different time before the constant-time compare ever runs.
solid answer
~50 s`subtle.ConstantTimeCompare` makes one operation content-independent; it says nothing about the code around it. In a webhook middleware the observable time is dominated by everything upstream: a missing or malformed signature header returns in microseconds, a per-sender key resolved from an id in the header costs a cache hit or a database round trip depending on whether that sender exists, a body read aborted early differs from one read to the end, and a replay or timestamp check placed before the comparison rejects on its own path. The comparison itself still branches on length, so a wrong-length decode returns before the loop. The fix is structural rather than cryptographic: read the body, resolve the key, decode and length-check the incoming value, compare once, and give every failure the same status and the same message through one exit. To show the problem, benchmark the handler with matching and mismatching inputs, not the comparison.
code
go · 18 linesfunc verifyWebhook(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
body, readErr := io.ReadAll(r.Body)
sig, decErr := hex.DecodeString(r.Header.Get("X-Signature"))
key, keyErr := lookupKey(r.Header.Get("X-Sender"))
ok := readErr == nil && decErr == nil && keyErr == nil &&
len(sig) == sha256.Size &&
hmac.Equal(sig, tagFor(key, body))
if !ok {
http.Error(w, "invalid signature", http.StatusUnauthorized)
return
}
r.Body = io.NopCloser(bytes.NewReader(body))
next.ServeHTTP(w, r)
})
}go deeper
Even without having built one, know the ordering rule: read the whole body, decode the incoming value, compute the expected one, compare once, and return the same error for every kind of failure.
Explain what work happens before the comparison — reading the body, decoding the header, resolving the key — and why an early return on any of those is observable no matter how carefully the comparison itself is written.
Show the diagnosis. Name the lines that fork, restructure the middleware to one verdict and one exit, back it with a benchmark of matching and mismatching inputs, and state clearly what that benchmark cannot prove.
Decide how far the discipline extends. Uniform failure handling costs latency and debuggability on every request, so own the call about which endpoints get it and how the team keeps that reasoning visible after you have moved on.
## What the primitive actually promises `subtle.ConstantTimeCompare` promises one thing: the time it takes does not depend on *the contents* of the two slices you hand it. That is a promise about a single function call, roughly a microsecond of work on a 32-byte value. It is not a promise about the request path that call sits in, and reviewing only the comparison is the most common way a verification path stays observable after the "fix" lands. ## Where the time actually goes Walk an inbound-webhook middleware from the top and list every place two requests can diverge: **The header is absent.** `r.Header.Get("X-Signature")` returns an empty string, and if the code returns right there the request is answered after a header-map lookup — orders of magnitude faster than a request that reaches the comparison. **The header does not decode.** `hex.DecodeString` fails on odd length or a non-hex character, and it fails *at the first bad character*, so even the decode itself is input-dependent. Again, an immediate return is a distinct, fast path. **The sender is unknown.** Real webhook systems carry a sender or key id and resolve a per-sender secret from it. A hit may come from an in-process map in nanoseconds; a miss may be a database round trip of several milliseconds, or the reverse. That single lookup dwarfs every cryptographic consideration in the handler and tells an observer whether an id exists. **The body is read differently.** If the code streams the body and abandons the read on the first sign of trouble, a rejected request transfers less data and returns sooner. The expected value is computed over the *whole* body, so the body has to be read in full anyway before any comparison is meaningful. **Something before the comparison rejects first.** A timestamp or replay check placed ahead of the comparison means a stale-but-correctly-signed request and a fresh-but-wrongly-signed one leave on different paths at different times. **The comparison's own length branch.** A decoded value of the wrong length returns 0 before the accumulating loop runs. That is documented and usually harmless — the expected size is a public constant — but it is a fork, and it belongs in your list. **The response differs.** A different status code, a different message, or a log line written on one path and not another is a louder signal than any of the timing above, and it costs nothing to observe. ## Restructuring the middleware The shape that removes the forks is boring, and that is the point: 1. Read the body to the end, unconditionally, and keep the bytes. 2. Resolve the sender's key, unconditionally, folding "unknown sender" into an error value rather than an early return. 3. Decode the incoming value and check its decoded length against the expected size. 4. Compute the expected value over the body. 5. Compare once, with `hmac.Equal` or `subtle.ConstantTimeCompare(...) == 1`. 6. Combine every error and the comparison result into **one** boolean, and give every false a single `http.Error` with one status and one message. Go's `&&` still short-circuits, so combining the errors in one expression does not literally equalise the work; what it does is remove the *early exits*, so the remaining variation is the cost of steps you deliberately perform on every request. Doing the expensive, forkable work (the body read, the key lookup) at the top, before any decision, is what actually flattens the profile. After that, one exit and one response. Also remember the plumbing consequence: `http.Request.Body` is a one-shot `io.ReadCloser`. Once the middleware has drained it, the next handler sees nothing unless you put the bytes back with `r.Body = io.NopCloser(bytes.NewReader(body))`. ## Showing it rather than asserting it The diagnostic that makes this concrete in a pull request is a side-by-side benchmark: one case with a matching value, one with a value that differs in the first byte, both over a realistic body size. Run them with `go test -bench . -benchmem`. With `bytes.Equal` the mismatch case is measurably faster on a few kilobytes; with `subtle.ConstantTimeCompare` the two cases converge to within noise. That is a five-minute demonstration and it settles the argument about the comparison. Then say the honest second half out loud, because it is what separates a senior answer from a confident one. The benchmark measures an in-process function call. It does not model the network, which usually swamps microsecond differences with jitter — and does *not* always swamp them, because an attacker can average over many samples. More importantly, a benchmark that shows no difference proves nothing about the paths it did not exercise: it never saw the unknown-sender lookup or the malformed-header return. Those you fix by construction and by reading the code, not by measuring. ## The reviewer's summary When you leave the comment, do not stop at "use `hmac.Equal`". Say which lines fork, name the early return you want removed, and ask for one exit with one response. The comparison is the easy part; the shape of the handler is the finding.
- How would you demonstrate the difference to the pull request author?A benchmark with two sub-cases — a matching value and one that differs in the first byte — over a realistic size, run with `go test -bench .`. With `bytes.Equal` the mismatch case is measurably faster; with `subtle.ConstantTimeCompare` the two converge. Then say plainly that a benchmark showing no difference proves nothing about the paths it never ran.
- The per-sender key is looked up by an id in the header. Why does that matter here?Because an unknown id fails at the lookup, before any comparison, on a path with completely different cost — an in-memory miss is nanoseconds, a database miss is milliseconds. That single fork tells an observer whether a sender exists, and it dwarfs anything the comparison could leak. Resolve the key unconditionally and fold its error into the same single verdict.
- Does comparing the hex strings instead of the decoded bytes help?No. `==` on two strings is an ordinary variable-time compare that returns at the first differing byte, which is exactly what you were trying to avoid, and case-insensitive helpers such as `strings.EqualFold` widen what you accept as a bonus defect. Decode with `hex.DecodeString`, check the decoded length, and compare the bytes.
- Your middleware reads the body to verify it. What must it do before calling the next handler?Put the bytes back. `http.Request.Body` is a one-shot `io.ReadCloser`, so once `io.ReadAll` has drained it the next handler reads nothing. Reassign `r.Body = io.NopCloser(bytes.NewReader(body))` before calling `next.ServeHTTP`. Closing the original body does not refill it, and nothing in net/http rewinds it for you.
saying these in an interview costs you the question
- Says the handler is safe because one call is constant-time
- Returns a different status for unknown sender than for bad signature
- Benchmarks only the comparison, never the handler
- Abandons the body read as soon as something looks wrong
- Treats a clean local benchmark as proof of safety