From ab7aef9443904ac16ad9cbcde55920fe75c42798 Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Sat, 6 Jun 2026 14:32:11 -0400 Subject: [PATCH] Normalize DNs in an RFC 4518 caseIgnore style way. --- composer.json | 2 +- src/FreeDSx/Ldap/Entry/Dn.php | 87 ++++++++--- src/FreeDSx/Ldap/Entry/DnNormalizer.php | 135 ++++++++++++++++++ .../Matching/DistinguishedNameComparator.php | 4 +- .../Subject/DnSubjectMatcher.php | 4 +- .../Subject/GroupSubjectMatcher.php | 9 +- .../Subject/SelfSubjectMatcher.php | 2 +- .../AccessControl/Target/DnTargetMatcher.php | 4 +- .../Storage/Adapter/InMemoryStorage.php | 6 +- .../Storage/Adapter/JsonFileStorage.php | 6 +- .../Backend/Storage/Adapter/PdoStorage.php | 10 +- .../Support/ArrayEntryStorageTrait.php | 13 +- tests/unit/Entry/DnNormalizerTest.php | 84 +++++++++++ .../DistinguishedNameComparatorTest.php | 20 +++ .../Storage/Adapter/InMemoryStorageTest.php | 11 ++ 15 files changed, 344 insertions(+), 53 deletions(-) create mode 100644 src/FreeDSx/Ldap/Entry/DnNormalizer.php create mode 100644 tests/unit/Entry/DnNormalizerTest.php diff --git a/composer.json b/composer.json index ba465679..3a2b2421 100644 --- a/composer.json +++ b/composer.json @@ -80,7 +80,7 @@ "docker compose -f tests/resources/openldap/docker-compose.yml up -d --build --wait", "LDAP_TESTS_ENABLED=1 php -d xdebug.mode=off vendor/bin/phpunit --testsuite integration" ], - "test-load": "@php -d xdebug.mode=off -d opcache.enable_cli=1 -d opcache.jit_buffer_size=128M -d opcache.jit=tracing tests/bin/ldap-load-test.php --backend=sqlite --runner=swoole --duration=10 --warmup=2 --clients=8 --output=text", + "test-load": "@php -d xdebug.mode=off -d opcache.enable_cli=1 -d opcache.jit=off tests/bin/ldap-load-test.php --backend=sqlite --runner=pcntl --duration=10 --warmup=2 --clients=8 --output=text", "test-load-compare": "@php -d xdebug.mode=off -d opcache.enable_cli=1 -d opcache.jit_buffer_size=128M -d opcache.jit=tracing tests/bin/ldap-bench-compare.php", "profile": "tests/profile/profile.sh", "profile-up": "docker compose -f tests/profile/docker-compose.yml up -d --build --wait", diff --git a/src/FreeDSx/Ldap/Entry/Dn.php b/src/FreeDSx/Ldap/Entry/Dn.php index 1521c5f4..273d9b08 100644 --- a/src/FreeDSx/Ldap/Entry/Dn.php +++ b/src/FreeDSx/Ldap/Entry/Dn.php @@ -26,7 +26,10 @@ use function implode; use function ltrim; use function preg_split; -use function strtolower; +use function str_ends_with; +use function strlen; +use function strpos; +use function substr; /** * Represents a Distinguished Name. @@ -41,6 +44,8 @@ class Dn implements IteratorAggregate, Countable, Stringable */ private ?array $pieces = null; + private ?Dn $normalized = null; + public function __construct(private readonly string $dn) {} public function __toString(): string @@ -48,6 +53,17 @@ public function __toString(): string return $this->dn; } + /** + * Wrap an already-canonical DN string, skipping re-normalization. + */ + public static function fromCanonical(string $canonical): self + { + $dn = new self($canonical); + $dn->normalized = $dn; + + return $dn; + } + /** * @throws UnexpectedValueException */ @@ -133,11 +149,18 @@ public static function isValid(Stringable|string $dn): bool } /** - * Return a normalised (lowercased) copy of this DN. + * Return the canonical (RFC 4518 caseIgnore) copy of this DN. */ public function normalize(): Dn { - return new Dn(strtolower($this->dn)); + if ($this->normalized !== null) { + return $this->normalized; + } + $canonical = DnNormalizer::canonicalize($this->dn); + + return $this->normalized = $canonical === $this->dn + ? $this + : self::fromCanonical($canonical); } /** @@ -147,15 +170,13 @@ public function normalize(): Dn */ public function isChildOf(Dn $parent): bool { - $parentDn = $parent->toString(); - if ($parentDn === '') { - return $this->getParent() === null - && $this->toString() !== ''; + $thisDn = $this->normalize()->toString(); + + if ($thisDn === '') { + return false; } - $myParent = $this->getParent(); - return $myParent !== null - && strtolower($myParent->toString()) === strtolower($parentDn); + return self::canonicalParent($thisDn) === $parent->normalize()->toString(); } /** @@ -165,27 +186,51 @@ public function isChildOf(Dn $parent): bool */ public function isDescendantOf(Dn $base): bool { - $baseDn = $base->toString(); - $thisDn = $this->toString(); + $baseDn = $base->normalize()->toString(); + $thisDn = $this->normalize()->toString(); if ($baseDn === '') { return $thisDn !== ''; } - - $baseLower = strtolower($baseDn); - if (strtolower($thisDn) === $baseLower) { + if ($thisDn === $baseDn) { return true; } + $ancestorSuffix = ',' . $baseDn; - $parent = $this->getParent(); - while ($parent !== null) { - if (strtolower($parent->toString()) === $baseLower) { - return true; + return str_ends_with($thisDn, $ancestorSuffix) + && self::isUnescaped($thisDn, strlen($thisDn) - strlen($ancestorSuffix)); + } + + /** + * Parent of an already-canonical DN: the substring after the first unescaped RDN separator. + */ + private static function canonicalParent(string $canonical): string + { + $offset = 0; + while (($pos = strpos($canonical, ',', $offset)) !== false) { + if (self::isUnescaped($canonical, $pos)) { + return substr($canonical, $pos + 1); } - $parent = $parent->getParent(); + $offset = $pos + 1; + } + + return ''; + } + + /** + * Whether the character at $pos is preceded by an even number of backslashes (i.e. not escaped). + */ + private static function isUnescaped( + string $value, + int $pos, + ): bool { + $slashes = 0; + + for ($i = $pos - 1; $i >= 0 && $value[$i] === '\\'; $i--) { + $slashes++; } - return false; + return $slashes % 2 === 0; } /** diff --git a/src/FreeDSx/Ldap/Entry/DnNormalizer.php b/src/FreeDSx/Ldap/Entry/DnNormalizer.php new file mode 100644 index 00000000..516a1ce7 --- /dev/null +++ b/src/FreeDSx/Ldap/Entry/DnNormalizer.php @@ -0,0 +1,135 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Entry; + +use FreeDSx\Ldap\Exception\InvalidArgumentException; +use FreeDSx\Ldap\Exception\UnexpectedValueException; +use FreeDSx\Ldap\Schema\Matching\Prep\StringPrep; +use FreeDSx\Ldap\Schema\Text; + +use function count; +use function implode; +use function preg_replace; +use function sort; +use function str_replace; +use function strtolower; +use function trim; + +/** + * Canonicalizes a DN for comparison and keying using the pragmatic RFC 4518 caseIgnore profile. + */ +final class DnNormalizer +{ + private static ?self $instance = null; + + private readonly StringPrep $prep; + + public function __construct() + { + $this->prep = new StringPrep(foldCase: true); + } + + /** + * Canonical DN form. + * + * Values are not unescaped, so an escaped edge space (cn=\ x\ ) does not fold like an unescaped one. + */ + public static function canonicalize(string $dn): string + { + return (self::$instance ??= new self())->doCanonicalize($dn); + } + + private function doCanonicalize(string $dn): string + { + if ($dn === '') { + return ''; + } + + try { + $rdns = (new Dn($dn))->toArray(); + } catch (UnexpectedValueException|InvalidArgumentException) { + return strtolower($dn); + } + + $ascii = Text::isAscii($dn); + $parts = []; + foreach ($rdns as $rdn) { + $parts[] = $this->canonicalizeRdn( + $rdn, + $ascii, + ); + } + + return implode( + ',', + $parts, + ); + } + + private function canonicalizeRdn( + Rdn $rdn, + bool $ascii, + ): string { + $components = []; + + foreach ($rdn->getAll() as $component) { + $components[] = strtolower(trim($component->getName())) + . '=' + . $this->canonicalizeValue( + $component->getValue(), + $ascii, + ); + } + + // Components of a multivalued RDN are an unordered set. Sort for a stable canonical form. + if (count($components) > 1) { + sort($components); + } + + return implode( + '+', + $components, + ); + } + + private function canonicalizeValue( + string $value, + bool $ascii, + ): string { + if ($ascii) { + return $this->canonicalizeAsciiValue($value); + } + + return $this->prep->prepareForEquality($value); + } + + private function canonicalizeAsciiValue(string $value): string + { + $folded = strtolower(str_replace( + "\0", + '', + $value, + )); + $collapsed = preg_replace( + '/[\x09-\x0D ]+/', + ' ', + $folded, + ) ?? $folded; + + return trim( + $collapsed, + ' ', + ); + } +} diff --git a/src/FreeDSx/Ldap/Schema/Matching/DistinguishedNameComparator.php b/src/FreeDSx/Ldap/Schema/Matching/DistinguishedNameComparator.php index f0e33a9e..f9faa6fd 100644 --- a/src/FreeDSx/Ldap/Schema/Matching/DistinguishedNameComparator.php +++ b/src/FreeDSx/Ldap/Schema/Matching/DistinguishedNameComparator.php @@ -46,8 +46,6 @@ public function substringMatches( private function normalize(string $dn): string { - return strtolower( - (new Dn($dn))->normalize()->toString(), - ); + return (new Dn($dn))->normalize()->toString(); } } diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Subject/DnSubjectMatcher.php b/src/FreeDSx/Ldap/Server/AccessControl/Subject/DnSubjectMatcher.php index ac81530a..95409386 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/Subject/DnSubjectMatcher.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/Subject/DnSubjectMatcher.php @@ -28,7 +28,7 @@ final class DnSubjectMatcher implements SubjectMatcherInterface public function __construct(string $dn) { - $this->normalizedDn = strtolower($dn); + $this->normalizedDn = (new Dn($dn))->normalize()->toString(); } public function matches( @@ -39,6 +39,6 @@ public function matches( return false; } - return strtolower($token->getResolvedDn()->toString()) === $this->normalizedDn; + return $token->getResolvedDn()->normalize()->toString() === $this->normalizedDn; } } diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Subject/GroupSubjectMatcher.php b/src/FreeDSx/Ldap/Server/AccessControl/Subject/GroupSubjectMatcher.php index 5b651ed3..949cb1ac 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/Subject/GroupSubjectMatcher.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/Subject/GroupSubjectMatcher.php @@ -77,10 +77,10 @@ public function matches( return false; } - $resolvedDn = self::normalizeDn($token->getResolvedDn()->toString()); + $resolvedDn = $token->getResolvedDn()->normalize()->toString(); foreach ($memberAttr->getValues() as $value) { - if (self::normalizeDn($value) === $resolvedDn) { + if ((new Dn($value))->normalize()->toString() === $resolvedDn) { return true; } } @@ -88,11 +88,6 @@ public function matches( return false; } - private static function normalizeDn(string $dn): string - { - return strtolower(preg_replace('/\s*([,=])\s*/', '$1', $dn) ?? $dn); - } - private function getGroupEntry(TokenInterface $token): ?Entry { if ($this->maxCacheSize === 0) { diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Subject/SelfSubjectMatcher.php b/src/FreeDSx/Ldap/Server/AccessControl/Subject/SelfSubjectMatcher.php index ae798e44..c31898a4 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/Subject/SelfSubjectMatcher.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/Subject/SelfSubjectMatcher.php @@ -32,6 +32,6 @@ public function matches( return false; } - return strtolower($token->getResolvedDn()->toString()) === strtolower($targetDn->toString()); + return $token->getResolvedDn()->normalize()->toString() === $targetDn->normalize()->toString(); } } diff --git a/src/FreeDSx/Ldap/Server/AccessControl/Target/DnTargetMatcher.php b/src/FreeDSx/Ldap/Server/AccessControl/Target/DnTargetMatcher.php index 3184b6c9..c10bb99f 100644 --- a/src/FreeDSx/Ldap/Server/AccessControl/Target/DnTargetMatcher.php +++ b/src/FreeDSx/Ldap/Server/AccessControl/Target/DnTargetMatcher.php @@ -26,11 +26,11 @@ final class DnTargetMatcher implements TargetMatcherInterface public function __construct(string $dn) { - $this->normalizedDn = strtolower($dn); + $this->normalizedDn = (new Dn($dn))->normalize()->toString(); } public function matches(Dn $dn): bool { - return strtolower($dn->toString()) === $this->normalizedDn; + return $dn->normalize()->toString() === $this->normalizedDn; } } diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/InMemoryStorage.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/InMemoryStorage.php index 092fe85c..58a65162 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/InMemoryStorage.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/InMemoryStorage.php @@ -46,12 +46,12 @@ public function __construct(array $entries = []) public function find(Dn $dn): ?Entry { - return $this->entries[$dn->toString()] ?? null; + return $this->entries[$dn->normalize()->toString()] ?? null; } public function exists(Dn $dn): bool { - return isset($this->entries[$dn->toString()]); + return isset($this->entries[$dn->normalize()->toString()]); } public function list(StorageListOptions $options): EntryStream @@ -69,7 +69,7 @@ public function store(Entry $entry): void public function remove(Dn $dn): void { - unset($this->entries[$dn->toString()]); + unset($this->entries[$dn->normalize()->toString()]); } public function atomic(callable $operation): void diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/JsonFileStorage.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/JsonFileStorage.php index 09fbd401..d78dc52b 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/JsonFileStorage.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/JsonFileStorage.php @@ -72,12 +72,12 @@ public static function forSwoole( public function find(Dn $dn): ?Entry { - return $this->read()[$dn->toString()] ?? null; + return $this->read()[$dn->normalize()->toString()] ?? null; } public function exists(Dn $dn): bool { - return isset($this->read()[$dn->toString()]); + return isset($this->read()[$dn->normalize()->toString()]); } public function list(StorageListOptions $options): EntryStream @@ -102,7 +102,7 @@ public function remove(Dn $dn): void { $this->withMutation(function (string $contents) use ($dn): string { $data = $this->decodeContents($contents); - unset($data[$dn->toString()]); + unset($data[$dn->normalize()->toString()]); return $this->encodeContents($data); }); diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/PdoStorage.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/PdoStorage.php index d10e0e8a..ab12ee31 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/PdoStorage.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/PdoStorage.php @@ -105,7 +105,7 @@ public function find(Dn $dn): ?Entry { $stmt = $this->prepareAndExecute( $this->dialect->queryFetchEntry(), - [$dn->toString()], + [$dn->normalize()->toString()], ); $row = $stmt->fetch(); @@ -118,7 +118,7 @@ public function exists(Dn $dn): bool { $stmt = $this->prepareAndExecute( $this->dialect->queryExists(), - [$dn->toString()], + [$dn->normalize()->toString()], ); return $stmt->fetch() !== false; @@ -134,7 +134,7 @@ public function list(StorageListOptions $options): EntryStream : null; $query = $this->queryBuilder->build( - $options->baseDn->toString(), + $options->baseDn->normalize()->toString(), $options->subtree, $filterResult, $sqlLimit, @@ -187,7 +187,7 @@ public function remove(Dn $dn): void { $this->prepareAndExecute( $this->dialect->queryDelete(), - [$dn->toString()], + [$dn->normalize()->toString()], ); } @@ -195,7 +195,7 @@ public function hasChildren(Dn $dn): bool { $stmt = $this->prepareAndExecute( $this->dialect->queryHasChildren(), - [$dn->toString()], + [$dn->normalize()->toString()], ); return $stmt->fetch() !== false; diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Support/ArrayEntryStorageTrait.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Support/ArrayEntryStorageTrait.php index ea2d746c..1c2ed274 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Support/ArrayEntryStorageTrait.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Support/ArrayEntryStorageTrait.php @@ -86,7 +86,8 @@ private function namingContextsFromArray(array $entries): array { $roots = []; foreach (array_keys($entries) as $normDn) { - $parent = (new Dn($normDn))->getParent()?->normalize()->toString() ?? ''; + $normDn = (string) $normDn; + $parent = (new Dn($normDn))->getParent()?->toString() ?? ''; if ($parent === '' || !isset($entries[$parent])) { $roots[] = new Dn($normDn); } @@ -114,11 +115,13 @@ private function yieldByScope( throw new TimeLimitExceededException(); } - $entryDn = new Dn($normDn); + $entryDn = Dn::fromCanonical((string) $normDn); - if ($subtree && $entryDn->isDescendantOf($baseDn)) { - yield $entry; - } elseif (!$subtree && $entryDn->isChildOf($baseDn)) { + $inScope = $subtree + ? $entryDn->isDescendantOf($baseDn) + : $entryDn->isChildOf($baseDn); + + if ($inScope) { yield $entry; } } diff --git a/tests/unit/Entry/DnNormalizerTest.php b/tests/unit/Entry/DnNormalizerTest.php new file mode 100644 index 00000000..007d09b6 --- /dev/null +++ b/tests/unit/Entry/DnNormalizerTest.php @@ -0,0 +1,84 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Tests\Unit\FreeDSx\Ldap\Entry; + +use FreeDSx\Ldap\Entry\DnNormalizer; +use PHPUnit\Framework\TestCase; + +final class DnNormalizerTest extends TestCase +{ + public function test_empty_dn_canonicalizes_to_empty_string(): void + { + self::assertSame( + '', + DnNormalizer::canonicalize(''), + ); + } + + public function test_it_folds_attribute_names_and_values_to_lower_case(): void + { + self::assertSame( + 'cn=foo,dc=example,dc=com', + DnNormalizer::canonicalize('CN=Foo,DC=Example,DC=Com'), + ); + } + + public function test_it_collapses_insignificant_whitespace_in_values(): void + { + self::assertSame( + 'cn=john smith,dc=example,dc=com', + DnNormalizer::canonicalize('cn=John Smith,dc=example,dc=com'), + ); + } + + public function test_it_removes_whitespace_around_separators(): void + { + self::assertSame( + 'cn=foo,dc=bar', + DnNormalizer::canonicalize('cn = foo , dc = bar'), + ); + } + + public function test_multivalued_rdn_components_are_order_independent(): void + { + self::assertSame( + DnNormalizer::canonicalize('cn=a+uid=b,dc=com'), + DnNormalizer::canonicalize('uid=b+cn=a,dc=com'), + ); + } + + public function test_rdn_order_is_significant(): void + { + self::assertNotSame( + DnNormalizer::canonicalize('cn=a,dc=b'), + DnNormalizer::canonicalize('dc=b,cn=a'), + ); + } + + public function test_escaped_comma_in_value_is_preserved(): void + { + self::assertSame( + 'cn=doe\,john,dc=example,dc=com', + DnNormalizer::canonicalize('cn=Doe\,John,dc=Example,dc=Com'), + ); + } + + public function test_malformed_dn_falls_back_to_lowercase_without_throwing(): void + { + self::assertSame( + 'not-a-valid-dn', + DnNormalizer::canonicalize('Not-A-Valid-DN'), + ); + } +} diff --git a/tests/unit/Schema/Matching/DistinguishedNameComparatorTest.php b/tests/unit/Schema/Matching/DistinguishedNameComparatorTest.php index c2767c73..631675aa 100644 --- a/tests/unit/Schema/Matching/DistinguishedNameComparatorTest.php +++ b/tests/unit/Schema/Matching/DistinguishedNameComparatorTest.php @@ -46,6 +46,26 @@ public function test_equals_case_insensitive(): void self::assertTrue($result); } + public function test_equals_ignores_insignificant_whitespace(): void + { + $result = $this->subject->equals( + 'cn=John Smith , dc=example , dc=com', + 'cn=john smith,dc=example,dc=com', + ); + + self::assertTrue($result); + } + + public function test_equals_ignores_multivalued_rdn_order(): void + { + $result = $this->subject->equals( + 'cn=a+uid=b,dc=com', + 'uid=b+cn=a,dc=com', + ); + + self::assertTrue($result); + } + public function test_equals_different_dn(): void { $result = $this->subject->equals( diff --git a/tests/unit/Server/Backend/Storage/Adapter/InMemoryStorageTest.php b/tests/unit/Server/Backend/Storage/Adapter/InMemoryStorageTest.php index 223f4352..6ad3f0ac 100644 --- a/tests/unit/Server/Backend/Storage/Adapter/InMemoryStorageTest.php +++ b/tests/unit/Server/Backend/Storage/Adapter/InMemoryStorageTest.php @@ -51,6 +51,17 @@ public function test_find_returns_null_for_unknown_norm_dn(): void self::assertNull($this->subject->find(new Dn('cn=nobody,dc=example,dc=com'))); } + public function test_find_matches_whitespace_and_case_variant_dn(): void + { + $entry = $this->subject->find(new Dn('CN = Alice , dc=example , dc=com')); + + self::assertNotNull($entry); + self::assertSame( + 'cn=Alice,dc=example,dc=com', + $entry->getDn()->toString(), + ); + } + public function test_list_returns_all_entries(): void { $entries = iterator_to_array($this->subject->list(StorageListOptions::matchAll(new Dn(''), true))->entries);