Need a change to the ACL code

[Nadezhda Ivanova]

Hi Andrew,
Currently, during a rename operation the acl module checks for permission to
modify "name" attribute, and in case the rename is actually moving the
object, permissions to delete the object and re-create it under the new
parent - these were the access checks mentioned in the documentation. We do
not check for permissions on the rdn, as according to docs its not required.
This case is working, at least with user objects, we have the python tests
to prove it.
I will add some test for renaming an OU, maybe we have a bug there.
If we do not have permission to modify another attribute, this will be
handled again by the acl module, on a modify operation.
This is done by the acl_modify and acl_rename functions in acl.c.
The rdn module - as I read the code - forbids modify operation on the rdn.
On a rename, it creates a new rename request, which should fail if we do not
have permissions to modify name or move. If it succeeds, the callback
modifies the rdn and the name attribute, which should fail again if we do
not have permission to modify those. So if we move the rdn module below the
acl, we will indeed not have an access check when modifying rdn any more,
and we may get insufficient access rights error instead of
LDB_ERR_NOT_ALLOWED_ON_RDN on modify.

So - and I hopefully got it this time :) - we may need to add a check for
permission to modify the rdn in the acl_rename? I say may, because this is
not documented, and I want to make a test against windows to assure we need
to do it.

Regards,
Nadya


On Tue, Mar 2, 2010 at 11:13 PM, Andrew Bartlett <abartlet at samba.org> wrote:

> On Tue, 2010-03-02 at 23:05 +0200, Nadezhda Ivanova wrote:
> > Hi Andrew,
> > I guess I wasnt paying attention to the patch.
> > Actually a rename/move operation has some additional access checks that
> need
> > to pass, so relying on the modifies will not do. But I'll apply your
> patch
> > and trace exactly what is happening, and we'll find a workaround. The
> change
> > affects also working with tdb, right? I mean, I do not have to
> reprovision
> > with openLDAP to see the changed flow?
>
> Correct.  I made the change to the standard 'with tdb' module stack to
> try and have consistent behaviour between backends.
>
> To make it more clear (I should have included an example up front):
>
> So, imagine I am not allowed to write to attribute 'ou', and I rename an
> object from 'ou=blah,dc=example,dc=com' to 'ou=foo,dc=example,dc=com'.
> Nothing at the moment, as I read the code, stops this.
>
> Andrew Bartlett
>
> --
> Andrew Bartlett                                http://samba.org/~abartlet/<http://samba.org/%7Eabartlet/>
> Authentication Developer, Samba Team           http://samba.org
> Samba Developer, Cisco Inc.
>
>