Skip to content

fix: Extend max collateral age (port #3941 to main) - #3944

Closed
barakeinav1 wants to merge 2 commits into
mainfrom
fix/extend-max-collateral-age-main
Closed

fix: Extend max collateral age (port #3941 to main)#3944
barakeinav1 wants to merge 2 commits into
mainfrom
fix/extend-max-collateral-age-main

Conversation

@barakeinav1

@barakeinav1 barakeinav1 commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Cherry-picks the one-line fix from #3941 (merged to release-v3.13) onto main.

Why

main-line nodes (async TEE verifier, #3714) currently can't generate a TEE attestation: MAX_COLLATERAL_AGE in crates/tee-authority/src/tee_authority.rs is 7 days, tighter than Intel's ~30-day PCK CRL re-issuance cadence. Intel's current CRL (thisUpdate = 2026-07-16, nextUpdate = 2026-08-15) is >7 days old, so quote generation fails fleet-wide — confirmed identical from Phala, Intel's PCS, and a local sgx-dcap-pccs.

#3941 fixed this on release-v3.13 (7d → 31d) but never landed on main. This ports the exact commit.

Closes #3943.

Copilot AI review requested due to automatic review settings July 23, 2026 14:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Ports the release-v3.13 fix to main by extending the PCCS collateral freshness bound so TEE quote generation doesn’t fail when Intel’s PCK CRL cadence exceeds the previous 7-day limit.

Changes:

  • Increase MAX_COLLATERAL_AGE from 7 days to 31 days to align with Intel’s ~30-day collateral window.
  • Update tests/comments to reference MAX_COLLATERAL_AGE rather than hard-coded 7/8/9-day values.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

/// collateral that share Intel's 30-day window: `tcb_info.issueDate`,
/// `qe_identity.issueDate`, and PCK CRL `thisUpdate`.
const MAX_COLLATERAL_AGE: time::Duration = time::Duration::days(7);
const MAX_COLLATERAL_AGE: time::Duration = time::Duration::days(31);
@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Pull request overview

Ports #3941 from release-v3.13 to main: bumps MAX_COLLATERAL_AGE from 7 to 31 days so PCCS collateral doesn't get rejected as stale between Intel's ~30-day PCK CRL re-issuance cycles, and reparameterizes the corresponding unit tests off the constant so future changes don't silently invalidate the boundary assertions.

Changes:

  • MAX_COLLATERAL_AGE: days(7) → days(31) in crates/tee-authority/src/tee_authority.rs.
  • Tests that used hard-coded 6/8/9 day offsets now derive from MAX_COLLATERAL_AGE ± days(1).
  • Nearby test doc-comments updated to reference [MAX_COLLATERAL_AGE] instead of the literal "7-day window".

Reviewed changes

Per-file summary
File Description
crates/tee-authority/src/tee_authority.rs Const bump 7d → 31d; boundary tests reparameterized on MAX_COLLATERAL_AGE; nearby test doc-comments delinked from the "7-day" number.

Findings

Blocking (must fix before merge):

  • crates/tee-authority/src/tee_authority.rs:227-234 — The doc comment on MAX_COLLATERAL_AGE still describes the old policy and is now actively misleading:

    • Line 227: "Maximum age accepted for PCCS collateral. Hard-coded at 7 days." — the constant is now 31 days.
    • Line 231: "7 days is stricter than Intel's 30-day nextUpdate window..." — 31 days is not stricter than 30 days; it's slightly more permissive, which is exactly the point of the change (allowing collateral to remain acceptable across Intel's ~30-day CRL re-issuance cadence). Leaving the rationale text unchanged inverts its meaning.

    Per CLAUDE.md §"Documentation alignment", doc-comments on the changed item that describe superseded invariants are a review-blocking issue, not a follow-up. Suggested rewrite:

    /// Maximum age accepted for PCCS collateral. Hard-coded at 31 days.
    /// Lives in the node binary (governed via image-hash approval), not
    /// as per-operator configuration.
    ///
    /// Intel re-signs the three collateral pieces on a ~30-day cycle
    /// (`nextUpdate` = `thisUpdate + 30 days`). A tighter node-side
    /// window (previously 7 days) rejected legitimate collateral whenever
    /// Intel's PCS lagged its own cycle by more than a few days, so we
    /// track just past the nominal 30-day cadence to absorb routine PCS
    /// re-issuance jitter. This is the freshness bound on the
    /// collateral's Intel signature and is independent of the contract's
    /// attestation expiry (`DEFAULT_EXPIRATION_DURATION_SECONDS`); the
    /// two windows serve different purposes and are not required to
    /// match.
    ///
    /// Applies uniformly to the three periodically re-signed pieces of
    /// collateral: `tcb_info.issueDate`, `qe_identity.issueDate`, and
    /// PCK CRL `thisUpdate`.
    
  • docs/securing-mpc-with-tee-design-doc.md:461 and :463 — the design doc still states Collateral older than 7 days is rejected and The 7-day value is hard-coded in the node binary.... Same doc-drift class, and this design doc is the operator-facing reference for the freshness policy. Update both bullets to state 31 days (and, if you want, add one sentence pointing at Intel's ~30-day CRL cycle as the reason).

Non-blocking (nits, follow-ups, suggestions):

  • crates/tee-authority/src/tee_authority.rs:1188,1209,1242,1285 — nice reparameterization. One micro-nit: MAX_COLLATERAL_AGE + time::Duration::days(1) reads more naturally than MAX_COLLATERAL_AGE - time::Duration::days(-1)-style expressions elsewhere; consider one shared helper (e.g. just_past_max_age()) if this pattern grows, but not required for this PR.
  • The port itself is a clean cherry-pick — no logic drift versus fix: Extend max collateral age #3941, and the boundary tests (should_accept_at_exact_boundary, should_reject_one_second_past_boundary) already exercise the new value symbolically, so they self-correct with any future change.

⚠️ Issues found

@barakeinav1

Copy link
Copy Markdown
Collaborator Author

superseded by #3945

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port max-collateral-age fix (#3941) to main

3 participants