[SSSD] [PATCH] Recreate FAST ccache as the sssd user

Jakub Hrozek jhrozek at redhat.com
Tue Dec 2 21:03:54 UTC 2014


On Tue, Dec 02, 2014 at 09:40:42PM +0100, Sumit Bose wrote:
> On Tue, Dec 02, 2014 at 02:54:42PM +0100, Jakub Hrozek wrote:
> > Hi,
> > 
> > these patches depend on Sumit's "[PATCHES] ldap_child, krb5_child: copy
> > keytab and FAST ccache into memory".
> > 
> > When applied, the FAST ccache is created as the SSSD so that no Kerberos
> > networking code runs as the root user. In order to do that, the
> > krb5_child receives the SSSD user IDs as parameters.
> > 
> > I also have tests for the other functions in the child_common.c module
> > but I will send them separately to speed up the review.
> 
> Patches are working well in my tests, I have a few comments to the 3rd
> patch.

[...]

> > From 74ae76ecea1d469ff9dc8937eacdd077707352b9 Mon Sep 17 00:00:00 2001
> > From: Jakub Hrozek <jhrozek at redhat.com>
> > Date: Fri, 28 Nov 2014 13:04:42 +0100
> > Subject: [PATCH 3/3] KRB5: Create the fast ccache in a child process

[...]

> > -    kerr = get_and_save_tgt_with_keytab(ctx, client_princ, keytab, ccname);
> > -    if (kerr != 0) {
> > -        DEBUG(SSSDBG_CRIT_FAILURE, "get_and_save_tgt_with_keytab failed.\n");
> > -        goto done;
> > +    /* Need to recreate the FAST ccache */
> > +    fchild_pid = fork();
> > +    switch (fchild_pid) {
> > +        case -1:
> > +            DEBUG(SSSDBG_CRIT_FAILURE, "fork failed\n");
> > +            kerr = EIO;
> > +            goto done;
> > +        case 0:
> > +            /* Child */
> 
> please change debug_prg_name for the child

Done. I hope keeping the same program name is OK since we just fork the
same process.

> 
> > +            kerr = become_user(fast_uid, fast_gid);
> > +            if (kerr != 0) {
> > +                DEBUG(SSSDBG_CRIT_FAILURE, "become_user failed: %d\n", kerr);
> > +                exit(1);
> > +            }
> > +            DEBUG(SSSDBG_TRACE_INTERNAL,
> > +                  "Running as [%"SPRIuid"][%"SPRIgid"].\n", geteuid(), getegid());
> > +
> > +            kerr = get_and_save_tgt_with_keytab(ctx, client_princ,
> > +                                                keytab, ccname);
> > +            if (kerr != 0) {
> > +                DEBUG(SSSDBG_CRIT_FAILURE,
> > +                      "get_and_save_tgt_with_keytab failed: %d\n", kerr);
> > +                exit(2);
> > +            }
> > +            exit(0);
> > +        default:
> > +            /* Parent */
> > +            do {
> > +                errno = 0;
> > +                kerr = waitpid(fchild_pid, &status, 0);
> > +            } while (kerr == -1 && errno == EINTR);
> > +
> > +            if (kerr > 0) {
> > +                kerr = EIO;
> > +                if (WIFEXITED(status)) {
> > +                    kerr = WEXITSTATUS(status);
> > +                    /* Don't blindly fail if the child fails, but check
> > +                     * the ccache again */
> > +                    if (kerr != 0) {
> > +                        DEBUG(SSSDBG_MINOR_FAILURE,
> > +                              "Creating FAST ccache failed, krb5_child will "
> > +                              "likely fail!\n");
> > +                    }
> > +                }
> 
> please add and else with a message the the child terminated
> unexpectedly.

Done.

> 
> > +            } else {
> > +                DEBUG(SSSDBG_FUNC_DATA,
> > +                    "Failed to wait for children %d\n", fchild_pid);
> > +                kerr = EIO;
> > +            }
> > +    }
> > +
> > +    /* Check the ccache times again. Should be updated ... */
> > +    memset(&tgtt, 0, sizeof(tgtt));
> > +    kerr = get_tgt_times(ctx, ccname, server_princ, client_princ, &tgtt);
> > +    if (kerr == 0) {
> > +        if (tgtt.endtime > time(NULL)) {
> > +            DEBUG(SSSDBG_FUNC_DATA, "FAST TGT was successfully recreated!\n");
> > +            goto done;
> > +        } else {
> > +            kerr = ERR_CREDS_EXPIRED;
> > +            goto done;
> > +        }
> >      }
> >  
> >      kerr = 0;
> > @@ -1889,7 +1932,8 @@ static int k5c_setup_fast(struct krb5_req *kr, bool demand)
> >          fast_principal = NULL;
> >      }
> >  
> > -    kerr = check_fast_ccache(kr, kr->ctx, fast_principal, fast_principal_realm,
> > +    kerr = check_fast_ccache(kr, kr->ctx, kr->fast_uid, kr->fast_gid,
> > +                             fast_principal, fast_principal_realm,
> >                               kr->keytab, &kr->fast_ccname);
> >      if (kerr != 0) {
> >          DEBUG(SSSDBG_CRIT_FAILURE, "check_fast_ccache failed.\n");
> > @@ -2253,6 +2297,9 @@ int main(int argc, const char *argv[])
> >      int debug_fd = -1;
> >      errno_t ret;
> >      krb5_error_code kerr;
> > +    char *mem_keytab;
> 
> unused

Oops, bad merge with your patches. Fixed.

> 
> > +    uid_t fast_uid;
> > +    gid_t fast_gid;
> >  
> >      struct poptOption long_options[] = {
> >          POPT_AUTOHELP
> > @@ -2267,6 +2314,10 @@ int main(int argc, const char *argv[])
> >          {"debug-to-stderr", 0, POPT_ARG_NONE | POPT_ARGFLAG_DOC_HIDDEN,
> >           &debug_to_stderr, 0,
> >           _("Send the debug output to stderr directly."), NULL },
> > +        {"fast-ccache-uid", 0, POPT_ARG_INT, &fast_uid, 0,
> > +          _("The user to create FAST ccache as"), NULL},
> > +        {"fast-ccache-gid", 0, POPT_ARG_INT, &fast_gid, 0,
> > +          _("The group to create FAST ccache as"), NULL},
> >          POPT_TABLEEND
> >      };
> >  

Thank you for the review. New patches are attached.
-------------- next part --------------
>From dbaa78b01d991b24492465294eb1fcf779af4949 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 1 Dec 2014 12:04:25 +0100
Subject: [PATCH 1/3] TESTS: Basic child tests

---
 Makefile.am                          |  25 ++++++
 src/tests/cmocka/test_child.c        |  83 +++++++++++++++++++
 src/tests/cmocka/test_child_common.c | 150 +++++++++++++++++++++++++++++++++++
 3 files changed, 258 insertions(+)
 create mode 100644 src/tests/cmocka/test_child.c
 create mode 100644 src/tests/cmocka/test_child_common.c

diff --git a/Makefile.am b/Makefile.am
index a903e350e1fd6988c3c5bb7c9a0e202a465c68c5..f84189c6d7ebe4baf6dfb3aad1ac776012e9e7cc 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -217,6 +217,7 @@ if HAVE_CMOCKA
         test_be_ptask \
         test_copy_ccache \
         test_copy_keytab \
+        test_child_common \
         $(NULL)
 
 if BUILD_IFP
@@ -234,6 +235,7 @@ endif   # HAVE_CMOCKA
 check_PROGRAMS = \
     stress-tests \
     krb5-child-test \
+    test-child \
     $(non_interactive_cmocka_based_tests) \
     $(non_interactive_check_based_tests)
 
@@ -2132,6 +2134,29 @@ test_copy_keytab_LDADD = \
     libsss_test_common.la \
     $(NULL)
 
+test_child_SOURCES = \
+    src/tests/cmocka/test_child.c \
+    $(NULL)
+test_child_LDADD = \
+    $(POPT_LIBS) \
+    $(SSSD_INTERNAL_LTLIBS) \
+    $(NULL)
+
+test_child_common_SOURCES = \
+    src/tests/cmocka/test_child_common.c \
+    $(NULL)
+test_child_common_CFLAGS = \
+    $(AM_CFLAGS) \
+    -DCHILD_DIR=\"$(builddir)\" \
+    $(NULL)
+test_child_common_LDADD = \
+    $(CMOCKA_LIBS) \
+    $(POPT_LIBS) \
+    $(TALLOC_LIBS) \
+    $(SSSD_INTERNAL_LTLIBS) \
+    libsss_test_common.la \
+    $(NULL)
+
 endif # HAVE_CMOCKA
 
 noinst_PROGRAMS = pam_test_client
diff --git a/src/tests/cmocka/test_child.c b/src/tests/cmocka/test_child.c
new file mode 100644
index 0000000000000000000000000000000000000000..a857afce1fe7c4f3949fe77fe4e0a24e15961a4f
--- /dev/null
+++ b/src/tests/cmocka/test_child.c
@@ -0,0 +1,83 @@
+/*
+    SSSD
+
+    Tests -- a simple test process that echoes input back
+
+    Authors:
+        Jakub Hrozek <jhrozek at redhat.com>
+
+    Copyright (C) 2014 Red Hat
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <sys/types.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <popt.h>
+
+#include "util/util.h"
+#include "util/child_common.h"
+
+int main(int argc, const char *argv[])
+{
+    int opt;
+    int debug_fd = -1;
+    poptContext pc;
+    const char *action;
+    ssize_t len = 0;
+    ssize_t written;
+    errno_t ret;
+    uint8_t *buf[IN_BUF_SIZE];
+    uid_t uid;
+    gid_t gid;
+
+    struct poptOption long_options[] = {
+        POPT_AUTOHELP
+        {"debug-level", 'd', POPT_ARG_INT, &debug_level, 0,
+         _("Debug level"), NULL},
+        {"debug-timestamps", 0, POPT_ARG_INT, &debug_timestamps, 0,
+         _("Add debug timestamps"), NULL},
+        {"debug-microseconds", 0, POPT_ARG_INT, &debug_microseconds, 0,
+         _("Show timestamps with microseconds"), NULL},
+        {"debug-fd", 0, POPT_ARG_INT, &debug_fd, 0,
+         _("An open file descriptor for the debug logs"), NULL},
+        {"debug-to-stderr", 0, POPT_ARG_NONE | POPT_ARGFLAG_DOC_HIDDEN, &debug_to_stderr, 0, \
+         _("Send the debug output to stderr directly."), NULL }, \
+        SSSD_SERVER_OPTS(uid, gid)
+        POPT_TABLEEND
+    };
+
+    /* Set debug level to invalid value so we can decide if -d 0 was used. */
+    debug_level = SSSDBG_INVALID;
+
+    pc = poptGetContext(argv[0], argc, argv, long_options, 0);
+    while((opt = poptGetNextOpt(pc)) != -1) {
+        switch(opt) {
+        default:
+        fprintf(stderr, "\nInvalid option %s: %s\n\n",
+                  poptBadOption(pc, 0), poptStrerror(opt));
+            poptPrintUsage(pc, stderr, 0);
+            _exit(1);
+        }
+    }
+
+    DEBUG(SSSDBG_TRACE_FUNC, "test_child completed successfully\n");
+    _exit(0);
+
+fail:
+    DEBUG(SSSDBG_TRACE_FUNC, "test_child completed successfully\n");
+    close(STDOUT_FILENO);
+    _exit(-1);
+}
diff --git a/src/tests/cmocka/test_child_common.c b/src/tests/cmocka/test_child_common.c
new file mode 100644
index 0000000000000000000000000000000000000000..f2cd8c0081c2e85432db1c9696102780dc990e75
--- /dev/null
+++ b/src/tests/cmocka/test_child_common.c
@@ -0,0 +1,150 @@
+/*
+    Authors:
+        Jakub Hrozek <jhrozek at redhat.com>
+
+    Copyright (C) 2014 Red Hat
+
+    SSSD tests: Child handlers
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <talloc.h>
+#include <tevent.h>
+#include <errno.h>
+#include <popt.h>
+
+#include "util/child_common.h"
+#include "tests/cmocka/common_mock.h"
+
+#define TEST_BIN    "test-child"
+#define ECHO_STR    "Hello child"
+
+struct child_test_ctx {
+    int pipefd_to_child[2];
+    int pipefd_from_child[2];
+
+    struct sss_test_ctx *test_ctx;
+};
+
+void child_test_setup(void **state)
+{
+    struct child_test_ctx *child_tctx;
+    errno_t ret;
+
+    check_leaks_push(global_talloc_context);
+    child_tctx = talloc(global_talloc_context, struct child_test_ctx);
+    assert_non_null(child_tctx);
+
+    child_tctx->test_ctx = create_ev_test_ctx(child_tctx);
+    assert_non_null(child_tctx->test_ctx);
+
+    ret = pipe(child_tctx->pipefd_from_child);
+    assert_int_not_equal(ret, -1);
+    DEBUG(SSSDBG_TRACE_LIBS, "from_child: %d:%d\n",
+                             child_tctx->pipefd_from_child[0],
+                             child_tctx->pipefd_from_child[1]);
+
+    ret = pipe(child_tctx->pipefd_to_child);
+    assert_int_not_equal(ret, -1);
+    DEBUG(SSSDBG_TRACE_LIBS, "to_child: %d:%d\n",
+                             child_tctx->pipefd_to_child[0],
+                             child_tctx->pipefd_to_child[1]);
+
+    *state = child_tctx;
+}
+
+void child_test_teardown(void **state)
+{
+    struct child_test_ctx *child_tctx = talloc_get_type(*state,
+                                                        struct child_test_ctx);
+
+    talloc_free(child_tctx);
+    check_leaks_pop(global_talloc_context);
+}
+
+/* Just make sure the exec works. The child does nothing but exits */
+void test_exec_child(void **state)
+{
+    errno_t ret;
+    pid_t child_pid;
+    int status;
+    struct child_test_ctx *child_tctx = talloc_get_type(*state,
+                                                        struct child_test_ctx);
+
+    child_pid = fork();
+    assert_int_not_equal(child_pid, -1);
+    if (child_pid == 0) {
+        ret = exec_child(child_tctx,
+                         child_tctx->pipefd_to_child,
+                         child_tctx->pipefd_from_child,
+                         CHILD_DIR"/"TEST_BIN, 2);
+        assert_int_equal(ret, EOK);
+    } else {
+            do {
+                errno = 0;
+                ret = waitpid(child_pid, &status, 0);
+            } while (ret == -1 && errno == EINTR);
+
+            if (ret > 0) {
+                ret = EIO;
+                if (WIFEXITED(status)) {
+                    ret = WEXITSTATUS(status);
+                    assert_int_equal(ret, 0);
+                }
+            } else {
+                DEBUG(SSSDBG_FUNC_DATA,
+                    "Failed to wait for children %d\n", child_pid);
+                ret = EIO;
+            }
+    }
+}
+
+int main(int argc, const char *argv[])
+{
+    int rv;
+    poptContext pc;
+    int opt;
+    struct poptOption long_options[] = {
+        POPT_AUTOHELP
+        SSSD_DEBUG_OPTS
+        POPT_TABLEEND
+    };
+
+    const UnitTest tests[] = {
+        unit_test_setup_teardown(test_exec_child,
+                                 child_test_setup,
+                                 child_test_teardown),
+    };
+
+    /* Set debug level to invalid value so we can deside if -d 0 was used. */
+    debug_level = SSSDBG_INVALID;
+
+    pc = poptGetContext(argv[0], argc, argv, long_options, 0);
+    while((opt = poptGetNextOpt(pc)) != -1) {
+        switch(opt) {
+        default:
+            fprintf(stderr, "\nInvalid option %s: %s\n\n",
+                    poptBadOption(pc, 0), poptStrerror(opt));
+            poptPrintUsage(pc, stderr, 0);
+            return 1;
+        }
+    }
+    poptFreeContext(pc);
+
+    DEBUG_CLI_INIT(debug_level);
+
+    rv = run_tests(tests);
+    return rv;
+}
-- 
1.9.3

-------------- next part --------------
>From 9fbbed2b30504c94bc23e482aa73393c2e36403f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 1 Dec 2014 12:25:24 +0100
Subject: [PATCH 2/3] Add extra_args to exec_child()

---
 src/providers/ad/ad_gpo.c               |  2 +-
 src/providers/ipa/ipa_selinux.c         |  3 ++-
 src/providers/krb5/krb5_child_handler.c |  3 ++-
 src/providers/ldap/sdap_child_helpers.c |  3 ++-
 src/tests/cmocka/test_child.c           | 31 ++++++++++++----------
 src/tests/cmocka/test_child_common.c    | 47 ++++++++++++++++++++++++++++++++-
 src/util/child_common.c                 | 23 ++++++++++++++--
 src/util/child_common.h                 |  3 ++-
 8 files changed, 93 insertions(+), 22 deletions(-)

diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
index 83edbe4fb5a7e617cab95c0330ceae31392b18b2..62715861c91484fa2a57e7cc13ba403c9096d9a7 100644
--- a/src/providers/ad/ad_gpo.c
+++ b/src/providers/ad/ad_gpo.c
@@ -3960,7 +3960,7 @@ gpo_fork_child(struct tevent_req *req)
     if (pid == 0) { /* child */
         err = exec_child(state,
                          pipefd_to_child, pipefd_from_child,
-                         GPO_CHILD, gpo_child_debug_fd);
+                         GPO_CHILD, gpo_child_debug_fd, NULL);
         DEBUG(SSSDBG_CRIT_FAILURE, "Could not exec gpo_child: [%d][%s].\n",
               err, strerror(err));
         return err;
diff --git a/src/providers/ipa/ipa_selinux.c b/src/providers/ipa/ipa_selinux.c
index 30ad6f0a7c4622ca5eb9a75ae4f57183543515c6..531258dac5c033b5896598e44e28a373d6cf5e3b 100644
--- a/src/providers/ipa/ipa_selinux.c
+++ b/src/providers/ipa/ipa_selinux.c
@@ -1036,7 +1036,8 @@ static errno_t selinux_fork_child(struct selinux_child_state *state)
     if (pid == 0) { /* child */
         ret = exec_child(state,
                          pipefd_to_child, pipefd_from_child,
-                         SELINUX_CHILD, selinux_child_debug_fd);
+                         SELINUX_CHILD, selinux_child_debug_fd,
+                         NULL);
         DEBUG(SSSDBG_CRIT_FAILURE, "Could not exec selinux_child: [%d][%s].\n",
               ret, sss_strerror(ret));
         return ret;
diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c
index 93961172c7a3a5d8f2a4fb320370037f188b5909..9bb61f65437694d8aa2109513b5f061dfc9ee21c 100644
--- a/src/providers/krb5/krb5_child_handler.c
+++ b/src/providers/krb5/krb5_child_handler.c
@@ -299,7 +299,8 @@ static errno_t fork_child(struct tevent_req *req)
     if (pid == 0) { /* child */
         err = exec_child(state,
                          pipefd_to_child, pipefd_from_child,
-                         KRB5_CHILD, state->kr->krb5_ctx->child_debug_fd);
+                         KRB5_CHILD, state->kr->krb5_ctx->child_debug_fd,
+                         NULL);
         if (err != EOK) {
             DEBUG(SSSDBG_CRIT_FAILURE, "Could not exec KRB5 child: [%d][%s].\n",
                       err, strerror(err));
diff --git a/src/providers/ldap/sdap_child_helpers.c b/src/providers/ldap/sdap_child_helpers.c
index 40010989021eb7cf77b96876b2d1c4119ed39163..b60891d2b41f9a359856eb22174128d7f07559fb 100644
--- a/src/providers/ldap/sdap_child_helpers.c
+++ b/src/providers/ldap/sdap_child_helpers.c
@@ -108,7 +108,8 @@ static errno_t sdap_fork_child(struct tevent_context *ev,
     if (pid == 0) { /* child */
         err = exec_child(child,
                          pipefd_to_child, pipefd_from_child,
-                         LDAP_CHILD, ldap_child_debug_fd);
+                         LDAP_CHILD, ldap_child_debug_fd,
+                         NULL);
         DEBUG(SSSDBG_CRIT_FAILURE, "Could not exec LDAP child: [%d][%s].\n",
                                     err, strerror(err));
         return err;
diff --git a/src/tests/cmocka/test_child.c b/src/tests/cmocka/test_child.c
index a857afce1fe7c4f3949fe77fe4e0a24e15961a4f..0bb50a4e005a26b1fbd332b17558d1284de77ea1 100644
--- a/src/tests/cmocka/test_child.c
+++ b/src/tests/cmocka/test_child.c
@@ -35,13 +35,9 @@ int main(int argc, const char *argv[])
     int opt;
     int debug_fd = -1;
     poptContext pc;
-    const char *action;
-    ssize_t len = 0;
-    ssize_t written;
-    errno_t ret;
-    uint8_t *buf[IN_BUF_SIZE];
-    uid_t uid;
-    gid_t gid;
+    const char *action = NULL;
+    const char *guitar;
+    const char *drums;
 
     struct poptOption long_options[] = {
         POPT_AUTOHELP
@@ -54,8 +50,9 @@ int main(int argc, const char *argv[])
         {"debug-fd", 0, POPT_ARG_INT, &debug_fd, 0,
          _("An open file descriptor for the debug logs"), NULL},
         {"debug-to-stderr", 0, POPT_ARG_NONE | POPT_ARGFLAG_DOC_HIDDEN, &debug_to_stderr, 0, \
-         _("Send the debug output to stderr directly."), NULL }, \
-        SSSD_SERVER_OPTS(uid, gid)
+         _("Send the debug output to stderr directly."), NULL },
+        {"guitar", 0, POPT_ARG_STRING, &guitar, 0, _("Who plays guitar"), NULL },
+        {"drums", 0, POPT_ARG_STRING, &drums, 0, _("Who plays drums"), NULL },
         POPT_TABLEEND
     };
 
@@ -73,11 +70,17 @@ int main(int argc, const char *argv[])
         }
     }
 
+    action = getenv("TEST_CHILD_ACTION");
+    if (action) {
+        if (strcasecmp(action, "check_extra_args") == 0) {
+            if (!(strcmp(guitar, "george") == 0 \
+                        && strcmp(drums, "ringo") == 0)) {
+                DEBUG(SSSDBG_CRIT_FAILURE, "This band sounds weird\n");
+                _exit(1);
+            }
+        }
+    }
+
     DEBUG(SSSDBG_TRACE_FUNC, "test_child completed successfully\n");
     _exit(0);
-
-fail:
-    DEBUG(SSSDBG_TRACE_FUNC, "test_child completed successfully\n");
-    close(STDOUT_FILENO);
-    _exit(-1);
 }
diff --git a/src/tests/cmocka/test_child_common.c b/src/tests/cmocka/test_child_common.c
index f2cd8c0081c2e85432db1c9696102780dc990e75..112ed0ad97294bc45eac7c2124155e6b1908ad92 100644
--- a/src/tests/cmocka/test_child_common.c
+++ b/src/tests/cmocka/test_child_common.c
@@ -89,7 +89,49 @@ void test_exec_child(void **state)
         ret = exec_child(child_tctx,
                          child_tctx->pipefd_to_child,
                          child_tctx->pipefd_from_child,
-                         CHILD_DIR"/"TEST_BIN, 2);
+                         CHILD_DIR"/"TEST_BIN, 2, NULL);
+        assert_int_equal(ret, EOK);
+    } else {
+            do {
+                errno = 0;
+                ret = waitpid(child_pid, &status, 0);
+            } while (ret == -1 && errno == EINTR);
+
+            if (ret > 0) {
+                ret = EIO;
+                if (WIFEXITED(status)) {
+                    ret = WEXITSTATUS(status);
+                    assert_int_equal(ret, 0);
+                }
+            } else {
+                DEBUG(SSSDBG_FUNC_DATA,
+                    "Failed to wait for children %d\n", child_pid);
+                ret = EIO;
+            }
+    }
+}
+
+/* Just make sure the exec works. The child does nothing but exits */
+void test_exec_child_extra_args(void **state)
+{
+    errno_t ret;
+    pid_t child_pid;
+    int status;
+    struct child_test_ctx *child_tctx = talloc_get_type(*state,
+                                                        struct child_test_ctx);
+    const char *extra_args[] = { "--guitar=george",
+                                 "--drums=ringo",
+                                 NULL };
+
+    setenv("TEST_CHILD_ACTION", "check_extra_args", 1);
+
+    child_pid = fork();
+    assert_int_not_equal(child_pid, -1);
+    if (child_pid == 0) {
+        ret = exec_child(child_tctx,
+                         child_tctx->pipefd_to_child,
+                         child_tctx->pipefd_from_child,
+                         CHILD_DIR"/"TEST_BIN, 2, extra_args);
         assert_int_equal(ret, EOK);
     } else {
             do {
@@ -126,6 +168,9 @@ int main(int argc, const char *argv[])
         unit_test_setup_teardown(test_exec_child,
                                  child_test_setup,
                                  child_test_teardown),
+        unit_test_setup_teardown(test_exec_child_extra_args,
+                                 child_test_setup,
+                                 child_test_teardown),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
diff --git a/src/util/child_common.c b/src/util/child_common.c
index cc6a8fa758bfa6efa511c28cd9121e759b590342..9710630f9773ae02258e4f0dd609a3d74978c8f4 100644
--- a/src/util/child_common.c
+++ b/src/util/child_common.c
@@ -623,6 +623,7 @@ static void child_invoke_callback(struct tevent_context *ev,
 static errno_t prepare_child_argv(TALLOC_CTX *mem_ctx,
                                   int child_debug_fd,
                                   const char *binary,
+                                  const char *extra_argv[],
                                   char ***_argv)
 {
     /*
@@ -632,6 +633,7 @@ static errno_t prepare_child_argv(TALLOC_CTX *mem_ctx,
     uint_t argc = 5;
     char ** argv;
     errno_t ret = EINVAL;
+    size_t i;
 
     /* Save the current state in case an interrupt changes it */
     bool child_debug_to_file = debug_to_file;
@@ -642,6 +644,10 @@ static errno_t prepare_child_argv(TALLOC_CTX *mem_ctx,
     if (child_debug_to_file) argc++;
     if (child_debug_stderr) argc++;
 
+    if (extra_argv) {
+        for (i = 0; extra_argv[i]; i++) argc++;
+    }
+
     /*
      * program name, debug_level, debug_to_file, debug_timestamps,
      * debug_microseconds and NULL
@@ -654,6 +660,17 @@ static errno_t prepare_child_argv(TALLOC_CTX *mem_ctx,
 
     argv[--argc] = NULL;
 
+    /* Add extra_attrs first */
+    if (extra_argv) {
+        for (i = 0; extra_argv[i]; i++) {
+            argv[--argc] = talloc_strdup(argv, extra_argv[i]);
+            if (argv[argc] == NULL) {
+                ret = ENOMEM;
+                goto fail;
+            }
+        }
+    }
+
     argv[--argc] = talloc_asprintf(argv, "--debug-level=%#.4x",
                               debug_level);
     if (argv[argc] == NULL) {
@@ -714,7 +731,8 @@ fail:
 
 errno_t exec_child(TALLOC_CTX *mem_ctx,
                    int *pipefd_to_child, int *pipefd_from_child,
-                   const char *binary, int debug_fd)
+                   const char *binary, int debug_fd,
+                   const char *extra_argv[])
 {
     int ret;
     errno_t err;
@@ -739,7 +757,8 @@ errno_t exec_child(TALLOC_CTX *mem_ctx,
     }
 
     ret = prepare_child_argv(mem_ctx, debug_fd,
-                             binary, &argv);
+                             binary, extra_argv,
+                             &argv);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, "prepare_child_argv.\n");
         return ret;
diff --git a/src/util/child_common.h b/src/util/child_common.h
index e159719a2fca70a4ea044d79530a0d21cb3577e8..e659388ece3677b7746c159d7de3e86171bb4146 100644
--- a/src/util/child_common.h
+++ b/src/util/child_common.h
@@ -114,7 +114,8 @@ void child_sig_handler(struct tevent_context *ev,
 /* Never returns EOK, ether returns an error, or doesn't return on success */
 errno_t exec_child(TALLOC_CTX *mem_ctx,
                    int *pipefd_to_child, int *pipefd_from_child,
-                   const char *binary, int debug_fd);
+                   const char *binary, int debug_fd,
+                   const char *extra_argv[]);
 
 void child_cleanup(int readfd, int writefd);
 
-- 
1.9.3

-------------- next part --------------
>From 84de2fb5c5fa6ee062056f0aeb17124f549208f6 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 28 Nov 2014 13:04:42 +0100
Subject: [PATCH 3/3] KRB5: Create the fast ccache in a child process

---
 src/providers/krb5/krb5_child.c         | 117 ++++++++++++++++++++++++--------
 src/providers/krb5/krb5_child_handler.c |  10 ++-
 2 files changed, 99 insertions(+), 28 deletions(-)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index cd78874db111e620a6a156f4310bc4e026a45121..3db2955d86f79ca95376ec91e81d794630ae00e8 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -74,6 +74,9 @@ struct krb5_req {
     bool old_cc_valid;
     bool old_cc_active;
     enum k5c_fast_opt fast_val;
+
+    uid_t fast_uid;
+    gid_t fast_gid;
 };
 
 static krb5_context krb5_error_ctx;
@@ -1009,17 +1012,6 @@ static krb5_error_code get_and_save_tgt(struct krb5_req *kr,
         DEBUG(SSSDBG_CONF_SETTINGS, "TGT validation is disabled.\n");
     }
 
-    if (kr->validate || kr->fast_ccname != NULL) {
-        /* We drop root privileges which were needed to read the keytab file
-         * for the validation of the credentials or for FAST here to run the
-         * ccache I/O operations with user privileges. */
-        kerr = become_user(kr->uid, kr->gid);
-        if (kerr != 0) {
-            DEBUG(SSSDBG_CRIT_FAILURE, "become_user failed.\n");
-            return kerr;
-        }
-    }
-
     /* If kr->ccname is cache collection (DIR:/...), we want to work
      * directly with file ccache (DIR::/...), but cache collection
      * should be returned back to back end.
@@ -1440,17 +1432,6 @@ static errno_t renew_tgt_child(struct krb5_req *kr)
         DEBUG(SSSDBG_CONF_SETTINGS, "TGT validation is disabled.\n");
     }
 
-    if (kr->validate || kr->fast_ccname != NULL) {
-        /* We drop root privileges which were needed to read the keytab file
-         * for the validation of the credentials or for FAST here to run the
-         * ccache I/O operations with user privileges. */
-        kerr = become_user(kr->uid, kr->gid);
-        if (kerr != 0) {
-            DEBUG(SSSDBG_CRIT_FAILURE, "become_user failed.\n");
-            goto done;
-        }
-    }
-
     kerr = krb5_cc_initialize(kr->ctx, ccache, kr->princ);
     if (kerr != 0) {
         KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);
@@ -1724,6 +1705,8 @@ done:
 
 static krb5_error_code check_fast_ccache(TALLOC_CTX *mem_ctx,
                                          krb5_context ctx,
+                                         uid_t fast_uid,
+                                         gid_t fast_gid,
                                          const char *primary,
                                          const char *realm,
                                          const char *keytab_name,
@@ -1737,6 +1720,8 @@ static krb5_error_code check_fast_ccache(TALLOC_CTX *mem_ctx,
     krb5_keytab keytab = NULL;
     krb5_principal client_princ = NULL;
     krb5_principal server_princ = NULL;
+    pid_t fchild_pid;
+    int status;
 
     tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) {
@@ -1794,10 +1779,78 @@ static krb5_error_code check_fast_ccache(TALLOC_CTX *mem_ctx,
         }
     }
 
-    kerr = get_and_save_tgt_with_keytab(ctx, client_princ, keytab, ccname);
-    if (kerr != 0) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "get_and_save_tgt_with_keytab failed.\n");
-        goto done;
+    /* Need to recreate the FAST ccache */
+    fchild_pid = fork();
+    switch (fchild_pid) {
+        case -1:
+            DEBUG(SSSDBG_CRIT_FAILURE, "fork failed\n");
+            kerr = EIO;
+            goto done;
+        case 0:
+            /* Child */
+            debug_prg_name = talloc_asprintf(NULL, "[sssd[krb5_child[%d]]]", getpid());
+            if (debug_prg_name == NULL) {
+                DEBUG(SSSDBG_CRIT_FAILURE, "talloc_asprintf failed.\n");
+                /* Try to carry on */
+            }
+
+            kerr = become_user(fast_uid, fast_gid);
+            if (kerr != 0) {
+                DEBUG(SSSDBG_CRIT_FAILURE, "become_user failed: %d\n", kerr);
+                exit(1);
+            }
+            DEBUG(SSSDBG_TRACE_INTERNAL,
+                  "Running as [%"SPRIuid"][%"SPRIgid"].\n", geteuid(), getegid());
+
+            kerr = get_and_save_tgt_with_keytab(ctx, client_princ,
+                                                keytab, ccname);
+            if (kerr != 0) {
+                DEBUG(SSSDBG_CRIT_FAILURE,
+                      "get_and_save_tgt_with_keytab failed: %d\n", kerr);
+                exit(2);
+            }
+            exit(0);
+        default:
+            /* Parent */
+            do {
+                errno = 0;
+                kerr = waitpid(fchild_pid, &status, 0);
+            } while (kerr == -1 && errno == EINTR);
+
+            if (kerr > 0) {
+                kerr = EIO;
+                if (WIFEXITED(status)) {
+                    kerr = WEXITSTATUS(status);
+                    /* Don't blindly fail if the child fails, but check
+                     * the ccache again */
+                    if (kerr != 0) {
+                        DEBUG(SSSDBG_MINOR_FAILURE,
+                              "Creating FAST ccache failed, krb5_child will "
+                              "likely fail!\n");
+                    }
+                } else {
+                    DEBUG(SSSDBG_CRIT_FAILURE,
+                          "krb5_child subprocess %d terminated unexpectedly\n",
+                          fchild_pid);
+                }
+            } else {
+                DEBUG(SSSDBG_FUNC_DATA,
+                    "Failed to wait for children %d\n", fchild_pid);
+                kerr = EIO;
+            }
+    }
+
+    /* Check the ccache times again. Should be updated ... */
+    memset(&tgtt, 0, sizeof(tgtt));
+    kerr = get_tgt_times(ctx, ccname, server_princ, client_princ, &tgtt);
+    if (kerr == 0) {
+        if (tgtt.endtime > time(NULL)) {
+            DEBUG(SSSDBG_FUNC_DATA, "FAST TGT was successfully recreated!\n");
+            goto done;
+        } else {
+            kerr = ERR_CREDS_EXPIRED;
+            goto done;
+        }
     }
 
     kerr = 0;
@@ -1889,7 +1942,8 @@ static int k5c_setup_fast(struct krb5_req *kr, bool demand)
         fast_principal = NULL;
     }
 
-    kerr = check_fast_ccache(kr, kr->ctx, fast_principal, fast_principal_realm,
+    kerr = check_fast_ccache(kr, kr->ctx, kr->fast_uid, kr->fast_gid,
+                             fast_principal, fast_principal_realm,
                              kr->keytab, &kr->fast_ccname);
     if (kerr != 0) {
         DEBUG(SSSDBG_CRIT_FAILURE, "check_fast_ccache failed.\n");
@@ -2253,6 +2307,8 @@ int main(int argc, const char *argv[])
     int debug_fd = -1;
     errno_t ret;
     krb5_error_code kerr;
+    uid_t fast_uid;
+    gid_t fast_gid;
 
     struct poptOption long_options[] = {
         POPT_AUTOHELP
@@ -2267,6 +2323,10 @@ int main(int argc, const char *argv[])
         {"debug-to-stderr", 0, POPT_ARG_NONE | POPT_ARGFLAG_DOC_HIDDEN,
          &debug_to_stderr, 0,
          _("Send the debug output to stderr directly."), NULL },
+        {"fast-ccache-uid", 0, POPT_ARG_INT, &fast_uid, 0,
+          _("The user to create FAST ccache as"), NULL},
+        {"fast-ccache-gid", 0, POPT_ARG_INT, &fast_gid, 0,
+          _("The group to create FAST ccache as"), NULL},
         POPT_TABLEEND
     };
 
@@ -2313,6 +2373,9 @@ int main(int argc, const char *argv[])
     }
     talloc_steal(kr, debug_prg_name);
 
+    kr->fast_uid = fast_uid;
+    kr->fast_gid = fast_gid;
+
     ret = k5c_recv_data(kr, STDIN_FILENO, &offline);
     if (ret != EOK) {
         goto done;
diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c
index 9bb61f65437694d8aa2109513b5f061dfc9ee21c..1454d220fb294abc339df6e862154012a03fdca0 100644
--- a/src/providers/krb5/krb5_child_handler.c
+++ b/src/providers/krb5/krb5_child_handler.c
@@ -278,6 +278,14 @@ static errno_t fork_child(struct tevent_req *req)
     errno_t err;
     struct handle_child_state *state = tevent_req_data(req,
                                                      struct handle_child_state);
+    const char *k5c_extra_args[3];
+
+    k5c_extra_args[0] = talloc_asprintf(state, "--fast-ccache-uid=%"SPRIuid, getuid());
+    k5c_extra_args[1] = talloc_asprintf(state, "--fast-ccache-gid=%"SPRIgid, getgid());
+    k5c_extra_args[2] = NULL;
+    if (k5c_extra_args[0] == NULL || k5c_extra_args[1] == NULL) {
+        return ENOMEM;
+    }
 
     ret = pipe(pipefd_from_child);
     if (ret == -1) {
@@ -300,7 +308,7 @@ static errno_t fork_child(struct tevent_req *req)
         err = exec_child(state,
                          pipefd_to_child, pipefd_from_child,
                          KRB5_CHILD, state->kr->krb5_ctx->child_debug_fd,
-                         NULL);
+                         k5c_extra_args);
         if (err != EOK) {
             DEBUG(SSSDBG_CRIT_FAILURE, "Could not exec KRB5 child: [%d][%s].\n",
                       err, strerror(err));
-- 
1.9.3



More information about the sssd-devel mailing list