[Patches] The way to remove gensec_update_ev()

classic Classic list List threaded Threaded
36 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
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:-)

Thanks!
metze

gensec-async-part1-01.patches.txt (147K) Download Attachment
signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
Hi,

here's the next chunk on top.

Both are combined the following branch:
https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-gensec-ok

https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-gensec-tmp
contains the change for the LDAP server, which also pass a private
autobuild, but it needs some more tests to be written.

While
https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-gensec
contains the unfinished parts:
- source4/libcli/smb_composite/sesssetup.c
- source4/lib/http/http_auth.c
- auth/gensec/spnego.c

While auth/gensec/spnego.c is the most difficult and time consuming part.

Please review and push:-)

Thanks!
metze

> 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:-)


gensec-async-part2-01.patches.txt (92K) Download Attachment
signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
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.

The other wrapper changes are much easier to follow, they just rename a
function and call it, but this one also removes hidden return macros
and changes input to in.  They are all great things, but just need to
be a prep patch so I know I'm not missing something.

Thank you so much for the hard work here.  I'll review the next set
shortly.

Andrew Bartlett

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba

commit-message-fixes.patch.txt (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
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.

> The other wrapper changes are much easier to follow, they just rename a
> function and call it, but this one also removes hidden return macros
> and changes input to in.  They are all great things, but just need to
> be a prep patch so I know I'm not missing something.
>
> Thank you so much for the hard work here.  I'll review the next set
> shortly.

Thanks!
metze



signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Wed, 2017-05-17 at 14:43 +0200, Stefan Metzmacher via samba-
technical wrote:
> Hi,
>
> here's the next chunk on top.

This looks pretty good.  It would be great to get another set of eyes
on it, but I'm still happy to say:

Reviewed-by: Andrew Bartlett <[hidden email]>

In the small notes department:

Subject: [PATCH 24/24] s4:libcli/ldap: just use gensec_update() in
 ldap_bind_sasl()

You could indicate that resolving this requires also resolving the
general case in LDB, as that is the API this is used from.  We would
need ldb_connect_send() and ldb_connect_recv() at a start.



> Both are combined the following branch:
> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-gensec-ok
>
> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-gensec-tmp
> contains the change for the LDAP server, which also pass a private
> autobuild, but it needs some more tests to be written.
>
> While
> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-gensec
> contains the unfinished parts:
> - source4/libcli/smb_composite/sesssetup.c
> - source4/lib/http/http_auth.c
> - auth/gensec/spnego.c
>
> While auth/gensec/spnego.c is the most difficult and time consuming part.

I'll also take a peek at these and see where you are going.

Thanks for all the hard work here!

Andrew Bartlett
--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
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.

metze


signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
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:

../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,

Andrew Bartlett

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
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,
>
> Andrew Bartlett
>
--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
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



signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
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


signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Sun, 2017-05-21 at 13:12 +0200, Stefan Metzmacher wrote:

>
> 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?

Yes.  (And I've done that)

Andrew Bartlett
--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Wed, 2017-05-17 at 14:43 +0200, Stefan Metzmacher via samba-
technical wrote:

> Hi,
>
> here's the next chunk on top.
>
> Both are combined the following branch:
> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/
> master3-gensec-ok
>
> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/
> master3-gensec-tmp
> contains the change for the LDAP server, which also pass a private
> autobuild, but it needs some more tests to be written.
>
> While
> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/
> master3-gensec
> contains the unfinished parts:
> - source4/libcli/smb_composite/sesssetup.c
> - source4/lib/http/http_auth.c
> - auth/gensec/spnego.c
>
> While auth/gensec/spnego.c is the most difficult and time consuming
> part.

I'm sorry to say, but there is a regression in 
65655f24842b864f32136c7ffed446223d416512

s4:librpc: use gensec_update_send() in dcerpc_bind_auth_send()
    
    Signed-off-by: Stefan Metzmacher <[hidden email]>
    Reviewed-by: Andrew Bartlett <[hidden email]>

I've got an AD domain that times out due to a trust that is down, and
on that domain it changes from:

Failed to bind to uuid e3514235-4b06-11d1-ab04-00c04fc2dcd2 for
ncacn_ip_tcp:192.168.252.190[6009,seal,target_hostname=WIN2012R2-7.ad-
2....co.nz,abstract_syntax=e3514235-4b06-11d1-ab04-
00c04fc2dcd2/0x00000004,localaddress=192.168.252.1]
NT_STATUS_IO_TIMEOUT
Join failed - cleaning up
Could not find machine account in secrets database: Failed to fetch
machine account password for AD-2 from both secrets.ldb (Could not find
entry to match filter: '(&(flatname=AD-2)(objectclass=primaryDomain))'
base: 'cn=Primary Domains': No such object: dsdb_search at
../source4/dsdb/common/util.c:4576) and from
/data/samba/samba4/prefix/private/secrets.tdb:
NT_STATUS_CANT_ACCESS_DOMAIN_INFO
Deleted CN=RUTH,OU=Domain Controllers,DC=ad-2,....,DC=co,DC=nz
Deleted CN=RUTH,CN=Servers,CN=Default-First-Site-
Name,CN=Sites,CN=Configuration,DC=ad-2,.....,DC=co,DC=nz
ERROR(runtime): uncaught exception - (-1073741643, '{Device Timeout}
The specified I/O operation on %hs was not completed before the time-
out period expired.')
  File "bin/python/samba/netcmd/__init__.py", line 176, in _run
    return self.run(*args, **kwargs)
  File "bin/python/samba/netcmd/domain.py", line 677, in run
    machinepass=machinepass, use_ntvfs=use_ntvfs,
dns_backend=dns_backend)
  File "bin/python/samba/join.py", line 1275, in join_DC
    ctx.do_join()
  File "bin/python/samba/join.py", line 1181, in do_join
    ctx.join_add_objects()
  File "bin/python/samba/join.py", line 618, in join_add_objects
    ctx.join_add_ntdsdsa()
  File "bin/python/samba/join.py", line 549, in join_add_ntdsdsa
    ctx.DsAddEntry([rec])
  File "bin/python/samba/join.py", line 444, in DsAddEntry
    ctx.drsuapi_connect()
  File "bin/python/samba/join.py", line 422, in drsuapi_connect
    ctx.drsuapi = drsuapi.drsuapi(binding_string, ctx.lp, ctx.creds)

to SIGABRT with:

==15975== Invalid read of size 8
==15975==    at 0xCAEB4E9: gensec_update_cleanup (gensec.c:471)
==15975==    by 0x70F7F7F: tevent_req_cleanup (tevent_req.c:139)
==15975==    by 0x70F8263: tevent_req_received (tevent_req.c:253)
==15975==    by 0x70F7EB9: tevent_req_destructor (tevent_req.c:107)
==15975==    by 0x69578BD: _tc_free_internal (talloc.c:1078)
==15975==    by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
==15975==    by 0x6957AEE: _tc_free_internal (talloc.c:1104)
==15975==    by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
==15975==    by 0x6957AEE: _tc_free_internal (talloc.c:1104)
==15975==    by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
==15975==    by 0x6957AEE: _tc_free_internal (talloc.c:1104)
==15975==    by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
==15975==    by 0x6957AEE: _tc_free_internal (talloc.c:1104)
==15975==    by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
==15975==    by 0x6957AEE: _tc_free_internal (talloc.c:1104)
==15975==    by 0x6957D5A: _talloc_free_internal (talloc.c:1174)
==15975==    by 0x6959022: _talloc_free (talloc.c:1716)
==15975==    by 0xA036CC7: dcerpc_pipe_connect_b_recv
(dcerpc_connect.c:1116)
==15975==    by 0xA036F78: continue_pipe_connect_b
(dcerpc_connect.c:1207)
==15975==    by 0xD6EF182: composite_error (composite.c:114)
==15975==    by 0xA036928: dcerpc_connect_timeout_handler
(dcerpc_connect.c:1009)
==15975==    by 0x70FE311: tevent_common_loop_timer_delay
(tevent_timed.c:369)
==15975==    by 0x70FFE24: epoll_event_loop (tevent_epoll.c:659)
==15975==    by 0x7100699: epoll_event_loop_once (tevent_epoll.c:930)
==15975==    by 0x70FD395: std_event_loop_once (tevent_standard.c:114)
==15975==    by 0x70F61C5: _tevent_loop_once (tevent.c:721)
==15975==    by 0xD91760F: smb_krb5_send_and_recv_func_int
(krb5_init_context.c:342)
==15975==    by 0xD9179A5: smb_krb5_send_and_recv_func
(krb5_init_context.c:431)
==15975==    by 0xF353620: krb5_sendto (send_to_kdc.c:391)
==15975==    by 0xF353D50: krb5_sendto_context (send_to_kdc.c:626)
==15975==    by 0xF3365F7: krb5_init_creds_get (init_creds_pw.c:1959)
==15975==    by 0xF3368D0: krb5_get_init_creds_password
(init_creds_pw.c:2038)
==15975==  Address 0x5e516b0 is 224 bytes inside a block of size 232
free'd
==15975==    at 0x4C2CDDB: free (vg_replace_malloc.c:530)
==15975==    by 0x6957CB6: _tc_free_internal (talloc.c:1148)
==15975==    by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
==15975==    by 0x6957AEE: _tc_free_internal (talloc.c:1104)
==15975==    by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
==15975==    by 0x6957AEE: _tc_free_internal (talloc.c:1104)
==15975==    by 0x6957D5A: _talloc_free_internal (talloc.c:1174)
==15975==    by 0x6959022: _talloc_free (talloc.c:1716)
==15975==    by 0xA036CC7: dcerpc_pipe_connect_b_recv
(dcerpc_connect.c:1116)
==15975==    by 0xA036F78: continue_pipe_connect_b
(dcerpc_connect.c:1207)
==15975==    by 0xD6EF182: composite_error (composite.c:114)
==15975==    by 0xA036928: dcerpc_connect_timeout_handler
(dcerpc_connect.c:1009)
==15975==    by 0x70FE311: tevent_common_loop_timer_delay
(tevent_timed.c:369)
==15975==    by 0x70FFE24: epoll_event_loop (tevent_epoll.c:659)
==15975==    by 0x7100699: epoll_event_loop_once (tevent_epoll.c:930)
==15975==    by 0x70FD395: std_event_loop_once (tevent_standard.c:114)
==15975==    by 0x70F61C5: _tevent_loop_once (tevent.c:721)
==15975==    by 0xD91760F: smb_krb5_send_and_recv_func_int
(krb5_init_context.c:342)
==15975==    by 0xD9179A5: smb_krb5_send_and_recv_func
(krb5_init_context.c:431)
==15975==    by 0xF353620: krb5_sendto (send_to_kdc.c:391)
==15975==    by 0xF353D50: krb5_sendto_context (send_to_kdc.c:626)
==15975==    by 0xF3365F7: krb5_init_creds_get (init_creds_pw.c:1959)
==15975==    by 0xF3368D0: krb5_get_init_creds_password
(init_creds_pw.c:2038)
==15975==    by 0xEAA7153: smb_krb5_kinit_password_ccache
(krb5_samba.c:1890)
==15975==    by 0xA258B53: kinit_to_ccache (kerberos_util.c:351)
==15975==    by 0xA2545D4: cli_credentials_get_named_ccache
(credentials_krb5.c:558)
==15975==    by 0xA254691: cli_credentials_get_ccache
(credentials_krb5.c:581)
==15975==    by 0xA254B17: cli_credentials_get_client_gss_creds
(credentials_krb5.c:703)
==15975==    by 0xCAEF276: gensec_gssapi_client_creds
(gensec_gssapi.c:316)
==15975==    by 0xCAEFA50: gensec_gssapi_update_internal
(gensec_gssapi.c:463)
==15975==    by 0xCAF17DD: gensec_gssapi_update_send
(gensec_gssapi.c:1053)
==15975==    by 0xCAEB200: gensec_update_ev (gensec.c:353)
==15975==  Block was alloc'd at
==15975==    at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==15975==    by 0x6956D20: __talloc_with_prefix (talloc.c:698)
==15975==    by 0x6956EB2: __talloc (talloc.c:739)
==15975==    by 0x69572C8: _talloc_named_const (talloc.c:896)
==15975==    by 0x695A69A: _talloc_zero (talloc.c:2341)
==15975==    by 0xCAECFCC: gensec_start (gensec_start.c:580)
==15975==    by 0xCAED36E: gensec_client_start (gensec_start.c:665)
==15975==    by 0xA02A06F: dcerpc_bind_auth_send (dcerpc_auth.c:339)
==15975==    by 0xA02D3C3: dcerpc_pipe_auth_send (dcerpc_util.c:698)
==15975==    by 0xA0367EC: continue_pipe_connect (dcerpc_connect.c:976)
==15975==    by 0xA0365CA: continue_pipe_connect_ncacn_ip_tcp
(dcerpc_connect.c:908)
==15975==    by 0xD6EF292: composite_done (composite.c:143)
==15975==    by 0xA03523F: continue_pipe_open_ncacn_ip_tcp
(dcerpc_connect.c:360)
==15975==    by 0xD6EF292: composite_done (composite.c:143)
==15975==    by 0xA02EEC7: continue_ip_open_socket (dcerpc_sock.c:273)
==15975==    by 0xD6EF292: composite_done (composite.c:143)
==15975==    by 0xA02E796: continue_socket_connect (dcerpc_sock.c:118)
==15975==    by 0xD6EF292: composite_done (composite.c:143)
==15975==    by 0xD6F1814: socket_connect_handler (connect.c:131)
==15975==    by 0x7100061: epoll_event_loop (tevent_epoll.c:728)
==15975==    by 0x7100699: epoll_event_loop_once (tevent_epoll.c:930)
==15975==    by 0x70FD395: std_event_loop_once (tevent_standard.c:114)
==15975==    by 0x70F61C5: _tevent_loop_once (tevent.c:721)
==15975==    by 0xD6EF020: composite_wait (composite.c:58)
==15975==    by 0xA036FE5: dcerpc_pipe_connect_recv
(dcerpc_connect.c:1226)
==15975==    by 0xA0370B5: dcerpc_pipe_connect (dcerpc_connect.c:1251)
==15975==    by 0x9A027E1: py_dcerpc_interface_init_helper
(pyrpc_util.c:217)
==15975==    by 0x20C4EF4A: interface_drsuapi_new (py_drsuapi.c:47159)
==15975==    by 0x1F5112: type_call.lto_priv.96 (typeobject.c:749)
==15975==    by 0x1EF672: PyObject_Call (abstract.c:2547)
==15975==    by 0x208E2E: do_call (ceval.c:4569)
==15975==    by 0x208E2E: call_function (ceval.c:4374)
==15975==    by 0x208E2E: PyEval_EvalFrameEx (ceval.c:2989)
==15975==    by 0x208C1E: fast_function (ceval.c:4437)
==15975==    by 0x208C1E: call_function (ceval.c:4372)
==15975==    by 0x208C1E: PyEval_EvalFrameEx (ceval.c:2989)
==15975== 


--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT  
https://catalyst.net.nz/services/samba





Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
Hi Andrew,

it seems I was to optimistic while removing the
inhibit_timeout_processing hack. There's a bit more to do
before we can remove it.

I have also a pending commit to fix a talloc_steal vs. talloc_reparent
and the removale of py_com.

Please review and push.

Thanks!
metze

> ==15975== Invalid read of size 8
> ==15975==    at 0xCAEB4E9: gensec_update_cleanup (gensec.c:471)
> ==15975==    by 0x70F7F7F: tevent_req_cleanup (tevent_req.c:139)
> ==15975==    by 0x70F8263: tevent_req_received (tevent_req.c:253)
> ==15975==    by 0x70F7EB9: tevent_req_destructor (tevent_req.c:107)
> ==15975==    by 0x69578BD: _tc_free_internal (talloc.c:1078)
> ==15975==    by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
> ==15975==    by 0x6957AEE: _tc_free_internal (talloc.c:1104)
> ==15975==    by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
> ==15975==    by 0x6957AEE: _tc_free_internal (talloc.c:1104)
> ==15975==    by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
> ==15975==    by 0x6957AEE: _tc_free_internal (talloc.c:1104)
> ==15975==    by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
> ==15975==    by 0x6957AEE: _tc_free_internal (talloc.c:1104)
> ==15975==    by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
> ==15975==    by 0x6957AEE: _tc_free_internal (talloc.c:1104)
> ==15975==    by 0x6957D5A: _talloc_free_internal (talloc.c:1174)
> ==15975==    by 0x6959022: _talloc_free (talloc.c:1716)
> ==15975==    by 0xA036CC7: dcerpc_pipe_connect_b_recv
> (dcerpc_connect.c:1116)
> ==15975==    by 0xA036F78: continue_pipe_connect_b
> (dcerpc_connect.c:1207)
> ==15975==    by 0xD6EF182: composite_error (composite.c:114)
> ==15975==    by 0xA036928: dcerpc_connect_timeout_handler
> (dcerpc_connect.c:1009)
> ==15975==    by 0x70FE311: tevent_common_loop_timer_delay
> (tevent_timed.c:369)
> ==15975==    by 0x70FFE24: epoll_event_loop (tevent_epoll.c:659)
> ==15975==    by 0x7100699: epoll_event_loop_once (tevent_epoll.c:930)
> ==15975==    by 0x70FD395: std_event_loop_once (tevent_standard.c:114)
> ==15975==    by 0x70F61C5: _tevent_loop_once (tevent.c:721)
> ==15975==    by 0xD91760F: smb_krb5_send_and_recv_func_int
> (krb5_init_context.c:342)
> ==15975==    by 0xD9179A5: smb_krb5_send_and_recv_func
> (krb5_init_context.c:431)
> ==15975==    by 0xF353620: krb5_sendto (send_to_kdc.c:391)
> ==15975==    by 0xF353D50: krb5_sendto_context (send_to_kdc.c:626)
> ==15975==    by 0xF3365F7: krb5_init_creds_get (init_creds_pw.c:1959)
> ==15975==    by 0xF3368D0: krb5_get_init_creds_password
> (init_creds_pw.c:2038)
> ==15975==  Address 0x5e516b0 is 224 bytes inside a block of size 232
> free'd
> ==15975==    at 0x4C2CDDB: free (vg_replace_malloc.c:530)
> ==15975==    by 0x6957CB6: _tc_free_internal (talloc.c:1148)
> ==15975==    by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
> ==15975==    by 0x6957AEE: _tc_free_internal (talloc.c:1104)
> ==15975==    by 0x6958BF8: _tc_free_children_internal (talloc.c:1593)
> ==15975==    by 0x6957AEE: _tc_free_internal (talloc.c:1104)
> ==15975==    by 0x6957D5A: _talloc_free_internal (talloc.c:1174)
> ==15975==    by 0x6959022: _talloc_free (talloc.c:1716)
> ==15975==    by 0xA036CC7: dcerpc_pipe_connect_b_recv
> (dcerpc_connect.c:1116)
> ==15975==    by 0xA036F78: continue_pipe_connect_b
> (dcerpc_connect.c:1207)
> ==15975==    by 0xD6EF182: composite_error (composite.c:114)
> ==15975==    by 0xA036928: dcerpc_connect_timeout_handler
> (dcerpc_connect.c:1009)
> ==15975==    by 0x70FE311: tevent_common_loop_timer_delay
> (tevent_timed.c:369)
> ==15975==    by 0x70FFE24: epoll_event_loop (tevent_epoll.c:659)
> ==15975==    by 0x7100699: epoll_event_loop_once (tevent_epoll.c:930)
> ==15975==    by 0x70FD395: std_event_loop_once (tevent_standard.c:114)
> ==15975==    by 0x70F61C5: _tevent_loop_once (tevent.c:721)
> ==15975==    by 0xD91760F: smb_krb5_send_and_recv_func_int
> (krb5_init_context.c:342)
> ==15975==    by 0xD9179A5: smb_krb5_send_and_recv_func
> (krb5_init_context.c:431)
> ==15975==    by 0xF353620: krb5_sendto (send_to_kdc.c:391)
> ==15975==    by 0xF353D50: krb5_sendto_context (send_to_kdc.c:626)
> ==15975==    by 0xF3365F7: krb5_init_creds_get (init_creds_pw.c:1959)
> ==15975==    by 0xF3368D0: krb5_get_init_creds_password
> (init_creds_pw.c:2038)
> ==15975==    by 0xEAA7153: smb_krb5_kinit_password_ccache
> (krb5_samba.c:1890)
> ==15975==    by 0xA258B53: kinit_to_ccache (kerberos_util.c:351)
> ==15975==    by 0xA2545D4: cli_credentials_get_named_ccache
> (credentials_krb5.c:558)
> ==15975==    by 0xA254691: cli_credentials_get_ccache
> (credentials_krb5.c:581)
> ==15975==    by 0xA254B17: cli_credentials_get_client_gss_creds
> (credentials_krb5.c:703)
> ==15975==    by 0xCAEF276: gensec_gssapi_client_creds
> (gensec_gssapi.c:316)
> ==15975==    by 0xCAEFA50: gensec_gssapi_update_internal
> (gensec_gssapi.c:463)
> ==15975==    by 0xCAF17DD: gensec_gssapi_update_send
> (gensec_gssapi.c:1053)
> ==15975==    by 0xCAEB200: gensec_update_ev (gensec.c:353)
> ==15975==  Block was alloc'd at
> ==15975==    at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
> ==15975==    by 0x6956D20: __talloc_with_prefix (talloc.c:698)
> ==15975==    by 0x6956EB2: __talloc (talloc.c:739)
> ==15975==    by 0x69572C8: _talloc_named_const (talloc.c:896)
> ==15975==    by 0x695A69A: _talloc_zero (talloc.c:2341)
> ==15975==    by 0xCAECFCC: gensec_start (gensec_start.c:580)
> ==15975==    by 0xCAED36E: gensec_client_start (gensec_start.c:665)
> ==15975==    by 0xA02A06F: dcerpc_bind_auth_send (dcerpc_auth.c:339)
> ==15975==    by 0xA02D3C3: dcerpc_pipe_auth_send (dcerpc_util.c:698)
> ==15975==    by 0xA0367EC: continue_pipe_connect (dcerpc_connect.c:976)
> ==15975==    by 0xA0365CA: continue_pipe_connect_ncacn_ip_tcp
> (dcerpc_connect.c:908)
> ==15975==    by 0xD6EF292: composite_done (composite.c:143)
> ==15975==    by 0xA03523F: continue_pipe_open_ncacn_ip_tcp
> (dcerpc_connect.c:360)
> ==15975==    by 0xD6EF292: composite_done (composite.c:143)
> ==15975==    by 0xA02EEC7: continue_ip_open_socket (dcerpc_sock.c:273)
> ==15975==    by 0xD6EF292: composite_done (composite.c:143)
> ==15975==    by 0xA02E796: continue_socket_connect (dcerpc_sock.c:118)
> ==15975==    by 0xD6EF292: composite_done (composite.c:143)
> ==15975==    by 0xD6F1814: socket_connect_handler (connect.c:131)
> ==15975==    by 0x7100061: epoll_event_loop (tevent_epoll.c:728)
> ==15975==    by 0x7100699: epoll_event_loop_once (tevent_epoll.c:930)
> ==15975==    by 0x70FD395: std_event_loop_once (tevent_standard.c:114)
> ==15975==    by 0x70F61C5: _tevent_loop_once (tevent.c:721)
> ==15975==    by 0xD6EF020: composite_wait (composite.c:58)
> ==15975==    by 0xA036FE5: dcerpc_pipe_connect_recv
> (dcerpc_connect.c:1226)
> ==15975==    by 0xA0370B5: dcerpc_pipe_connect (dcerpc_connect.c:1251)
> ==15975==    by 0x9A027E1: py_dcerpc_interface_init_helper
> (pyrpc_util.c:217)
> ==15975==    by 0x20C4EF4A: interface_drsuapi_new (py_drsuapi.c:47159)
> ==15975==    by 0x1F5112: type_call.lto_priv.96 (typeobject.c:749)
> ==15975==    by 0x1EF672: PyObject_Call (abstract.c:2547)
> ==15975==    by 0x208E2E: do_call (ceval.c:4569)
> ==15975==    by 0x208E2E: call_function (ceval.c:4374)
> ==15975==    by 0x208E2E: PyEval_EvalFrameEx (ceval.c:2989)
> ==15975==    by 0x208C1E: fast_function (ceval.c:4437)
> ==15975==    by 0x208C1E: call_function (ceval.c:4372)
> ==15975==    by 0x208C1E: PyEval_EvalFrameEx (ceval.c:2989)
> ==15975==
>
>


tmp.diff.txt (11K) Download Attachment
signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
On Mon, 2017-05-29 at 09:55 +0200, Stefan Metzmacher wrote:

> Hi Andrew,
>
> it seems I was to optimistic while removing the
> inhibit_timeout_processing hack. There's a bit more to do
> before we can remove it.
>
> I have also a pending commit to fix a talloc_steal vs. talloc_reparent
> and the removale of py_com.
>
> Please review and push.
>
> Thanks!
> metze

Thank you so much for getting back to me so promptly with that.  I'll
test it against the failing configuration in the morning and push it.

Thanks!

Andrew Bartlett
--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
Hi,

here's the next chunk for the LDAP server.

Also available under:
https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-gensec-ok

It just passed a private autobuild.

Please review and push:-)

Thanks!
metze

Am 29.05.2017 um 10:40 schrieb Andrew Bartlett:

> On Mon, 2017-05-29 at 09:55 +0200, Stefan Metzmacher wrote:
>> Hi Andrew,
>>
>> it seems I was to optimistic while removing the
>> inhibit_timeout_processing hack. There's a bit more to do
>> before we can remove it.
>>
>> I have also a pending commit to fix a talloc_steal vs. talloc_reparent
>> and the removale of py_com.
>>
>> Please review and push.
>>
>> Thanks!
>> metze
>
> Thank you so much for getting back to me so promptly with that.  I'll
> test it against the failing configuration in the morning and push it.
>
> Thanks!
>
> Andrew Bartlett
>


gensec-async-part3-01.patches.txt (94K) Download Attachment
signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
On Wed, 2017-06-14 at 00:04 +0200, Stefan Metzmacher wrote:

> Hi,
>
> here's the next chunk for the LDAP server.
>
> Also available under:
> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/
> master3-gensec-ok
>
> It just passed a private autobuild.
>
> Please review and push:-)
>
> Thanks!
> metze
Could we have some tests added, specifically that disrupt the
connection during the async processing?  I'm thinking of something that
might do part of the handshake, ideally against a trusted domain (but I
realise that isn't finished yet) and then drop the socket?

In the meantime I will look over the patches, and I thank you for your
continued attention to this area.

Thanks,

Andrew Bartlett
--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT  
https://catalyst.net.nz/services/samba




signature.asc (879 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
Am 14.06.2017 um 01:09 schrieb Andrew Bartlett:

> On Wed, 2017-06-14 at 00:04 +0200, Stefan Metzmacher wrote:
>> Hi,
>>
>> here's the next chunk for the LDAP server.
>>
>> Also available under:
>> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/
>> master3-gensec-ok
>>
>> It just passed a private autobuild.
>>
>> Please review and push:-)
>>
>> Thanks!
>> metze
>
> Could we have some tests added, specifically that disrupt the
> connection during the async processing?  I'm thinking of something that
> might do part of the handshake, ideally against a trusted domain (but I
> realise that isn't finished yet) and then drop the socket?
I don't see an easy way to write a test for it.

But if you have a close look at the
"s4:ldap_server: add call->wait_send/recv infrastructure" commit:
https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=6172e59eba6be35a8cdc3dc436ca166f1aeeefaa

You'll see that ldapsrv_call_read_next() (and also
ldapsrv_call_writev_start()) is deferred until the
waiting is over, which means we won't detect a connection drop
until we write or read from the socket.

So the outside behavior of the connection is still the same,
it's blocked during a bind call. The difference is that
other connections are no longer also blocked.

> In the meantime I will look over the patches, and I thank you for your
> continued attention to this area.

Thanks!
metze



signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
On Wed, 2017-06-14 at 02:04 +0200, Stefan Metzmacher wrote:

> Am 14.06.2017 um 01:09 schrieb Andrew Bartlett:
> > On Wed, 2017-06-14 at 00:04 +0200, Stefan Metzmacher wrote:
> > > Hi,
> > >
> > > here's the next chunk for the LDAP server.
> > >
> > > Also available under:
> > > https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/he
> > > ads/
> > > master3-gensec-ok
> > >
> > > It just passed a private autobuild.
> > >
> > > Please review and push:-)
> > >
> > > Thanks!
> > > metze
> >
> > Could we have some tests added, specifically that disrupt the
> > connection during the async processing?  I'm thinking of something
> > that
> > might do part of the handshake, ideally against a trusted domain
> > (but I
> > realise that isn't finished yet) and then drop the socket?
>
> I don't see an easy way to write a test for it.
OK.

> But if you have a close look at the
> "s4:ldap_server: add call->wait_send/recv infrastructure" commit:
> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=6172e59eb
> a6be35a8cdc3dc436ca166f1aeeefaa
>
> You'll see that ldapsrv_call_read_next() (and also
> ldapsrv_call_writev_start()) is deferred until the
> waiting is over, which means we won't detect a connection drop
> until we write or read from the socket.
>
> So the outside behavior of the connection is still the same,
> it's blocked during a bind call. The difference is that
> other connections are no longer also blocked.
Thanks.

> > In the meantime I will look over the patches, and I thank you for
> > your
> > continued attention to this area.
>
> Thanks!
> metze

I've had a first look over the patches, and I'll look over again.
Absent any major concerns I plan to get this into master in the next
day or so.

Thanks,

Andrew Bartlett

--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT  
https://catalyst.net.nz/services/samba




signature.asc (879 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
Hi,

just for the record:
https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-gensec

Contains an async version of spnego.c, it already passed:
make -j test FAIL_IMMEDIATELY=1 SOCKET_WRAPPER_KEEP_PCAP=1 TESTS="spnego"

I'm running private autobuilds now...

Then I just need to put the 128 commits into some
more useful once:-)

metze

Am 14.06.2017 um 02:42 schrieb Andrew Bartlett:

> On Wed, 2017-06-14 at 02:04 +0200, Stefan Metzmacher wrote:
>> Am 14.06.2017 um 01:09 schrieb Andrew Bartlett:
>>> On Wed, 2017-06-14 at 00:04 +0200, Stefan Metzmacher wrote:
>>>> Hi,
>>>>
>>>> here's the next chunk for the LDAP server.
>>>>
>>>> Also available under:
>>>> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/he
>>>> ads/
>>>> master3-gensec-ok
>>>>
>>>> It just passed a private autobuild.
>>>>
>>>> Please review and push:-)
>>>>
>>>> Thanks!
>>>> metze
>>>
>>> Could we have some tests added, specifically that disrupt the
>>> connection during the async processing?  I'm thinking of something
>>> that
>>> might do part of the handshake, ideally against a trusted domain
>>> (but I
>>> realise that isn't finished yet) and then drop the socket?
>>
>> I don't see an easy way to write a test for it.
>
> OK.
>
>> But if you have a close look at the
>> "s4:ldap_server: add call->wait_send/recv infrastructure" commit:
>> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=6172e59eb
>> a6be35a8cdc3dc436ca166f1aeeefaa
>>
>> You'll see that ldapsrv_call_read_next() (and also
>> ldapsrv_call_writev_start()) is deferred until the
>> waiting is over, which means we won't detect a connection drop
>> until we write or read from the socket.
>>
>> So the outside behavior of the connection is still the same,
>> it's blocked during a bind call. The difference is that
>> other connections are no longer also blocked.
>
> Thanks.
>
>>> In the meantime I will look over the patches, and I thank you for
>>> your
>>> continued attention to this area.
>>
>> Thanks!
>> metze
>
> I've had a first look over the patches, and I'll look over again.
> Absent any major concerns I plan to get this into master in the next
> day or so.
>
> Thanks,
>
> Andrew Bartlett
>


signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patches] The way to remove gensec_update_ev()

Samba - samba-technical mailing list
On Wed, 2017-06-14 at 17:39 +0200, Stefan Metzmacher wrote:

> Hi,
>
> just for the record:
> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/
> master3-gensec
>
> Contains an async version of spnego.c, it already passed:
> make -j test FAIL_IMMEDIATELY=1 SOCKET_WRAPPER_KEEP_PCAP=1
> TESTS="spnego"
>
> I'm running private autobuilds now...
>
> Then I just need to put the 128 commits into some
> more useful once:-)
Nice :-)

BTW I've pushed master3-gensec-ok to autobuild with my review.

Thanks,

Andrew Bartlett

--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT  
https://catalyst.net.nz/services/samba




signature.asc (879 bytes) Download Attachment
12