rework systemd's own process environment handling/passing

Stop importing non-sensical kernel-exported variables. All
parameters in the kernel command line are exported to the
initial environment of PID1, but suppressed if they are
recognized by kernel built-in code. The EFI booted kernel
will add further kernel-internal things which do not belong
into userspace.

The passed original environ data of the process is not touched
and preserved across re-execution, to allow external reading of
/proc/self/environ for process properties like container*=.

8 files changed, 77 insertions, 97 deletions

diff --git a/man/systemd.xml b/man/systemd.xml
index 1e54eac92..32bca0b60 100644
--- a/man/systemd.xml
+++ b/man/systemd.xml
@@ -1113,16 +1113,11 @@
                                 <term><varname>systemd.setenv=</varname></term>
 
                                 <listitem><para>Takes a string
-                                argument in the form
-                                VARIABLE=VALUE. May be used to set
-                                environment variables for the init
-                                process and all its children at boot
-                                time. May be used more than once to
-                                set multiple variables. If the equal
-                                sign and variable are missing it unsets
-                                an environment variable which might be
-                                passed in from the initial ram
-                                disk.</para></listitem>
+                                argument in the form VARIABLE=VALUE.
+                                May be used to set default environment
+                                variables to add to forked child processes.
+                                May be used more than once to set multiple
+                                variables.</para></listitem>
                         </varlistentry>
 
                         <varlistentry>
diff --git a/src/core/locale-setup.c b/src/core/locale-setup.c
index daf81d080..31374ac64 100644
--- a/src/core/locale-setup.c
+++ b/src/core/locale-setup.c
@@ -28,6 +28,7 @@
 #include "macro.h"
 #include "virt.h"
 #include "fileio.h"
+#include "strv.h"
 
 enum {
         /* We don't list LC_ALL here on purpose. People should be
@@ -67,7 +68,8 @@ static const char * const variable_names[_VARIABLE_MAX] = {
         [VARIABLE_LC_IDENTIFICATION] = "LC_IDENTIFICATION"
 };
 
-int locale_setup(void) {
+int locale_setup(char ***environment) {
+        char **env;
         char *variables[_VARIABLE_MAX] = {};
         int r = 0, i;
 
@@ -118,13 +120,16 @@ int locale_setup(void) {
         }
 
         for (i = 0; i < _VARIABLE_MAX; i++) {
-                if (variables[i]) {
-                        if (setenv(variable_names[i], variables[i], 1) < 0) {
-                                r = -errno;
-                                goto finish;
-                        }
-                } else
-                        unsetenv(variable_names[i]);
+                if (!variables[i])
+                        continue;
+
+                env = strv_appendf(*environment, "%s=%s", variable_names[i], variables[i]);
+                if (!env) {
+                        r = -ENOMEM;
+                        goto finish;
+                }
+
+                *environment = env;
         }
 
         r = 0;
diff --git a/src/core/locale-setup.h b/src/core/locale-setup.h
index 5a0f2f788..62c654c37 100644
--- a/src/core/locale-setup.h
+++ b/src/core/locale-setup.h
@@ -21,4 +21,4 @@
   along with systemd; If not, see <http://www.gnu.org/licenses/>.
 ***/
 
-int locale_setup(void);
+int locale_setup(char ***environment);
diff --git a/src/core/main.c b/src/core/main.c
index ad155e13c..91cbee2e5 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -64,7 +64,6 @@
 #endif
 #include "hostname-setup.h"
 #include "machine-id-setup.h"
-#include "locale-setup.h"
 #include "selinux-setup.h"
 #include "ima-setup.h"
 #include "fileio.h"
@@ -349,32 +348,21 @@ static int parse_proc_cmdline_word(const char *word) {
                         arg_default_std_error = r;
         } else if (startswith(word, "systemd.setenv=")) {
                 _cleanup_free_ char *cenv = NULL;
-                char *eq;
-                int r;
 
                 cenv = strdup(word + 15);
                 if (!cenv)
                         return -ENOMEM;
 
-                eq = strchr(cenv, '=');
-                if (!eq) {
-                        if (!env_name_is_valid(cenv))
-                                log_warning("Environment variable name '%s' is not valid. Ignoring.", cenv);
-                        else  {
-                                r = unsetenv(cenv);
-                                if (r < 0)
-                                        log_warning("Unsetting environment variable '%s' failed, ignoring: %m", cenv);
-                        }
-                } else {
-                        if (!env_assignment_is_valid(cenv))
-                                log_warning("Environment variable assignment '%s' is not valid. Ignoring.", cenv);
-                        else {
-                                *eq = 0;
-                                r = setenv(cenv, eq + 1, 1);
-                                if (r < 0)
-                                        log_warning("Setting environment variable '%s=%s' failed, ignoring: %m", cenv, eq + 1);
-                        }
-                }
+                if (env_assignment_is_valid(cenv)) {
+                        char **env;
+
+                        env = strv_env_set(arg_default_environment, cenv);
+                        if (env)
+                                arg_default_environment = env;
+                        else
+                                log_warning("Setting environment variable '%s' failed, ignoring: %m", cenv);
+                } else
+                        log_warning("Environment variable name '%s' is not valid. Ignoring.", cenv);
 
         } else if (startswith(word, "systemd.") ||
                    (in_initrd() && startswith(word, "rd.systemd."))) {
@@ -1445,42 +1433,7 @@ int main(int argc, char *argv[]) {
         if (serialization)
                 assert_se(fdset_remove(fds, fileno(serialization)) >= 0);
 
-        /* Set up PATH unless it is already set */
-        setenv("PATH",
-#ifdef HAVE_SPLIT_USR
-               "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
-#else
-               "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin",
-#endif
-               arg_running_as == SYSTEMD_SYSTEM);
-
         if (arg_running_as == SYSTEMD_SYSTEM) {
-                /* Unset some environment variables passed in from the
-                 * kernel that don't really make sense for us. */
-                unsetenv("HOME");
-                unsetenv("TERM");
-
-                /* When we are invoked by a shell, these might be set,
-                 * but make little sense to pass on */
-                unsetenv("PWD");
-                unsetenv("SHLVL");
-                unsetenv("_");
-
-                /* When we are invoked by a chroot-like tool such as
-                 * nspawn, these might be set, but make little sense
-                 * to pass on */
-                unsetenv("USER");
-                unsetenv("LOGNAME");
-
-                /* We suppress the socket activation env vars, as
-                 * we'll try to match *any* open fd to units if
-                 * possible. */
-                unsetenv("LISTEN_FDS");
-                unsetenv("LISTEN_PID");
-
-                /* All other variables are left as is, so that clients
-                 * can still read them via /proc/1/environ */
-
                 /* Become a session leader if we aren't one yet. */
                 setsid();
 
@@ -1528,8 +1481,6 @@ int main(int argc, char *argv[]) {
                 log_debug(PACKAGE_STRING " running in user mode. (" SYSTEMD_FEATURES ")");
 
         if (arg_running_as == SYSTEMD_SYSTEM && !skip_setup) {
-                locale_setup();
-
                 if (arg_show_status || plymouth_running())
                         status_welcome();
 
@@ -1595,7 +1546,7 @@ int main(int argc, char *argv[]) {
         manager_set_default_rlimits(m, arg_default_rlimit);
 
         if (arg_default_environment)
-                manager_set_default_environment(m, arg_default_environment);
+                manager_environment_add(m, arg_default_environment);
 
         manager_set_show_status(m, arg_show_status);
 
diff --git a/src/core/manager.c b/src/core/manager.c
index 6c082c96b..10ccffb40 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -55,6 +55,7 @@
 #include "util.h"
 #include "mkdir.h"
 #include "ratelimit.h"
+#include "locale-setup.h"
 #include "mount-setup.h"
 #include "unit-name.h"
 #include "dbus-unit.h"
@@ -454,22 +455,36 @@ static int manager_setup_signals(Manager *m) {
         return 0;
 }
 
-static void manager_strip_environment(Manager *m) {
+static int manager_default_environment(Manager *m) {
+#ifdef HAVE_SPLIT_USR
+        const char *path = "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin";
+#else
+        const char *path = "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin";
+#endif
+
         assert(m);
 
-        /* Remove variables from the inherited set that are part of
-         * the container interface:
-         * http://www.freedesktop.org/wiki/Software/systemd/ContainerInterface */
-        strv_remove_prefix(m->environment, "container=");
-        strv_remove_prefix(m->environment, "container_");
+        if (m->running_as == SYSTEMD_SYSTEM) {
+                /* The system manager always starts with a clean
+                 * environment for its children. It does not import
+                 * the kernel or the parents exported variables.
+                 *
+                 * The initial passed environ is untouched to keep
+                 * /proc/self/environ valid; it is used for tagging
+                 * the init process inside containers. */
+                m->environment = strv_new(path, NULL);
+
+                /* Import locale variables LC_*= from configuration */
+                locale_setup(&m->environment);
+        } else
+                /* The user manager passes its own environment
+                 * along to its children. */
+                m->environment = strv_copy(environ);
 
-        /* Remove variables from the inherited set that are part of
-         * the initrd interface:
-         * http://www.freedesktop.org/wiki/Software/systemd/InitrdInterface */
-        strv_remove_prefix(m->environment, "RD_");
+        if (!m->environment)
+                return -ENOMEM;
 
-        /* Drop invalid entries */
-        strv_env_clean(m->environment);
+        return 0;
 }
 
 int manager_new(SystemdRunningAs running_as, bool reexecuting, Manager **_m) {
@@ -505,12 +520,10 @@ int manager_new(SystemdRunningAs running_as, bool reexecuting, Manager **_m) {
         m->epoll_fd = m->dev_autofs_fd = -1;
         m->current_job_id = 1; /* start as id #1, so that we can leave #0 around as "null-like" value */
 
-        m->environment = strv_copy(environ);
-        if (!m->environment)
+        r = manager_default_environment(m);
+        if (r < 0)
                 goto fail;
 
-        manager_strip_environment(m);
-
         if (!(m->units = hashmap_new(string_hash_func, string_compare_func)))
                 goto fail;
 
@@ -2651,7 +2664,7 @@ void manager_undo_generators(Manager *m) {
         remove_generator_dir(m, &m->generator_unit_path_late);
 }
 
-int manager_set_default_environment(Manager *m, char **environment) {
+int manager_environment_add(Manager *m, char **environment) {
 
         char **e = NULL;
         assert(m);
diff --git a/src/core/manager.h b/src/core/manager.h
index bd068ac3c..3969553e3 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -281,7 +281,7 @@ unsigned manager_dispatch_load_queue(Manager *m);
 unsigned manager_dispatch_run_queue(Manager *m);
 unsigned manager_dispatch_dbus_queue(Manager *m);
 
-int manager_set_default_environment(Manager *m, char **environment);
+int manager_environment_add(Manager *m, char **environment);
 int manager_set_default_rlimits(Manager *m, struct rlimit **default_rlimit);
 
 int manager_loop(Manager *m);
diff --git a/src/shared/strv.c b/src/shared/strv.c
index a5ce7e959..3e7778d61 100644
--- a/src/shared/strv.c
+++ b/src/shared/strv.c
@@ -387,6 +387,21 @@ fail:
         return NULL;
 }
 
+char **strv_appendf(char **l, const char *format, ...) {
+        va_list ap;
+        _cleanup_free_ char *s = NULL;
+        int r;
+
+        va_start(ap, format);
+        r = vasprintf(&s, format, ap);
+        va_end(ap);
+
+        if (r < 0)
+                return NULL;
+
+        return strv_append(l, s);
+}
+
 int strv_push(char ***l, char *value) {
         char **c;
         unsigned n;
diff --git a/src/shared/strv.h b/src/shared/strv.h
index e35118752..4ade827a4 100644
--- a/src/shared/strv.h
+++ b/src/shared/strv.h
@@ -42,6 +42,7 @@ unsigned strv_length(char * const *l) _pure_;
 char **strv_merge(char **a, char **b);
 char **strv_merge_concat(char **a, char **b, const char *suffix);
 char **strv_append(char **l, const char *s);
+char **strv_appendf(char **l, const char *format, ...) _printf_attr_(2, 3);
 int strv_extend(char ***l, const char *value);
 int strv_push(char ***l, char *value);