On Thu, Mar 11, 2021 at 07:29:03AM -0500, Neal Gompa wrote:
> On Thu, Mar 11, 2021 at 6:35 AM Daniel P. Berrangé <berrange@xxxxxxxxxx> wrote:
> >
> > On Wed, Mar 10, 2021 at 01:50:43PM -0500, Neal Gompa wrote:
> > > On Wed, Mar 10, 2021 at 7:43 AM Nikola Knazekova <nknazeko@xxxxxxxxxx> wrote:
> > > >
> > > > From: Vit Mojzis <vmojzis@xxxxxxxxxx>
> > > >
> > > > Compile the policy using a shell script executed by meson.
> > > >
> > > > Signed-off-by: Vit Mojzis <vmojzis@xxxxxxxxxx>
> > > > ---
> > > > libvirt.spec.in | 12 ------------
> > > > meson.build | 12 ++++++++++++
> > > > selinux/compile_policy.sh | 39 +++++++++++++++++++++++++++++++++++++++
> > > > selinux/meson.build | 23 +++++++++++++++++++++++
> > > > 4 files changed, 74 insertions(+), 12 deletions(-)
> > > > create mode 100755 selinux/compile_policy.sh
> > > > create mode 100644 selinux/meson.build
> > > >
> > > > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > > > index db08d91043..de664084fa 100644
> > > > --- a/libvirt.spec.in
> > > > +++ b/libvirt.spec.in
> > > > @@ -1240,14 +1240,6 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y' %{_specdir}/%{name}.spec)
> > > > %{?arg_login_shell}
> > > >
> > > > %meson_build
> > > > -%if 0%{?with_selinux}
> > > > -# SELinux policy (originally from selinux-policy-contrib)
> > > > -# this policy module will override the production module
> > > > -cd selinux
> > > > -
> > > > -make -f %{_datadir}/selinux/devel/Makefile %{modulename}.pp
> > > > -bzip2 -9 %{modulename}.pp
> > > > -%endif
> > > >
> > > > %install
> > > > rm -fr %{buildroot}
> > > > @@ -1332,10 +1324,6 @@ mv $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_qemu_probes.stp \
> > > > %endif
> > > > %endif
> > > >
> > > > -%if 0%{?with_selinux}
> > > > -install -D -m 0644 selinux/%{modulename}.pp.bz2 %{buildroot}%{_datadir}/selinux/packages/%{selinuxtype}/%{modulename}.pp.bz2
> > > > -%endif
> > > > -
> > > > %check
> > > > # Building on slow archs, like emulated s390x in Fedora copr, requires
> > > > # raising the test timeout
> > > > diff --git a/meson.build b/meson.build
> > > > index c81c6ab205..d060e441b5 100644
> > > > --- a/meson.build
> > > > +++ b/meson.build
> > > > @@ -2183,6 +2183,18 @@ endif
> > > >
> > > > subdir('build-aux')
> > > >
> > > > +os_release = run_command('grep', '^ID=', '/etc/os-release').stdout()
> > > > +os_version = run_command('grep', '^VERSION_ID=', '/etc/os-release').stdout().split('=')
> > > > +if (os_version.length() == 2)
> > > > + os_version = os_version[1]
> > > > +else
> > > > + os_version = 0
> > > > +endif
> > > > +
> > > > +if ((os_release.contains('fedora') and os_version.version_compare('>32')) or
> > > > + (os_release.contains('rhel') and os_version.version_compare('>7')))
> > > > + subdir('selinux')
> > > > +endif
> > > >
> > > > # install pkgconfig files
> > > > pkgconfig_files = [
> > > > diff --git a/selinux/compile_policy.sh b/selinux/compile_policy.sh
> > > > new file mode 100755
> > > > index 0000000000..02780e4aed
> > > > --- /dev/null
> > > > +++ b/selinux/compile_policy.sh
> > > > @@ -0,0 +1,39 @@
> > > > +#!/bin/sh
> > > > +set -x
> > > > +
> > > > +if [[ $# -ne 5 ]] ; then
> > > > + echo "Usage: compile_policy.sh <policy>.te <policy>.if <policy>.fc <output>.pp <tmpdir>"
> > > > + exit 1
> > > > +fi
> > > > +
> > > > +# checkmodule requires consistent file names
> > > > +MODULE_NAME=$(basename -- "$1")
> > > > +MODULE_NAME=${MODULE_NAME%.*}
> > > > +
> > > > +M4PARAM="-D enable_mcs -D distro_redhat -D hide_broken_symptoms -D mls_num_sens=16 -D mls_num_cats=1024 -D mcs_num_cats=1024"
> > > > +SHAREDIR="/usr/share/selinux"
> > > > +HEADERDIR="$SHAREDIR/devel/include"
> > > > +M4SUPPORT=$(echo $HEADERDIR/support/*.spt)
> > > > +HEADER_LAYERS=$(find "/usr/share/selinux/devel/include"/* -maxdepth 0 -type d | grep -v "/usr/share/selinux/devel/include/support")
> > > > +HEADER_INTERFACES=""
> > > > +for LAYER in $HEADER_LAYERS
> > > > +do
> > > > + HEADER_INTERFACES="$HEADER_INTERFACES $(echo $LAYER/*.if)"
> > > > +done
> > > > +
> > > > +# prepare temp folder
> > > > +mkdir -p $5
> > > > +# remove old trash from the temp folder
> > > > +rm -rf "$5/iferror.m4 $5/all_interfaces.conf $5/$MODULE_NAME.*"
> > > > +# tmp/all_interfaces.conf
> > > > +echo "ifdef(\`__if_error',\`m4exit(1)')" > $5/iferror.m4
> > > > +echo "divert(-1)" > $5/all_interfaces.conf
> > > > +m4 $M4SUPPORT $HEADER_INTERFACES $2 $5/iferror.m4 | sed -e s/dollarsstar/\$\$\*/g >> $5/all_interfaces.conf
> > > > +echo "divert" >> $5/all_interfaces.conf
> > > > +# tmp/%.mod
> > > > +m4 $M4PARAM -s $M4SUPPORT $5/all_interfaces.conf $1 > $5/$MODULE_NAME.tmp
> > > > +/usr/bin/checkmodule -M -m $5/$MODULE_NAME.tmp -o $5/$MODULE_NAME.mod
> > > > +# tmp/%.mod.fc
> > > > +m4 $M4PARAM $M4SUPPORT $3 > $5/$MODULE_NAME.mod.fc
> > > > +# %.pp
> > > > +/usr/bin/semodule_package -o $4 -m $5/$MODULE_NAME.mod -f $5/$MODULE_NAME.mod.fc
> >
> > Can you change this to use Python, since our strategy is to eliminate
> > use of all scripting languages other than Python 3:
> >
> > https://libvirt.org/strategy.html
> >
> >
> > > > diff --git a/selinux/meson.build b/selinux/meson.build
> > > > new file mode 100644
> > > > index 0000000000..1c76fd40aa
> > > > --- /dev/null
> > > > +++ b/selinux/meson.build
> > > > @@ -0,0 +1,23 @@
> > > > +selinux_sources = [
> > > > + 'virt.te',
> > > > + 'virt.if',
> > > > + 'virt.fc',
> > > > +]
> > > > +
> > > > +compile_policy_prog = find_program('compile_policy.sh')
> > > > +
> > > > +virt_pp = custom_target('virt.pp',
> > > > + output : 'virt.pp',
> > > > + input : selinux_sources,
> > > > + command : [compile_policy_prog, '@INPUT@', '@OUTPUT@', 'selinux/tmp'],
> > > > + install : false)
> > > > +
> > > > +bzip2_prog = find_program('bzip2')
> > > > +
> > > > +bzip = custom_target('virt.pp.bz2',
> > > > + output : 'virt.pp.bz2',
> > > > + input : virt_pp,
> > > > + command : [bzip2_prog, '-c', '-9', '@INPUT@'],
> > > > + capture : true,
> > > > + install : true,
> > > > + install_dir : 'share/selinux/packages/targeted')
> > > > --
> > > > 2.29.2
> > > >
> > >
> > > This smells like a bad idea, because we're not relying on the
> > > framework that SELinux policies are supposed to be built with. I don't
> > > think we should do this.
> >
> > The important part is the use of tools for compiling the policy. The way
> > you glue them into a build system is a app specific, and it makes no sense
> > to use SELinux provided Makefiles, when our build system is meson.
> >
>
> Let's say I buy that argument (I don't). Even with that argument, this
> patch is wrong because it makes assumptions about how SELinux policies
> are structured on-disk. For example, the install directory is wrong,
> since it should be share/selinux/packages, not
> share/selinux/packages/targeted. If I were to accept that I might be
> wrong about the directory in the previous statement, that means that
> we're still wrong, because we don't have builds for mls and minimal
> policy targets. Finally, we're missing the policy interface file.
Well if there are bugs like these, that's what this review is intended
to catch, and they'll need to be addressed before this can merge.
> This sounds like you need to work with Meson and selinux-policy
> upstream to add support for natively building policy modules with
> Meson itself.
Sure it would be nice if there was a meson extension that dealt
with SELinux, but we need to implement something that works with
the meson releases that exist today. If meson gains selinux
support in future may be we can consider it then.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
