[PATCH] Fix 'net ads changetrustpw'

[Richard Sharpe]

On Thu, Aug 10, 2017 at 10:47 PM, Andreas Schneider <asn at samba.org> wrote:
> 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 :-)

OK, I can see what is happening. The indentation is wrong for the
whole file, at least in current master (pulled last night) as shown by
the attached screen shot.

In that case: RB+

-- 
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)