ryantimwilson / rpms / systemd

Forked from rpms/systemd 2 months ago
Clone
a19bc6
From ddad59f50e67a5e36cd9c40e774d28240a6a7c0c Mon Sep 17 00:00:00 2001
a19bc6
From: Filipe Brandenburger <filbranden@google.com>
a19bc6
Date: Wed, 11 Nov 2015 09:24:34 -0800
a19bc6
Subject: [PATCH] test-execute: Clarify interaction of PassEnvironment= and
a19bc6
 MANAGER_USER
a19bc6
a19bc6
@evverx brought up that test-execute runs under MANAGER_USER which
a19bc6
forwards all its environment variables to the services. It turns out it
a19bc6
only forwards those that were in the environment at the time of manager
a19bc6
creation, so this test was still working.
a19bc6
a19bc6
It was still possible to attack it by running something like:
a19bc6
  $ sudo VAR1=a VAR2=b VAR3=c ./test-execute
a19bc6
a19bc6
Prevent that attack by unsetting the three variables explicitly before
a19bc6
creating the manager for the test case.
a19bc6
a19bc6
Also add comments explaining the interactions with MANAGER_USER and,
a19bc6
while it has some caveats, this tests are still valid in that context.
a19bc6
a19bc6
Tested by checking that the test running with the variables set from the
a19bc6
external environment will still pass.
a19bc6
a19bc6
(cherry picked from commit e1abca2ee42e5938ee1f2542c3eba9e70edb0be2)
a19bc6
a19bc6
Related: #1426214
a19bc6
---
a19bc6
 src/test/test-execute.c | 21 +++++++++++++++++++++
a19bc6
 1 file changed, 21 insertions(+)
a19bc6
a19bc6
diff --git a/src/test/test-execute.c b/src/test/test-execute.c
a19bc6
index 8def194..6e5567c 100644
a19bc6
--- a/src/test/test-execute.c
a19bc6
+++ b/src/test/test-execute.c
a19bc6
@@ -143,6 +143,17 @@ static void test_exec_environment(Manager *m) {
a19bc6
 }
a19bc6
 
a19bc6
 static void test_exec_passenvironment(Manager *m) {
a19bc6
+        /* test-execute runs under MANAGER_USER which, by default, forwards all
a19bc6
+         * variables present in the environment, but only those that are
a19bc6
+         * present _at the time it is created_!
a19bc6
+         *
a19bc6
+         * So these PassEnvironment checks are still expected to work, since we
a19bc6
+         * are ensuring the variables are not present at manager creation (they
a19bc6
+         * are unset explicitly in main) and are only set here.
a19bc6
+         *
a19bc6
+         * This is still a good approximation of how a test for MANAGER_SYSTEM
a19bc6
+         * would work.
a19bc6
+         */
a19bc6
         assert_se(setenv("VAR1", "word1 word2", 1) == 0);
a19bc6
         assert_se(setenv("VAR2", "word3", 1) == 0);
a19bc6
         assert_se(setenv("VAR3", "$word 5 6", 1) == 0);
a19bc6
@@ -199,6 +210,16 @@ int main(int argc, char *argv[]) {
a19bc6
         assert_se(setenv("XDG_RUNTIME_DIR", "/tmp/", 1) == 0);
a19bc6
         assert_se(set_unit_path(TEST_DIR ":") >= 0);
a19bc6
 
a19bc6
+        /* Unset VAR1, VAR2 and VAR3 which are used in the PassEnvironment test
a19bc6
+         * cases, otherwise (and if they are present in the environment),
a19bc6
+         * `manager_default_environment` will copy them into the default
a19bc6
+         * environment which is passed to each created job, which will make the
a19bc6
+         * tests that expect those not to be present to fail.
a19bc6
+         */
a19bc6
+        assert_se(unsetenv("VAR1") == 0);
a19bc6
+        assert_se(unsetenv("VAR2") == 0);
a19bc6
+        assert_se(unsetenv("VAR3") == 0);
a19bc6
+
a19bc6
         r = manager_new(SYSTEMD_USER, true, &m);
a19bc6
         if (IN_SET(r, -EPERM, -EACCES, -EADDRINUSE, -EHOSTDOWN, -ENOENT)) {
a19bc6
                 printf("Skipping test: manager_new: %s", strerror(-r));