Skip to content

[stable23] fix potential unwarranted memberships in nested groups from LDAP #30343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
artonge merged 1 commit into from
Dec 30, 2021
Merged

[stable23] fix potential unwarranted memberships in nested groups from LDAP #30343

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fix potential unwarranted memberships in nested groups from LDAP 
- the issue was present only when using PHP based resolving of nested
  group members. Normally nested members are common in AD (and Samba4) and
  are resolved per LDAP_MATCHING_RULE_IN_CHAIN by default
- resolving nested members is recursive
- when the cache entry was created it happend for intermediate groups, too,
  containing members from the parent group
- the check was added to only cache the root group with its members
- a runtime cache stores intermediate ldap read results


Signed-off-by: Arthur Schiwon <[email protected]>
  • Loading branch information
@blizzz @backportbot
blizzz authored and backportbot[bot] committed Dec 20, 2021
commit bf81fa432a49736b218b2986e53cec30908ca4d7
16 changes: 14 additions & 2 deletions apps/user_ldap/lib/Group_LDAP.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,12 @@ private function _groupMembers(string $dnGroup, ?array &$seen = null): array {
// but not included in the results laters on
$excludeFromResult = $dnGroup;
}
// cache only base groups, otherwise groups get additional unwarranted members
$shouldCacheResult = count($seen) === 0;

static $rawMemberReads = []; // runtime cache for intermediate ldap read results
$allMembers = [];

if (array_key_exists($dnGroup, $seen)) {
return [];
}
Expand Down Expand Up @@ -290,7 +295,11 @@ private function _groupMembers(string $dnGroup, ?array &$seen = null): array {
}

$seen[$dnGroup] = 1;
$members = $this->access->readAttribute($dnGroup, $this->access->connection->ldapGroupMemberAssocAttr);
$members = $rawMemberReads[$dnGroup] ?? null;
if ($members === null) {
$members = $this->access->readAttribute($dnGroup, $this->access->connection->ldapGroupMemberAssocAttr);
$rawMemberReads[$dnGroup] = $members;
}
if (is_array($members)) {
$fetcher = function ($memberDN) use (&$seen) {
return $this->_groupMembers($memberDN, $seen);
Expand All @@ -306,7 +315,10 @@ private function _groupMembers(string $dnGroup, ?array &$seen = null): array {
}
}

$this->access->connection->writeToCache($cacheKey, $allMembers);
if ($shouldCacheResult) {
$this->access->connection->writeToCache($cacheKey, $allMembers);
unset($rawMemberReads[$dnGroup]);
}
if (isset($attemptedLdapMatchingRuleInChain)
&& $this->access->connection->ldapMatchingRuleInChainState === Configuration::LDAP_SERVER_FEATURE_UNKNOWN
&& !empty($allMembers)
Expand Down