[Samba] Re: XFS and inherit permissions bug?

David Disseldorp ddiss at sgi.com
Tue Nov 27 00:03:31 GMT 2007


Hi

On Fri, 09 Nov 2007 15:05:22 +0100
"initiators at free.fr" <initiators at free.fr> wrote:

> Hello
> 
> Here are some more informations.
> 
> General infos on my Samba configuration
> #######################################
> 
> The server is a Debian Etch with distro kernel & Samba package
> (2.6.18-5-686 & 3.0.24-6etch4).
> Users shell is set to /bin/false, they are only accessing this server
> through Samba.
> 
> All files are owned by user root (Administrator) and group
> smb-Administrators (Domain Admins). The basic rights are rwx for root
> and smb-Administrators and nothing for other.
> The inherit permissions parameter is set in smb.conf for Administrator
> user and Domain Admins group to have access to all the files, the
> inherit owner is set to have all files owned by user root, and all
> folders are setgid to have all files owned by group smb-Administrators.
> 
> The users get their access rights using acls and the inherit acls
> parameter is set in smb.conf.
> 
> The windows attributes (archive, hidden and system) are stored in
> extended attributes.

Finally got to the bottom of this one. To sum it up, the setgid bit is lost
by XFS under certain circumstances when performing acl_set_file() as non 
root during inherit_access_acl().

This is different to how EXT3 behaves in this case - setgid remains.

Samba 3.0.24 source/smbd/vfs.c:
    370 int vfs_MkDir(connection_struct *conn, const char *name, mode_t mode)
    371 {
    372     int ret;
    373     SMB_STRUCT_STAT sbuf;
    374
    375     if(!(ret=SMB_VFS_MKDIR(conn, name, mode))) {
    376
    377         inherit_access_acl(conn, name, mode);

After this there is a check whether any high mode bits are lost (setgid):

    384         if(mode & ~(S_IRWXU|S_IRWXG|S_IRWXO) &&
    385                 !SMB_VFS_STAT(conn,name,&sbuf) && (mode & ~sbuf.st_mode))
    386             SMB_VFS_CHMOD(conn,name,sbuf.st_mode | (mode & ~sbuf.st_mode));

Only problem is the SMB_VFS_CHMOD does a chmod_acl() which eventually ends up
calling acl_set_file(), and where back to where we started ;)

Anyhow this patch for 3.0.24 should fix the setgid inheritance problem:

----- start patch -----
Index: samba-3.0.24.vanilla/source/smbd/posix_acls.c
===================================================================
--- samba-3.0.24.vanilla.orig/source/smbd/posix_acls.c  2007-11-02 11:12:05.338179162 +1100
+++ samba-3.0.24.vanilla/source/smbd/posix_acls.c       2007-11-22 17:09:31.351873317 +1100
@@ -3450,7 +3450,12 @@
        if ((ret = chmod_acl_internals(conn, posix_acl, mode)) == -1)
                goto done;

+       /*
+        * high mode bits (SGID) may be lost if acl_set_file is not run as root
+        */
+       become_root();
        ret = SMB_VFS_SYS_ACL_SET_FILE(conn, to, SMB_ACL_TYPE_ACCESS, posix_acl);
+       unbecome_root();

  done:
----- end patch -----

The XFS team are looking into the issue. Thanks again for your bug report.

Cheers, Dave

> 
> 
> Reproducing the problem
> #######################
> 
> In the base dir of one of my shares I have:
> 
> root at samba1:~ # ll /srv/samba/data_inf/
> total 436
> drwxrws---+  7 root smb-Administrators .
> drwxr-xr-x  16 root root               ..
> drwxrws---+ 11 root smb-Administrators ARCHIVES_INF
> drwxrws---+  5 root smb-Administrators BROUILLON_INF
> -rw-rwx---+  1 root smb-Administrators DCI-INF-L-001-F.xls
> drwxrws---+ 10 root smb-Administrators ESPACE_INF
> drwxrws---+  6 root smb-Administrators ESPACE_INF_PUBLIC
> drwxrws---+  2 root smb-Administrators MODELES_INF
> root at samba1:~ # getfacl /srv/samba/data_inf/
> getfacl: Removing leading '/' from absolute path names
> # file: srv/samba/data_inf
> # owner: root
> # group: smb-Administrators
> user::rwx
> group::rwx
> group:smb-Inf:rwx
> group:smb-Bme-Fr:r-x
> mask::rwx
> other::---
> 
> >From a Windows client I create a new dir test1:
> 
> root at samba1:~ # ll /srv/samba/data_inf/
> total 440
> drwxrws---+  8 root smb-Administrators .
> drwxr-xr-x  16 root root               ..
> drwxrws---+ 11 root smb-Administrators ARCHIVES_INF
> drwxrws---+  5 root smb-Administrators BROUILLON_INF
> -rw-rwx---+  1 root smb-Administrators DCI-INF-L-001-F.xls
> drwxrws---+ 10 root smb-Administrators ESPACE_INF
> drwxrws---+  6 root smb-Administrators ESPACE_INF_PUBLIC
> drwxrws---+  2 root smb-Administrators MODELES_INF
> drwxrwx---+  2 root smb-Administrators test1
> root at samba1:~ # getfacl /srv/samba/data_inf/test1/
> getfacl: Removing leading '/' from absolute path names
> # file: srv/samba/data_inf/test1
> # owner: root
> # group: smb-Administrators
> user::rwx
> group::rwx
> group:smb-Inf:rwx
> group:smb-Bme-Fr:r-x
> mask::rwx
> other::---
> 
> The test1 dir is owned by the group smb-Administrators because the . dir
> is setgid, but it is not setgid.
> From a Windows client I create a new dir test2 in dir test1:
> 
> root at samba1:~ # ll /srv/samba/data_inf/test1/
> total 16
> drwxrwx---+ 3 root smb-Administrators   18 2007-11-09 14:37 .
> drwxrws---+ 8 root smb-Administrators 4096 2007-11-09 14:33 ..
> drwxrwx---+ 2 root smb-DomainUsers       6 2007-11-09 14:37 test2
> root at samba1:~ # getfacl /srv/samba/data_inf/test1/test2/
> getfacl: Removing leading '/' from absolute path names
> # file: srv/samba/data_inf/test1/test2
> # owner: root
> # group: smb-DomainUsers
> user::rwx
> group::rwx
> group:smb-Inf:rwx
> group:smb-Bme-Fr:r-x
> mask::rwx
> other::---
> 
> The test2 dir is owned by group smb-DomainUsers (Domain Users) that is
> the primary group of all users.
> 
> 
> my smb.conf
> ###########
> 
> #======================= Global Settings =======================
> 
> [global]
> 
>    netbios name = data
>    workgroup = bme-fr
>    server string = Samba %v
> 
>    smb ports = 139 445
> 
>    domain master = yes
>    preferred master = yes
> 
>    wins support = yes
>    name resolve order = wins bcast hosts
> 
>    time server = yes
> 
>    interfaces = eth1, lo
>    bind interfaces only = yes
> 
> 
> #### Debugging/Accounting ####
> 
> # One logfile per client
>    log file = /var/log/samba/log.%m
> 
> # Minimum logs in syslog, logs go to /var/log/samba/log.{smbd,nmbd}
>    syslog = 0
> 
>    log level = 1
>    max log size = 1000
> 
> # Do something sensible when Samba crashes: mail the admin a backtrace
>    panic action = /usr/share/samba/panic-action %d
> 
> 
> ####### Authentication #######
> 
>    security = user
>    encrypt passwords = true
>    domain logons = yes
>    passdb backend = tdbsam
>    username map = /etc/samba/smbusers
> 
>    guest account = nobody
>    map to guest = bad password
> 
> 
> ########## Printing ##########
> 
>    disable spoolss = yes
> 
> 
> ####### File sharing ########
> 
>    browseable = no
>    read only = yes
>    guest ok = no
> 
>    inherit owner = yes
>    inherit permissions = yes
>    inherit acls = yes
> 
>    map archive = no
>    map hidden = no
>    map system = no
>    store dos attributes = yes
> 
>    unix charset = utf8
>    dos charset = 850
> 
> 
> ############ Misc ############
> 
> # Most people will find that this option gives better performance.
> # See smb.conf(5) and /usr/share/doc/samba-doc/htmldocs/speed.html
> # for details
> # You may want to add the following on a Linux system:
> #         SO_RCVBUF=8192 SO_SNDBUF=8192
> ;   socket options = TCP_NODELAY
>    socket options = TCP_NODELAY SO_RCVBUF=8192 SO_SNDBUF=8192
> 
> 
> #======================= Share Definitions =======================
> 
> [IPC$]
>    comment = IPC$ share
>    path = /var/cache/samba/ipc
> 
> #[homes]
> #   comment = Home Directories
> #   read only = no
> #   path = /srv/samba/homes/%S
> #   valid users = %S
> 
> [netlogon]
>    comment = Network Logon Service
>    browseable = yes
> ;   read only = no
>    path = /srv/samba/netlogon
>    guest ok = yes
>    locking = no
> 
> #[profiles]
> #   comment = Profiles Share
> #   browseable = yes
> #   read only = no
> #   path = /srv/samba/profiles
> #;   profile acls = yes
> 
> [data_old]
> ;   comment =
>    browseable = yes
> ;   read only = no
>    path = /srv/samba/data_old
> 
> [data_inf]
> ;   comment =
>    browseable = yes
>    read only = no
>    path = /srv/samba/data_inf
> 
> [data_qua]
> ;   comment =
>    browseable = yes
>    read only = no
>    path = /srv/samba/data_qua
> 
> [data_prd]
> ;   comment =
>    browseable = yes
>    read only = no
>    path = /srv/samba/data_prd
> 
> [data_comm]
> ;   comment =
>    browseable = yes
>    read only = no
>    path = /srv/samba/data_comm
> 
> [data_rd]
> ;   comment =
>    browseable = yes
>    read only = no
>    path = /srv/samba/data_rd
> 
> [intra]
> ;   comment =
>    browseable = yes
>    read only = no
>    path = /srv/samba/intra
> 
> [data_it]
> ;   comment =
>    browseable = yes
>    read only = no
>    path = /srv/samba/data_it
> 
> [dci_image_bank]
> ;   comment =
>    browseable = yes
>    read only = no
>    path = /srv/samba/dci_image_bank
> 


More information about the samba mailing list