summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJakub Hrozek <jhrozek@redhat.com>2014-10-18 22:03:01 +0200
committerJakub Hrozek <jhrozek@redhat.com>2014-10-30 16:36:50 +0100
commit1710f23d8195ae8438b5c64cf9b745fb464c9a0d (patch)
tree9f80cd8b486e800202f00d2fb192b1fcce796a8b
parentaf8a05f6aea6acd39c4c921ac0fe648940ccafd4 (diff)
downloadsssd-1710f23d8195ae8438b5c64cf9b745fb464c9a0d.tar.gz
sssd-1710f23d8195ae8438b5c64cf9b745fb464c9a0d.tar.xz
sssd-1710f23d8195ae8438b5c64cf9b745fb464c9a0d.zip
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 wold be cumbersome. Check for illegal patterns when expanding the template instead.
-rw-r--r--src/providers/krb5/krb5_ccache.c35
-rw-r--r--src/providers/krb5/krb5_ccache.h4
-rw-r--r--src/providers/krb5/krb5_utils.c36
-rw-r--r--src/providers/krb5/krb5_utils.h4
-rw-r--r--src/tests/krb5_utils-tests.c78
5 files changed, 68 insertions, 89 deletions
diff --git a/src/providers/krb5/krb5_ccache.c b/src/providers/krb5/krb5_ccache.c
index 558696333..912d51aba 100644
--- 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;
@@ -163,7 +141,6 @@ static errno_t check_parent_stat(struct stat *parent_stat, uid_t uid)
}
errno_t create_ccache_dir(const char *ccdirname,
- pcre *illegal_re,
uid_t uid, gid_t gid)
{
int ret = EFAULT;
@@ -188,13 +165,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 +212,7 @@ done:
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 +256,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
index 9f0b3ac84..5ff98864e 100644
--- a/src/providers/krb5/krb5_ccache.h
+++ b/src/providers/krb5/krb5_ccache.h
@@ -36,11 +36,9 @@ struct tgt_times {
};
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
index 5f4078f2c..1ca16100c 100644
--- a/src/providers/krb5/krb5_utils.c
+++ b/src/providers/krb5/krb5_utils.c
@@ -202,9 +202,31 @@ done:
#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
index ce5ce1ebc..0155905b5 100644
--- 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_utils-tests.c b/src/tests/krb5_utils-tests.c
index 52d8a1857..409c0f01d 100644
--- a/src/tests/krb5_utils-tests.c
+++ b/src/tests/krb5_utils-tests.c
@@ -131,13 +131,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);
+ ret = sss_krb5_precreate_ccache(filename, 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);
+ ret = sss_krb5_precreate_ccache(filename, uid, gid);
fail_unless(ret == EOK, "sss_krb5_precreate_ccache failed.");
check_dir(dn3, uid, gid, 0700);
@@ -175,7 +175,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);
+ ret = sss_krb5_precreate_ccache(filename, 12345, 12345);
fail_unless(ret == EINVAL, "Creating private ccache dir in wrong user "
"dir does not failed with EINVAL.");
@@ -185,16 +185,14 @@ END_TEST
START_TEST(test_illegal_patterns)
{
- int ret;
char *cwd;
char *dirname;
char *filename;
- uid_t uid = getuid();
- gid_t gid = getgid();
pcre *illegal_re;
const char *errstr;
int errval;
int errpos;
+ char *result = NULL;
illegal_re = pcre_compile2(ILLEGAL_PATH_PATTERN, 0,
&errval, &errstr, &errpos, NULL);
@@ -209,33 +207,28 @@ START_TEST(test_illegal_patterns)
free(cwd);
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);
- fail_unless(ret == EINVAL, "create_ccache_dir allowed relative path [%s].",
- filename);
+ result = expand_ccname_template(tmp_ctx, kr, "abc/./ccfile", illegal_re, true, true);
+ fail_unless(result == NULL, "expand_ccname_template allowed relative path\n");
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);
- fail_unless(ret == EINVAL, "create_ccache_dir allowed "
- "illegal pattern '/./' in filename [%s].",
- filename);
+ result = expand_ccname_template(tmp_ctx, kr, filename, illegal_re, true, true);
+ fail_unless(result == NULL, "expand_ccname_template allowed "
+ "illegal pattern '/./'\n");
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);
- fail_unless(ret == EINVAL, "create_ccache_dir allowed "
- "illegal pattern '/../' in filename [%s].",
- filename);
+ result = expand_ccname_template(tmp_ctx, kr, filename, illegal_re, true, true);
+ fail_unless(result == NULL, "expand_ccname_template 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);
- fail_unless(ret == EINVAL, "create_ccache_dir allowed "
- "illegal pattern '//' in filename [%s].",
- filename);
+ result = expand_ccname_template(tmp_ctx, kr, filename, illegal_re, true, true);
+ fail_unless(result == NULL, "expand_ccname_template allowed "
+ "illegal pattern '//' in filename [%s].",
+ filename);
pcre_free(illegal_re);
}
@@ -248,17 +241,7 @@ START_TEST(test_cc_dir_create)
char *cwd;
uid_t uid = getuid();
gid_t gid = getgid();
- pcre *illegal_re;
errno_t ret;
- const char *errstr;
- int errval;
- int errpos;
-
- illegal_re = pcre_compile2(ILLEGAL_PATH_PATTERN, 0,
- &errval, &errstr, &errpos, NULL);
- fail_unless(illegal_re != NULL, "Invalid Regular Expression pattern at "
- " position %d. (Error: %d [%s])\n",
- errpos, errval, errstr);
cwd = getcwd(NULL, 0);
fail_unless(cwd != NULL, "getcwd failed.");
@@ -269,7 +252,7 @@ START_TEST(test_cc_dir_create)
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);
+ ret = sss_krb5_precreate_ccache(residual, uid, gid);
fail_unless(ret == EOK, "sss_krb5_precreate_ccache failed\n");
ret = rmdir(dirname);
if (ret < 0) ret = errno;
@@ -282,14 +265,13 @@ START_TEST(test_cc_dir_create)
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);
+ ret = sss_krb5_precreate_ccache(residual, 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);
- pcre_free(illegal_re);
}
END_TEST
@@ -356,7 +338,7 @@ static void do_test(const char *file_template, const char *dir_template,
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);
+ result = expand_ccname_template(tmp_ctx, kr, file_template, NULL, true, true);
fail_unless(result != NULL, "Cannot expand template [%s].", file_template);
fail_unless(strcmp(result, expected) == 0,
@@ -391,14 +373,14 @@ 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);
+ result = expand_ccname_template(tmp_ctx, kr, file_template, NULL, true, true);
fail_unless(result != NULL, "Cannot expand template [%s].", file_template);
fail_unless(strcmp(result, expected_cs) == 0,
"Expansion failed, result [%s], expected [%s].",
result, expected_cs);
- result = expand_ccname_template(tmp_ctx, kr, file_template, true, false);
+ result = expand_ccname_template(tmp_ctx, kr, file_template, NULL, true, false);
fail_unless(result != NULL, "Cannot expand template [%s].", file_template);
fail_unless(strcmp(result, expected_ci) == 0,
@@ -445,7 +427,7 @@ START_TEST(test_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);
+ result = expand_ccname_template(tmp_ctx, kr, "%d/"FILENAME, NULL, true, true);
fail_unless(result == NULL, "Using %%d in ccache dir should fail.");
}
@@ -461,7 +443,7 @@ START_TEST(test_pid)
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);
+ result = expand_ccname_template(tmp_ctx, kr, "%d/"FILENAME, NULL, true, true);
fail_unless(result == NULL, "Using %%P in ccache dir should fail.");
}
@@ -480,7 +462,7 @@ START_TEST(test_unknown_template)
char *result;
int ret;
- result = expand_ccname_template(tmp_ctx, kr, test_template, true, true);
+ result = expand_ccname_template(tmp_ctx, kr, test_template, NULL, true, true);
fail_unless(result == NULL, "Unknown template [%s] should fail.",
test_template);
@@ -488,7 +470,7 @@ START_TEST(test_unknown_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);
+ result = expand_ccname_template(tmp_ctx, kr, test_template, NULL, true, true);
fail_unless(result == NULL, "Unknown template [%s] should fail.",
test_template);
@@ -500,7 +482,7 @@ START_TEST(test_NULL)
char *test_template = NULL;
char *result;
- result = expand_ccname_template(tmp_ctx, kr, test_template, true, true);
+ result = expand_ccname_template(tmp_ctx, kr, test_template, NULL, true, true);
fail_unless(result == NULL, "Expected NULL as a result for an empty input.",
test_template);
@@ -512,7 +494,7 @@ START_TEST(test_no_substitution)
const char *test_template = BASE;
char *result;
- result = expand_ccname_template(tmp_ctx, kr, test_template, true, true);
+ result = expand_ccname_template(tmp_ctx, kr, test_template, NULL, true, true);
fail_unless(result != NULL, "Cannot expand template [%s].", test_template);
fail_unless(strcmp(result, test_template) == 0,
@@ -529,7 +511,7 @@ START_TEST(test_krb5_style_expansion)
file_template = BASE"/%{uid}/%{USERID}/%{euid}/%{username}";
expected = BASE"/"UID"/"UID"/"UID"/"USERNAME;
- result = expand_ccname_template(tmp_ctx, kr, file_template, true, true);
+ result = expand_ccname_template(tmp_ctx, kr, file_template, NULL, true, true);
fail_unless(result != NULL, "Cannot expand template [%s].", file_template);
fail_unless(strcmp(result, expected) == 0,
@@ -538,7 +520,7 @@ START_TEST(test_krb5_style_expansion)
file_template = BASE"/%{unknown}";
expected = BASE"/%{unknown}";
- result = expand_ccname_template(tmp_ctx, kr, file_template, true, false);
+ result = expand_ccname_template(tmp_ctx, kr, file_template, NULL, true, true);
fail_unless(result != NULL, "Cannot expand template [%s].", file_template);
fail_unless(strcmp(result, expected) == 0,