diff --git a/NEWS b/NEWS index 4ec5042e396132576c151d7c9f82a319931fc447..548068893edcb48f15fdc5bbd28eaf1702bdbc13 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,12 @@ documents those changes that are of interest to users and admins. the block are set correctly in accounting as not in error. -- Fixed issue where if slurmdbd is not up qos' are set up correctly for associations off of cache. + -- scontrol, squeue, sview all display the correct node, cpu count along with + correct corresponding nodelist on completing jobs. + -- Patch (Mark Grondona) fixes serious security vulnerability in SLURM in + the spank_job_env functionality. + -- Improve spank_job_env interface and documentation + -- Add ESPANK_NOT_LOCAL error code to spank_err_t * Changes in SLURM 2.1.0-pre8 ============================= diff --git a/doc/man/man8/spank.8 b/doc/man/man8/spank.8 index 237f0b3652752c15d2bf83e11be03f0441dff1a2..2e03b03c58e39a773531b51fa0fadcedaf51cd2e 100644 --- a/doc/man/man8/spank.8 +++ b/doc/man/man8/spank.8 @@ -172,20 +172,20 @@ and \fBunsetenv\fR(3) may be used in local context. Functions are also available from within the \fBSPANK\fR plugins to establish environment variables to be exported to the SLURM \fBPrologSlurmctld\fR, \fBProlog\fR, \fBEpilog\fR and \fBEpilogSlurmctld\fR -programs. While designed for \fBSPANK\fR plugin use, hackers could -insert arbitrary environment variables, so their use by the various -prolog and epilog programs should avoid possible security compromises. -SLURM does block the setting of LD_PRELOAD and PATH as a precausionary measure. -These environment variables are not otherwise visible -to the job or \fBSPANK\fR functions. -These environment variable functons may only called from the local contact. -The syntax of these functions is identical to the getenv, -setenv, and unsetenv functions respectively: +programs (the so-called \fBjob control\fR environment). +The name of environment variables established by these calls will be prepended +with the string \fISPANK_\fR in order to avoid any security implications +of arbitrary environment variable control. (After all, the job control +scripts do run as root or the SLURM user.). +.LP +These functions are available from \fBlocal\fR context only. .nf - char *\fBspank_get_job_env\fR(const char *name); - int \fBspank_set_job_env\fR(const char *name, const char *value, - int overwrite); - int \fBspank_unset_job_env\fR(const char *name); + + spank_err_t \fBspank_job_control_getenv\fR(spank_t spank, const char *var, + char *buf, int len); + spank_err_t \fBspank_job_control_setenv\fR(spank_t spank, const char *var, + const char *val, int overwrite); + spank_err_t \fBspank_job_control_unsetenv\fR(spank_t spank, const char *var); .fi .LP See \fBspank.h\fR for more information, and \fBEXAMPLES\fR below for an example diff --git a/slurm/spank.h b/slurm/spank.h index 4cb046d92cb257f31d979aa8a77925144b231b7a..0c59f422927420940cd17da69b74a196cb6f8058 100644 --- a/slurm/spank.h +++ b/slurm/spank.h @@ -170,6 +170,7 @@ enum spank_err { ESPANK_NOEXIST = 8, /* Id/pid doesn't exist on this node */ ESPANK_NOT_EXECD = 9, /* Lookup by pid requested, but no tasks running */ ESPANK_NOT_AVAIL = 10,/* SPANK item not available from this callback */ + ESPANK_NOT_LOCAL = 11,/* Function only valid in local/alloc context */ }; typedef enum spank_err spank_err_t; @@ -325,21 +326,48 @@ spank_err_t spank_setenv (spank_t spank, const char *var, const char *val, * Unset environment variable "var" in the environment of current job or * task in the spank handle. * - * Returns ESPANK_SUCCESS on sucess, o/w spank_err_t on failure: + * Returns ESPANK_SUCCESS on success, o/w spank_err_t on failure: * ESPANK_BAD_ARG = spank handle invalid or var is NULL. - * SPANK_NOT_REMOTE = not called from slurmd. + * ESPANK_NOT_REMOTE = not called from slurmd. */ spank_err_t spank_unsetenv (spank_t spank, const char *var); -/* External functions available from SPANK plugins to modify the environment - * which is exported to the SLURM Prolog and Epilog programs. These environment - * variables are not otherwise visible to the job or SPANK functions. The - * syntax of these functions is identical to the getenv, setenv, and unsetenv - * functions. */ -extern char *spank_get_job_env(const char *name); -extern int spank_set_job_env(const char *name, const char *value, - int overwrite); -extern int spank_unset_job_env(const char *name); +/* + * Set an environment variable "name" to "value" in the "job control" + * environment, which is an extra set of environment variables + * included in the environment of the SLURM prolog and epilog + * programs. Environment variables set via this function will + * be prepended with SPANK_ to differentiate them from other env + * vars, and to avoid security issues. + * + * Returns ESPANK_SUCCESS on success, o/w/ spank_err_t on failure: + * ESPANK_ENV_EXISTS = var exists in control env and overwrite == 0. + * ESPANK_NOT_LOCAL = not called in local context + */ +spank_err_t spank_job_control_setenv (spank_t sp, const char *name, + const char *value, int overwrite); + +/* + * Place a copy of environment variable "name" from the job control + * environment into a buffer buf of size len. + * + * Returns ESPANK_SUCCESS on success, o/w spank_err_t on failure: + * ESPANK_BAD_ARG = invalid spank handle or len <= 0 + * ESPANK_ENV_NOEXIST = environment var does not exist in control env + * ESPANK_NOSPACE = buffer too small, truncation occurred. + * ESPANK_NOT_LOCAL = not called in local context + */ +spank_err_t spank_job_control_getenv (spank_t sp, const char *name, + char *buf, int len); + +/* + * Unset environment variable "name" in the job control environment. + * + * Returns ESPANK_SUCCESS on success, o/w spank_err_t on failure: + * ESPANK_BAD_ARG = invalid spank handle or var is NULL + * ESPANK_NOT_LOCAL = not called in local context + */ +spank_err_t spank_job_control_unsetenv (spank_t sp, const char *name); /* * SLURM logging functions which are exported to plugins. diff --git a/src/common/plugstack.c b/src/common/plugstack.c index aad5bfd3eefd2a75f72e6909678657bf8f5dd18d..88ecfed246d2fd08cf1d8cff2b055a1499c50f1f 100644 --- a/src/common/plugstack.c +++ b/src/common/plugstack.c @@ -45,6 +45,7 @@ #include <stdlib.h> #include <libgen.h> #include <glob.h> +#include <dlfcn.h> #include "src/common/plugin.h" #include "src/common/xmalloc.h" @@ -1522,6 +1523,8 @@ const char * spank_strerror (spank_err_t err) return "Lookup by PID requested, but no tasks running"; case ESPANK_NOT_AVAIL: return "Item not available from this callback"; + case ESPANK_NOT_LOCAL: + return "Valid only in local or allocator context"; } return "Unknown"; @@ -1878,3 +1881,102 @@ spank_err_t spank_unsetenv (spank_t spank, const char *var) return (ESPANK_SUCCESS); } + + +/* + * Dynamically loaded versions of spank_*_job_env + */ +const char *dyn_spank_get_job_env (const char *name) +{ + void *h = dlopen (NULL, 0); + char * (*fn)(const char *n); + + fn = dlsym (h, "spank_get_job_env"); + if (fn == NULL) + return NULL; + + return ((*fn) (name)); +} + +int dyn_spank_set_job_env (const char *n, const char *v, int overwrite) +{ + void *h = dlopen (NULL, 0); + int (*fn)(const char *n, const char *v, int overwrite); + + fn = dlsym (h, "spank_set_job_env"); + if (fn == NULL) + return (-1); + + return ((*fn) (n, v, overwrite)); +} + +extern int dyn_spank_unset_job_env (const char *n) +{ + void *h = dlopen (NULL, 0); + int (*fn)(const char *n); + + fn = dlsym (h, "spank_unset_job_env"); + if (fn == NULL) + return (-1); + + return ((*fn) (n)); +} + + +spank_err_t spank_job_control_getenv (spank_t spank, const char *var, + char *buf, int len) +{ + const char *val; + if ((spank == NULL) || (spank->magic != SPANK_MAGIC)) + return (ESPANK_BAD_ARG); + + if ((var == NULL) || (buf == NULL) || (len <= 0)) + return (ESPANK_BAD_ARG); + + if (spank_remote (spank)) + return (ESPANK_NOT_LOCAL); + + val = dyn_spank_get_job_env (var); + if (val == NULL) + return (ESPANK_ENV_NOEXIST); + + if (strlcpy (buf, val, len) >= len) + return (ESPANK_NOSPACE); + + return (ESPANK_SUCCESS); +} + +spank_err_t spank_job_control_setenv (spank_t spank, const char *var, + const char *val, int overwrite) +{ + if ((spank == NULL) || (spank->magic != SPANK_MAGIC)) + return (ESPANK_BAD_ARG); + + if ((var == NULL) || (val == NULL)) + return (ESPANK_BAD_ARG); + + if (spank_remote (spank)) + return (ESPANK_NOT_LOCAL); + + if (dyn_spank_set_job_env (var, val, overwrite) < 0) + return (ESPANK_BAD_ARG); + + return (ESPANK_SUCCESS); +} + +spank_err_t spank_job_control_unsetenv (spank_t spank, const char *var) +{ + if ((spank == NULL) || (spank->magic != SPANK_MAGIC)) + return (ESPANK_BAD_ARG); + + if (var == NULL) + return (ESPANK_BAD_ARG); + + if (spank_remote (spank)) + return (ESPANK_NOT_LOCAL); + + if (dyn_spank_unset_job_env (var) < 0) + return (ESPANK_BAD_ARG); + + return (ESPANK_SUCCESS); +} diff --git a/src/common/slurm_protocol_defs.c b/src/common/slurm_protocol_defs.c index 95d4c0b9f1375251a20e83df45e0b886ee6ac3cf..1355fa464aa82ea3ecc8d7d3085276f7f91a337e 100644 --- a/src/common/slurm_protocol_defs.c +++ b/src/common/slurm_protocol_defs.c @@ -2021,20 +2021,20 @@ static void _make_lower(char *change) } } -/* Validate SPANK specified job environment does not contain any invalid - * names. Log failures using info() */ +/* + * Sanitize spank_job_env by prepending "SPANK_" to all entries, + * thus rendering them harmless in environment of scripts and + * programs running with root privileges. + */ extern bool valid_spank_job_env(char **spank_job_env, uint32_t spank_job_env_size, uid_t uid) { int i; for (i=0; i<spank_job_env_size; i++) { - if ((strncmp(spank_job_env[i], "LD_PRELOAD=", 11) == 0) || - (strncmp(spank_job_env[i], "PATH=", 5) == 0)) { - info("Invalid spank_job_env from uid %d: %s", - (int) uid, spank_job_env[i]); - return false; - } + char *entry = spank_job_env[i]; + spank_job_env[i] = xstrdup_printf ("SPANK_%s", entry); + xfree (entry); } return true; }