[1.0.0] Paging session limits / request ID validation / misc fixes.

docs/Server/Configuration.md

@@ -47,6 +47,7 @@ LDAP Server Configuration
     * [ServerOptions:setMaxSearchPageSize](#setmaxsearchpagesize)
     * [ServerOptions:setMaxSearchLookthrough](#setmaxsearchlookthrough)
     * [ServerOptions:setMaxSearchPagedLookthrough](#setmaxsearchpagedlookthrough)
+    * [ServerOptions:setMaxPagingSessions](#setmaxpagingsessions)
     * [ServerOptions:setSearchLimitRules](#setsearchlimitrules)
 * [Directory Synchronization](#directory-synchronization)
     * [ServerOptions:setReplicationConfig](#setreplicationconfig)
@@ -717,6 +718,15 @@ for ordinary searches. A value of `0` falls back to the regular lookthrough limi
 
 **Default**: `0` (use the regular lookthrough limit)
 
+------------------
+#### setMaxPagingSessions
+
+Cap how many paged searches one connection may leave unfinished, since each holds its result state until the connection
+closes. Starting one past the cap discards the least recently started session, logged as `paging.session_evicted`. A
+client resuming a discarded session is refused with an invalid cookie. A value of `0` removes the cap.
+
+**Default**: `25`
+
 ------------------
 #### setSearchLimitRules
 

docs/Server/Logging.md

@@ -45,6 +45,7 @@ $server = new LdapServer((new ServerOptions())->setLogger($logger));
 | `control.critical.rejected` | on      | notice | Client sent a critical control the server doesn't support |
 | `schema.violation`          | on      | notice | Add/Modify violates the schema (rejected, or allowed under Lenient mode / the Relax control) |
 | `session.disconnect_notice` | on      | notice | Server sends an unsolicited Notice of Disconnect          |
+| `paging.session_evicted`    | on      | notice | A connection hit `setMaxPagingSessions`, so its least recently started paged search was discarded |
 | `entry.added`               | off     | info   | Add succeeds (audit-trail)                                |
 | `entry.modified`            | off     | info   | Modify succeeds (audit-trail)                             |
 | `entry.deleted`             | off     | info   | Delete succeeds (audit-trail)                             |

src/FreeDSx/Ldap/Container/Provider/ConnectionGraphContainerProvider.php

@@ -76,6 +76,7 @@ private function makeBindStrategy(Container $container): PasswordPolicyBindStrat
             return new ReplicaBindStrategy(
                 $engine,
                 $container->get(ReplicaPasswordStateStoreInterface::class),
+                $container->get(WritableStorageBackend::class),
             );
         }
 

src/FreeDSx/Ldap/Container/Provider/HandlerContainerProvider.php

@@ -262,6 +262,7 @@ private function makePagingHandler(
             requestHistory: $context->requestHistory,
             schema: $options->getSchema(),
             limits: $searchLimits ?? $options->makeSearchLimits(),
+            eventLogger: $context->eventLogger,
         );
     }
 

src/FreeDSx/Ldap/Protocol/Bind/Sasl/OptionsBuilder/PlainMechanismOptionsBuilder.php

@@ -47,7 +47,14 @@ function (?string $authzId, string $authcId, string $password): bool {
                     MechanismName::PLAIN,
                 );
 
-                if ($identity === null || !$this->hashService->verify($password, $identity->password)) {
+                // Refuses a name that reached no stored value at the cost of one that did.
+                if ($identity === null) {
+                    $this->hashService->verifyDummy($password);
+
+                    return false;
+                }
+
+                if (!$this->hashService->verify($password, $identity->password)) {
                     return false;
                 }
 

src/FreeDSx/Ldap/Protocol/Bind/Sasl/SaslExchange.php

@@ -17,6 +17,7 @@
 use FreeDSx\Ldap\Entry\Dn;
 use FreeDSx\Ldap\Exception\InvalidArgumentException;
 use FreeDSx\Ldap\Exception\OperationException;
+use FreeDSx\Ldap\Exception\RequestValidationException;
 use FreeDSx\Ldap\Operation\LdapResult;
 use FreeDSx\Ldap\Operation\Request\SaslBindRequest;
 use FreeDSx\Ldap\Operation\Response\BindResponse;
@@ -303,9 +304,15 @@ private function sendBindInProgress(LdapMessageRequest $message, ?string $respon
      * Validates that the message received mid-exchange is a SASL bind continuation.
      *
      * @throws OperationException if the client sends a non-SASL request.
+     * @throws RequestValidationException if the continuation carries an unusable message ID.
      */
     private function requireSaslContinuation(LdapMessageRequest $message): SaslBindRequest
     {
+        // Continuations skip the validation middleware, and the challenge echoes back whatever ID arrives.
+        if ($message->getMessageId() === 0) {
+            throw new RequestValidationException('The message ID 0 cannot be used in a client request.');
+        }
+
         $request = $message->getRequest();
 
         if ($request instanceof SaslBindRequest) {

src/FreeDSx/Ldap/Protocol/Queue/ServerQueue.php

@@ -18,6 +18,7 @@
 use FreeDSx\Asn1\Exception\PartialPduException;
 use FreeDSx\Asn1\Type\AbstractType;
 use FreeDSx\Ldap\Exception\ProtocolException;
+use FreeDSx\Ldap\Exception\RequestValidationException;
 use FreeDSx\Ldap\Exception\RuntimeException;
 use FreeDSx\Ldap\Exception\UnsolicitedNotificationException;
 use FreeDSx\Ldap\Operation\Request\AbandonRequest;
@@ -35,6 +36,7 @@
 use FreeDSx\Socket\Socket;
 use Generator;
 
+use function count;
 use function strlen;
 
 /**
@@ -44,6 +46,11 @@
  */
 class ServerQueue extends LdapQueue implements ConnectionControl
 {
+    /**
+     * Well above what a pipelining client sends mid-stream, but low enough to bound what one connection can buffer.
+     */
+    private const MAX_PENDING_MESSAGES = 128;
+
     /**
      * @var LdapMessageRequest[]
      */
@@ -83,30 +90,35 @@ public function getMessage(?int $id = null): LdapMessageRequest
             return array_shift($this->pendingMessages);
         }
 
-        $message = $this->getAndValidateMessage($id);
-
-        if (!$message instanceof LdapMessageRequest) {
-            throw new ProtocolException(sprintf(
-                'Expected an instance of LdapMessageResponse but got: %s',
-                get_class($message),
-            ));
-        }
-
-        return $message;
+        return $this->readRequest($id);
     }
 
     /**
      * Checks whether an Abandon or Cancel targeting a message ID has arrived.
      *
      * Other messages received while peeking are buffered.
+     *
+     * @throws RequestValidationException if a peeked message carries an unusable message ID.
      */
     public function peekForCancelSignal(int $inFlightMessageId): ?LdapMessageRequest
     {
+        // Nothing drains the buffer until the stream ends, so reading pauses here, and the socket holds the backlog.
+        if (count($this->pendingMessages) >= self::MAX_PENDING_MESSAGES) {
+            return null;
+        }
+
         if (!$this->hasPendingData()) {
             return null;
         }
 
-        $message = $this->getMessage();
+        // Reads past anything already buffered; draining that here would re-inspect the same message forever.
+        $message = $this->readRequest(null);
+
+        // Peeking skips the validation middleware, and buffering defers a refusal a persist stream never reaches.
+        if ($message->getMessageId() === 0) {
+            throw new RequestValidationException('The message ID 0 cannot be used in a client request.');
+        }
+
         $request = $message->getRequest();
 
         if ($this->isAbandonOrCancelRequest($request, $inFlightMessageId)) {
@@ -204,6 +216,24 @@ private function applyInterceptors(LdapMessageResponse $response): LdapMessageRe
         return $response;
     }
 
+    /**
+     * @throws ProtocolException
+     * @throws UnsolicitedNotificationException
+     * @throws ConnectionException
+     */
+    private function readRequest(?int $id): LdapMessageRequest
+    {
+        $message = $this->getAndValidateMessage($id);
+        if (!$message instanceof LdapMessageRequest) {
+            throw new ProtocolException(sprintf(
+                'Expected an instance of LdapMessageResponse but got: %s',
+                get_class($message),
+            ));
+        }
+
+        return $message;
+    }
+
     /**
      * @param RequestInterface $request
      * @param int $inFlightMessageId

src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerPagingHandler.php

@@ -32,6 +32,9 @@
 use FreeDSx\Ldap\Server\RequestHistory;
 use FreeDSx\Ldap\Server\Backend\Storage\FilterEvaluatorInterface;
 use FreeDSx\Ldap\Server\Operation\SearchOperationResult;
+use FreeDSx\Ldap\Server\Logging\EventContext;
+use FreeDSx\Ldap\Server\Logging\EventLogger;
+use FreeDSx\Ldap\Server\Logging\ServerEvent;
 use FreeDSx\Ldap\Server\SearchLimits;
 use FreeDSx\Ldap\Server\Token\TokenInterface;
 use Generator;
@@ -55,6 +58,7 @@ public function __construct(
         private readonly Schema $schema,
         private readonly PagingRequestComparator $requestComparator = new PagingRequestComparator(),
         private readonly SearchLimits $limits = new SearchLimits(),
+        private readonly ?EventLogger $eventLogger = null,
     ) {}
 
     /**
@@ -392,12 +396,39 @@ private function findOrMakePagingRequest(LdapMessageRequest $message): PagingReq
             return $this->findPagingRequestOrThrow($pagingControl->getCookie());
         }
 
+        $this->evictOldestSessionAtLimit();
+
         $pagingRequest = $this->makePagingRequest($message);
         $this->requestHistory->pagingRequest()->add($pagingRequest);
 
         return $pagingRequest;
     }
 
+    /**
+     * Unfinished sessions would otherwise retain a generator apiece for the life of the connection.
+     */
+    private function evictOldestSessionAtLimit(): void
+    {
+        $limit = $this->limits->maxPagingSessions ?? 0;
+        $requests = $this->requestHistory->pagingRequest();
+
+        if ($limit <= 0 || $requests->count() < $limit) {
+            return;
+        }
+
+        $oldest = $requests->oldest();
+        if ($oldest === null) {
+            return;
+        }
+
+        $requests->remove($oldest);
+        $this->requestHistory->removePagingGenerator($oldest->getNextCookie());
+        $this->eventLogger?->record(
+            ServerEvent::PagingSessionEvicted,
+            [EventContext::LIMIT => $limit],
+        );
+    }
+
     /**
      * @throws OperationException
      */

src/FreeDSx/Ldap/Server/Backend/Auth/PasswordHashService.php

@@ -21,7 +21,7 @@
  *
  * @author Chad Sikorra <Chad.Sikorra@gmail.com>
  */
-final readonly class PasswordHashService
+readonly class PasswordHashService
 {
     /**
      * Read-only legacy prefixes recognized in addition to the writable {@see PasswordHashScheme} set.
@@ -35,7 +35,7 @@
     ];
 
     /**
-     * @param int|null $hashCost bcrypt cost forwarded to password_hash(); null uses the PHP default (10). Does not apply to argon2.
+     * @param int|null $hashCost bcrypt cost forwarded to password_hash(); null uses the PHP default. Does not apply to argon2.
      */
     public function __construct(
         private PasswordHashScheme $scheme = PasswordHashScheme::Bcrypt,

src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/SqlFilter/SqlFilterTranslatorTrait.php

@@ -617,7 +617,8 @@ private function validateAttribute(string $attribute): string
     {
         $lower = strtolower($attribute);
 
-        if (preg_match('/^([a-z][a-z0-9-]*|\d+(\.\d+)+)(;[a-z0-9-]+)*$/', $lower) !== 1) {
+        // The D modifier matters here: without it a trailing newline slips past the whitelist and into an identifier.
+        if (preg_match('/^([a-z][a-z0-9-]*|\d+(\.\d+)+)(;[a-z0-9-]+)*$/D', $lower) !== 1) {
             throw new InvalidAttributeException(sprintf(
                 'Attribute description "%s" is not a valid RFC 4512 attribute description.',
                 $attribute,

src/FreeDSx/Ldap/Server/Logging/EventContext.php

@@ -76,6 +76,8 @@ final class EventContext
 
     public const REMOVED = 'removed';
 
+    public const LIMIT = 'limit';
+
     public const DURATION_SECONDS = 'duration_seconds';
 
     public const NEW_RDN = 'new_rdn';

src/FreeDSx/Ldap/Server/Logging/EventLogPolicy.php

@@ -60,6 +60,7 @@ public static function default(): self
             ServerEvent::CriticalControlRejected,
             ServerEvent::SchemaViolation,
             ServerEvent::SyncEntrySkipped,
+            ServerEvent::PagingSessionEvicted,
             ServerEvent::JournalPruned,
             ServerEvent::JournalPruneFailed,
             ServerEvent::NoticeOfDisconnectSent,

src/FreeDSx/Ldap/Server/Logging/ServerEvent.php

@@ -45,6 +45,7 @@ enum ServerEvent: string
     case OperationRefused               = 'operation.refused';
     case SchemaViolation                = 'schema.violation';
     case SyncEntrySkipped               = 'sync.entry_skipped';
+    case PagingSessionEvicted           = 'paging.session_evicted';
     case JournalPruned                  = 'journal.pruned';
     case JournalPruneFailed             = 'journal.prune_failed';
     case NoticeOfDisconnectSent         = 'session.disconnect_notice';
@@ -75,6 +76,7 @@ public function level(): string
             self::NoticeOfDisconnectSent,
             self::WriteTimeout,
             self::IdleTimeout,
+            self::PagingSessionEvicted,
             self::PasswordPolicyExpired,
             self::PasswordPolicyChangeRejected => LogLevel::NOTICE,
             default => LogLevel::INFO,

src/FreeDSx/Ldap/Server/Middleware/RequestValidationMiddleware.php

@@ -19,21 +19,22 @@
 use FreeDSx\Ldap\Server\Middleware\Pipeline\MiddlewareInterface;
 use FreeDSx\Ldap\Server\Middleware\Pipeline\ServerRequestContext;
 
-use function in_array;
 use function sprintf;
 
 /**
- * Rejects a message whose ID is zero or already used on this connection (RFC 4511 §4.1.1.1).
+ * Rejects a message whose ID is zero or in use by an operation still being served (RFC 4511 §4.1.1.1).
  *
  * @internal
  * @author Chad Sikorra <Chad.Sikorra@gmail.com>
  */
 final class RequestValidationMiddleware implements MiddlewareInterface
 {
     /**
-     * @var int[]
+     * Operations still being served, keyed by ID so a lookup does not scan.
+     *
+     * @var array<int, true>
      */
-    private array $messageIds = [];
+    private array $outstanding = [];
 
     /**
      * @throws RequestValidationException
@@ -48,14 +49,18 @@ public function process(
             throw new RequestValidationException('The message ID 0 cannot be used in a client request.');
         }
 
-        // Stricter than RFC 4511 §4.1.1.1, which only forbids reusing an ID that is still outstanding. Since the
-        // server processes a connection's messages serially, rejecting any reused ID is safe and simpler.
-        if (in_array($messageId, $this->messageIds, true)) {
+        // RFC 4511 §4.1.1.1 forbids only an ID in use by an operation still being served.
+        if (isset($this->outstanding[$messageId])) {
             throw new RequestValidationException(sprintf('The message ID %s is not valid.', $messageId));
         }
 
-        $this->messageIds[] = $messageId;
+        $this->outstanding[$messageId] = true;
 
-        return $next->handle($context);
+        try {
+            // The response writer sits below this, so the operation is answered by the time the call returns.
+            return $next->handle($context);
+        } finally {
+            unset($this->outstanding[$messageId]);
+        }
     }
 }

src/FreeDSx/Ldap/Server/Paging/PagingRequests.php

@@ -36,6 +36,21 @@ public function add(PagingRequest $request): void
         $this->requests[] = $request;
     }
 
+    public function count(): int
+    {
+        return count($this->requests);
+    }
+
+    /**
+     * The least recently started request, since they are held in the order they were added.
+     */
+    public function oldest(): ?PagingRequest
+    {
+        $requests = array_values($this->requests);
+
+        return $requests[0] ?? null;
+    }
+
     public function remove(PagingRequest $toRemove): void
     {
         foreach ($this->requests as $i => $pagingRequest) {