[kernel-hardening] [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[kernel-hardening] [PATCH v6 0/2] security: tty: 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 v5:
* added acks/reviews

# Changes since v4:
* fixed typo

# Changes since v3:
* use get_user_ns and put_user_ns to take and drop references to the owner
  user namespace because CONFIG_USER_NS is an option

# Changes since v2:
* take/drop reference to user namespace on tty struct alloc/free to prevent
  use-after-free.

# Changes since v1:
* added owner_user_ns to tty_struct to enable capability checks against
  the namespace that created the tty.
* rewording in different places to make patchset purpose clear
* Added Documentation

[kernel-hardening] [PATCH v6 1/2] security: tty: 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

Acked-by: Serge Hallyn <serge@hallyn.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
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

[kernel-hardening] [PATCH v6 2/2] security: tty: 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.

Acked-by: Serge Hallyn <serge@hallyn.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
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..f7985cf 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -89,6 +89,7 @@ show up in /proc/sys/kernel:
 - sysctl_writes_strict
 - tainted
 - threads-max
+- tiocsti_restrict
 - unknown_nmi_panic
 - watchdog
 - watchdog_thresh
@@ -987,6 +988,26 @@ available RAM pages threads-max is reduced accordingly.
 
 ==============================================================
 
+tiocsti_restrict:
+
+This toggle indicates whether unprivileged users are prevented
+from using the TIOCSTI ioctl to inject commands into other processes
+which share a tty session.
+
+When tiocsti_restrict is set to (0) there are no restrictions(accept
+the default restriction of only being able to injection commands into
+one's own tty). When tiocsti_restrict is set to (1), users must
+have CAP_SYS_ADMIN to use the TIOCSTI ioctl.
+
+When user namespaces are in use, the check for the capability
+CAP_SYS_ADMIN is done against the user namespace that originally
+opened the tty.
+
+The kernel config option CONFIG_SECURITY_TIOCSTI_RESTRICT sets the
+default value of tiocsti_restrict.
+
+==============================================================
+
 unknown_nmi_panic:
 
 The value in this file affects behavior of handling NMI. When the
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index c276814..fe68d14 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2297,11 +2297,17 @@ static int tty_fasync(int fd, struct file *filp, int on)
  *	FIXME: may race normal receive processing
  */
 
+int tiocsti_restrict = IS_ENABLED(CONFIG_SECURITY_TIOCSTI_RESTRICT);
+
 static int tiocsti(struct tty_struct *tty, char __user *p)
 {
 	char ch, mbz = 0;
 	struct tty_ldisc *ld;
 
+	if (tiocsti_restrict && !ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)) {
+		pr_warn_ratelimited("TIOCSTI ioctl call blocked for non-privileged process\n");
+		return -EPERM;
+	}
 	if ((current->signal->tty != tty) && !capable(CAP_SYS_ADMIN))
 		return -EPERM;
 	if (get_user(ch, p))
diff --git a/include/linux/tty.h b/include/linux/tty.h
index d902d42..2fd7f49 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -344,6 +344,8 @@ struct tty_file_private {
 	struct list_head list;
 };
 
+extern int tiocsti_restrict;
+
 /* tty magic number */
 #define TTY_MAGIC		0x5401
 
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index acf0a5a..68d1363 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -67,6 +67,7 @@
 #include <linux/kexec.h>
 #include <linux/bpf.h>
 #include <linux/mount.h>
+#include <linux/tty.h>
 
 #include <linux/uaccess.h>
 #include <asm/processor.h>
@@ -833,6 +834,17 @@ static struct ctl_table kern_table[] = {
 		.extra2		= &two,
 	},
 #endif
+#if defined CONFIG_TTY
+	{
+		.procname	= "tiocsti_restrict",
+		.data		= &tiocsti_restrict,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax_sysadmin,
+		.extra1		= &zero,
+		.extra2		= &one,
+	},
+#endif
 	{
 		.procname	= "ngroups_max",
 		.data		= &ngroups_max,
diff --git a/security/Kconfig b/security/Kconfig
index 3ff1bf9..7d13331 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -18,6 +18,19 @@ config SECURITY_DMESG_RESTRICT
 
 	  If you are unsure how to answer this question, answer N.
 
+config SECURITY_TIOCSTI_RESTRICT
+	bool "Restrict unprivileged use of tiocsti command injection"
+	default n
+	help
+	  This enforces restrictions on unprivileged users injecting commands
+	  into other processes which share a tty session using the TIOCSTI
+	  ioctl. This option makes TIOCSTI use require CAP_SYS_ADMIN.
+
+	  If this option is not selected, no restrictions will be enforced
+	  unless the tiocsti_restrict sysctl is explicitly set to (1).
+
+	  If you are unsure how to answer this question, answer N.
+
 config SECURITY
 	bool "Enable different security models"
 	depends on SYSFS
-- 
2.10.2

[kernel-hardening] Re: [PATCH v6 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Greg KH]

On Fri, May 05, 2017 at 07:20:18PM -0400, Matt Brown wrote:
> 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.
> 
> Acked-by: Serge Hallyn <serge@hallyn.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> 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..f7985cf 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -89,6 +89,7 @@ show up in /proc/sys/kernel:
>  - sysctl_writes_strict
>  - tainted
>  - threads-max
> +- tiocsti_restrict
>  - unknown_nmi_panic
>  - watchdog
>  - watchdog_thresh
> @@ -987,6 +988,26 @@ available RAM pages threads-max is reduced accordingly.
>  
>  ==============================================================
>  
> +tiocsti_restrict:
> +
> +This toggle indicates whether unprivileged users are prevented
> +from using the TIOCSTI ioctl to inject commands into other processes
> +which share a tty session.
> +
> +When tiocsti_restrict is set to (0) there are no restrictions(accept
> +the default restriction of only being able to injection commands into
> +one's own tty). When tiocsti_restrict is set to (1), users must
> +have CAP_SYS_ADMIN to use the TIOCSTI ioctl.
> +
> +When user namespaces are in use, the check for the capability
> +CAP_SYS_ADMIN is done against the user namespace that originally
> +opened the tty.
> +
> +The kernel config option CONFIG_SECURITY_TIOCSTI_RESTRICT sets the
> +default value of tiocsti_restrict.
> +
> +==============================================================
> +
>  unknown_nmi_panic:
>  
>  The value in this file affects behavior of handling NMI. When the
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index c276814..fe68d14 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -2297,11 +2297,17 @@ static int tty_fasync(int fd, struct file *filp, int on)
>   *	FIXME: may race normal receive processing
>   */
>  
> +int tiocsti_restrict = IS_ENABLED(CONFIG_SECURITY_TIOCSTI_RESTRICT);
> +
>  static int tiocsti(struct tty_struct *tty, char __user *p)
>  {
>  	char ch, mbz = 0;
>  	struct tty_ldisc *ld;
>  
> +	if (tiocsti_restrict && !ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)) {
> +		pr_warn_ratelimited("TIOCSTI ioctl call blocked for non-privileged process\n");
> +		return -EPERM;

Always follow the proper kernel coding style rules, as I don't want to
have someone else have to come along and fix up the error you have added
here :(

checkpatch.pl is your friend, really...

And why not do a warning with the device that caused the problem to
happen?  dev_warn has a ratelimit I think right?  "raw" printk messages
like this don't help in trying to track down what/who caused the issue.

And finally, can userspace see the namespace for the tty?  Doesn't
things like checkpoint/restore need that in order to properly set the
tty connection back up when moving processes?

v7?  :)

thanks,

greg k-h

[kernel-hardening] Re: [PATCH v6 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Matt Brown]

On 5/18/17 9:31 AM, Greg KH wrote:
> On Fri, May 05, 2017 at 07:20:18PM -0400, Matt Brown wrote:
>> 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.
>>
>> Acked-by: Serge Hallyn <serge@hallyn.com>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>> 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..f7985cf 100644
>> --- a/Documentation/sysctl/kernel.txt
>> +++ b/Documentation/sysctl/kernel.txt
>> @@ -89,6 +89,7 @@ show up in /proc/sys/kernel:
>>  - sysctl_writes_strict
>>  - tainted
>>  - threads-max
>> +- tiocsti_restrict
>>  - unknown_nmi_panic
>>  - watchdog
>>  - watchdog_thresh
>> @@ -987,6 +988,26 @@ available RAM pages threads-max is reduced accordingly.
>>  
>>  ==============================================================
>>  
>> +tiocsti_restrict:
>> +
>> +This toggle indicates whether unprivileged users are prevented
>> +from using the TIOCSTI ioctl to inject commands into other processes
>> +which share a tty session.
>> +
>> +When tiocsti_restrict is set to (0) there are no restrictions(accept
>> +the default restriction of only being able to injection commands into
>> +one's own tty). When tiocsti_restrict is set to (1), users must
>> +have CAP_SYS_ADMIN to use the TIOCSTI ioctl.
>> +
>> +When user namespaces are in use, the check for the capability
>> +CAP_SYS_ADMIN is done against the user namespace that originally
>> +opened the tty.
>> +
>> +The kernel config option CONFIG_SECURITY_TIOCSTI_RESTRICT sets the
>> +default value of tiocsti_restrict.
>> +
>> +==============================================================
>> +
>>  unknown_nmi_panic:
>>  
>>  The value in this file affects behavior of handling NMI. When the
>> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
>> index c276814..fe68d14 100644
>> --- a/drivers/tty/tty_io.c
>> +++ b/drivers/tty/tty_io.c
>> @@ -2297,11 +2297,17 @@ static int tty_fasync(int fd, struct file *filp, int on)
>>   *	FIXME: may race normal receive processing
>>   */
>>  
>> +int tiocsti_restrict = IS_ENABLED(CONFIG_SECURITY_TIOCSTI_RESTRICT);
>> +
>>  static int tiocsti(struct tty_struct *tty, char __user *p)
>>  {
>>  	char ch, mbz = 0;
>>  	struct tty_ldisc *ld;
>>  
>> +	if (tiocsti_restrict && !ns_capable(tty->owner_user_ns,CAP_SYS_ADMIN)) {
>> +		pr_warn_ratelimited("TIOCSTI ioctl call blocked for non-privileged process\n");
>> +		return -EPERM;
> 
> Always follow the proper kernel coding style rules, as I don't want to
> have someone else have to come along and fix up the error you have added
> here :(
> 
> checkpatch.pl is your friend, really...
> 

My bad. Will fix these issues in v7.

> And why not do a warning with the device that caused the problem to
> happen?  dev_warn has a ratelimit I think right?  "raw" printk messages
> like this don't help in trying to track down what/who caused the issue.
> 

yes <linux/device.h> has dev_warn_ratelimited. I will use that in 7v.

> And finally, can userspace see the namespace for the tty?  Doesn't
> things like checkpoint/restore need that in order to properly set the
> tty connection back up when moving processes?

This seems like we would need to expose the owner_user_ns of the tty in procfs
somewhere. Section 1.7 Documentation/filesystems/proc.txt describes the
following files in /proc/tty:

Table 1-11: Files in /proc/tty
..............................................................................
 File          Content
 drivers       list of drivers and their usage
 ldiscs        registered line disciplines
 driver/serial usage statistic and status of single tty lines
..............................................................................

The drivers file is the one that gives the most information that we are
interested in.

However, the current layout combines information about multiple ttys by driver.
As I understand it, a single driver may have ttys that span across different
owner_user_ns. would it make sense to add a file /proc/tty/ns that would
contain the different tty to user namespace mappings? Or is there a better way
to do this? I would appreciate any feedback/ideas you have on this.

> 
> v7?  :)

v7 will be on its way soon. I'm not currently sure how to address the concern
of giving things like checkpoint/restore in userland a way to get the
owner_user_ns.

> 
> thanks,
> 
> greg k-h
> 

Thanks for the feedback,

Matt Brown

[kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Alan Cox]

On Fri,  5 May 2017 19:20:16 -0400
Matt Brown <matt@nmatt.com> wrote:

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

And it still doesn't deal with the fact that there are hundreds of other
ways to annoy the owner of a tty if it's passed to a lower privilege
child from framebuffer reprogramming through keyboard remaps.

The proper way to handle those cases is to create a pty/tty pair and use
that. Your patch is pure snake oil and if anything implies safety that
doesn't exist.

In addition your change to allow it to be used by root in the guest
completely invalidates any protection you have because I can push

"rm -rf /\n"

as root in my namespace and exit

The tty buffers are not flushed across the context change so the shell
you return to gets the input and oh dear....

Alan

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Daniel Micay]

On Wed, 2017-05-10 at 21:29 +0100, Alan Cox wrote:
> 
> In addition your change to allow it to be used by root in the guest
> completely invalidates any protection you have because I can push
> 
> "rm -rf /\n"
> 
> as root in my namespace and exit
> 
> The tty buffers are not flushed across the context change so the shell
> you return to gets the input and oh dear....
> 
> Alan

I might be missing something, but it looks like the patch tracks where
the tty was created and only allows this with CAP_SYS_ADMIN in the ns
where the tty came from.

[kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Matt Brown]

On 05/10/2017 04:29 PM, Alan Cox wrote:
> On Fri,  5 May 2017 19:20:16 -0400
> Matt Brown <matt@nmatt.com> wrote:
>
>> 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.
>
> And it still doesn't deal with the fact that there are hundreds of other
> ways to annoy the owner of a tty if it's passed to a lower privilege
> child from framebuffer reprogramming through keyboard remaps.
>
> The proper way to handle those cases is to create a pty/tty pair and use
> that. Your patch is pure snake oil and if anything implies safety that
> doesn't exist.
>

I'm not implying that my patch is supposed to provide safety for
"hundreds of other" issues. I'm looking to provide a way to lock down a
single TTY ioctl that has caused real security issues to arise. For
this reason, it's completely incorrect to say that this feature is
snake oil. My patch does exactly what it claims to do. No more no less.

> In addition your change to allow it to be used by root in the guest
> completely invalidates any protection you have because I can push
>
> "rm -rf /\n"
>
> as root in my namespace and exit
>
> The tty buffers are not flushed across the context change so the shell
> you return to gets the input and oh dear....

This is precisely what my patch prevents! With my protection enabled, a
container will only be able to use the TIOCSTI ioctl on a tty if that
container has CAP_SYS_ADMIN in the user namespace in which the tty was
created.

>
> Alan
>

[kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Nicolas Belouin]

[-- Attachment #1: Type: text/plain, Size: 2721 bytes --]

I haven't read your patch, but from its description, are you sure CAP_SYS_ADMIN is the right choice for such behavior ?
CAP_SYS_ADMIN is, from my point of view, a too broadly used capability.
I think CAP_SYS_TTY_CONFIG is a more appropriate capability for that particular purpose.

On May 13, 2017 9:52:58 PM GMT+02:00, Matt Brown <matt@nmatt.com> wrote:
>On 05/10/2017 04:29 PM, Alan Cox wrote:
>> On Fri,  5 May 2017 19:20:16 -0400
>> Matt Brown <matt@nmatt.com> wrote:
>>
>>> 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.
>>
>> And it still doesn't deal with the fact that there are hundreds of
>other
>> ways to annoy the owner of a tty if it's passed to a lower privilege
>> child from framebuffer reprogramming through keyboard remaps.
>>
>> The proper way to handle those cases is to create a pty/tty pair and
>use
>> that. Your patch is pure snake oil and if anything implies safety
>that
>> doesn't exist.
>>
>
>I'm not implying that my patch is supposed to provide safety for
>"hundreds of other" issues. I'm looking to provide a way to lock down a
>single TTY ioctl that has caused real security issues to arise. For
>this reason, it's completely incorrect to say that this feature is
>snake oil. My patch does exactly what it claims to do. No more no less.
>
>> In addition your change to allow it to be used by root in the guest
>> completely invalidates any protection you have because I can push
>>
>> "rm -rf /\n"
>>
>> as root in my namespace and exit
>>
>> The tty buffers are not flushed across the context change so the
>shell
>> you return to gets the input and oh dear....
>
>This is precisely what my patch prevents! With my protection enabled, a
>container will only be able to use the TIOCSTI ioctl on a tty if that
>container has CAP_SYS_ADMIN in the user namespace in which the tty was
>created.
>
>>
>> Alan
>>

Nicolas

[-- Attachment #2: Type: text/html, Size: 3764 bytes --]

[kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Alan Cox]

O> I'm not implying that my patch is supposed to provide safety for
> "hundreds of other" issues. I'm looking to provide a way to lock down a
> single TTY ioctl that has caused real security issues to arise. For

In other words you are not actually fixing anything.

> this reason, it's completely incorrect to say that this feature is
> snake oil. My patch does exactly what it claims to do. No more no less.
> 
> > In addition your change to allow it to be used by root in the guest
> > completely invalidates any protection you have because I can push
> >
> > "rm -rf /\n"
> >
> > as root in my namespace and exit
> >
> > The tty buffers are not flushed across the context change so the shell
> > you return to gets the input and oh dear....  
> 
> This is precisely what my patch prevents! With my protection enabled, a
> container will only be able to use the TIOCSTI ioctl on a tty if that
> container has CAP_SYS_ADMIN in the user namespace in which the tty was
> created.

Which is not necessarily the namespace of the process that next issues a
read().

This is snake oil. There is a correct and proper process for this use
case. That proper process is to create a new pty/tty pair. There are two
cases

- processes that do it right in which case the attacker is screwed and we
  don't need to mess up TIOCSTI handling for no reason.

- processes that do it wrong. If they do it wrong then they'll also use
  all the other holes and attacks available via the same path which are
  completely unfixable without making your system basically unusable.


So can we please apply the minimum of common sense reasoning to this and
drop the patch.

Alan

[kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Peter Dolding]

On Tue, May 16, 2017 at 6:57 AM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
> O> I'm not implying that my patch is supposed to provide safety for
>> "hundreds of other" issues. I'm looking to provide a way to lock down a
>> single TTY ioctl that has caused real security issues to arise. For
>
> In other words you are not actually fixing anything.
>
>> this reason, it's completely incorrect to say that this feature is
>> snake oil. My patch does exactly what it claims to do. No more no less.
>>
>> > In addition your change to allow it to be used by root in the guest
>> > completely invalidates any protection you have because I can push
>> >
>> > "rm -rf /\n"
>> >
>> > as root in my namespace and exit
>> >
>> > The tty buffers are not flushed across the context change so the shell
>> > you return to gets the input and oh dear....
>>
>> This is precisely what my patch prevents! With my protection enabled, a
>> container will only be able to use the TIOCSTI ioctl on a tty if that
>> container has CAP_SYS_ADMIN in the user namespace in which the tty was
>> created.
>
> Which is not necessarily the namespace of the process that next issues a
> read().
>
> This is snake oil. There is a correct and proper process for this use
> case. That proper process is to create a new pty/tty pair. There are two
> cases
>
> - processes that do it right in which case the attacker is screwed and we
>   don't need to mess up TIOCSTI handling for no reason.
>
> - processes that do it wrong. If they do it wrong then they'll also use
>   all the other holes and attacks available via the same path which are
>   completely unfixable without making your system basically unusable.
>
>
> So can we please apply the minimum of common sense reasoning to this and
> drop the patch.
>
> Alan
You missed some important.

From: http://man7.org/linux/man-pages/man7/capabilities.7.html
Don't choose CAP_SYS_ADMIN if you can possibly avoid it!
A vast  proportion of existing capability checks are associated with this
capability (see the partial list above).  It can plausibly be
called "the new root", since on the one hand, it confers a wide
range of powers, and on the other hand, its broad scope means that
 this is the capability that is required by many privileged
programs.  Don't make the problem worse.  The only new features
that should be associated with CAP_SYS_ADMIN are ones that closely
match existing uses in that silo.

This not only a improper fix the attempted fix is breach do
documentation.   CAP_SYS_ADMIN is that far overloaded it does not
require any more thrown in it direction.

This is one of the grsecurity patches mistakes.   GRKERNSEC_HARDEN_TTY
 is from 18 Feb 2016 this documentation as in place at the time they
wrote this.  Yes GRKERNSEC_HARDEN_TTY does exactly the same thing.
Yes Grsecurity guys did the same error and the grsecurity patches are
filled with this error.

The result is from the TIOCSTI patch done this way is you have to use
CAP_SYS_ADMIN to use TIOSCTI so opening up every exploit that
Grsecurity has added and every exploit CAP_SYS_ADMIN can do what is
quite a few.

Now I don't know if CAP_SYS_TTY_CONFIG what is an existing capability
might be what TIOCSTI should own under.

The reality here is CAP_SYS_ADMIN as become the Linux kernel security
equal what big kernel lock was for multi threading.

 In a ideal world CAP_SYS_ADMIN would not be used directly in most
cases.  Instead CAP_SYS_ADMIN would have a stack of sub capabilities
groups under it.

The excuse for doing it wrong grsecurity is
https://forums.grsecurity.net/viewtopic.php?f=7&t=2522

Yes most capabilities open up possibility of exploiting the system.
They are not in fact designed to prevent this.   They are designed to
limit the damage in case of malfunction so that a program/user has
only limited methods of damaging the system.  Like a program
malfunctioning with only limit capabilities if it does an action those
capabilities don't allow no damage will happen.   Now CAP_SYS_ADMIN is
for sure not limited.

But since grsecurity developers took the point of view these are False
Boundaries they then proceed to stack item after item under
CAP_SYS_ADMIN because the boundary made no sense to them.    Also some
mainline Linux Kernel developers are guilty of the same sin of
overloading CAP_SYS_ADMIN.

>From my point of view any new patching containing CAP_SYS_ADMIN
directly used should be bounced just for that.   If features need to
be added to CAP_SYS_ADMIN now they should have to go into another
capability that is enabled when  CAP_SYS_ADMIN is and hopeful if we do
this over time we will be able to clean up CAP_SYS_ADMIN into sanity.


Peter

[kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Matt Brown]

On 05/15/2017 07:10 PM, Peter Dolding wrote:
> On Tue, May 16, 2017 at 6:57 AM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
>> O> I'm not implying that my patch is supposed to provide safety for
>>> "hundreds of other" issues. I'm looking to provide a way to lock down a
>>> single TTY ioctl that has caused real security issues to arise. For
>>
>> In other words you are not actually fixing anything.
>>
>>> this reason, it's completely incorrect to say that this feature is
>>> snake oil. My patch does exactly what it claims to do. No more no less.
>>>
>>>> In addition your change to allow it to be used by root in the guest
>>>> completely invalidates any protection you have because I can push
>>>>
>>>> "rm -rf /\n"
>>>>
>>>> as root in my namespace and exit
>>>>
>>>> The tty buffers are not flushed across the context change so the shell
>>>> you return to gets the input and oh dear....
>>>
>>> This is precisely what my patch prevents! With my protection enabled, a
>>> container will only be able to use the TIOCSTI ioctl on a tty if that
>>> container has CAP_SYS_ADMIN in the user namespace in which the tty was
>>> created.
>>
>> Which is not necessarily the namespace of the process that next issues a
>> read().
>>
>> This is snake oil. There is a correct and proper process for this use
>> case. That proper process is to create a new pty/tty pair. There are two
>> cases
>>
>> - processes that do it right in which case the attacker is screwed and we
>>   don't need to mess up TIOCSTI handling for no reason.
>>
>> - processes that do it wrong. If they do it wrong then they'll also use
>>   all the other holes and attacks available via the same path which are
>>   completely unfixable without making your system basically unusable.
>>
>>
>> So can we please apply the minimum of common sense reasoning to this and
>> drop the patch.
>>
>> Alan
> You missed some important.
>
> From: http://man7.org/linux/man-pages/man7/capabilities.7.html
> Don't choose CAP_SYS_ADMIN if you can possibly avoid it!
> A vast  proportion of existing capability checks are associated with this
> capability (see the partial list above).  It can plausibly be
> called "the new root", since on the one hand, it confers a wide
> range of powers, and on the other hand, its broad scope means that
>  this is the capability that is required by many privileged
> programs.  Don't make the problem worse.  The only new features
> that should be associated with CAP_SYS_ADMIN are ones that closely
> match existing uses in that silo.
>

That last sentence is the key. CAP_SYS_ADMIN is already associated with
the TIOCSTI ioctl!

 From the same capabilities man page you quote above
under the CAP_SYS_ADMIN section:
employ the TIOCSTI ioctl(2) to insert characters into the input queue
of a terminal other than the caller's controlling terminal;

> This not only a improper fix the attempted fix is breach do
> documentation.   CAP_SYS_ADMIN is that far overloaded it does not
> require any more thrown in it direction.
>

See my comment above. This is clearly following the capabilities
documentation since CAP_SYS_ADMIN is already closely linked with the
TIOCSTI ioctl.

> This is one of the grsecurity patches mistakes.   GRKERNSEC_HARDEN_TTY
>  is from 18 Feb 2016 this documentation as in place at the time they
> wrote this.  Yes GRKERNSEC_HARDEN_TTY does exactly the same thing.
> Yes Grsecurity guys did the same error and the grsecurity patches are
> filled with this error.
>
> The result is from the TIOCSTI patch done this way is you have to use
> CAP_SYS_ADMIN to use TIOSCTI so opening up every exploit that
> Grsecurity has added and every exploit CAP_SYS_ADMIN can do what is
> quite a few.
>
> Now I don't know if CAP_SYS_TTY_CONFIG what is an existing capability
> might be what TIOCSTI should own under.
>

I could see a case being make for CAP_SYS_TTY_CONFIG. However I still
choose to do with CAP_SYS_ADMIN because it is already in use in the
TIOCSTI ioctl.

> The reality here is CAP_SYS_ADMIN as become the Linux kernel security
> equal what big kernel lock was for multi threading.
>
>  In a ideal world CAP_SYS_ADMIN would not be used directly in most
> cases.  Instead CAP_SYS_ADMIN would have a stack of sub capabilities
> groups under it.
>
> The excuse for doing it wrong grsecurity is
> https://forums.grsecurity.net/viewtopic.php?f=7&t=2522
>
> Yes most capabilities open up possibility of exploiting the system.
> They are not in fact designed to prevent this.   They are designed to
> limit the damage in case of malfunction so that a program/user has
> only limited methods of damaging the system.  Like a program
> malfunctioning with only limit capabilities if it does an action those
> capabilities don't allow no damage will happen.   Now CAP_SYS_ADMIN is
> for sure not limited.
>
> But since grsecurity developers took the point of view these are False
> Boundaries they then proceed to stack item after item under
> CAP_SYS_ADMIN because the boundary made no sense to them.    Also some
> mainline Linux Kernel developers are guilty of the same sin of
> overloading CAP_SYS_ADMIN.
>
> From my point of view any new patching containing CAP_SYS_ADMIN
> directly used should be bounced just for that.   If features need to
> be added to CAP_SYS_ADMIN now they should have to go into another
> capability that is enabled when  CAP_SYS_ADMIN is and hopeful if we do
> this over time we will be able to clean up CAP_SYS_ADMIN into sanity.
>

You might be right that CAP_SYS_ADMIN is overloaded, but my patch
barely adds anything to it since TIOCSTI already falls under its
control. It seems extreme to say this patch ought to be rejected just
because it contains CAP_SYS_ADMIN. If we want to fix the state of Linux
capabilities, then I suggest that should be a separate patchset to
reorganize them into a more modular set of controls.

>
> Peter
>

[kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Peter Dolding]

>
> I could see a case being make for CAP_SYS_TTY_CONFIG. However I still
> choose to do with CAP_SYS_ADMIN because it is already in use in the
> TIOCSTI ioctl.
>
Matt Brown don't give me existing behaviour.    CAP_SYS_ADMIN is
overload.   The documentation tells you that you are not to expand it
and you openly admit you have.

Does anything of TIOSCTI functionally say that it really should be in
CAP_SYS_ADMIN.

If functionality is going to cause security for containers maybe it
should be not in CAP_SYS_ADMIN but in its own capability that can
enabled on file by file base.

>
> You might be right that CAP_SYS_ADMIN is overloaded, but my patch
> barely adds anything to it since TIOCSTI already falls under its
> control. It seems extreme to say this patch ought to be rejected just
> because it contains CAP_SYS_ADMIN. If we want to fix the state of Linux
> capabilities, then I suggest that should be a separate patchset to
> reorganize them into a more modular set of controls.
>
We have end up with CAP_SYS_ADMIN a mess by the of a death by a
thousand cuts.   Each person to extend CAP_SYS_ADMIN to it current
mess said the same thing.   My patch barely added anything times that
by a few thousand and you end up with what we have today.   At some
point no more has to be said.

There is no point attempting to tidy it up of the rules are not put in
place so it does not turn into a mess again.

This is not something that is suitable to be done as one large
patchset.   This is better done in the same kind of method that made
it.  So every time people want to alter something associated with
CAP_SYS_ADMIN it has to get assessed and the patch has to be one that
partly corrects the existing mess.    Do this enough times and we will
no longer have a mess on CAP_SYS_ADMIN.

https://www.freedesktop.org/software/systemd/man/systemd.exec.html
Please note the CapabilityBoundingSet=

Your current patch adds no extra controls for me running a service
under systemd or anything else like it to say I don't want the
processes having the means to-do this even that they are running with
CAP_SYS_ADMIN to perform other tasks..

--employ the TIOCSTI ioctl(2) to insert characters into the input
queue of a terminal other than the caller's controlling terminal;--
This currently under CAP_SYS_ADMIN is vastly more powerful than the
one you are attempt to take away with your patch.  This one can send
messages into other terminals.   This is a vastly more powerful
version of TIOSCTI.

I fact this usage of TIOCSTI I personally think should require two
capabilities flags set.   CAP_SYS_ADMIN section left as it is at this
stage.   With TIOSCTI stuck behind another capability.

If you had added a new capability flag you could set file capabilities
on any of the old applications depending on the now secured behaviour.

https://github.com/lxc/lxc/commit/e986ea3dfa4a2957f71ae9bfaed406dd6e1ffff6

Also the general user TIOCSTI issue can be handled a different way as
LXC fix shows.  Where they uses a pty to isolate so meaning in their
fixed setup user TIOSCTI was not harmful but CAP_SYS_ADMIN TIOSCTI
still could be.  You patch as not address this problem because you
shoved everything under CAP_SYS_ADMIN.   But if you add different
capability to use TIOSCIT that is not CAP_SYS_ADMIN  to allow
CAP_SYS_ADMiN TIOSCTI functionality to be disabled.

This is what you see more often than not when you dig into this
patches adding more CAP_SYS_ADMIN.  Adding CAP_SYS_ADMIN is not fixing
a problem in most cases.   Breaking CAP_SYS_ADMIN functionality up
should be the goal not expand it.

Basically you have done something documentation has a note to
developer not to-do.   If you start looking at the problem what you
doing is not helping.   If people end up using CAP_SYS_ADMIN to access
TIOSCTI its giving the program a more powerful version of TIOSCTI to
do more harm with so reduced containment.  Totally anti to what you
are meant to be doing with capabilities..

[kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Matt Brown]

On 05/16/2017 05:01 AM, Peter Dolding wrote:
>>
>> I could see a case being make for CAP_SYS_TTY_CONFIG. However I still
>> choose to do with CAP_SYS_ADMIN because it is already in use in the
>> TIOCSTI ioctl.
>>
> Matt Brown don't give me existing behaviour.    CAP_SYS_ADMIN is
> overload.   The documentation tells you that you are not to expand it
> and you openly admit you have.
>

This is not true that I'm openly going against what the documentation
instructs. The part of the email chain where I show this got removed
somehow. Again I will refer to the capabilities man page that you
quoted.

 From http://man7.org/linux/man-pages/man7/capabilities.7.html

"Don't choose CAP_SYS_ADMIN if you can possibly avoid it!
...
The only new features that should be associated with CAP_SYS_ADMIN are
ones that closely match existing uses in that silo."

My feature affects the TIOCSTI ioctl. The TIOCSTI ioctl already falls
under CAP_SYS_ADMIN, therefore I actually *am* following the
documentation.

> Does anything of TIOSCTI functionally say that it really should be in
> CAP_SYS_ADMIN.
>
> If functionality is going to cause security for containers maybe it
> should be not in CAP_SYS_ADMIN but in its own capability that can
> enabled on file by file base.
>
>>
>> You might be right that CAP_SYS_ADMIN is overloaded, but my patch
>> barely adds anything to it since TIOCSTI already falls under its
>> control. It seems extreme to say this patch ought to be rejected just
>> because it contains CAP_SYS_ADMIN. If we want to fix the state of Linux
>> capabilities, then I suggest that should be a separate patchset to
>> reorganize them into a more modular set of controls.
>>
> We have end up with CAP_SYS_ADMIN a mess by the of a death by a
> thousand cuts.   Each person to extend CAP_SYS_ADMIN to it current
> mess said the same thing.   My patch barely added anything times that
> by a few thousand and you end up with what we have today.   At some
> point no more has to be said.
>
> There is no point attempting to tidy it up of the rules are not put in
> place so it does not turn into a mess again.
>
> This is not something that is suitable to be done as one large
> patchset.   This is better done in the same kind of method that made
> it.  So every time people want to alter something associated with
> CAP_SYS_ADMIN it has to get assessed and the patch has to be one that
> partly corrects the existing mess.    Do this enough times and we will
> no longer have a mess on CAP_SYS_ADMIN.
>
> https://www.freedesktop.org/software/systemd/man/systemd.exec.html
> Please note the CapabilityBoundingSet=
>
> Your current patch adds no extra controls for me running a service
> under systemd or anything else like it to say I don't want the
> processes having the means to-do this even that they are running with
> CAP_SYS_ADMIN to perform other tasks..
>
> --employ the TIOCSTI ioctl(2) to insert characters into the input
> queue of a terminal other than the caller's controlling terminal;--
> This currently under CAP_SYS_ADMIN is vastly more powerful than the
> one you are attempt to take away with your patch.  This one can send
> messages into other terminals.   This is a vastly more powerful
> version of TIOSCTI.
>
> I fact this usage of TIOCSTI I personally think should require two
> capabilities flags set.   CAP_SYS_ADMIN section left as it is at this
> stage.   With TIOSCTI stuck behind another capability.
>
> If you had added a new capability flag you could set file capabilities
> on any of the old applications depending on the now secured behaviour.
>
> https://github.com/lxc/lxc/commit/e986ea3dfa4a2957f71ae9bfaed406dd6e1ffff6
>
> Also the general user TIOCSTI issue can be handled a different way as
> LXC fix shows.  Where they uses a pty to isolate so meaning in their
> fixed setup user TIOSCTI was not harmful but CAP_SYS_ADMIN TIOSCTI
> still could be.  You patch as not address this problem because you
> shoved everything under CAP_SYS_ADMIN.   But if you add different
> capability to use TIOSCIT that is not CAP_SYS_ADMIN  to allow
> CAP_SYS_ADMiN TIOSCTI functionality to be disabled.
>
> This is what you see more often than not when you dig into this
> patches adding more CAP_SYS_ADMIN.  Adding CAP_SYS_ADMIN is not fixing
> a problem in most cases.   Breaking CAP_SYS_ADMIN functionality up
> should be the goal not expand it.
>
> Basically you have done something documentation has a note to
> developer not to-do.   If you start looking at the problem what you
> doing is not helping.   If people end up using CAP_SYS_ADMIN to access
> TIOSCTI its giving the program a more powerful version of TIOSCTI to
> do more harm with so reduced containment.  Totally anti to what you
> are meant to be doing with capabilities..
>

[kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Kees Cook]

On Tue, May 16, 2017 at 5:22 AM, Matt Brown <matt@nmatt.com> wrote:
> On 05/16/2017 05:01 AM, Peter Dolding wrote:
>>>
>>>
>>> I could see a case being make for CAP_SYS_TTY_CONFIG. However I still
>>> choose to do with CAP_SYS_ADMIN because it is already in use in the
>>> TIOCSTI ioctl.
>>>
>> Matt Brown don't give me existing behaviour.    CAP_SYS_ADMIN is
>> overload.   The documentation tells you that you are not to expand it
>> and you openly admit you have.
>>
>
> This is not true that I'm openly going against what the documentation
> instructs. The part of the email chain where I show this got removed
> somehow. Again I will refer to the capabilities man page that you
> quoted.
>
> From http://man7.org/linux/man-pages/man7/capabilities.7.html
>
> "Don't choose CAP_SYS_ADMIN if you can possibly avoid it!
> ...
> The only new features that should be associated with CAP_SYS_ADMIN are
> ones that closely match existing uses in that silo."
>
> My feature affects the TIOCSTI ioctl. The TIOCSTI ioctl already falls
> under CAP_SYS_ADMIN, therefore I actually *am* following the
> documentation.

CAP_SYS_ADMIN is the right choice here, I agree with Matt: it is
already in use for TIOCSTI. We can't trivially add new capabilities
flags (see the various giant threads debating this, the most recently
that I remember from the kernel lock-down series related to Secure
Boot).

>> I fact this usage of TIOCSTI I personally think should require two
>> capabilities flags set.   CAP_SYS_ADMIN section left as it is at this
>> stage.   With TIOSCTI stuck behind another capability.
>>
>> If you had added a new capability flag you could set file capabilities
>> on any of the old applications depending on the now secured behaviour.

If we're adjusting applications, they should be made to avoid TIOSCTI
completely. This looks to me a lot like the symlink restrictions: yes,
userspace should be fixed to the do the right thing, but why not
provide support to userspace to avoid the problem entirely?

-Kees

-- 
Kees Cook
Pixel Security

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Serge E. Hallyn]

Quoting Kees Cook (keescook@chromium.org):
> On Tue, May 16, 2017 at 5:22 AM, Matt Brown <matt@nmatt.com> wrote:
> > On 05/16/2017 05:01 AM, Peter Dolding wrote:
> >>>
> >>>
> >>> I could see a case being make for CAP_SYS_TTY_CONFIG. However I still
> >>> choose to do with CAP_SYS_ADMIN because it is already in use in the
> >>> TIOCSTI ioctl.
> >>>
> >> Matt Brown don't give me existing behaviour.    CAP_SYS_ADMIN is
> >> overload.   The documentation tells you that you are not to expand it
> >> and you openly admit you have.
> >>
> >
> > This is not true that I'm openly going against what the documentation
> > instructs. The part of the email chain where I show this got removed
> > somehow. Again I will refer to the capabilities man page that you
> > quoted.
> >
> > From http://man7.org/linux/man-pages/man7/capabilities.7.html
> >
> > "Don't choose CAP_SYS_ADMIN if you can possibly avoid it!
> > ...
> > The only new features that should be associated with CAP_SYS_ADMIN are
> > ones that closely match existing uses in that silo."
> >
> > My feature affects the TIOCSTI ioctl. The TIOCSTI ioctl already falls
> > under CAP_SYS_ADMIN, therefore I actually *am* following the
> > documentation.
> 
> CAP_SYS_ADMIN is the right choice here, I agree with Matt: it is
> already in use for TIOCSTI. We can't trivially add new capabilities
> flags (see the various giant threads debating this, the most recently
> that I remember from the kernel lock-down series related to Secure
> Boot).

Consideer that if we use CAP_SYS_TTY_CONFIG now, then any applications
which are currently being given CAP_SYS_ADMIN would need to be updated
with a second capability.  Not acceptable.  Even when we split up
CAP_SYSLOG, we took care to avoid that (by having the original capability
also suffice, so either capability worked).

> >> I fact this usage of TIOCSTI I personally think should require two
> >> capabilities flags set.   CAP_SYS_ADMIN section left as it is at this
> >> stage.   With TIOSCTI stuck behind another capability.
> >>
> >> If you had added a new capability flag you could set file capabilities
> >> on any of the old applications depending on the now secured behaviour.
> 
> If we're adjusting applications, they should be made to avoid TIOSCTI
> completely. This looks to me a lot like the symlink restrictions: yes,
> userspace should be fixed to the do the right thing, but why not
> provide support to userspace to avoid the problem entirely?
> 
> -Kees
> 
> -- 
> Kees Cook
> Pixel Security

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Peter Dolding]

On Wed, May 17, 2017 at 1:48 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> Quoting Kees Cook (keescook@chromium.org):
>> On Tue, May 16, 2017 at 5:22 AM, Matt Brown <matt@nmatt.com> wrote:
>> > On 05/16/2017 05:01 AM, Peter Dolding wrote:
>> >>>
>> >>>
>> >>> I could see a case being make for CAP_SYS_TTY_CONFIG. However I still
>> >>> choose to do with CAP_SYS_ADMIN because it is already in use in the
>> >>> TIOCSTI ioctl.
>> >>>
>> >> Matt Brown don't give me existing behaviour.    CAP_SYS_ADMIN is
>> >> overload.   The documentation tells you that you are not to expand it
>> >> and you openly admit you have.
>> >>
>> >
>> > This is not true that I'm openly going against what the documentation
>> > instructs. The part of the email chain where I show this got removed
>> > somehow. Again I will refer to the capabilities man page that you
>> > quoted.
>> >
>> > From http://man7.org/linux/man-pages/man7/capabilities.7.html
>> >
>> > "Don't choose CAP_SYS_ADMIN if you can possibly avoid it!
>> > ...
>> > The only new features that should be associated with CAP_SYS_ADMIN are
>> > ones that closely match existing uses in that silo."
>> >
>> > My feature affects the TIOCSTI ioctl. The TIOCSTI ioctl already falls
>> > under CAP_SYS_ADMIN, therefore I actually *am* following the
>> > documentation.
>>
>> CAP_SYS_ADMIN is the right choice here, I agree with Matt: it is
>> already in use for TIOCSTI. We can't trivially add new capabilities
>> flags (see the various giant threads debating this, the most recently
>> that I remember from the kernel lock-down series related to Secure
>> Boot).
>
> Consideer that if we use CAP_SYS_TTY_CONFIG now, then any applications
> which are currently being given CAP_SYS_ADMIN would need to be updated
> with a second capability.  Not acceptable.  Even when we split up
> CAP_SYSLOG, we took care to avoid that (by having the original capability
> also suffice, so either capability worked).
>
There is another option create a security bit.

That could be called SECBIT_CONTAINER.   This turns off functionally
like TIOCSTI that can be used to it break out with.

This case the mainlined code the TIOCSTI currently in CAP_SYS_ADMIN is
a container breaker its designed to allow reaching cross users and
TTYs.   SECBIT is a inverted capability so when it enabled it disables
something and once enabled it cannot be disabled.   So the lxc
addressed to the user TIOCSTI causing a breakout does not work against
the CAP_SYS_ADMIN one.   If there was a security bit that disabled
TIOCSTI completely that prevents all the escape paths by TIOCSTI.

There would also be room for a SECBIT_NO_OBSOLETE what is quite simple
make using obsolete functions application fatal.   Now with CAP_SYSLOG
if SECBIT_NO_OBSOLETE programs using the old capability could find the
feature removed.   So over time we can systematically remove the multi
entry path we have now as userspace updates and stops requiring the
second path.

There is more than 1 way to skin this cat.   There is no need to add
more to CAP_SYS_ADMIN and it particular bad when you consider having
to obey the Linux Rule of user-space compatibly would result in having
apply CAP_SYS_ADMIN to existing applications with Matts patch.

Peter

.

[kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Peter Dolding]

On Wed, May 17, 2017 at 12:28 AM, Kees Cook <keescook@chromium.org> wrote:
> On Tue, May 16, 2017 at 5:22 AM, Matt Brown <matt@nmatt.com> wrote:
>> On 05/16/2017 05:01 AM, Peter Dolding wrote:
>>>>
>>>>
>>>> I could see a case being make for CAP_SYS_TTY_CONFIG. However I still
>>>> choose to do with CAP_SYS_ADMIN because it is already in use in the
>>>> TIOCSTI ioctl.
>>>>
>>> Matt Brown don't give me existing behaviour.    CAP_SYS_ADMIN is
>>> overload.   The documentation tells you that you are not to expand it
>>> and you openly admit you have.
>>>
>>
>> This is not true that I'm openly going against what the documentation
>> instructs. The part of the email chain where I show this got removed
>> somehow. Again I will refer to the capabilities man page that you
>> quoted.
>>
>> From http://man7.org/linux/man-pages/man7/capabilities.7.html
>>
>> "Don't choose CAP_SYS_ADMIN if you can possibly avoid it!
>> ...
>> The only new features that should be associated with CAP_SYS_ADMIN are
>> ones that closely match existing uses in that silo."
>>
>> My feature affects the TIOCSTI ioctl. The TIOCSTI ioctl already falls
>> under CAP_SYS_ADMIN, therefore I actually *am* following the
>> documentation.
>
> CAP_SYS_ADMIN is the right choice here, I agree with Matt: it is
> already in use for TIOCSTI. We can't trivially add new capabilities
> flags (see the various giant threads debating this, the most recently
> that I remember from the kernel lock-down series related to Secure
> Boot).

We cannot just keep on expanding CAP_SYS_ADMIN either.
>
>>> I fact this usage of TIOCSTI I personally think should require two
>>> capabilities flags set.   CAP_SYS_ADMIN section left as it is at this
>>> stage.   With TIOSCTI stuck behind another capability.
>>>
>>> If you had added a new capability flag you could set file capabilities
>>> on any of the old applications depending on the now secured behaviour.
>
> If we're adjusting applications, they should be made to avoid TIOSCTI
> completely. This looks to me a lot like the symlink restrictions: yes,
> userspace should be fixed to the do the right thing, but why not
> provide support to userspace to avoid the problem entirely?
>
Kees I like but you have forgot the all important rule.   The Linus
Rule.    Existing applications must  have a method work.
  So modify applications  binary is not way out of problem.

Please note making CAP_SYS_ADMIN the only way to use TIOCSTI also
means setting CAP_SYS_ADMIN on all the existing applications to obey
the Linus Rule of not break userspace.   So this is why the patch is
strictly no as this means elevating privilege of existing applications
and possibly opening up more security flaws.

Reality any patch like the one we are talking about due to the Linus
Rule and the security risk it will open up obey this it just be
rejected.   There is another kind of way I will cover with Serge.

Peter Dolding.

[kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Matt Brown]

On 05/16/2017 05:43 PM, Peter Dolding wrote:
> On Wed, May 17, 2017 at 12:28 AM, Kees Cook <keescook@chromium.org> wrote:
>> On Tue, May 16, 2017 at 5:22 AM, Matt Brown <matt@nmatt.com> wrote:
>>> On 05/16/2017 05:01 AM, Peter Dolding wrote:
>>>>>
>>>>>
>>>>> I could see a case being make for CAP_SYS_TTY_CONFIG. However I still
>>>>> choose to do with CAP_SYS_ADMIN because it is already in use in the
>>>>> TIOCSTI ioctl.
>>>>>
>>>> Matt Brown don't give me existing behaviour.    CAP_SYS_ADMIN is
>>>> overload.   The documentation tells you that you are not to expand it
>>>> and you openly admit you have.
>>>>
>>>
>>> This is not true that I'm openly going against what the documentation
>>> instructs. The part of the email chain where I show this got removed
>>> somehow. Again I will refer to the capabilities man page that you
>>> quoted.
>>>
>>> From http://man7.org/linux/man-pages/man7/capabilities.7.html
>>>
>>> "Don't choose CAP_SYS_ADMIN if you can possibly avoid it!
>>> ...
>>> The only new features that should be associated with CAP_SYS_ADMIN are
>>> ones that closely match existing uses in that silo."
>>>
>>> My feature affects the TIOCSTI ioctl. The TIOCSTI ioctl already falls
>>> under CAP_SYS_ADMIN, therefore I actually *am* following the
>>> documentation.
>>
>> CAP_SYS_ADMIN is the right choice here, I agree with Matt: it is
>> already in use for TIOCSTI. We can't trivially add new capabilities
>> flags (see the various giant threads debating this, the most recently
>> that I remember from the kernel lock-down series related to Secure
>> Boot).
>
> We cannot just keep on expanding CAP_SYS_ADMIN either.
>>
>>>> I fact this usage of TIOCSTI I personally think should require two
>>>> capabilities flags set.   CAP_SYS_ADMIN section left as it is at this
>>>> stage.   With TIOSCTI stuck behind another capability.
>>>>
>>>> If you had added a new capability flag you could set file capabilities
>>>> on any of the old applications depending on the now secured behaviour.
>>
>> If we're adjusting applications, they should be made to avoid TIOSCTI
>> completely. This looks to me a lot like the symlink restrictions: yes,
>> userspace should be fixed to the do the right thing, but why not
>> provide support to userspace to avoid the problem entirely?
>>
> Kees I like but you have forgot the all important rule.   The Linus
> Rule.    Existing applications must  have a method work.
>   So modify applications  binary is not way out of problem.
>
> Please note making CAP_SYS_ADMIN the only way to use TIOCSTI also
> means setting CAP_SYS_ADMIN on all the existing applications to obey
> the Linus Rule of not break userspace.   So this is why the patch is
> strictly no as this means elevating privilege of existing applications
> and possibly opening up more security flaws.

This feature is not required so it is not "making CAP_SYS_ADMIN the
only way to use TIOCSTI". It defaults to no as to not break some
existing programs that use it.

>
> Reality any patch like the one we are talking about due to the Linus
> Rule and the security risk it will open up obey this it just be
> rejected.   There is another kind of way I will cover with Serge.
>
> Peter Dolding.
>

Matt

[kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Alan Cox]

> If we're adjusting applications, they should be made to avoid TIOSCTI
> completely. This looks to me a lot like the symlink restrictions: yes,
> userspace should be fixed to the do the right thing, but why not
> provide support to userspace to avoid the problem entirely?

We do it's called pty/tty. There isn't any other way to do this correctly
because TIOCSTI is just one hundreds of things the attacker can do to
make your life miserable in the case you create a child process of lower
security privilege and give it your tty file handle or worse (like some
container crapware) your X11 socket fd.

Does it really matter any more or less if I reprogram your enter key, use
TIOCSTI, set the baud rate, change all your fonts ?

The mainstream tools like sudo get this right (*). Blocking TIOCSTI fixes
nothing and breaks apps. If it magically fixed the problem it might make
sense but it doesn't. You actually have to get an adult to write the
relevant code.

Alan
(*) Almost. There's an old world trick of sending "+++" "ATE1" "rm -rf
*\r\n" to try and attack improperly configured remote modem sessions but
the stuff that matters is handled.

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Daniel Micay]

On Wed, 2017-05-17 at 17:41 +0100, Alan Cox wrote:
> > If we're adjusting applications, they should be made to avoid
> > TIOSCTI
> > completely. This looks to me a lot like the symlink restrictions:
> > yes,
> > userspace should be fixed to the do the right thing, but why not
> > provide support to userspace to avoid the problem entirely?
> 
> We do it's called pty/tty. There isn't any other way to do this
> correctly
> because TIOCSTI is just one hundreds of things the attacker can do to
> make your life miserable in the case you create a child process of
> lower
> security privilege and give it your tty file handle or worse (like
> some
> container crapware) your X11 socket fd.
> 
> Does it really matter any more or less if I reprogram your enter key,
> use
> TIOCSTI, set the baud rate, change all your fonts ?
> 
> The mainstream tools like sudo get this right (*). Blocking TIOCSTI
> fixes
> nothing and breaks apps. If it magically fixed the problem it might
> make
> sense but it doesn't. You actually have to get an adult to write the
> relevant code.

It gets it right because it was reported as a vulnerability and fixed,
which is the cycle many of these tools have gone through. It looks like
sudo and coreutils / shadow su were fixed in 2005, but there are more of
these tools.

CVE-2005-4890 (coreutils su, shadow su, sudo), CVE-2016-7545 (SELinux
sandbox utility), CVE-2016-2781 (chroot --userspec), CVE-2016-2779
(util-linux runuser), CVE-2016-2568 (pkexec)

I am not sure if the pkexec vulnerability is even fixed:

https://bugzilla.redhat.com/show_bug.cgi?id=1300746

CVE-2017-5226 is for bubblewrap/flatpak this year, and I'm sure there
were a lot more as I didn't bother to dig very deep.

I completely agree that it should be solved by doing things properly in
each application. However, it isn't solved properly everywhere and each
new application is making the same mistake. Providing this feature does
not break anything that people use in practice and it doesn't need to
solve every issue to be quite useful, it only needs to prevent local
privilege escalation in the form of code execution in many cases. Is
there another way to get code execution via that bubblewrap/flatpak bug
with this feature implemented? As far as I can tell, there isn't. It's a
problem even with this feature but a significantly less serious one.

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Boris Lukashev]

[-- Attachment #1: Type: text/plain, Size: 3752 bytes --]

To put a bit of a red team spin on this: if its not fixed in the kernel,
binaries/shellcode specifically written to abuse the functionality will be
introduced by the attacker to take advantage of the hole. Defense in depth
means mitigating each tier of the attacker's progression, not simply
blocking initial ingress/execution.
SELinux is not omnipresent, almost never properly configured, and from the
point of view of practical security in the critical infrastructures using
Linux, not an appropriate response when relating to kernel functionality -
"WONTFIX beacuause NOTMYPROBLEM" is what got us all here in the first
place.
Finally, far as compatibility goes, disabling the protective measure for
the small segment of valid violators of proper use semantics handles the
concern - either system-wide as a sysctl, or via the TPE implementation
itself.

Can we all agree that the primary purpose of this effort is to harden the
system, with cowtowing to "improperly implemented consumers" reduced to
only the most common cases which would actually impede users?
If we dont, it wont be long till elephant tears about Wine and VirtualBox
flood these discussions.

On Wed, May 17, 2017 at 2:25 PM, Daniel Micay <danielmicay@gmail.com> wrote:

> On Wed, 2017-05-17 at 17:41 +0100, Alan Cox wrote:
> > > If we're adjusting applications, they should be made to avoid
> > > TIOSCTI
> > > completely. This looks to me a lot like the symlink restrictions:
> > > yes,
> > > userspace should be fixed to the do the right thing, but why not
> > > provide support to userspace to avoid the problem entirely?
> >
> > We do it's called pty/tty. There isn't any other way to do this
> > correctly
> > because TIOCSTI is just one hundreds of things the attacker can do to
> > make your life miserable in the case you create a child process of
> > lower
> > security privilege and give it your tty file handle or worse (like
> > some
> > container crapware) your X11 socket fd.
> >
> > Does it really matter any more or less if I reprogram your enter key,
> > use
> > TIOCSTI, set the baud rate, change all your fonts ?
> >
> > The mainstream tools like sudo get this right (*). Blocking TIOCSTI
> > fixes
> > nothing and breaks apps. If it magically fixed the problem it might
> > make
> > sense but it doesn't. You actually have to get an adult to write the
> > relevant code.
>
> It gets it right because it was reported as a vulnerability and fixed,
> which is the cycle many of these tools have gone through. It looks like
> sudo and coreutils / shadow su were fixed in 2005, but there are more of
> these tools.
>
> CVE-2005-4890 (coreutils su, shadow su, sudo), CVE-2016-7545 (SELinux
> sandbox utility), CVE-2016-2781 (chroot --userspec), CVE-2016-2779
> (util-linux runuser), CVE-2016-2568 (pkexec)
>
> I am not sure if the pkexec vulnerability is even fixed:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1300746
>
> CVE-2017-5226 is for bubblewrap/flatpak this year, and I'm sure there
> were a lot more as I didn't bother to dig very deep.
>
> I completely agree that it should be solved by doing things properly in
> each application. However, it isn't solved properly everywhere and each
> new application is making the same mistake. Providing this feature does
> not break anything that people use in practice and it doesn't need to
> solve every issue to be quite useful, it only needs to prevent local
> privilege escalation in the form of code execution in many cases. Is
> there another way to get code execution via that bubblewrap/flatpak bug
> with this feature implemented? As far as I can tell, there isn't. It's a
> problem even with this feature but a significantly less serious one.
>



-- 
Boris Lukashev
Systems Architect
Semper Victus

[-- Attachment #2: Type: text/html, Size: 4713 bytes --]

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Kees Cook]

On Wed, May 17, 2017 at 11:25 AM, Daniel Micay <danielmicay@gmail.com> wrote:
> On Wed, 2017-05-17 at 17:41 +0100, Alan Cox wrote:
>> > If we're adjusting applications, they should be made to avoid
>> > TIOSCTI
>> > completely. This looks to me a lot like the symlink restrictions:
>> > yes,
>> > userspace should be fixed to the do the right thing, but why not
>> > provide support to userspace to avoid the problem entirely?
>>
>> We do it's called pty/tty. There isn't any other way to do this
>> correctly
>> because TIOCSTI is just one hundreds of things the attacker can do to
>> make your life miserable in the case you create a child process of
>> lower
>> security privilege and give it your tty file handle or worse (like
>> some
>> container crapware) your X11 socket fd.
>>
>> Does it really matter any more or less if I reprogram your enter key,
>> use
>> TIOCSTI, set the baud rate, change all your fonts ?
>>
>> The mainstream tools like sudo get this right (*). Blocking TIOCSTI
>> fixes
>> nothing and breaks apps. If it magically fixed the problem it might
>> make
>> sense but it doesn't. You actually have to get an adult to write the
>> relevant code.
>
> It gets it right because it was reported as a vulnerability and fixed,
> which is the cycle many of these tools have gone through. It looks like
> sudo and coreutils / shadow su were fixed in 2005, but there are more of
> these tools.
>
> CVE-2005-4890 (coreutils su, shadow su, sudo), CVE-2016-7545 (SELinux
> sandbox utility), CVE-2016-2781 (chroot --userspec), CVE-2016-2779
> (util-linux runuser), CVE-2016-2568 (pkexec)
>
> I am not sure if the pkexec vulnerability is even fixed:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1300746
>
> CVE-2017-5226 is for bubblewrap/flatpak this year, and I'm sure there
> were a lot more as I didn't bother to dig very deep.
>
> I completely agree that it should be solved by doing things properly in
> each application. However, it isn't solved properly everywhere and each
> new application is making the same mistake. Providing this feature does
> not break anything that people use in practice and it doesn't need to
> solve every issue to be quite useful, it only needs to prevent local
> privilege escalation in the form of code execution in many cases. Is
> there another way to get code execution via that bubblewrap/flatpak bug
> with this feature implemented? As far as I can tell, there isn't. It's a
> problem even with this feature but a significantly less serious one.

This patch solves a real problem when userspace does things wrong. For
those that do not want it, a sysctl exists (and is even default off).
Arguments about how userspace needs to be fixed is a total red
herring. Like symlink protections before, this should be available for
those that want it because it solves an interface problem that gets
regularly misused and causes real damage. The kernel has a
responsibility to protect userspace.

As Daniel mentions, there is a history of mistakes that appear to be
becoming even more common:
http://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=TIOCSTI

There are people who want this feature that do not care about xemacs
breaking. The default cannot be enabled because of the golden rule of
never breaking userspace, but entirely refusing to fix a regularly
misused feature is wrong-headed. We must accept that not only are bug
lifetimes long, but that bugs will keep getting reintroduced. When
there is a chance to kill a class of bugs or exploit techniques, it's
the kernel responsibility to do so. This is a clear case of that, and
the solution is concise.

If there is some better solution that the kernel can provide to
mitigate processes misusing ttys, then by all means, we can add that
too, but has nothing to do with refusing this change. This solves a
specific problem that in many cases is the only path to privilege
escalation (as Daniel mentioned). Refusing this change is nonsense.
"Your car shouldn't have seat belts because maybe something will stab
you through the windshield" isn't a reasonable argument to make.

-Kees

-- 
Kees Cook
Pixel Security

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Peter Dolding]

On Thu, May 18, 2017 at 1:18 PM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, May 17, 2017 at 11:25 AM, Daniel Micay <danielmicay@gmail.com> wrote:
>> On Wed, 2017-05-17 at 17:41 +0100, Alan Cox wrote:
>>> > If we're adjusting applications, they should be made to avoid
>>> > TIOSCTI
>>> > completely. This looks to me a lot like the symlink restrictions:
>>> > yes,
>>> > userspace should be fixed to the do the right thing, but why not
>>> > provide support to userspace to avoid the problem entirely?
>>>
>>> We do it's called pty/tty. There isn't any other way to do this
>>> correctly
>>> because TIOCSTI is just one hundreds of things the attacker can do to
>>> make your life miserable in the case you create a child process of
>>> lower
>>> security privilege and give it your tty file handle or worse (like
>>> some
>>> container crapware) your X11 socket fd.
>>>
>>> Does it really matter any more or less if I reprogram your enter key,
>>> use
>>> TIOCSTI, set the baud rate, change all your fonts ?
>>>
>>> The mainstream tools like sudo get this right (*). Blocking TIOCSTI
>>> fixes
>>> nothing and breaks apps. If it magically fixed the problem it might
>>> make
>>> sense but it doesn't. You actually have to get an adult to write the
>>> relevant code.
>>
>> It gets it right because it was reported as a vulnerability and fixed,
>> which is the cycle many of these tools have gone through. It looks like
>> sudo and coreutils / shadow su were fixed in 2005, but there are more of
>> these tools.
>>
>> CVE-2005-4890 (coreutils su, shadow su, sudo), CVE-2016-7545 (SELinux
>> sandbox utility), CVE-2016-2781 (chroot --userspec), CVE-2016-2779
>> (util-linux runuser), CVE-2016-2568 (pkexec)
>>
>> I am not sure if the pkexec vulnerability is even fixed:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1300746
>>
>> CVE-2017-5226 is for bubblewrap/flatpak this year, and I'm sure there
>> were a lot more as I didn't bother to dig very deep.
>>
>> I completely agree that it should be solved by doing things properly in
>> each application. However, it isn't solved properly everywhere and each
>> new application is making the same mistake. Providing this feature does
>> not break anything that people use in practice and it doesn't need to
>> solve every issue to be quite useful, it only needs to prevent local
>> privilege escalation in the form of code execution in many cases. Is
>> there another way to get code execution via that bubblewrap/flatpak bug
>> with this feature implemented? As far as I can tell, there isn't. It's a
>> problem even with this feature but a significantly less serious one.
>
> This patch solves a real problem when userspace does things wrong. For
> those that do not want it, a sysctl exists (and is even default off).
> Arguments about how userspace needs to be fixed is a total red
> herring. Like symlink protections before, this should be available for
> those that want it because it solves an interface problem that gets
> regularly misused and causes real damage. The kernel has a
> responsibility to protect userspace.
>
> As Daniel mentions, there is a history of mistakes that appear to be
> becoming even more common:
> http://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=TIOCSTI
>
> There are people who want this feature that do not care about xemacs
> breaking. The default cannot be enabled because of the golden rule of
> never breaking userspace, but entirely refusing to fix a regularly
> misused feature is wrong-headed. We must accept that not only are bug
> lifetimes long, but that bugs will keep getting reintroduced. When
> there is a chance to kill a class of bugs or exploit techniques, it's
> the kernel responsibility to do so. This is a clear case of that, and
> the solution is concise.

True but what is usage of TIOCSTI that is wrong in these CVE.   Is it
not when the returned input values values to tty crosses process
boundaries.  Like if when process terminated everything the process
had pushed back to TTY and Input it has received was lost those CVE
would not be possible.

Could those exist CVE when exploit happens have CAP_SYS_ADMIN as the
process being used to break out the answer is yes.   So restricting
TIOCSTI to CAP_SYS_ADMIN could be pointless.   Remember you have suid
bit applications one of those might have  bug that someone is able to
exploit to use the TIOCSTI.   So pushing to CAP_SYS_ADMIN make exploit
rarer does not remove it.

Please note you see TIOCSTI using in libc when they implement
ungetc().   So this is something that is quite well used.   I see
ungetc as kind of a bad idea to be implemented kernel level going to
tty shareable between processes.

> If there is some better solution that the kernel can provide to
> mitigate processes misusing ttys, then by all means, we can add that
> too, but has nothing to do with refusing this change. This solves a
> specific problem that in many cases is the only path to privilege
> escalation (as Daniel mentioned). Refusing this change is nonsense.
> "Your car shouldn't have seat belts because maybe something will stab
> you through the windshield" isn't a reasonable argument to make.

Using cap_sys_admin as fix is like removing car windsheld because
vision is being blocked by a rock hitting it.

Kees the problem with accepting a security fix that is wrong the
proper change never gets worked on.

I am not saying there is not a real problem here.   The fix is not
push it to CAP_SYS_ADMIN.   Due to TIOCSTI that cross process and tty
boundaries been used in security breaks and limit applications that
uses use as either Administrator or as normal User means this ability
does not own in CAP_SYS_ADMIN either.

The same is true of obsolete function calls that have been shoved into
CAP_SYS_ADMIN already there comes point where no valid userspace
application is using that function.   At that point the only thing
using that function is exploits so then really CAP_SYS_ADMIN should
not have access to those obsolete functions.   There is a real need
for CAP_SYS_OBSOLETE.   If a program has to ahve CAP_SYS_OBSOLETE set
on it this means it using functionally that is known busted in some
way..

This is the biggest problem here.   There is no real agreement how we
exterminate/restrict flawed functions to progressive reduce the number
of applications and users who can access the flawed functions to allow
them to be fully disabled for 99.99 percent of people using systems.

That is what people are not getting CAP_SYS_ADMIN has too many users
to be counted and mitigation as well.

Yes we must not break userspace but we also must mitigate against
userspace issues.   We do need a clear rules on how to fix these
security problem user-space is allowed to be broken and of course made
part of the Linux kernel documentation.   Altering the binary is for
sure out because the person operating the system may not have that
right.   Placing a flag on a binary so it works I would see as
acceptable as long as that flag was not grant a stack of other
privileges that application would have never had before..

Peter Dolding

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Boris Lukashev]

[-- Attachment #1: Type: text/plain, Size: 9834 bytes --]

How about a middle-ground wherein the mitigation, with the sysctl @ 0,
spams dmesg resulting in bug reports being filed by concerned users against
the offending application mentioned in the log? If the user or distribution
desires (say they dont have suid binaries - look at Copperhead), they can
enable the sysctl and enable mitigation.
>From what little i understand of security concerns in Linux' history,
compatibility seems to be the deciding factor in determining if and how
defenses are implemented. Is this not exactly what tunable sysctls are for
- providing new functionality which can be enabled or disabled allowing
adjacent components time to adjust to the change and make proper use of it?
This permits the modification, adjacent/dependent functionality, and a
measure of compatibility for legacy consumers.
In the nominal discussion about compatibility and security or performance
and security, the notion of expediency in delivering functionality is
rarely as much a factor as it is in this effort. There are a lot of users
out there who went from having some peace of mind from what they were
getting with grsec to loss and confusion. There are also use cases where
the consequences of leaking information or suffering compromise are outside
the realm of financial loss. I'm sure everyone wants a proper workflow
informing adjacent efforts/consumers and maintaining the maximum level
compatibility possible. Waiting for everyone to get on board, including
projects where committers may check their email twice a year, isnt viable
if the objective is to protect systems and people today. There's also the
bitrot concern...
Review of these patches may go faster if its a bit less academic in terms
of finding edge cases in potential consumers, and a bit more focused on the
content and defensive measures provided - or their flaws and weaknesses as
implemented, all of which should either inform the pull request or future
efforts on the existing codebase. From an engineering standpoint - if we
want to find the consumer problems for these changes (in kernel or
userspace), we can start by building a small army of build and test bots to
build on and run in the most common distributions, fuzzing their kernels
and userspace binaries, then feeding us output, stack traces, and other
actionable intelligence.


On Thu, May 18, 2017 at 10:48 PM, Peter Dolding <oiaohm@gmail.com> wrote:

> On Thu, May 18, 2017 at 1:18 PM, Kees Cook <keescook@chromium.org> wrote:
> > On Wed, May 17, 2017 at 11:25 AM, Daniel Micay <danielmicay@gmail.com>
> wrote:
> >> On Wed, 2017-05-17 at 17:41 +0100, Alan Cox wrote:
> >>> > If we're adjusting applications, they should be made to avoid
> >>> > TIOSCTI
> >>> > completely. This looks to me a lot like the symlink restrictions:
> >>> > yes,
> >>> > userspace should be fixed to the do the right thing, but why not
> >>> > provide support to userspace to avoid the problem entirely?
> >>>
> >>> We do it's called pty/tty. There isn't any other way to do this
> >>> correctly
> >>> because TIOCSTI is just one hundreds of things the attacker can do to
> >>> make your life miserable in the case you create a child process of
> >>> lower
> >>> security privilege and give it your tty file handle or worse (like
> >>> some
> >>> container crapware) your X11 socket fd.
> >>>
> >>> Does it really matter any more or less if I reprogram your enter key,
> >>> use
> >>> TIOCSTI, set the baud rate, change all your fonts ?
> >>>
> >>> The mainstream tools like sudo get this right (*). Blocking TIOCSTI
> >>> fixes
> >>> nothing and breaks apps. If it magically fixed the problem it might
> >>> make
> >>> sense but it doesn't. You actually have to get an adult to write the
> >>> relevant code.
> >>
> >> It gets it right because it was reported as a vulnerability and fixed,
> >> which is the cycle many of these tools have gone through. It looks like
> >> sudo and coreutils / shadow su were fixed in 2005, but there are more of
> >> these tools.
> >>
> >> CVE-2005-4890 (coreutils su, shadow su, sudo), CVE-2016-7545 (SELinux
> >> sandbox utility), CVE-2016-2781 (chroot --userspec), CVE-2016-2779
> >> (util-linux runuser), CVE-2016-2568 (pkexec)
> >>
> >> I am not sure if the pkexec vulnerability is even fixed:
> >>
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1300746
> >>
> >> CVE-2017-5226 is for bubblewrap/flatpak this year, and I'm sure there
> >> were a lot more as I didn't bother to dig very deep.
> >>
> >> I completely agree that it should be solved by doing things properly in
> >> each application. However, it isn't solved properly everywhere and each
> >> new application is making the same mistake. Providing this feature does
> >> not break anything that people use in practice and it doesn't need to
> >> solve every issue to be quite useful, it only needs to prevent local
> >> privilege escalation in the form of code execution in many cases. Is
> >> there another way to get code execution via that bubblewrap/flatpak bug
> >> with this feature implemented? As far as I can tell, there isn't. It's a
> >> problem even with this feature but a significantly less serious one.
> >
> > This patch solves a real problem when userspace does things wrong. For
> > those that do not want it, a sysctl exists (and is even default off).
> > Arguments about how userspace needs to be fixed is a total red
> > herring. Like symlink protections before, this should be available for
> > those that want it because it solves an interface problem that gets
> > regularly misused and causes real damage. The kernel has a
> > responsibility to protect userspace.
> >
> > As Daniel mentions, there is a history of mistakes that appear to be
> > becoming even more common:
> > http://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=TIOCSTI
> >
> > There are people who want this feature that do not care about xemacs
> > breaking. The default cannot be enabled because of the golden rule of
> > never breaking userspace, but entirely refusing to fix a regularly
> > misused feature is wrong-headed. We must accept that not only are bug
> > lifetimes long, but that bugs will keep getting reintroduced. When
> > there is a chance to kill a class of bugs or exploit techniques, it's
> > the kernel responsibility to do so. This is a clear case of that, and
> > the solution is concise.
>
> True but what is usage of TIOCSTI that is wrong in these CVE.   Is it
> not when the returned input values values to tty crosses process
> boundaries.  Like if when process terminated everything the process
> had pushed back to TTY and Input it has received was lost those CVE
> would not be possible.
>
> Could those exist CVE when exploit happens have CAP_SYS_ADMIN as the
> process being used to break out the answer is yes.   So restricting
> TIOCSTI to CAP_SYS_ADMIN could be pointless.   Remember you have suid
> bit applications one of those might have  bug that someone is able to
> exploit to use the TIOCSTI.   So pushing to CAP_SYS_ADMIN make exploit
> rarer does not remove it.
>
> Please note you see TIOCSTI using in libc when they implement
> ungetc().   So this is something that is quite well used.   I see
> ungetc as kind of a bad idea to be implemented kernel level going to
> tty shareable between processes.
>
> > If there is some better solution that the kernel can provide to
> > mitigate processes misusing ttys, then by all means, we can add that
> > too, but has nothing to do with refusing this change. This solves a
> > specific problem that in many cases is the only path to privilege
> > escalation (as Daniel mentioned). Refusing this change is nonsense.
> > "Your car shouldn't have seat belts because maybe something will stab
> > you through the windshield" isn't a reasonable argument to make.
>
> Using cap_sys_admin as fix is like removing car windsheld because
> vision is being blocked by a rock hitting it.
>
> Kees the problem with accepting a security fix that is wrong the
> proper change never gets worked on.
>
> I am not saying there is not a real problem here.   The fix is not
> push it to CAP_SYS_ADMIN.   Due to TIOCSTI that cross process and tty
> boundaries been used in security breaks and limit applications that
> uses use as either Administrator or as normal User means this ability
> does not own in CAP_SYS_ADMIN either.
>
> The same is true of obsolete function calls that have been shoved into
> CAP_SYS_ADMIN already there comes point where no valid userspace
> application is using that function.   At that point the only thing
> using that function is exploits so then really CAP_SYS_ADMIN should
> not have access to those obsolete functions.   There is a real need
> for CAP_SYS_OBSOLETE.   If a program has to ahve CAP_SYS_OBSOLETE set
> on it this means it using functionally that is known busted in some
> way..
>
> This is the biggest problem here.   There is no real agreement how we
> exterminate/restrict flawed functions to progressive reduce the number
> of applications and users who can access the flawed functions to allow
> them to be fully disabled for 99.99 percent of people using systems.
>
> That is what people are not getting CAP_SYS_ADMIN has too many users
> to be counted and mitigation as well.
>
> Yes we must not break userspace but we also must mitigate against
> userspace issues.   We do need a clear rules on how to fix these
> security problem user-space is allowed to be broken and of course made
> part of the Linux kernel documentation.   Altering the binary is for
> sure out because the person operating the system may not have that
> right.   Placing a flag on a binary so it works I would see as
> acceptable as long as that flag was not grant a stack of other
> privileges that application would have never had before..
>
> Peter Dolding
>



-- 
Boris Lukashev
Systems Architect
Semper Victus

[-- Attachment #2: Type: text/html, Size: 11805 bytes --]

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Serge E. Hallyn]

On Fri, May 19, 2017 at 12:48:17PM +1000, Peter Dolding wrote:
> Using cap_sys_admin as fix is like removing car windsheld because
> vision is being blocked by a rock hitting it.

Nonsense.  If the application has cap_sys_admin then it is less contained and
more trusted anyway.  If I went to the trouble to run an application in a
private user namespace (where it can have cap_sys_admin, but not targeted
at my tty) then it should be more contained.  That's the point of targeted
capabilities.

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Peter Dolding]

On Sat, May 20, 2017 at 12:33 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> On Fri, May 19, 2017 at 12:48:17PM +1000, Peter Dolding wrote:
>> Using cap_sys_admin as fix is like removing car windsheld because
>> vision is being blocked by a rock hitting it.
>
> Nonsense.  If the application has cap_sys_admin then it is less contained and
> more trusted anyway.  If I went to the trouble to run an application in a
> private user namespace (where it can have cap_sys_admin, but not targeted
> at my tty) then it should be more contained.  That's the point of targeted
> capabilities.

The thing that is missed every time is how much is cap_sys_admin.

So you are saying a user namespace has to be set up to contain the defect.

Really no application should have cap_sys_admin.

The theory of capabilities is that security should be broken down into
logical blocks.

So tty stuff should under a tty capabilities.

This one here should not be shoved into cap_sys_admin because can you
show a single case of a general used application performing this
action in the exploit way that is normal behaviour.

The exploits are doing behaviours that have no general place.

Its really simple to shove everything to cap_sys_admin instead of hey
lets look at the exploits how they work and if this should be fairly
blanked banned.  The behaviour that is question is being able push
chars into input stream and have them processed after application has
terminated or after application has switched to background.   That is
not pushing data into another tty.   Pushing data into a different tty
is already restricted to cap_sys_admin.

Personally from my point of view when application terminates or
switches to background  what ever it pushed back into the input buffer
should be junked and maybe a special cap to deal with rare case of
applications that expect this behavour.

Also please remember one of the application using this behaviour of
pushing stuff back to input buffer is csh.  In other words a general
user shell.   This will not be the only application that is general
usage after the change of pushing to cap_sys_admin that would also
have to be pushed to cap_sys_admin because they use TIOCSTI in a way
that the patch will block when the program does not have
cap_sys_admin.   So now you have more applications running as
cap_sys_admin so more security problems.

Peter Dolding.

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Serge E. Hallyn]

Quoting Peter Dolding (oiaohm@gmail.com):
> On Sat, May 20, 2017 at 12:33 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> > On Fri, May 19, 2017 at 12:48:17PM +1000, Peter Dolding wrote:
> >> Using cap_sys_admin as fix is like removing car windsheld because
> >> vision is being blocked by a rock hitting it.
> >
> > Nonsense.  If the application has cap_sys_admin then it is less contained and
> > more trusted anyway.  If I went to the trouble to run an application in a
> > private user namespace (where it can have cap_sys_admin, but not targeted
> > at my tty) then it should be more contained.  That's the point of targeted
> > capabilities.
> 
> The thing that is missed every time is how much is cap_sys_admin.
> 
> So you are saying a user namespace has to be set up to contain the defect.
> 
> Really no application should have cap_sys_admin.
> 
> The theory of capabilities is that security should be broken down into
> logical blocks.
> 
> So tty stuff should under a tty capabilities.

(last reply on this)

Currently capabilities.7 says

              * employ  the  TIOCSTI ioctl(2) to insert characters into the input queue of a
                terminal other than the caller's controlling terminal;

for CAP_SYS_ADMIN.

So you can create a new CAP_SYS_TIOCSSTI if you like, and offer a patch where
*both* CAP_SYS_ADMIN and CAP_SYS_ADMIN suffice.  Again, see CAP_SYSLOG for a
prior example.

What you may not do is change it so that on an older kernel you must have
CAP_SYS_ADMIN to use TIOCSTI, while on a newer one it does not suffice.

-serge

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Alan Cox]

> > So tty stuff should under a tty capabilities.  
> 
> (last reply on this)
> 
> Currently capabilities.7 says
> 
>               * employ  the  TIOCSTI ioctl(2) to insert characters into the input queue of a
>                 terminal other than the caller's controlling terminal;
> 
> for CAP_SYS_ADMIN.
> 
> So you can create a new CAP_SYS_TIOCSSTI if you like, and offer a patch where
> *both* CAP_SYS_ADMIN and CAP_SYS_ADMIN suffice.  Again, see CAP_SYSLOG for a
> prior example.

Even then it wouldn't be useful because the attacker can use every other
interface in the tty layer, many of which you can't magic away behind a
capability bit. And the applications would need changing to use the
feature - at which point any theoretical broken apps can instead be fixed
to use a pty/tty pair and actually fix the real problem.

Alan

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Peter Dolding]

On Wed, May 31, 2017 at 7:52 AM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
>> > So tty stuff should under a tty capabilities.
>>
>> (last reply on this)
>>
>> Currently capabilities.7 says
>>
>>               * employ  the  TIOCSTI ioctl(2) to insert characters into the input queue of a
>>                 terminal other than the caller's controlling terminal;
>>
>> for CAP_SYS_ADMIN.
>>
>> So you can create a new CAP_SYS_TIOCSSTI if you like, and offer a patch where
>> *both* CAP_SYS_ADMIN and CAP_SYS_ADMIN suffice.  Again, see CAP_SYSLOG for a
>> prior example.
>
> Even then it wouldn't be useful because the attacker can use every other
> interface in the tty layer, many of which you can't magic away behind a
> capability bit. And the applications would need changing to use the
> feature - at which point any theoretical broken apps can instead be fixed
> to use a pty/tty pair and actually fix the real problem.
>
Alan is right.   CAP_SYS_ADMIN allows crossing the tty barrier.

Broken applications that you can wrap in a pty/tty pair as the lxc
application does would be defeated if those applications move up to
CAP_SYS_ADMIN.  Because you have granted the high right of cross
pty/tty containment.

Pushing CAP_SYS_TIOSSTI out by itself without the feature in
CAP_SYS_ADMIN means broken applications can be allowed to run in like
a lxc container where they cannot go anywhere with the exploit because
the pty/tty they are picking is not going to get them very far at all.

Pushing TIOSSTI up to CAP_SYS_ADMIN to address this problem is wrong.
 Question is also how many applications use CAP_SYS_ADMIN feature to
push chars into other pty/tty on the system.   Pushing across pty/tty
barrier may not be a suitable feature to be generically in
CAP_SYS_ADMIN in the first place.

http://www.halfdog.net/Security/2012/TtyPushbackPrivilegeEscalation/

This here is example of TIOSSTI pushback as CAP_SYS_ADMIN being bad.
I don't know of a genuine program using push back in exploiting way
where the pushed back input is expected to be processed after the
program has terminated.

Really we need to work out how many breakage will in fact be caused by
majority restricting both pushback and write across tty barrier.
This is not like CAP_SYS_LOG these are features that can be used to
exploit system badly.   It is possible that the exploiting form of
TIOSSTI pushback is used by nothing genuine userspace in any properly
functional case.   So if that is the case unconstrained TIOSSTI
push-back would only be making application crashes worse.

The reason I want TIOSSTI pushback moved to its own CAP_SYS first is
to find out if anything is in fact using it as part of genuine usage
and allowing anyone caught out to work around it.    I am sorry this
is me most likely using X11 logic break it and see if anyone yells.
If no one complains disappear the feature completely then this closes
this form of exploit for good.

Peter Dolding

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Alan Cox]

> Alan is right.   CAP_SYS_ADMIN allows crossing the tty barrier.

I don't need CAP_ anything to mmap your frame buffer, or use selection to
cut and paste text into the terminal.

> Broken applications that you can wrap in a pty/tty pair as the lxc
> application does would be defeated if those applications move up to
> CAP_SYS_ADMIN.  Because you have granted the high right of cross
> pty/tty containment.

Yes

> I don't know of a genuine program using push back in exploiting way
> where the pushed back input is expected to be processed after the
> program has terminated.

So there are two real problems here

1. We don't know what namespace each character belongs to, so there's no
way we can construct a model where pushed symbols only appear in the
namespace they are pushed from. That would be a nice situation but it's
not at all obvious there is a sane way to implement it.

2. Focussing on TIOCSTI is just ignoring the bigger picture. TIOCSTI is
actually a lot less nasty in many situations than a framebuffer mmap and
spying attack where a container run from the console could sit and watch
you. TIOCSTI is in some ways the easiest to fix because setsid() will let
you mitigate it in certain cases whereas I'm fairly sure the selection
based console attack doesn't need controlling terminal rights.

In the case you have a less privileged subshell you need a whole new tty
context, and there's no obvious way for the kernel to magic one into
existance so that for example the container can change it's own baud rate
but not the baud rate of any app outside the container.

ioctl whitelisting will break stuff, but an SELinux/AppArmour/seccomp
whitelisting based solution is probably the only practical one you can
implement usefully, and for a lot of container users would be ok.

And yes there are genuine users of TIOCSTI although these days most
things just use their own pty/tty pair.

Alan

Re: [kernel-hardening] Re: [PATCH v6 0/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

[Serge E. Hallyn]

Quoting Alan Cox (gnomes@lxorguk.ukuu.org.uk):
> > Alan is right.   CAP_SYS_ADMIN allows crossing the tty barrier.
> 
> I don't need CAP_ anything to mmap your frame buffer, or use selection to
> cut and paste text into the terminal.
> 
> > Broken applications that you can wrap in a pty/tty pair as the lxc
> > application does would be defeated if those applications move up to
> > CAP_SYS_ADMIN.  Because you have granted the high right of cross
> > pty/tty containment.
> 
> Yes

Right.

> > I don't know of a genuine program using push back in exploiting way
> > where the pushed back input is expected to be processed after the
> > program has terminated.
> 
> So there are two real problems here
> 
> 1. We don't know what namespace each character belongs to, so there's no
> way we can construct a model where pushed symbols only appear in the
> namespace they are pushed from. That would be a nice situation but it's
> not at all obvious there is a sane way to implement it.
> 
> 2. Focussing on TIOCSTI is just ignoring the bigger picture. TIOCSTI is
> actually a lot less nasty in many situations than a framebuffer mmap and
> spying attack where a container run from the console could sit and watch
> you. TIOCSTI is in some ways the easiest to fix because setsid() will let
> you mitigate it in certain cases whereas I'm fairly sure the selection
> based console attack doesn't need controlling terminal rights.
> 
> In the case you have a less privileged subshell you need a whole new tty
> context, and there's no obvious way for the kernel to magic one into
> existance so that for example the container can change it's own baud rate
> but not the baud rate of any app outside the container.
> 
> ioctl whitelisting will break stuff, but an SELinux/AppArmour/seccomp
> whitelisting based solution is probably the only practical one you can
> implement usefully, and for a lot of container users would be ok.

Seccomp policy could refuse TIOCSTI to any fd, but that's not ideal.  It could
be customized per-application-start to only refuse TIOCSTI to the passed-in tty
fd, but you'd have to prevent dup then right?  Selinux can label the fd so it's
in fact ideal here.  But - is there something clever a seccomp policy could do
to work only on specified fds and any that were dup'ed?

Mind you, I of course agree that creating a new pty and passing only that in
is the sane fix.  Maybe in the end this thread will serve best as a loud
reminder / teaching moment about this issue for those about to write such an
application.