Merge pull request flux-framework#2877 from grondo/issue#2876

shell: make registered services secure by default

src/shell/input.c

@@ -167,8 +167,6 @@ static void shell_input_stdin_cb (flux_t *h,
     bool eof = false;
     json_t *o;
 
-    if (shell_svc_allowed (in->shell->svc, msg) < 0)
-        goto error;
     if (flux_request_unpack (msg, NULL, "o", &o) < 0)
         goto error;
     if (iodecode (o, NULL, NULL, NULL, NULL, &eof) < 0)

src/shell/output.c

@@ -403,8 +403,6 @@ static void shell_output_write_cb (flux_t *h,
     json_t *o;
     json_t *entry;
 
-    if (shell_svc_allowed (out->shell->svc, msg) < 0)
-        goto error;
     if (flux_request_unpack (msg, NULL, "o", &o) < 0)
         goto error;
     if (iodecode (o, NULL, NULL, NULL, NULL, &eof) < 0)

src/shell/shell.c

@@ -516,16 +516,66 @@ int flux_shell_add_event_handler (flux_shell_t *shell,
     return 0;
 }
 
+struct service_wrap_arg
+{
+    flux_shell_t *shell;
+    flux_msg_handler_f cb;
+    void *arg;
+};
+
+static void shell_service_wrap (flux_t *h,
+                                flux_msg_handler_t *mh,
+                                const flux_msg_t *msg,
+                                void *arg)
+{
+    struct service_wrap_arg *sarg = arg;
+
+    if (shell_svc_allowed (sarg->shell->svc, msg) < 0)
+        goto error;
+    (*sarg->cb) (h, mh, msg, sarg->arg);
+    return;
+error:
+    if (flux_respond_error (h, msg, errno, NULL) < 0)
+        shell_log_errno ("flux_respond");
+}
+
+static struct service_wrap_arg *
+service_wrap_arg_create (flux_shell_t *shell,
+                        flux_msg_handler_f cb,
+                        void *arg)
+{
+    struct service_wrap_arg *sarg = calloc (1, sizeof (*sarg));
+    if (!sarg)
+        return NULL;
+    sarg->shell = shell;
+    sarg->cb = cb;
+    sarg->arg = arg;
+    return sarg;
+}
+
 int flux_shell_service_register (flux_shell_t *shell,
                                  const char *method,
                                  flux_msg_handler_f cb,
                                  void *arg)
 {
+    struct service_wrap_arg *sarg = NULL;
+
     if (!shell || !method || !cb) {
         errno = EINVAL;
         return -1;
     }
-    return shell_svc_register (shell->svc, method, cb, arg);
+    if (!(sarg = service_wrap_arg_create (shell, cb, arg)))
+        return -1;
+
+    if (flux_shell_aux_set (shell, NULL, sarg, free) < 0) {
+        free (sarg);
+        return -1;
+    }
+
+    return shell_svc_register (shell->svc,
+                               method,
+                               shell_service_wrap,
+                               sarg);
 }
 
 flux_future_t *flux_shell_rpc_pack (flux_shell_t *shell,

t/t2610-job-shell-mpir.t

@@ -34,4 +34,21 @@ for test in 1:1 2:2 2:4 4:4 4:8 4:7; do
         flux job attach ${id}
     '
 done
+
+
+test_expect_success 'flux-shell: test security of proctable method' '
+    id=$(flux mini submit -o stop-tasks-in-exec /bin/true) &&
+    flux job wait-event -vt 5 -p guest.exec.eventlog \
+        -m sync=true ${id} shell.start &&
+    shell_rank=$(shell_leader_rank $id) &&
+    shell_service=$(shell_service $id) &&
+    ( export FLUX_HANDLE_USERID=9999 &&
+      export FLUX_HANDLE_ROLEMASK=0x2 &&
+      test_expect_code 1 ${mpir} $shell_rank $shell_service
+    ) &&
+    ${mpir} $(shell_leader_rank $id) $(shell_service $id) &&
+    flux job kill -s CONT ${id} &&
+    flux job attach ${id}
+'
+
 test_done