Skip to content

Commit

Permalink
shell: do not allow multiuser access to shell services by default
Browse files Browse the repository at this point in the history
Problem: Shell services registered by flux_shell_service_register()
are "open to all" by default, and require a call to shell_svc_allowed()
in each msg handler in order to secure the service against multi-user
access. However, this design requires repetitive calls in every service
message handler, and makes it more likely that plugins install insecure
services when this call is forgotten. Furthermore, shell_svc_allowed()
is not even exported publicly in the shell.h API, so it is impossible
to create secure services via external shell plugins.

Internally wrap all message handlers installed by shell_svc_register()
and call shell_svc_allowed() so that all services are secure by default.

If a use case ever arises that requires multiuser access to a shell
service, then a separate api call can be created to export an
insecure service, so that the security of the service is explicit.

Fixes flux-framework#2876
  • Loading branch information
grondo authored and trws committed Mar 30, 2020
1 parent 388bca7 commit 475e57a
Showing 1 changed file with 51 additions and 1 deletion.
52 changes: 51 additions & 1 deletion src/shell/shell.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 475e57a

Please sign in to comment.