fix: reject stale and future-dated attestation blobs at provider registration#129
fix: reject stale and future-dated attestation blobs at provider registration#129antojoseph wants to merge 2 commits into
Conversation
…stration Calls attestation.CheckTimestamp with a 5-minute max-age window immediately after VerifyJSON succeeds. A replayed or future-dated attestation blob is rejected and the provider is marked untrusted before any trust level is granted.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41b6cb20ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // provider could otherwise grant trust to a new connection without fresh | ||
| // hardware verification. 5 minutes matches the challenge cycle interval. | ||
| const attestationMaxAge = 5 * time.Minute | ||
| if !attestation.CheckTimestamp(result, attestationMaxAge) { |
There was a problem hiding this comment.
This new gate calls attestation.CheckTimestamp, but that helper currently returns time.Since(result.Timestamp) <= maxAge (coordinator/internal/attestation/attestation.go), which evaluates to true for future timestamps because time.Since is negative. As a result, future-dated blobs are still accepted here even though the log/error path says they are rejected, so an attacker can use a far-future attestation timestamp to bypass the intended replay protection window.
Useful? React with 👍 / 👎.
time.Since is negative for future timestamps, so the previous <= maxAge check was always true for future-dated blobs. Now explicitly rejects timestamps more than 30s in the future.
hankbobtheresearchoor
left a comment
hankbobtheresearchoor
left a comment
There was a problem hiding this comment.
LGTM. This adds the missing freshness gate before any provider trust is granted, and CheckTimestamp now explicitly rejects future-dated attestations instead of treating negative age as valid.
2 participants
Summary
attestation.CheckTimestampwith a 5-minute max-age window immediately afterVerifyJSONsucceeds at provider registrationMarkUntrustedand an early return — no trust level is grantedSecurity impact
Without this fix, an attacker who obtained a valid attestation blob (even legitimately, from a machine they control) could replay it indefinitely to register fake provider identities with
self_signedor higher trust. The 5-minute window matches the existing challenge cycle interval, so legitimate providers re-attesting normally are unaffected.Test plan
go test ./internal/api/... ./internal/attestation/...