[1.0.0] Security related fixes: value add / delete and sort keys.

docs/Server/Access-Control.md

@@ -449,7 +449,8 @@ A denied assertion is folded away before the query runs, so the attribute behave
 ```
 
 This is what stops a value being recovered a guess at a time. Reads are unaffected: denying the filter does not strip
-the value, and denying the read does not stop the filter. Pair the two to get both.
+the value, and denying the read does not stop the filter. Pair the two to get both. A value-level modify on a
+filter-denied attribute is gated the same way; see [Value-Level Modify](#value-level-modify).
 
 An extensible match that names no attribute type is refused with `inappropriateMatching`, since it asserts against
 every attribute at once and it's not possible to make attribute level ACL decisions against it.
@@ -502,10 +503,24 @@ Four things to keep in mind:
 - A grant is required in addition to read access. A permissive `AttributeRule` cannot re-expose a confidential
   attribute.
 - Administrators are locked out too until granted. Only the break-glass manager bypasses this.
-- Writes are unaffected, which is what keeps `userPassword` settable by its owner while unreadable.
+- Setting the value is unaffected. A whole-attribute replace overwrites unconditionally, so an owner can still set
+  `userPassword` while unable to read it.
 - A custom schema must carry the extension. Supplying your own `userPassword` definition through
   a schema source without it silently drops the protection.
 
+### Value-Level Modify
+
+A modify that names a specific value, an `add` or a `delete` of one value, is answered from the entry's current values:
+adding a value already present, or deleting one that is absent, fails with its own result code. That lets an identity
+that may write but not read an attribute confirm a value one guess at a time. Deleting the whole attribute is the same,
+revealing whether any value is present at all.
+
+So on an attribute withheld from filters, whether by `X-CONFIDENTIAL` or a [Filter Rule](#filter-rules), a value-level
+`add` or `delete` of another entry is refused with `insufficientAccessRights`. Use a whole-attribute replace, which
+overwrites unconditionally and reveals nothing about the prior value; a replace with no values still removes the
+attribute. The identity's own entry is exempt, so a self password change that supplies the old value (delete the old,
+add the new) still works.
+
 Replication is not affected. A content sync ships every visible entry whole, so a replica receives confidential
 attributes without a grant for them. See [Replication](Replication.md#access-control).
 

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

@@ -27,6 +27,8 @@
 use FreeDSx\Ldap\Server\AccessControl\AccessControlInterface;
 use FreeDSx\Ldap\Server\AccessControl\WithheldAttributePolicy;
 use FreeDSx\Ldap\Server\AccessControl\WithheldFilterRewriter;
+use FreeDSx\Ldap\Server\AccessControl\WithheldSortKeyFilter;
+use FreeDSx\Ldap\Server\AccessControl\WithheldValueModifyGuard;
 use FreeDSx\Ldap\Server\Middleware\AliasDereferenceMiddleware;
 use FreeDSx\Ldap\Server\Middleware\AssertionMiddleware;
 use FreeDSx\Ldap\Server\Middleware\AttributeTypeCanonicalizationMiddleware;
@@ -167,12 +169,16 @@ private function makeAliasDereferenceMiddleware(Container $container): AliasDere
 
     private function makeWithheldAttributeMiddleware(Container $container): WithheldAttributeMiddleware
     {
-        return new WithheldAttributeMiddleware(new WithheldFilterRewriter(
-            new WithheldAttributePolicy(
-                $container->get(AccessControlInterface::class),
-                $container->get(ServerOptions::class)->getSchema(),
-            ),
-        ));
+        $policy = new WithheldAttributePolicy(
+            $container->get(AccessControlInterface::class),
+            $container->get(ServerOptions::class)->getSchema(),
+        );
+
+        return new WithheldAttributeMiddleware(
+            new WithheldFilterRewriter($policy),
+            new WithheldSortKeyFilter($policy),
+            new WithheldValueModifyGuard($policy),
+        );
     }
 
     private function makeAssertionMiddleware(Container $container): AssertionMiddleware

src/FreeDSx/Ldap/Server/AccessControl/Subject/MatchesBoundIdentity.php

@@ -0,0 +1,42 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * This file is part of the FreeDSx LDAP package.
+ *
+ * (c) Chad Sikorra <Chad.Sikorra@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace FreeDSx\Ldap\Server\AccessControl\Subject;
+
+use FreeDSx\Ldap\Entry\Dn;
+use FreeDSx\Ldap\Server\Token\AuthenticatedTokenInterface;
+use FreeDSx\Ldap\Server\Token\TokenInterface;
+
+/**
+ * Answers whether a token's bound DN is the given target entry (case-insensitive), the shared notion of "self".
+ *
+ * @author Chad Sikorra <Chad.Sikorra@gmail.com>
+ */
+trait MatchesBoundIdentity
+{
+    /**
+     * Note: Without a target there is nothing to be the same as, and only an authenticated token has a bound DN.
+     *
+     * @param TokenInterface $token
+     * @param Dn|null $target
+     * @return bool
+     */
+    private function isBoundIdentity(
+        TokenInterface $token,
+        ?Dn $target,
+    ): bool {
+        return $target !== null
+            && $token instanceof AuthenticatedTokenInterface
+            && $token->getResolvedDn()->normalize()->toString() === $target->normalize()->toString();
+    }
+}

src/FreeDSx/Ldap/Server/AccessControl/Subject/SelfSubjectMatcher.php

@@ -14,7 +14,6 @@
 namespace FreeDSx\Ldap\Server\AccessControl\Subject;
 
 use FreeDSx\Ldap\Entry\Dn;
-use FreeDSx\Ldap\Server\Token\AuthenticatedTokenInterface;
 use FreeDSx\Ldap\Server\Token\TokenInterface;
 
 /**
@@ -24,15 +23,15 @@
  */
 final class SelfSubjectMatcher implements TargetDependentSubjectInterface
 {
+    use MatchesBoundIdentity;
+
     public function matches(
         TokenInterface $token,
         ?Dn $targetDn,
     ): bool {
-        // Without a target there is nothing to be the same as, so this can never be a self match.
-        if ($targetDn === null || !$token instanceof AuthenticatedTokenInterface) {
-            return false;
-        }
-
-        return $token->getResolvedDn()->normalize()->toString() === $targetDn->normalize()->toString();
+        return $this->isBoundIdentity(
+            $token,
+            $targetDn,
+        );
     }
 }

src/FreeDSx/Ldap/Server/AccessControl/WithheldSortKeyFilter.php

@@ -0,0 +1,51 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * This file is part of the FreeDSx LDAP package.
+ *
+ * (c) Chad Sikorra <Chad.Sikorra@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace FreeDSx\Ldap\Server\AccessControl;
+
+use FreeDSx\Ldap\Control\Sorting\SortingControl;
+use FreeDSx\Ldap\Control\Sorting\SortKey;
+use FreeDSx\Ldap\Server\Token\TokenInterface;
+
+use function array_filter;
+use function array_values;
+
+/**
+ * Drops sort keys naming an attribute withheld from the identity, so the result order cannot reflect its value.
+ *
+ * @internal
+ *
+ * @author Chad Sikorra <Chad.Sikorra@gmail.com>
+ */
+final readonly class WithheldSortKeyFilter
+{
+    public function __construct(private WithheldAttributePolicy $policy) {}
+
+    /**
+     * Removes any key on a withheld attribute, leaving the control present so the sort is still reported as performed.
+     */
+    public function stripWithheld(
+        SortingControl $control,
+        TokenInterface $token,
+    ): void {
+        $kept = array_values(array_filter(
+            $control->getSortKeys(),
+            fn(SortKey $sortKey): bool => !$this->policy->isWithheldFromFilter(
+                $sortKey->getAttribute(),
+                $token,
+            ),
+        ));
+
+        $control->setSortKeys(...$kept);
+    }
+}

src/FreeDSx/Ldap/Server/AccessControl/WithheldValueModifyGuard.php

@@ -0,0 +1,78 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * This file is part of the FreeDSx LDAP package.
+ *
+ * (c) Chad Sikorra <Chad.Sikorra@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace FreeDSx\Ldap\Server\AccessControl;
+
+use FreeDSx\Ldap\Entry\Change;
+use FreeDSx\Ldap\Exception\OperationException;
+use FreeDSx\Ldap\Operation\Request\ModifyRequest;
+use FreeDSx\Ldap\Operation\ResultCode;
+use FreeDSx\Ldap\Server\AccessControl\Subject\MatchesBoundIdentity;
+use FreeDSx\Ldap\Server\Token\TokenInterface;
+
+use function in_array;
+
+/**
+ * Refuses a value-level add or delete that would confirm a value of an attribute the identity may not read.
+ *
+ * @internal
+ *
+ * @author Chad Sikorra <Chad.Sikorra@gmail.com>
+ */
+final readonly class WithheldValueModifyGuard
+{
+    use MatchesBoundIdentity;
+
+    public function __construct(private WithheldAttributePolicy $policy) {}
+
+    /**
+     * A value assertion reveals through its result code whether the value is present, so it needs read of the attribute.
+     *
+     * A value assertion on the identity's own entry discloses nothing it does not already control.
+     *
+     * @throws OperationException
+     */
+    public function assertAllowed(
+        ModifyRequest $request,
+        TokenInterface $token,
+    ): void {
+        if ($this->isBoundIdentity($token, $request->getDn())) {
+            return;
+        }
+
+        foreach ($request->getChanges() as $change) {
+            if (!$this->isAddOrDelete($change)) {
+                continue;
+            }
+
+            if ($this->policy->isWithheldFromFilter($change->getAttribute()->getName(), $token)) {
+                throw new OperationException(
+                    'Insufficient access rights.',
+                    ResultCode::INSUFFICIENT_ACCESS_RIGHTS,
+                );
+            }
+        }
+    }
+
+    /**
+     * An add or delete is answered from the stored values, so its result code reveals them; a replace is unconditional.
+     */
+    private function isAddOrDelete(Change $change): bool
+    {
+        return in_array(
+            $change->getType(),
+            [Change::TYPE_ADD, Change::TYPE_DELETE],
+            true,
+        );
+    }
+}

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

@@ -13,19 +13,24 @@
 
 namespace FreeDSx\Ldap\Server\Middleware;
 
+use FreeDSx\Ldap\Control\Control;
+use FreeDSx\Ldap\Control\Sorting\SortingControl;
 use FreeDSx\Ldap\Operation\Request\CompareRequest;
+use FreeDSx\Ldap\Operation\Request\ModifyRequest;
 use FreeDSx\Ldap\Operation\Request\SearchRequest;
 use FreeDSx\Ldap\Protocol\Factory\ResponseFactory;
 use FreeDSx\Ldap\Protocol\Queue\Response\ResponseStream;
 use FreeDSx\Ldap\Server\AccessControl\WithheldFilterRewriter;
+use FreeDSx\Ldap\Server\AccessControl\WithheldSortKeyFilter;
+use FreeDSx\Ldap\Server\AccessControl\WithheldValueModifyGuard;
 use FreeDSx\Ldap\Server\Middleware\Pipeline\MiddlewareHandlerInterface;
 use FreeDSx\Ldap\Server\Middleware\Pipeline\MiddlewareInterface;
 use FreeDSx\Ldap\Server\Middleware\Pipeline\ServerRequestContext;
 use FreeDSx\Ldap\Server\Operation\CompareOperationResult;
 use FreeDSx\Ldap\Server\Operation\OperationOutcomeResult;
 
 /**
- * Withholds attributes from assertions before the request reaches storage.
+ * Withholds attributes from search and compare assertions, sort keys, and value-level modifies, before storage.
  *
  * @internal
  *
@@ -35,6 +40,8 @@
 {
     public function __construct(
         private WithheldFilterRewriter $rewriter,
+        private WithheldSortKeyFilter $sortKeys,
+        private WithheldValueModifyGuard $valueModify,
         private ResponseFactory $responseFactory = new ResponseFactory(),
     ) {}
 
@@ -47,10 +54,15 @@ public function process(
         if ($request instanceof SearchRequest) {
             return $this->processSearch($context, $next, $request);
         }
-
         if ($request instanceof CompareRequest) {
             return $this->processCompare($context, $next, $request);
         }
+        if ($request instanceof ModifyRequest) {
+            $this->valueModify->assertAllowed(
+                $request,
+                $context->tokenOrFail(),
+            );
+        }
 
         return $next->handle($context);
     }
@@ -60,9 +72,18 @@ private function processSearch(
         MiddlewareHandlerInterface $next,
         SearchRequest $request,
     ): ResponseStream {
+        $token = $context->tokenOrFail();
+
+        $sort = $context->message->controls()->get(Control::OID_SORTING);
+        if ($sort instanceof SortingControl) {
+            $this->sortKeys->stripWithheld(
+                $sort,
+                $token,
+            );
+        }
         $rewritten = $this->rewriter->rewrite(
             $request->getFilter(),
-            $context->tokenOrFail(),
+            $token,
         );
 
         if ($this->rewriter->isAbsoluteFalse($rewritten)) {
@@ -93,7 +114,6 @@ private function processCompare(
         if (!$this->rewriter->isAbsoluteFalse($rewritten)) {
             return $next->handle($context);
         }
-
         $result = CompareOperationResult::completed(
             $context->message,
             false,