feat(tee): TDX TCB appraisal, platform gates and acceptance policy - #1195
feat(tee): TDX TCB appraisal, platform gates and acceptance policy#1195odesenfans wants to merge 11 commits into
Conversation
dev was merged into main and deleted, so every trigger list named a branch that no longer exists: a PR against dev-2.1 ran no CI at all. Rather than swap in the new name and be back here at 2.2, match the glob dev*, which covers dev, dev-2.1 and whatever comes next. Note the glob does not cross a slash: dev* matches dev-2.1 but not dev/foo, which would need dev**. Every release branch so far uses the dash form. The od/** entries are untouched. They exist so a stacked PR (increment N+1 based on increment N's branch) carries CI before the stack reaches the release branch, and that is unchanged here.
…#1185) aleph-message 1.4.0 adds mode=tdx to the schema (TeePlatform.tdx, TdxRegisters, the per-platform registers union). This CRN has no TDX launch path yet, and a tdx message carries no firmware ref (measured modes forbid it), so letting one fall through build_create_vm_spec crashed on the firmware resolve with an error hiding the real cause. Reject measured modes other than SNP in the pre-I/O validation block, naming the mode. The existing SNP misrouting guard moves up with it: both fire before the rootfs download instead of after, matching the documented validation-before-I/O contract. Both guards now have tests.
New aleph_tee::tdx::quote module, sibling of the SNP report parser: parse_tdx_quote() walks header, v5 body descriptor, TD report body (1.0 and 1.5), signature data and the nested certification envelopes through a bounds-checked cursor. Structure only: signatures, the PCK chain and TCB policy are the next increments. The v5 body sits at offset 54, after a body descriptor; parsing it at the v4 offset (48) fails silently, shifting every register by six bytes into plausible-looking digests. The parser dispatches on the header version, and the fixture tests pin exact register values from two independent v5 sources. signed_region captures the exact bytes the quote signature covers: everything before the signature-data length field, descriptor included on v5. Confirmed against all four vendored fixtures, whose signatures verify over that range under their embedded attestation keys. Trailing bytes after the signature data are ignored (quotes arrive in padded fixed-size buffers); one upstream fixture deliberately carries trailing garbage to test exactly that.
foxpatch-aleph
left a comment
There was a problem hiding this comment.
The appraisal logic itself (walk, convergence, platform gates, policy, pinning) is correct and carefully documented, and the QE report offsets and OID/DER walk check out. However, the TCB Info and QE Identity documents' validity windows (issueDate/nextUpdate) are never checked against now, even though the collateral is supplied input and every other collateral window (CRLs, cert windows) is enforced. Because each collateral field is independently signed, an attacker can replay arbitrarily old, genuinely-signed Intel collateral with lowered SVN thresholds and have an unpatched platform appraise as UpToDate forever — defeating the increment's purpose and diverging from the DCAP reference, which threads an expiration check through every verification path and by default rejects expired collateral.
rust/crates/aleph-tee/src/tdx/tcb.rs (line 226): Blocking: the TCB Info and QE Identity bodies carry issueDate/nextUpdate (both fixtures do), but neither verify_tcb_info nor verify_qe_identity checks them against now. The collateral is supplied input here, just like the CRLs whose windows certs.rs::check_crl does enforce — and since each collateral field is independently signed, an attacker can pair a fresh PCK CRL with arbitrarily old TCB Info/QE Identity. Old TCB Info means old (lower) SVN thresholds, so a platform missing recent microcode appraises as UpToDate against the stale document when current collateral would say OutOfDate. The DCAP reference threads expiration_check_date through every verification path and reports a collateral_expiration_status, and Intel's appraisal sample policies only accept expired collateral via an explicit collateral_grace_period. Also note TcbInfo/QeIdentity are private and don't deserialize those fields, so a caller can't enforce this outside the module. Suggested fix: deserialize issueDate/nextUpdate and fail (or consult a grace-period knob on TdxTcbPolicy, matching the reference) when now is outside the window, plus a test with now_v4() + ~40 days pinning the rejection.
rust/crates/aleph-tee/src/tdx/pck_extension.rs (line 123): integer_u16 accepts negative DER INTEGERs: a first content byte with the MSB set (e.g. 0x80 = -128) is two's-complement negative, but this parses it as a large positive, which would inflate pcesvn in the walk (platform.pcesvn < level.tcb.pcesvn). Low risk since the source is an Intel-signed PCK, but rejecting values with the sign bit set is a one-line hardening that also documents the assumption.
rust/crates/aleph-tee/src/tdx/tcb.rs (line 69): The doc says Revoked "cannot be added to accepted_statuses", but accepted_statuses is a plain public BTreeSet that accepts TcbStatus::Revoked. The invariant is genuinely enforced in evaluate_tcb (good), so either soften the doc to say it's enforced there, or add a test pinning it: insert Revoked into a policy and assert a Revoked outcome is still rejected. Right now no test exercises the Revoked-vs-policy path, which is one of the security claims of this PR.
rust/crates/aleph-tee/src/tdx/tcb.rs (line 462): The denied_advisories enforcement is never executed by any test: the v4 fixture's accepted level carries no advisories, so this branch is dead in the suite. A synthetic-walk test (in the style of walk_falls_through_to_a_lower_status, which can put advisoryIDs on the matched level) with a denied advisory would cover the last policy dimension.
rust/crates/aleph-tee/src/tdx/tcb.rs (line 521): This asserts only is_err(), so it would also pass if the rejection came from an unrelated cause (e.g. an expired CRL window at now_outdated()). Asserting the error mentions "below every level" would pin the intended cause, as the sibling tests do for their failures.
rust/crates/aleph-tee/src/tdx/tcb.rs (line 95): Nit: "The outcome of a full TDX quote verification" — this is the TCB appraisal outcome; a full verification also returns registers and report_data (TdxVerification in verify.rs). Consider "The outcome of a TDX TCB appraisal" to avoid confusion.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev-2.1 #1195 +/- ##
========================================
Coverage 78.74% 78.74%
========================================
Files 145 145
Lines 15766 15766
Branches 994 994
========================================
Hits 12415 12415
Misses 3178 3178
Partials 173 173 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The certificate half of TDX quote verification, on top of the parser: - certs.rs pins the Intel SGX Provisioning Certification Root CA in-crate and requires the quote's embedded root to be byte-identical, the same fail-closed discipline as the pinned AMD ARK; an ignored network test compares the pin against what Intel currently serves. verify_pck_chain() walks signatures down from the pin, checks the three validity windows, and applies both CRLs, verifying CRL signatures against certificates from the quote's own verified chain so the collateral's issuer-chain fields are never a trust input. - collateral.rs carries the sgx_ql_qve_collateral-shaped JSON set; only the CRLs are consumed here, the rest feeds the TCB walk. - verify.rs composes verify_tdx_quote_chain(): Intel QE vendor gate, chain + CRLs, QE report signature under the PCK key, attestation-key binding (skipping it would let any key sign the body), and the quote signature over the signed region. Takes now as a parameter and never reads the clock: the vendored collateral's windows are all in the past, and freshness must stay testable against archived collateral. Fixtures: the Phala collateral pair for the v4 quote plus the outdated-TCB v5 quote and its collateral (same MIT source and pinned commit). The outdated pair passes the chain half by design: OutOfDate is a TCB status for the TCB walk, not a chain defect. The nix agent fileset gains the pinned root PEM: commonCargoSources keeps only .rs/.toml/lockfiles, and the measured agent build fails to compile without the include_bytes! target.
Completes aleph_tee::tdx quote verification on top of the chain half: - pck_extension.rs parses the platform identity (FMSPC, the 16 SGX component SVNs, PCESVN) from the PCK leaf's Intel SGX extension via a minimal bounds-checked DER walk keyed on the exact OIDs. - tcb.rs verifies Intel's signed TCB Info and QE Identity against the pinned root (signatures over the exact raw JSON bytes), walks the SVN ladder to the platform status (SGX components + PCESVN + the quote's TDX components), converges the TDX module and QE statuses worst-wins following the DCAP reference, enforces the TD_ATTRIBUTES.DEBUG gate, and applies a policy. The default accepts UpToDate and SWHardeningNeeded; Revoked is never acceptable and cannot be added to the accept set. - verify.rs gains verify_tdx_quote(), composing the chain half with the TCB appraisal into a TdxVerification carrying the pinned registers and report_data. Its doc records that success is still not a decision to trust a guest: the caller pins registers and binds report_data. The outdated fixture's PCK sits below every TCB level, so its walk finds no match (a rejection), matching the DCAP reference on that sample. Precise status transitions are unit-tested on the walk with synthetic TCB Info, independent of the fixtures.
aleph-tee compiles into the measured attest agent; the new modules move the launch measurements.
079aa8b to
bb15dce
Compare
foxpatch-aleph
left a comment
There was a problem hiding this comment.
The overall structure is strong: signature verification over raw JSON bytes, byte-level root pinning, a careful bounds-checked DER walk, and a correct severity ordering and convergence rule (I verified both against the dcap-qvl reference the fixtures come from, and re-ran the SVN/QE/module walks by hand against the fixture data — the genuine v4 quote does appraise UpToDate and the gate offsets are right). However, the appraisal is missing a core freshness gate: TCB Info and QE Identity carry issueDate/nextUpdate, but neither struct deserializes them, so only the certificate windows are checked. Intel's QVL rejects expired collateral documents; without that, a host can replay year-old Intel-signed collateral and keep a platform Intel has since downgraded appraised as UpToDate (the signer certs stay valid far longer than the documents). The verify.rs module doc's claim that freshness stays enforced in production is currently only true for CRLs and certs. Second, the TDX module identity check verifies MRSIGNERSEAM but omits the masked seam_attributes comparison the reference performs (this collateral pins attributes 00...00 under mask FF...FF, so a TD report with module attributes set should be rejected). Third, the module appraisal is skipped entirely when tee_tcb_svn[1] == 0 or tdxModuleIdentities is empty, where the reference falls back to the base tdxModule entry and still checks mrsigner — a fail-open in the identity check. All three are fixable within the current fixtures (the injected test clocks already sit inside both document windows).
rust/crates/aleph-tee/src/tdx/tcb.rs (line 226): Neither TcbInfo nor QeIdentity deserializes issueDate/nextUpdate, so the signed documents' own validity windows are never checked — only the signer-certificate windows in verify_signer_chain. Intel's QVL treats expired TCB Info/QE Identity as an error, and the reference implementation this PR's fixtures come from folds these windows into a collateral time window. Without this, a host can replay stale Intel-signed collateral indefinitely: a platform Intel has since downgraded (e.g. via a new advisory) continues to appraise as UpToDate, since the signer certs stay valid for years. The same gap applies to verify_qe_identity. The fixture clocks (2025-06-20 and 2026-02-19) are inside both windows, so adding the check won't break the tests — and it would let the doc claims in verify.rs ("freshness stays enforced in production") actually hold.
rust/crates/aleph-tee/src/tdx/tcb.rs (line 323): The module identity appraisal checks MRSIGNERSEAM but not seam_attributes. The reference also compares the TD report's SEAMATTRIBUTES against the identity's attributes under attributesMask, and this collateral pins attributes 00...00 under mask FF...FF — i.e. the module must run with no attributes set (notably the module's own DEBUG bit). A TD report with seam attributes set currently passes appraisal here but is rejected by the reference. The fields are already in the fixture collateral; it's a cheap addition to the same function.
rust/crates/aleph-tee/src/tdx/tcb.rs (line 312): This early return skips the module identity check entirely when tee_tcb_svn[1] == 0 or tdxModuleIdentities is empty. The reference instead falls back to the base tdxModule entry (and errors when it is absent), still enforcing the mrsigner/attributes comparison. As written this is fail-open: an identity check that silently disappears is worse than one that errors. Recommend mirroring the fallback, or at minimum treating a v3 TDX TCB Info with no identities as a rejection.
rust/crates/aleph-tee/src/tdx/tcb.rs (line 262): The walk takes the first satisfied level in JSON order. The reference deliberately does not rely on document order — it canonicalizes the levels (SGX components, then pcesvn, then TDX components) to mirror the QVL's sorted container. Intel publishes levels highest-first so this works in practice, but if a signed document ever shipped levels unordered, the walk could return a lower level's status (e.g. accept UpToDate when a higher satisfied level says OutOfDate). Sorting like the reference removes the assumption.
rust/crates/aleph-tee/src/tdx/certs.rs (line 198): Minor: verify_signer_chain checks the windows of the intermediate and signer but not of the pinned root, while verify_pck_chain does check the root's window. Since it's the same pinned cert, the two paths will disagree after 2049 — probably worth being consistent (either check it in both or neither).
rust/crates/aleph-tee/src/tdx/mod.rs (line 2): The module doc still says the TCB walk and platform gates "arrive in later increments" — this PR adds both, so the paragraph is now stale.
A first content byte with the sign bit set (and no leading 0x00) is a two's-complement negative, which parsed as a large positive would inflate the SVN in the walk. Reject it; documents the non-negative assumption. Low risk given the Intel-signed source, but one line.
Addresses the review's blocking finding plus two module-identity gaps: - The TCB Info and QE Identity documents carry issueDate/nextUpdate but neither was checked against now, only the signer certificates were. Since each collateral field is independently signed and the signers outlive the documents by years, a host could replay stale genuinely- signed collateral with lower SVN thresholds and keep an unpatched platform appraising as current. Deserialize both dates and reject a document whose window does not contain now. A small self-contained RFC3339 parser rather than a date crate, to avoid widening the measured agent's dependency tree. - The TDX module identity check now also compares SEAMATTRIBUTES against the identity under its mask (the reference does; the fixtures pin attributes 0 under mask FF), and falls back to the base tdxModule entry when no per-version identity applies instead of skipping the MRSIGNERSEAM/attributes gate entirely, which was fail-open. - Extract check_policy() so the Revoked-vs-policy and denied-advisory paths (both stated security properties) are unit-testable; soften the Revoked doc to say the rejection is enforced in evaluate_tcb, not by the set membership. New tests: expired TCB Info, the date parser, Revoked rejected even when inserted into the accept set, denied advisory, and the outdated walk now asserts its 'below every level' cause.
… doc verify_signer_chain now checks the pinned root's validity window like verify_pck_chain does, so the two paths agree (they share the pinned cert, valid to 2049). The tdx module doc no longer says the TCB walk and platform gates arrive later; this stack adds them.
|
Thanks, both rounds were sharp. Nine findings taken across four commits, one declined with rationale. Blocking — collateral freshness (the big one): TCB Info and QE Identity now have their Module identity (both R2 gaps): the check now also compares SEAMATTRIBUTES against the identity under Policy paths tested: extracted Smaller ones: negative DER integer rejected in Declined — sorting the TCB levels (R2, line 262): I'd leave this. First, the reference doesn't actually sort — dcap-qvl iterates Golden re-seeded (all changes are in the measured aleph-tee). Gates green: 44 tdx tests, fmt, clippy |
foxpatch-aleph
left a comment
There was a problem hiding this comment.
Solid, consistently fail-closed implementation of the DCAP TCB appraisal. The SVN walk, Intel convergence rule, module-identity and QE-identity gates, and the Revoked-is-never-acceptable invariant all match the DCAP reference semantics; the DER walker is bounds-checked with correct Intel OIDs; and the serde field mappings (including the advisory_i_ds -> advisoryIDs rename) check out against the real vendored collateral. The compose-and-gate API in verify.rs never returns registers for a rejected quote, and the doc explicitly scopes what a verification does not establish. Findings are documentation and test-coverage nits only.
rust/crates/aleph-tee/src/tdx/tcb.rs (line 76): This doc comment references "the SNP override, which admits any concrete named TCB", but no such override exists anywhere in this crate (a grep for it finds only this line). Either point at where the SNP override lives (another crate/increment?) or drop the comparison, since right now it is opaque to the reader.
rust/crates/aleph-tee/src/tdx/tcb.rs (line 577): The advisory path is only covered by the unit test on check_policy. The outdated fixture does carry advisories on its module-identity levels (INTEL-SA-01036/01099), but that sample fails inside walk_platform_tcb before any advisories are collected, so the end-to-end flow (collect advisories from real collateral, then block via a denied_advisories entry) is never exercised. Consider an end-to-end test that pairs the v4 quote with a policy denying an advisory that a synthetic or real document actually reports.
rust/crates/aleph-tee/src/tdx/pck_extension.rs (line 70): The length bytes are accumulated from a min()-clamped slice and only validated against buf.len() after the loop (line 73). It is correct, but hoisting the "truncated DER length" check above the loop would make the intent clearer and drop the saturating_sub clamp. Also worth a note that this walker accepts non-DER-canonical encodings — harmless here since the input is a signature-verified certificate.
rust/crates/aleph-tee/src/tdx/tcb.rs (line 286): parse_rfc3339_z validates month/day ranges but not hour/minute/second (or day-vs-month length), so e.g. hour 99 or Feb 30 parse to a shifted timestamp. Harmless for Intel-signed input, but a one-line range check would make the parser's guarantees obvious without relying on that.
Increment 4 of the Intel TDX design (#1144), stacked on #1194 (chain verification). Completes
aleph_tee::tdxquote verification: the TCB appraisal, platform gates, and acceptance policy on top of the chain half. Additive Rust plus a golden re-seed.Base: this PR targets
od/tdx-dcap-chain(#1194); review/merge #1194 first. The diff here is increment 4 alone.tdx/pck_extension.rs— parses the platform identity from the PCK leaf's Intel SGX extension (OID1.2.840.113741.1.13.1): FMSPC, the 16 SGX component SVNs (CPUSVN), and PCESVN. A minimal, fully bounds-checked DER walk keyed on the exact OIDs, not a general ASN.1 parser, since the structure is fixed and the input untrusted.tdx/tcb.rs— the appraisal:TcbStatus(7 variants, severity-ordered) with Intel's convergence rule;TdxTcbPolicy(accepted statuses + denied advisories) defaulting to acceptUpToDateandSWHardeningNeededonly.Revokedis never acceptable and cannot be added to the accept set — the deliberate asymmetry with the SNP override.verify_tcb_info/verify_qe_identity: Intel's signed documents verified against their issuer chains up to the pinned root (increment 3'sverify_signer_chain), signatures over the exact raw JSON bytes so a re-serialization can't slip a changed field past the check.TD_ATTRIBUTES.DEBUG == 0(a debuggable TD lets the host read guest memory — the single most important gate).tdx/verify.rs—verify_tdx_quote(quote, collateral, now, policy) -> TdxVerification, composing the chain half withevaluate_tcb. The result carries the pinned registers andreport_data, and its doc spells out that a success is still not a decision to trust a guest: the caller pins registers against the message'sLaunchMeasurementand binds freshness throughreport_data.Fixtures: reuses the Phala collateral pair (v4) and the outdated pair from #1194. Worth recording: the outdated sample's PCK reports a component SVN below every TCB level, so its walk finds no match (a rejection), which matches the DCAP reference's own behaviour on that sample — it's a "collateral/platform too old" negative, not a clean OutOfDate. Precise status transitions (fall-through to a lower level, the convergence rule) are covered by unit tests on the walk logic with synthetic TCB Info, independent of the fixtures.
Tests (18 new, 40 in the tdx module): genuine v4 →
UpToDateend to end; FMSPC mismatch (v4 quote against the other FMSPC's collateral); forged TCB Info body (signature no longer covers it); debuggable-TD gate; synthetic walk fall-through to a lower status; convergence rule; the fullverify_tdx_quoteaccepting a genuine quote and refusing to return registers for a rejected one; plus the PCK-extension parse pinned to exact FMSPC/CPUSVN/PCESVN values.Golden measurements: re-seeded (aleph-tee compiles into the measured attest agent).
🤖 Generated with Claude Code
https://claude.ai/code/session_01DUWEzYLgSidwttzQGtzB43