Stitch any CI into Tangled
buildkite: enforce freshness window on webhook signatures
VerifySignature previously accepted any cryptographically valid
"timestamp=<unix>,signature=<hex>" header regardless of how old the
timestamp was. An attacker who captured a single signed delivery
could replay it indefinitely, creating duplicate status events and
unbounded growth in the events table.
Reject signatures whose timestamp is more than MaxSignatureAge (5
minutes) from the local clock in either direction. The symmetric
bound also defeats implausibly future-dated stamps that would
otherwise mint a long replay window. The clock is read through a
package-level timeNow var so tests can pin it deterministically; the
existing fixed-timestamp test now stubs the clock and a new stale
case covers the rejection path.
+55
-11
+34
-11
internal/buildkite/buildkite.go
+34
-11
internal/buildkite/buildkite.go
···
288
288
WebhookModeSignature WebhookMode = "signature"
289
289
)
290
290
291
+
// MaxSignatureAge bounds how far the signed timestamp may be from
292
+
// the local clock before VerifySignature rejects the request as
293
+
// stale (or implausibly future-dated). Without this, an attacker who
294
+
// captures one valid signed delivery can replay it indefinitely,
295
+
// generating duplicate status events and unbounded growth in the
296
+
// events table. Five minutes matches Buildkite's own published
297
+
// guidance for verifying their signatures and is the same window
298
+
// Stripe et al. use; it absorbs realistic clock skew while keeping
299
+
// the replay surface tight.
300
+
const MaxSignatureAge = 5 * time.Minute
301
+
302
+
// timeNow is the clock VerifySignature consults for freshness. A
303
+
// package-level var so tests can pin "now" deterministically without
304
+
// touching the system clock or threading a clock argument through
305
+
// every caller.
306
+
var timeNow = time.Now
307
+
291
308
// VerifySignature validates the X-Buildkite-Signature header against
292
309
// secret using the documented "<timestamp>.<body>" HMAC-SHA256
293
-
// scheme. Returns nil when the header is well-formed and the digest
294
-
// matches; any other condition returns an error.
295
-
//
296
-
// We deliberately do NOT enforce a freshness window on timestamp:
297
-
// callers in practice consume idempotent or storage-deduplicated
298
-
// events, so a replayed event is at worst a duplicate publish.
299
-
// Callers that need stricter freshness should layer it on top.
310
+
// scheme. Returns nil when the header is well-formed, the digest
311
+
// matches, and the signed timestamp is within MaxSignatureAge of
312
+
// now; any other condition returns an error.
300
313
//
301
314
// The header format is "timestamp=<unix>,signature=<hex>".
302
315
func VerifySignature(header, secret string, body []byte) error {
···
325
338
if ts == "" || sig == "" {
326
339
return errors.New("malformed signature header")
327
340
}
328
-
// Sanity-check the timestamp is a parseable int. The value
329
-
// itself isn't validated against the clock (see comment above),
330
-
// but a non-numeric timestamp is structurally invalid.
331
-
if _, err := strconv.ParseInt(ts, 10, 64); err != nil {
341
+
tsInt, err := strconv.ParseInt(ts, 10, 64)
342
+
if err != nil {
332
343
return fmt.Errorf("invalid timestamp: %w", err)
344
+
}
345
+
// Freshness check: reject signatures whose timestamp is more
346
+
// than MaxSignatureAge away from now in either direction. The
347
+
// symmetric bound also rejects implausibly future-dated stamps,
348
+
// which would otherwise let an attacker mint a replay window
349
+
// well into the future.
350
+
skew := timeNow().Sub(time.Unix(tsInt, 0))
351
+
if skew < 0 {
352
+
skew = -skew
353
+
}
354
+
if skew > MaxSignatureAge {
355
+
return fmt.Errorf("signature timestamp outside freshness window (skew %s)", skew)
333
356
}
334
357
335
358
mac := hmac.New(sha256.New, []byte(secret))
+21
internal/buildkite/buildkite_test.go
+21
internal/buildkite/buildkite_test.go
···
16
16
"net/http/httptest"
17
17
"strings"
18
18
"testing"
19
+
"time"
19
20
)
20
21
21
22
// TestVerifySignature covers the HMAC mode end to end. The reference
···
26
27
body := []byte(`{"event":"build.finished"}`)
27
28
const ts = "1700000000"
28
29
30
+
// Pin the clock to "now == ts" so the freshness check is
31
+
// satisfied for the in-window cases and we can dial it past the
32
+
// max-age boundary for the stale case below.
33
+
tsTime := time.Unix(1700000000, 0)
34
+
prevNow := timeNow
35
+
timeNow = func() time.Time { return tsTime }
36
+
defer func() { timeNow = prevNow }()
37
+
29
38
mac := hmac.New(sha256.New, []byte(secret))
30
39
mac.Write([]byte(ts))
31
40
mac.Write([]byte("."))
32
41
mac.Write(body)
33
42
good := hex.EncodeToString(mac.Sum(nil))
34
43
44
+
// Same body/secret/timestamp, but signed for a moment far in
45
+
// the past relative to our pinned clock. A correct verifier
46
+
// must reject it even though the HMAC itself is valid, otherwise
47
+
// captured deliveries replay forever.
48
+
const staleTS = "1600000000"
49
+
staleMac := hmac.New(sha256.New, []byte(secret))
50
+
staleMac.Write([]byte(staleTS))
51
+
staleMac.Write([]byte("."))
52
+
staleMac.Write(body)
53
+
staleSig := hex.EncodeToString(staleMac.Sum(nil))
54
+
35
55
cases := []struct {
36
56
name string
37
57
header string
···
48
68
{"non-numeric timestamp", "timestamp=abc,signature=" + good, secret, body, true},
49
69
{"wrong signature", "timestamp=" + ts + ",signature=00", secret, body, true},
50
70
{"wrong body", "timestamp=" + ts + ",signature=" + good, secret, []byte("nope"), true},
71
+
{"stale timestamp", "timestamp=" + staleTS + ",signature=" + staleSig, secret, body, true},
51
72
}
52
73
for _, c := range cases {
53
74
t.Run(c.name, func(t *testing.T) {