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

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

[PATCH v3 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 | 4 ++++
 include/linux/tty.h  | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e6d1a65..03d5ea2 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -161,6 +161,7 @@ static void release_tty(struct tty_struct *tty, int idx);
  *	@tty: tty struct to free
  *
  *	Free the write buffers, tty queue and tty memory itself.
+ *	Decrement the owner_user_ns count.
  *
  *	Locking: none. Must be called after tty is definitely unused
  */
@@ -171,6 +172,7 @@ static void free_tty_struct(struct tty_struct *tty)
 	put_device(tty->dev);
 	kfree(tty->write_buf);
 	tty->magic = 0xDEADDEAD;
+	atomic_dec(&tty->owner_user_ns->count);
 	kfree(tty);
 }
 
@@ -3191,6 +3193,8 @@ struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx)
 	tty->index = idx;
 	tty_line_name(driver, idx, tty->name);
 	tty->dev = tty_get_device(tty);
+	tty->owner_user_ns = current_user_ns();
+	atomic_inc(&tty->owner_user_ns->count);
 
 	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

[PATCH v3 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>
> ---
>  drivers/tty/tty_io.c | 4 ++++
>  include/linux/tty.h  | 2 ++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index e6d1a65..03d5ea2 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -161,6 +161,7 @@ static void release_tty(struct tty_struct *tty, int idx);
>   *	@tty: tty struct to free
>   *
>   *	Free the write buffers, tty queue and tty memory itself.
> + *	Decrement the owner_user_ns count.
>   *
>   *	Locking: none. Must be called after tty is definitely unused
>   */
> @@ -171,6 +172,7 @@ static void free_tty_struct(struct tty_struct *tty)
>  	put_device(tty->dev);
>  	kfree(tty->write_buf);
>  	tty->magic = 0xDEADDEAD;
> +	atomic_dec(&tty->owner_user_ns->count);
>  	kfree(tty);
>  }
>  
> @@ -3191,6 +3193,8 @@ struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx)
>  	tty->index = idx;
>  	tty_line_name(driver, idx, tty->name);
>  	tty->dev = tty_get_device(tty);
> +	tty->owner_user_ns = current_user_ns();
> +	atomic_inc(&tty->owner_user_ns->count);

Hi,

CONFIG_USER_NS is an option, so unfortunately you need to do

	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

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

[Matt Brown]

On 04/23/2017 09:09 PM, Serge E. Hallyn 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>
>> ---
>>  drivers/tty/tty_io.c | 4 ++++
>>  include/linux/tty.h  | 2 ++
>>  2 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
>> index e6d1a65..03d5ea2 100644
>> --- a/drivers/tty/tty_io.c
>> +++ b/drivers/tty/tty_io.c
>> @@ -161,6 +161,7 @@ static void release_tty(struct tty_struct *tty, int idx);
>>   *	@tty: tty struct to free
>>   *
>>   *	Free the write buffers, tty queue and tty memory itself.
>> + *	Decrement the owner_user_ns count.
>>   *
>>   *	Locking: none. Must be called after tty is definitely unused
>>   */
>> @@ -171,6 +172,7 @@ static void free_tty_struct(struct tty_struct *tty)
>>  	put_device(tty->dev);
>>  	kfree(tty->write_buf);
>>  	tty->magic = 0xDEADDEAD;
>> +	atomic_dec(&tty->owner_user_ns->count);
>>  	kfree(tty);
>>  }
>>
>> @@ -3191,6 +3193,8 @@ struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx)
>>  	tty->index = idx;
>>  	tty_line_name(driver, idx, tty->name);
>>  	tty->dev = tty_get_device(tty);
>> +	tty->owner_user_ns = current_user_ns();
>> +	atomic_inc(&tty->owner_user_ns->count);
>
> Hi,
>
> CONFIG_USER_NS is an option, so unfortunately you need to do
>
> 	tty->owner_user_ns = get_user_ns(current_user_ns());
>

Does this mean I need to also use put_user_ns(tty->owner_user_ns) in
free_tty_struct instead of atomic_dec(&tty->owner_user_ns->count) ?

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

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

[Serge E. Hallyn]

Quoting Matt Brown (matt at nmatt.com):
> On 04/23/2017 09:09 PM, Serge E. Hallyn 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>
> >>---
> >> drivers/tty/tty_io.c | 4 ++++
> >> include/linux/tty.h  | 2 ++
> >> 2 files changed, 6 insertions(+)
> >>
> >>diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> >>index e6d1a65..03d5ea2 100644
> >>--- a/drivers/tty/tty_io.c
> >>+++ b/drivers/tty/tty_io.c
> >>@@ -161,6 +161,7 @@ static void release_tty(struct tty_struct *tty, int idx);
> >>  *	@tty: tty struct to free
> >>  *
> >>  *	Free the write buffers, tty queue and tty memory itself.
> >>+ *	Decrement the owner_user_ns count.
> >>  *
> >>  *	Locking: none. Must be called after tty is definitely unused
> >>  */
> >>@@ -171,6 +172,7 @@ static void free_tty_struct(struct tty_struct *tty)
> >> 	put_device(tty->dev);
> >> 	kfree(tty->write_buf);
> >> 	tty->magic = 0xDEADDEAD;
> >>+	atomic_dec(&tty->owner_user_ns->count);
> >> 	kfree(tty);
> >> }
> >>
> >>@@ -3191,6 +3193,8 @@ struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx)
> >> 	tty->index = idx;
> >> 	tty_line_name(driver, idx, tty->name);
> >> 	tty->dev = tty_get_device(tty);
> >>+	tty->owner_user_ns = current_user_ns();
> >>+	atomic_inc(&tty->owner_user_ns->count);
> >
> >Hi,
> >
> >CONFIG_USER_NS is an option, so unfortunately you need to do
> >
> >	tty->owner_user_ns = get_user_ns(current_user_ns());
> >
> 
> Does this mean I need to also use put_user_ns(tty->owner_user_ns) in
> free_tty_struct instead of atomic_dec(&tty->owner_user_ns->count) ?

Yes, and since in theory the tty could outlive all the tasks in the
user namespace it needs to be put_user_ns() anyway so as to free it
on last put.
--
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

[PATCH v3 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 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 03d5ea2..5667a5e 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2298,11 +2298,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

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

Thanks.

Acked-by: Serge Hallyn <serge@hallyn.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 03d5ea2..5667a5e 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -2298,11 +2298,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