Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 3 Feb 2020 15:15:56 -0700
From:   Ross Zwisler <zwisler@google.com>
To:     Aleksa Sarai <cyphar@cyphar.com>
Cc:     Matthew Wilcox <willy@infradead.org>,
        Ross Zwisler <zwisler@chromium.org>,
        linux-kernel@vger.kernel.org,
        Mattias Nissler <mnissler@chromium.org>,
        Benjamin Gordon <bmgordon@google.com>,
        Raul Rangel <rrangel@google.com>,
        Micah Morton <mortonm@google.com>,
        Dmitry Torokhov <dtor@google.com>, Jan Kara <jack@suse.cz>,
        David Howells <dhowells@redhat.com>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v4] Add a "nosymfollow" mount option.
Message-ID: <20200203221556.GA210383@google.com>
References: <20200131002750.257358-1-zwisler@google.com>
 <20200131004558.GA6699@bombadil.infradead.org>
 <20200131015134.5ovxakcavk2x4diz@yavin.dot.cyphar.com>
 <20200131212021.GA108613@google.com>
 <20200201062744.fehlhq3jtetfcxuw@yavin.dot.cyphar.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200201062744.fehlhq3jtetfcxuw@yavin.dot.cyphar.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sat, Feb 01, 2020 at 05:27:44PM +1100, Aleksa Sarai wrote:
> On 2020-01-31, Ross Zwisler <zwisler@google.com> wrote:
<>
> > On Fri, Jan 31, 2020 at 12:51:34PM +1100, Aleksa Sarai wrote:
> > If noxdev would involve a pathname traversal to make sure you don't ever leave
> > mounts with noxdev set, I think this could potentially cover the use cases I'm
> > worried about.  This would restrict symlink traversal to files within the same
> > filesystem, and would restrict traversal to both normal and bind mounts from
> > within the restricted filesystem, correct?
> 
> Yes, but it would have to block all mountpoint crossings including
> bind-mounts, because the obvious way of checking for mountpoint
> crossings (vfsmount comparisons) results in bind-mounts being seen as
> different mounts. This is how LOOKUP_NO_XDEV works. Would this be a
> show-stopped for ChromeOS?
>
> I personally find "noxdev" to be a semantically clearer statement of
> intention ("I don't want any lookup that reaches this mount-point to
> leave") than "nosymfollow" (though to be fair, this is closer in
> semantics to the other "no*" mount flags). But after looking at [1] and
> thinking about it for a bit, I don't really have a problem with either
> solution.

For ChromeOS we want to protect data both on user-provided filesystems (i.e.
USB attached drives and the like) as well as on our "stateful" partition.  

The noxdev mount option would resolve our concerns for user-provided
filesystems, but I don't think that we would be able to use it for stateful
because symlinks on stateful that point elsewhere within stable are still a
security risk.  There is more explanation on why this is the case in [1].
Thank you for linking to that, by the way.

I think our security concerns around both use cases, user-provided filesystems
and the stateful partition, can be resolved in ChromeOS with the nosymfollow
mount flag.  Based on that, my current preference is for the 'nosymfollow'
mount flag.

> The only problem is that "noxdev" would probably need to be settable on
> bind-mounts, and from [2] it looks like the new mount API struggles with
> configuring bind-mounts.
> 
> > > However, the underlying argument for "noxdev" was that you could use it
> > > to constrain something like "tar -xf" inside a mountpoint (which could
> > > -- in principle -- be a bind-mount). I'm not so sure that "nosymfollow"
> > > has similar "obviously useful" applications (though I'd be happy to be
> > > proven wrong).
> > 
> > In ChromeOS we use the LSM referenced in my patch to provide a blanket
> > enforcement that symlinks aren't traversed at all on user-supplied
> > filesystems, which are considered untrusted.  I'd essentially like to build on
> > the protections offered by LOOKUP_NO_SYMLINKS and extend that protection to
> > all accesses to user-supplied filesystems.
> 
> Yeah, after writing my mail I took a look at [1] and I agree that having
> a solution which helps older programs would be helpful. With openat2 and
> libpathrs[3] I'm hoping to lead the charge on a "rewrite userspace"
> effort, but waiting around for that to be complete probably isn't a
> workable solution. ;)

Sounds great.  Here, I'll merge the nosymfollow patch forward with the current
ToT which includes your openat2(2) changes, and we can go from there.

Thanks for all the feedback.

> [1]: https://sites.google.com/a/chromium.org/dev/chromium-os/chromiumos-design-docs/hardening-against-malicious-stateful-data#TOC-Restricting-symlink-traversal
> [2]: https://lwn.net/Articles/809125/
> [3]: https://github.com/openSUSE/libpathrs