Reduce code duplication between LDAP child and Kerberos child

Fixes: #294

4 files changed, 160 insertions, 234 deletions

diff --git a/server/providers/child_common.c b/server/providers/child_common.c
index 9ec09f29..15e0eefe 100644
--- a/server/providers/child_common.c
+++ b/server/providers/child_common.c
@@ -26,6 +26,7 @@
 #include <fcntl.h>
 #include <tevent.h>
 #include <sys/wait.h>
+#include <errno.h>
 
 #include "util/util.h"
 #include "util/find_uid.h"
@@ -193,3 +194,134 @@ void child_sig_handler(struct tevent_context *ev,
     return;
 }
 
+static errno_t prepare_child_argv(TALLOC_CTX *mem_ctx,
+                                  int child_debug_fd,
+                                  const char *binary,
+                                  char ***_argv)
+{
+    uint_t argc = 3; /* program name, debug_level and NULL */
+    char ** argv;
+    errno_t ret = EINVAL;
+
+    /* Save the current state in case an interrupt changes it */
+    bool child_debug_to_file = debug_to_file;
+    bool child_debug_timestamps = debug_timestamps;
+
+    if (child_debug_to_file) argc++;
+    if (child_debug_timestamps) argc++;
+
+    /* program name, debug_level,
+     * debug_to_file, debug_timestamps
+     * and NULL */
+    argv  = talloc_array(mem_ctx, char *, argc);
+    if (argv == NULL) {
+        DEBUG(1, ("talloc_array failed.\n"));
+        return ENOMEM;
+    }
+
+    argv[--argc] = NULL;
+
+    argv[--argc] = talloc_asprintf(argv, "--debug-level=%d",
+                              debug_level);
+    if (argv[argc] == NULL) {
+        ret = ENOMEM;
+        goto fail;
+    }
+
+    if (child_debug_to_file) {
+        argv[--argc] = talloc_asprintf(argv, "--debug-fd=%d",
+                                       child_debug_fd);
+        if (argv[argc] == NULL) {
+            ret = ENOMEM;
+            goto fail;
+        }
+    }
+
+    if (child_debug_timestamps) {
+        argv[--argc] = talloc_strdup(argv, "--debug-timestamps");
+        if (argv[argc] == NULL) {
+            ret = ENOMEM;
+            goto fail;
+        }
+    }
+
+    argv[--argc] = talloc_strdup(argv, binary);
+    if (argv[argc] == NULL) {
+        ret = ENOMEM;
+        goto fail;
+    }
+
+    if (argc != 0) {
+        ret = EINVAL;
+        goto fail;
+    }
+
+    *_argv = argv;
+
+    return EOK;
+
+fail:
+    talloc_free(argv);
+    return ret;
+}
+
+errno_t exec_child(TALLOC_CTX *mem_ctx,
+                   int *pipefd_to_child, int *pipefd_from_child,
+                   const char *binary, int debug_fd)
+{
+    int ret;
+    errno_t err;
+    char **argv;
+
+    close(pipefd_to_child[1]);
+    ret = dup2(pipefd_to_child[0], STDIN_FILENO);
+    if (ret == -1) {
+        err = errno;
+        DEBUG(1, ("dup2 failed [%d][%s].\n", err, strerror(err)));
+        return err;
+    }
+
+    close(pipefd_from_child[0]);
+    ret = dup2(pipefd_from_child[1], STDOUT_FILENO);
+    if (ret == -1) {
+        err = errno;
+        DEBUG(1, ("dup2 failed [%d][%s].\n", err, strerror(err)));
+        return err;
+    }
+
+    ret = prepare_child_argv(mem_ctx, debug_fd,
+                             binary, &argv);
+    if (ret != EOK) {
+        DEBUG(1, ("prepare_child_argv.\n"));
+        return ret;
+    }
+
+    ret = execv(binary, argv);
+    if (ret == -1) {
+        err = errno;
+        DEBUG(1, ("execv failed [%d][%s].\n", err, strerror(err)));
+        return err;
+    }
+
+    return EOK;
+}
+
+void child_cleanup(int readfd, int writefd)
+{
+    int ret;
+
+    if (readfd != -1) {
+        ret = close(readfd);
+        if (ret != EOK) {
+            ret = errno;
+            DEBUG(1, ("close failed [%d][%s].\n", errno, strerror(errno)));
+        }
+    }
+    if (writefd != -1) {
+        ret = close(writefd);
+        if (ret != EOK) {
+            ret = errno;
+            DEBUG(1, ("close failed [%d][%s].\n", errno, strerror(errno)));
+        }
+    }
+}
diff --git a/server/providers/child_common.h b/server/providers/child_common.h
index 75cb3a6f..894255b5 100644
--- a/server/providers/child_common.h
+++ b/server/providers/child_common.h
@@ -25,10 +25,13 @@
 #ifndef __CHILD_COMMON_H__
 #define __CHILD_COMMON_H__
 
+#include <errno.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <tevent.h>
 
+#include "util/util.h"
+
 #define IN_BUF_SIZE         512
 #define MAX_CHILD_MSG_SIZE  255
 
@@ -38,6 +41,11 @@ struct response {
     uint8_t *buf;
 };
 
+struct io_buffer {
+    uint8_t *data;
+    size_t size;
+};
+
 uint8_t *copy_buffer_and_add_zero(TALLOC_CTX *mem_ctx,
                                   const uint8_t *src, size_t len);
 
@@ -57,4 +65,10 @@ void child_sig_handler(struct tevent_context *ev,
                        struct tevent_signal *sige, int signum,
                        int count, void *__siginfo, void *pvt);
 
+errno_t exec_child(TALLOC_CTX *mem_ctx,
+                   int *pipefd_to_child, int *pipefd_from_child,
+                   const char *binary, int debug_fd);
+
+void child_cleanup(int readfd, int writefd);
+
 #endif /* __CHILD_COMMON_H__ */
diff --git a/server/providers/krb5/krb5_auth.c b/server/providers/krb5/krb5_auth.c
index 8848a510..0321ce82 100644
--- a/server/providers/krb5/krb5_auth.c
+++ b/server/providers/krb5/krb5_auth.c
@@ -45,11 +45,6 @@
 #define KRB5_CHILD SSSD_LIBEXEC_PATH"/krb5_child"
 #endif
 
-struct io_buffer {
-    uint8_t *data;
-    size_t size;
-};
-
 static errno_t add_krb5_env(struct dp_option *opts, const char *ccname,
                             struct pam_data *pd)
 {
@@ -463,24 +458,11 @@ static errno_t activate_child_timeout_handler(struct krb5child_req *kr)
 
 static int krb5_cleanup(void *ptr)
 {
-    int ret;
     struct krb5child_req *kr = talloc_get_type(ptr, struct krb5child_req);
 
     if (kr == NULL) return EOK;
 
-    if (kr->read_from_child_fd != -1) {
-        ret = close(kr->read_from_child_fd);
-        if (ret != EOK) {
-            DEBUG(1, ("close failed [%d][%s].\n", errno, strerror(errno)));
-        }
-    }
-    if (kr->write_to_child_fd != -1) {
-        ret = close(kr->write_to_child_fd);
-        if (ret != EOK) {
-            DEBUG(1, ("close failed [%d][%s].\n", errno, strerror(errno)));
-        }
-    }
-
+    child_cleanup(kr->read_from_child_fd, kr->write_to_child_fd);
     memset(kr, 0, sizeof(struct krb5child_req));
 
     return EOK;
@@ -528,76 +510,6 @@ failed:
     return err;
 }
 
-static errno_t prepare_child_argv(TALLOC_CTX *mem_ctx,
-                                  struct krb5child_req *kr,
-                                  char ***_argv)
-{
-    uint_t argc = 3; /* program name, debug_level and NULL */
-    char ** argv;
-    errno_t ret = EINVAL;
-
-    /* Save the current state in case an interrupt changes it */
-    bool child_debug_to_file = debug_to_file;
-    bool child_debug_timestamps = debug_timestamps;
-
-    if (child_debug_to_file) argc++;
-    if (child_debug_timestamps) argc++;
-
-    /* program name, debug_level,
-     * debug_to_file, debug_timestamps
-     * and NULL */
-    argv  = talloc_array(mem_ctx, char *, argc);
-    if (argv == NULL) {
-        DEBUG(1, ("talloc_array failed.\n"));
-        return ENOMEM;
-    }
-
-    argv[--argc] = NULL;
-
-    argv[--argc] = talloc_asprintf(argv, "--debug-level=%d",
-                              debug_level);
-    if (argv[argc] == NULL) {
-        ret = ENOMEM;
-        goto fail;
-    }
-
-    if (child_debug_to_file) {
-        argv[--argc] = talloc_asprintf(argv, "--debug-fd=%d",
-                                  kr->krb5_ctx->child_debug_fd);
-        if (argv[argc] == NULL) {
-            ret = ENOMEM;
-            goto fail;
-        }
-    }
-
-    if (child_debug_timestamps) {
-        argv[--argc] = talloc_strdup(argv, "--debug-timestamps");
-        if (argv[argc] == NULL) {
-            ret = ENOMEM;
-            goto fail;
-        }
-    }
-
-    argv[--argc] = talloc_strdup(argv, KRB5_CHILD);
-    if (argv[argc] == NULL) {
-        ret = ENOMEM;
-        goto fail;
-    }
-
-    if (argc != 0) {
-        ret = EINVAL;
-        goto fail;
-    }
-
-    *_argv = argv;
-
-    return EOK;
-
-fail:
-    talloc_free(argv);
-    return ret;
-}
-
 static errno_t fork_child(struct krb5child_req *kr)
 {
     int pipefd_to_child[2];
@@ -605,7 +517,6 @@ static errno_t fork_child(struct krb5child_req *kr)
     pid_t pid;
     int ret;
     errno_t err;
-    char **argv;
 
     ret = pipe(pipefd_from_child);
     if (ret == -1) {
@@ -637,32 +548,12 @@ static errno_t fork_child(struct krb5child_req *kr)
             }
         }
 
-        close(pipefd_to_child[1]);
-        ret = dup2(pipefd_to_child[0],STDIN_FILENO);
-        if (ret == -1) {
-            err = errno;
-            DEBUG(1, ("dup2 failed [%d][%s].\n", errno, strerror(errno)));
-            return err;
-        }
-
-        close(pipefd_from_child[0]);
-        ret = dup2(pipefd_from_child[1],STDOUT_FILENO);
-        if (ret == -1) {
-            err = errno;
-            DEBUG(1, ("dup2 failed [%d][%s].\n", errno, strerror(errno)));
-            return err;
-        }
-
-        ret = prepare_child_argv(kr, kr, &argv);
-        if (ret != EOK) {
-            DEBUG(1, ("prepare_child_argv.\n"));
-            return ret;
-        }
-
-        ret = execv(KRB5_CHILD, argv);
-        if (ret == -1) {
-            err = errno;
-            DEBUG(1, ("execv failed [%d][%s].\n", errno, strerror(errno)));
+        err = exec_child(kr,
+                         pipefd_to_child, pipefd_from_child,
+                         KRB5_CHILD, kr->krb5_ctx->child_debug_fd);
+        if (err != EOK) {
+            DEBUG(1, ("Could not exec LDAP child: [%d][%s].\n",
+                      err, strerror(err)));
             return err;
         }
     } else if (pid > 0) { /* parent */
diff --git a/server/providers/ldap/sdap_child_helpers.c b/server/providers/ldap/sdap_child_helpers.c
index 69b246ca..0972ec72 100644
--- a/server/providers/ldap/sdap_child_helpers.c
+++ b/server/providers/ldap/sdap_child_helpers.c
@@ -42,11 +42,6 @@
 #define LDAP_CHILD_USER  "nobody"
 #endif
 
-struct io_buffer {
-    uint8_t *data;
-    size_t size;
-};
-
 struct sdap_child_req {
     /* child info */
     pid_t child_pid;
@@ -67,26 +62,11 @@ struct sdap_child_req {
 
 static int sdap_child_req_destructor(void *ptr)
 {
-    int ret;
     struct sdap_child_req *cr = talloc_get_type(ptr, struct sdap_child_req);
 
     if (cr == NULL) return EOK;
 
-    if (cr->read_from_child_fd != -1) {
-        ret = close(cr->read_from_child_fd);
-        if (ret != EOK) {
-            ret = errno;
-            DEBUG(1, ("close failed [%d][%s].\n", ret, strerror(ret)));
-        }
-    }
-    if (cr->write_to_child_fd != -1) {
-        ret = close(cr->write_to_child_fd);
-        if (ret != EOK) {
-            ret = errno;
-            DEBUG(1, ("close failed [%d][%s].\n", ret, strerror(ret)));
-        }
-    }
-
+    child_cleanup(cr->read_from_child_fd, cr->write_to_child_fd);
     memset(cr, 0, sizeof(struct sdap_child_req));
 
     return EOK;
@@ -131,76 +111,6 @@ static errno_t activate_child_timeout_handler(struct sdap_child_req *child_req)
     return EOK;
 }
 
-static errno_t prepare_child_argv(TALLOC_CTX *mem_ctx,
-                                  struct sdap_child_req *child_req,
-                                  char ***_argv)
-{
-    uint_t argc = 3; /* program name, debug_level and NULL */
-    char ** argv;
-    errno_t ret = EINVAL;
-
-    /* Save the current state in case an interrupt changes it */
-    bool child_debug_to_file = debug_to_file;
-    bool child_debug_timestamps = debug_timestamps;
-
-    if (child_debug_to_file) argc++;
-    if (child_debug_timestamps) argc++;
-
-    /* program name, debug_level,
-     * debug_to_file, debug_timestamps
-     * and NULL */
-    argv  = talloc_array(mem_ctx, char *, argc);
-    if (argv == NULL) {
-        DEBUG(1, ("talloc_array failed.\n"));
-        return ENOMEM;
-    }
-
-    argv[--argc] = NULL;
-
-    argv[--argc] = talloc_asprintf(argv, "--debug-level=%d",
-                                   debug_level);
-    if (argv[argc] == NULL) {
-        ret = ENOMEM;
-        goto fail;
-    }
-
-    if (child_debug_to_file) {
-        argv[--argc] = talloc_asprintf(argv, "--debug-fd=%d",
-                                       ldap_child_debug_fd);
-        if (argv[argc] == NULL) {
-            ret = ENOMEM;
-            goto fail;
-        }
-    }
-
-    if (child_debug_timestamps) {
-        argv[--argc] = talloc_strdup(argv, "--debug-timestamps");
-        if (argv[argc] == NULL) {
-            ret = ENOMEM;
-            goto fail;
-        }
-    }
-
-    argv[--argc] = talloc_strdup(argv, LDAP_CHILD);
-    if (argv[argc] == NULL) {
-        ret = ENOMEM;
-        goto fail;
-    }
-
-    if (argc != 0) {
-        ret = EINVAL;
-        goto fail;
-    }
-
-    *_argv = argv;
-
-    return EOK;
-
-fail:
-    talloc_free(argv);
-    return ret;
-}
-
 static errno_t fork_ldap_child(struct sdap_child_req *child_req)
 {
     int pipefd_to_child[2];
@@ -208,7 +118,6 @@ static errno_t fork_ldap_child(struct sdap_child_req *child_req)
     pid_t pid;
     int ret;
     errno_t err;
-    char **argv;
 
     ret = pipe(pipefd_from_child);
     if (ret == -1) {
@@ -226,32 +135,12 @@ static errno_t fork_ldap_child(struct sdap_child_req *child_req)
     pid = fork();
 
     if (pid == 0) { /* child */
-        close(pipefd_to_child[1]);
-        ret = dup2(pipefd_to_child[0], STDIN_FILENO);
-        if (ret == -1) {
-            err = errno;
-            DEBUG(1, ("dup2 failed [%d][%s].\n", err, strerror(err)));
-            return err;
-        }
-
-        close(pipefd_from_child[0]);
-        ret = dup2(pipefd_from_child[1], STDOUT_FILENO);
-        if (ret == -1) {
-            err = errno;
-            DEBUG(1, ("dup2 failed [%d][%s].\n", err, strerror(err)));
-            return err;
-        }
-
-        ret = prepare_child_argv(child_req, child_req, &argv);
-        if (ret != EOK) {
-            DEBUG(1, ("prepare_child_argv.\n"));
-            return ret;
-        }
-
-        ret = execv(LDAP_CHILD, argv);
-        if (ret == -1) {
-            err = errno;
-            DEBUG(1, ("execv failed [%d][%s].\n", err, strerror(err)));
+        err = exec_child(child_req,
+                         pipefd_to_child, pipefd_from_child,
+                         LDAP_CHILD, ldap_child_debug_fd);
+        if (err != EOK) {
+            DEBUG(1, ("Could not exec LDAP child: [%d][%s].\n",
+                      err, strerror(err)));
             return err;
         }
     } else if (pid > 0) { /* parent */