[PATCH] Fix 'net ads changetrustpw'

[Andreas Schneider]

On Friday, 11 August 2017 02:06:15 CEST Richard Sharpe wrote:
> On Wed, Aug 9, 2017 at 9:45 AM, Andreas Schneider via samba-technical
> 
> <samba-technical at lists.samba.org> wrote:
> > Hi,
> > 
> > as always, untested code is broken code. So I broke 'net ads
> > changetrustpw' in Samba 4.6.
> > 
> > The attached patch fixes the issue and adds a tests which verifies that it
> > is working.
> > 
> > 
> > Please review and push if OK.
> 
> I have a couple of questions. While it looks OK,
> 
> 1. I can't see why you changed princ to NULL in the following:
> 
>           ret = krb5_set_password(context,
>                                                   &creds,
>                                                   discard_const_p(char,
> newpw), - princ,
>                                                   + NULL,
>                                                   &result_code,
>                                                   &result_code_string,
>                                                   &result_string);
> 
> Perhaps it's because we just got a changepw ticket for the princ, so
> it is established in the creds who we are changing the password for.

If we 'set' a password the client principal and the target principal are 
different. That isn't the case, we change our own password so we need to pass 
NULL.

> 
> 2. Can you fix the indentation in the call to krb5_set_password?

What indentation? It looks correct to me. You know that the patch tool adds a 
+ at the beginning so the indentation with tabs can look strange in patch 
files. Please apply the patch and check the source code :-)

> Other than that, I think the backticks are fine until someone changes
> them all to bash style $(...) for all tests.
> 
> So, conditional RB+ by me.


Thanks,


	Andreas

-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org