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

[Matt Brown]

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

# Changes since v4:
* fixed typo

# Changes since v3:
* use get_user_ns and put_user_ns to take and drop references to the owner
user namespace because CONFIG_USER_NS is an option

# Changes since v2:
* take/drop reference to user namespace on tty struct alloc/free to prevent
use-after-free.

# Changes since v1:
* added owner_user_ns to tty_struct to enable capability checks against
the namespace that created the tty.
* rewording in different places to make patchset purpose clear
* Added Documentation

[Matt Brown]

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

Signed-off-by: Matt Brown <ma...@nmatt.com>
---
Documentation/sysctl/kernel.txt | 21 +++++++++++++++++++++
drivers/tty/tty_io.c | 6 ++++++
include/linux/tty.h | 2 ++
kernel/sysctl.c | 12 ++++++++++++
security/Kconfig | 13 +++++++++++++
5 files changed, 54 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..fe68d14 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2297,11 +2297,17 @@ 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)) {
+ pr_warn_ratelimited("TIOCSTI ioctl call blocked 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 3ff1bf9..7d13331 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

[Matt Brown]

This patch adds struct user_namespace *owner_user_ns to the tty_struct.
Then it is set to current_user_ns() in the alloc_tty_struct function.

This is done to facilitate capability checks against the original user
namespace that allocated the tty.

E.g. ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)

This combined with the use of user namespace's will allow hardening
protections to be built to mitigate container escapes that utilize TTY
ioctls such as TIOCSTI.

See: https://bugzilla.redhat.com/show_bug.cgi?id=1411256

Signed-off-by: Matt Brown <ma...@nmatt.com>
---

drivers/tty/tty_io.c | 2 ++
include/linux/tty.h | 2 ++
2 files changed, 4 insertions(+)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e6d1a65..c276814 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -171,6 +171,7 @@ static void free_tty_struct(struct tty_struct *tty)
put_device(tty->dev);
kfree(tty->write_buf);
tty->magic = 0xDEADDEAD;
+ put_user_ns(tty->owner_user_ns);
kfree(tty);
}

@@ -3191,6 +3192,7 @@ struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx)
tty->index = idx;
tty_line_name(driver, idx, tty->name);
tty->dev = tty_get_device(tty);
+ tty->owner_user_ns = get_user_ns(current_user_ns());

return tty;
}
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1017e904..d902d42 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -12,6 +12,7 @@
#include <uapi/linux/tty.h>
#include <linux/rwsem.h>
#include <linux/llist.h>
+#include <linux/user_namespace.h>


/*
@@ -333,6 +334,7 @@ struct tty_struct {
/* If the tty has a pending do_SAK, queue it here - akpm */
struct work_struct SAK_work;
struct tty_port *port;
+ struct user_namespace *owner_user_ns;
};

/* Each of a tty's open files has private_data pointing to tty_file_private */
--
2.10.2

[Alan Cox]

> 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

Only in this case they are not.

If I am at the point I have the ability to send you TIOCSTI you already
lost because I can just open /dev/tty to get access to my controlling tty
and use write().

Alan

[Jann Horn]

In terms of PTYs, this patch does not try to prevent writes to a slave
device (which afaik is what /dev/tty will give you). It tries to prevent the
equivalent of writes to the master device. As far as I know, there is no
way to go from a slave to the corresponding master without having
access to the master in some other way already.

[One Thousand Gnomes]

Ok so the point I was trying to make about write and read is I can
already trash your channel when you su. Probably less irritatingly.

In the pty case yes I can go from the tty to pty trivially and then open
it, however the owner of the pty side would normally have exclusivity so
while it's a potential hole it isn't a trivial one.

If I want to do the equvalent of the TIOCSTI attack then I fork a process
and exit the parent. The child can now use ptrace to reprogram your shell
to do whatever interesting things it likes (eg running child processes
called "su" via a second pty/tty pair). Not exactly rocket science.

The tty layer does not try to manage this because it can't and two
processes with the same uid are not protected from one another in the
traditional Unix model. As with anything else when you try and glue
namespaces on top of a model not designed for it you get a pile of holes.

There is no safe way to fix it if you can't trust the environment you are
communicating through. Secure practice is either to make another
connection or if local to switch console and use SAK then login.

In the namespaces case it certainly makes sense to forbid a process in
one namespace from typing characters into another namespace but to me
that implies that tty sessions/job control are namespaced, and that makes
transitioning namespace or even just typing stuff into a docker container
shell rather more tricky to get right if you have to be in the right tty
session _and_ namespace to use the tty.

Alan

[Jann Horn]

Really? By "pty", are you referring to the master? If so, as far as I know,
to go from the slave to the master, you need one of:

- ptrace access to a process that already has an FD to the master, via
ptrace() or so (/proc/$pid/fd/$fd won't work)
- for a BSD PTY (which AFAIK isn't used much anymore), access to
/dev/ptyXX

Am I overlooking something?

> If I want to do the equvalent of the TIOCSTI attack then I fork a process
> and exit the parent. The child can now use ptrace to reprogram your shell
> to do whatever interesting things it likes (eg running child processes
> called "su" via a second pty/tty pair). Not exactly rocket science.

Why would the child be able to ptrace the shell? AFAICS, in the most
relevant scenarios, the child can't ptrace the shell because the
shell has a different UID (in the case of e.g. su or sudo). In other
scenarios, Yama, if enabled, should still stop you from doing that.
And even with containers that have the same UID as the calling user,
which I think is not exactly a good approach from a security perspective,
the PID namespace should stop you from ptracing things outside the
container.

> The tty layer does not try to manage this because it can't and two
> processes with the same uid are not protected from one another in the
> traditional Unix model. As with anything else when you try and glue
> namespaces on top of a model not designed for it you get a pile of holes.
>
> There is no safe way to fix it if you can't trust the environment you are
> communicating through. Secure practice is either to make another
> connection or if local to switch console and use SAK then login.

This is somewhat off-topic, but SAK has several security issues, one of
which is documented in the source code. I would be surprised if anyone
actually relied on that. Comment above __do_SAK():

/*
* This implements the "Secure Attention Key" --- the idea is to
* prevent trojan horses by killing all processes associated with this
* tty when the user hits the "Secure Attention Key". Required for
* super-paranoid applications --- see the Orange Book for more details.
* [...]
* Now, if it would be correct ;-/ The current code has a nasty hole -
* it doesn't catch files in flight. We may send the descriptor to ourselves
* via AF_UNIX socket, close it and later fetch from socket. FIXME.
* [...]
*/

[One Thousand Gnomes]

> Really? By "pty", are you referring to the master? If so, as far as I know,
> to go from the slave to the master, you need one of:
>
> - ptrace access to a process that already has an FD to the master, via
> ptrace() or so (/proc/$pid/fd/$fd won't work)
> - for a BSD PTY (which AFAIK isn't used much anymore), access to
> /dev/ptyXX

fstat() and then open *assuming* I have permissions.

>
> Am I overlooking something?
>
> > If I want to do the equvalent of the TIOCSTI attack then I fork a process
> > and exit the parent. The child can now use ptrace to reprogram your shell
> > to do whatever interesting things it likes (eg running child processes
> > called "su" via a second pty/tty pair). Not exactly rocket science.
>
> Why would the child be able to ptrace the shell? AFAICS, in the most
> relevant scenarios, the child can't ptrace the shell because the
> shell has a different UID (in the case of e.g. su or sudo). In other

If I am the attacker wanting to type something into your su when you go
and su from my account, or where the user account is trojanned I do the
following

fork
exit parent
child ptraces the shell (same uid as it's not setuid)

You type "su" return
The modified shell opens a new pty/tty pair and runs su over it
My ptrace hooks watch the pty/tty traffic until you go to the loo
My ptrace hooks switch the console
My ptrace hooks type lots of stuff and hack your machine while eating the
output

and you come back, do stuff and then exit

And if you are in X it's even easier and I don't even need to care about
sessions or anything. X has no mechanism to sanely fix the problem, but
Wayland does.

Fortunately in any sane container case we don't give X11 handles to the
container contents. In insane cases (flatpak for example) you lose.

> scenarios, Yama, if enabled, should still stop you from doing that.
> And even with containers that have the same UID as the calling user,
> which I think is not exactly a good approach from a security perspective,
> the PID namespace should stop you from ptracing things outside the
> container.

For the case where the goal is to stop something leaking out of a
container then I agree entirely - namespaces can be used to play
whackamole with that particular one or you could use a pty/tty pair which
is how "sudo" solves the same problem space.

Disabling TIOCSTI via some magic Kconfig is silly, but making it
namespace hard is not.

If you really care about container security you could just a lightweight
vm instead but that's a different discussion ;-)

Alan

[Jann Horn]

On Tue, Apr 25, 2017 at 11:21 PM, One Thousand Gnomes
<gno...@lxorguk.ukuu.org.uk> wrote:
>> Really? By "pty", are you referring to the master? If so, as far as I know,
>> to go from the slave to the master, you need one of:
>>
>> - ptrace access to a process that already has an FD to the master, via
>> ptrace() or so (/proc/$pid/fd/$fd won't work)
>> - for a BSD PTY (which AFAIK isn't used much anymore), access to
>> /dev/ptyXX
>
> fstat() and then open *assuming* I have permissions.

open() what? As far as I know, for System-V PTYs, there is no path you can
open() that will give you the PTY master. Am I missing something?

>> > If I want to do the equvalent of the TIOCSTI attack then I fork a process
>> > and exit the parent. The child can now use ptrace to reprogram your shell
>> > to do whatever interesting things it likes (eg running child processes
>> > called "su" via a second pty/tty pair). Not exactly rocket science.
>>
>> Why would the child be able to ptrace the shell? AFAICS, in the most
>> relevant scenarios, the child can't ptrace the shell because the
>> shell has a different UID (in the case of e.g. su or sudo). In other
>
> If I am the attacker wanting to type something into your su when you go
> and su from my account, or where the user account is trojanned I do the
> following
>
> fork
> exit parent
> child ptraces the shell (same uid as it's not setuid)
>
> You type "su" return
> The modified shell opens a new pty/tty pair and runs su over it
> My ptrace hooks watch the pty/tty traffic until you go to the loo
> My ptrace hooks switch the console
> My ptrace hooks type lots of stuff and hack your machine while eating the
> output
>
> and you come back, do stuff and then exit
>
> And if you are in X it's even easier and I don't even need to care about
> sessions or anything. X has no mechanism to sanely fix the problem, but
> Wayland does.

I think the "When using a program like su or sudo" in the patch description
refers to the usecase where you go from a more privileged context (e.g. a
root shell) to a less privileged one (e.g. a shell as a service-specific
account used to run a daemon), not the other way around.

[However, I do think that it's a nice side effect of this patch that it will
prevent a malicious program from directly injecting something like an
SSH command into my shell in a sufficiently hardened environment
(with LSM restrictions that prevent the malicious program from opening
SSH keyfiles or executing another program that can do that). Although
you could argue that in such a case, the LSM should be taking care of
blocking TIOCSTI.]

[One Thousand Gnomes]

> open() what? As far as I know, for System-V PTYs, there is no path you can
> open() that will give you the PTY master. Am I missing something?

Sorry brain fade - no.

Which is the sudo case and why sudo uses a separate pty/tty pair as it's
not just TIOCSTI that's an issue but there are a load of ioctls that do
things like cause signals to the process or are just annoying -
vhangup(), changing the speed etc

(And for console changing the keymap - which is a nasty one)

> [However, I do think that it's a nice side effect of this patch that it will
> prevent a malicious program from directly injecting something like an
> SSH command into my shell in a sufficiently hardened environment
> (with LSM restrictions that prevent the malicious program from opening
> SSH keyfiles or executing another program that can do that). Although
> you could argue that in such a case, the LSM should be taking care of
> blocking TIOCSTI.]

I would submit that creating a new pty/tty pair is the proper answer for
that case however. Making the tty calls respect namespaces is however
still a no-brainer IMHO.

Alan

[Matt Brown]

Are any of these annoyances potential security issues? I would be happy
to add patches or modify this one to include extra hardening measures.

[One Thousand Gnomes]

> > Which is the sudo case and why sudo uses a separate pty/tty pair as it's
> > not just TIOCSTI that's an issue but there are a load of ioctls that do
> > things like cause signals to the process or are just annoying -
> > vhangup(), changing the speed etc
> >
> > (And for console changing the keymap - which is a nasty one)
> >
>
> Are any of these annoyances potential security issues? I would be happy
> to add patches or modify this one to include extra hardening measures.

Or you could just use pty/tty pairs properly the way sudo and other
applications do perfectly well.

Lots of them are potential security issues - if I sent your console to
1x1 char, change the font and keymap you'd proably be peeved 8-)

It's not about hardening against all these (which would break lots of
legitimate use cases), it's about having the affected applications do the
right thing.

It makes sense that TIOCSTI honours namespaces. However it and everything
else are correctly handled by creating the lower security level process
with its own pty/tty pair.

Alan

[Kees Cook]

This looks like it's ready to go. Greg, can you include this in your
tree? That seems like the best place, even though it touches a few
areas.

Please consider it:

Reviewed-by: Kees Cook <kees...@chromium.org>

Thanks!

-Kees

>
> # Changes since v4:
> * fixed typo
>
> # Changes since v3:
> * use get_user_ns and put_user_ns to take and drop references to the owner
> user namespace because CONFIG_USER_NS is an option
>
> # Changes since v2:
> * take/drop reference to user namespace on tty struct alloc/free to prevent
> use-after-free.
>
> # Changes since v1:
> * added owner_user_ns to tty_struct to enable capability checks against
> the namespace that created the tty.
> * rewording in different places to make patchset purpose clear
> * Added Documentation

--
Kees Cook
Pixel Security

[Matt Brown]

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

Acked-by: Serge Hallyn <se...@hallyn.com>
Reviewed-by: Kees Cook <kees...@chromium.org>

Signed-off-by: Matt Brown <ma...@nmatt.com>
---

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index c276814..fe68d14 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c

@@ -2297,11 +2297,17 @@ 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)) {
+ pr_warn_ratelimited("TIOCSTI ioctl call blocked 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

[Matt Brown]

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

# Changes since v5:
* added acks/reviews

[Alan Cox]

On Fri, 5 May 2017 19:20:16 -0400
Matt Brown <ma...@nmatt.com> wrote:

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

And it still doesn't deal with the fact that there are hundreds of other
ways to annoy the owner of a tty if it's passed to a lower privilege
child from framebuffer reprogramming through keyboard remaps.

The proper way to handle those cases is to create a pty/tty pair and use
that. Your patch is pure snake oil and if anything implies safety that
doesn't exist.

In addition your change to allow it to be used by root in the guest
completely invalidates any protection you have because I can push

"rm -rf /\n"

as root in my namespace and exit

The tty buffers are not flushed across the context change so the shell
you return to gets the input and oh dear....

Alan

[Daniel Micay]

On Wed, 2017-05-10 at 21:29 +0100, Alan Cox wrote:
>
> In addition your change to allow it to be used by root in the guest
> completely invalidates any protection you have because I can push
>
> "rm -rf /\n"
>
> as root in my namespace and exit
>
> The tty buffers are not flushed across the context change so the shell
> you return to gets the input and oh dear....
>
> Alan

I might be missing something, but it looks like the patch tracks where
the tty was created and only allows this with CAP_SYS_ADMIN in the ns
where the tty came from.

[Matt Brown]

I'm not implying that my patch is supposed to provide safety for
"hundreds of other" issues. I'm looking to provide a way to lock down a
single TTY ioctl that has caused real security issues to arise. For
this reason, it's completely incorrect to say that this feature is
snake oil. My patch does exactly what it claims to do. No more no less.

> In addition your change to allow it to be used by root in the guest
> completely invalidates any protection you have because I can push
>
> "rm -rf /\n"
>
> as root in my namespace and exit
>
> The tty buffers are not flushed across the context change so the shell
> you return to gets the input and oh dear....

This is precisely what my patch prevents! With my protection enabled, a
container will only be able to use the TIOCSTI ioctl on a tty if that
container has CAP_SYS_ADMIN in the user namespace in which the tty was
created.

>
> Alan
>

[Alan Cox]

O> I'm not implying that my patch is supposed to provide safety for

> "hundreds of other" issues. I'm looking to provide a way to lock down a
> single TTY ioctl that has caused real security issues to arise. For

In other words you are not actually fixing anything.

> this reason, it's completely incorrect to say that this feature is
> snake oil. My patch does exactly what it claims to do. No more no less.
>
> > In addition your change to allow it to be used by root in the guest
> > completely invalidates any protection you have because I can push
> >
> > "rm -rf /\n"
> >
> > as root in my namespace and exit
> >
> > The tty buffers are not flushed across the context change so the shell
> > you return to gets the input and oh dear....
>
> This is precisely what my patch prevents! With my protection enabled, a
> container will only be able to use the TIOCSTI ioctl on a tty if that
> container has CAP_SYS_ADMIN in the user namespace in which the tty was
> created.

Which is not necessarily the namespace of the process that next issues a
read().

This is snake oil. There is a correct and proper process for this use
case. That proper process is to create a new pty/tty pair. There are two
cases

- processes that do it right in which case the attacker is screwed and we
don't need to mess up TIOCSTI handling for no reason.

- processes that do it wrong. If they do it wrong then they'll also use
all the other holes and attacks available via the same path which are
completely unfixable without making your system basically unusable.


So can we please apply the minimum of common sense reasoning to this and
drop the patch.

Alan

[Peter Dolding]

You missed some important.

From: http://man7.org/linux/man-pages/man7/capabilities.7.html
Don't choose CAP_SYS_ADMIN if you can possibly avoid it!
A vast proportion of existing capability checks are associated with this
capability (see the partial list above). It can plausibly be
called "the new root", since on the one hand, it confers a wide
range of powers, and on the other hand, its broad scope means that
this is the capability that is required by many privileged
programs. Don't make the problem worse. The only new features
that should be associated with CAP_SYS_ADMIN are ones that closely
match existing uses in that silo.

This not only a improper fix the attempted fix is breach do
documentation. CAP_SYS_ADMIN is that far overloaded it does not
require any more thrown in it direction.

This is one of the grsecurity patches mistakes. GRKERNSEC_HARDEN_TTY
is from 18 Feb 2016 this documentation as in place at the time they
wrote this. Yes GRKERNSEC_HARDEN_TTY does exactly the same thing.
Yes Grsecurity guys did the same error and the grsecurity patches are
filled with this error.

The result is from the TIOCSTI patch done this way is you have to use
CAP_SYS_ADMIN to use TIOSCTI so opening up every exploit that
Grsecurity has added and every exploit CAP_SYS_ADMIN can do what is
quite a few.

Now I don't know if CAP_SYS_TTY_CONFIG what is an existing capability
might be what TIOCSTI should own under.

The reality here is CAP_SYS_ADMIN as become the Linux kernel security
equal what big kernel lock was for multi threading.

In a ideal world CAP_SYS_ADMIN would not be used directly in most
cases. Instead CAP_SYS_ADMIN would have a stack of sub capabilities
groups under it.

The excuse for doing it wrong grsecurity is
https://forums.grsecurity.net/viewtopic.php?f=7&t=2522

Yes most capabilities open up possibility of exploiting the system.
They are not in fact designed to prevent this. They are designed to
limit the damage in case of malfunction so that a program/user has
only limited methods of damaging the system. Like a program
malfunctioning with only limit capabilities if it does an action those
capabilities don't allow no damage will happen. Now CAP_SYS_ADMIN is
for sure not limited.

But since grsecurity developers took the point of view these are False
Boundaries they then proceed to stack item after item under
CAP_SYS_ADMIN because the boundary made no sense to them. Also some
mainline Linux Kernel developers are guilty of the same sin of
overloading CAP_SYS_ADMIN.

From my point of view any new patching containing CAP_SYS_ADMIN
directly used should be bounced just for that. If features need to
be added to CAP_SYS_ADMIN now they should have to go into another
capability that is enabled when CAP_SYS_ADMIN is and hopeful if we do
this over time we will be able to clean up CAP_SYS_ADMIN into sanity.


Peter

[Matt Brown]

That last sentence is the key. CAP_SYS_ADMIN is already associated with
the TIOCSTI ioctl!

From the same capabilities man page you quote above
under the CAP_SYS_ADMIN section:
employ the TIOCSTI ioctl(2) to insert characters into the input queue
of a terminal other than the caller's controlling terminal;

> This not only a improper fix the attempted fix is breach do
> documentation. CAP_SYS_ADMIN is that far overloaded it does not
> require any more thrown in it direction.
>

See my comment above. This is clearly following the capabilities
documentation since CAP_SYS_ADMIN is already closely linked with the
TIOCSTI ioctl.

> This is one of the grsecurity patches mistakes. GRKERNSEC_HARDEN_TTY
> is from 18 Feb 2016 this documentation as in place at the time they
> wrote this. Yes GRKERNSEC_HARDEN_TTY does exactly the same thing.
> Yes Grsecurity guys did the same error and the grsecurity patches are
> filled with this error.
>
> The result is from the TIOCSTI patch done this way is you have to use
> CAP_SYS_ADMIN to use TIOSCTI so opening up every exploit that
> Grsecurity has added and every exploit CAP_SYS_ADMIN can do what is
> quite a few.
>
> Now I don't know if CAP_SYS_TTY_CONFIG what is an existing capability
> might be what TIOCSTI should own under.
>

I could see a case being make for CAP_SYS_TTY_CONFIG. However I still
choose to do with CAP_SYS_ADMIN because it is already in use in the
TIOCSTI ioctl.

> The reality here is CAP_SYS_ADMIN as become the Linux kernel security
> equal what big kernel lock was for multi threading.
>
> In a ideal world CAP_SYS_ADMIN would not be used directly in most
> cases. Instead CAP_SYS_ADMIN would have a stack of sub capabilities
> groups under it.
>
> The excuse for doing it wrong grsecurity is
> https://forums.grsecurity.net/viewtopic.php?f=7&t=2522
>
> Yes most capabilities open up possibility of exploiting the system.
> They are not in fact designed to prevent this. They are designed to
> limit the damage in case of malfunction so that a program/user has
> only limited methods of damaging the system. Like a program
> malfunctioning with only limit capabilities if it does an action those
> capabilities don't allow no damage will happen. Now CAP_SYS_ADMIN is
> for sure not limited.
>
> But since grsecurity developers took the point of view these are False
> Boundaries they then proceed to stack item after item under
> CAP_SYS_ADMIN because the boundary made no sense to them. Also some
> mainline Linux Kernel developers are guilty of the same sin of
> overloading CAP_SYS_ADMIN.
>
> From my point of view any new patching containing CAP_SYS_ADMIN
> directly used should be bounced just for that. If features need to
> be added to CAP_SYS_ADMIN now they should have to go into another
> capability that is enabled when CAP_SYS_ADMIN is and hopeful if we do
> this over time we will be able to clean up CAP_SYS_ADMIN into sanity.
>

You might be right that CAP_SYS_ADMIN is overloaded, but my patch
barely adds anything to it since TIOCSTI already falls under its
control. It seems extreme to say this patch ought to be rejected just
because it contains CAP_SYS_ADMIN. If we want to fix the state of Linux
capabilities, then I suggest that should be a separate patchset to
reorganize them into a more modular set of controls.

>
> Peter
>

[Peter Dolding]

>
> I could see a case being make for CAP_SYS_TTY_CONFIG. However I still
> choose to do with CAP_SYS_ADMIN because it is already in use in the
> TIOCSTI ioctl.
>

Matt Brown don't give me existing behaviour. CAP_SYS_ADMIN is
overload. The documentation tells you that you are not to expand it
and you openly admit you have.

Does anything of TIOSCTI functionally say that it really should be in
CAP_SYS_ADMIN.

If functionality is going to cause security for containers maybe it
should be not in CAP_SYS_ADMIN but in its own capability that can
enabled on file by file base.

>
> You might be right that CAP_SYS_ADMIN is overloaded, but my patch
> barely adds anything to it since TIOCSTI already falls under its
> control. It seems extreme to say this patch ought to be rejected just
> because it contains CAP_SYS_ADMIN. If we want to fix the state of Linux
> capabilities, then I suggest that should be a separate patchset to
> reorganize them into a more modular set of controls.
>

We have end up with CAP_SYS_ADMIN a mess by the of a death by a
thousand cuts. Each person to extend CAP_SYS_ADMIN to it current
mess said the same thing. My patch barely added anything times that
by a few thousand and you end up with what we have today. At some
point no more has to be said.

There is no point attempting to tidy it up of the rules are not put in
place so it does not turn into a mess again.

This is not something that is suitable to be done as one large
patchset. This is better done in the same kind of method that made
it. So every time people want to alter something associated with
CAP_SYS_ADMIN it has to get assessed and the patch has to be one that
partly corrects the existing mess. Do this enough times and we will
no longer have a mess on CAP_SYS_ADMIN.

https://www.freedesktop.org/software/systemd/man/systemd.exec.html
Please note the CapabilityBoundingSet=

Your current patch adds no extra controls for me running a service
under systemd or anything else like it to say I don't want the
processes having the means to-do this even that they are running with
CAP_SYS_ADMIN to perform other tasks..

--employ the TIOCSTI ioctl(2) to insert characters into the input
queue of a terminal other than the caller's controlling terminal;--
This currently under CAP_SYS_ADMIN is vastly more powerful than the
one you are attempt to take away with your patch. This one can send
messages into other terminals. This is a vastly more powerful
version of TIOSCTI.

I fact this usage of TIOCSTI I personally think should require two
capabilities flags set. CAP_SYS_ADMIN section left as it is at this
stage. With TIOSCTI stuck behind another capability.

If you had added a new capability flag you could set file capabilities
on any of the old applications depending on the now secured behaviour.

https://github.com/lxc/lxc/commit/e986ea3dfa4a2957f71ae9bfaed406dd6e1ffff6

Also the general user TIOCSTI issue can be handled a different way as
LXC fix shows. Where they uses a pty to isolate so meaning in their
fixed setup user TIOSCTI was not harmful but CAP_SYS_ADMIN TIOSCTI
still could be. You patch as not address this problem because you
shoved everything under CAP_SYS_ADMIN. But if you add different
capability to use TIOSCIT that is not CAP_SYS_ADMIN to allow
CAP_SYS_ADMiN TIOSCTI functionality to be disabled.

This is what you see more often than not when you dig into this
patches adding more CAP_SYS_ADMIN. Adding CAP_SYS_ADMIN is not fixing
a problem in most cases. Breaking CAP_SYS_ADMIN functionality up
should be the goal not expand it.

Basically you have done something documentation has a note to
developer not to-do. If you start looking at the problem what you
doing is not helping. If people end up using CAP_SYS_ADMIN to access
TIOSCTI its giving the program a more powerful version of TIOSCTI to
do more harm with so reduced containment. Totally anti to what you
are meant to be doing with capabilities..

[Matt Brown]

On 05/16/2017 05:01 AM, Peter Dolding wrote:
>>
>> I could see a case being make for CAP_SYS_TTY_CONFIG. However I still
>> choose to do with CAP_SYS_ADMIN because it is already in use in the
>> TIOCSTI ioctl.
>>
> Matt Brown don't give me existing behaviour. CAP_SYS_ADMIN is
> overload. The documentation tells you that you are not to expand it
> and you openly admit you have.
>

This is not true that I'm openly going against what the documentation
instructs. The part of the email chain where I show this got removed
somehow. Again I will refer to the capabilities man page that you
quoted.

From http://man7.org/linux/man-pages/man7/capabilities.7.html

"Don't choose CAP_SYS_ADMIN if you can possibly avoid it!

...

The only new features that should be associated with CAP_SYS_ADMIN are
ones that closely match existing uses in that silo."

My feature affects the TIOCSTI ioctl. The TIOCSTI ioctl already falls
under CAP_SYS_ADMIN, therefore I actually *am* following the
documentation.

[Kees Cook]

On Tue, May 16, 2017 at 5:22 AM, Matt Brown <ma...@nmatt.com> wrote:
> On 05/16/2017 05:01 AM, Peter Dolding wrote:
>>>
>>>
>>> I could see a case being make for CAP_SYS_TTY_CONFIG. However I still
>>> choose to do with CAP_SYS_ADMIN because it is already in use in the
>>> TIOCSTI ioctl.
>>>
>> Matt Brown don't give me existing behaviour. CAP_SYS_ADMIN is
>> overload. The documentation tells you that you are not to expand it
>> and you openly admit you have.
>>
>
> This is not true that I'm openly going against what the documentation
> instructs. The part of the email chain where I show this got removed
> somehow. Again I will refer to the capabilities man page that you
> quoted.
>
> From http://man7.org/linux/man-pages/man7/capabilities.7.html
>
> "Don't choose CAP_SYS_ADMIN if you can possibly avoid it!
> ...
> The only new features that should be associated with CAP_SYS_ADMIN are
> ones that closely match existing uses in that silo."
>
> My feature affects the TIOCSTI ioctl. The TIOCSTI ioctl already falls
> under CAP_SYS_ADMIN, therefore I actually *am* following the
> documentation.

CAP_SYS_ADMIN is the right choice here, I agree with Matt: it is
already in use for TIOCSTI. We can't trivially add new capabilities
flags (see the various giant threads debating this, the most recently
that I remember from the kernel lock-down series related to Secure
Boot).

>> I fact this usage of TIOCSTI I personally think should require two
>> capabilities flags set. CAP_SYS_ADMIN section left as it is at this
>> stage. With TIOSCTI stuck behind another capability.
>>
>> If you had added a new capability flag you could set file capabilities
>> on any of the old applications depending on the now secured behaviour.

If we're adjusting applications, they should be made to avoid TIOSCTI
completely. This looks to me a lot like the symlink restrictions: yes,
userspace should be fixed to the do the right thing, but why not
provide support to userspace to avoid the problem entirely?

-Kees

[Serge E. Hallyn]

Consideer that if we use CAP_SYS_TTY_CONFIG now, then any applications
which are currently being given CAP_SYS_ADMIN would need to be updated
with a second capability. Not acceptable. Even when we split up
CAP_SYSLOG, we took care to avoid that (by having the original capability
also suffice, so either capability worked).

[Peter Dolding]

We cannot just keep on expanding CAP_SYS_ADMIN either.

>
>>> I fact this usage of TIOCSTI I personally think should require two
>>> capabilities flags set. CAP_SYS_ADMIN section left as it is at this
>>> stage. With TIOSCTI stuck behind another capability.
>>>
>>> If you had added a new capability flag you could set file capabilities
>>> on any of the old applications depending on the now secured behaviour.
>
> If we're adjusting applications, they should be made to avoid TIOSCTI
> completely. This looks to me a lot like the symlink restrictions: yes,
> userspace should be fixed to the do the right thing, but why not
> provide support to userspace to avoid the problem entirely?
>

Kees I like but you have forgot the all important rule. The Linus
Rule. Existing applications must have a method work.
So modify applications binary is not way out of problem.

Please note making CAP_SYS_ADMIN the only way to use TIOCSTI also
means setting CAP_SYS_ADMIN on all the existing applications to obey
the Linus Rule of not break userspace. So this is why the patch is
strictly no as this means elevating privilege of existing applications
and possibly opening up more security flaws.

Reality any patch like the one we are talking about due to the Linus
Rule and the security risk it will open up obey this it just be
rejected. There is another kind of way I will cover with Serge.

Peter Dolding.

[Matt Brown]

This feature is not required so it is not "making CAP_SYS_ADMIN the
only way to use TIOCSTI". It defaults to no as to not break some
existing programs that use it.

>
> Reality any patch like the one we are talking about due to the Linus
> Rule and the security risk it will open up obey this it just be
> rejected. There is another kind of way I will cover with Serge.
>
> Peter Dolding.
>

Matt

[Peter Dolding]

There is another option create a security bit.

That could be called SECBIT_CONTAINER. This turns off functionally
like TIOCSTI that can be used to it break out with.

This case the mainlined code the TIOCSTI currently in CAP_SYS_ADMIN is
a container breaker its designed to allow reaching cross users and
TTYs. SECBIT is a inverted capability so when it enabled it disables
something and once enabled it cannot be disabled. So the lxc
addressed to the user TIOCSTI causing a breakout does not work against
the CAP_SYS_ADMIN one. If there was a security bit that disabled
TIOCSTI completely that prevents all the escape paths by TIOCSTI.

There would also be room for a SECBIT_NO_OBSOLETE what is quite simple
make using obsolete functions application fatal. Now with CAP_SYSLOG
if SECBIT_NO_OBSOLETE programs using the old capability could find the
feature removed. So over time we can systematically remove the multi
entry path we have now as userspace updates and stops requiring the
second path.

There is more than 1 way to skin this cat. There is no need to add
more to CAP_SYS_ADMIN and it particular bad when you consider having
to obey the Linux Rule of user-space compatibly would result in having
apply CAP_SYS_ADMIN to existing applications with Matts patch.

Peter

.

[Alan Cox]

> If we're adjusting applications, they should be made to avoid TIOSCTI
> completely. This looks to me a lot like the symlink restrictions: yes,
> userspace should be fixed to the do the right thing, but why not
> provide support to userspace to avoid the problem entirely?

We do it's called pty/tty. There isn't any other way to do this correctly
because TIOCSTI is just one hundreds of things the attacker can do to
make your life miserable in the case you create a child process of lower
security privilege and give it your tty file handle or worse (like some
container crapware) your X11 socket fd.

Does it really matter any more or less if I reprogram your enter key, use
TIOCSTI, set the baud rate, change all your fonts ?

The mainstream tools like sudo get this right (*). Blocking TIOCSTI fixes
nothing and breaks apps. If it magically fixed the problem it might make
sense but it doesn't. You actually have to get an adult to write the
relevant code.

Alan
(*) Almost. There's an old world trick of sending "+++" "ATE1" "rm -rf
*\r\n" to try and attack improperly configured remote modem sessions but
the stuff that matters is handled.

[Daniel Micay]

On Wed, 2017-05-17 at 17:41 +0100, Alan Cox wrote:
> > If we're adjusting applications, they should be made to avoid
> > TIOSCTI
> > completely. This looks to me a lot like the symlink restrictions:
> > yes,
> > userspace should be fixed to the do the right thing, but why not
> > provide support to userspace to avoid the problem entirely?
>
> We do it's called pty/tty. There isn't any other way to do this
> correctly
> because TIOCSTI is just one hundreds of things the attacker can do to
> make your life miserable in the case you create a child process of
> lower
> security privilege and give it your tty file handle or worse (like
> some
> container crapware) your X11 socket fd.
>
> Does it really matter any more or less if I reprogram your enter key,
> use
> TIOCSTI, set the baud rate, change all your fonts ?
>
> The mainstream tools like sudo get this right (*). Blocking TIOCSTI
> fixes
> nothing and breaks apps. If it magically fixed the problem it might
> make
> sense but it doesn't. You actually have to get an adult to write the
> relevant code.

It gets it right because it was reported as a vulnerability and fixed,
which is the cycle many of these tools have gone through. It looks like
sudo and coreutils / shadow su were fixed in 2005, but there are more of
these tools.

CVE-2005-4890 (coreutils su, shadow su, sudo), CVE-2016-7545 (SELinux
sandbox utility), CVE-2016-2781 (chroot --userspec), CVE-2016-2779
(util-linux runuser), CVE-2016-2568 (pkexec)

I am not sure if the pkexec vulnerability is even fixed:

https://bugzilla.redhat.com/show_bug.cgi?id=1300746

CVE-2017-5226 is for bubblewrap/flatpak this year, and I'm sure there
were a lot more as I didn't bother to dig very deep.

I completely agree that it should be solved by doing things properly in
each application. However, it isn't solved properly everywhere and each
new application is making the same mistake. Providing this feature does
not break anything that people use in practice and it doesn't need to
solve every issue to be quite useful, it only needs to prevent local
privilege escalation in the form of code execution in many cases. Is
there another way to get code execution via that bubblewrap/flatpak bug
with this feature implemented? As far as I can tell, there isn't. It's a
problem even with this feature but a significantly less serious one.

[Kees Cook]

This patch solves a real problem when userspace does things wrong. For
those that do not want it, a sysctl exists (and is even default off).
Arguments about how userspace needs to be fixed is a total red
herring. Like symlink protections before, this should be available for
those that want it because it solves an interface problem that gets
regularly misused and causes real damage. The kernel has a
responsibility to protect userspace.

As Daniel mentions, there is a history of mistakes that appear to be
becoming even more common:
http://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=TIOCSTI

There are people who want this feature that do not care about xemacs
breaking. The default cannot be enabled because of the golden rule of
never breaking userspace, but entirely refusing to fix a regularly
misused feature is wrong-headed. We must accept that not only are bug
lifetimes long, but that bugs will keep getting reintroduced. When
there is a chance to kill a class of bugs or exploit techniques, it's
the kernel responsibility to do so. This is a clear case of that, and
the solution is concise.

If there is some better solution that the kernel can provide to
mitigate processes misusing ttys, then by all means, we can add that
too, but has nothing to do with refusing this change. This solves a
specific problem that in many cases is the only path to privilege
escalation (as Daniel mentioned). Refusing this change is nonsense.
"Your car shouldn't have seat belts because maybe something will stab
you through the windshield" isn't a reasonable argument to make.

[Greg KH]

Always follow the proper kernel coding style rules, as I don't want to
have someone else have to come along and fix up the error you have added
here :(

checkpatch.pl is your friend, really...

And why not do a warning with the device that caused the problem to
happen? dev_warn has a ratelimit I think right? "raw" printk messages
like this don't help in trying to track down what/who caused the issue.

And finally, can userspace see the namespace for the tty? Doesn't
things like checkpoint/restore need that in order to properly set the
tty connection back up when moving processes?

v7? :)

thanks,

greg k-h

[Peter Dolding]

True but what is usage of TIOCSTI that is wrong in these CVE. Is it
not when the returned input values values to tty crosses process
boundaries. Like if when process terminated everything the process
had pushed back to TTY and Input it has received was lost those CVE
would not be possible.

Could those exist CVE when exploit happens have CAP_SYS_ADMIN as the
process being used to break out the answer is yes. So restricting
TIOCSTI to CAP_SYS_ADMIN could be pointless. Remember you have suid
bit applications one of those might have bug that someone is able to
exploit to use the TIOCSTI. So pushing to CAP_SYS_ADMIN make exploit
rarer does not remove it.

Please note you see TIOCSTI using in libc when they implement
ungetc(). So this is something that is quite well used. I see
ungetc as kind of a bad idea to be implemented kernel level going to
tty shareable between processes.

> If there is some better solution that the kernel can provide to
> mitigate processes misusing ttys, then by all means, we can add that
> too, but has nothing to do with refusing this change. This solves a
> specific problem that in many cases is the only path to privilege
> escalation (as Daniel mentioned). Refusing this change is nonsense.
> "Your car shouldn't have seat belts because maybe something will stab
> you through the windshield" isn't a reasonable argument to make.

Using cap_sys_admin as fix is like removing car windsheld because
vision is being blocked by a rock hitting it.

Kees the problem with accepting a security fix that is wrong the
proper change never gets worked on.

I am not saying there is not a real problem here. The fix is not
push it to CAP_SYS_ADMIN. Due to TIOCSTI that cross process and tty
boundaries been used in security breaks and limit applications that
uses use as either Administrator or as normal User means this ability
does not own in CAP_SYS_ADMIN either.

The same is true of obsolete function calls that have been shoved into
CAP_SYS_ADMIN already there comes point where no valid userspace
application is using that function. At that point the only thing
using that function is exploits so then really CAP_SYS_ADMIN should
not have access to those obsolete functions. There is a real need
for CAP_SYS_OBSOLETE. If a program has to ahve CAP_SYS_OBSOLETE set
on it this means it using functionally that is known busted in some
way..

This is the biggest problem here. There is no real agreement how we
exterminate/restrict flawed functions to progressive reduce the number
of applications and users who can access the flawed functions to allow
them to be fully disabled for 99.99 percent of people using systems.

That is what people are not getting CAP_SYS_ADMIN has too many users
to be counted and mitigation as well.

Yes we must not break userspace but we also must mitigate against
userspace issues. We do need a clear rules on how to fix these
security problem user-space is allowed to be broken and of course made
part of the Linux kernel documentation. Altering the binary is for
sure out because the person operating the system may not have that
right. Placing a flag on a binary so it works I would see as
acceptable as long as that flag was not grant a stack of other
privileges that application would have never had before..

Peter Dolding

[Matt Brown]

My bad. Will fix these issues in v7.

> And why not do a warning with the device that caused the problem to
> happen? dev_warn has a ratelimit I think right? "raw" printk messages
> like this don't help in trying to track down what/who caused the issue.
>

yes <linux/device.h> has dev_warn_ratelimited. I will use that in 7v.

> And finally, can userspace see the namespace for the tty? Doesn't
> things like checkpoint/restore need that in order to properly set the
> tty connection back up when moving processes?

This seems like we would need to expose the owner_user_ns of the tty in procfs
somewhere. Section 1.7 Documentation/filesystems/proc.txt describes the
following files in /proc/tty:

Table 1-11: Files in /proc/tty
..............................................................................
File Content
drivers list of drivers and their usage
ldiscs registered line disciplines
driver/serial usage statistic and status of single tty lines
..............................................................................

The drivers file is the one that gives the most information that we are
interested in.

However, the current layout combines information about multiple ttys by driver.
As I understand it, a single driver may have ttys that span across different
owner_user_ns. would it make sense to add a file /proc/tty/ns that would
contain the different tty to user namespace mappings? Or is there a better way
to do this? I would appreciate any feedback/ideas you have on this.

>
> v7? :)

v7 will be on its way soon. I'm not currently sure how to address the concern
of giving things like checkpoint/restore in userland a way to get the
owner_user_ns.

>
> thanks,
>
> greg k-h
>

Thanks for the feedback,

Matt Brown

[Serge E. Hallyn]

On Fri, May 19, 2017 at 12:48:17PM +1000, Peter Dolding wrote:
> Using cap_sys_admin as fix is like removing car windsheld because
> vision is being blocked by a rock hitting it.

Nonsense. If the application has cap_sys_admin then it is less contained and
more trusted anyway. If I went to the trouble to run an application in a
private user namespace (where it can have cap_sys_admin, but not targeted
at my tty) then it should be more contained. That's the point of targeted
capabilities.

[Peter Dolding]

The thing that is missed every time is how much is cap_sys_admin.

So you are saying a user namespace has to be set up to contain the defect.

Really no application should have cap_sys_admin.

The theory of capabilities is that security should be broken down into
logical blocks.

So tty stuff should under a tty capabilities.

This one here should not be shoved into cap_sys_admin because can you
show a single case of a general used application performing this
action in the exploit way that is normal behaviour.

The exploits are doing behaviours that have no general place.

Its really simple to shove everything to cap_sys_admin instead of hey
lets look at the exploits how they work and if this should be fairly
blanked banned. The behaviour that is question is being able push
chars into input stream and have them processed after application has
terminated or after application has switched to background. That is
not pushing data into another tty. Pushing data into a different tty
is already restricted to cap_sys_admin.

Personally from my point of view when application terminates or
switches to background what ever it pushed back into the input buffer
should be junked and maybe a special cap to deal with rare case of
applications that expect this behavour.

Also please remember one of the application using this behaviour of
pushing stuff back to input buffer is csh. In other words a general
user shell. This will not be the only application that is general
usage after the change of pushing to cap_sys_admin that would also
have to be pushed to cap_sys_admin because they use TIOCSTI in a way
that the patch will block when the program does not have
cap_sys_admin. So now you have more applications running as
cap_sys_admin so more security problems.

Peter Dolding.

[Matt Brown]

This patch adds struct user_namespace *owner_user_ns to the tty_struct.
Then it is set to current_user_ns() in the alloc_tty_struct function.

This is done to facilitate capability checks against the original user
namespace that allocated the tty.

E.g. ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)

This combined with the use of user namespace's will allow hardening
protections to be built to mitigate container escapes that utilize TTY
ioctls such as TIOCSTI.

See: https://bugzilla.redhat.com/show_bug.cgi?id=1411256

Acked-by: Serge Hallyn <se...@hallyn.com>
Reviewed-by: Kees Cook <kees...@chromium.org>
Signed-off-by: Matt Brown <ma...@nmatt.com>
---

drivers/tty/tty_io.c | 2 ++
include/linux/tty.h | 2 ++
2 files changed, 4 insertions(+)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e6d1a65..c276814 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -171,6 +171,7 @@ static void free_tty_struct(struct tty_struct *tty)
put_device(tty->dev);
kfree(tty->write_buf);
tty->magic = 0xDEADDEAD;
+ put_user_ns(tty->owner_user_ns);
kfree(tty);
}

@@ -3191,6 +3192,7 @@ struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx)
tty->index = idx;
tty_line_name(driver, idx, tty->name);
tty->dev = tty_get_device(tty);
+ tty->owner_user_ns = get_user_ns(current_user_ns());

return tty;
}
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1017e904..d902d42 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -12,6 +12,7 @@
#include <uapi/linux/tty.h>
#include <linux/rwsem.h>
#include <linux/llist.h>
+#include <linux/user_namespace.h>


/*
@@ -333,6 +334,7 @@ struct tty_struct {
/* If the tty has a pending do_SAK, queue it here - akpm */
struct work_struct SAK_work;
struct tty_port *port;
+ struct user_namespace *owner_user_ns;
};

/* Each of a tty's open files has private_data pointing to tty_file_private */
--
2.10.2

[Matt Brown]

Acked-by: Serge Hallyn <se...@hallyn.com>
Reviewed-by: Kees Cook <kees...@chromium.org>
Signed-off-by: Matt Brown <ma...@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/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

index 823ca1a..665b610 100644

[Matt Brown]

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

# Integration with CRIU:
* The CRIU dev team was contacted about any possible issues that
restrict_tiocsti might cause for the CRIU program. Their response was that
CRIU currently does not utilize the tiocsti ioctl, so it doesn't affect
them at the moment. They do wish in the future to do checks against
owner_user_ns, so we will work together on a way to make this information
available to userspace. In the mean time, The CRIU team is OK with this
patch moving forward as is.

# Changes since v6:
* fixed style issues
* changed error message to use dev_warn_ratelimited

[Alan Cox]

On Mon, 29 May 2017 17:38:00 -0400
Matt Brown <ma...@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]

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

--
Boris Lukashev
Systems Architect
Semper Victus

[Matt Brown]

On 5/29/17 6:26 PM, Alan Cox wrote:
> On Mon, 29 May 2017 17:38:00 -0400
> Matt Brown <ma...@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.

Can you give me an example of another ioctl that you can abuse to practically
gain code execution in the privileged context? Saying that the child process
could "cause mischief" is a bit vague.

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

It doesn't break anything because it is default n. People that enable this
option will understand they are disabling the tiocsti ioctl for non privileged
processes.

>
> NAK v7.

Rather than a NAK, could you explain how you would solve this problem in a way
that we can protect userspace from shooting itself in the foot.

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

Matt

[Casey Schaufler]

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.

[Matt Brown]

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.

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.

Thanks,
Matt Brown

[Casey Schaufler]

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.

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

[Matt Brown]

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

[Alan Cox]

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

[Casey Schaufler]

You picked a bad example ...

> 2. There exists an inexperienced user Bob that toggles X to "on".

... which is easy enough, however ...

> 3. Bob complains that X has broken userspace because he now cannot access his
> SSH key from firefox.

... that's not going to happen. Because the LSM you're referring to requires
additional configuration to "break" userspace. That, by the way, was a conscious
design choice. Now, I realize that's not the case for some other security modules,
but they have things like "permissive mode" to make it easy on accident prone Bob.

The analogy, while obvious, is invalid.

[Serge E. Hallyn]

Quoting Peter Dolding (oia...@gmail.com):
> On Sat, May 20, 2017 at 12:33 AM, Serge E. Hallyn <se...@hallyn.com> wrote:
> > On Fri, May 19, 2017 at 12:48:17PM +1000, Peter Dolding wrote:
> >> Using cap_sys_admin as fix is like removing car windsheld because
> >> vision is being blocked by a rock hitting it.
> >
> > Nonsense. If the application has cap_sys_admin then it is less contained and
> > more trusted anyway. If I went to the trouble to run an application in a
> > private user namespace (where it can have cap_sys_admin, but not targeted
> > at my tty) then it should be more contained. That's the point of targeted
> > capabilities.
>
> The thing that is missed every time is how much is cap_sys_admin.
>
> So you are saying a user namespace has to be set up to contain the defect.
>
> Really no application should have cap_sys_admin.
>
> The theory of capabilities is that security should be broken down into
> logical blocks.
>
> So tty stuff should under a tty capabilities.

(last reply on this)

Currently capabilities.7 says

* employ the TIOCSTI ioctl(2) to insert characters into the input queue of a
terminal other than the caller's controlling terminal;

for CAP_SYS_ADMIN.

So you can create a new CAP_SYS_TIOCSSTI if you like, and offer a patch where
*both* CAP_SYS_ADMIN and CAP_SYS_ADMIN suffice. Again, see CAP_SYSLOG for a
prior example.

What you may not do is change it so that on an older kernel you must have
CAP_SYS_ADMIN to use TIOCSTI, while on a newer one it does not suffice.

-serge

[Matt Brown]

So I may have been a bit sarcastic with my analogy, but my point is that it
seems like your definition of "breaking userspace" is too broad and could lead
to things getting labeled as breaking userspace that clearly are not.

[Matt Brown]

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.

[Daniel Micay]

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

[Stephen Smalley]

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 majo...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Nick Kralevich]

On Tue, May 30, 2017 at 11:32 AM, Stephen Smalley <s...@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

[Matt Brown]

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

[Daniel Micay]

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

[Alan Cox]

> > So tty stuff should under a tty capabilities.
>
> (last reply on this)
>
> Currently capabilities.7 says
>
> * employ the TIOCSTI ioctl(2) to insert characters into the input queue of a
> terminal other than the caller's controlling terminal;
>
> for CAP_SYS_ADMIN.
>
> So you can create a new CAP_SYS_TIOCSSTI if you like, and offer a patch where
> *both* CAP_SYS_ADMIN and CAP_SYS_ADMIN suffice. Again, see CAP_SYSLOG for a
> prior example.

Even then it wouldn't be useful because the attacker can use every other
interface in the tty layer, many of which you can't magic away behind a
capability bit. And the applications would need changing to use the
feature - at which point any theoretical broken apps can instead be fixed
to use a pty/tty pair and actually fix the real problem.

Alan

[Alan Cox]

On Tue, 30 May 2017 12:28:59 -0400
Matt Brown <ma...@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 ;-)]

[Matt Brown]

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?

[Matt Brown]

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.

[Daniel Micay]

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.

[Matt Brown]

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.

[Alan Cox]

> 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

[James Morris]

On Mon, 29 May 2017, 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.

Which effort? Kernel self protection is about protecting against flaws in
the kernel.

See:
https://kernsec.org/wiki/index.php/Kernel_Self_Protection_Project

"This project starts with the premise that kernel bugs have a very long
lifetime, and that the kernel must be designed in ways to protect against
these flaws."

We need to avoid conflating:

- hardening the kernel against attack; and
- modifying the kernel to try and harden userspace.

These patches are the latter, and the case for them is not as
straightforward.


- James
--
James Morris
<jmo...@namei.org>

[Matt Brown]

I agree that these patches aren't kernel self protection and I don't
believe I have claimed they are such a thing. These patches I'm
presenting are more akin to ptrace protections that are found in Yama.

[Peter Dolding]

Alan is right. CAP_SYS_ADMIN allows crossing the tty barrier.

Broken applications that you can wrap in a pty/tty pair as the lxc
application does would be defeated if those applications move up to
CAP_SYS_ADMIN. Because you have granted the high right of cross
pty/tty containment.

Pushing CAP_SYS_TIOSSTI out by itself without the feature in
CAP_SYS_ADMIN means broken applications can be allowed to run in like
a lxc container where they cannot go anywhere with the exploit because
the pty/tty they are picking is not going to get them very far at all.

Pushing TIOSSTI up to CAP_SYS_ADMIN to address this problem is wrong.
Question is also how many applications use CAP_SYS_ADMIN feature to
push chars into other pty/tty on the system. Pushing across pty/tty
barrier may not be a suitable feature to be generically in
CAP_SYS_ADMIN in the first place.

http://www.halfdog.net/Security/2012/TtyPushbackPrivilegeEscalation/

This here is example of TIOSSTI pushback as CAP_SYS_ADMIN being bad.
I don't know of a genuine program using push back in exploiting way
where the pushed back input is expected to be processed after the
program has terminated.

Really we need to work out how many breakage will in fact be caused by
majority restricting both pushback and write across tty barrier.
This is not like CAP_SYS_LOG these are features that can be used to
exploit system badly. It is possible that the exploiting form of
TIOSSTI pushback is used by nothing genuine userspace in any properly
functional case. So if that is the case unconstrained TIOSSTI
push-back would only be making application crashes worse.

The reason I want TIOSSTI pushback moved to its own CAP_SYS first is
to find out if anything is in fact using it as part of genuine usage
and allowing anyone caught out to work around it. I am sorry this
is me most likely using X11 logic break it and see if anyone yells.
If no one complains disappear the feature completely then this closes
this form of exploit for good.

Peter Dolding

[Alan Cox]

> Alan is right. CAP_SYS_ADMIN allows crossing the tty barrier.

I don't need CAP_ anything to mmap your frame buffer, or use selection to
cut and paste text into the terminal.

> Broken applications that you can wrap in a pty/tty pair as the lxc
> application does would be defeated if those applications move up to
> CAP_SYS_ADMIN. Because you have granted the high right of cross
> pty/tty containment.

Yes

> I don't know of a genuine program using push back in exploiting way
> where the pushed back input is expected to be processed after the
> program has terminated.

So there are two real problems here

1. We don't know what namespace each character belongs to, so there's no
way we can construct a model where pushed symbols only appear in the
namespace they are pushed from. That would be a nice situation but it's
not at all obvious there is a sane way to implement it.

2. Focussing on TIOCSTI is just ignoring the bigger picture. TIOCSTI is
actually a lot less nasty in many situations than a framebuffer mmap and
spying attack where a container run from the console could sit and watch
you. TIOCSTI is in some ways the easiest to fix because setsid() will let
you mitigate it in certain cases whereas I'm fairly sure the selection
based console attack doesn't need controlling terminal rights.

In the case you have a less privileged subshell you need a whole new tty
context, and there's no obvious way for the kernel to magic one into
existance so that for example the container can change it's own baud rate
but not the baud rate of any app outside the container.

ioctl whitelisting will break stuff, but an SELinux/AppArmour/seccomp
whitelisting based solution is probably the only practical one you can
implement usefully, and for a lot of container users would be ok.

And yes there are genuine users of TIOCSTI although these days most
things just use their own pty/tty pair.

Alan

[Serge E. Hallyn]

Quoting Alan Cox (gno...@lxorguk.ukuu.org.uk):
> > Alan is right. CAP_SYS_ADMIN allows crossing the tty barrier.
>
> I don't need CAP_ anything to mmap your frame buffer, or use selection to
> cut and paste text into the terminal.
>
> > Broken applications that you can wrap in a pty/tty pair as the lxc
> > application does would be defeated if those applications move up to
> > CAP_SYS_ADMIN. Because you have granted the high right of cross
> > pty/tty containment.
>
> Yes

Right.

> > I don't know of a genuine program using push back in exploiting way
> > where the pushed back input is expected to be processed after the
> > program has terminated.
>
> So there are two real problems here
>
> 1. We don't know what namespace each character belongs to, so there's no
> way we can construct a model where pushed symbols only appear in the
> namespace they are pushed from. That would be a nice situation but it's
> not at all obvious there is a sane way to implement it.
>
> 2. Focussing on TIOCSTI is just ignoring the bigger picture. TIOCSTI is
> actually a lot less nasty in many situations than a framebuffer mmap and
> spying attack where a container run from the console could sit and watch
> you. TIOCSTI is in some ways the easiest to fix because setsid() will let
> you mitigate it in certain cases whereas I'm fairly sure the selection
> based console attack doesn't need controlling terminal rights.
>
> In the case you have a less privileged subshell you need a whole new tty
> context, and there's no obvious way for the kernel to magic one into
> existance so that for example the container can change it's own baud rate
> but not the baud rate of any app outside the container.
>
> ioctl whitelisting will break stuff, but an SELinux/AppArmour/seccomp
> whitelisting based solution is probably the only practical one you can
> implement usefully, and for a lot of container users would be ok.

Seccomp policy could refuse TIOCSTI to any fd, but that's not ideal. It could
be customized per-application-start to only refuse TIOCSTI to the passed-in tty
fd, but you'd have to prevent dup then right? Selinux can label the fd so it's
in fact ideal here. But - is there something clever a seccomp policy could do
to work only on specified fds and any that were dup'ed?

Mind you, I of course agree that creating a new pty and passing only that in
is the sane fix. Maybe in the end this thread will serve best as a loud
reminder / teaching moment about this issue for those about to write such an
application.

[Kees Cook]

I still cannot wrap my head around why providing users with a
protection is a bad thing. Yes, the other tty games are bad, but this
fixes a specific and especially bad case that is easy to kill. It's
got a Kconfig and a sysctl. It's not on by default. This protects the
common case of privileged ttys that aren't attached to consoles, etc,
so while the framebuffer thing is an issue, it's not always an issue,
etc.

I'd like to hear Greg's thoughts on this series...

-Kees

--
Kees Cook
Pixel Security

[Alan Cox]

> I still cannot wrap my head around why providing users with a
> protection is a bad thing. Yes, the other tty games are bad, but this
> fixes a specific and especially bad case that is easy to kill. It's
> got a Kconfig and a sysctl. It's not on by default. This protects the
> common case of privileged ttys that aren't attached to consoles, etc,

Which just leads to stuff not getting fixed. Like all the code out there
today which is still vulnerable to selection based attacks because people
didn't do the job right when "fixing" stuff because they are not
thinking about security at a systems level but just tickboxing CVEs.

I'm not against doing something to protect the container folks, but that
something as with Android is a whitelist of ioctls. And if we need to do
this with a kernel hook lets do it properly.

Remember the namespace of the tty on creation
If the magic security flag is set then
Apply a whitelist to *any* tty ioctl call where the ns doesn't
match

and we might as well just take the Android whitelist since they've kindly
built it for us all!

In the tty layer it ends up being something around 10 lines of code and
some other file somewhere in security/ that's just a switch or similar
with the whitelisted ioctl codes in it.

That (or a similar SELinux ruleset) would actually fix the problem.
SELinux would be better because it can also apply the rules when doing
things like su/sudo/...

Alan

[Serge E. Hallyn]

Quoting Alan Cox (gno...@lxorguk.ukuu.org.uk):
> > I still cannot wrap my head around why providing users with a
> > protection is a bad thing. Yes, the other tty games are bad, but this
> > fixes a specific and especially bad case that is easy to kill. It's
> > got a Kconfig and a sysctl. It's not on by default. This protects the
> > common case of privileged ttys that aren't attached to consoles, etc,
>
> Which just leads to stuff not getting fixed. Like all the code out there
> today which is still vulnerable to selection based attacks because people
> didn't do the job right when "fixing" stuff because they are not
> thinking about security at a systems level but just tickboxing CVEs.
>
> I'm not against doing something to protect the container folks, but that
> something as with Android is a whitelist of ioctls. And if we need to do

Whitelist of ioctls (at least using seccomp) is not sufficient because
then we have to turn the ioctl always-off. But like you say we may want
to enable it for ptys which are created inside the container's user ns.

> this with a kernel hook lets do it properly.
>
> Remember the namespace of the tty on creation

Matt's patch does this,

> If the magic security flag is set then
> Apply a whitelist to *any* tty ioctl call where the ns doesn't
> match

Seems sensible.

[Kees Cook]

On Thu, Jun 1, 2017 at 6:08 AM, Alan Cox <gno...@lxorguk.ukuu.org.uk> wrote:
>> I still cannot wrap my head around why providing users with a
>> protection is a bad thing. Yes, the other tty games are bad, but this
>> fixes a specific and especially bad case that is easy to kill. It's
>> got a Kconfig and a sysctl. It's not on by default. This protects the
>> common case of privileged ttys that aren't attached to consoles, etc,
>
> Which just leads to stuff not getting fixed. Like all the code out there
> today which is still vulnerable to selection based attacks because people
> didn't do the job right when "fixing" stuff because they are not
> thinking about security at a systems level but just tickboxing CVEs.

There's a difference between "bugs" and "security bugs". Letting
security bugs continue to get exploited because we want to flush out
bugs seems insensitive to the people getting attacked. I'd rather
protect against a class of bug than have to endless fix each bug.

> I'm not against doing something to protect the container folks, but that
> something as with Android is a whitelist of ioctls. And if we need to do
> this with a kernel hook lets do it properly.
>
> Remember the namespace of the tty on creation
> If the magic security flag is set then
> Apply a whitelist to *any* tty ioctl call where the ns doesn't
> match
>
> and we might as well just take the Android whitelist since they've kindly
> built it for us all!
>
> In the tty layer it ends up being something around 10 lines of code and
> some other file somewhere in security/ that's just a switch or similar
> with the whitelisted ioctl codes in it.
>
> That (or a similar SELinux ruleset) would actually fix the problem.
> SELinux would be better because it can also apply the rules when doing
> things like su/sudo/...

Just to play devil's advocate, wouldn't such a system continue to not
address your physical-console concerns? I wouldn't want to limit the
protection to only containers (but it's a good start), since it
wouldn't protect people not using containers that still have a
privileged TTY attached badly somewhere.

If you're talking about wholistic SELinux policy, sure, I could
imagine a wholistic fix. But for the tons of people without a
comprehensive SELinux policy, the proposed protection continues to
make sense.

[Alan Cox]

On Thu, 1 Jun 2017 12:18:58 -0500

"Serge E. Hallyn" <se...@hallyn.com> wrote:

> Quoting Alan Cox (gno...@lxorguk.ukuu.org.uk):
> > > I still cannot wrap my head around why providing users with a
> > > protection is a bad thing. Yes, the other tty games are bad, but this
> > > fixes a specific and especially bad case that is easy to kill. It's
> > > got a Kconfig and a sysctl. It's not on by default. This protects the
> > > common case of privileged ttys that aren't attached to consoles, etc,
> >
> > Which just leads to stuff not getting fixed. Like all the code out there
> > today which is still vulnerable to selection based attacks because people
> > didn't do the job right when "fixing" stuff because they are not
> > thinking about security at a systems level but just tickboxing CVEs.
> >
> > I'm not against doing something to protect the container folks, but that
> > something as with Android is a whitelist of ioctls. And if we need to do
>
> Whitelist of ioctls (at least using seccomp) is not sufficient because
> then we have to turn the ioctl always-off. But like you say we may want
> to enable it for ptys which are created inside the container's user ns.
>
> > this with a kernel hook lets do it properly.
> >
> > Remember the namespace of the tty on creation
>
> Matt's patch does this,
>
> > If the magic security flag is set then
> > Apply a whitelist to *any* tty ioctl call where the ns doesn't
> > match
>
> Seems sensible.

I'm arguing that we need to swap the TIOCSTI test for a !whitelisted()
test to go with the namespace difference check. Then it makes sense
because we actually address the real problem.

Alan

[Alan Cox]

> There's a difference between "bugs" and "security bugs". Letting

Not really, it's merely a matter of severity of result. A non security
bug that hoses your hard disk is to anyone but security nutcases at
least as bad as a security hole.

> security bugs continue to get exploited because we want to flush out
> bugs seems insensitive to the people getting attacked. I'd rather
> protect against a class of bug than have to endless fix each bug.

The others are security bugs too to varying degree

> > I'm not against doing something to protect the container folks, but that
> > something as with Android is a whitelist of ioctls. And if we need to do
> > this with a kernel hook lets do it properly.
> >
> > Remember the namespace of the tty on creation
> > If the magic security flag is set then
> > Apply a whitelist to *any* tty ioctl call where the ns doesn't
> > match
> >
> > and we might as well just take the Android whitelist since they've kindly
> > built it for us all!
> >
> > In the tty layer it ends up being something around 10 lines of code and
> > some other file somewhere in security/ that's just a switch or similar
> > with the whitelisted ioctl codes in it.
> >
> > That (or a similar SELinux ruleset) would actually fix the problem.
> > SELinux would be better because it can also apply the rules when doing
> > things like su/sudo/...
>
> Just to play devil's advocate, wouldn't such a system continue to not
> address your physical-console concerns? I wouldn't want to limit the

It would for the cases that a whitelist and container check covers -
because the whitelist wouldn't allow you to do anything but boring stuff
on the tty. TIOCSTI is just one of a whole range of differently stupid
and annoying opportunities. Containers do not and should not be able to
set the keymap, change the video mode, use console selection, make funny
beepy noises, access video I/O registers and all the other stuff like
that. Nothing is going to break if we have a fairly conservative
whitelist.

> protection to only containers (but it's a good start), since it
> wouldn't protect people not using containers that still have a
> privileged TTY attached badly somewhere.

How are you going to magically fix the problem. I'm not opposed to fixing
the real problem but right now it appears to be a product of wishful
thinking not programming. What's the piece of security code that
magically discerns the fact you are running something untrusted at the
other end of your tty. SELinux can do it via labelling but I don't see
any generic automatic way for the kernel to magically work out when to
whitelist and when not to. If there is a better magic rule than
differing-namespace then provide the code.

You can't just disable TIOCSTI, it has users deal with it. You can
get away with disabling it for namespace crossing I think but if you do
that you need to disable a pile of others.

(If it breaks containers blocking TIOCSTI then we need to have a good
look at algorithms for deciding when to flush the input queue on exiting
a container or somesuch)

> If you're talking about wholistic SELinux policy, sure, I could
> imagine a wholistic fix. But for the tons of people without a
> comprehensive SELinux policy, the proposed protection continues to
> make sense.

No it doesn't. It's completely useless unless you actually bother to
address the other exploit opportunities.

Right now the proposal is a hack to do

if (TIOCSTI && different_namespace && magic_flag)

the proper solution is

if (!whitelisted(ioctl) && different_namespace && magic_flag)

The former is snake oil, the latter actually deals with the problem space
for namespaced stuff comprehensively and is only a tiny amount more code.

For non namespaced stuff it makes it no worse, if you don't allocate a
pty/tty pair properly then the gods are not going to magically save you
from on high sorry. And if you want to completely kill TIOCSTI even
though it's kind of pointless you can use seccomp.

We can make things a bit better for the non-namespaced cases by providing
a new ioctl that turns on/off whitelisting for that tty so that the
process can do

ioctl(tty_fd, TIOCTRUST, &zero);
execl("/home/foo/stupid-idea", "ooops", NULL);

that's a simple per tty flag and a trivial one condition extra check to
the test above. We do need some way to change it back however and that's
a bit trickier given we don't want the stupid-idea tool to be able to but
we do want the invoking shell to - maybe you have to be session leader ?

Alan

[Matt Brown]

This is not what my patch does. Mine is like:

if (TIOCSTI && !ns_capable(tty->owner_user_ns, CAP_SYS_ADMIN) &&
magic_flag)

in other words:
if (TIOCSTI && (different_owner_user_ns || !CAP_SYS_ADMIN) &&
magic_flag)

can you specify what you mean by different_namespace? which namespace?

The reason I brought in the user namespace was to solve an edge case
with nested containers. capable() will never be true in a container, so
nested containers would break if we didn't do the ns_capable check. This
is also the reason for the addition of owner_user_ns to tty_struct.

The point of my patch is to prevent any type of tiocsti activity where
the tty is given to an unprivileged process/container. The emphasis of
my check is on the CAP_SYS_ADMIN part, not the namespace part.

[Serge E. Hallyn]

I think you're focusing on the wrong thing. Your capable check (apart
from the fact that I think I've been convinced CAP_SYS_ADMIN is wrong)
is fine.

The key point is to not only check for TIOCSTI, but instead check for
a whitelisted ioctl.

What would the whitelist look like? Should configuing that be the way
that you enable/disable, instead of the sysctl in this patchset? So
by default the whitelist includes all ioctls (no change), but things
like sandboxes/sudo/container-starts can clear out the whitelist?

[Matt Brown]

Can't we also have a sysctl that toggles if CAP_SYS_ADMIN is involved in
this whitelist check? Otherwise someone might leave things out of the
whitelist just because they want to use those ioctls as a privileged
process. Also restricting a privileged user from ioctls with this
whitelist approach is going to be pointless because, if the whitelist is
configurable from userspace, they will just be able to modify the
whitelist.

>
> The key point is to not only check for TIOCSTI, but instead check for
> a whitelisted ioctl.
>
> What would the whitelist look like? Should configuing that be the way
> that you enable/disable, instead of the sysctl in this patchset? So
> by default the whitelist includes all ioctls (no change), but things
> like sandboxes/sudo/container-starts can clear out the whitelist?
>

I'm fine with moving this to an LSM that whitelists ioctls. I also want
to understand what a whitelist would like look and how you would
configure it? Does a sysctl that is a list of allowed ioctls work? I
don't want to just have a static whitelist that you can't change without
recompiling your kernel.

just running a sysctl -a on a linux box shows me one thing that looks
like a list: net.core.flow_limit_cpu_bitmap

[Serge E. Hallyn]

I'm cc:ing linux-api here because really we're designing an interesting
API.

> Can't we also have a sysctl that toggles if CAP_SYS_ADMIN is involved in
> this whitelist check? Otherwise someone might leave things out of the
> whitelist just because they want to use those ioctls as a privileged
> process.

I'm not quite sure what you're asking for here. Let me offer a precise
strawman design. I'm sure there are problems with it, it's just a starting
point.

system-wide whitelist (for now 'may_push_chars') is full by default.

By default, nothing changes - you can use those on your own tty, need
CAP_SYS_ADMIN against init_user_ns otherwise.

Introduce a new CAP_TTY_PRIVILEGED.

When may_push_chars is removed from the whitelist, you lose the ability
to use TIOCSTI on a tty - even your own - if you do not have CAP_TTY_PRIVILEGED
against the tty's user_ns.

> Also restricting a privileged user from ioctls with this
> whitelist approach is going to be pointless because, if the whitelist is
> configurable from userspace, they will just be able to modify the
> whitelist.
>
> >
> > The key point is to not only check for TIOCSTI, but instead check for
> > a whitelisted ioctl.
> >
> > What would the whitelist look like? Should configuing that be the way
> > that you enable/disable, instead of the sysctl in this patchset? So
> > by default the whitelist includes all ioctls (no change), but things
> > like sandboxes/sudo/container-starts can clear out the whitelist?
> >
>
> I'm fine with moving this to an LSM that whitelists ioctls. I also want

Right - what else would go into the whitelist? may_mmap?

[Matt Brown]

So is may_push_chars just an alias for TIOCSTI? Or are there some
potential whitelist members that would map to multiple ioctls?

> By default, nothing changes - you can use those on your own tty, need
> CAP_SYS_ADMIN against init_user_ns otherwise.
>
> Introduce a new CAP_TTY_PRIVILEGED.
>

I'm fine with this.

> When may_push_chars is removed from the whitelist, you lose the ability
> to use TIOCSTI on a tty - even your own - if you do not have CAP_TTY_PRIVILEGED
> against the tty's user_ns.
>

How do you propose storing/updating the whitelist? sysctl?

If it is a sysctl, would each whitelist member have a sysctl?
e.g.: kernel.ioctlwhitelist.may_push_chars = 1

Overall, I'm fine with this idea.

[Serge E. Hallyn]

Quoting Matt Brown (ma...@nmatt.com):
> On 6/2/17 12:57 PM, Serge E. Hallyn wrote:
> > I'm not quite sure what you're asking for here. Let me offer a precise
> > strawman design. I'm sure there are problems with it, it's just a starting
> > point.
> >
> > system-wide whitelist (for now 'may_push_chars') is full by default.
> >
>
> So is may_push_chars just an alias for TIOCSTI? Or are there some
> potential whitelist members that would map to multiple ioctls?

<shrug> I'm seeing it as only TIOCSTI right now.

> > By default, nothing changes - you can use those on your own tty, need
> > CAP_SYS_ADMIN against init_user_ns otherwise.
> >
> > Introduce a new CAP_TTY_PRIVILEGED.
> >
>
> I'm fine with this.
>
> > When may_push_chars is removed from the whitelist, you lose the ability
> > to use TIOCSTI on a tty - even your own - if you do not have CAP_TTY_PRIVILEGED
> > against the tty's user_ns.
> >
>
> How do you propose storing/updating the whitelist? sysctl?
>
> If it is a sysctl, would each whitelist member have a sysctl?
> e.g.: kernel.ioctlwhitelist.may_push_chars = 1
>
> Overall, I'm fine with this idea.

That sounds reasonable. Or a securityfs file - I guess not everyone
has securityfs, but if it were to become part of YAMA then that would
work.

-serge

[Kees Cook]

> Yama doesn't depend on securityfs does it?
>
> What do other people think? Should this be an addition to YAMA or its
> own thing?
>
> Alan Cox: what do you think of the above ioctl whitelisting scheme?

It's easy to stack LSMs, so since Yama is ptrace-focused, perhaps make
a separate one for TTY hardening?

[Matt Brown]

Sounds good. I also like the idea of them being separate.

Matt Brown

[Matt Brown]

[Alan Cox]

> Can't we also have a sysctl that toggles if CAP_SYS_ADMIN is involved in
> this whitelist check? Otherwise someone might leave things out of the
> whitelist just because they want to use those ioctls as a privileged
> process. Also restricting a privileged user from ioctls with this
> whitelist approach is going to be pointless because, if the whitelist is
> configurable from userspace, they will just be able to modify the
> whitelist.

Something like CAP_SYS_ADMIN is fine if you must have it configurable on
the grounds that CAP_SYS_ADMIN will let you override the list anyway.

> I'm fine with moving this to an LSM that whitelists ioctls. I also want
> to understand what a whitelist would like look and how you would
> configure it? Does a sysctl that is a list of allowed ioctls work? I
> don't want to just have a static whitelist that you can't change without
> recompiling your kernel.

That's odd because your previous argument was for a fixed one entry
whitelist containing TIOCSTI 8)

The list can probably be fixed. IMHO those wanting to do cleverer stuff
sre inevitably going to end up using a "grown up" LSM for the job because

a) they'll want to shape things like su not just containers
b) they will have cases where you want to lock down cleverly - eg you can
disable TIOCSTI use except by certain apps, and make those apps non
ptraceable so only existing real users of it can continue to use it.

For the whitelist you want most of the standard tty ioctls, but none of
the device specific ones, none of the hardware configuration ones, no
TIOCSTI, no selection, no line discipline change (or you can set a
network ldisc or various other evil and fascinating things).

I really think that for container type uses the whitelist can be fairly
clearly defined because we know a container isn't supposed to be screwing
with the hardware of the parent, configuring network interfaces or
pushing data to trip people up.

If we are prepared to accept nuisance attacks (messing up baud rate,
hangup, etc) then it's fairly easy to fix.

So I'd say it's all the generic tty ioctls except TIOCSTI and TIOCSETD
but it would be good to see what Android is going with and why.

You can still do some typeback stuff even then because the consoles and
terminals have ranges of query and requests you can trigger. As you can
use termios to absorb some symbols in the reply and map which one to use
for newline you can even type stuff. However it's very limited - hex
digits, [, c and some other oddments. People have looked hard at that and
I've yet to see a successful attack. Yes I can make you run test (aka
'[') but I've yet to see a way to use it for anything.

Alan

[Nick Kralevich]

On Fri, Jun 2, 2017 at 1:05 PM, Alan Cox <gno...@lxorguk.ukuu.org.uk> wrote:
> So I'd say it's all the generic tty ioctls except TIOCSTI and TIOCSETD
> but it would be good to see what Android is going with and why.

Android limits tty ioctls to the following whitelist:
TIOCOUTQ
FIOCLEX
TCGETS
TCSETS
TIOCGWINSZ
TIOCSWINSZ
TIOCSCTTY
TCSETSW
TCFLSH
TIOCSPGRP
TIOCGPGRP

See unpriv_tty_ioctls at
https://android.googlesource.com/platform/system/sepolicy/+/34b4b73729b288b4109b2225c1445eb58393b8cb/public/ioctl_macros#51


--
Nick Kralevich | Android Security | n...@google.com | 650.214.4037

[Matt Brown]

On 6/2/17 4:05 PM, Alan Cox wrote:
>> Can't we also have a sysctl that toggles if CAP_SYS_ADMIN is involved in
>> this whitelist check? Otherwise someone might leave things out of the
>> whitelist just because they want to use those ioctls as a privileged
>> process. Also restricting a privileged user from ioctls with this
>> whitelist approach is going to be pointless because, if the whitelist is
>> configurable from userspace, they will just be able to modify the
>> whitelist.
>
> Something like CAP_SYS_ADMIN is fine if you must have it configurable on
> the grounds that CAP_SYS_ADMIN will let you override the list anyway.
>
>> I'm fine with moving this to an LSM that whitelists ioctls. I also want
>> to understand what a whitelist would like look and how you would
>> configure it? Does a sysctl that is a list of allowed ioctls work? I
>> don't want to just have a static whitelist that you can't change without
>> recompiling your kernel.
>
> That's odd because your previous argument was for a fixed one entry
> whitelist containing TIOCSTI 8)
>

I just take some convincing that's all ;)

> The list can probably be fixed. IMHO those wanting to do cleverer stuff
> sre inevitably going to end up using a "grown up" LSM for the job because
>

I really want it to be more flexible if we are making this into a full
blown LSM. From drivers/tty/tty_ioctl.c I gather the following tty
ioctls:

TCGETA
TCGETS
TCGETS2
TCGETX
TCSETA
TCSETAF
TCSETAW
TCSETS
TCSETS2
TCSETSF
TCSETSF2
TCSETSW
TCSETSW2
TCSETX
TCSETXF
TCSETXW
TIOCGETC
TIOCGETP
TIOCGLCKTRMIOS
TIOCGLTC
TIOCGSOFTCAR
TIOCSETC
TIOCSETN
TIOCSETP
TIOCSLCKTRMIOS
TIOCSLTC
TIOCSSOFTCAR

would it be overkill to have a sysctl kernel.ttyioctlwhitelist.X where X
is one of the ioctls above?

(definitely will work on a better name than ttyioctlwhitelist)

> a) they'll want to shape things like su not just containers
> b) they will have cases where you want to lock down cleverly - eg you can
> disable TIOCSTI use except by certain apps, and make those apps non
> ptraceable so only existing real users of it can continue to use it.

I agree. When you have those kinds of requirements, a MAC is required.

[Alan Cox]

> TIOCSLCKTRMIOS

That one I'm more dubious about

> TIOCSLTC
> TIOCSSOFTCAR

tty_io.c also has a few and n_tty has a couple we'd want.

>
> would it be overkill to have a sysctl kernel.ttyioctlwhitelist.X where X
> is one of the ioctls above?

Why would anyone want to change the entries on that list

Alan

[Matt Brown]

Did you see Serge's proposed solution? I want us to not be talking past
each other. Serge proposed the following:

| By default, nothing changes - you can use those on your own tty, need
| CAP_SYS_ADMIN against init_user_ns otherwise.
|
| Introduce a new CAP_TTY_PRIVILEGED.
|

| When may_push_chars is removed from the whitelist, you lose the
| ability to use TIOCSTI on a tty - even your own - if you do not have
| CAP_TTY_PRIVILEGED against the tty's user_ns.

The question is how do you add/remove something from this whitelist? I
assume by add/remove we don't mean that you have to recompile your
kernel to change the whitelist!

you earlier said you wanted the check to look like this:

| if (!whitelisted(ioctl) && different_namespace && magic_flag)

I want to know which namespace you are talking about here. Did you mean
user_namespace? (the namespace I added tracking for in the tty_struct)

[Peter Dolding]

There are many ways to attempt to cure this problem. They some
that are just wrong.

Pushing stuff up to CAP_SYS_ADMIN is fairly much always wrong.

Using a whitelisted solution does have a downside but to use some
application that use TIOCSTI safely I have not had to push application
to CAP_SYS_ADMIN.

Another question due to the way the exploit work a broken TIOCSTI
where push back could be something someone as CAP_SYS_ADMIN run.

What I don't know if yet set when ever an application used TIOCSTI to
push back chars back into input that this would set input to be
flushed on tty disconnect or application termination would this break
any applications.

So it may be possible to allow applications to freely use TIOCSTI just
make sure that anything an application has pushed back into input
buffer cannot get to anything else.

The thing to remember is most times when applications are controlling
other applications they are not pushing data backwards on input..

Question I have is what is valid usage cases of TIOCSTI. Thinking
grscecurity got away with pushing this up to CAP_SYS_ADMIN there may
not be many.

If there is no valid usage of TIOCSTI across applications there is no
reason why TIOCSTI cannot be setup to automatically trigger input
flushs to prevent TIOCSTI inserted data getting anywhere.
.
This could be like X11 and it huge number of features where large
number were found that no one ever used just was created that way
because it was though like it would be useful.

My problem here is TIOCSTI might not need a flag at all. TIOCSTI
functionality maybe in need of limitations particularly if TIOCSTI
push back into input cross from one application to the next has no
genuine application usage.

So far no one has started that exploited TIOCSTI functionality exists
in any genuine application as expected functionality. I cannot find
example of where pushing back into input then going to background or
dieing/exiting and having that pushed back input processed is done by
any genuine application as expected functionality. That is something
that could be limited if there is no genuine users and close the door
without having to modify existing applications that don't expect to-do
that.

Its really simple to get focused in on quick fix to problems without
asking is the behaviour even required.

Peter Dolding

[Boris Lukashev]

On Mon, May 29, 2017 at 8:27 PM, Casey Schaufler <ca...@schaufler-ca.com> 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.
>

First off, thank you for the clear and rational responses - newbie
status in these upstreaming/kernel matters has a bit of a steep
learning curve.
I would however, have to disagree with the above in that there is a
very large number of purpose built Linux systems in play, home routers
being a good example, which effectively retain the same security
posture over their lifetime in an increasingly hostile operating
environment. Mitigation at every tier of the attack cycle is desirable
as a (configurable) default such as to at least reduce the impact of
compromise (they may leak memory containing the admin cred for the
web ui via some protocol error, but dont get shells, local privs, and
raw device access). Then there are all the servers out there which
would benefit from this, as they dont use those components but do
expose TTYs to dangerous consumers, and even the workstations in a
similar boat where they dont depend on faulty consumers but are
operated by faulty users. Devil's in the details right? Ideally, if
properly designed with input from greybeards, faults can be mostly
nullified and edge cases addressed with adjacent maintainers.

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

Wouldn't dream of taking that bet - the vast majority of Linux users
dont even know they're Linux users :). Disagree with the latter part
though, the majority of people rely on the appropriate organs of
society (military, police, security-focused devs) to provide for their
safety; they not only take for granted but largely abide by the
constraints placed by those in the know such as to ensure that safety.
A cynical example being that you may not know exactly what the stuff
in that hazmat-labled container will do to you (may make you into
Wolverine, or melt your bones), but you're not likely to open it and
take a sip - you trust the people who sealed it to know enough of the
details to have made that decision. Implementing changes like this
(properly scoped and implemented, as the refinement process seems to
be doing) makes a lot of sense top-down as it forces consumers to do
things the right way instead of waiting for them before adopting a
useful function.

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

Back to the newbie status bit, i've read and heard (in varying degrees
of satisfaction and frustration) that upstream makes it hard to adopt
significant changes as a culture. Obviously there are practical
concerns around compatibility, but there must be some avenue to have a
clear and rational discussion with Olympus about inducing a well
planned and short period of churn to affect changes which will greatly
extend the iconic notion of systems running stable and safe for years
at a time...
In practical terms, distributions ship tons of patches for kernel bugs
faster than you can reload a blunderbuss. While that is good practice,
and should continue, it results in constant operating efforts on the
parts of the consumer having to "manage their updates" since the bug
isn't restricted in access vectors or efficacy by a global defensive
measure, but requires direct patching to mitigate. Compliance beholden
systems, which tend to be in the critical path, suffer obligatory
burdens from the status quo - find me a hospital IT security manager
who hasn't thought of running (himself or his boss) head first through
his office window since the start of the quarter, and i'll betcha he
started work Friday.
A "security clamp-down" would be great if it were well coordinated
with the ecosystem such as to have bugs found by compile-time
approaches immediately addressed by their owning maintainers, quickly
spread use of read-only and randomized structures, memory allocations,
or whatever other defenses are implemented at the core/LSM
infrastructure tiers. Once the large changes are in, things go back to
business-as-usual, but without the ops engineers being restricted to
eating with dull plastic spoons.

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

Understood and agreed - getting the right scope and constraints for
the job is imperative for proper design and implementation of the
solution. As i'm reading the evolution of this thread i'm learning how
submissions evolve to fit the needs defined by members. Sort of rare
to see a process involving multiple participants these days which
hasn't devolved into a sandy goat-roping procedure.

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

Here i again, have to respectfully disagree. Waiting on "significant
interest" to patch their workstations and servers is what got a chunk
of the world looking for backups and coughing up bitcoins recently. If
there is a viable threat model, it should be addressed proactively, as
opposed to potential operating bugs which are shaken out in the course
of ops (which is how i understand current triage to work for bugs -
once its found, not potentially because conditions could create it).
Security as an afterthought can be seen as a form of complacency, and
as we've all read - "complacency kills."

>>
>> On Mon, May 29, 2017 at 6:26 PM, Alan Cox <gno...@lxorguk.ukuu.org.uk> wrote:
>>> On Mon, 29 May 2017 17:38:00 -0400
>>> Matt Brown <ma...@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
>>
>>
>