diff --git a/docs/Server/Configuration.md b/docs/Server/Configuration.md index 82459197..687ff8c4 100644 --- a/docs/Server/Configuration.md +++ b/docs/Server/Configuration.md @@ -41,6 +41,7 @@ LDAP Server Configuration * [ServerOptions:setMaxSearchSize](#setmaxsearchsize) * [ServerOptions:setMaxSearchTimeLimit](#setmaxsearchtimelimit) * [ServerOptions:setMaxSearchPageSize](#setmaxsearchpagesize) + * [ServerOptions:setMaxSearchLookthrough](#setmaxsearchlookthrough) * [SASL Options](#sasl-options) * [ServerOptions:setSaslMechanisms](#setsaslmechanisms) @@ -589,6 +590,19 @@ The maximum number of entries the server will return per page in a paged search. **Default**: `1000` +------------------ +#### setMaxSearchLookthrough + +Maximum number of entries the server will examine while evaluating a search before returning an `ADMIN_LIMIT_EXCEEDED` +result code. Unlike the size limit (entries returned), this caps entries inspected, so it bounds an unindexed filter that +scans many entries to return few. Raise it above the largest legitimate subtree a client may scan, or set `0` to disable. + +**Default**: `5000` + +> It applies only to filters evaluated in PHP (array/JSON backends, and SQL backends when the filter cannot be pushed to the +> index); indexed equality and prefix filters are bounded by the database and are not counted. Paged searches are subject to +> this limit cumulatively across all pages. + ## Monitoring Opt-in observability. See [Server Monitoring](Monitoring.md) for the full guide, the `cn=monitor` attributes, and the diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/PdoListQueryBuilder.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/PdoListQueryBuilder.php index 5728ef8a..3263dcd7 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/PdoListQueryBuilder.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/PdoListQueryBuilder.php @@ -36,7 +36,7 @@ public function build( string $base, bool $subtree, ?SqlFilterResult $filterResult, - ?int $sizeLimit, + ?int $sqlLimit, array $sortKeys, ): SqlQuery { $query = match (true) { @@ -52,8 +52,8 @@ public function build( ); } - if ($sizeLimit !== null) { - $query = $query->appending(' LIMIT ' . $sizeLimit); + if ($sqlLimit !== null) { + $query = $query->appending(' LIMIT ' . $sqlLimit); } return $query; diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/PdoStorage.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/PdoStorage.php index ab12ee31..f152828b 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/PdoStorage.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/PdoStorage.php @@ -129,9 +129,12 @@ public function list(StorageListOptions $options): EntryStream $filterResult = $this->translator->translate($options->filter); $isPreFiltered = $filterResult !== null && $filterResult->isExact; - $sqlLimit = $isPreFiltered && $options->sizeLimit > 0 - ? $options->sizeLimit - : null; + // Exact is bound by sizeLimit. Inexact is PHP re-evaluated: cap candidate transfer at lookthrough+1. + $sqlLimit = match (true) { + $isPreFiltered && $options->sizeLimit > 0 => $options->sizeLimit, + !$isPreFiltered && $options->lookthroughLimit > 0 => $options->lookthroughLimit + 1, + default => null, + }; $query = $this->queryBuilder->build( $options->baseDn->normalize()->toString(), diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/SearchStreamBuilder.php b/src/FreeDSx/Ldap/Server/Backend/Storage/SearchStreamBuilder.php index 6525ce2d..8733c3e5 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/SearchStreamBuilder.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/SearchStreamBuilder.php @@ -124,12 +124,23 @@ private function injectHasSubordinates(Entry $entry): Entry /** * @param Generator $generator * @return Generator + * @throws OperationException */ private function wrapWithFilterEvaluation( Generator $generator, FilterInterface $filter, ): Generator { + $lookthrough = $this->limits->maxSearchLookthrough; + $examined = 0; + foreach ($generator as $entry) { + if ($lookthrough > 0 && ++$examined > $lookthrough) { + throw new OperationException( + 'Administrative limit exceeded.', + ResultCode::ADMIN_LIMIT_EXCEEDED, + ); + } + if ($this->filterEvaluator->evaluate($entry, $filter)) { yield $entry; } diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/StorageListOptions.php b/src/FreeDSx/Ldap/Server/Backend/Storage/StorageListOptions.php index e463844f..72f92ecc 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/StorageListOptions.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/StorageListOptions.php @@ -35,6 +35,7 @@ public function __construct( public readonly int $timeLimit = 0, public readonly int $sizeLimit = 0, public readonly array $sortKeys = [], + public readonly int $lookthroughLimit = 0, ) {} /** diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/WritableStorageBackend.php b/src/FreeDSx/Ldap/Server/Backend/Storage/WritableStorageBackend.php index de40ea74..03d63d9b 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/WritableStorageBackend.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/WritableStorageBackend.php @@ -186,6 +186,7 @@ public function search( sortKeys: $sortingControl instanceof SortingControl ? $sortingControl->getSortKeys() : [], + lookthroughLimit: $this->limits->maxSearchLookthrough, ); try { diff --git a/src/FreeDSx/Ldap/Server/SearchLimits.php b/src/FreeDSx/Ldap/Server/SearchLimits.php index 884be108..04279351 100644 --- a/src/FreeDSx/Ldap/Server/SearchLimits.php +++ b/src/FreeDSx/Ldap/Server/SearchLimits.php @@ -22,5 +22,6 @@ public function __construct( public int $maxSearchSize = 0, public int $maxSearchTimeLimit = 0, public int $maxSearchPageSize = 0, + public int $maxSearchLookthrough = 0, ) {} } diff --git a/src/FreeDSx/Ldap/ServerOptions.php b/src/FreeDSx/Ldap/ServerOptions.php index 146aab35..816460fe 100644 --- a/src/FreeDSx/Ldap/ServerOptions.php +++ b/src/FreeDSx/Ldap/ServerOptions.php @@ -205,6 +205,8 @@ final class ServerOptions private int $maxSearchPageSize = 1000; + private int $maxSearchLookthrough = 5000; + private ?Closure $onServerReady = null; private ?ConfigReloaderInterface $configReloader = null; @@ -869,12 +871,28 @@ public function setMaxSearchPageSize(int $maxSearchPageSize): self return $this; } + /** + * Maximum entries examined per search before adminLimitExceeded (default 5000). Guards unindexed scans. Zero disables. + */ + public function getMaxSearchLookthrough(): int + { + return $this->maxSearchLookthrough; + } + + public function setMaxSearchLookthrough(int $maxSearchLookthrough): self + { + $this->maxSearchLookthrough = $maxSearchLookthrough; + + return $this; + } + public function makeSearchLimits(): SearchLimits { return new SearchLimits( maxSearchSize: $this->maxSearchSize, maxSearchTimeLimit: $this->maxSearchTimeLimit, maxSearchPageSize: $this->maxSearchPageSize, + maxSearchLookthrough: $this->maxSearchLookthrough, ); } diff --git a/tests/integration/LdapServerTest.php b/tests/integration/LdapServerTest.php index e658d659..fb9918bc 100644 --- a/tests/integration/LdapServerTest.php +++ b/tests/integration/LdapServerTest.php @@ -298,7 +298,7 @@ public function testWhoAmIWhenNotAuthenticated(): void public function testItCanHandlingPaging(): void { $this->stopServer(); - $this->createServerProcess('tcp', ['--entries=5000']); + $this->createServerProcess('tcp', ['--entries=5000', '--max-search-lookthrough=0']); $this->authenticate(); $allEntries = []; diff --git a/tests/integration/Storage/LdapBackendStorageTest.php b/tests/integration/Storage/LdapBackendStorageTest.php index 186628f3..721cf8ad 100644 --- a/tests/integration/Storage/LdapBackendStorageTest.php +++ b/tests/integration/Storage/LdapBackendStorageTest.php @@ -519,6 +519,29 @@ public function testSortControlPlacesMissingAttributeFirstWhenDescending(): void ); } + public function testInexactSearchTripsLookthroughLimit(): void + { + $this->stopServer(); + $this->createServerProcess( + 'tcp', + [ + ...static::storageExtraArgs(), + '--seed-entries=10', + '--max-search-lookthrough=3', + ], + ); + $this->authenticateUser(); + + $this->expectException(OperationException::class); + $this->expectExceptionCode(ResultCode::ADMIN_LIMIT_EXCEEDED); + + $this->ldapClient()->search( + Operations::search(Filters::endsWith('cn', 'zzz')) + ->base('dc=foo,dc=bar') + ->useSubtreeScope(), + ); + } + /** * Hook for subclasses to route the shared server through a different backend. * diff --git a/tests/support/LdapBackendStorageCommand.php b/tests/support/LdapBackendStorageCommand.php index 5a551cc9..a2515864 100644 --- a/tests/support/LdapBackendStorageCommand.php +++ b/tests/support/LdapBackendStorageCommand.php @@ -96,6 +96,13 @@ protected function configure(): void null, InputOption::VALUE_NONE, 'Enable the cn=monitor entry', + ) + ->addOption( + 'max-search-lookthrough', + null, + InputOption::VALUE_REQUIRED, + 'Maximum entries examined per search before adminLimitExceeded (0 = no limit)', + '0', ); } @@ -195,6 +202,7 @@ protected function execute( ->setSocketAcceptTimeout(0.1) ->setSchemaValidationMode($validationMode) ->setMonitorEnabled((bool) $input->getOption('monitor')) + ->setMaxSearchLookthrough((int) $this->getStringOption($input, 'max-search-lookthrough')) ->setOnServerReady(fn() => fwrite(STDOUT, 'server starting...' . PHP_EOL)); if ($input->getOption('allow-relax')) { diff --git a/tests/support/LdapServerCommand.php b/tests/support/LdapServerCommand.php index 25c666a4..643e6ed2 100644 --- a/tests/support/LdapServerCommand.php +++ b/tests/support/LdapServerCommand.php @@ -66,6 +66,13 @@ protected function configure(): void 'Number of extra entries to seed (used to test paging)', '0', ) + ->addOption( + 'max-search-lookthrough', + null, + InputOption::VALUE_REQUIRED, + 'Maximum entries examined per search before adminLimitExceeded (0 = no limit)', + '5000', + ) ->addOption( 'sasl', null, @@ -161,6 +168,7 @@ protected function execute( ->setUseSsl($useSsl) ->setAllowAnonymous($allowAnonymous) ->setSocketAcceptTimeout(0.1) + ->setMaxSearchLookthrough((int) $this->getStringOption($input, 'max-search-lookthrough')) ->setOnServerReady(fn() => fwrite(STDOUT, 'server starting...' . PHP_EOL)); if ($reloadFlagFile !== '') { diff --git a/tests/unit/Server/Backend/Storage/Adapter/SqliteStorageTest.php b/tests/unit/Server/Backend/Storage/Adapter/SqliteStorageTest.php index 50990926..425871f0 100644 --- a/tests/unit/Server/Backend/Storage/Adapter/SqliteStorageTest.php +++ b/tests/unit/Server/Backend/Storage/Adapter/SqliteStorageTest.php @@ -31,6 +31,7 @@ use FreeDSx\Ldap\Server\Backend\Storage\Exception\StorageIoException; use FreeDSx\Ldap\Server\Backend\Storage\StorageListOptions; use FreeDSx\Ldap\Server\Backend\Storage\WritableStorageBackend; +use FreeDSx\Ldap\Server\SearchLimits; use FreeDSx\Ldap\Control\ControlBag; use FreeDSx\Ldap\Server\Backend\Write\Command\AddCommand; use FreeDSx\Ldap\Server\Backend\Write\Command\DeleteCommand; @@ -282,6 +283,61 @@ public function test_search_matches_mixed_case_attribute_via_lowercase_filter(): ); } + public function test_search_inexact_filter_trips_lookthrough_limit(): void + { + $storage = SqliteStorage::forPcntl(':memory:'); + $backend = new WritableStorageBackend( + $storage, + new SearchLimits(maxSearchLookthrough: 2), + ); + $backend->add( + new AddCommand(new Entry(new Dn('dc=example,dc=com'), new Attribute('dc', 'example'))), + $this->systemContext(), + ); + foreach (['Ann', 'Bob', 'Cyd'] as $cn) { + $backend->add( + new AddCommand(new Entry(new Dn("cn={$cn},dc=example,dc=com"), new Attribute('cn', $cn))), + $this->context(), + ); + } + + self::expectException(OperationException::class); + self::expectExceptionCode(ResultCode::ADMIN_LIMIT_EXCEEDED); + + $request = (new SearchRequest(Filters::endsWith('cn', 'x'))) + ->base('dc=example,dc=com') + ->useSubtreeScope(); + iterator_to_array($backend->search($request)->entries); + } + + public function test_search_exact_filter_is_not_subject_to_lookthrough(): void + { + $storage = SqliteStorage::forPcntl(':memory:'); + $backend = new WritableStorageBackend( + $storage, + new SearchLimits(maxSearchLookthrough: 1), + ); + $backend->add( + new AddCommand(new Entry(new Dn('dc=example,dc=com'), new Attribute('dc', 'example'))), + $this->systemContext(), + ); + foreach (['Ann', 'Bob', 'Cyd'] as $cn) { + $backend->add( + new AddCommand(new Entry(new Dn("cn={$cn},dc=example,dc=com"), new Attribute('cn', $cn))), + $this->context(), + ); + } + + $request = (new SearchRequest(Filters::equal('cn', 'Ann'))) + ->base('dc=example,dc=com') + ->useSubtreeScope(); + + self::assertCount( + 1, + iterator_to_array($backend->search($request)->entries), + ); + } + public function test_atomic_rolls_back_on_exception(): void { $threw = false; diff --git a/tests/unit/Server/Backend/Storage/Adapter/WritableStorageBackendTest.php b/tests/unit/Server/Backend/Storage/Adapter/WritableStorageBackendTest.php index 0a27f2a1..0ef725a2 100644 --- a/tests/unit/Server/Backend/Storage/Adapter/WritableStorageBackendTest.php +++ b/tests/unit/Server/Backend/Storage/Adapter/WritableStorageBackendTest.php @@ -688,6 +688,39 @@ public function test_search_converts_time_limit_exception_to_operation_exception iterator_to_array($subject->search($request)->entries); } + public function test_search_trips_lookthrough_limit_when_examined_exceeds_cap(): void + { + $subject = new WritableStorageBackend( + new InMemoryStorage([$this->base, $this->alice, $this->bob]), + new SearchLimits(maxSearchLookthrough: 2), + ); + + self::expectException(OperationException::class); + self::expectExceptionCode(ResultCode::ADMIN_LIMIT_EXCEEDED); + + $request = (new SearchRequest(new PresentFilter('objectClass'))) + ->base('dc=example,dc=com') + ->useSubtreeScope(); + iterator_to_array($subject->search($request)->entries); + } + + public function test_search_does_not_trip_lookthrough_limit_within_cap(): void + { + $subject = new WritableStorageBackend( + new InMemoryStorage([$this->base, $this->alice, $this->bob]), + new SearchLimits(maxSearchLookthrough: 100), + ); + + $request = (new SearchRequest(new PresentFilter('objectClass'))) + ->base('dc=example,dc=com') + ->useSubtreeScope(); + + self::assertCount( + 3, + iterator_to_array($subject->search($request)->entries), + ); + } + public function test_add_converts_storage_io_exception_to_unavailable_operation_exception(): void { $ioException = new StorageIoException('Unable to publish the storage update.'); diff --git a/tests/unit/ServerOptionsTest.php b/tests/unit/ServerOptionsTest.php index 567e119b..0b7fc061 100644 --- a/tests/unit/ServerOptionsTest.php +++ b/tests/unit/ServerOptionsTest.php @@ -724,18 +724,38 @@ public function test_it_can_set_max_search_page_size(): void ); } + public function test_max_search_lookthrough_defaults_to_5000(): void + { + self::assertSame( + 5000, + $this->subject->getMaxSearchLookthrough(), + ); + } + + public function test_it_can_set_max_search_lookthrough(): void + { + $this->subject->setMaxSearchLookthrough(5000); + + self::assertSame( + 5000, + $this->subject->getMaxSearchLookthrough(), + ); + } + public function test_make_search_limits_reflects_current_options(): void { $this->subject ->setMaxSearchSize(500) ->setMaxSearchTimeLimit(60) - ->setMaxSearchPageSize(250); + ->setMaxSearchPageSize(250) + ->setMaxSearchLookthrough(5000); self::assertEquals( new SearchLimits( maxSearchSize: 500, maxSearchTimeLimit: 60, maxSearchPageSize: 250, + maxSearchLookthrough: 5000, ), $this->subject->makeSearchLimits(), );