| Message ID | 20210301171922.321907-1-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [userspace] fixfiles: do not exclude /dev and /run in -C mode | expand |
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
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 <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.
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).
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
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
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' \
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(-)