[Patches] The way to remove gensec_update_ev()

[Stefan Metzmacher]

Am 21.05.2017 um 13:12 schrieb Stefan Metzmacher via samba-technical:
> Am 19.05.2017 um 22:54 schrieb Andrew Bartlett:
>> On Sat, 2017-05-20 at 08:20 +1200, Andrew Bartlett via samba-technical
>> wrote:
>>> On Thu, 2017-05-18 at 16:31 +0200, Stefan Metzmacher wrote:
>>>> Am 17.05.2017 um 20:10 schrieb Stefan Metzmacher via samba-technical:
>>>>> Am 17.05.2017 um 19:56 schrieb Andrew Bartlett:
>>>>>> On Wed, 2017-05-17 at 14:12 +0200, Stefan Metzmacher via samba-
>>>>>> technical wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I'm currently working on the removal of gensec_update_ev(),
>>>>>>> which relies on nested event loops to be activated.
>>>>>>>
>>>>>>> If we want to have proper support for trusted domains in the
>>>>>>> as AD DC, we need to use real async authentication because
>>>>>>> we still use a single process model for the rpc server.
>>>>>>>
>>>>>>> So the first step is to make all users of gensec_update_ev()
>>>>>>> use gensec_update_send/recv instead.
>>>>>>>
>>>>>>> Once we have that we need to make the low level auth stack async
>>>>>>> for NTLMSSP (as a server) and Kerberos (as a client).
>>>>>>>
>>>>>>> Here's the first chunk of patches, they passed a private autobuild.
>>>>>>>
>>>>>>> Please review and push:-)
>>>>>>
>>>>>> I'm looking at these now.  I'm correcting a couple of minor issues in
>>>>>> commit messages (patch of patches attached), but I also noticed:
>>>>>>
>>>>>> Subject: [PATCH 25/35] auth/ntlmssp: add implement
>>>>>>  gensec_ntlmssp_update_send/recv()
>>>>>>
>>>>>> -	if (!out_mem_ctx) {
>>>>>> -		/* if the caller doesn't want to manage/own the
>>>>>> memory,
>>>>>> -		   we can put it on our context */
>>>>>> -		out_mem_ctx = ntlmssp_state;
>>>>>>
>>>>>> I don't think it is used, but can we have this as a distinct commit? 
>>>>>> I'm sorry to be petty, but it is hard enough reading the change to
>>>>>> async without also removing this at the same time.
>>>>>
>>>>> Ok, no problem.
>>>>
>>>> Done, the result is in
>>>> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-gensec-ok
>>>>
>>>> I've added your review to the last 24 patches (of chunk 2),
>>>> the first 39 don't have review tags yet.
>>>
>>> I've reviewed it, but it failed autobuild for samba-o3 with:
>>
>> The reviewed branch is at 
>>
>> https://git.samba.org/?p=abartlet/samba.git/.git;a=shortlog;h=refs/heads/metze-gensec
>>
>>> ../auth/ntlmssp/ntlmssp.c: In function gensec_ntlmssp_update_send:
>>> ../auth/ntlmssp/ntlmssp.c:179:31: error: i may be used uninitialized in
>>> this function [-Werror=maybe-uninitialized]
>>>   status = ntlmssp_callbacks[i].sync_fn(gensec_security,
>>>                                ^
>>> Sorry,
> 
> The compiler doesn't believe tevent_req_nterror() only returns true on
> NT_STATUS_IS_OK().
> 
> This diff removes the warning
> 
> @@ -161,7 +161,7 @@ static struct tevent_req
> *gensec_ntlmssp_update_send(TALLOC_CTX *mem_ctx,
>         struct tevent_req *req = NULL;
>         struct gensec_ntlmssp_update_state *state = NULL;
>         NTSTATUS status;
> -       uint32_t i;
> +       uint32_t i = 0;
> 
> Is it ok to squash that to the "auth/ntlmssp: add implement
> gensec_ntlmssp_update_send/recv()" commit and push everything?

"everything" is
https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-gensec-ok

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170521/2b570e8d/signature.sig>