On Tue, 4 Jan 2011 14:27:45 -0600
Shirish Pargaonkar <[email protected]> wrote:

> On Tue, Jan 4, 2011 at 2:09 PM, Jeff Layton <[email protected]> wrote:
> > On Tue, 4 Jan 2011 13:51:41 -0600
> > Shirish Pargaonkar <[email protected]> wrote:
> >
> >> On Tue, Jan 4, 2011 at 1:31 PM, Jeff Layton <[email protected]> wrote:
> >> > On Tue, 4 Jan 2011 13:20:38 -0600
> >> > Shirish Pargaonkar <[email protected]> wrote:
> >> >
> >> >> On Tue, Jan 4, 2011 at 1:11 PM, Jeff Layton <[email protected]> wrote:
> >> >> > On Mon,  6 Dec 2010 14:56:46 -0600
> >> >> > [email protected] wrote:
> >> >> >
> >> >> >> From: Shirish Pargaonkar <[email protected]>
> >> >> >>
> >> >> >>
> >> >> >> 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 <[email protected]>
> >> >> >> ---
> >> >> >>  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 <[email protected]>
> >> >> >
> >> >>
> >> >> 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?
> >> >>
> >> >
> >> > Yes, that would help some.
> >> >
> >> > That would give you the ability to present permissions that have some
> >> > basis in reality. You'll still have to figure out what to do when you
> >> > get a SID that can't be mapped for some reason.
> >>
> >> If we are using winbind, I do not see how this can happen.
> >> A SID perhaps could not be looked up but winbind, when asked,
> >> can/will always map a Owner SID to an uid and Group SID to a gid.
> >>
> >
> > Ahh ok. Good to know. Still, we need a meaningful owner to set it to
> > when the upcall fails. For instance, if the upcall isn't set up at
> > all...
> >
> 
> I think at least two things can go against, either upcall is not setup at all
> so an upcall fails or we run out of idmap range as specified in smb.conf
> file for either uid or gid or both and so upcall fails.
> 

Yep. The reason for the failure doesn't matter much. What matters is
what you do if it does. You'll need to set the uid or gid to something.

What will you set it to?

-- 
Jeff Layton <[email protected]>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to