tevent_abort_nesting crash in idmap_ad

[Jeremy Allison]

On Fri, Jun 24, 2016 at 10:46:17AM -0700, Jeremy Allison wrote:
> 
> Ah ! Talking here at the event to Alexander.
> 
> Inside tldap_gensec_bind_got_mechs() we could
> create a new sub-event loop:
> 
> new_ev_ctx = samba_tevent_context_init(frame);
> 
> Then run the gensec_update_send() calls
> under this new event context - rather than
> running it under the state->ev context.
> 
> So we'd have 2 event contexts running - one
> which we use to call gensec_update_send(),
> and one where we progress through the
> tldap_sasl_bind_send() -> tldap_gensec_bind_done()
> -> tldap_sasl_bind_recv() calls.
> 
> We have to alternate the calls to tevent_req_poll()
> to ensure we never run both event contexts at
> the same time, but wait for each completion event
> alternatively.
> 
> As this is a completely blocking operation
> anyway, this isn't going to make the code
> performace worse, it's going to make the
> internals more complex, but even with this
> added complexity it's simpler than having
> a nested event loop :-) :-).

Digging more.... This won't work - we'll
still get a nested loop, just under the
new event context.

This is due to the fact that the gensec spnego
backend isn't really synchronous (internally
it doesn't have a _send() or _recv() method)
- it fakes up asychronicity using immediate events - which
is why we're ending up with nested loops.

OK. Try #2... :-).

Inside tldap_gensec_bind_got_mechs() we
currently do:

gensec_update_send()
tevent_req_set_callback(..., tldap_gensec_update_done)

to nicely break the gensec updates up into
async operations.

However, we're under a blocking operation
anyway - tldap_gensec_bind().

So an easy fix would be to change the
gensec_update_send()/tevent_req_set_callback()
pairs for the gensec code inside the
source3/lib/tldap_gensec_bind.c code
to be synchronous.

Essentially we change the code to call
the synchronous:

gensec_update()

followed by a direct call to a slightly modified:

tldap_gensec_update_done()

rather than using the current async
mechanism - the spnego gensec backend
isn't async anyway.

As this is removing asynchronicity
this should be a manageable change.

I can have a go at doing this if you'd
like, or I'm happy to review such a
change if anyone else gets to it.

Volker, Metze, Ralph - is this a plan ?

It does allow us to keep the tldap code,
which is a *significant* improvement
on what we used to have, so I'd really
like to make this work.

Cheers,

	Jeremy.