Extended request in kludge acl

[Matthias Dieter Wallnöfer]

Hi Nadya,

I've tried to introduce the user <-> message equal DN check but didn't 
had in mind that also other authorized users (with the change permission 
set - as the default Administrator) can call "samr_ChangePasswordUser" 
(as it's with the standard LDAP user-password change). Therefore tests 
broke and I had to revert.
Then I thought about the behaviour that every one can change other 
people's password. And no, this isn't a security leak since you have to 
know the old password. And then it is just the same if you login with 
the other user credentials (username and the old password you know) and 
then perform the password change.

I really think we need to apply the short (first) version.

Greets,
Matthias

Nadezhda Ivanova wrote:
> Hi Andrew,
> We had some discussion with Matthias on IRC, and we came up with 3 
> ways to solve the problem.
> One way is to keep the control and apply the patch as it is. I really 
> do not like it because of reasons listed in previous mails. Another is 
> to use an extended operation, which would complicate things and is yet 
> another way to bypass security checks.
> The third way, which I am most in favor of, is for the samr to 
> actually start providing both passwords on a password change, so we 
> can use the standard flow of things. It just seems wrong to me to use 
> sambd_set_password instead of a function that will provide both, and 
> then introduce internal hacks to handle the problem.
>
> What do you think?
>
> Regards,
> Nadya
>
> On Thu, Jul 8, 2010 at 10:45 AM, Matthias Dieter Wallnöfer 
> <mdw at samba.org <mailto:mdw at samba.org>> wrote:
>
>     Could we discuss this on IRC with Andrew?
>
>     Matthias
>
>     Nadezhda Ivanova wrote:
>
>         Hi Matthias,
>         I thought some more about the patch and I have missed
>         something very important.
>         This patch makes it so given the control we actually execute a
>         reset operation with the permissions of a change operation.
>         However, every user account actually is given permission to
>         change every other accoun't password. There is an ACE that
>         comes from the defaultSd that gives this right to EVERYONE.
>         The only thing that we count on after that is that the user
>         knows the correct old password to verify identity. With the
>         replace its different, permission has to be granted. So what
>         you are proposing is to essentially allow everyone to reset a
>         user's password with samr, which worries me A LOT. Maybe there
>         is something I am missing on the samr side.
>
>         Sorry I did not think of this earlier...
>
>         Regards,
>         Nadya
>
>
>         On Thu, Jul 8, 2010 at 10:20 AM, Matthias Dieter Wallnöfer
>         <mdw at samba.org <mailto:mdw at samba.org> <mailto:mdw at samba.org
>         <mailto:mdw at samba.org>>> wrote:
>
>            Andrew,
>
>            Andrew Bartlett wrote:
>
>                On Thu, 2010-07-08 at 08:12 +0200, Matthias Dieter
>         Wallnöfer
>                wrote:
>                  The approach you suggest would also work, but would
>         not have
>                as strict a
>                control over the transaction, as you would not be in the
>                transaction
>                when the old pw was checked.   (I'm not sure this
>         matters in
>                practice,
>                given you could have the same race on multiple DCs anyway).
>
>            Well, we have only one search/read request and then one modify
>            one. I think it should be safe to split them up in two distinct
>            transactions since the latter one (mainly the code in
>            "password_hash" module) has to be performed in an atomic
>         manner.
>
>                Like the AS_SYSTEM control, this control is a little
>                dangerous, and
>                would have to be carefully restricted.  Please ensure
>         the kpasswd
>                password change code handles this too.
>
>            The control is strictly private. You cannot set it over the
>         LDAP
>            protocol. But anyway I'm fine to rechange the code when we do
>            support extended operations ACLs. The kpasswd patch does
>         naturally
>            also exist - it's here:
>         http://repo.or.cz/w/Samba/mdw.git/commitdiff/11f24a9a72d4c47359662ee5ba63433ac11e4b2c.
>
>            The code was tested manually and by "make test" and it does
>         work.
>            I hope you are fine about pushing it.
>
>            Matthias
>
>
>
>