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.