Repository navigation
Add _at_time variants for deterministic testing against real fixtures - #205
Open
crow (crow-004) wants to merge 1 commit into
Open
crow (crow-004) wants to merge 1 commit into
crow (crow-004) wants to merge 1 commit into
Conversation
validate_cert_trust_chain and validate_and_parse_attestation_doc both validate against the real system clock, with no way for a downstream consumer to override it: get_epoch()'s FAKETIME override is #[cfg(test)] -gated inside this crate, so it never compiles into the published library. That makes it impossible for a crate depending on this one to write a deterministic test against a real, previously-captured attestation document -- a real Nitro leaf certificate is only valid for a few hours (this crate's own test comments note ~3), so the test would start failing on its own a few hours after the fixture was captured, for no code reason. Adds validate_cert_trust_chain_at_time and validate_and_parse_attestation_doc_at_time: the same logic, parameterized by an explicit `now: u64` instead of calling get_epoch() internally. The existing functions become thin wrappers calling the new ones with get_epoch()? -- no behavior change, purely additive, nothing existing is touched. Verified against this crate's own real 2023 fixture (test-data/beta/valid-attestation-doc-bytes): the new _at_time function agrees with the existing real-clock function when given an equivalent "now" (passes regardless of whether the fixture happens to be currently expired), and correctly rejects it at the unix epoch (long before any real Nitro certificate existed). Confirmed this patch introduces no new test failures: a clean, unmodified checkout of main already has the same 6 pre-existing time_sensitive test failures locally (they need FAKETIME set, same as this repo's own CI config provides) -- not something this PR causes or is responsible for fixing. This is what let a downstream project (a TEE/HSM attestation-verifier tool) write its own first-ever genuine positive test against a real, captured Nitro attestation document, rather than only ever testing rejection paths (garbage input, no hardware present). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This branch has not been deployed
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
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
validate_cert_trust_chainandvalidate_and_parse_attestation_docboth validate against the real system clock, with no way for a downstream consumer to override it:get_epoch()'sFAKETIMEoverride is#[cfg(test)]-gated inside this crate, so it never compiles into the published library. That makes it impossible for a crate depending on this one (as a normal library dependency, not via#[cfg(test)]) to write a deterministic test against a real, previously-captured attestation document — a real Nitro leaf certificate is only valid for a few hours (this crate's own test comments note ~3), so such a test would start failing on its own a few hours after the fixture was captured, for no actual code reason.This PR adds
validate_cert_trust_chain_at_timeandvalidate_and_parse_attestation_doc_at_time: the same logic, parameterized by an explicitnow: u64instead of callingget_epoch()internally. The existing functions become thin wrappers calling the new ones withget_epoch()?— no behavior change, purely additive, nothing existing is touched.Why this came up
I'm building an independent attestation-verification CLI that depends on this crate. I wanted to write a genuine positive test ("a real captured document verifies successfully") instead of only testing rejection paths, and ran into exactly the limitation above — confirmed by reading this crate's real source (not assumed), including the fact that this crate's own tests already hit the same ~3-hour wall and work around it with
FAKETIME, which simply isn't available outside#[cfg(test)].Testing
test-data/beta/valid-attestation-doc-bytes), deliberately NOT asserting a specific pass/fail outcome tied to that fixture's own validity window (an earlier draft of this PR did exactly that, using the ~3-hour-old "approximately 15:15" timestamp this file's own comment records, and it failed — the comment's "approximately" turned out to matter). Instead:validate_and_parse_attestation_doc_at_time_matches_the_real_clock_entry_point_given_the_same_now: proves the new function agrees with the existing one when given an equivalent "now", regardless of whether the fixture is currently expired.validate_and_parse_attestation_doc_at_time_rejects_a_real_doc_at_the_unix_epoch: proves the time parameter has real effect (1970 predates every real Nitro cert).mainalready has the same 6 pre-existingtime_sensitivetest failures locally withoutFAKETIMEset (matches your own CI config providing it) — not something this PR causes or is in scope to fix.Happy to adjust naming/shape if you'd prefer a different API (e.g. accepting
webpki::Timedirectly, or a builder-style config) — went with the smallest, most obviously-safe additive change.🤖 Generated with Claude Code