diff --git a/src/CertManager.sol b/src/CertManager.sol index 1a5c0e2..bdd157d 100644 --- a/src/CertManager.sol +++ b/src/CertManager.sol @@ -453,18 +453,48 @@ contract CertManager is ICertManager { pure returns (int64 maxPathLen) { + require(certificate[valuePtr.header()] == 0x30, "invalid basicConstraints"); + maxPathLen = -1; - Asn1Ptr basicConstraintsPtr = certificate.firstChildOf(valuePtr); bool isCA; - if (certificate[basicConstraintsPtr.header()] == 0x01) { - require(basicConstraintsPtr.length() == 1, "invalid isCA bool value"); - isCA = certificate[basicConstraintsPtr.content()] == 0xff; - basicConstraintsPtr = certificate.nextSiblingOf(basicConstraintsPtr); + uint256 end = valuePtr.content() + valuePtr.length(); + uint256 cursor = valuePtr.content(); + + if (cursor < end) { + Asn1Ptr basicConstraintsPtr = certificate.firstChildOf(valuePtr); + cursor = _requireAsn1ChildWithin(basicConstraintsPtr, end); + + if (certificate[basicConstraintsPtr.header()] == 0x01) { + require(basicConstraintsPtr.length() == 1, "invalid isCA bool value"); + isCA = certificate[basicConstraintsPtr.content()] == 0xff; + + if (cursor == end) { + require(ca == isCA, "isCA must be true for CA certs"); + return maxPathLen; + } + + basicConstraintsPtr = certificate.nextSiblingOf(basicConstraintsPtr); + cursor = _requireAsn1ChildWithin(basicConstraintsPtr, end); + } + + require(ca == isCA, "isCA must be true for CA certs"); + + if (certificate[basicConstraintsPtr.header()] == 0x02) { + maxPathLen = int64(uint64(certificate.uintAt(basicConstraintsPtr))); + } else { + revert("invalid basicConstraints field"); + } + + require(cursor == end, "trailing basicConstraints fields"); + return maxPathLen; } + require(ca == isCA, "isCA must be true for CA certs"); - if (certificate[basicConstraintsPtr.header()] == 0x02) { - maxPathLen = int64(uint64(certificate.uintAt(basicConstraintsPtr))); - } + } + + function _requireAsn1ChildWithin(Asn1Ptr ptr, uint256 parentEnd) internal pure returns (uint256 childEnd) { + childEnd = ptr.header() + ptr.totalLength(); + require(childEnd <= parentEnd, "basicConstraints out of bounds"); } function _verifyKeyUsageExtension(bytes memory certificate, Asn1Ptr valuePtr, bool ca) internal pure { diff --git a/test/CertManager.t.sol b/test/CertManager.t.sol index 687e222..2d39e04 100644 --- a/test/CertManager.t.sol +++ b/test/CertManager.t.sol @@ -21,11 +21,23 @@ contract Asn1DecodeHarness { } } +contract CertManagerHarness is CertManager { + using Asn1Decode for bytes; + + constructor() CertManager(new P384Verifier()) {} + + function verifyBasicConstraints(bytes memory der, bool ca) external pure returns (int64) { + return _verifyBasicConstraintsExtension(der, der.root(), ca); + } +} + contract CertManagerTest is Test { Asn1DecodeHarness public harness; + CertManagerHarness public certManagerHarness; function setUp() public { harness = new Asn1DecodeHarness(); + certManagerHarness = new CertManagerHarness(); } // 's' INTEGER from cabundle[3] (2026-04-02 attestation): DER-encoded with a 0x00 @@ -41,6 +53,38 @@ contract CertManagerTest is Test { assertEq(lo, 0xa2eda9c549dc01460f5fe650814ebe0e7ee855d3bcffde95afd2e82e21df0eac); } + function test_BasicConstraintsEmptySequenceIsClientCert() public view { + assertEq(int256(certManagerHarness.verifyBasicConstraints(hex"3000", false)), -1); + } + + function test_BasicConstraintsEmptySequenceRejectsCACert() public { + vm.expectRevert("isCA must be true for CA certs"); + certManagerHarness.verifyBasicConstraints(hex"3000", true); + } + + function test_BasicConstraintsAcceptsCAWithoutPathLen() public view { + assertEq(int256(certManagerHarness.verifyBasicConstraints(hex"30030101ff", true)), -1); + } + + function test_BasicConstraintsAcceptsCAWithPathLen() public view { + assertEq(int256(certManagerHarness.verifyBasicConstraints(hex"30060101ff020100", true)), 0); + } + + function test_BasicConstraintsRejectsOutOfBoundsChild() public { + vm.expectRevert("basicConstraints out of bounds"); + certManagerHarness.verifyBasicConstraints(hex"3003020200", false); + } + + function test_BasicConstraintsRejectsTrailingFields() public { + vm.expectRevert("trailing basicConstraints fields"); + certManagerHarness.verifyBasicConstraints(hex"30090101ff020100020100", true); + } + + function test_BasicConstraintsRejectsUnknownField() public { + vm.expectRevert("invalid basicConstraints field"); + certManagerHarness.verifyBasicConstraints(hex"30020400", 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.