Re: [PATCH] cifs: send real uid of initiating process to upcall instead of mount uid

On Mon, 3 Aug 2009 15:45:51 -0500
Steve French <[email protected]> wrote:

> On Mon, Aug 3, 2009 at 3:20 PM, Jeff Layton <[email protected]> wrote:
> 
> > On Mon, 3 Aug 2009 14:40:19 -0500
> > Steve French <[email protected]> wrote:
> >
> > > On Mon, Aug 3, 2009 at 1:52 PM, Jeff Layton <[email protected]> wrote:
> > > > be able to trick the kernel into using a credcache to which the user
> > > doesn't
> > > > have access by setting the right uid= option on a mount?
> > > ...
> > > >  Seems saner and more secure to me.
> > >
> > > Are you saying "saner" to rename "uid=" for the purposes of the upcall...
> > > not
> > > just the change to pass the "uid of the process that initiated the
> > upcall"
> > > (not
> > > the "owner of the files" uid) (If so, I agree)
> > >
> > >
> >
> > I'm not sure I follow what you're saying...
> >
> > I'm saying that we should pass the real uid of the process that
> > initiated the upcall to the upcall. There's no point in passing the uid
> > of the process that owns the files on the mount to the upcall.
> >
> 
> Yes - I was agreeing on this :)
> 

Ok good. Though I mis-wrote that above. That should have read "There's
no point in passing the uid that owns the files on the mount..."

...but you get the idea.

> 
> >
> > I don't see the point of renaming the uid= "key" here. Can you
> > elaborate as to why you think it would be good to do so?
> >
> 
> I thought that you had also noted that using "uid" to mean more than
> one thing on mount was confusing, and thus could be confusing
> to also keep the same name in cifs.upcall.  If cifs.upcall does
> not care about the "default file owner" but does care about
> the process that initiated the upcall - seems like it would make
> sense to call it something distinct to eliminate any
> possible confusion.
> 

Ahh ok. The problem there is that would break older cifs.upcall
programs.

cifs.upcall currently looks for an option called "uid=". I'm just
proposing that we change how we populate that option. If we call it
something new, then older cifs.upcall programs won't know about the
change and wouldn't setuid to a user before trying to access the
credcache.

The catch here is that this change might be problematic for people that
expect that setting uid= to a value will point cifs.upcall at that
uid's credcache. If we're concerned about that, it might be better to
make this change more gradually.

Thoughts?

-- 
Jeff Layton <[email protected]>