Re: [PATCH v2 06/10] xsm: enable xsm to always be included

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.07.2021 22:47, Daniel P. Smith wrote:
> On 7/19/21 6:24 AM, Jan Beulich wrote:
>> On 12.07.2021 22:32, 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"
>>> -   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"
>>
>> I remain unconvinced of the removal of this top level option. I don't
>> view my concern on code size / performance as "unreasonable" (as Andrew
>> did call it) when comparing with the current !XSM configuration, and I
>> also remain to be convinced of people who had to simply answer 'n' to
>> the XSM kconfig prompt being happy to make an informed decision for all
>> the (previously sub-)options that they will now be prompted for. As
>> said before - it is one thing to re-work what exactly !XSM means
>> internally (and the conversion away from inline functions may very well
>> be a win; we simply don't know without you showing e.g. bloat-o-meter
>> like data). It is another to require in-depth knowledge to configure
>> Xen that previously wasn't required.
> 
> For me personally a concern about code size / performance itself is not
> "unreasonable" but I would say it is a bit disingenuous to use it to
> defend a position that the security framework should be special
> conditioned and kept convoluted considering, 1) other subsystems, e.g.
> iommu, do not appear to me to have the same subjugation requiring a
> special case of one hook set over another, 2) the architecture (Arm)
> which IMHO has the greatest concern over code size / performance is also
> the architecture with the only security supported configuration that
> requires an XSM enabled configuration, 3) this approach effectively
> requires the maintaining of two sets of hook handlers which increases
> work and risk for vulnerability introduction, and 4) based on the
> discussions at the Developers Summit, no one seemed to be aware that the
> only logical difference between !XSM and XSM was the invocation
> mechanism, inline vs call-site, let alone that XSM_HOOK represented no
> control check.
> 
> To bring context to the discussion, after applying the clean up patches
> (everything up to the patch dropping !XSM) of the patch set, the
> bloat-o-meter shows a 0.18% increase going from !XSM to XSM (without
> SILO and Flask). IMHO this increase does not justify keeping the
> convoluted gyrations being done to swap in an optimized security hook
> when no other security module is enabled. In fact we should be working
> to make the security framework clear and concise. I am all for
> maximizing performance while doing so but the end goal is for it to be
> clear and concise so that it can be reasoned about by everybody.

Well, I guess if the majority is with you then that's what is going to
happen.

> As to your position that this increases configuration complexity, I
> would have to strongly disagree. I have worked to ensure no new
> configuration steps are necessary.

I'm having a hard time seeing how this could be the case, especially
since ...

> The default config will only have XSM
> and the default/dummy policy enabled unless on Arm which will enable
> SILO and make it the default policy. Prior to this if XSM was enabled,
> the default policy was forced to Flask. While I am an advocate for
> Flask, I do not agree this is a reasonable configuration. It now works
> such that,
>     - if you enable only SILO, it is set as the default
>     - if you enable only Flask, it is set as the default
>     - if you enable both SILO & Flask, it uses SILO as the default
> Which basically works such that if one selects a policy, then it assumes
> that policy is desired to be used, and when more than one is selected it
> will default to the one that functions most like classic Dom0 model.

... I don't see how putting in place suitable defaults would suppress
any respective prompts during e.g. "make syncconfig". (I can see that
what you say may apply to e.g. "make menuconfig", which I think I've
indicated before I don't use myself and hence I don't care all that
much about.)

> IMHO this is much more intuitive. Now one alternative I am thinking
> about that might be a bit of a compromise is to move the default policy
> selection up to the same level as XSM menu. That would make it so one
> could immediately see what the default policy is but must go into the
> XSM menu if they want to alter what policy modules are available.

All of this appears to support my assumption that you're looking at
things from a "make menuconfig"-centric viewpoint.

Jan