Quick'n'dirty way to at least protect and correct rights on files

Currently the rights on some files are wrong due to wild "sudo" calls
in the deploy process.

The right way would be to get a proper privileges escalation using
oslo.privsep, but as this spec is for Stein, we still need some
corrections for the previous versions.

Please keep in mind this is a quick fix, NOT a final nor proper
solution.

Based on a previously abandonned (unmergeable) proposal.

Change-Id: I418d41c91d008283360ffaaa7b3a38354e405423
Closes-Bug: #1761595
Co-Authored-By: Emilien Macchi <emilien@redhat.com>
This commit is contained in:
Cédric Jeanneret 2018-07-05 13:55:36 +02:00
parent 00d308fd93
commit d32cfff375
4 changed files with 141 additions and 42 deletions

View File

@ -127,39 +127,40 @@ class TestDeployUndercloud(TestPluginV1):
self.assertRaises(exceptions.NotFound,
self.cmd._populate_templates_dir, '/foo')
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('getpass.getuser', return_value='stack')
@mock.patch('os.chmod')
@mock.patch('os.path.exists')
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('subprocess.check_call', autospec=True)
@mock.patch('tripleo_common.utils.passwords.generate_passwords')
@mock.patch('yaml.safe_dump')
def test_update_passwords_env_init(self, mock_dump, mock_pw,
mock_exists, mock_chmod):
def test_update_passwords_env_init(self, mock_dump, mock_pw, mock_cc,
mock_exists, mock_chmod, mock_user):
pw_dict = {"GeneratedPassword": 123}
pw_conf_path = os.path.join(self.temp_homedir,
'undercloud-passwords.conf')
t_pw_conf_path = os.path.join(
self.temp_homedir, 'tripleo-undercloud-passwords.yaml')
mock_pw.return_value = pw_dict
mock_exists.return_value = False
mock_open_context = mock.mock_open()
with mock.patch('six.moves.builtins.open', mock_open_context):
self.cmd._update_passwords_env(self.temp_homedir)
self.cmd._update_passwords_env(self.temp_homedir, 'stack')
mock_open_handle = mock_open_context()
mock_dump.assert_called_once_with({'parameter_defaults': pw_dict},
mock_open_handle,
default_flow_style=False)
chmod_calls = [mock.call(t_pw_conf_path, 0o600),
mock.call(pw_conf_path, 0o600)]
mock_chmod.assert_has_calls(chmod_calls)
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('getpass.getuser', return_value='stack')
@mock.patch('os.chmod')
@mock.patch('os.path.exists')
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('subprocess.check_call', autospec=True)
@mock.patch('tripleo_common.utils.passwords.generate_passwords')
@mock.patch('yaml.safe_dump')
def test_update_passwords_env_update(self, mock_dump, mock_pw,
mock_exists, mock_chmod):
def test_update_passwords_env_update(self, mock_dump, mock_pw, mock_cc,
mock_exists, mock_chmod, mock_user):
pw_dict = {"GeneratedPassword": 123}
pw_conf_path = os.path.join(self.temp_homedir,
'undercloud-passwords.conf')
@ -175,6 +176,7 @@ class TestDeployUndercloud(TestPluginV1):
t_pw.write('[auth]\nundercloud_db_password = abc\n')
self.cmd._update_passwords_env(self.temp_homedir,
'stack',
passwords={'ADefault': 456,
'ExistingKey':
'dontupdate'})
@ -185,9 +187,6 @@ class TestDeployUndercloud(TestPluginV1):
mock_dump.assert_called_once_with(expected_dict,
mock.ANY,
default_flow_style=False)
chmod_calls = [mock.call(t_pw_conf_path, 0o600),
mock.call(pw_conf_path, 0o600)]
mock_chmod.assert_has_calls(chmod_calls)
@mock.patch('heatclient.common.template_utils.'
'process_environment_and_files', return_value=({}, {}),
@ -604,6 +603,12 @@ class TestDeployUndercloud(TestPluginV1):
env
)
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('os.chmod')
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('subprocess.check_call', autospec=True)
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('getpass.getuser', return_value='stack')
@mock.patch('os.mkdir')
@mock.patch('six.moves.builtins.open')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
@ -641,7 +646,8 @@ class TestDeployUndercloud(TestPluginV1):
mock_wait_for_port, mock_createdirs,
mock_cleanupdirs, mock_launchansible,
mock_tarball, mock_templates_dir,
mock_open, mock_os):
mock_open, mock_os, mock_user, mock_cc,
mock_chmod):
parsed_args = self.check_parser(self.cmd,
['--local-ip', '127.0.0.1',
@ -649,6 +655,9 @@ class TestDeployUndercloud(TestPluginV1):
'--stack', 'undercloud',
'--output-dir', '/my',
'--standalone-role', 'Undercloud',
# TODO(cjeanner) drop once we have
# proper oslo.privsep
'--deployment-user', 'stack',
'-e', '/tmp/thtroot/puppet/foo.yaml',
'-e', '/tmp/thtroot//docker/bar.yaml',
'-e', '/tmp/thtroot42/notouch.yaml',

View File

@ -69,13 +69,15 @@ class TestUndercloudInstall(TestPluginV1):
mock_subprocess.assert_called_with(['instack-install-undercloud'])
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('getpass.getuser', return_value='stack')
@mock.patch('shutil.copy')
@mock.patch('os.mkdir')
@mock.patch('tripleoclient.utils.write_env_file', autospec=True)
@mock.patch('subprocess.check_call', autospec=True)
def test_undercloud_install_with_heat_customized(self, mock_subprocess,
mock_wr,
mock_os, mock_copy):
mock_wr, mock_os,
mock_copy, mock_user):
self.conf.config(output_dir='/foo')
self.conf.config(templates='/usertht')
self.conf.config(roles_file='foo/roles.yaml')
@ -115,6 +117,8 @@ class TestUndercloudInstall(TestPluginV1):
'/usertht/environments/use-dns-for-vips.yaml', '-e',
'/usertht/environments/services/undercloud-haproxy.yaml', '-e',
'/usertht/environments/services/undercloud-keepalived.yaml',
# TODO(cjeanner) drop once we have proper oslo.privsep
'--deployment-user', 'stack',
'--output-dir=/foo', '--cleanup', '-e',
'/foo/tripleo-config-generated-env-files/'
'undercloud_parameters.yaml',
@ -122,6 +126,8 @@ class TestUndercloudInstall(TestPluginV1):
'/usertht/undercloud-stack-vstate-dropin.yaml',
'--force-stack-update'])
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('getpass.getuser', return_value='stack')
@mock.patch('shutil.copy')
@mock.patch('os.mkdir')
@mock.patch('tripleoclient.utils.write_env_file', autospec=True)
@ -143,7 +149,7 @@ class TestUndercloudInstall(TestPluginV1):
mock_sroutes,
mock_masq,
mock_wr, mock_os,
mock_copy):
mock_copy, mock_user):
self.conf.config(net_config_override='/foo/net-config.json')
self.conf.config(local_interface='ethX')
self.conf.config(undercloud_public_host='4.3.2.1')
@ -277,7 +283,10 @@ class TestUndercloudInstall(TestPluginV1):
'/usr/share/openstack-tripleo-heat-templates/environments/'
'services/undercloud-haproxy.yaml', '-e',
'/usr/share/openstack-tripleo-heat-templates/environments/'
'services/undercloud-keepalived.yaml', '--output-dir=/home/stack',
'services/undercloud-keepalived.yaml',
# TODO(cjeanner) drop once we have proper oslo.privsep
'--deployment-user', 'stack',
'--output-dir=/home/stack',
'--cleanup', '-e',
'/home/stack/tripleo-config-generated-env-files/'
'undercloud_parameters.yaml',
@ -285,6 +294,8 @@ class TestUndercloudInstall(TestPluginV1):
'/usr/share/openstack-tripleo-heat-templates/'
'undercloud-stack-vstate-dropin.yaml'])
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('getpass.getuser', return_value='stack')
@mock.patch('six.moves.builtins.open')
@mock.patch('shutil.copy')
@mock.patch('os.mkdir')
@ -293,7 +304,7 @@ class TestUndercloudInstall(TestPluginV1):
def test_undercloud_install_with_heat_and_debug(self, mock_subprocess,
mock_wr,
mock_os, mock_copy,
mock_open):
mock_open, mock_user):
self.conf.config(undercloud_log_file='/foo/bar')
arglist = ['--use-heat', '--no-validations']
verifylist = []
@ -341,6 +352,8 @@ class TestUndercloudInstall(TestPluginV1):
'services/undercloud-haproxy.yaml', '-e',
'/usr/share/openstack-tripleo-heat-templates/environments/'
'services/undercloud-keepalived.yaml',
# TODO(cjeanner) drop once we have proper oslo.privsep
'--deployment-user', 'stack',
'--output-dir=/home/stack', '--cleanup',
'-e', '/home/stack/tripleo-config-generated-env-files/'
'undercloud_parameters.yaml',
@ -348,6 +361,8 @@ class TestUndercloudInstall(TestPluginV1):
'/usr/share/openstack-tripleo-heat-templates/'
'undercloud-stack-vstate-dropin.yaml'])
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('getpass.getuser', return_value='stack')
@mock.patch('six.moves.builtins.open')
@mock.patch('shutil.copy')
@mock.patch('os.mkdir')
@ -356,7 +371,7 @@ class TestUndercloudInstall(TestPluginV1):
def test_undercloud_install_with_heat_true(self, mock_subprocess,
mock_wr,
mock_os, mock_copy,
mock_open):
mock_open, mock_user):
self.conf.config(undercloud_log_file='/foo/bar')
arglist = ['--use-heat=True', '--no-validations']
verifylist = []
@ -401,19 +416,23 @@ class TestUndercloudInstall(TestPluginV1):
'services/undercloud-haproxy.yaml', '-e',
'/usr/share/openstack-tripleo-heat-templates/environments/'
'services/undercloud-keepalived.yaml',
# TODO(cjeanner) drop once we have proper oslo.privsep
'--deployment-user', 'stack',
'--output-dir=/home/stack', '--cleanup',
'-e', '/home/stack/tripleo-config-generated-env-files/'
'undercloud_parameters.yaml', '--log-file=/foo/bar', '-e',
'/usr/share/openstack-tripleo-heat-templates/'
'undercloud-stack-vstate-dropin.yaml'])
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('getpass.getuser', return_value='stack')
@mock.patch('shutil.copy')
@mock.patch('os.mkdir')
@mock.patch('tripleoclient.utils.write_env_file', autospec=True)
@mock.patch('subprocess.check_call', autospec=True)
def test_undercloud_install_with_swift_encryption(self, mock_subprocess,
mock_wr,
mock_os, mock_copy):
mock_wr, mock_os,
mock_copy, mock_user):
arglist = ['--use-heat', '--no-validations']
verifylist = []
self.conf.set_default('enable_swift_encryption', True)
@ -462,6 +481,7 @@ class TestUndercloudInstall(TestPluginV1):
'services/undercloud-haproxy.yaml', '-e',
'/usr/share/openstack-tripleo-heat-templates/environments/'
'services/undercloud-keepalived.yaml',
'--deployment-user', 'stack',
'--output-dir=/home/stack', '--cleanup',
'-e', '/home/stack/tripleo-config-generated-env-files/'
'undercloud_parameters.yaml',
@ -528,13 +548,15 @@ class TestUndercloudUpgrade(TestPluginV1):
]
)
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('getpass.getuser', return_value='stack')
@mock.patch('shutil.copy')
@mock.patch('os.mkdir')
@mock.patch('tripleoclient.utils.write_env_file', autospec=True)
@mock.patch('subprocess.check_call', autospec=True)
def test_undercloud_upgrade_with_heat_enabled(self, mock_subprocess,
mock_wr,
mock_os, mock_copy):
mock_wr, mock_os,
mock_copy, mock_user):
arglist = ['--use-heat', '--no-validations']
verifylist = []
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -581,6 +603,7 @@ class TestUndercloudUpgrade(TestPluginV1):
'services/undercloud-haproxy.yaml', '-e',
'/usr/share/openstack-tripleo-heat-templates/environments/'
'services/undercloud-keepalived.yaml',
'--deployment-user', 'stack',
'--output-dir=/home/stack', '--cleanup',
'-e', '/home/stack/tripleo-config-generated-env-files/'
'undercloud_parameters.yaml',
@ -588,13 +611,15 @@ class TestUndercloudUpgrade(TestPluginV1):
'/usr/share/openstack-tripleo-heat-templates/'
'undercloud-stack-vstate-dropin.yaml'])
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('getpass.getuser', return_value='stack')
@mock.patch('shutil.copy')
@mock.patch('os.mkdir')
@mock.patch('tripleoclient.utils.write_env_file', autospec=True)
@mock.patch('subprocess.check_call', autospec=True)
def test_undercloud_upgrade_with_heat_true(self, mock_subprocess,
mock_wr,
mock_os, mock_copy):
mock_wr, mock_os,
mock_copy, mock_user):
arglist = ['--use-heat=True', '--no-validations']
verifylist = []
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -641,6 +666,8 @@ class TestUndercloudUpgrade(TestPluginV1):
'services/undercloud-haproxy.yaml', '-e',
'/usr/share/openstack-tripleo-heat-templates/environments/'
'services/undercloud-keepalived.yaml',
# TODO(cjeanner) drop once we have proper oslo.privsep
'--deployment-user', 'stack',
'--output-dir=/home/stack', '--cleanup',
'-e', '/home/stack/tripleo-config-generated-env-files/'
'undercloud_parameters.yaml',
@ -648,13 +675,14 @@ class TestUndercloudUpgrade(TestPluginV1):
'/usr/share/openstack-tripleo-heat-templates/'
'undercloud-stack-vstate-dropin.yaml'])
@mock.patch('getpass.getuser', return_value='stack')
@mock.patch('shutil.copy')
@mock.patch('os.mkdir')
@mock.patch('tripleoclient.utils.write_env_file', autospec=True)
@mock.patch('subprocess.check_call', autospec=True)
def test_undercloud_upgrade_with_heat_and_yes(self, mock_subprocess,
mock_wr,
mock_os, mock_copy):
mock_wr, mock_os,
mock_user, mock_copy):
arglist = ['--use-heat', '--no-validations', '-y']
verifylist = []
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -701,6 +729,8 @@ class TestUndercloudUpgrade(TestPluginV1):
'services/undercloud-haproxy.yaml', '-e',
'/usr/share/openstack-tripleo-heat-templates/environments/'
'services/undercloud-keepalived.yaml',
# TODO(cjeanner) drop once we have proper oslo.privsep
'--deployment-user', 'stack',
'--output-dir=/home/stack', '--cleanup',
'-e', '/home/stack/tripleo-config-generated-env-files/'
'undercloud_parameters.yaml',
@ -708,13 +738,15 @@ class TestUndercloudUpgrade(TestPluginV1):
'/usr/share/openstack-tripleo-heat-templates/'
'undercloud-stack-vstate-dropin.yaml'])
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('getpass.getuser', return_value='stack')
@mock.patch('shutil.copy')
@mock.patch('os.mkdir')
@mock.patch('tripleoclient.utils.write_env_file', autospec=True)
@mock.patch('subprocess.check_call', autospec=True)
def test_undercloud_upgrade_with_heat_and_debug(self, mock_subprocess,
mock_wr,
mock_os, mock_copy):
mock_wr, mock_os,
mock_copy, mock_user):
arglist = ['--use-heat', '--no-validations']
verifylist = []
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -764,6 +796,7 @@ class TestUndercloudUpgrade(TestPluginV1):
'services/undercloud-haproxy.yaml', '-e',
'/usr/share/openstack-tripleo-heat-templates/environments/'
'services/undercloud-keepalived.yaml',
'--deployment-user', 'stack',
'--output-dir=/home/stack', '--cleanup',
'-e', '/home/stack/tripleo-config-generated-env-files/'
'undercloud_parameters.yaml',

View File

@ -22,6 +22,7 @@ import pwd
import re
import shutil
import six
import subprocess
import sys
import tarfile
import tempfile
@ -85,6 +86,38 @@ class Deploy(command.Command):
roles_data = None
stack_update_mark = None
stack_action = 'CREATE'
deployment_user = None
# NOTE(cjeanner) Quick'n'dirty way before we have proper
# escalation support through oslo.privsep
def _set_data_rights(self, file_name, user=None,
mode=0o600):
u = user or self.deployment_user
u_flag = None
f_flag = None
if u:
if os.path.exists(file_name):
try:
pwd.getpwnam(u)
cmd = 'sudo chown -R %s %s' % (u, file_name)
subprocess.check_call(cmd.split())
except KeyError:
u_flag = 'Unknown'
else:
f_flag = "Absent"
else:
u_flag = 'Undefined'
if u_flag:
self.log.warning(_('%(u_f)s user "%(u)s". You might need to '
'manually set ownership after the deploy')
% {'u_f': u_flag, 'u': user})
if f_flag:
self.log.warning(_('%(f)s file is %(f_f)s.')
% {'f': file_name, 'f_f': f_flag})
else:
os.chmod(file_name, mode)
def _set_roles_file(self, file_name=None, templates_dir=None):
"""Set the roles file for the deployment
@ -136,7 +169,7 @@ class Deploy(command.Command):
(self.output_dir,
datetime.utcnow().strftime('%Y%m%d%H%M%S'))
def _create_install_artifact(self):
def _create_install_artifact(self, user):
"""Create a tarball of the temporary folders used"""
self.log.debug(_("Preserving deployment artifacts"))
@ -160,6 +193,8 @@ class Deploy(command.Command):
msg = _("Unable to create artifact tarball, %s") % ex.message
self.log.error(msg)
raise exceptions.DeploymentError(msg)
# TODO(cjeanner) drop that once using oslo.privsep
self._set_data_rights(tar_filename, user=user)
return tar_filename
def _create_persistent_dirs(self):
@ -200,7 +235,7 @@ class Deploy(command.Command):
shutil.copytree(source_templates_dir, self.tht_render,
symlinks=True)
def _cleanup_working_dirs(self, cleanup=False):
def _cleanup_working_dirs(self, cleanup=False, user=None):
"""Cleanup temporary working directories
:param cleanup: Set to true if you DO want to cleanup the dirs
@ -216,8 +251,12 @@ class Deploy(command.Command):
else:
self.log.warning(_("Not cleaning working directory %s")
% self.tht_render)
# TODO(cjeanner) drop that once using oslo.privsep
self._set_data_rights(self.tht_render, user=user, mode=0o700)
self.log.warning(_("Not cleaning ansible directory %s")
% self.tmp_ansible_dir)
# TODO(cjeanner) drop that once using oslo.privsep
self._set_data_rights(self.tmp_ansible_dir, user=user, mode=0o700)
def _configure_puppet(self):
self.log.info(_('Configuring puppet modules symlinks ...'))
@ -225,7 +264,7 @@ class Deploy(command.Command):
constants.PUPPET_MODULES,
constants.PUPPET_BASE)
def _update_passwords_env(self, output_dir, passwords=None):
def _update_passwords_env(self, output_dir, user, passwords=None):
pw_file = os.path.join(output_dir, 'tripleo-undercloud-passwords.yaml')
undercloud_pw_file = os.path.join(output_dir,
'undercloud-passwords.conf')
@ -288,9 +327,9 @@ class Deploy(command.Command):
# This contains sensitive data so ensure it's not world-readable
with open(pw_file, 'w') as pf:
yaml.safe_dump(stack_env, pf, default_flow_style=False)
# Using chmod here instead of permissions on the open above so we don't
# have to fight with umask.
os.chmod(pw_file, 0o600)
# TODO(cjeanner) drop that once using oslo.privsep
# Do not forget to re-add os.chmod 0o600 on that one!
self._set_data_rights(pw_file, user=user)
# Write out an instack undercloud compatible version.
# This contains sensitive data so ensure it's not world-readable
with open(undercloud_pw_file, 'w') as pf:
@ -303,7 +342,10 @@ class Deploy(command.Command):
pw_key = re.sub('([a-z0-9])([A-Z])',
r'\1_\2', s1).lower()
pf.write('undercloud_%s: %s\n' % (pw_key, v))
os.chmod(undercloud_pw_file, 0o600)
# TODO(cjeanner) drop that once using oslo.privsep
# Do not forget to re-add os.chmod 0o600 on that one!
self._set_data_rights(undercloud_pw_file, user=user)
return pw_file
@ -519,7 +561,8 @@ class Deploy(command.Command):
for e in plan_env_data.get('environments', {})]
# this will allow the user to overwrite passwords with custom envs
pw_file = self._update_passwords_env(self.output_dir)
pw_file = self._update_passwords_env(self.output_dir,
parsed_args.deployment_user)
environments.append(pw_file)
# use deployed-server because we run os-collect-config locally
@ -769,6 +812,14 @@ class Deploy(command.Command):
help=_('User to execute the non-priveleged heat-all process. '
'Defaults to heat.')
)
# TODO(cjeanner) drop that once using oslo.privsep
parser.add_argument(
'--deployment-user',
dest='deployment_user',
default='stack',
help=_('User who executes the tripleo deploy command. '
'Defaults to stack.')
)
parser.add_argument(
'--heat-container-image', metavar='<HEAT_CONTAINER_IMAGE>',
dest='heat_container_image',
@ -977,8 +1028,12 @@ class Deploy(command.Command):
finally:
if not parsed_args.keep_running:
self._kill_heat(parsed_args)
tar_filename = self._create_install_artifact()
self._cleanup_working_dirs(cleanup=parsed_args.cleanup)
tar_filename = \
self._create_install_artifact(parsed_args.deployment_user)
self._cleanup_working_dirs(
cleanup=parsed_args.cleanup,
user=parsed_args.deployment_user
)
if tar_filename:
self.log.warning('Install artifact is located at %s' %
tar_filename)

View File

@ -467,6 +467,8 @@ def prepare_undercloud_deploy(upgrade=False, no_validations=False,
u = CONF.get('deployment_user') or utils.get_deployment_user()
env_data['DeploymentUser'] = u
# TODO(cjeanner) drop that once using oslo.privsep
deploy_args += ['--deployment-user', u]
deploy_args += ['--output-dir=%s' % CONF['output_dir']]
if not os.path.isdir(CONF['output_dir']):