All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Kernel Mailing List <linux-kernel@vger.kernel.org>,
	fuse-devel <fuse-devel@lists.sourceforge.net>,
	lxc-devel@lists.linuxcontainers.org,
	Serge Hallyn <serge.hallyn@ubuntu.com>,
	"Michael H. Warfield" <mhw@wittsend.com>
Subject: Re: [PATCH 3/3] fuse: Allow mounts from user namespaces
Date: Mon, 21 Jul 2014 11:02:53 -0700	[thread overview]
Message-ID: <87r41er3n6.fsf@x220.int.ebiederm.org> (raw)
In-Reply-To: <20140721143442.GD111224@ubuntu-hedt> (Seth Forshee's message of "Mon, 21 Jul 2014 09:34:42 -0500")

Seth Forshee <seth.forshee@canonical.com> writes:

> On Mon, Jul 21, 2014 at 03:09:14PM +0200, Miklos Szeredi wrote:
>> On Mon, Jul 21, 2014 at 2:47 PM, Seth Forshee
>> <seth.forshee@canonical.com> wrote:
>> > On Fri, Jul 18, 2014 at 05:33:23PM +0200, Miklos Szeredi wrote:
>> >> On Mon, Jul 14, 2014 at 9:18 PM, Seth Forshee
>> >> <seth.forshee@canonical.com> wrote:
>> >> > Update fuse to allow mounts from user namespaces. During mount
>> >> > current_user_ns() is stashed away,
>> >>
>> >> Same thing here.  While practically this may work, it's theoretically
>> >> wrong, and possibly may go wrong in special situations.   In fuse
>> >> there's no official "server process", so storing information, like
>> >> namespace, about one is going to be wrong.
>> >
>> > What you're suggesting would probably work fine when dealing with pids.
>> > It's not going to work though for the checks I've added in
>> > fuse_allow_current_process() that the process is in the mount owner's
>> > user ns, and without those checks or something similar I don't think
>> > it's safe to permit allow_other for user ns mounts.
>> 
>> You can add that check in fuse_dev_do_read() as well.  If the
>> fsuid/fsgid doesn't exist in the "server's" namespace, then set
>> req->out.h.error and call request_end().
>
> Okay, that seems like it should work.
>
>> > Can you elaborate on what special situations might violate these
>> > assumptions or otherwise cause problems?
>> 
>> What's preventing a fuse fs implementation from handling FUSE_INIT in
>> one process and then handling the rest in a different process
>> (possibly in a different namespace)?
>
> Nothing, but I'm having a hard time imagining why that would ever be
> useful. The user/group ids passed in the mount options would have to be
> mapped into that namespace, otherwise all requests will just fail in the
> check you suggest above. The only thing I can think of would be if
> someone wanted to proxy mounts trough a process in a more privileged
> context, but then the main point of these patches is to make that
> unnecessary.
>
> But I also think your approach should work just as well as mine for the
> use cases that do make sense to me, so I'll go ahead and give it a
> try.

A few observations that I don't think I have seen come up in this
thread.

In my earlier experiments with mounting filesystems in other user
namespaces it did wind up making sense to have a notion of this is
the user namespace that things are represented in on disk, and
that wound up covering odd corner cases like acls.  For fuse
I don't recall if any of those corner cases exists.

At the same time my conversion experience also showed that performing
the conversion to/from kuid and kgids as close to the user space
interface as close as possible was the lease error prone and most
secure way of handling things.

For the file descriptors used for talking to a fuse server I would be
inclined to capture a user and pid namespace at open time to use for
your conversions.   This avoids using current and getting into the
problem of file descriptors that change behavior when passed from
process to process.

I definitely agree that using kuids kgids, and struct pid through the
fuse filesystem internally is the least error prone way to go.

Eric

  reply	other threads:[~2014-07-21 18:06 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-14 19:18 [PATCH 0/3] fuse: Allow mounts in containers Seth Forshee
2014-07-14 19:18 ` [PATCH 1/3] fuse/dev: Fix unbalanced calls to kunmap_atomic() during splice I/O Seth Forshee
2014-07-18 15:21   ` Miklos Szeredi
2014-07-21 12:18     ` Seth Forshee
2014-07-14 19:18 ` [PATCH 2/3] fuse: Translate pid making a request into the server's pid namespace Seth Forshee
2014-07-18 15:29   ` Miklos Szeredi
2014-07-14 19:18 ` [PATCH 3/3] fuse: Allow mounts from user namespaces Seth Forshee
2014-07-18 15:33   ` Miklos Szeredi
2014-07-21 12:47     ` Seth Forshee
2014-07-21 13:09       ` Miklos Szeredi
2014-07-21 14:34         ` Seth Forshee
2014-07-21 18:02           ` Eric W. Biederman [this message]
2014-07-22  3:30             ` Seth Forshee
2014-07-25 19:46               ` Seth Forshee
2014-07-26 16:27                 ` Miklos Szeredi
2014-08-15 13:15                   ` Seth Forshee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87r41er3n6.fsf@x220.int.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=fuse-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lxc-devel@lists.linuxcontainers.org \
    --cc=mhw@wittsend.com \
    --cc=miklos@szeredi.hu \
    --cc=serge.hallyn@ubuntu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.