diff --git a/interfaces/multiproof/tee/INitroEnclaveVerifier.sol b/interfaces/multiproof/tee/INitroEnclaveVerifier.sol index 1ab1c830..2d7de2dd 100644 --- a/interfaces/multiproof/tee/INitroEnclaveVerifier.sol +++ b/interfaces/multiproof/tee/INitroEnclaveVerifier.sol @@ -160,6 +160,12 @@ interface INitroEnclaveVerifier { */ function proofSubmitter() external view returns (address); + /** + * @dev Returns the address authorized to revoke intermediate certificates + * @return Address of the revoker (address(0) if disabled) + */ + function revoker() external view returns (address); + /** * @dev Retrieves the configuration for a specific coprocessor * @param _zkCoProcessor Type of ZK coprocessor (RiscZero or Succinct) @@ -227,6 +233,15 @@ interface INitroEnclaveVerifier { */ function setProofSubmitter(address _proofSubmitter) external; + /** + * @dev Updates the revoker address + * @param _newRevoker New revoker address (can be address(0) to disable the revoker role) + * + * Requirements: + * - Only callable by contract owner + */ + function setRevoker(address _newRevoker) external; + /** * @dev Configures the zero-knowledge verification parameters for a specific coprocessor * @param _zkCoProcessor Type of ZK coprocessor (RiscZero or Succinct) @@ -249,7 +264,7 @@ interface INitroEnclaveVerifier { * @param _certHash Hash of the certificate to revoke * * Requirements: - * - Only callable by contract owner + * - Only callable by contract owner or revoker * - Certificate must exist in the trusted set */ function revokeCert(bytes32 _certHash) external; diff --git a/scripts/multiproof/DeployRiscZeroStack.s.sol b/scripts/multiproof/DeployRiscZeroStack.s.sol index cb49ed3d..51d643c3 100644 --- a/scripts/multiproof/DeployRiscZeroStack.s.sol +++ b/scripts/multiproof/DeployRiscZeroStack.s.sol @@ -131,6 +131,7 @@ contract DeployRiscZeroStack is Script { // Use owner as placeholder proofSubmitter; must be updated to TEEProverRegistry // address after deployment via setProofSubmitter(). + // Revoker starts as address(0); must be set via setRevoker() after deployment. NitroEnclaveVerifier nev = new NitroEnclaveVerifier( owner, NITRO_MAX_TIME_DIFF, @@ -138,6 +139,7 @@ contract DeployRiscZeroStack is Script { trustedCertExpiries, nitroRootCert, owner, + address(0), ZkCoProcessorType.RiscZero, zkConfig, bytes32(0) diff --git a/snapshots/abi/NitroEnclaveVerifier.json b/snapshots/abi/NitroEnclaveVerifier.json index b9cd223e..fb95a558 100644 --- a/snapshots/abi/NitroEnclaveVerifier.json +++ b/snapshots/abi/NitroEnclaveVerifier.json @@ -31,6 +31,11 @@ "name": "initialProofSubmitter", "type": "address" }, + { + "internalType": "address", + "name": "initialRevoker", + "type": "address" + }, { "internalType": "enum ZkCoProcessorType", "name": "zkCoProcessor", @@ -416,6 +421,19 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [], + "name": "revoker", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "rootCert", @@ -455,6 +473,19 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "newRevoker", + "type": "address" + } + ], + "name": "setRevoker", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [ { @@ -881,6 +912,19 @@ "name": "ProofSubmitterChanged", "type": "event" }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "newRevoker", + "type": "address" + } + ], + "name": "RevokerUpdated", + "type": "event" + }, { "anonymous": false, "inputs": [ @@ -1010,6 +1054,11 @@ "name": "AlreadyInitialized", "type": "error" }, + { + "inputs": [], + "name": "CallerNotOwnerOrRevoker", + "type": "error" + }, { "inputs": [], "name": "CallerNotProofSubmitter", diff --git a/snapshots/semver-lock.json b/snapshots/semver-lock.json index 8e9d2ae9..9f9e0383 100644 --- a/snapshots/semver-lock.json +++ b/snapshots/semver-lock.json @@ -236,8 +236,8 @@ "sourceCodeHash": "0x032f11e23c188b939ba6cbdc446271b0caad2f4da00535b164a3489291162762" }, "src/multiproof/tee/NitroEnclaveVerifier.sol:NitroEnclaveVerifier": { - "initCodeHash": "0xac14e4b9130c476ba4ab77ecf70b36153921a9fe6ef3ab96c825aba8d9d18473", - "sourceCodeHash": "0x03614e89919c06e56e91b217702345edf74755c12c082475c4c742f45c201415" + "initCodeHash": "0xe94739c3fe1d693ea3022804cea2a0d063ed954fbdbbf1d60d79090e8edbdc64", + "sourceCodeHash": "0x9f9ac5cdff6e537cf1ac70700e060a4c984c17aa3bd50e2f44cee4448dda1a88" }, "src/multiproof/tee/TEEProverRegistry.sol:TEEProverRegistry": { "initCodeHash": "0xfaec22f48eeef0f4ee0575b0e91028a1330fbf6faf52c2fce0f68b500a023970", diff --git a/snapshots/storageLayout/NitroEnclaveVerifier.json b/snapshots/storageLayout/NitroEnclaveVerifier.json index 3b3596fa..fcf6a6ec 100644 --- a/snapshots/storageLayout/NitroEnclaveVerifier.json +++ b/snapshots/storageLayout/NitroEnclaveVerifier.json @@ -6,46 +6,53 @@ "slot": "0", "type": "address" }, + { + "bytes": "20", + "label": "revoker", + "offset": 0, + "slot": "1", + "type": "address" + }, { "bytes": "32", "label": "zkConfig", "offset": 0, - "slot": "1", + "slot": "2", "type": "mapping(enum ZkCoProcessorType => struct ZkCoProcessorConfig)" }, { "bytes": "32", "label": "trustedIntermediateCerts", "offset": 0, - "slot": "2", + "slot": "3", "type": "mapping(bytes32 => uint64)" }, { "bytes": "8", "label": "maxTimeDiff", "offset": 0, - "slot": "3", + "slot": "4", "type": "uint64" }, { "bytes": "32", "label": "rootCert", "offset": 0, - "slot": "4", + "slot": "5", "type": "bytes32" }, { "bytes": "32", "label": "_zkVerifierRoutes", "offset": 0, - "slot": "5", + "slot": "6", "type": "mapping(enum ZkCoProcessorType => mapping(bytes4 => address))" }, { "bytes": "32", "label": "_verifierProofIds", "offset": 0, - "slot": "6", + "slot": "7", "type": "mapping(enum ZkCoProcessorType => bytes32)" } ] \ No newline at end of file diff --git a/src/multiproof/tee/NitroEnclaveVerifier.sol b/src/multiproof/tee/NitroEnclaveVerifier.sol index aaba1b8f..0a624380 100644 --- a/src/multiproof/tee/NitroEnclaveVerifier.sol +++ b/src/multiproof/tee/NitroEnclaveVerifier.sol @@ -49,6 +49,9 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier, ISemver { /// @dev Address that can submit proofs address public proofSubmitter; + /// @dev Address authorized to revoke intermediate certificates (in addition to owner) + address public revoker; + /// @dev Configuration mapping for each supported ZK coprocessor type mapping(ZkCoProcessorType => ZkCoProcessorConfig) public zkConfig; @@ -108,6 +111,9 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier, ISemver { /// @dev Thrown when a zero address is provided for the verifier error InvalidVerifierAddress(); + /// @dev Thrown when caller is neither the owner nor the revoker + error CallerNotOwnerOrRevoker(); + // ============ Events ============ /// @dev Emitted when a new verifier program ID is added/updated @@ -143,9 +149,18 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier, ISemver { /// @dev Event emitted when the maximum time difference is updated event MaxTimeDiffUpdated(uint64 newMaxTimeDiff); + /// @dev Event emitted when the revoker address is updated + event RevokerUpdated(address indexed newRevoker); + /// @dev Thrown when initializeTrustedCerts and initializeTrustedCertExpiries have different lengths error CertExpiriesLengthMismatch(uint256 certsLen, uint256 expiriesLen); + /// @dev Restricts access to the owner or the revoker + modifier onlyOwnerOrRevoker() { + if (msg.sender != owner() && msg.sender != revoker) revert CallerNotOwnerOrRevoker(); + _; + } + /** * @dev Initializes the contract with owner, time tolerance and initial trusted certificates * @param owner Address to be set as the contract owner @@ -154,6 +169,7 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier, ISemver { * @param initializeTrustedCertExpiries Array of notAfter timestamps (seconds) for each initial cert * @param initialRootCert Hash of the AWS Nitro Enclave root certificate * @param initialProofSubmitter Address that is authorized to submit proofs + * @param initialRevoker Address authorized to revoke intermediate certificates (can be address(0) to disable) * @param zkCoProcessor Type of ZK coprocessor to configure (RiscZero or Succinct) * @param config Configuration parameters for the ZK coprocessor * @param verifierProofId The verifierProofId corresponding to the verifierId in config @@ -165,6 +181,7 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier, ISemver { uint64[] memory initializeTrustedCertExpiries, bytes32 initialRootCert, address initialProofSubmitter, + address initialRevoker, ZkCoProcessorType zkCoProcessor, ZkCoProcessorConfig memory config, bytes32 verifierProofId @@ -180,6 +197,7 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier, ISemver { _initializeOwner(owner); _setRootCert(initialRootCert); _setProofSubmitter(initialProofSubmitter); + revoker = initialRevoker; _setZkConfiguration(zkCoProcessor, config, verifierProofId); } @@ -321,13 +339,13 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier, ISemver { * @param certHash Hash of the certificate to revoke * * Requirements: - * - Only callable by contract owner + * - Only callable by contract owner or revoker * - Certificate must exist in the trusted intermediate certificates set * - * This function allows the owner to revoke compromised intermediate certificates + * This function allows the owner or revoker to revoke compromised intermediate certificates * without affecting the root certificate or other trusted certificates. */ - function revokeCert(bytes32 certHash) external onlyOwner { + function revokeCert(bytes32 certHash) external onlyOwnerOrRevoker { if (trustedIntermediateCerts[certHash] == 0) { revert CertificateNotFound(certHash); } @@ -424,6 +442,18 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier, ISemver { _setProofSubmitter(submitter); } + /** + * @dev Updates the revoker address + * @param newRevoker New revoker address (can be address(0) to disable the revoker role) + * + * Requirements: + * - Only callable by contract owner + */ + function setRevoker(address newRevoker) external onlyOwner { + revoker = newRevoker; + emit RevokerUpdated(newRevoker); + } + // ============ Verification Functions ============ /** @@ -662,8 +692,8 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier, ISemver { } /// @notice Semantic version. - /// @custom:semver 0.2.0 + /// @custom:semver 0.3.0 function version() public pure virtual returns (string memory) { - return "0.2.0"; + return "0.3.0"; } } diff --git a/test/multiproof/NitroEnclaveVerifier.t.sol b/test/multiproof/NitroEnclaveVerifier.t.sol index 94857bd8..9088a851 100644 --- a/test/multiproof/NitroEnclaveVerifier.t.sol +++ b/test/multiproof/NitroEnclaveVerifier.t.sol @@ -19,6 +19,7 @@ contract NitroEnclaveVerifierTest is Test { address public owner; address public submitter; + address public revokerAddr; address public mockRiscZeroVerifier; address public mockSP1Verifier; @@ -44,6 +45,7 @@ contract NitroEnclaveVerifierTest is Test { owner = address(this); submitter = makeAddr("submitter"); + revokerAddr = makeAddr("revoker"); mockRiscZeroVerifier = makeAddr("mock-riscZero-verifier"); mockSP1Verifier = makeAddr("mock-sp1-verifier"); @@ -63,6 +65,7 @@ contract NitroEnclaveVerifierTest is Test { trustedCertExpiries, ROOT_CERT, submitter, + revokerAddr, ZkCoProcessorType.Succinct, zkCfg, VERIFIER_PROOF_ID @@ -91,7 +94,7 @@ contract NitroEnclaveVerifierTest is Test { ZkCoProcessorConfig({ verifierId: bytes32(0), aggregatorId: bytes32(0), zkVerifier: address(0) }); vm.expectRevert(NitroEnclaveVerifier.ZeroMaxTimeDiff.selector); new NitroEnclaveVerifier( - owner, 0, certs, expiries, bytes32(0), submitter, ZkCoProcessorType.Succinct, zkCfg, bytes32(0) + owner, 0, certs, expiries, bytes32(0), submitter, address(0), ZkCoProcessorType.Succinct, zkCfg, bytes32(0) ); } @@ -103,7 +106,16 @@ contract NitroEnclaveVerifierTest is Test { ZkCoProcessorConfig({ verifierId: bytes32(0), aggregatorId: bytes32(0), zkVerifier: address(0) }); vm.expectRevert(abi.encodeWithSelector(NitroEnclaveVerifier.CertExpiriesLengthMismatch.selector, 1, 0)); new NitroEnclaveVerifier( - owner, MAX_TIME_DIFF, certs, expiries, bytes32(0), submitter, ZkCoProcessorType.Succinct, zkCfg, bytes32(0) + owner, + MAX_TIME_DIFF, + certs, + expiries, + bytes32(0), + submitter, + address(0), + ZkCoProcessorType.Succinct, + zkCfg, + bytes32(0) ); } @@ -200,12 +212,86 @@ contract NitroEnclaveVerifierTest is Test { verifier.revokeCert(unknown); } - function testRevokeCertRevertsIfNotOwner() public { + function testRevokeCertRevertsIfNotOwnerOrRevoker() public { vm.prank(submitter); - vm.expectRevert(); + vm.expectRevert(NitroEnclaveVerifier.CallerNotOwnerOrRevoker.selector); + verifier.revokeCert(INTERMEDIATE_CERT_1); + } + + // ============ Revoker Role Tests ============ + + function testConstructorSetsRevoker() public view { + assertEq(verifier.revoker(), revokerAddr); + } + + function testConstructorAcceptsZeroRevoker() public { + bytes32[] memory certs = new bytes32[](0); + uint64[] memory expiries = new uint64[](0); + ZkCoProcessorConfig memory zkCfg = + ZkCoProcessorConfig({ verifierId: VERIFIER_ID, aggregatorId: AGGREGATOR_ID, zkVerifier: mockSP1Verifier }); + NitroEnclaveVerifier v = new NitroEnclaveVerifier( + owner, + MAX_TIME_DIFF, + certs, + expiries, + ROOT_CERT, + submitter, + address(0), + ZkCoProcessorType.Succinct, + zkCfg, + VERIFIER_PROOF_ID + ); + assertEq(v.revoker(), address(0)); + } + + function testRevokerCanRevokeCert() public { + assertGt(verifier.trustedIntermediateCerts(INTERMEDIATE_CERT_1), 0); + vm.prank(revokerAddr); + verifier.revokeCert(INTERMEDIATE_CERT_1); + assertEq(verifier.trustedIntermediateCerts(INTERMEDIATE_CERT_1), 0); + } + + function testOwnerCanStillRevokeCert() public { + assertGt(verifier.trustedIntermediateCerts(INTERMEDIATE_CERT_1), 0); + verifier.revokeCert(INTERMEDIATE_CERT_1); + assertEq(verifier.trustedIntermediateCerts(INTERMEDIATE_CERT_1), 0); + } + + function testSetRevoker() public { + address newRevoker = makeAddr("new-revoker"); + verifier.setRevoker(newRevoker); + assertEq(verifier.revoker(), newRevoker); + } + + function testSetRevokerToZeroDisablesRole() public { + verifier.setRevoker(address(0)); + assertEq(verifier.revoker(), address(0)); + + // Now revoker (zero address) can't call revokeCert + vm.prank(revokerAddr); + vm.expectRevert(NitroEnclaveVerifier.CallerNotOwnerOrRevoker.selector); verifier.revokeCert(INTERMEDIATE_CERT_1); } + function testSetRevokerEmitsEvent() public { + address newRevoker = makeAddr("new-revoker"); + vm.expectEmit(true, false, false, false); + emit NitroEnclaveVerifier.RevokerUpdated(newRevoker); + verifier.setRevoker(newRevoker); + } + + function testSetRevokerRevertsIfNotOwner() public { + vm.prank(submitter); + vm.expectRevert(); + verifier.setRevoker(makeAddr("anyone")); + } + + function testSetRevokerRevertsIfCalledByRevoker() public { + vm.prank(revokerAddr); + vm.expectRevert(); + verifier.setRevoker(makeAddr("anyone")); + } + // ============ updateVerifierId Tests ============ function testUpdateVerifierId() public {