From f22d1a7c002811b8ed1fc2046ec3e57b4f1a4934 Mon Sep 17 00:00:00 2001 From: Elena Nadolinski Date: Thu, 11 Jun 2026 13:10:38 -0700 Subject: [PATCH 1/3] Support Nitro certificate revocation --- CHANGELOG.md | 10 ++ README.md | 24 ++- docs/hinted-p384-nitro-attestation.md | 57 +++++-- src/CertManager.sol | 86 +++++++++- src/ICertManager.sol | 18 +++ src/NitroValidator.sol | 4 +- test/IndefiniteLengthCbor.t.sol | 26 +++ test/hinted/HintedNitroAttestation.t.sol | 197 +++++++++++++++++++++++ 8 files changed, 397 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1bc2119..26998d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ All notable changes to this project are documented here. The format is based on ## [Unreleased] +### Added +- Operational certificate revocation in `CertManager`: an owner-managed `revoker` can mark one or + many certificate hashes revoked, and the owner can rotate the revoker or undo accidental + revocations. + +### Changed +- Certificate verification and cached reuse now reject revoked certificates and revoked cached + parent-chain ancestors independently of `notAfter`. + ### Fixed - Reject non-canonical P-384 public key coordinates greater than or equal to the field prime `p`. @@ -49,4 +58,5 @@ yet a general-availability release. in NatSpec, the README, and the design doc. - Moved the demo `CertManagerDemo` out of `src/` into `test/helpers/`. +[Unreleased]: https://github.com/base/nitro-validator/compare/v2.0.0-rc.1...HEAD [2.0.0-rc.1]: https://github.com/base/nitro-validator/releases/tag/v2.0.0-rc.1 diff --git a/README.md b/README.md index 69d72b0..bf15453 100644 --- a/README.md +++ b/README.md @@ -3,8 +3,10 @@ This repo provides solidity contracts for the verification of attestations generated by AWS Nitro Enclaves, as outlined in [this doc](https://github.com/aws/aws-nitro-enclaves-nsm-api/blob/4b851f3006c6fa98f23dcffb2cba03b39de9b8af/docs/attestation_process.md#3-attestation-document-validation). -This library does not currently support certificate revocation, which is disabled in AWS's attestation verification documentation +AWS's attestation verification documentation disables CRL checks in its sample flow [here](https://github.com/aws/aws-nitro-enclaves-nsm-api/blob/4b851f3006c6fa98f23dcffb2cba03b39de9b8af/docs/attestation_process.md#32-syntactical-validation). +This library supports operational revocation with an authorized revoker: operators monitor AWS CRLs +off-chain and call `CertManager.revokeCert` / `revokeCerts` for affected certificate hashes. ## Hinted P-384 verification @@ -33,9 +35,12 @@ For the full design, security argument, and measured gas, see Deploy in this order (the verifier references are immutable): 1. `P384Verifier` -2. `CertManager(p384Verifier)` — pins the AWS Nitro root CA in its constructor. +2. `CertManager(p384Verifier)` — pins the AWS Nitro root CA and sets the deployer as owner/revoker. 3. `NitroValidator(certManager, p384Verifier)` +After deployment, move ownership to the production admin and set the operational revoker with +`transferOwnership` / `setRevoker`. + ### Verification flow Verification has two phases. Certificate chains are reused across many attestations from the same @@ -69,6 +74,17 @@ node tools/p384_hints.js attestation --attestation <0x COSE> --pubkey <0x leaf p Production callers should reimplement this in their backend language; the contract re-verifies every hint, so the generator is trusted only for liveness, never for correctness. +### Revocation operations + +`CertManager` does not fetch or parse AWS CRLs on-chain. Instead, an authorized `revoker` address +marks certificate hashes revoked after checking AWS CRLs off-chain. Revoked certificates are rejected +on both cold verification and cached reuse, independently of `notAfter`. Cached descendants are also +rejected when their cached parent chain contains a revoked certificate. + +- The deployer starts as both `owner` and `revoker`. +- The owner can call `transferOwnership`, `setRevoker`, and `unrevokeCert`. +- The revoker can call `revokeCert` or `revokeCerts`. + ### Example consumer ```solidity @@ -141,8 +157,8 @@ integrator (see [docs](docs/hinted-p384-nitro-attestation.md#integrator-responsi attestation fields instead. - **Enclave policy** — checking `pcrs` / `moduleID` against the enclave image(s) you trust is your responsibility. -- **Revocation** — not supported (consistent with AWS's documented validation process, linked - above). +- **Revocation operations** — the contract enforces the on-chain revoked set, but an off-chain + operator must monitor AWS CRLs and submit revoked certificate hashes. ## Build diff --git a/docs/hinted-p384-nitro-attestation.md b/docs/hinted-p384-nitro-attestation.md index 8d745a5..27f947d 100644 --- a/docs/hinted-p384-nitro-attestation.md +++ b/docs/hinted-p384-nitro-attestation.md @@ -218,8 +218,9 @@ Three deployable contracts: limit. It uses the hint-aware `ECDSA384` library vendored at `src/vendor/ECDSA384.sol` (see `src/vendor/README.md`). `CertManager` and `NitroValidator` hold **immutable** references to it. -- **`CertManager`** — parses/validates certificates, caches verified ones, and - pins the AWS Nitro root. Implements `ICertManager`. +- **`CertManager`** — parses/validates certificates, caches verified ones, pins the + AWS Nitro root, and enforces an owner-managed revocation set. Implements + `ICertManager`. - **`NitroValidator`** — parses the CBOR/COSE attestation and drives the certificate chain through `CertManager`. @@ -372,9 +373,27 @@ Practical reuse cases: **Cache reuse** is allowed when: the submitted DER hashes to a cached cert; the cert is unexpired (`notAfter ≥ block.timestamp`); the cached CA/client role matches; and -`parentCertHash` matches the parent used during cold verification. The cache is global -on-chain state — once any caller verifies a cert, others reuse it until expiry, but -only under the same parent binding. +`parentCertHash` matches the parent used during cold verification; and neither the cert +nor its cached parent chain is revoked. The cache is global on-chain state — once any +caller verifies a cert, others reuse it until expiry or revocation, but only under the +same parent binding. + +### Revocation model +AWS's Nitro attestation documentation disables CRL checking in its sample validation +flow. This implementation keeps CRL parsing off-chain and exposes an operational +revocation hook on-chain: + +- the `CertManager` deployer starts as both `owner` and `revoker`; +- the owner can transfer ownership, rotate the revoker, and undo accidental + revocations with `unrevokeCert`; +- the revoker can call `revokeCert` / `revokeCerts` for AWS certificate hashes after + checking AWS CRLs off-chain. + +Revoked certs are rejected during cold verification, cached reuse, and warm attestation +bundle re-walks. Parent-chain revocation is also enforced for cached intermediates, so a +cached descendant cannot keep verifying through an ancestor that was later revoked. +Revocation is checked independently of `notAfter`, so a revoked cert is untrusted even if +its X.509 validity period has not expired. **Warm-only guard.** `validateAttestationWithHints` re-runs the cabundle checks with an *empty* hint stream. Cached certs return before signature verification; a missing cert @@ -439,7 +458,7 @@ Runtime sizes (`forge build --sizes`); EIP-170 limit is 24,576 bytes: | contract | runtime size | margin | |----------|-------------:|-------:| | `P384Verifier` | 7,805 | 16,771 | -| `CertManager` | 19,620 | 4,956 | +| `CertManager` | 21,518 | 3,058 | | `NitroValidator` | 14,062 | 10,514 | (Test-only helper contracts are not part of the deployable contract set.) @@ -448,13 +467,13 @@ Runtime sizes (`forge build --sizes`); EIP-170 limit is 24,576 bytes: The hinted contracts are exercised against the real fixture and adversarial inputs. Covered failure modes: mutated hint, truncated hint, surplus hint, wrong parent hash, -expired cached cert, expired cert on first (cold) verification, the `notAfter` validity -boundary, CA/client role mismatch, missing warm cache, invalid final signature, -out-of-range ECDSA scalars (`r=0`, `r≥n`, `s=0`, `s>lowSmax`), disabled unhinted -entrypoints, EIP-170 fit, and off-chain↔on-chain hint equivalence. The DER, CBOR, and -byte-slicing parsers additionally have direct unit and fuzz tests for malformed and -out-of-bounds input (`test/Asn1Decode.t.sol`, `test/CborDecode.t.sol`, -`test/LibBytes.t.sol`). +revoked certs, revoked parents, expired cached cert, expired cert on first (cold) +verification, the `notAfter` validity boundary, CA/client role mismatch, missing warm +cache, invalid final signature, out-of-range ECDSA scalars (`r=0`, `r≥n`, `s=0`, +`s>lowSmax`), disabled unhinted entrypoints, EIP-170 fit, and off-chain↔on-chain hint +equivalence. The DER, CBOR, and byte-slicing parsers additionally have direct unit and +fuzz tests for malformed and out-of-bounds input (`test/Asn1Decode.t.sol`, +`test/CborDecode.t.sol`, `test/LibBytes.t.sol`). | invariant | component | how it is tested | |-----------|-----------|------------------| @@ -464,6 +483,7 @@ out-of-bounds input (`test/Asn1Decode.t.sol`, `test/CborDecode.t.sol`, | hinted verifier matches the original accept/reject set | `P384Verifier` | accepts a valid signature; rejects mutated hash / signature / public key | | no unhinted fallback via hinted entrypoints | `CertManager` | the unhinted entrypoints revert | | warm validation requires cached certs | `NitroValidator` | empty-hint final validation reverts when a cert is uncached | +| revoked certs are never trusted | `CertManager` | revoked cold/cached certs, revoked parents/ancestors, and revoked root/leaf warm paths revert | | out-of-range scalars are rejected | `P384Verifier` | `r=0` / `r≥n` / `s=0` / `s>lowSmax` signatures return false | | certificate validity is enforced at the boundary | `CertManager` | cold-path expiry reverts; valid at `notAfter`, expired at `notAfter+1` | | parsers reject malformed / out-of-bounds input | `Asn1Decode`, `CborDecode`, `LibBytes` | direct unit + fuzz tests for bad tags, lengths, types, and slices | @@ -516,10 +536,9 @@ deliberately left to the caller and must be handled in the consuming contract: - **Freshness / anti-replay.** `validateAttestationWithHints` only checks that `timestamp` is non-zero and that `nonce` is within a size bound; it never compares `timestamp` (milliseconds) to `block.timestamp` (seconds) nor matches `nonce` to a - challenge. A valid - attestation can be replayed until its short-lived leaf certificate expires. If you - need freshness, compare `ptrs.timestamp / 1000` to `block.timestamp` and/or verify - `ptrs.nonce` against a value you issued. + challenge. A valid attestation can be replayed until its short-lived leaf certificate + expires or is revoked. If you need freshness, compare `ptrs.timestamp / 1000` to + `block.timestamp` and/or verify `ptrs.nonce` against a value you issued. - **Signature malleability.** Low-S is intentionally not enforced (AWS does not guarantee low-S; see `CURVE_LOW_S_MAX` in `ECDSA384Curve.sol`), so for a valid signature `(r, s)` the twin `(r, n−s)` also verifies. This cannot forge an @@ -528,6 +547,10 @@ deliberately left to the caller and must be handled in the consuming contract: attestation fields (e.g. `moduleID + timestamp + nonce`). - **Enclave-image / PCR policy.** The contract returns the parsed `pcrs` and `moduleID`; deciding which enclave images you trust is application policy. +- **CRL monitoring.** `CertManager` enforces certificate hashes that have been marked + revoked on-chain, but it does not fetch or parse AWS CRLs. A trusted off-chain + operator must monitor AWS CRLs and submit `revokeCert` / `revokeCerts` transactions + promptly. ## 11. On-chain demo diff --git a/src/CertManager.sol b/src/CertManager.sol index 658d1bf..2a4f547 100644 --- a/src/CertManager.sol +++ b/src/CertManager.sol @@ -11,13 +11,17 @@ import {IP384Verifier} from "./IP384Verifier.sol"; // Manages a mapping of verified certificates and their metadata. // The root of trust is the AWS Nitro root cert. -// Certificate revocation is not currently supported. +// Certificate revocation is applied by an authorized revoker that tracks AWS CRLs off-chain. contract CertManager is ICertManager { using Asn1Decode for bytes; using LibAsn1Ptr for Asn1Ptr; using LibBytes for bytes; event CertVerified(bytes32 indexed certHash); + event CertRevoked(bytes32 indexed certHash); + event CertUnrevoked(bytes32 indexed certHash); + event OwnershipTransferred(address indexed previousOwner, address indexed newOwner); + event RevokerUpdated(address indexed previousRevoker, address indexed newRevoker); // root CA certificate constants (don't store it to reduce contract size) bytes32 public constant ROOT_CA_CERT_HASH = 0x311d96fcd5c5e0ccf72ef548e2ea7d4c0cd53ad7c4cc49e67471aed41d61f185; @@ -48,12 +52,35 @@ contract CertManager is ICertManager { mapping(bytes32 => bytes) public verified; // certHash -> parent cert hash used during cold verification mapping(bytes32 => bytes32) internal verifiedParent; + mapping(bytes32 => bool) public revoked; IP384Verifier public immutable p384Verifier; + address public owner; + address public revoker; + + modifier onlyOwner() { + _onlyOwner(); + _; + } + + modifier onlyRevoker() { + _onlyRevoker(); + _; + } + + function _onlyOwner() internal view { + require(msg.sender == owner, "not owner"); + } + + function _onlyRevoker() internal view { + require(msg.sender == revoker, "not revoker"); + } constructor(IP384Verifier p384Verifier_) { require(address(p384Verifier_) != address(0), "missing P384 verifier"); p384Verifier = p384Verifier_; + owner = msg.sender; + revoker = msg.sender; _saveVerified( ROOT_CA_CERT_HASH, VerifiedCert({ @@ -64,6 +91,8 @@ contract CertManager is ICertManager { pubKey: ROOT_CA_CERT_PUB_KEY }) ); + emit OwnershipTransferred(address(0), msg.sender); + emit RevokerUpdated(address(0), msg.sender); } /// @notice DEPRECATED — always reverts. The fully on-chain (non-hinted) path is too expensive @@ -108,6 +137,56 @@ contract CertManager is ICertManager { return _loadVerified(certHash); } + function isRevoked(bytes32 certHash) external view returns (bool) { + return revoked[certHash]; + } + + function transferOwnership(address newOwner) external onlyOwner { + require(newOwner != address(0), "invalid owner"); + emit OwnershipTransferred(owner, newOwner); + owner = newOwner; + } + + function setRevoker(address newRevoker) external onlyOwner { + require(newRevoker != address(0), "invalid revoker"); + emit RevokerUpdated(revoker, newRevoker); + revoker = newRevoker; + } + + function revokeCert(bytes32 certHash) external onlyRevoker { + _revokeCert(certHash); + } + + function revokeCerts(bytes32[] calldata certHashes) external onlyRevoker { + for (uint256 i = 0; i < certHashes.length; ++i) { + _revokeCert(certHashes[i]); + } + } + + function unrevokeCert(bytes32 certHash) external onlyOwner { + revoked[certHash] = false; + emit CertUnrevoked(certHash); + } + + function _revokeCert(bytes32 certHash) internal { + revoked[certHash] = true; + emit CertRevoked(certHash); + } + + function _requireNotRevoked(bytes32 certHash) internal view { + require(!revoked[certHash], "cert revoked"); + } + + function _requireCachedChainNotRevoked(bytes32 certHash) internal view { + while (certHash != bytes32(0)) { + _requireNotRevoked(certHash); + if (certHash == ROOT_CA_CERT_HASH) { + return; + } + certHash = verifiedParent[certHash]; + } + } + function _verifyCert( bytes memory certificate, bytes32 certHash, @@ -115,9 +194,12 @@ contract CertManager is ICertManager { bytes32 parentCertHash, bytes memory signatureHints ) internal returns (VerifiedCert memory) { - VerifiedCert memory parent = _loadVerified(parentCertHash); + _requireNotRevoked(certHash); + VerifiedCert memory parent; if (certHash != ROOT_CA_CERT_HASH) { + parent = _loadVerified(parentCertHash); require(parent.pubKey.length > 0, "parent cert unverified"); + _requireCachedChainNotRevoked(parentCertHash); require(!_certificateExpired(parent.notAfter), "parent cert expired"); require(parent.ca, "parent cert is not a CA"); require(!ca || parent.maxPathLen != 0, "maxPathLen exceeded"); diff --git a/src/ICertManager.sol b/src/ICertManager.sol index e072abe..c68414b 100644 --- a/src/ICertManager.sol +++ b/src/ICertManager.sol @@ -12,6 +12,12 @@ interface ICertManager { // --- Active (hinted) entrypoints --- + function owner() external view returns (address); + + function revoker() external view returns (address); + + function revoked(bytes32 certHash) external view returns (bool); + function verifyCACertWithHints(bytes memory cert, bytes32 parentCertHash, bytes memory signatureHints) external returns (bytes32); @@ -22,6 +28,18 @@ interface ICertManager { function loadVerified(bytes32 certHash) external view returns (VerifiedCert memory); + function isRevoked(bytes32 certHash) external view returns (bool); + + function transferOwnership(address newOwner) external; + + function setRevoker(address newRevoker) external; + + function revokeCert(bytes32 certHash) external; + + function revokeCerts(bytes32[] calldata certHashes) external; + + function unrevokeCert(bytes32 certHash) external; + // --- DEPRECATED: these always revert; use the *WithHints variants above. --- /// @dev DEPRECATED — always reverts ("use hinted cert verification"). diff --git a/src/NitroValidator.sol b/src/NitroValidator.sol index a760895..e87eae1 100644 --- a/src/NitroValidator.sol +++ b/src/NitroValidator.sol @@ -91,8 +91,8 @@ contract NitroValidator { /// well-formed, but deliberately does NOT enforce: /// - Freshness / anti-replay: `ptrs.timestamp` is only checked to be non-zero and `nonce` /// is only length-bounded. A valid attestation can be replayed until its leaf cert - /// expires. Callers that need freshness must compare `ptrs.timestamp / 1000` to - /// `block.timestamp` and/or match `ptrs.nonce` against a challenge they issued. + /// expires or is revoked. Callers that need freshness must compare `ptrs.timestamp / 1000` + /// to `block.timestamp` and/or match `ptrs.nonce` against a challenge they issued. /// - Signature non-malleability: low-S is not enforced (see {ECDSA384Curve.CURVE_LOW_S_MAX}), /// so do not use `signature` (or its hash) as a uniqueness key — dedupe on attestation /// fields instead. diff --git a/test/IndefiniteLengthCbor.t.sol b/test/IndefiniteLengthCbor.t.sol index 90b0059..2f31395 100644 --- a/test/IndefiniteLengthCbor.t.sol +++ b/test/IndefiniteLengthCbor.t.sol @@ -87,6 +87,18 @@ contract NitroValidatorHarness is NitroValidator { /// @notice Minimal ICertManager stub; _parseAttestation is pure so this is never called. contract StubCertManager is ICertManager { + function owner() external pure returns (address) { + return address(0); + } + + function revoker() external pure returns (address) { + return address(0); + } + + function revoked(bytes32) external pure returns (bool) { + return false; + } + function verifyCACert(bytes memory, bytes32) external pure returns (bytes32) { return bytes32(0); } @@ -110,6 +122,20 @@ contract StubCertManager is ICertManager { function loadVerified(bytes32) external pure returns (VerifiedCert memory v) { return v; } + + function isRevoked(bytes32) external pure returns (bool) { + return false; + } + + function transferOwnership(address) external pure {} + + function setRevoker(address) external pure {} + + function revokeCert(bytes32) external pure {} + + function revokeCerts(bytes32[] calldata) external pure {} + + function unrevokeCert(bytes32) external pure {} } // ────────────────────────────────────────────────────────────── diff --git a/test/hinted/HintedNitroAttestation.t.sol b/test/hinted/HintedNitroAttestation.t.sol index 0e7c495..a92bfbb 100644 --- a/test/hinted/HintedNitroAttestation.t.sol +++ b/test/hinted/HintedNitroAttestation.t.sol @@ -135,6 +135,203 @@ contract HintedNitroAttestationTest is Test { certManager.verifyClientCertWithHints(clientCert, keccak256(rootCert), ""); } + function test_CertManagerRevocationRoles() public { + assertEq(certManager.owner(), address(this)); + assertEq(certManager.revoker(), address(this)); + + bytes32 certHash = keccak256("cert"); + certManager.revokeCert(certHash); + assertTrue(certManager.revoked(certHash)); + assertTrue(certManager.isRevoked(certHash)); + + address newRevoker = address(0xBEEF); + vm.prank(address(0xCAFE)); + vm.expectRevert("not owner"); + certManager.setRevoker(newRevoker); + + vm.expectRevert("invalid revoker"); + certManager.setRevoker(address(0)); + + certManager.setRevoker(newRevoker); + assertEq(certManager.revoker(), newRevoker); + + bytes32 otherCertHash = keccak256("other cert"); + vm.prank(address(0xCAFE)); + vm.expectRevert("not revoker"); + certManager.revokeCert(otherCertHash); + + vm.prank(newRevoker); + certManager.revokeCert(otherCertHash); + assertTrue(certManager.revoked(otherCertHash)); + + vm.prank(newRevoker); + vm.expectRevert("not owner"); + certManager.unrevokeCert(otherCertHash); + + certManager.unrevokeCert(otherCertHash); + assertFalse(certManager.revoked(otherCertHash)); + + vm.expectRevert("invalid owner"); + certManager.transferOwnership(address(0)); + + address newOwner = address(0xA11CE); + vm.prank(address(0xCAFE)); + vm.expectRevert("not owner"); + certManager.transferOwnership(newOwner); + + certManager.transferOwnership(newOwner); + assertEq(certManager.owner(), newOwner); + + vm.expectRevert("not owner"); + certManager.setRevoker(address(0x1234)); + + vm.prank(newOwner); + certManager.setRevoker(address(0x1234)); + assertEq(certManager.revoker(), address(0x1234)); + } + + function test_CertManagerRevokerCanBatchRevoke() public { + address newRevoker = address(0xBEEF); + certManager.setRevoker(newRevoker); + + bytes32[] memory certHashes = new bytes32[](2); + certHashes[0] = keccak256("cert 1"); + certHashes[1] = keccak256("cert 2"); + + vm.prank(newRevoker); + certManager.revokeCerts(certHashes); + + assertTrue(certManager.revoked(certHashes[0])); + assertTrue(certManager.revoked(certHashes[1])); + } + + function test_HintedCACertRejectsRevokedColdCert() public { + bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); + (bytes memory attestationTbs,) = validator.decodeAttestationTbs(attestation); + NitroValidator.Ptrs memory ptrs = parser.parseAttestation(attestationTbs); + (bytes memory caCert, bytes32 parentHash, bytes memory parentPubKey) = _firstNonRootCA(attestationTbs, ptrs); + bytes memory hints = hintCollector.collectCertSignatureHints(caCert, parentPubKey); + + certManager.revokeCert(keccak256(caCert)); + + vm.expectRevert("cert revoked"); + certManager.verifyCACertWithHints(caCert, parentHash, hints); + } + + function test_HintedCACertRejectsRevokedCachedCert() public { + bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); + (bytes memory attestationTbs,) = validator.decodeAttestationTbs(attestation); + NitroValidator.Ptrs memory ptrs = parser.parseAttestation(attestationTbs); + (bytes memory caCert, bytes32 parentHash, bytes memory parentPubKey) = _firstNonRootCA(attestationTbs, ptrs); + bytes memory hints = hintCollector.collectCertSignatureHints(caCert, parentPubKey); + bytes32 caHash = certManager.verifyCACertWithHints(caCert, parentHash, hints); + + certManager.revokeCert(caHash); + + vm.expectRevert("cert revoked"); + certManager.verifyCACertWithHints(caCert, parentHash, ""); + } + + function test_HintedCAChildRejectsRevokedParent() public { + bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); + (bytes memory attestationTbs,) = validator.decodeAttestationTbs(attestation); + NitroValidator.Ptrs memory ptrs = parser.parseAttestation(attestationTbs); + + bytes memory rootCert = attestationTbs.slice(ptrs.cabundle[0]); + bytes32 rootHash = keccak256(rootCert); + bytes memory ca1 = attestationTbs.slice(ptrs.cabundle[1]); + bytes memory ca1Hints = hintCollector.collectCertSignatureHints(ca1, certManager.loadVerified(rootHash).pubKey); + bytes32 ca1Hash = certManager.verifyCACertWithHints(ca1, rootHash, ca1Hints); + + bytes memory ca2 = attestationTbs.slice(ptrs.cabundle[2]); + certManager.revokeCert(ca1Hash); + + vm.expectRevert("cert revoked"); + certManager.verifyCACertWithHints(ca2, ca1Hash, ""); + } + + function test_HintedCAChildRejectsRevokedAncestor() public { + bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); + (bytes memory attestationTbs,) = validator.decodeAttestationTbs(attestation); + NitroValidator.Ptrs memory ptrs = parser.parseAttestation(attestationTbs); + + bytes memory rootCert = attestationTbs.slice(ptrs.cabundle[0]); + bytes32 rootHash = keccak256(rootCert); + bytes memory ca1 = attestationTbs.slice(ptrs.cabundle[1]); + bytes memory ca1Hints = hintCollector.collectCertSignatureHints(ca1, certManager.loadVerified(rootHash).pubKey); + bytes32 ca1Hash = certManager.verifyCACertWithHints(ca1, rootHash, ca1Hints); + bytes memory ca2 = attestationTbs.slice(ptrs.cabundle[2]); + bytes memory ca2Hints = hintCollector.collectCertSignatureHints(ca2, certManager.loadVerified(ca1Hash).pubKey); + + certManager.revokeCert(rootHash); + + vm.expectRevert("cert revoked"); + certManager.verifyCACertWithHints(ca2, ca1Hash, ca2Hints); + } + + function test_HintedCachedCertRejectsRevokedAncestor() public { + bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); + (bytes memory attestationTbs,) = validator.decodeAttestationTbs(attestation); + NitroValidator.Ptrs memory ptrs = parser.parseAttestation(attestationTbs); + + bytes memory rootCert = attestationTbs.slice(ptrs.cabundle[0]); + bytes32 rootHash = keccak256(rootCert); + bytes memory ca1 = attestationTbs.slice(ptrs.cabundle[1]); + bytes memory ca1Hints = hintCollector.collectCertSignatureHints(ca1, certManager.loadVerified(rootHash).pubKey); + bytes32 ca1Hash = certManager.verifyCACertWithHints(ca1, rootHash, ca1Hints); + bytes memory ca2 = attestationTbs.slice(ptrs.cabundle[2]); + bytes memory ca2Hints = hintCollector.collectCertSignatureHints(ca2, certManager.loadVerified(ca1Hash).pubKey); + certManager.verifyCACertWithHints(ca2, ca1Hash, ca2Hints); + + certManager.revokeCert(rootHash); + + vm.expectRevert("cert revoked"); + certManager.verifyCACertWithHints(ca2, ca1Hash, ""); + } + + function test_HintedValidationRejectsRevokedLeaf() public { + bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); + (bytes memory attestationTbs, bytes memory signature) = validator.decodeAttestationTbs(attestation); + NitroValidator.Ptrs memory ptrs = parser.parseAttestation(attestationTbs); + ICertManager.VerifiedCert memory leaf = _cacheCertBundleWithHints(attestationTbs); + bytes memory hash = Sha2Ext.sha384(attestationTbs, 0, attestationTbs.length); + bytes memory attestationHints = hintCollector.collectVerifyHints(hash, signature, leaf.pubKey); + + certManager.revokeCert(keccak256(attestationTbs.slice(ptrs.cert))); + + vm.expectRevert("cert revoked"); + validator.validateAttestationWithHints(attestationTbs, signature, attestationHints); + } + + function test_HintedValidationRejectsRevokedRoot() public { + bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); + (bytes memory attestationTbs, bytes memory signature) = validator.decodeAttestationTbs(attestation); + ICertManager.VerifiedCert memory leaf = _cacheCertBundleWithHints(attestationTbs); + bytes memory hash = Sha2Ext.sha384(attestationTbs, 0, attestationTbs.length); + bytes memory attestationHints = hintCollector.collectVerifyHints(hash, signature, leaf.pubKey); + + certManager.revokeCert(certManager.ROOT_CA_CERT_HASH()); + + vm.expectRevert("cert revoked"); + validator.validateAttestationWithHints(attestationTbs, signature, attestationHints); + } + + function test_HintedCachedCertCanVerifyAfterOwnerUnrevokes() public { + bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); + (bytes memory attestationTbs,) = validator.decodeAttestationTbs(attestation); + NitroValidator.Ptrs memory ptrs = parser.parseAttestation(attestationTbs); + (bytes memory caCert, bytes32 parentHash, bytes memory parentPubKey) = _firstNonRootCA(attestationTbs, ptrs); + bytes memory hints = hintCollector.collectCertSignatureHints(caCert, parentPubKey); + bytes32 caHash = certManager.verifyCACertWithHints(caCert, parentHash, hints); + + certManager.revokeCert(caHash); + vm.expectRevert("cert revoked"); + certManager.verifyCACertWithHints(caCert, parentHash, ""); + + certManager.unrevokeCert(caHash); + assertEq(certManager.verifyCACertWithHints(caCert, parentHash, ""), caHash); + } + function test_HintedCACertRejectsExpiredCachedCert() public { bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); (bytes memory attestationTbs,) = validator.decodeAttestationTbs(attestation); From 2a139285ef09cda8362b7f710e89a781f2068d93 Mon Sep 17 00:00:00 2001 From: Elena Nadolinski Date: Sat, 13 Jun 2026 09:04:50 -0700 Subject: [PATCH 2/3] Address revocation review feedback --- CHANGELOG.md | 4 ++ README.md | 20 ++++++--- docs/hinted-p384-nitro-attestation.md | 26 ++++++++--- src/CertManager.sol | 34 +++++++++------ src/ICertManager.sol | 1 + test/hinted/HintedNitroAttestation.t.sol | 55 ++++++++++++++++++++++++ 6 files changed, 113 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26998d6..11ccd48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ All notable changes to this project are documented here. The format is based on ### Changed - Certificate verification and cached reuse now reject revoked certificates and revoked cached parent-chain ancestors independently of `notAfter`. +- Root certificate revocation is owner-only, while non-root revocation remains delegated to the + revoker role. +- Cold certificate verification rejects submitted cert bytes with trailing data or fields after the + signature, keeping revocation keys aligned with the parsed certificate bytes. ### Fixed - Reject non-canonical P-384 public key coordinates greater than or equal to the field prime `p`. diff --git a/README.md b/README.md index bf15453..927b64b 100644 --- a/README.md +++ b/README.md @@ -77,13 +77,20 @@ hint, so the generator is trusted only for liveness, never for correctness. ### Revocation operations `CertManager` does not fetch or parse AWS CRLs on-chain. Instead, an authorized `revoker` address -marks certificate hashes revoked after checking AWS CRLs off-chain. Revoked certificates are rejected -on both cold verification and cached reuse, independently of `notAfter`. Cached descendants are also -rejected when their cached parent chain contains a revoked certificate. +marks certificate hashes revoked after checking AWS CRLs off-chain. A certificate hash is +`keccak256(certBytes)`, where `certBytes` are the exact X.509 DER bytes submitted to +`verifyCACertWithHints` / `verifyClientCertWithHints`; AWS CRLs identify certificates by +issuer/serial, so the operator must resolve CRL entries to these submitted certificate bytes +off-chain. Revoked certificates are rejected on both cold verification and cached reuse, +independently of `notAfter`. Cached descendants are also rejected when their cached parent chain +contains a revoked certificate. - The deployer starts as both `owner` and `revoker`. -- The owner can call `transferOwnership`, `setRevoker`, and `unrevokeCert`. -- The revoker can call `revokeCert` or `revokeCerts`. +- The owner can call `transferOwnership`, `setRevoker`, `unrevokeCert`, and revoke + `ROOT_CA_CERT_HASH` as an emergency global halt. +- The revoker can call `revokeCert` or `revokeCerts` for non-root certificate hashes. +- `loadVerified` is a raw cache read; returned metadata does not imply the certificate is + currently trusted. ### Example consumer @@ -158,7 +165,8 @@ integrator (see [docs](docs/hinted-p384-nitro-attestation.md#integrator-responsi - **Enclave policy** — checking `pcrs` / `moduleID` against the enclave image(s) you trust is your responsibility. - **Revocation operations** — the contract enforces the on-chain revoked set, but an off-chain - operator must monitor AWS CRLs and submit revoked certificate hashes. + operator must monitor AWS CRLs, map issuer/serial entries to exact certificate-byte hashes, and + submit revoked certificate hashes. ## Build diff --git a/docs/hinted-p384-nitro-attestation.md b/docs/hinted-p384-nitro-attestation.md index 27f947d..e1eb323 100644 --- a/docs/hinted-p384-nitro-attestation.md +++ b/docs/hinted-p384-nitro-attestation.md @@ -384,10 +384,18 @@ flow. This implementation keeps CRL parsing off-chain and exposes an operational revocation hook on-chain: - the `CertManager` deployer starts as both `owner` and `revoker`; -- the owner can transfer ownership, rotate the revoker, and undo accidental - revocations with `unrevokeCert`; -- the revoker can call `revokeCert` / `revokeCerts` for AWS certificate hashes after - checking AWS CRLs off-chain. +- the owner can transfer ownership, rotate the revoker, undo accidental revocations + with `unrevokeCert`, and revoke `ROOT_CA_CERT_HASH` as an emergency global halt; +- the revoker can call `revokeCert` / `revokeCerts` for non-root AWS certificate hashes + after checking AWS CRLs off-chain. + +Revocation keys are byte-identity hashes: `keccak256(certBytes)`, where `certBytes` +are the exact X.509 DER bytes submitted to `verifyCACertWithHints` / +`verifyClientCertWithHints`. AWS CRLs identify certificates by issuer and serial, so +operators must resolve CRL entries to the exact submitted certificate bytes off-chain +before submitting revocation transactions. Cold verification rejects certificate byte +strings whose outer ASN.1 certificate object does not consume all submitted bytes, or +whose certificate sequence contains fields after the signature. Revoked certs are rejected during cold verification, cached reuse, and warm attestation bundle re-walks. Parent-chain revocation is also enforced for cached intermediates, so a @@ -395,6 +403,10 @@ cached descendant cannot keep verifying through an ancestor that was later revok Revocation is checked independently of `notAfter`, so a revoked cert is untrusted even if its X.509 validity period has not expired. +`loadVerified` is intentionally a raw cache read. A non-empty return value means the cert +metadata was cached previously; it does not imply the cert is currently trusted, unexpired, +or unrevoked. + **Warm-only guard.** `validateAttestationWithHints` re-runs the cabundle checks with an *empty* hint stream. Cached certs return before signature verification; a missing cert sends the empty stream into P384 verification and reverts with @@ -458,7 +470,7 @@ Runtime sizes (`forge build --sizes`); EIP-170 limit is 24,576 bytes: | contract | runtime size | margin | |----------|-------------:|-------:| | `P384Verifier` | 7,805 | 16,771 | -| `CertManager` | 21,518 | 3,058 | +| `CertManager` | 21,849 | 2,727 | | `NitroValidator` | 14,062 | 10,514 | (Test-only helper contracts are not part of the deployable contract set.) @@ -549,8 +561,8 @@ deliberately left to the caller and must be handled in the consuming contract: `moduleID`; deciding which enclave images you trust is application policy. - **CRL monitoring.** `CertManager` enforces certificate hashes that have been marked revoked on-chain, but it does not fetch or parse AWS CRLs. A trusted off-chain - operator must monitor AWS CRLs and submit `revokeCert` / `revokeCerts` transactions - promptly. + operator must monitor AWS CRLs, map issuer/serial entries to exact submitted + certificate-byte hashes, and submit `revokeCert` / `revokeCerts` transactions promptly. ## 11. On-chain demo diff --git a/src/CertManager.sol b/src/CertManager.sol index 2a4f547..92edb21 100644 --- a/src/CertManager.sol +++ b/src/CertManager.sol @@ -63,11 +63,6 @@ contract CertManager is ICertManager { _; } - modifier onlyRevoker() { - _onlyRevoker(); - _; - } - function _onlyOwner() internal view { require(msg.sender == owner, "not owner"); } @@ -133,6 +128,9 @@ contract CertManager is ICertManager { return _verifyCert(cert, keccak256(cert), false, parentCertHash, signatureHints); } + /// @notice Return raw cached certificate metadata without current trust checks. + /// @dev A non-empty return value only means the cert was cached previously. It may now be + /// expired or revoked; use the verification entrypoints for trust-aware reuse. function loadVerified(bytes32 certHash) external view returns (VerifiedCert memory) { return _loadVerified(certHash); } @@ -153,12 +151,14 @@ contract CertManager is ICertManager { revoker = newRevoker; } - function revokeCert(bytes32 certHash) external onlyRevoker { + function revokeCert(bytes32 certHash) external { + _requireCanRevoke(certHash); _revokeCert(certHash); } - function revokeCerts(bytes32[] calldata certHashes) external onlyRevoker { + function revokeCerts(bytes32[] calldata certHashes) external { for (uint256 i = 0; i < certHashes.length; ++i) { + _requireCanRevoke(certHashes[i]); _revokeCert(certHashes[i]); } } @@ -173,6 +173,14 @@ contract CertManager is ICertManager { emit CertRevoked(certHash); } + function _requireCanRevoke(bytes32 certHash) internal view { + if (certHash == ROOT_CA_CERT_HASH) { + _onlyOwner(); + } else { + _onlyRevoker(); + } + } + function _requireNotRevoked(bytes32 certHash) internal view { require(!revoked[certHash], "cert revoked"); } @@ -232,6 +240,7 @@ contract CertManager is ICertManager { bytes memory signatureHints ) internal view returns (VerifiedCert memory cert) { Asn1Ptr root = certificate.root(); + require(root.totalLength() == certificate.length, "invalid cert length"); Asn1Ptr tbsCertPtr = certificate.firstChildOf(root); (uint64 notAfter, int64 maxPathLen, bytes32 issuerHash, bytes32 subjectHash, bytes memory pubKey) = _parseTbs(certificate, tbsCertPtr, ca); @@ -419,19 +428,16 @@ contract CertManager is ICertManager { ) internal view { Asn1Ptr sigAlgoPtr = certificate.nextSiblingOf(ptr); require(certificate.keccak(sigAlgoPtr.content(), sigAlgoPtr.length()) == CERT_ALGO_OID, "invalid cert sig algo"); + Asn1Ptr sigPtr = certificate.nextSiblingOf(sigAlgoPtr); + require(sigPtr.header() + sigPtr.totalLength() == certificate.length, "trailing cert fields"); bytes memory hash = Sha2Ext.sha384(certificate, ptr.header(), ptr.totalLength()); - bytes memory sigPacked = _certSignature(certificate, sigAlgoPtr); + bytes memory sigPacked = _certSignature(certificate, sigPtr); require(p384Verifier.verifyP384SignatureWithHints(hash, sigPacked, pubKey, signatureHints), "invalid sig"); } - function _certSignature(bytes memory certificate, Asn1Ptr sigAlgoPtr) - internal - pure - returns (bytes memory sigPacked) - { - Asn1Ptr sigPtr = certificate.nextSiblingOf(sigAlgoPtr); + function _certSignature(bytes memory certificate, Asn1Ptr sigPtr) internal pure returns (bytes memory sigPacked) { Asn1Ptr sigBPtr = certificate.bitstring(sigPtr); Asn1Ptr sigRoot = certificate.rootOf(sigBPtr); Asn1Ptr sigRPtr = certificate.firstChildOf(sigRoot); diff --git a/src/ICertManager.sol b/src/ICertManager.sol index c68414b..69a4a5c 100644 --- a/src/ICertManager.sol +++ b/src/ICertManager.sol @@ -26,6 +26,7 @@ interface ICertManager { external returns (VerifiedCert memory); + /// @notice Raw cache read. A returned cert may be expired or revoked. function loadVerified(bytes32 certHash) external view returns (VerifiedCert memory); function isRevoked(bytes32 certHash) external view returns (bool); diff --git a/test/hinted/HintedNitroAttestation.t.sol b/test/hinted/HintedNitroAttestation.t.sol index a92bfbb..339c6ff 100644 --- a/test/hinted/HintedNitroAttestation.t.sol +++ b/test/hinted/HintedNitroAttestation.t.sol @@ -205,6 +205,50 @@ contract HintedNitroAttestationTest is Test { assertTrue(certManager.revoked(certHashes[1])); } + function test_CertManagerRootRevocationRequiresOwner() public { + address newRevoker = address(0xBEEF); + certManager.setRevoker(newRevoker); + bytes32 rootHash = certManager.ROOT_CA_CERT_HASH(); + + vm.prank(newRevoker); + vm.expectRevert("not owner"); + certManager.revokeCert(rootHash); + assertFalse(certManager.revoked(rootHash)); + + bytes32[] memory certHashes = new bytes32[](2); + certHashes[0] = keccak256("non-root cert"); + certHashes[1] = rootHash; + + vm.prank(newRevoker); + vm.expectRevert("not owner"); + certManager.revokeCerts(certHashes); + assertFalse(certManager.revoked(certHashes[0])); + assertFalse(certManager.revoked(rootHash)); + + certManager.revokeCert(rootHash); + assertTrue(certManager.revoked(rootHash)); + } + + function test_HintedCACertRejectsTrailingBytes() public { + bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); + (bytes memory attestationTbs,) = validator.decodeAttestationTbs(attestation); + NitroValidator.Ptrs memory ptrs = parser.parseAttestation(attestationTbs); + (bytes memory caCert, bytes32 parentHash,) = _firstNonRootCA(attestationTbs, ptrs); + + vm.expectRevert("invalid cert length"); + certManager.verifyCACertWithHints(abi.encodePacked(caCert, bytes1(0x00)), parentHash, ""); + } + + function test_HintedCACertRejectsTrailingFieldInsideCertificateSequence() public { + bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); + (bytes memory attestationTbs,) = validator.decodeAttestationTbs(attestation); + NitroValidator.Ptrs memory ptrs = parser.parseAttestation(attestationTbs); + (bytes memory caCert, bytes32 parentHash,) = _firstNonRootCA(attestationTbs, ptrs); + + vm.expectRevert("trailing cert fields"); + certManager.verifyCACertWithHints(_appendInsideOuterSequence(caCert, bytes1(0x00)), parentHash, ""); + } + function test_HintedCACertRejectsRevokedColdCert() public { bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); (bytes memory attestationTbs,) = validator.decodeAttestationTbs(attestation); @@ -915,6 +959,17 @@ contract HintedNitroAttestationTest is Test { output[index] = bytes1(uint8(output[index]) ^ 1); } + function _appendInsideOuterSequence(bytes memory der, bytes1 value) internal pure returns (bytes memory output) { + require(der.length >= 4 && der[0] == 0x30 && der[1] == 0x82, "test: expected long sequence"); + uint256 length = uint256(uint8(der[2])) << 8 | uint8(der[3]); + require(length + 4 == der.length, "test: unexpected sequence length"); + length += 1; + + output = abi.encodePacked(der, value); + output[2] = bytes1(uint8(length >> 8)); + output[3] = bytes1(uint8(length)); + } + function _repairMissingPublicKeyBytes(bytes memory attestation) internal pure returns (bytes memory repaired) { // The pasted Base64 sample is missing "ic_" in the CBOR key // "public_key", but the key length and outer COSE payload length still From 3bfa38708c47bc0d3e522390468c45abba19edf4 Mon Sep 17 00:00:00 2001 From: Elena Nadolinski Date: Mon, 15 Jun 2026 11:45:11 -0700 Subject: [PATCH 3/3] Key revocation by (issuer, serial) identity instead of cert bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Leopold's review: byte-keyed revocation (revoked[keccak256(cert)]) was bypassable. ECDSA signatures are malleable — for a valid (r, s) the twin (r, n-s) also verifies — and DER is re-encodable, so a revoked certificate could be re-presented with different bytes that hash differently but still verify, slipping past the revoked set. Strict DER alone would not fix this (both signatures are valid DER) and enforcing low-S risks bricking on AWS certs. Revocation is now keyed by keccak256(issuerHash, serialHash) — the (issuer, serial) identity AWS CRLs already use, which lives inside the CA-signed TBS and is therefore fixed for every byte-encoding of a given certificate. - Add CertManager.computeCertId(certDER) so operators derive the key (and can replicate it off-chain straight from CRL issuer/serial entries). - Record each cert's identity at cold verification (certIdentity mapping) so the warm path and parent-chain walk check revocation without re-parsing. - Root halt stays keyed by the pinned ROOT_CA_CERT_HASH (the root is never parsed on-chain) and remains owner-only. - Tests: revoke real CA/leaf certs via computeCertId; add test_RevocationIdentityIsInvariantToSignatureBytes showing a signature-byte variant (different keccak256) maps to the same identity key. - Docs: README, design doc, CHANGELOG updated to the identity-keyed model. Generated with Claude Code Co-Authored-By: Claude --- CHANGELOG.md | 16 ++-- README.md | 25 +++--- docs/hinted-p384-nitro-attestation.md | 39 ++++++---- src/CertManager.sol | 97 ++++++++++++++++++------ test/hinted/HintedNitroAttestation.t.sol | 37 +++++++-- 5 files changed, 154 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11ccd48..29b1b31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,16 +8,22 @@ All notable changes to this project are documented here. The format is based on ### Added - Operational certificate revocation in `CertManager`: an owner-managed `revoker` can mark one or - many certificate hashes revoked, and the owner can rotate the revoker or undo accidental - revocations. + many certificates revoked, and the owner can rotate the revoker or undo accidental revocations. +- `CertManager.computeCertId(certDER)`: returns a certificate's `(issuer, serial)` revocation + identity key. ### Changed - Certificate verification and cached reuse now reject revoked certificates and revoked cached parent-chain ancestors independently of `notAfter`. -- Root certificate revocation is owner-only, while non-root revocation remains delegated to the - revoker role. +- Revocation is keyed by the `(issuer, serial)` identity `keccak256(issuerHash, serialHash)` (what + AWS CRLs use), not by `keccak256(certBytes)`. Byte-keying was bypassable, because ECDSA signature + malleability and DER re-encoding let a revoked certificate be re-presented with different bytes + that still verify; the signature-protected identity closes that gap and lets operators revoke + directly from CRL issuer/serial entries. +- Root certificate revocation is owner-only (keyed by the pinned `ROOT_CA_CERT_HASH`, since the root + is never parsed on-chain), while non-root revocation remains delegated to the revoker role. - Cold certificate verification rejects submitted cert bytes with trailing data or fields after the - signature, keeping revocation keys aligned with the parsed certificate bytes. + signature. ### Fixed - Reject non-canonical P-384 public key coordinates greater than or equal to the field prime `p`. diff --git a/README.md b/README.md index 927b64b..1c6b346 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ This repo provides solidity contracts for the verification of attestations gener AWS's attestation verification documentation disables CRL checks in its sample flow [here](https://github.com/aws/aws-nitro-enclaves-nsm-api/blob/4b851f3006c6fa98f23dcffb2cba03b39de9b8af/docs/attestation_process.md#32-syntactical-validation). This library supports operational revocation with an authorized revoker: operators monitor AWS CRLs -off-chain and call `CertManager.revokeCert` / `revokeCerts` for affected certificate hashes. +off-chain and call `CertManager.revokeCert` / `revokeCerts` for affected certificate identity keys. ## Hinted P-384 verification @@ -77,18 +77,22 @@ hint, so the generator is trusted only for liveness, never for correctness. ### Revocation operations `CertManager` does not fetch or parse AWS CRLs on-chain. Instead, an authorized `revoker` address -marks certificate hashes revoked after checking AWS CRLs off-chain. A certificate hash is -`keccak256(certBytes)`, where `certBytes` are the exact X.509 DER bytes submitted to -`verifyCACertWithHints` / `verifyClientCertWithHints`; AWS CRLs identify certificates by -issuer/serial, so the operator must resolve CRL entries to these submitted certificate bytes -off-chain. Revoked certificates are rejected on both cold verification and cached reuse, +marks certificates revoked after checking AWS CRLs off-chain. Revocation is keyed by the +certificate's **(issuer, serial) identity** — `keccak256(issuerHash, serialHash)`, the same identity +AWS CRLs use to list revoked certs — not by `keccak256(certBytes)`. Raw cert bytes are **not** a +stable identity: ECDSA signatures are malleable (the `(r, n-s)` twin also verifies) and DER is +re-encodable, so a byte-keyed revocation could be bypassed by a re-encoded twin of the revoked cert. +Keying on the signature-protected (issuer, serial) pair closes that gap and lets operators revoke +straight from CRL data. Compute the key with `CertManager.computeCertId(certDER)` (or replicate it +off-chain). Revoked certificates are rejected on both cold verification and cached reuse, independently of `notAfter`. Cached descendants are also rejected when their cached parent chain contains a revoked certificate. - The deployer starts as both `owner` and `revoker`. - The owner can call `transferOwnership`, `setRevoker`, `unrevokeCert`, and revoke - `ROOT_CA_CERT_HASH` as an emergency global halt. -- The revoker can call `revokeCert` or `revokeCerts` for non-root certificate hashes. + `ROOT_CA_CERT_HASH` as an emergency global halt (the root is identified by its pinned hash, since + it is never parsed on-chain). +- The revoker can call `revokeCert` or `revokeCerts` for non-root certificate identity keys. - `loadVerified` is a raw cache read; returned metadata does not imply the certificate is currently trusted. @@ -165,8 +169,9 @@ integrator (see [docs](docs/hinted-p384-nitro-attestation.md#integrator-responsi - **Enclave policy** — checking `pcrs` / `moduleID` against the enclave image(s) you trust is your responsibility. - **Revocation operations** — the contract enforces the on-chain revoked set, but an off-chain - operator must monitor AWS CRLs, map issuer/serial entries to exact certificate-byte hashes, and - submit revoked certificate hashes. + operator must monitor AWS CRLs and submit the affected certificate identity keys + (`keccak256(issuerHash, serialHash)`, computed via `computeCertId` or directly from the CRL's + issuer/serial entries). ## Build diff --git a/docs/hinted-p384-nitro-attestation.md b/docs/hinted-p384-nitro-attestation.md index e1eb323..69177f0 100644 --- a/docs/hinted-p384-nitro-attestation.md +++ b/docs/hinted-p384-nitro-attestation.md @@ -386,16 +386,23 @@ revocation hook on-chain: - the `CertManager` deployer starts as both `owner` and `revoker`; - the owner can transfer ownership, rotate the revoker, undo accidental revocations with `unrevokeCert`, and revoke `ROOT_CA_CERT_HASH` as an emergency global halt; -- the revoker can call `revokeCert` / `revokeCerts` for non-root AWS certificate hashes - after checking AWS CRLs off-chain. - -Revocation keys are byte-identity hashes: `keccak256(certBytes)`, where `certBytes` -are the exact X.509 DER bytes submitted to `verifyCACertWithHints` / -`verifyClientCertWithHints`. AWS CRLs identify certificates by issuer and serial, so -operators must resolve CRL entries to the exact submitted certificate bytes off-chain -before submitting revocation transactions. Cold verification rejects certificate byte -strings whose outer ASN.1 certificate object does not consume all submitted bytes, or -whose certificate sequence contains fields after the signature. +- the revoker can call `revokeCert` / `revokeCerts` for non-root AWS certificate + identity keys after checking AWS CRLs off-chain. + +Revocation keys are **(issuer, serial) identities**: `keccak256(issuerHash, serialHash)`, +the same pair AWS CRLs use to list revoked certificates. `CertManager.computeCertId(certDER)` +returns this key (operators can also replicate it off-chain directly from a CRL entry). The +key is deliberately **not** `keccak256(certBytes)`: raw cert bytes are not a stable identity, +because ECDSA signatures are malleable (for a valid `(r, s)` the twin `(r, n-s)` also verifies) +and DER can be re-encoded — so a byte-keyed revocation could be bypassed by submitting a +re-encoded twin of the revoked certificate, which hashes differently but still verifies. The +(issuer, serial) pair lives inside the CA-signed TBS, so it is fixed for every byte-encoding of +a given certificate, and the identity recorded when a cert is first verified matches the key an +operator computes from the CRL. The root is the one exception: it is never parsed on-chain (it +is pinned by `ROOT_CA_CERT_HASH`), so its emergency-halt key is that pinned hash. Cold +verification additionally rejects certificate byte strings whose outer ASN.1 certificate object +does not consume all submitted bytes, or whose certificate sequence contains fields after the +signature. Revoked certs are rejected during cold verification, cached reuse, and warm attestation bundle re-walks. Parent-chain revocation is also enforced for cached intermediates, so a @@ -470,7 +477,7 @@ Runtime sizes (`forge build --sizes`); EIP-170 limit is 24,576 bytes: | contract | runtime size | margin | |----------|-------------:|-------:| | `P384Verifier` | 7,805 | 16,771 | -| `CertManager` | 21,849 | 2,727 | +| `CertManager` | 22,297 | 2,279 | | `NitroValidator` | 14,062 | 10,514 | (Test-only helper contracts are not part of the deployable contract set.) @@ -559,10 +566,12 @@ deliberately left to the caller and must be handled in the consuming contract: attestation fields (e.g. `moduleID + timestamp + nonce`). - **Enclave-image / PCR policy.** The contract returns the parsed `pcrs` and `moduleID`; deciding which enclave images you trust is application policy. -- **CRL monitoring.** `CertManager` enforces certificate hashes that have been marked - revoked on-chain, but it does not fetch or parse AWS CRLs. A trusted off-chain - operator must monitor AWS CRLs, map issuer/serial entries to exact submitted - certificate-byte hashes, and submit `revokeCert` / `revokeCerts` transactions promptly. +- **CRL monitoring.** `CertManager` enforces the certificate identity keys that have been + marked revoked on-chain, but it does not fetch or parse AWS CRLs. A trusted off-chain + operator must monitor AWS CRLs and submit `revokeCert` / `revokeCerts` transactions + promptly, passing each affected cert's `(issuer, serial)` identity key + (`keccak256(issuerHash, serialHash)`, via `computeCertId` or computed directly from the + CRL entry). ## 11. On-chain demo diff --git a/src/CertManager.sol b/src/CertManager.sol index 92edb21..305d528 100644 --- a/src/CertManager.sol +++ b/src/CertManager.sol @@ -52,6 +52,13 @@ contract CertManager is ICertManager { mapping(bytes32 => bytes) public verified; // certHash -> parent cert hash used during cold verification mapping(bytes32 => bytes32) internal verifiedParent; + // certHash -> revocation identity key (keccak256(issuerHash, serialHash)), recorded at cold + // verification so the warm path and parent-chain walk can check revocation without re-parsing. + mapping(bytes32 => bytes32) internal certIdentity; + // revocation set, keyed by the (issuer, serial) identity for non-root certs (see {computeCertId}) + // and by the pinned ROOT_CA_CERT_HASH for the root emergency halt. NOT keyed by keccak256(cert): + // raw cert bytes are not a stable identity (ECDSA signatures are malleable and DER is re-encodable), + // so byte-keying would let a re-encoded twin of a revoked cert slip through. mapping(bytes32 => bool) public revoked; IP384Verifier public immutable p384Verifier; @@ -135,8 +142,20 @@ contract CertManager is ICertManager { return _loadVerified(certHash); } - function isRevoked(bytes32 certHash) external view returns (bool) { - return revoked[certHash]; + function isRevoked(bytes32 certId) external view returns (bool) { + return revoked[certId]; + } + + /// @notice Compute the revocation identity key for a certificate. + /// @dev Returns `keccak256(issuerHash, serialHash)` — the (issuer DN, serial number) pair that + /// uniquely identifies an X.509 certificate and that AWS CRLs use to list revoked certs. + /// This key is invariant to ECDSA signature malleability (the `(r, n-s)` twin) and to DER + /// re-encoding, unlike `keccak256(cert)`. Operators pass the result to {revokeCert} / + /// {revokeCerts}; the same value is recorded on-chain when the cert is first verified, so a + /// revocation applies to every byte-encoding of that certificate. Reverts on malformed DER. + function computeCertId(bytes memory cert) external pure returns (bytes32) { + Asn1Ptr tbsCertPtr = cert.firstChildOf(cert.root()); + return _certIdentity(cert, tbsCertPtr); } function transferOwnership(address newOwner) external onlyOwner { @@ -151,43 +170,54 @@ contract CertManager is ICertManager { revoker = newRevoker; } - function revokeCert(bytes32 certHash) external { - _requireCanRevoke(certHash); - _revokeCert(certHash); + /// @notice Revoke a certificate by its identity key (see {computeCertId}); use ROOT_CA_CERT_HASH + /// to trigger the owner-only emergency global halt. + function revokeCert(bytes32 certId) external { + _requireCanRevoke(certId); + _revokeCert(certId); } - function revokeCerts(bytes32[] calldata certHashes) external { - for (uint256 i = 0; i < certHashes.length; ++i) { - _requireCanRevoke(certHashes[i]); - _revokeCert(certHashes[i]); + function revokeCerts(bytes32[] calldata certIds) external { + for (uint256 i = 0; i < certIds.length; ++i) { + _requireCanRevoke(certIds[i]); + _revokeCert(certIds[i]); } } - function unrevokeCert(bytes32 certHash) external onlyOwner { - revoked[certHash] = false; - emit CertUnrevoked(certHash); + function unrevokeCert(bytes32 certId) external onlyOwner { + revoked[certId] = false; + emit CertUnrevoked(certId); } - function _revokeCert(bytes32 certHash) internal { - revoked[certHash] = true; - emit CertRevoked(certHash); + function _revokeCert(bytes32 certId) internal { + revoked[certId] = true; + emit CertRevoked(certId); } - function _requireCanRevoke(bytes32 certHash) internal view { - if (certHash == ROOT_CA_CERT_HASH) { + function _requireCanRevoke(bytes32 certId) internal view { + // The root is identified by its pinned cert hash (it is never parsed on-chain); revoking it + // halts all validation, so it is owner-only. Non-root revocation by (issuer, serial) identity + // is delegated to the revoker role. + if (certId == ROOT_CA_CERT_HASH) { _onlyOwner(); } else { _onlyRevoker(); } } - function _requireNotRevoked(bytes32 certHash) internal view { - require(!revoked[certHash], "cert revoked"); + function _requireNotRevoked(bytes32 certId) internal view { + require(!revoked[certId], "cert revoked"); + } + + /// @dev The revocation key for a cached cert: the pinned hash for the root, otherwise the + /// (issuer, serial) identity recorded at cold verification. + function _revocationKey(bytes32 certHash) internal view returns (bytes32) { + return certHash == ROOT_CA_CERT_HASH ? ROOT_CA_CERT_HASH : certIdentity[certHash]; } function _requireCachedChainNotRevoked(bytes32 certHash) internal view { while (certHash != bytes32(0)) { - _requireNotRevoked(certHash); + _requireNotRevoked(_revocationKey(certHash)); if (certHash == ROOT_CA_CERT_HASH) { return; } @@ -202,7 +232,6 @@ contract CertManager is ICertManager { bytes32 parentCertHash, bytes memory signatureHints ) internal returns (VerifiedCert memory) { - _requireNotRevoked(certHash); VerifiedCert memory parent; if (certHash != ROOT_CA_CERT_HASH) { parent = _loadVerified(parentCertHash); @@ -216,6 +245,7 @@ contract CertManager is ICertManager { // skip verification if already verified VerifiedCert memory cert = _loadVerified(certHash); if (cert.pubKey.length != 0) { + _requireNotRevoked(_revocationKey(certHash)); require(!_certificateExpired(cert.notAfter), "cert expired"); require(cert.ca == ca, "cert is not a CA"); if (certHash != ROOT_CA_CERT_HASH) { @@ -224,9 +254,13 @@ contract CertManager is ICertManager { return cert; } - cert = _verifyUncachedCert(certificate, ca, parent, signatureHints); + bytes32 identity; + (cert, identity) = _verifyUncachedCert(certificate, ca, parent, signatureHints); + // Reject by (issuer, serial) identity so a re-encoded twin of a revoked cert cannot pass. + _requireNotRevoked(identity); _saveVerified(certHash, cert); verifiedParent[certHash] = parentCertHash; + certIdentity[certHash] = identity; emit CertVerified(certHash); @@ -238,7 +272,7 @@ contract CertManager is ICertManager { bool ca, VerifiedCert memory parent, bytes memory signatureHints - ) internal view returns (VerifiedCert memory cert) { + ) internal view returns (VerifiedCert memory cert, bytes32 identity) { Asn1Ptr root = certificate.root(); require(root.totalLength() == certificate.length, "invalid cert length"); Asn1Ptr tbsCertPtr = certificate.firstChildOf(root); @@ -247,6 +281,8 @@ contract CertManager is ICertManager { require(parent.subjectHash == issuerHash, "issuer / subject mismatch"); + identity = _certIdentity(certificate, tbsCertPtr); + // constrain maxPathLen to parent's maxPathLen-1 if (parent.maxPathLen > 0 && (maxPathLen < 0 || maxPathLen >= parent.maxPathLen)) { maxPathLen = parent.maxPathLen - 1; @@ -259,6 +295,21 @@ contract CertManager is ICertManager { }); } + /// @dev Derives the (issuer, serial) revocation identity from a parsed certificate. The serial is + /// the second TBS field (after the explicit [0] version) and the issuer DN is the fourth + /// (after the inner signature algorithm); both lie inside the CA-signed TBS, so the identity + /// is fixed for any byte-encoding of the certificate that verifies. Mirrors the issuer-hash + /// derivation in `_parseTbsInner`. + function _certIdentity(bytes memory certificate, Asn1Ptr tbsCertPtr) internal pure returns (bytes32) { + Asn1Ptr versionPtr = certificate.firstChildOf(tbsCertPtr); + Asn1Ptr serialPtr = certificate.nextSiblingOf(versionPtr); + Asn1Ptr sigAlgoPtr = certificate.nextSiblingOf(serialPtr); + Asn1Ptr issuerPtr = certificate.nextSiblingOf(sigAlgoPtr); + bytes32 serialHash = certificate.keccak(serialPtr.content(), serialPtr.length()); + bytes32 issuerHash = certificate.keccak(issuerPtr.content(), issuerPtr.length()); + return keccak256(abi.encodePacked(issuerHash, serialHash)); + } + function _parseTbs(bytes memory certificate, Asn1Ptr ptr, bool ca) internal view diff --git a/test/hinted/HintedNitroAttestation.t.sol b/test/hinted/HintedNitroAttestation.t.sol index 339c6ff..f68f60a 100644 --- a/test/hinted/HintedNitroAttestation.t.sol +++ b/test/hinted/HintedNitroAttestation.t.sol @@ -256,7 +256,7 @@ contract HintedNitroAttestationTest is Test { (bytes memory caCert, bytes32 parentHash, bytes memory parentPubKey) = _firstNonRootCA(attestationTbs, ptrs); bytes memory hints = hintCollector.collectCertSignatureHints(caCert, parentPubKey); - certManager.revokeCert(keccak256(caCert)); + certManager.revokeCert(certManager.computeCertId(caCert)); vm.expectRevert("cert revoked"); certManager.verifyCACertWithHints(caCert, parentHash, hints); @@ -268,9 +268,9 @@ contract HintedNitroAttestationTest is Test { NitroValidator.Ptrs memory ptrs = parser.parseAttestation(attestationTbs); (bytes memory caCert, bytes32 parentHash, bytes memory parentPubKey) = _firstNonRootCA(attestationTbs, ptrs); bytes memory hints = hintCollector.collectCertSignatureHints(caCert, parentPubKey); - bytes32 caHash = certManager.verifyCACertWithHints(caCert, parentHash, hints); + certManager.verifyCACertWithHints(caCert, parentHash, hints); - certManager.revokeCert(caHash); + certManager.revokeCert(certManager.computeCertId(caCert)); vm.expectRevert("cert revoked"); certManager.verifyCACertWithHints(caCert, parentHash, ""); @@ -288,7 +288,7 @@ contract HintedNitroAttestationTest is Test { bytes32 ca1Hash = certManager.verifyCACertWithHints(ca1, rootHash, ca1Hints); bytes memory ca2 = attestationTbs.slice(ptrs.cabundle[2]); - certManager.revokeCert(ca1Hash); + certManager.revokeCert(certManager.computeCertId(ca1)); vm.expectRevert("cert revoked"); certManager.verifyCACertWithHints(ca2, ca1Hash, ""); @@ -341,7 +341,7 @@ contract HintedNitroAttestationTest is Test { bytes memory hash = Sha2Ext.sha384(attestationTbs, 0, attestationTbs.length); bytes memory attestationHints = hintCollector.collectVerifyHints(hash, signature, leaf.pubKey); - certManager.revokeCert(keccak256(attestationTbs.slice(ptrs.cert))); + certManager.revokeCert(certManager.computeCertId(attestationTbs.slice(ptrs.cert))); vm.expectRevert("cert revoked"); validator.validateAttestationWithHints(attestationTbs, signature, attestationHints); @@ -367,15 +367,38 @@ contract HintedNitroAttestationTest is Test { (bytes memory caCert, bytes32 parentHash, bytes memory parentPubKey) = _firstNonRootCA(attestationTbs, ptrs); bytes memory hints = hintCollector.collectCertSignatureHints(caCert, parentPubKey); bytes32 caHash = certManager.verifyCACertWithHints(caCert, parentHash, hints); + bytes32 caId = certManager.computeCertId(caCert); - certManager.revokeCert(caHash); + certManager.revokeCert(caId); vm.expectRevert("cert revoked"); certManager.verifyCACertWithHints(caCert, parentHash, ""); - certManager.unrevokeCert(caHash); + certManager.unrevokeCert(caId); assertEq(certManager.verifyCACertWithHints(caCert, parentHash, ""), caHash); } + /// @dev Revocation is keyed by the (issuer, serial) identity, not by keccak256(cert), so a cert + /// whose bytes differ only outside the CA-signed TBS — e.g. an ECDSA-malleable `(r, n-s)` + /// signature twin or a DER re-encoding of the signature — maps to the SAME revocation key + /// and cannot slip past a revocation. Here we mutate a signature byte to model that variant. + function test_RevocationIdentityIsInvariantToSignatureBytes() public view { + bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); + (bytes memory attestationTbs,) = validator.decodeAttestationTbs(attestation); + NitroValidator.Ptrs memory ptrs = parser.parseAttestation(attestationTbs); + (bytes memory caCert,,) = _firstNonRootCA(attestationTbs, ptrs); + + bytes memory variant = bytes.concat(caCert); + // Flip the last signature byte (inside the trailing s INTEGER, outside the TBS). + variant[variant.length - 1] = bytes1(uint8(variant[variant.length - 1]) ^ 0x01); + + assertTrue(keccak256(caCert) != keccak256(variant), "byte hashes differ"); + assertEq( + certManager.computeCertId(caCert), + certManager.computeCertId(variant), + "identity invariant to signature bytes" + ); + } + function test_HintedCACertRejectsExpiredCachedCert() public { bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); (bytes memory attestationTbs,) = validator.decodeAttestationTbs(attestation);