[SSSD] [PATCH] Fetch one-way trust keytabs on sssd restart again

Jakub Hrozek jhrozek at redhat.com
Wed Aug 12 12:20:23 UTC 2015


On Fri, Aug 07, 2015 at 12:22:39PM +0200, Pavel Březina wrote:
> On 07/30/2015 09:52 PM, Jakub Hrozek wrote:
> >On Thu, Jul 30, 2015 at 09:46:11PM +0200, Jakub Hrozek wrote:
> >>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.
> >
> >Sorry, I shouldn't be sending patches before Coverity results arrive.
> >Attached version fixes error handling in the first patch and fixes an
> >unused variable in the second one.
> 
> Hi,
> the code looks good. I just have an idea to move the talloc destructor that
> ensure the temporary file will get unlinked into sss_unique_file.
> 
> We can provide a talloc context there and setup a destructor if requested.
> Something like:
> 
> sss_unique_file(owner, file)
>   if owner != NULL
>      talloc_set_destructor

Hi,

please see the attached patches. Since the unique file code is not
totally trivial (even though tested) I will move using the
sss_unique_file() interface in other sssd code into a different patchset
-- I would like to apply these patches to downstream and changing the
mkstemp() calls might be too risky there.
-------------- next part --------------
>From 785af2aa94691c96e56caa94420b83c5ea346cfc Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 12 Aug 2015 12:41:44 +0200
Subject: [PATCH 1/2] UTIL: Provide a common interface to safely create
 temporary files

---
 src/tests/cmocka/test_utils.c | 175 ++++++++++++++++++++++++++++++++++++++++++
 src/util/util.c               | 127 ++++++++++++++++++++++++++++++
 src/util/util.h               |  21 +++++
 3 files changed, 323 insertions(+)

diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c
index d3d00feda0bdd4048519f90ba48ae9d1042a95a1..c7ebe0997ec00197e8852bedbcf26ef1f6394fc3 100644
--- a/src/tests/cmocka/test_utils.c
+++ b/src/tests/cmocka/test_utils.c
@@ -1212,6 +1212,168 @@ 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 assert_destructor(TALLOC_CTX *owner,
+                              struct unique_file_test_ctx *test_ctx)
+{
+    int fd;
+    errno_t ret;
+    char *check_filename;
+
+    /* Test that the destructor works */
+    if (owner == NULL) {
+        return;
+    }
+
+    check_filename = talloc_strdup(test_ctx, test_ctx->filename);
+    assert_non_null(check_filename);
+
+    talloc_free(owner);
+
+    ret = check_and_open_readonly(test_ctx->filename, &fd,
+                                  geteuid(), getegid(),
+                                  (S_IRUSR | S_IWUSR | S_IFREG), 0);
+    close(fd);
+    assert_int_not_equal(ret, EOK);
+}
+
+static void sss_unique_file_test(struct unique_file_test_ctx *test_ctx,
+                                 bool test_destructor)
+{
+    int fd;
+    errno_t ret;
+    struct stat sb;
+    TALLOC_CTX *owner = NULL;
+
+    if (test_destructor) {
+        owner = talloc_new(test_ctx);
+        assert_non_null(owner);
+    }
+
+    fd = sss_unique_file(owner, 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);
+
+    assert_destructor(owner, test_ctx);
+}
+
+static void test_sss_unique_file(void **state)
+{
+    struct unique_file_test_ctx *test_ctx;
+    test_ctx = talloc_get_type(*state, struct unique_file_test_ctx);
+    sss_unique_file_test(test_ctx, false);
+}
+
+static void test_sss_unique_file_destruct(void **state)
+{
+    struct unique_file_test_ctx *test_ctx;
+    test_ctx = talloc_get_type(*state, struct unique_file_test_ctx);
+    sss_unique_file_test(test_ctx, true);
+}
+
+static void test_sss_unique_file_neg(void **state)
+{
+    int fd;
+    errno_t ret;
+
+    fd = sss_unique_file(NULL, discard_const("badpattern"), &ret);
+    assert_int_equal(fd, -1);
+    assert_int_equal(ret, EINVAL);
+}
+
+static void sss_unique_filename_test(struct unique_file_test_ctx *test_ctx,
+                                     bool test_destructor)
+{
+    int fd;
+    errno_t ret;
+    char *tmp_filename;
+    TALLOC_CTX *owner = NULL;
+
+    tmp_filename = talloc_strdup(test_ctx, test_ctx->filename);
+    assert_non_null(tmp_filename);
+
+    if (test_destructor) {
+        owner = talloc_new(test_ctx);
+        assert_non_null(owner);
+    }
+
+    ret = sss_unique_filename(owner, test_ctx->filename);
+    assert_int_equal(ret, EOK);
+
+    assert_int_equal(strncmp(test_ctx->filename,
+                             tmp_filename,
+                             strlen(tmp_filename) - sizeof("XXXXXX")),
+                     0);
+
+    ret = check_and_open_readonly(test_ctx->filename, &fd,
+                                  geteuid(), getegid(),
+                                  (S_IRUSR | S_IWUSR | S_IFREG), 0);
+    close(fd);
+    assert_int_equal(ret, EOK);
+
+    assert_destructor(owner, test_ctx);
+}
+
+static void test_sss_unique_filename(void **state)
+{
+    struct unique_file_test_ctx *test_ctx;
+
+    test_ctx = talloc_get_type(*state, struct unique_file_test_ctx);
+    sss_unique_filename_test(test_ctx, false);
+}
+
+static void test_sss_unique_filename_destruct(void **state)
+{
+    struct unique_file_test_ctx *test_ctx;
+
+    test_ctx = talloc_get_type(*state, struct unique_file_test_ctx);
+    sss_unique_filename_test(test_ctx, true);
+}
+
 int main(int argc, const char *argv[])
 {
     poptContext pc;
@@ -1275,6 +1437,19 @@ 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_setup_teardown(test_sss_unique_file_destruct,
+                                        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),
+        cmocka_unit_test_setup_teardown(test_sss_unique_filename_destruct,
+                                        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..f0925051a08970b191f61a533cd49dd53ae128e0 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -979,3 +979,130 @@ errno_t sss_utc_to_time_t(const char *str, const char *format, time_t *_unix_tim
     *_unix_time = ut;
     return EOK;
 }
+
+struct tmpfile_watch {
+    const char *filename;
+};
+
+static int unlink_dbg(const char *filename)
+{
+    errno_t ret;
+
+    ret = unlink(filename);
+    if (ret != 0) {
+        if (errno == 2) {
+            DEBUG(SSSDBG_TRACE_INTERNAL,
+                  "File already removed: [%s]\n", filename);
+            return 0;
+        } else {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "Cannot remove temporary file [%s]\n", filename);
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+static int unique_filename_destructor(void *memptr)
+{
+    struct tmpfile_watch *tw = talloc_get_type(memptr, struct tmpfile_watch);
+
+    if (tw == NULL || tw->filename == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "BUG: Wrong private pointer\n");
+        return -1;
+    }
+
+    DEBUG(SSSDBG_TRACE_INTERNAL, "Unlinking [%s]\n", tw->filename);
+
+    return unlink_dbg(tw->filename);
+}
+
+static struct tmpfile_watch *tmpfile_watch_set(TALLOC_CTX *owner,
+                                               const char *filename)
+{
+    struct tmpfile_watch *tw = NULL;
+
+    tw = talloc_zero(owner, struct tmpfile_watch);
+    if (tw == NULL) {
+        return NULL;
+    }
+
+    tw->filename = talloc_strdup(tw, filename);
+    if (tw->filename == NULL) {
+        talloc_free(tw);
+        return NULL;
+    }
+
+    talloc_set_destructor((TALLOC_CTX *) tw,
+                          unique_filename_destructor);
+    return tw;
+}
+
+int sss_unique_file_ex(TALLOC_CTX *owner,
+                       char *path_tmpl,
+                       mode_t file_umask,
+                       errno_t *_err)
+{
+    size_t tmpl_len;
+    errno_t ret;
+    int fd = -1;
+    mode_t old_umask;
+    struct tmpfile_watch *tw = NULL;
+
+    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;
+    }
+
+    if (owner != NULL) {
+        tw = tmpfile_watch_set(owner, path_tmpl);
+        if (tw == NULL) {
+            unlink_dbg(path_tmpl);
+            ret = ENOMEM;
+            goto done;
+        }
+    }
+
+    ret = EOK;
+done:
+    if (_err) {
+        *_err = ret;
+    }
+    return fd;
+}
+
+int sss_unique_file(TALLOC_CTX *owner,
+                    char *path_tmpl,
+                    errno_t *_err)
+{
+    return sss_unique_file_ex(owner, path_tmpl, 077, _err);
+}
+
+errno_t sss_unique_filename(TALLOC_CTX *owner, char *path_tmpl)
+{
+    int fd;
+    errno_t ret;
+
+    fd = sss_unique_file(owner, path_tmpl, &ret);
+    /* We only care about a unique file name */
+    if (fd >= 0) {
+        close(fd);
+    }
+
+    return ret;
+}
diff --git a/src/util/util.h b/src/util/util.h
index 94a3ddea839f0998cb7796f1d2fe13f743de3aaf..a20d1d82eb8f10dac515ad25e7e424713bb1c099 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -658,4 +658,25 @@ 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(TALLOC_CTX *mem_ctx,
+                       char *path_tmpl,
+                       mode_t file_umask,
+                       errno_t *_err);
+int sss_unique_file(TALLOC_CTX *owner,
+                    char *path_tmpl,
+                    errno_t *_err);
+
+/* Creates a unique filename using mkstemp with secure umask. The template
+ * must end with XXXXXX
+ *
+ * path_tmpl must be a talloc context. Destructor would be set on the filename
+ * so that it's guaranteed the file is removed.
+ */
+int sss_unique_filename(TALLOC_CTX *owner, char *path_tmpl);
+
 #endif /* __SSSD_UTIL_H__ */
-- 
2.4.3

-------------- next part --------------
>From 8d214b67a8f4bfa0ddef1c2ba35c8c0cf2f233cc Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 24 Jul 2015 13:13:08 +0200
Subject: [PATCH 2/2] IPA: 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     |  83 +++++++++----
 src/tests/cmocka/test_ipa_subdomains_server.c | 166 ++++++++++++++++++++++++--
 4 files changed, 221 insertions(+), 34 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 8b64317d6dce9a1ee8614916395b9afd9f11f382..ed107fd5dc76b768176a3d7236b0bf1c75f212bf 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2550,6 +2550,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..dfecab1bc362b5772379bae6d51f9cef8443f225 100644
--- a/src/providers/ipa/ipa_subdomains_server.c
+++ b/src/providers/ipa/ipa_subdomains_server.c
@@ -445,6 +445,17 @@ 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 [%d][%s]\n",
+              ret, sss_strerror(ret));
+        exit(1);
+    }
+
     errno = 0;
     ret = execle(IPA_GETKEYTAB_PATH, IPA_GETKEYTAB_PATH,
                  "-r", "-s", server, "-p", principal, "-k", keytab_path, NULL,
@@ -561,6 +572,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;
@@ -660,21 +672,20 @@ 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, state->new_keytab);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Cannot create temporary keytab name\n");
         return ret;
     }
 
     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 +702,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 +721,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..fb9bd80e299c05fa230599d442fa361ae757dcd3 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(TALLOC_CTX *owner, char *path_tmpl);
+
+errno_t __wrap_sss_unique_filename(TALLOC_CTX *owner, char *path_tmpl)
+{
+    int ret;
+    int sret;
+
+    ret = __real_sss_unique_filename(owner, path_tmpl);
+    if (ret == EOK) {
+
+        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



More information about the sssd-devel mailing list