PR2: per-provider fail-closed MeasurementPolicy (Chutes attested-provider) - #746
Conversation
PR2 of the Chutes attested-provider integration (stacked on #745's ProviderTier). The OS-image-hash gate is currently a single global env (ALLOWED_IMAGE_HASHES) that is fail-open (empty => skip) and shared across all providers. Safe for NEAR's own fleet, unsafe for an attested third party: an empty allowlist would silently accept arbitrary software, and one provider's measurement would satisfy another's backend. Introduce MeasurementPolicy (per-provider, selected by ProviderTier at construction, never from a report field): - near()/near_from_env(): reproduce today's exact semantics (empty = skip). - attested3p(): Attested3p tier, TCB-up-to-date enforced by default. - assert_enforceable(): fail-closed guard — Attested3p + empty allowlist is REJECTED; no-op for Near (keeps skip) and NonAttested. AttestationVerifier now holds a MeasurementPolicy; new()/from_env() keep their signatures and build a Near policy (existing callers/tests untouched, NEAR path byte-for-byte unchanged). with_policy() added for per-provider verifiers (PR7). verify_attestation_report calls assert_enforceable() as step 0. Tests: 4 policy unit tests + 2 deterministic fail-closed integration tests; GLM-5/Qwen fixtures unchanged (4/4); services lib 347 pass; e2e_all 546 pass; fmt + clippy clean. No runtime behavior change (no Attested3p provider wired yet).
GitHub Actions
Inactive
![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 introduces a per-provider MeasurementPolicy to replace the global environment-driven attestation verification, allowing stricter fail-closed policies for third-party providers while preserving NEAR's historical fail-open behavior. Feedback on the changes highlights that the newly introduced require_dstack_event_log field is not yet respected during verification, which could cause failures for providers that do not supply an event log; a conditional check is suggested to resolve this.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| let event_log_data = | ||
| self.verify_rtmr3_and_extract(attestation_report, &td_report.rt_mr3)?; |
There was a problem hiding this comment.
The newly introduced require_dstack_event_log field in MeasurementPolicy is not respected during attestation verification. Currently, verify_rtmr3_and_extract is called unconditionally, which will cause verification to fail with a MissingField("event_log") error if a provider has require_dstack_event_log set to false and does not provide an event log. We should conditionally verify the event log only if it is required by the policy or if it is present in the report.
let event_log_data = if self.policy.require_dstack_event_log
|| attestation_report.contains_key("event_log")
{
self.verify_rtmr3_and_extract(attestation_report, &td_report.rt_mr3)?
} else {
EventLogData {
os_image_hash: None,
compose_hash: None,
}
};
Review — PR2: per-provider fail-closed MeasurementPolicyReviewed the full diff against No critical issues found. One non-blocking note for awareness:
✅ Approved — safe to merge; the note above is a forward-looking guardrail, not a blocker. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a per-provider MeasurementPolicy abstraction for attestation verification so that third-party attested providers can be configured fail-closed (empty OS-image-hash allowlist rejects), while preserving NEAR’s existing fail-open-on-empty behavior.
Changes:
- Added
MeasurementPolicy(tier-scoped allowlist + TCB requirements) with a fail-closedassert_enforceable()guard forProviderTier::Attested3p. - Refactored
AttestationVerifierto store aMeasurementPolicy, addedwith_policy(..), and enforced the policy guard as step 0 of verification. - Added deterministic tests validating the fail-closed guard ordering and that NEAR’s empty-allowlist path does not fail closed.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
crates/services/src/attestation/measurement.rs |
New policy type and guard for per-tier measurement enforcement. |
crates/services/src/attestation/verification.rs |
Threads MeasurementPolicy through verifier; adds fail-closed guard and uses policy fields for checks. |
crates/services/src/attestation/mod.rs |
Exposes the new measurement module and re-exports MeasurementPolicy. |
crates/services/tests/attestation_verification_test.rs |
Adds deterministic integration tests for fail-closed vs NEAR behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub fn near(allowed_os_image_hashes: HashSet<String>, require_tcb_up_to_date: bool) -> Self { | ||
| Self { | ||
| tier: ProviderTier::Near, | ||
| allowed_os_image_hashes, | ||
| require_tcb_up_to_date, | ||
| require_dstack_event_log: true, | ||
| } | ||
| } | ||
|
|
||
| /// An attested third-party ([`ProviderTier::Attested3p`]) policy. TCB-up-to-date | ||
| /// is enforced by default (stricter than NEAR's own fleet), and an empty | ||
| /// `allowed_os_image_hashes` is rejected by [`Self::assert_enforceable`]. | ||
| pub fn attested3p(allowed_os_image_hashes: HashSet<String>) -> Self { | ||
| Self { | ||
| tier: ProviderTier::Attested3p, | ||
| allowed_os_image_hashes, | ||
| require_tcb_up_to_date: true, | ||
| require_dstack_event_log: true, | ||
| } |
| } else if self.policy.enforces_image_hash() { | ||
| return Err(AttestationVerificationError::ImageHashMismatch( | ||
| "ALLOWED_IMAGE_HASHES configured but no os-image-hash in event log".to_string(), | ||
| "image-hash allowlist configured but no os-image-hash in event log".to_string(), | ||
| )); |
PierreLeGuen
left a comment
PierreLeGuen
left a comment
There was a problem hiding this comment.
Requesting changes for one security-sensitive policy invariant issue.
Finding:
crates/services/src/attestation/measurement.rs:104:assert_enforceable()only fails closed forAttested3pwhenrequire_dstack_event_logis true, but that flag is public and is not actually used byAttestationVerifierto change verification behavior in this PR. A caller can constructMeasurementPolicy { tier: ProviderTier::Attested3p, allowed_os_image_hashes: HashSet::new(), require_dstack_event_log: false, ... }, pass it towith_policy(), and the verifier will still require/replay an event log but will skip the OS image hash allowlist entirely. That contradicts the PR’s core invariant that Attested3p + empty allowlist is rejected fail-closed, and it creates a footgun for the later provider wiring. Until a real non-event-log fallback exists and enforces its own measurement pins, please make the fields private / route construction through safe constructors, or reject empty Attested3p allowlists unconditionally.
Checked locally:
cargo test -p services attestation::measurement -- --nocapturepassed (4 unit tests)cargo test -p services --test attestation_verification_test attested3p_empty_allowlist_fails_closed_before_verification -- --nocapturepassedcargo test -p services --test attestation_verification_test near_empty_allowlist_does_not_fail_closed -- --nocapturepassedcargo check -p servicespassedgit diff --check origin/main...HEADpassed
GitHub checks were also green when refreshed (Lint, Test Suite, security_audit, claude-review).
…ds, normalize, restore error string Review feedback on #746: - Pierre (blocking) + gemini: `require_dstack_event_log` was a public flag that gated the fail-closed check but had no other effect in this PR — a caller could build `MeasurementPolicy { tier: Attested3p, allowed_os_image_hashes: empty, require_dstack_event_log: false }` and slip past `assert_enforceable()` while the OS-image-hash check was skipped. The register-pin path it implied does not exist until a later PR. Fix: drop the field entirely from this PR; the fail-closed guard is now UNCONDITIONAL for Attested3p + empty allowlist. The flag returns in the PR that adds the real (measurement-enforcing) register-pin fallback. - Pierre: make construction safe. MeasurementPolicy fields are now PRIVATE; policies can only be built via near()/near_from_env()/attested3p(), with accessors tier()/require_tcb_up_to_date()/enforces_image_hash()/ allows_image_hash(). Impossible to assemble an attested policy in a fail-open state. - Copilot: normalize allowlist entries (trim, strip 0x, lowercase) in all constructors so a non-lowercase/0x-prefixed allowlist value still matches. - Copilot: restore the original "ALLOWED_IMAGE_HASHES configured but no os-image-hash in event log" error string (NEAR log-monitoring stability). Tests: 5 policy unit tests (incl. normalization) + 2 deterministic fail-closed integration tests; GLM-5/Qwen fixtures unchanged; services lib 348 pass; e2e_all 546 pass; fmt + clippy clean. NEAR path still byte-for-byte unchanged.
GitHub Actions
Inactive
|
Thanks all — addressed in @PierreLeGuen (blocking) + @gemini-code-assist — the pub fn assert_enforceable(&self) -> Result<(), AttestationVerificationError> {
if self.tier == ProviderTier::Attested3p && self.allowed_os_image_hashes.is_empty() {
return Err(/* fail-closed */);
}
Ok(())
}The flag will return in the later PR that adds the real, measurement-enforcing register-pin fallback (where it actually changes verification behavior and is enforced), not as a no-op knob that can disable safety. @PierreLeGuen — safe construction. @copilot — allowlist normalization. All constructors now normalize entries (trim, strip @copilot — error string. Restored the original Local: 5 policy unit tests + 2 fail-closed integration tests, GLM-5/Qwen fixtures unchanged, services lib 348 pass, |
|
The changes look excellent. By making the |
PierreLeGuen
left a comment
PierreLeGuen
left a comment
There was a problem hiding this comment.
Reviewed the current head (311d751) locally, including the full diff in crates/services/src/attestation/{measurement,verification}.rs, the attestation tests, existing verifier call sites in the provider pool, PR discussion, and GitHub checks. The prior fail-open construction issue is fixed: MeasurementPolicy fields are private, the unused event-log flag is removed, and Attested3p + empty allowlist now fails closed unconditionally before quote parsing.
Local checks run:
cargo test -p services measurement::tests --libcargo test -p services --test attestation_verification_test allowlistcargo fmt --all -- --checkgit diff --check origin/main...HEAD
GitHub checks were green when refreshed: Lint, Test Suite, security_audit.
… (remove padded-address fallback) (#748) * attestation: pluggable per-tier ReportDataVerifier (strict binding for 3p) PR3 of the Chutes attested-provider integration (follows #745, #746). A TDX quote's report_data binds the request+backend: [32:64]=nonce (freshness), [0:32]=SHA256(signing_address ‖ tls_cert_fingerprint) when a fingerprint is present. NEAR's verifier also accepts a fallback when no fingerprint is present: [0:32]=padded(signing_address). That fallback drops the TLS co-binding — fine for our own fleet (always sends a fingerprint), but a connection-hijack hole for an attested third party whose nodes we don't operate. Make report_data verification pluggable: - ReportDataVerifier trait. - NearReportDataVerifier: today's logic verbatim (fingerprint binding + padded fallback). The previous inline verify_report_data body, moved unchanged. - StrictBoundReportDataVerifier: requires tls_cert_fingerprint (no fallback), re-checks nonce, verifies the fingerprint binding. AttestationVerifier holds Arc<dyn ReportDataVerifier>, selected from policy.tier() at construction (never from a report field): Near/NonAttested -> Near, Attested3p -> Strict. new()/from_env() build a Near policy => NEAR path byte-for-byte unchanged. Dropped the unused signing_algo argument. Tests: 6 report_data unit tests (incl. strict-rejects-missing-fp, strict-rejects-stale-nonce-replay); GLM-5/Qwen fixtures + PR2 fail-closed tests unchanged; services lib 354 pass; e2e_all 546 pass; fmt + clippy clean. No runtime behavior change (no Attested3p provider wired yet). * attestation: apply strict report_data binding to ALL tiers incl. NEAR Per review: the code will be audited and NEAR must be held to the same bar as third parties — no weaker verification path for our own fleet. Remove NearReportDataVerifier and the padded-`signing_address` fallback entirely. StrictBoundReportDataVerifier is now the single report_data verifier for every attested tier: tls_cert_fingerprint is mandatory (report_data[0:32] = SHA256(signing_address ‖ tls_cert_fingerprint)) and report_data[32:64] = nonce. The Arc<dyn ReportDataVerifier> seam is kept so a provider with a different report_data layout can plug its own verifier later. This is a deliberate NEAR hardening, NOT a no-op: NEAR backends already bind the fingerprint (the fallback was never exercised), so it should be transparent, but it changes NEAR verification and must be validated across all NEAR models in staging before prod promotion — any model not binding a fingerprint now fails loudly (correct) instead of silently downgrading trust. Evidence it's safe: the live fixture tests (test_verify_glm5_attestation, test_verify_qwen35_attestation) now run under the strict verifier against real production NEAR backends and pass. report_data unit tests cover rejects-missing-fp and rejects-stale-nonce-replay. services lib 352 + e2e_all 546 green; fmt+clippy clean.
…nt GPU evidence) (#749) * attestation: per-tier GPU-evidence requirement policy PR4 of the Chutes attested-provider integration (follows #745, #746, #748). Completes the per-tier verifier policy story (TCB floor #746, report_data #748, GPU evidence now) before the Chutes provider code. verify_gpu_evidence is best-effort today: absent nvidia_payload -> Ok(None) ("acceptable for non-GPU CVMs"). Correct for NEAR's mixed fleet, but an attested third party that advertises confidential GPU compute (e.g. Chutes/Hopper) could omit GPU evidence and silently pass with gpu_verdict=None. Add MeasurementPolicy.require_gpu_evidence (private + accessor): - near()/near_from_env() -> false (historical best-effort, NEAR unchanged). - attested3p() -> true. verify_gpu_evidence's absent-payload branch now consults the policy: Ok(None) when not required (NEAR), Err(GpuVerificationFailed) when required (attested 3p). The mandatory nonce binding + NRAS verification on present evidence are unchanged. Tests: gpu_evidence_required_only_for_attested3p (policy); gpu_evidence_absent_ok_for_near_but_rejected_for_attested3p (deterministic verify test, no network — absent branch returns before NRAS). GLM-5/Qwen fixtures unchanged; services lib 354 + e2e_all 546 green; fmt + clippy clean. No NEAR behavior change. * address review: reject malformed (non-string) nvidia_payload instead of skipping gemini (security-high) + Copilot flagged a type-confusion gap in verify_gpu_evidence: `get("nvidia_payload").and_then(|v| v.as_str())` returns None when the field is PRESENT but not a string (object/number/bool), so a malformed payload was treated as "absent" and silently skipped GPU verification (for NEAR / best-effort tiers). Distinguish the three cases explicitly: - absent / null / empty string -> no GPU evidence offered (policy decides: Ok(None) for NEAR, Err for an attested 3p that requires it) - non-empty string -> the evidence to verify - present but not a string -> MALFORMED, always Err for every tier Test extended: a present-but-non-string nvidia_payload errors for both NEAR and attested3p (never silently skipped). services lib green; fixtures unchanged; fmt + clippy clean.
…, hard-off) (#752) * inference_providers: add attested::chutes::Provider data-path skeleton PR5 of the Chutes attested-provider integration — first Chutes provider code (PR1-PR4 were verifier groundwork). Adds attested::chutes::Provider, an OpenAI-compatible inference client over https://llm.chutes.ai/v1. The data path is delegated to an inner ExternalProvider (ProviderConfig::OpenAiCompatible) — Chutes' inference API is OpenAI-compatible, so chat/stream/text/image/audio/score/rerank/embeddings/privacy/models reuse the proven, already-tested path; no duplicated HTTP/SSE code. It's a distinct type under attested::chutes (not just an external provider) because a later PR bolts a ChutesBackendVerifier onto it (fetch /evidence, transform TDX quote + GPU evidence into NEAR's attestation format, run the shared verifier under the strict Attested3p policy from #746/#748/#749). get_attestation_report and get_signature are explicitly stubbed as "not yet wired (hard-off gate)" — NOT delegated to the inner external provider's "unsupported" path — so the skeleton can never be mistaken for verified. Hard-off: nothing constructs a chutes::Provider yet (no pool wiring), so it's unreachable at runtime. Pool wiring + enable flag land with the verifier PR; production enablement stays gated on confirming Chutes' evidence format. Tests: construction keeps the model id + defaults to the Chutes base URL; attestation/signature return the "not yet wired" error. inference_providers lib 238 + e2e_all 546 green; fmt + clippy clean. No runtime behavior change. * address review: redact Config api_key in Debug, private fields, guard timeout, match error variants PR5 review feedback (#752): - Pierre (blocking) + Copilot + claude[bot]: Config derived Debug while holding a plaintext api_key -> secret leak via any {:?}. Make Config fields private (only constructor is Config::new) and implement a custom Debug that redacts api_key. - Copilot: timeout_seconds: i64 is cast `as u64` in the HTTP backend; a negative value would underflow to an effectively infinite timeout. new() now replaces a non-positive timeout with DEFAULT_TIMEOUT_SECONDS (300); private fields prevent a struct-literal bypass. - Copilot: tests asserted on Debug strings (brittle). Match the concrete error variant (AttestationError::FetchError / CompletionError::CompletionError) and assert on the message instead. Added config_non_positive_timeout_falls_back_to_default (also asserts the api_key is redacted, not leaked). inference_providers lib green; fmt + clippy clean. * address review: add Config::with_base_url builder + timeout_seconds() accessor; robust test assertions Pierre's follow-up observations on #752: - Config::new hardcoded base_url though the field is configurable + exposed via base_url(). Add a `with_base_url(..)` builder so the verifier-wiring PR + its integration/mock-server tests can override the URL (and the accessor is now meaningful). - The timeout test substring-matched Debug output and used a 1-char key. Add a timeout_seconds() accessor and assert on it (not Debug); split out config_debug_redacts_api_key using a realistic-length secret so the leak check is meaningful. Add with_base_url_overrides_default. inference_providers lib green; fmt + clippy clean. Still additive + unwired.
Summary
PR2 of the Chutes attested-provider integration (stacked on #745's
ProviderTier). Introduces a per-provider, fail-closedMeasurementPolicyand threads it throughAttestationVerifier. NEAR's own-fleet verification path is byte-for-byte unchanged; the new behavior only applies to attested third-party providers (none wired yet — that's PR7).No provider uses the
Attested3ppolicy at runtime in this PR; this is the verifier groundwork that PR7 (Chutes) selects per-tier at pool construction.The problem it fixes
Today the OS-image-hash gate is a single global env (
ALLOWED_IMAGE_HASHES) that is fail-open: when empty it skips the check entirely (verification.rsold:224/:232), and oneArc<AttestationVerifier>is shared across every provider. That is acceptable for NEAR's own fleet, but for a third-party attested provider it has two holes:What changed
crates/services/src/attestation/measurement.rs—MeasurementPolicy { tier, allowed_os_image_hashes, require_tcb_up_to_date }with private fields (construction only via safe constructors):near(..)/near_from_env()— reproduce the exact previous semantics (empty allowlist = skip). Allowlist entries are normalized (trim, strip0x, lowercase).attested3p(..)—Attested3ptier;require_tcb_up_to_datedefaults true (stricter than NEAR). Allowlist entries are normalized.assert_enforceable()— the fail-closed guard: anAttested3ppolicy with an empty allowlist is unconditionally rejected, never skipped. No-op forNear(keeps skip). The guard no longer depends on any flag a caller can flip.enforces_image_hash()— true iff the allowlist is non-empty.allows_image_hash(hash)— normalizes the lookup hash before checking, so0xDEADBEEFanddeadbeefare equivalent.AttestationVerifiernow holds aMeasurementPolicyinstead of the bareallowed_image_hashes+require_tcb_up_to_datefields.new(allowed, pccs, require_tcb)andfrom_env()keep their signatures and build aNearpolicy → existing callers/tests untouched, NEAR behavior identical.with_policy(policy, pccs)constructor for per-provider verifiers (PR7).verify_attestation_reportnow callsself.policy.assert_enforceable()?as step 0 (before any quote work), and reads the TCB flag + image-hash allowlist from the policy."ALLOWED_IMAGE_HASHES configured but no os-image-hash in event log"— log-based monitoring is unaffected.Decision: keep NEAR fail-open-on-empty, make 3p fail-closed
We deliberately do not flip NEAR to fail-closed in this PR — that would be a behavior change to the production fleet (some envs run without
ALLOWED_IMAGE_HASHES) and is out of scope. The fail-closed semantics are gated strictly onProviderTier::Attested3p, and are unconditional — there is no flag a caller can set to bypass them:Because fields are private and construction is only via safe constructors, it is impossible to assemble an
Attested3ppolicy in a fail-open state. Because the tier is chosen at construction fromProviderTier(never from a report field), a Chutes policy can never validate a NEAR backend or vice-versa.Tests
measurement::tests(5): empty-Near enforceable+skips, non-empty-Near enforces, empty-Attested3p rejected (unconditional), non-empty-Attested3p enforced+TCB-required, allowlist entries normalized (0xDEADBEEFmatchesdeadbeef).attestation_verification_test(2, deterministic, no network):attested3p_empty_allowlist_fails_closed_before_verification— proves the guard fires first (an empty report yields the fail-closed error, notMissingField("intel_quote")).near_empty_allowlist_does_not_fail_closed— provesNearstill reaches real verification (historical behavior preserved).serviceslib suite 348 pass;cargo build --workspacegreen; fmt + clippy clean; localcargo test --test e2e_allgreen.Impact
Attested3pprovider wired). NEAR identical.new/from_envAPI; the only new control-flow (the fail-closed guard) is unreachable forNear.ReportDataVerifierstrict) and PR7 (Chuteswith_policy(chutes_policy(), ..)).🤖 Generated with Claude Code