389-ds-base

Clone
Source Code
GIT

#47553 Enhance ACIs to have more control over MODRDN operations
Closed: wontfix None Opened 11 years ago by nkinder.

Closed: wontfix
Reopen Issue

The current ACI implementation is somewhat limited with the way it handles MODRDN operations where newsuperior is used. For example, consider the case where you want to move an entry as follows:

uid=tuser,ou=ou1,dc=example,dc=com -> uid=tuser,ou=ou2,dc=example,dc=com

To allow someone to perform this operation, they need the "add" permission on "ou=ou2,dc=example,dc=com". This permission would also allow one to add a brand new entry to "ou=ou2,dc=example,dc=com". Certain use cases might want to allow a user to move an entry into a particular portion of the tree, but disallow adding a new entry.

We could extend the ACI code to have a new permission for move operations that is separate from the "add" permission. This permission would mean that the target could be used as the destination for a move operation.

In addition, it would be nice to be able to control the source for a move operation in ACIs. This would allow one to have an ACI that says "Allow user X to move entries from ou=1 to ou=2". This would likely require a new ACI keyword to specify the source (or multiple sources) for the operation. The destination could likely just use the ACI target.

I have investigated a new permission keyword to control MODDN.
I would propose two new keywords (final keywords to be defined)
* moddn_to
* moddn_from

Then we may grant (allow/deny) the permission to move (MODDN) entry from/to given target entries.
For example, with the following DIT and aci definition:

{{{
dn: dc=example,dc=com
...
aci: (target = "ldap:///cn=staged user,dc=example,dc=com")(version 3.0; acl "M
ODDN from"; allow (moddn_from) userdn = "ldap:///cn=bind_entry,dc=example,dc=
com";)
aci: (target = "ldap:///cn=accounts,dc=example,dc=com")(version 3.0; acl "MODD
N to"; allow (moddn_to) userdn = "ldap:///cn=bind_entry,dc=example,dc=com";)

dn: cn=bind_entry,dc=example,dc=com

dn: cn=staged user,dc=example,dc=com

dn: cn=accounts,dc=example,dc=com

dn: cn=new_account0,cn=staged user,dc=example,dc=com

dn: cn=new_account1,cn=staged user,dc=example,dc=com

dn: cn=new_account2,cn=staged user,dc=example,dc=com

...

}}}

being bound as 'cn=bind_entry,dc=example,dc=com' we are allowed to move (MODDN)

{{{
cn=new_account0,cn=staged user,dc=example,dc=com -> cn=new_account0,cn=accounts,dc=example,dc=com
}}}

But we are not allow to ADD entries under cn=accounts,dc=example,dc=com or DEL entries under cn=staged user,dc=example,dc=com.
In addition with this aci definition we are only allowed to move STAGING -> PRODUCTION.
Additional ACI are required to move PRODUCTION -> STAGING.

Does this solution address your concern ?

{{{
507 } else if (( filterChoice == LDAP_FILTER_SUBSTRINGS) &&
508 (type & ACI_TARGET_MODDN)) {
509 if (is_target_to) {
510 type |= ACI_TARGET_MODDN_TO_PATTERN;
511 } else {
512 type |= ACI_TARGET_MODDN_FROM_PATTERN;
513 }
514 }
}}}
Is it possible for an aci to have both a target_to that is a pattern, and a target_from that is a pattern?

Yes it is possible but in separated aci information. For example with this aci:

{{{
aci: (target_to="ldap:///uid=,cn=production,dc=redhat")(target_from="ldap:///uid=,cn=staging,dc=redhat")
(version 3.0; acl \"MODDN from staging to production\"; allow (moddn)
userdn = \"ldap:///uid=bind_entry,dc=redhat\";)
}}}

__aclp__parse_aci will be called for '(target_to="ldap:///uid=,cn=production,dc=redhat")' then it will be called for '(target_from="ldap:///uid=,cn=staging,dc=redhat")'.

So the type will be ACI_TARGET_MODDN | ACI_TARGET_MODDN_TO_PATTERN for the first element and
(ACI_TARGET_MODDN | ACI_TARGET_MODDN_FROM_PATTERN) for the second

ok - ack

As far as the test code - ack - but it looks as though we copy/paste the topology() method into a lot of test code. Is there some way to abstract this code and put it into some sort of test library? We should open a separate ticket for that.

'''master'''

git merge ticket47553_single_aci
Updating 07d4b61..dc17d63
Fast-forward
dirsrvtests/tickets/ticket47553_single_aci_test.py | 1187 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
ldap/schema/01core389.ldif | 1 +
ldap/servers/plugins/acl/acl.c | 250 ++++++++++++++----
ldap/servers/plugins/acl/acl.h | 47 ++--
ldap/servers/plugins/acl/aclanom.c | 40 ++-
ldap/servers/plugins/acl/acleffectiverights.c | 7 +
ldap/servers/plugins/acl/aclparse.c | 117 +++++++-
ldap/servers/plugins/acl/aclplugin.c | 2 +-
ldap/servers/plugins/acl/aclutil.c | 5 +-
ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 59 ++++-
ldap/servers/slapd/config.c | 17 +-
ldap/servers/slapd/libglobs.c | 33 +++
ldap/servers/slapd/proto-slap.h | 2 +
ldap/servers/slapd/slap.h | 2 +
ldap/servers/slapd/slapi-plugin.h | 7 +-
15 files changed, 1678 insertions(+), 98 deletions(-)
create mode 100644 dirsrvtests/tickets/ticket47553_single_aci_test.py

git push origin master

Counting objects: 50, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (26/26), done.
Writing objects: 100% (26/26), 12.49 KiB, done.
Total 26 (delta 22), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
07d4b61..dc17d63 master -> master

commit dc17d63
Author: Thierry bordaz (tbordaz) tbordaz@redhat.com
Date: Fri Feb 28 14:24:53 2014 +0100

Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1118014

Description: Macro SLAPI_ACL_ALL does not contain SLAPI_ACL_MODDN.
Thus, even though all operations are allowed by "allow (all)", just
modrdn fails with "Insufficient access (50)".

git patch file (master) -- aci: add the SLAPI_ACL_MODDN bit to SLAPI_ACL_ALL.
0001-Ticket-47553-Enhance-ACIs-to-have-more-control-over-.2.patch

Steps:
Set the "allow (all)" aci for the admin user to the suffix dc=example,dc=com.
{{{
dn: dc=example,dc=com
aci: (targetattr="*")(version 3.0; acl "Configuration Administrators Group"; a
llow (all) groupdn="ldap:///cn=Configuration Administrators,ou=Groups,ou=Topo
logyManagement,o=NetscapeRoot";)
}}}
Without the patch, modrdn fails.
{{{
$ ldapmodify -x -h localhost -p 389 -D "uid=admin,ou=Administrators,ou=TopologyManagement,o=NetscapeRoot" -w <pw>
dn: uid=tuser0,ou=people,dc=example,dc=com
changetype: modrdn
newrdn: uid=tuser0
deleteoldrdn: 1
newsuperior: dc=example,dc=com

modifying rdn of entry "uid=tuser0,ou=people,dc=example,dc=com"
ldap_rename: Insufficient access (50)
additional info: Insufficient 'moddn' privilege to move an entry to 'dc=example,dc=com'.
}}}

Replying to [comment:14 nhosoi]:

Steps:
Set the "allow (all)" aci for the admin user to the suffix dc=example,dc=com.
{{{
dn: dc=example,dc=com
aci: (targetattr="*")(version 3.0; acl "Configuration Administrators Group"; a
llow (all) groupdn="ldap:///cn=Configuration Administrators,ou=Groups,ou=Topo
logyManagement,o=NetscapeRoot";)
}}}
Without the patch, modrdn fails.
{{{
$ ldapmodify -x -h localhost -p 389 -D "uid=admin,ou=Administrators,ou=TopologyManagement,o=NetscapeRoot" -w <pw>
dn: uid=tuser0,ou=people,dc=example,dc=com
changetype: modrdn
newrdn: uid=tuser0
deleteoldrdn: 1
newsuperior: dc=example,dc=com

modifying rdn of entry "uid=tuser0,ou=people,dc=example,dc=com"
ldap_rename: Insufficient access (50)
additional info: Insufficient 'moddn' privilege to move an entry to 'dc=example,dc=com'.
}}}

Noriko,

The fix looks good. I was wondering if the value of SLAPI_ACL_ALL should depend on the toggle config_get_moddn_aci. But in fact I can not think to a use case where it is necessary.

Ack for your fix.
The review https://fedorahosted.org/389/attachment/ticket/47553/0001-Ticket-47553-Enhance-ACIs-to-have-more-control-over-.3.patch is still pending (GER issue). Please feel free to switch the review status of that bug

Thank you so much for the deeper analysis. You covered multiple points I missed...

I reviewed your proposal and ack'ed.

The review ​https://fedorahosted.org/389/attachment/ticket/47553/0001-Ticket-47553-Enhance-ACIs-to-have-more-control-over-.3.patch is still pending (GER issue).

Pushed the patch: 0001-Ticket-47553-Enhance-ACIs-to-have-more-control-over-.2.patch to master:
07c1bc2..4aafe74 master -> master
commit 4aafe74

Pushed to 389-ds-base-1.3.3:
40a4951..8a3b351 389-ds-base-1.3.3 -> 389-ds-base-1.3.3
commit 8a3b351

Hi Thierry, please push your patches and close this ticket.
0001-Ticket-47553-Enhance-ACIs-to-have-more-control-over-.3.patch​ added
0002-Ticket-47553-test-case.patch​ added
Thank you!!

push 0001-Ticket-47553-Enhance-ACIs-to-have-more-control-over-.3.patch+0002-Ticket-47553-test-case.patch​

'''master'''
git merge ticket47553_ger

Updating 4aafe74..be67f81
Fast-forward
dirsrvtests/tickets/ticket47553_ger.py | 553 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
ldap/servers/plugins/acl/acleffectiverights.c | 67 +++++-----
2 files changed, 590 insertions(+), 30 deletions(-)
create mode 100644 dirsrvtests/tickets/ticket47553_ger.py

git push origin master
Counting objects: 18, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (10/10), done.
Writing objects: 100% (10/10), 6.07 KiB, done.
Total 10 (delta 6), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
4aafe74..be67f81 master -> master

commit be67f81
Author: Thierry bordaz (tbordaz) tbordaz@redhat.com
Date: Thu Oct 16 17:24:15 2014 +0200

'''389-ds-base-1.3.3'''

git push origin 389-ds-base-1.3.3
Counting objects: 18, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (10/10), done.
Writing objects: 100% (10/10), 6.07 KiB, done.
Total 10 (delta 6), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
8a3b351..96d7459 389-ds-base-1.3.3 -> 389-ds-base-1.3.3

commit 96d7459
Author: Thierry bordaz (tbordaz) tbordaz@redhat.com
Date: Thu Oct 16 17:24:15 2014 +0200

Fix error in lib389 test

4606c8b..513393b master -> master
commit 513393b
Author: Mark Reynolds mreynolds@redhat.com
Date: Thu May 21 15:49:21 2015 -0400

148e510..9ba030f 389-ds-base-1.3.3 -> 389-ds-base-1.3.3
commit 9ba030f

Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1257294

Please, review my slightly revised patch.

Python 2 > Python 3 fix.

cb950de..81dcdd9 master -> master
commit 81dcdd9
Author: Simon Pichugin spichugi@redhat.com
Date: Mon Aug 31 17:22:34 2015 +0200

973d1d9..5babb68 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit 5babb68

670629e..16bd6af 389-ds-base-1.3.3 -> 389-ds-base-1.3.3
commit 16bd6af

Metadata Update from @spichugi:
- Issue assigned to tbordaz
- Issue set to the milestone: 1.3.3 - 3/14 (March)
7 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/890

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)
4 years ago

Log in to comment on this ticket.

Metadata
None
None
None
major
ack
IPA
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.