Merge pull request flux-framework#163 from grondo/input-plugin

imp: support helper to get input to exec command

configure.ac

@@ -122,6 +122,11 @@ AC_CHECK_HEADERS( \
   [linux/magic.h] \
 )
 
+#
+#  Checks for functions
+#
+AC_CHECK_FUNC([pipe2], [], AC_MSG_FAILURE([Required function pipe2 missing]))
+
 #
 #  Checks for packages
 #

src/imp/Makefile.am

@@ -62,7 +62,9 @@ IMP_SOURCES = \
 	run.c \
 	exec/user.h \
 	exec/user.c \
-	exec/exec.c
+	exec/exec.c \
+	exec/safe_popen.h \
+	exec/safe_popen.c
 
 if HAVE_PAM
 IMP_SOURCES += \
@@ -135,7 +137,8 @@ TESTS = \
 	test_privsep.t \
 	test_impcmd.t \
 	test_passwd.t \
-	test_pidinfo.t
+	test_pidinfo.t \
+	test_safe_popen.t
 
 check_PROGRAMS = \
 	$(TESTS)
@@ -187,3 +190,11 @@ test_pidinfo_t_SOURCES = \
 	imp_log.c \
 	imp_log.h
 test_pidinfo_t_LDADD = $(test_ldadd)
+
+test_safe_popen_t_SOURCES = \
+	test/safe_popen.c \
+	exec/safe_popen.c \
+	exec/safe_popen.h \
+	imp_log.c \
+	imp_log.h
+test_safe_popen_t_LDADD= $(test_ldadd)

src/imp/exec/exec.c

@@ -17,6 +17,8 @@
  * Signed J as key "J" in JSON object on stdin, path to requested
  *  job shell and single argument on cmdline.
  *
+ * If FLUX_IMP_EXEC_HELPER is set, then execute the value of this
+ *  variable and read input from there.
  */
 
 #if HAVE_CONFIG_H
@@ -43,6 +45,7 @@
 #include "privsep.h"
 #include "passwd.h"
 #include "user.h"
+#include "safe_popen.h"
 
 #if HAVE_PAM
 #include "pam.h"
@@ -371,8 +374,31 @@ static void imp_exec_put_kv (struct imp_exec *exec,
         imp_die (1, "exec: Failed to set job shell arguments");
 }
 
+/*  Read IMP input using a helper process
+ */
+static void imp_exec_init_helper (struct imp_exec *exec,
+                                  char *helper)
+{
+        int status;
+        struct safe_popen *sp;
+
+        if (!(sp = safe_popen (helper)))
+            imp_die (1, "exec: failed to invoke helper: %s", helper);
+
+        imp_exec_init_stream (exec, safe_popen_fp (sp));
+
+        if (safe_popen_wait (sp, &status) < 0
+            || status != 0)
+            imp_die (1, "exec: helper %s failed with status=0x%04x",
+                     helper,
+                     status);
+
+        safe_popen_destroy (sp);
+}
+
 int imp_exec_unprivileged (struct imp_state *imp, struct kv *kv)
 {
+    char *helper;
     struct imp_exec *exec = imp_exec_create (imp);
     if (!exec)
         imp_die (1, "exec: initialization failure");
@@ -381,8 +407,16 @@ int imp_exec_unprivileged (struct imp_state *imp, struct kv *kv)
         imp_die (1, "exec: user %s not in allowed-users list",
                     exec->imp_pwd->pw_name);
 
-    /* Read input from stdin, cmdline: */
-    imp_exec_init_stream (exec, stdin);
+    if ((helper = getenv ("FLUX_IMP_EXEC_HELPER"))) {
+        if (strlen (helper) == 0)
+            imp_die (1, "exec: FLUX_IMP_EXEC_HELPER is empty");
+        /* Read input from helper command */
+        imp_exec_init_helper (exec, helper);
+    }
+    else {
+        /* Read input from stdin, cmdline: */
+        imp_exec_init_stream (exec, stdin);
+    }
 
     /* XXX; Parse jobspec if necessary, disabled for now: */
     //if (!(jobspec = json_loads (spec, 0, &err)))

src/imp/exec/safe_popen.c

@@ -0,0 +1,121 @@
+/************************************************************\
+ * Copyright 2023 Lawrence Livermore National Security, LLC
+ * (c.f. AUTHORS, NOTICE.LLNS, COPYING)
+ *
+ * This file is part of the Flux resource manager framework.
+ * For details, see https://github.com/flux-framework.
+ *
+ * SPDX-License-Identifier: LGPL-3.0
+\************************************************************/
+
+#ifndef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <errno.h>
+
+#include "src/libutil/argsplit.h"
+#include "safe_popen.h"
+#include "imp_log.h"
+
+struct safe_popen {
+    pid_t pid;
+    FILE *fp;
+};
+
+void safe_popen_destroy (struct safe_popen *sp)
+{
+    if (sp) {
+        int saved_errno = errno;
+        fclose (sp->fp);
+        free (sp);
+        errno = saved_errno;
+    }
+}
+
+struct safe_popen * safe_popen (const char *cmd)
+{
+    struct safe_popen *sp;
+    int pfds[2] = {-1, -1};
+
+    if (cmd == NULL || strlen (cmd) == 0) {
+        errno = EINVAL;
+        return NULL;
+    }
+
+    if (!(sp = calloc (1, sizeof (*sp)))
+        || pipe (pfds) < 0
+        || !(sp->fp = fdopen (pfds[0], "r"))
+        || (sp->pid = fork ()) < 0) {
+        imp_warn ("Failed to setup child for popen: %s", strerror (errno));
+        goto error;
+    }
+
+    if (sp->pid == 0) {
+        /*  Child process: Use pfds[1] as stdout
+         */
+        char **argv = argsplit (cmd);
+        if (!argv) {
+            fprintf (stderr, "imp: popen: failed to tokenize '%s'\n", cmd);
+            _exit (126);
+        }
+        if (dup2 (pfds[1], STDOUT_FILENO) < 0) {
+            fprintf (stderr, "imp: popen: dup2: %s\n", strerror (errno));
+            _exit (126);
+        }
+        (void) close (pfds[0]);
+        (void) close (pfds[1]);
+        setsid ();
+        execvp (argv[0], argv);
+        fprintf (stderr, "imp: popen: %s: %s\n", argv[0], strerror (errno));
+        if (errno == ENOENT)
+            _exit (127);
+        _exit (126);
+    }
+
+    /* Parent
+     */
+    (void) close (pfds[1]);
+
+    return sp;
+error:
+    safe_popen_destroy (sp);
+    if (pfds[0] > 0) {
+        (void) close (pfds[0]);
+        (void) close (pfds[1]);
+    }
+    return NULL;
+}
+
+FILE *safe_popen_fp (struct safe_popen *sp)
+{
+    if (!sp || !sp->fp) {
+        errno = EINVAL;
+        return NULL;
+    }
+    return sp->fp;
+}
+
+int safe_popen_wait (struct safe_popen *sp, int *statusp)
+{
+    pid_t pid;
+
+    if (!sp || sp->pid <= (pid_t) 0) {
+        errno = EINVAL;
+        return -1;
+    }
+    do {
+        pid = waitpid (sp->pid, statusp, 0);
+    } while (pid == -1 && errno == EINTR);
+
+    return pid == -1 ? -1 : 0;
+}
+
+/* vi: ts=4 sw=4 expandtab
+ */

src/imp/exec/safe_popen.h

@@ -0,0 +1,47 @@
+/************************************************************\
+ * Copyright 2023 Lawrence Livermore National Security, LLC
+ * (c.f. AUTHORS, NOTICE.LLNS, COPYING)
+ *
+ * This file is part of the Flux resource manager framework.
+ * For details, see https://github.com/flux-framework.
+ *
+ * SPDX-License-Identifier: LGPL-3.0
+\************************************************************/
+
+#ifndef HAVE_IMP_EXEC_SAFE_POPEN_H
+#define HAVE_IMP_EXEC_SAFE_POPEN_H 1
+
+#include <stdio.h>
+
+struct safe_popen;
+
+/*  Safer and simpler version of popen(3):
+ *  - does not invoke shell
+ *  - splits `cmd` on whitespace only
+ *
+ *  Returns a new safe_popen object on success, NULL with errno set
+ *   on failure.
+ *  EINVAL - cmd is NULL or an empty string.
+ *  ENOMEM - out of memory
+ */
+struct safe_popen *safe_popen (const char *cmd);
+
+/*  Return FILE * associated with safe_popen object */
+FILE *safe_popen_fp (struct safe_popen *sp);
+
+/*  Call waitpid(2) on process spawned by safe_popen(), placing process
+ *  status in `status` if non-NULL.
+ *  Returns 0 on success or -1 with errno set on error.
+ *  Special exit codes:
+ *   - 127: exec failed to find command
+ *   - 126: exec failed for other reason
+ *   - 125: internal error in child process before exec
+ */
+int safe_popen_wait (struct safe_popen *sp, int *status);
+
+/*  Destroy safe_popen object. Close open file descriptor and free
+ *   associated memory.
+ */
+void safe_popen_destroy (struct safe_popen *sp);
+
+#endif /* !HAVE_IMP_EXEC_SAFE_POPEN_H */

src/imp/imp_log.c

@@ -201,31 +201,40 @@ static void vlog_msg (int level, const char *format, va_list ap,
 void imp_say (const char *fmt, ...)
 {
     va_list ap;
+    int saved_errno;
     if (imp_logger.level < IMP_LOG_INFO)
         return;
+    saved_errno = errno;
     va_start (ap, fmt);
     vlog_msg (IMP_LOG_INFO, fmt, ap, imp_logger.outputs);
     va_end (ap);
+    errno = saved_errno;
 }
 
 void imp_warn (const char *fmt, ...)
 {
     va_list ap;
+    int saved_errno;
     if (imp_logger.level < IMP_LOG_WARNING)
         return;
+    saved_errno = errno;
     va_start (ap, fmt);
     vlog_msg (IMP_LOG_WARNING, fmt, ap, imp_logger.outputs);
     va_end (ap);
+    errno = saved_errno;
 }
 
 void imp_debug (const char *fmt, ...)
 {
     va_list ap;
+    int saved_errno;
     if (imp_logger.level < IMP_LOG_DEBUG)
         return;
+    saved_errno = errno;
     va_start (ap, fmt);
     vlog_msg (IMP_LOG_DEBUG, fmt, ap, imp_logger.outputs);
     va_end (ap);
+    errno = saved_errno;
 }
 
 void imp_die (int code, const char *fmt, ...)

src/imp/privsep.c

@@ -21,6 +21,7 @@
 #include <signal.h>
 #include <string.h>
 #include <errno.h>
+#include <fcntl.h>
 
 /*  Max size of KV array allowed to be sent over privsep pipe */
 #define PRIVSEP_MAX_KVLEN 1024*1024*4
@@ -147,7 +148,8 @@ privsep_t * privsep_init (privsep_child_f fn, void *arg)
     ps->wfd = -1;
     ps->rfd = -1;
 
-    if (pipe (ps->upfds) < 0 || pipe (ps->ppfds) < 0) {
+    if (pipe2 (ps->upfds, O_CLOEXEC) < 0
+        || pipe2 (ps->ppfds, O_CLOEXEC) < 0) {
         imp_warn ("privsep_init: pipe: %s\n", strerror (errno));
         privsep_destroy (ps);
         return (NULL);