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