KRB5_CHILD: don't check if FILE:/DIR: path accessible in advance

Inaccessible FILE:/DIR: path is a system configuration error that
needs to be fixed anyway. It doesn't make much difference to fail
before or after credentials check in this case.

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>

Author: alexey-tikhonov

src/providers/krb5/krb5_ccache.c

@@ -112,154 +112,6 @@ static errno_t find_ccdir_parent_data(TALLOC_CTX *mem_ctx,
     return ret;
 }
 
-static errno_t check_parent_stat(struct stat *parent_stat, uid_t uid)
-{
-    if (parent_stat->st_uid != 0 && parent_stat->st_uid != uid) {
-        DEBUG(SSSDBG_CRIT_FAILURE,
-              "Private directory can only be created below a directory "
-              "belonging to root or to [%"SPRIuid"].\n", uid);
-        return EINVAL;
-    }
-
-    if (parent_stat->st_uid == uid) {
-        if (!(parent_stat->st_mode & S_IXUSR)) {
-            DEBUG(SSSDBG_CRIT_FAILURE,
-                  "Parent directory does not have the search bit set for "
-                   "the owner.\n");
-            return EINVAL;
-        }
-    } else {
-        if (!(parent_stat->st_mode & S_IXOTH)) {
-            DEBUG(SSSDBG_CRIT_FAILURE,
-                  "Parent directory does not have the search bit set for "
-                   "others.\n");
-            return EINVAL;
-        }
-    }
-
-    return EOK;
-}
-
-static errno_t create_ccache_dir(const char *ccdirname, uid_t uid, gid_t gid)
-{
-    int ret = EFAULT;
-    struct stat parent_stat;
-    struct string_list *missing_parents = NULL;
-    struct string_list *li = NULL;
-    mode_t old_umask;
-    mode_t new_dir_mode;
-    TALLOC_CTX *tmp_ctx = NULL;
-
-    tmp_ctx = talloc_new(NULL);
-    if (tmp_ctx == NULL) {
-        DEBUG(SSSDBG_CRIT_FAILURE,
-              "talloc_new failed.\n");
-        return ENOMEM;
-    }
-
-    if (*ccdirname != '/') {
-        DEBUG(SSSDBG_MINOR_FAILURE,
-              "Only absolute paths are allowed, not [%s] .\n", ccdirname);
-        ret = EINVAL;
-        goto done;
-    }
-
-    ret = find_ccdir_parent_data(tmp_ctx, ccdirname, &parent_stat,
-                                 &missing_parents);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_MINOR_FAILURE,
-              "find_ccdir_parent_data failed.\n");
-        goto done;
-    }
-
-    ret = check_parent_stat(&parent_stat, uid);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_FATAL_FAILURE,
-              "Check the ownership and permissions of krb5_ccachedir: [%s].\n",
-              ccdirname);
-        goto done;
-    }
-
-    DLIST_FOR_EACH(li, missing_parents) {
-        DEBUG(SSSDBG_TRACE_INTERNAL,
-              "Creating directory [%s].\n", li->s);
-        new_dir_mode = 0700;
-
-        old_umask = umask(0000);
-        ret = mkdir(li->s, new_dir_mode);
-        umask(old_umask);
-        if (ret != EOK) {
-            ret = errno;
-            DEBUG(SSSDBG_MINOR_FAILURE,
-                  "mkdir [%s] failed: [%d][%s].\n", li->s, ret,
-                   strerror(ret));
-            goto done;
-        }
-        ret = chown(li->s, uid, gid);
-        if (ret != EOK) {
-            ret = errno;
-            DEBUG(SSSDBG_MINOR_FAILURE,
-                  "chown failed [%d][%s].\n", ret, strerror(ret));
-            goto done;
-        }
-    }
-
-    ret = EOK;
-
-done:
-    talloc_free(tmp_ctx);
-    return ret;
-}
-
-errno_t sss_krb5_precreate_ccache(const char *ccname, uid_t uid, gid_t gid)
-{
-    TALLOC_CTX *tmp_ctx = NULL;
-    const char *filename;
-    char *ccdirname;
-    char *end;
-    errno_t ret;
-
-    if (ccname[0] == '/') {
-        filename = ccname;
-    } else if (strncmp(ccname, "FILE:", 5) == 0) {
-        filename = ccname + 5;
-    } else if (strncmp(ccname, "DIR:", 4) == 0) {
-        filename = ccname + 4;
-    } else {
-        /* only FILE and DIR types need precreation so far, we ignore any
-         * other type */
-        return EOK;
-    }
-
-    tmp_ctx = talloc_new(NULL);
-    if (!tmp_ctx) return ENOMEM;
-
-    ccdirname = talloc_strdup(tmp_ctx, filename);
-    if (ccdirname == NULL) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup failed.\n");
-        ret = ENOMEM;
-        goto done;
-    }
-
-    /* We'll remove all trailing slashes from the back so that
-     * we only pass /some/path to find_ccdir_parent_data, not
-     * /some/path/ */
-    do {
-        end = strrchr(ccdirname, '/');
-        if (end == NULL || end == ccdirname) {
-            DEBUG(SSSDBG_CRIT_FAILURE, "Cannot find parent directory of [%s], "
-                  "/ is not allowed.\n", ccdirname);
-            ret = EINVAL;
-            goto done;
-        }
-        *end = '\0';
-    } while (*(end+1) == '\0');
-
-    ret = create_ccache_dir(ccdirname, uid, gid);
-done:
-    talloc_free(tmp_ctx);
-    return ret;
-}
 
 struct sss_krb5_ccache {
     struct sss_creds *creds;

src/providers/krb5/krb5_ccache.h

@@ -35,8 +35,6 @@ struct tgt_times {
     time_t renew_till;
 };
 
-errno_t sss_krb5_precreate_ccache(const char *ccname, uid_t uid, gid_t gid);
-
 errno_t sss_krb5_cc_destroy(const char *ccname, uid_t uid, gid_t gid);
 
 errno_t sss_krb5_check_ccache_princ(krb5_context kctx,

src/providers/krb5/krb5_child.c

@@ -3945,94 +3945,53 @@ static errno_t old_ccache_valid(struct krb5_req *kr, bool *_valid)
     return EOK;
 }
 
-static int k5c_check_old_ccache(struct krb5_req *kr)
+static void k5c_ccache_check(struct krb5_req *kr, uint32_t offline)
 {
     errno_t ret;
 
-    if (kr->old_ccname) {
-        ret = old_ccache_valid(kr, &kr->old_cc_valid);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, "old_ccache_valid failed.\n");
-            return ret;
-        }
+    kr->old_cc_valid = false;
+    kr->old_cc_active = false;
+
+    if (kr->pd->cmd == SSS_PAM_ACCT_MGMT) {
+        return;
+    }
+
+    if (kr->old_ccname == NULL) {
+        return;
+    }
 
+    ret = old_ccache_valid(kr, &kr->old_cc_valid);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "old_ccache_valid() failed.\n");
+    } else {
         ret = check_if_uid_is_active(kr->uid, &kr->old_cc_active);
         if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, "check_if_uid_is_active failed.\n");
-            return ret;
+            DEBUG(SSSDBG_OP_FAILURE, "check_if_uid_is_active() failed.\n");
+            kr->old_cc_valid = false;
         }
-
-        DEBUG(SSSDBG_TRACE_ALL,
-                "Ccache_file is [%s] and is %s active and TGT is %s valid.\n",
-                kr->old_ccname ? kr->old_ccname : "not set",
-                kr->old_cc_active ? "" : "not",
-                kr->old_cc_valid ? "" : "not");
     }
 
-    return EOK;
-}
-
-static int k5c_precreate_ccache(struct krb5_req *kr, uint32_t offline)
-{
-    errno_t ret;
+    DEBUG(SSSDBG_TRACE_ALL,
+            "Old ccache is [%s] and is %s active and TGT is %s valid.\n",
+            kr->old_ccname ? kr->old_ccname : "not set",
+            kr->old_cc_active ? "" : "not",
+            kr->old_cc_valid ? "" : "not");
 
-    /* The ccache file should be (re)created if one of the following conditions
+    /* Old ccache can't be used if one of the following conditions
      * is true:
-     * - it doesn't exist (kr->old_ccname == NULL)
+     * - the backend is offline and the current cache file not used and
+     * it does not contain a valid TGT (1)
      * - 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
-     * (offline && !kr->old_cc_active && 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
-     * (offline && !kr->old_cc_active && !kr->valid_tgt)
+     * request (2)
      */
-    if (kr->old_ccname == NULL ||
-            (offline && !kr->old_cc_active && !kr->old_cc_valid) ||
-            (!offline && !kr->old_cc_active && kr->pd->cmd != SSS_CMD_RENEW)) {
-        DEBUG(SSSDBG_TRACE_ALL, "Recreating ccache\n");
-
-        ret = sss_krb5_precreate_ccache(kr->ccname, kr->uid, kr->gid);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, "ccache check failed.\n");
-            return ret;
-        }
+    if ((offline && !kr->old_cc_active && !kr->old_cc_valid) || /* (1) */
+        (!offline && !kr->old_cc_active && kr->pd->cmd != SSS_CMD_RENEW)) /* (2) */ {
+        DEBUG(SSSDBG_TRACE_ALL, "Ignoring old ccache\n");
     } else {
         /* We can reuse the old ccache */
         kr->ccname = kr->old_ccname;
     }
-
-    return EOK;
-}
-
-static int k5c_ccache_check(struct krb5_req *kr, uint32_t offline)
-{
-    errno_t ret;
-
-    if (kr->pd->cmd == SSS_PAM_ACCT_MGMT) {
-        return EOK;
-    }
-
-    ret = k5c_check_old_ccache(kr);
-    if (ret != 0) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "Cannot check old ccache [%s]: [%d][%s]. " \
-                                   "Assuming old cache is invalid " \
-                                   "and not used.\n",
-                                   kr->old_ccname, ret, sss_strerror(ret));
-    }
-
-    /* Pre-creating the ccache must be done as root, otherwise we can't mkdir
-     * some of the DIR: cache components. One example is /run/user/$UID because
-     * logind doesn't create the directory until the session phase, whereas
-     * we need the directory during the auth phase already
-     */
-    ret = k5c_precreate_ccache(kr, offline);
-    if (ret != 0) {
-        DEBUG(SSSDBG_OP_FAILURE, "Cannot precreate ccache\n");
-        return ret;
-    }
-
-    return EOK;
 }
 
 static int k5c_setup(struct krb5_req *kr, uint32_t offline)
@@ -4207,11 +4166,7 @@ static krb5_error_code privileged_krb5_setup(struct krb5_req *kr,
         return ret;
     }
 
-    ret = k5c_ccache_check(kr, offline);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "k5c_ccache_check() failed.\n");
-        return ret;
-    }
+    k5c_ccache_check(kr, offline);
 
     if (!(offline ||
             (kr->fast_val == K5C_FAST_NEVER && kr->validate == false))) {