>From 228bfdcc000e93be5908ef9ba28aee13385c258d Mon Sep 17 00:00:00 2001
From: Martin Nagy <mnagy@redhat.com>
Date: Tue, 20 Apr 2010 21:08:05 +0200
Subject: [PATCH] Make ldap simple bind asynchronous

We make openldap use asynchronous connections by setting the
LDAP_OPT_CONNECT_ASYNC option. When ldap_sasl_bind() is first called, it
can either return LDAP_X_CONNECTING without blocking, or succeed
immediately without blocking (e.g. when connecting to localhost). If
LDAP_X_CONNECTING is returned, the function has to be called again. We
create a callback that is called whenever openldap creates a new file
descriptor. This callback registers the file descriptor with tevent to
watch for its availability for reading and writing.

The problem is that this callback is called from ldap_sasl_bind() and
has no means to know the tevent_req that is associated with the bind
task. We solve this by partitioning simple_bind_send() into three
functions: simple_bind_send() creates the tevent_req and registers an
immediate wakeup to simple_bind_wakeup_bind() which invokes openldap and
can be called two times. So simple_bind_wakeup_bind() saves its
tevent_req to a place that is accessible by the openldap callback. Then,
when the file descriptor is ready for writing, the tevent fd callback
can notify the callback. The third function, simple_bind_wakeup_op_add(),
finishes the whole transaction by calling sdap_op_add().

This is a source of another problem: What if the simple bind tevent_req
is freed? The subreq will become a dangling pointer and will erroneously
be invoked by the tevent fd callback. To avoid this, we talloc the
fd_event_item from the wakeup tevent_req. Another problem caused by this
is that we need to make sure the tevent fd callback has a chance to be
called and may use talloc_steal() before the request is finished. This
of course causes yet another problem, since ldap_sasl_bind() can
occasionally return without blocking, so simple_bind_wakeup_bind() has
to know if it was called already or not. If not, it means that the
request is not stolen yet, and we have to wait for that.
---
 src/providers/ldap/sdap.h                  |    2 +
 src/providers/ldap/sdap_async.c            |   17 ++++-
 src/providers/ldap/sdap_async_connection.c |  121 ++++++++++++++++++++++------
 src/providers/ldap/sdap_async_private.h    |    7 ++
 src/providers/ldap/sdap_fd_events.c        |   78 ++++++++++++++++--
 5 files changed, 192 insertions(+), 33 deletions(-)

diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index aca9845..ad8e559 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -59,12 +59,14 @@ struct fd_event_item {
 
     int fd;
     struct tevent_fd *fde;
+    struct tevent_req *wakeup_req;
 };
 
 struct ldap_cb_data {
     struct sdap_handle *sh;
     struct tevent_context *ev;
     struct fd_event_item *fd_list;
+    struct tevent_req *fd_wakeup_req;
 };
 
 struct sdap_handle {
diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index a25c50b..14e6931 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -124,6 +124,21 @@ static void sdap_process_next_reply(struct tevent_context *ev,
                                     struct tevent_timer *te,
                                     struct timeval tv, void *pvt);
 
+void sdap_async_ldap_result(struct tevent_context *ev,
+                            struct tevent_fd *fde,
+                            uint16_t flags, void *pvt)
+{
+    struct ldap_cb_data *cb_data = talloc_get_type(pvt, struct ldap_cb_data);
+
+    if (flags & TEVENT_FD_WRITE) {
+        sdap_finish_bind(cb_data, fde);
+    }
+
+    if (flags & TEVENT_FD_READ) {
+        sdap_process_result(ev, cb_data->sh);
+    }
+}
+
 void sdap_ldap_result(struct tevent_context *ev, struct tevent_fd *fde,
                       uint16_t flags, void *pvt)
 {
@@ -137,7 +152,7 @@ static void sdap_ldap_next_result(struct tevent_context *ev,
     sdap_process_result(ev, pvt);
 }
 
-static void sdap_process_result(struct tevent_context *ev, void *pvt)
+void sdap_process_result(struct tevent_context *ev, void *pvt)
 {
     struct sdap_handle *sh = talloc_get_type(pvt, struct sdap_handle);
     struct timeval no_timeout = {0, 0};
diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c
index 33edf2f..5f1c012 100644
--- a/src/providers/ldap/sdap_async_connection.c
+++ b/src/providers/ldap/sdap_async_connection.c
@@ -252,11 +252,11 @@ struct simple_bind_state {
     struct sdap_msg *reply;
     struct sdap_ppolicy_data *ppolicy;
     int result;
+    bool sasl_bind_called;
+    int msgid;
 };
 
-static void simple_bind_done(struct sdap_op *op,
-                             struct sdap_msg *reply,
-                             int error, void *pvt);
+static void simple_bind_wakeup_bind(struct tevent_req *subreq);
 
 static struct tevent_req *simple_bind_send(TALLOC_CTX *memctx,
                                            struct tevent_context *ev,
@@ -265,16 +265,13 @@ static struct tevent_req *simple_bind_send(TALLOC_CTX *memctx,
                                            struct berval *pw)
 {
     struct tevent_req *req;
+    struct tevent_req *subreq;
     struct simple_bind_state *state;
-    int ret = EOK;
-    int msgid;
-    int ldap_err;
-    LDAPControl *request_controls[2];
 
     req = tevent_req_create(memctx, &state, struct simple_bind_state);
     if (!req) return NULL;
 
-    state->reply = talloc(state, struct sdap_msg);
+    state->reply = talloc_zero(state, struct sdap_msg);
     if (!state->reply) {
         talloc_zfree(req);
         return NULL;
@@ -284,6 +281,34 @@ static struct tevent_req *simple_bind_send(TALLOC_CTX *memctx,
     state->sh = sh;
     state->user_dn = user_dn;
     state->pw = pw;
+    state->sasl_bind_called = false;
+
+    subreq = tevent_wakeup_send(state, ev, tevent_timeval_current_ofs(0, 0));
+    if (subreq == NULL) {
+        talloc_zfree(req);
+        return NULL;
+    }
+    tevent_req_set_callback(subreq, simple_bind_wakeup_bind, req);
+
+    return req;
+}
+
+static void simple_bind_wakeup_op_add(struct tevent_req *subreq);
+
+static void simple_bind_wakeup_bind(struct tevent_req *subreq)
+{
+    struct simple_bind_state *state;
+    int ret;
+    int ldap_err;
+    struct sdap_handle *sh;
+    LDAPControl *request_controls[2];
+    struct tevent_req *req;
+
+    req = tevent_req_callback_data(subreq, struct tevent_req);
+    state = tevent_req_data(req, struct simple_bind_state);
+    sh = state->sh;
+
+    DEBUG(4, ("Executing simple bind as: %s\n", state->user_dn));
 
     ret = sss_ldap_control_create(LDAP_CONTROL_PASSWORDPOLICYREQUEST,
                                   0, NULL, 0, &request_controls[0]);
@@ -293,14 +318,24 @@ static struct tevent_req *simple_bind_send(TALLOC_CTX *memctx,
     }
     request_controls[1] = NULL;
 
-    DEBUG(4, ("Executing simple bind as: %s\n", state->user_dn));
-
-    ret = ldap_sasl_bind(state->sh->ldap, state->user_dn, LDAP_SASL_SIMPLE,
-                         state->pw, request_controls, NULL, &msgid);
+    set_fd_wakeup_req(sh, subreq);
+    ret = ldap_sasl_bind(sh->ldap, state->user_dn, LDAP_SASL_SIMPLE,
+                         state->pw, request_controls, NULL, &state->msgid);
     ldap_control_free(request_controls[0]);
-    if (ret == -1 || msgid == -1) {
-        ret = ldap_get_option(state->sh->ldap,
-                              LDAP_OPT_RESULT_CODE, &ldap_err);
+    set_fd_wakeup_req(sh, NULL);
+
+    if (ret == LDAP_X_CONNECTING) {
+        /* The ldap socket is not connected yet, we will let the ldap
+         * connect callback call us again. */
+        DEBUG(4, ("Simple bind is in progress\n"));
+        if (state->sasl_bind_called) {
+            DEBUG(1, ("Bug: ldap_sasl_bind() shouldn't have been called "
+                      "more than once\n"));
+        }
+        state->sasl_bind_called = true;
+        return;
+    } else if (ret == -1 || state->msgid == -1) {
+        ret = ldap_get_option(sh->ldap, LDAP_OPT_RESULT_CODE, &ldap_err);
         if (ret != LDAP_OPT_SUCCESS) {
             DEBUG(1, ("ldap_bind failed (couldn't get ldap error)\n"));
             ret = LDAP_LOCAL_ERROR;
@@ -311,31 +346,65 @@ static struct tevent_req *simple_bind_send(TALLOC_CTX *memctx,
         }
         goto fail;
     }
-    DEBUG(8, ("ldap simple bind sent, msgid = %d\n", msgid));
+    DEBUG(8, ("ldap simple bind sent, msgid = %d\n", state->msgid));
 
     if (!sh->connected) {
-        ret = sdap_set_connected(sh, ev);
+        ret = sdap_set_connected(sh, state->ev);
         if (ret) goto fail;
     }
 
-    /* FIXME: get timeouts from configuration, for now 5 secs. */
-    ret = sdap_op_add(state, ev, sh, msgid,
-                      simple_bind_done, req, 5, &state->op);
-    if (ret) {
-        DEBUG(1, ("Failed to set up operation!\n"));
-        goto fail;
+    /* Set our callback to the next step. */
+    tevent_req_set_callback(subreq, simple_bind_wakeup_op_add, req);
+
+    if (!state->sasl_bind_called) {
+        /* The fd callback was not called yet. This means that subreq is
+         * still used as a memory context for fd_event_item. We will give
+         * it a chance to talloc_steal() it and invoke the wakeup for the
+         * next step. */
+        return;
     }
 
-    return req;
+    /* The fd callback was called and we can safely complete the bind
+     * ourselves, we will invoke the next step. */
+    tevent_req_notify_callback(subreq);
+    return;
 
 fail:
+    tevent_wakeup_recv(subreq);
+    talloc_zfree(subreq);
+
     if (ret == LDAP_SERVER_DOWN) {
         tevent_req_error(req, ETIMEDOUT);
     } else {
         tevent_req_error(req, EIO);
     }
-    tevent_req_post(req, ev);
-    return req;
+}
+
+static void simple_bind_done(struct sdap_op *op,
+                             struct sdap_msg *reply,
+                             int error, void *pvt);
+
+static void simple_bind_wakeup_op_add(struct tevent_req *subreq)
+{
+    struct simple_bind_state *state;
+    struct tevent_req *req;
+    int ret;
+
+    req = tevent_req_callback_data(subreq, struct tevent_req);
+    state = tevent_req_data(req, struct simple_bind_state);
+
+    DEBUG(4, ("Waiting for response from the server\n"));
+    tevent_wakeup_recv(subreq);
+    talloc_zfree(subreq);
+
+    /* FIXME: get timeouts from configuration, for now 5 secs. */
+    ret = sdap_op_add(state, state->ev, state->sh, state->msgid,
+                      simple_bind_done, req, 5, &state->op);
+    if (ret) {
+        DEBUG(1, ("Failed to set up operation!\n"));
+        tevent_req_error(req, EIO);
+        tevent_req_post(req, state->ev);
+    }
 }
 
 static void simple_bind_done(struct sdap_op *op,
diff --git a/src/providers/ldap/sdap_async_private.h b/src/providers/ldap/sdap_async_private.h
index 75597c6..e8d0e54 100644
--- a/src/providers/ldap/sdap_async_private.h
+++ b/src/providers/ldap/sdap_async_private.h
@@ -30,6 +30,13 @@ struct sdap_handle *sdap_handle_create(TALLOC_CTX *memctx);
 
 void sdap_ldap_result(struct tevent_context *ev, struct tevent_fd *fde,
                       uint16_t flags, void *pvt);
+void set_fd_wakeup_req(struct sdap_handle *sh,
+                       struct tevent_req *wakeup_req);
+void sdap_async_ldap_result(struct tevent_context *ev,
+                            struct tevent_fd *fde,
+                            uint16_t flags, void *pvt);
+void sdap_finish_bind(struct ldap_cb_data *cb_data,
+                      struct tevent_fd *fde);
 
 int setup_ldap_connection_callbacks(struct sdap_handle *sh,
                                     struct tevent_context *ev);
diff --git a/src/providers/ldap/sdap_fd_events.c b/src/providers/ldap/sdap_fd_events.c
index 4b9a8b2..495a233 100644
--- a/src/providers/ldap/sdap_fd_events.c
+++ b/src/providers/ldap/sdap_fd_events.c
@@ -41,6 +41,49 @@ struct sdap_fd_events {
 };
 
 #ifdef HAVE_LDAP_CONNCB
+void sdap_finish_bind(struct ldap_cb_data *cb_data,
+                      struct tevent_fd *fde);
+
+# ifdef LDAP_OPT_CONNECT_ASYNC
+void sdap_finish_bind(struct ldap_cb_data *cb_data,
+                      struct tevent_fd *fde)
+{
+    struct fd_event_item *fd_event_item;
+
+    DEBUG(8, ("Trace: sh[%p], ldap[%p], fde[%p]\n",
+              cb_data->sh, cb_data->sh->ldap, fde));
+
+    DLIST_FOR_EACH(fd_event_item, cb_data->fd_list) {
+        if (fd_event_item->fde == fde) {
+            break;
+        }
+    }
+    if (fd_event_item != NULL && fd_event_item->wakeup_req != NULL) {
+        talloc_steal(cb_data, fd_event_item);
+        tevent_req_notify_callback(fd_event_item->wakeup_req);
+    } else if (fd_event_item == NULL) {
+        DEBUG(1, ("Bug: Couldn't find fd_event_item\n"));
+    }
+    tevent_fd_set_flags(fde, tevent_fd_get_flags(fde) & ~TEVENT_FD_WRITE);
+}
+
+void set_fd_wakeup_req(struct sdap_handle *sh,
+                       struct tevent_req *wakeup_req)
+{
+    struct ldap_cb_data *cb_data;
+
+    cb_data = talloc_get_type(sh->sdap_fd_events->conncb->lc_arg, struct ldap_cb_data);
+    cb_data->fd_wakeup_req = wakeup_req;
+}
+# else /* !LDAP_OPT_CONNECT_ASYNC */
+void sdap_finish_bind(struct ldap_cb_data *cb_data,
+                      struct tevent_fd *fde)
+{
+    (void)cb_data;
+    tevent_fd_set_flags(fde, tevent_fd_get_flags(fde) & ~TEVENT_FD_WRITE);
+}
+# endif /* LDAP_OPT_CONNECT_ASYNC */
+
 static int remove_connection_callback(TALLOC_CTX *mem_ctx)
 {
     int lret;
@@ -83,21 +126,35 @@ static int sdap_ldap_connect_callback_add(LDAP *ld, Sockbuf *sb,
     DEBUG(9, ("New LDAP connection to [%s] with fd [%d].\n",
               ldap_url_desc2str(srv), ber_fd));
 
-    fd_event_item = talloc_zero(cb_data, struct fd_event_item);
+    /* We allocate the fd_event_item on a provided memory context.
+     * After the connection is established, we will talloc_steal()
+     * it to cb_data. */
+    if (cb_data->fd_wakeup_req) {
+        fd_event_item = talloc_zero(cb_data->fd_wakeup_req,
+                                    struct fd_event_item);
+    } else {
+        fd_event_item = talloc_zero(cb_data, struct fd_event_item);
+    }
     if (fd_event_item == NULL) {
         DEBUG(1, ("talloc failed.\n"));
         return ENOMEM;
     }
 
     fd_event_item->fde = tevent_add_fd(cb_data->ev, fd_event_item, ber_fd,
+#ifdef LDAP_OPT_CONNECT_ASYNC
+                                       TEVENT_FD_READ | TEVENT_FD_WRITE,
+                                       sdap_async_ldap_result,
+#else
                                        TEVENT_FD_READ, sdap_ldap_result,
-                                       cb_data->sh);
+#endif
+                                       cb_data);
     if (fd_event_item->fde == NULL) {
         DEBUG(1, ("tevent_add_fd failed.\n"));
         talloc_free(fd_event_item);
         return ENOMEM;
     }
     fd_event_item->fd = ber_fd;
+    fd_event_item->wakeup_req = cb_data->fd_wakeup_req;
 
     DLIST_ADD(cb_data->fd_list, fd_event_item);
 
@@ -140,7 +197,7 @@ static void sdap_ldap_connect_callback_del(LDAP *ld, Sockbuf *sb,
     return;
 }
 
-#else
+#else /* !HAVE_LDAP_CONNCB */
 
 static int get_fd_from_ldap(LDAP *ldap, int *fd)
 {
@@ -190,10 +247,19 @@ static int sdap_install_ldap_callbacks(struct sdap_handle *sh,
               sh, (int)sh->connected, sh->ops, sh->sdap_fd_events->fde,
               sh->ldap));
 
+#ifdef LDAP_OPT_CONNECT_ASYNC
+    ret = ldap_set_option(sh->ldap, LDAP_OPT_CONNECT_ASYNC, LDAP_OPT_ON);
+    if (ret != LDAP_OPT_SUCCESS) {
+        DEBUG(1, ("Failed to set connection as asynchronous\n"));
+        talloc_zfree(sh->sdap_fd_events);
+        return EIO;
+    }
+#endif
+
     return EOK;
 }
 
-#endif
+#endif /* HAVE_LDAP_CONNCB */
 
 
 errno_t setup_ldap_connection_callbacks(struct sdap_handle *sh,
@@ -248,10 +314,10 @@ errno_t setup_ldap_connection_callbacks(struct sdap_handle *sh,
 fail:
     talloc_zfree(sh->sdap_fd_events);
     return ret;
-#else
+#else /* !HAVE_LDAP_CONNCB */
     DEBUG(9, ("LDAP connection callbacks are not supported.\n"));
     return EOK;
-#endif
+#endif /* HAVE_LDAP_CONNCB */
 }
 
 errno_t sdap_set_connected(struct sdap_handle *sh, struct tevent_context *ev)
-- 
1.6.2.5

