From f485b93f475b119e3b8fa6c9cf740207e2d2d7ac Mon Sep 17 00:00:00 2001 From: Yuriy Taraday Date: Wed, 4 Mar 2015 14:50:25 +0300 Subject: Remove env changing support in daemon mode It introduced a security issue since these env vars are not filtered by either sudo or rootwrap. This change reverts changes in common code from Iace26738f910a18a5d1d3479fad949027e5a3816 (most of them) and purges ability to specify env in arguments for daemon. Environment should be provided to callee process using EnvFilter and /usr/bin/env. Change-Id: Iafbc493d6158f3ea85b3d74cb37c29e161a1099f --- README.rst | 2 -- oslo_rootwrap/client.py | 6 +++--- oslo_rootwrap/daemon.py | 6 +----- oslo_rootwrap/filters.py | 10 ++++------ oslo_rootwrap/tests/test_functional.py | 7 ------- oslo_rootwrap/wrapper.py | 5 ++--- tests/test_functional.py | 7 ------- 7 files changed, 10 insertions(+), 33 deletions(-) diff --git a/README.rst b/README.rst index 6585cb8..75de3a4 100644 --- a/README.rst +++ b/README.rst @@ -338,8 +338,6 @@ The class provides one method ``execute`` with following arguments: * ``userargs`` - list of command line arguments that are to be used to run the command; -* ``env`` - dict of environment variables to be set for it (by default it's an - empty dict, so all environment variables are stripped); * ``stdin`` - string to be passed to standard input of child process. The method returns 3-tuple containing: diff --git a/oslo_rootwrap/client.py b/oslo_rootwrap/client.py index 5163772..cb01ce6 100644 --- a/oslo_rootwrap/client.py +++ b/oslo_rootwrap/client.py @@ -127,12 +127,12 @@ class Client(object): self._initialize() return self._proxy - def execute(self, cmd, env=None, stdin=None): + def execute(self, cmd, stdin=None): self._ensure_initialized() proxy = self._proxy retry = False try: - res = proxy.run_one_command(cmd, env, stdin) + res = proxy.run_one_command(cmd, stdin) except (EOFError, IOError): retry = True # res can be None if we received final None sent by dying server thread @@ -140,5 +140,5 @@ class Client(object): # at this point. if retry or res is None: proxy = self._restart(proxy) - res = proxy.run_one_command(cmd, env, stdin) + res = proxy.run_one_command(cmd, stdin) return res diff --git a/oslo_rootwrap/daemon.py b/oslo_rootwrap/daemon.py index 7bda2c2..21c3717 100644 --- a/oslo_rootwrap/daemon.py +++ b/oslo_rootwrap/daemon.py @@ -43,16 +43,12 @@ class RootwrapClass(object): self.config = config self.filters = filters - def run_one_command(self, userargs, env=None, stdin=None): - if env is None: - env = {} - + def run_one_command(self, userargs, stdin=None): obj = wrapper.start_subprocess( self.filters, userargs, exec_dirs=self.config.exec_dirs, log=self.config.use_syslog, close_fds=True, - env=env, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE) diff --git a/oslo_rootwrap/filters.py b/oslo_rootwrap/filters.py index b8747ae..1d5da26 100644 --- a/oslo_rootwrap/filters.py +++ b/oslo_rootwrap/filters.py @@ -57,9 +57,9 @@ class CommandFilter(object): return ['sudo', '-u', self.run_as, to_exec] + userargs[1:] return [to_exec] + userargs[1:] - def get_environment(self, userargs, env=None): + def get_environment(self, userargs): """Returns specific environment to set, None if none.""" - return env + return None class RegExpFilter(CommandFilter): @@ -277,10 +277,8 @@ class EnvFilter(CommandFilter): to_exec = self.get_exec(exec_dirs=exec_dirs) or self.exec_path return [to_exec] + self.exec_args(userargs)[1:] - def get_environment(self, userargs, env=None): - if env is None: - env = os.environ - env = env.copy() + def get_environment(self, userargs): + env = os.environ.copy() # ignore leading 'env' if userargs[0] == 'env': diff --git a/oslo_rootwrap/tests/test_functional.py b/oslo_rootwrap/tests/test_functional.py index aa7a5a9..407df65 100644 --- a/oslo_rootwrap/tests/test_functional.py +++ b/oslo_rootwrap/tests/test_functional.py @@ -162,13 +162,6 @@ class RootwrapDaemonTest(_FunctionalBase, testtools.TestCase): # Expect client to succesfully restart daemon and run simple request self.test_run_once() - def test_env_setting(self): - code, out, err = self.execute(['sh', '-c', 'echo $SOMEVAR'], - env={'SOMEVAR': 'teststr'}) - self.assertEqual(0, code) - self.assertEqual(b'teststr\n', out) - self.assertEqual(b'', err) - def _exec_thread(self, fifo_path): try: # Run a shell script that signals calling process through FIFO and diff --git a/oslo_rootwrap/wrapper.py b/oslo_rootwrap/wrapper.py index 6136d8f..bbf6814 100644 --- a/oslo_rootwrap/wrapper.py +++ b/oslo_rootwrap/wrapper.py @@ -190,8 +190,7 @@ def _getlogin(): os.getenv('LOGNAME')) -def start_subprocess(filter_list, userargs, exec_dirs=[], log=False, - env=None, **kwargs): +def start_subprocess(filter_list, userargs, exec_dirs=[], log=False, **kwargs): filtermatch = match_filter(filter_list, userargs, exec_dirs) command = filtermatch.get_command(userargs, exec_dirs) @@ -202,6 +201,6 @@ def start_subprocess(filter_list, userargs, exec_dirs=[], log=False, obj = subprocess.Popen(command, preexec_fn=_subprocess_setup, - env=filtermatch.get_environment(userargs, env=env), + env=filtermatch.get_environment(userargs), **kwargs) return obj diff --git a/tests/test_functional.py b/tests/test_functional.py index ed24d46..221b691 100644 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -166,13 +166,6 @@ class RootwrapDaemonTest(_FunctionalBase, testtools.TestCase): # Expect client to succesfully restart daemon and run simple request self.test_run_once() - def test_env_setting(self): - code, out, err = self.execute(['sh', '-c', 'echo $SOMEVAR'], - env={'SOMEVAR': 'teststr'}) - self.assertEqual(0, code) - self.assertEqual(b'teststr\n', out) - self.assertEqual(b'', err) - def _exec_thread(self, fifo_path): try: # Run a shell script that signals calling process through FIFO and -- cgit v1.2.1