On (04/04/13 12:09), Simo Sorce wrote: >On Thu, 2013-04-04 at 12:14 +0200, Lukas Slebodnik wrote: >> ehlo, >> >> In krb5-libs 1.11, function krb5_cc_resolve verify if credential cache dir >> exists. If it doesn't exist, than it will be created. We >> have to ensure, that it will be created with right uid, gid and permissions. >> >> There is check in krb_auth_send, whether cached user data in ldb contains >> SYSDB_CCACHE_FILE attribute. If it is available, then old ccache is checked >> in >> function check_old_ccache. Gdm clean user directory (in /run/user/) >> after logout. This is the reason why SYSDB_CCACHE_FILE attribute was found >> in ldb cache and directory did not exist and then in function krb5_cc_resolve >> was created with wrong permissions. >> >> Patch attached. > >Although this approach may work I am not really comfortable with it. > >Here is an alternative approach: > >1. Before calling krb5_cc_resolve() do a stat. function cc_residual_is_used call stat, but nonexisting file(directory) was ignored (handled like EOK) >2. If the file/dir does not exist just return an error w/o calling any >krb5 function at all. > >I think this should also solve the issue, and will not involve change of >uid/gid. > >This method has a very small race that can hardly be triggered though, >it would require someone to remove the cache after the stat and before >krb5_cc_resolve() is called. In that case auth would fail, but we should >be able to recover on the next authentication by removing the file if it >has the wrong permissions (I think this is already done, if not we can >fix it with a second patch that adds checking for uid/gid after the stat >and a call to cc_file_rmove/cc_dir_remove). cc_dir_remove call cc_file_remove, so directory will not be removed. > >Simo. >
Rewritten patch attached. LS
>From 670147f5cbda9e455df95093cce44392db4a9cfd Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <lsleb...@redhat.com> Date: Sat, 6 Apr 2013 17:58:53 +0200 Subject: [PATCH] Fix krbcc dir creation issue with MIT krb5 1.11 In krb5-libs >= 1.11, function krb5_cc_resolve verify if credential cache dir exists. If it doesn't exist, than it will be created with process permissions and not user permissions. Function cc_residual_is_used has already checked for non existing directory, but it wasn't considered to be a failure and therefore next call of krb5_init_context will create directory with wrong permissions. Now if directory doesn't exist, it will be handled like there was not ccache attribute in sysdb cache. https://fedorahosted.org/sssd/ticket/1822 --- src/providers/krb5/krb5_auth.c | 12 +++++++++++- src/providers/krb5/krb5_utils.c | 10 +++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c index 00025bfc156eaf641217194c6301f4d70a773a73..5baea0bc84bb6991d32300210d4bb4db3bcee5d0 100644 --- a/src/providers/krb5/krb5_auth.c +++ b/src/providers/krb5/krb5_auth.c @@ -106,6 +106,11 @@ check_old_ccache(const char *old_ccache, struct krb5child_req *kr, ret = old_cc_ops->check_existing(old_ccache, kr->uid, realm, kr->upn, cc_template, active, valid); + if (ret == ENOENT) { + DEBUG(SSSDBG_TRACE_FUNC, + ("Saved ccache %s doesn't exist.\n", old_ccache)); + return ret; + } if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("Cannot check if saved ccache %s is active and valid\n", @@ -617,7 +622,12 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx, ret = check_old_ccache(ccache_file, kr, realm, &kr->active_ccache, &kr->valid_tgt); - if (ret != EOK) { + 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")); goto done; diff --git a/src/providers/krb5/krb5_utils.c b/src/providers/krb5/krb5_utils.c index ad77c7cc8305a98cc263cd7c6222979f361d0155..3ea3d2676aac1aa9f370cde87278e91914524778 100644 --- a/src/providers/krb5/krb5_utils.c +++ b/src/providers/krb5/krb5_utils.c @@ -776,7 +776,7 @@ cc_residual_is_used(uid_t uid, const char *ccname, DEBUG(SSSDBG_FUNC_DATA, ("Cache file [%s] does not exist, " "it will be recreated\n", ccname)); *result = false; - return EOK; + return ENOENT; } DEBUG(SSSDBG_OP_FAILURE, @@ -869,10 +869,13 @@ cc_file_check_existing(const char *location, uid_t uid, ret = cc_residual_is_used(uid, filename, SSS_KRB5_TYPE_FILE, &active); if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, ("Could not check if ccache is active. " - "Will create a new one.\n")); + if (ret != ENOENT) { + DEBUG(SSSDBG_OP_FAILURE, + ("Could not check if ccache is active.\n")); + } cc_check_template(cc_template); active = false; + return ret; } kerr = krb5_init_context(&context); @@ -1044,6 +1047,7 @@ cc_dir_check_existing(const char *location, uid_t uid, "Will create a new one.\n")); cc_check_template(cc_template); active = false; + goto done; } krberr = krb5_init_context(&context); -- 1.8.2
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel