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

[Jan Beulich]

>>> On 29.11.17 at 16:33,  wrote:
> On Wed, Nov 29, 2017 at 1:19 AM, Jan Beulich  wrote:
> On 28.11.17 at 19:06,  wrote:
>>> --- 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"
>>> + default n
>>
>> This is pointless.
> 
> Care to elaborate?

If you omit the line, the default will still be 'n'.

Jan


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

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

[Tamas K Lengyel]

On Wed, Nov 29, 2017 at 1:19 AM, Jan Beulich  wrote:
 On 28.11.17 at 19:06,  wrote:
>> --- 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"
>> + default n
>
> This is pointless.

Care to elaborate?

>
>> + depends on XSM && XSM_POLICY
>
> Please add only direct dependencies - XSM_POLICY already
> depends on XSM.
>

Sure.

Tamas

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

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

[Tamas K Lengyel]

On Wed, Nov 29, 2017 at 5:29 AM, George Dunlap  wrote:
> On 11/28/2017 07:04 PM, Tamas K Lengyel wrote:
>> On Tue, Nov 28, 2017 at 12:00 PM, Andrew Cooper
>>  wrote:
>>> On 28/11/17 18:06, Tamas K Lengyel wrote:
 From: Tamas K Lengyel 

 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 
 ---
 Cc: Andrew Cooper 
 Cc: George Dunlap 
 Cc: Ian Jackson 
 Cc: Jan Beulich 
 Cc: Konrad Rzeszutek Wilk 
 Cc: Stefano Stabellini 
 Cc: Tim Deegan 
 Cc: Wei Liu 
 Cc: Daniel De Graaf 
 Cc: ope...@googlegroups.com
 ---
  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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Jan Beulich]

>>> On 28.11.17 at 19:06,  wrote:
> --- 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"
> + default n

This is pointless.

> + depends on XSM && XSM_POLICY

Please add only direct dependencies - XSM_POLICY already
depends on XSM.

Jan


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

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

[Tamas K Lengyel]

On Tue, Nov 28, 2017 at 12:00 PM, Andrew Cooper
 wrote:
> On 28/11/17 18:06, Tamas K Lengyel wrote:
>> From: Tamas K Lengyel 
>>
>> 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 
>> ---
>> Cc: Andrew Cooper 
>> Cc: George Dunlap 
>> Cc: Ian Jackson 
>> Cc: Jan Beulich 
>> Cc: Konrad Rzeszutek Wilk 
>> Cc: Stefano Stabellini 
>> Cc: Tim Deegan 
>> Cc: Wei Liu 
>> Cc: Daniel De Graaf 
>> Cc: ope...@googlegroups.com
>> ---
>>  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.

Tamas

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

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

[Andrew Cooper]

On 28/11/17 18:06, Tamas K Lengyel wrote:
> From: Tamas K Lengyel 
>
> 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 
> ---
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> Cc: Ian Jackson 
> Cc: Jan Beulich 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Stefano Stabellini 
> Cc: Tim Deegan 
> Cc: Wei Liu 
> Cc: Daniel De Graaf 
> Cc: ope...@googlegroups.com
> ---
>  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?

~Andrew

> + default n
> + depends on XSM && XSM_POLICY
> + ---help---
> +   Set this option to 'Y' to have the hypervisor ignore the security
> +   policy provided by the bootloader, and use ONLY the built-in
> +   security policy.
> +
> +   This can be used to ensure only verified security policies are
> +   loaded during boot time.
> +
> +   If unsure, say N.
> +
>  config LATE_HWDOM
>   bool "Dedicated hardware domain"
>   default n
>


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

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

[Daniel De Graaf]

On 11/28/2017 01:06 PM, Tamas K Lengyel wrote:

From: Tamas K Lengyel 

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 


Acked-by: Daniel De Graaf 

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