2017-04-25 04:16:09

by Matt Brown

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

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


2017-04-25 04:16:31

by Matt Brown

[permalink] [raw]
Subject: [PATCH v5 1/2] security: tty: Add owner user namespace to tty_struct

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 <[email protected]>
---
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

2017-04-25 04:16:50

by Matt Brown

[permalink] [raw]
Subject: [PATCH v5 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

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

This patch depends on patch 1/2

This patch was inspired from GRKERNSEC_HARDEN_TTY.

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

Possible effects on userland:

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

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

Threat Model/Patch Rational:

>From grsecurity's config for GRKERNSEC_HARDEN_TTY.

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

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

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

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

Signed-off-by: Matt Brown <[email protected]>
---
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

2017-04-25 13:48:30

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

> 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

2017-04-25 13:57:02

by Jann Horn

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On Tue, Apr 25, 2017 at 3:47 PM, Alan Cox <[email protected]> wrote:
>> 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().

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.

2017-04-25 19:31:32

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On Tue, 25 Apr 2017 15:56:32 +0200
Jann Horn <[email protected]> wrote:

> On Tue, Apr 25, 2017 at 3:47 PM, Alan Cox <[email protected]> wrote:
> >> 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().
>
> 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.

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

2017-04-25 20:07:23

by Jann Horn

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On Tue, Apr 25, 2017 at 9:30 PM, One Thousand Gnomes
<[email protected]> wrote:
> On Tue, 25 Apr 2017 15:56:32 +0200
> Jann Horn <[email protected]> wrote:
>
>> On Tue, Apr 25, 2017 at 3:47 PM, Alan Cox <[email protected]> wrote:
>> >> 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().
>>
>> 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.
>
> 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.

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.
* [...]
*/


> 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

2017-04-25 21:22:41

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

> 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

2017-04-25 21:45:20

by Jann Horn

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On Tue, Apr 25, 2017 at 11:21 PM, One Thousand Gnomes
<[email protected]> 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.]

2017-04-26 12:48:32

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

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

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

2017-04-26 14:22:36

by Matt Brown

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

On 04/26/2017 08:47 AM, One Thousand Gnomes wrote:
>> 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.
>>
>>>>> 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.
>
> 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.

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

2017-04-27 12:35:18

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

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