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 | 1 +
include/linux/tty.h | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e6d1a65..e774385 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3191,6 +3191,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 = 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
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 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..c15c660 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 otherprocesses
+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 e774385..2e15dca 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2296,11 +2296,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
On Sun, Apr 23, 2017 at 9:24 AM, Matt Brown <[email protected]> wrote:
> 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 | 1 +
> include/linux/tty.h | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index e6d1a65..e774385 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -3191,6 +3191,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 = current_user_ns();
Why are you not taking a reference to the userns?
On 04/23/2017 01:02 PM, Jann Horn wrote:
> On Sun, Apr 23, 2017 at 9:24 AM, Matt Brown <[email protected]> wrote:
>> 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 | 1 +
>> include/linux/tty.h | 2 ++
>> 2 files changed, 3 insertions(+)
>>
>> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
>> index e6d1a65..e774385 100644
>> --- a/drivers/tty/tty_io.c
>> +++ b/drivers/tty/tty_io.c
>> @@ -3191,6 +3191,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 = current_user_ns();
>
> Why are you not taking a reference to the userns?
>
current_user_ns() returns the user namespace of the current task. What
do you mean "not taking a reference to the userns?"
On Sun, Apr 23, 2017 at 10:23 PM, Matt Brown <[email protected]> wrote:
> On 04/23/2017 01:02 PM, Jann Horn wrote:
>>
>> On Sun, Apr 23, 2017 at 9:24 AM, Matt Brown <[email protected]> wrote:
>>>
>>> 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 | 1 +
>>> include/linux/tty.h | 2 ++
>>> 2 files changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
>>> index e6d1a65..e774385 100644
>>> --- a/drivers/tty/tty_io.c
>>> +++ b/drivers/tty/tty_io.c
>>> @@ -3191,6 +3191,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 = current_user_ns();
>>
>>
>> Why are you not taking a reference to the userns?
>>
>
> current_user_ns() returns the user namespace of the current task. What
> do you mean "not taking a reference to the userns?"
For managing the lifetime of various object types, including
struct user_namespace, the kernel uses reference counting.
You can see that struct user_namespace has a member named
"count" of type atomic_t. This member tracks how many
references to a user namespace exist in the kernel, not counting
some types of temporary references.
Basically, when this counter reaches zero, the kernel assumes that
nothing uses the namespace anymore and frees the
struct user_namespace. Attempting to use the struct user_namespace
after that would cause a use-after-free bug.
This means that when you store a reference to a
user_namespace in a tty_struct, you need to increment the "count"
member of the user_namespace ("take a reference"), and when the
tty_struct is released, you need to decrement the "count" member of
the user_namespace back down ("drop a reference"). To change the
count, you should use the helper functions get_user_ns() and
put_user_ns().