[PATCH] VFS: add config options to enable link restrictions

[PATCH] VFS: add config options to enable link restrictions

[Kees Cook]

There are situations where devices running without initrds may need
very early protection from link vulnerabilities, so make these sysctls
configurable at build time, since 561ec64ae67e ("VFS: don't do protected
{sym,hard}links by default") has disabled the protections by default.

Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: stable@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/Kconfig |   39 +++++++++++++++++++++++++++++++++++++++
 fs/namei.c |    6 ++++--
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/fs/Kconfig b/fs/Kconfig
index f95ae3a..46ae2dc 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -276,4 +276,43 @@ endif # NETWORK_FILESYSTEMS
 source "fs/nls/Kconfig"
 source "fs/dlm/Kconfig"
 
+config PROTECTED_SYMLINKS
+	bool "Protect symlink following in sticky world-writable dirs"
+	default n
+	help
+	  Solve the class of ToCToU symlink race vulnerabilities by
+	  permitting symlinks to be followed only when outside a sticky
+	  world-writable directory, or when the uid of the symlink and
+	  follower match, or when the directory and symlink owners match.
+
+	  When PROC_SYSCTL is enabled, this setting can also be controlled
+	  via /proc/sys/kernel/protected_symlinks.
+
+	  See Documentation/sysctl/fs.txt for details.
+
+config PROTECTED_SYMLINKS_SYSCTL
+	int
+	default "1" if PROTECTED_SYMLINKS
+	default "0"
+
+config PROTECTED_HARDLINKS
+	bool "Protect hardlink creation to non-accessible files"
+	default n
+	help
+	  Solve the class of ToCToU hardlink race vulnerabilities by
+	  permitting hardlinks to be created only when to a regular file
+	  that is owned by the user, or is readable and writable by the
+	  user. Also blocks users from "pinning" vulnerable setuid/setgid
+	  programs from being upgraded by the administrator.
+
+	  When PROC_SYSCTL is enabled, this setting can also be controlled
+	  via /proc/sys/kernel/protected_hardlinks.
+
+	  See Documentation/sysctl/fs.txt for details.
+
+config PROTECTED_HARDLINKS_SYSCTL
+	int
+	default "1" if PROTECTED_HARDLINKS
+	default "0"
+
 endmenu
diff --git a/fs/namei.c b/fs/namei.c
index 937f9d5..21854df 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -705,8 +705,10 @@ static inline void put_link(struct nameidata *nd, struct path *link, void *cooki
 	path_put(link);
 }
 
-int sysctl_protected_symlinks __read_mostly = 0;
-int sysctl_protected_hardlinks __read_mostly = 0;
+int sysctl_protected_symlinks __read_mostly =
+	CONFIG_PROTECTED_SYMLINKS_SYSCTL;
+int sysctl_protected_hardlinks __read_mostly =
+	CONFIG_PROTECTED_HARDLINKS_SYSCTL;
 
 /**
  * may_follow_link - Check symlink following for unsafe situations
-- 
1.7.9.5


-- 
Kees Cook
Chrome OS Security

Re: [PATCH] VFS: add config options to enable link restrictions

[Al Viro]

On Fri, Oct 26, 2012 at 11:50:21AM -0700, Kees Cook wrote:

> +config PROTECTED_SYMLINKS_SYSCTL
> +	int
> +	default "1" if PROTECTED_SYMLINKS
> +	default "0"

If anything, use IS_ENABLED instead...

> -int sysctl_protected_symlinks __read_mostly = 0;
> -int sysctl_protected_hardlinks __read_mostly = 0;
> +int sysctl_protected_symlinks __read_mostly =
> +	CONFIG_PROTECTED_SYMLINKS_SYSCTL;
> +int sysctl_protected_hardlinks __read_mostly =
> +	CONFIG_PROTECTED_HARDLINKS_SYSCTL;

Re: [PATCH] VFS: add config options to enable link restrictions

[Linus Torvalds]

On Fri, Oct 26, 2012 at 11:50 AM, Kees Cook <keescook@chromium.org> wrote:
> There are situations where devices running without initrds may need
> very early protection from link vulnerabilities

I really don't see what this argument is all about.

If you don't have initrd, you still have early bootup scripts etc.

If your early bootup has security problems, you have security
problems. It has nothing to do with initrd, or with restricted links,
or anything else.

I also refuse to add these kinds of micro-management config options
and ask any kind of normal person these kinds of "do you want this
random crazy feature". A config option would need to be way more sane,
not this kind of micro-management.

                 Linus

Re: [PATCH] VFS: add config options to enable link restrictions

[Kees Cook]

On Fri, Oct 26, 2012 at 12:21 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Oct 26, 2012 at 11:50 AM, Kees Cook <keescook@chromium.org> wrote:
>> There are situations where devices running without initrds may need
>> very early protection from link vulnerabilities
>
> I really don't see what this argument is all about.
>
> If you don't have initrd, you still have early bootup scripts etc.
>
> If your early bootup has security problems, you have security
> problems. It has nothing to do with initrd, or with restricted links,
> or anything else.

I think there's value in being able to enable these protections at
build-time so there's no need for a distro to have to ship extra
files/lines, spend time setting it, etc. This isn't like other
tunables, IMO.

> I also refuse to add these kinds of micro-management config options
> and ask any kind of normal person these kinds of "do you want this
> random crazy feature". A config option would need to be way more sane,
> not this kind of micro-management.

Would a single config item be acceptable? What would be an agreeable
way to enable this at build-time?

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH] VFS: add config options to enable link restrictions

[Linus Torvalds]

On Fri, Oct 26, 2012 at 12:46 PM, Kees Cook <keescook@chromium.org> wrote:
>
> I think there's value in being able to enable these protections at
> build-time so there's no need for a distro to have to ship extra
> files/lines, spend time setting it, etc.

The "value in not having to change the distro" is valid value. That
said, two lines to rc.local does that too.

But dammit, every single discussion I see, you use some *other*
argument for it, like "people don't have initrd" or whatever crazy
crap. That's what I was objecting to.

> Would a single config item be acceptable? What would be an agreeable
> way to enable this at build-time?

I dunno. Maybe a CONFIG_LOCKDOWN thing that sets a few of our other
default values as well. Or at least hide the config options from
normal users behind some extra security menu, so that people just
don't have to care this deeply. Because most people *don't* care.

I also don't see why it would be a build-time config option at all.
Why not just expose them as option using module_param_named(), and
then you can do it in the kernel command flags (and set it in your
grub.conf etc).

            Linus

Re: [PATCH] VFS: add config options to enable link restrictions

[Kees Cook]

On Fri, Oct 26, 2012 at 1:12 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Oct 26, 2012 at 12:46 PM, Kees Cook <keescook@chromium.org> wrote:
>> Would a single config item be acceptable? What would be an agreeable
>> way to enable this at build-time?
>
> I dunno. Maybe a CONFIG_LOCKDOWN thing that sets a few of our other
> default values as well. Or at least hide the config options from
> normal users behind some extra security menu, so that people just
> don't have to care this deeply. Because most people *don't* care.

My take is that while most people don't care, they also should have it
enabled. It really is only corner-cases that run into problems. All
the other situations are actual security vulnerabilities. So I'd
really like to make this as easy as possible to be "on by default" for
some extremely early definition of "by default".

> I also don't see why it would be a build-time config option at all.
> Why not just expose them as option using module_param_named(), and
> then you can do it in the kernel command flags (and set it in your
> grub.conf etc).

I'd like it to be the exception to turn it _off_, rather than the
exception to turn it on.

I'm happy to start collecting things under CONFIG_LOCKDOWN, but I
worry it'd just turn into the opposite of CONFIG_EXPERIMENTAL where if
there was some value in it that someone didn't want on, they'd leave
it off, etc. Most distros have infrastructure for maintaining their
desired configs, so I didn't think the granularity was very bad as I
sent it.

Every distro will ship with this enabled (except perhaps Damn
Vulnerable Linux), so why make it harder?

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH] VFS: add config options to enable link restrictions

[Linus Torvalds]

On Fri, Oct 26, 2012 at 1:23 PM, Kees Cook <keescook@chromium.org> wrote:
>
> I'd like it to be the exception to turn it _off_, rather than the
> exception to turn it on.

Kees, you don't seem to understand.

Breaking applications is unacceptable. End of story. It's broken them.
Get over it.

Distributions can turn it on with a security app. They already have it
for selinux selection etc. That's where this makes sense. Then
sysadmins can set it once (maybe it can default on there, at least for
new installs), and be done with it.

Your "IT HAS TO BE DONE AT BOOT TIME, THE SKY IS FALLING, NOTHING ELSE
IS ACCEPTABLE!" ranting is a disease. Stop it.

                   Linus

Re: [PATCH] VFS: add config options to enable link restrictions

[Kees Cook]

On Fri, Oct 26, 2012 at 1:27 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Oct 26, 2012 at 1:23 PM, Kees Cook <keescook@chromium.org> wrote:
>>
>> I'd like it to be the exception to turn it _off_, rather than the
>> exception to turn it on.
>
> Kees, you don't seem to understand.
>
> Breaking applications is unacceptable. End of story. It's broken them.
> Get over it.

No, I get that. I've been over it. I can handle it being off by
default. I just want there to be a way to make it enabled at build
time. I'll explore some other options; it's sensible to tie it to
other settings/things that are security-sensitive.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH] VFS: add config options to enable link restrictions

[Boaz Harrosh]

On 10/26/2012 01:23 PM, Kees Cook wrote:
> 
> Every distro will ship with this enabled (except perhaps Damn
> Vulnerable Linux), so why make it harder?
> 

So please remind me why can't it be on by default in code.
And the normal sysctl to turn it off for these who want to
experiment with "filesystem corruption".

So the basic premise is that you must not have any
filesystem corruption at the parts used by boot up until
the init portion that turns "filesystem corruption" on

> -Kees
> 

Cheers
Boaz