diff --git a/src/CertManager.sol b/src/CertManager.sol index 29ecfd0..f7596ec 100644 --- a/src/CertManager.sol +++ b/src/CertManager.sol @@ -360,8 +360,10 @@ contract CertManager is ICertManager { function _certIdentity(bytes memory certificate, Asn1Ptr tbsCertPtr) internal pure returns (bytes32) { Asn1Ptr versionPtr = certificate.firstChildOf(tbsCertPtr); Asn1Ptr serialPtr = certificate.nextSiblingOf(versionPtr); + _requireAsn1Tag(certificate, serialPtr, 0x02); Asn1Ptr sigAlgoPtr = certificate.nextSiblingOf(serialPtr); Asn1Ptr issuerPtr = certificate.nextSiblingOf(sigAlgoPtr); + _requireAsn1Tag(certificate, issuerPtr, 0x30); bytes32 serialHash = certificate.keccak(serialPtr.content(), serialPtr.length()); bytes32 issuerHash = certificate.keccak(issuerPtr.content(), issuerPtr.length()); return keccak256(abi.encodePacked(issuerHash, serialHash)); @@ -375,7 +377,9 @@ contract CertManager is ICertManager { _requireAsn1Tag(certificate, ptr, 0x30); Asn1Ptr versionPtr = certificate.firstChildOf(ptr); _requireAsn1Tag(certificate, versionPtr, 0xa0); - Asn1Ptr sigAlgoPtr = certificate.nextSiblingOf(certificate.nextSiblingOf(versionPtr)); + Asn1Ptr serialPtr = certificate.nextSiblingOf(versionPtr); + _requireAsn1Tag(certificate, serialPtr, 0x02); + Asn1Ptr sigAlgoPtr = certificate.nextSiblingOf(serialPtr); _requireAsn1Tag(certificate, sigAlgoPtr, 0x30); if (certificate.keccak(sigAlgoPtr.content(), sigAlgoPtr.length()) != CERT_ALGO_OID) { @@ -432,6 +436,8 @@ contract CertManager is ICertManager { Asn1Ptr subjectPublicKeyPtr = certificate.nextSiblingOf(pubKeyAlgoPtr); Asn1Ptr subjectPubKeyPtr = certificate.bitstring(subjectPublicKeyPtr); + _requireAsn1Tag(certificate, pubKeyAlgoIdPtr, 0x06); + _requireAsn1Tag(certificate, algoParamsPtr, 0x06); if (certificate.keccak(pubKeyAlgoIdPtr.content(), pubKeyAlgoIdPtr.length()) != EC_PUB_KEY_OID) { revert InvalidSubjectPublicKey(); } @@ -482,6 +488,7 @@ contract CertManager is ICertManager { if (extensionEnd > end) revert InvalidExtension(); _requireAsn1Tag(certificate, extensionPtr, 0x30); Asn1Ptr oidPtr = certificate.firstChildOf(extensionPtr); + _requireAsn1Tag(certificate, oidPtr, 0x06); bytes32 oid = certificate.keccak(oidPtr.content(), oidPtr.length()); bool recognized = oid == BASIC_CONSTRAINTS_OID || oid == KEY_USAGE_OID; diff --git a/test/CertManager.t.sol b/test/CertManager.t.sol index 6db1293..32d206b 100644 --- a/test/CertManager.t.sol +++ b/test/CertManager.t.sol @@ -166,6 +166,49 @@ contract CertManagerTest is Test { cm.verifyCACertWithHints(mutated, keccak256(CB0), ""); } + function test_ParseTbsRejectsSerialTagSubstitution() public { + vm.warp(1775145600); + CertManager cm = new CertManager(new P384Verifier()); + + bytes memory mutated = bytes.concat(CB1); + Asn1Ptr root = mutated.root(); + Asn1Ptr tbsPtr = mutated.firstChildOf(root); + Asn1Ptr versionPtr = mutated.firstChildOf(tbsPtr); + Asn1Ptr serialPtr = mutated.nextSiblingOf(versionPtr); + mutated[serialPtr.header()] = 0x04; + + vm.expectRevert(CertManager.InvalidAsn1Tag.selector); + cm.verifyCACertWithHints(mutated, keccak256(CB0), ""); + } + + function test_ComputeCertIdRejectsSerialTagSubstitution() public { + bytes memory mutated = bytes.concat(CB1); + Asn1Ptr root = mutated.root(); + Asn1Ptr tbsPtr = mutated.firstChildOf(root); + Asn1Ptr versionPtr = mutated.firstChildOf(tbsPtr); + Asn1Ptr serialPtr = mutated.nextSiblingOf(versionPtr); + mutated[serialPtr.header()] = 0x04; + + CertManager cm = new CertManager(new P384Verifier()); + vm.expectRevert(CertManager.InvalidAsn1Tag.selector); + cm.computeCertId(mutated); + } + + function test_ComputeCertIdRejectsIssuerTagSubstitution() public { + bytes memory mutated = bytes.concat(CB1); + Asn1Ptr root = mutated.root(); + Asn1Ptr tbsPtr = mutated.firstChildOf(root); + Asn1Ptr versionPtr = mutated.firstChildOf(tbsPtr); + Asn1Ptr serialPtr = mutated.nextSiblingOf(versionPtr); + Asn1Ptr sigAlgoPtr = mutated.nextSiblingOf(serialPtr); + Asn1Ptr issuerPtr = mutated.nextSiblingOf(sigAlgoPtr); + mutated[issuerPtr.header()] = 0x31; + + CertManager cm = new CertManager(new P384Verifier()); + vm.expectRevert(CertManager.InvalidAsn1Tag.selector); + cm.computeCertId(mutated); + } + function test_ParsePubKeyAcceptsUncompressedP384Point() public view { bytes memory pubKey = _patternBytes(96); bytes memory spki = abi.encodePacked(hex"3076301006072a8648ce3d020106052b8104002203620004", pubKey); @@ -205,6 +248,40 @@ contract CertManagerTest is Test { certManagerPubKeyHarness.parsePubKey(spki); } + function test_ParsePubKeyRejectsAlgorithmOidTagSubstitution() public { + bytes memory pubKey = _patternBytes(96); + bytes memory mutated = abi.encodePacked(hex"3076301006072a8648ce3d020106052b8104002203620004", pubKey); + Asn1Ptr algorithmPtr = mutated.firstChildOf(mutated.root()); + Asn1Ptr algorithmOidPtr = mutated.firstChildOf(algorithmPtr); + mutated[algorithmOidPtr.header()] = 0x05; + + vm.expectRevert(CertManager.InvalidAsn1Tag.selector); + certManagerPubKeyHarness.parsePubKey(mutated); + } + + function test_ParsePubKeyRejectsCurveOidTagSubstitution() public { + bytes memory pubKey = _patternBytes(96); + bytes memory mutated = abi.encodePacked(hex"3076301006072a8648ce3d020106052b8104002203620004", pubKey); + Asn1Ptr algorithmPtr = mutated.firstChildOf(mutated.root()); + Asn1Ptr algorithmOidPtr = mutated.firstChildOf(algorithmPtr); + Asn1Ptr curveOidPtr = mutated.nextSiblingOf(algorithmOidPtr); + mutated[curveOidPtr.header()] = 0x05; + + vm.expectRevert(CertManager.InvalidAsn1Tag.selector); + certManagerPubKeyHarness.parsePubKey(mutated); + } + + function test_VerifyExtensionsRejectsOidTagSubstitution() public { + bytes memory mutated = _extensions(bytes.concat(_basicConstraintsExtension(), _keyUsageExtension())); + Asn1Ptr extensionSequencePtr = mutated.firstChildOf(mutated.root()); + Asn1Ptr extensionPtr = mutated.firstChildOf(extensionSequencePtr); + Asn1Ptr oidPtr = mutated.firstChildOf(extensionPtr); + mutated[oidPtr.header()] = 0x05; + + vm.expectRevert(CertManager.InvalidAsn1Tag.selector); + certManagerExtensionsHarness.verifyExtensions(mutated, true); + } + function test_VerifyExtensionsAllowsUnknownNonCriticalExtension() public view { bytes memory unknownNameConstraints = hex"30090603551d1e04023000";