Please support nested groups

[akorn]

Currently the code seems to assume that the value of a member attribute is the DN of a user, and treats the RDN as the uid (even if it happens to be called cn and ldap_uidattr is unset, and even if the LDAP member entity doesn't have the posixAccount objectClass, which I'd say is a bug).

Under rfc2307bis, group members can also be groups; libnss-ldap also supports this.

I think nsscache should perform a transitive closure on the groups it obtains from LDAP. I wrote a shell script that does this, as a proof of concept; it's a bit crude, but it works. It allows arbitrary group nesting (even across groups that don't have the posixGroup objectClass). It doesn't prevent infinite recursion though, and it's very slow.

Ironically, getent group gets it right if nss is configured to use libnss-ldap, so that instead of running nsscache, we could just use getent -- if that wouldn't defeat the purpose of using nss-cache. :)

#!/bin/zsh
#
# Purpose: get groups from LDAP recursively and extract their members.

typeset -A isgroup
typeset -A isposixgroup
typeset -A members # Contains DNs
typeset -A memberuids
typeset -A processed_DNs
typeset -A uids
typeset -A groups
typeset -A memberfilter

GROUPBASE="$1"
GROUPFILTER="$2"        # only print groups matching this filter; use objectClass=posixGroup
MEMBERFILTER="$3"       # only print members matching this filter; use objectClass=posixAccount

function process_ldapsearch() {
        local currentdn=""
        local memberdn=""
        local memberuid=""
        while read attrib val; do case "$attrib $val" in
                dn:*)
                        currentdn="$val"
                        ;;
                objectClass:\ groupOfNames)
                        isgroup[$currentdn]=1
                        ;;
                objectClass:\ posixGroup)
                        isposixgroup[$currentdn]=1
                        ;;
                member:\ *)
                        memberdn="$val"
                        # Do we already have this member in the current group? If not, add it.
                        [[ "$members[$currentdn]" =~ \b${memberdn}\b ]] || members[$currentdn]="$members[$currentdn] $memberdn"
                        [[ "${memberdn/,$GROUPBASE/}" = "${memberdn}" ]] && # Is this member located directly under our specified GROUPBASE dn?
                                [[ "${memberdn/uid=/}" = "${memberdn}" ]] && { # Process members that are outside our search base and that are not obviously users
                                [[ $processed_DNs[$memberdn] = "" ]] && ldapsearch -LLL -x -o ldif-wrap=no -b "${memberdn}" | process_ldapsearch
                                processed_DNs[$memberdn]=1
                        }
                        ;;
                memberUid:*)
                        memberuid="$val"
                        # Do we already have this member in the current group? If not, add it.
                        [[ "$memberuids[$currentdn]" =~ \b$memberuid\b ]] || memberuids[$currentdn]="$memberuids[$currentdn] $memberuid"
                        ;;
                *)
                        ;;
        esac; done
}

function memberfilter_match() {
        [[ -z "$MEMBERFILTER" ]] && return 0
        [[ -n "$memberfilter[$1]" ]] && return $memberfilter[$1]
        if [[ $(ldapsearch -s base -LLL -x -o ldif-wrap=no -b "$i" "($MEMBERFILTER)" dn) = "dn: $i" ]]; then
                memberfilter[$1]="0"
        else
                memberfilter[$1]="1"
        fi
        return $memberfilter[$1]
}

function getmembers() {
        local group=$1
        local -A curmembers
        local i
        local j
        for i in ${(z)members[$group]}; do
                if (( ${+isgroup[$i]} || ${+isposixgroup[$i]} )); then
                        for j in $(getmembers $i); do
                                curmembers[$j]=1
                        done
                else
                        if memberfilter_match $i; then
                                i=${i/uid=/}
                                i=${i/,*/}
                                curmembers[$i]=1
                        fi
                fi
        done
        for i in ${(z)memberuids[$group]}; do
                curmembers[$i]=1
        done
        echo ${(k)curmembers}
        return 0;
}

# main()
ldapsearch -LLL -x -b "$GROUPBASE" -o ldif-wrap=no | process_ldapsearch

# Merge the list of posixGroups and groupOfNames groups; the keys of the groups hash will form a list of all groups
for group in ${(k)isposixgroup} ${(k)isgroup}; do
        groups[$group]=1
done

for group in ${(k)groups}; do
        if [[ -z "$GROUPFILTER" ]] || [[ $(ldapsearch -s base -LLL -x -b "$group" -o ldif-wrap=no "($GROUPFILTER)" dn) = "dn: $group" ]] then
                groupname=${group/cn=/}
                if ! [[ "${groupname/,$GROUPBASE/}" = "${groupname}" ]]; then   # Was this particular group inside our search base or outside it?
                        groupname=${groupname/,*/}
                        echo $groupname $(getmembers $group | tr ' ' '\n' | sort | tr '\n' ' ')
                fi
        fi
done

[jaqx0r]

This seems reasonable.

Do you want to work on this?

[akorn]

No. Unfortunately I have neither the required python skills, nor (more importantly) time. :(

I can work around the problem by not using nss-cache at all; instead, I'll periodically run getent passwd and getent group on a box with nss set to use LDAP, and copy the results to the LDAP servers (well, actually, I'll use svn or git to distribute the flat files).

This works for me because my only problem is that there are race conditions when the LDAP servers all try to start simultaneously and look up uid/gid mappings in LDAP; if the LDAP servers themselves use flat files, that's good enough for me. I don't need shadow, netgroup, automount or anything else, just uid-name, gid-name and group-member mappings, and this getent based kludge will work for me. (I'm writing this down because it may work for others too.)

[jaqx0r]

Understood. I too am short on time so this will have to wait for now.

On Wed, 6 Jan 2016, 18:26 Dr. András Korn notifications@github.com wrote:

No. Unfortunately I have neither the required python skills, nor (more importantly) time. :(

I can work around the problem by not using nss-cache at all; instead, I'll periodically run getent passwd and getent group on a box with nss set to use LDAP, and copy the results to the LDAP servers (well, actually, I'll use svn or git to distribute the flat files).

This works for me because my only problem is that there are race conditions when the LDAP servers all try to start simultaneously and look up uid/gid mappings in LDAP; if the LDAP servers themselves use flat files, that's good enough for me. I don't need shadow, netgroup, automount or anything else, just uid-name, gid-name and group-member mappings, and this getent based kludge will work for me. (I'm writing this down because it may work for others too.)

— Reply to this email directly or view it on GitHub https://github.com/google/nsscache/issues/68#issuecomment-169257506.

[kev009]

@jaqx0r do you have a preference on how this should be implemented? I can do it quite easily in python, but maybe there are some reasons it should be in libnss-cache?

[jaqx0r]

Without having thought about it too hard, my feeling is that libnss-cache is the wrong place because that's in the critical read path, so it should be done in nsscache (i.e. the python).

This might prove to be bad for really large nestings of groups but we can burn that bridge when we come to it.

On Fri., 7 Oct. 2016, 15:40 Kevin Bowling, notifications@github.com wrote:

@jaqx0r https://github.com/jaqx0r do you have a preference on how this should be implemented? I can do it quite easily in python, but maybe there are some reasons it should be in libnss-cache?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/nsscache/issues/68#issuecomment-252151517, or mute the thread https://github.com/notifications/unsubscribe-auth/AC5b-85GXnF8-nSW48xzNt4HEgJrs18aks5qxc1EgaJpZM4G9yd1 .

[kev009]

I was thinking it has some considerations like, a recursive group member that is not in ldap should be preserved because one of the other nss providers may have it.. but after writing it out I don't see why doing that in the python side wont work.

[JaseFace]

Did anyone have some time to start this up yet?

[trenton42]

Looking through the python, it appears that there would be an issue mapping the recursive groups on incremental updates. For --full runs it is fine, but when running without --full, only modified groups will show up in the search result. In order to properly update all nested groups, several queries would have to be made (query for any groups that contain the changed group, then any groups that contain those groups, etc.), so depending on the ldap layout, it could greatly diminish the efficiency of doing an incremental update. It may also be possible to cache those group to group relationships, but that would also require more code changes.

[trenton42]

This first stab will cache nested groups, but only on a full sync: https://github.com/google/nsscache/compare/master...trenton42:nested-groups

Using this code with incremental updates will be subtly broken, as changes in memberships in child groups will not propagate up to parent groups

[jaqx0r]

Thanks for working on this! If we can hide this feature behind a flag then I'm happy to merge it so people can try it out.

On Fri, 26 Oct 2018 at 11:20, Trenton Broughton notifications@github.com wrote:

This first stab will cache nested groups, but only on a full sync: master...trenton42:nested-groups https://github.com/google/nsscache/compare/master...trenton42:nested-groups

Using this code with incremental updates will be subtly broken, as changes in memberships in child groups will not propagate up to parent groups

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/nsscache/issues/68#issuecomment-433499493, or mute the thread https://github.com/notifications/unsubscribe-auth/AC5b-9UqqSWE-5ASLRR_MFRbq0gnasiUks5uo1J3gaJpZM4G9yd1 .

[trenton42]

Thanks @jaqx0r! I added those changes in #84. Let me know if that looks good.