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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

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

> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> ---
> 
>  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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel