Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1652725 > unrolled thread

[PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-29 23:40 +0200
    Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require  CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-05-30 00:30 +0200
      Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Boris Lukashev <blukashev@sempervictus.com> - 2017-05-30 02:00 +0200
        Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Casey Schaufler <casey@schaufler-ca.com> - 2017-05-30 02:30 +0200
          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-30 04:10 +0200
            Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Casey Schaufler <casey@schaufler-ca.com> - 2017-05-30 04:50 +0200
              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-30 05:20 +0200
                Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-05-30 14:30 +0200
                  Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-30 18:30 +0200
                    Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Daniel Micay <danielmicay@gmail.com> - 2017-05-30 18:50 +0200
                    Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Stephen Smalley <sds@tycho.nsa.gov> - 2017-05-30 20:30 +0200
                      Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Nick Kralevich <nnk@google.com> - 2017-05-30 20:50 +0200
                        Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-30 21:00 +0200
                          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Daniel Micay <danielmicay@gmail.com> - 2017-05-30 22:30 +0200
                            Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-31 01:10 +0200
                              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Daniel Micay <danielmicay@gmail.com> - 2017-05-31 01:50 +0200
                                Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-31 02:00 +0200
                    Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-05-31 01:00 +0200
                      Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-31 01:20 +0200
                        Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-05-31 02:00 +0200
                          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Kees Cook <keescook@chromium.org> - 2017-06-01 04:40 +0200
                            Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-06-01 15:10 +0200
                              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN "Serge E. Hallyn" <serge@hallyn.com> - 2017-06-01 19:20 +0200
                                Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-06-01 23:30 +0200
                              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Kees Cook <keescook@chromium.org> - 2017-06-01 21:00 +0200
                                Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-06-01 23:30 +0200
                                  Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-06-02 16:50 +0200
                                    Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN "Serge E. Hallyn" <serge@hallyn.com> - 2017-06-02 17:40 +0200
                                      Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-06-02 18:10 +0200
                                        Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN "Serge E. Hallyn" <serge@hallyn.com> - 2017-06-02 19:00 +0200
                                          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-06-02 19:40 +0200
                                            Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN "Serge E. Hallyn" <serge@hallyn.com> - 2017-06-02 20:20 +0200
                                              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-06-02 21:30 +0200
                                              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Kees Cook <keescook@chromium.org> - 2017-06-02 21:30 +0200
                                              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-06-02 21:30 +0200
                                        Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-06-02 22:10 +0200
                                          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Nick Kralevich <nnk@google.com> - 2017-06-02 22:20 +0200
                                          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-06-02 22:50 +0200
                                            Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make  TIOCSTI ioctl require CAP_SYS_ADMIN Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-06-04 00:10 +0200
                                              Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-06-04 00:30 +0200
                                                Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Peter Dolding <oiaohm@gmail.com> - 2017-06-04 05:40 +0200
                Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Casey Schaufler <casey@schaufler-ca.com> - 2017-05-30 17:30 +0200
                  Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-30 18:10 +0200
          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Boris Lukashev <blukashev@sempervictus.com> - 2017-06-04 08:30 +0200
        Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN James Morris <jmorris@namei.org> - 2017-05-31 04:50 +0200
          Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI  ioctl require CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-31 06:20 +0200
      Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require  CAP_SYS_ADMIN Matt Brown <matt@nmatt.com> - 2017-05-30 02:20 +0200

Page 1 of 3  [1] 2 3  Next page →

#1652725 — [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

This introduces the tiocsti_restrict sysctl, whose default is controlled
via CONFIG_SECURITY_TIOCSTI_RESTRICT. When activated, this control
restricts all TIOCSTI ioctl calls from non CAP_SYS_ADMIN users.

This patch depends on patch 1/2

This patch was inspired from GRKERNSEC_HARDEN_TTY.

This patch would have prevented
https://bugzilla.redhat.com/show_bug.cgi?id=1411256 under the following
conditions:
* non-privileged container
* container run inside new user namespace

Possible effects on userland:

There could be a few user programs that would be effected by this
change.
See: <https://codesearch.debian.net/search?q=ioctl%5C%28.*TIOCSTI>
notable programs are: agetty, csh, xemacs and tcsh

However, I still believe that this change is worth it given that the
Kconfig defaults to n. This will be a feature that is turned on for the
same reason that people activate it when using grsecurity. Users of this
opt-in feature will realize that they are choosing security over some OS
features like unprivileged TIOCSTI ioctls, as should be clear in the
Kconfig help message.

Threat Model/Patch Rational:

From grsecurity's config for GRKERNSEC_HARDEN_TTY.

 | There are very few legitimate uses for this functionality and it
 | has made vulnerabilities in several 'su'-like programs possible in
 | the past.  Even without these vulnerabilities, it provides an
 | attacker with an easy mechanism to move laterally among other
 | processes within the same user's compromised session.

So if one process within a tty session becomes compromised it can follow
that additional processes, that are thought to be in different security
boundaries, can be compromised as a result. When using a program like su
or sudo, these additional processes could be in a tty session where TTY
file descriptors are indeed shared over privilege boundaries.

This is also an excellent writeup about the issue:
<http://www.halfdog.net/Security/2012/TtyPushbackPrivilegeEscalation/>

When user namespaces are in use, the check for the capability
CAP_SYS_ADMIN is done against the user namespace that originally opened
the tty.

Acked-by: Serge Hallyn <serge@hallyn.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Matt Brown <matt@nmatt.com>
---
 Documentation/sysctl/kernel.txt | 21 +++++++++++++++++++++
 drivers/tty/tty_io.c            |  8 ++++++++
 include/linux/tty.h             |  2 ++
 kernel/sysctl.c                 | 12 ++++++++++++
 security/Kconfig                | 13 +++++++++++++
 5 files changed, 56 insertions(+)

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index bac23c1..f7985cf 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -89,6 +89,7 @@ show up in /proc/sys/kernel:
 - sysctl_writes_strict
 - tainted
 - threads-max
+- tiocsti_restrict
 - unknown_nmi_panic
 - watchdog
 - watchdog_thresh
@@ -987,6 +988,26 @@ available RAM pages threads-max is reduced accordingly.
 
 ==============================================================
 
+tiocsti_restrict:
+
+This toggle indicates whether unprivileged users are prevented
+from using the TIOCSTI ioctl to inject commands into other processes
+which share a tty session.
+
+When tiocsti_restrict is set to (0) there are no restrictions(accept
+the default restriction of only being able to injection commands into
+one's own tty). When tiocsti_restrict is set to (1), users must
+have CAP_SYS_ADMIN to use the TIOCSTI ioctl.
+
+When user namespaces are in use, the check for the capability
+CAP_SYS_ADMIN is done against the user namespace that originally
+opened the tty.
+
+The kernel config option CONFIG_SECURITY_TIOCSTI_RESTRICT sets the
+default value of tiocsti_restrict.
+
+==============================================================
+
 unknown_nmi_panic:
 
 The value in this file affects behavior of handling NMI. When the
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index c276814..0f2733d 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2297,11 +2297,19 @@ static int tty_fasync(int fd, struct file *filp, int on)
  *	FIXME: may race normal receive processing
  */
 
+int tiocsti_restrict = IS_ENABLED(CONFIG_SECURITY_TIOCSTI_RESTRICT);
+
 static int tiocsti(struct tty_struct *tty, char __user *p)
 {
 	char ch, mbz = 0;
 	struct tty_ldisc *ld;
 
+	if (tiocsti_restrict &&
+		!ns_capable(tty->owner_user_ns, CAP_SYS_ADMIN)) {
+		dev_warn_ratelimited(tty->dev,
+			"Denied TIOCSTI ioctl for non-privileged process\n");
+		return -EPERM;
+	}
 	if ((current->signal->tty != tty) && !capable(CAP_SYS_ADMIN))
 		return -EPERM;
 	if (get_user(ch, p))
diff --git a/include/linux/tty.h b/include/linux/tty.h
index d902d42..2fd7f49 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -344,6 +344,8 @@ struct tty_file_private {
 	struct list_head list;
 };
 
+extern int tiocsti_restrict;
+
 /* tty magic number */
 #define TTY_MAGIC		0x5401
 
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index acf0a5a..68d1363 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -67,6 +67,7 @@
 #include <linux/kexec.h>
 #include <linux/bpf.h>
 #include <linux/mount.h>
+#include <linux/tty.h>
 
 #include <linux/uaccess.h>
 #include <asm/processor.h>
@@ -833,6 +834,17 @@ static struct ctl_table kern_table[] = {
 		.extra2		= &two,
 	},
 #endif
+#if defined CONFIG_TTY
+	{
+		.procname	= "tiocsti_restrict",
+		.data		= &tiocsti_restrict,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax_sysadmin,
+		.extra1		= &zero,
+		.extra2		= &one,
+	},
+#endif
 	{
 		.procname	= "ngroups_max",
 		.data		= &ngroups_max,
diff --git a/security/Kconfig b/security/Kconfig
index 823ca1a..665b610 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -18,6 +18,19 @@ config SECURITY_DMESG_RESTRICT
 
 	  If you are unsure how to answer this question, answer N.
 
+config SECURITY_TIOCSTI_RESTRICT
+	bool "Restrict unprivileged use of tiocsti command injection"
+	default n
+	help
+	  This enforces restrictions on unprivileged users injecting commands
+	  into other processes which share a tty session using the TIOCSTI
+	  ioctl. This option makes TIOCSTI use require CAP_SYS_ADMIN.
+
+	  If this option is not selected, no restrictions will be enforced
+	  unless the tiocsti_restrict sysctl is explicitly set to (1).
+
+	  If you are unsure how to answer this question, answer N.
+
 config SECURITY
 	bool "Enable different security models"
 	depends on SYSFS
-- 
2.10.2

[toc] | [next] | [standalone]

#1652732 — Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On Mon, 29 May 2017 17:38:00 -0400
Matt Brown <matt@nmatt.com> wrote:

> This introduces the tiocsti_restrict sysctl, whose default is controlled
> via CONFIG_SECURITY_TIOCSTI_RESTRICT. When activated, this control
> restricts all TIOCSTI ioctl calls from non CAP_SYS_ADMIN users.

Which is really quite pointless as I keep pointing out and you keep
reposting this nonsense.

> 
> This patch depends on patch 1/2
> 
> This patch was inspired from GRKERNSEC_HARDEN_TTY.
> 
> This patch would have prevented
> https://bugzilla.redhat.com/show_bug.cgi?id=1411256 under the following
> conditions:
> * non-privileged container
> * container run inside new user namespace

And assuming no other ioctl could be used in an attack. Only there are
rather a lot of ways an app with access to a tty can cause mischief if
it's the same controlling tty as the higher privileged context that
launched it.

Properly written code allocates a new pty/tty pair for the lower
privileged session. If the code doesn't do that then your change merely
modifies the degree of mayhem it can cause. If it does it right then your
patch is pointless.

> Possible effects on userland:
> 
> There could be a few user programs that would be effected by this
> change.

In other words, it's yet another weird config option that breaks stuff.


NAK v7.

Alan

[toc] | [prev] | [next] | [standalone]

#1652745 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

With all due respect sir, i believe your review falls short of the
purpose of this effort - to harden the kernel against flaws in
userspace. Comments along the line of "if <userspace> does it right
then your patch is pointless" are not relevant to the context of
securing kernel functions/interfaces. What userspace should do has
little bearing on defensive measures actually implemented in the
kernel - if we took the approach of "someone else is responsible for
that" in military operations, the world would be a much darker and
different place today. Those who have the luxury of standoff from the
critical impacts of security vulnerabilities may not take into account
the fact that peoples lives literally depend on Linux getting a lot
more secure, and quickly.
If this work were not valuable, it wouldnt be an enabled kernel option
on a massive number of kernels with attack surfaces reduced by the
compound protections offered by the grsec patch set. I can't speak for
the grsec people, but having read a small fraction of the commentary
around the subject of mainline integration, it seems to me that NAKs
like this are exactly why they had no interest in even trying - this
review is based on the cultural views of the kernel community, not on
the security benefits offered by the work in the current state of
affairs (where userspace is broken). The purpose of each of these
protections (being ported over from grsec) is not to offer carte
blanche defense against all attackers and vectors, but to prevent
specific classes of bugs from reducing the security posture of the
system. By implementing these defenses in a layered manner we can
significantly reduce our kernel attack surface. Once userspace catches
up and does things the right way, and has no capacity for doing them
the wrong way (aka, nothing attackers can use to bypass the proper
userspace behavior), then the functionality really does become
pointless, and can then be removed.
From a practical perspective, can alternative solutions be offered
along with NAKs? Killing things on the vine isnt great, and if a
security measure is being denied, upstream should provide their
solution to how they want to address the problem (or just an outline
to guide the hardened folks).

On Mon, May 29, 2017 at 6:26 PM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
> On Mon, 29 May 2017 17:38:00 -0400
> Matt Brown <matt@nmatt.com> wrote:
>
>> This introduces the tiocsti_restrict sysctl, whose default is controlled
>> via CONFIG_SECURITY_TIOCSTI_RESTRICT. When activated, this control
>> restricts all TIOCSTI ioctl calls from non CAP_SYS_ADMIN users.
>
> Which is really quite pointless as I keep pointing out and you keep
> reposting this nonsense.
>
>>
>> This patch depends on patch 1/2
>>
>> This patch was inspired from GRKERNSEC_HARDEN_TTY.
>>
>> This patch would have prevented
>> https://bugzilla.redhat.com/show_bug.cgi?id=1411256 under the following
>> conditions:
>> * non-privileged container
>> * container run inside new user namespace
>
> And assuming no other ioctl could be used in an attack. Only there are
> rather a lot of ways an app with access to a tty can cause mischief if
> it's the same controlling tty as the higher privileged context that
> launched it.
>
> Properly written code allocates a new pty/tty pair for the lower
> privileged session. If the code doesn't do that then your change merely
> modifies the degree of mayhem it can cause. If it does it right then your
> patch is pointless.
>
>> Possible effects on userland:
>>
>> There could be a few user programs that would be effected by this
>> change.
>
> In other words, it's yet another weird config option that breaks stuff.
>
>
> NAK v7.
>
> Alan



-- 
Boris Lukashev
Systems Architect
Semper Victus

[toc] | [prev] | [next] | [standalone]

#1652748 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 5/29/2017 4:51 PM, Boris Lukashev wrote:
> With all due respect sir, i believe your review falls short of the
> purpose of this effort - to harden the kernel against flaws in
> userspace. Comments along the line of "if <userspace> does it right
> then your patch is pointless" are not relevant to the context of
> securing kernel functions/interfaces. What userspace should do has
> little bearing on defensive measures actually implemented in the
> kernel - if we took the approach of "someone else is responsible for
> that" in military operations, the world would be a much darker and
> different place today. Those who have the luxury of standoff from the
> critical impacts of security vulnerabilities may not take into account
> the fact that peoples lives literally depend on Linux getting a lot
> more secure, and quickly.

You are not going to help anyone with a kernel configuration that
breaks agetty, csh, xemacs and tcsh. The opportunities for using
such a configuration are limited.

> If this work were not valuable, it wouldnt be an enabled kernel option
> on a massive number of kernels with attack surfaces reduced by the
> compound protections offered by the grsec patch set.

I'll bet you a beverage that 99-44/100% of the people who have
this enabled have no clue that it's even there. And if they did,
most of them would turn it off.

> I can't speak for
> the grsec people, but having read a small fraction of the commentary
> around the subject of mainline integration, it seems to me that NAKs
> like this are exactly why they had no interest in even trying - this
> review is based on the cultural views of the kernel community, not on
> the security benefits offered by the work in the current state of
> affairs (where userspace is broken).

A security clamp-down that breaks important stuff is going
to have a tough row to hoe going upstream. Same with a performance
enhancement that breaks things.

> The purpose of each of these
> protections (being ported over from grsec) is not to offer carte
> blanche defense against all attackers and vectors, but to prevent
> specific classes of bugs from reducing the security posture of the
> system. By implementing these defenses in a layered manner we can
> significantly reduce our kernel attack surface.

Sure, but they have to work right. That's an important reason to do
small changes. A change that isn't acceptable can be rejected without
slowing the general progress.

> Once userspace catches
> up and does things the right way, and has no capacity for doing them
> the wrong way (aka, nothing attackers can use to bypass the proper
> userspace behavior), then the functionality really does become
> pointless, and can then be removed.

Well, until someone comes along with yet another spiffy feature
like containers and breaks it again. This is why a really good
solution is required, and the one proposed isn't up to snuff.

> >From a practical perspective, can alternative solutions be offered
> along with NAKs?

They often are, but let's face it, not everyone has the time,
desire and/or expertise to solve every problem that comes up.

> Killing things on the vine isnt great, and if a
> security measure is being denied, upstream should provide their
> solution to how they want to address the problem (or just an outline
> to guide the hardened folks).

The impact of a "security measure" can exceed the value provided.
That is, I understand, the basis of the NAK. We need to be careful
to keep in mind that, until such time as there is substantial interest
in the sort of systemic changes that truly remove this class of issue,
we're going to have to justify the risk/reward trade off when we try
to introduce a change.

>
> On Mon, May 29, 2017 at 6:26 PM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
>> On Mon, 29 May 2017 17:38:00 -0400
>> Matt Brown <matt@nmatt.com> wrote:
>>
>>> This introduces the tiocsti_restrict sysctl, whose default is controlled
>>> via CONFIG_SECURITY_TIOCSTI_RESTRICT. When activated, this control
>>> restricts all TIOCSTI ioctl calls from non CAP_SYS_ADMIN users.
>> Which is really quite pointless as I keep pointing out and you keep
>> reposting this nonsense.
>>
>>> This patch depends on patch 1/2
>>>
>>> This patch was inspired from GRKERNSEC_HARDEN_TTY.
>>>
>>> This patch would have prevented
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1411256 under the following
>>> conditions:
>>> * non-privileged container
>>> * container run inside new user namespace
>> And assuming no other ioctl could be used in an attack. Only there are
>> rather a lot of ways an app with access to a tty can cause mischief if
>> it's the same controlling tty as the higher privileged context that
>> launched it.
>>
>> Properly written code allocates a new pty/tty pair for the lower
>> privileged session. If the code doesn't do that then your change merely
>> modifies the degree of mayhem it can cause. If it does it right then your
>> patch is pointless.
>>
>>> Possible effects on userland:
>>>
>>> There could be a few user programs that would be effected by this
>>> change.
>> In other words, it's yet another weird config option that breaks stuff.
>>
>>
>> NAK v7.
>>
>> Alan
>
>

[toc] | [prev] | [next] | [standalone]

#1652768 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

Casey Schaufler,

First I must start this off by saying I really appreciate your presentation on
LSMs that is up on youtube. I've got a LSM in the works and your talk has
helped me a bunch.

On 5/29/17 8:27 PM, Casey Schaufler wrote:
> On 5/29/2017 4:51 PM, Boris Lukashev wrote:
>> With all due respect sir, i believe your review falls short of the
>> purpose of this effort - to harden the kernel against flaws in
>> userspace. Comments along the line of "if <userspace> does it right
>> then your patch is pointless" are not relevant to the context of
>> securing kernel functions/interfaces. What userspace should do has
>> little bearing on defensive measures actually implemented in the
>> kernel - if we took the approach of "someone else is responsible for
>> that" in military operations, the world would be a much darker and
>> different place today. Those who have the luxury of standoff from the
>> critical impacts of security vulnerabilities may not take into account
>> the fact that peoples lives literally depend on Linux getting a lot
>> more secure, and quickly.
> 
> You are not going to help anyone with a kernel configuration that
> breaks agetty, csh, xemacs and tcsh. The opportunities for using
> such a configuration are limited.

This patch does not break these programs as you imply. 99% of users of these
programs will not be effected. Its not like the TIOCSTI ioctl is a critical
part of these programs.

Also as I've stated elsewhere, this is not breaking userspace because this
Kconfig/sysctl defaults to n. If someone is using the programs listed above in
a way that does utilize an unprivileged call to the TIOCSTI ioctl, they can
turn this feature off.

> 
>> If this work were not valuable, it wouldnt be an enabled kernel option
>> on a massive number of kernels with attack surfaces reduced by the
>> compound protections offered by the grsec patch set.
> 
> I'll bet you a beverage that 99-44/100% of the people who have
> this enabled have no clue that it's even there. And if they did,
> most of them would turn it off.
> 

First, I don't know how to parse "99-44/100%" and therefore do not wish to
wager a beverage on such confusing odds ;)

Second, as stated above, this feature is off by default. However, I would expect
this sysctl to show up in lists of procedures for hardening linux servers.

>> I can't speak for
>> the grsec people, but having read a small fraction of the commentary
>> around the subject of mainline integration, it seems to me that NAKs
>> like this are exactly why they had no interest in even trying - this
>> review is based on the cultural views of the kernel community, not on
>> the security benefits offered by the work in the current state of
>> affairs (where userspace is broken).
> 
> A security clamp-down that breaks important stuff is going
> to have a tough row to hoe going upstream. Same with a performance
> enhancement that breaks things.
> 
>> The purpose of each of these
>> protections (being ported over from grsec) is not to offer carte
>> blanche defense against all attackers and vectors, but to prevent
>> specific classes of bugs from reducing the security posture of the
>> system. By implementing these defenses in a layered manner we can
>> significantly reduce our kernel attack surface.
> 
> Sure, but they have to work right. That's an important reason to do
> small changes. A change that isn't acceptable can be rejected without
> slowing the general progress.
> 
>> Once userspace catches
>> up and does things the right way, and has no capacity for doing them
>> the wrong way (aka, nothing attackers can use to bypass the proper
>> userspace behavior), then the functionality really does become
>> pointless, and can then be removed.
> 
> Well, until someone comes along with yet another spiffy feature
> like containers and breaks it again. This is why a really good
> solution is required, and the one proposed isn't up to snuff.
> 

Can you please state your reasons for why you believe this solution is not "up
to snuff?" So far myself and others have given what I believe to be sound
responses to any objections to this patch.

>> >From a practical perspective, can alternative solutions be offered
>> along with NAKs?
> 
> They often are, but let's face it, not everyone has the time,
> desire and/or expertise to solve every problem that comes up.
> 
>> Killing things on the vine isnt great, and if a
>> security measure is being denied, upstream should provide their
>> solution to how they want to address the problem (or just an outline
>> to guide the hardened folks).
> 
> The impact of a "security measure" can exceed the value provided.
> That is, I understand, the basis of the NAK. We need to be careful
> to keep in mind that, until such time as there is substantial interest
> in the sort of systemic changes that truly remove this class of issue,
> we're going to have to justify the risk/reward trade off when we try
> to introduce a change.
> 
>>
>> On Mon, May 29, 2017 at 6:26 PM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
>>> On Mon, 29 May 2017 17:38:00 -0400
>>> Matt Brown <matt@nmatt.com> wrote:
>>>
>>>> This introduces the tiocsti_restrict sysctl, whose default is controlled
>>>> via CONFIG_SECURITY_TIOCSTI_RESTRICT. When activated, this control
>>>> restricts all TIOCSTI ioctl calls from non CAP_SYS_ADMIN users.
>>> Which is really quite pointless as I keep pointing out and you keep
>>> reposting this nonsense.
>>>
>>>> This patch depends on patch 1/2
>>>>
>>>> This patch was inspired from GRKERNSEC_HARDEN_TTY.
>>>>
>>>> This patch would have prevented
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1411256 under the following
>>>> conditions:
>>>> * non-privileged container
>>>> * container run inside new user namespace
>>> And assuming no other ioctl could be used in an attack. Only there are
>>> rather a lot of ways an app with access to a tty can cause mischief if
>>> it's the same controlling tty as the higher privileged context that
>>> launched it.
>>>
>>> Properly written code allocates a new pty/tty pair for the lower
>>> privileged session. If the code doesn't do that then your change merely
>>> modifies the degree of mayhem it can cause. If it does it right then your
>>> patch is pointless.
>>>
>>>> Possible effects on userland:
>>>>
>>>> There could be a few user programs that would be effected by this
>>>> change.
>>> In other words, it's yet another weird config option that breaks stuff.
>>>
>>>
>>> NAK v7.
>>>
>>> Alan
>>
>>
> 

Thanks,
Matt Brown

[toc] | [prev] | [next] | [standalone]

#1652780 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 5/29/2017 7:00 PM, Matt Brown wrote:
> Casey Schaufler,
>
> First I must start this off by saying I really appreciate your presentation on
> LSMs that is up on youtube. I've got a LSM in the works and your talk has
> helped me a bunch.

Thank you. Feedback (especially positive) is always appreciated.

>
> On 5/29/17 8:27 PM, Casey Schaufler wrote:
>> On 5/29/2017 4:51 PM, Boris Lukashev wrote:
>>> With all due respect sir, i believe your review falls short of the
>>> purpose of this effort - to harden the kernel against flaws in
>>> userspace. Comments along the line of "if <userspace> does it right
>>> then your patch is pointless" are not relevant to the context of
>>> securing kernel functions/interfaces. What userspace should do has
>>> little bearing on defensive measures actually implemented in the
>>> kernel - if we took the approach of "someone else is responsible for
>>> that" in military operations, the world would be a much darker and
>>> different place today. Those who have the luxury of standoff from the
>>> critical impacts of security vulnerabilities may not take into account
>>> the fact that peoples lives literally depend on Linux getting a lot
>>> more secure, and quickly.
>> You are not going to help anyone with a kernel configuration that
>> breaks agetty, csh, xemacs and tcsh. The opportunities for using
>> such a configuration are limited.
> This patch does not break these programs as you imply. 99% of users of these
> programs will not be effected. Its not like the TIOCSTI ioctl is a critical
> part of these programs.

Most likely not.

>
> Also as I've stated elsewhere, this is not breaking userspace because this
> Kconfig/sysctl defaults to n. If someone is using the programs listed above in
> a way that does utilize an unprivileged call to the TIOCSTI ioctl, they can
> turn this feature off.

Default "off" does not mean it doesn't break userspace. It means that it might
not break userspace in your environment. Or it might, depending on the whim of
the build tool of the day.

>
>>> If this work were not valuable, it wouldnt be an enabled kernel option
>>> on a massive number of kernels with attack surfaces reduced by the
>>> compound protections offered by the grsec patch set.
>> I'll bet you a beverage that 99-44/100% of the people who have
>> this enabled have no clue that it's even there. And if they did,
>> most of them would turn it off.
>>
> First, I don't know how to parse "99-44/100%" and therefore do not wish to
> wager a beverage on such confusing odds ;)

99.44%. And I loose a *lot* of beverage bets.

> Second, as stated above, this feature is off by default. However, I would expect
> this sysctl to show up in lists of procedures for hardening linux servers.

It's esoteric enough that I expect that if anyone got bitten by it
word would get out and no one would use it thereafter.

>
>>> I can't speak for
>>> the grsec people, but having read a small fraction of the commentary
>>> around the subject of mainline integration, it seems to me that NAKs
>>> like this are exactly why they had no interest in even trying - this
>>> review is based on the cultural views of the kernel community, not on
>>> the security benefits offered by the work in the current state of
>>> affairs (where userspace is broken).
>> A security clamp-down that breaks important stuff is going
>> to have a tough row to hoe going upstream. Same with a performance
>> enhancement that breaks things.
>>
>>> The purpose of each of these
>>> protections (being ported over from grsec) is not to offer carte
>>> blanche defense against all attackers and vectors, but to prevent
>>> specific classes of bugs from reducing the security posture of the
>>> system. By implementing these defenses in a layered manner we can
>>> significantly reduce our kernel attack surface.
>> Sure, but they have to work right. That's an important reason to do
>> small changes. A change that isn't acceptable can be rejected without
>> slowing the general progress.
>>
>>> Once userspace catches
>>> up and does things the right way, and has no capacity for doing them
>>> the wrong way (aka, nothing attackers can use to bypass the proper
>>> userspace behavior), then the functionality really does become
>>> pointless, and can then be removed.
>> Well, until someone comes along with yet another spiffy feature
>> like containers and breaks it again. This is why a really good
>> solution is required, and the one proposed isn't up to snuff.
>>
> Can you please state your reasons for why you believe this solution is not "up
> to snuff?" So far myself and others have given what I believe to be sound
> responses to any objections to this patch.

If you can't convince Alan, who know ways more about ttys than anyone
ought to, it's not up to snuff.

>
>>> >From a practical perspective, can alternative solutions be offered
>>> along with NAKs?
>> They often are, but let's face it, not everyone has the time,
>> desire and/or expertise to solve every problem that comes up.
>>
>>> Killing things on the vine isnt great, and if a
>>> security measure is being denied, upstream should provide their
>>> solution to how they want to address the problem (or just an outline
>>> to guide the hardened folks).
>> The impact of a "security measure" can exceed the value provided.
>> That is, I understand, the basis of the NAK. We need to be careful
>> to keep in mind that, until such time as there is substantial interest
>> in the sort of systemic changes that truly remove this class of issue,
>> we're going to have to justify the risk/reward trade off when we try
>> to introduce a change.
>>
>>> On Mon, May 29, 2017 at 6:26 PM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
>>>> On Mon, 29 May 2017 17:38:00 -0400
>>>> Matt Brown <matt@nmatt.com> wrote:
>>>>
>>>>> This introduces the tiocsti_restrict sysctl, whose default is controlled
>>>>> via CONFIG_SECURITY_TIOCSTI_RESTRICT. When activated, this control
>>>>> restricts all TIOCSTI ioctl calls from non CAP_SYS_ADMIN users.
>>>> Which is really quite pointless as I keep pointing out and you keep
>>>> reposting this nonsense.
>>>>
>>>>> This patch depends on patch 1/2
>>>>>
>>>>> This patch was inspired from GRKERNSEC_HARDEN_TTY.
>>>>>
>>>>> This patch would have prevented
>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1411256 under the following
>>>>> conditions:
>>>>> * non-privileged container
>>>>> * container run inside new user namespace
>>>> And assuming no other ioctl could be used in an attack. Only there are
>>>> rather a lot of ways an app with access to a tty can cause mischief if
>>>> it's the same controlling tty as the higher privileged context that
>>>> launched it.
>>>>
>>>> Properly written code allocates a new pty/tty pair for the lower
>>>> privileged session. If the code doesn't do that then your change merely
>>>> modifies the degree of mayhem it can cause. If it does it right then your
>>>> patch is pointless.
>>>>
>>>>> Possible effects on userland:
>>>>>
>>>>> There could be a few user programs that would be effected by this
>>>>> change.
>>>> In other words, it's yet another weird config option that breaks stuff.
>>>>
>>>>
>>>> NAK v7.
>>>>
>>>> Alan
>>>
> Thanks,
> Matt Brown
>

[toc] | [prev] | [next] | [standalone]

#1652787 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 5/29/17 10:46 PM, Casey Schaufler wrote:
> On 5/29/2017 7:00 PM, Matt Brown wrote:
>> Casey Schaufler,
>>
>> First I must start this off by saying I really appreciate your presentation on
>> LSMs that is up on youtube. I've got a LSM in the works and your talk has
>> helped me a bunch.
> 
> Thank you. Feedback (especially positive) is always appreciated.
> 
>>
>> On 5/29/17 8:27 PM, Casey Schaufler wrote:
>>> On 5/29/2017 4:51 PM, Boris Lukashev wrote:
>>>> With all due respect sir, i believe your review falls short of the
>>>> purpose of this effort - to harden the kernel against flaws in
>>>> userspace. Comments along the line of "if <userspace> does it right
>>>> then your patch is pointless" are not relevant to the context of
>>>> securing kernel functions/interfaces. What userspace should do has
>>>> little bearing on defensive measures actually implemented in the
>>>> kernel - if we took the approach of "someone else is responsible for
>>>> that" in military operations, the world would be a much darker and
>>>> different place today. Those who have the luxury of standoff from the
>>>> critical impacts of security vulnerabilities may not take into account
>>>> the fact that peoples lives literally depend on Linux getting a lot
>>>> more secure, and quickly.
>>> You are not going to help anyone with a kernel configuration that
>>> breaks agetty, csh, xemacs and tcsh. The opportunities for using
>>> such a configuration are limited.
>> This patch does not break these programs as you imply. 99% of users of these
>> programs will not be effected. Its not like the TIOCSTI ioctl is a critical
>> part of these programs.
> 
> Most likely not.
> 
>>
>> Also as I've stated elsewhere, this is not breaking userspace because this
>> Kconfig/sysctl defaults to n. If someone is using the programs listed above in
>> a way that does utilize an unprivileged call to the TIOCSTI ioctl, they can
>> turn this feature off.
> 
> Default "off" does not mean it doesn't break userspace. It means that it might
> not break userspace in your environment. Or it might, depending on the whim of
> the build tool of the day.
> 

By this logic, it seems like any introduced feature which toggles some security
feature could be seen as "breaking userspace." For example:

1. Let there exist a LSM X that is set to "off" by default.
	(let's say its a simpler type of MAC ;)
2. There exists an inexperienced user Bob that toggles X to "on".
3. Bob complains that X has broken userspace because he now cannot access his
SSH key from firefox.

build tool's will always have important impacts on a system based on the config
that is used.

My understanding of the "don't break userspace" rule has always been to not
change existing, userspace facing, APIs/ioctls/system calls/etc. I don't
believe that my patch does this.

>>
>>>> If this work were not valuable, it wouldnt be an enabled kernel option
>>>> on a massive number of kernels with attack surfaces reduced by the
>>>> compound protections offered by the grsec patch set.
>>> I'll bet you a beverage that 99-44/100% of the people who have
>>> this enabled have no clue that it's even there. And if they did,
>>> most of them would turn it off.
>>>
>> First, I don't know how to parse "99-44/100%" and therefore do not wish to
>> wager a beverage on such confusing odds ;)
> 
> 99.44%. And I loose a *lot* of beverage bets.
> 
>> Second, as stated above, this feature is off by default. However, I would expect
>> this sysctl to show up in lists of procedures for hardening linux servers.
> 
> It's esoteric enough that I expect that if anyone got bitten by it
> word would get out and no one would use it thereafter.
> 

As we know in the security world, esoteric things can have major a impact. I
have not looked thought all of these, but I imagine most of them could have
been prevented by this patch.

https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=tiocsti

>>
>>>> I can't speak for
>>>> the grsec people, but having read a small fraction of the commentary
>>>> around the subject of mainline integration, it seems to me that NAKs
>>>> like this are exactly why they had no interest in even trying - this
>>>> review is based on the cultural views of the kernel community, not on
>>>> the security benefits offered by the work in the current state of
>>>> affairs (where userspace is broken).
>>> A security clamp-down that breaks important stuff is going
>>> to have a tough row to hoe going upstream. Same with a performance
>>> enhancement that breaks things.
>>>
>>>> The purpose of each of these
>>>> protections (being ported over from grsec) is not to offer carte
>>>> blanche defense against all attackers and vectors, but to prevent
>>>> specific classes of bugs from reducing the security posture of the
>>>> system. By implementing these defenses in a layered manner we can
>>>> significantly reduce our kernel attack surface.
>>> Sure, but they have to work right. That's an important reason to do
>>> small changes. A change that isn't acceptable can be rejected without
>>> slowing the general progress.
>>>
>>>> Once userspace catches
>>>> up and does things the right way, and has no capacity for doing them
>>>> the wrong way (aka, nothing attackers can use to bypass the proper
>>>> userspace behavior), then the functionality really does become
>>>> pointless, and can then be removed.
>>> Well, until someone comes along with yet another spiffy feature
>>> like containers and breaks it again. This is why a really good
>>> solution is required, and the one proposed isn't up to snuff.
>>>
>> Can you please state your reasons for why you believe this solution is not "up
>> to snuff?" So far myself and others have given what I believe to be sound
>> responses to any objections to this patch.
> 
> If you can't convince Alan, who know ways more about ttys than anyone
> ought to, it's not up to snuff.
> 
>>
>>>> >From a practical perspective, can alternative solutions be offered
>>>> along with NAKs?
>>> They often are, but let's face it, not everyone has the time,
>>> desire and/or expertise to solve every problem that comes up.
>>>
>>>> Killing things on the vine isnt great, and if a
>>>> security measure is being denied, upstream should provide their
>>>> solution to how they want to address the problem (or just an outline
>>>> to guide the hardened folks).
>>> The impact of a "security measure" can exceed the value provided.
>>> That is, I understand, the basis of the NAK. We need to be careful
>>> to keep in mind that, until such time as there is substantial interest
>>> in the sort of systemic changes that truly remove this class of issue,
>>> we're going to have to justify the risk/reward trade off when we try
>>> to introduce a change.
>>>
>>>> On Mon, May 29, 2017 at 6:26 PM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
>>>>> On Mon, 29 May 2017 17:38:00 -0400
>>>>> Matt Brown <matt@nmatt.com> wrote:
>>>>>
>>>>>> This introduces the tiocsti_restrict sysctl, whose default is controlled
>>>>>> via CONFIG_SECURITY_TIOCSTI_RESTRICT. When activated, this control
>>>>>> restricts all TIOCSTI ioctl calls from non CAP_SYS_ADMIN users.
>>>>> Which is really quite pointless as I keep pointing out and you keep
>>>>> reposting this nonsense.
>>>>>
>>>>>> This patch depends on patch 1/2
>>>>>>
>>>>>> This patch was inspired from GRKERNSEC_HARDEN_TTY.
>>>>>>
>>>>>> This patch would have prevented
>>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1411256 under the following
>>>>>> conditions:
>>>>>> * non-privileged container
>>>>>> * container run inside new user namespace
>>>>> And assuming no other ioctl could be used in an attack. Only there are
>>>>> rather a lot of ways an app with access to a tty can cause mischief if
>>>>> it's the same controlling tty as the higher privileged context that
>>>>> launched it.
>>>>>
>>>>> Properly written code allocates a new pty/tty pair for the lower
>>>>> privileged session. If the code doesn't do that then your change merely
>>>>> modifies the degree of mayhem it can cause. If it does it right then your
>>>>> patch is pointless.
>>>>>
>>>>>> Possible effects on userland:
>>>>>>
>>>>>> There could be a few user programs that would be effected by this
>>>>>> change.
>>>>> In other words, it's yet another weird config option that breaks stuff.
>>>>>
>>>>>
>>>>> NAK v7.
>>>>>
>>>>> Alan
>>>>
>> Thanks,
>> Matt Brown
>>
> 

[toc] | [prev] | [next] | [standalone]

#1653183 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

Look there are two problems here

1. TIOCSTI has users

2. You don't actually fix anything

The underlying problem is that if you give your tty handle to another
process which you don't trust you are screwed. It's fundamental to the
design of the Unix tty model and it's made worse in Linux by the fact
that we use the tty descriptor to access all sorts of other console state
(which makes a ton of sense).

Many years ago a few people got this wrong. All those apps got fixes back
then. They allocate a tty/pty pair and create a new session over that.
The potentially hostile other app only gets to screw itself.

If it was only about TIOCSTI then your patch would still not make sense
because you could use on of the existing LSMs to actually write yourself
some rules about who can and can't use TIOCSTI. For that matter you can
even use the seccomp feature today to do this without touching your
kernel because the ioctl number is a value so you can just block ioctl
with argument 2 of TIOCSTI.

So please explain why we need an obscure kernel config option that normal
users will not understand which protects against nothing and can be
done already ?

Alan

[toc] | [prev] | [next] | [standalone]

#1653360 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 5/30/17 8:24 AM, Alan Cox wrote:
> Look there are two problems here
> 
> 1. TIOCSTI has users

I don't see how this is a problem.

> 
> 2. You don't actually fix anything
> 
> The underlying problem is that if you give your tty handle to another
> process which you don't trust you are screwed. It's fundamental to the
> design of the Unix tty model and it's made worse in Linux by the fact
> that we use the tty descriptor to access all sorts of other console state
> (which makes a ton of sense).
> 
> Many years ago a few people got this wrong. All those apps got fixes back
> then. They allocate a tty/pty pair and create a new session over that.
> The potentially hostile other app only gets to screw itself.
> 

Many years ago? We already got one in 2017, as well as a bunch last year.
See: https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=tiocsti

> If it was only about TIOCSTI then your patch would still not make sense
> because you could use on of the existing LSMs to actually write yourself
> some rules about who can and can't use TIOCSTI. For that matter you can
> even use the seccomp feature today to do this without touching your
> kernel because the ioctl number is a value so you can just block ioctl
> with argument 2 of TIOCSTI.
> 

Seccomp requires the program in question to "opt-in" so to speak and set
certain restrictions on itself. However as you state above, any TIOCSTI
protection doesn't matter if the program correctly allocates a tty/pty pair.
This protections seeks to protect users from programs that don't do things
correctly. Rather than killing bugs, this feature attempts to kill an entire
bug class that shows little sign of slowing down in the world of containers and
sandboxes.

> So please explain why we need an obscure kernel config option that normal
> users will not understand which protects against nothing and can be
> done already ?
> 
> Alan
> 

[toc] | [prev] | [next] | [standalone]

#1653386 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

> Seccomp requires the program in question to "opt-in" so to speak and set
> certain restrictions on itself. However as you state above, any TIOCSTI
> protection doesn't matter if the program correctly allocates a tty/pty pair.
> This protections seeks to protect users from programs that don't do things
> correctly. Rather than killing bugs, this feature attempts to kill an entire
> bug class that shows little sign of slowing down in the world of containers and
> sandboxes.

It's possible to do it in PID1 as root without NO_NEW_PRIVS set, but
there isn't an existing implementation of that. It's not included in
init systems like systemd. There's no way to toggle that off at
runtime one that's done like this sysctl though. If a system
administrator wants to enable it, they'll need to modify a
configuration file and reboot if it was even supported by the init
system. It's the same argument that was used against
perf_event_paranoid=3. Meanwhile, perf_event_paranoid=3 is a mandatory
requirement for every Android device and toggling it at runtime is
*necessary* since that's exposed as a system property writable by the
Android Debug Bridge shell user (i.e. physical access via USB + ADB
enabled within the OS + ADB key of the ADB client accepted). There's
less use case for TIOCSTI so toggling it on at runtime isn't as
important, but a toggle like this is a LOT friendlier than a seccomp
blacklist even if that was supported by common init systems, and it's
not.

[toc] | [prev] | [next] | [standalone]

#1653469 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On Tue, 2017-05-30 at 12:28 -0400, Matt Brown wrote:
> On 5/30/17 8:24 AM, Alan Cox wrote:
> > Look there are two problems here
> > 
> > 1. TIOCSTI has users
> 
> I don't see how this is a problem.
> 
> > 
> > 2. You don't actually fix anything
> > 
> > The underlying problem is that if you give your tty handle to
> > another
> > process which you don't trust you are screwed. It's fundamental to
> > the
> > design of the Unix tty model and it's made worse in Linux by the
> > fact
> > that we use the tty descriptor to access all sorts of other console
> > state
> > (which makes a ton of sense).
> > 
> > Many years ago a few people got this wrong. All those apps got
> > fixes back
> > then. They allocate a tty/pty pair and create a new session over
> > that.
> > The potentially hostile other app only gets to screw itself.
> > 
> 
> Many years ago? We already got one in 2017, as well as a bunch last
> year.
> See: https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=tiocsti
> 
> > If it was only about TIOCSTI then your patch would still not make
> > sense
> > because you could use on of the existing LSMs to actually write
> > yourself
> > some rules about who can and can't use TIOCSTI. For that matter you
> > can
> > even use the seccomp feature today to do this without touching your
> > kernel because the ioctl number is a value so you can just block
> > ioctl
> > with argument 2 of TIOCSTI.
> > 
> 
> Seccomp requires the program in question to "opt-in" so to speak and
> set
> certain restrictions on itself. However as you state above, any
> TIOCSTI
> protection doesn't matter if the program correctly allocates a
> tty/pty pair.
> This protections seeks to protect users from programs that don't do
> things
> correctly. Rather than killing bugs, this feature attempts to kill an
> entire
> bug class that shows little sign of slowing down in the world of
> containers and
> sandboxes.

Just FYI, you can also restrict TIOCSTI (or any other ioctl command)
via SELinux ioctl whitelisting, and Android is using that feature to
restrict TIOCSTI usage in Android O (at least based on the developer
previews to date, also in AOSP master).

> 
> > So please explain why we need an obscure kernel config option that
> > normal
> > users will not understand which protects against nothing and can be
> > done already ?
> > 
> > Alan
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]

#1653485 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On Tue, May 30, 2017 at 11:32 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>> Seccomp requires the program in question to "opt-in" so to speak and
>> set
>> certain restrictions on itself. However as you state above, any
>> TIOCSTI
>> protection doesn't matter if the program correctly allocates a
>> tty/pty pair.
>> This protections seeks to protect users from programs that don't do
>> things
>> correctly. Rather than killing bugs, this feature attempts to kill an
>> entire
>> bug class that shows little sign of slowing down in the world of
>> containers and
>> sandboxes.
>
> Just FYI, you can also restrict TIOCSTI (or any other ioctl command)
> via SELinux ioctl whitelisting, and Android is using that feature to
> restrict TIOCSTI usage in Android O (at least based on the developer
> previews to date, also in AOSP master).

For reference, this is https://android-review.googlesource.com/306278
, where we moved to a whitelist for handling ioctls for ptys.

-- Nick

[toc] | [prev] | [next] | [standalone]

#1653496 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 5/30/17 2:44 PM, Nick Kralevich wrote:
> On Tue, May 30, 2017 at 11:32 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>> Seccomp requires the program in question to "opt-in" so to speak and
>>> set
>>> certain restrictions on itself. However as you state above, any
>>> TIOCSTI
>>> protection doesn't matter if the program correctly allocates a
>>> tty/pty pair.
>>> This protections seeks to protect users from programs that don't do
>>> things
>>> correctly. Rather than killing bugs, this feature attempts to kill an
>>> entire
>>> bug class that shows little sign of slowing down in the world of
>>> containers and
>>> sandboxes.
>>
>> Just FYI, you can also restrict TIOCSTI (or any other ioctl command)
>> via SELinux ioctl whitelisting, and Android is using that feature to
>> restrict TIOCSTI usage in Android O (at least based on the developer
>> previews to date, also in AOSP master).
> 
> For reference, this is https://android-review.googlesource.com/306278
> , where we moved to a whitelist for handling ioctls for ptys.
> 
> -- Nick
> 

Thanks, I didn't know that android was doing this. I still think this feature
is worthwhile for people to be able to harden their systems against this attack
vector without having to implement a MAC.

Matt

[toc] | [prev] | [next] | [standalone]

#1653542 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

> Thanks, I didn't know that android was doing this. I still think this
> feature
> is worthwhile for people to be able to harden their systems against
> this attack
> vector without having to implement a MAC.

Since there's a capable LSM hook for ioctl already, it means it could go
in Yama with ptrace_scope but core kernel code would still need to be
changed to track the owning tty. I think Yama vs. core kernel shouldn't
matter much anymore due to stackable LSMs.

Not the case for perf_event_paranoid=3 where a) there's already a sysctl
exposed which would be unfortunate to duplicate, b) there isn't an LSM
hook yet (AFAIK).

The toggles for ptrace and perf events are more useful though since
they're very commonly used debugging features vs. this obscure, rarely
used ioctl that in practice no one will notice is missing. It's still
friendlier to have a toggle than a seccomp policy requiring a reboot to
get rid of it, or worse compiling it out of the kernel.

[toc] | [prev] | [next] | [standalone]

#1653708 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 5/30/17 4:22 PM, Daniel Micay wrote:
>> Thanks, I didn't know that android was doing this. I still think this
>> feature
>> is worthwhile for people to be able to harden their systems against
>> this attack
>> vector without having to implement a MAC.
> 
> Since there's a capable LSM hook for ioctl already, it means it could go
> in Yama with ptrace_scope but core kernel code would still need to be
> changed to track the owning tty. I think Yama vs. core kernel shouldn't
> matter much anymore due to stackable LSMs.
> 

What does everyone think about a v8 that moves this feature under Yama and uses
the file_ioctl LSM hook?

> Not the case for perf_event_paranoid=3 where a) there's already a sysctl
> exposed which would be unfortunate to duplicate, b) there isn't an LSM
> hook yet (AFAIK).
> 
> The toggles for ptrace and perf events are more useful though since
> they're very commonly used debugging features vs. this obscure, rarely
> used ioctl that in practice no one will notice is missing. It's still
> friendlier to have a toggle than a seccomp policy requiring a reboot to
> get rid of it, or worse compiling it out of the kernel.
> 

[toc] | [prev] | [next] | [standalone]

#1653722 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On Tue, 2017-05-30 at 19:00 -0400, Matt Brown wrote:
> On 5/30/17 4:22 PM, Daniel Micay wrote:
> > > Thanks, I didn't know that android was doing this. I still think
> > > this
> > > feature
> > > is worthwhile for people to be able to harden their systems
> > > against
> > > this attack
> > > vector without having to implement a MAC.
> > 
> > Since there's a capable LSM hook for ioctl already, it means it
> > could go
> > in Yama with ptrace_scope but core kernel code would still need to
> > be
> > changed to track the owning tty. I think Yama vs. core kernel
> > shouldn't
> > matter much anymore due to stackable LSMs.
> > 
> 
> What does everyone think about a v8 that moves this feature under Yama
> and uses
> the file_ioctl LSM hook?

It would only make a difference if it could be fully contained there, as
in not depending on tracking the tty owner.

[toc] | [prev] | [next] | [standalone]

#1653724 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 5/30/17 7:40 PM, Daniel Micay wrote:
> On Tue, 2017-05-30 at 19:00 -0400, Matt Brown wrote:
>> On 5/30/17 4:22 PM, Daniel Micay wrote:
>>>> Thanks, I didn't know that android was doing this. I still think
>>>> this
>>>> feature
>>>> is worthwhile for people to be able to harden their systems
>>>> against
>>>> this attack
>>>> vector without having to implement a MAC.
>>>
>>> Since there's a capable LSM hook for ioctl already, it means it
>>> could go
>>> in Yama with ptrace_scope but core kernel code would still need to
>>> be
>>> changed to track the owning tty. I think Yama vs. core kernel
>>> shouldn't
>>> matter much anymore due to stackable LSMs.
>>>
>>
>> What does everyone think about a v8 that moves this feature under Yama
>> and uses
>> the file_ioctl LSM hook?
> 
> It would only make a difference if it could be fully contained there, as
> in not depending on tracking the tty owner.
> 

For the reasons discussed earlier (to allow for nested containers where one of
the containers is privileged) we want to track the user namespace that owns the tty.

[toc] | [prev] | [next] | [standalone]

#1653703 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On Tue, 30 May 2017 12:28:59 -0400
Matt Brown <matt@nmatt.com> wrote:

> On 5/30/17 8:24 AM, Alan Cox wrote:
> > Look there are two problems here
> > 
> > 1. TIOCSTI has users  
> 
> I don't see how this is a problem.

Which is unfortunate. To start with if it didn't have users we could just
delete it.

> > 
> > 2. You don't actually fix anything
> > 
> > The underlying problem is that if you give your tty handle to another
> > process which you don't trust you are screwed. It's fundamental to the
> > design of the Unix tty model and it's made worse in Linux by the fact
> > that we use the tty descriptor to access all sorts of other console state
> > (which makes a ton of sense).
> > 
> > Many years ago a few people got this wrong. All those apps got fixes back
> > then. They allocate a tty/pty pair and create a new session over that.
> > The potentially hostile other app only gets to screw itself.
> >   
> 
> Many years ago? We already got one in 2017, as well as a bunch last year.
> See: https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=tiocsti

All the apps got fixed at the time. The fact the next generation of
forgot to learn from it is unfortunate but hardly new. Also every single
one of those that exposes a tty in that way allows other annoying
behaviours via other ioctl interfaces so none of them would have been
properly mitigated.

If you really want to do that particular bit of snake oiling then you can
use the existing SELinux, seccomp and related interfaces. They can even
do the job properly by whitelisting or blocking long lists of ioctls.

> This protections seeks to protect users from programs that don't do things
> correctly. Rather than killing bugs, this feature attempts to kill an entire
> bug class that shows little sign of slowing down in the world of containers and
> sandboxes.

Well maybe the people writing them need to learn what they are doing and
stop passing random file descriptors into their container (I've even seen
people handing X file handles into their 'container').

The kernel can do some things to help programmers but it can't stop
people writing crap. Anyone writing code that crosses security boundaries
should have at least a vague idea of what they are doing.

The only way you'd actually really prevent this would be to magically
open a new pty/tty pair and substitute the file handlers plus a data
copying thread when someone created a namespace.

Now you can actually do that with the ptrace functionality in seccomp but
it would still be fairly insane to expect the kernel to handle.

Alan
[Actually even more sensible would be to revert the entire sorry
container mess and use VMs but it's a bit late for that ;-)]

[toc] | [prev] | [next] | [standalone]

#1653712 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 5/30/17 6:51 PM, Alan Cox wrote:
> On Tue, 30 May 2017 12:28:59 -0400
> Matt Brown <matt@nmatt.com> wrote:
> 
>> On 5/30/17 8:24 AM, Alan Cox wrote:
>>> Look there are two problems here
>>>
>>> 1. TIOCSTI has users  
>>
>> I don't see how this is a problem.
> 
> Which is unfortunate. To start with if it didn't have users we could just
> delete it.
> 
>>>
>>> 2. You don't actually fix anything
>>>
>>> The underlying problem is that if you give your tty handle to another
>>> process which you don't trust you are screwed. It's fundamental to the
>>> design of the Unix tty model and it's made worse in Linux by the fact
>>> that we use the tty descriptor to access all sorts of other console state
>>> (which makes a ton of sense).
>>>
>>> Many years ago a few people got this wrong. All those apps got fixes back
>>> then. They allocate a tty/pty pair and create a new session over that.
>>> The potentially hostile other app only gets to screw itself.
>>>   
>>
>> Many years ago? We already got one in 2017, as well as a bunch last year.
>> See: https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=tiocsti
> 
> All the apps got fixed at the time. The fact the next generation of
> forgot to learn from it is unfortunate but hardly new. Also every single
> one of those that exposes a tty in that way allows other annoying
> behaviours via other ioctl interfaces so none of them would have been
> properly mitigated.
> 

This is my point. Apps will continue to shoot themselves in the foot. Of course
the correct response to one of these vulns is to not pass ttys across a
security boundary. We have an opportunity here to reduce the impact of this bug
class at the kernel level. Rejecting this mitigation because the real solution
is to use a tty/pty pair is like saying we should reject ASLR because the real
solution to buffer overflows is proper bounds checking.

> If you really want to do that particular bit of snake oiling then you can
> use the existing SELinux, seccomp and related interfaces. They can even
> do the job properly by whitelisting or blocking long lists of ioctls.
> 
>> This protections seeks to protect users from programs that don't do things
>> correctly. Rather than killing bugs, this feature attempts to kill an entire
>> bug class that shows little sign of slowing down in the world of containers and
>> sandboxes.
> 
> Well maybe the people writing them need to learn what they are doing and
> stop passing random file descriptors into their container (I've even seen
> people handing X file handles into their 'container').
> 
> The kernel can do some things to help programmers but it can't stop
> people writing crap. Anyone writing code that crosses security boundaries
> should have at least a vague idea of what they are doing.
> 
> The only way you'd actually really prevent this would be to magically
> open a new pty/tty pair and substitute the file handlers plus a data
> copying thread when someone created a namespace.
> 
> Now you can actually do that with the ptrace functionality in seccomp but
> it would still be fairly insane to expect the kernel to handle.
> 
> Alan
> [Actually even more sensible would be to revert the entire sorry
> container mess and use VMs but it's a bit late for that ;-)]
> 

Totally agree. VMs >> Containers but the cat is out of the bag and we can't put
it back.

[toc] | [prev] | [next] | [standalone]

#1653723 — Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

> This is my point. Apps will continue to shoot themselves in the foot. Of course
> the correct response to one of these vulns is to not pass ttys across a
> security boundary. We have an opportunity here to reduce the impact of this bug
> class at the kernel level. 

Not really.

If you pass me your console for example I can mmap your framebuffer and
spy on you all day. Or I could reprogram your fonts, your keyboard, your
video mode, or use set and paste selection to write stuff. If you are
using X and you can't get tty handles right you'll no doubt pass me a
copy of your X file descriptor in which case I own your display, your
keyboard and your mouse and I don't need to use TIOCSTI there either.

There are so many different attacks based upon that screwup that the
kernel cannot defend against them. You aren't exactly reducing the impact.

Alan

[toc] | [prev] | [next] | [standalone]

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web