[Patches] The way to remove gensec_update_ev()

Stefan Metzmacher metze at samba.org
Sun May 21 11:12:59 UTC 2017


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?

Thanks!
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/88854e57/signature.sig>


More information about the samba-technical mailing list