Add validation for unique KeyIds in certificate chain

Author: kduma

SPECIFICATION.md

@@ -120,6 +120,8 @@ Reminder: This matrix validates only the end‑entity subset requirement. The is
 Notes
 - A certificate with `ROOT_CA` must be self‑signed, but it may also carry `INTERMEDIATE_CA` or `CA` (or both).
 - The presence of CA‑level flags does not prevent a certificate from also carrying end‑entity flags; those end‑entity bits govern what end‑entity flags it may delegate to subjects, not necessarily whether it acts as an end‑entity itself.
+- **Certificate uniqueness**: Each certificate in a chain must have a unique `KeyId`. Self‑signed `ROOT_CA` certificates can only appear as the final (root) certificate in a chain, never in the middle.
+- **End‑entity inheritance**: The subset rule (`Child.EndEntity ⊆ Issuer.EndEntity`) applies to all certificate pairs in a chain without exception, including self‑signed certificates.
 
 ---
 
@@ -128,12 +130,13 @@ Notes
 1. Verify structure and lengths.
 2. Ensure each certificate has at least one signature.
 3. Compute `KeyId` as `SHA-256(PubKey)[0..15]` and verify it matches the embedded value.
-4. Build a path from leaf to a trusted root by matching `SignKeyId` to parent `KeyId`.
-5. For each child/parent pair (issuer = parent):
+4. Verify that all certificates in the chain have unique `KeyId` values. No two certificates in the same chain may share the same `KeyId`.
+5. Build a path from leaf to a trusted root by matching `SignKeyId` to parent `KeyId`.
+6. For each child/parent pair (issuer = parent):
     - For non-CA children: Issuer must have `CA`.
     - For CA-level children (has any of `ROOT_CA`, `INTERMEDIATE_CA`, `CA`): issuer must have `INTERMEDIATE_CA`.
-    - End‑entity inheritance: For each end‑entity bit (0x0100 through 0x8000), if child has it, issuer must also have it (`Child.EndEntity ⊆ Issuer.EndEntity`).
-6. A certificate with `ROOT_CA` must be self‑signed and present in the trust store.
+    - End‑entity inheritance: For each end‑entity bit (0x0100 through 0x8000), if child has it, issuer must also have it (`Child.EndEntity ⊆ Issuer.EndEntity`). This validation applies to **all** certificate pairs without exception.
+7. A certificate with `ROOT_CA` must be self‑signed and present in the trust store.
 
 ---
 

src/Validator.php

@@ -114,6 +114,18 @@ private static function validatePath(array $path, TrustStore $store): array
             }
         }
 
+        // Verify that all certificates in the path have unique KeyIds
+        // SPECIFICATION.md: Chain validation algorithm — certificates in a chain must have unique KeyIds
+        $seenKeyIds = [];
+        foreach ($path as $certificate) {
+            $keyIdString = $certificate->key->id->toString();
+            if (in_array($keyIdString, $seenKeyIds, true)) {
+                $errors[] = new ValidationError('Certificate chain contains duplicate KeyIds', $certificate, 'unique key id validation');
+                return ['isValid' => false, 'errors' => $errors, 'warnings' => $warnings];
+            }
+            $seenKeyIds[] = $keyIdString;
+        }
+
         // Verify the last certificate in path is a root CA
         $rootCert = end($path);
         if (!$rootCert->isRootCA()) {
@@ -263,11 +275,6 @@ private static function validateEndEntityFlagInheritance(Certificate $certificat
     {
         $errors = [];
 
-        // ROOT_CA certificates (self-signed) can have any end-entity flags
-        if ($signer->isRootCA() && $certificate->key->id->equals($signer->key->id)) {
-            return ['isValid' => true, 'errors' => $errors];
-        }
-
         // Get end-entity flags for both certificates
         // SPECIFICATION.md: "End‑Entity Inheritance Matrix" and
         // "End‑entity flags (non‑CA) inheritance"

tests/ValidatorTest.php

@@ -824,9 +824,9 @@ public function testEndEntityFlagsMustBeSubsetOfSigner()
         $this->assertTrue(array_any($messages, fn ($m) => str_contains($m, 'Certificate end-entity flags must be a subset of signer')));
     }
 
-    public function testRootSelfSignedAllowsAnyEndEntityFlagsForSameKey()
+    public function testDuplicateKeyIdsInChainAreRejected()
     {
-        // Hit the early return in validateEndEntityFlagInheritance when signer is self-signed ROOT_CA
+        // Test that chains with duplicate KeyIds are rejected
         $rootKey = Ed25519::makeKeyPair();
 
         // Root certificate: ROOT_CA + CA, self-signed
@@ -840,9 +840,9 @@ public function testRootSelfSignedAllowsAnyEndEntityFlagsForSameKey()
         $rootSelfSig = Signature::make($rootCertBase->toBinaryForSigning(), $rootKey);
         $rootCert = $rootCertBase->with(signatures: [$rootSelfSig]);
 
-        // Child certificate that uses the SAME key id/public key as root, with an end-entity flag
+        // Child certificate that uses the SAME key id/public key as root (duplicate KeyId)
         $childBase = new Certificate(
-            key: $rootKey->toPublicKey(),
+            key: $rootKey->toPublicKey(),  // Same key = same KeyId
             description: 'child_same_key',
             userDescriptors: [new UserDescriptor(DescriptorType::USERNAME, 'child')],
             flags: CertificateFlagsCollection::fromList([CertificateFlag::END_ENTITY_FLAG_1]),
@@ -858,6 +858,52 @@ public function testRootSelfSignedAllowsAnyEndEntityFlagsForSameKey()
 
         $trustStore = new TrustStore([$rootCert]);
 
+        $result = Validator::validateChain($chain, $trustStore);
+        $this->assertFalse($result->isValid);
+        $messages = $result->getErrorMessages();
+        $this->assertTrue(array_any($messages, fn ($m) => str_contains($m, 'Certificate chain contains duplicate KeyIds')));
+    }
+
+    public function testEndEntityFlagInheritanceValidatedForAllCertificates()
+    {
+        // Test that end-entity flag inheritance is validated for all certificates, no exceptions
+        $root_ca = $this->makeTestCert('root_ca', [CertificateFlag::ROOT_CA, CertificateFlag::INTERMEDIATE_CA, CertificateFlag::CA], ['root_ca']);
+        $intermediate_ca = $this->makeTestCert('intermediate_ca', [CertificateFlag::INTERMEDIATE_CA, CertificateFlag::CA, CertificateFlag::END_ENTITY_FLAG_1], ['root_ca']);
+        $end_entity = $this->makeTestCert('end_entity', [CertificateFlag::END_ENTITY_FLAG_1, CertificateFlag::END_ENTITY_FLAG_2], ['intermediate_ca']);
+
+        $chain = new Chain([
+            $end_entity,
+            $intermediate_ca,
+            $root_ca,
+        ]);
+
+        $trustStore = new TrustStore([
+            $root_ca,
+        ]);
+
+        $result = Validator::validateChain($chain, $trustStore);
+        $this->assertFalse($result->isValid);
+        $messages = $result->getErrorMessages();
+        $this->assertTrue(array_any($messages, fn ($m) => str_contains($m, 'Certificate end-entity flags must be a subset of signer')));
+    }
+
+    public function testUniqueKeyIdsAllowValidChain()
+    {
+        // Test that chains with unique KeyIds work correctly
+        $root_ca = $this->makeTestCert('root_ca', [CertificateFlag::ROOT_CA, CertificateFlag::INTERMEDIATE_CA, CertificateFlag::CA, CertificateFlag::END_ENTITY_FLAG_1], ['root_ca']);
+        $intermediate_ca = $this->makeTestCert('intermediate_ca', [CertificateFlag::INTERMEDIATE_CA, CertificateFlag::CA, CertificateFlag::END_ENTITY_FLAG_1], ['root_ca']);
+        $end_entity = $this->makeTestCert('end_entity', [CertificateFlag::END_ENTITY_FLAG_1], ['intermediate_ca']);
+
+        $chain = new Chain([
+            $end_entity,
+            $intermediate_ca,
+            $root_ca,
+        ]);
+
+        $trustStore = new TrustStore([
+            $root_ca,
+        ]);
+
         $result = Validator::validateChain($chain, $trustStore);
         $this->assertTrue($result->isValid);
     }