Steve,
I have added a test to the buildbot to verify the fix.
I.e. two ACEs when file are created, one for mode and one for AuthenticatedUsers
and that after chmod we still have two ACEs but the one for mode has
been updated.
The test is cifs/107
and it can also show how we can now modify the mountoptions we need on
a test by test
basis by using -o remount, ... wooohooo new mount api :-)
On Mon, Feb 14, 2022 at 9:47 AM Ronnie Sahlberg <lsahlber@xxxxxxxxxx> wrote:
>
> Steve, List,
>
> Here is a small patch htat fixes an issue with modefromsid where
> it would strip off and remove all the ACEs that grants us access to the file.
> It fixes this by restoring the "allow AuthenticatedUsers access" ACE that is stripped in
>
> set_chmod_dacl():
> /* If it's any one of the ACE we're replacing, skip! */
> if (((compare_sids(&pntace->sid, &sid_unix_NFS_mode) == 0) ||
> (compare_sids(&pntace->sid, pownersid) == 0) ||
> (compare_sids(&pntace->sid, pgrpsid) == 0) ||
> (compare_sids(&pntace->sid, &sid_everyone) == 0) ||
> (compare_sids(&pntace->sid, &sid_authusers) == 0))) {
> goto next_ace;
> }
>
> This part is confusing since form many of these cases we are NOT replacing
> all these ACEs but only some of them but the code unconditionally removes
> all of them, contrary to what the comment suggests.
>
> I think some of my confusion here is that afaik we don't have good documentation
> of how modefromsid, and idsfromsid, are supposed to work, what the
> restrictions are or the expected semantics.
> We need to document both modefromsid and idsfromsid and what the expected
> semantics are for when either of them or both of them are enabled.
>
>
>
>
