diff options
-rw-r--r-- | src/buildstream/_cas/casdprocessmanager.py | 61 | ||||
-rw-r--r-- | src/buildstream/testing/_sourcetests/source_determinism.py | 9 | ||||
-rw-r--r-- | src/buildstream/testing/_utils/site.py | 5 | ||||
-rw-r--r-- | src/buildstream/utils.py | 2 | ||||
-rw-r--r-- | tests/frontend/buildcheckout.py | 8 | ||||
-rw-r--r-- | tests/frontend/pull.py | 4 | ||||
-rw-r--r-- | tests/integration/filter.py | 2 | ||||
-rw-r--r-- | tests/integration/source-determinism.py | 9 |
8 files changed, 87 insertions, 13 deletions
diff --git a/src/buildstream/_cas/casdprocessmanager.py b/src/buildstream/_cas/casdprocessmanager.py index 68bb88ef0..4c9d80209 100644 --- a/src/buildstream/_cas/casdprocessmanager.py +++ b/src/buildstream/_cas/casdprocessmanager.py @@ -18,8 +18,10 @@ import contextlib import os +import random import shutil import signal +import stat import subprocess import tempfile import time @@ -52,10 +54,7 @@ class CASDProcessManager: def __init__(self, path, log_dir, log_level, cache_quota, protect_session_blobs): self._log_dir = log_dir - # Place socket in global/user temporary directory to avoid hitting - # the socket path length limit. - self._socket_tempdir = tempfile.mkdtemp(prefix="buildstream") - self._socket_path = os.path.join(self._socket_tempdir, "casd.sock") + self._socket_path = self._make_socket_path(path) self._connection_string = "unix:" + self._socket_path casd_args = [utils.get_host_tool("buildbox-casd")] @@ -80,6 +79,60 @@ class CASDProcessManager: with _signals.blocked([signal.SIGINT], ignore=False): self.process = subprocess.Popen(casd_args, cwd=path, stdout=logfile_fp, stderr=subprocess.STDOUT) + # _make_socket_path() + # + # Create a path to the CASD socket, ensuring that we don't exceed + # the socket path limit. + # + # Note that we *may* exceed the path limit if the python-chosen + # tmpdir path is very long, though this should be /tmp. + # + # Args: + # path (str): The root directory for the CAS repository. + # + # Returns: + # (str) - The path to the CASD socket. + # + def _make_socket_path(self, path): + self._socket_tempdir = tempfile.mkdtemp(prefix="buildstream") + # mkdtemp will create this directory in the "most secure" + # way. This translates to "u+rwx,go-rwx". + # + # This is a good thing, generally, since it prevents us + # from leaking sensitive information to other users, but + # it's a problem for the workflow for userchroot, since + # the setuid casd binary will not share a uid with the + # user creating the tempdir. + # + # Instead, we chmod the directory 755, and only place a + # symlink to the CAS directory in here, which will allow the + # CASD process RWX access to a directory without leaking build + # information. + os.chmod( + self._socket_tempdir, + stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR | stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH | stat.S_IXOTH, + ) + + os.symlink(path, os.path.join(self._socket_tempdir, "cas")) + # FIXME: There is a potential race condition here; if multiple + # instances happen to create the same socket path, at least + # one will try to talk to the same server as us. + # + # There's no real way to avoid this from our side; we'd need + # buildbox-casd to tell us that it could not create a fresh + # socket. + # + # We could probably make this even safer by including some + # thread/process-specific information, but we're not really + # supporting this use case anyway; it's mostly here fore + # testing, and to help more gracefully handle the situation. + # + # Note: this uses the same random string generation principle + # as cpython, so this is probably a safe file name. + random_name = "".join([random.choice("abcdefghijklmnopqrstuvwxyz0123456789_") for _ in range(8)]) + socket_name = "casserver-{}.sock".format(random_name) + return os.path.join(self._socket_tempdir, "cas", socket_name) + # _rotate_and_get_next_logfile() # # Get the logfile to use for casd diff --git a/src/buildstream/testing/_sourcetests/source_determinism.py b/src/buildstream/testing/_sourcetests/source_determinism.py index 27664e0c2..f84d18ba2 100644 --- a/src/buildstream/testing/_sourcetests/source_determinism.py +++ b/src/buildstream/testing/_sourcetests/source_determinism.py @@ -23,7 +23,7 @@ import os import pytest from buildstream import _yaml -from .._utils.site import HAVE_SANDBOX +from .._utils.site import HAVE_SANDBOX, CASD_SEPARATE_USER from .. import create_repo from .. import cli # pylint: disable=unused-import from .utils import kind # pylint: disable=unused-import @@ -101,4 +101,9 @@ def test_deterministic_source_umask(cli, tmpdir, datafiles, kind): os.umask(old_umask) cli.remove_artifact_from_cache(project, element_name) - assert get_value_for_umask(0o022) == get_value_for_umask(0o077) + if CASD_SEPARATE_USER: + # buildbox-casd running as separate user of the same group can't + # function in a test environment with a too restrictive umask. + assert get_value_for_umask(0o002) == get_value_for_umask(0o007) + else: + assert get_value_for_umask(0o022) == get_value_for_umask(0o077) diff --git a/src/buildstream/testing/_utils/site.py b/src/buildstream/testing/_utils/site.py index 953d21607..5507df0f7 100644 --- a/src/buildstream/testing/_utils/site.py +++ b/src/buildstream/testing/_utils/site.py @@ -2,6 +2,7 @@ # so we dont have to repeat this everywhere # import os +import stat import subprocess import sys import platform @@ -65,6 +66,10 @@ try: except ImportError: HAVE_ARPY = False +casd_path = utils.get_host_tool("buildbox-casd") +CASD_SEPARATE_USER = bool(os.stat(casd_path).st_mode & stat.S_ISUID) +del casd_path + try: utils.get_host_tool("buildbox") HAVE_BUILDBOX = True diff --git a/src/buildstream/utils.py b/src/buildstream/utils.py index 5009be338..b6716a29d 100644 --- a/src/buildstream/utils.py +++ b/src/buildstream/utils.py @@ -326,7 +326,7 @@ def safe_link(src: str, dest: str, *, result: Optional[FileListResult] = None, _ if e.errno == errno.EEXIST and not _unlink: # Target exists already, unlink and try again safe_link(src, dest, result=result, _unlink=True) - elif e.errno == errno.EXDEV: + elif e.errno == errno.EXDEV or e.errno == errno.EPERM: safe_copy(src, dest) else: raise UtilError("Failed to link '{} -> {}': {}".format(src, dest, e)) from e diff --git a/tests/frontend/buildcheckout.py b/tests/frontend/buildcheckout.py index ffe564e81..d6a216daf 100644 --- a/tests/frontend/buildcheckout.py +++ b/tests/frontend/buildcheckout.py @@ -10,7 +10,7 @@ import pytest from buildstream.testing import cli # pylint: disable=unused-import from buildstream.testing import create_repo -from buildstream.testing._utils.site import IS_WINDOWS +from buildstream.testing._utils.site import IS_WINDOWS, CASD_SEPARATE_USER from buildstream import _yaml from buildstream._exceptions import ErrorDomain, LoadErrorReason from buildstream import utils @@ -35,6 +35,9 @@ def strict_args(args, strict): [("strict", "copies"), ("strict", "hardlinks"), ("non-strict", "copies"), ("non-strict", "hardlinks"),], ) def test_build_checkout(datafiles, cli, strict, hardlinks): + if CASD_SEPARATE_USER and hardlinks == "hardlinks": + pytest.xfail("Cannot hardlink with buildbox-casd running as a separate user") + project = str(datafiles) checkout = os.path.join(cli.directory, "checkout") @@ -580,6 +583,9 @@ def test_build_checkout_nonempty(datafiles, cli, hardlinks): @pytest.mark.datafiles(DATA_DIR) @pytest.mark.parametrize("hardlinks", [("copies"), ("hardlinks")]) def test_build_checkout_force(datafiles, cli, hardlinks): + if CASD_SEPARATE_USER and hardlinks == "hardlinks": + pytest.xfail("Cannot hardlink with buildbox-casd running as a separate user") + project = str(datafiles) checkout = os.path.join(cli.directory, "checkout") filename = os.path.join(checkout, "file.txt") diff --git a/tests/frontend/pull.py b/tests/frontend/pull.py index fcfd447b7..3ae394fd1 100644 --- a/tests/frontend/pull.py +++ b/tests/frontend/pull.py @@ -414,7 +414,7 @@ def test_pull_access_rights(cli, tmpdir, datafiles): result = cli.run( project=project, - args=["artifact", "checkout", "--hardlinks", "--no-integrate", "compose-all.bst", "--directory", checkout], + args=["artifact", "checkout", "--no-integrate", "compose-all.bst", "--directory", checkout], ) result.assert_success() @@ -440,7 +440,7 @@ def test_pull_access_rights(cli, tmpdir, datafiles): result = cli.run( project=project, - args=["artifact", "checkout", "--hardlinks", "--no-integrate", "compose-all.bst", "--directory", checkout], + args=["artifact", "checkout", "--no-integrate", "compose-all.bst", "--directory", checkout], ) result.assert_success() diff --git a/tests/integration/filter.py b/tests/integration/filter.py index f13521cc8..2fca8957c 100644 --- a/tests/integration/filter.py +++ b/tests/integration/filter.py @@ -29,7 +29,7 @@ def test_filter_pass_integration(datafiles, cli): checkout_dir = os.path.join(project, "filter") result = cli.run( project=project, - args=["artifact", "checkout", "--integrate", "--hardlinks", "--directory", checkout_dir, "filter/filter.bst"], + args=["artifact", "checkout", "--integrate", "--directory", checkout_dir, "filter/filter.bst"], ) result.assert_success() diff --git a/tests/integration/source-determinism.py b/tests/integration/source-determinism.py index a349f9eeb..14559759d 100644 --- a/tests/integration/source-determinism.py +++ b/tests/integration/source-determinism.py @@ -6,7 +6,7 @@ import pytest from buildstream import _yaml from buildstream.testing import cli_integration as cli # pylint: disable=unused-import -from buildstream.testing._utils.site import HAVE_SANDBOX +from buildstream.testing._utils.site import HAVE_SANDBOX, CASD_SEPARATE_USER DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "project") @@ -69,4 +69,9 @@ def test_deterministic_source_local(cli, tmpdir, datafiles): finally: cli.remove_artifact_from_cache(project, element_name) - assert get_value_for_mask(0o7777) == get_value_for_mask(0o0700) + if CASD_SEPARATE_USER: + # buildbox-casd running as separate user of the same group can't + # read files with too restrictive permissions. + assert get_value_for_mask(0o7777) == get_value_for_mask(0o0770) + else: + assert get_value_for_mask(0o7777) == get_value_for_mask(0o0700) |