Bug 469243
Summary: | ACL: support group filter | ||
---|---|---|---|
Product: | [Retired] 389 | Reporter: | Noriko Hosoi <nhosoi> |
Component: | Security - Access Control (ACL) | Assignee: | Deon Ballard <dlackey> |
Status: | CLOSED CURRENTRELEASE | QA Contact: | Chandrasekar Kannan <ckannan> |
Severity: | medium | Docs Contact: | |
Priority: | high | ||
Version: | 1.1.3 | CC: | benl, ckannan, jgalipea, nkinder, rmeggins |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | 8.1 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2009-04-29 23:07:19 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, 458165, 493682 | ||
Attachments: |
Description
Noriko Hosoi
2008-10-30 19:20:18 UTC
Created attachment 321987 [details]
cvs diff ldap/servers/plugins/acl/acllas.c
Description:
. if userattr in aci has 3 '?' marks and no attribute (value between the first and the second '?') with #GROUPDN as in the following example, the userattr value before #GROUPDN is treated as LDAPURL
aci: (target=ldap:///dc=example,dc=com)(targetattr=*)(version 3.0; acl "acl1"; allow(all) userattr = "ldap:///ou=Groups, dc=example,dc=com??sub?(cn=*Administrators)#GROUPDN";)
. internally, the search with the filter from the base dn runs
. if the bind dn belong to a group and the group matches the above search results, the bind user has the access rights defined in the aci.
Created attachment 321988 [details]
sample ldif
Test case 1
. Import the attached sample ldif file
. Bind as any of these users who belong to groups which are returned from the search with the filter (cn=*Administrators) (note: userpassword == uid) and modify any entry under "dc=example,dc=com", which should be successfully done.
dn: uid=ONewcomb16, ou=Human Resources, dc=example,dc=com
dn: uid=JDann8, ou=Product Development, dc=example,dc=com
dn: uid=KDias0, ou=Product Testing, dc=example,dc=com
dn: uid=JCrapco0, ou=Product Development, dc=example,dc=com
dn: uid=LJoudrey13, ou=Product Development, dc=example,dc=comq
dn: uid=ITogasaki4, ou=Product Testing, dc=example,dc=com
. Bind as any other users and modify an entry under "dc=example,dc=com" other than "self". The modify operation should fail with an error "Insufficient 'write' privilege".
Created attachment 321989 [details]
sample ldif 2
Test case 2 (nested group)
Repeat the same steps in Test case 1.
Where do the internal search results get free'd in your patch from comment#1? The approach in the patch looks good, I just want to make sure we don't have a leak. (In reply to comment #4) > Where do the internal search results get free'd in your patch from comment#1? > The approach in the patch looks good, I just want to make sure we don't have a > leak. Oops, I should have missed it. Let me run valgrind to find more... Thanks, Nathan! Created attachment 321995 [details]
Updated cvs diff ldap/servers/plugin/acl/acllas.c
Thanks to Nathan for finding out the leak.
I added slapi_free_search_results_internal to release the search results:
+ slapi_free_search_results_internal(myPb);
I ran the test cases with valgrind and verified there is no leaks.
Created attachment 322000 [details]
Yet another updated cvs diff ldap/servers/plugins/acl/acllas.c
Another good catch by Nathan (Thanks!!)
Even if the internal search fails, we should call slapi_free_search_results_internal. Otherwise, if internal_ops stores referrals, it leaks.
I ran valgrind with this test aci, which returns none.
aci: (target=ldap:///dc=example,dc=com)(targetattr=*)(version 3.0; acl "acl4"; allow(all) userattr = "ldap:///ou=Groups, dc=example,dc=com??sub?(cn=Bogus*)#GROUPDN";)
Created attachment 322005 [details]
cvs commit message
Reviewed by Nathan (Thank you!!)
Checked in into CVS HEAD.
While I was writing a doc, I learned my fix is against the usage of userattr. The userattr specifies an attribute, which is supposed to be in the target entry. If the attribute value matches the bind dn (#USERDN) or the group that the bind dn belongs to (#GROUPDN) or the search result from the ldapurl (#LDAPURL), the bind dn is treated to have the access right. To achieve the request to support the group filter, we should rather extend groupdn to support the ldapurl as userdn already does. Created attachment 322141 [details]
cvs diff ldap/servers/plugins/acl/acllas.c
Fix description:
. check the value of groupdn is the full ldapurl or not by ldap_url_parse.
. if yes, run the search and get the search results.
otherwise, evaluate the bind dn for the value as usual.
. evaluate the bind dn against each group returned from the search.
. additionally, added the code to trim the beginning and trailig spaces from
the groupdn value, which is needed for ldap_url_parse.
Created attachment 322143 [details]
sample ldif
Test case
. import sample ldif
. these users should be able to modify any entries in dc=example,dc=com since they belong to the group returned by the search with this filter (cn=Dir*Administrators):
dn: uid=ONewcomb16, ou=Human Resources, dc=example,dc=com
dn: uid=JDann8, ou=Product Development, dc=example,dc=com
dn: uid=KDias0, ou=Product Testing, dc=example,dc=com
. these users should be able to modify any entries in dc=example,dc=com since they belong to the group "LDAP Administrators"
dn: uid=JCrapco0, ou=Product Development, dc=example,dc=com
dn: uid=LJoudrey13, ou=Product Development, dc=example,dc=com
dn: uid=ITogasaki4, ou=Product Testing, dc=example,dc=com
. other users should not be able to modify entries except him/herself.
* start the server with valgrind, run the above modify operations, shutdown the server and check the valgrind output if it contains DS_LASGroupDnEval or not. If the function is found, leak is newly introduced.
Created attachment 322187 [details]
cvs commit message
Reviewed by Nathan (Thank you!!)
Checked in into CVS HEAD.
Created attachment 322188 [details]
update request in the 8.1 administrator's guide
Commit one line fix Fix Description: unset value for lud_scope is -1, not NULL /cvs/dirsec/ldapserver/ldap/servers/plugins/acl/acllas.c,v retrieving revision 1.13 retrieving revision 1.14 diff -u -r1.13 -r1.14 --- acllas.c 1 Nov 2008 22:09:16 -0000 1.13 +++ acllas.c 5 Nov 2008 18:14:37 -0000 1.14 @@ -861,7 +861,7 @@ /* Groupdn is full ldapurl? */ if (0 == ldap_url_parse(groupNameOrig, &ludp) && NULL != ludp->lud_dn && - NULL != ludp->lud_scope && + -1 != ludp->lud_scope && NULL != ludp->lud_filter) { /* Yes, it is full ldapurl; Let's run the search */ myPb = slapi_pblock_new (); Regarding the complex groupdn syntax, the value of groupdn takes single LDAPURL. Each groupdn is connected by "or" or "and". '(' and ')' can be used to evaluate the expression inside of the parentheses. Here's the example: (groupdn = "ldap:///ou=Groups, dc=example,dc=com??sub?(cn=*s_0)" or groupdn = "ldap:///ou=Groups,dc=example,dc=com??sub?(cn=*s_1)") and groupdn = "ldap:///ou=Groups, dc=example,dc=com??sub?(cn=*s_2)" groupdn exceptionally supports the format groupdn = "LDAPURI0 || LDAPURL1 || LDAPURL2 || ...", but it does not do groupdn = "LDAPURI0 && LDAPURL1 && LDAPURL2 && ..." Also, parentheses cannot be used inside of the double quotes for the grouping purpose. For "and", we should use the format groupdn = "LDAPURL0" and groupdn = "LDAPURL1". The same is true for the keyword "userdn". Hi Deon, In addition to the comment #13, could you add a "NOTE" for these userdn, groupdn, and roledn keyword to the admin guide that if 2 or more expressions are to be connected by "AND", we cannot do xxxdn = "LDAPURI0 && LDAPURL1 && LDAPURL2 && ...", but we should do xxxdn = "LDAPURL0" and xxxdn = "LDAPURL1" and xxxdn = "LDAPURL2" and ... (where xxx could be user or group or role). Thanks! --noriko http://www.redhat.com/docs/manuals/dir-server/ag/8.0/Managing_Access_Control-Bind_Rules.html 6.4.2. Defining User Access - userdn Keyword User access is defined using the userdn keyword. The userdn keyword requires one or more valid distinguished names in the following format: userdn = "ldap:///dn [|| ldap:///dn]...[||ldap:///dn]" 6.4.3. Defining Group Access - groupdn Keyword [...] The groupdn keyword requires one or more valid distinguished names in the following format: groupdn="ldap:///dn [|| ldap:///dn]...[|| ldap:///dn]" 6.4.4. Defining Role Access - roledn Keyword [...] The roledn keyword requires one or more valid distinguished names in the following format : roledn = "ldap:///dn [|| ldap:///dn]... [|| ldap:///dn]" *** Bug 445769 has been marked as a duplicate of this bug. *** new group ACL functionality exists and is being tested with automated acceptance tests - Sections 6.4.1. Bind Rule Syntax and 6.4.3. Defining Group Access - groupdn Keyword in admin guide verified. 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/RHEA-2009-0455.html |