[PATCH userspace] fixfiles: do not exclude /dev and /run in -C mode

[PATCH userspace] fixfiles: do not exclude /dev and /run in -C mode

[Ondrej Mosnacek]

I can't think of a good reason why they should be excluded. On the
contrary, excluding them can cause trouble very easily if some labeling
rules for these directories change. For example, we changed the label
for /dev/nvme* from nvme_device_t to fixed_disk_device_t in Fedora
(updating the allow rules accordingly) and after policy update they
ended up with an invalid context, causing denials.

Thus, remove /dev and /run from the excludes. While there, also add
/root to the basic excludes to match the regex that excludes fc rules
(that should be effectively no functional change).

I did a sanity check on my system by running `restorecon -nv /dev /run`
and it didn't report any label differences.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 policycoreutils/scripts/fixfiles | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
index 30dadb4f..6fb12e04 100755
--- a/policycoreutils/scripts/fixfiles
+++ b/policycoreutils/scripts/fixfiles
@@ -162,7 +162,7 @@ newer() {
 #
 diff_filecontext() {
 EXCLUDEDIRS="`exclude_dirs_from_relabelling`"
-for i in /sys /proc /dev /run /mnt /var/tmp /var/lib/BackupPC /home /tmp /dev; do
+for i in /sys /proc /mnt /var/tmp /var/lib/BackupPC /home /root /tmp; do
     [ -e $i ]  && EXCLUDEDIRS="${EXCLUDEDIRS} -e $i";
 done
 LogExcluded
@@ -175,7 +175,7 @@ if [ -f ${PREFC} -a -x /usr/bin/diff ]; then
 	sed -r -e 's,:s0, ,g' $FC | sort -u | \
 	/usr/bin/diff -b ${PREFCTEMPFILE} - | \
 	    grep '^[<>]'|cut -c3-| grep ^/ | \
-	    egrep -v '(^/home|^/root|^/tmp|^/dev)' |\
+	    egrep -v '(^/home|^/root|^/tmp)' |\
 	sed -r -e 's,[[:blank:]].*,,g' \
 	       -e 's|\(([/[:alnum:]]+)\)\?|{\1,}|g' \
 	       -e 's|([/[:alnum:]])\?|{\1,}|g' \
-- 
2.29.2

Re: [PATCH userspace] fixfiles: do not exclude /dev and /run in -C mode

[Petr Lautrbach]

Ondrej Mosnacek <omosnace@redhat.com> writes:

> I can't think of a good reason why they should be excluded. On the
> contrary, excluding them can cause trouble very easily if some labeling
> rules for these directories change. For example, we changed the label
> for /dev/nvme* from nvme_device_t to fixed_disk_device_t in Fedora
> (updating the allow rules accordingly) and after policy update they
> ended up with an invalid context, causing denials.

I guess that /dev/ is there in order to avoid relabeling tty devices and block
the user from access:

[root@localhost ~]# ls -Z /dev/tty1
user_u:object_r:user_tty_device_t:s0 /dev/tty1

[root@localhost ~]# matchpathcon /dev/tty1
/dev/tty1       system_u:object_r:tty_device_t:s0


> Thus, remove /dev and /run from the excludes. While there, also add
> /root to the basic excludes to match the regex that excludes fc rules
> (that should be effectively no functional change).
>
> I did a sanity check on my system by running `restorecon -nv /dev /run`
> and it didn't report any label differences.
>
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>  policycoreutils/scripts/fixfiles | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
> index 30dadb4f..6fb12e04 100755
> --- a/policycoreutils/scripts/fixfiles
> +++ b/policycoreutils/scripts/fixfiles
> @@ -162,7 +162,7 @@ newer() {
>  #
>  diff_filecontext() {
>  EXCLUDEDIRS="`exclude_dirs_from_relabelling`"
> -for i in /sys /proc /dev /run /mnt /var/tmp /var/lib/BackupPC /home /tmp /dev; do
> +for i in /sys /proc /mnt /var/tmp /var/lib/BackupPC /home /root /tmp; do
>      [ -e $i ]  && EXCLUDEDIRS="${EXCLUDEDIRS} -e $i";
>  done
>  LogExcluded
> @@ -175,7 +175,7 @@ if [ -f ${PREFC} -a -x /usr/bin/diff ]; then
>  	sed -r -e 's,:s0, ,g' $FC | sort -u | \
>  	/usr/bin/diff -b ${PREFCTEMPFILE} - | \
>  	    grep '^[<>]'|cut -c3-| grep ^/ | \
> -	    egrep -v '(^/home|^/root|^/tmp|^/dev)' |\
> +	    egrep -v '(^/home|^/root|^/tmp)' |\
>  	sed -r -e 's,[[:blank:]].*,,g' \
>  	       -e 's|\(([/[:alnum:]]+)\)\?|{\1,}|g' \
>  	       -e 's|([/[:alnum:]])\?|{\1,}|g' \
> -- 
> 2.29.2

Re: [PATCH userspace] fixfiles: do not exclude /dev and /run in -C mode

[Ondrej Mosnacek]

On Thu, Mar 4, 2021 at 3:44 PM Petr Lautrbach <plautrba@redhat.com> wrote:
> Ondrej Mosnacek <omosnace@redhat.com> writes:
>
> > I can't think of a good reason why they should be excluded. On the
> > contrary, excluding them can cause trouble very easily if some labeling
> > rules for these directories change. For example, we changed the label
> > for /dev/nvme* from nvme_device_t to fixed_disk_device_t in Fedora
> > (updating the allow rules accordingly) and after policy update they
> > ended up with an invalid context, causing denials.
>
> I guess that /dev/ is there in order to avoid relabeling tty devices and block
> the user from access:
>
> [root@localhost ~]# ls -Z /dev/tty1
> user_u:object_r:user_tty_device_t:s0 /dev/tty1
>
> [root@localhost ~]# matchpathcon /dev/tty1
> /dev/tty1       system_u:object_r:tty_device_t:s0

$ sudo chcon user_u:object_r:user_tty_device_t:s0 /dev/tty21
$ ll -Z /dev/tty21
crw--w----. 1 root tty user_u:object_r:user_tty_device_t:s0 4, 21 feb
26 15:13 /dev/tty21
$ sudo restorecon -v /dev/tty21
/dev/tty21 not reset as customized by admin to
user_u:object_r:user_tty_device_t:s0
$ ll -Z /dev/tty21
crw--w----. 1 root tty user_u:object_r:user_tty_device_t:s0 4, 21 feb
26 15:13 /dev/tty21

$ cat /etc/selinux/targeted/contexts/customizable_types
container_file_t
sandbox_file_t
svirt_image_t
svirt_home_t
svirt_sandbox_file_t
virt_content_t
httpd_user_htaccess_t
httpd_user_script_exec_t
httpd_user_rw_content_t
httpd_user_ra_content_t
httpd_user_content_t
git_session_content_t
home_bin_t
user_tty_device_t

restorecon also doesn't change the user portion of the label if I only
change that to user_u (leaving the type as tty_device_t).

-- 
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.

Re: [PATCH userspace] fixfiles: do not exclude /dev and /run in -C mode

[Petr Lautrbach]

Ondrej Mosnacek <omosnace@redhat.com> writes:

> On Thu, Mar 4, 2021 at 3:44 PM Petr Lautrbach <plautrba@redhat.com> wrote:
>> Ondrej Mosnacek <omosnace@redhat.com> writes:
>>
>> > I can't think of a good reason why they should be excluded. On the
>> > contrary, excluding them can cause trouble very easily if some labeling
>> > rules for these directories change. For example, we changed the label
>> > for /dev/nvme* from nvme_device_t to fixed_disk_device_t in Fedora
>> > (updating the allow rules accordingly) and after policy update they
>> > ended up with an invalid context, causing denials.
>>
>> I guess that /dev/ is there in order to avoid relabeling tty devices and block
>> the user from access:
>>
>> [root@localhost ~]# ls -Z /dev/tty1
>> user_u:object_r:user_tty_device_t:s0 /dev/tty1
>>
>> [root@localhost ~]# matchpathcon /dev/tty1
>> /dev/tty1       system_u:object_r:tty_device_t:s0
>
> $ sudo chcon user_u:object_r:user_tty_device_t:s0 /dev/tty21
> $ ll -Z /dev/tty21
> crw--w----. 1 root tty user_u:object_r:user_tty_device_t:s0 4, 21 feb
> 26 15:13 /dev/tty21
> $ sudo restorecon -v /dev/tty21
> /dev/tty21 not reset as customized by admin to
> user_u:object_r:user_tty_device_t:s0
> $ ll -Z /dev/tty21
> crw--w----. 1 root tty user_u:object_r:user_tty_device_t:s0 4, 21 feb
> 26 15:13 /dev/tty21
>
> $ cat /etc/selinux/targeted/contexts/customizable_types
> container_file_t
> sandbox_file_t
> svirt_image_t
> svirt_home_t
> svirt_sandbox_file_t
> virt_content_t
> httpd_user_htaccess_t
> httpd_user_script_exec_t
> httpd_user_rw_content_t
> httpd_user_ra_content_t
> httpd_user_content_t
> git_session_content_t
> home_bin_t
> user_tty_device_t
>
> restorecon also doesn't change the user portion of the label if I only
> change that to user_u (leaving the type as tty_device_t).
>

Right.

But what is the reason why /dev/ was listed, and /run? And /dev was
there twice so it was probably important to skip /dev/.

Could be the reason that both /dev and /run are important for running
system? There are about 350 file contexts entries related to /dev and
more than 600 to /run. But the only customizable file is
user_tty_device_t.

I don't know. I'd like to avoid such change it there's no clear
confirmation it's not needed anymore.

Re: [PATCH userspace] fixfiles: do not exclude /dev and /run in -C mode

[Dominick Grift]

Ondrej Mosnacek <omosnace@redhat.com> writes:

> On Thu, Mar 4, 2021 at 3:44 PM Petr Lautrbach <plautrba@redhat.com> wrote:
>> Ondrej Mosnacek <omosnace@redhat.com> writes:
>>
>> > I can't think of a good reason why they should be excluded. On the
>> > contrary, excluding them can cause trouble very easily if some labeling
>> > rules for these directories change. For example, we changed the label
>> > for /dev/nvme* from nvme_device_t to fixed_disk_device_t in Fedora
>> > (updating the allow rules accordingly) and after policy update they
>> > ended up with an invalid context, causing denials.
>>
>> I guess that /dev/ is there in order to avoid relabeling tty devices and block
>> the user from access:
>>
>> [root@localhost ~]# ls -Z /dev/tty1
>> user_u:object_r:user_tty_device_t:s0 /dev/tty1
>>
>> [root@localhost ~]# matchpathcon /dev/tty1
>> /dev/tty1       system_u:object_r:tty_device_t:s0

user_tty_device_t is probably a customizable_type and so as long as you dont
`--force` it should not reset.

>
> $ sudo chcon user_u:object_r:user_tty_device_t:s0 /dev/tty21
> $ ll -Z /dev/tty21
> crw--w----. 1 root tty user_u:object_r:user_tty_device_t:s0 4, 21 feb
> 26 15:13 /dev/tty21
> $ sudo restorecon -v /dev/tty21
> /dev/tty21 not reset as customized by admin to
> user_u:object_r:user_tty_device_t:s0
> $ ll -Z /dev/tty21
> crw--w----. 1 root tty user_u:object_r:user_tty_device_t:s0 4, 21 feb
> 26 15:13 /dev/tty21
>
> $ cat /etc/selinux/targeted/contexts/customizable_types
> container_file_t
> sandbox_file_t
> svirt_image_t
> svirt_home_t
> svirt_sandbox_file_t
> virt_content_t
> httpd_user_htaccess_t
> httpd_user_script_exec_t
> httpd_user_rw_content_t
> httpd_user_ra_content_t
> httpd_user_content_t
> git_session_content_t
> home_bin_t
> user_tty_device_t
>
> restorecon also doesn't change the user portion of the label if I only
> change that to user_u (leaving the type as tty_device_t).

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift

Re: [PATCH userspace] fixfiles: do not exclude /dev and /run in -C mode

[Petr Lautrbach]

Ondrej Mosnacek <omosnace@redhat.com> writes:

> I can't think of a good reason why they should be excluded. On the
> contrary, excluding them can cause trouble very easily if some labeling
> rules for these directories change. For example, we changed the label
> for /dev/nvme* from nvme_device_t to fixed_disk_device_t in Fedora
> (updating the allow rules accordingly) and after policy update they
> ended up with an invalid context, causing denials.
>
> Thus, remove /dev and /run from the excludes. While there, also add
> /root to the basic excludes to match the regex that excludes fc rules
> (that should be effectively no functional change).
>
> I did a sanity check on my system by running `restorecon -nv /dev /run`
> and it didn't report any label differences.
>
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
>

Acked-by: Petr Lautrbach <plautrba@redhat.com>


> ---
>  policycoreutils/scripts/fixfiles | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
> index 30dadb4f..6fb12e04 100755
> --- a/policycoreutils/scripts/fixfiles
> +++ b/policycoreutils/scripts/fixfiles
> @@ -162,7 +162,7 @@ newer() {
>  #
>  diff_filecontext() {
>  EXCLUDEDIRS="`exclude_dirs_from_relabelling`"
> -for i in /sys /proc /dev /run /mnt /var/tmp /var/lib/BackupPC /home /tmp /dev; do
> +for i in /sys /proc /mnt /var/tmp /var/lib/BackupPC /home /root /tmp; do
>      [ -e $i ]  && EXCLUDEDIRS="${EXCLUDEDIRS} -e $i";
>  done
>  LogExcluded
> @@ -175,7 +175,7 @@ if [ -f ${PREFC} -a -x /usr/bin/diff ]; then
>  	sed -r -e 's,:s0, ,g' $FC | sort -u | \
>  	/usr/bin/diff -b ${PREFCTEMPFILE} - | \
>  	    grep '^[<>]'|cut -c3-| grep ^/ | \
> -	    egrep -v '(^/home|^/root|^/tmp|^/dev)' |\
> +	    egrep -v '(^/home|^/root|^/tmp)' |\
>  	sed -r -e 's,[[:blank:]].*,,g' \
>  	       -e 's|\(([/[:alnum:]]+)\)\?|{\1,}|g' \
>  	       -e 's|([/[:alnum:]])\?|{\1,}|g' \
> -- 
> 2.29.2

Re: [PATCH userspace] fixfiles: do not exclude /dev and /run in -C mode

[Petr Lautrbach]

Petr Lautrbach <plautrba@redhat.com> writes:

> Ondrej Mosnacek <omosnace@redhat.com> writes:
>
>> I can't think of a good reason why they should be excluded. On the
>> contrary, excluding them can cause trouble very easily if some labeling
>> rules for these directories change. For example, we changed the label
>> for /dev/nvme* from nvme_device_t to fixed_disk_device_t in Fedora
>> (updating the allow rules accordingly) and after policy update they
>> ended up with an invalid context, causing denials.
>>
>> Thus, remove /dev and /run from the excludes. While there, also add
>> /root to the basic excludes to match the regex that excludes fc rules
>> (that should be effectively no functional change).
>>
>> I did a sanity check on my system by running `restorecon -nv /dev /run`
>> and it didn't report any label differences.
>>
>> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
>>
>
> Acked-by: Petr Lautrbach <plautrba@redhat.com>
>

Merged


>
>> ---
>>  policycoreutils/scripts/fixfiles | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles
>> index 30dadb4f..6fb12e04 100755
>> --- a/policycoreutils/scripts/fixfiles
>> +++ b/policycoreutils/scripts/fixfiles
>> @@ -162,7 +162,7 @@ newer() {
>>  #
>>  diff_filecontext() {
>>  EXCLUDEDIRS="`exclude_dirs_from_relabelling`"
>> -for i in /sys /proc /dev /run /mnt /var/tmp /var/lib/BackupPC /home /tmp /dev; do
>> +for i in /sys /proc /mnt /var/tmp /var/lib/BackupPC /home /root /tmp; do
>>      [ -e $i ]  && EXCLUDEDIRS="${EXCLUDEDIRS} -e $i";
>>  done
>>  LogExcluded
>> @@ -175,7 +175,7 @@ if [ -f ${PREFC} -a -x /usr/bin/diff ]; then
>>  	sed -r -e 's,:s0, ,g' $FC | sort -u | \
>>  	/usr/bin/diff -b ${PREFCTEMPFILE} - | \
>>  	    grep '^[<>]'|cut -c3-| grep ^/ | \
>> -	    egrep -v '(^/home|^/root|^/tmp|^/dev)' |\
>> +	    egrep -v '(^/home|^/root|^/tmp)' |\
>>  	sed -r -e 's,[[:blank:]].*,,g' \
>>  	       -e 's|\(([/[:alnum:]]+)\)\?|{\1,}|g' \
>>  	       -e 's|([/[:alnum:]])\?|{\1,}|g' \
>> -- 
>> 2.29.2