Thanks to Jim Wang of Wesoft for his good eyes... Line 2260 of posix_acl() (function acl_group_override()) reads: ... &&(lp_acl_group_control(SNUM(conn) || ... note the missing ) after SNUM(conn) It should read: &&(lp_acl_group_control(SNUM(conn)) || ... and get rid of a ')' at the end
Well spotted - will be in 3.0.23d - thanks ! Jeremy.
We confirmed with our customer that in fact after making the change users who belonged to the file's primary group can now modify the ACLs. However reading smb.conf documentation about dos filemode = yes, it seems that this is still too restrictive (and the customer says it is now more restrictive than it used to be in 3.0.20b). According to dos filemode documentation: "Enabling this parameter allows a user who has write access to the file (by whatever means) to modify the permissions (including ACL)" We have not tested this change yet ourselves, but this is what we plan to try: ... if (errno EACCESS || errno == EPERM) { if (!CAN_WRITE(conn)) return False; if (lp_acl_group_control(SNUM(conn) && current_user_in_group(prim_gid)) return True; if (lp_dos_filemode(SNUM(conn)) { SMB_STRUCT_STAT st; files_struct *fsp = open_file_fchmod(conn, fname, &st); if (fsp) { close_file_fchmod(fsp); return True; } } return False; } If you think this is bad just close the bug again :) Customer's example: example, the directory dept root@kashyyyk:/export/shared$ getfacl dept # file: dept # owner: cens_owner # group: 11 user::rwx group::rwx #effective:rwx group:cens_it-staff:rwx #effective:rwx mask:rwx other:r-x default:user::rwx default:group::rwx default:group:cens_it-staff:rwx default:mask:rwx default:other:r-x The group cens_it-staff cannot update permissions, even with a group and default group acl set. If we chgrp the directory so that cens_it-staff owns it... root@kashyyyk:/export/shared$ getfacl dept # file: dept # owner: cens_owner # group: cens_it-staff user::rwx group::rwx #effective:--x other:r-x default:user::rwx default:group::rwx default:group:cens_it-staff:rwx default:mask:rwx default:other:r-x Now members of the group cens_it-staff change change acl's from windows. This also appears to be a regression from the previously working samba release. Any thoughts?
Here is how Jim Yang modified the code to conform to our understanding of the "dos filemode" directive. 2257c2257 < static BOOL acl_group_override(connection_struct *conn, gid_t prim_gid,char *fname) --- > static BOOL acl_group_override(connection_struct *conn, gid_t prim_gid) 2259c2259,2261 < if (errno==EACCES || errno == EPERM) --- > if ( (errno == EACCES || errno == EPERM) > && (lp_acl_group_control(SNUM(conn) || lp_dos_filemode(SNUM(conn)))) > && current_user_in_group(prim_gid) ) 2260a2263,2264 > return True; > } 2262,2285c2266 < if (!CAN_WRITE(conn)) < return False; < < /* This make sure that when acl group control = yes, only user in the group can change the ownership */ < if (lp_acl_group_control(SNUM(conn)) && current_user_in_group(prim_gid)) < return True; < < /* When dos filemode = yes, we need to check if the current credential has write permission < if yes, then user can change the group permission < if no, then user does not have right to change permission */ < if (lp_dos_filemode(SNUM(conn))) < { < SMB_STRUCT_STAT st; < files_struct *fsp; < BOOL bacc = False; < < bacc = can_access_file(conn,fname,&st,FILE_WRITE_DATA); < < if (bacc == True) < return bacc; < } < return False; < } < --- > return False; 2482c2463 < if (acl_group_override(conn, prim_gid,fsp->fsp_name)) { --- > if (acl_group_override(conn, prim_gid)) { 2513c2494 < if (acl_group_override(conn, prim_gid,fsp->fsp_name)) { --- > if (acl_group_override(conn, prim_gid)) { 3252c3233 < if (acl_group_override(conn, sbuf.st_gid,fsp->fsp_name)) { --- > if (acl_group_override(conn, sbuf.st_gid)) { 3299c3280 < if (acl_group_override(conn, sbuf.st_gid,fsp->fsp_name)) { --- > if (acl_group_override(conn, sbuf.st_gid)) {
fixed.