On Thu, 23 May 2019 16:27:00 +0000 Dmitry Bogatov wrote:
>
>
> Also, every WITH_FOO flag doubles number of configurations your program
> have. Once you have dozen of flags, you no longer can test all of
> configurations.
Why not?
>
> I am surprised, that there is so much controversy on whether it is good
> to have some feature of program pluggable without re-compilation. The
> only real concern that was raised, as I see it, is how SELinux interacts
> with extra fork/exec.
The proposed patch doesn't seem to make the feature pluggable, it just
moves the feature to a second binary. There isn't anything here which
toggles the feature on/off, or lets the admin enable/disable it.
That being said, I'd be open to applying this upstream, with four
conditions or changes:
1. The selinux-check binary should probably be renamed so it is clear
this program is part of the SysV init suite. Maybe calling it something
like /sbin/sysvinit-selinux-check? I'm open to suggestions as to what it
should be called.
2. The patch should respect the compile-time define WITH_SELINUX in both
init.c and in the check-selinux.c code. Otherwise the build will either
fail or attempt to perform unnecessary fork/exec on systems without
SELinux, like Hurd and kFreeBSD. In fact, the Makefile should skip
building & installing check-selinux.c completely if SELinux is not
available on that platform. Otherwise we are making things more resource
hungry and complex on systems where SELinux is not included, rather than
less.
3. The new binary/helper program should have a man page.
4. There should be a test or script presented that demonstrates SELinux
still works properly with this approach. I don't want to open a security
issue by moving SELinux support to a non-PID1 binary.
