Bug 244229 - targetattr not verified against schema when setting an aci
targetattr not verified against schema when setting an aci
Status: CLOSED CURRENTRELEASE
Product: 389
Classification: Community
Component: Security - Access Control (ACL) (Show other bugs)
7.1
All Linux
high Severity low
: ---
: ---
Assigned To: Noriko Hosoi
Viktor Ashirov
:
Depends On:
Blocks: 639035
  Show dependency treegraph
 
Reported: 2007-06-14 12:25 EDT by Simo Sorce
Modified: 2015-12-07 11:54 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2015-12-07 11:54:51 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
git patch file (master) (7.37 KB, patch)
2010-10-14 19:09 EDT, Noriko Hosoi
no flags Details | Diff
revised git patch file (master) (13.91 KB, patch)
2010-10-15 14:00 EDT, Noriko Hosoi
nhosoi: review?
rmeggins: review+
Details | Diff

  None (edit)
Description Simo Sorce 2007-06-14 12:25:52 EDT
Description of problem:
The attributes specified in targetattr are not chacked against the schema for
validity

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

How reproducible:
set an aci using ldapmodify (not the console), use the targetattr option and
specify a non existing attribute, the aci will be accepted

Actual results:
the aci is accepted

Expected results:
the aci is refused

Additional info:

think about finding it late that you specified targetatt != userPasswodr (note
the wrong spelling) instead of targetattr != userPassword
Comment 4 Noriko Hosoi 2010-10-14 19:09:44 EDT
Created attachment 453598 [details]
git patch file (master)

Description:
1. When acl contains targetattr keyword:
   (targetattr [!]= "attribute_1 || attribute_2 ...|| attribute_n"),
   where attribute_n does not contain '*', the current ACL plugin
   accepts any attribute_n value even if it is not defined in the
   schema.  This patch rejects the aci if it contains attribute_n
   not defined in schema with this error message:
     NSACLPlugin - targetattr "attribute_n" does not exist in schema.
     Please add attributeTypes "attribute_n" to schema if necessary.
2. To implement 1, slapi APIs slapi_attr_syntax_exists and
   slapi_vattr_type_exists are added.
3. An attributeTypes "connection" is added to 01core389.ldif which
   is referred in an aci of cn=monitor.

Files:
 ldap/schema/01core389.ldif
 ldap/servers/plugins/acl/aclparse.c
 ldap/servers/slapd/attrsyntax.c
 ldap/servers/slapd/slapi-plugin.h
 ldap/servers/slapd/vattr.c
Comment 5 Noriko Hosoi 2010-10-14 19:28:49 EDT
Steps to verify:
$ ldapmodify ... << EOF
dn: dc=example,dc=com
changetype: modify
add: aci
aci: (target=ldap:///dc=example,dc=com)(targetattr!=roomnum)(version 3.0; acl "acl3"; allow(read, search, compare) userdn = "ldap:///anyone";)
EOF
ldap_modify: Invalid syntax
ldap_modify: additional info: ACL Syntax Error(-5):(target=ldap:///dc=example,dc=com)(targetattr!=roomnum)(version 3.0; acl \22acl3\22; allow(read, search, compare) userdn = \22ldap:///anyone\22;)

# egrep NSACLPlugin /var/log/dirsrv/slapd-ID/errors
[..] NSACLPlugin - targetattr "roomnum" does not exist in schema. Please add attributeTypes "roomnum" to schema if necessary.
[..] NSACLPlugin - ACL Syntax Error(-5):(target=ldap:///dc=example,dc=com)(targetattr!=roomnum)(version 3.0; acl \22acl3\22; allow(read, search, compare) userdn = \22ldap:///anyone\22;)
Comment 6 Rich Megginson 2010-10-14 20:45:09 EDT
Is it possible to have the message returned from ldapmodify say which attribute is the problem?
Comment 7 Rich Megginson 2010-10-15 11:09:06 EDT
Why do you need the vattr type exists?  That is, virtual attributes should be in the schema too - wouldn't they be returned by slapi_attr_syntax_exists?
Comment 8 Noriko Hosoi 2010-10-15 13:38:51 EDT
(In reply to comment #6)
> Is it possible to have the message returned from ldapmodify say which attribute
> is the problem?

This is a sample output from ldapmodify.  Is this clear/clean enough?
ldap_modify: Invalid syntax
ldap_modify: additional info: targetattr "roomnum" does not exist in schema. Please add attributeTypes "roomnum" to schema if necessary. ACL Syntax Error(-5):(target=ldap:///dc=example,dc=com)(targetattr!=\22cn || roomnum\22)(version 3.0; acl \22acl3\22; allow(read, search, compare) userdn = \22ldap:///anyone\22;)

(In reply to comment #7)
> Why do you need the vattr type exists?  That is, virtual attributes should be
> in the schema too - wouldn't they be returned by slapi_attr_syntax_exists?

Thanks for your comment!  I was not certain about it.  I removed the vattr check and the helper api.
Comment 9 Rich Megginson 2010-10-15 13:45:44 EDT
(In reply to comment #8)
> (In reply to comment #6)
> > Is it possible to have the message returned from ldapmodify say which attribute
> > is the problem?
> 
> This is a sample output from ldapmodify.  Is this clear/clean enough?
> ldap_modify: Invalid syntax
> ldap_modify: additional info: targetattr "roomnum" does not exist in schema.
> Please add attributeTypes "roomnum" to schema if necessary. ACL Syntax
> Error(-5):(target=ldap:///dc=example,dc=com)(targetattr!=\22cn ||
> roomnum\22)(version 3.0; acl \22acl3\22; allow(read, search, compare) userdn =
> \22ldap:///anyone\22;)

Yes, this looks good.

> 
> (In reply to comment #7)
> > Why do you need the vattr type exists?  That is, virtual attributes should be
> > in the schema too - wouldn't they be returned by slapi_attr_syntax_exists?
> 
> Thanks for your comment!  I was not certain about it.  I removed the vattr
> check and the helper api.
Comment 10 Noriko Hosoi 2010-10-15 14:00:29 EDT
Created attachment 453769 [details]
revised git patch file (master)

Following the comments by Rich, revised the proposal so that:
1) the error message is sent to the client, as well.
2) removed the useless virtual attribute type name checking.
Comment 11 Rich Megginson 2010-10-15 14:22:02 EDT
Comment on attachment 453769 [details]
revised git patch file (master)

ship it
Comment 12 Noriko Hosoi 2010-10-15 14:32:15 EDT
(In reply to comment #11)
> 
> ship it

Reviewed by Rich (Thanks!!!).

Pushed to master.

$ git merge 244229
Updating 032790e..0b7a846
Fast-forward
 ldap/schema/01core389.ldif          |    1 +
 ldap/servers/plugins/acl/acl.c      |    2 +-
 ldap/servers/plugins/acl/acl.h      |    4 +-
 ldap/servers/plugins/acl/acllist.c  |    2 +-
 ldap/servers/plugins/acl/aclparse.c |   87 ++++++++++++++++++++++-------------
 ldap/servers/slapd/attrsyntax.c     |    6 ++
 ldap/servers/slapd/slapi-plugin.h   |    8 +++
 7 files changed, 74 insertions(+), 36 deletions(-)

$ git push
Counting objects: 29, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (15/15), done.
Writing objects: 100% (15/15), 2.55 KiB, done.
Total 15 (delta 13), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
   032790e..0b7a846  master -> master
Comment 13 Amita Sharma 2011-05-31 03:11:44 EDT
[root@testvm home]# ldapmodify -x -h localhost -p 389 -D "cn=Directory Manager" -w XXX << EOF
dn: dc=example,dc=com
changetype: modify
add: aci
aci: (target=ldap:///dc=example,dc=com)(targetattr!=roomnum)(version 3.0; acl "acl3"; allow(read, search, compare) userdn = "ldap:///anyone";)
EOF

modifying entry "dc=example,dc=com"
ldap_modify: Invalid syntax (21)
	additional info: targetattr "roomnum" does not exist in schema. Please add attributeTypes "roomnum" to schema if necessary. ACL Syntax Error(-5):(target=ldap:///dc=example,dc=com)(targetattr!=roomnum)(version 3.0; acl \22acl3\22; allow(read, search, compare) userdn = \22ldap:///anyone\22;)

[root@testvm home]# egrep NSACLPlugin /var/log/dirsrv/slapd-testvm/errors
[31/May/2011:12:39:36 +051800] NSACLPlugin - __aclp__init_targetattr: targetattr "roomnum" does not exist in schema. Please add attributeTypes "roomnum" to schema if necessary. 
[31/May/2011:12:39:36 +051800] NSACLPlugin - ACL Syntax Error(-5):(target=ldap:///dc=example,dc=com)(targetattr!=roomnum)(version 3.0; acl \22acl3\22; allow(read, search, compare) userdn = \22ldap:///anyone\22;)

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