KRB5: Move checking for illegal RE to krb5_utils.c

Otherwise we would have to link krb5_child with pcre and transfer the regex, which would be cumbersome. Check for illegal patterns when expanding the template instead. Related: https://fedorahosted.org/sssd/ticket/2370 Reviewed-by: Sumit Bose <[email protected]> Reviewed-by: Lukáš Slebodník <[email protected]>
SSSD · Nov 18, 2014 · 7c5cd2e · 7c5cd2e
1 parent 45aeb92
commit 7c5cd2e
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 97 deletions.
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
@@ -302,15 +302,16 @@ static errno_t krb5_auth_prepare_ccache_name(struct krb5child_req *kr,
             DEBUG(SSSDBG_TRACE_ALL, "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,
+            kr->ccname = expand_ccname_template(kr, kr, ccname_template,
+                                                kr->krb5_ctx->illegal_path_re,
+                                                true,
                                                 be_ctx->domain->case_sensitive);
             if (kr->ccname == NULL) {
                 DEBUG(SSSDBG_CRIT_FAILURE, "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");

diff --git a/src/providers/krb5/krb5_ccache.c b/src/providers/krb5/krb5_ccache.c
@@ -33,28 +33,6 @@
 #include "util/sss_krb5.h"
 #include "util/util.h"
 
-static errno_t
-check_ccache_re(const char *filename, pcre *illegal_re)
-{
-    errno_t ret;
-
-    ret = pcre_exec(illegal_re, NULL, filename, strlen(filename),
-                    0, 0, NULL, 0);
-    if (ret == 0) {
-        DEBUG(SSSDBG_OP_FAILURE,
-              "Illegal pattern in ccache directory name [%s].\n", filename);
-        return EINVAL;
-    } else if (ret == PCRE_ERROR_NOMATCH) {
-        DEBUG(SSSDBG_TRACE_LIBS,
-              "Ccache directory name [%s] does not contain "
-               "illegal patterns.\n", filename);
-        return EOK;
-    }
-
-    DEBUG(SSSDBG_CRIT_FAILURE, "pcre_exec failed [%d].\n", ret);
-    return EFAULT;
-}
-
 struct string_list {
     struct string_list *next;
     struct string_list *prev;
@@ -162,9 +140,7 @@ static errno_t check_parent_stat(struct stat *parent_stat, uid_t uid)
     return EOK;
 }
 
-errno_t create_ccache_dir(const char *ccdirname,
-                          pcre *illegal_re,
-                          uid_t uid, gid_t gid)
+static errno_t create_ccache_dir(const char *ccdirname, uid_t uid, gid_t gid)
 {
     int ret = EFAULT;
     struct stat parent_stat;
@@ -188,13 +164,6 @@ errno_t create_ccache_dir(const char *ccdirname,
         goto done;
     }
 
-    if (illegal_re != NULL) {
-        ret = check_ccache_re(ccdirname, illegal_re);
-        if (ret != EOK) {
-            goto done;
-        }
-    }
-
     ret = find_ccdir_parent_data(tmp_ctx, ccdirname, &parent_stat,
                                  &missing_parents);
     if (ret != EOK) {
@@ -242,8 +211,7 @@ errno_t create_ccache_dir(const char *ccdirname,
     return ret;
 }
 
-errno_t sss_krb5_precreate_ccache(const char *ccname, pcre *illegal_re,
-                                  uid_t uid, gid_t gid)
+errno_t sss_krb5_precreate_ccache(const char *ccname, uid_t uid, gid_t gid)
 {
     TALLOC_CTX *tmp_ctx = NULL;
     const char *filename;
@@ -287,7 +255,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);
+    ret = create_ccache_dir(ccdirname, uid, gid);
 done:
     talloc_free(tmp_ctx);
     return ret;

diff --git a/src/providers/krb5/krb5_ccache.h b/src/providers/krb5/krb5_ccache.h
@@ -35,12 +35,7 @@ struct tgt_times {
     time_t renew_till;
 };
 
-errno_t create_ccache_dir(const char *ccdirname,
-                          pcre *illegal_re,
-                          uid_t uid, gid_t gid);
-
-errno_t sss_krb5_precreate_ccache(const char *ccname, pcre *illegal_re,
-                                  uid_t uid, gid_t gid);
+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);
 

diff --git a/src/providers/krb5/krb5_utils.c b/src/providers/krb5/krb5_utils.c
@@ -202,9 +202,31 @@ errno_t check_if_cached_upn_needs_update(struct sysdb_ctx *sysdb,
 #define S_EXP_USERNAME "{username}"
 #define L_EXP_USERNAME (sizeof(S_EXP_USERNAME) - 1)
 
+static errno_t
+check_ccache_re(const char *filename, pcre *illegal_re)
+{
+    errno_t ret;
+
+    ret = pcre_exec(illegal_re, NULL, filename, strlen(filename),
+                    0, 0, NULL, 0);
+    if (ret == 0) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              "Illegal pattern in ccache directory name [%s].\n", filename);
+        return EINVAL;
+    } else if (ret == PCRE_ERROR_NOMATCH) {
+        DEBUG(SSSDBG_TRACE_LIBS,
+              "Ccache directory name [%s] does not contain "
+               "illegal patterns.\n", filename);
+        return EOK;
+    }
+
+    DEBUG(SSSDBG_CRIT_FAILURE, "pcre_exec failed [%d].\n", ret);
+    return EFAULT;
+}
+
 char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
-                             const char *template, bool file_mode,
-                             bool case_sensitive)
+                             const char *template, pcre *illegal_re,
+                             bool file_mode, bool case_sensitive)
 {
     char *copy;
     char *p;
@@ -217,6 +239,7 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
     TALLOC_CTX *tmp_ctx = NULL;
     char action;
     bool rerun;
+    int ret;
 
     if (template == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Missing template.\n");
@@ -320,7 +343,7 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
                     }
 
                     dummy = expand_ccname_template(tmp_ctx, kr, cache_dir_tmpl,
-                                                   false, case_sensitive);
+                                                   illegal_re, false, case_sensitive);
                     if (dummy == NULL) {
                         DEBUG(SSSDBG_CRIT_FAILURE,
                               "Expanding credential cache directory "
@@ -411,6 +434,13 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
         goto done;
     }
 
+    if (illegal_re != NULL) {
+        ret = check_ccache_re(result, illegal_re);
+        if (ret != EOK) {
+            goto done;
+        }
+    }
+
     res = talloc_move(mem_ctx, &result);
 done:
     talloc_zfree(tmp_ctx);

diff --git a/src/providers/krb5/krb5_utils.h b/src/providers/krb5/krb5_utils.h
@@ -43,8 +43,8 @@ errno_t check_if_cached_upn_needs_update(struct sysdb_ctx *sysdb,
                                          const char *upn);
 
 char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
-                             const char *template, bool file_mode,
-                             bool case_sensitive);
+                             const char *template, pcre *illegal_re,
+                             bool file_mode, bool case_sensitive);
 
 errno_t get_domain_or_subdomain(struct be_ctx *be_ctx,
                                 char *domain_name,

diff --git a/src/tests/krb5_child-test.c b/src/tests/krb5_child-test.c
@@ -239,6 +239,7 @@ create_dummy_req(TALLOC_CTX *mem_ctx, const char *user,
         kr->ccname = expand_ccname_template(kr, kr,
                                         dp_opt_get_cstring(kr->krb5_ctx->opts,
                                                            KRB5_CCNAME_TMPL),
+                                            kr->krb5_ctx->illegal_path_re,
                                             true, true);
         if (!kr->ccname) goto fail;
 
@@ -254,7 +255,6 @@ create_dummy_req(TALLOC_CTX *mem_ctx, const char *user,
             kr->ccname, kr->uid, kr->gid);
 
     ret = sss_krb5_precreate_ccache(kr->ccname,
-                                    kr->krb5_ctx->illegal_path_re,
                                     kr->uid, kr->gid);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "create_ccache_dir failed.\n");