[PATCH v4] Add a "nosymfollow" mount option.

[PATCH v4] Add a "nosymfollow" mount option.

[Ross Zwisler]

From: Mattias Nissler <mnissler@chromium.org>

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.

Signed-off-by: Mattias Nissler <mnissler@chromium.org>
Signed-off-by: Ross Zwisler <zwisler@google.com>

---

This was previously posted a few years ago:

v2: https://patchwork.kernel.org/patch/9384153/
v3: https://lore.kernel.org/patchwork/patch/736423/

The problem that this patch solves still exists.  I rebased and retested
this patch against kernel v5.5.

FreeBSD solves this with an equivalent flag:

https://github.com/freebsd/freebsd/blob/master/sys/kern/vfs_lookup.c#L1040
https://www.freebsd.org/cgi/man.cgi?mount(8)

And ChromeOS has been solving this with 200+ lines of LSM code:

https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1334029/

This kernel patch is much shorter (13 lines!) and IMO is a much cleaner
solution.  Let's reconsider getting this merged.

There is some follow-up work that will need to be done:
 - Upstream support for the flag to util-linux.  Example CL that I've
   been testing with:
   https://github.com/rzwisler/util-linux/commit/e3b8e365492e8cc87c750c4946eb013a486978d2
 - Update man pages for mount(8) and mount(2).
 - Update man page for statfs(2).
 - Add this option to the new fsmount(2) syscall:
   https://lwn.net/Articles/802096/

I'm happy to take care of these, but wanted to get feedback on the
kernel patch first.
---
 fs/namei.c                 | 3 +++
 fs/namespace.c             | 2 ++
 fs/proc_namespace.c        | 1 +
 fs/statfs.c                | 2 ++
 include/linux/mount.h      | 3 ++-
 include/linux/statfs.h     | 1 +
 include/uapi/linux/mount.h | 1 +
 7 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/namei.c b/fs/namei.c
index 4fb61e0754ed6..f198a0ea9b1c0 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1059,6 +1059,9 @@ const char *get_link(struct nameidata *nd)
 		touch_atime(&last->link);
 	}
 
+	if (nd->path.mnt->mnt_flags & MNT_NOSYMFOLLOW)
+		return ERR_PTR(-EACCES);
+
 	error = security_inode_follow_link(dentry, inode,
 					   nd->flags & LOOKUP_RCU);
 	if (unlikely(error))
diff --git a/fs/namespace.c b/fs/namespace.c
index 5e1bf611a9eb6..240421e02940d 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3109,6 +3109,8 @@ long do_mount(const char *dev_name, const char __user *dir_name,
 		mnt_flags &= ~(MNT_RELATIME | MNT_NOATIME);
 	if (flags & MS_RDONLY)
 		mnt_flags |= MNT_READONLY;
+	if (flags & MS_NOSYMFOLLOW)
+		mnt_flags |= MNT_NOSYMFOLLOW;
 
 	/* The default atime for remount is preservation */
 	if ((flags & MS_REMOUNT) &&
diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
index 273ee82d8aa97..91a552f617406 100644
--- a/fs/proc_namespace.c
+++ b/fs/proc_namespace.c
@@ -70,6 +70,7 @@ static void show_mnt_opts(struct seq_file *m, struct vfsmount *mnt)
 		{ MNT_NOATIME, ",noatime" },
 		{ MNT_NODIRATIME, ",nodiratime" },
 		{ MNT_RELATIME, ",relatime" },
+		{ MNT_NOSYMFOLLOW, ",nosymfollow" },
 		{ 0, NULL }
 	};
 	const struct proc_fs_info *fs_infop;
diff --git a/fs/statfs.c b/fs/statfs.c
index 2616424012ea7..59f33752c1311 100644
--- a/fs/statfs.c
+++ b/fs/statfs.c
@@ -29,6 +29,8 @@ static int flags_by_mnt(int mnt_flags)
 		flags |= ST_NODIRATIME;
 	if (mnt_flags & MNT_RELATIME)
 		flags |= ST_RELATIME;
+	if (mnt_flags & MNT_NOSYMFOLLOW)
+		flags |= ST_NOSYMFOLLOW;
 	return flags;
 }
 
diff --git a/include/linux/mount.h b/include/linux/mount.h
index bf8cc4108b8f9..ff2d132c21f5d 100644
--- a/include/linux/mount.h
+++ b/include/linux/mount.h
@@ -30,6 +30,7 @@ struct fs_context;
 #define MNT_NODIRATIME	0x10
 #define MNT_RELATIME	0x20
 #define MNT_READONLY	0x40	/* does the user want this to be r/o? */
+#define MNT_NOSYMFOLLOW	0x80
 
 #define MNT_SHRINKABLE	0x100
 #define MNT_WRITE_HOLD	0x200
@@ -46,7 +47,7 @@ struct fs_context;
 #define MNT_SHARED_MASK	(MNT_UNBINDABLE)
 #define MNT_USER_SETTABLE_MASK  (MNT_NOSUID | MNT_NODEV | MNT_NOEXEC \
 				 | MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME \
-				 | MNT_READONLY)
+				 | MNT_READONLY | MNT_NOSYMFOLLOW)
 #define MNT_ATIME_MASK (MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME )
 
 #define MNT_INTERNAL_FLAGS (MNT_SHARED | MNT_WRITE_HOLD | MNT_INTERNAL | \
diff --git a/include/linux/statfs.h b/include/linux/statfs.h
index 9bc69edb8f188..fac4356ea1bfc 100644
--- a/include/linux/statfs.h
+++ b/include/linux/statfs.h
@@ -40,6 +40,7 @@ struct kstatfs {
 #define ST_NOATIME	0x0400	/* do not update access times */
 #define ST_NODIRATIME	0x0800	/* do not update directory access times */
 #define ST_RELATIME	0x1000	/* update atime relative to mtime/ctime */
+#define ST_NOSYMFOLLOW	0x2000	/* do not follow symlinks */
 
 struct dentry;
 extern int vfs_get_fsid(struct dentry *dentry, __kernel_fsid_t *fsid);
diff --git a/include/uapi/linux/mount.h b/include/uapi/linux/mount.h
index 96a0240f23fed..c268ea586dbf4 100644
--- a/include/uapi/linux/mount.h
+++ b/include/uapi/linux/mount.h
@@ -34,6 +34,7 @@
 #define MS_I_VERSION	(1<<23) /* Update inode I_version field */
 #define MS_STRICTATIME	(1<<24) /* Always perform atime updates */
 #define MS_LAZYTIME	(1<<25) /* Update the on-disk [acm]times lazily */
+#define MS_NOSYMFOLLOW	(1<<26) /* Do not follow symlinks */
 
 /* These sb flags are internal to the kernel */
 #define MS_SUBMOUNT     (1<<26)
-- 
2.25.0.341.g760bfbb309-goog

Re: [PATCH v4] Add a "nosymfollow" mount option.

[Matthew Wilcox]

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?

https://lore.kernel.org/linux-fsdevel/20200129142709.GX23230@ZenIV.linux.org.uk/

Re: [PATCH v4] Add a "nosymfollow" mount option.

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 2081 bytes --]

On 2020-01-30, Matthew Wilcox <willy@infradead.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).

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

If FreeBSD also has "nosymfollow", are there many applications where it
is used over O_BENEATH (and how many would be serviced by
LOOKUP_NO_SYMLINKS)?

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4] Add a "nosymfollow" mount option.

[Ross Zwisler]

On Thu, Jan 30, 2020 at 5:46 PM Matthew Wilcox <willy@infradead.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?
>
> https://lore.kernel.org/linux-fsdevel/20200129142709.GX23230@ZenIV.linux.org.uk/

Thank you for the pointer.  No, I don't think that this really meets
our needs because it requires code to be modified to use the new
openat2 system call.  Our goal is to be able to place restrictions on
untrusted user supplied filesystems so that legacy programs will be
protected from malicious symlinks.

Re: [PATCH v4] Add a "nosymfollow" mount option.

[Ross Zwisler]

On Fri, Jan 31, 2020 at 12:51:34PM +1100, Aleksa Sarai wrote:
> On 2020-01-30, Matthew Wilcox <willy@infradead.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?

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?

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

> If FreeBSD also has "nosymfollow", are there many applications where it
> is used over O_BENEATH (and how many would be serviced by
> LOOKUP_NO_SYMLINKS)?

Sorry, I don't have any good info on whether nosymfollow and O_BENEATH are
commonly used together in FreeBSD.

Re: [PATCH v4] Add a "nosymfollow" mount option.

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 5610 bytes --]

On 2020-01-31, Ross Zwisler <zwisler@google.com> wrote:
> On Fri, Jan 31, 2020 at 12:51:34PM +1100, Aleksa Sarai wrote:
> > On 2020-01-30, Matthew Wilcox <willy@infradead.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/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4] Add a "nosymfollow" mount option.

[Ross Zwisler]

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

Re: [PATCH v4] Add a "nosymfollow" mount option.

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 2412 bytes --]

On 2020-02-03, Ross Zwisler <zwisler@google.com> wrote:
> 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.

Fair enough. I can work on and send "noxdev" separately -- I only
brought it up because the attack scenarios (and connection to openat2)
are both fairly similar.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]