diff --git a/lix/libstore/platform/linux.cc b/lix/libstore/platform/linux.cc index b072e77d3..7822502de 100644 --- a/lix/libstore/platform/linux.cc +++ b/lix/libstore/platform/linux.cc @@ -223,7 +223,6 @@ static std::vector compileSyscallFilter() // Run check-syscalls to determine which new syscalls should be added. // New syscalls must be audited and handled in a way that blocks the following dangerous operations: // * Creation of non-empty setuid/setgid files - // * Creation of extended attributes (including ACLs) // // BEGIN extract-syscalls allowSyscall(ctx, SCMP_SYS(accept)); @@ -313,7 +312,7 @@ static std::vector compileSyscallFilter() allowSyscall(ctx, SCMP_SYS(fork)); allowSyscall(ctx, SCMP_SYS(fremovexattr)); allowSyscall(ctx, SCMP_SYS(fsconfig)); - // skip fsetxattr (dangerous) + allowSyscall(ctx, SCMP_SYS(fsetxattr)); allowSyscall(ctx, SCMP_SYS(fsmount)); allowSyscall(ctx, SCMP_SYS(fsopen)); allowSyscall(ctx, SCMP_SYS(fspick)); @@ -417,7 +416,7 @@ static std::vector compileSyscallFilter() allowSyscall(ctx, SCMP_SYS(lookup_dcookie)); allowSyscall(ctx, SCMP_SYS(lremovexattr)); allowSyscall(ctx, SCMP_SYS(lseek)); - // skip lsetxattr (dangerous) + allowSyscall(ctx, SCMP_SYS(lsetxattr)); allowSyscall(ctx, SCMP_SYS(lstat)); allowSyscall(ctx, SCMP_SYS(lstat64)); allowSyscall(ctx, SCMP_SYS(madvise)); @@ -615,7 +614,7 @@ static std::vector compileSyscallFilter() allowSyscall(ctx, SCMP_SYS(set_tls)); allowSyscall(ctx, SCMP_SYS(setuid)); allowSyscall(ctx, SCMP_SYS(setuid32)); - // skip setxattr (dangerous) + allowSyscall(ctx, SCMP_SYS(setxattr)); allowSyscall(ctx, SCMP_SYS(sgetmask)); allowSyscall(ctx, SCMP_SYS(shmat)); allowSyscall(ctx, SCMP_SYS(shmctl)); @@ -722,13 +721,6 @@ static std::vector compileSyscallFilter() ALLOW_CHMOD_IF_SAFE(ctx, SCMP_SYS(fchmodat), 2); ALLOW_CHMOD_IF_SAFE(ctx, SCMP_SYS(fchmodat2), 2); - // setxattr family: prevent creation of extended attributes or ACLs. - // Not all filesystems support them, and they're incompatible with the NAR format. - if (seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(setxattr), 0) != 0 || - seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(lsetxattr), 0) != 0 || - seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(fsetxattr), 0) != 0) - throw SysError("unable to add seccomp rule"); - Pipe filterPipe; filterPipe.create(); auto filterBytes_ = std::async([&]() { diff --git a/package.nix b/package.nix index 9e496984e..7b202b9d9 100644 --- a/package.nix +++ b/package.nix @@ -245,13 +245,19 @@ stdenv.mkDerivation (finalAttrs: { p.python-frontmatter p.pycapnp ] - ++ lib.optionals finalAttrs.doCheck [ + ++ lib.optionals finalAttrs.doCheck ([ (dontWrapPython p.pytest) p.pytest-xdist p.pytest-timeout p.ruff p.aiohttp - ] + + # Remove once https://github.com/NixOS/nixpkgs/pull/476848 lands here. + (p.pyxattr.overrideAttrs (old: { + buildInputs = lib.filter (lib.meta.availableOn stdenv.hostPlatform) old.buildInputs; + meta.platforms = old.meta.platforms ++ lib.platforms.darwin; + })) + ]) ); buildTestShell = @@ -262,7 +268,18 @@ stdenv.mkDerivation (finalAttrs: { else null; - buildTestEnv = if hostPlatform.isLinux then "${pkgsStatic.busybox}/bin" else null; + buildTestEnv = + if hostPlatform.isLinux then + lib.makeBinPath [ + pkgsStatic.busybox + pkgsStatic.acl + ] + else + lib.makeBinPath [ + pkgs.darwin.file_cmds + pkgs.darwin.shell_cmds + pkgs.darwin.text_cmds + ]; src = fileset.toSource { root = ./.; @@ -618,6 +635,12 @@ stdenv.mkDerivation (finalAttrs: { p.xdg-base-dirs p.packaging p.xonsh + + # Remove once https://github.com/NixOS/nixpkgs/pull/476848 lands here. + (p.pyxattr.overrideAttrs (old: { + buildInputs = lib.filter (lib.meta.availableOn stdenv.hostPlatform) old.buildInputs; + meta.platforms = old.meta.platforms ++ lib.platforms.darwin; + })) ] ); pythonEnv = python3.pythonOnBuildForHost.withPackages pythonPackages; diff --git a/tests/functional2/build/assets/xattrs.nix b/tests/functional2/build/assets/xattrs.nix new file mode 100644 index 000000000..c00ec49de --- /dev/null +++ b/tests/functional2/build/assets/xattrs.nix @@ -0,0 +1,102 @@ +with import ./config.nix; +let + setXattrLinux = inodeTgt: name: value: "setfattr -n ${name} -v \"${value}\" ${inodeTgt}"; + setExtendedAttribute = if isDarwin then setXattrDarwin else setXattrLinux; + setAndCheckACLLinux = inodeTgt: perm: '' + setfacl -m ${perm} ${inodeTgt} + getfacl -an ${inodeTgt} | grep -i "mask::rw." || { echo no mask in the ACL, setfacl ineffective?; exit 1; } + ''; + setAndCheckACL = if isDarwin then setAndCheckACLDarwin else setAndCheckACLLinux; + + wellKnownLinuxNames = { + "user.mime_type" = "text/html5"; + "user.deriver" = "yue"; + + # On Linux, inside of the sandbox, you cannot write to these because CAP_SYS_ADMIN is required. + # Those are protected xattrs. + # Root inside the root namespace can set those. + # "trusted.md5sum" = "..."; + # "security.selinux" = "..."; + }; + wellKnownACLs = [ + # sets xattr system.posix_acl_access on Linux. + "u:1000:rwX" + ]; + mapAttrsToList = fn: attrs: map (k: fn k attrs.${k}) (builtins.attrNames attrs); + + linuxScenariosForAnInode = withACL: inodeTgt: + builtins.concatStringsSep "\n" ( + (mapAttrsToList (name: value: setExtendedAttribute inodeTgt name value) wellKnownLinuxNames) + ++ (if withACL then map (setAndCheckACL inodeTgt) wellKnownACLs else [ ]) + ); + + setXattrDarwin = inodeTgt: name: value: "xattr -w ${name} \"${value}\" ${inodeTgt}"; + setAndCheckACLDarwin = inodeTgt: perm: '' + chmod +a "${perm}" "${inodeTgt}" + ls -el "${inodeTgt}" + ls -el "${inodeTgt}" | grep -Ei '${perm}' + ''; + # NOTE(Qyriad): Darwin ACLs have some kind of sorting order, so ideally we should parse them to check them. + # However: fuck that, for now. So we've just written them in The Order here so the text-match. + wellKnownACLsDarwin = [ + "group:everyone allow read,write" + "group:staff allow read,delete,writesecurity,chown" + ]; + darwinScenariosForAnInode = withACL: inodeTgt: + builtins.concatStringsSep "\n" ( + (mapAttrsToList (name: value: setXattrDarwin inodeTgt name value) wellKnownLinuxNames) + ++ (if withACL then map (setAndCheckACLDarwin inodeTgt) wellKnownACLsDarwin else [ ]) + ); + + scenariosForAnInode = if isDarwin then darwinScenariosForAnInode else linuxScenariosForAnInode; +in +{ + during-build = mkDerivation { + name = "xattrs-during-build"; + buildCommand = '' + touch work + ${scenariosForAnInode true "./work"} + echo meow > $out + ''; + }; + + in-root-outputs-file = mkDerivation { + name = "xattrs-on-root-output-file"; + buildCommand = '' + touch $out + + ${ + # We are not allowed to set ACLs on the root of the output directory. + # There's an explicit suspicious permission check that will cause this build to be rejected. + scenariosForAnInode false "$out" + } + ''; + }; + + in-root-outputs-dir = mkDerivation { + name = "xattrs-on-root-output-dir"; + buildCommand = '' + touch $out + + ${ + # We are not allowed to set ACLs on the root of the output directory. + # There's an explicit suspicious permission check that will cause this build to be rejected. + scenariosForAnInode false "$out" + } + ''; + }; + + in-output-content = mkDerivation { + name = "xattrs-under-directory-output"; + buildCommand = '' + mkdir -p $out + touch $out/test + mkdir -p $out/test2 $out/test3 + touch $out/test3/test4 + + ${scenariosForAnInode true "$out/test"} + ${scenariosForAnInode true "$out/test2"} + ${scenariosForAnInode true "$out/test3/test4"} + ''; + }; +} diff --git a/tests/functional2/build/test_xattrs.py b/tests/functional2/build/test_xattrs.py new file mode 100644 index 000000000..7143313ef --- /dev/null +++ b/tests/functional2/build/test_xattrs.py @@ -0,0 +1,46 @@ +import sys + +import pytest +from pathlib import Path +from functional2.testlib.fixtures.file_helper import with_files +from functional2.testlib.fixtures.nix import Nix +from functional2.testlib.utils import get_global_asset, CopyFile +from functional2.testlib.xattrs import verify_no_xattrs_in_tree, skip_if_xattrs_are_unsupported + + +@with_files( + {"config.nix": get_global_asset("config.nix"), "xattrs.nix": CopyFile("assets/xattrs.nix")} +) +def test_xattrs_during_build(nix: Nix): + skip_if_xattrs_are_unsupported(nix.env) + + nix.nix_build(["xattrs.nix", "-A", "during-build", "--no-out-link"]).run().ok() + + +def build_and_get_store_path(nix: Nix, asset: str, attribute: str) -> Path: + return nix.physical_store_path_for( + nix.nix_build([asset, "-A", attribute, "--no-out-link"]).run().ok().stdout_plain + ) + + +@with_files( + {"config.nix": get_global_asset("config.nix"), "xattrs.nix": CopyFile("assets/xattrs.nix")} +) +@pytest.mark.skipif( + sys.platform != "linux", reason="xattrs scrubbing is not expected to function outside of Linux" +) +def test_xattrs_in_output(nix: Nix): + skip_if_xattrs_are_unsupported(nix.env) + + # We assert that xattrs producing derivations in the outputs should complete with no xattrs in the final output path. + # NOTE: if another platform is added, another `verify_no_acl_in_tree` + # should be added to ensure that ACLs are truly removed. + # On Linux, this is not necessary. + output_path = build_and_get_store_path(nix, "xattrs.nix", "in-root-outputs-file") + verify_no_xattrs_in_tree(output_path) + + output_path = build_and_get_store_path(nix, "xattrs.nix", "in-root-outputs-dir") + verify_no_xattrs_in_tree(output_path) + + output_path = build_and_get_store_path(nix, "xattrs.nix", "in-output-content") + verify_no_xattrs_in_tree(output_path) diff --git a/tests/functional2/store/test_xattrs.py b/tests/functional2/store/test_xattrs.py new file mode 100644 index 000000000..de352b153 --- /dev/null +++ b/tests/functional2/store/test_xattrs.py @@ -0,0 +1,123 @@ +import pytest +import sys + +from functional2.testlib.fixtures.file_helper import with_files, File +from functional2.testlib.fixtures.nix import Nix +from functional2.testlib.xattrs import verify_no_xattrs_in_tree, skip_if_xattrs_are_unsupported + +import xattr +import json +import re + + +@with_files({"file-with-xattrs": File("hi")}) +@pytest.mark.parametrize("hash_fn", ["sha1", "sha256"]) +@pytest.mark.skipif( + sys.platform != "linux", reason="xattrs scrubbing is not expected to function outside of Linux" +) +def test_add_xattrs_fixed_flat_file(nix: Nix, hash_fn: str): + skip_if_xattrs_are_unsupported(nix.env) + + xattr.set(nix.env.dirs.home / "file-with-xattrs", "user.deriver", "test") + + res = nix.nix_store(["--add-fixed", hash_fn, "file-with-xattrs"]).run().ok() + verify_no_xattrs_in_tree(nix.physical_store_path_for(res.stdout_plain)) + + +@with_files({"file-with-xattrs": File("hi")}) +@pytest.mark.skipif( + sys.platform != "linux", reason="xattrs scrubbing is not expected to function outside of Linux" +) +def test_add_xattrs_flat_file(nix: Nix): + skip_if_xattrs_are_unsupported(nix.env) + + xattr.set(nix.env.dirs.home / "file-with-xattrs", "user.deriver", "test") + + res = nix.nix_store(["--add", "file-with-xattrs"]).run().ok() + verify_no_xattrs_in_tree(nix.physical_store_path_for(res.stdout_plain)) + + +@with_files({"dir-with-xattrs": {"test1": File("hi")}}) +@pytest.mark.parametrize("hash_fn", ["sha1", "sha256"]) +@pytest.mark.skipif( + sys.platform != "linux", reason="xattrs scrubbing is not expected to function outside of Linux" +) +def test_add_xattrs_directories(nix: Nix, hash_fn: str): + skip_if_xattrs_are_unsupported(nix.env) + + xattr.set(nix.env.dirs.home / "dir-with-xattrs", "user.deriver", "hi") + xattr.set(nix.env.dirs.home / "dir-with-xattrs/test1", "user.deriver", "hi") + + res = nix.nix_store(["--add-fixed", hash_fn, "--recursive", "dir-with-xattrs"]).run().ok() + verify_no_xattrs_in_tree(nix.physical_store_path_for(res.stdout_plain)) + + +@with_files({"file-with-xattrs": File("hi"), "dir-with-xattrs": {"test1": File("hi")}}) +@pytest.mark.skipif( + sys.platform != "linux", reason="xattrs scrubbing is not expected to function outside of Linux" +) +def test_interpolating_xattrs_file(nix: Nix): + skip_if_xattrs_are_unsupported(nix.env) + + xattr.set(nix.env.dirs.home / "file-with-xattrs", "user.deriver", "test") + xattr.set(nix.env.dirs.home / "dir-with-xattrs", "user.deriver", "hi") + xattr.set(nix.env.dirs.home / "dir-with-xattrs/test1", "user.deriver", "hi") + + res = nix.eval('"${./file-with-xattrs}"', flags=["--impure"]).ok() + store_path = json.loads(res.stdout_plain) + verify_no_xattrs_in_tree(nix.physical_store_path_for(store_path)) + + res = nix.eval('"${./dir-with-xattrs}"', flags=["--impure"]).ok() + store_path = json.loads(res.stdout_plain) + verify_no_xattrs_in_tree(nix.physical_store_path_for(store_path)) + + +@with_files({"file-with-xattrs": File("hi"), "dir-with-xattrs": {"test1": File("hi")}}) +@pytest.mark.skipif( + sys.platform != "linux", reason="xattrs scrubbing is not expected to function outside of Linux" +) +def test_builtins_path_xattrs(nix: Nix): + skip_if_xattrs_are_unsupported(nix.env) + + xattr.set(nix.env.dirs.home / "file-with-xattrs", "user.deriver", "test") + xattr.set(nix.env.dirs.home / "dir-with-xattrs", "user.deriver", "hi") + xattr.set(nix.env.dirs.home / "dir-with-xattrs/test1", "user.deriver", "hi") + + res = nix.eval( + 'builtins.path { name = "test-file"; path = ./file-with-xattrs; }', flags=["--impure"] + ).ok() + store_path = json.loads(res.stdout_plain) + verify_no_xattrs_in_tree(nix.physical_store_path_for(store_path)) + + res = nix.eval( + 'builtins.path { name = "test-file"; path = ./file-with-xattrs; recursive = true; }', + flags=["--impure"], + ).ok() + store_path = json.loads(res.stdout_plain) + verify_no_xattrs_in_tree(nix.physical_store_path_for(store_path)) + + res = nix.eval( + 'builtins.path { name = "test-dir"; path = ./dir-with-xattrs; recursive = true; }', + flags=["--impure"], + ).ok() + store_path = json.loads(res.stdout_plain) + verify_no_xattrs_in_tree(nix.physical_store_path_for(store_path)) + + +@with_files({"file-with-xattrs": File("hi"), "dir-with-xattrs": {"test1": File("hi")}}) +@pytest.mark.skipif( + sys.platform != "linux", reason="xattrs scrubbing is not expected to function outside of Linux" +) +def test_nix_prefetch_xattrs(nix: Nix): + skip_if_xattrs_are_unsupported(nix.env) + + xattr.set(nix.env.dirs.home / "file-with-xattrs", "user.deriver", "test") + xattr.set(nix.env.dirs.home / "dir-with-xattrs", "user.deriver", "hi") + xattr.set(nix.env.dirs.home / "dir-with-xattrs/test1", "user.deriver", "hi") + + locate_path_re = re.compile(r"path is '(.*)'") + for p in ("file-with-xattrs", "dir-with-xattrs"): + res = nix.nix_prefetch_url([f"file://{nix.env.dirs.home / p}"]).run().ok() + matches = locate_path_re.findall(res.stderr_plain) + assert matches, "nix prefetch must return at least one path" + verify_no_xattrs_in_tree(nix.physical_store_path_for(matches[0])) diff --git a/tests/functional2/testlib/fixtures/test_env.py b/tests/functional2/testlib/fixtures/test_env.py index c4576016c..9c7d80006 100644 --- a/tests/functional2/testlib/fixtures/test_env.py +++ b/tests/functional2/testlib/fixtures/test_env.py @@ -134,7 +134,8 @@ def test_env_to_env(tmp_path: Path): "PATH", "BUILD_TEST_SHELL", "TMPDIR", - } | ({"_NIX_TEST_NO_SANDBOX"} if sys.platform == "darwin" else {"BUILD_TEST_ENV"}) + "BUILD_TEST_ENV", + } | ({"_NIX_TEST_NO_SANDBOX"} if sys.platform == "darwin" else set()) def test_path_inits_build_shell(): diff --git a/tests/functional2/testlib/global_assets/config.nix.template b/tests/functional2/testlib/global_assets/config.nix.template index ad68f5507..21db5ef0d 100644 --- a/tests/functional2/testlib/global_assets/config.nix.template +++ b/tests/functional2/testlib/global_assets/config.nix.template @@ -12,6 +12,9 @@ rec { system = "@system@"; + isLinux = (builtins.match ".+-linux$" system) != null; + isDarwin = (builtins.match ".+-darwin$" system) != null; + shared = builtins.getEnv "_NIX_TEST_SHARED"; mkDerivation = args: diff --git a/tests/functional2/testlib/global_assets/dependencies/dependencies.nix b/tests/functional2/testlib/global_assets/dependencies/dependencies.nix index d9e3e3f1f..f939c0d51 100644 --- a/tests/functional2/testlib/global_assets/dependencies/dependencies.nix +++ b/tests/functional2/testlib/global_assets/dependencies/dependencies.nix @@ -17,7 +17,7 @@ let buildCommand = '' mkdir $out # Space-filler to test GC stats reporting - head -c 100k /dev/zero > $out/filler + head -c 100000 /dev/zero > $out/filler echo BAR > $out/bar echo ${input0} > $out/input0 echo ${input3} > $out/input3 diff --git a/tests/functional2/testlib/xattrs.py b/tests/functional2/testlib/xattrs.py new file mode 100644 index 000000000..2e454d2b3 --- /dev/null +++ b/tests/functional2/testlib/xattrs.py @@ -0,0 +1,53 @@ +import xattr +import pytest + +from pathlib import Path +from functional2.testlib.fixtures.env import ManagedEnv + + +def has_xattrs(path: Path) -> bool: + """ + Check if a file or directory has any extended attributes. + """ + try: + xattrs_list = xattr.list(str(path), nofollow=True) + return bool(xattrs_list) + except OSError: + # If an error occurs, either because the file doesn't exist or no xattrs, return False + return False + + +def verify_no_xattrs_in_tree(root_dir: Path) -> None: + """ + Traverse a directory tree and verify no file or directory has xattrs. + """ + if root_dir.is_file(): + assert not has_xattrs(root_dir) + return + + for entry in root_dir.iterdir(): + assert not has_xattrs(entry) + if entry.is_dir(): + verify_no_xattrs_in_tree(entry) + + +def test_set_clear_xattrs_in(dir_: Path) -> bool: + try: + xattr.set(dir_, "user.test", "1", nofollow=True) + xattr.remove(dir_, "user.test", nofollow=True) + return True + except OSError: + return False + + +def skip_if_xattrs_are_unsupported(env: ManagedEnv) -> None: + """ + Skip the current test if xattrs are unsupported in, either: + - the test root + - the fixture root + """ + if not test_set_clear_xattrs_in(env.dirs.test_root): + pytest.skip("xattrs cannot be used in the test root") + + if not test_set_clear_xattrs_in(env.dirs.home): + pytest.skip("xattrs cannot be used in the fixture directory")