Train-only: Reduce permission of config-download files

In stable/train, the config-download process is executed by mistral and
some files are stored under /var/lib/mistral.
If a file in the directory has o+r permission, then other users can
read the file content. The generated files such as group var file
includes some sensitive items like passwords and should not be visible
to anonymous users.

This ensures we remove permission for other users so that we prevent
other users from sneaking files.

Closes-Bug: #1990226
Resolves: rhbz#2120660
Change-Id: Idc78964f560fc7a5766cf164c65d48adcbed4532

tripleo_common/actions/config.py

@@ -53,6 +53,9 @@ class GetOvercloudConfig(base.TripleOAction):
         self.container_config = container_config
 
     def run(self, context):
+        # Set umask to avoid a=rwx
+        os.umask(0o007)
+
         heat = self.get_orchestration_client(context)
         swift = self.get_object_client(context)
 
@@ -97,9 +100,13 @@ class GetOvercloudConfig(base.TripleOAction):
             with open(tmp_tarball.name, 'rb') as t:
                 swift.put_object(self.container_config,
                                  '%s.tar.gz' % self.container_config, t)
+
         if os.path.exists(config_path):
             shutil.rmtree(config_path)
 
+        # NOTE(tkajinam): Reset umask
+        os.umask(0o000)
+
 
 class DownloadConfigAction(base.TripleOAction):
     """Download the container config from swift
@@ -119,6 +126,13 @@ class DownloadConfigAction(base.TripleOAction):
                 prefix='tripleo-', suffix='-config')
 
     def run(self, context):
+        # Set umask to avoid a=rwx
+        os.umask(0o007)
+
+        # remove existing files so that we recreate all files with proper umask
+        if os.path.exists(self.work_dir):
+            shutil.rmtree(self.work_dir)
+
         swift = self.get_object_client(context)
         swiftutils.download_container(swift, self.container_config,
                                       self.work_dir)
@@ -127,4 +141,5 @@ class DownloadConfigAction(base.TripleOAction):
         if os.path.exists(symlink_path):
             os.unlink(symlink_path)
         os.symlink(self.work_dir, symlink_path)
+        os.umask(0o000)
         return self.work_dir

tripleo_common/tests/actions/test_config.py

@@ -62,6 +62,7 @@ class GetOvercloudConfigActionTest(base.TestCase):
 
         self.ctx = mock.MagicMock()
 
+    @mock.patch('tripleo_common.actions.config.os.umask')
     @mock.patch('tripleo_common.utils.swift.delete_container')
     @mock.patch('tripleo_common.utils.swift.download_container')
     @mock.patch('tripleo_common.actions.base.TripleOAction.'
@@ -72,7 +73,8 @@ class GetOvercloudConfigActionTest(base.TestCase):
                  mock_config,
                  mock_orchestration_client,
                  mock_swift_download,
-                 mock_swift_delete):
+                 mock_swift_delete,
+                 mock_os_umask):
         heat = mock.MagicMock()
         heat.stacks.get.return_value = mock.MagicMock(
             stack_name='stack', id='stack_id')
@@ -130,6 +132,7 @@ class DownloadConfigActionTest(base.TestCase):
 
         self.ctx = mock.MagicMock()
 
+    @mock.patch('tripleo_common.actions.config.os.umask')
     @mock.patch('tripleo_common.actions.config.os.unlink')
     @mock.patch('tripleo_common.actions.config.os.path.exists')
     @mock.patch('tripleo_common.actions.config.os.symlink')
@@ -139,14 +142,17 @@ class DownloadConfigActionTest(base.TestCase):
                  mock_swiftutils,
                  mock_os_symlink,
                  mock_os_path_exists,
-                 mock_os_unlink):
+                 mock_os_unlink,
+                 mock_os_umask):
         mock_mkdtemp.return_value = '/tmp/tripleo-foo-config'
+        mock_os_path_exists.return_value = False
         action = config.DownloadConfigAction(self.config_container)
         action.run(self.ctx)
         mock_swiftutils.assert_called_once_with(self.swift,
                                                 self.config_container,
                                                 '/tmp/tripleo-foo-config')
 
+    @mock.patch('tripleo_common.actions.config.os.umask')
     @mock.patch('tripleo_common.actions.config.os.path.exists')
     @mock.patch('tripleo_common.actions.config.os.symlink')
     @mock.patch('tripleo_common.utils.swift.download_container')
@@ -155,7 +161,8 @@ class DownloadConfigActionTest(base.TestCase):
             self, mock_mkdtemp,
             mock_swiftutils,
             mock_os_symlink,
-            mock_os_path_exists):
+            mock_os_path_exists,
+            mock_os_umask):
         mock_mkdtemp.return_value = '/var/lib/mistral/uuid'
         mock_os_path_exists.return_value = False
         action = config.DownloadConfigAction(self.config_container)
@@ -180,7 +187,7 @@ class DownloadConfigActionTest(base.TestCase):
             mock_os_path_exists,
             mock_os_unlink):
         mock_mkdtemp.return_value = '/var/lib/mistral/uuid'
-        mock_os_path_exists.return_value = True
+        mock_os_path_exists.side_effect = [False, True]
         action = config.DownloadConfigAction(self.config_container)
         action.run(self.ctx)
         mock_swiftutils.assert_called_once_with(self.swift,