On Wed, 2013-09-25 at 14:04 +0200, Jakub Hrozek wrote: > On Tue, Sep 24, 2013 at 08:53:32AM -0400, Simo Sorce wrote: > > On Tue, 2013-09-24 at 12:59 +0200, Jakub Hrozek wrote: > > > On Fri, Sep 20, 2013 at 01:04:06PM -0400, Simo Sorce wrote: > > > > 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. > > > > > > Sorry, I didn't have the time to test the first two patches yet, but I'd > > > like to have the third one pushed asap. > > > > > > So ACK to the third patch. I think it matches what we agreed on IRC -- > > > only positive match from logind is now authoritative, negative is only > > > taken into account if checking proc failed. > > > > You really need the first one too. It fixes a bug that causes issues on > > authentication. > > > > Simo. > > OK, ACK to the first patch, too. I haven't found any issues with it > (although I must confess I only tested with DIR cache). > > But the second patch seems to break offline authentication. Normally, > when a user logs in offline, he is supposed to have an expired TGT so > that it can be renewed once sssd gets online. But with the 2nd patch I'm not > seeing that behaviour: > > $ su - [email protected] > Password: > [[email protected]@adclient ~]$ klist > klist: No credentials cache found (ticket cache > DIR::/run/user/1983201105/krb5cc/tkt)
Haven't found what causes this yet, however in the meanwhile attached find the patch rebased on top of the one for #2071 I just sent to the other thread. Simo. -- Simo Sorce * Red Hat, Inc * New York
>From e362a8039693dbc238cd3361b8137552c2e52963 Mon Sep 17 00:00:00 2001 From: Simo Sorce <[email protected]> Date: Thu, 19 Sep 2013 18:06:34 -0400 Subject: [PATCH] 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 | 114 ++++++++++++++++++++--------------------- 1 file changed, 57 insertions(+), 57 deletions(-) diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c index 7478d5a5a69976c389fb7357f98017e566c8e163..4dc92adfa94fff6bae3377bd39ea332e68b39899 100644 --- a/src/providers/krb5/krb5_auth.c +++ b/src/providers/krb5/krb5_auth.c @@ -299,37 +299,65 @@ 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); - 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); + 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); - 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); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, ("ccache precreation failed.\n")); + return ret; } return EOK; @@ -596,39 +624,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 +741,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
_______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
