Merge branch 'DEV-3731' into development

preston4896 · preston4896 · commit 33baea43c6cb · 2025-02-14T13:34:09.000+08:00
diff --git a/src/bases/DaoBase.sol b/src/bases/DaoBase.sol
@@ -12,6 +12,9 @@ import "../interfaces/IDaoAttestationResolver.sol";
 abstract contract DaoBase {
     IDaoAttestationResolver public immutable resolver;
 
+    // 72bd8361
+    error Duplicate_Collateral();
+
     constructor(address _resolver) {
         resolver = IDaoAttestationResolver(_resolver);
     }
@@ -58,6 +61,22 @@ abstract contract DaoBase {
         return _fetchDataFromResolver(key, hash);
     }
 
+    /**
+     * @notice check whether the hash for the provided collateral already exists in the PCCS
+     * @param key - the key to locate the collateral attestation
+     * @param hash - the hash of the collateral
+     */
+    function _checkCollateralDuplicate(bytes32 key, bytes32 hash) internal view {
+        // if a matching hash is found, that means the caller is attempting to re-upsert duplicate collateral
+        bytes memory existingHashData = _fetchDataFromResolver(key, true);
+        if (existingHashData.length > 0) {
+            bytes32 existingHash = abi.decode(existingHashData, (bytes32));
+            if (existingHash == hash) {
+                revert Duplicate_Collateral();
+            }
+        }
+    }
+
     /// @dev https://github.com/Vectorized/solady/blob/4964e3e2da1bc86b0394f63a90821f51d60a260b/src/utils/JSONParserLib.sol#L339-L364
     /// @dev Parses an unsigned integer from a string (in hexadecimal, i.e. base 16).
     /// Reverts if `s` is not a valid uint256 hex string matching the RegEx
diff --git a/src/bases/EnclaveIdentityDao.sol b/src/bases/EnclaveIdentityDao.sol
@@ -89,9 +89,13 @@ abstract contract EnclaveIdentityDao is DaoBase, SigVerifyBase {
         external
         returns (bytes32 attestationId)
     {
-        _validateQeIdentity(enclaveIdentityObj);
-        (bytes32 key, bytes memory req) = _buildEnclaveIdentityAttestationRequest(id, version, enclaveIdentityObj);
+        bytes32 key = ENCLAVE_ID_KEY(id, version);
         bytes32 hash = sha256(bytes(enclaveIdentityObj.identityStr));
+
+        _checkCollateralDuplicate(key, hash);
+
+        _validateQeIdentity(enclaveIdentityObj, hash);
+        bytes memory req = _buildEnclaveIdentityAttestationRequest(id, version, key, enclaveIdentityObj);
         attestationId = _attestEnclaveIdentity(req, hash, key);
 
         emit UpsertedEnclaveIdentity(id, version);
@@ -125,8 +129,9 @@ abstract contract EnclaveIdentityDao is DaoBase, SigVerifyBase {
     function _buildEnclaveIdentityAttestationRequest(
         uint256 id,
         uint256 version,
+        bytes32 key,
         EnclaveIdentityJsonObj calldata enclaveIdentityObj
-    ) private view returns (bytes32 key, bytes memory reqData) {
+    ) private view returns (bytes memory reqData) {
         IdentityObj memory identity = EnclaveIdentityLib.parseIdentityString(enclaveIdentityObj.identityStr);
         if (id != uint256(identity.id)) {
             revert Enclave_Id_Mismatch();
@@ -141,13 +146,12 @@ abstract contract EnclaveIdentityDao is DaoBase, SigVerifyBase {
         }
 
         // make sure new collateral is "newer"
-        key = ENCLAVE_ID_KEY(id, version);
-        bytes memory existingData = _onFetchDataFromResolver(key, false);
+        bytes memory existingData = _fetchDataFromResolver(key, false);
         if (existingData.length > 0) {
             (IdentityObj memory existingIdentity, , ) =
                 abi.decode(existingData, (IdentityObj, string, bytes));
             bool outOfDate = existingIdentity.tcbEvaluationDataNumber > identity.tcbEvaluationDataNumber ||
-                existingIdentity.issueDateTimestamp > identity.issueDateTimestamp;
+                existingIdentity.issueDateTimestamp >= identity.issueDateTimestamp;
             if (outOfDate) {
                 revert Enclave_Id_Out_Of_Date();
             }
@@ -159,12 +163,12 @@ abstract contract EnclaveIdentityDao is DaoBase, SigVerifyBase {
     /**
      * @notice validates IdentityString is signed by Intel TCB Signing Cert
      */
-    function _validateQeIdentity(EnclaveIdentityJsonObj calldata enclaveIdentityObj) private view {
+    function _validateQeIdentity(EnclaveIdentityJsonObj calldata enclaveIdentityObj, bytes32 hash) private view {
         bytes memory signingDer = _fetchDataFromResolver(Pcs.PCS_KEY(CA.SIGNING, false), false);
 
         // Validate signature
         bool sigVerified =
-            verifySignature(sha256(bytes(enclaveIdentityObj.identityStr)), enclaveIdentityObj.signature, signingDer);
+            verifySignature(hash, enclaveIdentityObj.signature, signingDer);
 
         if (!sigVerified) {
             revert Invalid_TCB_Cert_Signature();
diff --git a/src/bases/FmspcTcbDao.sol b/src/bases/FmspcTcbDao.sol
@@ -101,20 +101,40 @@ abstract contract FmspcTcbDao is DaoBase, SigVerifyBase {
      * @param tcbInfoObj See {FmspcTcbHelper.sol} to learn more about the structure definition
      */
     function upsertFmspcTcb(TcbInfoJsonObj calldata tcbInfoObj) external returns (bytes32 attestationId) {
-        _validateTcbInfo(tcbInfoObj);
-        (
-            bytes memory req, 
-            bytes32 key, 
-            uint8 tcbId, 
-            bytes6 fmspc, 
-            uint32 version, 
-            uint64 issueDateTimestamp,
-            uint32 evaluationDataNumber
-        ) = _buildTcbAttestationRequest(tcbInfoObj);
         bytes32 hash = sha256(bytes(tcbInfoObj.tcbInfoStr));
+
+        // parse tcb info basic here so we can compute the key
+        (
+            TcbInfoBasic memory tcbInfo,
+            string memory tcbLevelsString,
+            string memory tdxModuleString,
+            string memory tdxModuleIdentitiesString
+        ) = FmspcTcbLib.parseTcbString(tcbInfoObj.tcbInfoStr);
+        bytes32 key = FMSPC_TCB_KEY(uint8(tcbInfo.id), tcbInfo.fmspc, tcbInfo.version);
+        
+        _checkCollateralDuplicate(key, hash);
+        _validateTcbInfo(tcbInfoObj);
+
+        bytes memory req = _buildTcbAttestationRequest(
+            key,
+            tcbInfoObj,
+            tcbInfo,
+            tcbLevelsString,
+            tdxModuleString,
+            tdxModuleIdentitiesString
+        );
+
         attestationId = _attestTcb(req, hash, key);
-        _storeTcbInfoIssueEvaluation(key, issueDateTimestamp, evaluationDataNumber);
-        emit UpsertedFmpscTcb(tcbId, fmspc, version);
+        _storeTcbInfoIssueEvaluation(
+            key, 
+            tcbInfo.issueDate, 
+            tcbInfo.evaluationDataNumber
+        );
+        emit UpsertedFmpscTcb(
+            uint8(tcbInfo.id),
+            tcbInfo.fmspc,
+            tcbInfo.version
+        );
     }
 
     /**
@@ -142,55 +162,35 @@ abstract contract FmspcTcbDao is DaoBase, SigVerifyBase {
     /**
      * @notice constructs the TcbInfo.json attestation data
      */
-    function _buildTcbAttestationRequest(TcbInfoJsonObj calldata tcbInfoObj)
+    function _buildTcbAttestationRequest(
+        bytes32 key,
+        TcbInfoJsonObj calldata tcbInfoObj,
+        TcbInfoBasic memory tcbInfo,
+        string memory tcbLevelsString,
+        string memory tdxModuleString,
+        string memory tdxModuleIdentitiesString
+    )
         private
         view
-        returns 
-        (
-            bytes memory reqData, 
-            bytes32 key, 
-            uint8 id,
-            bytes6 fmspc, 
-            uint32 version,
-            uint64 issueDateTimestamp,
-            uint32 evaluationDataNumber
-        )
+        returns (bytes memory reqData)
     {
-        TcbInfoBasic memory tcbInfo;
-
-        string memory tcbLevelsString;
-        string memory tdxModuleString;
-        string memory tdxModuleIdentitiesString;
-        (
-            tcbInfo, 
-            tcbLevelsString, 
-            tdxModuleString, 
-            tdxModuleIdentitiesString
-        ) = FmspcTcbLib.parseTcbString(tcbInfoObj.tcbInfoStr);
-
         // check expiration before continuing...
         if (block.timestamp < tcbInfo.issueDate || block.timestamp > tcbInfo.nextUpdate) {
             revert TCB_Expired();
         }
 
         // Make sure new collateral is "newer"
-        id = uint8(tcbInfo.id);
-        fmspc = tcbInfo.fmspc;
-        version = tcbInfo.version;
-        key = FMSPC_TCB_KEY(id, fmspc, version);
         (uint64 existingIssueDate, uint32 existingEvaluationDataNumber) = _loadTcbInfoIssueEvaluation(key);
         if (existingIssueDate > 0) {
             /// I don't think there can be a scenario where an existing tcbinfo with a higher evaluation data number
             /// to be issued BEFORE a new tcbinfo with a lower evaluation data number
             bool outOfDate = tcbInfo.evaluationDataNumber < existingEvaluationDataNumber ||
-                tcbInfo.issueDate < existingIssueDate;
+                tcbInfo.issueDate <= existingIssueDate;
             if (outOfDate) {
                 revert TCB_Out_Of_Date();
             }
         }
 
-        issueDateTimestamp = tcbInfo.issueDate;
-        evaluationDataNumber = tcbInfo.evaluationDataNumber;
         TCBLevelsObj[] memory tcbLevels = FmspcTcbLib.parseTcbLevels(tcbInfo.version, tcbLevelsString);
         bytes memory encodedTcbLevels = _encodeTcbLevels(tcbLevels);
         if (tcbInfo.version < 3) {
diff --git a/src/bases/PckDao.sol b/src/bases/PckDao.sol
@@ -277,27 +277,30 @@ abstract contract PckDao is DaoBase, SigVerifyBase {
     function _validatePck(CA ca, bytes memory der, bytes16 qeid, bytes2 pceid, bytes18 tcbm) internal view returns (bytes32 hash, bytes32 key) {
         X509CertObj memory pck = pckLib.parseX509DER(der);
         
-        // Step 0: Check whether the pck has expired
+        hash = keccak256(pck.tbs);
+        key = PCK_KEY(qeid, pceid, tcbm);
+
+        // Step 0: Check whether the certificate has been previously attested
+        _checkCollateralDuplicate(key, hash);
+
+        // Step 1: Check whether the pck has expired
         bool notExpired = block.timestamp > pck.validityNotBefore && block.timestamp < pck.validityNotAfter;
         if (!notExpired) {
             revert Certificate_Expired();
         }
 
-        hash = keccak256(pck.tbs);
-        key = PCK_KEY(qeid, pceid, tcbm);
-
-        // Step 1: Rollback prevention: new certificate should not have an issued date
+        // Step 2: Rollback prevention: new certificate should not have an issued date
         // that is older than the existing certificate
         bytes memory existingData = _fetchDataFromResolver(key, false);
         if (existingData.length > 0) {
             (uint256 existingCertNotValidBefore, ) = pckLib.getCertValidity(existingData);
-            bool outOfDate = existingCertNotValidBefore > pck.validityNotBefore;
+            bool outOfDate = existingCertNotValidBefore >= pck.validityNotBefore;
             if (outOfDate) {
                 revert Pck_Out_Of_Date();
             }
         }
 
-        // Step 2: Check Issuer and Subject names
+        // Step 3: Check Issuer and Subject names
         string memory expectedIssuer;
         if (ca == CA.PLATFORM) {
             expectedIssuer = PCK_PLATFORM_CA_COMMON_NAME;
@@ -311,10 +314,10 @@ abstract contract PckDao is DaoBase, SigVerifyBase {
             revert Invalid_Subject_Name();
         }
 
-        // Step 3: validate PCEID and TCBm
+        // Step 4: validate PCEID and TCBm
         _validatePckTcb(pceid, tcbm, der, pck.extensionPtr);
 
-        // Step 4: Check whether the pck has been revoked
+        // Step 5: Check whether the pck has been revoked
         bytes memory crlData = _fetchDataFromResolver(Pcs.PCS_KEY(ca, true), false);
         if (crlData.length > 0) {
             bool revocable = crlLib.serialNumberIsRevoked(pck.serialNumber, crlData);
@@ -323,7 +326,7 @@ abstract contract PckDao is DaoBase, SigVerifyBase {
             }
         }
 
-        // Step 5: Check signature
+        // Step 6: Check signature
         bytes memory issuerCert = _fetchDataFromResolver(Pcs.PCS_KEY(ca, false), false);
         if (issuerCert.length > 0) {
             bytes32 digest = sha256(pck.tbs);
diff --git a/src/bases/PcsDao.sol b/src/bases/PcsDao.sol
@@ -142,6 +142,7 @@ abstract contract PcsDao is DaoBase, SigVerifyBase {
 
     function _upsertPcsCrl(CA ca, bytes calldata crl) private returns (bytes32 attestationId) {
         (bytes32 hash, bytes32 key) = _validatePcsCrl(ca, crl);
+
         attestationId = _attestPcs(crl, hash, key);
 
         emit UpsertedPCSCollateral(ca, true);
@@ -151,6 +152,12 @@ abstract contract PcsDao is DaoBase, SigVerifyBase {
         X509Helper x509Lib = X509Helper(x509);
         X509CertObj memory currentCert = x509Lib.parseX509DER(cert);
 
+        key = PCS_KEY(ca, false);
+        hash = keccak256(currentCert.tbs);
+
+        // Step 0: Check whether the provided certificate has been previously attested
+        _checkCollateralDuplicate(key, hash);
+
         // Step 1: Check whether cert has expired
         bool validTimestamp = 
             block.timestamp > currentCert.validityNotBefore && 
@@ -161,11 +168,10 @@ abstract contract PcsDao is DaoBase, SigVerifyBase {
 
         // Step 2: Rollback prevention: new certificate should not have an issued date
         // that is older than the existing certificate
-        key = PCS_KEY(ca, false);
         bytes memory existingData = _fetchDataFromResolver(key, false);
         if (existingData.length > 0) {
             (uint256 existingCertNotValidBefore, ) = x509Lib.getCertValidity(existingData);
-            bool outOfDate = existingCertNotValidBefore > currentCert.validityNotBefore;
+            bool outOfDate = existingCertNotValidBefore >= currentCert.validityNotBefore;
             if (outOfDate) {
                 revert Certificate_Out_Of_Date();
             }
@@ -222,12 +228,15 @@ abstract contract PcsDao is DaoBase, SigVerifyBase {
         if (!sigVerified) {
             revert Invalid_Signature();
         }
-
-        hash = keccak256(currentCert.tbs);
     }
 
     function _validatePcsCrl(CA ca, bytes calldata crl) private view returns (bytes32 hash, bytes32 key) {
         X509CRLObj memory currentCrl = crlLib.parseCRLDER(crl);
+
+        key = PCS_KEY(ca, true);
+        hash = keccak256(currentCrl.tbs);
+
+        _checkCollateralDuplicate(key, hash);
         
         // Step 1: Check whether CRL has expired
         bool validTimestamp = 
@@ -239,11 +248,10 @@ abstract contract PcsDao is DaoBase, SigVerifyBase {
 
         // Step 2: Rollback prevention: new CRL should not have an issued date
         // that is older than the existing CRL
-        key = PCS_KEY(ca, true);
         bytes memory existingData = _fetchDataFromResolver(key, false);
         if (existingData.length > 0) {
             (uint256 existingCrlNotValidBefore, ) = crlLib.getCrlValidity(existingData);
-            bool outOfDate = existingCrlNotValidBefore > currentCrl.validityNotBefore;
+            bool outOfDate = existingCrlNotValidBefore >= currentCrl.validityNotBefore;
             if (outOfDate) {
                 revert Certificate_Out_Of_Date();
             }
@@ -266,8 +274,6 @@ abstract contract PcsDao is DaoBase, SigVerifyBase {
         if (!sigVerified) {
             revert Invalid_Signature();
         }
-
-        hash = keccak256(currentCrl.tbs);
     }
 
     function _getIssuer(CA ca) private view returns (bytes memory issuerCert) {
diff --git a/test/identity/AutomataEnclaveIdentityTest.t.sol b/test/identity/AutomataEnclaveIdentityTest.t.sol
@@ -4,6 +4,7 @@ pragma solidity ^0.8.0;
 import "../pcs/PCSSetupBase.t.sol";
 import "./IdentityConstants.t.sol";
 import {AutomataEnclaveIdentityDao} from "../../src/automata_pccs/AutomataEnclaveIdentityDao.sol";
+import {DaoBase} from "../../src/bases/DaoBase.sol";
 
 contract AutomataEnclaveIdentityDaoTest is PCSSetupBase, IdentityConstants {
     function setUp() public override {
@@ -24,6 +25,12 @@ contract AutomataEnclaveIdentityDaoTest is PCSSetupBase, IdentityConstants {
         EnclaveIdentityJsonObj memory fetched = enclaveIdDao.getEnclaveIdentity(id, version);
         assertEq(fetched.signature, enclaveIdentityObj.signature);
         assertEq(keccak256(bytes(fetched.identityStr)), keccak256(bytes(enclaveIdentityObj.identityStr)));
+    
+        // duplicate check
+        vm.expectRevert(abi.encodeWithSelector(
+            DaoBase.Duplicate_Collateral.selector
+        ));
+        enclaveIdDao.upsertEnclaveIdentity(id, version, enclaveIdentityObj);
     }
 
     function testTcbIssuerChain() public readAsAuthorizedCaller {
diff --git a/test/pcs/AutomataPckDaoTest.t.sol b/test/pcs/AutomataPckDaoTest.t.sol
@@ -3,6 +3,7 @@ pragma solidity ^0.8.0;
 
 import "../pcs/PCSSetupBase.t.sol";
 import {AutomataPckDao} from "../../src/automata_pccs/AutomataPckDao.sol";
+import {DaoBase} from "../../src/bases/DaoBase.sol";
 
 contract AutomataPckDaoTest is PCSSetupBase {
     // TEMP: placeholder only, circle back on this to verify the inputs
@@ -38,6 +39,13 @@ contract AutomataPckDaoTest is PCSSetupBase {
         assertEq(keccak256(bytes(fetchedTcbm)), keccak256(bytes(tcbm)));
     }
 
+    function testDuplicatePckUpsert() public {
+        vm.expectRevert(abi.encodeWithSelector(
+            DaoBase.Duplicate_Collateral.selector
+        ));
+        pck.upsertPckCert(CA.PLATFORM, qeid, pceid, tcbm, pckDer);
+    }
+
     function testPckIssuerChain() public readAsAuthorizedCaller {
         (bytes memory intermediateCert, bytes memory rootCert) = pck.getPckCertChain(CA.PLATFORM);
         assertEq(keccak256(platformDer), keccak256(intermediateCert));
diff --git a/test/pcs/AutomataPcsDaoTest.t.sol b/test/pcs/AutomataPcsDaoTest.t.sol
diff --git a/test/tcb/AutomataTcbDaoTest.t.sol b/test/tcb/AutomataTcbDaoTest.t.sol
