On 04/19/2017 07:53 PM, Serge E. Hallyn wrote:
Quoting Matt Brown (m...@nmatt.com):
On 04/19/2017 12:58 AM, Serge E. Hallyn wrote:
On Tue, Apr 18, 2017 at 11:45:26PM -0400, Matt Brown wrote:
This patch reproduces GRKERNSEC_HARDEN_TTY functionality from the grsecurity
project in-kernel.
This will create the Kconfig SECURITY_TIOCSTI_RESTRICT and the corresponding
sysctl kernel.tiocsti_restrict that, when activated, restrict all TIOCSTI
ioctl calls from non CAP_SYS_ADMIN users.
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
It's not worthless, but note that for instance before this was fixed
in lxc, this patch would not have helped with escapes from privileged
containers.
I assume you are talking about this CVE:
https://bugzilla.redhat.com/show_bug.cgi?id=1411256
In retrospect, is there any way that an escape from a privileged
container with the this bug could have been prevented?
I don't know, that's what I was probing for. Detecting that the pgrp
or session - heck, the pid namespace - has changed would seem like a
good indicator that it shouldn't be able to push.
pgrp and session won't do because in the case we are discussing
current->signal->tty is the same as tty.
This is the current check that is already in place:
| if ((current->signal->tty != tty) && !capable(CAP_SYS_ADMIN))
| return -EPERM;
The only thing I could find to detect the tty message coming from a
container is as follows:
| task_active_pid_ns(current)->level
This will be zero when run on the host, but 1 when run inside a
container. However this is very much a hack and could probably break
some userland stuff where there are multiple levels of namespaces.
The real problem is that there are no TTY namespaces. I don't think we
can solve this problem for CAP_SYS_ADMIN containers unless we want to
introduce a config that allows one to override normal CAP_SYS_ADMIN
functionality by denying TIOCSTI ioctls for processes whom
task_active_pid_ns(current)->level is equal to 0.
In the mean time, I think we can go ahead with this feature to give
people the ability to lock down non CAP_SYS_ADMIN containers/processes.
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/>
Signed-off-by: Matt Brown <m...@nmatt.com>
---
drivers/tty/tty_io.c | 4 ++++
include/linux/tty.h | 2 ++
kernel/sysctl.c | 12 ++++++++++++
security/Kconfig | 13 +++++++++++++
4 files changed, 31 insertions(+)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e6d1a65..31894e8 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2296,11 +2296,15 @@ 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 && !capable(CAP_SYS_ADMIN))
+ 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 1017e904..7011102 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -342,6 +342,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
This is an odd way to name this. Shouldn't the name reflect that it
is setting the default, rather than enabling the feature?
Besides that, I'm ok with the patch.
+ 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
