Message-ID: <20200201062744.fehlhq3jtetfcxuw@yavin.dot.cyphar.com>
Date:   Sat, 1 Feb 2020 17:27:44 +1100
From:   Aleksa Sarai <cyphar@...har.com>
To:     Ross Zwisler <zwisler@...gle.com>
Cc:     Matthew Wilcox <willy@...radead.org>,
        Ross Zwisler <zwisler@...omium.org>,
        linux-kernel@...r.kernel.org,
        Mattias Nissler <mnissler@...omium.org>,
        Benjamin Gordon <bmgordon@...gle.com>,
        Raul Rangel <rrangel@...gle.com>,
        Micah Morton <mortonm@...gle.com>,
        Dmitry Torokhov <dtor@...gle.com>, Jan Kara <jack@...e.cz>,
        David Howells <dhowells@...hat.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v4] Add a "nosymfollow" mount option.

On 2020-01-31, Ross Zwisler <zwisler@...gle.com> wrote:
> On Fri, Jan 31, 2020 at 12:51:34PM +1100, Aleksa Sarai wrote:
> > On 2020-01-30, Matthew Wilcox <willy@...radead.org> wrote:
> > > On Thu, Jan 30, 2020 at 05:27:50PM -0700, Ross Zwisler wrote:
> > > > For mounts that have the new "nosymfollow" option, don't follow
> > > > symlinks when resolving paths. The new option is similar in spirit to
> > > > the existing "nodev", "noexec", and "nosuid" options. Various BSD
> > > > variants have been supporting the "nosymfollow" mount option for a
> > > > long time with equivalent implementations.
> > > > 
> > > > Note that symlinks may still be created on file systems mounted with
> > > > the "nosymfollow" option present. readlink() remains functional, so
> > > > user space code that is aware of symlinks can still choose to follow
> > > > them explicitly.
> > > > 
> > > > Setting the "nosymfollow" mount option helps prevent privileged
> > > > writers from modifying files unintentionally in case there is an
> > > > unexpected link along the accessed path. The "nosymfollow" option is
> > > > thus useful as a defensive measure for systems that need to deal with
> > > > untrusted file systems in privileged contexts.
> > > 
> > > The openat2 series was just merged yesterday which includes a
> > > LOOKUP_NO_SYMLINKS option.  Is this enough for your needs, or do you
> > > need the mount option?
> > 
> > I have discussed a theoretical "noxdev" mount option (which is
> > effectively LOOKUP_NO_XDEV) with Howells (added to Cc) in the past, and
> > the main argument for having a mount option is that you can apply the
> > protection to older programs without having to rewrite them to use
> > openat2(2).
> 
> Ah, yep, this is exactly what we're trying to achieve with the "nosymfollow"
> mount option: protect existing programs from malicious filesystems without
> having to modify those programs.
> 
> The types of attacks we are concerned about are pretty well summarized in this
> LWN article from over a decade ago:
> 
> https://lwn.net/Articles/250468/
> 
> And searching around (I just Googled "symlink exploit") it's pretty easy to
> find related security blogs and CVEs.
> 
> The noxdev mount option seems interesting, bug I don't fully understand yet
> how it would work.  With the openat2() syscall it's clear which things need to
> be part of the same mount: the dfd (or CWD in the case of AT_FDCWD) and the
> filename you're opening.  How would this work for the noxdev mount option and
> the legacy open(2) syscall, for example?  Would you just always compare
> 'pathname' with the current working directory?  Examine 'pathname' and make
> sure that if any filesystems in that path have 'noxdev' set, you never
> traverse out of them?  Something else?

The idea is that "noxdev" would be "sticky" (or if you prefer, like a
glue trap). As soon as you walk into a mountpoint that has "noxdev", you
cannot cross any subsequent mountpoint boundaries (a-la LOOKUP_NO_XDEV).

> 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.

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. ;)

[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

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists