Den tis 2 jan. 2024 kl 12:03 skrev Joe Orton <jor...@redhat.com>:
> On Tue, Dec 26, 2023 at 07:17:51PM +0100, Daniel Sahlberg wrote:
> > Hi,
> >
> > apr_uid_current() can retur the user id and primary group id of a user.
> Is
> > there a way to find out if a user also has secondary groups (something
> > similar to getgrouplist(3)?
> >
> > The Subversion project has some bug reports where a user has R/W access
> to
> > a certain file via a secondary group, but APR doesn't pick up the
> secondary
> > groups and thus we don't think the user has R/W access. I'd like to
> improve
> > this by also considering all secondary groups.
>
> How are you testing for readability/writability here exactly? On Unix
> the right way is using access() but there isn't an APR wrapper for it.
> (Trying to manually check against user/groups is not a reliable way to
> test, not just because of groups but also because of things like setuid
> processes and RBAC systems which may exist on top of the user/groups.)
>
I also thought about RBACs (but I didn't know that was the term) and we are
checking exactly the wrong way:
[[[
SVN_ERR(svn_io_stat(&file_info, path, APR_FINFO_PROT | APR_FINFO_OWNER,
pool));
...
#if defined(APR_HAS_USER) && !defined(WIN32) &&!defined(__OS2__)
apr_err = apr_uid_current(&uid, &gid, pool);
if (apr_err)
return svn_error_wrap_apr(apr_err, _("Error getting UID of process"));
/* Check write bit for current user. */
if (apr_uid_compare(uid, file_info->user) == APR_SUCCESS)
*read_only = !(file_info->protection & APR_UWRITE);
else if (apr_gid_compare(gid, file_info->group) == APR_SUCCESS)
*read_only = !(file_info->protection & APR_GWRITE);
else
*read_only = !(file_info->protection & APR_WWRITE);
#endif
]]]
(And for the eagle eyed reader, the code above is copied from two different
functions, thus file_info is a struct in one case and a pointer-to-struct
in another, but I assume the principle should be clear enough).
The origin of the code is from ~2002 (checking for APR_*EXECUTE), copied to
a new function around 2011 (checking for APR_*WRITE).
My idea was to incrementally improve the code but maybe a better way is to
switch to access() completely. access() seems to be widely available but I
will have to read up on the setuid properties to make sure we don't change
how things has worked in the past.
Background: Subversion has the ability to say a file "needs locking", if a
particular user/working copy doesn't hold the "lock" the file should be
read-only (and inversely if the user holds the lock the file should be
writeable). We check for W access using the code above and then update the
permissions accordingly.
Kind regards,
Daniel
