[PATCH] tools: make FLASK utils build unconditional

[PATCH] tools: make FLASK utils build unconditional

[Doug Goldstein]

The flask utilities only have dependencies on libxc so there's no
downside to always building it. Distros and projects based on Xen can
put these in a different package to not install them for all users.
Prior to this change FLASK_ENABLE needed to be set at the top level to
build the utilities and the tools/configure script would build the FLASK
policy by default, but only if the utilities were built.

This change makes item 3 from
http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg01796.html
a happen by default.

CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---
 tools/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/Makefile b/tools/Makefile
index 9f74ac7..3f9289b 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -5,7 +5,7 @@ SUBDIRS-y :=
 SUBDIRS-y += include
 SUBDIRS-y += libs
 SUBDIRS-y += libxc
-SUBDIRS-$(FLASK_ENABLE) += flask
+SUBDIRS-y += flask
 SUBDIRS-y += xenstore
 SUBDIRS-y += misc
 SUBDIRS-y += examples
-- 
2.4.10

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

[Andrew Cooper]

On 15/01/16 17:39, Doug Goldstein wrote:
> The flask utilities only have dependencies on libxc so there's no
> downside to always building it. Distros and projects based on Xen can
> put these in a different package to not install them for all users.
> Prior to this change FLASK_ENABLE needed to be set at the top level to
> build the utilities and the tools/configure script would build the FLASK
> policy by default, but only if the utilities were built.
>
> This change makes item 3 from
> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg01796.html
> a happen by default.
>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
>  tools/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/Makefile b/tools/Makefile
> index 9f74ac7..3f9289b 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -5,7 +5,7 @@ SUBDIRS-y :=
>  SUBDIRS-y += include
>  SUBDIRS-y += libs
>  SUBDIRS-y += libxc
> -SUBDIRS-$(FLASK_ENABLE) += flask
> +SUBDIRS-y += flask
>  SUBDIRS-y += xenstore
>  SUBDIRS-y += misc
>  SUBDIRS-y += examples

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

[Ian Campbell]

On Fri, 2016-01-15 at 19:48 +0000, Andrew Cooper wrote:
> On 15/01/16 17:39, Doug Goldstein wrote:
> > The flask utilities only have dependencies on libxc so there's no
> > downside to always building it. Distros and projects based on Xen can
> > put these in a different package to not install them for all users.
> > Prior to this change FLASK_ENABLE needed to be set at the top level to
> > build the utilities and the tools/configure script would build the
> > FLASK
> > policy by default, but only if the utilities were built.
> > 
> > This change makes item 3 from
> > http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg01796.ht
> > ml
> > a happen by default.
> > 
> > CC: Ian Jackson <ian.jackson@eu.citrix.com>
> > CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > CC: Ian Campbell <ian.campbell@citrix.com>
> > CC: Wei Liu <wei.liu2@citrix.com>
> > Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

As noted elsewhere I have acked + applied htis.

[PATCH] tools: make flask utils build unconditional

[Doug Goldstein]

The flask utilities only have dependencies on libxc so there's no
downside to always building it. Distros and projects based on Xen can
put these utilities into a different package and not install them for
everyone. Prior to this change FLASK_ENABLE needs to be a top level
variable however after this change FLASK_ENABLE only affects xen/.

Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---
 tools/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/Makefile b/tools/Makefile
index 820ca40..2f773fd 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -4,7 +4,7 @@ include $(XEN_ROOT)/tools/Rules.mk
 SUBDIRS-y :=
 SUBDIRS-y += include
 SUBDIRS-y += libxc
-SUBDIRS-$(FLASK_ENABLE) += flask
+SUBDIRS-y += flask
 SUBDIRS-y += xenstore
 SUBDIRS-y += misc
 SUBDIRS-y += examples
-- 
2.4.10

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

[Andrew Cooper]

On 22/12/15 04:46, Doug Goldstein wrote:
> The flask utilities only have dependencies on libxc so there's no
> downside to always building it. Distros and projects based on Xen can
> put these utilities into a different package and not install them for
> everyone. Prior to this change FLASK_ENABLE needs to be a top level
> variable however after this change FLASK_ENABLE only affects xen/.
>
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>

CC'ing Daniel as this is a flask related change.

FWIW, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
>  tools/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/Makefile b/tools/Makefile
> index 820ca40..2f773fd 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -4,7 +4,7 @@ include $(XEN_ROOT)/tools/Rules.mk
>  SUBDIRS-y :=
>  SUBDIRS-y += include
>  SUBDIRS-y += libxc
> -SUBDIRS-$(FLASK_ENABLE) += flask
> +SUBDIRS-y += flask
>  SUBDIRS-y += xenstore
>  SUBDIRS-y += misc
>  SUBDIRS-y += examples

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

[Wei Liu]

On Mon, Dec 21, 2015 at 10:46:43PM -0600, Doug Goldstein wrote:
> The flask utilities only have dependencies on libxc so there's no
> downside to always building it. Distros and projects based on Xen can
> put these utilities into a different package and not install them for
> everyone. Prior to this change FLASK_ENABLE needs to be a top level
> variable however after this change FLASK_ENABLE only affects xen/.
> 
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>

While it is true about the dependency, enabling flask build
unconditionally in toolstack will leave a xenpolicy-$VERSION installed
in /boot. Not sure how that will be useful to Xen without flask support.

It is true that packager can pick that into separate package, but for
people installing from source, this might be a bit confusing.

Wei.

> ---
>  tools/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 820ca40..2f773fd 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -4,7 +4,7 @@ include $(XEN_ROOT)/tools/Rules.mk
>  SUBDIRS-y :=
>  SUBDIRS-y += include
>  SUBDIRS-y += libxc
> -SUBDIRS-$(FLASK_ENABLE) += flask
> +SUBDIRS-y += flask
>  SUBDIRS-y += xenstore
>  SUBDIRS-y += misc
>  SUBDIRS-y += examples
> -- 
> 2.4.10
> 

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

[Doug Goldstein]

[-- Attachment #1.1: Type: text/plain, Size: 1674 bytes --]

On 1/4/16 6:28 AM, Wei Liu wrote:
> On Mon, Dec 21, 2015 at 10:46:43PM -0600, Doug Goldstein wrote:
>> The flask utilities only have dependencies on libxc so there's no
>> downside to always building it. Distros and projects based on Xen can
>> put these utilities into a different package and not install them for
>> everyone. Prior to this change FLASK_ENABLE needs to be a top level
>> variable however after this change FLASK_ENABLE only affects xen/.
>>
>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> 
> While it is true about the dependency, enabling flask build
> unconditionally in toolstack will leave a xenpolicy-$VERSION installed
> in /boot. Not sure how that will be useful to Xen without flask support.
> 
> It is true that packager can pick that into separate package, but for
> people installing from source, this might be a bit confusing.
> 
> Wei.

Wei,

This is actually not correct. The installation of that file is
controlled by the ./configure script with --enable-xsmpolicy and
--disable-xsmpolicy.

This patch just makes the utilities be unconditionally built.

--
Doug


> 
>> ---
>>  tools/Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/Makefile b/tools/Makefile
>> index 820ca40..2f773fd 100644
>> --- a/tools/Makefile
>> +++ b/tools/Makefile
>> @@ -4,7 +4,7 @@ include $(XEN_ROOT)/tools/Rules.mk
>>  SUBDIRS-y :=
>>  SUBDIRS-y += include
>>  SUBDIRS-y += libxc
>> -SUBDIRS-$(FLASK_ENABLE) += flask
>> +SUBDIRS-y += flask
>>  SUBDIRS-y += xenstore
>>  SUBDIRS-y += misc
>>  SUBDIRS-y += examples
>> -- 
>> 2.4.10
>>


-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Wei Liu]

On Mon, Jan 04, 2016 at 08:14:23AM -0600, Doug Goldstein wrote:
> On 1/4/16 6:28 AM, Wei Liu wrote:
> > On Mon, Dec 21, 2015 at 10:46:43PM -0600, Doug Goldstein wrote:
> >> The flask utilities only have dependencies on libxc so there's no
> >> downside to always building it. Distros and projects based on Xen can
> >> put these utilities into a different package and not install them for
> >> everyone. Prior to this change FLASK_ENABLE needs to be a top level
> >> variable however after this change FLASK_ENABLE only affects xen/.
> >>
> >> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> > 
> > While it is true about the dependency, enabling flask build
> > unconditionally in toolstack will leave a xenpolicy-$VERSION installed
> > in /boot. Not sure how that will be useful to Xen without flask support.
> > 
> > It is true that packager can pick that into separate package, but for
> > people installing from source, this might be a bit confusing.
> > 
> > Wei.
> 
> Wei,
> 
> This is actually not correct. The installation of that file is
> controlled by the ./configure script with --enable-xsmpolicy and
> --disable-xsmpolicy.
> 
> This patch just makes the utilities be unconditionally built.
> 

You're right. I misread. Sorry for the noise.

Acked-by: Wei Liu <wei.liu2@citrix.com>

Wei.

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

[Ian Campbell]

On Mon, 2016-01-04 at 14:26 +0000, Wei Liu wrote:
> On Mon, Jan 04, 2016 at 08:14:23AM -0600, Doug Goldstein wrote:
> > On 1/4/16 6:28 AM, Wei Liu wrote:
> > > On Mon, Dec 21, 2015 at 10:46:43PM -0600, Doug Goldstein wrote:
> > > > The flask utilities only have dependencies on libxc so there's no
> > > > downside to always building it. Distros and projects based on Xen
> > > > can
> > > > put these utilities into a different package and not install them
> > > > for
> > > > everyone. Prior to this change FLASK_ENABLE needs to be a top level
> > > > variable however after this change FLASK_ENABLE only affects xen/.
> > > > 
> > > > Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> > > 
> > > While it is true about the dependency, enabling flask build
> > > unconditionally in toolstack will leave a xenpolicy-$VERSION
> > > installed
> > > in /boot. Not sure how that will be useful to Xen without flask
> > > support.
> > > 
> > > It is true that packager can pick that into separate package, but for
> > > people installing from source, this might be a bit confusing.
> > > 
> > > Wei.
> > 
> > Wei,
> > 
> > This is actually not correct. The installation of that file is
> > controlled by the ./configure script with --enable-xsmpolicy and
> > --disable-xsmpolicy.
> > 
> > This patch just makes the utilities be unconditionally built.
> > 
> 
> You're right. I misread. Sorry for the noise.
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>

I went to apply this but I seem to now have an extra policy file installed
after my test build:
--- ../FILE_LIST.BASE.staging.x86_64    2015-12-16 09:45:00.000000000 +0000
+++ ../FILE_LIST.staging.x86_64 2016-01-05 14:27:58.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

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.

My pre-commit script doesn't mention xsm at all, i.e. nothing passed to
./configure nor written to .config or xen/.config.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Ian Campbell]

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-*.

So the question is do we mind that?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Wei Liu]

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.

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

[Ian Campbell]

On Tue, 2016-01-05 at 16:13 +0000, Wei Liu wrote:
> 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.

With XSM disabled?

>  As you said it only depends on availability
> of checkpolicy (part of generic SELinux utils, not the ones we build).

But then why does this file only show up for me with this patch applied?

You initially objected to this patch because you thought it would add this
file, but it seems like you have always had it. Is the answer just that you
only just found that you always had it?

> 
> 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.

Indeed.

> 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.

Which is what? This patch as is? (and what is the first proposition?)

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Wei Liu]

On Tue, Jan 05, 2016 at 04:24:19PM +0000, Ian Campbell wrote:
> On Tue, 2016-01-05 at 16:13 +0000, Wei Liu wrote:
> > 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.
> 
> With XSM disabled?
> 
> >  As you said it only depends on availability
> > of checkpolicy (part of generic SELinux utils, not the ones we build).
> 
> But then why does this file only show up for me with this patch applied?
> 
> You initially objected to this patch because you thought it would add this
> file, but it seems like you have always had it. Is the answer just that you
> only just found that you always had it?
> 

Hmm... After I make distclean, things changed.

So to be clear: without this patch applied, I don't have xenpolicy file
even if checkpolicy is available. This patch does alter the behaviour
somehow.

I'm in the middle of rebasing one patch series,  so I haven't looked
into all the details.

> > 
> > 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.
> 
> Indeed.
> 
> > 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.
> 
> Which is what? This patch as is? (and what is the first proposition?)
> 

That was referring to grub generating XSM entries. First solution is my
not yet upstream patch; second is to make gurb grep .config for flask
information.

Wei.

> Ian.

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

[Doug Goldstein]

[-- Attachment #1.1: Type: text/plain, Size: 6053 bytes --]

On 1/5/16 10:42 AM, Wei Liu wrote:
> On Tue, Jan 05, 2016 at 04:24:19PM +0000, Ian Campbell wrote:
>> On Tue, 2016-01-05 at 16:13 +0000, Wei Liu wrote:
>>> 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.
>>
>> With XSM disabled?
>>
>>>  As you said it only depends on availability
>>> of checkpolicy (part of generic SELinux utils, not the ones we build).
>>
>> But then why does this file only show up for me with this patch applied?
>>
>> You initially objected to this patch because you thought it would add this
>> file, but it seems like you have always had it. Is the answer just that you
>> only just found that you always had it?
>>
> 
> Hmm... After I make distclean, things changed.
> 
> So to be clear: without this patch applied, I don't have xenpolicy file
> even if checkpolicy is available. This patch does alter the behaviour
> somehow.
> 
> I'm in the middle of rebasing one patch series,  so I haven't looked
> into all the details.
> 
>>>
>>> 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.
>>
>> Indeed.
>>
>>> 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.
>>
>> Which is what? This patch as is? (and what is the first proposition?)
>>
> 
> That was referring to grub generating XSM entries. First solution is my
> not yet upstream patch; second is to make gurb grep .config for flask
> information.
> 
> Wei.
> 
>> Ian.

Ok so I'm at a loss what steps I need to take. I've submitted patches to
put the config in /boot so that this check can be made but there's a
disagreement if that's even necessary or not.

Do I need to supply a patch to make --disable-xsmpolicy the default so
that this change doesn't generate the policy by default? The point of
this patch is to compile the necessarily bits always which will help
shake out bugs earlier. If we don't want the policy file to be installed
then we should use the proper setting for that and not the fact that the
utility isn't being compiled.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Wei Liu]

On Fri, Jan 08, 2016 at 12:49:07PM -0600, Doug Goldstein wrote:
[...]
> Ok so I'm at a loss what steps I need to take. I've submitted patches to
> put the config in /boot so that this check can be made but there's a
> disagreement if that's even necessary or not.
> 

That's a bit unfortunate. :-(

But if I'm not mistaken that's orthogonal to this problem, right? That's
one more step down the road regarding grub integration.

> Do I need to supply a patch to make --disable-xsmpolicy the default so
> that this change doesn't generate the policy by default? The point of
> this patch is to compile the necessarily bits always which will help
> shake out bugs earlier. If we don't want the policy file to be installed
> then we should use the proper setting for that and not the fact that the
> utility isn't being compiled.
> 

I think one solution would be to modify flask/Makefile to guard policy
compilation against (FLASK_ENABLE && FLASK_POLICY).

What do you think? Admittedly I haven't followed closely all the KConfig
work so I might be talking nonsense.

Ian and Ian?

Wei.

> -- 
> Doug Goldstein
> 

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

[Doug Goldstein]

[-- Attachment #1.1: Type: text/plain, Size: 1806 bytes --]

On 1/11/16 9:19 AM, Wei Liu wrote:
> On Fri, Jan 08, 2016 at 12:49:07PM -0600, Doug Goldstein wrote:
> [...]
>> Ok so I'm at a loss what steps I need to take. I've submitted patches to
>> put the config in /boot so that this check can be made but there's a
>> disagreement if that's even necessary or not.
>>
> 
> That's a bit unfortunate. :-(
> 
> But if I'm not mistaken that's orthogonal to this problem, right? That's
> one more step down the road regarding grub integration.
> 
>> Do I need to supply a patch to make --disable-xsmpolicy the default so
>> that this change doesn't generate the policy by default? The point of
>> this patch is to compile the necessarily bits always which will help
>> shake out bugs earlier. If we don't want the policy file to be installed
>> then we should use the proper setting for that and not the fact that the
>> utility isn't being compiled.
>>
> 
> I think one solution would be to modify flask/Makefile to guard policy
> compilation against (FLASK_ENABLE && FLASK_POLICY).
> 
> What do you think? Admittedly I haven't followed closely all the KConfig
> work so I might be talking nonsense.
> 
> Ian and Ian?
> 
> Wei.

Wei (and Ian and Ian and Daniel),

There's already a guard against compiling the policy in the tools/
directory's configure script called --{enable,disable}-xsmpolicy What I
could do is disable it by default because it is currently enabled by
default.

I honestly think that would be an improvement because we would compile
all the source code (causing us to shake bugs out earlier) but only
generate the policy when the user explicitly requests it. Right now the
policy is made whenever the utilities are compiled.

Let me know if that sounds appealing to you.

Thanks.
-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Wei Liu]

On Mon, Jan 11, 2016 at 11:10:35AM -0600, Doug Goldstein wrote:
> On 1/11/16 9:19 AM, Wei Liu wrote:
> > On Fri, Jan 08, 2016 at 12:49:07PM -0600, Doug Goldstein wrote:
> > [...]
> >> Ok so I'm at a loss what steps I need to take. I've submitted patches to
> >> put the config in /boot so that this check can be made but there's a
> >> disagreement if that's even necessary or not.
> >>
> > 
> > That's a bit unfortunate. :-(
> > 
> > But if I'm not mistaken that's orthogonal to this problem, right? That's
> > one more step down the road regarding grub integration.
> > 
> >> Do I need to supply a patch to make --disable-xsmpolicy the default so
> >> that this change doesn't generate the policy by default? The point of
> >> this patch is to compile the necessarily bits always which will help
> >> shake out bugs earlier. If we don't want the policy file to be installed
> >> then we should use the proper setting for that and not the fact that the
> >> utility isn't being compiled.
> >>
> > 
> > I think one solution would be to modify flask/Makefile to guard policy
> > compilation against (FLASK_ENABLE && FLASK_POLICY).
> > 
> > What do you think? Admittedly I haven't followed closely all the KConfig
> > work so I might be talking nonsense.
> > 
> > Ian and Ian?
> > 
> > Wei.
> 
> Wei (and Ian and Ian and Daniel),
> 
> There's already a guard against compiling the policy in the tools/
> directory's configure script called --{enable,disable}-xsmpolicy What I
> could do is disable it by default because it is currently enabled by
> default.
> 
> I honestly think that would be an improvement because we would compile
> all the source code (causing us to shake bugs out earlier) but only
> generate the policy when the user explicitly requests it. Right now the
> policy is made whenever the utilities are compiled.
> 
> Let me know if that sounds appealing to you.
> 

Fine by me.

I don't really have a strong opinion at this point. My original concern
that the installed xenpolicy file interferes with grub was based on the
assumption that we only had version numbers as indicator to match
hypervisor binary and xenpolicy file. But now since I think there is
better way to generate grub entry I don't think my objection based on
the (bad) assumption to this patch is relevant anymore.

Wei.

> Thanks.
> -- 
> Doug Goldstein
> 

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

[Doug Goldstein]

[-- Attachment #1.1: Type: text/plain, Size: 4554 bytes --]

On 1/5/16 10:13 AM, Wei Liu wrote:
> 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.

So I submitted a patch to put the .config in /boot to have the ability
to do this. I figured it needed to be in /boot because that's how the
existing 20_linux_xen but there's some disagreement on the location. If
we can resolve that I'll happily update the 20_linux_xen patch as well.

http://lists.xenproject.org/archives/html/xen-devel/2015-12/msg02369.html


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


-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Ian Campbell]

On Tue, 2016-01-05 at 10:34 -0600, Doug Goldstein wrote:
> On 1/5/16 10:13 AM, Wei Liu wrote:
> > 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.
> 
> So I submitted a patch to put the .config in /boot to have the ability
> to do this. I figured it needed to be in /boot because that's how the
> existing 20_linux_xen but there's some disagreement on the location. If
> we can resolve that I'll happily update the 20_linux_xen patch as well.
> 
> http://lists.xenproject.org/archives/html/xen-devel/2015-12/msg02369.html

It would be worth referring that thread back to this one too.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel