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

[Christof Schmitt]

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?

> > 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