Re: [Xen-devel] [PATCH] XSM: add Kconfig option to override bootloader provided policy

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 11/28/2017 07:04 PM, Tamas K Lengyel wrote:
> On Tue, Nov 28, 2017 at 12:00 PM, Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 28/11/17 18:06, Tamas K Lengyel wrote:
>>> From: Tamas K Lengyel <lengyelt@xxxxxxxxxxxx>
>>>
>>> Currently the built-in XSM policy only gets used if there is no other policy
>>> specified during boot. In this patch we add a Kconfig option to specify to 
>>> only
>>> use built-in policy during boot. This is particularly important when booting
>>> Xen through the shim to ensure the XSM policy gets measured and that it 
>>> can't
>>> be replaced by another unmeasured policy by the bootloader. Note that the 
>>> XSM
>>> policy can still be updated after boot (from dom0 for example) if the 
>>> built-in
>>> policy allows it.
>>>
>>> Signed-off-by: Tamas K Lengyel <lengyelt@xxxxxxxxxxxx>
>>> ---
>>> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
>>> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
>>> Cc: Jan Beulich <jbeulich@xxxxxxxx>
>>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>>> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>> Cc: Tim Deegan <tim@xxxxxxx>
>>> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
>>> Cc: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
>>> Cc: openxt@xxxxxxxxxxxxxxxx
>>> ---
>>>  xen/common/Kconfig | 14 ++++++++++++++
>>>  xen/xsm/xsm_core.c |  2 ++
>>>  2 files changed, 16 insertions(+)
>>>
>>> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
>>> index 103ef44cb5..5ad0d03f37 100644
>>> --- a/xen/common/Kconfig
>>> +++ b/xen/common/Kconfig
>>> @@ -140,6 +140,20 @@ config XSM_POLICY
>>>
>>>         If unsure, say Y.
>>>
>>> +config XSM_POLICY_OVERRIDE
>>> +     bool "Built-in security policy overrides bootloader provided policy"
>>
>> The overall change certainly looks good and it is obvious why it is a
>> benefit.  However, text/functionality like this is cognitively hard to
>> follow, and _OVERRIDE isn't obviously as to its functionality at a glance.
>>
>> Wouldn't it be better to have XSM_BOOTLOADER_POLICY (or possibly
>> XSM_ALLOW_?), which defaults to y, and can be forced off for extra security?
>>
> 
> I'm certainly open to alternate naming suggestions. The current one is
> based on an existing option that implements a similar feature with
> this naming (CMDLINE_OVERRIDE), while the XSM_POLICY part is from the
> existing XSM_POLICY option.

I agree with Andy.  I think CMDLINE_OVERRIDE is either mis-implemented
or mis-named: The real way to have your built-in "commandline"
*override* the bootloader-supplied one would be to have it parsed
second.  As it is, you're not *overriding* it, you're just *ignoring*
it, which is not the same.

I think XSM_ALLOW_BOOTLOADER_POLICY is probably a better name.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel