feat(NitroEnclaveVerifier): durable on-chain certificate revocation [non-urgent, post-Multiproving]#276
Open
leopoldjoy wants to merge 1 commit into
Open
feat(NitroEnclaveVerifier): durable on-chain certificate revocation [non-urgent, post-Multiproving]#276leopoldjoy wants to merge 1 commit into
leopoldjoy wants to merge 1 commit into
Conversation
Defense-in-depth hardening for CHAIN-4194 / Immunefi #75608. Adds a persistent revokedCerts mapping that survives the _cacheNewCert suffix rewrite, so a previously revoked intermediate cannot be silently re-trusted by submitting a new attestation that re-derives the same accumulated path digest. Not urgent. Can ship post-Multiproving launch (Base V2 upgrade) since multiproof is the primary mitigation for any single verifier gap; this PR closes a defense-in-depth hole that requires either CA key compromise (Tier B) or an honest registrar/admin race (Tier A). Bumps semver to 0.4.0 (additive).
Collaborator
🟡 Heimdall Review Status
|
leopoldjoy
added a commit
to base/base
that referenced
this pull request
May 11, 2026
…ation Adds an optional NitroEnclaveVerifier client to the registrar so that each CRL cycle pre-screens parsed cert chains against the on-chain revokedCerts mapping (CHAIN-4194 / Immunefi #75608 hardening) before issuing any revokeCert tx. Backwards compatible: the pre-check fail-opens against deployments that do not yet expose revokedCerts (selector-not-found reverts are caught, logged via warn!, and counted by the new onchain_revocation_check_errors metric). The companion onchain PR (base/contracts#276) is therefore not required to be deployed first or in lockstep — the registrar functions normally against either the existing or upgraded NitroEnclaveVerifier. Production CLI/env compatibility preserved: --crl-nitro-verifier-address and BASE_REGISTRAR_CRL_NITRO_VERIFIER_ADDRESS unchanged. Removes dead BoundlessConfig::nitro_verifier_address field and the unused --nitro-verifier-address flag (was wired to BoundlessArgs but never consumed by BoundlessProver).
github-actions Bot
pushed a commit
to dysnix/base
that referenced
this pull request
May 12, 2026
…ation (backwards compatible) (base#2624) * feat(tee-registrar): pre-check on-chain revokedCerts before CRL revocation Adds an optional NitroEnclaveVerifier client to the registrar so that each CRL cycle pre-screens parsed cert chains against the on-chain revokedCerts mapping (CHAIN-4194 / Immunefi #75608 hardening) before issuing any revokeCert tx. Backwards compatible: the pre-check fail-opens against deployments that do not yet expose revokedCerts (selector-not-found reverts are caught, logged via warn!, and counted by the new onchain_revocation_check_errors metric). The companion onchain PR (base/contracts#276) is therefore not required to be deployed first or in lockstep — the registrar functions normally against either the existing or upgraded NitroEnclaveVerifier. Production CLI/env compatibility preserved: --crl-nitro-verifier-address and BASE_REGISTRAR_CRL_NITRO_VERIFIER_ADDRESS unchanged. Removes dead BoundlessConfig::nitro_verifier_address field and the unused --nitro-verifier-address flag (was wired to BoundlessArgs but never consumed by BoundlessProver). * fix(tee-registrar): fail-fast in RegistrationDriver::new on missing nitro_verifier Addresses PR base#2624 review finding: when crl.enabled is true but nitro_verifier is None, check_and_revoke_crls returned RegistrarError::Config which the caller treated as fail-open — silently disabling both the on-chain pre-check and AWS CRL enforcement. CLI validation prevents this in production today, but a misconfigured RegistrationDriver constructor (tests, alternate binaries, future refactors splitting CLI parsing from driver wiring) could reach the silent-bypass state. Reject the misconfiguration at construction so the invariant cannot be violated downstream; the runtime ok_or_else becomes an expect documenting the contract. * style: cargo +nightly fmt * fix(tee-registrar): tighten CRL setup invariants and de-duplicate verifier error context Addresses two PR base#2624 review findings on commit 1d6c3b2: - verifier.rs: NitroVerifierClient::is_revoked previously stored the full ContractError display in the NitroVerifierCall context field while also passing the same error as #[source]. Error chain renders the inner message twice. Replace with a structured call label ("revokedCerts(0x...)") matching the variant docstring example, letting #[source] carry the underlying error verbatim — no duplication. Drops the standalone map_contract_error helper. - driver.rs: RegistrationDriver::new now also fails fast when build_crl_http_client returns an error with crl.enabled=true. The prior warn!+None branch silently degraded Layer 2 (AWS CRL fetch) for the entire process lifetime while leaving Layer 1 active — exactly the silent-bypass class the previous fail-fast for nitro_verifier closed. The runtime ok_or_else in check_and_revoke_crls becomes an expect mirroring the nitro_verifier invariant pattern. * style(tee-registrar): trim verbose inline comments Remove design-rationale and justification commentary added in this PR that duplicated function-level docs, the PR description, or AGENTS.md conventions. No logic, test coverage, or public API surface changes. Net: ~150 fewer lines. * style: cargo +nightly fmt
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
2 participants
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.
Priority
Not urgent. This change can ship after the Multiproving launch (Base V2 upgrade). Multiproving is the primary mitigation for any single-verifier gap; this PR closes a defense-in-depth hole that would otherwise require either a CA key compromise (Tier B) or an honest-but-racing registrar/admin scenario (Tier A) to exploit. There is no known active exploitation path against the current production configuration.
Summary
revokedCertsmapping that survives the_cacheNewCertsuffix rewrite, so a previously revoked intermediate cert path cannot be silently re-trusted by submitting a new attestation that re-derives the same accumulated path digest.unrevokeCert(bytes32) onlyOwnerfor governed re-trust (two-step: unrevoke, then re-cache via the next successfulverify)._cacheNewCert(Pass 1) and_verifyJournal(Pass 2) when a sentinel-keyed digest is encountered, plus an early short-circuit incheckTrustedIntermediateCerts.0.3.0→0.4.0(additive — no storage layout changes that move existing slots; only a new mapping appended).Background — Immunefi #75608 / CHAIN-4194
_cacheNewCertpreviously rewrote suffix entries unconditionally. If the prefix length is1(the production registrar default), revoking an intermediate viarevokeCertonly zeroed the cached entry — a subsequent attestation re-deriving the same accumulated path digest would re-cache and re-trust it. The newrevokedCerts[digest] = truesentinel is checked on every code path that would otherwise mark a cert as trusted, making revocation durable across attestations.Triage: Low. Tier B requires CA key compromise to forge a new chain that hashes back to the revoked digest; Tier A requires a registrar honest-mistake / admin race. Both are mitigated in production today by operational controls and (post-launch) by Multiproving. This PR is hardening, not an incident response.
Changes
src/L1/proofs/tee/NitroEnclaveVerifier.solmapping(bytes32 => bool) public revokedCerts(appended, layout-safe).revokeCert: sets sentinel + zeroes any existing cached entry, emitsCertRevoked.unrevokeCert: clears sentinel only, emitsCertUnrevoked(re-trust requires a freshverifycall to re-cache)._cacheNewCert: skips writing trusted suffix entries whose accumulated digest is sentineled._verifyJournal: rejects withCertificateRevoked(digest)when traversal encounters a sentineled digest.checkTrustedIntermediateCerts: short-circuits tofalseif any digest in the chain is sentineled.interfaces/L1/proofs/tee/INitroEnclaveVerifier.sol: declaresrevokedCerts,unrevokeCert,CertUnrevokedevent,CertificateNotRevokederror.test/L1/proofs/NitroEnclaveVerifier.t.sol: 8 new tests covering revoke→re-attest rejection, unrevoke flow, sentinel collision behaviour, owner gating, and event emission. All 81/81 NitroEnclaveVerifier and 366/366 L1 proofs tests pass.NitroEnclaveVerifieronly.Compatibility
revokedCertsis not present, so the offchain change can be deployed in either order relative to this onchain upgrade.Out of Scope / Notes
snapshots/abi/AggregateVerifier.json; intentionally not included in this PR.