| Message ID | 20200131002750.257358-1-zwisler@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] Add a "nosymfollow" mount option. | expand |
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/
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)?
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.
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.
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
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
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.
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)