Merge pull request #1852 from filbranden/passenv6

execute: Add new PassEnvironment= directive (v4)

Makefile.am

@@ -1546,6 +1546,10 @@ EXTRA_DIST += \
 	test/test-execute/exec-environment-empty.service \
 	test/test-execute/exec-environment-multiple.service \
 	test/test-execute/exec-environment.service \
+	test/test-execute/exec-passenvironment-absent.service \
+	test/test-execute/exec-passenvironment-empty.service \
+	test/test-execute/exec-passenvironment-repeated.service \
+	test/test-execute/exec-passenvironment.service \
 	test/test-execute/exec-group.service \
 	test/test-execute/exec-ignoresigpipe-no.service \
 	test/test-execute/exec-ignoresigpipe-yes.service \

TODO

@@ -79,8 +79,6 @@ Features:
   prefixed with /sys generally special.
   http://lists.freedesktop.org/archives/systemd-devel/2015-June/032962.html
 
-* Add PassEnvironment= setting to service units, to import select env vars from PID 1 into the service env block
-
 * nspawn: fix logic always print a final newline on output.
   https://github.com/systemd/systemd/pull/272#issuecomment-113153176
 

man/systemd.exec.xml

@@ -302,6 +302,33 @@
         earlier setting.</para></listitem>
       </varlistentry>
 
+      <varlistentry>
+        <term><varname>PassEnvironment=</varname></term>
+
+        <listitem><para>Pass environment variables from the systemd system
+        manager to executed processes. Takes a space-separated list of variable
+        names. This option may be specified more than once, in which case all
+        listed variables will be set. If the empty string is assigned to this
+        option, the list of environment variables is reset, all prior
+        assignments have no effect. Variables that are not set in the system
+        manager will not be passed and will be silently ignored.</para>
+
+        <para>Variables passed from this setting are overridden by those passed
+        from <varname>Environment=</varname> or
+        <varname>EnvironmentFile=</varname>.</para>
+
+        <para>Example:
+        <programlisting>PassEnvironment=VAR1 VAR2 VAR3</programlisting>
+        passes three variables <literal>VAR1</literal>,
+        <literal>VAR2</literal>, <literal>VAR3</literal>
+        with the values set for those variables in PID1.</para>
+
+        <para>
+        See
+        <citerefentry project='man-pages'><refentrytitle>environ</refentrytitle><manvolnum>7</manvolnum></citerefentry>
+        for details about environment variables.</para></listitem>
+      </varlistentry>
+
       <varlistentry>
         <term><varname>StandardInput=</varname></term>
         <listitem><para>Controls where file descriptor 0 (STDIN) of

shell-completion/bash/systemd-run

@@ -86,7 +86,7 @@ _systemd_run() {
                          TTYPath= SyslogIdentifier= SyslogLevelPrefix= SyslogLevel=
                          SyslogFacility= TimerSlackNSec= OOMScoreAdjust= ReadWriteDirectories=
                          ReadOnlyDirectories= InaccessibleDirectories= EnvironmentFile=
-                         ProtectSystem= ProtectHome= RuntimeDirectory='
+                         ProtectSystem= ProtectHome= RuntimeDirectory= PassEnvironment='
 
             COMPREPLY=( $(compgen -W '$comps' -- "$cur") )
             return 0

shell-completion/zsh/_systemd-run

@@ -39,7 +39,7 @@ _arguments \
                 TTYPath= SyslogIdentifier= SyslogLevelPrefix= SyslogLevel= \
                 SyslogFacility= TimerSlackNSec= OOMScoreAdjust= ReadWriteDirectories= \
                 ReadOnlyDirectories= InaccessibleDirectories= EnvironmentFile= \
-                ProtectSystem= ProtectHome= RuntimeDirectory= \
+                ProtectSystem= ProtectHome= RuntimeDirectory= PassEnvironment= \
                 ))' \
         '--description=[Description for unit]:description' \
         '--slice=[Run in the specified slice]:slices:__slices' \

src/basic/env-util.c

@@ -138,6 +138,21 @@ bool strv_env_is_valid(char **e) {
         return true;
 }
 
+bool strv_env_name_is_valid(char **l) {
+        char **p, **q;
+
+        STRV_FOREACH(p, l) {
+                if (!env_name_is_valid(*p))
+                        return false;
+
+                STRV_FOREACH(q, p + 1)
+                        if (streq(*p, *q))
+                                return false;
+        }
+
+        return true;
+}
+
 bool strv_env_name_or_assignment_is_valid(char **l) {
         char **p, **q;
 

src/basic/env-util.h

@@ -36,6 +36,7 @@ bool strv_env_is_valid(char **e);
 #define strv_env_clean(l) strv_env_clean_with_callback(l, NULL, NULL)
 char **strv_env_clean_with_callback(char **l, void (*invalid_callback)(const char *p, void *userdata), void *userdata);
 
+bool strv_env_name_is_valid(char **l);
 bool strv_env_name_or_assignment_is_valid(char **l);
 
 char **strv_env_merge(unsigned n_lists, ...);

src/core/dbus-execute.c

@@ -629,6 +629,7 @@ const sd_bus_vtable bus_exec_vtable[] = {
         SD_BUS_VTABLE_START(0),
         SD_BUS_PROPERTY("Environment", "as", NULL, offsetof(ExecContext, environment), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("EnvironmentFiles", "a(sb)", property_get_environment_files, 0, SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("PassEnvironment", "as", NULL, offsetof(ExecContext, pass_environment), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("UMask", "u", bus_property_get_mode, offsetof(ExecContext, umask), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("LimitCPU", "t", bus_property_get_rlimit, offsetof(ExecContext, rlimit[RLIMIT_CPU]), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("LimitFSIZE", "t", bus_property_get_rlimit, offsetof(ExecContext, rlimit[RLIMIT_FSIZE]), SD_BUS_VTABLE_PROPERTY_CONST),
@@ -1289,6 +1290,38 @@ int bus_exec_context_set_transient_property(
 
                 return 1;
 
+        } else if (streq(name, "PassEnvironment")) {
+
+                _cleanup_strv_free_ char **l = NULL;
+
+                r = sd_bus_message_read_strv(message, &l);
+                if (r < 0)
+                        return r;
+
+                if (!strv_env_name_is_valid(l))
+                        return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid PassEnvironment block.");
+
+                if (mode != UNIT_CHECK) {
+                        if (strv_isempty(l)) {
+                                c->pass_environment = strv_free(c->pass_environment);
+                                unit_write_drop_in_private_format(u, mode, name, "PassEnvironment=\n");
+                        } else {
+                                _cleanup_free_ char *joined = NULL;
+
+                                r = strv_extend_strv(&c->pass_environment, l, true);
+                                if (r < 0)
+                                        return r;
+
+                                joined = strv_join_quoted(c->pass_environment);
+                                if (!joined)
+                                        return -ENOMEM;
+
+                                unit_write_drop_in_private_format(u, mode, name, "PassEnvironment=%s\n", joined);
+                        }
+                }
+
+                return 1;
+
         } else if (STR_IN_SET(name, "ReadWriteDirectories", "ReadOnlyDirectories", "InaccessibleDirectories")) {
 
                 _cleanup_strv_free_ char **l = NULL;

src/core/execute.c

@@ -1332,6 +1332,34 @@ static int build_environment(
         return 0;
 }
 
+static int build_pass_environment(const ExecContext *c, char ***ret) {
+        _cleanup_strv_free_ char **pass_env = NULL;
+        size_t n_env = 0, n_bufsize = 0;
+        char **i;
+
+        STRV_FOREACH(i, c->pass_environment) {
+                _cleanup_free_ char *x = NULL;
+                char *v;
+
+                v = getenv(*i);
+                if (!v)
+                        continue;
+                x = strjoin(*i, "=", v, NULL);
+                if (!x)
+                        return -ENOMEM;
+                if (!GREEDY_REALLOC(pass_env, n_bufsize, n_env + 2))
+                        return -ENOMEM;
+                pass_env[n_env++] = x;
+                pass_env[n_env] = NULL;
+                x = NULL;
+        }
+
+        *ret = pass_env;
+        pass_env = NULL;
+
+        return 0;
+}
+
 static bool exec_needs_mount_namespace(
                 const ExecContext *context,
                 const ExecParameters *params,
@@ -1412,7 +1440,7 @@ static int exec_child(
                 char **files_env,
                 int *exit_status) {
 
-        _cleanup_strv_free_ char **our_env = NULL, **pam_env = NULL, **final_env = NULL, **final_argv = NULL;
+        _cleanup_strv_free_ char **our_env = NULL, **pass_env = NULL, **pam_env = NULL, **final_env = NULL, **final_argv = NULL;
         _cleanup_free_ char *mac_selinux_context_net = NULL;
         const char *username = NULL, *home = NULL, *shell = NULL, *wd;
         uid_t uid = UID_INVALID;
@@ -1928,9 +1956,16 @@ static int exec_child(
                 return r;
         }
 
-        final_env = strv_env_merge(5,
+        r = build_pass_environment(context, &pass_env);
+        if (r < 0) {
+                *exit_status = EXIT_MEMORY;
+                return r;
+        }
+
+        final_env = strv_env_merge(6,
                                    params->environment,
                                    our_env,
+                                   pass_env,
                                    context->environment,
                                    files_env,
                                    pam_env,
@@ -2088,6 +2123,7 @@ void exec_context_done(ExecContext *c) {
 
         c->environment = strv_free(c->environment);
         c->environment_files = strv_free(c->environment_files);
+        c->pass_environment = strv_free(c->pass_environment);
 
         for (l = 0; l < ELEMENTSOF(c->rlimit); l++)
                 c->rlimit[l] = mfree(c->rlimit[l]);
@@ -2358,6 +2394,9 @@ void exec_context_dump(ExecContext *c, FILE* f, const char *prefix) {
         STRV_FOREACH(e, c->environment_files)
                 fprintf(f, "%sEnvironmentFile: %s\n", prefix, *e);
 
+        STRV_FOREACH(e, c->pass_environment)
+                fprintf(f, "%sPassEnvironment: %s\n", prefix, *e);
+
         fprintf(f, "%sRuntimeDirectoryMode: %04o\n", prefix, c->runtime_directory_mode);
 
         STRV_FOREACH(d, c->runtime_directory)

src/core/execute.h

@@ -99,6 +99,7 @@ struct ExecRuntime {
 struct ExecContext {
         char **environment;
         char **environment_files;
+        char **pass_environment;
 
         struct rlimit *rlimit[_RLIMIT_MAX];
         char *working_directory, *root_directory;

src/core/load-fragment-gperf.gperf.m4

@@ -33,6 +33,7 @@ $1.CPUAffinity,                  config_parse_exec_cpu_affinity,     0,
 $1.UMask,                        config_parse_mode,                  0,                             offsetof($1, exec_context.umask)
 $1.Environment,                  config_parse_environ,               0,                             offsetof($1, exec_context.environment)
 $1.EnvironmentFile,              config_parse_unit_env_file,         0,                             offsetof($1, exec_context.environment_files)
+$1.PassEnvironment,              config_parse_pass_environ,          0,                             offsetof($1, exec_context.pass_environment)
 $1.StandardInput,                config_parse_input,                 0,                             offsetof($1, exec_context.std_input)
 $1.StandardOutput,               config_parse_output,                0,                             offsetof($1, exec_context.std_output)
 $1.StandardError,                config_parse_output,                0,                             offsetof($1, exec_context.std_error)

src/core/load-fragment.c

@@ -2196,6 +2196,70 @@ int config_parse_environ(const char *unit,
         return 0;
 }
 
+int config_parse_pass_environ(const char *unit,
+                              const char *filename,
+                              unsigned line,
+                              const char *section,
+                              unsigned section_line,
+                              const char *lvalue,
+                              int ltype,
+                              const char *rvalue,
+                              void *data,
+                              void *userdata) {
+
+        const char *whole_rvalue = rvalue;
+        char*** passenv = data;
+        _cleanup_strv_free_ char **n = NULL;
+        size_t nlen = 0, nbufsize = 0;
+        int r;
+
+        assert(filename);
+        assert(lvalue);
+        assert(rvalue);
+        assert(data);
+
+        if (isempty(rvalue)) {
+                /* Empty assignment resets the list */
+                *passenv = strv_free(*passenv);
+                return 0;
+        }
+
+        for (;;) {
+                _cleanup_free_ char *word = NULL;
+
+                r = extract_first_word(&rvalue, &word, WHITESPACE, EXTRACT_QUOTES);
+                if (r == 0)
+                        break;
+                if (r == -ENOMEM)
+                        return log_oom();
+                if (r < 0) {
+                        log_syntax(unit, LOG_ERR, filename, line, r,
+                                   "Trailing garbage in %s, ignoring: %s", lvalue, whole_rvalue);
+                        break;
+                }
+
+                if (!env_name_is_valid(word)) {
+                        log_syntax(unit, LOG_ERR, filename, line, EINVAL,
+                                   "Invalid environment name for %s, ignoring: %s", lvalue, word);
+                        continue;
+                }
+
+                if (!GREEDY_REALLOC(n, nbufsize, nlen + 2))
+                        return log_oom();
+                n[nlen++] = word;
+                n[nlen] = NULL;
+                word = NULL;
+        }
+
+        if (n) {
+                r = strv_extend_strv(passenv, n, true);
+                if (r < 0)
+                        return r;
+        }
+
+        return 0;
+}
+
 int config_parse_ip_tos(const char *unit,
                         const char *filename,
                         unsigned line,

src/core/load-fragment.h

@@ -84,6 +84,7 @@ int config_parse_syscall_filter(const char *unit, const char *filename, unsigned
 int config_parse_syscall_archs(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_syscall_errno(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_environ(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
+int config_parse_pass_environ(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_unit_slice(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_cpu_shares(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_memory_limit(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);

src/shared/bus-util.c

@@ -1654,7 +1654,7 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen
 
                 r = sd_bus_message_append(m, "v", "i", i);
 
-        } else if (streq(field, "Environment")) {
+        } else if (STR_IN_SET(field, "Environment", "PassEnvironment")) {
                 const char *p;
 
                 r = sd_bus_message_open_container(m, 'v', "as");
@@ -1678,9 +1678,16 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen
                         if (r == 0)
                                 break;
 
-                        if (!env_assignment_is_valid(word)) {
-                                log_error("Invalid environment assignment: %s", eq);
-                                return -EINVAL;
+                        if (streq(field, "Environment")) {
+                                if (!env_assignment_is_valid(word)) {
+                                        log_error("Invalid environment assignment: %s", word);
+                                        return -EINVAL;
+                                }
+                        } else {  /* PassEnvironment */
+                                if (!env_name_is_valid(word)) {
+                                        log_error("Invalid environment variable name: %s", word);
+                                        return -EINVAL;
+                                }
                         }
 
                         r = sd_bus_message_append_basic(m, 's', word);

src/test/test-execute.c

@@ -168,6 +168,30 @@ static void test_exec_environmentfile(Manager *m) {
         unlink("/tmp/test-exec_environmentfile.conf");
 }
 
+static void test_exec_passenvironment(Manager *m) {
+        /* test-execute runs under MANAGER_USER which, by default, forwards all
+         * variables present in the environment, but only those that are
+         * present _at the time it is created_!
+         *
+         * So these PassEnvironment checks are still expected to work, since we
+         * are ensuring the variables are not present at manager creation (they
+         * are unset explicitly in main) and are only set here.
+         *
+         * This is still a good approximation of how a test for MANAGER_SYSTEM
+         * would work.
+         */
+        assert_se(setenv("VAR1", "word1 word2", 1) == 0);
+        assert_se(setenv("VAR2", "word3", 1) == 0);
+        assert_se(setenv("VAR3", "$word 5 6", 1) == 0);
+        test(m, "exec-passenvironment.service", 0, CLD_EXITED);
+        test(m, "exec-passenvironment-repeated.service", 0, CLD_EXITED);
+        test(m, "exec-passenvironment-empty.service", 0, CLD_EXITED);
+        assert_se(unsetenv("VAR1") == 0);
+        assert_se(unsetenv("VAR2") == 0);
+        assert_se(unsetenv("VAR3") == 0);
+        test(m, "exec-passenvironment-absent.service", 0, CLD_EXITED);
+}
+
 static void test_exec_umask(Manager *m) {
         test(m, "exec-umask-default.service", 0, CLD_EXITED);
         test(m, "exec-umask-0177.service", 0, CLD_EXITED);
@@ -237,6 +261,7 @@ int main(int argc, char *argv[]) {
                 test_exec_group,
                 test_exec_environment,
                 test_exec_environmentfile,
+                test_exec_passenvironment,
                 test_exec_umask,
                 test_exec_runtimedirectory,
                 test_exec_capabilityboundingset,
@@ -260,6 +285,16 @@ int main(int argc, char *argv[]) {
         assert_se(setenv("XDG_RUNTIME_DIR", "/tmp/", 1) == 0);
         assert_se(set_unit_path(TEST_DIR "/test-execute/") >= 0);
 
+        /* Unset VAR1, VAR2 and VAR3 which are used in the PassEnvironment test
+         * cases, otherwise (and if they are present in the environment),
+         * `manager_default_environment` will copy them into the default
+         * environment which is passed to each created job, which will make the
+         * tests that expect those not to be present to fail.
+         */
+        assert_se(unsetenv("VAR1") == 0);
+        assert_se(unsetenv("VAR2") == 0);
+        assert_se(unsetenv("VAR3") == 0);
+
         r = manager_new(MANAGER_USER, true, &m);
         if (IN_SET(r, -EPERM, -EACCES, -EADDRINUSE, -EHOSTDOWN, -ENOENT)) {
                 printf("Skipping test: manager_new: %s", strerror(-r));

src/test/test-unit-file.c

@@ -741,6 +741,44 @@ static void test_config_parse_rlimit(void) {
         rl[RLIMIT_RTTIME] = mfree(rl[RLIMIT_RTTIME]);
 }
 
+static void test_config_parse_pass_environ(void) {
+        /* int config_parse_pass_environ(
+                 const char *unit,
+                 const char *filename,
+                 unsigned line,
+                 const char *section,
+                 unsigned section_line,
+                 const char *lvalue,
+                 int ltype,
+                 const char *rvalue,
+                 void *data,
+                 void *userdata) */
+        int r;
+        _cleanup_strv_free_ char **passenv = NULL;
+
+        r = config_parse_pass_environ(NULL, "fake", 1, "section", 1,
+                              "PassEnvironment", 0, "A B",
+                              &passenv, NULL);
+        assert_se(r >= 0);
+        assert_se(strv_length(passenv) == 2);
+        assert_se(streq(passenv[0], "A"));
+        assert_se(streq(passenv[1], "B"));
+
+        r = config_parse_pass_environ(NULL, "fake", 1, "section", 1,
+                              "PassEnvironment", 0, "",
+                              &passenv, NULL);
+        assert_se(r >= 0);
+        assert_se(strv_isempty(passenv));
+
+        r = config_parse_pass_environ(NULL, "fake", 1, "section", 1,
+                              "PassEnvironment", 0, "'invalid name' 'normal_name' A=1 \\",
+                              &passenv, NULL);
+        assert_se(r >= 0);
+        assert_se(strv_length(passenv) == 1);
+        assert_se(streq(passenv[0], "normal_name"));
+
+}
+
 int main(int argc, char *argv[]) {
         int r;
 
@@ -751,6 +789,7 @@ int main(int argc, char *argv[]) {
         test_config_parse_exec();
         test_config_parse_bounding_set();
         test_config_parse_rlimit();
+        test_config_parse_pass_environ();
         test_load_env_file_1();
         test_load_env_file_2();
         test_load_env_file_3();

test/test-execute/exec-passenvironment-absent.service

@@ -0,0 +1,7 @@
+[Unit]
+Description=Test for PassEnvironment with variables absent from the execution environment
+
+[Service]
+ExecStart=/bin/sh -x -c 'test "$${VAR1-unset}" = "unset" && test "$${VAR2-unset}" = "unset" && test "$${VAR3-unset}" = "unset"'
+Type=oneshot
+PassEnvironment=VAR1 VAR2 VAR3

test/test-execute/exec-passenvironment-empty.service

@@ -0,0 +1,8 @@
+[Unit]
+Description=Test for PassEnvironment and erasing the variable list
+
+[Service]
+ExecStart=/bin/sh -x -c 'test "$${VAR1-unset}" = "unset" && test "$${VAR2-unset}" = "unset" && test "$${VAR3-unset}" = "unset"'
+Type=oneshot
+PassEnvironment=VAR1 VAR2 VAR3
+PassEnvironment=

test/test-execute/exec-passenvironment-repeated.service

@@ -0,0 +1,8 @@
+[Unit]
+Description=Test for PassEnvironment with a variable name repeated
+
+[Service]
+ExecStart=/bin/sh -x -c 'test "$$VAR1" = "word1 word2" && test "$$VAR2" = word3 && test "$$VAR3" = "\\$$word 5 6"'
+Type=oneshot
+PassEnvironment=VAR1 VAR2
+PassEnvironment=VAR1 VAR3

test/test-execute/exec-passenvironment.service

@@ -0,0 +1,7 @@
+[Unit]
+Description=Test for PassEnvironment
+
+[Service]
+ExecStart=/bin/sh -x -c 'test "$$VAR1" = "word1 word2" && test "$$VAR2" = word3 && test "$$VAR3" = "\\$$word 5 6"'
+Type=oneshot
+PassEnvironment=VAR1 VAR2 VAR3