Re: [PATCH 3/6] xsm: enabling xsm to always be included

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 6/18/21 7:53 AM, Andrew Cooper wrote:
> On 18/06/2021 00:39, Daniel P. Smith wrote:
>> The only difference between !CONFIG_XSM and CONFIG_XSM with !CONFIG_XSM_SILO 
>> and !CONFIG_XSM_FLASK
>> is whether the XSM hooks in dummy.h are called as static inline functions or 
>> as function
>> pointers to static functions. As such this commit,
>>  * eliminates CONFIG_XSM
>>  * introduces CONFIG_XSM_EVTCHN_LABELING as replacement for enabling event 
>> channel labels
>>  * makes CONFIG_XSM_SILO AND CONFIG_XSM_FLASK default to no
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
>> ---
>>  xen/common/Kconfig            |  55 ++++-----
>>  xen/include/xen/sched.h       |   2 +-
>>  xen/include/xsm/xsm-core.h    |  26 ----
>>  xen/include/xsm/xsm.h         |   8 --
>>  xen/xsm/Makefile              |   4 +-
>>  xen/xsm/dummy.c               |   4 +-
>>  xen/{include => }/xsm/dummy.h | 220 ++++++++++++++++------------------
>>  xen/xsm/silo.c                |  17 +--
>>  xen/xsm/xsm_core.c            |   4 -
>>  9 files changed, 142 insertions(+), 198 deletions(-)
>>  rename xen/{include => }/xsm/dummy.h (63%)
>>
>> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
>> index 0ddd18e11a..203ad7ea23 100644
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -197,22 +197,33 @@ config XENOPROF
>>  
>>        If unsure, say Y.
>>  
>> -config XSM
>> -    bool "Xen Security Modules support"
>> -    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.
>> +menu "Xen Security Modules"
>>  
>> -      If unsure, say N.
>> +choice
>> +    prompt "Default XSM module"
>> +    default XSM_SILO_DEFAULT if XSM_SILO && ARM
>> +    default XSM_FLASK_DEFAULT if XSM_FLASK
>> +    default XSM_SILO_DEFAULT if XSM_SILO
>> +    default XSM_DUMMY_DEFAULT
>> +    config XSM_DUMMY_DEFAULT
>> +            bool "Match non-XSM behavior"
> 
> There is no non-XSM behaviour any more.
> 
> Is it time to rename Dummy to "traditional dom0-all-powerful" or

Well, I left as dummy since that is what it has been known by thus far
and additionally the subsequent patch set was going to rename this to
XSM_ROLES/"XSM Role-based Access Control" For the intermediate time, I
can change the wording to reflect the correct state.

>> +    config XSM_FLASK_DEFAULT
>> +            bool "FLux Advanced Security Kernel" if XSM_FLASK
>> +    config XSM_SILO_DEFAULT
>> +            bool "SILO" if XSM_SILO
>> +endchoice
>> +
>> +config XSM_EVTCHN_LABELING
>> +    bool "Enables security labeling of event channels"
>> +    default n
>> +    ---help---
>> +      This enables an XSM module to label and enforce access control over
>> +      event channels.
> 
> Please use help rather than ---help--- for new options (its changed in
> upstream Kconfig).  The indentation of the help message wants to be one
> tab, then two spaces.  (Yes, sadly...)

ack

>>  config XSM_FLASK
>> -    def_bool y
>> +    def_bool n
>>      prompt "FLux Advanced Security Kernel support"
>> -    depends on XSM
>> +    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
>> @@ -250,9 +261,8 @@ config XSM_FLASK_POLICY
>>        If unsure, say Y.
>>  
>>  config XSM_SILO
>> -    def_bool y
>> +    def_bool n
> 
> I'm not sure we want to alter the FLASK/SILO defaults.  SILO in
> particular is mandatory on ARM, and without it, you're in a security
> unsupported configuration.
The intent here is the default is the classic dom0 configuration. What
if I did,

def bool n
def bool y if ARM

v/r
dps