become_root() Necessary for Group Quota in quotas.c?

[Orion]

On 8/12/24 17:41, Christof Schmitt via samba-technical wrote:
> On Fri, Aug 09, 2024 at 03:00:48PM -0600, Orion via samba-technical wrote:
>> Here is my proposed patch of the file source3/smbd/quotas.c to fix the issue
>> I outlined previously:
>>
>> @@ -458,10 +458,8 @@ try_group_quota:
>>              S_ISDIR(fname->st.st_ex_mode) &&
>>              fname->st.st_ex_mode & S_ISGID) {
>>                  id.gid = fname->st.st_ex_gid;
>> -               become_root();
>>                  r = SMB_VFS_GET_QUOTA(conn, fname, SMB_GROUP_QUOTA_TYPE, id,
>>                                        &D);
>> -               unbecome_root();
>>          } else {
>>                  id.gid = getegid();
>>                  r = SMB_VFS_GET_QUOTA(conn, fname, SMB_GROUP_QUOTA_TYPE, id,
>>
>> Any comments would be appreciated.
>>
>> On 8/1/24 17:06, Orion wrote:
>>> We have a situation in which the root user does not have access to the
>>> files contained in a directory with SGID set. When a user navigates to
>>> this directory and Samba attempts to request the quota for the
>>> directory, it fails when the code later attempts to do a stat on the
>>> file because of lack of permissions.
> This sounds confusing. Why would the root not have access to the
> directory?
The server is making the request over NFS and root squashing is enabled 
on the machine which causes the permission issue.
>>> In the file samba/source3/smbd/quotas.c if the SGID bit is set, the code
>>> changes the effective GID to the GID of the folder, uses become_root()
>>> to change to root, and then requests the quota. However, if the SGID bit
>>> is not set, quotas.c requests the quota without using the become_root()
>>> function. The question I have is, why when the SGID bit is set does it
>>> change to root before requesting the quota?
>>>
>>> Can we remove the become_root() function call for this instance and
>>> request the quota as the GID of the folder?
>>>
>>> I tested removing the become_root() and unbecome_root() function calls,
>>> and for our particular setup, it works perfectly. However, I am curious
>>> if there is some other setup that may have issues if this bit is
>>> removed.
> Orion,
>
> while i worked on this a while ago, i do not recall the details of this
> change. Looking at the overall picture this could result in a scenario
> of one user querying quota for a completely different group, not the
> user's primary or secondary group.  Is it guaranteed that any user can
> always query any group quota? If so, your change is valid. If not, we
> could look into first issuing the GET_QUOTA call without root
> privileges, and repeat it when permission is denied (EPERM?) as root.
>
> Regards,
>
> Christof
>
Thanks for the response!

I do not believe that it is necessary for the quota to be requested as 
root because this will allow any user to get the quota of any group 
whether or not they have permission to do so. For example, Linux 
quota-tools does not allow users to request quota for groups that they 
do not have access to. Instead it returns a permission denied error. 
FreeBSD (and presumably the other BSDs) also follow this pattern:

"Non-super-users can use the -g flag and optional group argument to view 
only the limits of groups of which they are members." - 
man.freebsd.org/cgi/man.cgi?query=quota

Since the systems should not make a call for a quota outside of which 
they have permissions, I believe we can move forward with just removing 
the become_root() and unbecome_root() parts.