[PATCH v4 0/2] tiocsti-restrict : make TIOCSTI ioctl require CAP_SYS_ADMIN

[PATCH v4 0/2] tiocsti-restrict : 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 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

[PATCH v4 0/2] tiocsti-restrict : 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 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

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] [PATCH v4 0/2] tiocsti-restrict : 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 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

[PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

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

[PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

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

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

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

[PATCH v4 2/2] tiocsti-restrict : make TIOCSTI ioctl require CAP_SYS_ADMIN

[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

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

[PATCH v4 2/2] tiocsti-restrict : make TIOCSTI ioctl require CAP_SYS_ADMIN

[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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] [PATCH v4 2/2] tiocsti-restrict : make TIOCSTI ioctl require CAP_SYS_ADMIN

[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

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

Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Serge E. Hallyn]

Quoting Matt Brown (matt@nmatt.com):
> 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 <matt@nmatt.com>

Acked-by: Serge Hallyn <serge@hallyn.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

[PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Serge E. Hallyn]

Quoting Matt Brown (matt at nmatt.com):
> 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 <matt@nmatt.com>

Acked-by: Serge Hallyn <serge@hallyn.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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Serge E. Hallyn]

Quoting Matt Brown (matt@nmatt.com):
> 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 <matt@nmatt.com>

Acked-by: Serge Hallyn <serge@hallyn.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

Re: [PATCH v4 2/2] tiocsti-restrict : make TIOCSTI ioctl require CAP_SYS_ADMIN

[Serge E. Hallyn]

Quoting Matt Brown (matt@nmatt.com):
> 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 <matt@nmatt.com>

Acked-by: Serge Hallyn <serge@hallyn.com>

(typo below)

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

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

[PATCH v4 2/2] tiocsti-restrict : make TIOCSTI ioctl require CAP_SYS_ADMIN

[Serge E. Hallyn]

Quoting Matt Brown (matt at nmatt.com):
> 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 <matt@nmatt.com>

Acked-by: Serge Hallyn <serge@hallyn.com>

(typo below)

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

space between 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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v4 2/2] tiocsti-restrict : make TIOCSTI ioctl require CAP_SYS_ADMIN

[Serge E. Hallyn]

Quoting Matt Brown (matt@nmatt.com):
> 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 <matt@nmatt.com>

Acked-by: Serge Hallyn <serge@hallyn.com>

(typo below)

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

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

Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Alan Cox]

On Mon, 24 Apr 2017 01:15:11 -0400
Matt Brown <matt@nmatt.com> 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.

Regardles of the TIOCSTI usefulness this makes complete sense.

Alan

[PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Alan Cox]

On Mon, 24 Apr 2017 01:15:11 -0400
Matt Brown <matt@nmatt.com> 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.

Regardles of the TIOCSTI usefulness this makes complete sense.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Alan Cox]

On Mon, 24 Apr 2017 01:15:11 -0400
Matt Brown <matt@nmatt.com> 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.

Regardles of the TIOCSTI usefulness this makes complete sense.

Alan

Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Kees Cook]

On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> Quoting Matt Brown (matt@nmatt.com):
>> 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 <matt@nmatt.com>
>
> Acked-by: Serge Hallyn <serge@hallyn.com>

This Ack didn't end up in the v5, but I think it stands, yes?

Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
included be preferred?

-Kees

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



-- 
Kees Cook
Pixel Security

[PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Kees Cook]

On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> Quoting Matt Brown (matt at nmatt.com):
>> 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 <matt@nmatt.com>
>
> Acked-by: Serge Hallyn <serge@hallyn.com>

This Ack didn't end up in the v5, but I think it stands, yes?

Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
included be preferred?

-Kees

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



-- 
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Kees Cook]

On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> Quoting Matt Brown (matt@nmatt.com):
>> 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 <matt@nmatt.com>
>
> Acked-by: Serge Hallyn <serge@hallyn.com>

This Ack didn't end up in the v5, but I think it stands, yes?

Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
included be preferred?

-Kees

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



-- 
Kees Cook
Pixel Security

Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Greg KH]

On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> > Quoting Matt Brown (matt@nmatt.com):
> >> 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 <matt@nmatt.com>
> >
> > Acked-by: Serge Hallyn <serge@hallyn.com>
> 
> This Ack didn't end up in the v5, but I think it stands, yes?
> 
> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
> included be preferred?

v6 would be great, and we are dropping patch 2 from the series, right?
I was expecting this to be resent.  I'll start looking at new patches
like this after 4.12-rc1 is out.

thanks,

greg k-h

[PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Greg KH]

On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> > Quoting Matt Brown (matt at nmatt.com):
> >> 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 <matt@nmatt.com>
> >
> > Acked-by: Serge Hallyn <serge@hallyn.com>
> 
> This Ack didn't end up in the v5, but I think it stands, yes?
> 
> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
> included be preferred?

v6 would be great, and we are dropping patch 2 from the series, right?
I was expecting this to be resent.  I'll start looking at new patches
like this after 4.12-rc1 is out.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Greg KH]

On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> > Quoting Matt Brown (matt@nmatt.com):
> >> 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 <matt@nmatt.com>
> >
> > Acked-by: Serge Hallyn <serge@hallyn.com>
> 
> This Ack didn't end up in the v5, but I think it stands, yes?
> 
> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
> included be preferred?

v6 would be great, and we are dropping patch 2 from the series, right?
I was expecting this to be resent.  I'll start looking at new patches
like this after 4.12-rc1 is out.

thanks,

greg k-h

Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Matt Brown]

On 05/03/2017 03:45 PM, Greg KH wrote:
> On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
>> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
>>> Quoting Matt Brown (matt@nmatt.com):
>>>> 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 <matt@nmatt.com>
>>>
>>> Acked-by: Serge Hallyn <serge@hallyn.com>
>>
>> This Ack didn't end up in the v5, but I think it stands, yes?
>>
>> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
>> included be preferred?
>
> v6 would be great, and we are dropping patch 2 from the series, right?
> I was expecting this to be resent.  I'll start looking at new patches
> like this after 4.12-rc1 is out.
>

I will create a v6 with the Acks/Reviews. I'd like to keep patch 2 in
since that got acked by at least Serge. (Kees also? or just patch 1?)

> thanks,
>
> greg k-h
>

[PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Matt Brown]

On 05/03/2017 03:45 PM, Greg KH wrote:
> On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
>> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
>>> Quoting Matt Brown (matt at nmatt.com):
>>>> 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 <matt@nmatt.com>
>>>
>>> Acked-by: Serge Hallyn <serge@hallyn.com>
>>
>> This Ack didn't end up in the v5, but I think it stands, yes?
>>
>> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
>> included be preferred?
>
> v6 would be great, and we are dropping patch 2 from the series, right?
> I was expecting this to be resent.  I'll start looking at new patches
> like this after 4.12-rc1 is out.
>

I will create a v6 with the Acks/Reviews. I'd like to keep patch 2 in
since that got acked by at least Serge. (Kees also? or just patch 1?)

> thanks,
>
> greg k-h
>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Matt Brown]

On 05/03/2017 03:45 PM, Greg KH wrote:
> On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
>> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
>>> Quoting Matt Brown (matt@nmatt.com):
>>>> 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 <matt@nmatt.com>
>>>
>>> Acked-by: Serge Hallyn <serge@hallyn.com>
>>
>> This Ack didn't end up in the v5, but I think it stands, yes?
>>
>> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
>> included be preferred?
>
> v6 would be great, and we are dropping patch 2 from the series, right?
> I was expecting this to be resent.  I'll start looking at new patches
> like this after 4.12-rc1 is out.
>

I will create a v6 with the Acks/Reviews. I'd like to keep patch 2 in
since that got acked by at least Serge. (Kees also? or just patch 1?)

> thanks,
>
> greg k-h
>

Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Kees Cook]

On Wed, May 3, 2017 at 1:02 PM, Matt Brown <matt@nmatt.com> wrote:
> On 05/03/2017 03:45 PM, Greg KH wrote:
>>
>> On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
>>>
>>> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com>
>>> wrote:
>>>>
>>>> Quoting Matt Brown (matt@nmatt.com):
>>>>>
>>>>> 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 <matt@nmatt.com>
>>>>
>>>>
>>>> Acked-by: Serge Hallyn <serge@hallyn.com>
>>>
>>>
>>> This Ack didn't end up in the v5, but I think it stands, yes?
>>>
>>> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
>>> included be preferred?
>>
>>
>> v6 would be great, and we are dropping patch 2 from the series, right?
>> I was expecting this to be resent.  I'll start looking at new patches
>> like this after 4.12-rc1 is out.
>>
>
> I will create a v6 with the Acks/Reviews. I'd like to keep patch 2 in
> since that got acked by at least Serge. (Kees also? or just patch 1?)

Sorry, I meant that patch 2's ack from serge got dropped accidentally.
i.e. he Acked v4, but it wasn't in v5.

Serge, just to double-check, does your Ack stand?

-Kees

-- 
Kees Cook
Pixel Security

[PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Kees Cook]

On Wed, May 3, 2017 at 1:02 PM, Matt Brown <matt@nmatt.com> wrote:
> On 05/03/2017 03:45 PM, Greg KH wrote:
>>
>> On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
>>>
>>> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com>
>>> wrote:
>>>>
>>>> Quoting Matt Brown (matt at nmatt.com):
>>>>>
>>>>> 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 <matt@nmatt.com>
>>>>
>>>>
>>>> Acked-by: Serge Hallyn <serge@hallyn.com>
>>>
>>>
>>> This Ack didn't end up in the v5, but I think it stands, yes?
>>>
>>> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
>>> included be preferred?
>>
>>
>> v6 would be great, and we are dropping patch 2 from the series, right?
>> I was expecting this to be resent.  I'll start looking at new patches
>> like this after 4.12-rc1 is out.
>>
>
> I will create a v6 with the Acks/Reviews. I'd like to keep patch 2 in
> since that got acked by at least Serge. (Kees also? or just patch 1?)

Sorry, I meant that patch 2's ack from serge got dropped accidentally.
i.e. he Acked v4, but it wasn't in v5.

Serge, just to double-check, does your Ack stand?

-Kees

-- 
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kernel-hardening] Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Kees Cook]

On Wed, May 3, 2017 at 1:02 PM, Matt Brown <matt@nmatt.com> wrote:
> On 05/03/2017 03:45 PM, Greg KH wrote:
>>
>> On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
>>>
>>> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com>
>>> wrote:
>>>>
>>>> Quoting Matt Brown (matt@nmatt.com):
>>>>>
>>>>> 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 <matt@nmatt.com>
>>>>
>>>>
>>>> Acked-by: Serge Hallyn <serge@hallyn.com>
>>>
>>>
>>> This Ack didn't end up in the v5, but I think it stands, yes?
>>>
>>> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
>>> included be preferred?
>>
>>
>> v6 would be great, and we are dropping patch 2 from the series, right?
>> I was expecting this to be resent.  I'll start looking at new patches
>> like this after 4.12-rc1 is out.
>>
>
> I will create a v6 with the Acks/Reviews. I'd like to keep patch 2 in
> since that got acked by at least Serge. (Kees also? or just patch 1?)

Sorry, I meant that patch 2's ack from serge got dropped accidentally.
i.e. he Acked v4, but it wasn't in v5.

Serge, just to double-check, does your Ack stand?

-Kees

-- 
Kees Cook
Pixel Security

Re: [kernel-hardening] Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Serge E. Hallyn]

On Wed, May 03, 2017 at 01:19:41PM -0700, Kees Cook wrote:
> On Wed, May 3, 2017 at 1:02 PM, Matt Brown <matt@nmatt.com> wrote:
> > On 05/03/2017 03:45 PM, Greg KH wrote:
> >>
> >> On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
> >>>
> >>> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com>
> >>> wrote:
> >>>>
> >>>> Quoting Matt Brown (matt@nmatt.com):
> >>>>>
> >>>>> 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 <matt@nmatt.com>
> >>>>
> >>>>
> >>>> Acked-by: Serge Hallyn <serge@hallyn.com>
> >>>
> >>>
> >>> This Ack didn't end up in the v5, but I think it stands, yes?
> >>>
> >>> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
> >>> included be preferred?
> >>
> >>
> >> v6 would be great, and we are dropping patch 2 from the series, right?
> >> I was expecting this to be resent.  I'll start looking at new patches
> >> like this after 4.12-rc1 is out.
> >>
> >
> > I will create a v6 with the Acks/Reviews. I'd like to keep patch 2 in
> > since that got acked by at least Serge. (Kees also? or just patch 1?)
> 
> Sorry, I meant that patch 2's ack from serge got dropped accidentally.
> i.e. he Acked v4, but it wasn't in v5.
> 
> Serge, just to double-check, does your Ack stand?

Yes.

thanks,
-serge

[kernel-hardening] Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Serge E. Hallyn]

On Wed, May 03, 2017 at 01:19:41PM -0700, Kees Cook wrote:
> On Wed, May 3, 2017 at 1:02 PM, Matt Brown <matt@nmatt.com> wrote:
> > On 05/03/2017 03:45 PM, Greg KH wrote:
> >>
> >> On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
> >>>
> >>> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com>
> >>> wrote:
> >>>>
> >>>> Quoting Matt Brown (matt at nmatt.com):
> >>>>>
> >>>>> 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 <matt@nmatt.com>
> >>>>
> >>>>
> >>>> Acked-by: Serge Hallyn <serge@hallyn.com>
> >>>
> >>>
> >>> This Ack didn't end up in the v5, but I think it stands, yes?
> >>>
> >>> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
> >>> included be preferred?
> >>
> >>
> >> v6 would be great, and we are dropping patch 2 from the series, right?
> >> I was expecting this to be resent.  I'll start looking at new patches
> >> like this after 4.12-rc1 is out.
> >>
> >
> > I will create a v6 with the Acks/Reviews. I'd like to keep patch 2 in
> > since that got acked by at least Serge. (Kees also? or just patch 1?)
> 
> Sorry, I meant that patch 2's ack from serge got dropped accidentally.
> i.e. he Acked v4, but it wasn't in v5.
> 
> Serge, just to double-check, does your Ack stand?

Yes.

thanks,
-serge
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kernel-hardening] Re: [PATCH v4 1/2] tiocsti-restrict : Add owner user namespace to tty_struct

[Serge E. Hallyn]

On Wed, May 03, 2017 at 01:19:41PM -0700, Kees Cook wrote:
> On Wed, May 3, 2017 at 1:02 PM, Matt Brown <matt@nmatt.com> wrote:
> > On 05/03/2017 03:45 PM, Greg KH wrote:
> >>
> >> On Wed, May 03, 2017 at 12:32:07PM -0700, Kees Cook wrote:
> >>>
> >>> On Mon, Apr 24, 2017 at 6:57 AM, Serge E. Hallyn <serge@hallyn.com>
> >>> wrote:
> >>>>
> >>>> Quoting Matt Brown (matt@nmatt.com):
> >>>>>
> >>>>> 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 <matt@nmatt.com>
> >>>>
> >>>>
> >>>> Acked-by: Serge Hallyn <serge@hallyn.com>
> >>>
> >>>
> >>> This Ack didn't end up in the v5, but I think it stands, yes?
> >>>
> >>> Greg, is the v5 okay to pull for you or would a v6 with Acks/Reviews
> >>> included be preferred?
> >>
> >>
> >> v6 would be great, and we are dropping patch 2 from the series, right?
> >> I was expecting this to be resent.  I'll start looking at new patches
> >> like this after 4.12-rc1 is out.
> >>
> >
> > I will create a v6 with the Acks/Reviews. I'd like to keep patch 2 in
> > since that got acked by at least Serge. (Kees also? or just patch 1?)
> 
> Sorry, I meant that patch 2's ack from serge got dropped accidentally.
> i.e. he Acked v4, but it wasn't in v5.
> 
> Serge, just to double-check, does your Ack stand?

Yes.

thanks,
-serge