Skip to content

PCR revocation security: lazy enforcement, type-bound PCRs#34

Merged
adambalogh merged 33 commits intomainfrom
kha/pcr-revocation-security-fix
Mar 10, 2026
Merged

PCR revocation security: lazy enforcement, type-bound PCRs#34
adambalogh merged 33 commits intomainfrom
kha/pcr-revocation-security-fix

Conversation

@khalifaT
Copy link
Collaborator

@khalifaT khalifaT commented Mar 9, 2026

Summary

  • PCRs are keyed by TEE type: approvedPCRs changed from mapping(bytes32 => ApprovedPCR) to mapping(uint8 => mapping(bytes32 => ApprovedPCR)). The same PCR hash can be approved independently per TEE type.
  • Separated approve and revoke: approvePCR(pcrs, version, teeType) only approves; revokePCR(pcrHash, teeType, gracePeriod) only revokes (immediate or with grace period). The old previousPcrHash rotation pattern is removed.
  • Lazy PCR enforcement: revokePCR flips a flag or sets an expiry — no gas bomb from iterating all active TEEs. Revoked PCRs are caught at activateTEE() and heartbeat() checkpoints via _requirePCRValidForTEE.
  • Active TEE lists are per-type: _activeTEEList and _activeTEEIndex are now mapping(uint8 => ...), so queries like getActivatedTEEs(teeType) only return TEEs of that type.
  • New getLiveTEEs(teeType) query: returns TEEs that are activated, have a valid PCR, and a fresh heartbeat — fully verified on-chain.
  • New removeTEE(teeId): permanently deletes a TEE from all storage (indexes + mapping), for cleaning up decommissioned TEEs.
  • onlyTEEOwnerOrAdmin modifier: extracted shared access control for deactivateTEE, activateTEE, and removeTEE.
  • Re-approval support: revoked or expired PCRs can be re-approved; PCRAlreadyExists error added to prevent silent overwrites of active PCRs.
  • Removed convenience getters: getPublicKey, getTLSCertificate, isActive, getPaymentAddress removed — callers use getTEE() instead.
  • Removed computeMessageHash: unused utility removed from contract.

Design Decisions

Why lazy enforcement over automatic deactivation?

  • revokePCR must always be callable, even with thousands of active TEEs
  • A revoked TEE stays in _activeTEEList until its next heartbeat fails, but this is a stale index entry — not a security hole
  • Every meaningful checkpoint (registration, reactivation, heartbeat) validates both PCR status and type match via _requirePCRValidForTEE

Revocation modes:

  • revokePCR(hash, teeType, 0) — immediate: PCR is instantly invalid, TEEs fail on next heartbeat
  • revokePCR(hash, teeType, N) — grace period: TEEs keep running for N seconds, then caught lazily

Per-type active lists:

  • Queries no longer need to filter by type client-side
  • getActivatedTEEs(teeType) is O(1) storage read instead of O(n) filter

Files Changed

File Change
TEERegistry.sol Core contract: type-keyed PCRs, per-type active lists, lazy enforcement, removeTEE, getLiveTEEs
lifecycle.js New/expanded test suite for lifecycle, removal, and revocation scenarios
registry.js Unit tests updated for new signatures and behavior
settlementRelay.js Test updated for new function signatures
TEETestHelper.sol Test helper wrappers updated
local_tee_workflow.go Integration tests updated
client.go CLI client updated for new ABI
pcr.go / tee.go CLI commands updated (--tee-type on approve, --grace-period on revoke)
TEERegistry.json ABI artifact regenerated
README.md Example code updated

Test Plan

  • Unit tests pass (cd tests/solidity/suites/tee && truffle test)
  • Integration tests pass with live enclave
  • Verify PCRTypeMismatch reverts when registering TEE with wrong type
  • Verify grace period expiry is enforced at heartbeat
  • Verify re-approval of revoked PCR works
  • Verify removeTEE cleans up all indexes

@adambalogh adambalogh marked this pull request as ready for review March 9, 2026 17:35
@adambalogh adambalogh requested a review from Copilot March 9, 2026 17:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Implements defense-in-depth for PCR revocation in the TEERegistry Solidity contract, adding duplicate PCR prevention, enhanced PCR approval metadata (previous hash + grace period), and multi-checkpoint PCR enforcement (registration, activation, heartbeat), with corresponding test and integration workflow updates.

Changes:

  • Extend approvePCR to include previousPcrHash + gracePeriod, and emit an enhanced PCRApproved event.
  • Add immediate PCR revocation handling (revokePCR) with batch deactivation support and lazy enforcement in activateTEE/heartbeat.
  • Add/extend tests and integration script coverage for revocation, grace periods, and duplicate PCR prevention.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File Description
contracts/solidity/TEERegistry.sol Adds PCR duplicate prevention, enhanced PCR approval metadata, PCR revocation with batch deactivation, and additional PCR checks during activation/heartbeat.
tests/solidity/suites/tee/test/registry.js Adds a new test suite covering duplicate PCR prevention, PCR revocation, grace period approval/event fields, and admin-only batch deactivation.
scripts/integration/local_tee_workflow.go Updates integration workflow (RPC URL, bytecode) and adds a new section + helper calls to exercise PCR revocation/grace/duplicate behavior.

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

@adambalogh adambalogh added the run-ci Run ETE Testing Suite label Mar 9, 2026
@adambalogh adambalogh requested a review from Copilot March 9, 2026 20:44
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.


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

adambalogh and others added 3 commits March 9, 2026 16:49
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ient/og-evm into kha/pcr-revocation-security-fix
@adambalogh adambalogh changed the title update revocation process PCR revocation security: lazy enforcement, type-bound PCRs Mar 9, 2026
@adambalogh adambalogh added run-ci Run ETE Testing Suite and removed run-ci Run ETE Testing Suite labels Mar 9, 2026
@adambalogh adambalogh added run-ci Run ETE Testing Suite and removed run-ci Run ETE Testing Suite labels Mar 9, 2026
@adambalogh adambalogh requested a review from Copilot March 9, 2026 21:29
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.


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

@adambalogh adambalogh added run-ci Run ETE Testing Suite and removed run-ci Run ETE Testing Suite labels Mar 10, 2026
@adambalogh adambalogh added run-ci Run ETE Testing Suite and removed run-ci Run ETE Testing Suite labels Mar 10, 2026
@adambalogh adambalogh added run-ci Run ETE Testing Suite and removed run-ci Run ETE Testing Suite labels Mar 10, 2026
balogh.adam@icloud.com added 3 commits March 9, 2026 21:01
@adambalogh adambalogh added run-ci Run ETE Testing Suite and removed run-ci Run ETE Testing Suite labels Mar 10, 2026
balogh.adam@icloud.com added 2 commits March 9, 2026 21:07
@adambalogh adambalogh merged commit 758e149 into main Mar 10, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci Run ETE Testing Suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants