[SSSD] [PATCH] Fix krbcc dir creation issue with MIT krb5 1.11

Lukas Slebodnik lslebodn at redhat.com
Wed Apr 10 09:45:59 UTC 2013


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
-------------- next part --------------
>From 670147f5cbda9e455df95093cce44392db4a9cfd Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at 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



More information about the sssd-devel mailing list