Re: [PATCH] cifs: Add mount option named backup

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

 



On Sun, Sep 18, 2011 at 7:00 AM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
> On Sat, 17 Sep 2011 14:04:56 -0500
> Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
>
>> On Tue, Aug 30, 2011 at 12:54 PM, Steve French <smfrench@xxxxxxxxx> wrote:
>> > On Tue, Aug 30, 2011 at 12:32 PM, Shirish Pargaonkar
>> > <shirishpargaonkar@xxxxxxxxx> wrote:
>> >> On Tue, Aug 30, 2011 at 12:09 PM, Steve French <smfrench@xxxxxxxxx> wrote:
>> >>> On Tue, Aug 30, 2011 at 9:52 AM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
>> >>>> On Tue, 30 Aug 2011 09:03:05 -0500
>> >>>> Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
>> >>>>
>> >>>>> On Mon, Aug 29, 2011 at 7:29 PM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
>> >>>>> > On Mon, 29 Aug 2011 16:24:33 -0500
>> >>>>> > Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
>> >>>>> >
>> >>>>> >> On Tue, Aug 23, 2011 at 8:15 AM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
>> >>>>> >> > On Mon, 22 Aug 2011 08:33:49 -0500
>> >>>>> >> > Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
>> >>>>> >> >
>> >>>>> >> >> On Fri, Aug 12, 2011 at 11:33 AM,  <shirishpargaonkar@xxxxxxxxx> wrote:
>> >>>>> >> >> > From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>> >>>>> >> >> >
>> >>>>> >> >> >
>> >>>>> >> >> > Add mount option backup.
>> >>>>> >> >> >
>> >>>>> >> >> > It allows an authenticated user to access files with the intent to back them
>> >>>>> >> >> > up including their ACLs, who may not have access permission but has
>> >>>>> >> >> > "Backup files and directories user right" on them (by virtue of being part
>> >>>>> >> >> > of the built-in group Backup Operators.
>> >>>>> >> >> >
>> >>>>> >> >> > If an authenticated user is not part of the built-in group Backup Operators
>> >>>>> >> >> > at the server, access to such files is denied.
>> >>>>> >> >> >
>> >>>>> >> >> >
>> >>>>> >> >> > Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>> >>>>> >> >> > ---
>> >>>>> >> >>
>> >>>>> >> >>
>> >>>>> >> >> Jeff,  Steve,
>> >>>>> >> >>
>> >>>>> >> >> Any comments on this patch (and manpage patch in cifs-utils)?
>> >>>>> >> >>
>> >>>>> >> >
>> >>>>> >> > This seems like a really nasty kludge. It doesn't seem like the
>> >>>>> >> > implications of this have been carefully considered.
>> >>>>> >> >
>> >>>>> >> > What happens I mount with the "backup" flag and do not have the
>> >>>>> >> > necessary permissions on the server to use the flag in an open? Will
>> >>>>> >> > this new flag be mutually exclusive with "multiuser"?
>> >>>>> >> >
>> >>>>> >> > One idea that might be better is to come up with way to mark certain
>> >>>>> >> > (unix) users with the appropriate flag. If all the backup users were in
>> >>>>> >> > a certan group, for instance, then you could use that info to decide
>> >>>>> >> > whether to set the flag in the open calls.
>> >>>>> >> >
>> >>>>> >> > --
>> >>>>> >> > Jeff Layton <jlayton@xxxxxxxxx>
>> >>>>> >> >
>> >>>>> >>
>> >>>>> >> Jeff, one comment, it is not the (unix) user that matters, it is the
>> >>>>> >> user on the server (authenticated) user at the server because the
>> >>>>> >> user right to access a file in backup mode can be granted only
>> >>>>> >> to a user at the server.
>> >>>>> >>
>> >>>>> >
>> >>>>> > Right, I realize that.
>> >>>>> >
>> >>>>> >> I think care should be taken to make sure that backup and
>> >>>>> >> multiuser are mutually exclusive mount options in mount.cifs.
>> >>>>> >>
>> >>>>> >
>> >>>>> > My objection here is more fundamental...
>> >>>>> >
>> >>>>> > This patchset lends itself to a single, specific use. You can create a
>> >>>>> > mount that you want to use for backups. Typically, when running backups
>> >>>>> > like this you also intend for this mount to be used by only one (unix)
>> >>>>> > user.
>> >>>>> >
>> >>>>> > Adding a "backup" option is tantamount to adding extra privileges to
>> >>>>> > this mount for anyone who accesses it. However, it's not clear to me
>> >>>>> > how these extra privileges will be secured from other users that don't
>> >>>>> > necessarily need them.
>> >>>>> >
>> >>>>> > It seems to me that it would be far more useful to find a way to only
>> >>>>> > add these extra "backup" permissions for certain unix users that are
>> >>>>> > accessing the mount.
>> >>>>> >
>> >>>>> > Does that make sense?
>> >>>>> >
>> >>>>> > --
>> >>>>> > Jeff Layton <jlayton@xxxxxxxxx>
>> >>>>> >
>> >>>>>
>> >>>>> Jeff, definitely. Thanks.  I think one way might be to specify an
>> >>>>> user or uid and restrict the usage of that mount to that user id?
>> >>>>> So you can specify mount option as  either backup=userxyz
>> >>>>> or backup=1001 perhaps?
>> >>>>>
>> >>>>
>> >>>> Sure, that might be reasonable.
>> >>>>
>> >>>> Since backup privileges are tied to a group on the server, we could
>> >>>> also mirror that semantic on the client. Have a backup=<gid> and turn
>> >>>> on the backup flag for any unix user who is in that group. That would
>> >>>> seem to allow for better integration with things like nss_winbind.
>> >>>
>> >>> That seems like a good idea - although I prefer that it allow
>> >>> uid and/or gid to be specified for easier usability
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> Thanks,
>> >>>
>> >>> Steve
>> >>>
>> >>
>> >> Steve, how would we be able to distinguish between uids and gids?
>> >> The same id e.g 1001, could be either an uid or a gid!
>> >> The same applies to a name, it could be a group as well as
>> >> an user name.
>> > One option - allow them to distinguished via different mount option:
>> >
>> > presumably "backupuid="    and "backupgid="    If we choose to retain
>> > "backup" as a mount option as well - then when neither backupuid nor
>> > backupgid is specified then the uid of the process mounting would be
>> > used as "backupuid" with no backupgid implied by default.
>> >
>> >
>> >
>> > --
>> > Thanks,
>> >
>> > Steve
>> >
>>
>> Not sure if there is a way to check in kernel whether a user/uid is
>> part of a certain group/gid!
>> If not, I think the options are to do so by perhaps making a upcall
>> or verify whether the specified group/gid is the primary group of
>> the user/uid accessing the file (in the backup intent mode) on
>> the server.
>> My preference is for the second option and we can clarify this in the
>> man page stanza of this mount option (backupgid).
>>
>
> There most certainly is a way to check this in the kernel. It has to do
> this all the time to check permissions. See in_group_p().
>
> You don't want to check whether the current uid is in a particular
> group, but rather whether the current task credential is. Otherwise
> that code won't work if someone runs this under newgrp and the group is
> password protected.
>
> --
> Jeff Layton <jlayton@xxxxxxxxx>
>

So a check should be whether the gid specified with the
mount option backupgid is either same as egid of the task
or one of the gids of the supplementary groups to which
euid of the task belongs to!

Not sure how the acquired egid using newgroup api/command
and password protection comes into the picture in this context.

Regards,

Shirish
--
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