Bug 1029386

It seems correct, sulogin(1) scans /dev and calls fstatat() for all devices (to get console tty) and there is /dev/kcore symlink to /proc/kcore.

Assigning to selinux policy.


[Just for the record, this sulogin code seems stupid as we can use dirent->d_type from readdir() or/and device names from /proc/consoles, but this code improvement is probably better to do in upstream tree. The current code with /dev scan is good enough for the current RHEL7.]

commit a421f5f060eaf5c0d500dbda68ca4670c1a6c96c
Author: Miroslav Grepl <mgrepl>
Date:   Tue Nov 26 16:23:56 2013 +0100

    Allow sulogin to getattr on /proc/kcore

Created attachment 830130 [details]
screenshot from my VM booted with "single" parameter

(In reply to Karel Zak from comment #2)
> [Just for the record, this sulogin code seems stupid as we can use
> dirent->d_type from readdir() or/and device names from /proc/consoles, but
> this code improvement is probably better to do in upstream tree. The current
> code with /dev scan is good enough for the current RHEL7.]

thanks for the analysis; so, you have filed upstream bugreport, and you are about to file a bug for selinux-policy referencing that upstream bug with a note that once the code gets reworked this change in policy can get reverted, right? :-)

(In reply to Karel Volný from comment #7)
> (In reply to Karel Zak from comment #2)
> > [Just for the record, this sulogin code seems stupid as we can use
> > dirent->d_type from readdir() or/and device names from /proc/consoles, but
> > this code improvement is probably better to do in upstream tree. The current
> > code with /dev scan is good enough for the current RHEL7.]
> 
> thanks for the analysis; so, you have filed upstream bugreport

It's already improved in upstream code (commit commit 3deb67f50d513a1a74a62e8e248357e844e701a2), so it calls fstat() for character devices only if readdir() returns d_type (this depends on FS type and libc).

> and you are
> about to file a bug for selinux-policy referencing that upstream bug with a

well, it's not a bug if any code calls stat() to get device type, the problem is unnecessary selinux paranoia...

> note that once the code gets reworked this change in policy can get
> reverted, right? :-)

I think it will be better to keep this change in the selinux policy to make things more robust. It's really no security sensitive thing.

(In reply to Karel Zak from comment #8)
> (In reply to Karel Volný from comment #7)
> > (In reply to Karel Zak from comment #2)
> > > [Just for the record, this sulogin code seems stupid as we can use
> > > dirent->d_type from readdir() or/and device names from /proc/consoles, but
> > > this code improvement is probably better to do in upstream tree. The current
> > > code with /dev scan is good enough for the current RHEL7.]
> > 
> > thanks for the analysis; so, you have filed upstream bugreport
> 
> It's already improved in upstream code (commit commit
> 3deb67f50d513a1a74a62e8e248357e844e701a2), so it calls fstat() for character
> devices only if readdir() returns d_type (this depends on FS type and libc).

ok, cool!

> > and you are
> > about to file a bug for selinux-policy referencing that upstream bug with a
> 
> well, it's not a bug if any code calls stat() to get device type, the
> problem is unnecessary selinux paranoia...

I thought being paranoid is the point of selinux ... anyways, selinux guys were informed via this bug, so let's leave it upon them how do they feel about keeping this change after the code change gets downstream

Milos,
does it work with sys_admin capability?

# rpm -qa selinux-policy\*
selinux-policy-doc-3.12.1-108.el7.noarch
selinux-policy-targeted-3.12.1-108.el7.noarch
selinux-policy-devel-3.12.1-108.el7.noarch
selinux-policy-3.12.1-108.el7.noarch
selinux-policy-minimum-3.12.1-108.el7.noarch
selinux-policy-mls-3.12.1-108.el7.noarch
# sesearch -s sulogin_t -t sulogin_t -c capability -A -C
Found 1 semantic av rules:
   allow sulogin_t sulogin_t : capability { dac_override sys_tty_config } ; 

# 

I still see the sys_admin AVC (the second AVC mentioned in comment#3).

Going to test the scenario with custom policy module.

[    2.816177] type=1400 audit(1386341301.995:4): avc:  denied  { getattr } for  pid=396 comm="sulogin" path="/dev/initctl" dev="devtmpfs" ino=15467 scontext=system_u:system_r:sulogin_t:s0-s0:c0.c1023 tcontext=system_u:object_r:initctl_t:s0 tclass=fifo_file

This AVC appears when following rule is present and the VM is booted into single mode.

allow sulogin_t sulogin_t : capability { sys_admin };

Following policy module solved the problem on my VM, where selinux-policy-3.12.1-108.el7 is installed:

module mypolicy 1.0;

require {
        type sulogin_t;
        type initctl_t;
        class capability { sys_admin };
        class fifo_file { getattr };
}

#============= useradd_t ==============
allow sulogin_t sulogin_t : capability { sys_admin };
allow sulogin_t initctl_t : fifo_file { getattr };

This request was resolved in Red Hat Enterprise Linux 7.0.

Contact your manager or support representative in case you have further questions about the request.