[PATCH] Restrict access to TIOCLINUX

[PATCH] Restrict access to TIOCLINUX

[Hanno Böck]

Hi,

I'm sending this here before I'll try to send it to lkml and the
respective maintainers to get some feedback first.

The TIOCLINUX functionality in the kernel can be abused for privilege
escalation, similar to TIOCSTI. I considered a few options how to fix
this, and this is what I came up with.


Restrict access to TIOCLINUX

TIOCLINUX can be used for privilege escalation on virtual terminals when
code is executed via tools like su/sudo.
By abusing the selection features a lower-privileged application can
write content to the console, select and copy/paste that content and
thereby executing code on the privileged account. See also the poc here:
  https://www.openwall.com/lists/oss-security/2023/03/14/3

Selection is usually used by tools like gpm that provide mouse features
on the virtual console. gpm already runs as root (due to earlier
changes that restrict access to a user on the current tty), therefore
it will still work with this change.

The security problem mitigated is similar to the security risks caused
by TIOCSTI, which, since kernel 6.2, can be disabled with
CONFIG_LEGACY_TIOCSTI=n.

Signed-off-by: Hanno Böck <hanno@hboeck.de>
---
 drivers/tty/vt/vt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 3c2ea9c098f7..3671173109b8 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3146,10 +3146,14 @@ int tioclinux(struct tty_struct *tty, unsigned
long arg) switch (type)
 	{
 		case TIOCL_SETSEL:
+			if (!capable(CAP_SYS_ADMIN))
+				return -EPERM;
 			ret = set_selection_user((struct
tiocl_selection __user *)(p+1), tty);
 			break;
 		case TIOCL_PASTESEL:
+			if (!capable(CAP_SYS_ADMIN))
+				return -EPERM;
 			ret = paste_selection(tty);
 			break;
 		case TIOCL_UNBLANKSCREEN:
@@ -3158,6 +3162,8 @@ int tioclinux(struct tty_struct *tty, unsigned
long arg) console_unlock();
 			break;
 		case TIOCL_SELLOADLUT:
+			if (!capable(CAP_SYS_ADMIN))
+				return -EPERM;
 			console_lock();
 			ret = sel_loadlut(p);
 			console_unlock();
-- 
2.40.0

-- 
Hanno Böck
https://hboeck.de/

Re: [PATCH] Restrict access to TIOCLINUX

[Greg KH]

On Sun, Apr 02, 2023 at 04:08:15PM +0200, Hanno Böck wrote:
> Hi,
> 
> I'm sending this here before I'll try to send it to lkml and the
> respective maintainers to get some feedback first.
> 
> The TIOCLINUX functionality in the kernel can be abused for privilege
> escalation, similar to TIOCSTI. I considered a few options how to fix
> this, and this is what I came up with.
> 
> 
> Restrict access to TIOCLINUX
> 
> TIOCLINUX can be used for privilege escalation on virtual terminals when
> code is executed via tools like su/sudo.
> By abusing the selection features a lower-privileged application can
> write content to the console, select and copy/paste that content and
> thereby executing code on the privileged account. See also the poc here:
>   https://www.openwall.com/lists/oss-security/2023/03/14/3
> 
> Selection is usually used by tools like gpm that provide mouse features
> on the virtual console. gpm already runs as root (due to earlier
> changes that restrict access to a user on the current tty), therefore
> it will still work with this change.
> 
> The security problem mitigated is similar to the security risks caused
> by TIOCSTI, which, since kernel 6.2, can be disabled with
> CONFIG_LEGACY_TIOCSTI=n.
> 
> Signed-off-by: Hanno Böck <hanno@hboeck.de>
> ---
>  drivers/tty/vt/vt.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 3c2ea9c098f7..3671173109b8 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -3146,10 +3146,14 @@ int tioclinux(struct tty_struct *tty, unsigned
> long arg) switch (type)
>  	{
>  		case TIOCL_SETSEL:
> +			if (!capable(CAP_SYS_ADMIN))
> +				return -EPERM;

You just now broke any normal user programs that required this (or the
other ioctls), and so you are going to have to force them to be run with
CAP_SYS_ADMIN permissions?  That feels like you are going backwards in
security, not forwards.

And you didn't change anything for programs like gpm that already had
root permission (and shouldn't that permission be dropped anyway?)

thanks,

greg k-h

Re: [PATCH] Restrict access to TIOCLINUX

[Hanno Böck]

On Sun, 2 Apr 2023 16:55:01 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:

> You just now broke any normal user programs that required this (or the
> other ioctls), and so you are going to have to force them to be run
> with CAP_SYS_ADMIN permissions? 

Are you aware of such normal user programs?
It was my impression that this is a relatively obscure feature and gpm
is pretty much the only tool using it.

> And you didn't change anything for programs like gpm that already had
> root permission (and shouldn't that permission be dropped anyway?)

Well, you could restrict all that to a specific capability. However, it
is my understanding that the existing capability system is limited in
the number of capabilities and new ones should only be introduced in
rare cases. It does not seem a feature probably few people use anyway
deserves a new capability.

Do you have other proposals how to fix this issue? One could introduce
an option like for TIOCSTI that allows disabling selection features by
default.


-- 
Hanno Böck
https://hboeck.de/

Re: [PATCH] Restrict access to TIOCLINUX

[Greg KH]

On Sun, Apr 02, 2023 at 07:16:52PM +0200, Hanno Böck wrote:
> On Sun, 2 Apr 2023 16:55:01 +0200
> Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > You just now broke any normal user programs that required this (or the
> > other ioctls), and so you are going to have to force them to be run
> > with CAP_SYS_ADMIN permissions? 
> 
> Are you aware of such normal user programs?
> It was my impression that this is a relatively obscure feature and gpm
> is pretty much the only tool using it.

"Pretty much" does not mean "none" :(

> > And you didn't change anything for programs like gpm that already had
> > root permission (and shouldn't that permission be dropped anyway?)
> 
> Well, you could restrict all that to a specific capability. However, it
> is my understanding that the existing capability system is limited in
> the number of capabilities and new ones should only be introduced in
> rare cases. It does not seem a feature probably few people use anyway
> deserves a new capability.

I did not suggest that a new capability be created for this, that would
be an abust of the capability levels for sure.

> Do you have other proposals how to fix this issue? One could introduce
> an option like for TIOCSTI that allows disabling selection features by
> default.

What exact issue are you trying to fix here?

thanks,

greg k-h

Re: [PATCH] Restrict access to TIOCLINUX

[Hanno Böck]

On Sun, 2 Apr 2023 19:23:44 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:

> > Do you have other proposals how to fix this issue? One could
> > introduce an option like for TIOCSTI that allows disabling
> > selection features by default.  
> 
> What exact issue are you trying to fix here?

The fact that the selection features of TIOCLINUX can be used for
privilege escalation.

I already mentioned this in the original patch description, but I think
the minitty.c example here illustrates this well:
https://www.openwall.com/lists/oss-security/2023/03/14/3

Compile it, do
sudo -u [anynonprivilegeduser] ./minitty

It'll execute shell code with root permission.


-- 
Hanno Böck
https://hboeck.de/

Re: [PATCH] Restrict access to TIOCLINUX

[Greg KH]

On Sun, Apr 02, 2023 at 07:33:10PM +0200, Hanno Böck wrote:
> On Sun, 2 Apr 2023 19:23:44 +0200
> Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > > Do you have other proposals how to fix this issue? One could
> > > introduce an option like for TIOCSTI that allows disabling
> > > selection features by default.  
> > 
> > What exact issue are you trying to fix here?
> 
> The fact that the selection features of TIOCLINUX can be used for
> privilege escalation.

Only if you had root permissions already, and then go to try to run
something using su or sudo as someone with less permission, right?

And as you already had permissions before, it's not really an
excalation, or am I missing something?

> I already mentioned this in the original patch description, but I think
> the minitty.c example here illustrates this well:
> https://www.openwall.com/lists/oss-security/2023/03/14/3
> 
> Compile it, do
> sudo -u [anynonprivilegeduser] ./minitty
> 
> It'll execute shell code with root permission.

That doesn't work if you run it from a user without root permissions to
start with, right?

thanks,

greg k-h

Re: [PATCH] Restrict access to TIOCLINUX

[Jordan Glover]

On Sunday, April 2nd, 2023 at 7:44 PM, Greg KH <gregkh@linuxfoundation.org> wrote:


> On Sun, Apr 02, 2023 at 07:33:10PM +0200, Hanno Böck wrote:
> 
> > On Sun, 2 Apr 2023 19:23:44 +0200
> > Greg KH gregkh@linuxfoundation.org wrote:
> > 
> > > > Do you have other proposals how to fix this issue? One could
> > > > introduce an option like for TIOCSTI that allows disabling
> > > > selection features by default.
> > > 
> > > What exact issue are you trying to fix here?
> > 
> > The fact that the selection features of TIOCLINUX can be used for
> > privilege escalation.
> 
> 
> Only if you had root permissions already, and then go to try to run
> something using su or sudo as someone with less permission, right?
> 
> And as you already had permissions before, it's not really an
> excalation, or am I missing something?
> 
> > I already mentioned this in the original patch description, but I think
> > the minitty.c example here illustrates this well:
> > https://www.openwall.com/lists/oss-security/2023/03/14/3
> > 
> > Compile it, do
> > sudo -u [anynonprivilegeduser] ./minitty
> > 
> > It'll execute shell code with root permission.
> 
> 
> That doesn't work if you run it from a user without root permissions to
> start with, right?
> 
> thanks,
> 
> greg k-h

The problem in the example is that sudo executed unpriv process which then re-gained the privs of sudo itself. It doesn't need to be sudo or even root - the same problem affects all containers/sandboxes (privileged or not) in linux - the (supposedly) contained process can use TIOCSTI/TIOCLINUX to break out of the container/sandbox (unless they're blocked as well).

BTW: you seem in favor of restricting TIOCSTI [1] which landed in kernel so why suddenly question same problem here?

[1] https://lore.kernel.org/all/Y0pIRKpPwqk2Igu%2F@kroah.com/raw

Re: [PATCH] Restrict access to TIOCLINUX

[Günther Noack]

Hello!

+CC the people involved in TIOCSTI

This patch seems sensible to me --
and I would like to kindly ask you to reconsider it.

On Sun, Apr 02, 2023 at 07:23:44PM +0200, Greg KH wrote:
> On Sun, Apr 02, 2023 at 07:16:52PM +0200, Hanno Böck wrote:
> > On Sun, 2 Apr 2023 16:55:01 +0200
> > Greg KH <gregkh@linuxfoundation.org> wrote:
> > 
> > > You just now broke any normal user programs that required this (or the
> > > other ioctls), and so you are going to have to force them to be run
> > > with CAP_SYS_ADMIN permissions? 
> > 
> > Are you aware of such normal user programs?
> > It was my impression that this is a relatively obscure feature and gpm
> > is pretty much the only tool using it.
> 
> "Pretty much" does not mean "none" :(

This patch only affects TIOCLINUX subcodes which are responsible for text
cut-and-paste, TIOCL_SETSEL, TIOCL_PASTESEL and TIOCL_SELLOADLUT.

The only program that I am aware of which uses cut&paste on the console is gpm.
My web searches for these subcode names have only surfaced Linux header files
and discussions about their security problems.


> > > And you didn't change anything for programs like gpm that already had
> > > root permission (and shouldn't that permission be dropped anyway?)
> > 
> > Well, you could restrict all that to a specific capability. However, it
> > is my understanding that the existing capability system is limited in
> > the number of capabilities and new ones should only be introduced in
> > rare cases. It does not seem a feature probably few people use anyway
> > deserves a new capability.
> 
> I did not suggest that a new capability be created for this, that would
> be an abust of the capability levels for sure.
> 
> > Do you have other proposals how to fix this issue? One could introduce
> > an option like for TIOCSTI that allows disabling selection features by
> > default.
> 
> What exact issue are you trying to fix here?

It's the same problem as with TIOCSTI, which got (optionally) disabled for
non-CAP_SYS_ADMIN in commit 83efeeeb3d04 ("tty: Allow TIOCSTI to be disabled")
and commit 690c8b804ad2 ("TIOCSTI: always enable for CAP_SYS_ADMIN").

The number of exploits which have used TIOCSTI in the past is long[1] and has
affected multiple sandboxing and sudo-like tools.  If the user is using the
console, TIOCLINUX's cut&paste functionality can replace TIOCSTI in these
exploits.

We have this problem with the Landlock LSM as well, with both TIOCSTI and these
TIOCLINUX subcodes.

Here is an example scenario:

* User runs a vulnerable version of the "ping" command from the console.

* The "ping" command is a hardened version which puts itself into a Landlock
  sandbox, but it still has the TTY FD through stdout.

* Ping gets buffer-overflow-exploited by an attacker through ping responses.

* The attacker can't directly access the file system, but the attacker can
  escape the sandbox by controlling the surrounding (non-sandboxed) shell on its
  terminal through TIOCLINUX.

The ping example is not completely made up -- FreeBSD had such a vulnerability
in its ping utility in 2022[2].  The impact of the vulnerability was mitigated
by FreeBSD's Capsicum sandboxing.

The correct solution for the problem on Linux is to my knowledge to create a
pty/tty pair, but that is somewhat impractical for small utilities like ping, in
order to restrict themselves (they would need to create a sidecar process to
shovel the data back and forth).  Workarounds include setsid() and seccomp-bpf,
but they also have their limits and are not a clean solution.  We've previously
discussed it in [3].

I do believe that requiring CAP_SYS_ADMIN for TIOCLINUX's TIOCL_PASTESEL subcode
would be a better approach than so many sudo-style and sandboxing tools having
to learn this lesson the hard way.  Can we please reconsider this patch?

—Günther

[1] https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=TIOCSTI
[2] https://www.freebsd.org/security/advisories/FreeBSD-SA-22:15.ping.asc
[3] https://lore.kernel.org/all/20230626.0a8f70d4228e@gnoack.org/

-- 
Sent using Mutt 🐕 Woof Woof

Re: [PATCH] Restrict access to TIOCLINUX

[Greg KH]

On Fri, Aug 18, 2023 at 06:10:18PM +0200, Günther Noack wrote:
> Hello!
> 
> +CC the people involved in TIOCSTI
> 
> This patch seems sensible to me --
> and I would like to kindly ask you to reconsider it.
> 
> On Sun, Apr 02, 2023 at 07:23:44PM +0200, Greg KH wrote:
> > On Sun, Apr 02, 2023 at 07:16:52PM +0200, Hanno Böck wrote:
> > > On Sun, 2 Apr 2023 16:55:01 +0200
> > > Greg KH <gregkh@linuxfoundation.org> wrote:
> > > 
> > > > You just now broke any normal user programs that required this (or the
> > > > other ioctls), and so you are going to have to force them to be run
> > > > with CAP_SYS_ADMIN permissions? 
> > > 
> > > Are you aware of such normal user programs?
> > > It was my impression that this is a relatively obscure feature and gpm
> > > is pretty much the only tool using it.
> > 
> > "Pretty much" does not mean "none" :(
> 
> This patch only affects TIOCLINUX subcodes which are responsible for text
> cut-and-paste, TIOCL_SETSEL, TIOCL_PASTESEL and TIOCL_SELLOADLUT.
> 
> The only program that I am aware of which uses cut&paste on the console is gpm.
> My web searches for these subcode names have only surfaced Linux header files
> and discussions about their security problems.

Is gpm running with the needed permissions already?

> > > > And you didn't change anything for programs like gpm that already had
> > > > root permission (and shouldn't that permission be dropped anyway?)
> > > 
> > > Well, you could restrict all that to a specific capability. However, it
> > > is my understanding that the existing capability system is limited in
> > > the number of capabilities and new ones should only be introduced in
> > > rare cases. It does not seem a feature probably few people use anyway
> > > deserves a new capability.
> > 
> > I did not suggest that a new capability be created for this, that would
> > be an abust of the capability levels for sure.
> > 
> > > Do you have other proposals how to fix this issue? One could introduce
> > > an option like for TIOCSTI that allows disabling selection features by
> > > default.
> > 
> > What exact issue are you trying to fix here?
> 
> It's the same problem as with TIOCSTI, which got (optionally) disabled for
> non-CAP_SYS_ADMIN in commit 83efeeeb3d04 ("tty: Allow TIOCSTI to be disabled")
> and commit 690c8b804ad2 ("TIOCSTI: always enable for CAP_SYS_ADMIN").
> 
> The number of exploits which have used TIOCSTI in the past is long[1] and has
> affected multiple sandboxing and sudo-like tools.  If the user is using the
> console, TIOCLINUX's cut&paste functionality can replace TIOCSTI in these
> exploits.
> 
> We have this problem with the Landlock LSM as well, with both TIOCSTI and these
> TIOCLINUX subcodes.
> 
> Here is an example scenario:
> 
> * User runs a vulnerable version of the "ping" command from the console.

Don't do that :)

> * The "ping" command is a hardened version which puts itself into a Landlock
>   sandbox, but it still has the TTY FD through stdout.
> 
> * Ping gets buffer-overflow-exploited by an attacker through ping responses.

You allowed a root-permissioned program to accept unsolicted network
code, why is it the kernel's issue here?

> * The attacker can't directly access the file system, but the attacker can
>   escape the sandbox by controlling the surrounding (non-sandboxed) shell on its
>   terminal through TIOCLINUX.
> 
> The ping example is not completely made up -- FreeBSD had such a vulnerability
> in its ping utility in 2022[2].  The impact of the vulnerability was mitigated
> by FreeBSD's Capsicum sandboxing.
> 
> The correct solution for the problem on Linux is to my knowledge to create a
> pty/tty pair, but that is somewhat impractical for small utilities like ping, in
> order to restrict themselves (they would need to create a sidecar process to
> shovel the data back and forth).  Workarounds include setsid() and seccomp-bpf,
> but they also have their limits and are not a clean solution.  We've previously
> discussed it in [3].
> 
> I do believe that requiring CAP_SYS_ADMIN for TIOCLINUX's TIOCL_PASTESEL subcode
> would be a better approach than so many sudo-style and sandboxing tools having
> to learn this lesson the hard way.  Can we please reconsider this patch?

Have you verified that nothing will break with this?

If so, it needs to be submitted in a form that could be accepted (this
one was not, so I couldn't take it even if I wanted to), and please add
a tested-by from you and we will be glad to reconsider it.

thanks,

greg k-h

Re: [PATCH] Restrict access to TIOCLINUX

[Boris Lukashev]

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

See this asked a lot, and as someone who works mostly in languages with robust test facilities, I gotta ask: why isn't kernel code mandated to be submitted with tests?

The maple tree thing was kind of a mess and was a much bigger change than the stuff people push in these security machinations. With the pushback upstream gives on this stuff its no wonder that actual progress on kernsec is made by an out of tree third party. Upstream does not make it easy to improve boundaries and controls or establish coherent awareness.

Introducing tests and a public, expandable, well-defined threat model against which defensive measures are built would permit smaller/piecemeal efforts to cobble together a stronger overall posture (and automate regression testing for new features which are not necessarily seen as being in the security domain).

-Boris

On August 22, 2023 8:07:24 AM EDT, Greg KH <gregkh@linuxfoundation.org> wrote:
>On Fri, Aug 18, 2023 at 06:10:18PM +0200, Günther Noack wrote:
>> Hello!
>> 
>> +CC the people involved in TIOCSTI
>> 
>> This patch seems sensible to me --
>> and I would like to kindly ask you to reconsider it.
>> 
>> On Sun, Apr 02, 2023 at 07:23:44PM +0200, Greg KH wrote:
>> > On Sun, Apr 02, 2023 at 07:16:52PM +0200, Hanno Böck wrote:
>> > > On Sun, 2 Apr 2023 16:55:01 +0200
>> > > Greg KH <gregkh@linuxfoundation.org> wrote:
>> > > 
>> > > > You just now broke any normal user programs that required this (or the
>> > > > other ioctls), and so you are going to have to force them to be run
>> > > > with CAP_SYS_ADMIN permissions? 
>> > > 
>> > > Are you aware of such normal user programs?
>> > > It was my impression that this is a relatively obscure feature and gpm
>> > > is pretty much the only tool using it.
>> > 
>> > "Pretty much" does not mean "none" :(
>> 
>> This patch only affects TIOCLINUX subcodes which are responsible for text
>> cut-and-paste, TIOCL_SETSEL, TIOCL_PASTESEL and TIOCL_SELLOADLUT.
>> 
>> The only program that I am aware of which uses cut&paste on the console is gpm.
>> My web searches for these subcode names have only surfaced Linux header files
>> and discussions about their security problems.
>
>Is gpm running with the needed permissions already?
>
>> > > > And you didn't change anything for programs like gpm that already had
>> > > > root permission (and shouldn't that permission be dropped anyway?)
>> > > 
>> > > Well, you could restrict all that to a specific capability. However, it
>> > > is my understanding that the existing capability system is limited in
>> > > the number of capabilities and new ones should only be introduced in
>> > > rare cases. It does not seem a feature probably few people use anyway
>> > > deserves a new capability.
>> > 
>> > I did not suggest that a new capability be created for this, that would
>> > be an abust of the capability levels for sure.
>> > 
>> > > Do you have other proposals how to fix this issue? One could introduce
>> > > an option like for TIOCSTI that allows disabling selection features by
>> > > default.
>> > 
>> > What exact issue are you trying to fix here?
>> 
>> It's the same problem as with TIOCSTI, which got (optionally) disabled for
>> non-CAP_SYS_ADMIN in commit 83efeeeb3d04 ("tty: Allow TIOCSTI to be disabled")
>> and commit 690c8b804ad2 ("TIOCSTI: always enable for CAP_SYS_ADMIN").
>> 
>> The number of exploits which have used TIOCSTI in the past is long[1] and has
>> affected multiple sandboxing and sudo-like tools.  If the user is using the
>> console, TIOCLINUX's cut&paste functionality can replace TIOCSTI in these
>> exploits.
>> 
>> We have this problem with the Landlock LSM as well, with both TIOCSTI and these
>> TIOCLINUX subcodes.
>> 
>> Here is an example scenario:
>> 
>> * User runs a vulnerable version of the "ping" command from the console.
>
>Don't do that :)
>
>> * The "ping" command is a hardened version which puts itself into a Landlock
>>   sandbox, but it still has the TTY FD through stdout.
>> 
>> * Ping gets buffer-overflow-exploited by an attacker through ping responses.
>
>You allowed a root-permissioned program to accept unsolicted network
>code, why is it the kernel's issue here?
>
>> * The attacker can't directly access the file system, but the attacker can
>>   escape the sandbox by controlling the surrounding (non-sandboxed) shell on its
>>   terminal through TIOCLINUX.
>> 
>> The ping example is not completely made up -- FreeBSD had such a vulnerability
>> in its ping utility in 2022[2].  The impact of the vulnerability was mitigated
>> by FreeBSD's Capsicum sandboxing.
>> 
>> The correct solution for the problem on Linux is to my knowledge to create a
>> pty/tty pair, but that is somewhat impractical for small utilities like ping, in
>> order to restrict themselves (they would need to create a sidecar process to
>> shovel the data back and forth).  Workarounds include setsid() and seccomp-bpf,
>> but they also have their limits and are not a clean solution.  We've previously
>> discussed it in [3].
>> 
>> I do believe that requiring CAP_SYS_ADMIN for TIOCLINUX's TIOCL_PASTESEL subcode
>> would be a better approach than so many sudo-style and sandboxing tools having
>> to learn this lesson the hard way.  Can we please reconsider this patch?
>
>Have you verified that nothing will break with this?
>
>If so, it needs to be submitted in a form that could be accepted (this
>one was not, so I couldn't take it even if I wanted to), and please add
>a tested-by from you and we will be glad to reconsider it.
>
>thanks,
>
>greg k-h

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

Re: [PATCH] Restrict access to TIOCLINUX

[Greg KH]

On Tue, Aug 22, 2023 at 08:51:03AM -0400, Boris Lukashev wrote:
> See this asked a lot, and as someone who works mostly in languages
> with robust test facilities, I gotta ask: why isn't kernel code
> mandated to be submitted with tests?

Because for most/many of the kernel api, we don't have tests yet.  LTP
covers a lot of this type of thing, so no need to duplicate that.  But
yes, if you want to mandate this for your subsystem, that would be
great!

And many subsystems do mandate that, like drm and bpf.   So if you want
to bring the current LTP tty tests into the kernel test framework, I'll
gladly take those patches.

But I fail to see how this is relevant for this proposed change, which
would restrict access to an ioctl, how would a test help here?

thanks,

greg k-h

Re: [PATCH] Restrict access to TIOCLINUX

[Günther Noack]

Hi!

Thanks for re-considering this patch set!

On Tue, Aug 22, 2023 at 02:07:24PM +0200, Greg KH wrote:
> On Fri, Aug 18, 2023 at 06:10:18PM +0200, Günther Noack wrote:
> > The only program that I am aware of which uses cut&paste on the console is gpm.
> > My web searches for these subcode names have only surfaced Linux header files
> > and discussions about their security problems.
> 
> Is gpm running with the needed permissions already?

Yes, this should work.

GPM runs as root with the CAP_SYS_ADMIN capability (and many others).


> > We have this problem with the Landlock LSM as well, with both TIOCSTI and these
> > TIOCLINUX subcodes.
> > 
> > Here is an example scenario:
> > 
> > * User runs a vulnerable version of the "ping" command from the console.
> 
> Don't do that :)
> 
> > * The "ping" command is a hardened version which puts itself into a Landlock
> >   sandbox, but it still has the TTY FD through stdout.
> > 
> > * Ping gets buffer-overflow-exploited by an attacker through ping responses.
> 
> You allowed a root-permissioned program to accept unsolicted network
> code, why is it the kernel's issue here?

I did not mean to imply that ping runs as root due to the setuid flag.  In this
scenario, ping runs as a normal user with only a few additional networking
capabilities (as I believe it is common on most distributions now?).

Also, as Landlock is a sandboxing feature, it makes sense to assume from that
perspective that the confined process is already hostile.

So the privilege boundary that the example is about is not that the ping process
was successfully attacked (that is a problem too), but it is that the
now-hostile ping process can escape the sandbox through the TTY file descriptor.

It is the *shell* which has more privileges for accessing *files* than the ping
process has.  This is because the ping process has self-confined itself with a
Landlock sandbox before it was attacked, and therefore ping itself only has very
limited access to files.

ping is indeed a bit unusual because of the special capabilities it needs -- the
same example would also apply to netcat, or any other Unix utility which
commonly gets invoked from the command line, which processes untrusted input,
and which should sandbox itself.


> > * The attacker can't directly access the file system, but the attacker can
> >   escape the sandbox by controlling the surrounding (non-sandboxed) shell on its
> >   terminal through TIOCLINUX.
> > 
> > The ping example is not completely made up -- FreeBSD had such a vulnerability
> > in its ping utility in 2022[2].  The impact of the vulnerability was mitigated
> > by FreeBSD's Capsicum sandboxing.
> > 
> > The correct solution for the problem on Linux is to my knowledge to create a
> > pty/tty pair, but that is somewhat impractical for small utilities like ping, in
> > order to restrict themselves (they would need to create a sidecar process to
> > shovel the data back and forth).  Workarounds include setsid() and seccomp-bpf,
> > but they also have their limits and are not a clean solution.  We've previously
> > discussed it in [3].
> > 
> > I do believe that requiring CAP_SYS_ADMIN for TIOCLINUX's TIOCL_PASTESEL subcode
> > would be a better approach than so many sudo-style and sandboxing tools having
> > to learn this lesson the hard way.  Can we please reconsider this patch?
> 
> Have you verified that nothing will break with this?

Not yet, but I am happy to try it out.


> If so, it needs to be submitted in a form that could be accepted (this
> one was not, so I couldn't take it even if I wanted to), and please add
> a tested-by from you and we will be glad to reconsider it.

Thanks, will do.

In my understanding from the thread, the outstanding problems that were
discussed were:

 - compatibility with GPM -- I can try this out.
 - clarification of why this is needed -- I hope the additional
   discussion in this thread has clarified this.
 - submittable "form"

By the non-submittable "form", I assume you mean the formatting and maybe
phrasing of the e-mail, so that it can be cleanly applied to git?  Or was there
anything in the code which I missed?

Thanks,
—Günther

-- 
Sent using Mutt 🐕 Woof Woof

Re: [PATCH] Restrict access to TIOCLINUX

[Greg KH]

On Tue, Aug 22, 2023 at 08:22:04PM +0200, Günther Noack wrote:
> By the non-submittable "form", I assume you mean the formatting and maybe
> phrasing of the e-mail, so that it can be cleanly applied to git?  Or was there
> anything in the code which I missed?

I only looked at the format, that was incorrect.  You can test the patch
to verify if the code is correct before submitting it :)

thanks,

greg k-h