su: add --whitelist-environment

* 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: https://github.com/karelzak/util-linux/issues/221
Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/configure.ac b/configure.ac
index a41ca7689e3b507bc3a63b2e21137a9edb430ab0..995d1d70e1da0f92e8eebe01a77bdf58d9446f0f 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -454,6 +454,7 @@ AC_CHECK_DECL([SO_PASSCRED],
                #include <sys/socket.h>])
 
 AC_CHECK_FUNCS([ \
+       clearenv \
        __fpurge \
        fpurge \
        __fpending \

diff --git a/login-utils/runuser.1 b/login-utils/runuser.1
index 40b6f9b3a4392188ed573c12e095fb9de76ffd7b..af834e840a121ddaecbcfae1074f7f28e0266d75 100644 (file)
--- a/login-utils/runuser.1
+++ b/login-utils/runuser.1
@@ -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
@@ -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

diff --git a/login-utils/su-common.c b/login-utils/su-common.c
index c1b1a04e48dddb83773a07ec95393f0ca9c97adc..1b1370922c9816709ec2d93905b15c35c051cea9 100644 (file)
--- a/login-utils/su-common.c
+++ b/login-utils/su-common.c
@@ -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"
@@ -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
@@ -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;
@@ -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.
         */
@@ -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);
@@ -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);
@@ -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;
@@ -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;

diff --git a/login-utils/su.1 b/login-utils/su.1
index 84ca104ef4d86f55e504bbfe1763e775b3ffa537..709c5e65505dace9103cbbc3ce46ea9e33c67fd8 100644 (file)
--- a/login-utils/su.1
+++ b/login-utils/su.1
@@ -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
@@ -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