autofs-5.1.6 - fix ldap sasl reconnect problem From: Ian Kent When performing an ldap sasl connection a two step initialisation was being done in an attempt to partially reuse existing connection setup. But if a network connectivity problem occurs the connection can end up only half initialized and recovery after connectivity is restored fails. So get rid of the two step initialization, as it's benefit was at best questionable, so that connection attempts either succeed or completely fail. This leaves the connection completely uninitialized if there's a network conectivity problem, ready for a new connection attempt. Signed-off-by: Ian Kent --- CHANGELOG | 1 include/lookup_ldap.h | 1 modules/cyrus-sasl.c | 131 +++++++++++++++++++++++++------------------------ 3 files changed, 68 insertions(+), 65 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 12f1e2b4..be221af2 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,6 +15,7 @@ xx/xx/2020 autofs-5.1.7 - configure.in: Remove unneeded second call to PKG_PROG_PKG_CONFIG. - configure.in: Do not append parentheses to PKG_PROG_PKG_CONFIG. - Use PKG_CHECK_MODULES to detect the libxml2 library. +- fix ldap sasl reconnect problem. 07/10/2019 autofs-5.1.6 - support strictexpire mount option. diff --git a/include/lookup_ldap.h b/include/lookup_ldap.h index 3a7aba78..3a107782 100644 --- a/include/lookup_ldap.h +++ b/include/lookup_ldap.h @@ -87,7 +87,6 @@ struct lookup_context { char *secret; char *client_princ; char *client_cc; - int kinit_done; int kinit_successful; #ifdef WITH_SASL /* Kerberos */ diff --git a/modules/cyrus-sasl.c b/modules/cyrus-sasl.c index cf596b8b..ae046e01 100644 --- a/modules/cyrus-sasl.c +++ b/modules/cyrus-sasl.c @@ -396,9 +396,9 @@ do_sasl_bind(unsigned logopt, LDAP *ld, sasl_conn_t *conn, const char **clientou * cache, add the TGT to that cache, and set the environment variable so * that the sasl/krb5 libraries can find our credentials. * - * Returns 0 upon success. ctxt->kinit_done and ctxt->kinit_successful - * are set for cleanup purposes. The krb5 context and ccache entries in - * the lookup_context are also filled in. + * Returns 0 upon success. ctxt->kinit_successful is set for cleanup + * purposes. The krb5 context and ccache entries in the lookup_context + * are also filled in. * * Upon failure, -1 is returned. */ @@ -412,9 +412,16 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt) const char *realm_name; int status, realm_length; - if (ctxt->kinit_done) + status = pthread_mutex_lock(&krb5cc_mutex); + if (status) + fatal(status); + + if (ctxt->kinit_successful) { + status = pthread_mutex_unlock(&krb5cc_mutex); + if (status) + fatal(status); return 0; - ctxt->kinit_done = 1; + } debug(logopt, "initializing kerberos ticket: client principal %s", @@ -423,15 +430,14 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt) ret = krb5_init_context(&ctxt->krb5ctxt); if (ret) { error(logopt, "krb5_init_context failed with %d", ret); - return -1; + goto out_unlock; } ret = krb5_cc_resolve(ctxt->krb5ctxt, krb5ccval, &ctxt->krb5_ccache); if (ret) { error(logopt, "krb5_cc_resolve failed with error %d", ret); - krb5_free_context(ctxt->krb5ctxt); - return -1; + goto out_free_context; } if (ctxt->client_princ) { @@ -515,19 +521,11 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt) goto out_cleanup_unparse; } - status = pthread_mutex_lock(&krb5cc_mutex); - if (status) - fatal(status); - if (krb5cc_in_use++ == 0) /* tell the cache what the default principal is */ ret = krb5_cc_initialize(ctxt->krb5ctxt, ctxt->krb5_ccache, krb5_client_princ); - status = pthread_mutex_unlock(&krb5cc_mutex); - if (status) - fatal(status); - if (ret) { error(logopt, "krb5_cc_initialize failed with error %d", ret); @@ -550,6 +548,10 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt) } ctxt->kinit_successful = 1; + status = pthread_mutex_unlock(&krb5cc_mutex); + if (status) + fatal(status); + debug(logopt, "Kerberos authentication was successful!"); krb5_free_unparsed_name(ctxt->krb5ctxt, tgs_name); @@ -568,10 +570,6 @@ out_cleanup_unparse: out_cleanup_client_princ: krb5_free_principal(ctxt->krb5ctxt, krb5_client_princ); out_cleanup_cc: - status = pthread_mutex_lock(&krb5cc_mutex); - if (status) - fatal(status); - if (krb5cc_in_use) ret = krb5_cc_close(ctxt->krb5ctxt, ctxt->krb5_ccache); else @@ -579,22 +577,21 @@ out_cleanup_cc: if (ret) warn(logopt, "krb5_cc_destroy failed with non-fatal error %d", ret); - +out_free_context: + krb5_free_context(ctxt->krb5ctxt); +out_unlock: status = pthread_mutex_unlock(&krb5cc_mutex); if (status) fatal(status); - - krb5_free_context(ctxt->krb5ctxt); - return -1; } /* * Check a client given external credential cache. * - * Returns 0 upon success. ctxt->kinit_done and ctxt->kinit_successful - * are set for cleanup purposes. The krb5 context and ccache entries in - * the lookup_context are also filled in. + * Returns 0 upon success. ctxt->kinit_successful is set for cleanup + * purposes. The krb5 context and ccache entries in the lookup_context + * are also filled in. * * Upon failure, -1 is returned. */ @@ -605,10 +602,18 @@ sasl_do_kinit_ext_cc(unsigned logopt, struct lookup_context *ctxt) krb5_principal krb5_client_princ; krb5_error_code ret; char *cc_princ, *client_princ; + int status; + + status = pthread_mutex_lock(&krb5cc_mutex); + if (status) + fatal(status); - if (ctxt->kinit_done) + if (ctxt->kinit_successful) { + status = pthread_mutex_unlock(&krb5cc_mutex); + if (status) + fatal(status); return 0; - ctxt->kinit_done = 1; + } debug(logopt, "using external credential cache for auth: client principal %s", @@ -617,33 +622,26 @@ sasl_do_kinit_ext_cc(unsigned logopt, struct lookup_context *ctxt) ret = krb5_init_context(&ctxt->krb5ctxt); if (ret) { error(logopt, "krb5_init_context failed with %d", ret); - return -1; + goto out_unlock; } ret = krb5_cc_resolve(ctxt->krb5ctxt, ctxt->client_cc, &ctxt->krb5_ccache); if (ret) { error(logopt, "krb5_cc_resolve failed with error %d", ret); - krb5_cc_close(ctxt->krb5ctxt, ctxt->krb5_ccache); - krb5_free_context(ctxt->krb5ctxt); - return -1; + goto out_cleanup_cc; } ret = krb5_cc_get_principal(ctxt->krb5ctxt, ctxt->krb5_ccache, &def_princ); if (ret) { error(logopt, "krb5_cc_get_principal failed with error %d", ret); - krb5_cc_close(ctxt->krb5ctxt, ctxt->krb5_ccache); - krb5_free_context(ctxt->krb5ctxt); - return -1; + goto out_cleanup_cc; } ret = krb5_unparse_name(ctxt->krb5ctxt, def_princ, &cc_princ); if (ret) { error(logopt, "krb5_unparse_name failed with error %d", ret); - krb5_free_principal(ctxt->krb5ctxt, def_princ); - krb5_cc_close(ctxt->krb5ctxt, ctxt->krb5_ccache); - krb5_free_context(ctxt->krb5ctxt); - return -1; + goto out_cleanup_def_princ; } debug(logopt, "external credential cache default principal %s", cc_princ); @@ -666,10 +664,8 @@ sasl_do_kinit_ext_cc(unsigned logopt, struct lookup_context *ctxt) error(logopt, "krb5_sname_to_principal failed for " "%s with error %d", default_client, ret); - krb5_free_principal(ctxt->krb5ctxt, def_princ); - krb5_cc_close(ctxt->krb5ctxt, ctxt->krb5_ccache); - krb5_free_context(ctxt->krb5ctxt); - return -1; + krb5_free_unparsed_name(ctxt->krb5ctxt, cc_princ); + goto out_cleanup_def_princ; } @@ -680,10 +676,8 @@ sasl_do_kinit_ext_cc(unsigned logopt, struct lookup_context *ctxt) "krb5_unparse_name failed with error %d", ret); krb5_free_principal(ctxt->krb5ctxt, krb5_client_princ); - krb5_free_principal(ctxt->krb5ctxt, def_princ); - krb5_cc_close(ctxt->krb5ctxt, ctxt->krb5_ccache); - krb5_free_context(ctxt->krb5ctxt); - return -1; + krb5_free_unparsed_name(ctxt->krb5ctxt, cc_princ); + goto out_cleanup_def_princ; } debug(logopt, @@ -710,10 +704,7 @@ sasl_do_kinit_ext_cc(unsigned logopt, struct lookup_context *ctxt) if (!ctxt->client_princ) krb5_free_unparsed_name(ctxt->krb5ctxt, client_princ); krb5_free_unparsed_name(ctxt->krb5ctxt, cc_princ); - krb5_free_principal(ctxt->krb5ctxt, def_princ); - krb5_cc_close(ctxt->krb5ctxt, ctxt->krb5_ccache); - krb5_free_context(ctxt->krb5ctxt); - return -1; + goto out_cleanup_def_princ; } if (!ctxt->client_princ) @@ -724,15 +715,24 @@ sasl_do_kinit_ext_cc(unsigned logopt, struct lookup_context *ctxt) /* Set the environment variable to point to the external cred cache */ if (setenv(krb5ccenv, ctxt->client_cc, 1) != 0) { error(logopt, "setenv failed with %d", errno); - krb5_cc_close(ctxt->krb5ctxt, ctxt->krb5_ccache); - krb5_free_context(ctxt->krb5ctxt); - return -1; + goto out_cleanup_cc; } ctxt->kinit_successful = 1; debug(logopt, "Kerberos authentication was successful!"); return 0; + +out_cleanup_def_princ: + krb5_free_principal(ctxt->krb5ctxt, def_princ); +out_cleanup_cc: + krb5_cc_close(ctxt->krb5ctxt, ctxt->krb5_ccache); + krb5_free_context(ctxt->krb5ctxt); +out_unlock: + status = pthread_mutex_unlock(&krb5cc_mutex); + if (status) + fatal(status); + return -1; } /* @@ -974,11 +974,19 @@ void autofs_sasl_dispose(struct ldap_conn *conn, struct lookup_context *ctxt) { int status, ret; + status = pthread_mutex_lock(&krb5cc_mutex); + if (status) + fatal(status); + if (ctxt->sasl_mech && !strncmp(ctxt->sasl_mech, "EXTERNAL", 8)) { if (conn && conn->ldap) { ldap_unbind_s(conn->ldap); conn->ldap = NULL; + ctxt->kinit_successful = 0; } + status = pthread_mutex_unlock(&krb5cc_mutex); + if (status) + fatal(status); return; } @@ -988,10 +996,6 @@ void autofs_sasl_dispose(struct ldap_conn *conn, struct lookup_context *ctxt) } if (ctxt->kinit_successful) { - status = pthread_mutex_lock(&krb5cc_mutex); - if (status) - fatal(status); - if (--krb5cc_in_use || ctxt->client_cc) ret = krb5_cc_close(ctxt->krb5ctxt, ctxt->krb5_ccache); else @@ -1000,19 +1004,18 @@ void autofs_sasl_dispose(struct ldap_conn *conn, struct lookup_context *ctxt) logmsg("krb5_cc_destroy failed with non-fatal error %d", ret); - status = pthread_mutex_unlock(&krb5cc_mutex); - if (status) - fatal(status); - krb5_free_context(ctxt->krb5ctxt); if (unsetenv(krb5ccenv) != 0) logerr("unsetenv failed with error %d", errno); ctxt->krb5ctxt = NULL; ctxt->krb5_ccache = NULL; - ctxt->kinit_done = 0; ctxt->kinit_successful = 0; } + + status = pthread_mutex_unlock(&krb5cc_mutex); + if (status) + fatal(status); } static void *sasl_mutex_new(void)