Re: [PATCH] tools: make flask utils build unconditional

[Wei Liu <wei.liu2@citrix.com>]

On Tue, Jan 05, 2016 at 03:36:21PM +0000, Ian Campbell wrote:
> On Tue, 2016-01-05 at 14:37 +0000, Ian Campbell wrote:
> > 
> > which on the basis of this discussion I wasn't expecting. I didn't see this
> > new file on i686 or ARM*.
> > 
> > My baseline is from the last time I committed, which would be last year, so
> > maybe something other than my current batch of patches has caused this.
> > 
> > I'm going to drop this one for now and (hopefully) get the rest of the
> > batch squared away. Afterwards I'll take another look (with a new baseline
> > filelist), but if someone can explain it in the meantime that would be
> > super.
> 
> So with a fresh basline I still see:
> 
> --- ../FILE_LIST.BASE.staging.x86_64    2016-01-05 14:50:32.000000000 +0000
> +++ ../FILE_LIST.staging.x86_64 2016-01-05 15:11:15.000000000 +0000
> @@ -6,6 +6,7 @@
>  dist/install/boot/xen-4.7-unstable.gz
>  dist/install/boot/xen-4.gz
>  dist/install/boot/xen.gz
> +dist/install/boot/xenpolicy-4.7-unstable
>  dist/install/etc
>  dist/install/etc/bash_completion.d
>  dist/install/etc/bash_completion.d/xl.sh
> @@ -386,6 +387,12 @@
>  dist/install/usr/local/lib/xen/libexec
>  dist/install/usr/local/lib/xen/libexec/qemu-bridge-helper
>  dist/install/usr/local/sbin
> +dist/install/usr/local/sbin/flask-get-bool
> +dist/install/usr/local/sbin/flask-getenforce
> +dist/install/usr/local/sbin/flask-label-pci
> +dist/install/usr/local/sbin/flask-loadpolicy
> +dist/install/usr/local/sbin/flask-set-bool
> +dist/install/usr/local/sbin/flask-setenforce
>  dist/install/usr/local/sbin/gdbsx
>  dist/install/usr/local/sbin/gtracestat
>  dist/install/usr/local/sbin/gtraceview
> *** FILES DIFFER ***
> 
> On i686 and ARM* I only see the (expected) second hunk.
> 
> I think the i686 case is explainable by the lack of a hypervisor build
> there, but I'm unsure why ARM* and x86_64 should differ in this regard.
> 
> config/Tools.mk is y only on x86_64, not on the others, which obviously
> explains things, but the question is why only on x86_64 (I presume this has
> always been the case and it was previously masked, but I've not checked).
> 
> Ah, OK, I misread
> 
> AX_ARG_DEFAULT_ENABLE([xsmpolicy], [Disable XSM policy compilation])
> 
> as being default disable, actually the default is "enabled iff checkpolicy
> is installed" and it happens to be that it is only installed in my x86_64
> build env.
> 
> So, in the end I think Wei was correct and this change will now, in some
> circumstances, end up installing a /boot/xenpolicy-*.
> 

I don't think it is related to this patch. I see an xenpoilcy file
without this patch applied. As you said it only depends on availability
of checkpolicy (part of generic SELinux utils, not the ones we build).

That said, let me try to answer the following question.

> So the question is do we mind that?
> 

We might or might not. See below.

I once submitted a patch to grub that look into /boot and generate XSM
entries if there is policy file. The patch is not yet merged though.

Since there is no way at the moment to tell if xen.gz has flask enabled,
my not yet upstreamed patch only matches the version number of xen.gz and
xenpolicy. Installing xenpolicy when xen.gz is not flaks-capable will
make grub generate an XSM entry nonetheless, which makes no sense.

Of course all the above is based on the theory that my grub patch is
going to be upstreamed.

Things have changed since I first submitted that patch. Doug's Kconfig
work is good. With .config installed in suitable location we can make
grub grep for flask information in config, hence avoiding generating
wrong entries.  I think this is better solution as we don't need to use
version number to match xen.gz and xenpolicy. If we go down this route
we don't mind having random xenpolicy lying around in /boot.

We just need to reach an agreement on how to proceed. I would vote for
the second solution.

Wei.


> Ian.