Split out and improve attestation verification from PR discussion

Co-authored-by: Tim Düsterhus <[email protected]>

Author: asgrim

src/SelfManage/Verify/FailedToVerifyRelease.php

@@ -12,6 +12,7 @@
 use function implode;
 use function is_array;
 use function sprintf;
+use function strlen;
 use function trim;
 
 class FailedToVerifyRelease extends RuntimeException
@@ -57,6 +58,16 @@ public static function fromNoIssuerCertificateInTrustedRoot(array|string $issuer
         ));
     }
 
+    public static function fromInvalidDerEncodedStringLength(string $derEncodedString, int $expectedLength): self
+    {
+        return new self(sprintf(
+            'DER encoded string length of "%s" was wrong; expected %d characters, was actually %d characters',
+            $derEncodedString,
+            $expectedLength,
+            strlen($derEncodedString),
+        ));
+    }
+
     public static function fromMismatchingExtensionValues(string $extension, string $expected, string $actual): self
     {
         return new self(sprintf(

src/SelfManage/Verify/FallbackVerificationUsingOpenSsl.php

@@ -76,8 +76,11 @@ public function verify(ReleaseMetadata $releaseMetadata, BinaryFile $pharFilenam
              *  - https://docs.sigstore.dev/logging/verify-release/
              *  - https://github.com/secure-systems-lab/dsse/blob/master/protocol.md#protocol
              */
-            $this->assertCertificateClaims($attestation);
-            $output->writeln('#' . $attestationIndex . ': Certificate facts verified.', OutputInterface::VERBOSITY_VERBOSE);
+            $this->assertCertificateSignedByTrustedRoot($attestation);
+            $output->writeln('#' . $attestationIndex . ': Certificate was signed by a trusted root.', OutputInterface::VERBOSITY_VERBOSE);
+
+            $this->assertCertificateExtensionClaims($attestation);
+            $output->writeln('#' . $attestationIndex . ': Certificate extension claims match.', OutputInterface::VERBOSITY_VERBOSE);
 
             $this->assertDigestFromAttestationMatchesActual($pharFilename, $attestation);
             $output->writeln('#' . $attestationIndex . ': Payload digest matches downloaded file.', OutputInterface::VERBOSITY_VERBOSE);
@@ -89,39 +92,9 @@ public function verify(ReleaseMetadata $releaseMetadata, BinaryFile $pharFilenam
         $output->writeln('<info>✅ Verified the new PIE version (using fallback verification)</info>');
     }
 
-    private function assertCertificateClaims(Attestation $attestation): void
+    private function assertCertificateSignedByTrustedRoot(Attestation $attestation): void
     {
         $attestationCertificateInfo = openssl_x509_parse($attestation->certificate);
-        Assert::isArray($attestationCertificateInfo['extensions']);
-
-        /**
-         * See {@link https://github.com/sigstore/fulcio/blob/main/docs/oid-info.md#136141572641--fulcio} for details
-         * on the Fulcio extension keys; note the values are DER-encoded strings; the ASN.1 tag is UTF8String (0x0C).
-         *
-         * First up, check the extension values are what we expect; these are hard-coded, as we don't expect them
-         * to change unless the namespace/repo name change, etc.
-         */
-        foreach (self::ATTESTATION_CERTIFICATE_EXPECTED_EXTENSION_VALUES as $extension => $expectedValue) {
-            Assert::keyExists($attestationCertificateInfo['extensions'], $extension);
-            Assert::stringNotEmpty($attestationCertificateInfo['extensions'][$extension]);
-            $actualValue = $attestationCertificateInfo['extensions'][$extension];
-
-            // First character (the ASN.1 tag) is expected to be UTF8String (0x0C)
-            if (ord($actualValue[0]) !== 0x0C) {
-                throw FailedToVerifyRelease::fromMismatchingExtensionValues($extension, $expectedValue, $actualValue);
-            }
-
-            /**
-             * Second character is expected to be the length of the actual value
-             * as long as they are less than 127 bytes (short form)
-             *
-             * @link https://www.oss.com/asn1/resources/asn1-made-simple/asn1-quick-reference/basic-encoding-rules.html#Lengths
-             */
-            $derDecodedValue = substr($actualValue, 2, ord($actualValue[1]));
-            if ($derDecodedValue !== $expectedValue) {
-                throw FailedToVerifyRelease::fromMismatchingExtensionValues($extension, $expectedValue, $derDecodedValue);
-            }
-        }
 
         // @todo process in place to make sure this gets updated frequently enough: gh attestation trusted-root > resources/trusted-root.jsonl
         $trustedRootJsonLines = explode("\n", trim(file_get_contents($this->trustedRootFilePath)));
@@ -141,7 +114,11 @@ private function assertCertificateClaims(Attestation $attestation): void
             $decoded = json_decode($jsonLine, true);
 
             // No certificate authorities defined in this JSON line, skip it...
-            if (! is_array($decoded) || ! array_key_exists('certificateAuthorities', $decoded)) {
+            if (
+                ! is_array($decoded)
+                || ! array_key_exists('certificateAuthorities', $decoded)
+                || ! is_array($decoded['certificateAuthorities'])
+            ) {
                 continue;
             }
 
@@ -208,6 +185,46 @@ private function assertCertificateClaims(Attestation $attestation): void
         throw FailedToVerifyRelease::fromNoIssuerCertificateInTrustedRoot($attestationCertificateInfo['issuer']);
     }
 
+    private function assertCertificateExtensionClaims(Attestation $attestation): void
+    {
+        $attestationCertificateInfo = openssl_x509_parse($attestation->certificate);
+        Assert::isArray($attestationCertificateInfo['extensions']);
+
+        /**
+         * See {@link https://github.com/sigstore/fulcio/blob/main/docs/oid-info.md#136141572641--fulcio} for details
+         * on the Fulcio extension keys; note the values are DER-encoded strings; the ASN.1 tag is UTF8String (0x0C).
+         *
+         * Check the extension values are what we expect; these are hard-coded, as we don't expect them
+         * to change unless the namespace/repo name change, etc.
+         */
+        foreach (self::ATTESTATION_CERTIFICATE_EXPECTED_EXTENSION_VALUES as $extension => $expectedValue) {
+            Assert::keyExists($attestationCertificateInfo['extensions'], $extension);
+            Assert::stringNotEmpty($attestationCertificateInfo['extensions'][$extension]);
+            $actualValue = $attestationCertificateInfo['extensions'][$extension];
+
+            // First character (the ASN.1 tag) is expected to be UTF8String (0x0C)
+            if (ord($actualValue[0]) !== 0x0C) {
+                throw FailedToVerifyRelease::fromMismatchingExtensionValues($extension, $expectedValue, $actualValue);
+            }
+
+            /**
+             * Second character is expected to be the length of the actual value
+             * as long as they are less than 127 bytes (short form)
+             *
+             * @link https://www.oss.com/asn1/resources/asn1-made-simple/asn1-quick-reference/basic-encoding-rules.html#Lengths
+             */
+            $expectedValueLength = ord($actualValue[1]);
+            if (strlen($actualValue) !== 2 + $expectedValueLength) {
+                throw FailedToVerifyRelease::fromInvalidDerEncodedStringLength($actualValue, 2 + $expectedValueLength);
+            }
+
+            $derDecodedValue = substr($actualValue, 2, $expectedValueLength);
+            if ($derDecodedValue !== $expectedValue) {
+                throw FailedToVerifyRelease::fromMismatchingExtensionValues($extension, $expectedValue, $derDecodedValue);
+            }
+        }
+    }
+
     private function verifyDsseEnvelopeSignature(ReleaseMetadata $releaseMetadata, int $attestationIndex, Attestation $attestation): void
     {
         if (! extension_loaded('openssl')) {