diff options
author | bst-marge-bot <marge-bot@buildstream.build> | 2019-11-25 13:07:45 +0000 |
---|---|---|
committer | bst-marge-bot <marge-bot@buildstream.build> | 2019-11-25 13:07:45 +0000 |
commit | e516e1c065b5129630fd62c2115be1ea8bd7d658 (patch) | |
tree | 604d3fc75780ee3c942526bc16b6b228b379ef94 | |
parent | c6e2ea93a0bf8dcc34623339063e6a91b2eb3d51 (diff) | |
parent | 18f8e38161d58cf31a56f1de53d5cda9cb4d470b (diff) | |
download | buildstream-e516e1c065b5129630fd62c2115be1ea8bd7d658.tar.gz |
Merge branch 'juerg/umask' into 'master'
Respect umask for created file and directories
See merge request BuildStream/buildstream!1724
-rw-r--r-- | src/buildstream/element.py | 21 | ||||
-rw-r--r-- | src/buildstream/plugins/sources/tar.py | 9 | ||||
-rw-r--r-- | src/buildstream/testing/integration.py | 4 | ||||
-rw-r--r-- | src/buildstream/utils.py | 37 |
4 files changed, 53 insertions, 18 deletions
diff --git a/src/buildstream/element.py b/src/buildstream/element.py index a8c6bfa8f..e40dc3c8c 100644 --- a/src/buildstream/element.py +++ b/src/buildstream/element.py @@ -81,7 +81,6 @@ import contextlib from contextlib import contextmanager from functools import partial from itertools import chain -import tempfile import string from typing import cast, TYPE_CHECKING, Any, Dict, Iterator, List, Optional @@ -94,7 +93,6 @@ from ._exceptions import BstError, LoadError, LoadErrorReason, ImplError, ErrorD from .utils import FileListResult from . import utils from . import _cachekey -from . import _signals from . import _site from ._platform import Platform from .node import Node @@ -1635,13 +1633,10 @@ class Element(Plugin): # Explicitly clean it up, keep the build dir around if exceptions are raised os.makedirs(context.builddir, exist_ok=True) - rootdir = tempfile.mkdtemp(prefix="{}-".format(self.normal_name), dir=context.builddir) - # Cleanup the build directory on explicit SIGTERM - def cleanup_rootdir(): - utils._force_rmtree(rootdir) - - with _signals.terminator(cleanup_rootdir), self.__sandbox( + with utils._tempdir( + prefix="{}-".format(self.normal_name), dir=context.builddir + ) as rootdir, self.__sandbox( rootdir, output_file, output_file, self.__sandbox_config ) as sandbox: # noqa @@ -1696,8 +1691,6 @@ class Element(Plugin): raise else: return self._cache_artifact(rootdir, sandbox, collect) - finally: - cleanup_rootdir() def _cache_artifact(self, rootdir, sandbox, collect): @@ -2587,17 +2580,15 @@ class Element(Plugin): else: os.makedirs(context.builddir, exist_ok=True) - rootdir = tempfile.mkdtemp(prefix="{}-".format(self.normal_name), dir=context.builddir) # Recursive contextmanager... - with self.__sandbox( + with utils._tempdir( + prefix="{}-".format(self.normal_name), dir=context.builddir + ) as rootdir, self.__sandbox( rootdir, stdout=stdout, stderr=stderr, config=config, allow_remote=allow_remote, bare_directory=False ) as sandbox: yield sandbox - # Cleanup the build dir - utils._force_rmtree(rootdir) - @classmethod def __compose_default_splits(cls, project, defaults, is_junction): diff --git a/src/buildstream/plugins/sources/tar.py b/src/buildstream/plugins/sources/tar.py index 658cc2735..e35b52f85 100644 --- a/src/buildstream/plugins/sources/tar.py +++ b/src/buildstream/plugins/sources/tar.py @@ -76,8 +76,13 @@ class ReadableTarInfo(tarfile.TarInfo): @property def mode(self): - # ensure file is readable by owner - return self.__permission | 0o400 + # Respect umask instead of the file mode stored in the archive. + # The only bit used from the embedded mode is the executable bit for files. + umask = utils.get_umask() + if self.isdir() or bool(self.__permission | 0o100): + return 0o777 & ~umask + else: + return 0o666 & ~umask @mode.setter def mode(self, permission): diff --git a/src/buildstream/testing/integration.py b/src/buildstream/testing/integration.py index 584d7da1b..9a1a48816 100644 --- a/src/buildstream/testing/integration.py +++ b/src/buildstream/testing/integration.py @@ -28,6 +28,8 @@ import tempfile import pytest +from buildstream import utils + # Return a list of files relative to the given directory def walk_dir(root): @@ -66,6 +68,8 @@ class IntegrationCache: # the artifacts directory try: self.cachedir = tempfile.mkdtemp(dir=self.root, prefix="cache-") + # Apply mode allowed by umask + os.chmod(self.cachedir, 0o777 & ~utils.get_umask()) except OSError as e: raise AssertionError("Unable to create test directory !") from e diff --git a/src/buildstream/utils.py b/src/buildstream/utils.py index 181ea1df9..7f7bf67b2 100644 --- a/src/buildstream/utils.py +++ b/src/buildstream/utils.py @@ -65,6 +65,11 @@ _INITIAL_NUM_THREADS_IN_MAIN_PROCESS = 1 # Number of seconds to wait for background threads to exit. _AWAIT_THREADS_TIMEOUT_SECONDS = 5 +# The process's file mode creation mask. +# Impossible to retrieve without temporarily changing it on POSIX. +_UMASK = os.umask(0o777) +os.umask(_UMASK) + class UtilError(BstError): """Raised by utility functions when system calls fail. @@ -602,6 +607,8 @@ def save_file_atomic( if tempdir is None: tempdir = os.path.dirname(filename) fd, tempname = tempfile.mkstemp(dir=tempdir) + # Apply mode allowed by umask + os.fchmod(fd, 0o666 & ~_UMASK) os.close(fd) f = open( @@ -638,6 +645,17 @@ def save_file_atomic( raise +# get_umask(): +# +# Get the process's file mode creation mask without changing it. +# +# Returns: +# (int) The process's file mode creation mask. +# +def get_umask(): + return _UMASK + + # _get_dir_size(): # # Get the disk usage of a given directory in bytes. @@ -1002,6 +1020,13 @@ def _set_deterministic_mtime(directory): # # A context manager for doing work in a temporary directory. # +# NOTE: Unlike mkdtemp(), this method may not restrict access to other +# users. The process umask is the only access restriction, similar +# to mkdir(). +# This is potentially insecure. Do not create directories in /tmp +# with this method. *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 @@ -1015,7 +1040,14 @@ def _set_deterministic_mtime(directory): # supports cleaning up the temp directory on SIGTERM. # @contextmanager -def _tempdir(suffix="", prefix="tmp", dir=None): # pylint: disable=redefined-builtin +def _tempdir(*, suffix="", prefix="tmp", dir): # pylint: disable=redefined-builtin + # Do not allow fallback to a global temp directory. Due to the chmod + # below, this method is not safe to be used in global temp + # directories such as /tmp. + assert ( + dir + ), "Creating directories in the public fallback `/tmp` is dangerous. Please use a directory with tight access controls." + tempdir = tempfile.mkdtemp(suffix=suffix, prefix=prefix, dir=dir) def cleanup_tempdir(): @@ -1024,6 +1056,9 @@ def _tempdir(suffix="", prefix="tmp", dir=None): # pylint: disable=redefined-bu try: with _signals.terminator(cleanup_tempdir): + # Apply mode allowed by umask + os.chmod(tempdir, 0o777 & ~_UMASK) + yield tempdir finally: cleanup_tempdir() |