Re: [PATCH 1/2] fs: add inode helpers for fsuid and fsgid

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

 



James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> writes:

> On Wed, 2017-02-15 at 15:29 +1300, Eric W. Biederman wrote:
>> James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> writes:
>> 
>> > On Tue, 2017-02-14 at 20:46 +1300, Eric W. Biederman wrote:
>> > > James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> writes:
>> > > 
>> > > > Now that we have two different views of filesystem ids (the 
>> > > > filesystem view and the kernel view), we have a problem in that
>> > > > current_fsuid/fsgid() return the kernel view but are sometimes
>> > > > used 
>> > > > in filesystem code where the filesystem view shoud be used. 
>> > > >  This
>> > > > patch introduces helpers to produce the filesystem view of
>> > > > current 
>> > > > fsuid and fsgid.
>> > > 
>> > > If I am reading this right what we are seeing is that xfs
>> > > explicitly
>> > > opted out of type safety with predictable results.   Accidentally
>> > > confusing kuids and uids, which is potentially security issue.
>> > > 
>> > > All of that said where are you getting sb->s_user_ns !=
>> > > &init_user_ns
>> > > for an xfs filesystem?
>> 
>> James please answer this question:
>> 
>>  Where are you getting sb->s_user_ns != &init_user_ns for an xfs
>> filesystem?
>
> I'm playing with a patch that allows host admin to set up an
> unprivileged container for a guest to use.  One of the extensions is to
> allow anything possessing capability(CAP_SYS_ADMIN) to make s_user_ns
> follow mnt_ns->user_ns for new mounts (as an option).  The idea was to
> see if root could set up an id shifted container with just the current
> s_user_ns infrastructure.
>
>> None of this matters if sb->s_user_ns == &init_user_ns.
>> 
>> This is signification because only xfs keeps any in-core data 
>> structure in it's on-disk encoding.  So this problem is xfs specific.
>>    So understanding how you are getting xfs to have sb->s_user_ns !=
>> &init_user_ns is important for discussing which direction we go with
>> helper functions here.
>> 
>> xfs with sb->s_user_ns == &init_user_ns is perfectly fine and as such 
>> no fixes are needed.
>
> So what you're saying is that unless the unprivileged container could
> mount the filesystem itself (i.e. only those possessing the
> FS_USERNS_MOUNT flag) the filesystems are going to be full of problems
> like this.  I suppose whether it's worthwhile trying to fix them all
> depends on whether the ability of the administrator to set up an id
> shifted container is useful or not.

Yes.  Setting s_user_ns and expecting everything to work with a
review/test cycle of the filesystem to shake out any rough edges is
likely to be problematic.  For historical reasons I actually expect xfs
is especially bad in this regard.  So in practice I would definitely
start a feature like that with another filesystem.

I would be happy to have a FS_S_USER_NS flag to say all that is well,
and the filesystem supports s_user_ns != &init_user_ns.  The bar is much
lower if a trusted user with CAP_SYS_ADMIN is mounting the filesystem
than if an unprivileged user is mounting the filesystem.  As we don't
have to worry about specially crafted malicious filesystem images.

In practice I think I would have passed in the user namespace via a file
descriptor to mount rather than inheriting it from the mount namespace
(more flexibility for roughly the same amount of code).

Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux