diff --git a/docs/Server/Monitoring.md b/docs/Server/Monitoring.md index 30f3aeaa..dc5b3638 100644 --- a/docs/Server/Monitoring.md +++ b/docs/Server/Monitoring.md @@ -54,6 +54,7 @@ off rather than returned empty. | `connectionsRejected` | Connections turned away at the connection limit. | | `connectionsWriteTimeouts`, `connectionsIdleTimeouts` | Connections closed by the write or idle timeout. | | `connectionsRequestSizeExceeded` | Connections dropped because a request exceeded `setMaxRequestSize`. | +| `connectionsProtocolErrors` | Connections dropped by a malformed/undecodable request PDU (Notice of Disconnection). | | `connectionsMax` | The configured connection limit (`0` is unlimited). | | `operationsCompleted`, `operationsFailed` | Total operations and the failed subset. | | `operationsByType` | Per-type counts, e.g. `search=1402, bind=210, add=8`. | diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler.php index 1c783ced..cb565fb7 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler.php @@ -101,6 +101,7 @@ public function handle(): ?ConnectionObservation # Per RFC 4511 §4.1.1, a PDU that cannot be processed (malformed) warrants a disconnect with a protocol # error. The NoticeOfDisconnectSent event records the specific reason. $this->sendNoticeOfDisconnect('The message could not be processed.'); + $closeReason = ConnectionObservation::ProtocolError; } catch (Throwable $e) { if ($this->queue->isConnected()) { $this->sendNoticeOfDisconnect(cause: $e); diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerMonitorHandler.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerMonitorHandler.php index 4d43f807..fd7c84e7 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerMonitorHandler.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerMonitorHandler.php @@ -100,6 +100,7 @@ private function attributes(MetricsSnapshot $snapshot): array 'connectionsWriteTimeouts' => [(string) $connections->writeTimeouts], 'connectionsIdleTimeouts' => [(string) $connections->idleTimeouts], 'connectionsRequestSizeExceeded' => [(string) $connections->requestSizeExceeded], + 'connectionsProtocolErrors' => [(string) $connections->protocolErrors], 'connectionsMax' => [(string) $this->options->getMaxConnections()], 'operationsCompleted' => [(string) $operations->total()], 'operationsFailed' => [(string) $operations->totalErrors()], diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerPagingHandler.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerPagingHandler.php index 90d88dff..78e45f3a 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerPagingHandler.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerPagingHandler.php @@ -14,7 +14,6 @@ namespace FreeDSx\Ldap\Protocol\ServerProtocolHandler; use FreeDSx\Ldap\Control\PagingControl; -use FreeDSx\Ldap\Control\Sorting\SortingResponseControl; use FreeDSx\Ldap\Entry\Entries; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; @@ -119,9 +118,12 @@ public function handleRequest( $controls[] = new PagingControl(0, ''); } - $sortControl = $this->sortingControl($message); - if ($sortControl !== null) { - $controls[] = new SortingResponseControl(0); + $sortResponse = $this->sortingResponseControl( + $this->sortingControl($message), + $this->schema, + ); + if ($sortResponse !== null) { + $controls[] = $sortResponse; } $pagingRequest->markProcessed(); diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchHandler.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchHandler.php index a9923dc5..a1336b38 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchHandler.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchHandler.php @@ -13,7 +13,6 @@ namespace FreeDSx\Ldap\Protocol\ServerProtocolHandler; -use FreeDSx\Ldap\Control\Sorting\SortingResponseControl; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Operation\Request\AbandonRequest; use FreeDSx\Ldap\Operation\Request\CancelRequest; @@ -88,9 +87,12 @@ public function handleRequest( $state, ); - $sortControl = $this->sortingControl($message); - $responseControls = $sortControl !== null - ? [new SortingResponseControl(0)] + $sortResponse = $this->sortingResponseControl( + $this->sortingControl($message), + $this->schema, + ); + $responseControls = $sortResponse !== null + ? [$sortResponse] : []; $this->sendEntriesToClient( diff --git a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchTrait.php b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchTrait.php index e6f1932a..f5dd1eec 100644 --- a/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchTrait.php +++ b/src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchTrait.php @@ -17,6 +17,7 @@ use FreeDSx\Ldap\Control\ControlBag; use FreeDSx\Ldap\Control\PagingControl; use FreeDSx\Ldap\Control\Sorting\SortingControl; +use FreeDSx\Ldap\Control\Sorting\SortingResponseControl; use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Exception\RuntimeException; @@ -30,6 +31,7 @@ use FreeDSx\Ldap\Protocol\LdapMessageRequest; use FreeDSx\Ldap\Protocol\LdapMessageResponse; use FreeDSx\Ldap\Protocol\Queue\ServerQueue; +use FreeDSx\Ldap\Schema\Schema; use Generator; trait ServerSearchTrait @@ -191,4 +193,41 @@ private function sortingControl(LdapMessageRequest $message): ?SortingControl ? $control : null; } + + /** + * The RFC 2891 sort response control, with the result code per §1.2 (first unsortable key wins). + */ + private function sortingResponseControl( + ?SortingControl $sortControl, + Schema $schema, + ): ?SortingResponseControl { + if ($sortControl === null) { + return null; + } + + foreach ($sortControl->getSortKeys() as $sortKey) { + $attribute = $sortKey->getAttribute(); + $attributeType = $schema->getAttributeType($attribute); + + if ($attributeType === null) { + return new SortingResponseControl( + ResultCode::NO_SUCH_ATTRIBUTE, + $attribute, + ); + } + + $orderingRule = $sortKey->getOrderingRule(); + $unknownRule = $orderingRule !== null + && $schema->getMatchingRule($orderingRule) === null; + + if ($unknownRule || ($orderingRule === null && $attributeType->orderingOid === null)) { + return new SortingResponseControl( + ResultCode::INAPPROPRIATE_MATCHING, + $attribute, + ); + } + } + + return new SortingResponseControl(ResultCode::SUCCESS); + } } diff --git a/src/FreeDSx/Ldap/Server/Metrics/Observation/ConnectionObservation.php b/src/FreeDSx/Ldap/Server/Metrics/Observation/ConnectionObservation.php index 74c7d4c7..9e990f2b 100644 --- a/src/FreeDSx/Ldap/Server/Metrics/Observation/ConnectionObservation.php +++ b/src/FreeDSx/Ldap/Server/Metrics/Observation/ConnectionObservation.php @@ -31,4 +31,6 @@ enum ConnectionObservation: string case IdleTimeout = 'idle_timeout'; case RequestSizeExceeded = 'request_size_exceeded'; + + case ProtocolError = 'protocol_error'; } diff --git a/src/FreeDSx/Ldap/Server/Metrics/Recorder/InMemoryMetricsRecorder.php b/src/FreeDSx/Ldap/Server/Metrics/Recorder/InMemoryMetricsRecorder.php index 0b8a6c05..a60e5999 100644 --- a/src/FreeDSx/Ldap/Server/Metrics/Recorder/InMemoryMetricsRecorder.php +++ b/src/FreeDSx/Ldap/Server/Metrics/Recorder/InMemoryMetricsRecorder.php @@ -75,6 +75,11 @@ final class InMemoryMetricsRecorder implements MetricsRecorderInterface, Metrics */ private int $requestSizeExceeded = 0; + /** + * @var int<0, max> + */ + private int $protocolErrors = 0; + /** * @var array> */ @@ -177,6 +182,7 @@ public function connectionObserved(ConnectionObservation $observation): void ConnectionObservation::WriteTimeout => $this->writeTimeouts++, ConnectionObservation::IdleTimeout => $this->idleTimeouts++, ConnectionObservation::RequestSizeExceeded => $this->requestSizeExceeded++, + ConnectionObservation::ProtocolError => $this->protocolErrors++, }; } @@ -206,6 +212,7 @@ public function snapshot(): MetricsSnapshot $this->writeTimeouts, $this->idleTimeouts, $this->requestSizeExceeded, + $this->protocolErrors, ), $this->operationMetrics(), $this->operationsInProgress, diff --git a/src/FreeDSx/Ldap/Server/Metrics/Snapshot/ConnectionMetrics.php b/src/FreeDSx/Ldap/Server/Metrics/Snapshot/ConnectionMetrics.php index ee7c73d2..33bc57b0 100644 --- a/src/FreeDSx/Ldap/Server/Metrics/Snapshot/ConnectionMetrics.php +++ b/src/FreeDSx/Ldap/Server/Metrics/Snapshot/ConnectionMetrics.php @@ -27,6 +27,7 @@ public function __construct( public int $writeTimeouts = 0, public int $idleTimeouts = 0, public int $requestSizeExceeded = 0, + public int $protocolErrors = 0, ) {} /** @@ -41,6 +42,7 @@ public function toArray(): array 'write_timeouts' => $this->writeTimeouts, 'idle_timeouts' => $this->idleTimeouts, 'request_size_exceeded' => $this->requestSizeExceeded, + 'protocol_errors' => $this->protocolErrors, ]; } @@ -56,6 +58,7 @@ public static function fromArray(array $data): self writeTimeouts: SnapshotValue::toInt($data['write_timeouts'] ?? null), idleTimeouts: SnapshotValue::toInt($data['idle_timeouts'] ?? null), requestSizeExceeded: SnapshotValue::toInt($data['request_size_exceeded'] ?? null), + protocolErrors: SnapshotValue::toInt($data['protocol_errors'] ?? null), ); } } diff --git a/src/FreeDSx/Ldap/Server/ServerRunner/PcntlServerRunner.php b/src/FreeDSx/Ldap/Server/ServerRunner/PcntlServerRunner.php index 618c0fe9..2d211b7f 100644 --- a/src/FreeDSx/Ldap/Server/ServerRunner/PcntlServerRunner.php +++ b/src/FreeDSx/Ldap/Server/ServerRunner/PcntlServerRunner.php @@ -54,6 +54,8 @@ class PcntlServerRunner implements ServerRunnerInterface private const EXIT_CODE_REQUEST_SIZE_EXCEEDED = 12; + private const EXIT_CODE_PROTOCOL_ERROR = 13; + private SocketServer $server; /** @@ -214,6 +216,7 @@ private function childExitCodeFor(?ConnectionObservation $closeReason): int ConnectionObservation::WriteTimeout => self::EXIT_CODE_WRITE_TIMEOUT, ConnectionObservation::IdleTimeout => self::EXIT_CODE_IDLE_TIMEOUT, ConnectionObservation::RequestSizeExceeded => self::EXIT_CODE_REQUEST_SIZE_EXCEEDED, + ConnectionObservation::ProtocolError => self::EXIT_CODE_PROTOCOL_ERROR, default => 0, }; } @@ -233,6 +236,7 @@ private function recordChildCloseReason( self::EXIT_CODE_WRITE_TIMEOUT => ConnectionObservation::WriteTimeout, self::EXIT_CODE_IDLE_TIMEOUT => ConnectionObservation::IdleTimeout, self::EXIT_CODE_REQUEST_SIZE_EXCEEDED => ConnectionObservation::RequestSizeExceeded, + self::EXIT_CODE_PROTOCOL_ERROR => ConnectionObservation::ProtocolError, default => null, }; diff --git a/tests/unit/Protocol/ServerProtocolHandler/ServerMonitorHandlerTest.php b/tests/unit/Protocol/ServerProtocolHandler/ServerMonitorHandlerTest.php index 28adf04c..9f3c5e58 100644 --- a/tests/unit/Protocol/ServerProtocolHandler/ServerMonitorHandlerTest.php +++ b/tests/unit/Protocol/ServerProtocolHandler/ServerMonitorHandlerTest.php @@ -258,6 +258,19 @@ public function test_it_reports_connections_closed_by_an_oversized_request(): vo ); } + public function test_it_reports_connections_closed_by_a_protocol_error(): void + { + $this->metrics->connectionObserved(ConnectionObservation::ProtocolError); + $this->metrics->connectionObserved(ConnectionObservation::ProtocolError); + + $entry = $this->handleAndCaptureEntry(); + + self::assertSame( + ['2'], + $entry->get('connectionsProtocolErrors')?->getValues(), + ); + } + public function test_it_reports_traffic_totals(): void { $this->metrics->trafficObserved(new TrafficObservation( diff --git a/tests/unit/Protocol/ServerProtocolHandler/ServerPagingHandlerTest.php b/tests/unit/Protocol/ServerProtocolHandler/ServerPagingHandlerTest.php index 927e7ded..ddfb96f9 100644 --- a/tests/unit/Protocol/ServerProtocolHandler/ServerPagingHandlerTest.php +++ b/tests/unit/Protocol/ServerProtocolHandler/ServerPagingHandlerTest.php @@ -31,6 +31,7 @@ use FreeDSx\Ldap\Protocol\Queue\ServerQueue; use FreeDSx\Ldap\Protocol\ServerProtocolHandler\ServerPagingHandler; use FreeDSx\Ldap\Schema\Schema; +use FreeDSx\Ldap\Schema\StandardSchemaProvider; use FreeDSx\Ldap\Search\Filters; use FreeDSx\Ldap\Server\AccessControl\AccessControlInterface; use FreeDSx\Ldap\Server\Backend\LdapBackendInterface; @@ -77,7 +78,7 @@ protected function setUp(): void $this->mockFilterEvaluator = $this->createMock(FilterEvaluatorInterface::class); $this->mockAccessControl = $this->createMock(AccessControlInterface::class); $this->requestHistory = new RequestHistory(); - $this->schema = new Schema(); + $this->schema = StandardSchemaProvider::buildCore(); $this->sentMessages = []; $this->mockFilterEvaluator @@ -623,6 +624,39 @@ public function test_sort_control_appends_sorting_response_control_to_done_messa ); } + public function test_sort_by_unknown_attribute_reports_no_such_attribute(): void + { + $message = new LdapMessageRequest( + 2, + $this->makeSearchRequest(), + new PagingControl(10, ''), + new SortingControl(SortKey::ascending('bogusAttr')), + ); + + $this->mockBackend + ->method('search') + ->willReturn(new EntryStream($this->makeGenerator())); + + $this->subject->handleRequest( + $message, + $this->mockToken, + ); + + $sortControl = $this->doneMessage()->controls()->get(Control::OID_SORTING_RESPONSE); + self::assertInstanceOf( + SortingResponseControl::class, + $sortControl, + ); + self::assertSame( + ResultCode::NO_SUCH_ATTRIBUTE, + $sortControl->getResult(), + ); + self::assertSame( + 'bogusAttr', + $sortControl->getAttribute(), + ); + } + public function test_no_sort_control_does_not_append_sorting_response_control(): void { $message = $this->makeSearchMessage(size: 10); diff --git a/tests/unit/Protocol/ServerProtocolHandler/ServerSearchHandlerTest.php b/tests/unit/Protocol/ServerProtocolHandler/ServerSearchHandlerTest.php index dcbc7377..1d61cb46 100644 --- a/tests/unit/Protocol/ServerProtocolHandler/ServerSearchHandlerTest.php +++ b/tests/unit/Protocol/ServerProtocolHandler/ServerSearchHandlerTest.php @@ -32,6 +32,7 @@ use FreeDSx\Ldap\Protocol\Queue\ServerQueue; use FreeDSx\Ldap\Protocol\ServerProtocolHandler\ServerSearchHandler; use FreeDSx\Ldap\Schema\Schema; +use FreeDSx\Ldap\Schema\StandardSchemaProvider; use FreeDSx\Ldap\Search\Filters; use FreeDSx\Ldap\Server\AccessControl\AccessControlInterface; use FreeDSx\Ldap\Server\Backend\LdapBackendInterface; @@ -73,7 +74,7 @@ protected function setUp(): void $this->mockBackend = $this->createMock(LdapBackendInterface::class); $this->mockFilterEvaluator = $this->createMock(FilterEvaluatorInterface::class); $this->mockAccessControl = $this->createMock(AccessControlInterface::class); - $this->schema = new Schema(); + $this->schema = StandardSchemaProvider::buildCore(); $this->sentMessages = []; $this->mockAccessControl @@ -646,6 +647,70 @@ public function test_sort_control_appends_sorting_response_control_to_done_messa ); } + public function test_sort_by_unknown_attribute_reports_no_such_attribute(): void + { + $search = new LdapMessageRequest( + 2, + (new SearchRequest(Filters::present('cn')))->base('dc=foo,dc=bar'), + new SortingControl(SortKey::ascending('bogusAttr')), + ); + + $this->mockBackend + ->method('search') + ->willReturn(new EntryStream($this->makeGenerator())); + + $this->subject->handleRequest( + $search, + $this->mockToken, + ); + + $done = end($this->sentMessages); + self::assertInstanceOf(LdapMessageResponse::class, $done); + $sortControl = $done->controls()->get(Control::OID_SORTING_RESPONSE); + self::assertInstanceOf( + SortingResponseControl::class, + $sortControl, + ); + self::assertSame( + ResultCode::NO_SUCH_ATTRIBUTE, + $sortControl->getResult(), + ); + self::assertSame( + 'bogusAttr', + $sortControl->getAttribute(), + ); + } + + public function test_sort_by_attribute_without_ordering_rule_reports_inappropriate_matching(): void + { + $search = new LdapMessageRequest( + 2, + (new SearchRequest(Filters::present('cn')))->base('dc=foo,dc=bar'), + new SortingControl(SortKey::ascending('userPassword')), + ); + + $this->mockBackend + ->method('search') + ->willReturn(new EntryStream($this->makeGenerator())); + + $this->subject->handleRequest( + $search, + $this->mockToken, + ); + + $done = end($this->sentMessages); + self::assertInstanceOf(LdapMessageResponse::class, $done); + $sortControl = $done->controls()->get(Control::OID_SORTING_RESPONSE); + self::assertInstanceOf( + SortingResponseControl::class, + $sortControl, + ); + self::assertSame( + ResultCode::INAPPROPRIATE_MATCHING, + $sortControl->getResult(), + ); + } + public function test_no_sort_control_does_not_append_sorting_response_control(): void { $search = new LdapMessageRequest( diff --git a/tests/unit/Protocol/ServerProtocolHandlerTest.php b/tests/unit/Protocol/ServerProtocolHandlerTest.php index 4ef3a9e7..ebad250a 100644 --- a/tests/unit/Protocol/ServerProtocolHandlerTest.php +++ b/tests/unit/Protocol/ServerProtocolHandlerTest.php @@ -159,8 +159,13 @@ public function test_it_sends_a_notice_of_disconnect_on_a_protocol_exception_fro $this->expectNoticeOfDisconnect('The message could not be processed.'); - $this->handlerWith(new StubMiddlewareHandler(OperationOutcomeResult::succeeded())) + $closeReason = $this->handlerWith(new StubMiddlewareHandler(OperationOutcomeResult::succeeded())) ->handle(); + + self::assertSame( + ConnectionObservation::ProtocolError, + $closeReason, + ); } public function test_a_request_size_exceeded_is_recorded_and_sends_a_notice_of_disconnect(): void @@ -196,8 +201,13 @@ public function test_it_sends_a_notice_of_disconnect_on_an_encoder_exception_fro $this->expectNoticeOfDisconnect('The message could not be processed.'); - $this->handlerWith(new StubMiddlewareHandler(OperationOutcomeResult::succeeded())) + $closeReason = $this->handlerWith(new StubMiddlewareHandler(OperationOutcomeResult::succeeded())) ->handle(); + + self::assertSame( + ConnectionObservation::ProtocolError, + $closeReason, + ); } public function test_it_ends_normally_on_a_socket_exception_from_the_message_queue(): void diff --git a/tests/unit/Server/Metrics/Recorder/InMemoryMetricsRecorderTest.php b/tests/unit/Server/Metrics/Recorder/InMemoryMetricsRecorderTest.php index 8bad9511..07046cc5 100644 --- a/tests/unit/Server/Metrics/Recorder/InMemoryMetricsRecorderTest.php +++ b/tests/unit/Server/Metrics/Recorder/InMemoryMetricsRecorderTest.php @@ -187,6 +187,7 @@ public function test_it_counts_rejected_and_timed_out_connections(): void $this->subject->connectionObserved(ConnectionObservation::WriteTimeout); $this->subject->connectionObserved(ConnectionObservation::IdleTimeout); $this->subject->connectionObserved(ConnectionObservation::RequestSizeExceeded); + $this->subject->connectionObserved(ConnectionObservation::ProtocolError); $connections = $this->subject->snapshot()->connections; @@ -206,6 +207,10 @@ public function test_it_counts_rejected_and_timed_out_connections(): void 1, $connections->requestSizeExceeded, ); + self::assertSame( + 1, + $connections->protocolErrors, + ); } public function test_taking_the_delta_returns_the_metrics_and_resets_them(): void