Use seteuid instead of su to control privileges

Control the privileges by setting the effective UID before running the
command. Earlier we used to run command using su -c "USER".

Original EUID is restored after running the command. This is required to
run multiple commands in succession with different run-as users.

Change-Id: I414fc6a802f11deb320b43c6d011f802a42c40c9
Partial-Bug: #1312246

2 files changed, 148 insertions, 40 deletions

diff --git a/heat_cfntools/cfntools/cfn_helper.py b/heat_cfntools/cfntools/cfn_helper.py
index 7e75cf9..b285d24 100644
--- a/heat_cfntools/cfntools/cfn_helper.py
+++ b/heat_cfntools/cfntools/cfn_helper.py
@@ -1,4 +1,4 @@
-#
+
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
 #    not use this file except in compliance with the License. You may obtain
 #    a copy of the License at
@@ -19,6 +19,7 @@ Not implemented yet:
       - placeholders are ignored
 """
 import atexit
+import contextlib
 import errno
 import functools
 import grp
@@ -40,6 +41,7 @@ import six.moves.configparser as ConfigParser
 import subprocess
 import tempfile
 
+
 # Override BOTO_CONFIG, which makes boto look only at the specified
 # config file, instead of the default locations
 os.environ['BOTO_CONFIG'] = '/var/lib/heat-cfntools/cfn-boto-cfg'
@@ -152,6 +154,33 @@ class Hook(object):
              self.action)
 
 
+class ControlledPrivilegesFailureException(Exception):
+    pass
+
+
+@contextlib.contextmanager
+def controlled_privileges(user):
+    orig_euid = None
+    try:
+        real = pwd.getpwnam(user)
+        if os.geteuid() != real.pw_uid:
+            orig_euid = os.geteuid()
+            os.seteuid(real.pw_uid)
+            LOG.debug("Privileges set for user %s" % user)
+    except Exception as e:
+        raise ControlledPrivilegesFailureException(e)
+
+    try:
+        yield
+    finally:
+        if orig_euid is not None:
+            try:
+                os.seteuid(orig_euid)
+                LOG.debug("Original privileges restored.")
+            except Exception as e:
+                LOG.error("Error restoring privileges %s" % e)
+
+
 class CommandRunner(object):
     """Helper class to run a command and store the output."""
 
@@ -180,19 +209,34 @@ class CommandRunner(object):
             self
         """
         LOG.debug("Running command: %s" % self._command)
-        cmd = ['su', user, '-c', self._command]
-        subproc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
-                                   stderr=subprocess.PIPE, cwd=cwd, env=env)
-        output = subproc.communicate()
-
-        self._status = subproc.returncode
-        self._stdout = output[0]
-        self._stderr = output[1]
+
+        cmd = self._command
+
+        # Ensure commands are passed as strings only as we run them
+        # using shell
+        assert isinstance(cmd, six.string_types)
+
+        try:
+            with controlled_privileges(user):
+                subproc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
+                                           stderr=subprocess.PIPE, cwd=cwd,
+                                           env=env, shell=True)
+                output = subproc.communicate()
+                self._status = subproc.returncode
+                self._stdout = output[0]
+                self._stderr = output[1]
+        except ControlledPrivilegesFailureException as e:
+            LOG.error("Error setting privileges for user '%s': %s"
+                      % (user, e))
+            self._status = 126
+            self._stderr = six.text_type(e)
+
         if self._status:
             LOG.debug("Return code of %d after executing: '%s'\n"
                       "stdout: '%s'\n"
                       "stderr: '%s'" % (self._status, cmd, self._stdout,
                                         self._stderr))
+
         if self._next:
             self._next.run()
         return self
diff --git a/heat_cfntools/tests/test_cfn_helper.py b/heat_cfntools/tests/test_cfn_helper.py
index 59020e2..a862fd9 100644
--- a/heat_cfntools/tests/test_cfn_helper.py
+++ b/heat_cfntools/tests/test_cfn_helper.py
@@ -27,9 +27,10 @@ from heat_cfntools.cfntools import cfn_helper
 
 
 def popen_root_calls(calls):
-    kwargs = {'env': None, 'cwd': None, 'stderr': -1, 'stdout': -1}
+    kwargs = {'env': None, 'cwd': None, 'stderr': -1, 'stdout': -1,
+              'shell': True}
     return [
-        mock.call(['su', 'root', '-c', call], **kwargs)
+        mock.call(call, **kwargs)
         for call in calls
     ]
 
@@ -47,13 +48,16 @@ class FakePOpen():
         pass
 
 
+@mock.patch.object(cfn_helper.pwd, 'getpwnam')
+@mock.patch.object(cfn_helper.os, 'seteuid')
+@mock.patch.object(cfn_helper.os, 'geteuid')
 class TestCommandRunner(testtools.TestCase):
 
-    def test_command_runner(self):
+    def test_command_runner(self, mock_geteuid, mock_seteuid, mock_getpwnam):
         def returns(*args, **kwargs):
-            if args[0][3] == '/bin/command1':
+            if args[0] == '/bin/command1':
                 return FakePOpen('All good')
-            elif args[0][3] == '/bin/command2':
+            elif args[0] == '/bin/command2':
                 return FakePOpen('Doing something', 'error', -1)
             else:
                 raise Exception('This should never happen')
@@ -73,13 +77,64 @@ class TestCommandRunner(testtools.TestCase):
             calls = popen_root_calls(['/bin/command1', '/bin/command2'])
             mock_popen.assert_has_calls(calls)
 
+    def test_privileges_are_lowered_for_non_root_user(self, mock_geteuid,
+                                                      mock_seteuid,
+                                                      mock_getpwnam):
+        pw_entry = mock.MagicMock()
+        pw_entry.pw_uid = 1001
+        mock_getpwnam.return_value = pw_entry
+        mock_geteuid.return_value = 0
+        calls = [mock.call(1001), mock.call(0)]
+        with mock.patch('subprocess.Popen') as mock_popen:
+            command = '/bin/command --option=value arg1 arg2'
+            cmd = cfn_helper.CommandRunner(command)
+            cmd.run(user='nonroot')
+            self.assertTrue(mock_geteuid.called)
+            mock_getpwnam.assert_called_once_with('nonroot')
+            mock_seteuid.assert_has_calls(calls)
+            self.assertTrue(mock_popen.called)
+
+    def test_run_returns_when_cannot_set_privileges(self, mock_geteuid,
+                                                    mock_seteuid,
+                                                    mock_getpwnam):
+        msg = '[Error 1] Permission Denied'
+        mock_seteuid.side_effect = Exception(msg)
+        with mock.patch('subprocess.Popen') as mock_popen:
+            command = '/bin/command2'
+            cmd = cfn_helper.CommandRunner(command)
+            cmd.run(user='nonroot')
+            self.assertTrue(mock_getpwnam.called)
+            self.assertTrue(mock_seteuid.called)
+            self.assertFalse(mock_popen.called)
+            self.assertEqual(126, cmd.status)
+            self.assertEqual(msg, cmd.stderr)
+
+    def test_privileges_are_restored_for_command_failure(self, mock_geteuid,
+                                                         mock_seteuid,
+                                                         mock_getpwnam):
+        pw_entry = mock.MagicMock()
+        pw_entry.pw_uid = 1001
+        mock_getpwnam.return_value = pw_entry
+        mock_geteuid.return_value = 0
+        calls = [mock.call(1001), mock.call(0)]
+        with mock.patch('subprocess.Popen') as mock_popen:
+            mock_popen.side_effect = ValueError('Something wrong')
+            command = '/bin/command --option=value arg1 arg2'
+            cmd = cfn_helper.CommandRunner(command)
+            self.assertRaises(ValueError, cmd.run, user='nonroot')
+            self.assertTrue(mock_geteuid.called)
+            mock_getpwnam.assert_called_once_with('nonroot')
+            mock_seteuid.assert_has_calls(calls)
+            self.assertTrue(mock_popen.called)
+
 
+@mock.patch.object(cfn_helper, 'controlled_privileges')
 class TestPackages(testtools.TestCase):
 
-    def test_yum_install(self):
+    def test_yum_install(self, mock_cp):
 
         def returns(*args, **kwargs):
-            if args[0][3].startswith('rpm -q '):
+            if args[0].startswith('rpm -q '):
                 return FakePOpen(returncode=1)
             else:
                 return FakePOpen(returncode=0)
@@ -103,11 +158,11 @@ class TestPackages(testtools.TestCase):
             cfn_helper.PackagesHandler(packages).apply_packages()
             mock_popen.assert_has_calls(calls, any_order=True)
 
-    def test_dnf_install_yum_unavailable(self):
+    def test_dnf_install_yum_unavailable(self, mock_cp):
 
         def returns(*args, **kwargs):
-            if args[0][3].startswith('rpm -q ') \
-               or args[0][3] == 'which yum':
+            if args[0].startswith('rpm -q ') \
+               or args[0] == 'which yum':
                 return FakePOpen(returncode=1)
             else:
                 return FakePOpen(returncode=0)
@@ -131,10 +186,10 @@ class TestPackages(testtools.TestCase):
             cfn_helper.PackagesHandler(packages).apply_packages()
             mock_popen.assert_has_calls(calls, any_order=True)
 
-    def test_dnf_install(self):
+    def test_dnf_install(self, mock_cp):
 
         def returns(*args, **kwargs):
-            if args[0][3].startswith('rpm -q '):
+            if args[0].startswith('rpm -q '):
                 return FakePOpen(returncode=1)
             else:
                 return FakePOpen(returncode=0)
@@ -158,10 +213,10 @@ class TestPackages(testtools.TestCase):
             cfn_helper.PackagesHandler(packages).apply_packages()
             mock_popen.assert_has_calls(calls, any_order=True)
 
-    def test_zypper_install(self):
+    def test_zypper_install(self, mock_cp):
 
         def returns(*args, **kwargs):
-            if args[0][3].startswith('rpm -q '):
+            if args[0].startswith('rpm -q '):
                 return FakePOpen(returncode=1)
             else:
                 return FakePOpen(returncode=0)
@@ -185,7 +240,7 @@ class TestPackages(testtools.TestCase):
             cfn_helper.PackagesHandler(packages).apply_packages()
             mock_popen.assert_has_calls(calls, any_order=True)
 
-    def test_apt_install(self):
+    def test_apt_install(self, mock_cp):
         packages = {
             "apt": {
                 "mysql-server": [],
@@ -200,9 +255,10 @@ class TestPackages(testtools.TestCase):
             self.assertTrue(mock_popen.called)
 
 
+@mock.patch.object(cfn_helper, 'controlled_privileges')
 class TestServicesHandler(testtools.TestCase):
 
-    def test_services_handler_systemd(self):
+    def test_services_handler_systemd(self, mock_cp):
         calls = []
         returns = []
 
@@ -272,7 +328,7 @@ class TestServicesHandler(testtools.TestCase):
                 mock_popen.assert_has_calls(calls, any_order=True)
             mock_exists.assert_called_with('/bin/systemctl')
 
-    def test_services_handler_systemd_disabled(self):
+    def test_services_handler_systemd_disabled(self, mock_cp):
         calls = []
 
         # apply_services
@@ -307,7 +363,7 @@ class TestServicesHandler(testtools.TestCase):
                 mock_popen.assert_has_calls(calls, any_order=True)
             mock_exists.assert_called_with('/bin/systemctl')
 
-    def test_services_handler_sysv_service_chkconfig(self):
+    def test_services_handler_sysv_service_chkconfig(self, mock_cp):
 
         def exists(*args, **kwargs):
             return args[0] != '/bin/systemctl'
@@ -367,7 +423,7 @@ class TestServicesHandler(testtools.TestCase):
             mock_exists.assert_any_call('/sbin/service')
             mock_exists.assert_any_call('/sbin/chkconfig')
 
-    def test_services_handler_sysv_disabled_service_chkconfig(self):
+    def test_services_handler_sysv_disabled_service_chkconfig(self, mock_cp):
         def exists(*args, **kwargs):
             return args[0] != '/bin/systemctl'
 
@@ -405,7 +461,7 @@ class TestServicesHandler(testtools.TestCase):
             mock_exists.assert_any_call('/sbin/service')
             mock_exists.assert_any_call('/sbin/chkconfig')
 
-    def test_services_handler_sysv_systemctl(self):
+    def test_services_handler_sysv_systemctl(self, mock_cp):
         calls = []
         returns = []
 
@@ -459,7 +515,7 @@ class TestServicesHandler(testtools.TestCase):
                 mock_popen.assert_has_calls(calls)
             mock_exists.assert_called_with('/bin/systemctl')
 
-    def test_services_handler_sysv_disabled_systemctl(self):
+    def test_services_handler_sysv_disabled_systemctl(self, mock_cp):
         calls = []
 
         # apply_services
@@ -492,7 +548,7 @@ class TestServicesHandler(testtools.TestCase):
                 mock_popen.assert_has_calls(calls)
             mock_exists.assert_called_with('/bin/systemctl')
 
-    def test_services_handler_sysv_service_updaterc(self):
+    def test_services_handler_sysv_service_updaterc(self, mock_cp):
         calls = []
         returns = []
 
@@ -548,7 +604,7 @@ class TestServicesHandler(testtools.TestCase):
             mock_exists.assert_any_call('/sbin/service')
             mock_exists.assert_any_call('/sbin/chkconfig')
 
-    def test_services_handler_sysv_disabled_service_updaterc(self):
+    def test_services_handler_sysv_disabled_service_updaterc(self, mock_cp):
         calls = []
         returns = []
 
@@ -619,7 +675,8 @@ interval=120''' % fcreds.name).encode('UTF-8'))
         self.assertIn('invalid credentials file', str(e))
         fcreds.close()
 
-    def test_hup_config(self):
+    @mock.patch.object(cfn_helper, 'controlled_privileges')
+    def test_hup_config(self, mock_cp):
         hooks_conf = tempfile.NamedTemporaryFile()
 
         def write_hook_conf(f, name, triggers, path, action):
@@ -1030,7 +1087,8 @@ class TestMetadataRetrieve(testtools.TestCase):
 
             self.assertEqual(meta_in, meta_out)
 
-    def test_nova_meta_curl(self):
+    @mock.patch.object(cfn_helper, 'controlled_privileges')
+    def test_nova_meta_curl(self, mock_cp):
         url = 'http://169.254.169.254/openstack/2012-08-10/meta_data.json'
         temp_home = tempfile.mkdtemp()
         cache_path = os.path.join(temp_home, 'meta_data.json')
@@ -1065,7 +1123,8 @@ class TestMetadataRetrieve(testtools.TestCase):
             mock_popen.assert_has_calls(
                 popen_root_calls(['curl -o %s %s' % (cache_path, url)]))
 
-    def test_nova_meta_curl_corrupt(self):
+    @mock.patch.object(cfn_helper, 'controlled_privileges')
+    def test_nova_meta_curl_corrupt(self, mock_cp):
         url = 'http://169.254.169.254/openstack/2012-08-10/meta_data.json'
         temp_home = tempfile.mkdtemp()
         cache_path = os.path.join(temp_home, 'meta_data.json')
@@ -1093,7 +1152,8 @@ class TestMetadataRetrieve(testtools.TestCase):
             mock_popen.assert_has_calls(
                 popen_root_calls(['curl -o %s %s' % (cache_path, url)]))
 
-    def test_nova_meta_curl_failed(self):
+    @mock.patch.object(cfn_helper, 'controlled_privileges')
+    def test_nova_meta_curl_failed(self, mock_cp):
         url = 'http://169.254.169.254/openstack/2012-08-10/meta_data.json'
         temp_home = tempfile.mkdtemp()
         cache_path = os.path.join(temp_home, 'meta_data.json')
@@ -1169,7 +1229,8 @@ class TestCfnInit(testtools.TestCase):
             md.cfn_init()
             self.assertThat(foo_file.name, ttm.FileContains('bar'))
 
-    def test_cfn_init_with_ignore_errors_false(self):
+    @mock.patch.object(cfn_helper, 'controlled_privileges')
+    def test_cfn_init_with_ignore_errors_false(self, mock_cp):
         md_data = {"AWS::CloudFormation::Init": {"config": {"commands": {
             "00_foo": {"command": "/bin/command1",
                        "ignoreErrors": "false"}}}}}
@@ -1181,7 +1242,8 @@ class TestCfnInit(testtools.TestCase):
             self.assertRaises(cfn_helper.CommandsHandlerRunError, md.cfn_init)
             mock_popen.assert_has_calls(popen_root_calls(['/bin/command1']))
 
-    def test_cfn_init_with_ignore_errors_true(self):
+    @mock.patch.object(cfn_helper, 'controlled_privileges')
+    def test_cfn_init_with_ignore_errors_true(self, mock_cp):
         calls = []
         returns = []
         calls.append('/bin/command1')
@@ -1228,7 +1290,8 @@ class TestSourcesHandler(testtools.TestCase):
                 mock_popen.assert_has_calls(calls)
             mock_mkdtemp.assert_called_with()
 
-    def test_apply_sources_github(self):
+    @mock.patch.object(cfn_helper, 'controlled_privileges')
+    def test_apply_sources_github(self, mock_cp):
         url = "https://github.com/NoSuchProject/tarball/NoSuchTarball"
         dest = tempfile.mkdtemp()
         self.addCleanup(os.rmdir, dest)
@@ -1241,7 +1304,8 @@ class TestSourcesHandler(testtools.TestCase):
             sh.apply_sources()
             mock_popen.assert_has_calls(calls)
 
-    def test_apply_sources_general(self):
+    @mock.patch.object(cfn_helper, 'controlled_privileges')
+    def test_apply_sources_general(self, mock_cp):
         url = "https://website.no.existe/a/b/c/file.tar.gz"
         dest = tempfile.mkdtemp()
         self.addCleanup(os.rmdir, dest)