Do ACLs still get removed with this patch?

You mean the nfs4 ones in question? After running nix-channel --update and
nix-env -u (both of which failed without the patch), I do:

$ getfattr --physical --recursive --name system.nfs4_acl /nix/

and the acls seem to be there.

Can you please try this:

$ nix-build --option filter-syscalls false -E 'with import <nixpkgs> {}; runCommand "foo" { buildInputs = [ acl ]; } "mkdir $out; date > $out/x; setfacl -m u:1234:rwx $out/x"'

$ getfacl ./result/x

The result should be

# file: result/x
# owner: root
# group: root
user::r--
group::r--
other::r--

This does require the filter-syscalls option, which I just added (1dd29d7), so you need to build Nix master for this.

From inside the install prefix:

$ NIX_BIN_DIR=$(pwd)/bin NIX_DATA_DIR=$(pwd)/share nix-build --option filter-syscalls false -E 'with import <nixpkgs> {}; runCommand "foo" { buildInputs = [ acl ]; } "mkdir $out; date > $out/x; setfacl -m u:1234:rwx $out/x"'
these derivations will be built:
  /home/philipp/nix-store/4jg1yiq3jiiglpwqlxpnn9bxhcsz7mz6-foo.drv
building path(s) '/home/philipp/nix-store/3qy0yyd8j8930hfqaa6mf9sz2mi6bpy7-foo'
building '/home/philipp/nix-store/4jg1yiq3jiiglpwqlxpnn9bxhcsz7mz6-foo.drv'...
setfacl: /home/philipp/nix-store/3qy0yyd8j8930hfqaa6mf9sz2mi6bpy7-foo/x: Operation not supported
builder for '/home/philipp/nix-store/4jg1yiq3jiiglpwqlxpnn9bxhcsz7mz6-foo.drv' failed with exit code 1
error: build of '/home/philipp/nix-store/4jg1yiq3jiiglpwqlxpnn9bxhcsz7mz6-foo.drv' failed

Where /home/ is mounted over NFS.

Is that with the filter-syscalls option? setfacl failing suggests that the syscall filter is still active.

Yes, I rebased the patch on top of yours and recompiled nix before
running that command. You’d get an “warning: unknown setting”
warning otherwise.

In strace I observe this:

[pid  5357] execve("/home/philipp/nix-store/m6q5l3ddrhjqv8a8rlychfl9d6fpn2lv-acl-2.2.52-bin/bin/setfacl", ["setfacl", "-m", "u:1234:rwx", "/home/philipp/nix-store/3qy0yyd8"...], 0x755008 /* 36 vars */) = 0
…
[pid  5357] lstat("/home/philipp/nix-store/3qy0yyd8j8930hfqaa6mf9sz2mi6bpy7-foo/x", {st_mode=S_IFREG|0644, st_size=29, ...}) = 0
[pid  5357] getxattr("/home/philipp/nix-store/3qy0yyd8j8930hfqaa6mf9sz2mi6bpy7-foo/x", "system.posix_acl_access", 0x7fffffffcbb0, 132) = -1 EOPNOTSUPP (Operation not supported)
[pid  5357] setxattr("/home/philipp/nix-store/3qy0yyd8j8930hfqaa6mf9sz2mi6bpy7-foo/x", "system.posix_acl_access", "\2\0\0\0\1\0\6\0\377\377\377\377\2\0\7\0\322\4\0\0\4\0\4\0\377\377\377\377\20\0\7", 44, 0) = -1 EOPNOTSUPP (Operation not supported)
[pid  5357] write(2, "setfacl: /home/philipp/nix-store"..., 97) = 97

Could we also do this for Lustre file systems?
I'm getting the folowing error on a Lustre file system.

error: removing extended attribute ‘lustre.lov‘

Ping! cc NixOS/nixpkgs#29778

@edolstra maybe instead of playing whac-a-mole with these whitelisted attributes, we should just make a nix.conf option for them? It's going to be impossible to predict all the weird special cases people have locally and having a conf option seems like it allows people to fix their own issues. Of course, we might still want nfs4_acl in the default set...

With filter-syscalls shouldn't we just leave ACLs alone?

I'd rather not rely exclusively on the seccomp filter.

I just reproduced the setup with the proposed patch. As reported earlier, the patch seems to work, but the test with 'setfacl' fails with 'Operation not supported'.

I'm not an expert on ACL nor NFS, but it seems to me that the "Operation not supported" is due to NFS4 not supporting setfacl (but nfs4_setacl). Running setfacl -m u:1234:rwx /path/on/nfs also fails with the same error message. When running the setfacl nix-build on a non-nfs /nix/store the getfacl shows the desired permissions as mentioned in a previous comments.

The following gist fix-fs-attrs.nix contains all the steps to reproduce/test the patch. The acls are removed as expected if nix-shell ./fix-fs-attrs.nix -A shell --command 'testacl' runs successfully.

As the 'acl' package is not available on darwin, I run it with the current lnl7/nix docker image (nix-2.1.2, nixpkgs-unstable-2018-09-21) and the script is successfull. On a nixops deployed host, I had to replace the u:1234:rwx with the u:$(whoami):rwx to avoid a setfacl: dWithAcl/x: Invalid argument. I suppose this has to do with sandboxing on nixos and that the test script is still okay like that, isn't it?

Does this help to make progress on this issue?

nix-shell ./fix-fs-attrs.nix -A shell --command 'testacl'
...
building '/nix/store/mlj6n16vc0ygvcs4kigimw57qfircsss-buildWithAcl.drv'...
getfacl: Removing leading '/' from absolute path names
# file: nix/store/5bda66j38sayqyb5lp35z7dwy84rg16l-buildWithAcl/x
# owner: nixbld1
# group: nixbld
user::rw-
user:1234:rwx
group::r--
mask::rwx
other::r--

/nix/store/5bda66j38sayqyb5lp35z7dwy84rg16l-buildWithAcl
# owner: root
# group: root
user::r--
group::r--
other::r--

ACL Removal:
successful

This patch was mentioned in https://rgoswami.me/posts/local-nix-no-root/

I just ran into this bug on a NFS /home using unprivileged userns.

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/error-invalidpath-nix-env/10851/1

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/building-a-statically-linked-nix-for-hpc-environments/10865/6

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-portable-nix-static-permissionless-install-free-pre-configured/11719/12

This is now part of https://github.com/DavHau/nix-portable/

I marked this as stale due to inactivity. → More info

Still needed.

Could this profit from the recent ACL design ideas? https://gist.github.com/edolstra/afa5a41d4acbc0d6c8cccfede7fd4792