From: Nick Porter Date: Mon, 18 Oct 2021 13:10:09 +0000 (+0100) Subject: v4: Fixes for tls and sasl LDAP connections (#4275) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bab1ea092883e98a2b13aa83736585c4cd1759d9;p=thirdparty%2Ffreeradius-server.git 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 b9b23889bb5..e6955062a0d 100644 --- 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 7fa7f9d8d3f..729d92d1898 100644 --- 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 2ea04cc03f2..f235d144200 100644 --- 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 ad121c45dc9..fbdee52dfa1 100644 --- 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 7b64b371204..5c67f04e287 100644 --- 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,