Re: [PATCH] cifs: Use mask of ACEs for SID Everyone to calculate all three permissions user, group, and other

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 4, 2011 at 4:10 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> On Tue, 4 Jan 2011 14:27:45 -0600
> Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
>
>> On Tue, Jan 4, 2011 at 2:09 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>> > On Tue, 4 Jan 2011 13:51:41 -0600
>> > Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
>> >
>> >> On Tue, Jan 4, 2011 at 1:31 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>> >> > On Tue, 4 Jan 2011 13:20:38 -0600
>> >> > Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
>> >> >
>> >> >> 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?
>> >> >>
>> >> >
>> >> > 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?
>

I think it should default to 0 (superuser).

> --
> Jeff Layton <jlayton@xxxxxxxxxx>
>
--
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


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux