Skip to content

fix: reject unknown critical cert extensions #43

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
leanthebean wants to merge 1 commit into
base: main
Choose a base branch
from
+51 −8
Open

fix: reject unknown critical cert extensions #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions src/CertManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,16 +416,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) {
Expand All @@ -435,6 +435,8 @@ contract CertManager is ICertManager {
keyUsageFound = true;
_verifyKeyUsageExtension(certificate, valuePtr, ca);
}
} else {
require(!critical, "unsupported critical extension");
}

if (extensionPtr.content() + extensionPtr.length() == end) {
Expand Down
41 changes: 41 additions & 0 deletions test/CertManager.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,23 @@ contract Asn1DecodeHarness {
}
}

contract CertManagerHarness is CertManager {
using Asn1Decode for bytes;

constructor() CertManager(new P384Verifier()) {}

function verifyExtensions(bytes memory der, bool ca) external pure returns (int64) {
return _verifyExtensions(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
Expand All @@ -41,6 +53,25 @@ contract CertManagerTest is Test {
assertEq(lo, 0xa2eda9c549dc01460f5fe650814ebe0e7ee855d3bcffde95afd2e82e21df0eac);
}

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.
Expand Down Expand Up @@ -155,4 +186,14 @@ 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
);
}
}
Loading