Coinbase: NitroEnclaveVerifier

Description

The ZK prover verifies that the certificate chain is valid at the attestation timestamp (doc.timestamp / 1000). TEEProverRegistry then allows attestations up to MAX_AGE = 60 minutes old. This means a certificate that expires at time T can still produce valid attestations until T + 60 minutes, because:

1. An attestation generated at time T - 1 (just before expiry) has a valid cert chain
2. This attestation can be submitted to the contract up to T + 59min59s

In practice, this is acceptable since certificate private keys are unlikely to be compromised within 1 hour of expiration.

Note: This issue was reported in a previous audit and was fixed by documenting the edge-case. However, the new journal.certExpiries field allows the NitroEnclaveVerifier to now check all the certificate timestamp to ensure that they are still valid.

Recommendation

Consider ensuring that block.timestamp is lower than or equal to certificate timestamps in journal.certExpiries for all not-yet-trusted certificates.

This can be implemented in _verifyJournal with the following patch.

diff --git a/src/multiproof/tee/NitroEnclaveVerifier.sol b/src/multiproof/tee/NitroEnclaveVerifier.solindex 0a624380..684012ce 100644--- a/src/multiproof/tee/NitroEnclaveVerifier.sol+++ b/src/multiproof/tee/NitroEnclaveVerifier.sol@@ -619,6 +619,14 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier, ISemver {                 return journal;             }         }+        // Check any remaining certificates in the chain that are not yet trusted+        for (uint256 i = journal.trustedCertsPrefixLen; i < journal.certs.length; i++) {+            uint64 expiry = journal.certExpiries[i];+            if (block.timestamp > expiry) {+                journal.result = VerificationResult.InvalidTimestamp;+                return journal;+            }+        }         uint64 timestamp = journal.timestamp / 1000;         if (timestamp + maxTimeDiff <= block.timestamp || timestamp >= block.timestamp) {             journal.result = VerificationResult.InvalidTimestamp;

Coinbase

Fixed in PR251.

Cantina

Fixed. The patch has been applied.

Description

The revokeCert() function allows the revoker and the owner to revoke a specific certHash by deleting it from the trustedIntermediateCerts mapping.

However, this function does not allow to revoke the child certificates that depend on the certHash being revoked. This may lead to inconsistencies where child certificates are still marked as trusted.

Note: This has no impact thanks to the rootCert check in _verifyJournal. An untrusted intermediate certificate will required all its child certificates to go through a new certificate verification.

Recommendation

Revoking child certificates could be handled by tracking the certificate path dependency on-chain.

Alternatively, this can be handled off-chain by the revoker.

Coinbase

Acknowledged.

Cantina

Acknowledged.

Description

The codebase shows multiple outdated comments:

• INitroEnclaveVerifier.sol#L28-L29: "Note: This struct stores the "latest" (active) program identifiers. Multiple versions can be supported simultaneously via the version management functions."
• INitroEnclaveVerifier.sol#L141: "Multi-version program support for seamless upgrades"
• NitroEnclaveVerifier.sol#L323: "Note: Program IDs are automatically added to the supported version sets"

These comments are not valid anymore as recent changes removed the support of multiple program identifiers. Now, a single configuration is accepted instead of a set of configurations.

Recommendation

Modify the comments to match the new behavior of the codebase.

Coinbase

Fixed in PR251.

Cantina

Fixed. The comments have been removed.

Description

The checkTrustedIntermediateCerts function may revert with an Index out of bounds error when the input certs length is zero.

This is because the code expects at least one element by reading certs[0].

Recommendation

While the code is not vulnerable, reverting with a dedicated error on zero length can be better for debugging.

Coinbase

Acknowledged.

Cantina

Acknowledged.

Description

The new revokeCert functionality allows the revoker and the owner to revoke specific certificates. The natspec comments specifically indicate the following.

/**     * @dev Revokes a trusted intermediate certificate     * @param certHash Hash of the certificate to revoke     *     * Requirements:     * - Only callable by contract owner or revoker     * - Certificate must exist in the trusted intermediate certificates set     *     * This function allows the owner or revoker to revoke compromised intermediate certificates     * without affecting the root certificate or other trusted certificates.     */

However, this revocation mechanism is not a "blocklist". This means that a compromised intermediate certificate can be revoked through revokeCert and then marked as trusted again through a ZK proof verification.

This is mainly due to the fact the Certification Revocation Lists (CRL) are not supported by the zkVM program.

Recommendation

Trusted certificates can only be added by a trusted party, this reduces the impact of this issue.

Consider adding comments to mention the fact that revoked certificates can be trusted again.

Coinbase

Fixed in PR251.

Cantina

Fixed. The edge-case has been documented.