Skip to content

fix: validate certificate public key encoding #42

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
Open

fix: validate certificate public key encoding #42

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
43 changes: 24 additions & 19 deletions src/CertManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ contract CertManager is ICertManager {
using LibAsn1Ptr for Asn1Ptr;
using LibBytes for bytes;

error InvalidBasicConstraints();
error InvalidSubjectPublicKey();

event CertVerified(bytes32 indexed certHash);
event CertRevoked(bytes32 indexed certHash);
event CertUnrevoked(bytes32 indexed certHash);
Expand Down Expand Up @@ -400,17 +403,19 @@ contract CertManager is ICertManager {
Asn1Ptr subjectPublicKeyPtr = certificate.nextSiblingOf(pubKeyAlgoPtr);
Asn1Ptr subjectPubKeyPtr = certificate.bitstring(subjectPublicKeyPtr);

require(
certificate.keccak(pubKeyAlgoIdPtr.content(), pubKeyAlgoIdPtr.length()) == EC_PUB_KEY_OID,
"invalid cert algo id"
);
require(
certificate.keccak(algoParamsPtr.content(), algoParamsPtr.length()) == SECP_384_R1_OID,
"invalid cert algo param"
);
if (certificate.keccak(pubKeyAlgoIdPtr.content(), pubKeyAlgoIdPtr.length()) != EC_PUB_KEY_OID) {
revert InvalidSubjectPublicKey();
}
if (certificate.keccak(algoParamsPtr.content(), algoParamsPtr.length()) != SECP_384_R1_OID) {
revert InvalidSubjectPublicKey();
}

uint256 end = subjectPubKeyPtr.content() + subjectPubKeyPtr.length();
subjectPubKey = certificate.slice(end - 96, 96);
uint256 keyStart = subjectPubKeyPtr.content();
uint256 keyLength = subjectPubKeyPtr.length();
if (keyLength != 97 || keyStart + keyLength > certificate.length || certificate[keyStart] != 0x04) {
revert InvalidSubjectPublicKey();
}
subjectPubKey = certificate.slice(keyStart + 1, 96);
}

function _verifyValidity(bytes memory certificate, Asn1Ptr validityPtr) internal view returns (uint64 notAfter) {
Expand Down Expand Up @@ -481,7 +486,7 @@ contract CertManager is ICertManager {
pure
returns (int64 maxPathLen)
{
require(certificate[valuePtr.header()] == 0x30, "invalid basicConstraints");
if (certificate[valuePtr.header()] != 0x30) revert InvalidBasicConstraints();

maxPathLen = -1;
bool isCA;
Expand All @@ -493,37 +498,37 @@ contract CertManager is ICertManager {
cursor = _requireAsn1ChildWithin(basicConstraintsPtr, end);

if (certificate[basicConstraintsPtr.header()] == 0x01) {
require(basicConstraintsPtr.length() == 1, "invalid isCA bool value");
if (basicConstraintsPtr.length() != 1) revert InvalidBasicConstraints();
isCA = certificate[basicConstraintsPtr.content()] == 0xff;

if (cursor == end) {
require(ca == isCA, "isCA must be true for CA certs");
if (ca != isCA) revert InvalidBasicConstraints();
return maxPathLen;
}

basicConstraintsPtr = certificate.nextSiblingOf(basicConstraintsPtr);
cursor = _requireAsn1ChildWithin(basicConstraintsPtr, end);
}

require(ca == isCA, "isCA must be true for CA certs");
if (ca != isCA) revert InvalidBasicConstraints();

if (certificate[basicConstraintsPtr.header()] == 0x02) {
require(basicConstraintsPtr.length() > 0, "invalid pathLenConstraint");
if (basicConstraintsPtr.length() == 0) revert InvalidBasicConstraints();
maxPathLen = int64(uint64(certificate.uintAt(basicConstraintsPtr)));
} else {
revert("invalid basicConstraints field");
revert InvalidBasicConstraints();
}

require(cursor == end, "trailing basicConstraints fields");
if (cursor != end) revert InvalidBasicConstraints();
return maxPathLen;
}

require(ca == isCA, "isCA must be true for CA certs");
if (ca != isCA) revert InvalidBasicConstraints();
}

function _requireAsn1ChildWithin(Asn1Ptr ptr, uint256 parentEnd) internal pure returns (uint256 childEnd) {
childEnd = ptr.header() + ptr.totalLength();
require(childEnd <= parentEnd, "basicConstraints out of bounds");
if (childEnd > parentEnd) revert InvalidBasicConstraints();
}

function _verifyKeyUsageExtension(bytes memory certificate, Asn1Ptr valuePtr, bool ca) internal pure {
Expand Down
77 changes: 72 additions & 5 deletions test/CertManager.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,37 @@ contract CertManagerHarness is CertManager {
}
}

contract CertManagerPubKeyHarness is CertManager {
using Asn1Decode for bytes;

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

function parsePubKey(bytes memory subjectPublicKeyInfo) external pure returns (bytes memory) {
return _parsePubKey(subjectPublicKeyInfo, subjectPublicKeyInfo.root());
}

function parsePubKeyAt(bytes memory certificate, uint256 header, uint256 content, uint256 length)
external
pure
returns (bytes memory)
{
return _parsePubKey(certificate, LibAsn1Ptr.toAsn1Ptr(header, content, length));
}
}

contract CertManagerTest is Test {
using Asn1Decode for bytes;
using LibAsn1Ptr for Asn1Ptr;
using LibBytes for bytes;

Asn1DecodeHarness public harness;
CertManagerHarness public certManagerHarness;
CertManagerPubKeyHarness public certManagerPubKeyHarness;

function setUp() public {
harness = new Asn1DecodeHarness();
certManagerHarness = new CertManagerHarness();
certManagerPubKeyHarness = new CertManagerPubKeyHarness();
}

// 's' INTEGER from cabundle[3] (2026-04-02 attestation): DER-encoded with a 0x00
Expand All @@ -63,7 +83,7 @@ contract CertManagerTest is Test {
}

function test_BasicConstraintsEmptySequenceRejectsCACert() public {
vm.expectRevert("isCA must be true for CA certs");
vm.expectRevert(CertManager.InvalidBasicConstraints.selector);
certManagerHarness.verifyBasicConstraints(hex"3000", true);
}

Expand All @@ -76,25 +96,65 @@ contract CertManagerTest is Test {
}

function test_BasicConstraintsRejectsEmptyPathLen() public {
vm.expectRevert("invalid pathLenConstraint");
vm.expectRevert(CertManager.InvalidBasicConstraints.selector);
certManagerHarness.verifyBasicConstraints(hex"30050101ff0200", true);
}

function test_BasicConstraintsRejectsOutOfBoundsChild() public {
vm.expectRevert("basicConstraints out of bounds");
vm.expectRevert(CertManager.InvalidBasicConstraints.selector);
certManagerHarness.verifyBasicConstraints(hex"3003020200", false);
}

function test_BasicConstraintsRejectsTrailingFields() public {
vm.expectRevert("trailing basicConstraints fields");
vm.expectRevert(CertManager.InvalidBasicConstraints.selector);
certManagerHarness.verifyBasicConstraints(hex"30090101ff020100020100", true);
}

function test_BasicConstraintsRejectsUnknownField() public {
vm.expectRevert("invalid basicConstraints field");
vm.expectRevert(CertManager.InvalidBasicConstraints.selector);
certManagerHarness.verifyBasicConstraints(hex"30020400", false);
}

function test_ParsePubKeyAcceptsUncompressedP384Point() public view {
bytes memory pubKey = _patternBytes(96);
bytes memory spki = abi.encodePacked(hex"3076301006072a8648ce3d020106052b8104002203620004", pubKey);

assertEq(certManagerPubKeyHarness.parsePubKey(spki), pubKey);
}

function test_ParsePubKeyRejectsCompressedP384Point() public {
bytes memory compressedKey = _patternBytes(48);
bytes memory spki = abi.encodePacked(hex"3046301006072a8648ce3d020106052b8104002203320002", compressedKey);
bytes memory paddedCertificate = abi.encodePacked(new bytes(128), spki);

vm.expectRevert(CertManager.InvalidSubjectPublicKey.selector);
certManagerPubKeyHarness.parsePubKeyAt(paddedCertificate, 128, 130, 0x46);
}

function test_ParsePubKeyRejectsOversizedP384Point() public {
bytes memory oversizedKey = _patternBytes(97);
bytes memory spki = abi.encodePacked(hex"3077301006072a8648ce3d020106052b8104002203630004", oversizedKey);

vm.expectRevert(CertManager.InvalidSubjectPublicKey.selector);
certManagerPubKeyHarness.parsePubKey(spki);
}

function test_ParsePubKeyRejectsTruncatedP384Point() public {
bytes memory truncatedKey = _patternBytes(95);
bytes memory spki = abi.encodePacked(hex"3076301006072a8648ce3d020106052b8104002203620004", truncatedKey);

vm.expectRevert(CertManager.InvalidSubjectPublicKey.selector);
certManagerPubKeyHarness.parsePubKey(spki);
}

function test_ParsePubKeyRejectsMissingUncompressedPrefix() public {
bytes memory pubKey = _patternBytes(96);
bytes memory spki = abi.encodePacked(hex"3076301006072a8648ce3d020106052b8104002203620002", pubKey);

vm.expectRevert(CertManager.InvalidSubjectPublicKey.selector);
certManagerPubKeyHarness.parsePubKey(spki);
}

// 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 @@ -345,6 +405,13 @@ contract CertManagerTest is Test {

return der;
}

function _patternBytes(uint256 len) internal pure returns (bytes memory out) {
out = new bytes(len);
for (uint256 i = 0; i < len; i++) {
out[i] = bytes1(uint8(i + 1));
}
}
}

/// @dev Exposes the internal revocation-chain walk and lets tests seed the `verifiedParent`
Expand Down
Loading