Re: [PATCH] selinux: Allow access to user_devpts

[Stefano Brivio <sbrivio@redhat.com>]

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