v4: Fixes for tls and sasl LDAP connections (#4275)

* Free old result before the new one is fetched, not after

* Zero timeout actually causes async connections to hang

* Handle fd discovery for sasl and start_tls the same as simple bind

* Interactive SASL callback needs to set lengths on replies

* Better SASL debug message - if we already have a result then we're continuing

* bind_ctx is parented off the connection - so signalling that the connection...

... is in error will do the free

* Add request to fr_ldap_referral_t

So we know what request to resume when the reply comes

* Use the correct freeing routine for berval

diff --git a/src/lib/ldap/base.h b/src/lib/ldap/base.h
index b9b23889bb5a40088ec0f65a1050439bc43b8058..e6955062a0d21980c09d1284c26d57a069f875a4 100644 (file)
--- a/src/lib/ldap/base.h
+++ b/src/lib/ldap/base.h
@@ -464,6 +464,7 @@ typedef struct fr_ldap_referral_s {
        char const              *identity;      //!< Bind identity for referral connection
        char const              *password;      //!< Bind password for referral connecition
        fr_ldap_thread_trunk_t  *ttrunk;        //!< Trunk this referral should use
+       request_t               *request;       //!< Request this referral relates to
 } fr_ldap_referral_t;
 
 /** Holds arguments for the async bind operation
@@ -804,7 +805,7 @@ int         fr_ldap_parse_url_extensions(LDAPControl **sss, size_t sss_len, char *exten
 /*
  *     referral.c - Handle LDAP referrals
  */
-fr_ldap_referral_t     *fr_ldap_referral_alloc(TALLOC_CTX *ctx);
+fr_ldap_referral_t     *fr_ldap_referral_alloc(TALLOC_CTX *ctx, request_t *request);
 
 int            fr_ldap_referral_follow(fr_ldap_thread_t *thread, request_t *request, fr_ldap_query_t *query);
 

diff --git a/src/lib/ldap/bind.c b/src/lib/ldap/bind.c
index 7fa7f9d8d3f0f0291af03bf3d695ba0843a56e5c..729d92d18984f28583c571a48f1dccf89a9b79bc 100644 (file)
--- a/src/lib/ldap/bind.c
+++ b/src/lib/ldap/bind.c
@@ -76,13 +76,13 @@ static void _ldap_bind_io_read(UNUSED fr_event_list_t *el, UNUSED int fd, UNUSED
                PERROR("Bind as \"%s\" to \"%s\" not permitted",
                       *bind_ctx->bind_dn ? bind_ctx->bind_dn : "(anonymous)", c->config->server);
                fr_ldap_state_error(c);         /* Restart the connection state machine */
-               break;
+               return;
 
        default:
                PERROR("Bind as \"%s\" to \"%s\" failed",
                       *bind_ctx->bind_dn ? bind_ctx->bind_dn : "(anonymous)", c->config->server);
                fr_ldap_state_error(c);         /* Restart the connection state machine */
-               break;
+               return;
        }
        talloc_free(bind_ctx);                  /* Also removes fd events */
 }

diff --git a/src/lib/ldap/referral.c b/src/lib/ldap/referral.c
index 2ea04cc03f2ec5aec100c10f63bf6c46551d9088..f235d144200ffe183cd921e2218897f80ef4280a 100644 (file)
--- a/src/lib/ldap/referral.c
+++ b/src/lib/ldap/referral.c
@@ -44,7 +44,7 @@ static int _fr_ldap_referral_free(fr_ldap_referral_t *referral)
  *     - a new referral structure on success
  *     - NULL on failure
  */
-fr_ldap_referral_t *fr_ldap_referral_alloc(TALLOC_CTX *ctx)
+fr_ldap_referral_t *fr_ldap_referral_alloc(TALLOC_CTX *ctx, request_t *request)
 {
        fr_ldap_referral_t      *referral;
 
@@ -53,6 +53,7 @@ fr_ldap_referral_t *fr_ldap_referral_alloc(TALLOC_CTX *ctx)
                PERROR("Failed to allocate LDAP referral container");
                return NULL;
        }
+       referral->request = request;
        talloc_set_destructor(referral, _fr_ldap_referral_free);
 
        return referral;
@@ -76,7 +77,7 @@ static void _ldap_referral_send(UNUSED fr_trunk_t *trunk, UNUSED fr_trunk_state_
         *      Enqueue referral query on active trunk connection
         */
        query->referral = referral;
-       query->treq = fr_trunk_request_alloc(referral->ttrunk->trunk, NULL);
+       query->treq = fr_trunk_request_alloc(referral->ttrunk->trunk, referral->request);
        fr_trunk_request_enqueue(&query->treq, referral->ttrunk->trunk, NULL, query, NULL);
 
        DEBUG3("Pending LDAP referral query queued on active trunk");
@@ -130,7 +131,7 @@ int fr_ldap_referral_follow(fr_ldap_thread_t *t, request_t *request, fr_ldap_que
                        continue;
                }
 
-               referral = fr_ldap_referral_alloc(query);
+               referral = fr_ldap_referral_alloc(query, request);
                if (!referral) continue;
 
                referral->query = query;

diff --git a/src/lib/ldap/sasl.c b/src/lib/ldap/sasl.c
index ad121c45dc9c0168e166ec334dfe8f7b5b4c8681..fbdee52dfa1a3aac8cfc0eb9da87b1e333fee1ff 100644 (file)
--- a/src/lib/ldap/sasl.c
+++ b/src/lib/ldap/sasl.c
@@ -88,18 +88,22 @@ static int _sasl_interact(UNUSED LDAP *handle, UNUSED unsigned flags, void *uctx
                switch (cb_p->id) {
                case SASL_CB_AUTHNAME:
                        cb_p->result = sasl_ctx->identity;
+                       cb_p->len = strlen(sasl_ctx->identity);
                        break;
 
                case SASL_CB_PASS:
                        cb_p->result = sasl_ctx->password;
+                       cb_p->len = strlen(sasl_ctx->password);
                        break;
 
                case SASL_CB_USER:
                        cb_p->result = sasl_ctx->proxy ? sasl_ctx->proxy : sasl_ctx->identity;
+                       cb_p->len = sasl_ctx->proxy ? strlen(sasl_ctx->proxy) : strlen(sasl_ctx->identity);
                        break;
 
                case SASL_CB_GETREALM:
                        cb_p->result = sasl_ctx->realm;
+                       cb_p->len = strlen(sasl_ctx->realm);
                        break;
 
                default:
@@ -123,6 +127,14 @@ static void _ldap_sasl_bind_io_read(fr_event_list_t *el, int fd, UNUSED int flag
        fr_ldap_connection_t    *c = sasl_ctx->c;
        fr_ldap_rcode_t         status;
 
+       /*
+        *      Free the old result (if there is one)
+        */
+       if (sasl_ctx->result) {
+               ldap_msgfree(sasl_ctx->result);
+               sasl_ctx->result = NULL;
+       }
+
        /*
         *      If LDAP parse result indicates there was an error
         *      then we're done.
@@ -136,14 +148,6 @@ static void _ldap_sasl_bind_io_read(fr_event_list_t *el, int fd, UNUSED int flag
                struct berval                   *srv_cred;
                int                             ret;
 
-               /*
-                *      Free the old result (if there is one)
-                */
-               if (sasl_ctx->result) {
-                       ldap_msgfree(sasl_ctx->result);
-                       sasl_ctx->result = NULL;
-               }
-
                ret = ldap_parse_sasl_bind_result(c->handle, sasl_ctx->result, &srv_cred, 0);
                if (ret != LDAP_SUCCESS) {
                        ERROR("SASL decode failed (bind failed): %s", ldap_err2string(ret));
@@ -154,7 +158,7 @@ static void _ldap_sasl_bind_io_read(fr_event_list_t *el, int fd, UNUSED int flag
                }
 
                DEBUG3("SASL response  : %pV", fr_box_strvalue_len(srv_cred->bv_val, srv_cred->bv_len));
-               ldap_memfree(srv_cred);
+               ber_bvfree(srv_cred);
 
                /*
                 *      If we need to continue, wait until the
@@ -200,13 +204,8 @@ static void _ldap_sasl_bind_io_write(fr_event_list_t *el, int fd, UNUSED int fla
                              NUM_ELEMENTS(our_clientctrls),
                              c, sasl_ctx->serverctrls, sasl_ctx->clientctrls);
 
-       DEBUG2("Starting SASL mech(s): %s", sasl_ctx->mechs);
+       DEBUG2("%s SASL mech(s): %s", (sasl_ctx->result == NULL ? "Starting" : "Continuing"), sasl_ctx->mechs);
 
-       /*
-        *      Set timeout to be 0.0, which is the magic
-        *      non-blocking value.
-        */
-       (void) ldap_set_option(c->handle, LDAP_OPT_NETWORK_TIMEOUT, &fr_time_delta_to_timeval(fr_time_delta_wrap(0)));
        ret = ldap_sasl_interactive_bind(c->handle, NULL, sasl_ctx->mechs,
                                         our_serverctrls, our_clientctrls,
                                         LDAP_SASL_AUTOMATIC,
@@ -243,6 +242,11 @@ static void _ldap_sasl_bind_io_write(fr_event_list_t *el, int fd, UNUSED int fla
         *      Want to read more SASL stuff...
         */
        case LDAP_SASL_BIND_IN_PROGRESS:
+               if (fd < 0) {
+                       ret = ldap_get_option(c->handle, LDAP_OPT_DESC, &fd);
+                       if ((ret != LDAP_OPT_SUCCESS) || (fd < 0)) goto error;
+               }
+               c->fd = fd;
                ret = fr_event_fd_insert(sasl_ctx, el, fd,
                                         _ldap_sasl_bind_io_read,
                                         NULL,
@@ -319,7 +323,11 @@ int fr_ldap_sasl_bind_async(fr_ldap_connection_t *c,
 
        el = c->conn->el;
 
-       if (ldap_get_option(c->handle, LDAP_OPT_DESC, &fd) == LDAP_SUCCESS) {
+       /*
+        *      ldap_get_option can return LDAP_SUCCESS even if the fd is not yet available
+        *      - hence the test for fd >= 0
+        */
+       if ((ldap_get_option(c->handle, LDAP_OPT_DESC, &fd) == LDAP_SUCCESS) && (fd >= 0)){
                int ret;
 
                ret = fr_event_fd_insert(sasl_ctx, el, fd,

diff --git a/src/lib/ldap/start_tls.c b/src/lib/ldap/start_tls.c
index 7b64b371204dbcdb1304222fb635a7314f7e6b4d..5c67f04e287e183188b57d5c71cc9b1ebdb4135a 100644 (file)
--- a/src/lib/ldap/start_tls.c
+++ b/src/lib/ldap/start_tls.c
@@ -161,11 +161,6 @@ static void _ldap_start_tls_io_write(fr_event_list_t *el, int fd, UNUSED int fla
                              NUM_ELEMENTS(our_clientctrls),
                              c, tls_ctx->serverctrls, tls_ctx->clientctrls);
 
-       /*
-        *      Set timeout to be 0.0, which is the magic
-        *      non-blocking value.
-        */
-       (void) ldap_set_option(c->handle, LDAP_OPT_NETWORK_TIMEOUT, &fr_time_delta_to_timeval(fr_time_delta_wrap(0)));
        ret = ldap_start_tls(c->handle, our_serverctrls, our_clientctrls, &tls_ctx->msgid);
        /*
         *      If the handle was not connected, this operation
@@ -193,6 +188,11 @@ static void _ldap_start_tls_io_write(fr_event_list_t *el, int fd, UNUSED int fla
                break;
 
        case LDAP_SUCCESS:
+               if (fd < 0) {
+                       ret = ldap_get_option(c->handle, LDAP_OPT_DESC, &fd);
+                       if ((ret != LDAP_OPT_SUCCESS) || (fd < 0)) goto error;
+               }
+               c->fd = fd;
                ret = fr_event_fd_insert(tls_ctx, el, fd,
                                         _ldap_start_tls_io_read,
                                         NULL,
@@ -234,7 +234,11 @@ int fr_ldap_start_tls_async(fr_ldap_connection_t *c, LDAPControl **serverctrls,
 
        el = c->conn->el;
 
-       if (ldap_get_option(c->handle, LDAP_OPT_DESC, &fd) == LDAP_SUCCESS) {
+       /*
+        *      ldap_get_option can return LDAP_SUCCESS even if the fd is not yet available
+        *      - hence the test for fd >= 0
+        */
+       if ((ldap_get_option(c->handle, LDAP_OPT_DESC, &fd) == LDAP_SUCCESS) && (fd >= 0)) {
                int ret;
 
                ret = fr_event_fd_insert(tls_ctx, el, fd,