[buildstream] 10/19: Make temporary staging directories group-accessible

[ro...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

root pushed a commit to branch tlater/casd-socket-permissions
in repository https://gitbox.apache.org/repos/asf/buildstream.git

commit 5fc38355ae19aef73ffe024171ab7028a4dfefd2
Author: Tristan Maat <tr...@codethink.co.uk>
AuthorDate: Tue Nov 5 11:32:38 2019 +0000

    Make temporary staging directories group-accessible
    
    This is again required because of the userchroot workflow requiring
    two UIDs. By default, python will create temporary directories with
    u+rwx,og-rwx, so that we don't leak information to the public /tmp.
    
    This is irrelevant here since we are in directories that shouldn't be
    as easily accessible anyway (since they are usually far inside
    "$HOME", or at least configured by the sysadmin to be reasonably
    safe), and a hindrance to adopt userchroot as a first-class sandbox.
---
 src/buildstream/_artifact.py    |  4 ++++
 src/buildstream/_sourcecache.py |  2 +-
 src/buildstream/element.py      |  2 +-
 src/buildstream/utils.py        | 43 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/src/buildstream/_artifact.py b/src/buildstream/_artifact.py
index e5174ea..711d402 100644
--- a/src/buildstream/_artifact.py
+++ b/src/buildstream/_artifact.py
@@ -163,7 +163,11 @@ class Artifact():
 
         # Store public data
         with utils._tempnamedfile_name(dir=self._tmpdir) as tmpname:
+            # FIXME: This overrides the original file. Should check if
+            # that has side-effects besides re-setting the permissions
+            # (hence _group_tempnamedfile_name is useless here).
             _yaml.roundtrip_dump(publicdata, tmpname)
+            os.chmod(tmpname, utils.URWX_GRWX)
             public_data_digest = self._cas.add_object(path=tmpname, link_directly=True)
             artifact.public_data.CopyFrom(public_data_digest)
             size += public_data_digest.size_bytes
diff --git a/src/buildstream/_sourcecache.py b/src/buildstream/_sourcecache.py
index cdbe5b9..7708f3d 100644
--- a/src/buildstream/_sourcecache.py
+++ b/src/buildstream/_sourcecache.py
@@ -185,7 +185,7 @@ class SourceCache(BaseCache):
             vdir.import_files(self.export(previous_source))
 
         if not source.BST_STAGE_VIRTUAL_DIRECTORY:
-            with utils._tempdir(dir=self.context.tmpdir, prefix='staging-temp') as tmpdir:
+            with utils._group_tempdir(dir=self.context.tmpdir, prefix='staging-temp') as tmpdir:
                 if not vdir.is_empty():
                     vdir.export_files(tmpdir)
                 source._stage(tmpdir)
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 5fa8f14..7bb8bc0 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -1453,7 +1453,7 @@ class Element(Plugin):
         # It's advantageous to have this temporary directory on
         # the same file system as the rest of our cache.
         with self.timed_activity("Staging sources", silent_nested=True), \
-            utils._tempdir(dir=context.tmpdir, prefix='staging-temp') as temp_staging_directory:
+            utils._group_tempdir(dir=context.tmpdir, prefix='staging-temp') as temp_staging_directory:
 
             import_dir = temp_staging_directory
 
diff --git a/src/buildstream/utils.py b/src/buildstream/utils.py
index de7c14b..e9f0fb7 100644
--- a/src/buildstream/utils.py
+++ b/src/buildstream/utils.py
@@ -65,6 +65,16 @@ _INITIAL_NUM_THREADS_IN_MAIN_PROCESS = 1
 # Number of seconds to wait for background threads to exit.
 _AWAIT_THREADS_TIMEOUT_SECONDS = 5
 
+# Bit flags for ug+rwx,o-rwx permissions
+URWX_GRWX = (
+    stat.S_IRUSR |
+    stat.S_IWUSR |
+    stat.S_IXUSR |
+    stat.S_IRGRP |
+    stat.S_IWGRP |
+    stat.S_IXGRP
+)
+
 
 class UtilError(BstError):
     """Raised by utility functions when system calls fail.
@@ -1010,6 +1020,32 @@ def _tempdir(suffix="", prefix="tmp", dir=None):  # pylint: disable=redefined-bu
         cleanup_tempdir()
 
 
+# _group_tempdir()
+#
+# Same as _tempdir(), but it allows RWX access to the entire user
+# *group*, instead of just the user.
+#
+# NOTE: This is potentially insecure. If created in a directory with
+#       too open permissions, this will allow all users of the same
+#       group to read files in here, which may leak
+#       information. *Only* use this in directories whose parents are
+#       more tightly controlled (i.e., non-public directories).
+#
+# Args:
+#    dir (str): A path to a parent directory for the temporary directory
+#    suffix (str): A suffix for the temproary directory name
+#    prefix (str): A prefix for the temporary directory name
+#
+# Yields:
+#    (str): The temporary directory
+#
+@contextmanager
+def _group_tempdir(**kwargs):
+    with _tempdir() as tempdir:
+        os.chmod(tempdir, URWX_GRWX)
+        yield tempdir
+
+
 # _tempnamedfile()
 #
 # A context manager for doing work on an open temporary file
@@ -1100,6 +1136,13 @@ def _tempnamedfile_name(dir):  # pylint: disable=redefined-builtin
             rm_tempfile()
 
 
+@contextmanager
+def _group_tempnamedfile_name(dir):
+    with _tempnamedfile_name(dir) as temp:
+        os.chmod(temp, URWX_GRWX)
+        yield temp
+
+
 # _kill_process_tree()
 #
 # Brutally murder a process and all of its children