Skip to content

Commit

Permalink
su: add --whitelist-environment
Browse files Browse the repository at this point in the history 
* usable with --login to whitelist specified environment variables

* the list is ignored for the core variables like HOME, SHELL, USER,
  LOGNAME and PATH (su --login always resets these variables)

Note that su(1) requires password and after successful authentication
user has full control over the session, so he can set arbitrary
environment variables. The whitelist makes things more user friendly
only.

The patch removes unnecessary optimization when allocate environ[]. It
seems better to keep all in glibc hands and just reset the environment
array only.

Addresses: util-linux#221
Signed-off-by: Karel Zak <[email protected]>
  • Loading branch information
@karelzak
karelzak committed Aug 15, 2018
1 parent df5ccf2 commit 75efef9
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 16 deletions.
1 change: 1 addition & 0 deletions configure.ac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,7 @@ AC_CHECK_DECL([SO_PASSCRED],
#include <sys/socket.h>])

AC_CHECK_FUNCS([ \
clearenv \
__fpurge \
fpurge \
__fpending \
Expand Down
10 changes: 10 additions & 0 deletions login-utils/runuser.1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ login:
o
clears all the environment variables except for
.B TERM
and variables specified by \fB\-\-whitelist\-environment\fR
.TP
o
initializes the environment variables
Expand Down Expand Up @@ -144,6 +145,15 @@ Same as
.B \-c ,
but do not create a new session. (Discouraged.)
.TP
.BR \-w , " \-\-whitelist\-environment" = \fIlist
Don't reset environment variables specified in comma separated \fIlist\fR when clears
environment for \fB\-\-login\fR. The whitelist is ignored for the environment variables
.BR HOME ,
.BR SHELL ,
.BR USER ,
.BR LOGNAME ", and"
.BR PATH "."
.TP
.BR \-V , " \-\-version"
Display version information and exit.
.TP
Expand Down
116 changes: 100 additions & 16 deletions login-utils/su-common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,11 @@
#include "pathnames.h"
#include "env.h"
#include "closestream.h"
#include "strv.h"
#include "strutils.h"
#include "ttyutils.h"
#include "pwdutils.h"
#include "optutils.h"

#include "logindefs.h"
#include "su-common.h"
Expand Down Expand Up @@ -130,6 +132,9 @@ struct su_context {
pid_t child; /* fork() baby */
int childstatus; /* wait() status */

char **env_whitelist_names; /* environment whitelist */
char **env_whitelist_vals;

struct sigaction oldact[SIGNALS_IDX_COUNT]; /* original sigactions indexed by SIG*_IDX */

#ifdef USE_PTY
Expand Down Expand Up @@ -925,6 +930,56 @@ static void create_watching_parent(struct su_context *su)
exit(status);
}

/* Adds @name from the current environment to the whitelist. If @name is not
* set then nothing is added to the whitelist and returns 1.
*/
static int env_whitelist_add(struct su_context *su, const char *name)
{
const char *env = getenv(name);

if (!env)
return 1;
if (strv_extend(&su->env_whitelist_names, name))
err_oom();
if (strv_extend(&su->env_whitelist_vals, env))
err_oom();
return 0;
}

static int env_whitelist_setenv(struct su_context *su, int overwrite)
{
char **one;
size_t i = 0;
int rc;

STRV_FOREACH(one, su->env_whitelist_names) {
rc = setenv(*one, su->env_whitelist_vals[i], overwrite);
if (rc)
return rc;
i++;
}

return 0;
}

/* Creates (add to) whitelist from comma delimited string */
static int env_whitelist_from_string(struct su_context *su, const char *str)
{
char **all = strv_split(str, ",");
char **one;

if (!all) {
if (errno == ENOMEM)
err_oom();
return -EINVAL;
}

STRV_FOREACH(one, all)
env_whitelist_add(su, *one);
strv_free(all);
return 0;
}

static void setenv_path(const struct passwd *pw)
{
int rc;
Expand All @@ -949,26 +1004,38 @@ static void modify_environment(struct su_context *su, const char *shell)
DBG(MISC, ul_debug("modify environ[]"));

/* Leave TERM unchanged. Set HOME, SHELL, USER, LOGNAME, PATH.
* Unset all other environment variables.
*
* Unset all other environment variables, but follow
* --whitelist-environment if specified.
*/
if (su->simulate_login) {
char *term = getenv("TERM");
if (term)
term = xstrdup(term);

environ = xmalloc((6 + ! !term) * sizeof(char *));
environ[0] = NULL;
if (term) {
xsetenv("TERM", term, 1);
free(term);
}

xsetenv("HOME", pw->pw_dir, 1);
/* leave TERM unchanged */
env_whitelist_add(su, "TERM");

/* Note that original su(1) has allocated environ[] by malloc
* to the number of expected variables. This seems unnecessary
* optimization as libc later realloc(current_size+2) and for
* empty environ[] the curren_size is zero. It seems better to
* keep all logic around environment in glibc's hands.
* --kzak [Aug 2018]
*/
#ifdef HAVE_CLEARENV
clearenv();
#else
environ = NULL;
#endif
/* always reset */
if (shell)
xsetenv("SHELL", shell, 1);

setenv_path(pw);

xsetenv("HOME", pw->pw_dir, 1);
xsetenv("USER", pw->pw_name, 1);
xsetenv("LOGNAME", pw->pw_name, 1);
setenv_path(pw);

/* apply all from whitelist, but no overwrite */
env_whitelist_setenv(su, 0);

/* Set HOME, SHELL, and (if not becoming a superuser) USER and LOGNAME.
*/
Expand Down Expand Up @@ -1089,7 +1156,10 @@ static bool is_restricted_shell(const char *shell)

static void usage_common(void)
{
fputs(_(" -m, -p, --preserve-environment do not reset environment variables\n"), stdout);
fputs(_(" -m, -p, --preserve-environment do not reset environment variables\n"), stdout);
fputs(_(" -w, --whitelist-environment <list> don't reset specified variables\n"), stdout);
fputs(USAGE_SEPARATOR, stdout);

fputs(_(" -g, --group <group> specify the primary group\n"), stdout);
fputs(_(" -G, --supp-group <group> specify a supplemental group\n"), stdout);
fputs(USAGE_SEPARATOR, stdout);
Expand Down Expand Up @@ -1234,10 +1304,17 @@ int su_main(int argc, char **argv, int mode)
{"group", required_argument, NULL, 'g'},
{"supp-group", required_argument, NULL, 'G'},
{"user", required_argument, NULL, 'u'}, /* runuser only */
{"whitelist-environment", required_argument, NULL, 'w'},
{"help", no_argument, 0, 'h'},
{"version", no_argument, 0, 'V'},
{NULL, 0, NULL, 0}
};
static const ul_excl_t excl[] = { /* rows and cols in ASCII order */
{ 'm', 'w' }, /* preserve-environment, whitelist-environment */
{ 'p', 'w' }, /* preserve-environment, whitelist-environment */
{ 0 }
};
int excl_st[ARRAY_SIZE(excl)] = UL_EXCL_STATUS_INIT;

setlocale(LC_ALL, "");
bindtextdomain(PACKAGE, LOCALEDIR);
Expand All @@ -1248,8 +1325,11 @@ int su_main(int argc, char **argv, int mode)
su->conv.appdata_ptr = (void *) su;

while ((optc =
getopt_long(argc, argv, "c:fg:G:lmpPs:u:hV", longopts,
getopt_long(argc, argv, "c:fg:G:lmpPs:u:hVw:", longopts,
NULL)) != -1) {

err_exclusive_options(optc, longopts, excl, excl_st);

switch (optc) {
case 'c':
command = optarg;
Expand Down Expand Up @@ -1283,6 +1363,10 @@ int su_main(int argc, char **argv, int mode)
su->change_environment = false;
break;

case 'w':
env_whitelist_from_string(su, optarg);
break;

case 'P':
#ifdef USE_PTY
su->pty = 1;
Expand Down
10 changes: 10 additions & 0 deletions login-utils/su.1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ login:
o
clears all the environment variables except
.B TERM
and variables specified by \fB\-\-whitelist\-environment\fR
.TP
o
initializes the environment variables
Expand Down Expand Up @@ -153,6 +154,15 @@ Same as
.B \-c
but do not create a new session. (Discouraged.)
.TP
.BR \-w , " \-\-whitelist\-environment" = \fIlist
Don't reset environment variables specified in comma separated \fIlist\fR when clears
environment for \fB\-\-login\fR. The whitelist is ignored for the environment variables
.BR HOME ,
.BR SHELL ,
.BR USER ,
.BR LOGNAME ", and"
.BR PATH "."
.TP
.BR \-V , " \-\-version"
Display version information and exit.
.TP
Expand Down

0 comments on commit 75efef9

Please sign in to comment.