Skip to content
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.

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

Please support nested groups #68

Open
akorn opened this issue Jan 3, 2016 · 11 comments
Open

Please support nested groups #68

akorn opened this issue Jan 3, 2016 · 11 comments

Comments

@akorn
Copy link

@akorn akorn commented Jan 3, 2016

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 comment has been minimized.

Copy link
Contributor

@jaqx0r jaqx0r commented Jan 6, 2016

This seems reasonable.

Do you want to work on this?

@akorn

This comment has been minimized.

Copy link
Author

@akorn akorn commented Jan 6, 2016

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

This comment has been minimized.

Copy link
Contributor

@jaqx0r jaqx0r commented Jan 6, 2016

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
#68 (comment).

@kev009

This comment has been minimized.

Copy link
Contributor

@kev009 kev009 commented Oct 7, 2016

@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

This comment has been minimized.

Copy link
Contributor

@jaqx0r jaqx0r commented Oct 7, 2016

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
#68 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AC5b-85GXnF8-nSW48xzNt4HEgJrs18aks5qxc1EgaJpZM4G9yd1
.

@kev009

This comment has been minimized.

Copy link
Contributor

@kev009 kev009 commented Oct 7, 2016

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

This comment has been minimized.

Copy link

@JaseFace JaseFace commented May 15, 2018

Did anyone have some time to start this up yet?

@trenton42

This comment has been minimized.

Copy link

@trenton42 trenton42 commented Oct 26, 2018

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 comment has been minimized.

Copy link

@trenton42 trenton42 commented Oct 26, 2018

This first stab will cache nested groups, but only on a full sync: 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

This comment has been minimized.

Copy link
Contributor

@jaqx0r jaqx0r commented Oct 27, 2018

@trenton42 trenton42 mentioned this issue Oct 29, 2018
@trenton42

This comment has been minimized.

Copy link

@trenton42 trenton42 commented Oct 29, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.