]> source.dussan.org Git - nextcloud-server.git/commitdiff
fix potential unwarranted memberships in nested groups from LDAP 30341/head
authorArthur Schiwon <blizzz@arthur-schiwon.de>
Tue, 19 Oct 2021 20:00:13 +0000 (22:00 +0200)
committerbackportbot[bot] <backportbot[bot]@users.noreply.github.com>
Mon, 20 Dec 2021 09:13:22 +0000 (09:13 +0000)
- 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 <blizzz@arthur-schiwon.de>
apps/user_ldap/lib/Group_LDAP.php

index a1ceeffba58324727dc9267e442c06b715ac010a..8bf13ed90fd63e9e0ad77f909151e1f52737cdce 100644 (file)
@@ -249,7 +249,12 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
                        // 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 [];
                }
@@ -291,7 +296,11 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
                }
 
                $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);
@@ -307,7 +316,10 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I
                        }
                }
 
-               $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)