fix: reject unknown critical cert extensions

Author: leanthebean

src/CertManager.sol

@@ -444,16 +444,16 @@ contract CertManager is ICertManager {
         while (true) {
             Asn1Ptr oidPtr = certificate.firstChildOf(extensionPtr);
             bytes32 oid = certificate.keccak(oidPtr.content(), oidPtr.length());
+            Asn1Ptr valuePtr = certificate.nextSiblingOf(oidPtr);
+            bool critical;
 
-            if (oid == BASIC_CONSTRAINTS_OID || oid == KEY_USAGE_OID) {
-                Asn1Ptr valuePtr = certificate.nextSiblingOf(oidPtr);
-
-                if (certificate[valuePtr.header()] == 0x01) {
-                    // skip optional critical bool
-                    require(valuePtr.length() == 1, "invalid critical bool value");
-                    valuePtr = certificate.nextSiblingOf(valuePtr);
-                }
+            if (certificate[valuePtr.header()] == 0x01) {
+                require(valuePtr.length() == 1, "invalid critical bool value");
+                critical = certificate[valuePtr.content()] != 0x00;
+                valuePtr = certificate.nextSiblingOf(valuePtr);
+            }
 
+            if (oid == BASIC_CONSTRAINTS_OID || oid == KEY_USAGE_OID) {
                 valuePtr = certificate.octetString(valuePtr);
 
                 if (oid == BASIC_CONSTRAINTS_OID) {
@@ -463,6 +463,8 @@ contract CertManager is ICertManager {
                     keyUsageFound = true;
                     _verifyKeyUsageExtension(certificate, valuePtr, ca);
                 }
+            } else {
+                require(!critical, "unsupported critical extension");
             }
 
             if (extensionPtr.content() + extensionPtr.length() == end) {

test/CertManager.t.sol

@@ -30,6 +30,10 @@ contract CertManagerHarness is CertManager {
     function verifyBasicConstraints(bytes memory der, bool ca) external pure returns (int64) {
         return _verifyBasicConstraintsExtension(der, der.root(), ca);
     }
+
+    function verifyExtensions(bytes memory der, bool ca) external pure returns (int64) {
+        return _verifyExtensions(der, der.root(), ca);
+    }
 }
 
 contract CertManagerTest is Test {
@@ -95,6 +99,25 @@ contract CertManagerTest is Test {
         certManagerHarness.verifyBasicConstraints(hex"30020400", false);
     }
 
+    function test_VerifyExtensionsAllowsUnknownNonCriticalExtension() public view {
+        bytes memory unknownNameConstraints = hex"30090603551d1e04023000";
+
+        assertEq(int256(certManagerHarness.verifyExtensions(_clientExtensionsWith(unknownNameConstraints), false)), -1);
+    }
+
+    function test_VerifyExtensionsAllowsUnknownCriticalFalseExtension() public view {
+        bytes memory unknownNameConstraints = hex"300c0603551d1e01010004023000";
+
+        assertEq(int256(certManagerHarness.verifyExtensions(_clientExtensionsWith(unknownNameConstraints), false)), -1);
+    }
+
+    function test_VerifyExtensionsRejectsUnknownCriticalExtension() public {
+        bytes memory unknownNameConstraints = hex"300c0603551d1e0101ff04023000";
+
+        vm.expectRevert("unsupported critical extension");
+        certManagerHarness.verifyExtensions(_clientExtensionsWith(unknownNameConstraints), false);
+    }
+
     // Cert chain from the 2026-04-02 ~15:35 UTC dev attestation that produced the live revert.
     // CB0 is the AWS Nitro root (keccak256(CB0) == CertManager.ROOT_CA_CERT_HASH, pinned in the
     // constructor), so the chain is verified starting from CB1.
@@ -345,6 +368,16 @@ contract CertManagerTest is Test {
 
         return der;
     }
+
+    function _clientExtensionsWith(bytes memory extraExtension) internal pure returns (bytes memory) {
+        bytes memory body =
+            abi.encodePacked(hex"300c0603551d130101ff04023000", hex"300e0603551d0f0101ff040403020780", extraExtension);
+
+        return
+            abi.encodePacked(
+                bytes1(0xa3), bytes1(uint8(body.length + 2)), bytes1(0x30), bytes1(uint8(body.length)), body
+            );
+    }
 }
 
 /// @dev Exposes the internal revocation-chain walk and lets tests seed the `verifiedParent`