Fix wrong EdDSA key encoding (#437)

phpstan-baseline.neon

@@ -1501,11 +1501,6 @@ parameters:
             count: 1
             path: src/webauthn/src/AttestationStatement/AppleAttestationStatementSupport.php
 
-        -
-            message: "#^Cannot access offset 1 on array\\|false\\.$#"
-            count: 3
-            path: src/webauthn/src/AttestationStatement/AttestationObjectLoader.php
-
         -
             message: "#^Parameter \\#1 \\$data of static method Webauthn\\\\TrustPath\\\\TrustPathLoader\\:\\:loadTrustPath\\(\\) expects array, mixed given\\.$#"
             count: 1
@@ -1836,6 +1831,31 @@ parameters:
             count: 1
             path: src/webauthn/src/AuthenticatorAttestationResponseValidator.php
 
+        -
+            message: "#^Cannot access offset 1 on array\\|false\\.$#"
+            count: 2
+            path: src/webauthn/src/AuthenticatorDataLoader.php
+
+        -
+            message: "#^Parameter \\#1 \\$search of function str_replace expects array\\|string, string\\|false given\\.$#"
+            count: 1
+            path: src/webauthn/src/AuthenticatorDataLoader.php
+
+        -
+            message: "#^Parameter \\#2 \\$callback of function array_reduce expects callable\\(string, mixed\\)\\: string, Closure\\(string, string\\)\\: non\\-empty\\-string given\\.$#"
+            count: 1
+            path: src/webauthn/src/AuthenticatorDataLoader.php
+
+        -
+            message: "#^Parameter \\#2 \\$needle of function mb_strpos expects string, string\\|false given\\.$#"
+            count: 1
+            path: src/webauthn/src/AuthenticatorDataLoader.php
+
+        -
+            message: "#^Parameter \\#2 \\$replace of function str_replace expects array\\|string, string\\|false given\\.$#"
+            count: 1
+            path: src/webauthn/src/AuthenticatorDataLoader.php
+
         -
             message: """
                 #^Access to deprecated property \\$requireResidentKey of class Webauthn\\\\AuthenticatorSelectionCriteria\\:
@@ -1890,11 +1910,6 @@ parameters:
             count: 1
             path: src/webauthn/src/PublicKeyCredentialDescriptorCollection.php
 
-        -
-            message: "#^Cannot access offset 1 on array\\|false\\.$#"
-            count: 2
-            path: src/webauthn/src/PublicKeyCredentialLoader.php
-
         -
             message: "#^Parameter \\#1 \\$json of method Webauthn\\\\PublicKeyCredentialLoader\\:\\:loadArray\\(\\) expects array, mixed given\\.$#"
             count: 1

src/webauthn/src/AttestationStatement/AttestationObjectLoader.php

@@ -6,19 +6,13 @@
 
 use function array_key_exists;
 use CBOR\Decoder;
-use CBOR\MapObject;
 use CBOR\Normalizable;
 use function is_array;
-use function ord;
 use Psr\EventDispatcher\EventDispatcherInterface;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
-use Symfony\Component\Uid\Uuid;
 use Throwable;
-use function unpack;
-use Webauthn\AttestedCredentialData;
-use Webauthn\AuthenticationExtensions\AuthenticationExtensionsClientOutputsLoader;
-use Webauthn\AuthenticatorData;
+use Webauthn\AuthenticatorDataLoader;
 use Webauthn\Event\AttestationObjectLoaded;
 use Webauthn\Exception\InvalidDataException;
 use Webauthn\MetadataService\CanLogData;
@@ -29,20 +23,13 @@
 
 class AttestationObjectLoader implements CanDispatchEvents, CanLogData
 {
-    private const FLAG_AT = 0b01000000;
-
-    private const FLAG_ED = 0b10000000;
-
-    private readonly Decoder $decoder;
-
     private LoggerInterface $logger;
 
     private EventDispatcherInterface $dispatcher;
 
     public function __construct(
         private readonly AttestationStatementSupportManager $attestationStatementSupportManager
     ) {
-        $this->decoder = Decoder::create();
         $this->logger = new NullLogger();
         $this->dispatcher = new NullEventDispatcher();
     }
@@ -65,7 +52,7 @@ public function load(string $data): AttestationObject
             ]);
             $decodedData = Base64::decode($data);
             $stream = new StringStream($decodedData);
-            $parsed = $this->decoder->decode($stream);
+            $parsed = Decoder::create()->decode($stream);
 
             $this->logger->info('Loading the Attestation Statement');
             $parsed instanceof Normalizable || throw InvalidDataException::create(
@@ -94,69 +81,17 @@ public function load(string $data): AttestationObject
                 $attestationObject,
                 'Invalid attestation object'
             );
-            $authData = $attestationObject['authData'];
 
             $attestationStatementSupport = $this->attestationStatementSupportManager->get($attestationObject['fmt']);
             $attestationStatement = $attestationStatementSupport->load($attestationObject);
             $this->logger->info('Attestation Statement loaded');
             $this->logger->debug('Attestation Statement loaded', [
                 'attestationStatement' => $attestationStatement,
             ]);
+            $authData = $attestationObject['authData'];
+            $authDataLoader = AuthenticatorDataLoader::create();
+            $authenticatorData = $authDataLoader->load($authData);
 
-            $authDataStream = new StringStream($authData);
-            $rp_id_hash = $authDataStream->read(32);
-            $flags = $authDataStream->read(1);
-            $signCount = $authDataStream->read(4);
-            $signCount = unpack('N', $signCount);
-            $this->logger->debug(sprintf('Signature counter: %d', $signCount[1]));
-
-            $attestedCredentialData = null;
-            if (0 !== (ord($flags) & self::FLAG_AT)) {
-                $this->logger->info('Attested Credential Data is present');
-                $aaguid = Uuid::fromBinary($authDataStream->read(16));
-                $credentialLength = $authDataStream->read(2);
-                $credentialLength = unpack('n', $credentialLength);
-                $credentialId = $authDataStream->read($credentialLength[1]);
-                $credentialPublicKey = $this->decoder->decode($authDataStream);
-                $credentialPublicKey instanceof MapObject || throw InvalidDataException::create(
-                    $credentialPublicKey,
-                    'The data does not contain a valid credential public key.'
-                );
-                $attestedCredentialData = new AttestedCredentialData(
-                    $aaguid,
-                    $credentialId,
-                    (string) $credentialPublicKey
-                );
-                $this->logger->info('Attested Credential Data loaded');
-                $this->logger->debug('Attested Credential Data loaded', [
-                    'at' => $attestedCredentialData,
-                ]);
-            }
-
-            $extension = null;
-            if (0 !== (ord($flags) & self::FLAG_ED)) {
-                $this->logger->info('Extension Data loaded');
-                $extension = $this->decoder->decode($authDataStream);
-                $extension = AuthenticationExtensionsClientOutputsLoader::load($extension);
-                $this->logger->info('Extension Data loaded');
-                $this->logger->debug('Extension Data loaded', [
-                    'ed' => $extension,
-                ]);
-            }
-            $authDataStream->isEOF() || throw InvalidDataException::create(
-                null,
-                'Invalid authentication data. Presence of extra bytes.'
-            );
-            $authDataStream->close();
-
-            $authenticatorData = new AuthenticatorData(
-                $authData,
-                $rp_id_hash,
-                $flags,
-                $signCount[1],
-                $attestedCredentialData,
-                $extension
-            );
             $attestationObject = new AttestationObject($data, $attestationStatement, $authenticatorData);
             $this->logger->info('Attestation Object loaded');
             $this->logger->debug('Attestation Object', [

src/webauthn/src/AuthenticatorData.php

@@ -12,17 +12,17 @@
  */
 class AuthenticatorData
 {
-    private const FLAG_UP = 0b00000001;
+    final public const FLAG_UP = 0b00000001;
 
-    private const FLAG_RFU1 = 0b00000010;
+    final public const FLAG_RFU1 = 0b00000010;
 
-    private const FLAG_UV = 0b00000100;
+    final public const FLAG_UV = 0b00000100;
 
-    private const FLAG_RFU2 = 0b00111000;
+    final public const FLAG_RFU2 = 0b00111000;
 
-    private const FLAG_AT = 0b01000000;
+    final public const FLAG_AT = 0b01000000;
 
-    private const FLAG_ED = 0b10000000;
+    final public const FLAG_ED = 0b10000000;
 
     public function __construct(
         protected string $authData,

src/webauthn/src/AuthenticatorDataLoader.php

@@ -0,0 +1,117 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Webauthn;
+
+use CBOR\ByteStringObject;
+use CBOR\Decoder;
+use CBOR\ListObject;
+use CBOR\MapObject;
+use CBOR\NegativeIntegerObject;
+use CBOR\TextStringObject;
+use CBOR\UnsignedIntegerObject;
+use function chr;
+use function ord;
+use Symfony\Component\Uid\Uuid;
+use Webauthn\AuthenticationExtensions\AuthenticationExtensionsClientOutputsLoader;
+use Webauthn\Exception\InvalidDataException;
+
+final class AuthenticatorDataLoader
+{
+    private readonly Decoder $decoder;
+
+    private function __construct()
+    {
+        $this->decoder = Decoder::create();
+    }
+
+    public static function create(): self
+    {
+        return new self();
+    }
+
+    public function load(string $authData): AuthenticatorData
+    {
+        $authData = $this->fixIncorrectEdDSAKey($authData);
+        $authDataStream = new StringStream($authData);
+        $rp_id_hash = $authDataStream->read(32);
+        $flags = $authDataStream->read(1);
+        $signCount = $authDataStream->read(4);
+        $signCount = unpack('N', $signCount);
+
+        $attestedCredentialData = null;
+        if (0 !== (ord($flags) & AuthenticatorData::FLAG_AT)) {
+            $aaguid = Uuid::fromBinary($authDataStream->read(16));
+            $credentialLength = $authDataStream->read(2);
+            $credentialLength = unpack('n', $credentialLength);
+            $credentialId = $authDataStream->read($credentialLength[1]);
+            $credentialPublicKey = $this->decoder->decode($authDataStream);
+            $credentialPublicKey instanceof MapObject || throw InvalidDataException::create(
+                $authData,
+                'The data does not contain a valid credential public key.'
+            );
+            $attestedCredentialData = new AttestedCredentialData(
+                $aaguid,
+                $credentialId,
+                (string) $credentialPublicKey
+            );
+        }
+
+        $extension = null;
+        if (0 !== (ord($flags) & AuthenticatorData::FLAG_ED)) {
+            $extension = $this->decoder->decode($authDataStream);
+            $extension = AuthenticationExtensionsClientOutputsLoader::load($extension);
+        }
+        $authDataStream->isEOF() || throw InvalidDataException::create(
+            $authData,
+            'Invalid authentication data. Presence of extra bytes.'
+        );
+        $authDataStream->close();
+
+        return new AuthenticatorData(
+            $authData,
+            $rp_id_hash,
+            $flags,
+            $signCount[1],
+            $attestedCredentialData,
+            $extension
+        );
+    }
+
+    private function fixIncorrectEdDSAKey(string $data): string
+    {
+        $needle = hex2bin('a301634f4b500327206745643235353139');
+        $correct = hex2bin('a401634f4b500327206745643235353139');
+        $position = mb_strpos($data, $needle, 0, '8bit');
+        if ($position === false) {
+            return $data;
+        }
+
+        $begin = mb_substr($data, 0, $position, '8bit');
+        $end = mb_substr($data, $position, null, '8bit');
+        $end = str_replace($needle, $correct, $end);
+        $cbor = new StringStream($end);
+        $badKey = $this->decoder->decode($cbor);
+
+        ($badKey instanceof MapObject && $cbor->isEOF()) || throw InvalidDataException::create(
+            $end,
+            'Invalid authentication data. Presence of extra bytes.'
+        );
+        $badX = $badKey->get(-2);
+        $badX instanceof ListObject || throw InvalidDataException::create($end, 'Invalid authentication data.');
+        $keyBytes = array_reduce(
+            $badX->normalize(),
+            static fn (string $carry, string $item): string => $carry . chr((int) $item),
+            ''
+        );
+        $correctX = ByteStringObject::create($keyBytes);
+        $correctKey = MapObject::create()
+            ->add(UnsignedIntegerObject::create(1), ByteStringObject::create('OKP'))
+            ->add(UnsignedIntegerObject::create(3), NegativeIntegerObject::create(-8))
+            ->add(NegativeIntegerObject::create(-1), TextStringObject::create('Ed25519'))
+            ->add(NegativeIntegerObject::create(-2), $correctX);
+
+        return $begin . $correctKey;
+    }
+}