Re: [Buildroot] [PATCH] package/policycoreutils: Add service to handle selinux autorelabel

[Arnout Vandecappelle <arnout@mind.be>]

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
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot