Bugs from PHPStan fixed. Level 8 (over 9) reached

web-auth · Mar 24, 2022 · e7ef02b · e7ef02b
1 parent 1faf324
commit e7ef02b
Show file tree

Hide file tree

Showing 19 changed files with 81 additions and 43 deletions.
diff --git a/phpstan.neon b/phpstan.neon
@@ -1,12 +1,17 @@
 parameters:
-    level: 7
+    level: 8
     paths:
         - src
     ignoreErrors:
+        - '#Parameter \#1 \$key of function openssl_pkey_get_details expects OpenSSLAsymmetricKey, resource given\.#'
         -
             message: '#Method Webauthn\\Bundle\\DependencyInjection\\Factory\\Security\\WebauthnFactory::.*\(\) has parameter \$config(s)? with no value type specified in iterable type array\.#'
             path: src/symfony/src/DependencyInjection/Factory/Security/WebauthnFactory.php
             count: 2
+        -
+            message: '#Parameter \#1 \$length of function random_bytes expects int<1, max>, int given\.#'
+            path: src/cose/src/Algorithm/Signature/RSA/PSSRSA.php
+            count: 1
         -
             message: '#Method Webauthn\\Bundle\\DependencyInjection\\WebauthnExtension::.*\(\) has parameter \$config(s)? with no value type specified in iterable type array\.#'
             path: src/symfony/src/DependencyInjection/WebauthnExtension.php
@@ -24,9 +29,13 @@ parameters:
             path: src/symfony/src/DependencyInjection/Configuration.php
             count: 1
         -
-            message: '#Call to an undefined method Symfony\\Component\\Config\\Definition\\Builder\\NodeParentInterface::(.*)\(\)\.#'
+            message: '#Cannot call method scalarNode\(\) on Symfony\\Component\\Config\\Definition\\Builder\\NodeParentInterface\|null\.#'
             path: src/symfony/src/DependencyInjection/Factory/Security/WebauthnFactory.php
             count: 1
+#        -
+#            message: '#Call to an undefined method Symfony\\Component\\Config\\Definition\\Builder\\NodeParentInterface::(.*)\(\)\.#'
+#            path: src/symfony/src/DependencyInjection/Factory/Security/WebauthnFactory.php
+#            count: 1
         -
             message: '#Parameter (.*) of class FG\\ASN1\\Universal\\Integer constructor expects int, string given\.#'
             path:  src/cose/src/Key/RsaKey.php

diff --git a/src/cose/src/Key/Ec2Key.php b/src/cose/src/Key/Ec2Key.php
@@ -52,7 +52,7 @@ class Ec2Key extends Key
     ];
 
     /**
-     * @param array<int, mixed> $data
+     * @param array<int|string, mixed> $data
      */
     public function __construct(array $data)
     {
@@ -83,7 +83,7 @@ public function __construct(array $data)
     }
 
     /**
-     * @param array<int, mixed> $data
+     * @param array<int|string, mixed> $data
      */
     public static function create(array $data): self
     {

diff --git a/src/cose/src/Key/Key.php b/src/cose/src/Key/Key.php
@@ -28,12 +28,12 @@ class Key
     public const BASE_IV = 5;
 
     /**
-     * @var array<int, mixed>
+     * @var array<int|string, mixed>
      */
     private array $data;
 
     /**
-     * @param array<int, mixed> $data
+     * @param array<int|string, mixed> $data
      */
     public function __construct(array $data)
     {
@@ -42,7 +42,7 @@ public function __construct(array $data)
     }
 
     /**
-     * @param array<int, mixed> $data
+     * @param array<int|string, mixed> $data
      */
     public static function create(array $data): self
     {
@@ -76,19 +76,19 @@ public function alg(): int
     }
 
     /**
-     * @return array<int, mixed>
+     * @return array<int|string, mixed>
      */
     public function getData(): array
     {
         return $this->data;
     }
 
-    public function has(int $key): bool
+    public function has(int|string $key): bool
     {
         return array_key_exists($key, $this->data);
     }
 
-    public function get(int $key): mixed
+    public function get(int|string $key): mixed
     {
         Assertion::keyExists($this->data, $key, sprintf('The key has no data at index %d', $key));
 

diff --git a/src/cose/src/Key/OkpKey.php b/src/cose/src/Key/OkpKey.php
@@ -31,7 +31,7 @@ class OkpKey extends Key
     ];
 
     /**
-     * @param array<int, mixed> $data
+     * @param array<int|string, mixed> $data
      */
     public function __construct(array $data)
     {
@@ -47,7 +47,7 @@ public function __construct(array $data)
     }
 
     /**
-     * @param array<int, mixed> $data
+     * @param array<int|string, mixed> $data
      */
     public static function create(array $data): self
     {

diff --git a/src/cose/src/Key/RsaKey.php b/src/cose/src/Key/RsaKey.php
@@ -41,7 +41,7 @@ class RsaKey extends Key
     public const DATA_TI = -12;
 
     /**
-     * @param array<int, mixed> $data
+     * @param array<int|string, mixed> $data
      */
     public function __construct(array $data)
     {
@@ -56,7 +56,7 @@ public function __construct(array $data)
     }
 
     /**
-     * @param array<int, mixed> $data
+     * @param array<int|string, mixed> $data
      */
     public static function create(array $data): self
     {

diff --git a/src/cose/src/Key/SymmetricKey.php b/src/cose/src/Key/SymmetricKey.php
@@ -11,7 +11,7 @@ class SymmetricKey extends Key
     public const DATA_K = -1;
 
     /**
-     * @param array<int, mixed> $data
+     * @param array<int|string, mixed> $data
      */
     public function __construct(array $data)
     {
@@ -25,7 +25,7 @@ public function __construct(array $data)
     }
 
     /**
-     * @param array<int, mixed> $data
+     * @param array<int|string, mixed> $data
      */
     public static function create(array $data): self
     {

diff --git a/src/metadata-service/src/Service/DistantResourceMetadataService.php b/src/metadata-service/src/Service/DistantResourceMetadataService.php
@@ -44,20 +44,25 @@ public static function create(
     public function list(): iterable
     {
         $this->loadData();
+        Assertion::notNull($this->statement, 'Unable to load the metadata statement');
+        $aaguid = $this->statement->getAaguid();
+        Assertion::notNull($aaguid, 'Unable to load the metadata statement');
 
-        yield from [$this->statement->getAaguid()];
+        yield from [$aaguid];
     }
 
     public function has(string $aaguid): bool
     {
         $this->loadData();
+        Assertion::notNull($this->statement, 'Unable to load the metadata statement');
 
         return $aaguid === $this->statement->getAaguid();
     }
 
     public function get(string $aaguid): MetadataStatement
     {
         $this->loadData();
+        Assertion::notNull($this->statement, 'Unable to load the metadata statement');
 
         if ($aaguid === $this->statement->getAaguid()) {
             return $this->statement;

diff --git a/src/metadata-service/src/Service/FidoAllianceCompliantMetadataService.php b/src/metadata-service/src/Service/FidoAllianceCompliantMetadataService.php
@@ -173,7 +173,9 @@ private function getJwsPayload(string $token, array &$rootCertificates): string
         $verifier = new JWSVerifier(new AlgorithmManager([new ES256(), new RS256()]));
         $isValid = $verifier->verifyWithKey($jws, $key, 0);
         Assertion::true($isValid, 'Invalid response from the metadata service. The token signature is invalid.');
+        $payload = $jws->getPayload();
+        Assertion::notNull($payload, 'Invalid response from the metadata service. The payload is missing.');
 
-        return $jws->getPayload();
+        return $payload;
     }
 }
diff --git a/src/metadata-service/src/Service/LocalResourceMetadataService.php b/src/metadata-service/src/Service/LocalResourceMetadataService.php
@@ -4,6 +4,7 @@
 
 namespace Webauthn\MetadataService\Service;
 
+use Assert\Assertion;
 use InvalidArgumentException;
 use ParagonIE\ConstantTime\Base64;
 use function Safe\file_get_contents;
@@ -28,20 +29,25 @@ public static function create(string $filename, bool $isBase64Encoded = false):
     public function list(): iterable
     {
         $this->loadData();
+        Assertion::notNull($this->statement, 'Unable to load the metadata statement');
+        $aaguid = $this->statement->getAaguid();
+        Assertion::notNull($aaguid, 'Unable to load the metadata statement');
 
-        yield from [$this->statement->getAaguid()];
+        yield from [$aaguid];
     }
 
     public function has(string $aaguid): bool
     {
         $this->loadData();
+        Assertion::notNull($this->statement, 'Unable to load the metadata statement');
 
         return $aaguid === $this->statement->getAaguid();
     }
 
     public function get(string $aaguid): MetadataStatement
     {
         $this->loadData();
+        Assertion::notNull($this->statement, 'Unable to load the metadata statement');
 
         if ($aaguid === $this->statement->getAaguid()) {
             return $this->statement;

diff --git a/src/metadata-service/src/Service/MetadataBLOBPayloadEntry.php b/src/metadata-service/src/Service/MetadataBLOBPayloadEntry.php
@@ -131,12 +131,12 @@ public function getTimeOfLastStatusChange(): string
         return $this->timeOfLastStatusChange;
     }
 
-    public function getRogueListURL(): string
+    public function getRogueListURL(): string|null
     {
         return $this->rogueListURL;
     }
 
-    public function getRogueListHash(): string
+    public function getRogueListHash(): string|null
     {
         return $this->rogueListHash;
     }

diff --git a/src/metadata-service/src/Statement/BiometricStatusReport.php b/src/metadata-service/src/Statement/BiometricStatusReport.php
@@ -22,12 +22,12 @@ class BiometricStatusReport implements JsonSerializable
 
     private ?string $certificationRequirementsVersion = null;
 
-    public function getCertLevel(): int
+    public function getCertLevel(): int|null
     {
         return $this->certLevel;
     }
 
-    public function getModality(): int
+    public function getModality(): int|null
     {
         return $this->modality;
     }

diff --git a/src/metadata-service/src/Statement/MetadataStatement.php b/src/metadata-service/src/Statement/MetadataStatement.php
@@ -157,7 +157,7 @@ class MetadataStatement implements JsonSerializable
      */
     private array $supportedExtensions = [];
 
-    private AuthenticatorGetInfo $authenticatorGetInfo;
+    private null|AuthenticatorGetInfo $authenticatorGetInfo = null;
 
     /**
      * @var string[]
@@ -225,7 +225,7 @@ public function isFreshUserVerificationRequired(): ?bool
         return $this->isFreshUserVerificationRequired;
     }
 
-    public function getAuthenticatorGetInfo(): AuthenticatorGetInfo
+    public function getAuthenticatorGetInfo(): AuthenticatorGetInfo|null
     {
         return $this->authenticatorGetInfo;
     }
@@ -404,12 +404,24 @@ public static function createFromArray(array $data): self
         foreach ($requiredKeys as $key) {
             Assertion::keyExists($data, $key, sprintf('The parameter "%s" is missing', $key));
         }
-        Assertion::allString($data['authenticationAlgorithms'], 'Invalid Metadata Statement');
-        Assertion::allString($data['publicKeyAlgAndEncodings'], 'Invalid Metadata Statement');
-        Assertion::allString($data['attestationTypes'], 'Invalid Metadata Statement');
-        Assertion::allString($data['matcherProtection'], 'Invalid Metadata Statement');
-        Assertion::allString($data['tcDisplay'], 'Invalid Metadata Statement');
-        Assertion::allString($data['attestationRootCertificates'], 'Invalid Metadata Statement');
+        $subObjects = [
+            'authenticationAlgorithms',
+            'publicKeyAlgAndEncodings',
+            'attestationTypes',
+            'matcherProtection',
+            'tcDisplay',
+            'attestationRootCertificates',
+        ];
+        foreach ($subObjects as $subObject) {
+            Assertion::isArray(
+                $data[$subObject],
+                sprintf('Invalid Metadata Statement. The parameter "%s" shall be a list of strings.', $subObject)
+            );
+            Assertion::allString(
+                $data[$subObject],
+                sprintf('Invalid Metadata Statement. The parameter "%s" shall be a list of strings.', $subObject)
+            );
+        }
 
         $object = new self(
             $data['description'],

diff --git a/src/symfony/src/Security/Http/Authenticator/WebauthnAuthenticator.php b/src/symfony/src/Security/Http/Authenticator/WebauthnAuthenticator.php
@@ -107,9 +107,11 @@ public function createToken(Passport $passport, string $firewallName): TokenInte
                 ->getAuthData()
             ;
         }
+        $userEntity = $credentialsBadge->getPublicKeyCredentialUserEntity();
+        Assertion::notNull($userEntity, 'The user entity is missing');
 
         $token = new  WebauthnToken(
-            $credentialsBadge->getPublicKeyCredentialUserEntity(),
+            $userEntity,
             $credentialsBadge->getPublicKeyCredentialOptions(),
             $credentialsBadge->getPublicKeyCredentialSource()
                 ->getPublicKeyCredentialDescriptor(),

diff --git a/src/webauthn/src/AttestationStatement/AndroidSafetyNetAttestationStatementSupport.php b/src/webauthn/src/AttestationStatement/AndroidSafetyNetAttestationStatementSupport.php
@@ -208,7 +208,7 @@ private function validatePayload(
     private function validateSignature(JWS $jws, CertificateTrustPath $trustPath): void
     {
         $jwk = JWKFactory::createFromCertificate($trustPath->getCertificates()[0]);
-        $isValid = $this->jwsVerifier->verifyWithKey($jws, $jwk, 0);
+        $isValid = $this->jwsVerifier?->verifyWithKey($jws, $jwk, 0);
         Assertion::true($isValid, 'Invalid response signature');
     }
 

diff --git a/src/webauthn/src/AttestationStatement/FidoU2FAttestationStatementSupport.php b/src/webauthn/src/AttestationStatement/FidoU2FAttestationStatementSupport.php
@@ -75,7 +75,7 @@ public function isValid(
     ): bool {
         Assertion::eq(
             $authenticatorData->getAttestedCredentialData()
-                ->getAaguid()
+                ?->getAaguid()
                 ->__toString(),
             '00000000-0000-0000-0000-000000000000',
             'Invalid AAGUID for fido-u2f attestation statement. Shall be "00000000-0000-0000-0000-000000000000"'
@@ -86,11 +86,11 @@ public function isValid(
         $dataToVerify .= $authenticatorData->getRpIdHash();
         $dataToVerify .= $clientDataJSONHash;
         $dataToVerify .= $authenticatorData->getAttestedCredentialData()
-            ->getCredentialId()
+            ?->getCredentialId()
         ;
         $dataToVerify .= $this->extractPublicKey(
             $authenticatorData->getAttestedCredentialData()
-                ->getCredentialPublicKey()
+                ?->getCredentialPublicKey()
         );
 
         return openssl_verify(

diff --git a/src/webauthn/src/AttestationStatement/TPMAttestationStatementSupport.php b/src/webauthn/src/AttestationStatement/TPMAttestationStatementSupport.php
@@ -92,11 +92,9 @@ public function isValid(
             $attToBeSignedHash,
             'Invalid attestation hash'
         );
-        $this->checkUniquePublicKey(
-            $attestationStatement->get('parsedPubArea')['unique'],
-            $authenticatorData->getAttestedCredentialData()
-                ->getCredentialPublicKey()
-        );
+        $credentialPublicKey = $authenticatorData->getAttestedCredentialData()?->getCredentialPublicKey();
+        Assertion::notNull($credentialPublicKey, 'Not credential public key available in the attested credential data');
+        $this->checkUniquePublicKey($attestationStatement->get('parsedPubArea')['unique'], $credentialPublicKey);
 
         return match (true) {
             $attestationStatement->getTrustPath() instanceof CertificateTrustPath => $this->processWithCertificate(

diff --git a/src/webauthn/src/AuthenticatorAssertionResponseValidator.php b/src/webauthn/src/AuthenticatorAssertionResponseValidator.php
@@ -181,7 +181,7 @@ public function check(
                 'Invalid attestation object. Unexpected object.'
             );
             $coseKey = Key::create($credentialPublicKeyStream->normalize());
-            $algorithm = $this->algorithmManager->get($coseKey->alg());
+            $algorithm = $this->algorithmManager?->get($coseKey->alg());
             Assertion::isInstanceOf(
                 $algorithm,
                 Signature::class,

diff --git a/src/webauthn/src/AuthenticatorAttestationResponseValidator.php b/src/webauthn/src/AuthenticatorAttestationResponseValidator.php
@@ -350,6 +350,9 @@ private function createPublicKeyCredentialSource(
         AttestationObject $attestationObject,
         string $userHandle
     ): PublicKeyCredentialSource {
+        $credentialPublicKey = $attestedCredentialData->getCredentialPublicKey();
+        Assertion::notNull($credentialPublicKey, 'Not credential public key available in the attested credential data');
+
         return new PublicKeyCredentialSource(
             $credentialId,
             PublicKeyCredentialDescriptor::CREDENTIAL_TYPE_PUBLIC_KEY,
@@ -359,7 +362,7 @@ private function createPublicKeyCredentialSource(
             $attestationObject->getAttStmt()
                 ->getTrustPath(),
             $attestedCredentialData->getAaguid(),
-            $attestedCredentialData->getCredentialPublicKey(),
+            $credentialPublicKey,
             $userHandle,
             $attestationObject->getAuthData()
                 ->getSignCount()