Re: [Xen-devel] [PATCH] flask: change default state to enforcing

2016-03-15 Thread Anshul Makkar


-Original Message-
From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of Konrad 
Rzeszutek Wilk
Sent: 10 March 2016 19:12
To: Daniel De Graaf ; Ian Jackson 
; jbeul...@suse.com
Cc: xen-de...@lists.xenproject.org; car...@cardoe.com; Andrew Cooper 

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 ; 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  .. however:

> Signed-off-by: Daniel De Graaf 
> ---
> 
>  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 0x400
>  
> @@ -76,7 +75,10 @@ static void __init parse_flask_param(char *s)
>  else if ( !strcmp(s, "disabled") )
>  flask_bootparam = FLASK_BOOTPARAM_DISABLED;
&g

Re: [Xen-devel] [PATCH] flask: change default state to enforcing

2016-03-11 Thread Daniel De Graaf

On 03/11/2016 10:43 AM, Jan Beulich wrote:

On 11.03.16 at 16:39,  wrote:

On 03/11/2016 04:07 AM, Jan Beulich wrote:

On 10.03.16 at 19:30,  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


Re: [Xen-devel] [PATCH] flask: change default state to enforcing

2016-03-11 Thread Jan Beulich
>>> On 11.03.16 at 16:39,  wrote:
> On 03/11/2016 04:07 AM, Jan Beulich wrote:
> On 10.03.16 at 19:30,  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


Re: [Xen-devel] [PATCH] flask: change default state to enforcing

2016-03-11 Thread Daniel De Graaf

On 03/11/2016 04:07 AM, Jan Beulich wrote:

On 10.03.16 at 19:30,  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


Re: [Xen-devel] [PATCH] flask: change default state to enforcing

2016-03-11 Thread Konrad Rzeszutek Wilk
On Fri, Mar 11, 2016 at 02:07:11AM -0700, Jan Beulich wrote:
> >>> On 10.03.16 at 19:30,  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 ; 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 
> 
> 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


Re: [Xen-devel] [PATCH] flask: change default state to enforcing

2016-03-11 Thread Jan Beulich
>>> On 10.03.16 at 19:30,  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 ; 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 

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


Re: [Xen-devel] [PATCH] flask: change default state to enforcing

2016-03-10 Thread Daniel De Graaf

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


Re: [Xen-devel] [PATCH] flask: change default state to enforcing

2016-03-10 Thread Konrad Rzeszutek Wilk
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 ; 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 
.. however:

> Signed-off-by: Daniel De Graaf 
> ---
> 
>  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 0x400
>  
> @@ -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


[Xen-devel] [PATCH] flask: change default state to enforcing

2016-03-10 Thread Daniel De Graaf
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 ; 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 
---

 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 0x400
 
@@ -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