linux-kernel - Re: [PATCH RESEND v2 16/18] fuse: Support fuse filesystems outside of init_user

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 25 Mar 2016 15:31:48 -0500
From:	Seth Forshee <seth.forshee@...onical.com>
To:	Miklos Szeredi <miklos@...redi.hu>
Cc:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Serge Hallyn <serge.hallyn@...onical.com>,
	Richard Weinberger <richard.weinberger@...il.com>,
	Austin S Hemmelgarn <ahferroin7@...il.com>,
	Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-bcache@...r.kernel.org, dm-devel@...hat.com,
	linux-raid@...r.kernel.org, linux-mtd@...ts.infradead.org,
	Linux-Fsdevel <linux-fsdevel@...r.kernel.org>,
	fuse-devel <fuse-devel@...ts.sourceforge.net>,
	LSM <linux-security-module@...r.kernel.org>,
	selinux@...ho.nsa.gov
Subject: Re: [PATCH RESEND v2 16/18] fuse: Support fuse filesystems outside
 of init_user_ns

On Mon, Mar 14, 2016 at 09:58:43PM +0100, Miklos Szeredi wrote:
> On Wed, Mar 9, 2016 at 6:07 PM, Seth Forshee <seth.forshee@...onical.com> wrote:
> > On Wed, Mar 09, 2016 at 04:51:42PM +0100, Miklos Szeredi wrote:
> >> On Wed, Mar 9, 2016 at 4:25 PM, Seth Forshee <seth.forshee@...onical.com> wrote:
> >> > On Wed, Mar 09, 2016 at 03:48:22PM +0100, Miklos Szeredi wrote:
> >>
> >> >> Can't we use current_cred()->uid/gid? Or fsuid/fsgid maybe?
> >> >
> >> > That would be a departure from the current behavior in the !allow_other
> >> > case for unprivileged users. Since those mounts are done by an suid
> >> > helper all of those ids would be root in the userns, wouldn't they?
> >>
> >> Well, actually this is what the helper does:
> >>
> >>     sprintf(d, "fd=%i,rootmode=%o,user_id=%u,group_id=%u",
> >>         fd, rootmode, getuid(), getgid());
> >
> > Sorry, I was thinking of euid. So this may not be a problem.
> >
> >> So it just uses the current uid/gid.  Apparently no reason to do this
> >> in userland, we could just as well set these in the kernel.  Except
> >> for possible backward compatibility problems for things not using the
> >> helper.
> >>
> >> BUT if the mount is unprivileged or it's a userns mount, or anything
> >> previously not possible, then we are not constrained by the backward
> >> compatibility issues, and can go with the saner solution.
> >>
> >> Does that not make sense?
> >
> > But we generally do want backwards compatibility, and we want userspace
> > software to be able to expect the same behavior whether or not it's
> > running in a user namespaced container. Obviously we can't always have
> > things 100% identical, but we shouldn't break things unless we really
> > need to.
> >
> > However it may be that this isn't actually going to break assumptions of
> > existing software like I had feared. My preference is still to not
> > change any userspace-visible behaviors since we never know what software
> > might have made assumptions based on those behaviors. But if you're
> > confident that it won't break anything I'm willing to give it a try.
> 
> I'm quite confident it won't make a difference.

I was just about to go make these changes and discovered that the
user_id and group_id options are already mandatory, due to this check at
the bottom of parse_fuse_opt():

        if (!d->fd_present || !d->rootmode_present ||
            !d->user_id_present || !d->group_id_present)
                return 0;

So I'll simply drop those two lines which supply default values for
these options.

Thanks,
Seth