[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openldap-devel
Subject:    Possible bug in acl.c
From:       Kean Johnston <kean.johnston () gmail ! com>
Date:       2010-04-13 12:46:34
Message-ID: 4BC467AA.1070402 () gmail ! com
[Download RAW message or body]

Assumption: the following ACL should result in $0 being expanded for the set:

access to dn.one="ou=hosts,dc=example,dc=com" attrs=authorizedService
         by set.expand="[cn=access,$0]/member* & user" compare
         by * =rsdx break

Reason for assumption: man slapd.access states:
    Forms of the <what> clause other than regex may provide  submatches  as
    well.   The  base(object), the sub(tree), the one(level), and the chil-
    dren forms  provide  $0  as  the  match  of  the  entire  string.   The
    sub(tree),  the  one(level),  and the children forms also provide $1 as
    the match of the rightmost part of the DN  as  defined  in  the  <what>
    clause.

Bug: does not work as expected. The reason is that in slap.h slap_style_t 
starts with ACL_STYLE_REGEX = 0, so any structure that uses slap_style_t 
and uses memset to null out the structure will have its default style be 
ACL_STYLE_REGEX. In acl.c there are 4 places where you test for 
ACL_STYLE_REGEX on a->acl_attrval_style without checking if an actual 
attribute value was supplied. The patch below fixes those cases. The better 
(arguably) fix would be to change slap_style_t to start with ACL_STYLE_NONE 
= 0, and then explicitly set the style when it is encountered in 
aclparse.c. However, I did not want to change slap.h in case it changes 
some ABI and the change to aclparse.c is larger.

As things currently stand, dn.expand, set.expand and group.expand will not 
expand $0 and $1 as documented if you use dn.{base,one,sub,children} in the 
what clause.

If my assumptions are correct and this should work, I will file a proper 
bug in ITS.

Kean

["aclbug.diff" (text/plain)]

--- acl.c.jkj	2010-04-13 07:06:12.000000000 -0500
+++ acl.c	2010-04-13 07:09:56.000000000 -0500
@@ -794,7 +794,8 @@
 			MATCHES_MEMSET( &tmp_matches );
 			tmp_data = &tmp_matches.dn_data[0];
 
-			if ( a->acl_attrval_style == ACL_STYLE_REGEX )
+			if ( a->acl_attrval.bv_len &&
+				( a->acl_attrval_style == ACL_STYLE_REGEX ) )
 				tmp_matchesp = matches;
 			else switch ( a->acl_dn_style ) {
 			case ACL_STYLE_REGEX:
@@ -861,7 +862,8 @@
 			bv.bv_val = buf;
 
 			/* Expand value regex */
-			if ( a->acl_attrval_style == ACL_STYLE_REGEX )
+			if ( a->acl_attrval.bv_len &&
+				( a->acl_attrval_style == ACL_STYLE_REGEX ) )
 				tmp_matchesp = matches;
 			else switch ( a->acl_dn_style ) {
 			case ACL_STYLE_REGEX:
@@ -1548,7 +1550,8 @@
 
 				rc = 0;
 
-				if ( a->acl_attrval_style == ACL_STYLE_REGEX )
+				if ( a->acl_attrval.bv_len &&
+					( a->acl_attrval_style == ACL_STYLE_REGEX ) )
 					tmp_matchesp = matches;
 				else switch ( a->acl_dn_style ) {
 				case ACL_STYLE_REGEX:
@@ -1638,7 +1641,8 @@
 
 				rc = 0;
 
-				if ( a->acl_attrval_style == ACL_STYLE_REGEX )
+				if ( a->acl_attrval.bv_len &&
+					( a->acl_attrval_style == ACL_STYLE_REGEX ) )
 					tmp_matchesp = matches;
 				else switch ( a->acl_dn_style ) {
 				case ACL_STYLE_REGEX:


[prev in list] [next in list] [prev in thread] [next in thread]