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

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

 



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