Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

samba: Make adjustments to vfs_acl_xattr configuration #109

Merged
merged 3 commits into from
Jul 8, 2024

Conversation

anoopcs9
Copy link
Collaborator

@anoopcs9 anoopcs9 commented Jun 24, 2024

Following changes are made:

  • Remove 'ignore system acls' to keep lossy mapping to POSIX ACLs.
  • Configure xattr name to store NT ACLs as "user.NTACL".

We introduce a temporary workaround to allow smbd to assume cap_dac_override when operating as root until official fixes are available.

@anoopcs9 anoopcs9 force-pushed the acl-xattr-user-ns branch 2 times, most recently from 9beb728 to 0693121 Compare June 28, 2024 06:55
anoopcs9 added 2 commits July 4, 2024 11:21
Make sure that the Samba's mapping to POSIX ACLs is present on the
backend file system so that high level access control entries(if not
fine grained or rich as in NT ACLs) are visible to other protocol
clients as a reference.

Signed-off-by: Anoop C S <[email protected]>
security namespace when used for storing NTACLs with acl_xattr VFS
module requires CAP_SYS_ADMIN capability to successfully modify xattrs.
vfs_acl_xattr is capable enough to configure the xattr namespace(and
literally the whole name) using module options in smb.conf. In certain
deployment situations where services are run within containers we lack
CAP_SYS_ADMIN which then calls for a more flexible xattr name/namespace.

Signed-off-by: Anoop C S <[email protected]>
@anoopcs9 anoopcs9 force-pushed the acl-xattr-user-ns branch from 0693121 to 37750e2 Compare July 4, 2024 05:51
@anoopcs9
Copy link
Collaborator Author

anoopcs9 commented Jul 4, 2024

With the changes from this PR we have the following failures:

XFS and CephFS kclient

test: samba3.smb2.create.aclfile
time: 2024-07-04 06:37:01.295294Z
Testing nttrans create with sec_desc on files
basic create
adding a new ACE
creating a file with an initial ACL
time: 2024-07-04 06:37:01.319565Z
failure: samba3.smb2.create.aclfile [
Exception: (../../source4/torture/smb2/create.c:635) Incorrect status NT_STATUS_ACCESS_DENIED - should be NT_STATUS_OK

test: samba3.smb2.create.acldir
time: 2024-07-04 06:37:01.319599Z
Testing nttrans create with sec_desc on directories
basic create
adding a new ACE
creating a file with an initial ACL
time: 2024-07-04 06:37:01.346791Z
failure: samba3.smb2.create.acldir [
Exception: (../../source4/torture/smb2/create.c:635) Incorrect status NT_STATUS_ACCESS_DENIED - should be NT_STATUS_OK

test: samba3.smb2.session.reauth4
time: 2024-07-04 06:32:14.029002Z
time: 2024-07-04 06:32:14.051817Z
failure: samba3.smb2.session.reauth4 [
Exception: ../../source4/torture/smb2/session.c:603: status was NT_STATUS_OBJECT_NAME_NOT_FOUND, expected NT_STATUS_OK: smb2_setinfo_file failed

Earlier with ignore system acls set all files and directories created had write permission for others(because samba internally enforces settings like create mask = 0666, directory mask = 0777 etc in this case) and thus smbd didn't have to assume cap_dac_override(SELinux prevents it though) to perform fsetxattr(fd, "security.NTACL",...) when operating as root(cap_sys_admin was enough(and allowed)).

On the other side when ignore system acls is unset(and acl_xattr:security_acl_name = user.NTACL) the situation is different where even root will have to override permissions, which is prevented by SELinux at the moment on CentOS Stream 9 packages(fix to allow cap_dac_override is available with later selinux-policy versions), to perform fsetxattr(fd, "user.NTACL",...).

Even with customized local SELinux policy smb2.session.reauth4(but not others) still remained unsuccessful with the error changed to NT_STATUS_ACCESS_DENIED.

test: samba3.smb2.session.reauth4
time: 2024-07-04 06:32:14.029002Z
time: 2024-07-04 06:32:14.051817Z
failure: samba3.smb2.session.reauth4 [
Exception: ../../source4/torture/smb2/session.c:603: status was NT_STATUS_ACCESS_DENIED, expected NT_STATUS_OK: smb2_setinfo_file failed

But this time it failed to set POSIX ACL(system.posix_acl_access) because the attempt(from smbd) was made as Anonymous user(nobody) even though we operate using a valid open file handle from the owner. That's one of the places where Samba slightly bend POSIX ACL behaviour in the direction of Windows.

CephFS vfs
libcephfs approach survived any of these configuration changes as it seems acting smart enough with open file handles. It includes the situation where fsetxattr(fd, "security.NTACL",...) is allowed for non-root users in a setup with smbd operating without become_root()(only cap_dac_override).

@anoopcs9 anoopcs9 marked this pull request as ready for review July 4, 2024 11:28
For the time being, until the official fix[1] is available with standard
selinux-policy packages, compile and insert a local SELinux policy to
allow smbd to use cap_dac_override in certain situations to cope with
limitations of lossy mapping to POSIX ACLs. Refer comments from [2] for
more details.

[1] fedora-selinux/selinux-policy#1990
[2] samba-in-kubernetes#109

Signed-off-by: Anoop C S <[email protected]>
@anoopcs9
Copy link
Collaborator Author

anoopcs9 commented Jul 5, 2024

I've added an extra commit to temporarily get around the SELinux problems until fixes come in via official packages. So we are left with only the failure from smb2.session.reauth4.

test: samba3.smb2.session.reauth4
time: 2024-07-05 07:25:39.890697Z
time: 2024-07-05 07:25:39.977272Z
failure: samba3.smb2.session.reauth4 [
Exception: ../../source4/torture/smb2/session.c:603: status was NT_STATUS_ACCESS_DENIED, expected NT_STATUS_OK: smb2_setinfo_file failed

Since we do not have variant specific flapping list for each backend I'm not sure whether to add smb2.session.reauth4 blindly to flapping.cephfs as it will affect both CephFS(kclient) and CephFS(vfs). @spuiuk any thoughts?

@anoopcs9 anoopcs9 requested a review from xhernandez July 5, 2024 08:31
anoopcs9 added a commit to anoopcs9/sit-test-cases that referenced this pull request Jul 5, 2024
With recent changes to smb.conf[1] smb2.session.reauth4 is expected to
fail on XFS. Refer [1] for more deails.

[1] samba-in-kubernetes/sit-environment#109

Signed-off-by: Anoop C S <[email protected]>
anoopcs9 added a commit to anoopcs9/sit-test-cases that referenced this pull request Jul 5, 2024
With recent changes to smb.conf[1], smb2.session.reauth4 is expected to
fail on CephFS kclient method. Refer [1] for more details.

Also remember that this is not a problem with CephFS vfs approach but
our clone of selftest mechanism currently lacks the ability to consider
flapping lists based on variants for a particular backend.

[1] samba-in-kubernetes/sit-environment#109

Signed-off-by: Anoop C S <[email protected]>
@spuiuk
Copy link
Collaborator

spuiuk commented Jul 5, 2024

But this time it failed to set POSIX ACL(system.posix_acl_access) because the attempt(from smbd) was made as Anonymous user(nobody) even though we operate using a valid open file handle from the owner. That's one of the places where Samba slightly bend POSIX ACL behaviour in the direction of Windows.

I thought that this specific problem was only caused on the vfs_ceph2 module which used the anonymouns uid/gid to set UserPerms before making the ceph_ll_setxattr(). vfs_ceph shouldn't really hit this problem with anonymous uid/gid since it totally ignores it and uses the uid/gid setup when initialising the ceph call.
Edit: I just realised that this is for the Ceph vfs kernel mount case.

Adding @synarete to this conversation since he seems to be hitting this problem on vfs_ceph_new and didn't have "ignore system acls" set to true in his test setup.

@spuiuk
Copy link
Collaborator

spuiuk commented Jul 5, 2024

Let's cover the flapping list issue in the corresponding PR for sit-test-cases
samba-in-kubernetes/sit-test-cases#87

anoopcs9 added a commit to anoopcs9/sit-environment that referenced this pull request Jul 5, 2024
For the time being, until the official fix[1] is available with standard
selinux-policy packages, compile and insert a local SELinux policy to
allow smbd to use cap_dac_override in certain situations to cope with
limitations of lossy mapping to POSIX ACLs. Refer comments from [2] for
more details.

[1] fedora-selinux/selinux-policy#1990
[2] samba-in-kubernetes#109

Signed-off-by: Anoop C S <[email protected]>
@anoopcs9
Copy link
Collaborator Author

anoopcs9 commented Jul 5, 2024

Adding @synarete to this conversation since he seems to be hitting this problem on vfs_ceph_new and didn't have "ignore system acls" set to true in his test setup.

I gave it a try with the following share configuration..

[share-cephfs-vfs]
comment = Volume 'share' from cephfs(vfs)
vfs objects = acl_xattr ceph_snapshots ceph_new
ceph_new:config_file = /etc/ceph/sit.ceph.conf
ceph_new:user_id = sit
path = /volumes/_nogroup/share/c5fe416c-5e88-4852-8fb9-8d854f70bfe3
browseable = yes
read only = no
acl_xattr:security_acl_name = user.NTACL

..and it turned out to be successful here.

anoopcs9 added a commit to anoopcs9/sit-test-cases that referenced this pull request Jul 5, 2024
With certain samba configurations we might require a separate flapping
list for variants of a backend. One such situation is detailed in [1]
where specific test run results differ among backend variants.

[1] samba-in-kubernetes/sit-environment#109

Signed-off-by: Anoop C S <[email protected]>
anoopcs9 added a commit to anoopcs9/sit-test-cases that referenced this pull request Jul 5, 2024
With certain samba configurations we might require a separate flapping
list for variants of a backend. One such situation is detailed in [1]
where specific test run results differ among variants of a particular
backend.

[1] samba-in-kubernetes/sit-environment#109

Signed-off-by: Anoop C S <[email protected]>
anoopcs9 added a commit to anoopcs9/sit-test-cases that referenced this pull request Jul 5, 2024
With recent changes to smb.conf[1] smb2.session.reauth4 is expected to
fail on XFS. Refer [1] for more deails.

[1] samba-in-kubernetes/sit-environment#109

Signed-off-by: Anoop C S <[email protected]>
anoopcs9 added a commit to anoopcs9/sit-test-cases that referenced this pull request Jul 5, 2024
With recent changes to smb.conf[1], smb2.session.reauth4 is expected to
fail on CephFS kclient method. Refer [1] for more details.

[1] samba-in-kubernetes/sit-environment#109

Signed-off-by: Anoop C S <[email protected]>
@spuiuk
Copy link
Collaborator

spuiuk commented Jul 6, 2024

I am curious about why it fails with ceph kernel mount. The uid/gid at the time of the operation would be set to nobody/nobody but the open fd would have been opened as the user and the effective uid of the process is set to 0. That should have worked in my opinion.

@anoopcs9
Copy link
Collaborator Author

anoopcs9 commented Jul 6, 2024

I am curious about why it fails with ceph kernel mount. The uid/gid at the time of the operation would be set to nobody/nobody but the open fd would have been opened as the user and the effective uid of the process is set to 0. That should have worked in my opinion.

As I explained above #109 (comment) the failure happens when we try to set POSIX ACL(system.posix_acl_access) as Anonymous user(not as root). In effect smbd performs the operation as nobody and the file system lacks write permission for anyone but owner. Thus we face EACCES which is valid from kernel perspective. We are stuck with the exact same situation for XFS and that's why we need a distinction in the flapping list for variants for a backend.

Copy link
Collaborator

@spuiuk spuiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just had a call with Anoop where he described the problem in more detail. Makes sense now.

ACK

spuiuk pushed a commit to samba-in-kubernetes/sit-test-cases that referenced this pull request Jul 8, 2024
With certain samba configurations we might require a separate flapping
list for variants of a backend. One such situation is detailed in [1]
where specific test run results differ among variants of a particular
backend.

[1] samba-in-kubernetes/sit-environment#109

Signed-off-by: Anoop C S <[email protected]>
anoopcs9 added a commit to anoopcs9/sit-test-cases that referenced this pull request Jul 8, 2024
With recent changes to smb.conf[1] smb2.session.reauth4 is expected to
fail on XFS. Refer [1] for more deails.

[1] samba-in-kubernetes/sit-environment#109

Signed-off-by: Anoop C S <[email protected]>
anoopcs9 added a commit to anoopcs9/sit-test-cases that referenced this pull request Jul 8, 2024
With recent changes to smb.conf[1], smb2.session.reauth4 is expected to
fail on CephFS kclient method. Refer [1] for more details.

Also remember that this is not a problem with CephFS vfs approach but
our clone of selftest mechanism currently lacks the ability to consider
flapping lists based on variants for a particular backend.

[1] samba-in-kubernetes/sit-environment#109

Signed-off-by: Anoop C S <[email protected]>
@anoopcs9 anoopcs9 merged commit 1f17aee into samba-in-kubernetes:main Jul 8, 2024
9 checks passed
@anoopcs9 anoopcs9 deleted the acl-xattr-user-ns branch July 8, 2024 15:17
anoopcs9 added a commit to samba-in-kubernetes/sit-test-cases that referenced this pull request Jul 8, 2024
With recent changes to smb.conf[1] smb2.session.reauth4 is expected to
fail on XFS. Refer [1] for more deails.

[1] samba-in-kubernetes/sit-environment#109

Signed-off-by: Anoop C S <[email protected]>
anoopcs9 added a commit to samba-in-kubernetes/sit-test-cases that referenced this pull request Jul 8, 2024
With recent changes to smb.conf[1], smb2.session.reauth4 is expected to
fail on CephFS kclient method. Refer [1] for more details.

Also remember that this is not a problem with CephFS vfs approach but
our clone of selftest mechanism currently lacks the ability to consider
flapping lists based on variants for a particular backend.

[1] samba-in-kubernetes/sit-environment#109

Signed-off-by: Anoop C S <[email protected]>
Shwetha-Acharya pushed a commit to Shwetha-Acharya/sit-test-cases that referenced this pull request Aug 6, 2024
With certain samba configurations we might require a separate flapping
list for variants of a backend. One such situation is detailed in [1]
where specific test run results differ among variants of a particular
backend.

[1] samba-in-kubernetes/sit-environment#109

Signed-off-by: Anoop C S <[email protected]>
Shwetha-Acharya pushed a commit to Shwetha-Acharya/sit-test-cases that referenced this pull request Aug 6, 2024
With recent changes to smb.conf[1] smb2.session.reauth4 is expected to
fail on XFS. Refer [1] for more deails.

[1] samba-in-kubernetes/sit-environment#109

Signed-off-by: Anoop C S <[email protected]>
Shwetha-Acharya pushed a commit to Shwetha-Acharya/sit-test-cases that referenced this pull request Aug 6, 2024
With recent changes to smb.conf[1], smb2.session.reauth4 is expected to
fail on CephFS kclient method. Refer [1] for more details.

Also remember that this is not a problem with CephFS vfs approach but
our clone of selftest mechanism currently lacks the ability to consider
flapping lists based on variants for a particular backend.

[1] samba-in-kubernetes/sit-environment#109

Signed-off-by: Anoop C S <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants