Bug 469243 - ACL: support group filter
Summary: ACL: support group filter
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: 389
Classification: Retired
Component: Security - Access Control (ACL)
Version: 1.1.3
Hardware: All
OS: Linux
high
medium
Target Milestone: ---
Assignee: Deon Ballard
QA Contact: Chandrasekar Kannan
URL:
Whiteboard:
: 445769 (view as bug list)
Depends On:
Blocks: 249650 458165 FDS1.2.0
TreeView+ depends on / blocked
 
Reported: 2008-10-30 19:20 UTC by Noriko Hosoi
Modified: 2015-01-04 23:34 UTC (History)
5 users (show)

Fixed In Version: 8.1
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-04-29 23:07:19 UTC


Attachments (Terms of Use)
cvs diff ldap/servers/plugins/acl/acllas.c (6.44 KB, patch)
2008-10-30 20:49 UTC, Noriko Hosoi
no flags Details | Diff
sample ldif (1.43 MB, text/plain)
2008-10-30 20:59 UTC, Noriko Hosoi
no flags Details
sample ldif 2 (1.43 MB, text/plain)
2008-10-30 21:11 UTC, Noriko Hosoi
no flags Details
Updated cvs diff ldap/servers/plugin/acl/acllas.c (6.49 KB, patch)
2008-10-30 23:17 UTC, Noriko Hosoi
no flags Details | Diff
Yet another updated cvs diff ldap/servers/plugins/acl/acllas.c (6.62 KB, patch)
2008-10-31 00:03 UTC, Noriko Hosoi
no flags Details | Diff
cvs commit message (453 bytes, text/plain)
2008-10-31 00:16 UTC, Noriko Hosoi
no flags Details
cvs diff ldap/servers/plugins/acl/acllas.c (14.93 KB, patch)
2008-10-31 22:21 UTC, Noriko Hosoi
no flags Details | Diff
sample ldif (1.43 MB, text/plain)
2008-10-31 22:32 UTC, Noriko Hosoi
no flags Details
cvs commit message (927 bytes, text/plain)
2008-11-01 22:10 UTC, Noriko Hosoi
no flags Details
update request in the 8.1 administrator's guide (2.12 KB, text/html)
2008-11-01 22:16 UTC, Noriko Hosoi
no flags Details

Description Noriko Hosoi 2008-10-30 19:20:18 UTC
Description of problem:
To apply an ACI to the bind user, a filter is available for a user, but not for a group the bind user belong to.
    6.4.5.1.4. Example with LDAPURL Bind Type
    The following associates the userattr keyword with a bind based on an 
    LDAP filter:
    userattr = "myfilter#LDAPURL
    The bind rule is evaluated to be true if the bind DN matches the filter 
    specified in the myfilter attribute of the targeted entry. The myfilter 
    attribute can be replaced by any attribute that contains an LDAP filter.

We have a request to extend the functionality to the group.

The keyword userattr takes "#GROUPDN" in the value to specify the value in front of "#GROUPDN" is for the group.  It already accepts ldapurl format although it's not a complete ldapurl including a filter.  I'm proposing to change it to accept complete ldapurl to specify the groups to have the access right. 

    6.4.5.1.2. Example with GROUPDN Bind Type
    The following associates the userattr keyword with a bind based on a 
    group DN:
    userattr = "owner#GROUPDN"
    The bind rule is evaluated to be true if the bind DN is a member of the 
    group specified in the owner attribute of the targeted entry. For example, 
    you can use this mechanism to allow a group to manage employees' status 
    information. You can use an attribute other than owner as long as the 
    attribute you use contains the DN of a group entry.

    The group you point to can be a dynamic group, and the DN of the group can 
    be under any suffix in the database. However, the evaluation of this type 
    of ACI by the server is very resource intensive.

    If you are using static groups that are under the same suffix as the 
    targeted entry, you can use the following expression:

    userattr = "ldap:///dc=example,dc=com?owner#GROUPDN"

    In this example, the group entry is under the dc=example,dc=com suffix. The 
    server can process this type of syntax more quickly than the previous 
    example.

Comment 1 Noriko Hosoi 2008-10-30 20:49:52 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.

Comment 2 Noriko Hosoi 2008-10-30 20:59:14 UTC
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".

Comment 3 Noriko Hosoi 2008-10-30 21:11:30 UTC
Created attachment 321989 [details]
sample ldif 2

Test case 2 (nested group)
Repeat the same steps in Test case 1.

Comment 4 Nathan Kinder 2008-10-30 22:22:57 UTC
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.

Comment 5 Noriko Hosoi 2008-10-30 22:29:08 UTC
(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!

Comment 6 Noriko Hosoi 2008-10-30 23:17:42 UTC
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.

Comment 7 Noriko Hosoi 2008-10-31 00:03:41 UTC
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";)

Comment 8 Noriko Hosoi 2008-10-31 00:16:53 UTC
Created attachment 322005 [details]
cvs commit message

Reviewed by Nathan (Thank you!!)

Checked in into CVS HEAD.

Comment 9 Noriko Hosoi 2008-10-31 22:13:45 UTC
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.

Comment 10 Noriko Hosoi 2008-10-31 22:21:29 UTC
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.

Comment 11 Noriko Hosoi 2008-10-31 22:32:13 UTC
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.

Comment 12 Noriko Hosoi 2008-11-01 22:10:45 UTC
Created attachment 322187 [details]
cvs commit message

Reviewed by Nathan (Thank you!!)

Checked in into CVS HEAD.

Comment 13 Noriko Hosoi 2008-11-01 22:16:45 UTC
Created attachment 322188 [details]
update request in the 8.1 administrator's guide

Comment 14 Rich Megginson 2008-11-05 18:16:09 UTC
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 ();

Comment 15 Noriko Hosoi 2008-12-11 22:18:25 UTC
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".

Comment 16 Noriko Hosoi 2008-12-11 22:38:07 UTC
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]"

Comment 17 Nathan Kinder 2008-12-13 17:07:50 UTC
*** Bug 445769 has been marked as a duplicate of this bug. ***

Comment 19 Jenny Severance 2009-03-16 15:24:01 UTC
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.

Comment 20 Chandrasekar Kannan 2009-04-29 23:07:19 UTC
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


Note You need to log in before you can comment on or make changes to this bug.