Skip to content

Commit

Permalink
Add FakeCredentialGenerator for preventing username enumeration (#603)
Browse files Browse the repository at this point in the history
* Add FakeCredentialGenerator for fake credentials generation

The update introduces a new FakeCredentialGenerator and its simple implementation, SimpleFakeCredentialGenerator, for generating fake credentials. This addition helps prevent username enumeration by providing fake credentials for nonexistent users. Changes have been made across multiple files, including service configuration updates and logic changes in the ProfileBasedRequestOptionsBuilder.
  • Loading branch information
Spomky authored Jun 29, 2024
1 parent a2e942b commit 51b7a85
Show file tree
Hide file tree
Showing 10 changed files with 138 additions and 10 deletions.
6 changes: 0 additions & 6 deletions .github/workflows/integrate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,6 @@ jobs:
run: |
vendor/bin/deptrac analyse --fail-on-uncovered --no-cache
- name: "SonarCloud Scan"
uses: "sonarsource/sonarcloud-github-action@master"
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}

rector_checkstyle:
name: "6️⃣ Rector Checkstyle"
needs:
Expand Down
15 changes: 15 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -797,11 +797,21 @@ parameters:
count: 1
path: src/symfony/src/CredentialOptionsBuilder/ProfileBasedRequestOptionsBuilder.php

-
message: "#^Parameter \\#1 \\$userEntity of method Webauthn\\\\Bundle\\\\CredentialOptionsBuilder\\\\ProfileBasedRequestOptionsBuilder\\:\\:getCredentials\\(\\) expects Webauthn\\\\PublicKeyCredentialUserEntity, Webauthn\\\\PublicKeyCredentialUserEntity\\|null given\\.$#"
count: 1
path: src/symfony/src/CredentialOptionsBuilder/ProfileBasedRequestOptionsBuilder.php

-
message: "#^Parameter \\$credentialSourceRepository of method Webauthn\\\\Bundle\\\\CredentialOptionsBuilder\\\\ProfileBasedRequestOptionsBuilder\\:\\:__construct\\(\\) has typehint with deprecated interface Webauthn\\\\PublicKeyCredentialSourceRepository\\.$#"
count: 1
path: src/symfony/src/CredentialOptionsBuilder/ProfileBasedRequestOptionsBuilder.php

-
message: "#^Should not use function \"dump\", please change the code\\.$#"
count: 1
path: src/symfony/src/CredentialOptionsBuilder/ProfileBasedRequestOptionsBuilder.php

-
message: """
#^Fetching class constant class of deprecated class Webauthn\\\\Bundle\\\\Event\\\\AuthenticatorAssertionResponseValidationFailedEvent\\:
Expand Down Expand Up @@ -3170,6 +3180,11 @@ parameters:
count: 1
path: src/webauthn/src/PublicKeyCredentialSource.php

-
message: "#^Method Webauthn\\\\SimpleFakeCredentialGenerator\\:\\:generate\\(\\) should return array\\<Webauthn\\\\PublicKeyCredentialDescriptor\\> but returns mixed\\.$#"
count: 1
path: src/webauthn/src/SimpleFakeCredentialGenerator.php

-
message: "#^Method Webauthn\\\\StringStream\\:\\:read\\(\\) should return string but returns string\\|false\\.$#"
count: 1
Expand Down
7 changes: 5 additions & 2 deletions src/symfony/src/Controller/AssertionControllerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use Webauthn\Bundle\Security\Handler\SuccessHandler;
use Webauthn\Bundle\Security\Storage\OptionsStorage;
use Webauthn\Bundle\Service\PublicKeyCredentialRequestOptionsFactory;
use Webauthn\FakeCredentialGenerator;
use Webauthn\MetadataService\CanLogData;
use Webauthn\PublicKeyCredentialLoader;
use Webauthn\PublicKeyCredentialSourceRepository;
Expand All @@ -34,7 +35,8 @@ public function __construct(
private readonly null|PublicKeyCredentialLoader $publicKeyCredentialLoader,
private readonly AuthenticatorAssertionResponseValidator $authenticatorAssertionResponseValidator,
private readonly PublicKeyCredentialUserEntityRepositoryInterface $publicKeyCredentialUserEntityRepository,
private readonly PublicKeyCredentialSourceRepository|PublicKeyCredentialSourceRepositoryInterface $publicKeyCredentialSourceRepository
private readonly PublicKeyCredentialSourceRepository|PublicKeyCredentialSourceRepositoryInterface $publicKeyCredentialSourceRepository,
private readonly null|FakeCredentialGenerator $fakeCredentialGenerator = null,
) {
if ($this->publicKeyCredentialLoader !== null) {
trigger_deprecation(
Expand Down Expand Up @@ -68,6 +70,7 @@ public function createAssertionRequestController(
$this->publicKeyCredentialSourceRepository,
$this->publicKeyCredentialRequestOptionsFactory,
$profile,
$this->fakeCredentialGenerator,
);

return $this->createRequestController($optionsBuilder, $optionStorage, $optionsHandler, $failureHandler);
Expand All @@ -84,7 +87,7 @@ public function createRequestController(
$optionStorage,
$optionsHandler,
$failureHandler,
$this->logger
$this->logger,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Webauthn\Bundle\Repository\PublicKeyCredentialSourceRepositoryInterface;
use Webauthn\Bundle\Repository\PublicKeyCredentialUserEntityRepositoryInterface;
use Webauthn\Bundle\Service\PublicKeyCredentialRequestOptionsFactory;
use Webauthn\FakeCredentialGenerator;
use Webauthn\PublicKeyCredentialDescriptor;
use Webauthn\PublicKeyCredentialRequestOptions;
use Webauthn\PublicKeyCredentialSource;
Expand All @@ -32,6 +33,7 @@ public function __construct(
private readonly PublicKeyCredentialSourceRepository|PublicKeyCredentialSourceRepositoryInterface $credentialSourceRepository,
private readonly PublicKeyCredentialRequestOptionsFactory $publicKeyCredentialRequestOptionsFactory,
private readonly string $profile,
private readonly null|FakeCredentialGenerator $fakeCredentialGenerator = null,
) {
if (! $this->credentialSourceRepository instanceof PublicKeyCredentialSourceRepositoryInterface) {
trigger_deprecation(
Expand Down Expand Up @@ -71,7 +73,16 @@ public function getFromRequest(
$userEntity = $optionsRequest->username === null ? null : $this->userEntityRepository->findOneByUsername(
$optionsRequest->username
);
$allowedCredentials = $userEntity === null ? [] : $this->getCredentials($userEntity);
dump($this->fakeCredentialGenerator?->generate($request, $optionsRequest->username ?? ''));
$allowedCredentials = match (true) {
$userEntity === null && $optionsRequest->username === null, $userEntity === null && $optionsRequest->username !== null && $this->fakeCredentialGenerator === null => [],
$userEntity === null && $optionsRequest->username !== null && $this->fakeCredentialGenerator !== null => $this->fakeCredentialGenerator->generate(
$request,
$optionsRequest->username
),
default => $this->getCredentials($userEntity),
};

return $this->publicKeyCredentialRequestOptionsFactory->create(
$this->profile,
$allowedCredentials,
Expand Down
8 changes: 8 additions & 0 deletions src/symfony/src/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use Webauthn\Counter\ThrowExceptionIfInvalid;
use Webauthn\MetadataService\CertificateChain\PhpCertificateChainValidator;
use Webauthn\PublicKeyCredentialCreationOptions;
use Webauthn\SimpleFakeCredentialGenerator;

final class Configuration implements ConfigurationInterface
{
Expand Down Expand Up @@ -62,6 +63,13 @@ public function getConfigTreeBuilder(): TreeBuilder
->defaultNull()
->info('Creates PSR-7 HTTP Request and Response instances from Symfony ones.')
->end()
->scalarNode('fake_credential_generator')
->defaultValue(SimpleFakeCredentialGenerator::class)
->cannotBeEmpty()
->info(
'A service that implements the FakeCredentialGenerator to generate fake credentials for preventing username enumeration.'
)
->end()
->scalarNode('clock')
->defaultValue('webauthn.clock.default')
->info('PSR-20 Clock service.')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Symfony\Component\Config\Definition\Builder\ParentNodeDefinitionInterface;
use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\HttpFoundation\Request;
Expand All @@ -35,6 +36,7 @@
use Webauthn\Bundle\Service\PublicKeyCredentialCreationOptionsFactory;
use Webauthn\Bundle\Service\PublicKeyCredentialRequestOptionsFactory;
use Webauthn\Denormalizer\WebauthnSerializerFactory;
use Webauthn\FakeCredentialGenerator;
use function array_key_exists;
use function assert;

Expand Down Expand Up @@ -490,7 +492,7 @@ private function getAssertionOptionsBuilderId(
new Reference(PublicKeyCredentialSourceRepositoryInterface::class),
new Reference(PublicKeyCredentialRequestOptionsFactory::class),
$config['profile'],
new Reference(WebauthnSerializerFactory::class),
new Reference(FakeCredentialGenerator::class, ContainerInterface::NULL_ON_INVALID_REFERENCE),
]);
$container->setDefinition($optionsBuilderId, $optionsBuilder);

Expand Down
4 changes: 4 additions & 0 deletions src/symfony/src/DependencyInjection/WebauthnExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Symfony\Component\Config\FileLocator;
use Symfony\Component\Config\Loader\FileLoader;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Extension\Extension;
use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface;
Expand Down Expand Up @@ -45,6 +46,7 @@
use Webauthn\CeremonyStep\CeremonyStepManagerFactory;
use Webauthn\CeremonyStep\TopOriginValidator;
use Webauthn\Counter\CounterChecker;
use Webauthn\FakeCredentialGenerator;
use Webauthn\MetadataService\CanLogData;
use Webauthn\MetadataService\CertificateChain\CertificateChainValidator;
use Webauthn\MetadataService\Event\CanDispatchEvents;
Expand Down Expand Up @@ -100,6 +102,7 @@ public function load(array $configs, ContainerBuilder $container): void
$container->setAlias('webauthn.http_client', $config['http_client']);
$container->setAlias('webauthn.logger', $config['logger']);

$container->setAlias(FakeCredentialGenerator::class, $config['fake_credential_generator']);
$container->setAlias(PublicKeyCredentialSourceRepository::class, $config['credential_repository']);
$container->setAlias(PublicKeyCredentialSourceRepositoryInterface::class, $config['credential_repository']);
$container->setAlias(PublicKeyCredentialUserEntityRepository::class, $config['user_repository']);
Expand Down Expand Up @@ -287,6 +290,7 @@ private function loadRequestControllersSupport(ContainerBuilder $container, arra
new Reference(PublicKeyCredentialSourceRepositoryInterface::class),
new Reference(PublicKeyCredentialRequestOptionsFactory::class),
$requestConfig['profile'],
new Reference(FakeCredentialGenerator::class, ContainerInterface::NULL_ON_INVALID_REFERENCE),
]);
$container->setDefinition($assertionOptionsBuilderId, $assertionOptionsBuilder);
}
Expand Down
9 changes: 9 additions & 0 deletions src/symfony/src/Resources/config/services.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
declare(strict_types=1);

use Lcobucci\Clock\SystemClock;
use Psr\Cache\CacheItemPoolInterface;
use Psr\Http\Message\RequestFactoryInterface;
use Psr\Log\NullLogger;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
Expand Down Expand Up @@ -42,10 +43,12 @@
use Webauthn\Denormalizer\PublicKeyCredentialSourceDenormalizer;
use Webauthn\Denormalizer\PublicKeyCredentialUserEntityDenormalizer;
use Webauthn\Denormalizer\WebauthnSerializerFactory;
use Webauthn\FakeCredentialGenerator;
use Webauthn\MetadataService\Denormalizer\ExtensionDescriptorDenormalizer;
use Webauthn\MetadataService\Denormalizer\MetadataStatementSerializerFactory;
use Webauthn\PublicKeyCredentialLoader;
use Webauthn\PublicKeyCredentialSourceRepository;
use Webauthn\SimpleFakeCredentialGenerator;
use Webauthn\TokenBinding\IgnoreTokenBindingHandler;
use Webauthn\TokenBinding\SecTokenBindingHandler;
use Webauthn\TokenBinding\TokenBindingNotSupportedHandler;
Expand Down Expand Up @@ -80,6 +83,11 @@
->args([param('webauthn.secured_relying_party_ids')])
;

$container
->set(SimpleFakeCredentialGenerator::class)
->args([service(CacheItemPoolInterface::class)->nullOnInvalid()])
;

$container
->set('webauthn.ceremony_step_manager.request')
->class(CeremonyStepManager::class)
Expand Down Expand Up @@ -162,6 +170,7 @@
service(AuthenticatorAssertionResponseValidator::class),
service(PublicKeyCredentialUserEntityRepositoryInterface::class),
service(PublicKeyCredentialSourceRepository::class)->nullOnInvalid(),
service(FakeCredentialGenerator::class)->nullOnInvalid(),
]);

$container
Expand Down
15 changes: 15 additions & 0 deletions src/webauthn/src/FakeCredentialGenerator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

declare(strict_types=1);

namespace Webauthn;

use Symfony\Component\HttpFoundation\Request;

interface FakeCredentialGenerator
{
/**
* @return PublicKeyCredentialDescriptor[]
*/
public function generate(Request $request, string $username): array;
}
67 changes: 67 additions & 0 deletions src/webauthn/src/SimpleFakeCredentialGenerator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<?php

declare(strict_types=1);

namespace Webauthn;

use Psr\Cache\CacheItemPoolInterface;
use Symfony\Component\HttpFoundation\Request;
use function count;
use function is_int;

final class SimpleFakeCredentialGenerator implements FakeCredentialGenerator
{
public function __construct(
private readonly null|CacheItemPoolInterface $cache = null
) {
}

/**
* @return PublicKeyCredentialDescriptor[]
*/
public function generate(Request $request, string $username): array
{
if ($this->cache === null) {
return $this->generateCredentials($username);
}

$cacheKey = 'fake_credentials_' . hash('xxh128', $username);
$cacheItem = $this->cache->getItem($cacheKey);
if ($cacheItem->isHit()) {
return $cacheItem->get();
}

$credentials = $this->generateCredentials($username);
$cacheItem->set($credentials);
$this->cache->save($cacheItem);

return $credentials;
}

/**
* @return PublicKeyCredentialDescriptor[]
*/
private function generateCredentials(string $username): array
{
$transports = [
PublicKeyCredentialDescriptor::AUTHENTICATOR_TRANSPORT_USB,
PublicKeyCredentialDescriptor::AUTHENTICATOR_TRANSPORT_NFC,
PublicKeyCredentialDescriptor::AUTHENTICATOR_TRANSPORT_BLE,
];
$credentials = [];
for ($i = 0; $i < random_int(1, 3); $i++) {
$randomTransportKeys = array_rand($transports, random_int(1, count($transports)));
if (is_int($randomTransportKeys)) {
$randomTransportKeys = [$randomTransportKeys];
}
$randomTransports = array_values(array_intersect_key($transports, array_flip($randomTransportKeys)));
$credentials[] = PublicKeyCredentialDescriptor::create(
PublicKeyCredentialDescriptor::CREDENTIAL_TYPE_PUBLIC_KEY,
hash('sha256', random_bytes(16) . $username),
$randomTransports
);
}

return $credentials;
}
}

0 comments on commit 51b7a85

Please sign in to comment.