Hi, while looking at expand_ccname_template() becasue of shadowing rewind() I realized that there a some issues with some of the new krb5.conf templates. This patch fixes them and adds some tests to avoid similar issues in the future.
There is one change in behaviour. If the name in the %{} braces does not match any of the known krb5.conf templates for UNIX the new code returns an error while the old just returned something, which in most case will not be the original input. Please tell me if you prefer the original input in this case so that I can change the patch accordingly. bye, Sumit
From a5b1a0530ceb0e858d5c7e849a85e884f55e1487 Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Mon, 2 Sep 2013 17:37:35 +0200 Subject: [PATCH] expand_ccname_template: fixes and tests --- src/providers/krb5/krb5_utils.c | 17 +++++++++++++---- src/tests/krb5_utils-tests.c | 34 ++++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/providers/krb5/krb5_utils.c b/src/providers/krb5/krb5_utils.c index dc4c448..6e9d4fb 100644 --- a/src/providers/krb5/krb5_utils.c +++ b/src/providers/krb5/krb5_utils.c @@ -327,7 +327,8 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr, case '{': if (strncmp(n, S_EXP_TEMP, L_EXP_TEMP) == 0) { /* let the libkrb5 library resolve this */ - result = talloc_asprintf_append(result, "%%"S_EXP_TEMP); + result = talloc_asprintf_append(result, + "%s%%"S_EXP_TEMP, p); n += L_EXP_TEMP - 1; } else if (strncmp(n , S_EXP_UID, L_EXP_UID) == 0) { action = 'U'; @@ -348,6 +349,7 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr, continue; } else if (strncmp(n , S_EXP_NULL, L_EXP_NULL) == 0) { /* skip immediately */ + result = talloc_asprintf_append(result, "%s", p); n += L_EXP_NULL - 1; } else if (strncmp(n , S_EXP_USERNAME, L_EXP_USERNAME) == 0) { action = 'u'; @@ -356,16 +358,23 @@ char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr, continue; } else if (strncmp(n , S_EXP_LIBDIR, L_EXP_LIBDIR) == 0) { /* skip, only the libkrb5 library can resolve this */ - result = talloc_asprintf_append(result, "%%"S_EXP_LIBDIR); + result = talloc_asprintf_append(result, + "%s%%"S_EXP_LIBDIR, p); n += L_EXP_LIBDIR - 1; } else if (strncmp(n , S_EXP_BINDIR, L_EXP_BINDIR) == 0) { /* skip, only the libkrb5 library can resolve this */ - result = talloc_asprintf_append(result, "%%"S_EXP_BINDIR); + result = talloc_asprintf_append(result, + "%s%%"S_EXP_BINDIR, p); n += L_EXP_BINDIR - 1; } else if (strncmp(n , S_EXP_SBINDIR, L_EXP_SBINDIR) == 0) { /* skip, only the libkrb5 library can resolve this */ - result = talloc_asprintf_append(result, "%%"S_EXP_SBINDIR); + result = talloc_asprintf_append(result, + "%s%%"S_EXP_SBINDIR, p); n += L_EXP_SBINDIR - 1; + } else { + DEBUG(1, ("format error, " \ + "unknown krb5.conf template [%s].\n", template)); + goto done; } break; default: diff --git a/src/tests/krb5_utils-tests.c b/src/tests/krb5_utils-tests.c index 4be3ad0..4d1b948 100644 --- a/src/tests/krb5_utils-tests.c +++ b/src/tests/krb5_utils-tests.c @@ -479,14 +479,42 @@ static void do_test(const char *file_template, const char *dir_template, fail_unless(result != NULL, "Cannot expand template [%s].", file_template); fail_unless(strcmp(result, expected) == 0, - "Expansion failed, result [%s], expected [%s].", - result, expected); + "Expansion failed for tempate [%s], " \ + "result [%s], expected [%s].", + file_template, result, expected); fail_unless(private_path == expected_private_path, "Unexprected private path, get [%s], expected [%s].", private_path ? "true" : "false", expected_private_path ? "true" : "false"); } +START_TEST(test_krb5_conf_templates) +{ + do_test(BASE"%{TEMP}/"FILENAME, CCACHE_DIR, BASE"%{TEMP}/"FILENAME, false); + do_test(BASE"_%{uid}", CCACHE_DIR, BASE"_"UID, false); + do_test(BASE"_%{euid}", CCACHE_DIR, BASE"_"UID, false); + do_test(BASE"_%{USERID}", CCACHE_DIR, BASE"_"UID, false); + do_test(BASE"_%{null}_", CCACHE_DIR, BASE"__", false); + do_test(BASE"_%{LIBDIR}_", CCACHE_DIR, BASE"_%{LIBDIR}_", false); + do_test(BASE"_%{BINDIR}_", CCACHE_DIR, BASE"_%{BINDIR}_", false); + do_test(BASE"_%{SBINDIR}_", CCACHE_DIR, BASE"_%{SBINDIR}_", false); + do_test(BASE"_%{username}", CCACHE_DIR, BASE"_"USERNAME, false); +} +END_TEST + +START_TEST(test_krb5_conf_illegal_templates) +{ + char *result; + bool private_path = false; + const char *test_template = BASE"_%{illegal_template}_"; + + result = expand_ccname_template(tmp_ctx, kr, BASE"_%{illegal_template}_", + true, true, &private_path); + fail_unless(result == NULL, "Illegal template [%s] should fail.", + test_template); +} +END_TEST + START_TEST(test_multiple_substitutions) { do_test(BASE"_%u_%U_%u", CCACHE_DIR, BASE"_"USERNAME"_"UID"_"USERNAME, false); @@ -738,6 +766,8 @@ Suite *krb5_utils_suite (void) tcase_add_test (tc_ccname_template, test_pid); tcase_add_test (tc_ccname_template, test_percent); tcase_add_test (tc_ccname_template, test_multiple_substitutions); + tcase_add_test (tc_ccname_template, test_krb5_conf_templates); + tcase_add_test (tc_ccname_template, test_krb5_conf_illegal_templates); suite_add_tcase (s, tc_ccname_template); TCase *tc_create_dir = tcase_create("create_dir"); -- 1.7.7.6
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel