libstore/platform/linux: allow xattrs in the sandbox

Fixes #838.

xattrs were historically disabled in 2017, since then, NFS v4.2 gained
support for xattrs and almost all filesystems support xattrs. In
addition, moving the build directory to a xattrs-supporting filesystem
is always an option. Programs may exploit xattrs (including ACLs) during
their build process and testing phase, to better serve these programs,
we will remove this limitation.

Lix will use POSIX ACLs for its UNIX domain sockets in the future and be
one of these programs which will run tests making use of xattrs, while
not writing any xattrs in the derivation outputs themselves.

xattrs are still scrubbed from derivation outputs so it is not possible
to obtain an advantage by writing a security-related xattr to a well
chosen file.

Tests were added to test these scenarios on Linux. Darwin is carefully
excluded because of #1008 and #1090, solving that is welcome. In the
meantime, they are marked xfail.

Change-Id: Ia3255eeb8442e83db4f10dcb5a51cbc368a2550d
Signed-off-by: Raito Bezarius <raito@lix.systems>

lix/libstore/platform/linux.cc

@@ -223,7 +223,6 @@ static std::vector<struct sock_filter> 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<struct sock_filter> 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<struct sock_filter> 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<struct sock_filter> 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<struct sock_filter> 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([&]() {

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;

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"}
+    '';
+  };
+}

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)

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]))

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():

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:

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

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")