From 9aa1a6a05ac28fa420d234d0d5a4e222994d20ed Mon Sep 17 00:00:00 2001
From: zll600 <3400692417@qq.com>
Date: Tue, 25 Nov 2025 23:22:56 +0800
Subject: [PATCH] chore: simplify metadata statement check

---
 .../CeremonyStep/CheckMetadataStatement.php   | 76 ++++++++++---------
 1 file changed, 40 insertions(+), 36 deletions(-)

diff --git a/src/webauthn/src/CeremonyStep/CheckMetadataStatement.php b/src/webauthn/src/CeremonyStep/CheckMetadataStatement.php
index 356e1288..d4e87b49 100644
--- a/src/webauthn/src/CeremonyStep/CheckMetadataStatement.php
+++ b/src/webauthn/src/CeremonyStep/CheckMetadataStatement.php
@@ -79,41 +79,49 @@ public function process(
         $attestedCredentialData !== null || throw AuthenticatorResponseVerificationException::create(
             'No attested credential data found'
         );
-        $aaguid = $attestedCredentialData->aaguid
-            ->__toString();
-        if ($publicKeyCredentialOptions->attestation === null || $publicKeyCredentialOptions->attestation === PublicKeyCredentialCreationOptions::ATTESTATION_CONVEYANCE_PREFERENCE_NONE) {
-            $this->logger->debug('No attestation is asked.');
-            if ($aaguid === '00000000-0000-0000-0000-000000000000' && in_array(
-                $attestationStatement->type,
-                [AttestationStatement::TYPE_NONE, AttestationStatement::TYPE_SELF],
-                true
-            )) {
-                $this->logger->debug('The Attestation Statement is anonymous.');
-                $this->checkCertificateChain($attestationStatement, null);
-                return;
-            }
+
+        /**
+         * RP does not want to verify the authentication attestation.
+         * @see https://www.w3.org/TR/webauthn-3/#dom-attestationconveyancepreference-none
+         */
+        if (in_array(
+            $publicKeyCredentialOptions->attestation,
+            [null, PublicKeyCredentialCreationOptions::ATTESTATION_CONVEYANCE_PREFERENCE_NONE],
+            true
+        )) {
             return;
         }
-        // If no Attestation Statement has been returned or if null AAGUID (=00000000-0000-0000-0000-000000000000)
-        // => nothing to check
-        if ($attestationStatement->type === AttestationStatement::TYPE_NONE) {
-            $this->logger->debug('No attestation returned.');
-            //No attestation is returned. We shall ensure that the AAGUID is a null one.
-            //if ($aaguid !== '00000000-0000-0000-0000-000000000000') {
-            //$this->logger->debug('Anonymization required. AAGUID and Attestation Statement changed.', [
-            //    'aaguid' => $aaguid,
-            //    'AttestationStatement' => $attestationStatement,
-            //]);
-            //$attestedCredentialData->aaguid = Uuid::fromString('00000000-0000-0000-0000-000000000000');
-            //    return;
-            //}
+
+        /**
+         * No trust path available for verification
+         *
+         * @see https://www.w3.org/TR/webauthn-3/#none
+         * @see https://www.w3.org/TR/webauthn-3/#self
+         */
+        if (in_array(
+            $attestationStatement->type,
+            [AttestationStatement::TYPE_NONE, AttestationStatement::TYPE_SELF],
+            true
+        )) {
             return;
         }
+
+        $aaguid = $attestedCredentialData->aaguid
+            ->__toString();
+        /**
+         * All-zeros AAGUID: either privacy placeholder or U2F device (which predates AAGUID).
+         * 1) Privacy Placeholder indicates the authenticator does not provide detailed information.
+         * 2) U2F device can not provide useful AAGUID.
+         *
+         * So Metadata Statement lookup by AAGUID not possible, skip it.
+         *
+         * @see https://www.w3.org/TR/webauthn-3/#sctn-createCredential
+         * @see https://fidoalliance.org/specs/fido-v2.2-ps-20250714/fido-client-to-authenticator-protocol-v2.2-ps-20250714.html#u2f-authenticatorMakeCredential-interoperability
+         */
         if ($aaguid === '00000000-0000-0000-0000-000000000000') {
-            //No need to continue if the AAGUID is null.
-            // This could be the case e.g. with AnonCA type
             return;
         }
+
         //The MDS Repository is mandatory here
         $this->metadataStatementRepository !== null || throw AuthenticatorResponseVerificationException::create(
             'The Metadata Statement Repository is mandatory when requesting attestation objects.'
@@ -171,17 +179,13 @@ private function checkStatusReport(string $aaguid): void
 
     private function checkCertificateChain(
         AttestationStatement $attestationStatement,
-        ?MetadataStatement $metadataStatement
+        MetadataStatement $metadataStatement
     ): void {
         $trustPath = $attestationStatement->trustPath;
-        if (! $trustPath instanceof CertificateTrustPath) {
-            return;
-        }
+        $trustPath instanceof CertificateTrustPath || throw AuthenticatorResponseVerificationException::create(
+            'Certificate trust path is required for attestation verification'
+        );
         $authenticatorCertificates = $trustPath->certificates;
-        if ($metadataStatement === null) {
-            $this->certificateChainValidator?->check($authenticatorCertificates, []);
-            return;
-        }
         $trustedCertificates = CertificateToolbox::fixPEMStructures(
             $metadataStatement->attestationRootCertificates
         );