Hi,
the attached patches implement fetching the keytab for one-way trusts on
each sssd restart. This is in order for admin to be able to call service
sssd restart and have fresh keytabs in case the trust was re-established
in the meantime.
Even though retrieving the keytabs is quite expensive operation,
restarting the sssd instance on the IPA server should be quite rare.
>From c18113d310ef7a0f988739b8e031cc5e6c36a198 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Fri, 24 Jul 2015 13:12:43 +0200
Subject: [PATCH 1/2] UTIL: New utility function sss_unique_filename() to
create a random filename
Instead of copying the same code using mkstemp over and over, provide a
common version.
---
src/providers/ad/ad_gpo_child.c | 8 +--
src/providers/krb5/krb5_child.c | 37 ++++++--------
src/providers/krb5/krb5_common.c | 8 +--
src/providers/ldap/ldap_child.c | 15 ++----
src/responder/ssh/sshsrv_cmd.c | 6 +--
src/tests/cmocka/test_utils.c | 103 +++++++++++++++++++++++++++++++++++++++
src/util/util.c | 50 +++++++++++++++++++
src/util/util.h | 13 +++++
8 files changed, 188 insertions(+), 52 deletions(-)
diff --git a/src/providers/ad/ad_gpo_child.c b/src/providers/ad/ad_gpo_child.c
index
03951af041463000d51695fe1a86609ed6c7c1c2..e0e0c068ff496bf28ec9fa4ab7b2105238719217
100644
--- a/src/providers/ad/ad_gpo_child.c
+++ b/src/providers/ad/ad_gpo_child.c
@@ -276,7 +276,6 @@ static errno_t gpo_cache_store_file(const char *smb_path,
int fd = -1;
char *tmp_name = NULL;
ssize_t written;
- mode_t old_umask;
char *filename = NULL;
char *smb_path_with_suffix = NULL;
TALLOC_CTX *tmp_ctx = NULL;
@@ -318,13 +317,10 @@ static errno_t gpo_cache_store_file(const char *smb_path,
goto done;
}
- old_umask = umask(077);
- fd = mkstemp(tmp_name);
- umask(old_umask);
+ fd = sss_unique_file(tmp_name, &ret);
if (fd == -1) {
- ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE,
- "mkstemp failed [%d][%s].\n", ret, strerror(ret));
+ "sss_unique_file failed [%d][%s].\n", ret, strerror(ret));
goto done;
}
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index
2c5e446a0e0c3f55261a39d8d3f3bc09aded3cb9..a37f912b64ec09f7c7f232ad6efd64926bfa8e74
100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -668,11 +668,8 @@ done:
static errno_t handle_randomized(char *in)
{
- size_t ccname_len;
char *ccname = NULL;
int ret;
- int fd;
- mode_t old_umask;
/* We only treat the FILE type case in a special way due to the history
* of storing FILE type ccache in /tmp and associated security issues */
@@ -684,26 +681,20 @@ static errno_t handle_randomized(char *in)
return EOK;
}
- ccname_len = strlen(ccname);
- if (ccname_len >= 6 && strcmp(ccname + (ccname_len - 6), "XXXXXX") == 0) {
- /* NOTE: this call is only used to create a unique name, as later
- * krb5_cc_initialize() will unlink and recreate the file.
- * This is ok because this part of the code is called with
- * privileges already dropped when handling user ccache, or the ccache
- * is stored in a private directory. So we do not have huge issues if
- * something races, we mostly care only about not accidentally use
- * an existing name and thus failing in the process of saving the
- * cache. Malicious races can only be avoided by libkrb5 itself. */
- old_umask = umask(077);
- fd = mkstemp(ccname);
- umask(old_umask);
- if (fd == -1) {
- ret = errno;
- DEBUG(SSSDBG_CRIT_FAILURE,
- "mkstemp(\"%s\") failed [%d]: %s!\n",
- ccname, ret, strerror(ret));
- return ret;
- }
+ /* NOTE: this call is only used to create a unique name, as later
+ * krb5_cc_initialize() will unlink and recreate the file.
+ * This is ok because this part of the code is called with
+ * privileges already dropped when handling user ccache, or the ccache
+ * is stored in a private directory. So we do not have huge issues if
+ * something races, we mostly care only about not accidentally use
+ * an existing name and thus failing in the process of saving the
+ * cache. Malicious races can only be avoided by libkrb5 itself. */
+ ret = sss_unique_filename(ccname);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "mkstemp(\"%s\") failed [%d]: %s!\n",
+ ccname, ret, strerror(ret));
+ return ret;
}
return EOK;
diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c
index
81d4048b63dba98706bbef1936df7f10f79e1ae5..9d3f1a20193285b49f7a55fef5deb806780cc662
100644
--- a/src/providers/krb5/krb5_common.c
+++ b/src/providers/krb5/krb5_common.c
@@ -419,7 +419,6 @@ errno_t write_krb5info_file(const char *realm, const char
*server,
const char *name_tmpl = NULL;
size_t server_len;
ssize_t written;
- mode_t old_umask;
if (realm == NULL || *realm == '\0' || server == NULL || *server == '\0' ||
service == NULL || *service == '\0') {
@@ -459,13 +458,10 @@ errno_t write_krb5info_file(const char *realm, const char
*server,
goto done;
}
- old_umask = umask(077);
- fd = mkstemp(tmp_name);
- umask(old_umask);
+ fd = sss_unique_file(tmp_name, &ret);
if (fd == -1) {
- ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE,
- "mkstemp failed [%d][%s].\n", ret, strerror(ret));
+ "sss_unique_file failed [%d][%s].\n", ret, strerror(ret));
goto done;
}
diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c
index
82481d6e75c86f7be49625a669691b235589d9a7..338345d89fa0484eaa8fcf99f1e0f10eb61a1d91
100644
--- a/src/providers/ldap/ldap_child.c
+++ b/src/providers/ldap/ldap_child.c
@@ -254,7 +254,6 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX
*memctx,
const char **ccname_out,
time_t *expire_time_out)
{
- int fd;
char *ccname;
char *ccname_dummy;
char *realm_name = NULL;
@@ -274,7 +273,6 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX
*memctx,
TALLOC_CTX *tmp_ctx;
char *ccname_file_dummy = NULL;
char *ccname_file;
- mode_t old_umask;
tmp_ctx = talloc_new(memctx);
if (tmp_ctx == NULL) {
@@ -408,21 +406,14 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX
*memctx,
goto done;
}
- old_umask = umask(077);
- fd = mkstemp(ccname_file_dummy);
- umask(old_umask);
- if (fd == -1) {
- ret = errno;
+ ret = sss_unique_filename(ccname_file_dummy);
+ if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
- "mkstemp failed: %s:[%d].\n",
+ "sss_unique_filename failed: %s:[%d].\n",
strerror(ret), ret);
krberr = KRB5KRB_ERR_GENERIC;
goto done;
}
- /* We only care about creating a unique file name here, we don't
- * need the fd
- */
- close(fd);
krberr = krb5_get_init_creds_keytab(context, &my_creds, kprinc,
keytab, 0, NULL, &options);
diff --git a/src/responder/ssh/sshsrv_cmd.c b/src/responder/ssh/sshsrv_cmd.c
index
4833587910cade32ecb0b5f65b417d58a498b01e..8c650d04030ed6984f064448320df2cddc66f6e8
100644
--- a/src/responder/ssh/sshsrv_cmd.c
+++ b/src/responder/ssh/sshsrv_cmd.c
@@ -555,7 +555,6 @@ ssh_host_pubkeys_update_known_hosts(struct ssh_cmd_ctx
*cmd_ctx)
char *filename = NULL;
char *entstr;
ssize_t wret;
- mode_t old_mask;
tmp_ctx = talloc_new(NULL);
if (!tmp_ctx) {
@@ -578,12 +577,9 @@ ssh_host_pubkeys_update_known_hosts(struct ssh_cmd_ctx
*cmd_ctx)
goto done;
}
- old_mask = umask(0133);
- fd = mkstemp(filename);
- umask(old_mask);
+ fd = sss_unique_file_ex(filename, 0133, &ret);
if (fd == -1) {
filename = NULL;
- ret = errno;
goto done;
}
diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c
index
d3d00feda0bdd4048519f90ba48ae9d1042a95a1..94aa7675f3024d573fe75cdfedca37f6800b0513
100644
--- a/src/tests/cmocka/test_utils.c
+++ b/src/tests/cmocka/test_utils.c
@@ -1212,6 +1212,102 @@ void test_fix_domain_in_name_list(void **state)
talloc_free(dom);
}
+struct unique_file_test_ctx {
+ char *filename;
+};
+
+static int unique_file_test_setup(void **state)
+{
+ struct unique_file_test_ctx *test_ctx;
+
+ assert_true(leak_check_setup());
+ check_leaks_push(global_talloc_context);
+
+ test_ctx = talloc_zero(global_talloc_context, struct unique_file_test_ctx);
+ assert_non_null(test_ctx);
+
+ test_ctx->filename = talloc_strdup(test_ctx, "test_unique_file_XXXXXX");
+ assert_non_null(test_ctx);
+
+ *state = test_ctx;
+ return 0;
+}
+
+static int unique_file_test_teardown(void **state)
+{
+ struct unique_file_test_ctx *test_ctx;
+ errno_t ret;
+
+ test_ctx = talloc_get_type(*state, struct unique_file_test_ctx);
+
+ errno = 0;
+ ret = unlink(test_ctx->filename);
+ if (ret != 0 && errno != ENOENT) {
+ fail();
+ }
+
+ talloc_free(test_ctx);
+ assert_true(check_leaks_pop(global_talloc_context) == true);
+ assert_true(leak_check_teardown());
+ return 0;
+}
+
+static void test_sss_unique_file(void **state)
+{
+ struct unique_file_test_ctx *test_ctx;
+ int fd;
+ errno_t ret;
+ struct stat sb;
+
+ test_ctx = talloc_get_type(*state, struct unique_file_test_ctx);
+
+ fd = sss_unique_file(test_ctx->filename, &ret);
+ assert_int_not_equal(fd, -1);
+ assert_int_equal(ret, EOK);
+
+ ret = check_fd(fd, geteuid(), getegid(),
+ (S_IRUSR | S_IWUSR | S_IFREG), 0, &sb);
+ close(fd);
+ assert_int_equal(ret, EOK);
+}
+
+static void test_sss_unique_file_neg(void **state)
+{
+ int fd;
+ errno_t ret;
+
+ fd = sss_unique_file(discard_const("badpattern"), &ret);
+ assert_int_equal(fd, -1);
+ assert_int_equal(ret, EINVAL);
+}
+
+static void test_sss_unique_filename(void **state)
+{
+ struct unique_file_test_ctx *test_ctx;
+ int fd;
+ errno_t ret;
+ char *tmp_filename;
+
+ test_ctx = talloc_get_type(*state, struct unique_file_test_ctx);
+
+ tmp_filename = talloc_strdup(test_ctx, test_ctx->filename);
+ assert_non_null(tmp_filename);
+
+ ret = sss_unique_filename(test_ctx->filename);
+ assert_int_equal(ret, EOK);
+
+ assert_int_equal(strncmp(test_ctx->filename,
+ tmp_filename,
+ strlen(tmp_filename) - sizeof("XXXXXX")),
+ 0);
+ talloc_free(tmp_filename);
+
+ ret = check_and_open_readonly(test_ctx->filename, &fd,
+ geteuid(), getegid(),
+ (S_IRUSR | S_IWUSR | S_IFREG), 0);
+ close(fd);
+}
+
int main(int argc, const char *argv[])
{
poptContext pc;
@@ -1275,6 +1371,13 @@ int main(int argc, const char *argv[])
cmocka_unit_test_setup_teardown(test_fix_domain_in_name_list,
confdb_test_setup,
confdb_test_teardown),
+ cmocka_unit_test_setup_teardown(test_sss_unique_file,
+ unique_file_test_setup,
+ unique_file_test_teardown),
+ cmocka_unit_test(test_sss_unique_file_neg),
+ cmocka_unit_test_setup_teardown(test_sss_unique_filename,
+ unique_file_test_setup,
+ unique_file_test_teardown),
};
/* Set debug level to invalid value so we can deside if -d 0 was used. */
diff --git a/src/util/util.c b/src/util/util.c
index
782cd026b7928e607a8980fb5f333c794feb5b1a..d1210861b61365651fa949bd9b79284d6b19e2a4
100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -979,3 +979,53 @@ errno_t sss_utc_to_time_t(const char *str, const char
*format, time_t *_unix_tim
*_unix_time = ut;
return EOK;
}
+
+int sss_unique_file_ex(char *path_tmpl, mode_t file_umask, errno_t *_err)
+{
+ size_t tmpl_len;
+ errno_t ret;
+ int fd = -1;
+ mode_t old_umask;
+
+ tmpl_len = strlen(path_tmpl);
+ if (tmpl_len < 6 || strcmp(path_tmpl + (tmpl_len - 6), "XXXXXX") != 0) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Template too short or doesn't end with XXXXXX!\n");
+ ret = EINVAL;
+ goto done;
+ }
+
+ old_umask = umask(file_umask);
+ fd = mkstemp(path_tmpl);
+ umask(old_umask);
+ if (fd == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_OP_FAILURE,
+ "mkstemp(\"%s\") failed [%d]: %s!\n",
+ path_tmpl, ret, strerror(ret));
+ goto done;
+ }
+
+ ret = EOK;
+done:
+ if (_err) {
+ *_err = ret;
+ }
+ return fd;
+}
+
+int sss_unique_file(char *path_tmpl, errno_t *_err)
+{
+ return sss_unique_file_ex(path_tmpl, 077, _err);
+}
+
+errno_t sss_unique_filename(char *path_tmpl)
+{
+ int fd;
+ errno_t ret;
+
+ fd = sss_unique_file(path_tmpl, &ret);
+ /* We only care about a unique file name */
+ close(fd);
+ return ret;
+}
diff --git a/src/util/util.h b/src/util/util.h
index
94a3ddea839f0998cb7796f1d2fe13f743de3aaf..ab72eed85fb9457f50acce9ba009425c1b04e59d
100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -658,4 +658,17 @@ int get_seuser(TALLOC_CTX *mem_ctx, const char *login_name,
/* convert time from generalized form to unix time */
errno_t sss_utc_to_time_t(const char *str, const char *format, time_t
*unix_time);
+/* Creates a unique file using mkstemp with provided umask. The template
+ * must end with XXXXXX. Returns the fd, sets _err to an errno value on error.
+ *
+ * Prefer using sss_unique_file() as it uses a secure umask internally.
+ */
+int sss_unique_file_ex(char *path_tmpl, mode_t file_umask, errno_t *_err);
+int sss_unique_file(char *path_tmpl, errno_t *_err);
+
+/* Creates a unique filename using mkstemp with secure umask. The template
+ * must end with XXXXXX
+ */
+int sss_unique_filename(char *path_tmpl);
+
#endif /* __SSSD_UTIL_H__ */
--
2.4.3
>From bb67c34268196f78bc8e0258325cf349fdc97271 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Fri, 24 Jul 2015 13:13:08 +0200
Subject: [PATCH 2/2] Always re-fetch the keytab from the IPA server
Even if a keytab for one-way trust exists, re-fetch the keytab again and
try to use it. Fall back to the previous one if it exists.
This is in order to allow the admin to re-establish the trust keytabs
with a simple sssd restart.
---
Makefile.am | 2 +
src/providers/ipa/ipa_subdomains.c | 4 +-
src/providers/ipa/ipa_subdomains_server.c | 99 +++++++++++----
src/tests/cmocka/test_ipa_subdomains_server.c | 166 ++++++++++++++++++++++++--
4 files changed, 237 insertions(+), 34 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index
912bfc6641465ef5cd2ff2cce9975b4027c3218d..8d9b73c0d2099cb54de3f2b2bd6814a968f74705
100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2518,6 +2518,8 @@ test_ipa_subdom_server_LDFLAGS = \
-Wl,-wrap,krb5_kt_default \
-Wl,-wrap,execle \
-Wl,-wrap,execve \
+ -Wl,-wrap,rename \
+ -Wl,-wrap,sss_unique_filename \
$(NULL)
test_ipa_subdom_server_LDADD = \
$(PAM_LIBS) \
diff --git a/src/providers/ipa/ipa_subdomains.c
b/src/providers/ipa/ipa_subdomains.c
index
cec8b3918b8f832e2c7376a867448fe876da6ffc..b2e2fec353f7b168d28a880cb0f1b6181abb1ccb
100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -264,7 +264,7 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
ret = get_idmap_data_from_range(r, domain_name, &name1, &sid1, &rid1,
&range1, &mapping1);
if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, ("get_idmap_data_from_range failed.\n"));
+ DEBUG(SSSDBG_OP_FAILURE, "get_idmap_data_from_range failed.\n");
goto done;
}
for (d = 0; d < c; d++) {
@@ -272,7 +272,7 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
&sid2, &rid2, &range2, &mapping2);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE,
- ("get_idmap_data_from_range failed.\n"));
+ "get_idmap_data_from_range failed.\n");
goto done;
}
diff --git a/src/providers/ipa/ipa_subdomains_server.c
b/src/providers/ipa/ipa_subdomains_server.c
index
4bfea61e6dd0a02f6b723a39f7ba236c914009b0..f03965d022a93ed56e0761dd940866c4f204de40
100644
--- a/src/providers/ipa/ipa_subdomains_server.c
+++ b/src/providers/ipa/ipa_subdomains_server.c
@@ -445,6 +445,15 @@ static void ipa_getkeytab_exec(const char *ccache,
exit(1);
}
+ /* ipa-getkeytab cannot add keys to an empty file, let's unlink it and only
+ * use the filename */
+ ret = unlink(keytab_path);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE, "Failed to unlink the temporary ccname!\n");
+ exit(1);
+ }
+
errno = 0;
ret = execle(IPA_GETKEYTAB_PATH, IPA_GETKEYTAB_PATH,
"-r", "-s", server, "-p", principal, "-k", keytab_path, NULL,
@@ -561,6 +570,7 @@ struct ipa_server_trust_add_state {
uint32_t direction;
const char *forest;
const char *keytab;
+ char *new_keytab;
const char *principal;
const char *forest_realm;
const char *ccache;
@@ -646,6 +656,22 @@ immediate:
return req;
}
+static int ipa_server_trust_rm_tmp_file(void *memptr)
+{
+ errno_t ret;
+ struct ipa_server_trust_add_state *state =
+ talloc_get_type(memptr, struct ipa_server_trust_add_state);
+
+ ret = unlink(state->new_keytab);
+ if (ret != 0 && errno != ENOENT) {
+ ret = errno;
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Cannot remove temporary keytab: %d\n", ret);
+ }
+
+ return 0;
+}
+
static errno_t ipa_server_trust_add_1way(struct tevent_req *req)
{
errno_t ret;
@@ -660,21 +686,22 @@ static errno_t ipa_server_trust_add_1way(struct
tevent_req *req)
return EIO;
}
- ret = ipa_check_keytab(state->keytab,
- state->id_ctx->server_mode->kt_owner_uid,
- state->id_ctx->server_mode->kt_owner_gid);
- if (ret == EOK) {
- DEBUG(SSSDBG_TRACE_FUNC,
- "Keytab already present, can add the trust\n");
- return EOK;
- } else if (ret != ENOENT) {
- DEBUG(SSSDBG_OP_FAILURE,
- "Failed to check for keytab: %d\n", ret);
+ state->new_keytab = talloc_asprintf(state, "%sXXXXXX", state->keytab);
+ if (state->new_keytab == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Cannot set up ipa_get_keytab\n");
+ return ENOMEM;
+ }
+
+ ret = sss_unique_filename(state->new_keytab);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Cannot create temporary keytab name\n");
return ret;
}
+ /* Make sure the file is always removed */
+ talloc_set_destructor((TALLOC_CTX *) state, ipa_server_trust_rm_tmp_file);
DEBUG(SSSDBG_TRACE_FUNC,
- "No keytab for %s\n", state->subdom->name);
+ "Will re-fetch keytab for %s\n", state->subdom->name);
hostname = dp_opt_get_string(state->id_ctx->ipa_options->basic,
IPA_HOSTNAME);
@@ -691,7 +718,7 @@ static errno_t ipa_server_trust_add_1way(struct tevent_req
*req)
state->ccache,
hostname,
state->principal,
- state->keytab);
+ state->new_keytab);
if (subreq == NULL) {
return ENOMEM;
}
@@ -710,23 +737,49 @@ static void ipa_server_trust_1way_kt_done(struct
tevent_req *subreq)
ret = ipa_getkeytab_recv(subreq, NULL);
talloc_zfree(subreq);
if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, "ipa_getkeytab_recv failed: %d\n", ret);
- tevent_req_error(req, ret);
- return;
+ /* Do not fail here, but try to check and use the previous keytab,
+ * if any */
+ DEBUG(SSSDBG_MINOR_FAILURE, "ipa_getkeytab_recv failed: %d\n", ret);
+ } else {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Keytab successfully retrieved to %s\n", state->new_keytab);
}
- DEBUG(SSSDBG_TRACE_FUNC,
- "Keytab successfully retrieved to %s\n", state->keytab);
-
- ret = ipa_check_keytab(state->keytab,
+ ret = ipa_check_keytab(state->new_keytab,
state->id_ctx->server_mode->kt_owner_uid,
state->id_ctx->server_mode->kt_owner_gid);
- if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, "ipa_check_keytab failed: %d\n", ret);
- tevent_req_error(req, ret);
- return;
+ if (ret == EOK) {
+ ret = rename(state->new_keytab, state->keytab);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "rename failed [%d][%s].\n", ret, strerror(ret));
+ tevent_req_error(req, ret);
+ return;
+ }
+ DEBUG(SSSDBG_TRACE_INTERNAL, "Keytab renamed to %s\n", state->keytab);
+ } else if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "Trying to recover and use the previous keytab, if available\n");
+ ret = ipa_check_keytab(state->keytab,
+ state->id_ctx->server_mode->kt_owner_uid,
+ state->id_ctx->server_mode->kt_owner_gid);
+ if (ret == EOK) {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "The previous keytab %s contains the expected principal\n",
+ state->keytab);
+ } else {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Cannot use the old keytab: %d\n", ret);
+ /* Nothing we can do now */
+ tevent_req_error(req, ret);
+ return;
+ }
}
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Keytab %s contains the expected principals\n", state->new_keytab);
+
ret = ipa_server_trust_add_step(req);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE,
diff --git a/src/tests/cmocka/test_ipa_subdomains_server.c
b/src/tests/cmocka/test_ipa_subdomains_server.c
index
a4cb8c2b7538dc84b74e0227205b73780844b652..1d433caa732d5685cc0d8ad6f07752c29674ce8b
100644
--- a/src/tests/cmocka/test_ipa_subdomains_server.c
+++ b/src/tests/cmocka/test_ipa_subdomains_server.c
@@ -66,33 +66,79 @@
#define ONEWAY_PRINC DOM_FLAT"$"
#define ONEWAY_AUTHID ONEWAY_PRINC"@"SUBDOM_REALM
+static bool global_rename_called;
+
krb5_error_code __wrap_krb5_kt_default(krb5_context context, krb5_keytab *id)
{
return krb5_kt_resolve(context, KEYTAB_PATH, id);
}
-static void create_dummy_keytab(void)
+static void create_dummy_keytab(const char *dummy_kt)
{
errno_t ret;
- assert_non_null(ONEWAY_KEYTAB);
+ assert_non_null(dummy_kt);
mock_keytab_with_contents(global_talloc_context,
- ONEWAY_KEYTAB, ONEWAY_AUTHID);
+ dummy_kt, ONEWAY_AUTHID);
- ret = access(ONEWAY_KEYTAB, R_OK);
+ ret = access(dummy_kt, R_OK);
assert_int_equal(ret, 0);
}
+static int wrap_exec(void)
+{
+ const char *test_kt;
+ const char *fail_creating_kt;
+
+ test_kt = getenv("TEST_KT_ENV");
+ if (test_kt == NULL) {
+ _exit(1);
+ }
+ unsetenv("TEST_KT_ENV");
+
+ fail_creating_kt = getenv("KT_CREATE_FAIL");
+ if (fail_creating_kt != NULL) {
+ _exit(1);
+ }
+
+ create_dummy_keytab(test_kt);
+ _exit(0);
+
+ return 1; /* Should not happen */
+}
+
int __wrap_execle(const char *path, const char *arg, ...)
{
- create_dummy_keytab();
- _exit(0);
+ return wrap_exec();
}
int __wrap_execve(const char *path, const char *arg, ...)
{
- create_dummy_keytab();
- _exit(0);
+ return wrap_exec();
+}
+
+errno_t __real_sss_unique_filename(char *path_tmpl);
+
+errno_t __wrap_sss_unique_filename(char *path_tmpl)
+{
+ int ret;
+
+ ret = __real_sss_unique_filename(path_tmpl);
+ if (ret == EOK) {
+ int sret;
+
+ sret = setenv("TEST_KT_ENV", path_tmpl, 1);
+ assert_int_equal(sret, 0);
+ }
+ return ret;
+}
+
+int __real_rename(const char *old, const char *new);
+
+int __wrap_rename(const char *old, const char *new)
+{
+ global_rename_called = true;
+ return __real_rename(old, new);
}
struct trust_test_ctx {
@@ -100,6 +146,7 @@ struct trust_test_ctx {
struct be_ctx *be_ctx;
struct ipa_id_ctx *ipa_ctx;
+ bool expect_rename;
};
static struct ipa_id_ctx *mock_ipa_ctx(TALLOC_CTX *mem_ctx,
@@ -244,6 +291,8 @@ static int test_ipa_server_create_trusts_setup(void **state)
mock_keytab_with_contents(test_ctx, KEYTAB_PATH, KEYTAB_TEST_PRINC);
+ global_rename_called = false;
+
*state = test_ctx;
return 0;
}
@@ -260,6 +309,11 @@ static int test_ipa_server_create_trusts_teardown(void
**state)
unlink(ONEWAY_KEYTAB);
/* Ignore failures */
+ /* If a test needs this variable, it should be set again in
+ * each test
+ */
+ unsetenv("KT_CREATE_FAIL");
+
talloc_free(test_ctx);
return 0;
}
@@ -612,6 +666,8 @@ static void test_ipa_server_create_oneway(void **state)
assert_null(test_ctx->ipa_ctx->server_mode->trusts);
+ test_ctx->expect_rename = true;
+
req = ipa_server_create_trusts_send(test_ctx,
test_ctx->tctx->ev,
test_ctx->be_ctx,
@@ -635,6 +691,8 @@ static void test_ipa_server_create_trusts_oneway(struct
tevent_req *req)
talloc_zfree(req);
assert_int_equal(ret, EOK);
+ assert_true(test_ctx->expect_rename == global_rename_called);
+
ret = access(ONEWAY_KEYTAB, R_OK);
assert_int_equal(ret, 0);
@@ -674,10 +732,46 @@ static void test_ipa_server_create_oneway_kt_exists(void
**state)
add_test_1way_subdomains(test_ctx);
- create_dummy_keytab();
+ create_dummy_keytab(ONEWAY_KEYTAB);
ret = access(ONEWAY_KEYTAB, R_OK);
assert_int_equal(ret, 0);
+ test_ctx->expect_rename = true;
+
+ assert_null(test_ctx->ipa_ctx->server_mode->trusts);
+
+ req = ipa_server_create_trusts_send(test_ctx,
+ test_ctx->tctx->ev,
+ test_ctx->be_ctx,
+ test_ctx->ipa_ctx,
+ test_ctx->be_ctx->domain);
+ assert_non_null(req);
+
+ tevent_req_set_callback(req, test_ipa_server_create_trusts_oneway,
test_ctx);
+
+ ret = test_ev_loop(test_ctx->tctx);
+ assert_int_equal(ret, ERR_OK);
+}
+
+/* Test scenario where a keytab already exists, but refresh fails. In this
case,
+ * sssd should attempt to reuse the previous keytab
+ */
+static void test_ipa_server_create_oneway_kt_refresh_fallback(void **state)
+{
+ struct trust_test_ctx *test_ctx =
+ talloc_get_type(*state, struct trust_test_ctx);
+ struct tevent_req *req;
+ errno_t ret;
+
+ add_test_1way_subdomains(test_ctx);
+
+ create_dummy_keytab(ONEWAY_KEYTAB);
+ ret = access(ONEWAY_KEYTAB, R_OK);
+ assert_int_equal(ret, 0);
+
+ setenv("KT_CREATE_FAIL", "1", 1);
+ test_ctx->expect_rename = false;
+
assert_null(test_ctx->ipa_ctx->server_mode->trusts);
req = ipa_server_create_trusts_send(test_ctx,
@@ -693,6 +787,54 @@ static void test_ipa_server_create_oneway_kt_exists(void
**state)
assert_int_equal(ret, ERR_OK);
}
+/* Tests case where there's no keytab and retrieving fails. Just fail the
+ * request in that case
+ */
+static void test_ipa_server_create_trusts_oneway_fail(struct tevent_req *req);
+
+static void test_ipa_server_create_oneway_kt_refresh_fail(void **state)
+{
+ struct trust_test_ctx *test_ctx =
+ talloc_get_type(*state, struct trust_test_ctx);
+ struct tevent_req *req;
+ errno_t ret;
+
+ add_test_1way_subdomains(test_ctx);
+
+ setenv("KT_CREATE_FAIL", "1", 1);
+ test_ctx->expect_rename = false;
+
+ assert_null(test_ctx->ipa_ctx->server_mode->trusts);
+
+ req = ipa_server_create_trusts_send(test_ctx,
+ test_ctx->tctx->ev,
+ test_ctx->be_ctx,
+ test_ctx->ipa_ctx,
+ test_ctx->be_ctx->domain);
+ assert_non_null(req);
+
+ tevent_req_set_callback(req,
+ test_ipa_server_create_trusts_oneway_fail,
+ test_ctx);
+
+ ret = test_ev_loop(test_ctx->tctx);
+ assert_int_equal(ret, ERR_OK);
+}
+
+static void test_ipa_server_create_trusts_oneway_fail(struct tevent_req *req)
+{
+ struct trust_test_ctx *test_ctx = \
+ tevent_req_callback_data(req, struct trust_test_ctx);
+ errno_t ret;
+
+ ret = ipa_server_create_trusts_recv(req);
+ assert_int_not_equal(ret, EOK);
+
+ assert_true(test_ctx->expect_rename == global_rename_called);
+
+ test_ev_done(test_ctx->tctx, EOK);
+}
+
static void test_ipa_server_trust_oneway_init(void **state)
{
struct trust_test_ctx *test_ctx =
@@ -749,6 +891,12 @@ int main(int argc, const char *argv[])
cmocka_unit_test_setup_teardown(test_ipa_server_create_oneway_kt_exists,
test_ipa_server_create_trusts_setup,
test_ipa_server_create_trusts_teardown),
+
cmocka_unit_test_setup_teardown(test_ipa_server_create_oneway_kt_refresh_fallback,
+ test_ipa_server_create_trusts_setup,
+
test_ipa_server_create_trusts_teardown),
+
cmocka_unit_test_setup_teardown(test_ipa_server_create_oneway_kt_refresh_fail,
+ test_ipa_server_create_trusts_setup,
+
test_ipa_server_create_trusts_teardown),
cmocka_unit_test_setup_teardown(test_ipa_server_trust_oneway_init,
test_ipa_server_create_trusts_setup,
test_ipa_server_create_trusts_teardown),
--
2.4.3
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
