xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* XSM permissive by default.
@ 2016-03-09  1:51 Konrad Rzeszutek Wilk
  2016-03-09  2:11 ` Doug Goldstein
  2016-03-09 13:24 ` Andrew Cooper
  0 siblings, 2 replies; 20+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-03-09  1:51 UTC (permalink / raw)
  To: dgdegra, xen-devel, andrew.cooper3

Hey,

I was wondering if it we should change the default flask_bootparam
option from permissive to disabled?

The reason being is that I was startled to see that my xSplice
code was able to patch the hypervisor from within an PV guest!

Further testing showed that I could do 'xl debug-keys R' from
within the guests. This being possible with released 4.6 if I have
XSM enabled.

All of this is due to the fact that I had forgotten to load the policy,
but Xen just told me:

Flask:  Access controls disabled until policy is loaded.

which is an understatement. I somehow had expected that if no
policy was loaded it would revert to the dummy one which has the
same permission as the non-XSM build. Ha! What a surprise..

Now that the XSM is enabled via config it becomes much more
easy to enable it..

Or perhaps change the code to flask so that if there are any
errors loading the policy it uses the dummy one?

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: XSM permissive by default.
  2016-03-09  1:51 XSM permissive by default Konrad Rzeszutek Wilk
@ 2016-03-09  2:11 ` Doug Goldstein
  2016-03-09 13:24 ` Andrew Cooper
  1 sibling, 0 replies; 20+ messages in thread
From: Doug Goldstein @ 2016-03-09  2:11 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, dgdegra, xen-devel, andrew.cooper3,
	Machon Gregory


[-- Attachment #1.1.1: Type: text/plain, Size: 1338 bytes --]

On 3/8/16 7:51 PM, Konrad Rzeszutek Wilk wrote:
> Hey,
> 
> I was wondering if it we should change the default flask_bootparam
> option from permissive to disabled?
> 
> The reason being is that I was startled to see that my xSplice
> code was able to patch the hypervisor from within an PV guest!
> 
> Further testing showed that I could do 'xl debug-keys R' from
> within the guests. This being possible with released 4.6 if I have
> XSM enabled.
> 
> All of this is due to the fact that I had forgotten to load the policy,
> but Xen just told me:
> 
> Flask:  Access controls disabled until policy is loaded.
> 
> which is an understatement. I somehow had expected that if no
> policy was loaded it would revert to the dummy one which has the
> same permission as the non-XSM build. Ha! What a surprise..

That's certainly been my assumption as well.

> 
> Now that the XSM is enabled via config it becomes much more
> easy to enable it..
> 
> Or perhaps change the code to flask so that if there are any
> errors loading the policy it uses the dummy one?
> 

To me that's what that error message from flask meant so I think that's
the most sane default. Being in a worse state than if you had built
without it.

Machon, Something to consider for the Yocto builds as well.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: XSM permissive by default.
  2016-03-09  1:51 XSM permissive by default Konrad Rzeszutek Wilk
  2016-03-09  2:11 ` Doug Goldstein
@ 2016-03-09 13:24 ` Andrew Cooper
  2016-03-09 21:17   ` Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 20+ messages in thread
From: Andrew Cooper @ 2016-03-09 13:24 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, dgdegra, xen-devel

On 09/03/16 01:51, Konrad Rzeszutek Wilk wrote:
> Hey,
>
> I was wondering if it we should change the default flask_bootparam
> option from permissive to disabled?
>
> The reason being is that I was startled to see that my xSplice
> code was able to patch the hypervisor from within an PV guest!
>
> Further testing showed that I could do 'xl debug-keys R' from
> within the guests. This being possible with released 4.6 if I have
> XSM enabled.
>
> All of this is due to the fact that I had forgotten to load the policy,
> but Xen just told me:
>
> Flask:  Access controls disabled until policy is loaded.
>
> which is an understatement. I somehow had expected that if no
> policy was loaded it would revert to the dummy one which has the
> same permission as the non-XSM build. Ha! What a surprise..
>
> Now that the XSM is enabled via config it becomes much more
> easy to enable it..
>
> Or perhaps change the code to flask so that if there are any
> errors loading the policy it uses the dummy one?

By the looks of it, "permissive" shouldn't be an available option at all.

If a misconfiguration occurs, the behaviour should revert back to the
current "dom0 all powerful, everything else unprivileged" state which
currently exists without XSM.

~Andrew

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: XSM permissive by default.
  2016-03-09 13:24 ` Andrew Cooper
@ 2016-03-09 21:17   ` Konrad Rzeszutek Wilk
  2016-03-09 22:09     ` Daniel De Graaf
  0 siblings, 1 reply; 20+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-03-09 21:17 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel, dgdegra

On Wed, Mar 09, 2016 at 01:24:15PM +0000, Andrew Cooper wrote:
> On 09/03/16 01:51, Konrad Rzeszutek Wilk wrote:
> > Hey,
> >
> > I was wondering if it we should change the default flask_bootparam
> > option from permissive to disabled?
> >
> > The reason being is that I was startled to see that my xSplice
> > code was able to patch the hypervisor from within an PV guest!
> >
> > Further testing showed that I could do 'xl debug-keys R' from
> > within the guests. This being possible with released 4.6 if I have
> > XSM enabled.
> >
> > All of this is due to the fact that I had forgotten to load the policy,
> > but Xen just told me:
> >
> > Flask:  Access controls disabled until policy is loaded.
> >
> > which is an understatement. I somehow had expected that if no
> > policy was loaded it would revert to the dummy one which has the
> > same permission as the non-XSM build. Ha! What a surprise..
> >
> > Now that the XSM is enabled via config it becomes much more
> > easy to enable it..
> >
> > Or perhaps change the code to flask so that if there are any

s/flask/dummy/
> > errors loading the policy it uses the dummy one?
> 
> By the looks of it, "permissive" shouldn't be an available option at all.
> 
> If a misconfiguration occurs, the behaviour should revert back to the
> current "dom0 all powerful, everything else unprivileged" state which
> currently exists without XSM.

Looking deeper in the code I believe it should be possible to swap
from the 'flask_ops' to the 'dummy_ops' (which is what you have without
XSM) if there is a failure during booting to load the policy file
(or the person simply forgot to include it). 

However it was not clear to me whether changing the ops from dummy_ops
to flask_ops during runtime (When the policy being loaded) would work.
It looks like it should be possible as FLASK_DISABLE does it..

Or whether one can FLASK_LOAD if the ops are dummy_ops instead
of flask_ops.

I will try to spin out a patch for this next week.
> 
> ~Andrew

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: XSM permissive by default.
  2016-03-09 21:17   ` Konrad Rzeszutek Wilk
@ 2016-03-09 22:09     ` Daniel De Graaf
  2016-03-10  2:40       ` Doug Goldstein
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel De Graaf @ 2016-03-09 22:09 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Andrew Cooper; +Cc: xen-devel

On 03/09/2016 04:17 PM, Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 09, 2016 at 01:24:15PM +0000, Andrew Cooper wrote:
>> On 09/03/16 01:51, Konrad Rzeszutek Wilk wrote:
>>> Hey,
>>>
>>> I was wondering if it we should change the default flask_bootparam
>>> option from permissive to disabled?
>>>
>>> The reason being is that I was startled to see that my xSplice
>>> code was able to patch the hypervisor from within an PV guest!
>>>
>>> Further testing showed that I could do 'xl debug-keys R' from
>>> within the guests. This being possible with released 4.6 if I have
>>> XSM enabled.
>>>
>>> All of this is due to the fact that I had forgotten to load the policy,
>>> but Xen just told me:
>>>
>>> Flask:  Access controls disabled until policy is loaded.
>>>
>>> which is an understatement. I somehow had expected that if no
>>> policy was loaded it would revert to the dummy one which has the
>>> same permission as the non-XSM build. Ha! What a surprise..
>>>
>>> Now that the XSM is enabled via config it becomes much more
>>> easy to enable it..
>>>
>>> Or perhaps change the code to flask so that if there are any
>
> s/flask/dummy/
>>> errors loading the policy it uses the dummy one?
>>
>> By the looks of it, "permissive" shouldn't be an available option at all.

Permissive is meant for developing (or debugging) a disaggregated system,
where the restrictions on non-dom0 would also break the system.  However,
I agree that it needs to be harder to end up in this mode by accident.

The simplest solution in my opinion is to change the boot parameter to
default to "flask=enforcing", which will fail the boot if a policy is
not available prior to dom0 creation.  This would require any setup
where the policy is loaded from userspace to explicitly specify
"flask=late", whereas they can currently get away with no parameter.

Another solution would be to default to "flask=late" and either deny the
creation of domains if a policy is not present, or automatically revert
to the dummy module on domain creation with no loaded policy.  The latter
probably deserves a different name ("flask=auto"?).

>> If a misconfiguration occurs, the behaviour should revert back to the
>> current "dom0 all powerful, everything else unprivileged" state which
>> currently exists without XSM.
>
> Looking deeper in the code I believe it should be possible to swap
> from the 'flask_ops' to the 'dummy_ops' (which is what you have without
> XSM) if there is a failure during booting to load the policy file
> (or the person simply forgot to include it).
>
> However it was not clear to me whether changing the ops from dummy_ops
> to flask_ops during runtime (When the policy being loaded) would work.
> It looks like it should be possible as FLASK_DISABLE does it..

The main issue with starting with dummy and then switching to FLASK is
that any domains created while using the dummy policy won't have
flask_domain_alloc_security called to populate domain->ssid, and the
rest of the flask code relies on this being non-NULL. The same would
be true for event channels, but inlining the field to save space makes
that a non-issue.

> Or whether one can FLASK_LOAD if the ops are dummy_ops instead
> of flask_ops.

Right, the flask_op hypercall is also disconnected in the dummy module.

>
> I will try to spin out a patch for this next week.
>>
>> ~Andrew

-- 
Daniel De Graaf
National Security Agency

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: XSM permissive by default.
  2016-03-09 22:09     ` Daniel De Graaf
@ 2016-03-10  2:40       ` Doug Goldstein
  2016-03-10 17:10         ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 20+ messages in thread
From: Doug Goldstein @ 2016-03-10  2:40 UTC (permalink / raw)
  To: Daniel De Graaf, Konrad Rzeszutek Wilk, Andrew Cooper; +Cc: xen-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 2101 bytes --]

On 3/9/16 4:09 PM, Daniel De Graaf wrote:
> On 03/09/2016 04:17 PM, Konrad Rzeszutek Wilk wrote:
>> On Wed, Mar 09, 2016 at 01:24:15PM +0000, Andrew Cooper wrote:
>>> On 09/03/16 01:51, Konrad Rzeszutek Wilk wrote:
>>>> Hey,
>>>>
>>>> I was wondering if it we should change the default flask_bootparam
>>>> option from permissive to disabled?

>>>
>>> By the looks of it, "permissive" shouldn't be an available option at
>>> all.
> 
> Permissive is meant for developing (or debugging) a disaggregated system,
> where the restrictions on non-dom0 would also break the system.  However,
> I agree that it needs to be harder to end up in this mode by accident.
> 
> The simplest solution in my opinion is to change the boot parameter to
> default to "flask=enforcing", which will fail the boot if a policy is
> not available prior to dom0 creation.  This would require any setup
> where the policy is loaded from userspace to explicitly specify
> "flask=late", whereas they can currently get away with no parameter.
> 
> Another solution would be to default to "flask=late" and either deny the
> creation of domains if a policy is not present, or automatically revert
> to the dummy module on domain creation with no loaded policy.  The latter
> probably deserves a different name ("flask=auto"?).
> 

Honestly I'm in favor of secure by default approach. Since Xen is not
built with flask by default to me the sane approach would be to default
the system to "flask=enforcing".

"flask=late" not allowing the creation of domains sounds good but what
if you're using a disaggregated dom0 with some domDs and one of them
needs to be up to fetch your policy? Just a hypothetical.

XSMs like LSMs just aren't meant to be swapped around at runtime and
like Daniel points out if go down the road of swapping to the dummy
module there could be further dragons and whose to say someone won't
look at that and put something in that allows you to switch to another
later on (yes I know there's only really 1 but I'm speaking of the
hypothetical).

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: XSM permissive by default.
  2016-03-10  2:40       ` Doug Goldstein
@ 2016-03-10 17:10         ` Konrad Rzeszutek Wilk
  2016-03-10 17:34           ` Doug Goldstein
                             ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-03-10 17:10 UTC (permalink / raw)
  To: Doug Goldstein; +Cc: Andrew Cooper, Daniel De Graaf, xen-devel

On Wed, Mar 09, 2016 at 08:40:05PM -0600, Doug Goldstein wrote:
> On 3/9/16 4:09 PM, Daniel De Graaf wrote:
> > On 03/09/2016 04:17 PM, Konrad Rzeszutek Wilk wrote:
> >> On Wed, Mar 09, 2016 at 01:24:15PM +0000, Andrew Cooper wrote:
> >>> On 09/03/16 01:51, Konrad Rzeszutek Wilk wrote:
> >>>> Hey,
> >>>>
> >>>> I was wondering if it we should change the default flask_bootparam
> >>>> option from permissive to disabled?
> 
> >>>
> >>> By the looks of it, "permissive" shouldn't be an available option at
> >>> all.
> > 
> > Permissive is meant for developing (or debugging) a disaggregated system,
> > where the restrictions on non-dom0 would also break the system.  However,
> > I agree that it needs to be harder to end up in this mode by accident.
> > 
> > The simplest solution in my opinion is to change the boot parameter to
> > default to "flask=enforcing", which will fail the boot if a policy is
> > not available prior to dom0 creation.  This would require any setup
> > where the policy is loaded from userspace to explicitly specify
> > "flask=late", whereas they can currently get away with no parameter.
> > 
> > Another solution would be to default to "flask=late" and either deny the
> > creation of domains if a policy is not present, or automatically revert
> > to the dummy module on domain creation with no loaded policy.  The latter
> > probably deserves a different name ("flask=auto"?).
> > 
> 
> Honestly I'm in favor of secure by default approach. Since Xen is not
> built with flask by default to me the sane approach would be to default
> the system to "flask=enforcing".
> 
> "flask=late" not allowing the creation of domains sounds good but what
> if you're using a disaggregated dom0 with some domDs and one of them
> needs to be up to fetch your policy? Just a hypothetical.
> 
> XSMs like LSMs just aren't meant to be swapped around at runtime and
> like Daniel points out if go down the road of swapping to the dummy
> module there could be further dragons and whose to say someone won't
> look at that and put something in that allows you to switch to another
> later on (yes I know there's only really 1 but I'm speaking of the
> hypothetical).


I presume this patch would be to folks +1:

From 3373a50f386b41eea6ecede4b430e4fa09b2fe7e Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Thu, 10 Mar 2016 12:05:29 -0500
Subject: [PATCH] flask: By default be in FLASK_BOOTPARAM_ENFORCING mode.

By default the mode was 'permissive' which is "meant for
developing (or debugging) a disaggregated system,
where the restrictions on non-dom0 would also break the system."

However this default mode made it possible to boot an machine
in this state if a policy file during bootup was not provided.

The end was less secure than with XSM-enabled - any guest
could do any operation (including rebooting the machine).

Alternative solutions such as switching from flask to dummy.
However "The main issue with starting with dummy and then
switching to FLASK is that any domains created while using
the dummy policy won't have flask_domain_alloc_security called
to populate domain->ssid, and the rest of the flask code relies
on this being non-NULL. The same would be true for event channels,
but inlining the field to save space makes that a non-issue."

(both excerpts are from Daniel De Graaf emails).

This is a much easier fix.

Suggested-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 docs/misc/xen-command-line.markdown | 2 +-
 xen/xsm/flask/flask_op.c            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index ca77e3b..9e77f8a 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -662,7 +662,7 @@ to use the default.
 ### flask
 > `= permissive | enforcing | late | disabled`
 
-> Default: `permissive`
+> Default: `enforcing`
 
 Specify how the FLASK security server should be configured.  This option is only
 available if the hypervisor was compiled with XSM support (which can be enabled
diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index f4f5dd1..aaed75d 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -25,7 +25,7 @@
 #define _copy_to_guest copy_to_guest
 #define _copy_from_guest copy_from_guest
 
-enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE;
+enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_ENFORCING;
 static void parse_flask_param(char *s);
 custom_param("flask", parse_flask_param);
 
-- 
2.5.0


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: XSM permissive by default.
  2016-03-10 17:10         ` Konrad Rzeszutek Wilk
@ 2016-03-10 17:34           ` Doug Goldstein
  2016-03-10 17:44           ` Andrew Cooper
                             ` (2 subsequent siblings)
  3 siblings, 0 replies; 20+ messages in thread
From: Doug Goldstein @ 2016-03-10 17:34 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: Andrew Cooper, Daniel De Graaf, xen-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 5260 bytes --]

On 3/10/16 11:10 AM, Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 09, 2016 at 08:40:05PM -0600, Doug Goldstein wrote:
>> On 3/9/16 4:09 PM, Daniel De Graaf wrote:
>>> On 03/09/2016 04:17 PM, Konrad Rzeszutek Wilk wrote:
>>>> On Wed, Mar 09, 2016 at 01:24:15PM +0000, Andrew Cooper wrote:
>>>>> On 09/03/16 01:51, Konrad Rzeszutek Wilk wrote:
>>>>>> Hey,
>>>>>>
>>>>>> I was wondering if it we should change the default flask_bootparam
>>>>>> option from permissive to disabled?
>>
>>>>>
>>>>> By the looks of it, "permissive" shouldn't be an available option at
>>>>> all.
>>>
>>> Permissive is meant for developing (or debugging) a disaggregated system,
>>> where the restrictions on non-dom0 would also break the system.  However,
>>> I agree that it needs to be harder to end up in this mode by accident.
>>>
>>> The simplest solution in my opinion is to change the boot parameter to
>>> default to "flask=enforcing", which will fail the boot if a policy is
>>> not available prior to dom0 creation.  This would require any setup
>>> where the policy is loaded from userspace to explicitly specify
>>> "flask=late", whereas they can currently get away with no parameter.
>>>
>>> Another solution would be to default to "flask=late" and either deny the
>>> creation of domains if a policy is not present, or automatically revert
>>> to the dummy module on domain creation with no loaded policy.  The latter
>>> probably deserves a different name ("flask=auto"?).
>>>
>>
>> Honestly I'm in favor of secure by default approach. Since Xen is not
>> built with flask by default to me the sane approach would be to default
>> the system to "flask=enforcing".
>>
>> "flask=late" not allowing the creation of domains sounds good but what
>> if you're using a disaggregated dom0 with some domDs and one of them
>> needs to be up to fetch your policy? Just a hypothetical.
>>
>> XSMs like LSMs just aren't meant to be swapped around at runtime and
>> like Daniel points out if go down the road of swapping to the dummy
>> module there could be further dragons and whose to say someone won't
>> look at that and put something in that allows you to switch to another
>> later on (yes I know there's only really 1 but I'm speaking of the
>> hypothetical).
> 
> 
> I presume this patch would be to folks +1:
> 
> From 3373a50f386b41eea6ecede4b430e4fa09b2fe7e Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Thu, 10 Mar 2016 12:05:29 -0500
> Subject: [PATCH] flask: By default be in FLASK_BOOTPARAM_ENFORCING mode.
> 
> By default the mode was 'permissive' which is "meant for
> developing (or debugging) a disaggregated system,
> where the restrictions on non-dom0 would also break the system."
> 
> However this default mode made it possible to boot an machine
> in this state if a policy file during bootup was not provided.
> 
> The end was less secure than with XSM-enabled - any guest
> could do any operation (including rebooting the machine).
> 
> Alternative solutions such as switching from flask to dummy.
> However "The main issue with starting with dummy and then
> switching to FLASK is that any domains created while using
> the dummy policy won't have flask_domain_alloc_security called
> to populate domain->ssid, and the rest of the flask code relies
> on this being non-NULL. The same would be true for event channels,
> but inlining the field to save space makes that a non-issue."
> 
> (both excerpts are from Daniel De Graaf emails).
> 
> This is a much easier fix.
> 
> Suggested-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  docs/misc/xen-command-line.markdown | 2 +-
>  xen/xsm/flask/flask_op.c            | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index ca77e3b..9e77f8a 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -662,7 +662,7 @@ to use the default.
>  ### flask
>  > `= permissive | enforcing | late | disabled`
>  
> -> Default: `permissive`
> +> Default: `enforcing`
>  
>  Specify how the FLASK security server should be configured.  This option is only
>  available if the hypervisor was compiled with XSM support (which can be enabled
> diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
> index f4f5dd1..aaed75d 100644
> --- a/xen/xsm/flask/flask_op.c
> +++ b/xen/xsm/flask/flask_op.c
> @@ -25,7 +25,7 @@
>  #define _copy_to_guest copy_to_guest
>  #define _copy_from_guest copy_from_guest
>  
> -enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE;
> +enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_ENFORCING;
>  static void parse_flask_param(char *s);
>  custom_param("flask", parse_flask_param);
>  
> 

+1

You also found a spot that I didn't find when doing the Kconfig
conversion of CONFIG_XSM / CONFIG_FLASK. My search didn't catch
XSM\_ENABLE so follow on patch from me for that will be coming.

Reviewed-by: Doug Goldstein <cardoe@cardoe.com>

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: XSM permissive by default.
  2016-03-10 17:10         ` Konrad Rzeszutek Wilk
  2016-03-10 17:34           ` Doug Goldstein
@ 2016-03-10 17:44           ` Andrew Cooper
  2016-03-10 18:30           ` [PATCH] flask: change default state to enforcing Daniel De Graaf
  2016-04-04 17:12           ` XSM permissive by default Ian Jackson
  3 siblings, 0 replies; 20+ messages in thread
From: Andrew Cooper @ 2016-03-10 17:44 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Doug Goldstein; +Cc: xen-devel, Daniel De Graaf

On 10/03/16 17:10, Konrad Rzeszutek Wilk wrote:
> I presume this patch would be to folks +1:
>
> From 3373a50f386b41eea6ecede4b430e4fa09b2fe7e Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Thu, 10 Mar 2016 12:05:29 -0500
> Subject: [PATCH] flask: By default be in FLASK_BOOTPARAM_ENFORCING mode.
>
> By default the mode was 'permissive' which is "meant for
> developing (or debugging) a disaggregated system,
> where the restrictions on non-dom0 would also break the system."
>
> However this default mode made it possible to boot an machine
> in this state if a policy file during bootup was not provided.
>
> The end was less secure than with XSM-enabled - any guest
> could do any operation (including rebooting the machine).
>
> Alternative solutions such as switching from flask to dummy.
> However "The main issue with starting with dummy and then
> switching to FLASK is that any domains created while using
> the dummy policy won't have flask_domain_alloc_security called
> to populate domain->ssid, and the rest of the flask code relies
> on this being non-NULL. The same would be true for event channels,
> but inlining the field to save space makes that a non-issue."
>
> (both excerpts are from Daniel De Graaf emails).
>
> This is a much easier fix.
>
> Suggested-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

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

> ---
>  docs/misc/xen-command-line.markdown | 2 +-
>  xen/xsm/flask/flask_op.c            | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index ca77e3b..9e77f8a 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -662,7 +662,7 @@ to use the default.
>  ### flask
>  > `= permissive | enforcing | late | disabled`
>  
> -> Default: `permissive`
> +> Default: `enforcing`
>  
>  Specify how the FLASK security server should be configured.  This option is only
>  available if the hypervisor was compiled with XSM support (which can be enabled
> diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
> index f4f5dd1..aaed75d 100644
> --- a/xen/xsm/flask/flask_op.c
> +++ b/xen/xsm/flask/flask_op.c
> @@ -25,7 +25,7 @@
>  #define _copy_to_guest copy_to_guest
>  #define _copy_from_guest copy_from_guest
>  
> -enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE;
> +enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_ENFORCING;
>  static void parse_flask_param(char *s);
>  custom_param("flask", parse_flask_param);
>  


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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] flask: change default state to enforcing
  2016-03-10 17:10         ` Konrad Rzeszutek Wilk
  2016-03-10 17:34           ` Doug Goldstein
  2016-03-10 17:44           ` Andrew Cooper
@ 2016-03-10 18:30           ` Daniel De Graaf
  2016-03-10 19:12             ` Konrad Rzeszutek Wilk
  2016-03-11  9:07             ` Jan Beulich
  2016-04-04 17:12           ` XSM permissive by default Ian Jackson
  3 siblings, 2 replies; 20+ messages in thread
From: Daniel De Graaf @ 2016-03-10 18:30 UTC (permalink / raw)
  To: xen-devel; +Cc: andrew.cooper3, Daniel De Graaf, cardoe

The previous default of "permissive" is meant for developing or
debugging a disaggregated system.  However, this default makes it too
easy to accidentally boot a machine in this state, which does not place
any restrictions on guests.  This is not suitable for normal systems
because any guest can perform any operation (including operations like
rebooting the machine, kexec, and reading or writing another domain's
memory).

This change will cause the boot to fail if you do not specify an XSM
policy during boot; if you need to load a policy from dom0, use the
"flask=late" boot parameter.

Originally by Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; modified
to also change the default value of flask_enforcing so that the policy
is not still in permissive mode.  This also removes the (no longer
documented) command line argument directly changing that variable since
it has been superseded by the flask= parameter.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---

 docs/misc/xen-command-line.markdown |  2 +-
 docs/misc/xsm-flask.txt             | 12 ++++++------
 xen/xsm/flask/flask_op.c            |  8 +++++---
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index ca77e3b..9e77f8a 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -662,7 +662,7 @@ to use the default.
 ### flask
 > `= permissive | enforcing | late | disabled`
 
-> Default: `permissive`
+> Default: `enforcing`
 
 Specify how the FLASK security server should be configured.  This option is only
 available if the hypervisor was compiled with XSM support (which can be enabled
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index fb2fe9f..00a2b13 100644
--- a/docs/misc/xsm-flask.txt
+++ b/docs/misc/xsm-flask.txt
@@ -283,12 +283,12 @@ for passthrough, run:
 
 This command must be rerun on each boot or after any policy reload.
 
-The example policy was only tested with simple domain creation and may be
-missing rules allowing accesses by dom0 or domU when a number of hypervisor
-features are used. When first loading or writing a policy, you should run FLASK
-in permissive mode (the default) and check the Xen logs (xl dmesg) for AVC
-denials before using it in enforcing mode (flask_enforcing=1 on the command
-line, or xl setenforce).
+When first loading or writing a policy, you should run FLASK in permissive mode
+(flask=permissive on the command line) and check the Xen logs (xl dmesg) for AVC
+denials before using it in enforcing mode (the default value of the boot
+parameter, which can also be changed using xl setenforce).  When using the
+default types for domains (domU_t), the example policy shipped with Xen should
+allow the same operations on or between domains as when not using FLASK.
 
 
 MLS/MCS policy
diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index f4f5dd1..cdb462c 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -25,12 +25,11 @@
 #define _copy_to_guest copy_to_guest
 #define _copy_from_guest copy_from_guest
 
-enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE;
+enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_ENFORCING;
 static void parse_flask_param(char *s);
 custom_param("flask", parse_flask_param);
 
-bool_t __read_mostly flask_enforcing = 0;
-boolean_param("flask_enforcing", flask_enforcing);
+bool_t __read_mostly flask_enforcing = 1;
 
 #define MAX_POLICY_SIZE 0x4000000
 
@@ -76,7 +75,10 @@ static void __init parse_flask_param(char *s)
     else if ( !strcmp(s, "disabled") )
         flask_bootparam = FLASK_BOOTPARAM_DISABLED;
     else if ( !strcmp(s, "permissive") )
+    {
+        flask_enforcing = 0;
         flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE;
+    }
     else
         flask_bootparam = FLASK_BOOTPARAM_INVALID;
 }
-- 
2.5.0


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] flask: change default state to enforcing
  2016-03-10 18:30           ` [PATCH] flask: change default state to enforcing Daniel De Graaf
@ 2016-03-10 19:12             ` Konrad Rzeszutek Wilk
  2016-03-10 19:37               ` Daniel De Graaf
  2016-03-15 14:48               ` Anshul Makkar
  2016-03-11  9:07             ` Jan Beulich
  1 sibling, 2 replies; 20+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-03-10 19:12 UTC (permalink / raw)
  To: Daniel De Graaf, ian.jackson, jbeulich; +Cc: xen-devel, cardoe, andrew.cooper3

On Thu, Mar 10, 2016 at 01:30:29PM -0500, Daniel De Graaf wrote:

I've added Ian and Jan on the email as scripts/get_maintainer.pl spits out
their names (Oddly not yours?)
> The previous default of "permissive" is meant for developing or
> debugging a disaggregated system.  However, this default makes it too
> easy to accidentally boot a machine in this state, which does not place
> any restrictions on guests.  This is not suitable for normal systems
> because any guest can perform any operation (including operations like
> rebooting the machine, kexec, and reading or writing another domain's
> memory).
> 
> This change will cause the boot to fail if you do not specify an XSM
> policy during boot; if you need to load a policy from dom0, use the
> "flask=late" boot parameter.
> 
> Originally by Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; modified
> to also change the default value of flask_enforcing so that the policy
> is not still in permissive mode.  This also removes the (no longer
> documented) command line argument directly changing that variable since
> it has been superseded by the flask= parameter.
> 

Reviwed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
.. however:

> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
> 
>  docs/misc/xen-command-line.markdown |  2 +-
>  docs/misc/xsm-flask.txt             | 12 ++++++------
>  xen/xsm/flask/flask_op.c            |  8 +++++---
>  3 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index ca77e3b..9e77f8a 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -662,7 +662,7 @@ to use the default.
>  ### flask
>  > `= permissive | enforcing | late | disabled`
>  
> -> Default: `permissive`
> +> Default: `enforcing`
>  
>  Specify how the FLASK security server should be configured.  This option is only
>  available if the hypervisor was compiled with XSM support (which can be enabled
> diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
> index fb2fe9f..00a2b13 100644
> --- a/docs/misc/xsm-flask.txt
> +++ b/docs/misc/xsm-flask.txt
> @@ -283,12 +283,12 @@ for passthrough, run:
>  
>  This command must be rerun on each boot or after any policy reload.
>  
> -The example policy was only tested with simple domain creation and may be
> -missing rules allowing accesses by dom0 or domU when a number of hypervisor
> -features are used. When first loading or writing a policy, you should run FLASK
> -in permissive mode (the default) and check the Xen logs (xl dmesg) for AVC
> -denials before using it in enforcing mode (flask_enforcing=1 on the command
> -line, or xl setenforce).
> +When first loading or writing a policy, you should run FLASK in permissive mode
> +(flask=permissive on the command line) and check the Xen logs (xl dmesg) for AVC
> +denials before using it in enforcing mode (the default value of the boot
> +parameter, which can also be changed using xl setenforce).  When using the
> +default types for domains (domU_t), the example policy shipped with Xen should
> +allow the same operations on or between domains as when not using FLASK.
>  
>  
>  MLS/MCS policy
> diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
> index f4f5dd1..cdb462c 100644
> --- a/xen/xsm/flask/flask_op.c
> +++ b/xen/xsm/flask/flask_op.c
> @@ -25,12 +25,11 @@
>  #define _copy_to_guest copy_to_guest
>  #define _copy_from_guest copy_from_guest
>  
> -enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE;
> +enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_ENFORCING;
>  static void parse_flask_param(char *s);
>  custom_param("flask", parse_flask_param);
>  
> -bool_t __read_mostly flask_enforcing = 0;
> -boolean_param("flask_enforcing", flask_enforcing);
> +bool_t __read_mostly flask_enforcing = 1;

Since you set that to the default value should the parse_flask_param
'flask_enforcing = 1' for the 'enforcing' and 'late' be removed?

(If you agree, the committer could do it).

>  
>  #define MAX_POLICY_SIZE 0x4000000
>  
> @@ -76,7 +75,10 @@ static void __init parse_flask_param(char *s)
>      else if ( !strcmp(s, "disabled") )
>          flask_bootparam = FLASK_BOOTPARAM_DISABLED;
>      else if ( !strcmp(s, "permissive") )
> +    {
> +        flask_enforcing = 0;
>          flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE;
> +    }
>      else
>          flask_bootparam = FLASK_BOOTPARAM_INVALID;
>  }
> -- 
> 2.5.0
> 

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] flask: change default state to enforcing
  2016-03-10 19:12             ` Konrad Rzeszutek Wilk
@ 2016-03-10 19:37               ` Daniel De Graaf
  2016-03-15 14:48               ` Anshul Makkar
  1 sibling, 0 replies; 20+ messages in thread
From: Daniel De Graaf @ 2016-03-10 19:37 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, ian.jackson, jbeulich
  Cc: xen-devel, cardoe, andrew.cooper3

On 03/10/2016 02:12 PM, Konrad Rzeszutek Wilk wrote:
> On Thu, Mar 10, 2016 at 01:30:29PM -0500, Daniel De Graaf wrote:
>
> I've added Ian and Jan on the email as scripts/get_maintainer.pl spits out
> their names (Oddly not yours?)
>> The previous default of "permissive" is meant for developing or
>> debugging a disaggregated system.  However, this default makes it too
>> easy to accidentally boot a machine in this state, which does not place
>> any restrictions on guests.  This is not suitable for normal systems
>> because any guest can perform any operation (including operations like
>> rebooting the machine, kexec, and reading or writing another domain's
>> memory).
>>
>> This change will cause the boot to fail if you do not specify an XSM
>> policy during boot; if you need to load a policy from dom0, use the
>> "flask=late" boot parameter.
>>
>> Originally by Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; modified
>> to also change the default value of flask_enforcing so that the policy
>> is not still in permissive mode.  This also removes the (no longer
>> documented) command line argument directly changing that variable since
>> it has been superseded by the flask= parameter.
>>
>
> Reviwed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> .. however:
>
[...]
>
> Since you set that to the default value should the parse_flask_param
> 'flask_enforcing = 1' for the 'enforcing' and 'late' be removed?
>
> (If you agree, the committer could do it).

Sure.  I left them in so that a command line such as
"flask=permissive flask=enforcing" would do the right thing, but I
haven't checked that that is even possible.


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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] flask: change default state to enforcing
  2016-03-10 18:30           ` [PATCH] flask: change default state to enforcing Daniel De Graaf
  2016-03-10 19:12             ` Konrad Rzeszutek Wilk
@ 2016-03-11  9:07             ` Jan Beulich
  2016-03-11 14:58               ` Konrad Rzeszutek Wilk
  2016-03-11 15:39               ` Daniel De Graaf
  1 sibling, 2 replies; 20+ messages in thread
From: Jan Beulich @ 2016-03-11  9:07 UTC (permalink / raw)
  To: Daniel De Graaf; +Cc: andrew.cooper3, cardoe, xen-devel

>>> On 10.03.16 at 19:30, <dgdegra@tycho.nsa.gov> wrote:
> This change will cause the boot to fail if you do not specify an XSM
> policy during boot; if you need to load a policy from dom0, use the
> "flask=late" boot parameter.

And what mode is the system in until that happens? From the
command line doc, I understand it would be in not-enforcing
mode, but that seems contrary to the code (already before
your change) setting flask_enforcing to 1 in that case.

> Originally by Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; modified
> to also change the default value of flask_enforcing so that the policy
> is not still in permissive mode.  This also removes the (no longer
> documented) command line argument directly changing that variable since
> it has been superseded by the flask= parameter.
> 
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Question now is - who is to be considered the author of this patch?

Jan


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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] flask: change default state to enforcing
  2016-03-11  9:07             ` Jan Beulich
@ 2016-03-11 14:58               ` Konrad Rzeszutek Wilk
  2016-03-11 15:39               ` Daniel De Graaf
  1 sibling, 0 replies; 20+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-03-11 14:58 UTC (permalink / raw)
  To: Jan Beulich; +Cc: andrew.cooper3, Daniel De Graaf, cardoe, xen-devel

On Fri, Mar 11, 2016 at 02:07:11AM -0700, Jan Beulich wrote:
> >>> On 10.03.16 at 19:30, <dgdegra@tycho.nsa.gov> wrote:
> > This change will cause the boot to fail if you do not specify an XSM
> > policy during boot; if you need to load a policy from dom0, use the
> > "flask=late" boot parameter.
> 
> And what mode is the system in until that happens? From the
> command line doc, I understand it would be in not-enforcing
> mode, but that seems contrary to the code (already before
> your change) setting flask_enforcing to 1 in that case.
> 
> > Originally by Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; modified
> > to also change the default value of flask_enforcing so that the policy
> > is not still in permissive mode.  This also removes the (no longer
> > documented) command line argument directly changing that variable since
> > it has been superseded by the flask= parameter.
> > 
> > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> Question now is - who is to be considered the author of this patch?

Daniel.


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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] flask: change default state to enforcing
  2016-03-11  9:07             ` Jan Beulich
  2016-03-11 14:58               ` Konrad Rzeszutek Wilk
@ 2016-03-11 15:39               ` Daniel De Graaf
  2016-03-11 15:43                 ` Jan Beulich
  1 sibling, 1 reply; 20+ messages in thread
From: Daniel De Graaf @ 2016-03-11 15:39 UTC (permalink / raw)
  To: Jan Beulich; +Cc: andrew.cooper3, cardoe, xen-devel

On 03/11/2016 04:07 AM, Jan Beulich wrote:
>>>> On 10.03.16 at 19:30, <dgdegra@tycho.nsa.gov> wrote:
>> This change will cause the boot to fail if you do not specify an XSM
>> policy during boot; if you need to load a policy from dom0, use the
>> "flask=late" boot parameter.
>
> And what mode is the system in until that happens? From the
> command line doc, I understand it would be in not-enforcing
> mode, but that seems contrary to the code (already before
> your change) setting flask_enforcing to 1 in that case.

The FLASK code does not deny any actions until a policy has been loaded,
so the flask_enforcing value only takes effect then.  With flask=late,
userspace code can also adjust the value (xl setenforce) before loading
the policy.

-- 
Daniel De Graaf
National Security Agency

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] flask: change default state to enforcing
  2016-03-11 15:39               ` Daniel De Graaf
@ 2016-03-11 15:43                 ` Jan Beulich
  2016-03-11 15:51                   ` Daniel De Graaf
  0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2016-03-11 15:43 UTC (permalink / raw)
  To: Daniel De Graaf; +Cc: andrew.cooper3, cardoe, xen-devel

>>> On 11.03.16 at 16:39, <dgdegra@tycho.nsa.gov> wrote:
> On 03/11/2016 04:07 AM, Jan Beulich wrote:
>>>>> On 10.03.16 at 19:30, <dgdegra@tycho.nsa.gov> wrote:
>>> This change will cause the boot to fail if you do not specify an XSM
>>> policy during boot; if you need to load a policy from dom0, use the
>>> "flask=late" boot parameter.
>>
>> And what mode is the system in until that happens? From the
>> command line doc, I understand it would be in not-enforcing
>> mode, but that seems contrary to the code (already before
>> your change) setting flask_enforcing to 1 in that case.
> 
> The FLASK code does not deny any actions until a policy has been loaded,
> so the flask_enforcing value only takes effect then.  With flask=late,
> userspace code can also adjust the value (xl setenforce) before loading
> the policy.

So doesn't this leave the system again in an insecure state then?

Jan


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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] flask: change default state to enforcing
  2016-03-11 15:43                 ` Jan Beulich
@ 2016-03-11 15:51                   ` Daniel De Graaf
  0 siblings, 0 replies; 20+ messages in thread
From: Daniel De Graaf @ 2016-03-11 15:51 UTC (permalink / raw)
  To: Jan Beulich; +Cc: andrew.cooper3, cardoe, xen-devel

On 03/11/2016 10:43 AM, Jan Beulich wrote:
>>>> On 11.03.16 at 16:39, <dgdegra@tycho.nsa.gov> wrote:
>> On 03/11/2016 04:07 AM, Jan Beulich wrote:
>>>>>> On 10.03.16 at 19:30, <dgdegra@tycho.nsa.gov> wrote:
>>>> This change will cause the boot to fail if you do not specify an XSM
>>>> policy during boot; if you need to load a policy from dom0, use the
>>>> "flask=late" boot parameter.
>>>
>>> And what mode is the system in until that happens? From the
>>> command line doc, I understand it would be in not-enforcing
>>> mode, but that seems contrary to the code (already before
>>> your change) setting flask_enforcing to 1 in that case.
>>
>> The FLASK code does not deny any actions until a policy has been loaded,
>> so the flask_enforcing value only takes effect then.  With flask=late,
>> userspace code can also adjust the value (xl setenforce) before loading
>> the policy.
>
> So doesn't this leave the system again in an insecure state then?
>
> Jan

It does, at least until the policy is loaded.  The point of "flask=late" is
that dom0 loads the policy early in boot, preferably before creating any
domains.  Since all xen architectures now support loading the policy from
the bootloader, this is never a required mode of operation.  We did discuss
preventing the creation of domains without a policy loaded to avoid making
this mistake, but since this is no longer the default, I don't think that
type of guard isnecessary.

-- 
Daniel De Graaf
National Security Agency

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] flask: change default state to enforcing
  2016-03-10 19:12             ` Konrad Rzeszutek Wilk
  2016-03-10 19:37               ` Daniel De Graaf
@ 2016-03-15 14:48               ` Anshul Makkar
  1 sibling, 0 replies; 20+ messages in thread
From: Anshul Makkar @ 2016-03-15 14:48 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Daniel De Graaf, Ian Jackson, jbeulich
  Cc: xen-devel, cardoe, Andrew Cooper



-----Original Message-----
From: Xen-devel [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of Konrad Rzeszutek Wilk
Sent: 10 March 2016 19:12
To: Daniel De Graaf <dgdegra@tycho.nsa.gov>; Ian Jackson <Ian.Jackson@citrix.com>; jbeulich@suse.com
Cc: xen-devel@lists.xenproject.org; cardoe@cardoe.com; Andrew Cooper <Andrew.Cooper3@citrix.com>
Subject: Re: [Xen-devel] [PATCH] flask: change default state to enforcing

On Thu, Mar 10, 2016 at 01:30:29PM -0500, Daniel De Graaf wrote:

I've added Ian and Jan on the email as scripts/get_maintainer.pl spits out their names (Oddly not yours?)
> The previous default of "permissive" is meant for developing or 
> debugging a disaggregated system.  However, this default makes it too 
> easy to accidentally boot a machine in this state, which does not 
> place any restrictions on guests.  This is not suitable for normal 
> systems because any guest can perform any operation (including 
> operations like rebooting the machine, kexec, and reading or writing 
> another domain's memory).
> 
> This change will cause the boot to fail if you do not specify an XSM 
> policy during boot; if you need to load a policy from dom0, use the 
> "flask=late" boot parameter.
> 
> Originally by Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; modified 
> to also change the default value of flask_enforcing so that the policy 
> is not still in permissive mode.  This also removes the (no longer
> documented) command line argument directly changing that variable 
> since it has been superseded by the flask= parameter.
> 

Reviwed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> .. however:

> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
> 
>  docs/misc/xen-command-line.markdown |  2 +-
>  docs/misc/xsm-flask.txt             | 12 ++++++------
>  xen/xsm/flask/flask_op.c            |  8 +++++---
>  3 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.markdown 
> b/docs/misc/xen-command-line.markdown
> index ca77e3b..9e77f8a 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -662,7 +662,7 @@ to use the default.
>  ### flask
>  > `= permissive | enforcing | late | disabled`
>  
> -> Default: `permissive`
> +> Default: `enforcing`
>  
>  Specify how the FLASK security server should be configured.  This 
> option is only  available if the hypervisor was compiled with XSM 
> support (which can be enabled diff --git a/docs/misc/xsm-flask.txt 
> b/docs/misc/xsm-flask.txt index fb2fe9f..00a2b13 100644
> --- a/docs/misc/xsm-flask.txt
> +++ b/docs/misc/xsm-flask.txt
> @@ -283,12 +283,12 @@ for passthrough, run:
>  
>  This command must be rerun on each boot or after any policy reload.
>  
> -The example policy was only tested with simple domain creation and 
> may be -missing rules allowing accesses by dom0 or domU when a number 
> of hypervisor -features are used. When first loading or writing a 
> policy, you should run FLASK -in permissive mode (the default) and 
> check the Xen logs (xl dmesg) for AVC -denials before using it in 
> enforcing mode (flask_enforcing=1 on the command -line, or xl setenforce).
> +When first loading or writing a policy, you should run FLASK in 
> +permissive mode (flask=permissive on the command line) and check the 
> +Xen logs (xl dmesg) for AVC denials before using it in enforcing mode 
> +(the default value of the boot parameter, which can also be changed 
> +using xl setenforce).  When using the default types for domains 
> +(domU_t), the example policy shipped with Xen should allow the same operations on or between domains as when not using FLASK.
>  
>  
>  MLS/MCS policy
> diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c index 
> f4f5dd1..cdb462c 100644
> --- a/xen/xsm/flask/flask_op.c
> +++ b/xen/xsm/flask/flask_op.c
> @@ -25,12 +25,11 @@
>  #define _copy_to_guest copy_to_guest
>  #define _copy_from_guest copy_from_guest
>  
> -enum flask_bootparam_t __read_mostly flask_bootparam = 
> FLASK_BOOTPARAM_PERMISSIVE;
> +enum flask_bootparam_t __read_mostly flask_bootparam = 
> +FLASK_BOOTPARAM_ENFORCING;
>  static void parse_flask_param(char *s);  custom_param("flask", 
> parse_flask_param);
>  
> -bool_t __read_mostly flask_enforcing = 0; 
> -boolean_param("flask_enforcing", flask_enforcing);
> +bool_t __read_mostly flask_enforcing = 1;

Since you set that to the default value should the parse_flask_param 'flask_enforcing = 1' for the 'enforcing' and 'late' be removed?

(If you agree, the committer could do it).

>  
>  #define MAX_POLICY_SIZE 0x4000000
>  
> @@ -76,7 +75,10 @@ static void __init parse_flask_param(char *s)
>      else if ( !strcmp(s, "disabled") )
>          flask_bootparam = FLASK_BOOTPARAM_DISABLED;
>      else if ( !strcmp(s, "permissive") )
> +    {
> +        flask_enforcing = 0;
>          flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE;
> +    }
>      else
>          flask_bootparam = FLASK_BOOTPARAM_INVALID;  }
> --
> 2.5.0
> 

There is no need to explicitly set flask_enforcing=0.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: XSM permissive by default.
  2016-03-10 17:10         ` Konrad Rzeszutek Wilk
                             ` (2 preceding siblings ...)
  2016-03-10 18:30           ` [PATCH] flask: change default state to enforcing Daniel De Graaf
@ 2016-04-04 17:12           ` Ian Jackson
  2016-04-05  8:03             ` Jan Beulich
  3 siblings, 1 reply; 20+ messages in thread
From: Ian Jackson @ 2016-04-04 17:12 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Andrew Cooper, Daniel De Graaf, Doug Goldstein, xen-devel

Konrad Rzeszutek Wilk writes ("Re: [Xen-devel] XSM permissive by default."):
> I presume this patch would be to folks +1:
> 
> From 3373a50f386b41eea6ecede4b430e4fa09b2fe7e Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Thu, 10 Mar 2016 12:05:29 -0500
> Subject: [PATCH] flask: By default be in FLASK_BOOTPARAM_ENFORCING mode.

This has a Reviewed-by from Doug Goldstein and Andrew Cooper.  And
then it was revised by Daniel.  But AFAICT it has not yet been
committed.  I think this is an important change that is in 4.7.

Is there some reason why this patch hasn't been committed ?

Thanks,
Ian.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: XSM permissive by default.
  2016-04-04 17:12           ` XSM permissive by default Ian Jackson
@ 2016-04-05  8:03             ` Jan Beulich
  0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2016-04-05  8:03 UTC (permalink / raw)
  To: Ian Jackson, Konrad Rzeszutek Wilk
  Cc: Andrew Cooper, Daniel De Graaf, Doug Goldstein, xen-devel

>>> On 04.04.16 at 19:12, <Ian.Jackson@eu.citrix.com> wrote:
> Konrad Rzeszutek Wilk writes ("Re: [Xen-devel] XSM permissive by default."):
>> I presume this patch would be to folks +1:
>> 
>> From 3373a50f386b41eea6ecede4b430e4fa09b2fe7e Mon Sep 17 00:00:00 2001
>> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Date: Thu, 10 Mar 2016 12:05:29 -0500
>> Subject: [PATCH] flask: By default be in FLASK_BOOTPARAM_ENFORCING mode.
> 
> This has a Reviewed-by from Doug Goldstein and Andrew Cooper.  And
> then it was revised by Daniel.  But AFAICT it has not yet been
> committed.  I think this is an important change that is in 4.7.
> 
> Is there some reason why this patch hasn't been committed ?

There are two reasons: The thread here isn't rooting at a patch
submission, i.e. to me looks more like a discussion (and as such
isn't even a possible subject for queuing). And then I can't find
any ack from Daniel.

Jan


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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2016-04-05  8:03 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-09  1:51 XSM permissive by default Konrad Rzeszutek Wilk
2016-03-09  2:11 ` Doug Goldstein
2016-03-09 13:24 ` Andrew Cooper
2016-03-09 21:17   ` Konrad Rzeszutek Wilk
2016-03-09 22:09     ` Daniel De Graaf
2016-03-10  2:40       ` Doug Goldstein
2016-03-10 17:10         ` Konrad Rzeszutek Wilk
2016-03-10 17:34           ` Doug Goldstein
2016-03-10 17:44           ` Andrew Cooper
2016-03-10 18:30           ` [PATCH] flask: change default state to enforcing Daniel De Graaf
2016-03-10 19:12             ` Konrad Rzeszutek Wilk
2016-03-10 19:37               ` Daniel De Graaf
2016-03-15 14:48               ` Anshul Makkar
2016-03-11  9:07             ` Jan Beulich
2016-03-11 14:58               ` Konrad Rzeszutek Wilk
2016-03-11 15:39               ` Daniel De Graaf
2016-03-11 15:43                 ` Jan Beulich
2016-03-11 15:51                   ` Daniel De Graaf
2016-04-04 17:12           ` XSM permissive by default Ian Jackson
2016-04-05  8:03             ` Jan Beulich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).