Bug 439628
Summary: | memberOf: does not verify all the indirect groups before deleting a memberOf value | ||||||
---|---|---|---|---|---|---|---|
Product: | [Retired] 389 | Reporter: | Andrey Ivanov <andrey.ivanov> | ||||
Component: | Server - memberOf Plug-in | Assignee: | Nathan Kinder <nkinder> | ||||
Status: | CLOSED ERRATA | QA Contact: | Chandrasekar Kannan <ckannan> | ||||
Severity: | high | Docs Contact: | |||||
Priority: | low | ||||||
Version: | 1.1.0 | CC: | benl, bforte, mgregg, rmeggins | ||||
Target Milestone: | --- | ||||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2008-08-04 18:21:11 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Bug Depends On: | |||||||
Bug Blocks: | 249650 | ||||||
Attachments: |
|
Description
Andrey Ivanov
2008-03-29 20:17:11 UTC
The verification should be sort of the one made in memberof_test_membership_callback(). By the way the comments to this function in the code incorrectly name it "memberof_memberof_search_callback()" : Line 1316 : /* memberof_memberof_search_callback() * for each attribute in the memberof attribute * determine if the entry is still a member * * test each for direct membership * move groups entry is memberof to member group * test remaining groups for membership in member groups * iterate until a pass fails to move a group over to member groups * remaining groups should be deleted */ Created attachment 302951 [details]
CVS Diffs
These diffs fix the reported issue. The fix is to check if an entry is an
indirect group member before removing the memberOf attribute value form the
entry.
I had to add a few new helper functions to perform this check. The new utility
functions allow you to check if a specific entry is a member of a specific
group by looking at member values only. There is a function for checking if
the entry is a direct member, and one that will check for direct or indirect
membership. Both of these functions will not modify the entry, which is what
was lacking in the previous code. These utility functions required a recursive
function underneath to trace through indirect memberships. This recursive
function will detect group loopings to ensure it doesn't endlessly recurse.
I also did some refactoring and commenting of existing code to make things more
clear. These changes have been checked for memory leaks with valgrind, and
I've also checked for regressions of other recently fixed bugs in the memberOf
plug-in.
Your fix looks good to me. Checked into ldapserver (HEAD). Thanks to Noriko for her review! Checking in memberof.c; /cvs/dirsec/ldapserver/ldap/servers/plugins/memberof/memberof.c,v <-- memberof.c new revision: 1.7; previous revision: 1.6 done *** Bug 454688 has been marked as a duplicate of this bug. *** Bug verified, test on rhel5.2 64 bit Test 1: follow the same test as reported, and test result is pass Test 2: follow the same setup, but go further. Total 10 nested groups being created, user001 is direct member of each nested group, then delete the direct relation one by one, verify if user001 still memberof each nested group. The test result: pass Test output: server64[07/22/08 18:24]~ >./relation.sh u001 successfully added === create all === grp001 successfully added grp002 successfully added grp003 successfully added grp004 successfully added grp005 successfully added grp006 successfully added grp007 successfully added grp008 successfully added grp009 successfully added === append all === u001 successfully added to grp002 grp002 successfully added to grp001 u001 successfully added to grp003 grp003 successfully added to grp002 u001 successfully added to grp004 grp004 successfully added to grp003 u001 successfully added to grp005 grp005 successfully added to grp004 u001 successfully added to grp006 grp006 successfully added to grp005 u001 successfully added to grp007 grp007 successfully added to grp006 u001 successfully added to grp008 grp008 successfully added to grp007 u001 successfully added to grp009 grp009 successfully added to grp008 === remove all === u001 successfully removed from grp002 dn: cn=grp002,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1155 Full Name: grp002 Description: layer 2 Members: user 001: uid=u001,cn=users,cn=accounts,dc=ipaqa,dc=com grp003: cn=grp003,cn=groups,cn=accounts,dc=ipaqa,dc=com grp004: cn=grp004,cn=groups,cn=accounts,dc=ipaqa,dc=com grp005: cn=grp005,cn=groups,cn=accounts,dc=ipaqa,dc=com grp006: cn=grp006,cn=groups,cn=accounts,dc=ipaqa,dc=com grp007: cn=grp007,cn=groups,cn=accounts,dc=ipaqa,dc=com grp008: cn=grp008,cn=groups,cn=accounts,dc=ipaqa,dc=com grp009: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com Continue? u001 successfully removed from grp003 dn: cn=grp003,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1156 Full Name: grp003 Description: layer 3 Members: user 001: uid=u001,cn=users,cn=accounts,dc=ipaqa,dc=com grp004: cn=grp004,cn=groups,cn=accounts,dc=ipaqa,dc=com grp005: cn=grp005,cn=groups,cn=accounts,dc=ipaqa,dc=com grp006: cn=grp006,cn=groups,cn=accounts,dc=ipaqa,dc=com grp007: cn=grp007,cn=groups,cn=accounts,dc=ipaqa,dc=com grp008: cn=grp008,cn=groups,cn=accounts,dc=ipaqa,dc=com grp009: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com Continue? u001 successfully removed from grp004 dn: cn=grp004,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1157 Full Name: grp004 Description: layer 4 Members: user 001: uid=u001,cn=users,cn=accounts,dc=ipaqa,dc=com grp005: cn=grp005,cn=groups,cn=accounts,dc=ipaqa,dc=com grp006: cn=grp006,cn=groups,cn=accounts,dc=ipaqa,dc=com grp007: cn=grp007,cn=groups,cn=accounts,dc=ipaqa,dc=com grp008: cn=grp008,cn=groups,cn=accounts,dc=ipaqa,dc=com grp009: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com Continue? u001 successfully removed from grp005 dn: cn=grp005,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1158 Full Name: grp005 Description: layer 5 Members: user 001: uid=u001,cn=users,cn=accounts,dc=ipaqa,dc=com grp006: cn=grp006,cn=groups,cn=accounts,dc=ipaqa,dc=com grp007: cn=grp007,cn=groups,cn=accounts,dc=ipaqa,dc=com grp008: cn=grp008,cn=groups,cn=accounts,dc=ipaqa,dc=com grp009: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com Continue? u001 successfully removed from grp006 dn: cn=grp006,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1159 Full Name: grp006 Description: layer 6 Members: user 001: uid=u001,cn=users,cn=accounts,dc=ipaqa,dc=com grp007: cn=grp007,cn=groups,cn=accounts,dc=ipaqa,dc=com grp008: cn=grp008,cn=groups,cn=accounts,dc=ipaqa,dc=com grp009: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com Continue? u001 successfully removed from grp007 dn: cn=grp007,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1160 Full Name: grp007 Description: layer 7 Members: user 001: uid=u001,cn=users,cn=accounts,dc=ipaqa,dc=com grp008: cn=grp008,cn=groups,cn=accounts,dc=ipaqa,dc=com grp009: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com Continue? u001 successfully removed from grp008 dn: cn=grp008,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1161 Full Name: grp008 Description: layer 8 Members: user 001: uid=u001,cn=users,cn=accounts,dc=ipaqa,dc=com grp009: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com Continue? u001 successfully removed from grp009 dn: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1162 Full Name: grp009 Description: layer 9 Continue? the test script is below: server64[07/22/08 18:27]~ >cat < relation.sh #!/bin/sh createAll(){ for N in 1 2 3 4 5 6 7 8 9 do ipa-addgroup -d "layer $N" grp00$N done } clearAll(){ for N in 1 2 3 4 5 6 7 8 9 do ipa-delgroup grp00$N done } appendAll(){ for N in 2 3 4 5 6 7 8 9 do M=$(expr $N - 1) ipa-modgroup -a u001 grp00$N ipa-modgroup -g grp00$N grp00$M done } removeAll(){ for N in 2 3 4 5 6 7 8 9 do ipa-modgroup -r u001 grp00$N ipa-findgroup grp00$N echo ipa-finduser -a u001 echo Continue? read done } findAll(){ for N in 1 2 3 4 5 6 7 8 9 do ipa-findgroup grp00$N ipa-finduser u001 echo "Continue?" read done } ipa-adduser -c GECOS -d /home/u001 -f user -l 001 -p redhat123 -s /bin/bash u001 echo "=== create all ===" createAll echo "=== append all ===" appendAll #echo "=== find all ===" #findAll echo "=== remove all ===" removeAll echo "===clear all ===" clearAll ipa-deluser u001 Gentlefolk, A quick question from the errata gallery. If I'm understanding the above a-right, when we have u001 is a direct member of grp001 u001 is a direct member of grp002 grp001 is a direct member of grp002 which means, u001 is an indirect member of grp002 if we remove u001 from direct membership of grp2 the old behaviour meant: u001 was not an indirect member of grp002 but the new behaviour means: u001 is still an indirect member of grp002 Correct? Also, is there any visible or otherwise meaningful difference between being an indirect or direct member of a group? The memberOf attribute value appears to be the same either way. BTW, these questions pertain to the following from the forthcoming errata note. If my reading above is correct and there is also no difference between a direct or indirect membership, I'm looking to go with: * if an entry was both directly and indirectly a member of a group and the direct membership was removed, the indirect membership was, erroneously, also removed. When deleting a direct membership, this updated package now checks to see if an entry is also an indirect group member. If an entry is an indirect member of a group, the memberOf attribute is not altered. OTOH, if my reading above is correct and there *is* a difference between a direct and indirect membership, I'm looking to go with: * if an entry was both directly and indirectly a member of a group and the direct membership was removed, the indirect membership was, erroneously, also removed. When deleting a direct membership, this updated package now checks to see if an entry is also an indirect group member. If an entry is an indirect member of a group, the direct membership is still deleted but the indirect membership remains intact. All responses, suggestions and recipes for really great pavlova gratefully accepted. Regards, Brian Forte. (In reply to comment #10) > Gentlefolk, > > A quick question from the errata gallery. > > If I'm understanding the above a-right, when we have > > u001 is a direct member of grp001 > u001 is a direct member of grp002 > grp001 is a direct member of grp002 > > which means, > u001 is an indirect member of grp002 > > if we remove u001 from direct membership of grp2 > > the old behaviour meant: > u001 was not an indirect member of grp002 > > but the new behaviour means: > u001 is still an indirect member of grp002 > > Correct? Yes. The user was always supposed to be an indirect member, but the memberOf attribute in u001 for grp002 was missing before the fix. > > Also, is there any visible or otherwise meaningful difference between being an indirect or direct member of a group? > The memberOf attribute value appears to be the same either way. That is correct. As far as a user entry is concerned, there is no distinction between a direct group and an indirect group. All groups are equal, and the memberOf attribute will appear the same. > > BTW, these questions pertain to the following from the forthcoming errata note. > > If my reading above is correct and there is also no difference between a direct or indirect membership, I'm looking > to go with: > > * if an entry was both directly and indirectly a member of a group and the direct membership was removed, the > indirect membership was, erroneously, also removed. When deleting a direct membership, this updated package > now checks to see if an entry is also an indirect group member. If an entry is an indirect member of a group, the > memberOf attribute is not altered. This description is accurate. > > OTOH, if my reading above is correct and there *is* a difference between a direct and indirect membership, I'm > looking to go with: > > * if an entry was both directly and indirectly a member of a group and the direct membership was removed, the > indirect membership was, erroneously, also removed. When deleting a direct membership, this updated package > now checks to see if an entry is also an indirect group member. If an entry is an indirect member of a group, the > direct membership is still deleted but the indirect membership remains intact. > > All responses, suggestions and recipes for really great pavlova gratefully accepted. > > Regards, > > Brian Forte. An advisory has been issued which should help the problem described in this bug report. This report is therefore being closed with a resolution of ERRATA. For more information on therefore solution and/or where to find the updated files, please follow the link below. You may reopen this bug report if the solution does not work for you. http://rhn.redhat.com/errata/RHBA-2008-0643.html |