Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 232910 - ACI targetattr list parser is whitespace sensitive
Summary: ACI targetattr list parser is whitespace sensitive
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: 389
Classification: Retired
Component: Security - Access Control (ACL)
Version: 1.0.4
Hardware: All
OS: Linux
high
medium
Target Milestone: ---
Assignee: Rich Megginson
QA Contact: Viktor Ashirov
URL:
Whiteboard:
Depends On:
Blocks: FDS1.1.0
TreeView+ depends on / blocked
 
Reported: 2007-03-19 14:06 UTC by Martin
Modified: 2015-12-07 16:31 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-12-07 16:31:19 UTC


Attachments (Terms of Use)
diffs (deleted)
2007-10-18 20:09 UTC, Rich Megginson
no flags Details | Diff
new diffs (deleted)
2007-10-18 20:20 UTC, Rich Megginson
no flags Details | Diff
cvs commit log (deleted)
2007-10-18 20:55 UTC, Rich Megginson
no flags Details

Description Martin 2007-03-19 14:06:57 UTC
Description of problem:

I have the following aci in some changetype ldif that
I want to use to maintain the server acls:

dn: dc=qmul,dc=ac,dc=uk
changetype: modify
replace: aci
aci: (targetattr="qmulStudentID||
                  qmulStudentAdvisor||
                  qmulStudentCategory||
                  qmulStudent||
                  qmulStudentCampusCode||
                  qmulStudentType||
                  qmulStudentCompletionDate||
                  qmulStudentCreateDate||
                  qmulStudentDevYear||
                  qmulStudentDeptCode||
                  qmulStudentEnrolCode||
                  qmulStudentProgCode||
                  qmulStudentProgTypeCode||
                  qmulStudentPublish||
                  qmulStudentTerminationDate||
                  qmulStudentYear" )
 (version 3.0;
  acl "allow qmulStudent attribute access by ip.";
  deny (all)
  (userdn="ldap:///anyone") and
  (ip!="127.0.0.1" and 
   ip!="138.37.8.140")
  ;)
aci: 
 (targetattr = "*")
 (version 3.0;
  acl "allow basic information access by ip.";
  allow (read,compare,search)
  (userdn = "ldap:///anyone") and
  (ip="138.37.8.140" or 
   ip="138.37.8.220" or
   ip="127.0.0.1")
  ;)

Curiously, when searching from 8.220 I see that qmulStudentID and
qmulStudentYear (but no other qmulStudent*) attributes are listed
in the search output.

If I remove the space from the last line of the targetattr section.
i.e.

-                  qmulStudentYear" )
+                  qmulStudentYear")

Then the aci behaves as expected.

Version-Release number of selected component (if applicable):

1.0.4

How reproducible:

Always.


Steps to Reproduce:
1. Apply the aci with ldapmodify (the openldap one if it is of
 any significance).

2. Do a search from 138.37.8.220
  
Actual results:

qmulStudentID and qmulStudentYear are in the output.

Expected results:

They shouldnt be there.

Comment 2 Rich Megginson 2007-10-18 20:09:54 UTC
Created attachment 231501 [details]
diffs

Comment 3 Rich Megginson 2007-10-18 20:20:05 UTC
Created attachment 231531 [details]
new diffs

Comment 4 Rich Megginson 2007-10-18 20:55:46 UTC
Created attachment 231601 [details]
cvs commit log

Reviewed by: nkinder, nhosoi (Thanks!)
Files: see diff
Branch: HEAD
Fix Description: Need to trim trailing whitespace from the targetattr clause. 
I noticed that targetattrfilters had the same problem, except it returned
ACL_SYNTAX_ERR in that case, so I changed targetattr to do the same.
Platforms tested: RHEL5 x86_64
Flag Day: no
Doc impact: no

Comment 5 Rich Megginson 2007-10-19 19:02:06 UTC
Checking in aclparse.c;
/cvs/dirsec/ldapserver/ldap/servers/plugins/acl/aclparse.c,v  <--  aclparse.c
new revision: 1.9; previous revision: 1.8
done


Fix Description: I made it too sensitive.  The parser should allow simple
unquoted strings.  However, if it begins with a quote, it must end with a quote.

Index: aclparse.c
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/plugins/acl/aclparse.c,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -r1.8 -r1.9
--- aclparse.c	18 Oct 2007 20:55:10 -0000	1.8
+++ aclparse.c	19 Oct 2007 19:01:16 -0000	1.9
@@ -1234,14 +1234,21 @@
 	__acl_strip_leading_space(&s);
 	__acl_strip_trailing_space(s);
 	len = strlen(s);
-	if (*s == '"' && s[len-1] == '"') {
-		s[len-1] = '\0';
-		s++;
-	} else {
-		slapi_log_error(SLAPI_LOG_FATAL, plugin_name,
-						"__aclp__init_targetattr: Error: The statement does not begin and end
with a \": [%s]\n",
-						s);
-		return ACL_SYNTAX_ERR;
+    /* Simple targetattr statements may not be quoted e.g.
+       targetattr=* or targetattr=userPassword
+       if it begins with a quote, it must end with one as well
+    */
+	if (*s == '"') {
+		s++; /* skip leading quote */
+        if (s[len-1] == '"') {
+            s[len-1] = '\0'; /* trim trailing quote */
+        } else {
+            /* error - if it begins with a quote, it must end with a quote */
+            slapi_log_error(SLAPI_LOG_FATAL, plugin_name,
+                            "__aclp__init_targetattr: Error: The statement does
not begin and end with a \": [%s]\n",
+                            attr_val);
+            return ACL_SYNTAX_ERR;
+        }
 	}
 
 	str = s;

Comment 6 Rich Megginson 2007-10-19 20:08:59 UTC
*** Bug 340281 has been marked as a duplicate of this bug. ***

Comment 7 Rich Megginson 2007-10-19 22:16:08 UTC
Checking in aclparse.c;
/cvs/dirsec/ldapserver/ldap/servers/plugins/acl/aclparse.c,v  <--  aclparse.c
new revision: 1.10; previous revision: 1.9
done

ix Description: In addition to the previous fixes, test for quote at end of
string before incrementing s - otherwise test will always fail.



Index: aclparse.c
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/plugins/acl/aclparse.c,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -r1.9 -r1.10
--- aclparse.c	19 Oct 2007 19:01:16 -0000	1.9
+++ aclparse.c	19 Oct 2007 22:14:56 -0000	1.10
@@ -1239,16 +1239,16 @@
        if it begins with a quote, it must end with one as well
     */
 	if (*s == '"') {
+		if (s[len-1] == '"') {
+			s[len-1] = '\0'; /* trim trailing quote */
+		} else {
+			/* error - if it begins with a quote, it must end with a quote */
+			slapi_log_error(SLAPI_LOG_FATAL, plugin_name,
+							"__aclp__init_targetattr: Error: The statement does not begin and end
with a \": [%s]\n",
+							attr_val);
+			return ACL_SYNTAX_ERR;
+		}
 		s++; /* skip leading quote */
-        if (s[len-1] == '"') {
-            s[len-1] = '\0'; /* trim trailing quote */
-        } else {
-            /* error - if it begins with a quote, it must end with a quote */
-            slapi_log_error(SLAPI_LOG_FATAL, plugin_name,
-                            "__aclp__init_targetattr: Error: The statement does
not begin and end with a \": [%s]\n",
-                            attr_val);
-            return ACL_SYNTAX_ERR;
-        }
 	}
 
 	str = s;


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