Re: [PATCH 2/2] flask: create unified "flask=" boot parameter

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

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