45Drives / cockpit-file-sharing

A Cockpit plugin to easily manage samba and NFS file sharing.
GNU General Public License v3.0
596 stars 32 forks source link

Cannot add LDAP users to shares #24

Closed sgallagher closed 2 years ago

sgallagher commented 2 years ago

This tool relies on parsing the output of getent passwd in order to get a user list for adding to shares, however this does not work in many cases when the system is a domain member. For example, the default behavior of SSSD is to not populate the full contents of the user list via getpwent() calls, due to the extreme performance impact that loading so many users can cause.

It would be better if this Cockpit application instead (or additionally) provided a text-entry for the usernames that would just validate that getent passwd <username> (or getpwnam('username')) returns a valid entry before accepting it.

baileyallison commented 2 years ago

@sgallagher The valid users/groups option is a straight drop in of the smb parameter "valid users = " option within SMB conf. This is only intended for use with local linux created users, not meant for any Active Directory users.

One limitation of this being either you need a Windows Active Directory, or a Samba Active Directory. I suppose if you're using LDAP through another method without one of those two this isn't feasible as they are a hard requirement for setting up extended/Windows ACLs with Samba, however where you mentioned system is a domain member I am assuming you've either got Samba acting as a Domain Controller with Samba as a Domain Member, or a Windows Active Directory environment with Samba as a Domain Member.

When configuring shares that utilize Windows ACLs/are domain bound it's actually recommended by samba to not utilize any local linux user options when configuring, such as valid users. From the samba documentation on setting up shares that are bound to domains:

"Do not set ANY additional share parameters, such as force user or valid users. Adding them to the share definition can prevent you from configuring or using the share."

Setting_up_a_Share_Using_Windows_ACLs#Adding_a_Share

Instead we typically set these shares and their permissions directly through Windows/Extended ACLs, and configure options on the share to completely ignore the linux/system ACLs if we're bound to an Active Directory/Domain, letting the Windows ACLs handle the permissions for the share entirely, rather than using the base user:group 777 ACLs to allow access.

Setting_Share_Permissions_and_ACL

Setting_ACLs_on_a_Folder

File System Extended ACLs in the Back End

We also typically use winbind when binding a node(s) to an active directory, and we leave the setting to enum users/groups off for the exact reason you mentioned.

If you're to enable that or the respective option for SSSD it should allow the users/groups to show up within the tab, but if you're creating a share to be accessed by clients that are domain bound/using the AD for authentication then assigning them as valid users/groups via samba shouldn't need to be done, nor is it recommended to be done per the samba docs as managing permissions and access through windows/with extended ACLs can allow the same permission control/access control.

Perhaps somebody else can speak on if adding this is a possibility/if it's not currently enabled by mistake, however I believe it is currently set this way due to:

  1. Leaving enum users/groups disabled for due to performance implications.
  2. It not being best practices/recommended when configuring SMB shares using Active Directory users to authenticate/manage permissions via smb conf options such as "valid users =" or modifying local user/group permissions on linux side

Due to this, both of these options for valid users/groups are intended to only ever query users/groups that are local to the system that cockpit-file-sharing is running on.

sgallagher commented 2 years ago

@sgallagher The valid users/groups option is a straight drop in of the smb parameter "valid users = " option within SMB conf. This is only intended for use with local linux created users, not meant for any Active Directory users.

One limitation of this being either you need a Windows Active Directory, or a Samba Active Directory. I suppose if you're using LDAP through another method without one of those two this isn't feasible as they are a hard requirement for setting up extended/Windows ACLs with Samba, however where you mentioned system is a domain member I am assuming you've either got Samba acting as a Domain Controller with Samba as a Domain Member, or a Windows Active Directory environment with Samba as a Domain Member.

I was also thinking of FreeIPA / Red Hat Identity Management domains, but in most practical cases these will probably be in a cross-realm trust with an AD forest, so probably this isn't really a major issue.

When configuring shares that utilize Windows ACLs/are domain bound it's actually recommended by samba to not utilize any local linux user options when configuring, such as valid users. From the samba documentation on setting up shares that are bound to domains:

"Do not set ANY additional share parameters, such as force user or valid users. Adding them to the share definition can prevent you from configuring or using the share."

OK, so maybe a better solution would be to have cockpit-file-sharing detect whether the system is bound to a domain (Cockpit already has a facility for this that uses realmd. If Cockpit is joined to a domain, then the cockpit-file-sharing page should probably just default to having the "Windows ACLs" box checked (or even forced) so that people get the appropriate behavior by default.

hortimech commented 2 years ago

One of the problems here is that the code relies on 'winbind enum users = yes' and 'winbind enum groups = yes' being set in the Samba smb.conf file, both of which are not recommended being set because they can slow things down and Samba works without them being set.

joshuaboud commented 2 years ago

Latest version no longer requires user or group enumeration