On Tue, Jan 4, 2011 at 1:11 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > On Mon, 6 Dec 2010 14:56:46 -0600 > shirishpargaonkar@xxxxxxxxx wrote: > >> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> >> >> >> If a DACL has entries for ACEs for SID Everyone and Authenticated Users, >> factor in mask in respective entries during calculation of permissions >> for all three, user, group, and other. >> >> http://technet.microsoft.com/en-us/library/bb463216.aspx >> >> >> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> >> --- >> fs/cifs/cifsacl.c | 13 +++++++++++-- >> 1 files changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c >> index c6ebea0..a520091 100644 >> --- a/fs/cifs/cifsacl.c >> +++ b/fs/cifs/cifsacl.c >> @@ -43,9 +43,12 @@ static struct cifs_wksid wksidarr[NUM_WK_SIDS] = { >> ; >> >> >> -/* security id for everyone */ >> +/* security id for everyone/world system group */ >> static const struct cifs_sid sid_everyone = { >> 1, 1, {0, 0, 0, 0, 0, 1}, {0} }; >> +/* security id for Authenticated Users system group */ >> +static const struct cifs_sid sid_authusers = { >> + 1, 1, {0, 0, 0, 0, 0, 5}, {11} }; >> /* group users */ >> static const struct cifs_sid sid_user = {1, 2 , {0, 0, 0, 0, 0, 5}, {} }; >> >> @@ -367,7 +370,7 @@ static void parse_dacl(struct cifs_acl *pdacl, char *end_of_acl, >> if (num_aces > 0) { >> umode_t user_mask = S_IRWXU; >> umode_t group_mask = S_IRWXG; >> - umode_t other_mask = S_IRWXO; >> + umode_t other_mask = S_IRWXU | S_IRWXG | S_IRWXO; >> >> ppace = kmalloc(num_aces * sizeof(struct cifs_ace *), >> GFP_KERNEL); >> @@ -392,6 +395,12 @@ static void parse_dacl(struct cifs_acl *pdacl, char *end_of_acl, >> ppace[i]->type, >> &fattr->cf_mode, >> &other_mask); >> + if (compare_sids(&(ppace[i]->sid), &sid_authusers)) >> + access_flags_to_mode(ppace[i]->access_req, >> + ppace[i]->type, >> + &fattr->cf_mode, >> + &other_mask); >> + >> >> /* memcpy((void *)(&(cifscred->aces[i])), >> (void *)ppace[i], > > This looks like a harmless change... > > Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx> > Jeff, Thanks. > I have real doubts however about the owner/group parts of the cifsacl > scheme. It sets the user and group mode bits without setting the user > and group owners to meaningful values. So, those permissions have no > basis in reality. With the sid to uid and gid mapping code using winbind in the works, would that address these concerns? > > That's especially scary since cifs uses these for permissions > enforcement by default, so that's a box of chocolates (no telling what > you'll get). > -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
