Addresses #2053 and #2094 Check old ccaches only if needed, always precreate containing directory if we are goign to proceed with authentication and always fallback to old algorithm to check user presence on the system if systemd-login fails or returns negative result.
(Patches as agreed on IRC after several discussions) All tested and apparently working. Simo. -- Simo Sorce * Red Hat, Inc * New York
>From cc8b2c83308e0805684a7c24ee53c7bfceaab55c Mon Sep 17 00:00:00 2001 From: Simo Sorce <s...@redhat.com> Date: Thu, 19 Sep 2013 16:32:23 -0400 Subject: [PATCH 1/4] krb5: Be more lenient on failures for old ccache Fix a check for an error return code that can be returned when the ccache is not found. Even in case of other errors still do not fail authentication but allow it to proceed using a new ccache file if necessary. Related: https://fedorahosted.org/sssd/ticket/2053 --- src/providers/krb5/krb5_auth.c | 2 +- src/providers/krb5/krb5_utils.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c index fe3e6aba74d9bc22cbd313c6eaa6a085ba32431f..edb47c90be281a6109a84cd46c643408c7b12a77 100644 --- a/src/providers/krb5/krb5_auth.c +++ b/src/providers/krb5/krb5_auth.c @@ -607,7 +607,7 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx, } else if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, ("check_if_ccache_file_is_used failed.\n")); - goto done; + ccache_file = NULL; } } else { kr->active_ccache = false; diff --git a/src/providers/krb5/krb5_utils.c b/src/providers/krb5/krb5_utils.c index 8d10a8346b89ac0ffd86c65d0b639150200a282d..bd5a38fd3c34a6573acb76dbaa31d691326e222a 100644 --- a/src/providers/krb5/krb5_utils.c +++ b/src/providers/krb5/krb5_utils.c @@ -1057,7 +1057,7 @@ errno_t sss_krb5_cc_verify_ccache(const char *ccname, uid_t uid, gid_t gid, kerr = krb5_cc_retrieve_cred(cc->context, cc->ccache, KRB5_TC_MATCH_TIMES, &mcred, &cred); if (kerr) { - if (kerr == KRB5_CC_NOTFOUND) { + if (kerr == KRB5_CC_NOTFOUND || KRB5_FCC_NOFILE) { DEBUG(SSSDBG_TRACE_INTERNAL, ("TGT not found or expired.\n")); ret = EINVAL; } else { -- 1.8.3.1
>From cd08cb5222195d91bb1e0ccd884f35085bde4b73 Mon Sep 17 00:00:00 2001 From: Simo Sorce <s...@redhat.com> Date: Thu, 19 Sep 2013 18:06:34 -0400 Subject: [PATCH 2/4] krb5: Improve ccache name checks Consolidate all the code that decides what the ccache name will be in one function. Conditionalize checking for the old ccache only on the fact that the new and old name (if any) are actually the same. Resolves: https://fedorahosted.org/sssd/ticket/2053 --- src/providers/krb5/krb5_auth.c | 117 +++++++++++++++++++++-------------------- 1 file changed, 59 insertions(+), 58 deletions(-) diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c index edb47c90be281a6109a84cd46c643408c7b12a77..6df34a8aa0ae4a40ad43eb030a49570a7e860ab8 100644 --- a/src/providers/krb5/krb5_auth.c +++ b/src/providers/krb5/krb5_auth.c @@ -292,6 +292,7 @@ static errno_t krb5_auth_prepare_ccache_name(struct krb5child_req *kr, struct be_ctx *be_ctx) { const char *ccname_template; + const char *realm; bool private_path = false; errno_t ret; @@ -299,38 +300,66 @@ static errno_t krb5_auth_prepare_ccache_name(struct krb5child_req *kr, kr->is_offline = be_is_offline(be_ctx); } - /* The ccache file should be (re)created if one of the following conditions - * is true: - * - it doesn't exist (kr->ccname == NULL) - * - the backend is online and the current ccache file is not used, i.e - * the related user is currently not logged in and it is not a renewal - * request - * (!kr->is_offline && !kr->active_ccache && kr->pd->cmd != SSS_CMD_RENEW) - * - the backend is offline and the current cache file not used and - * it does not contain a valid tgt - * (kr->is_offline && !kr->active_ccache && !kr->valid_tgt) - */ - if (kr->ccname == NULL || - (kr->is_offline && !kr->active_ccache && !kr->valid_tgt) || - (!kr->is_offline && !kr->active_ccache && kr->pd->cmd != SSS_CMD_RENEW)) { - DEBUG(9, ("Recreating ccache file.\n")); - ccname_template = dp_opt_get_cstring(kr->krb5_ctx->opts, - KRB5_CCNAME_TMPL); - kr->ccname = expand_ccname_template(kr, kr, ccname_template, true, - be_ctx->domain->case_sensitive, - &private_path); - if (kr->ccname == NULL) { - DEBUG(1, ("expand_ccname_template failed.\n")); - return ENOMEM; - } + ccname_template = dp_opt_get_cstring(kr->krb5_ctx->opts, KRB5_CCNAME_TMPL); + kr->ccname = expand_ccname_template(kr, kr, ccname_template, true, + be_ctx->domain->case_sensitive, + &private_path); + if (kr->ccname == NULL) { + DEBUG(1, ("expand_ccname_template failed.\n")); + return ENOMEM; + } - ret = sss_krb5_precreate_ccache(kr->ccname, - kr->krb5_ctx->illegal_path_re, - kr->uid, kr->gid, private_path); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, ("ccache creation failed.\n")); - return ret; + if (kr->old_ccname) { + if (strcmp(kr->ccname, kr->old_ccname) != 0) { + /* it makes sense to test only if old and new ccache name are + * different, otherwise we are going to use the old one anyways. + * NOTE: this check properly treats randomized names as different + * because the old one has an actual random component while + * kr->ccname still has the template form so they will not match + */ + realm = dp_opt_get_cstring(kr->krb5_ctx->opts, KRB5_REALM); + ret = check_old_ccache(kr->old_ccname, kr, realm, + &kr->active_ccache, &kr->valid_tgt); + if (ret == ENOENT) { + DEBUG(SSSDBG_FUNC_DATA, + ("Ignoring ccache attribute [%s], because it doesn't" + "exist.\n", kr->old_ccname)); + kr->old_ccname = NULL; + } else if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + ("check_old_ccache failed with error number %d, " + "ignoring error.\n", ret)); } + } else { + /* IMPORTANT: unset old_ccname or it may trigger a removal of + * the 'old' ccache later, but this is not the 'old' one as it + * is identical to the new one + */ + kr->old_ccname = NULL; + } + } else { + kr->active_ccache = false; + kr->valid_tgt = false; + DEBUG(4, ("No old ccache file for user [%s] found.\n", kr->pd->user)); + } + if (kr->old_ccname && kr->active_ccache) { + /* ok the old ccache file name is valid and in use let's use it + * instead of the template */ + kr->ccname = kr->old_ccname; + kr->old_ccname = NULL; + } + DEBUG(9, ("Ccache_file is [%s] and is %s active and TGT is %s valid.\n", + kr->ccname, + kr->active_ccache ? "" : "not", + kr->valid_tgt ? "" : "not")); + + /* always recreate the ccache directory path */ + ret = sss_krb5_precreate_ccache(kr->ccname, + kr->krb5_ctx->illegal_path_re, + kr->uid, kr->gid, private_path); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, ("ccache precreation failed.\n")); + return ret; } return EOK; @@ -596,39 +625,12 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx, SYSDB_CCACHE_FILE, NULL); if (ccache_file != NULL) { - ret = check_old_ccache(ccache_file, kr, realm, - &kr->active_ccache, - &kr->valid_tgt); - if (ret == ENOENT) { - DEBUG(SSSDBG_FUNC_DATA, - ("Ignoring ccache attribute [%s], because it doesn't" - "exist.\n", ccache_file)); - ccache_file = NULL; - } else if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, - ("check_if_ccache_file_is_used failed.\n")); - ccache_file = NULL; - } - } else { - kr->active_ccache = false; - kr->valid_tgt = false; - DEBUG(4, ("No ccache file for user [%s] found.\n", pd->user)); - } - DEBUG(9, ("Ccache_file is [%s] and is %s active and TGT is %s valid.\n", - ccache_file ? ccache_file : "not set", - kr->active_ccache ? "" : "not", - kr->valid_tgt ? "" : "not")); - if (ccache_file != NULL) { - kr->ccname = ccache_file; kr->old_ccname = talloc_strdup(kr, ccache_file); if (kr->old_ccname == NULL) { DEBUG(1, ("talloc_strdup failed.\n")); ret = ENOMEM; goto done; } - } else { - kr->ccname = NULL; - kr->old_ccname = NULL; } break; @@ -740,7 +742,6 @@ static void krb5_auth_resolve_done(struct tevent_req *subreq) if (kr->valid_tgt || kr->active_ccache) { DEBUG(9, ("Valid TGT available or " "ccache file is already in use.\n")); - kr->ccname = kr->old_ccname; msg = talloc_asprintf(kr->pd, "%s=%s", CCACHE_ENV_NAME, kr->ccname); if (msg == NULL) { -- 1.8.3.1
>From 17b007f9d390572a00598e9c94ccc1fee5522496 Mon Sep 17 00:00:00 2001 From: Simo Sorce <s...@redhat.com> Date: Fri, 20 Sep 2013 10:01:24 -0400 Subject: [PATCH 3/4] util: Allways fall back to old find_uid method systemd-login still fails with su/sudo login shells, so always fall back for now. Resolves: https://fedorahosted.org/sssd/ticket/2094 --- src/util/find_uid.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/util/find_uid.c b/src/util/find_uid.c index 63b34645748c8af589a0c2999fcf749ef4f75502..85017f7b649c13de46dc67a1164667042338b650 100644 --- a/src/util/find_uid.c +++ b/src/util/find_uid.c @@ -309,15 +309,15 @@ errno_t check_if_uid_is_active(uid_t uid, bool *result) ret = sd_uid_get_sessions(uid, 0, NULL); if (ret > 0) { *result = true; - } - if (ret == 0) { - *result = false; - } - if (ret >= 0) { return EOK; } - DEBUG(SSSDBG_CRIT_FAILURE, ("systemd-login gave error %d: %s\n", - -ret, strerror(-ret))); + if (ret == 0) { + *result = false; + } + if (ret < 0) { + DEBUG(SSSDBG_CRIT_FAILURE, ("systemd-login gave error %d: %s\n", + -ret, strerror(-ret))); + } /* fall back to the old method */ #endif -- 1.8.3.1
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
