Re: [PATCH v6 09/10] kconfig: update xsm config to reflect reality

["Daniel P. Smith" <dpsmith@apertussolutions.com>]

On 9/17/21 8:09 AM, Jan Beulich wrote:
> On 10.09.2021 22:13, Daniel P. Smith wrote:
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -200,23 +200,20 @@ config XENOPROF
>>  
>>  	  If unsure, say Y.
>>  
>> -config XSM
>> -	bool "Xen Security Modules support"
>> +config XSM_CONFIGURABLE
>> +	bool "Configure Xen Security Modules"
>>  	default ARM
>> -	---help---
>> -	  Enables the security framework known as Xen Security Modules which
>> -	  allows administrators fine-grained control over a Xen domain and
>> -	  its capabilities by defining permissible interactions between domains,
>> -	  the hypervisor itself, and related resources such as memory and
>> -	  devices.
>> +	help
>> +	  Allows for configuring the Xen Security Modules (XSM) policy or policies
>> +	  modules that will be availble and which will be the default.
>>  
>>  	  If unsure, say N.
>>  
>>  config XSM_FLASK
>> -	def_bool y
>> -	prompt "FLux Advanced Security Kernel support"
>> -	depends on XSM
>> -	---help---
>> +	bool "FLux Advanced Security Kernel support"
>> +	depends on XSM_CONFIGURABLE
>> +	select XSM_EVTCHN_LABELING
>> +	help
>>  	  Enables FLASK (FLux Advanced Security Kernel) as the access control
>>  	  mechanism used by the XSM framework.  This provides a mandatory access
>>  	  control framework by which security enforcement, isolation, and
> 
> I continue to think that the default here and ...
> 
>> @@ -253,10 +250,10 @@ config XSM_FLASK_POLICY
>>  	  If unsure, say Y.
>>  
>>  config XSM_SILO
>> -	def_bool y
>> -	prompt "SILO support"
>> -	depends on XSM
>> -	---help---
>> +	bool "SILO support"
>> +	default y if ARM
>> +	depends on XSM_CONFIGURABLE
>> +	help
>>  	  Enables SILO as the access control mechanism used by the XSM framework.
>>  	  This is not the default module, add boot parameter xsm=silo to choose
>>  	  it. This will deny any unmediated communication channels (grant tables
> 
> ... here should not change. If it changes, the change needs justifying
> in the description.

IMHO the configure system should not presumptuously assume that if a
user selects XSM_CONFIGURABLE that the Flask module, which is not
currently security supported, should be enabled. I would agree that we
could turn on the SILO module since it is security supported. Later when
I am able to get Flask into a security supported state, I would be all
for enabling it as well. A more roadmap-ish idea is to see SILO subsumed
as a select-able policy under Flask, but that is a bit of a digression.

I will add to the commit message to clarify this position that is being
encoded.

>> @@ -282,15 +279,15 @@ endchoice
>>  config LATE_HWDOM
>>  	bool "Dedicated hardware domain"
>>  	default n
>> -	depends on XSM && X86
>> -	---help---
>> +	depends on XSM_FLASK && X86
>> +	help
>>  	  Allows the creation of a dedicated hardware domain distinct from
>>  	  domain 0 that manages devices without needing access to other
>>  	  privileged functionality such as the ability to manage domains.
>>  	  This requires that the actual domain 0 be a stub domain that
>>  	  constructs the actual hardware domain instead of initializing the
>>  	  hardware itself.  Because the hardware domain needs access to
>> -	  hypercalls not available to unprivileged guests, an XSM policy
>> +	  hypercalls not available to unprivileged guests, an XSM Flask policy
>>  	  is required to properly define the privilege of these domains.
> 
> I also continue to think that this would better be a separate change.
> Or if not, the seemingly unrelated change needs mentioning in the
> description (to make clear it's not a stray change).

Apologies that I missed the suggestion to break this out. Since this
really is more of a general clean-up over being part of the intended
improvement for this patch set, I will break it out and move it to the
front of the patch set.

v/r,
dps