[PATCH] XSM: add Kconfig option to override bootloader provided policy

[PATCH] XSM: add Kconfig option to override bootloader provided policy

[Tamas K Lengyel]

From: Tamas K Lengyel <lengyelt@ainfosec.com>

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@ainfosec.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: openxt@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"
+	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
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 08994ee7a1..2d5c1d3fec 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -39,7 +39,9 @@ static inline int verify(struct xsm_operations *ops)
 static int __init xsm_core_init(const void *policy_buffer, size_t policy_size)
 {
 #ifdef CONFIG_XSM_POLICY
+#ifndef CONFIG_XSM_POLICY_OVERRIDE
     if ( policy_size == 0 )
+#endif
     {
         policy_buffer = xsm_init_policy;
         policy_size = xsm_init_policy_size;
-- 
2.14.2


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

Re: [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 <lengyelt@ainfosec.com>
> 
> 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@ainfosec.com>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

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

Re: [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 <lengyelt@ainfosec.com>
>
> 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@ainfosec.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Cc: openxt@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: [PATCH] XSM: add Kconfig option to override bootloader provided policy

[Tamas K Lengyel]

On Tue, Nov 28, 2017 at 12:00 PM, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> On 28/11/17 18:06, Tamas K Lengyel wrote:
>> From: Tamas K Lengyel <lengyelt@ainfosec.com>
>>
>> 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@ainfosec.com>
>> ---
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Tim Deegan <tim@xen.org>
>> Cc: Wei Liu <wei.liu2@citrix.com>
>> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>> Cc: openxt@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: [PATCH] XSM: add Kconfig option to override bootloader provided policy

[Jan Beulich]

>>> On 28.11.17 at 19:06, <tamas@tklengyel.com> 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: [PATCH] XSM: add Kconfig option to override bootloader provided policy

[George Dunlap]

On 11/28/2017 07:04 PM, Tamas K Lengyel wrote:
> On Tue, Nov 28, 2017 at 12:00 PM, Andrew Cooper
> <andrew.cooper3@citrix.com> wrote:
>> On 28/11/17 18:06, Tamas K Lengyel wrote:
>>> From: Tamas K Lengyel <lengyelt@ainfosec.com>
>>>
>>> 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@ainfosec.com>
>>> ---
>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
>>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>>> Cc: Jan Beulich <jbeulich@suse.com>
>>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>>> Cc: Tim Deegan <tim@xen.org>
>>> Cc: Wei Liu <wei.liu2@citrix.com>
>>> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>> Cc: openxt@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.

 -George

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

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

[Tamas K Lengyel]

On Wed, Nov 29, 2017 at 5:29 AM, George Dunlap <george.dunlap@citrix.com> 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@citrix.com> wrote:
>>> On 28/11/17 18:06, Tamas K Lengyel wrote:
>>>> From: Tamas K Lengyel <lengyelt@ainfosec.com>
>>>>
>>>> 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@ainfosec.com>
>>>> ---
>>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
>>>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>>>> Cc: Jan Beulich <jbeulich@suse.com>
>>>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>>>> Cc: Tim Deegan <tim@xen.org>
>>>> Cc: Wei Liu <wei.liu2@citrix.com>
>>>> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>>> Cc: openxt@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: [PATCH] XSM: add Kconfig option to override bootloader provided policy

[Tamas K Lengyel]

On Wed, Nov 29, 2017 at 1:19 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 28.11.17 at 19:06, <tamas@tklengyel.com> 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: [PATCH] XSM: add Kconfig option to override bootloader provided policy

[Jan Beulich]

>>> On 29.11.17 at 16:33, <tamas@tklengyel.com> wrote:
> On Wed, Nov 29, 2017 at 1:19 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 28.11.17 at 19:06, <tamas@tklengyel.com> 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