From 75978d7869b7081667722bb15567ed5ff787dc00 Mon Sep 17 00:00:00 2001 From: Steven Hardy Date: Fri, 14 Jul 2017 14:56:12 +0100 Subject: [PATCH] Restrict file permissions of config download data This could contain sensitive configuration data so ensure we set the file modes so they are only accessible to the user Change-Id: I6a83efe61482f0c24f9a7d8e1035aeb25ce7f5cc --- .../overcloud_config/test_overcloud_config.py | 64 ++++++++++--------- tripleoclient/v1/overcloud_config.py | 10 ++- 2 files changed, 40 insertions(+), 34 deletions(-) diff --git a/tripleoclient/tests/v1/overcloud_config/test_overcloud_config.py b/tripleoclient/tests/v1/overcloud_config/test_overcloud_config.py index 5fea93dbe..e56118447 100644 --- a/tripleoclient/tests/v1/overcloud_config/test_overcloud_config.py +++ b/tripleoclient/tests/v1/overcloud_config/test_overcloud_config.py @@ -11,6 +11,7 @@ # under the License. import mock +import stat from mock import call from osc_lib.tests import utils @@ -29,12 +30,8 @@ class TestOvercloudConfig(utils.TestCommand): self.app.client_manager.orchestration = mock.Mock() self.workflow = self.app.client_manager.workflow_engine - @mock.patch('os.mkdir') - @mock.patch('six.moves.builtins.open') @mock.patch('tempfile.mkdtemp', autospec=True) - def test_overcloud_config_generate_config(self, mock_tmpdir, - mock_open, mock_mkdir): - + def test_overcloud_config_generate_config(self, mock_tmpdir): arglist = ['--name', 'overcloud', '--config-dir', '/tmp'] verifylist = [ ('name', 'overcloud'), @@ -44,36 +41,37 @@ class TestOvercloudConfig(utils.TestCommand): 'logging_sources', 'monitoring_subscriptions', 'service_config_settings', 'service_metadata_settings', - 'service_names', 'step_config', + 'service_names', 'upgrade_batch_tasks', 'upgrade_tasks'] fake_role = [role for role in fakes.FAKE_STACK['outputs'][0]['output_value']] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - clients = self.app.client_manager orchestration_client = clients.orchestration orchestration_client.stacks.get.return_value = fakes.create_tht_stack() mock_tmpdir.return_value = "/tmp/tht" - self.cmd.take_action(parsed_args) - expected_mkdir_calls = [call('/tmp/tht/%s' % r) for r in fake_role] - mock_mkdir.assert_has_calls(expected_mkdir_calls, any_order=True) - expected_calls = [] - for config in config_type_list: - for role in fake_role: - if config == 'step_config': - expected_calls += [call('/tmp/tht/%s/%s.pp' % - (role, config), 'w')] - else: - expected_calls += [call('/tmp/tht/%s/%s.yaml' % - (role, config), 'w')] - mock_open.assert_has_calls(expected_calls, any_order=True) + with mock.patch('os.open') as open, \ + mock.patch('os.fdopen'), mock.patch('os.mkdir') as mkdir: + self.cmd.take_action(parsed_args) + expected_mkdir_calls = [call('/tmp/tht/%s' % r, + stat.S_IRWXU) for r in fake_role] + mkdir.assert_has_calls(expected_mkdir_calls, any_order=True) + expected_calls = [] + for config in config_type_list: + for role in fake_role: + if config == 'step_config': + expected_calls += [call('/tmp/tht/%s/%s.pp' % + (role, config), 65, + stat.S_IRUSR | stat.S_IWUSR)] + else: + expected_calls += [call('/tmp/tht/%s/%s.yaml' % + (role, config), 65, + stat.S_IRUSR | stat.S_IWUSR)] + open.assert_has_calls(expected_calls, any_order=True) - @mock.patch('os.mkdir') - @mock.patch('six.moves.builtins.open') @mock.patch('tempfile.mkdtemp', autospec=True) - def test_overcloud_config_one_config_type(self, mock_tmpdir, mock_open, - mock_mkdir): + def test_overcloud_config_one_config_type(self, mock_tmpdir): arglist = ['--name', 'overcloud', '--config-dir', '/tmp', '--config-type', ['config_settings']] @@ -92,13 +90,17 @@ class TestOvercloudConfig(utils.TestCommand): orchestration_client = clients.orchestration orchestration_client.stacks.get.return_value = fakes.create_tht_stack() mock_tmpdir.return_value = "/tmp/tht" - self.cmd.take_action(parsed_args) - expected_mkdir_calls = [call('/tmp/tht/%s' % r) for r in fake_role] - mock_mkdir.assert_has_calls(expected_mkdir_calls, any_order=True) - expected_calls = [call('/tmp/tht/%s/%s.yaml' - % (r, expected_config_type), 'w') - for r in fake_role] - mock_open.assert_has_calls(expected_calls, any_order=True) + with mock.patch('os.open') as open, \ + mock.patch('os.fdopen'), mock.patch('os.mkdir') as mkdir: + self.cmd.take_action(parsed_args) + expected_mkdir_calls = [call('/tmp/tht/%s' % r, + stat.S_IRWXU) for r in fake_role] + mkdir.assert_has_calls(expected_mkdir_calls, any_order=True) + expected_calls = [call('/tmp/tht/%s/%s.yaml' % + (r, expected_config_type), 65, + stat.S_IRUSR | stat.S_IWUSR) + for r in fake_role] + open.assert_has_calls(expected_calls, any_order=True) @mock.patch('os.mkdir') @mock.patch('six.moves.builtins.open') diff --git a/tripleoclient/v1/overcloud_config.py b/tripleoclient/v1/overcloud_config.py index 6a9bc7f58..dd36ec365 100644 --- a/tripleoclient/v1/overcloud_config.py +++ b/tripleoclient/v1/overcloud_config.py @@ -64,7 +64,7 @@ class DownloadConfig(command.Command): def _mkdir(self, dirname): if not os.path.exists(dirname): try: - os.mkdir(dirname) + os.mkdir(dirname, 0o700) except OSError as e: message = 'Failed to create: %s, error: %s' % (dirname, str(e)) @@ -90,7 +90,9 @@ class DownloadConfig(command.Command): for config in parsed_args.config_type or role.keys(): if config == 'step_config': filepath = os.path.join(role_path, 'step_config.pp') - with open(filepath, 'w') as step_config: + with os.fdopen(os.open(filepath, + os.O_WRONLY | os.O_CREAT, 0o600), + 'w') as step_config: step_config.write('\n'.join(step for step in role[config] if step is not None)) @@ -106,7 +108,9 @@ class DownloadConfig(command.Command): str(e)) raise KeyError(message) filepath = os.path.join(role_path, '%s.yaml' % config) - with open(filepath, 'w') as conf_file: + with os.fdopen(os.open(filepath, + os.O_WRONLY | os.O_CREAT, 0o600), + 'w') as conf_file: yaml.safe_dump(data, conf_file, default_flow_style=False)