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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Wed, Nov 29, 2017 at 5:29 AM, George Dunlap <george.dunlap@xxxxxxxxxx> wrote:
> 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.
>

SGTM

Tamas

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