[PATCH] selinux: Allow access to user_devpts

[PATCH] selinux: Allow access to user_devpts

[Derek Schrock]

Allow access to user_devpts.

	$ pasta --version
	pasta 0^20240510.g7288448-1.fc40.x86_64
	...
	$ awk '' < /dev/null
	$ pasta --version
	$

While this might be a awk bug it appears pasta should still have access
to devpts.
---
 contrib/selinux/pasta.te | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/selinux/pasta.te b/contrib/selinux/pasta.te
index 0ceda06..4e36c3f 100644
--- a/contrib/selinux/pasta.te
+++ b/contrib/selinux/pasta.te
@@ -211,3 +211,4 @@ allow pasta_t ifconfig_t:process { noatsecure rlimitinh siginh };
 allow pasta_t netutils_t:process { noatsecure rlimitinh siginh };
 allow pasta_t ping_t:process { noatsecure rlimitinh siginh };
 allow pasta_t user_tty_device_t:chr_file { append read write };
+allow pasta_t user_devpts_t:chr_file { append read write };
-- 
@@ -211,3 +211,4 @@ allow pasta_t ifconfig_t:process { noatsecure rlimitinh siginh };
 allow pasta_t netutils_t:process { noatsecure rlimitinh siginh };
 allow pasta_t ping_t:process { noatsecure rlimitinh siginh };
 allow pasta_t user_tty_device_t:chr_file { append read write };
+allow pasta_t user_devpts_t:chr_file { append read write };
-- 
2.45.0

Re: [PATCH] selinux: Allow access to user_devpts

[David Gibson]

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

On Sun, May 26, 2024 at 06:28:42PM -0400, Derek Schrock wrote:
> Allow access to user_devpts.
> 
> 	$ pasta --version
> 	pasta 0^20240510.g7288448-1.fc40.x86_64
> 	...
> 	$ awk '' < /dev/null
> 	$ pasta --version
> 	$
> 
> While this might be a awk bug it appears pasta should still have access
> to devpts.

It's not clear to me why pasta would need any access to /dev/pts.  The
shell that pasta spawns does, of course, but it should already live in
a difference security context.

> ---
>  contrib/selinux/pasta.te | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/contrib/selinux/pasta.te b/contrib/selinux/pasta.te
> index 0ceda06..4e36c3f 100644
> --- a/contrib/selinux/pasta.te
> +++ b/contrib/selinux/pasta.te
> @@ -211,3 +211,4 @@ allow pasta_t ifconfig_t:process { noatsecure rlimitinh siginh };
>  allow pasta_t netutils_t:process { noatsecure rlimitinh siginh };
>  allow pasta_t ping_t:process { noatsecure rlimitinh siginh };
>  allow pasta_t user_tty_device_t:chr_file { append read write };
> +allow pasta_t user_devpts_t:chr_file { append read write };

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] selinux: Allow access to user_devpts

[Stefano Brivio]

On Tue, 28 May 2024 16:55:55 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Sun, May 26, 2024 at 06:28:42PM -0400, Derek Schrock wrote:
> > Allow access to user_devpts.
> > 
> > 	$ pasta --version
> > 	pasta 0^20240510.g7288448-1.fc40.x86_64
> > 	...
> > 	$ awk '' < /dev/null
> > 	$ pasta --version
> > 	$
> > 
> > While this might be a awk bug it appears pasta should still have access
> > to devpts.  

Derek, thanks for the patch!

> It's not clear to me why pasta would need any access to /dev/pts.  The
> shell that pasta spawns does, of course, but it should already live in
> a difference security context.

Note that that doesn't happen in a shell pasta spawned: pasta --version
doesn't do that.

It's just that after that awk comamnd, enabling access to
user_tty_device_t doesn't seem to be enough anymore, we need
user_devpts_t then. Which is probably something reasonable to enable
anyway.

-- 
Stefano

Re: [PATCH] selinux: Allow access to user_devpts

[David Gibson]

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

On Tue, May 28, 2024 at 10:12:56AM +0200, Stefano Brivio wrote:
> On Tue, 28 May 2024 16:55:55 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Sun, May 26, 2024 at 06:28:42PM -0400, Derek Schrock wrote:
> > > Allow access to user_devpts.
> > > 
> > > 	$ pasta --version
> > > 	pasta 0^20240510.g7288448-1.fc40.x86_64
> > > 	...
> > > 	$ awk '' < /dev/null
> > > 	$ pasta --version
> > > 	$
> > > 
> > > While this might be a awk bug it appears pasta should still have access
> > > to devpts.  
> 
> Derek, thanks for the patch!
> 
> > It's not clear to me why pasta would need any access to /dev/pts.  The
> > shell that pasta spawns does, of course, but it should already live in
> > a difference security context.
> 
> Note that that doesn't happen in a shell pasta spawned: pasta --version
> doesn't do that.

Oh, good point.  I missed what was going on in that example.

> It's just that after that awk comamnd, enabling access to
> user_tty_device_t doesn't seem to be enough anymore, we need
> user_devpts_t then. Which is probably something reasonable to enable
> anyway.

Hmmm.. this still doesn't make sense to me.  AFAIK, /dev/pts is about
managing pseudo-ttys, I see no reason we'd need to do that.  Our
stdout *could* be a pseudo-tty, I suppose.  But surely selinux can't
be requiring us to explicitly allow for any possible stdout/stderr
target?  Especially not one as completely routine as a pseudo-tty -
that will be the case for anything run in an xterm.

I also can't fathom why running awk would change anything.  Could
there be something bogus in the selinux profile of the original shell
which allows the awk invocation to change the context somehow?

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] selinux: Allow access to user_devpts

[Derek Schrock]

On Tue, May 28, 2024 at 05:25:15AM EDT, David Gibson wrote:
> On Tue, May 28, 2024 at 10:12:56AM +0200, Stefano Brivio wrote:
> > On Tue, 28 May 2024 16:55:55 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > > On Sun, May 26, 2024 at 06:28:42PM -0400, Derek Schrock wrote:
> > > > Allow access to user_devpts.
> > > > 
> > > > 	$ pasta --version
> > > > 	pasta 0^20240510.g7288448-1.fc40.x86_64
> > > > 	...
> > > > 	$ awk '' < /dev/null
> > > > 	$ pasta --version
> > > > 	$
> > > > 
> > > > While this might be a awk bug it appears pasta should still have access
> > > > to devpts.  
> > 
> > Derek, thanks for the patch!
> > 
> > > It's not clear to me why pasta would need any access to /dev/pts.  The
> > > shell that pasta spawns does, of course, but it should already live in
> > > a difference security context.
> > 
> > Note that that doesn't happen in a shell pasta spawned: pasta --version
> > doesn't do that.
> 
> Oh, good point.  I missed what was going on in that example.
> 
> > It's just that after that awk comamnd, enabling access to
> > user_tty_device_t doesn't seem to be enough anymore, we need
> > user_devpts_t then. Which is probably something reasonable to enable
> > anyway.
> 
> Hmmm.. this still doesn't make sense to me.  AFAIK, /dev/pts is about
> managing pseudo-ttys, I see no reason we'd need to do that.  Our
> stdout *could* be a pseudo-tty, I suppose.  But surely selinux can't
> be requiring us to explicitly allow for any possible stdout/stderr
> target?  Especially not one as completely routine as a pseudo-tty -
> that will be the case for anything run in an xterm.
> 
> I also can't fathom why running awk would change anything.  Could
> there be something bogus in the selinux profile of the original shell
> which allows the awk invocation to change the context somehow?

Don't know if it means anything but stdout still works just not to the
interactive shell with pasta post awk:

	$ awk '' < /dev/null
	$ pasta --version | wc -l 
	7
	$

This is also reproducible in rocky9 (most likely RHEL9 too).  If that's
the case do you want me a ticket with Red Hat?
create a case with Red Hat

Re: [PATCH] selinux: Allow access to user_devpts

[Stefano Brivio]

On Tue, 28 May 2024 14:11:37 -0400
Derek Schrock <dereks@lifeofadishwasher.com> wrote:

> On Tue, May 28, 2024 at 05:25:15AM EDT, David Gibson wrote:
> > On Tue, May 28, 2024 at 10:12:56AM +0200, Stefano Brivio wrote:  
> > > On Tue, 28 May 2024 16:55:55 +1000
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > >   
> > > > On Sun, May 26, 2024 at 06:28:42PM -0400, Derek Schrock wrote:  
> > > > > Allow access to user_devpts.
> > > > > 
> > > > > 	$ pasta --version
> > > > > 	pasta 0^20240510.g7288448-1.fc40.x86_64
> > > > > 	...
> > > > > 	$ awk '' < /dev/null
> > > > > 	$ pasta --version
> > > > > 	$
> > > > > 
> > > > > While this might be a awk bug it appears pasta should still have access
> > > > > to devpts.    
> > > 
> > > Derek, thanks for the patch!
> > >   
> > > > It's not clear to me why pasta would need any access to /dev/pts.  The
> > > > shell that pasta spawns does, of course, but it should already live in
> > > > a difference security context.  
> > > 
> > > Note that that doesn't happen in a shell pasta spawned: pasta --version
> > > doesn't do that.  
> > 
> > Oh, good point.  I missed what was going on in that example.
> >   
> > > It's just that after that awk comamnd, enabling access to
> > > user_tty_device_t doesn't seem to be enough anymore, we need
> > > user_devpts_t then. Which is probably something reasonable to enable
> > > anyway.  
> > 
> > Hmmm.. this still doesn't make sense to me.  AFAIK, /dev/pts is about
> > managing pseudo-ttys, I see no reason we'd need to do that.  Our
> > stdout *could* be a pseudo-tty, I suppose.  But surely selinux can't
> > be requiring us to explicitly allow for any possible stdout/stderr
> > target?

...there might be something that subsumes both possibilities (or all of
them), I need to look into it.

> > Especially not one as completely routine as a pseudo-tty -
> > that will be the case for anything run in an xterm.
> > 
> > I also can't fathom why running awk would change anything.  Could
> > there be something bogus in the selinux profile of the original shell
> > which allows the awk invocation to change the context somehow?  
> 
> Don't know if it means anything but stdout still works just not to the
> interactive shell with pasta post awk:
> 
> 	$ awk '' < /dev/null
> 	$ pasta --version | wc -l 
> 	7
> 	$
> 
> This is also reproducible in rocky9 (most likely RHEL9 too).  If that's
> the case do you want me a ticket with Red Hat?
> create a case with Red Hat

I don't think we can exclude an issue with passt's upstream SELinux
policy, yet. I'm off this week, let me have a look next week and I'll
get back to you.

-- 
Stefano

Re: [PATCH] selinux: Allow access to user_devpts

[Stefano Brivio]

On Tue, 28 May 2024 19:25:15 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, May 28, 2024 at 10:12:56AM +0200, Stefano Brivio wrote:
> > On Tue, 28 May 2024 16:55:55 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > On Sun, May 26, 2024 at 06:28:42PM -0400, Derek Schrock wrote:  
> > > > Allow access to user_devpts.
> > > > 
> > > > 	$ pasta --version
> > > > 	pasta 0^20240510.g7288448-1.fc40.x86_64
> > > > 	...
> > > > 	$ awk '' < /dev/null
> > > > 	$ pasta --version
> > > > 	$
> > > > 
> > > > While this might be a awk bug it appears pasta should still have access
> > > > to devpts.    
> > 
> > Derek, thanks for the patch!
> >   
> > > It's not clear to me why pasta would need any access to /dev/pts.  The
> > > shell that pasta spawns does, of course, but it should already live in
> > > a difference security context.  
> > 
> > Note that that doesn't happen in a shell pasta spawned: pasta --version
> > doesn't do that.  
> 
> Oh, good point.  I missed what was going on in that example.
> 
> > It's just that after that awk comamnd, enabling access to
> > user_tty_device_t doesn't seem to be enough anymore, we need
> > user_devpts_t then. Which is probably something reasonable to enable
> > anyway.  
> 
> Hmmm.. this still doesn't make sense to me.  AFAIK, /dev/pts is about
> managing pseudo-ttys, I see no reason we'd need to do that.  Our
> stdout *could* be a pseudo-tty, I suppose.  But surely selinux can't
> be requiring us to explicitly allow for any possible stdout/stderr
> target?  Especially not one as completely routine as a pseudo-tty -
> that will be the case for anything run in an xterm.

In general, one should use the userdom_use_user_terminals interface
provided by system/userdomain.if (this is Fedora's core policy,
https://github.com/fedora-selinux/selinux-policy.git):

  interface(`userdom_use_user_terminals',`
      gen_require(`
          type user_tty_device_t, user_devpts_t;
      ')
  
      allow $1 user_tty_device_t:chr_file rw_term_perms;
      allow $1 user_devpts_t:chr_file rw_term_perms;
  ')

but I would like to avoid using it because we don't really need to
"use" terminals: pasta doesn't need to ioctl() or open() any, and
rw_term_perms is defined as follows:

  define(`rw_inherited_term_perms', `{ getattr lock read write append ioctl }')
  define(`rw_term_perms', `{ rw_inherited_term_perms open }')

That's why I went with the current:

  allow pasta_t user_tty_device_t:chr_file { append read write };

instead, and if you look at e.g. userdom_base_user_template template, also in
system/userdomain.if:

        allow $1_usertype user_devpts_t:chr_file { setattr rw_chr_file_perms };
        term_create_pty($1_usertype, user_devpts_t)
        # avoid annoying messages on terminal hangup on role change
        dontaudit $1_usertype user_devpts_t:chr_file ioctl;

        allow $1_usertype user_tty_device_t:chr_file { setattr rw_chr_file_perms };
        # avoid annoying messages on terminal hangup on role change
        dontaudit $1_usertype user_tty_device_t:chr_file ioctl;

rules seem to be generally repeated for user_devpts_t and
user_tty_device_t, that is, there seems to be no type subsuming both.

> I also can't fathom why running awk would change anything.  Could
> there be something bogus in the selinux profile of the original shell
> which allows the awk invocation to change the context somehow?

I can reproduce this on Fedora 40 and Fedora Rawhide, and it also
happens after running 'awk --version' with no redirection whatsoever.
But not, say, after 'ls', or 'ls --version'. I can't see any domain
change, either:

  $ ps -eZ -q $$
  LABEL                               PID TTY          TIME CMD
  unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 96009 pts/0 00:00:00 bash
  $ awk --version | wc -l
  15
  $ ps -eZ -q $$
  LABEL                               PID TTY          TIME CMD
  unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 96009 pts/0 00:00:00 bash
  $ pasta --version
  $

There's no difference between labels of ls and awk:

  $ ls -lZ /usr/bin/awk /usr/bin/gawk /usr/bin/ls
  lrwxrwxrwx. 1 root root system_u:object_r:bin_t:s0      4 Jan 24 00:00 /usr/bin/awk -> gawk
  -rwxr-xr-x. 1 root root system_u:object_r:bin_t:s0 764024 Jan 24 00:00 /usr/bin/gawk
  -rwxr-xr-x. 1 root root system_u:object_r:bin_t:s0 145480 Apr  2 00:00 /usr/bin/ls

and I don't see anything different being done on stdout:

  $ strace -e ioctl awk --version
  ioctl(1, TCGETS, {c_iflag=ICRNL|IXON|IUTF8, c_oflag=NL0|CR0|TAB0|BS0|VT0|FF0|OPOST|ONLCR, c_cflag=B38400|CS8|CREAD, c_lflag=ISIG|ICANON|ECHO|ECHOE|ECHOK|IEXTEN|ECHOCTL|ECHOKE, ...}) = 0
  GNU Awk 5.3.0, API 4.0, PMA Avon 8-g1, (GNU MPFR 4.2.1, GNU MP 6.3.0)
  [...]

  $ strace -e ioctl ls
  ioctl(1, TCGETS, {c_iflag=ICRNL|IXON|IUTF8, c_oflag=NL0|CR0|TAB0|BS0|VT0|FF0|OPOST|ONLCR, c_cflag=B38400|CS8|CREAD, c_lflag=ISIG|ICANON|ECHO|ECHOE|ECHOK|IEXTEN|ECHOCTL|ECHOKE, ...}) = 0
  ioctl(1, TIOCGWINSZ, {ws_row=56, ws_col=191, ws_xpixel=0, ws_ypixel=0}) = 0
  [...]

so, while this looks weird and it would be interesting to understand
what's so special with awk, the issue fixed by this patch is anyway a
quite annoying one, not strictly related to whatever is happening with
awk, and for the moment I would be inclined to apply the patch as it is,
unless somebody sees any harm in that.

-- 
Stefano

Re: [PATCH] selinux: Allow access to user_devpts

[Stefano Brivio]

On Sun, 26 May 2024 18:28:42 -0400
Derek Schrock <dereks@lifeofadishwasher.com> wrote:

> Allow access to user_devpts.
> 
> 	$ pasta --version
> 	pasta 0^20240510.g7288448-1.fc40.x86_64
> 	...
> 	$ awk '' < /dev/null
> 	$ pasta --version
> 	$
> 
> While this might be a awk bug it appears pasta should still have access
> to devpts.
> ---
>  contrib/selinux/pasta.te | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/contrib/selinux/pasta.te b/contrib/selinux/pasta.te
> index 0ceda06..4e36c3f 100644
> --- a/contrib/selinux/pasta.te
> +++ b/contrib/selinux/pasta.te
> @@ -211,3 +211,4 @@ allow pasta_t ifconfig_t:process { noatsecure rlimitinh siginh };
>  allow pasta_t netutils_t:process { noatsecure rlimitinh siginh };
>  allow pasta_t ping_t:process { noatsecure rlimitinh siginh };
>  allow pasta_t user_tty_device_t:chr_file { append read write };
> +allow pasta_t user_devpts_t:chr_file { append read write };

Applied, thanks.

-- 
Stefano