Message ID | 20210907125841.509792-1-jose.pekkarinen@unikie.com |
---|---|
State | Rejected |
Headers | show |
Series | package/policycoreutils: Add service to handle selinux autorelabel | expand |
Hi, Can I get some comments here? I'm starting to do some tests with 2021.08 and I find unlabeled stuff like the following ones: [ 10.534555] SELinux: Context Default is not valid (left unmapped). [ 10.562318] audit: type=1400 audit(1632913977.130:4): avc: denied { read } for pid=108 comm="auditd" name="audit" dev="vda" ino=16387 scontext=system_u:system_r:auditd_t tcontext=system_u:object_r:unlabeled_t tclass=dir permissive=1 trawcon="Default" [ 10.579085] audit: type=1400 audit(1632913977.146:5): avc: denied { open } for pid=108 comm="auditd" path="/var/log/audit" dev="vda" ino=16387 scontext=system_u:system_r:auditd_t tcontext=system_u:object_r:unlabeled_t tclass=dir permissive=1 trawcon="Def" [ 10.594226] audit: type=1400 audit(1632913977.146:6): avc: denied { getattr } for pid=108 comm="auditd" path="/var/log/audit" dev="vda" ino=16387 scontext=system_u:system_r:auditd_t tcontext=system_u:object_r:unlabeled_t tclass=dir permissive=1 trawcon="" [ 10.610371] audit: type=1400 audit(1632913977.146:7): avc: denied { search } for pid=108 comm="auditd" name="audit" dev="vda" ino=16387 scontext=system_u:system_r:auditd_t tcontext=system_u:object_r:unlabeled_t tclass=dir permissive=1 trawcon="Default" [ 10.629470] audit: type=1400 audit(1632913977.197:8): avc: denied { setattr } for pid=109 comm="auditd" name="audit" dev="vda" ino=16387 scontext=system_u:system_r:auditd_t tcontext=system_u:object_r:unlabeled_t tclass=dir permissive=1 trawcon="Default" [ 10.646993] audit: type=1400 audit(1632913977.214:9): avc: denied { write } for pid=109 comm="auditd" name="audit" dev="vda" ino=16387 scontext=system_u:system_r:auditd_t tcontext=system_u:object_r:unlabeled_t tclass=dir permissive=1 trawcon="Default" [ 10.662781] audit: type=1400 audit(1632913977.214:10): avc: denied { add_name } for pid=109 comm="auditd" name="audit.log" scontext=system_u:system_r:auditd_t tcontext=system_u:object_r:unlabeled_t tclass=dir permissive=1 trawcon="Default" [ 10.677266] audit: type=1400 audit(1632913977.214:11): avc: denied { create } for pid=109 comm="auditd" name="audit.log" scontext=system_u:system_r:auditd_t tcontext=system_u:object_r:unlabeled_t tclass=file permissive=1 These makes me think adding some via to handle the autorelabel mechanism is still needed. Best regards. José. On Tue, Sep 7, 2021 at 3:58 PM José Pekkarinen <jose.pekkarinen@unikie.com> wrote: > This patch adds a system service to check whether the > autorelabel via is requested or not, and produce the > labeling of the system under the loaded final kernel, > including automatically populated fs by the kernel. > > Signed-off-by: José Pekkarinen <jose.pekkarinen@unikie.com> > --- > .../policycoreutils/S00selinux-autorelabel | 49 +++++++++++++++++++ > package/policycoreutils/policycoreutils.mk | 5 ++ > 2 files changed, 54 insertions(+) > create mode 100644 package/policycoreutils/S00selinux-autorelabel > > diff --git a/package/policycoreutils/S00selinux-autorelabel > b/package/policycoreutils/S00selinux-autorelabel > new file mode 100644 > index 0000000000..7a47db891f > --- /dev/null > +++ b/package/policycoreutils/S00selinux-autorelabel > @@ -0,0 +1,49 @@ > +#!/bin/sh > + > +DAEMON="Autorelabel check" > + > +start() { > + printf 'Starting %s: ' "$DAEMON" > + > + if [ -f /.autorelabel ]; then > + echo "Relabeling" > + echo "*** Warning -- SELinux ${SELINUXTYPE} policy relabel is > required." > + echo "*** Relabeling could take a very long time, depending on > file" > + echo "*** system size and speed of hard drives." > + mount -a > + setfiles -m -r / > + > + # Remove label > + rm -f /.autorelabel || failed "Failed to remove the autorelabel > flag" > + > + # Reboot to activate relabeled file system > + echo "Automatic reboot in progress." > + reboot -f > + fi > + > + echo "OK" > + return 0 > +} > + > +stop() { > + printf 'Stopping %s: ' "$DAEMON" > + echo "OK" > + return 0 > +} > + > +restart() { > + stop > + sleep 1 > + start > +} > + > +case "$1" in > + start|stop|restart) > + "$1";; > + reload) > + # Restart, since there is no true "reload" feature. > + restart;; > + *) > + echo "Usage: $0 {start|stop|restart|reload}" > + exit 1 > +esac > diff --git a/package/policycoreutils/policycoreutils.mk > b/package/policycoreutils/policycoreutils.mk > index 5290c5b9f8..f698698059 100644 > --- a/package/policycoreutils/policycoreutils.mk > +++ b/package/policycoreutils/policycoreutils.mk > @@ -93,5 +93,10 @@ define HOST_POLICYCOREUTILS_INSTALL_CMDS > ) > endef > > +define POLICYCOREUTILS_INSTALL_INIT_SYSV > + $(INSTALL) -m 0755 -D > package/policycoreutils/S00selinux-autorelabel \ > + $(TARGET_DIR)/etc/init.d/S00selinux-autorelabel > +endef > + > $(eval $(generic-package)) > $(eval $(host-generic-package)) > -- > 2.25.1 > >
On 29/09/2021 13:37, José Pekkarinen wrote: > > Hi, > > Can I get some comments here? I'm starting The problem is that we are lacking people willing/able to review SELinux related stuff. I'm adding some people in Cc who have been doing SELinux stuff, but I don't think any of them really dare calling themselves experts. I'll make a few comments on the patch, but lacking any understanding at all about SELinux, it's going to be more questions than answers. > > to do some tests with 2021.08 and I find unlabeled > stuff like the following ones: > > [ 10.534555] SELinux: Context Default is not valid (left unmapped). > [ 10.562318] audit: type=1400 audit(1632913977.130:4): avc: denied { read } > for pid=108 comm="auditd" name="audit" dev="vda" ino=16387 > scontext=system_u:system_r:auditd_t tcontext=system_u:object_r:unlabeled_t > tclass=dir permissive=1 trawcon="Default" > [ 10.579085] audit: type=1400 audit(1632913977.146:5): avc: denied { open } for pid=108 comm="auditd" path="/var/log/audit" dev="vda" ino=16387 scontext=system_u:system_r:auditd_t tcontext=system_u:object_r:unlabeled_t tclass=dir permissive=1 trawcon="Def" /var/log/audit is on a tmpfs, and below you do a reboot after relabeling, so how can that fix the problem? [snip] > > These makes me think adding some via to > > handle the autorelabel mechanism is still needed. I don't know what a via is in this context... > > Best regards. > > José. > > > On Tue, Sep 7, 2021 at 3:58 PM José Pekkarinen <jose.pekkarinen@unikie.com > <mailto:jose.pekkarinen@unikie.com>> wrote: > > This patch adds a system service to check whether the When you say "system service", it sounds like a systemd service, while it really is an init script. > autorelabel via is requested or not, and produce the > labeling of the system under the loaded final kernel, > including automatically populated fs by the kernel. "automatically populated fs by the kernel", that would be proc, sys and dev, right? So why not handle just those? For anything that is part of the rootfs, relabeling sounds like the wrong thing to do - it should be done by Buildroot as part of the rootfs creation. Can't this even be a way of bypassing SELinux? Also, the "automatically populated fs by the kernel" seems to be inconsistent with the reboot below... > Signed-off-by: José Pekkarinen <jose.pekkarinen@unikie.com > <mailto:jose.pekkarinen@unikie.com>> > --- > .../policycoreutils/S00selinux-autorelabel | 49 +++++++++++++++++++ > package/policycoreutils/policycoreutils.mk <http://policycoreutils.mk> > | 5 ++ > 2 files changed, 54 insertions(+) > create mode 100644 package/policycoreutils/S00selinux-autorelabel > > diff --git a/package/policycoreutils/S00selinux-autorelabel > b/package/policycoreutils/S00selinux-autorelabel > new file mode 100644 > index 0000000000..7a47db891f > --- /dev/null > +++ b/package/policycoreutils/S00selinux-autorelabel > @@ -0,0 +1,49 @@ > +#!/bin/sh > + > +DAEMON="Autorelabel check" > + > +start() { > + printf 'Starting %s: ' "$DAEMON" > + > + if [ -f /.autorelabel ]; then Who is going to create this file? What with a readonly rootfs? At very least, it should be a variable that can be overridden in /etc/default/selinux-autorelabel. > + echo "Relabeling" > + echo "*** Warning -- SELinux ${SELINUXTYPE} policy relabel is > required." > + echo "*** Relabeling could take a very long time, depending on file" > + echo "*** system size and speed of hard drives." > + mount -a This is already done in inittab, before the init scripts are run, so it shouldn't be needed. > + setfiles -m -r / > + > + # Remove label > + rm -f /.autorelabel || failed "Failed to remove the autorelabel flag" There is no "failed" function - you really have to use echo and exit. > + > + # Reboot to activate relabeled file system > + echo "Automatic reboot in progress." > + reboot -f Why is reboot needed? This is before audit or any other daemon (other than /sbin/init) is started. The only thing that should be running at this point is /sbin/init, with a single child /etc/init.d/rcS, with a single child which is this script. Do any of them need to have the new labels? > + fi > + > + echo "OK" > + return 0 > +} > + > +stop() { > + printf 'Stopping %s: ' "$DAEMON" I would leave the stop completely empty, without message. (nitpick) Regards, Arnout > + echo "OK" > + return 0 > +} > + > +restart() { > + stop > + sleep 1 > + start > +} > + > +case "$1" in > + start|stop|restart) > + "$1";; > + reload) > + # Restart, since there is no true "reload" feature. > + restart;; > + *) > + echo "Usage: $0 {start|stop|restart|reload}" > + exit 1 > +esac > diff --git a/package/policycoreutils/policycoreutils.mk > <http://policycoreutils.mk> b/package/policycoreutils/policycoreutils.mk > <http://policycoreutils.mk> > index 5290c5b9f8..f698698059 100644 > --- a/package/policycoreutils/policycoreutils.mk <http://policycoreutils.mk> > +++ b/package/policycoreutils/policycoreutils.mk <http://policycoreutils.mk> > @@ -93,5 +93,10 @@ define HOST_POLICYCOREUTILS_INSTALL_CMDS > ) > endef > > +define POLICYCOREUTILS_INSTALL_INIT_SYSV > + $(INSTALL) -m 0755 -D package/policycoreutils/S00selinux-autorelabel \ > + $(TARGET_DIR)/etc/init.d/S00selinux-autorelabel > +endef > + > $(eval $(generic-package)) > $(eval $(host-generic-package)) > -- > 2.25.1 > > > > -- > > José. > > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot >
Hello José, Quoting José Pekkarinen (2021-09-07 14:58:41) > This patch adds a system service to check whether the > autorelabel via is requested or not, and produce the > labeling of the system under the loaded final kernel, > including automatically populated fs by the kernel. I don't think this is the correct fix nor it can work across reboots without relabelling the whole system each time. Relabelling the entire system is usually done in distros when updating the policy but here in Buildroot the policy is tied to a given image version and an update in the policy should come with a new image. Now, I'm not saying there is no issue. When using devtmpfs (otherwise /dev is already labeled at build time) device nodes have a default label that might not match the loaded policy. The labelling has to be done by an userspace daemon, usually udev (but restorecond could work with the right config). This is needed as new devices might appear later. The issue here I believe is there is a gap between devtmpfs is mounted and udev starts, while other daemons start accessing it. What is usually done IIRC is to run restorecon on /dev right after it is mounted. If I'm not mistaken systemd does something similar by default. This needs investigation but I hope the above gave some pointers. Thanks, Antoine
diff --git a/package/policycoreutils/S00selinux-autorelabel b/package/policycoreutils/S00selinux-autorelabel new file mode 100644 index 0000000000..7a47db891f --- /dev/null +++ b/package/policycoreutils/S00selinux-autorelabel @@ -0,0 +1,49 @@ +#!/bin/sh + +DAEMON="Autorelabel check" + +start() { + printf 'Starting %s: ' "$DAEMON" + + if [ -f /.autorelabel ]; then + echo "Relabeling" + echo "*** Warning -- SELinux ${SELINUXTYPE} policy relabel is required." + echo "*** Relabeling could take a very long time, depending on file" + echo "*** system size and speed of hard drives." + mount -a + setfiles -m -r / + + # Remove label + rm -f /.autorelabel || failed "Failed to remove the autorelabel flag" + + # Reboot to activate relabeled file system + echo "Automatic reboot in progress." + reboot -f + fi + + echo "OK" + return 0 +} + +stop() { + printf 'Stopping %s: ' "$DAEMON" + echo "OK" + return 0 +} + +restart() { + stop + sleep 1 + start +} + +case "$1" in + start|stop|restart) + "$1";; + reload) + # Restart, since there is no true "reload" feature. + restart;; + *) + echo "Usage: $0 {start|stop|restart|reload}" + exit 1 +esac diff --git a/package/policycoreutils/policycoreutils.mk b/package/policycoreutils/policycoreutils.mk index 5290c5b9f8..f698698059 100644 --- a/package/policycoreutils/policycoreutils.mk +++ b/package/policycoreutils/policycoreutils.mk @@ -93,5 +93,10 @@ define HOST_POLICYCOREUTILS_INSTALL_CMDS ) endef +define POLICYCOREUTILS_INSTALL_INIT_SYSV + $(INSTALL) -m 0755 -D package/policycoreutils/S00selinux-autorelabel \ + $(TARGET_DIR)/etc/init.d/S00selinux-autorelabel +endef + $(eval $(generic-package)) $(eval $(host-generic-package))
This patch adds a system service to check whether the autorelabel via is requested or not, and produce the labeling of the system under the loaded final kernel, including automatically populated fs by the kernel. Signed-off-by: José Pekkarinen <jose.pekkarinen@unikie.com> --- .../policycoreutils/S00selinux-autorelabel | 49 +++++++++++++++++++ package/policycoreutils/policycoreutils.mk | 5 ++ 2 files changed, 54 insertions(+) create mode 100644 package/policycoreutils/S00selinux-autorelabel