>From 923055de95bb47c2e326b7dd743ea4f3ad20313d Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Sat, 31 Aug 2013 22:44:06 -0400
Subject: [PATCH 15/15] krb5: More correct approach to public dir expansion

Commit 29752834fbf3a19e4e117668abfce4e4c7c48ee4 (Add expandable sequences
to krb5_ccachedir) introduce a new behavior that tried to take in account
the fact that parent directories may need to be created as accessible to
anyone by setting them to 01777 permissions.

However the test that determines whether the path shuld be public or not
is flawed and bleeds into pure file templates with undeiserable consequences.

Theis patch corrects thi behavior by clearly returning a specific path if it
need to be made public and preserves any subdir in the template with user
private ownership.

Resolves:
https://fedorahosted.org/sssd/ticket/2071
---
 src/providers/krb5/krb5_auth.c  |  40 ++++----
 src/providers/krb5/krb5_auth.h  |   4 +-
 src/providers/krb5/krb5_utils.c | 213 ++++++++++++++++++++++++++++------------
 src/providers/krb5/krb5_utils.h |  14 +--
 src/tests/krb5_child-test.c     |  20 ++--
 src/tests/krb5_utils-tests.c    | 197 +++++++++++++++++++------------------
 6 files changed, 292 insertions(+), 196 deletions(-)

diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index 7cfa3e943c15c0f515f3372079cee18dd7978c92..829eb5c81756eab2f7bb29d339dc4cbd966044d8 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -293,7 +293,7 @@ static errno_t krb5_auth_prepare_ccache_name(struct krb5child_req *kr,
                                              int *pam_status, int *dp_err)
 {
     const char *ccname_template;
-    bool private_path = false;
+    char *public_dir = NULL;
     errno_t ret;
 
     if (!kr->is_offline) {
@@ -314,24 +314,24 @@ static errno_t krb5_auth_prepare_ccache_name(struct krb5child_req *kr,
     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;
-            }
+        DEBUG(SSSDBG_TRACE_ALL, ("Precreating  ccache file.\n"));
+        ccname_template = dp_opt_get_cstring(kr->krb5_ctx->opts,
+                                             KRB5_CCNAME_TMPL);
+        ret = expand_ccname_template(kr, kr, ccname_template,
+                                     be_ctx->domain->case_sensitive,
+                                     &public_dir, &kr->ccname);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("expand_ccname_template failed.\n"));
+            return ret;
+        }
 
-            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;
-            }
+        ret = sss_krb5_precreate_ccache(kr->ccname, public_dir,
+                                        kr->krb5_ctx->illegal_path_re,
+                                        kr->uid, kr->gid);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("ccache creation failed.\n"));
+            return ret;
+        }
     }
 
     return EOK;
@@ -620,9 +620,9 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
                   kr->active_ccache ? "" : "not",
                   kr->valid_tgt ? "" : "not"));
         if (ccache_file != NULL) {
-            kr->ccname = ccache_file;
+            kr->ccname = talloc_strdup(kr, ccache_file);
             kr->old_ccname = talloc_strdup(kr, ccache_file);
-            if (kr->old_ccname == NULL) {
+            if (kr->ccname == NULL || kr->old_ccname == NULL) {
                 DEBUG(1, ("talloc_strdup failed.\n"));
                 ret = ENOMEM;
                 goto done;
diff --git a/src/providers/krb5/krb5_auth.h b/src/providers/krb5/krb5_auth.h
index 022dc9b7645f18d01a8a334371e178aa470d92a1..b2ad09468d862533f65d7d72861ff9313761191f 100644
--- a/src/providers/krb5/krb5_auth.h
+++ b/src/providers/krb5/krb5_auth.h
@@ -41,8 +41,8 @@ struct krb5child_req {
     struct pam_data *pd;
     struct krb5_ctx *krb5_ctx;
 
-    const char *ccname;
-    const char *old_ccname;
+    char *ccname;
+    char *old_ccname;
     const char *homedir;
     char *upn;
     uid_t uid;
diff --git a/src/providers/krb5/krb5_utils.c b/src/providers/krb5/krb5_utils.c
index bb933d7f722cbef90b73f4a721382165572c69b9..c16f32ab3d78ccf8599ade0684ebc1f52937b768 100644
--- a/src/providers/krb5/krb5_utils.c
+++ b/src/providers/krb5/krb5_utils.c
@@ -166,41 +166,36 @@ done:
 #define S_EXP_USERNAME "{username}"
 #define L_EXP_USERNAME (sizeof(S_EXP_USERNAME) - 1)
 
-char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
-                             const char *template, bool file_mode,
-                             bool case_sensitive, bool *private_path)
+static errno_t expand_template(TALLOC_CTX *tmp_ctx, struct krb5child_req *kr,
+                               const char *template, bool case_sensitive,
+                               bool dir_mode, char **pubdir, char **ccname)
 {
     char *copy;
     char *p;
     char *n;
     char *result = NULL;
-    char *dummy;
     char *name;
-    char *res = NULL;
     const char *cache_dir_tmpl;
-    TALLOC_CTX *tmp_ctx = NULL;
     char action;
     bool rerun;
+    errno_t ret;
+    bool public_path = true;
+    char *dirname;
 
-    *private_path = false;
-
-    if (template == NULL) {
-        DEBUG(1, ("Missing template.\n"));
-        return NULL;
-    }
-
-    tmp_ctx = talloc_new(NULL);
-    if (!tmp_ctx) return NULL;
+    *pubdir = NULL;
+    *ccname = NULL;
 
     copy = talloc_strdup(tmp_ctx, template);
     if (copy == NULL) {
         DEBUG(1, ("talloc_strdup failed.\n"));
+        ret = ENOMEM;
         goto done;
     }
 
     result = talloc_strdup(tmp_ctx, "");
     if (result == NULL) {
         DEBUG(1, ("talloc_strdup failed.\n"));
+        ret = ENOMEM;
         goto done;
     }
 
@@ -210,6 +205,7 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
         n++;
         if ( *n == '\0' ) {
             DEBUG(1, ("format error, single %% at the end of the template.\n"));
+            ret = EINVAL;
             goto done;
         }
 
@@ -222,6 +218,7 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
                 if (kr->pd->user == NULL) {
                     DEBUG(1, ("Cannot expand user name template "
                               "because user name is empty.\n"));
+                    ret = EINVAL;
                     goto done;
                 }
                 name = sss_get_cased_name(tmp_ctx, kr->pd->user,
@@ -229,12 +226,13 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
                 if (!name) {
                     DEBUG(SSSDBG_CRIT_FAILURE,
                           ("sss_get_cased_name failed\n"));
+                    ret = ENOMEM;
                     goto done;
                 }
 
                 result = talloc_asprintf_append(result, "%s%s", p,
                                                 name);
-                if (!file_mode) *private_path = true;
+                public_path = false;
                 break;
             case 'U':
                 if (kr->uid <= 0) {
@@ -244,7 +242,7 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
                 }
                 result = talloc_asprintf_append(result, "%s%d", p,
                                                 kr->uid);
-                if (!file_mode) *private_path = true;
+                public_path = false;
                 break;
             case 'p':
                 if (kr->upn == NULL) {
@@ -253,60 +251,75 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
                     goto done;
                 }
                 result = talloc_asprintf_append(result, "%s%s", p, kr->upn);
-                if (!file_mode) *private_path = true;
+                public_path = false;
                 break;
             case '%':
                 result = talloc_asprintf_append(result, "%s%%", p);
                 break;
             case 'r':
-                dummy = dp_opt_get_string(kr->krb5_ctx->opts, KRB5_REALM);
-                if (dummy == NULL) {
+                name = dp_opt_get_string(kr->krb5_ctx->opts, KRB5_REALM);
+                if (name == NULL) {
                     DEBUG(1, ("Missing kerberos realm.\n"));
+                    ret = EINVAL;
                     goto done;
                 }
-                result = talloc_asprintf_append(result, "%s%s", p, dummy);
+                result = talloc_asprintf_append(result, "%s%s", p, name);
                 break;
             case 'h':
                 if (kr->homedir == NULL) {
                     DEBUG(1, ("Cannot expand home directory template "
                               "because the path is not available.\n"));
+                    ret = EINVAL;
                     goto done;
                 }
                 result = talloc_asprintf_append(result, "%s%s", p, kr->homedir);
-                if (!file_mode) *private_path = true;
+                public_path = false;
                 break;
             case 'd':
-                if (file_mode) {
-                    cache_dir_tmpl = dp_opt_get_string(kr->krb5_ctx->opts,
-                                                       KRB5_CCACHEDIR);
-                    if (cache_dir_tmpl == NULL) {
-                        DEBUG(1, ("Missing credential cache directory.\n"));
-                        goto done;
-                    }
+                if (dir_mode) {
+                    DEBUG(SSSDBG_CRIT_FAILURE,
+                          ("'%%d' is not allowed in this template.\n"));
+                    ret = EINVAL;
+                    goto done;
+                }
+
+                cache_dir_tmpl = dp_opt_get_string(kr->krb5_ctx->opts,
+                                                   KRB5_CCACHEDIR);
+                if (cache_dir_tmpl == NULL) {
+                    DEBUG(1, ("Missing credential cache directory.\n"));
+                    ret = EINVAL;
+                    goto done;
+                }
 
-                    dummy = expand_ccname_template(tmp_ctx, kr, cache_dir_tmpl,
-                                                   false, case_sensitive,
-                                                   private_path);
-                    if (dummy == NULL) {
-                        DEBUG(1, ("Expanding credential cache directory "
-                                  "template failed.\n"));
+                ret = expand_template(tmp_ctx, kr, cache_dir_tmpl,
+                                      case_sensitive, true, &dirname, &name);
+                if (ret) {
+                    DEBUG(1, ("Expanding credential cache directory "
+                              "template failed.\n"));
+                    goto done;
+                }
+                if (dirname) {
+                    *pubdir = talloc_asprintf(tmp_ctx, "%s%s%s",
+                                              result, p, dirname);
+                    if (*pubdir == NULL) {
+                        DEBUG(SSSDBG_CRIT_FAILURE,
+                              ("talloc_asprintf failed.\n"));
+                        ret = ENOMEM;
                         goto done;
                     }
-                    result = talloc_asprintf_append(result, "%s%s", p, dummy);
-                    talloc_zfree(dummy);
-                } else {
-                    DEBUG(1, ("'%%d' is not allowed in this template.\n"));
-                    goto done;
                 }
+                result = talloc_asprintf_append(result, "%s%s", p, name);
                 break;
             case 'P':
-                if (!file_mode) {
+                if (dir_mode) {
                     DEBUG(1, ("'%%P' is not allowed in this template.\n"));
+                    ret = EINVAL;
                     goto done;
                 }
                 if (kr->pd->cli_pid == 0) {
                     DEBUG(1, ("Cannot expand PID template "
                               "because PID is not available.\n"));
+                    ret = EINVAL;
                     goto done;
                 }
                 result = talloc_asprintf_append(result, "%s%d", p,
@@ -355,12 +368,14 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
                 break;
             default:
                 DEBUG(1, ("format error, unknown template [%%%c].\n", *n));
+                ret = EINVAL;
                 goto done;
             }
         }
 
         if (result == NULL) {
             DEBUG(1, ("talloc_asprintf_append failed.\n"));
+            ret = ENOMEM;
             goto done;
         }
 
@@ -370,13 +385,71 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
     result = talloc_asprintf_append(result, "%s", p);
     if (result == NULL) {
         DEBUG(1, ("talloc_asprintf_append failed.\n"));
+        ret = ENOMEM;
         goto done;
     }
+    if (dir_mode && public_path) {
+        *pubdir = talloc_strdup(tmp_ctx, result);
+        if (*pubdir == NULL) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strdup failed.\n"));
+            ret = ENOMEM;
+            goto done;
+        }
+    }
+    *ccname = result;
+done:
+    return ret;
+}
+
+errno_t expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
+                               const char *template, bool case_sensitive,
+                               char **public_dir, char **cc_name)
+{
+    TALLOC_CTX *tmp_ctx = NULL;
+    const char *residual;
+    char *pubdir = NULL;
+    char *ccname = NULL;
+    errno_t ret;
+
+    *public_dir = NULL;
+    *cc_name = NULL;
+
+    if (template == NULL) {
+        DEBUG(1, ("Missing template.\n"));
+        return EINVAL;
+    }
+
+    residual = strchr(template, ':');
+    if (!residual) residual = template;
+    else residual++;
+
+    tmp_ctx = talloc_new(mem_ctx);
+    if (!tmp_ctx) return ENOMEM;
+
+    ret = expand_template(tmp_ctx, kr, residual,
+                          case_sensitive, false, &pubdir, &ccname);
+    if (ret) {
+        goto done;
+    }
+
+    if (residual == template) {
+        *cc_name = talloc_move(mem_ctx, &ccname);
+    } else {
+        *cc_name = talloc_asprintf(mem_ctx, "%.*s%s",
+                                   (int)(residual - template), template,
+                                   ccname);
+        if (*cc_name == NULL) {
+            ret = ENOMEM;
+            goto done;
+        }
+    }
+    if (pubdir) {
+        *public_dir = talloc_move(mem_ctx, &pubdir);
+    }
 
-    res = talloc_move(mem_ctx, &result);
 done:
     talloc_zfree(tmp_ctx);
-    return res;
+    return ret;
 }
 
 static errno_t check_parent_stat(bool private_path, struct stat *parent_stat,
@@ -424,6 +497,7 @@ static errno_t check_parent_stat(bool private_path, struct stat *parent_stat,
 struct string_list {
     struct string_list *next;
     struct string_list *prev;
+    size_t len_s;
     char *s;
 };
 
@@ -468,10 +542,16 @@ static errno_t find_ccdir_parent_data(TALLOC_CTX *mem_ctx,
               ("talloc_strdup failed.\n"));
         return ENOMEM;
     }
+    li->len_s = strlen(li->s);
+    /* no trailing slashes */
+    while (li->s[li->len_s - 1] == '/') {
+        li->len_s--;
+        li->s[li->len_s] = '\0';
+    }
 
     DLIST_ADD(*missing_parents, li);
 
-    parent = talloc_strdup(mem_ctx, ccdirname);
+    parent = talloc_strdup(mem_ctx, li->s);
     if (parent == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE,
               ("talloc_strdup failed.\n"));
@@ -480,7 +560,7 @@ static errno_t find_ccdir_parent_data(TALLOC_CTX *mem_ctx,
 
     /* We'll remove all trailing slashes from the back so that
      * we only pass /some/path to find_ccdir_parent_data, not
-     * /some/path */
+     * /some/path/ */
     do {
         end = strrchr(parent, '/');
         if (end == NULL || end == parent) {
@@ -522,9 +602,8 @@ check_ccache_re(const char *filename, pcre *illegal_re)
     return EFAULT;
 }
 
-errno_t
-create_ccache_dir(const char *ccdirname, pcre *illegal_re,
-                  uid_t uid, gid_t gid, bool private_path)
+errno_t create_ccache_dir(const char *ccdirname, const char *pubdir,
+                          pcre *illegal_re, uid_t uid, gid_t gid)
 {
     int ret = EFAULT;
     struct stat parent_stat;
@@ -533,6 +612,9 @@ create_ccache_dir(const char *ccdirname, pcre *illegal_re,
     mode_t old_umask;
     mode_t new_dir_mode;
     TALLOC_CTX *tmp_ctx = NULL;
+    size_t len_pubdir = 0;
+    bool in_pubdir;
+    bool is_pubdir;
 
     tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) {
@@ -563,26 +645,35 @@ create_ccache_dir(const char *ccdirname, pcre *illegal_re,
         goto done;
     }
 
-    ret = check_parent_stat(private_path, &parent_stat, uid, gid);
+    ret = check_parent_stat((pubdir == NULL), &parent_stat, uid, gid);
     if (ret != EOK) {
         DEBUG(SSSDBG_MINOR_FAILURE,
               ("check_parent_stat failed for %s directory [%s].\n",
-               private_path ? "private" : "public", ccdirname));
+               (pubdir == NULL) ? "private" : "public", ccdirname));
         goto done;
     }
 
+    if (pubdir) {
+        len_pubdir = strlen(pubdir);
+        while (pubdir[len_pubdir - 1] == '/') len_pubdir--;
+    }
+
     DLIST_FOR_EACH(li, missing_parents) {
         DEBUG(SSSDBG_TRACE_INTERNAL,
               ("Creating directory [%s].\n", li->s));
-        if (li->next == NULL) {
-            new_dir_mode = private_path ? 0700 : 01777;
-        } else {
-            if (private_path &&
-                parent_stat.st_uid == uid && parent_stat.st_gid == gid) {
-                new_dir_mode = 0700;
+        in_pubdir = false;
+        if (len_pubdir && (strncmp(pubdir, li->s, li->len_s) == 0)) {
+            in_pubdir = true;
+            if (len_pubdir == li->len_s) is_pubdir = true;
+        }
+        if (in_pubdir) {
+            if (is_pubdir) {
+                new_dir_mode = 01777;
             } else {
                 new_dir_mode = 0755;
             }
+        } else {
+            new_dir_mode = 0700;
         }
 
         old_umask = umask(0000);
@@ -595,9 +686,7 @@ create_ccache_dir(const char *ccdirname, pcre *illegal_re,
                    strerror(ret)));
             goto done;
         }
-        if (private_path &&
-            ((parent_stat.st_uid == uid && parent_stat.st_gid == gid) ||
-             li->next == NULL)) {
+        if (!in_pubdir) {
             ret = chown(li->s, uid, gid);
             if (ret != EOK) {
                 ret = errno;
@@ -722,8 +811,8 @@ done:
     return EOK;
 }
 
-errno_t sss_krb5_precreate_ccache(const char *ccname, pcre *illegal_re,
-                                  uid_t uid, gid_t gid, bool private_path)
+errno_t sss_krb5_precreate_ccache(const char *ccname, const char *pubdir,
+                                  pcre *illegal_re, uid_t uid, gid_t gid)
 {
     TALLOC_CTX *tmp_ctx = NULL;
     const char *filename;
@@ -767,7 +856,7 @@ errno_t sss_krb5_precreate_ccache(const char *ccname, pcre *illegal_re,
         *end = '\0';
     } while (*(end+1) == '\0');
 
-    ret = create_ccache_dir(ccdirname, illegal_re, uid, gid, private_path);
+    ret = create_ccache_dir(ccdirname, pubdir, illegal_re, uid, gid);
 done:
     talloc_free(tmp_ctx);
     return ret;
diff --git a/src/providers/krb5/krb5_utils.h b/src/providers/krb5/krb5_utils.h
index e41309e765b58f7cd2567f1d13fea30d093fd344..fe107e79e8d485d7dcc4199d695c8bf24b5e50d9 100644
--- a/src/providers/krb5/krb5_utils.h
+++ b/src/providers/krb5/krb5_utils.h
@@ -42,12 +42,12 @@ errno_t check_if_cached_upn_needs_update(struct sysdb_ctx *sysdb,
                                          const char *user,
                                          const char *upn);
 
-errno_t create_ccache_dir(const char *dirname, pcre *illegal_re,
-                          uid_t uid, gid_t gid, bool private_path);
+errno_t create_ccache_dir(const char *ccdirname, const char *pubdir,
+                          pcre *illegal_re, uid_t uid, gid_t gid);
 
-char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
-                             const char *template, bool file_mode,
-                             bool case_sensitive, bool *private_path);
+errno_t expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
+                               const char *template, bool case_sensitive,
+                               char **public_dir, char **ccname);
 
 errno_t become_user(uid_t uid, gid_t gid);
 struct sss_creds;
@@ -57,8 +57,8 @@ errno_t switch_creds(TALLOC_CTX *mem_ctx,
                      struct sss_creds **saved_creds);
 errno_t restore_creds(struct sss_creds *saved_creds);
 
-errno_t sss_krb5_precreate_ccache(const char *ccname, pcre *illegal_re,
-                                  uid_t uid, gid_t gid, bool private_path);
+errno_t sss_krb5_precreate_ccache(const char *ccname, const char *pubdir,
+                                  pcre *illegal_re, 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(uid_t uid, gid_t gid,
                                     const char *ccname, const char *principal);
diff --git a/src/tests/krb5_child-test.c b/src/tests/krb5_child-test.c
index c32ccc318e9826d598bb29a766ed1839fd9bf066..0e089523f798f622845caa512c5e45334ddbe3e5 100644
--- a/src/tests/krb5_child-test.c
+++ b/src/tests/krb5_child-test.c
@@ -198,7 +198,7 @@ create_dummy_req(TALLOC_CTX *mem_ctx, const char *user,
 {
     struct krb5child_req *kr;
     struct passwd *pwd;
-    bool private = false;
+    char *pubdir = NULL;
     errno_t ret;
     const char *tmpl;
 
@@ -247,15 +247,13 @@ create_dummy_req(TALLOC_CTX *mem_ctx, const char *user,
     }
 
     if (!ccname) {
-        kr->ccname = expand_ccname_template(kr, kr,
-                                        dp_opt_get_cstring(kr->krb5_ctx->opts,
-                                                           KRB5_CCNAME_TMPL),
-                                            true, true, &private);
-        if (!kr->ccname) goto fail;
+        ret = expand_ccname_template(kr, kr, tmpl, true, &pubdir, &kr->ccname);
+        if (ret) goto fail;
 
-        DEBUG(SSSDBG_FUNC_DATA, ("ccname [%s] uid [%llu] gid [%llu]\n",
-              kr->ccname, (unsigned long long) kr->uid,
-              (unsigned long long) kr->gid));
+        DEBUG(SSSDBG_FUNC_DATA,
+              ("ccname [%s] pubdir [%s] uid [%llu] gid [%llu]\n",
+               kr->ccname, pubdir ? pubdir : "NULL",
+               (unsigned long long) kr->uid, (unsigned long long) kr->gid));
     } else {
         kr->ccname = talloc_strdup(kr, ccname);
     }
@@ -264,9 +262,9 @@ create_dummy_req(TALLOC_CTX *mem_ctx, const char *user,
     DEBUG(SSSDBG_FUNC_DATA, ("ccname [%s] uid [%llu] gid [%llu]\n",
             kr->ccname, kr->uid, kr->gid));
 
-    ret = sss_krb5_precreate_ccache(kr->ccname,
+    ret = sss_krb5_precreate_ccache(kr->ccname, pubdir,
                                     kr->krb5_ctx->illegal_path_re,
-                                    kr->uid, kr->gid, private);
+                                    kr->uid, kr->gid);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("create_ccache_dir failed.\n"));
         goto fail;
diff --git a/src/tests/krb5_utils-tests.c b/src/tests/krb5_utils-tests.c
index ea0292569135e0fc22a44251cfc4e8719a15837f..96fac4f1bb6f51caefdaf5f637ee42e7dbfbc9bb 100644
--- a/src/tests/krb5_utils-tests.c
+++ b/src/tests/krb5_utils-tests.c
@@ -117,13 +117,13 @@ START_TEST(test_pub_ccache_dir)
 
     ret = chmod(testpath, 0754);
     fail_unless(ret == EOK, "chmod failed.");
-    ret = sss_krb5_precreate_ccache(filename, NULL, 12345, 12345, false);
+    ret = sss_krb5_precreate_ccache(filename, dirname, NULL, 12345, 12345);
     fail_unless(ret == EINVAL, "sss_krb5_precreate_ccache does not return EINVAL "
                                "while x-bit is missing.");
 
     ret = chmod(testpath, 0755);
     fail_unless(ret == EOK, "chmod failed.");
-    ret = sss_krb5_precreate_ccache(filename, NULL, 12345, 12345, false);
+    ret = sss_krb5_precreate_ccache(filename, dirname, NULL, 12345, 12345);
     fail_unless(ret == EOK, "sss_krb5_precreate_ccache failed.");
 
     check_dir(subdirname, 0, 0, 01777);
@@ -158,9 +158,9 @@ START_TEST(test_pub_ccache_dir_in_user_dir)
     filename = talloc_asprintf(tmp_ctx, "%s/ccfile", subdirname);
     fail_unless(filename != NULL, "talloc_asprintf failed.");
 
-    ret = sss_krb5_precreate_ccache(filename, NULL, 12345, 12345, false);
+    ret = sss_krb5_precreate_ccache(filename, dirname, NULL, 12345, 12345);
     fail_unless(ret == EINVAL, "Creating public ccache dir in user dir "
-                               "does not failed with EINVAL.");
+                               "does not fail with EINVAL.");
 
     RMDIR(dirname);
 }
@@ -193,13 +193,13 @@ START_TEST(test_priv_ccache_dir)
 
     ret = chmod(testpath, 0754);
     fail_unless(ret == EOK, "chmod failed.");
-    ret = sss_krb5_precreate_ccache(filename, NULL, uid, gid, true);
+    ret = sss_krb5_precreate_ccache(filename, NULL, NULL, uid, gid);
     fail_unless(ret == EINVAL, "sss_krb5_precreate_ccache does not return EINVAL "
                                "while x-bit is missing.");
 
     ret = chmod(testpath, 0755);
     fail_unless(ret == EOK, "chmod failed.");
-    ret = sss_krb5_precreate_ccache(filename, NULL, uid, gid, true);
+    ret = sss_krb5_precreate_ccache(filename, NULL, NULL, uid, gid);
     fail_unless(ret == EOK, "sss_krb5_precreate_ccache failed.");
 
     check_dir(subdir, uid, gid, 0700);
@@ -248,13 +248,13 @@ START_TEST(test_private_ccache_dir_in_user_dir)
 
     ret = chmod(user_dir, 0600);
     fail_unless(ret == EOK, "chmod failed.");
-    ret = sss_krb5_precreate_ccache(filename, NULL, uid, gid, true);
+    ret = sss_krb5_precreate_ccache(filename, NULL, NULL, uid, gid);
     fail_unless(ret == EINVAL, "sss_krb5_precreate_ccache does not return EINVAL "
                                "while x-bit is missing.");
 
     ret = chmod(user_dir, 0700);
     fail_unless(ret == EOK, "chmod failed.");
-    ret = sss_krb5_precreate_ccache(filename, NULL, uid, gid, true);
+    ret = sss_krb5_precreate_ccache(filename, NULL, NULL, uid, gid);
     fail_unless(ret == EOK, "sss_krb5_precreate_ccache failed.");
 
     check_dir(dn3, uid, gid, 0700);
@@ -292,7 +292,7 @@ START_TEST(test_private_ccache_dir_in_wrong_user_dir)
     filename = talloc_asprintf(tmp_ctx, "%s/ccfile", subdirname);
     fail_unless(filename != NULL, "talloc_asprintf failed.");
 
-    ret = sss_krb5_precreate_ccache(filename, NULL, 12345, 12345, true);
+    ret = sss_krb5_precreate_ccache(filename, NULL, NULL, 12345, 12345);
     fail_unless(ret == EINVAL, "Creating private ccache dir in wrong user "
                                "dir does not failed with EINVAL.");
 
@@ -304,6 +304,7 @@ START_TEST(test_illegal_patterns)
 {
     int ret;
     char *cwd;
+    char *base;
     char *dirname;
     char *filename;
     uid_t uid = getuid();
@@ -322,38 +323,49 @@ START_TEST(test_illegal_patterns)
     cwd = getcwd(NULL, 0);
     fail_unless(cwd != NULL, "getcwd failed.");
 
-    dirname = talloc_asprintf(tmp_ctx, "%s/%s/priv_ccdir", cwd, TESTS_PATH);
+    base = talloc_strdup(tmp_ctx, cwd);
     free(cwd);
+    fail_unless(base != NULL, "talloc_asprintf failed.");
+
+    dirname = talloc_asprintf(tmp_ctx, "%s/%s/priv_ccdir", base, TESTS_PATH);
     fail_unless(dirname != NULL, "talloc_asprintf failed.");
 
 
     filename = talloc_asprintf(tmp_ctx, "abc/./ccfile");
     fail_unless(filename != NULL, "talloc_asprintf failed.");
-    ret = create_ccache_dir(filename, illegal_re, uid, gid, true);
+    ret = create_ccache_dir(filename, NULL, illegal_re, uid, gid);
     fail_unless(ret == EINVAL, "create_ccache_dir allowed relative path [%s].",
                                filename);
 
     filename = talloc_asprintf(tmp_ctx, "%s/abc/./ccfile", dirname);
     fail_unless(filename != NULL, "talloc_asprintf failed.");
-    ret = create_ccache_dir(filename, illegal_re, uid, gid, true);
+    ret = create_ccache_dir(filename, NULL, illegal_re, uid, gid);
     fail_unless(ret == EINVAL, "create_ccache_dir allowed "
                                "illegal pattern '/./' in filename [%s].",
                                filename);
 
     filename = talloc_asprintf(tmp_ctx, "%s/abc/../ccfile", dirname);
     fail_unless(filename != NULL, "talloc_asprintf failed.");
-    ret = create_ccache_dir(filename, illegal_re, uid, gid, true);
+    ret = create_ccache_dir(filename, NULL, illegal_re, uid, gid);
     fail_unless(ret == EINVAL, "create_ccache_dir allowed "
                                "illegal pattern '/../' in filename [%s].",
                                filename);
 
     filename = talloc_asprintf(tmp_ctx, "%s/abc//ccfile", dirname);
     fail_unless(filename != NULL, "talloc_asprintf failed.");
-    ret = create_ccache_dir(filename, illegal_re, uid, gid, true);
+    ret = create_ccache_dir(filename, NULL, illegal_re, uid, gid);
     fail_unless(ret == EINVAL, "create_ccache_dir allowed "
                                "illegal pattern '//' in filename [%s].",
                                filename);
 
+    dirname = talloc_asprintf(tmp_ctx, "%s/%s/pubdir", base, TESTS_PATH);
+    fail_unless(dirname != NULL, "talloc_asprintf failed.");
+    filename = talloc_asprintf(tmp_ctx, "%s/priv/ate/ccfile", dirname);
+    fail_unless(filename != NULL, "talloc_asprintf failed.");
+
+    ret = create_ccache_dir(filename, dirname, illegal_re, uid, gid);
+    fail_unless(ret == EINVAL,
+                "create_ccache_dir should fail for [%s].", filename);
 }
 END_TEST
 
@@ -362,6 +374,7 @@ START_TEST(test_cc_dir_create)
     char *residual;
     char *dirname;
     char *cwd;
+    char *base;
     uid_t uid = getuid();
     gid_t gid = getgid();
     pcre *illegal_re;
@@ -379,13 +392,17 @@ START_TEST(test_cc_dir_create)
     cwd = getcwd(NULL, 0);
     fail_unless(cwd != NULL, "getcwd failed.");
 
+    base = talloc_strdup(tmp_ctx, cwd);
+    free(cwd);
+    fail_unless(base != NULL, "talloc_asprintf failed.");
+
     dirname = talloc_asprintf(tmp_ctx, "%s/%s/user_dir",
-                              cwd, TESTS_PATH);
+                              base, TESTS_PATH);
     fail_unless(dirname != NULL, "talloc_asprintf failed.");
     residual = talloc_asprintf(tmp_ctx, "DIR:%s/%s", dirname, "ccdir");
     fail_unless(residual != NULL, "talloc_asprintf failed.");
 
-    ret = sss_krb5_precreate_ccache(residual, illegal_re, uid, gid, true);
+    ret = sss_krb5_precreate_ccache(residual, NULL, illegal_re, uid, gid);
     fail_unless(ret == EOK, "sss_krb5_precreate_ccache failed\n");
     ret = rmdir(dirname);
     if (ret < 0) ret = errno;
@@ -393,18 +410,17 @@ START_TEST(test_cc_dir_create)
     talloc_free(residual);
 
     dirname = talloc_asprintf(tmp_ctx, "%s/%s/user_dir2",
-                              cwd, TESTS_PATH);
+                              base, TESTS_PATH);
     fail_unless(dirname != NULL, "talloc_asprintf failed.");
     residual = talloc_asprintf(tmp_ctx, "DIR:%s/%s", dirname, "ccdir/");
     fail_unless(residual != NULL, "talloc_asprintf failed.");
 
-    ret = sss_krb5_precreate_ccache(residual, illegal_re, uid, gid, true);
+    ret = sss_krb5_precreate_ccache(residual, NULL, illegal_re, uid, gid);
     fail_unless(ret == EOK, "sss_krb5_precreate_ccache failed\n");
     ret = rmdir(dirname);
     if (ret < 0) ret = errno;
     fail_unless(ret == 0, "Cannot remove %s: %s\n", dirname, strerror(ret));
     talloc_free(residual);
-    free(cwd);
 }
 END_TEST
 
@@ -463,48 +479,51 @@ void free_talloc_context(void)
 }
 
 static void do_test(const char *file_template, const char *dir_template,
-                    const char *expected, const bool expected_private_path)
+                    const char *expected, const char *expected_pubdir)
 {
     char *result;
+    char *pubdir;
     int ret;
-    bool private_path = false;
 
     ret = dp_opt_set_string(kr->krb5_ctx->opts, KRB5_CCACHEDIR, dir_template);
     fail_unless(ret == EOK, "Failed to set Ccache dir");
 
-    result = expand_ccname_template(tmp_ctx, kr, file_template, true,
-                                    true, &private_path);
+    ret = expand_ccname_template(tmp_ctx, kr, file_template,
+                                 true, &pubdir, &result);
 
     fail_unless(result != NULL, "Cannot expand template [%s].", file_template);
-    fail_unless(strcmp(result, expected) == 0,
+    fail_unless(result && expected && strcmp(result, expected) == 0,
                 "Expansion failed, result [%s], expected [%s].",
-                result, expected);
-    fail_unless(private_path == expected_private_path,
-                "Unexpected private path, get [%s], expected [%s].",
-                private_path ? "true" : "false",
-                expected_private_path ? "true" : "false");
+                result ? result : "NULL", expected ? expected : "NULL");
+    fail_unless((expected_pubdir ?
+                    (pubdir && (strcmp(pubdir, expected_pubdir) == 0)) :
+                    (pubdir == NULL)),
+                "Unexpected pubdir, got [%s], expected [%s].",
+                pubdir ? pubdir : "NULL",
+                expected_pubdir ? expected_pubdir : "NULL");
 }
 
 START_TEST(test_multiple_substitutions)
 {
-    do_test(BASE"_%u_%U_%u", CCACHE_DIR, BASE"_"USERNAME"_"UID"_"USERNAME, false);
+    do_test(BASE"_%u_%U_%u", CCACHE_DIR,
+            BASE"_"USERNAME"_"UID"_"USERNAME, NULL);
     do_test("%d/"FILENAME, BASE"_%u_%U_%u",
-            BASE"_"USERNAME"_"UID"_"USERNAME"/"FILENAME, true);
+            BASE"_"USERNAME"_"UID"_"USERNAME"/"FILENAME, NULL);
 }
 END_TEST
 
 START_TEST(test_username)
 {
-    do_test(BASE"_%u", CCACHE_DIR, BASE"_"USERNAME, false);
-    do_test("%d/"FILENAME, BASE"_%u", BASE"_"USERNAME"/"FILENAME, true);
+    do_test(BASE"_%u", CCACHE_DIR, BASE"_"USERNAME, NULL);
+    do_test("%d/"FILENAME, BASE"_%u", BASE"_"USERNAME"/"FILENAME, NULL);
 }
 END_TEST
 
 START_TEST(test_case_sensitive)
 {
     char *result;
+    char *pubdir;
     int ret;
-    bool private_path = false;
     const char *file_template = BASE"_%u";
     const char *expected_cs = BASE"_TestUser";
     const char *expected_ci = BASE"_testuser";
@@ -513,98 +532,92 @@ START_TEST(test_case_sensitive)
     ret = dp_opt_set_string(kr->krb5_ctx->opts, KRB5_CCACHEDIR, CCACHE_DIR);
     fail_unless(ret == EOK, "Failed to set Ccache dir");
 
-    result = expand_ccname_template(tmp_ctx, kr, file_template, true,
-                                    true, &private_path);
+    ret = expand_ccname_template(tmp_ctx, kr, file_template,
+                                 true, &pubdir, &result);
 
     fail_unless(result != NULL, "Cannot expand template [%s].", file_template);
-    fail_unless(strcmp(result, expected_cs) == 0,
+    fail_unless(result && expected_cs && strcmp(result, expected_cs) == 0,
                 "Expansion failed, result [%s], expected [%s].",
-                result, expected_cs);
+                result ? result : "NULL", expected_cs ? expected_cs : "NULL");
 
-    result = expand_ccname_template(tmp_ctx, kr, file_template, true,
-                                    false, &private_path);
+    ret = expand_ccname_template(tmp_ctx, kr, file_template,
+                                 false, &pubdir, &result);
 
     fail_unless(result != NULL, "Cannot expand template [%s].", file_template);
-    fail_unless(strcmp(result, expected_ci) == 0,
+    fail_unless(result && expected_ci && strcmp(result, expected_ci) == 0,
                 "Expansion failed, result [%s], expected [%s].",
-                result, expected_ci);
+                result ? result : "NULL", expected_ci ? expected_ci : "NULL");
 }
 END_TEST
 
 START_TEST(test_uid)
 {
-    do_test(BASE"_%U", CCACHE_DIR, BASE"_"UID, false);
-    do_test("%d/"FILENAME, BASE"_%U", BASE"_"UID"/"FILENAME, true);
+    do_test(BASE"_%U", CCACHE_DIR, BASE"_"UID, NULL);
+    do_test("%d/"FILENAME, BASE"_%U", BASE"_"UID"/"FILENAME, NULL);
 }
 END_TEST
 
 START_TEST(test_upn)
 {
-    do_test(BASE"_%p", CCACHE_DIR, BASE"_"PRINCIPAL_NAME, false);
-    do_test("%d/"FILENAME, BASE"_%p", BASE"_"PRINCIPAL_NAME"/"FILENAME, true);
+    do_test(BASE"_%p", CCACHE_DIR, BASE"_"PRINCIPAL_NAME, NULL);
+    do_test("%d/"FILENAME, BASE"_%p", BASE"_"PRINCIPAL_NAME"/"FILENAME, NULL);
 }
 END_TEST
 
 START_TEST(test_realm)
 {
-    do_test(BASE"_%r", CCACHE_DIR, BASE"_"REALM, false);
-    do_test("%d/"FILENAME, BASE"_%r", BASE"_"REALM"/"FILENAME, false);
+    do_test(BASE"_%r", CCACHE_DIR, BASE"_"REALM, NULL);
+    do_test("%d/"FILENAME, BASE"_%r", BASE"_"REALM"/"FILENAME, BASE"_"REALM);
 }
 END_TEST
 
 START_TEST(test_home)
 {
-    do_test(BASE"_%h", CCACHE_DIR, BASE"_"HOME_DIRECTORY, false);
-    do_test("%d/"FILENAME, BASE"_%h", BASE"_"HOME_DIRECTORY"/"FILENAME, true);
+    do_test(BASE"_%h", CCACHE_DIR, BASE"_"HOME_DIRECTORY, NULL);
+    do_test("%d/"FILENAME, BASE"_%h", BASE"_"HOME_DIRECTORY"/"FILENAME, NULL);
 }
 END_TEST
 
 START_TEST(test_ccache_dir)
 {
     char *result;
+    char *pubdir;
     int ret;
-    bool private_path = false;
 
-    do_test(BASE"_%d", CCACHE_DIR, BASE"_"CCACHE_DIR, false);
+    do_test(BASE"_%d", CCACHE_DIR, BASE"_"CCACHE_DIR, BASE"_"CCACHE_DIR);
 
     ret = dp_opt_set_string(kr->krb5_ctx->opts, KRB5_CCACHEDIR, BASE"_%d");
     fail_unless(ret == EOK, "Failed to set Ccache dir");
 
-    result = expand_ccname_template(tmp_ctx, kr, "%d/"FILENAME, true,
-                                    true, &private_path);
+    ret = expand_ccname_template(tmp_ctx, kr, "%d/"FILENAME,
+                                 true, &pubdir, &result);
 
     fail_unless(result == NULL, "Using %%d in ccache dir should fail.");
-    fail_unless(private_path == false,
-                "Unexpected private path, get [%s], expected [%s].",
-                private_path ? "true" : "false", "false");
 }
 END_TEST
 
 START_TEST(test_pid)
 {
     char *result;
+    char *pubdir;
     int ret;
-    bool private_path = false;
 
-    do_test(BASE"_%P", CCACHE_DIR, BASE"_"PID, false);
+    do_test(BASE"_%P", CCACHE_DIR, BASE"_"PID, NULL);
 
     ret = dp_opt_set_string(kr->krb5_ctx->opts, KRB5_CCACHEDIR, BASE"_%P");
     fail_unless(ret == EOK, "Failed to set Ccache dir");
 
-    result = expand_ccname_template(tmp_ctx, kr, "%d/"FILENAME, true,
-                                    true, &private_path);
+    ret = expand_ccname_template(tmp_ctx, kr, "%d/"FILENAME,
+                                 true, &pubdir, &result);
 
     fail_unless(result == NULL, "Using %%P in ccache dir should fail.");
-    fail_unless(private_path == false,
-                "Unexpected private path, get [%s], expected [%s].",
-                private_path ? "true" : "false", "false");
 }
 END_TEST
 
 START_TEST(test_percent)
 {
-    do_test(BASE"_%%", CCACHE_DIR, BASE"_%", false);
-    do_test("%d/"FILENAME, BASE"_%%", BASE"_%/"FILENAME, false);
+    do_test(BASE"_%%", CCACHE_DIR, BASE"_%", NULL);
+    do_test("%d/"FILENAME, BASE"_%%", BASE"_%/"FILENAME, BASE"_%");
 }
 END_TEST
 
@@ -612,11 +625,11 @@ START_TEST(test_unknow_template)
 {
     const char *test_template = BASE"_%X";
     char *result;
+    char *pubdir;
     int ret;
-    bool private_path = false;
 
-    result = expand_ccname_template(tmp_ctx, kr, test_template, true,
-                                    true, &private_path);
+    ret = expand_ccname_template(tmp_ctx, kr, test_template,
+                                 true, &pubdir, &result);
 
     fail_unless(result == NULL, "Unknown template [%s] should fail.",
                 test_template);
@@ -624,14 +637,11 @@ START_TEST(test_unknow_template)
     ret = dp_opt_set_string(kr->krb5_ctx->opts, KRB5_CCACHEDIR, BASE"_%X");
     fail_unless(ret == EOK, "Failed to set Ccache dir");
     test_template = "%d/"FILENAME;
-    result = expand_ccname_template(tmp_ctx, kr, test_template, true,
-                                    true, &private_path);
+    ret = expand_ccname_template(tmp_ctx, kr, test_template,
+                                 true, &pubdir, &result);
 
     fail_unless(result == NULL, "Unknown template [%s] should fail.",
                 test_template);
-    fail_unless(private_path == false,
-                "Unexpected private path, get [%s], expected [%s].",
-                private_path ? "true" : "false", "false");
 }
 END_TEST
 
@@ -639,16 +649,14 @@ START_TEST(test_NULL)
 {
     char *test_template = NULL;
     char *result;
-    bool private_path = false;
+    char *pubdir;
+    int ret;
 
-    result = expand_ccname_template(tmp_ctx, kr, test_template, true,
-                                    true, &private_path);
+    ret = expand_ccname_template(tmp_ctx, kr, test_template,
+                                 true, &pubdir, &result);
 
-    fail_unless(result == NULL, "Expected NULL as a result for an empty input.",
-                test_template);
-    fail_unless(private_path == false,
-                "Unexpected private path, get [%s], expected [%s].",
-                private_path ? "true" : "false", "false");
+    fail_unless(result == NULL,
+                "Expected NULL as a result for an empty input.");
 }
 END_TEST
 
@@ -656,32 +664,33 @@ START_TEST(test_no_substitution)
 {
     const char *test_template = BASE;
     char *result;
-    bool private_path = false;
+    char *pubdir;
+    int ret;
 
-    result = expand_ccname_template(tmp_ctx, kr, test_template, true,
-                                    true, &private_path);
+    ret = expand_ccname_template(tmp_ctx, kr, test_template,
+                                 true, &pubdir, &result);
 
     fail_unless(result != NULL, "Cannot expand template [%s].", test_template);
     fail_unless(strcmp(result, test_template) == 0,
                 "Expansion failed, result [%s], expected [%s].",
                 result, test_template);
-    fail_unless(private_path == false,
-                "Unexpected private path, get [%s], expected [%s].",
-                private_path ? "true" : "false", "false");
+    fail_unless(pubdir == NULL,
+                "Unexpected pubdir, got [%s], expected [NULL].", pubdir);
 }
 END_TEST
 
 START_TEST(test_krb5_style_expansion)
 {
-    char *result;
-    bool private_path = false;
     const char *file_template;
     const char *expected;
+    char *result;
+    char *pubdir;
+    int ret;
 
     file_template = BASE"/%{uid}/%{USERID}/%{euid}/%{username}";
     expected = BASE"/"UID"/"UID"/"UID"/"USERNAME;
-    result = expand_ccname_template(tmp_ctx, kr, file_template, true,
-                                    true, &private_path);
+    ret = expand_ccname_template(tmp_ctx, kr, file_template,
+                                 true, &pubdir, &result);
 
     fail_unless(result != NULL, "Cannot expand template [%s].", file_template);
     fail_unless(strcmp(result, expected) == 0,
@@ -690,8 +699,8 @@ START_TEST(test_krb5_style_expansion)
 
     file_template = BASE"/%{unknown}";
     expected = BASE"/%{unknown}";
-    result = expand_ccname_template(tmp_ctx, kr, file_template, true,
-                                    false, &private_path);
+    ret = expand_ccname_template(tmp_ctx, kr, file_template,
+                                 true, &pubdir, &result);
 
     fail_unless(result != NULL, "Cannot expand template [%s].", file_template);
     fail_unless(strcmp(result, expected) == 0,
-- 
1.8.3.1

