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-dc] 4.18 no longer requires CAP_SYS_ADMIN with new security.NTACL option #118

Open
scara opened this issue Aug 7, 2023 · 5 comments

Comments

@scara
Copy link

scara commented Aug 7, 2023

Hello Everyone,
new to Samba as DC in Container, just looking at your solution.

Based on https://gitlab.com/samba-team/samba/-/merge_requests/2557 and acl_xattr:security_acl_name i.e. Samba 4.18.0, is SYS_ADMIN still required in:

AFAIK probably no, if we assign user.NTACL to acl_xattr:security_acl_name.

TIA,
Matteo

@instantlinux
Copy link
Owner

The 4.18 samba docs give this description:

This option allows to redefine the default location for the NTACL extended attribute (xattr). If not set, NTACL xattrs are written to security.NTACL which is a protected location, which means the content of the security.NTACL attribute is not accessible from normal users outside of Samba. When this option is set to use a user-defined value, e.g. user.NTACL then any user can potentially access and overwrite this information. The module prevents access to this xattr over SMB, but the xattr may still be accessed by other means (eg local access, SSH, NFS). This option must only be used when this consequence is clearly understood and when specific precautions are taken to avoid compromising the ACL content.

However I don't want to make this change without a better description of the change. The items in bold need further clarification.

@scara
Copy link
Author

scara commented Aug 22, 2023

Hi @instantlinux,

The items in bold need further clarification.

I'm not a Samba expert but I think that the "previous" statement provides what requested:

When this option is set to use a user-defined value, e.g. user.NTACL then any user can potentially access and overwrite this information. The module prevents access to this xattr over SMB, but the xattr may still be accessed by other means (eg local access, SSH, NFS)

Using a kind-of privileged container is not a great approach from a security perspective; maybe the same as giving the possibility to view/alter those ACLs locally when accessing the container: if we better secure the hosting environment (i.e. less privileges) we could IMHO say that a local access could be more under control.

HTH,
Matteo

@instantlinux
Copy link
Owner

It's not clear to me what that means: "any user can potentially access and overwrite" but I want to spell out in the README what that vulnerability means for users with "local access, ssh or NFS". I understand your argument, that I should make this change right away: but if it's just a matter of setting acl_xattr:security_acl_name = user.NTACL in smb.conf, that can already be done with the published container. So when I add this as the default value, I want to understand this myself (I don't) and properly explain it to others who use this image.

@instantlinux
Copy link
Owner

I'm leaving this open until an authoritative source of information explains this statement in the release update for 4.18 describing security.NTACL: "This option must only be used when this consequence is clearly understood and when specific precautions are taken to avoid compromising the ACL content."

Users of this image on Docker Hub won't be able to "clearly understand" what those precautions need to be until I add this information to this repo, at least in the README file if not also in a warning message emitted at startup time. So the default has to stay the same for now.

@instantlinux instantlinux changed the title [samba-dc] CAP_SYS_ADMIN could be no more required [samba-dc] 4.18 no longer requires CAP_SYS_ADMIN with new security.NTACL option Sep 11, 2023
@instantlinux
Copy link
Owner

Please check the updated README in PR #133, which has this text:

Version 4.18 introduced a security.NTACL feature intended to allow samba-dc to run within a container without the CAP_SYS_ADMIN permission. See the section New option to change the NT ACL default location in the features added/changed documentation. The helm chart defined here can be locally modified to support this but it's left as an exercise for advanced users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants