All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] flask: create unified "flask=" boot parameter
  2015-03-03 17:00 [PATCH v3 0/2] flask: Handle policy load failures properly Daniel De Graaf
@ 2015-03-03 17:00 ` Daniel De Graaf
  2015-03-03 17:12   ` Jan Beulich
  2015-03-06 12:22   ` Wei Liu
  0 siblings, 2 replies; 10+ messages in thread
From: Daniel De Graaf @ 2015-03-03 17:00 UTC (permalink / raw)
  To: xen-devel; +Cc: Daniel De Graaf, julien.grall, wei.liu2, ian.campbell

This unifies the flask_enforcing and flask_enabled boot parameters into
a single parameter with additional states.  Defined options are:

 force - require policy to be loaded at boot time and enforce it
 late - bootloader policy is not used; later loadpolicy is enforcing
 permissive - a missing or broken policy does not panic
 disabled - revert to dummy (no XSM) policy.  Was flask_enabled=0

The default mode remains "permissive" and the flask_enforcing boot
parameter is retained for compatibility.  If flask_enforcing=1 is
specified and flask= is not, the bootloader policy will be loaded in
enforcing mode if present, but errors will disable access controls until
a successful loadpolicy instead of causing a panic at boot.

Suggested-by: Julien Grall <julien.grall@linaro.org>
Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 docs/man/xl.pod.1                |  4 ++--
 docs/misc/xsm-flask.txt          | 48 +++++++++++++++++++---------------------
 xen/xsm/flask/flask_op.c         | 34 ++++++++++++++++++++++++----
 xen/xsm/flask/hooks.c            | 22 ++++++++++--------
 xen/xsm/flask/include/security.h |  8 ++++++-
 5 files changed, 75 insertions(+), 41 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 6b89ba8..48b8f98 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1441,8 +1441,8 @@ Determine if the FLASK security module is loaded and enforcing its policy.
 =item B<setenforce> I<1|0|Enforcing|Permissive>
 
 Enable or disable enforcing of the FLASK access controls. The default is
-permissive and can be changed using the flask_enforcing option on the
-hypervisor's command line.
+permissive, but this can be changed to enforcing by specifying "flask=enforcing"
+or "flask=late" on the hypervisor's command line.
 
 =item B<loadpolicy> I<policy-file>
 
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 9559028..efe8d50 100644
--- a/docs/misc/xsm-flask.txt
+++ b/docs/misc/xsm-flask.txt
@@ -400,28 +400,26 @@ may require multiple passes to find all required ranges.
 Additional notes on XSM:FLASK
 -----------------------------
 
-1) xen command line parameters
-
-	a) flask_enforcing
-	
-	The default value for flask_enforcing is '0'.  This parameter causes the 
-	platform to boot in permissive mode which means that the policy is loaded 
-	but not enforced.  This mode is often helpful for developing new systems 
-	and policies as the policy violations are reported on the xen console and 
-	may be viewed in dom0 through 'xl dmesg'.
-	
-	To boot the platform into enforcing mode, which means that the policy is
-	loaded and enforced, append 'flask_enforcing=1' on the grub line.
-	
-	This parameter may also be changed through the flask hypercall.
-	
-	b) flask_enabled
-	
-	The default value for flask_enabled is '1'.  This parameter causes the
-	platform to enable the FLASK security module under the XSM framework.
-	The parameter may be enabled/disabled only once per boot.  If the parameter
-	is set to '0', only a reboot can re-enable flask.  When flask_enabled is '0'
-	the DUMMY module is enforced.
-
-	This parameter may also be changed through the flask hypercall.  But may
-	only be performed once per boot.
+The xen command line accepts these values for the "flask=" parameter:
+
+ * permissive [default]
+     This is intended for development and is not suitable for use with untrusted
+     guests.  If a policy is provided by the bootloader, it will be loaded;
+     errors will be reported to the ring buffer but will not prevent booting.
+     The policy can be changed to enforcing mode using "xl setenforce".
+ * force or enforcing
+     This requires a security policy to be provided by the bootloader and will
+     enable enforcing prior to the creation of domain 0.  If a valid policy is
+     not provided, the hypervisor will not continue booting.
+ * late
+     This disabled loading of the security policy from the bootloader.  FLASK
+     will be enabled but will not enforce access controls until a policy is
+     loaded by a domain using "xl loadpolicy" or similar commands.  Once a
+     policy is loaded, FLASK will run in enforcing mode unless "xl setenforce"
+     has disabled this.
+ * disabled
+     This causes the XSM framework to revert to the dummy module.  The dummy
+     module provides the same security policy as is used when compiling the
+     hypervisor without support for XSM.  The xsm_op hypercall can be used to
+     switch to this mode after boot, but there is no way to re-enable FLASK
+     once the dummy module is loaded.
diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index 0e89360..8db9b1e 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -24,11 +24,12 @@
 #define _copy_to_guest copy_to_guest
 #define _copy_from_guest copy_from_guest
 
-int flask_enforcing = 0;
-integer_param("flask_enforcing", flask_enforcing);
+int __read_mostly flask_bootparam = FLASK_BOOTPARAM_DEFAULT;
+static void parse_flask_param(char *s);
+custom_param("flask", parse_flask_param);
 
-int flask_enabled = 1;
-integer_param("flask_enabled", flask_enabled);
+int __read_mostly flask_enforcing = 0;
+integer_param("flask_enforcing", flask_enforcing);
 
 #define MAX_POLICY_SIZE 0x4000000
 
@@ -60,6 +61,26 @@ extern int ss_initialized;
 
 extern struct xsm_operations *original_ops;
 
+static void __init parse_flask_param(char *s)
+{
+    if ( !strcmp(s, "force") || !strcmp(s, "enforcing") )
+    {
+        flask_enforcing = 1;
+        flask_bootparam = FLASK_BOOTPARAM_FORCE;
+    }
+    else if ( !strcmp(s, "late") )
+    {
+        flask_enforcing = 1;
+        flask_bootparam = FLASK_BOOTPARAM_LATELOAD;
+    }
+    else if ( !strcmp(s, "disabled") )
+        flask_bootparam = FLASK_BOOTPARAM_DISABLED;
+    else if ( !strcmp(s, "permissive") )
+        flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE;
+    else
+        panic("Invalid value specified for flask= boot parameter");
+}
+
 static int domain_has_security(struct domain *d, u32 perms)
 {
     struct domain_security_struct *dsec;
@@ -502,6 +523,7 @@ static int flask_security_load(struct xen_flask_load *load)
 {
     int ret;
     void *buf = NULL;
+    bool_t is_reload = ss_initialized;
 
     ret = domain_has_security(current->domain, SECURITY__LOAD_POLICY);
     if ( ret )
@@ -526,6 +548,10 @@ static int flask_security_load(struct xen_flask_load *load)
     if ( ret )
         goto out;
 
+    if ( !is_reload )
+        printk(XENLOG_INFO "Flask: Policy loaded, continuing in %s mode.\n",
+            flask_enforcing ? "enforcing" : "permissive");
+
     xfree(bool_pending_values);
     bool_pending_values = NULL;
     ret = 0;
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index dad5deb..00c8843 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1628,28 +1628,32 @@ static struct xsm_operations flask_ops = {
 
 static __init void flask_init(void)
 {
-    int ret = 0;
+    int ret = -ENOENT;
 
-    if ( !flask_enabled )
+    if ( flask_bootparam == FLASK_BOOTPARAM_DISABLED )
     {
-        printk("Flask:  Disabled at boot.\n");
+        printk(XENLOG_INFO "Flask:  Disabled at boot.\n");
         return;
     }
 
-    printk("Flask:  Initializing.\n");
-
     avc_init();
 
     original_ops = xsm_ops;
     if ( register_xsm(&flask_ops) )
         panic("Flask: Unable to register with XSM");
 
-    ret = security_load_policy(policy_buffer, policy_size);
+    if ( policy_size && flask_bootparam != FLASK_BOOTPARAM_LATELOAD )
+        ret = security_load_policy(policy_buffer, policy_size);
+
+    if ( ret && flask_bootparam == FLASK_BOOTPARAM_FORCE )
+        panic("Unable to load FLASK policy");
 
-    if ( flask_enforcing )
-        printk("Flask:  Starting in enforcing mode.\n");
+    if ( ret )
+        printk(XENLOG_INFO "Flask:  Access controls disabled until policy is loaded.\n");
+    else if ( flask_enforcing )
+        printk(XENLOG_INFO "Flask:  Starting in enforcing mode.\n");
     else
-        printk("Flask:  Starting in permissive mode.\n");
+        printk(XENLOG_INFO "Flask:  Starting in permissive mode.\n");
 }
 
 xsm_initcall(flask_init);
diff --git a/xen/xsm/flask/include/security.h b/xen/xsm/flask/include/security.h
index dea0143..bd2a4bc 100644
--- a/xen/xsm/flask/include/security.h
+++ b/xen/xsm/flask/include/security.h
@@ -35,7 +35,13 @@
 #define POLICYDB_VERSION_MIN   POLICYDB_VERSION_BASE
 #define POLICYDB_VERSION_MAX   POLICYDB_VERSION_BOUNDARY
 
-extern int flask_enabled;
+#define FLASK_BOOTPARAM_DEFAULT    0
+#define FLASK_BOOTPARAM_PERMISSIVE 0
+#define FLASK_BOOTPARAM_FORCE      1
+#define FLASK_BOOTPARAM_LATELOAD   2
+#define FLASK_BOOTPARAM_DISABLED   3
+
+extern int flask_bootparam;
 extern int flask_mls_enabled;
 
 int security_load_policy(void * data, size_t len);
-- 
2.1.0

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

* Re: [PATCH 2/2] flask: create unified "flask=" boot parameter
  2015-03-03 17:00 ` [PATCH 2/2] flask: create unified "flask=" boot parameter Daniel De Graaf
@ 2015-03-03 17:12   ` Jan Beulich
  2015-03-06 12:22   ` Wei Liu
  1 sibling, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2015-03-03 17:12 UTC (permalink / raw)
  To: Daniel De Graaf; +Cc: xen-devel, julien.grall, wei.liu2, ian.campbell

>>> On 03.03.15 at 18:00, <dgdegra@tycho.nsa.gov> wrote:
> --- a/docs/misc/xsm-flask.txt
> +++ b/docs/misc/xsm-flask.txt
> @@ -400,28 +400,26 @@ may require multiple passes to find all required ranges.
>  Additional notes on XSM:FLASK
>  -----------------------------
>  
> -1) xen command line parameters
> -
> -	a) flask_enforcing
> -	
> -	The default value for flask_enforcing is '0'.  This parameter causes the 
> -	platform to boot in permissive mode which means that the policy is loaded 
> -	but not enforced.  This mode is often helpful for developing new systems 
> -	and policies as the policy violations are reported on the xen console and 
> -	may be viewed in dom0 through 'xl dmesg'.
> -	
> -	To boot the platform into enforcing mode, which means that the policy is
> -	loaded and enforced, append 'flask_enforcing=1' on the grub line.
> -	
> -	This parameter may also be changed through the flask hypercall.
> -	
> -	b) flask_enabled
> -	
> -	The default value for flask_enabled is '1'.  This parameter causes the
> -	platform to enable the FLASK security module under the XSM framework.
> -	The parameter may be enabled/disabled only once per boot.  If the parameter
> -	is set to '0', only a reboot can re-enable flask.  When flask_enabled is '0'
> -	the DUMMY module is enforced.
> -
> -	This parameter may also be changed through the flask hypercall.  But may
> -	only be performed once per boot.
> +The xen command line accepts these values for the "flask=" parameter:
> +
> + * permissive [default]
> +     This is intended for development and is not suitable for use with untrusted
> +     guests.  If a policy is provided by the bootloader, it will be loaded;
> +     errors will be reported to the ring buffer but will not prevent booting.
> +     The policy can be changed to enforcing mode using "xl setenforce".
> + * force or enforcing
> +     This requires a security policy to be provided by the bootloader and will
> +     enable enforcing prior to the creation of domain 0.  If a valid policy is
> +     not provided, the hypervisor will not continue booting.
> + * late
> +     This disabled loading of the security policy from the bootloader.  FLASK
> +     will be enabled but will not enforce access controls until a policy is
> +     loaded by a domain using "xl loadpolicy" or similar commands.  Once a
> +     policy is loaded, FLASK will run in enforcing mode unless "xl setenforce"
> +     has disabled this.
> + * disabled
> +     This causes the XSM framework to revert to the dummy module.  The dummy
> +     module provides the same security policy as is used when compiling the
> +     hypervisor without support for XSM.  The xsm_op hypercall can be used to
> +     switch to this mode after boot, but there is no way to re-enable FLASK
> +     once the dummy module is loaded.

Rather than editing this here, I think this would better be moved into
xen-command-line.markdown. In any event you'll want to update that
file for the option rename.

Jan

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

* Re: [PATCH 2/2] flask: create unified "flask=" boot parameter
  2015-03-03 17:00 ` [PATCH 2/2] flask: create unified "flask=" boot parameter Daniel De Graaf
  2015-03-03 17:12   ` Jan Beulich
@ 2015-03-06 12:22   ` Wei Liu
  2015-03-06 18:53     ` Daniel De Graaf
  1 sibling, 1 reply; 10+ messages in thread
From: Wei Liu @ 2015-03-06 12:22 UTC (permalink / raw)
  To: Daniel De Graaf; +Cc: xen-devel, julien.grall, wei.liu2, ian.campbell

On Tue, Mar 03, 2015 at 12:00:19PM -0500, Daniel De Graaf wrote:
[...]
> diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> index 6b89ba8..48b8f98 100644
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -1441,8 +1441,8 @@ Determine if the FLASK security module is loaded and enforcing its policy.
>  =item B<setenforce> I<1|0|Enforcing|Permissive>
>  
>  Enable or disable enforcing of the FLASK access controls. The default is
> -permissive and can be changed using the flask_enforcing option on the
> -hypervisor's command line.
> +permissive, but this can be changed to enforcing by specifying "flask=enforcing"
> +or "flask=late" on the hypervisor's command line.
>  

This part looks good to me.

>  =item B<loadpolicy> I<policy-file>
>  
> diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
> index 9559028..efe8d50 100644
> --- a/docs/misc/xsm-flask.txt
> +++ b/docs/misc/xsm-flask.txt
> @@ -400,28 +400,26 @@ may require multiple passes to find all required ranges.
>  Additional notes on XSM:FLASK
>  -----------------------------
>  
> -1) xen command line parameters
> -
> -	a) flask_enforcing
> -	
> -	The default value for flask_enforcing is '0'.  This parameter causes the 
> -	platform to boot in permissive mode which means that the policy is loaded 
> -	but not enforced.  This mode is often helpful for developing new systems 
> -	and policies as the policy violations are reported on the xen console and 
> -	may be viewed in dom0 through 'xl dmesg'.
> -	
> -	To boot the platform into enforcing mode, which means that the policy is
> -	loaded and enforced, append 'flask_enforcing=1' on the grub line.
> -	
> -	This parameter may also be changed through the flask hypercall.
> -	
> -	b) flask_enabled
> -	
> -	The default value for flask_enabled is '1'.  This parameter causes the
> -	platform to enable the FLASK security module under the XSM framework.
> -	The parameter may be enabled/disabled only once per boot.  If the parameter
> -	is set to '0', only a reboot can re-enable flask.  When flask_enabled is '0'
> -	the DUMMY module is enforced.
> -
> -	This parameter may also be changed through the flask hypercall.  But may
> -	only be performed once per boot.
> +The xen command line accepts these values for the "flask=" parameter:
> +
> + * permissive [default]
> +     This is intended for development and is not suitable for use with untrusted
> +     guests.  If a policy is provided by the bootloader, it will be loaded;
> +     errors will be reported to the ring buffer but will not prevent booting.
> +     The policy can be changed to enforcing mode using "xl setenforce".
> + * force or enforcing
> +     This requires a security policy to be provided by the bootloader and will
> +     enable enforcing prior to the creation of domain 0.  If a valid policy is
> +     not provided, the hypervisor will not continue booting.
> + * late
> +     This disabled loading of the security policy from the bootloader.  FLASK
> +     will be enabled but will not enforce access controls until a policy is
> +     loaded by a domain using "xl loadpolicy" or similar commands.  Once a
> +     policy is loaded, FLASK will run in enforcing mode unless "xl setenforce"
> +     has disabled this.
> + * disabled
> +     This causes the XSM framework to revert to the dummy module.  The dummy
> +     module provides the same security policy as is used when compiling the
> +     hypervisor without support for XSM.  The xsm_op hypercall can be used to
> +     switch to this mode after boot, but there is no way to re-enable FLASK
> +     once the dummy module is loaded.
> diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
> index 0e89360..8db9b1e 100644
> --- a/xen/xsm/flask/flask_op.c
> +++ b/xen/xsm/flask/flask_op.c
> @@ -24,11 +24,12 @@
>  #define _copy_to_guest copy_to_guest
>  #define _copy_from_guest copy_from_guest
>  
> -int flask_enforcing = 0;
> -integer_param("flask_enforcing", flask_enforcing);
> +int __read_mostly flask_bootparam = FLASK_BOOTPARAM_DEFAULT;
> +static void parse_flask_param(char *s);
> +custom_param("flask", parse_flask_param);
>  
> -int flask_enabled = 1;
> -integer_param("flask_enabled", flask_enabled);

I am of the opinion that we need to support old syntax. I don't know if
anyone is actually using xsm given the status it is in, so my opinion is
not very strong.

Wei.

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

* Re: [PATCH 2/2] flask: create unified "flask=" boot parameter
  2015-03-06 12:22   ` Wei Liu
@ 2015-03-06 18:53     ` Daniel De Graaf
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel De Graaf @ 2015-03-06 18:53 UTC (permalink / raw)
  To: Wei Liu; +Cc: xen-devel, julien.grall, ian.campbell

On 03/06/2015 07:22 AM, Wei Liu wrote:
> On Tue, Mar 03, 2015 at 12:00:19PM -0500, Daniel De Graaf wrote:
> [...]
>> diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
>> index 6b89ba8..48b8f98 100644
>> --- a/docs/man/xl.pod.1
>> +++ b/docs/man/xl.pod.1
>> @@ -1441,8 +1441,8 @@ Determine if the FLASK security module is loaded and enforcing its policy.
>>   =item B<setenforce> I<1|0|Enforcing|Permissive>
>>
>>   Enable or disable enforcing of the FLASK access controls. The default is
>> -permissive and can be changed using the flask_enforcing option on the
>> -hypervisor's command line.
>> +permissive, but this can be changed to enforcing by specifying "flask=enforcing"
>> +or "flask=late" on the hypervisor's command line.
>>
>
> This part looks good to me.
>
>>   =item B<loadpolicy> I<policy-file>
>>
>> diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
>> index 9559028..efe8d50 100644
>> --- a/docs/misc/xsm-flask.txt
>> +++ b/docs/misc/xsm-flask.txt
>> @@ -400,28 +400,26 @@ may require multiple passes to find all required ranges.
>>   Additional notes on XSM:FLASK
>>   -----------------------------
>>
>> -1) xen command line parameters
>> -
>> -	a) flask_enforcing
>> -	
>> -	The default value for flask_enforcing is '0'.  This parameter causes the
>> -	platform to boot in permissive mode which means that the policy is loaded
>> -	but not enforced.  This mode is often helpful for developing new systems
>> -	and policies as the policy violations are reported on the xen console and
>> -	may be viewed in dom0 through 'xl dmesg'.
>> -	
>> -	To boot the platform into enforcing mode, which means that the policy is
>> -	loaded and enforced, append 'flask_enforcing=1' on the grub line.
>> -	
>> -	This parameter may also be changed through the flask hypercall.
>> -	
>> -	b) flask_enabled
>> -	
>> -	The default value for flask_enabled is '1'.  This parameter causes the
>> -	platform to enable the FLASK security module under the XSM framework.
>> -	The parameter may be enabled/disabled only once per boot.  If the parameter
>> -	is set to '0', only a reboot can re-enable flask.  When flask_enabled is '0'
>> -	the DUMMY module is enforced.
>> -
>> -	This parameter may also be changed through the flask hypercall.  But may
>> -	only be performed once per boot.
>> +The xen command line accepts these values for the "flask=" parameter:
>> +
>> + * permissive [default]
>> +     This is intended for development and is not suitable for use with untrusted
>> +     guests.  If a policy is provided by the bootloader, it will be loaded;
>> +     errors will be reported to the ring buffer but will not prevent booting.
>> +     The policy can be changed to enforcing mode using "xl setenforce".
>> + * force or enforcing
>> +     This requires a security policy to be provided by the bootloader and will
>> +     enable enforcing prior to the creation of domain 0.  If a valid policy is
>> +     not provided, the hypervisor will not continue booting.
>> + * late
>> +     This disabled loading of the security policy from the bootloader.  FLASK
>> +     will be enabled but will not enforce access controls until a policy is
>> +     loaded by a domain using "xl loadpolicy" or similar commands.  Once a
>> +     policy is loaded, FLASK will run in enforcing mode unless "xl setenforce"
>> +     has disabled this.
>> + * disabled
>> +     This causes the XSM framework to revert to the dummy module.  The dummy
>> +     module provides the same security policy as is used when compiling the
>> +     hypervisor without support for XSM.  The xsm_op hypercall can be used to
>> +     switch to this mode after boot, but there is no way to re-enable FLASK
>> +     once the dummy module is loaded.
>> diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
>> index 0e89360..8db9b1e 100644
>> --- a/xen/xsm/flask/flask_op.c
>> +++ b/xen/xsm/flask/flask_op.c
>> @@ -24,11 +24,12 @@
>>   #define _copy_to_guest copy_to_guest
>>   #define _copy_from_guest copy_from_guest
>>
>> -int flask_enforcing = 0;
>> -integer_param("flask_enforcing", flask_enforcing);
>> +int __read_mostly flask_bootparam = FLASK_BOOTPARAM_DEFAULT;
>> +static void parse_flask_param(char *s);
>> +custom_param("flask", parse_flask_param);
>>
>> -int flask_enabled = 1;
>> -integer_param("flask_enabled", flask_enabled);
>
> I am of the opinion that we need to support old syntax. I don't know if
> anyone is actually using xsm given the status it is in, so my opinion is
> not very strong.
>
> Wei.

This does support the old syntax for flask_enforcing, which is the more
important/used of the two options.  The flask_enabled option is only used
to disable XSM without recompiling the hypervisor; since enabling XSM is
not the default, I expect most people who do not use XSM will simply not
enable it at compile time.

It would not be hard to add another custom_param hook for flask_enabled
to support the old syntax, if anyone thinks it is needed.

-- 
Daniel De Graaf
National Security Agency

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

* [PATCH v4 0/2] flask: Handle policy load failures properly
@ 2015-03-06 21:43 Daniel De Graaf
  2015-03-06 21:43 ` [PATCH 1/2] flask: clean up initialization and #defines Daniel De Graaf
  2015-03-06 21:43 ` [PATCH 2/2] flask: create unified "flask=" boot parameter Daniel De Graaf
  0 siblings, 2 replies; 10+ messages in thread
From: Daniel De Graaf @ 2015-03-06 21:43 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, wei.liu2, ian.campbell

Chagnes from v3:
 - Moved documentation to xen-command-line.markdown

Changes from v2:
 - Add "flask=" parameter and split off cleanup patch

[PATCH 1/2] flask: clean up initialization and #defines
[PATCH 2/2] flask: create unified "flask=" boot parameter

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

* [PATCH 1/2] flask: clean up initialization and #defines
  2015-03-06 21:43 [PATCH v4 0/2] flask: Handle policy load failures properly Daniel De Graaf
@ 2015-03-06 21:43 ` Daniel De Graaf
  2015-03-09 11:00   ` Andrew Cooper
  2015-03-06 21:43 ` [PATCH 2/2] flask: create unified "flask=" boot parameter Daniel De Graaf
  1 sibling, 1 reply; 10+ messages in thread
From: Daniel De Graaf @ 2015-03-06 21:43 UTC (permalink / raw)
  To: xen-devel; +Cc: Daniel De Graaf, julien.grall, wei.liu2, ian.campbell

This removes the FLASK_DEVELOP and FLASK_BOOTPARAM configuration
parameters which have never been settable by users.  Disabling the
FLASK_DEVELOP configuration option has not produced a compiling
hypervisor for some time, and the FLASK_BOOTPARAM option will be
replaced with a more flexible boot parameter.

This also changes the return type of xsm_initcall_t to void to properly
reflect the fact that the caller ignores the return value.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/include/xen/config.h         | 4 ----
 xen/include/xsm/xsm.h            | 2 +-
 xen/xsm/flask/avc.c              | 2 --
 xen/xsm/flask/flask_op.c         | 4 ----
 xen/xsm/flask/hooks.c            | 6 ++----
 xen/xsm/flask/include/avc.h      | 4 ----
 xen/xsm/flask/include/security.h | 5 -----
 7 files changed, 3 insertions(+), 24 deletions(-)

diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
index 7bef8a6..d3d9911 100644
--- a/xen/include/xen/config.h
+++ b/xen/include/xen/config.h
@@ -100,10 +100,6 @@
 
 #ifdef FLASK_ENABLE
 #define XSM_MAGIC 0xf97cff8c
-/* Enable permissive mode (xl setenforce or flask_enforcing parameter) */
-#define FLASK_DEVELOP 1
-/* Allow runtime disabling of FLASK via the flask_enable parameter */
-#define FLASK_BOOTPARAM 1
 /* Maintain statistics on the access vector cache */
 #define FLASK_AVC_STATS 1
 #endif
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 4ce089f..0437735 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -42,7 +42,7 @@ typedef enum xsm_default xsm_default_t;
 extern char *policy_buffer;
 extern u32 policy_size;
 
-typedef int (*xsm_initcall_t)(void);
+typedef void (*xsm_initcall_t)(void);
 
 extern xsm_initcall_t __xsm_initcall_start[], __xsm_initcall_end[];
 
diff --git a/xen/xsm/flask/avc.c b/xen/xsm/flask/avc.c
index fc6580e..b1a4f8a 100644
--- a/xen/xsm/flask/avc.c
+++ b/xen/xsm/flask/avc.c
@@ -251,8 +251,6 @@ void __init avc_init(void)
     }
     atomic_set(&avc_cache.active_nodes, 0);
     atomic_set(&avc_cache.lru_hint, 0);
-
-    printk("AVC INITIALIZED\n");
 }
 
 int avc_get_hash_stats(struct xen_flask_hash_stats *arg)
diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index 7743aac..0e89360 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -24,15 +24,11 @@
 #define _copy_to_guest copy_to_guest
 #define _copy_from_guest copy_from_guest
 
-#ifdef FLASK_DEVELOP
 int flask_enforcing = 0;
 integer_param("flask_enforcing", flask_enforcing);
-#endif
 
-#ifdef FLASK_BOOTPARAM
 int flask_enabled = 1;
 integer_param("flask_enabled", flask_enabled);
-#endif
 
 #define MAX_POLICY_SIZE 0x4000000
 
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 65094bb..dad5deb 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1626,14 +1626,14 @@ static struct xsm_operations flask_ops = {
 #endif
 };
 
-static __init int flask_init(void)
+static __init void flask_init(void)
 {
     int ret = 0;
 
     if ( !flask_enabled )
     {
         printk("Flask:  Disabled at boot.\n");
-        return 0;
+        return;
     }
 
     printk("Flask:  Initializing.\n");
@@ -1650,8 +1650,6 @@ static __init int flask_init(void)
         printk("Flask:  Starting in enforcing mode.\n");
     else
         printk("Flask:  Starting in permissive mode.\n");
-
-    return ret;
 }
 
 xsm_initcall(flask_init);
diff --git a/xen/xsm/flask/include/avc.h b/xen/xsm/flask/include/avc.h
index 42a5e4b..a00a6eb 100644
--- a/xen/xsm/flask/include/avc.h
+++ b/xen/xsm/flask/include/avc.h
@@ -17,11 +17,7 @@
 #include "av_permissions.h"
 #include "security.h"
 
-#ifdef FLASK_DEVELOP
 extern int flask_enforcing;
-#else
-#define flask_enforcing 1
-#endif
 
 /*
  * An entry in the AVC.
diff --git a/xen/xsm/flask/include/security.h b/xen/xsm/flask/include/security.h
index 348f018..dea0143 100644
--- a/xen/xsm/flask/include/security.h
+++ b/xen/xsm/flask/include/security.h
@@ -35,12 +35,7 @@
 #define POLICYDB_VERSION_MIN   POLICYDB_VERSION_BASE
 #define POLICYDB_VERSION_MAX   POLICYDB_VERSION_BOUNDARY
 
-#ifdef FLASK_BOOTPARAM
 extern int flask_enabled;
-#else
-#define flask_enabled 1
-#endif
-
 extern int flask_mls_enabled;
 
 int security_load_policy(void * data, size_t len);
-- 
2.1.0

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

* [PATCH 2/2] flask: create unified "flask=" boot parameter
  2015-03-06 21:43 [PATCH v4 0/2] flask: Handle policy load failures properly Daniel De Graaf
  2015-03-06 21:43 ` [PATCH 1/2] flask: clean up initialization and #defines Daniel De Graaf
@ 2015-03-06 21:43 ` Daniel De Graaf
  2015-03-09 11:19   ` Andrew Cooper
  2015-03-09 12:25   ` Jan Beulich
  1 sibling, 2 replies; 10+ messages in thread
From: Daniel De Graaf @ 2015-03-06 21:43 UTC (permalink / raw)
  To: xen-devel; +Cc: Daniel De Graaf, julien.grall, wei.liu2, ian.campbell

This unifies the flask_enforcing and flask_enabled boot parameters into
a single parameter with additional states.  Defined options are:

 force - require policy to be loaded at boot time and enforce it
 late - bootloader policy is not used; later loadpolicy is enforcing
 permissive - a missing or broken policy does not panic
 disabled - revert to dummy (no XSM) policy.  Was flask_enabled=0

The default mode remains "permissive" and the flask_enforcing boot
parameter is retained for compatibility.  If flask_enforcing=1 is
specified and flask= is not, the bootloader policy will be loaded in
enforcing mode if present, but errors will disable access controls until
a successful loadpolicy instead of causing a panic at boot.

Suggested-by: Julien Grall <julien.grall@linaro.org>
Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 docs/man/xl.pod.1                   |  4 ++--
 docs/misc/xen-command-line.markdown | 30 +++++++++++++++++++++++++-----
 docs/misc/xsm-flask.txt             | 29 -----------------------------
 xen/xsm/flask/flask_op.c            | 34 ++++++++++++++++++++++++++++++----
 xen/xsm/flask/hooks.c               | 22 +++++++++++++---------
 xen/xsm/flask/include/security.h    |  8 +++++++-
 6 files changed, 77 insertions(+), 50 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 6b89ba8..48b8f98 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1441,8 +1441,8 @@ Determine if the FLASK security module is loaded and enforcing its policy.
 =item B<setenforce> I<1|0|Enforcing|Permissive>
 
 Enable or disable enforcing of the FLASK access controls. The default is
-permissive and can be changed using the flask_enforcing option on the
-hypervisor's command line.
+permissive, but this can be changed to enforcing by specifying "flask=enforcing"
+or "flask=late" on the hypervisor's command line.
 
 =item B<loadpolicy> I<policy-file>
 
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 9b458e1..505617b 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -626,11 +626,31 @@ hardware domain is architecture dependent.
 Note that specifying zero as domU value means zero, while for dom0 it means
 to use the default.
 
-### flask\_enabled
-> `= <integer>`
-
-### flask\_enforcing
-> `= <integer>`
+### flask
+> `= permissive | force | enforcing | late | disabled`
+
+> Default: `permissive`
+
+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
+by setting XSM\_ENABLE = y in .config).
+
+* `permissive`: This is intended for development and is not suitable for use
+  with untrusted guests.  If a policy is provided by the bootloader, it will be
+  loaded; errors will be reported to the ring buffer but will not prevent
+  booting.  The policy can be changed to enforcing mode using "xl setenforce".
+* `force` or `enforcing`: This requires a security policy to be provided by the
+  bootloader and will enter enforcing mode prior to the creation of domain 0.
+  If a valid policy is not provided, the hypervisor will not continue booting.
+* `late`: This disables loading of the security policy from the bootloader.
+  FLASK will be enabled but will not enforce access controls until a policy is
+  loaded by a domain using "xl loadpolicy".  Once a policy is loaded, FLASK will
+  run in enforcing mode unless "xl setenforce" has changed that setting.
+* `disabled`: This causes the XSM framework to revert to the dummy module.  The
+  dummy module provides the same security policy as is used when compiling the
+  hypervisor without support for XSM.  The xsm\_op hypercall can also be used to
+  switch to this mode after boot, but there is no way to re-enable FLASK once
+  the dummy module is loaded.
 
 ### font
 > `= <height>` where height is `8x8 | 8x14 | 8x16`
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 9559028..ab05913 100644
--- a/docs/misc/xsm-flask.txt
+++ b/docs/misc/xsm-flask.txt
@@ -396,32 +396,3 @@ the ranges being denied to more easily determine what resources are required.
 When running in permissive mode, only the first denial of a given
 source/destination is printed to the log, so labeling devices using this method
 may require multiple passes to find all required ranges.
-
-Additional notes on XSM:FLASK
------------------------------
-
-1) xen command line parameters
-
-	a) flask_enforcing
-	
-	The default value for flask_enforcing is '0'.  This parameter causes the 
-	platform to boot in permissive mode which means that the policy is loaded 
-	but not enforced.  This mode is often helpful for developing new systems 
-	and policies as the policy violations are reported on the xen console and 
-	may be viewed in dom0 through 'xl dmesg'.
-	
-	To boot the platform into enforcing mode, which means that the policy is
-	loaded and enforced, append 'flask_enforcing=1' on the grub line.
-	
-	This parameter may also be changed through the flask hypercall.
-	
-	b) flask_enabled
-	
-	The default value for flask_enabled is '1'.  This parameter causes the
-	platform to enable the FLASK security module under the XSM framework.
-	The parameter may be enabled/disabled only once per boot.  If the parameter
-	is set to '0', only a reboot can re-enable flask.  When flask_enabled is '0'
-	the DUMMY module is enforced.
-
-	This parameter may also be changed through the flask hypercall.  But may
-	only be performed once per boot.
diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index 0e89360..8db9b1e 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -24,11 +24,12 @@
 #define _copy_to_guest copy_to_guest
 #define _copy_from_guest copy_from_guest
 
-int flask_enforcing = 0;
-integer_param("flask_enforcing", flask_enforcing);
+int __read_mostly flask_bootparam = FLASK_BOOTPARAM_DEFAULT;
+static void parse_flask_param(char *s);
+custom_param("flask", parse_flask_param);
 
-int flask_enabled = 1;
-integer_param("flask_enabled", flask_enabled);
+int __read_mostly flask_enforcing = 0;
+integer_param("flask_enforcing", flask_enforcing);
 
 #define MAX_POLICY_SIZE 0x4000000
 
@@ -60,6 +61,26 @@ extern int ss_initialized;
 
 extern struct xsm_operations *original_ops;
 
+static void __init parse_flask_param(char *s)
+{
+    if ( !strcmp(s, "force") || !strcmp(s, "enforcing") )
+    {
+        flask_enforcing = 1;
+        flask_bootparam = FLASK_BOOTPARAM_FORCE;
+    }
+    else if ( !strcmp(s, "late") )
+    {
+        flask_enforcing = 1;
+        flask_bootparam = FLASK_BOOTPARAM_LATELOAD;
+    }
+    else if ( !strcmp(s, "disabled") )
+        flask_bootparam = FLASK_BOOTPARAM_DISABLED;
+    else if ( !strcmp(s, "permissive") )
+        flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE;
+    else
+        panic("Invalid value specified for flask= boot parameter");
+}
+
 static int domain_has_security(struct domain *d, u32 perms)
 {
     struct domain_security_struct *dsec;
@@ -502,6 +523,7 @@ static int flask_security_load(struct xen_flask_load *load)
 {
     int ret;
     void *buf = NULL;
+    bool_t is_reload = ss_initialized;
 
     ret = domain_has_security(current->domain, SECURITY__LOAD_POLICY);
     if ( ret )
@@ -526,6 +548,10 @@ static int flask_security_load(struct xen_flask_load *load)
     if ( ret )
         goto out;
 
+    if ( !is_reload )
+        printk(XENLOG_INFO "Flask: Policy loaded, continuing in %s mode.\n",
+            flask_enforcing ? "enforcing" : "permissive");
+
     xfree(bool_pending_values);
     bool_pending_values = NULL;
     ret = 0;
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index dad5deb..00c8843 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1628,28 +1628,32 @@ static struct xsm_operations flask_ops = {
 
 static __init void flask_init(void)
 {
-    int ret = 0;
+    int ret = -ENOENT;
 
-    if ( !flask_enabled )
+    if ( flask_bootparam == FLASK_BOOTPARAM_DISABLED )
     {
-        printk("Flask:  Disabled at boot.\n");
+        printk(XENLOG_INFO "Flask:  Disabled at boot.\n");
         return;
     }
 
-    printk("Flask:  Initializing.\n");
-
     avc_init();
 
     original_ops = xsm_ops;
     if ( register_xsm(&flask_ops) )
         panic("Flask: Unable to register with XSM");
 
-    ret = security_load_policy(policy_buffer, policy_size);
+    if ( policy_size && flask_bootparam != FLASK_BOOTPARAM_LATELOAD )
+        ret = security_load_policy(policy_buffer, policy_size);
+
+    if ( ret && flask_bootparam == FLASK_BOOTPARAM_FORCE )
+        panic("Unable to load FLASK policy");
 
-    if ( flask_enforcing )
-        printk("Flask:  Starting in enforcing mode.\n");
+    if ( ret )
+        printk(XENLOG_INFO "Flask:  Access controls disabled until policy is loaded.\n");
+    else if ( flask_enforcing )
+        printk(XENLOG_INFO "Flask:  Starting in enforcing mode.\n");
     else
-        printk("Flask:  Starting in permissive mode.\n");
+        printk(XENLOG_INFO "Flask:  Starting in permissive mode.\n");
 }
 
 xsm_initcall(flask_init);
diff --git a/xen/xsm/flask/include/security.h b/xen/xsm/flask/include/security.h
index dea0143..bd2a4bc 100644
--- a/xen/xsm/flask/include/security.h
+++ b/xen/xsm/flask/include/security.h
@@ -35,7 +35,13 @@
 #define POLICYDB_VERSION_MIN   POLICYDB_VERSION_BASE
 #define POLICYDB_VERSION_MAX   POLICYDB_VERSION_BOUNDARY
 
-extern int flask_enabled;
+#define FLASK_BOOTPARAM_DEFAULT    0
+#define FLASK_BOOTPARAM_PERMISSIVE 0
+#define FLASK_BOOTPARAM_FORCE      1
+#define FLASK_BOOTPARAM_LATELOAD   2
+#define FLASK_BOOTPARAM_DISABLED   3
+
+extern int flask_bootparam;
 extern int flask_mls_enabled;
 
 int security_load_policy(void * data, size_t len);
-- 
2.1.0

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

* Re: [PATCH 1/2] flask: clean up initialization and #defines
  2015-03-06 21:43 ` [PATCH 1/2] flask: clean up initialization and #defines Daniel De Graaf
@ 2015-03-09 11:00   ` Andrew Cooper
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Cooper @ 2015-03-09 11:00 UTC (permalink / raw)
  To: Daniel De Graaf, xen-devel; +Cc: julien.grall, wei.liu2, ian.campbell

On 06/03/15 21:43, Daniel De Graaf wrote:
> This removes the FLASK_DEVELOP and FLASK_BOOTPARAM configuration
> parameters which have never been settable by users.  Disabling the
> FLASK_DEVELOP configuration option has not produced a compiling
> hypervisor for some time, and the FLASK_BOOTPARAM option will be
> replaced with a more flexible boot parameter.
>
> This also changes the return type of xsm_initcall_t to void to properly
> reflect the fact that the caller ignores the return value.
>
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>

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

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

* Re: [PATCH 2/2] flask: create unified "flask=" boot parameter
  2015-03-06 21:43 ` [PATCH 2/2] flask: create unified "flask=" boot parameter Daniel De Graaf
@ 2015-03-09 11:19   ` Andrew Cooper
  2015-03-09 12:25   ` Jan Beulich
  1 sibling, 0 replies; 10+ messages in thread
From: Andrew Cooper @ 2015-03-09 11:19 UTC (permalink / raw)
  To: Daniel De Graaf, xen-devel; +Cc: julien.grall, wei.liu2, ian.campbell

On 06/03/15 21:43, Daniel De Graaf wrote:
> This unifies the flask_enforcing and flask_enabled boot parameters into
> a single parameter with additional states.  Defined options are:
>
>  force - require policy to be loaded at boot time and enforce it
>  late - bootloader policy is not used; later loadpolicy is enforcing
>  permissive - a missing or broken policy does not panic
>  disabled - revert to dummy (no XSM) policy.  Was flask_enabled=0
>
> The default mode remains "permissive" and the flask_enforcing boot
> parameter is retained for compatibility.  If flask_enforcing=1 is
> specified and flask= is not, the bootloader policy will be loaded in
> enforcing mode if present, but errors will disable access controls until
> a successful loadpolicy instead of causing a panic at boot.
>
> Suggested-by: Julien Grall <julien.grall@linaro.org>
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  docs/man/xl.pod.1                   |  4 ++--
>  docs/misc/xen-command-line.markdown | 30 +++++++++++++++++++++++++-----
>  docs/misc/xsm-flask.txt             | 29 -----------------------------
>  xen/xsm/flask/flask_op.c            | 34 ++++++++++++++++++++++++++++++----
>  xen/xsm/flask/hooks.c               | 22 +++++++++++++---------
>  xen/xsm/flask/include/security.h    |  8 +++++++-
>  6 files changed, 77 insertions(+), 50 deletions(-)
>
> diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> index 6b89ba8..48b8f98 100644
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -1441,8 +1441,8 @@ Determine if the FLASK security module is loaded and enforcing its policy.
>  =item B<setenforce> I<1|0|Enforcing|Permissive>
>  
>  Enable or disable enforcing of the FLASK access controls. The default is
> -permissive and can be changed using the flask_enforcing option on the
> -hypervisor's command line.
> +permissive, but this can be changed to enforcing by specifying "flask=enforcing"
> +or "flask=late" on the hypervisor's command line.
>  
>  =item B<loadpolicy> I<policy-file>
>  
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index 9b458e1..505617b 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -626,11 +626,31 @@ hardware domain is architecture dependent.
>  Note that specifying zero as domU value means zero, while for dom0 it means
>  to use the default.
>  
> -### flask\_enabled
> -> `= <integer>`
> -
> -### flask\_enforcing
> -> `= <integer>`
> +### flask
> +> `= permissive | force | enforcing | late | disabled`
> +
> +> Default: `permissive`
> +
> +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
> +by setting XSM\_ENABLE = y in .config).
> +
> +* `permissive`: This is intended for development and is not suitable for use
> +  with untrusted guests.  If a policy is provided by the bootloader, it will be
> +  loaded; errors will be reported to the ring buffer but will not prevent
> +  booting.  The policy can be changed to enforcing mode using "xl setenforce".
> +* `force` or `enforcing`: This requires a security policy to be provided by the
> +  bootloader and will enter enforcing mode prior to the creation of domain 0.
> +  If a valid policy is not provided, the hypervisor will not continue booting.
> +* `late`: This disables loading of the security policy from the bootloader.
> +  FLASK will be enabled but will not enforce access controls until a policy is
> +  loaded by a domain using "xl loadpolicy".  Once a policy is loaded, FLASK will
> +  run in enforcing mode unless "xl setenforce" has changed that setting.
> +* `disabled`: This causes the XSM framework to revert to the dummy module.  The
> +  dummy module provides the same security policy as is used when compiling the
> +  hypervisor without support for XSM.  The xsm\_op hypercall can also be used to
> +  switch to this mode after boot, but there is no way to re-enable FLASK once
> +  the dummy module is loaded.
>  
>  ### font
>  > `= <height>` where height is `8x8 | 8x14 | 8x16`
> diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
> index 9559028..ab05913 100644
> --- a/docs/misc/xsm-flask.txt
> +++ b/docs/misc/xsm-flask.txt
> @@ -396,32 +396,3 @@ the ranges being denied to more easily determine what resources are required.
>  When running in permissive mode, only the first denial of a given
>  source/destination is printed to the log, so labeling devices using this method
>  may require multiple passes to find all required ranges.
> -
> -Additional notes on XSM:FLASK
> ------------------------------
> -
> -1) xen command line parameters
> -
> -	a) flask_enforcing
> -	
> -	The default value for flask_enforcing is '0'.  This parameter causes the 
> -	platform to boot in permissive mode which means that the policy is loaded 
> -	but not enforced.  This mode is often helpful for developing new systems 
> -	and policies as the policy violations are reported on the xen console and 
> -	may be viewed in dom0 through 'xl dmesg'.
> -	
> -	To boot the platform into enforcing mode, which means that the policy is
> -	loaded and enforced, append 'flask_enforcing=1' on the grub line.
> -	
> -	This parameter may also be changed through the flask hypercall.
> -	
> -	b) flask_enabled
> -	
> -	The default value for flask_enabled is '1'.  This parameter causes the
> -	platform to enable the FLASK security module under the XSM framework.
> -	The parameter may be enabled/disabled only once per boot.  If the parameter
> -	is set to '0', only a reboot can re-enable flask.  When flask_enabled is '0'
> -	the DUMMY module is enforced.
> -
> -	This parameter may also be changed through the flask hypercall.  But may
> -	only be performed once per boot.
> diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
> index 0e89360..8db9b1e 100644
> --- a/xen/xsm/flask/flask_op.c
> +++ b/xen/xsm/flask/flask_op.c
> @@ -24,11 +24,12 @@
>  #define _copy_to_guest copy_to_guest
>  #define _copy_from_guest copy_from_guest
>  
> -int flask_enforcing = 0;
> -integer_param("flask_enforcing", flask_enforcing);
> +int __read_mostly flask_bootparam = FLASK_BOOTPARAM_DEFAULT;

It would be cleaner for this to be an enum.  The exact numeric values
are not interesting.

> +static void parse_flask_param(char *s);
> +custom_param("flask", parse_flask_param);
>  
> -int flask_enabled = 1;
> -integer_param("flask_enabled", flask_enabled);
> +int __read_mostly flask_enforcing = 0;
> +integer_param("flask_enforcing", flask_enforcing);

It would also be cleaner for flask_enforcing to become a bool_t, seeing
as you area already changing it.

>  
>  #define MAX_POLICY_SIZE 0x4000000
>  
> @@ -60,6 +61,26 @@ extern int ss_initialized;
>  
>  extern struct xsm_operations *original_ops;
>  
> +static void __init parse_flask_param(char *s)
> +{
> +    if ( !strcmp(s, "force") || !strcmp(s, "enforcing") )
> +    {
> +        flask_enforcing = 1;
> +        flask_bootparam = FLASK_BOOTPARAM_FORCE;
> +    }
> +    else if ( !strcmp(s, "late") )
> +    {
> +        flask_enforcing = 1;
> +        flask_bootparam = FLASK_BOOTPARAM_LATELOAD;
> +    }
> +    else if ( !strcmp(s, "disabled") )
> +        flask_bootparam = FLASK_BOOTPARAM_DISABLED;
> +    else if ( !strcmp(s, "permissive") )
> +        flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE;
> +    else
> +        panic("Invalid value specified for flask= boot parameter");

This panic is not useful.  Command line parameters are parsed before the
console is started, which means that the message will not find its way
anywhere useful.

It would be better to have a sentinal unknown value and possibly
panicking in flask_init(), which is much later in boot after the APs
have been brought up.

~Andrew

> +}
> +
>  static int domain_has_security(struct domain *d, u32 perms)
>  {
>      struct domain_security_struct *dsec;
> @@ -502,6 +523,7 @@ static int flask_security_load(struct xen_flask_load *load)
>  {
>      int ret;
>      void *buf = NULL;
> +    bool_t is_reload = ss_initialized;
>  
>      ret = domain_has_security(current->domain, SECURITY__LOAD_POLICY);
>      if ( ret )
> @@ -526,6 +548,10 @@ static int flask_security_load(struct xen_flask_load *load)
>      if ( ret )
>          goto out;
>  
> +    if ( !is_reload )
> +        printk(XENLOG_INFO "Flask: Policy loaded, continuing in %s mode.\n",
> +            flask_enforcing ? "enforcing" : "permissive");
> +
>      xfree(bool_pending_values);
>      bool_pending_values = NULL;
>      ret = 0;
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index dad5deb..00c8843 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1628,28 +1628,32 @@ static struct xsm_operations flask_ops = {
>  
>  static __init void flask_init(void)
>  {
> -    int ret = 0;
> +    int ret = -ENOENT;
>  
> -    if ( !flask_enabled )
> +    if ( flask_bootparam == FLASK_BOOTPARAM_DISABLED )
>      {
> -        printk("Flask:  Disabled at boot.\n");
> +        printk(XENLOG_INFO "Flask:  Disabled at boot.\n");
>          return;
>      }
>  
> -    printk("Flask:  Initializing.\n");
> -
>      avc_init();
>  
>      original_ops = xsm_ops;
>      if ( register_xsm(&flask_ops) )
>          panic("Flask: Unable to register with XSM");
>  
> -    ret = security_load_policy(policy_buffer, policy_size);
> +    if ( policy_size && flask_bootparam != FLASK_BOOTPARAM_LATELOAD )
> +        ret = security_load_policy(policy_buffer, policy_size);
> +
> +    if ( ret && flask_bootparam == FLASK_BOOTPARAM_FORCE )
> +        panic("Unable to load FLASK policy");
>  
> -    if ( flask_enforcing )
> -        printk("Flask:  Starting in enforcing mode.\n");
> +    if ( ret )
> +        printk(XENLOG_INFO "Flask:  Access controls disabled until policy is loaded.\n");
> +    else if ( flask_enforcing )
> +        printk(XENLOG_INFO "Flask:  Starting in enforcing mode.\n");
>      else
> -        printk("Flask:  Starting in permissive mode.\n");
> +        printk(XENLOG_INFO "Flask:  Starting in permissive mode.\n");
>  }
>  
>  xsm_initcall(flask_init);
> diff --git a/xen/xsm/flask/include/security.h b/xen/xsm/flask/include/security.h
> index dea0143..bd2a4bc 100644
> --- a/xen/xsm/flask/include/security.h
> +++ b/xen/xsm/flask/include/security.h
> @@ -35,7 +35,13 @@
>  #define POLICYDB_VERSION_MIN   POLICYDB_VERSION_BASE
>  #define POLICYDB_VERSION_MAX   POLICYDB_VERSION_BOUNDARY
>  
> -extern int flask_enabled;
> +#define FLASK_BOOTPARAM_DEFAULT    0
> +#define FLASK_BOOTPARAM_PERMISSIVE 0
> +#define FLASK_BOOTPARAM_FORCE      1
> +#define FLASK_BOOTPARAM_LATELOAD   2
> +#define FLASK_BOOTPARAM_DISABLED   3
> +
> +extern int flask_bootparam;
>  extern int flask_mls_enabled;
>  
>  int security_load_policy(void * data, size_t len);

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

* Re: [PATCH 2/2] flask: create unified "flask=" boot parameter
  2015-03-06 21:43 ` [PATCH 2/2] flask: create unified "flask=" boot parameter Daniel De Graaf
  2015-03-09 11:19   ` Andrew Cooper
@ 2015-03-09 12:25   ` Jan Beulich
  1 sibling, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2015-03-09 12:25 UTC (permalink / raw)
  To: Daniel De Graaf; +Cc: xen-devel, julien.grall, wei.liu2, ian.campbell

>>> On 06.03.15 at 22:43, <dgdegra@tycho.nsa.gov> wrote:
> @@ -60,6 +61,26 @@ extern int ss_initialized;
>  
>  extern struct xsm_operations *original_ops;
>  
> +static void __init parse_flask_param(char *s)
> +{
> +    if ( !strcmp(s, "force") || !strcmp(s, "enforcing") )

So what's the point of allowing two values with identical meaning here?

Jan

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

end of thread, other threads:[~2015-03-09 12:26 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-06 21:43 [PATCH v4 0/2] flask: Handle policy load failures properly Daniel De Graaf
2015-03-06 21:43 ` [PATCH 1/2] flask: clean up initialization and #defines Daniel De Graaf
2015-03-09 11:00   ` Andrew Cooper
2015-03-06 21:43 ` [PATCH 2/2] flask: create unified "flask=" boot parameter Daniel De Graaf
2015-03-09 11:19   ` Andrew Cooper
2015-03-09 12:25   ` Jan Beulich
  -- strict thread matches above, loose matches on Subject: below --
2015-03-03 17:00 [PATCH v3 0/2] flask: Handle policy load failures properly Daniel De Graaf
2015-03-03 17:00 ` [PATCH 2/2] flask: create unified "flask=" boot parameter Daniel De Graaf
2015-03-03 17:12   ` Jan Beulich
2015-03-06 12:22   ` Wei Liu
2015-03-06 18:53     ` Daniel De Graaf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.