[Xen-devel] [PATCH v3 1/2] xsm: add config option for denied string

[Xen-devel] [PATCH v3 1/2] xsm: add config option for denied string

[Sergey Dyasli]

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v2 --> v3:
- new patch

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: Julien Grall <julien@xen.org>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Wei Liu <wl@xen.org>
CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
CC: Doug Goldstein <cardoe@cardoe.com>
---
 xen/common/Kconfig   | 8 ++++++++
 xen/common/version.c | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index b3d161d057..f0a3f0da0f 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -236,6 +236,14 @@ choice
 		bool "SILO" if XSM_SILO
 endchoice
 
+config XSM_DENIED_STRING
+	string "xen_version denied string"
+	default "<denied>"
+	depends on XSM
+	---help---
+	  A string which substitutes sensitive information returned via
+	  xen_version hypercall to non-privileged guests
+
 config LATE_HWDOM
 	bool "Dedicated hardware domain"
 	default n
diff --git a/xen/common/version.c b/xen/common/version.c
index 937eb1281c..14b205af48 100644
--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -67,7 +67,7 @@ const char *xen_banner(void)
 
 const char *xen_deny(void)
 {
-    return "<denied>";
+    return CONFIG_XSM_DENIED_STRING;
 }
 
 static const void *build_id_p __read_mostly;
-- 
2.17.1


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

[Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Sergey Dyasli]

Hide the following information that can help identify the running Xen
binary version: XENVER_extraversion, XENVER_compile_info, XENVER_changeset.
This makes harder for malicious guests to fingerprint Xen to identify
exploitable systems.

Add explicit cases for XENVER_commandline and XENVER_build_id as well
for better code readability.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v2 --> v3:
- Remove hvmloader filtering
- Add ASSERT_UNREACHABLE

v1 --> v2:
- Added xsm_filter_denied() to hvmloader instead of modifying xen_deny()
- Made behaviour the same for both Release and Debug builds
- XENVER_capabilities is no longer hided

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: Julien Grall <julien@xen.org>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Wei Liu <wl@xen.org>
CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
CC: Doug Goldstein <cardoe@cardoe.com>
---
 xen/include/xsm/dummy.h | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index b8e185e6fa..c00186d7b6 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -750,16 +750,23 @@ static XSM_INLINE int xsm_xen_version (XSM_DEFAULT_ARG uint32_t op)
     case XENVER_get_features:
         /* These sub-ops ignore the permission checks and return data. */
         return 0;
-    case XENVER_extraversion:
-    case XENVER_compile_info:
+
     case XENVER_capabilities:
-    case XENVER_changeset:
     case XENVER_pagesize:
     case XENVER_guest_handle:
         /* These MUST always be accessible to any guest by default. */
         return xsm_default_action(XSM_HOOK, current->domain, NULL);
-    default:
+
+    case XENVER_extraversion:
+    case XENVER_compile_info:
+    case XENVER_changeset:
+    case XENVER_commandline:
+    case XENVER_build_id:
         return xsm_default_action(XSM_PRIV, current->domain, NULL);
+
+    default:
+        ASSERT_UNREACHABLE();
+        return -EPERM;
     }
 }
 
-- 
2.17.1


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

Re: [Xen-devel] [PATCH v3 1/2] xsm: add config option for denied string

[Jan Beulich]

On 17.01.2020 17:44, Sergey Dyasli wrote:
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

In principle
Acked-by: Jan Beulich <jbeulich@suse.com>

But I think it would be nice to have a non-empty description, at
least to reason why the option addition is deemed useful.

> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -236,6 +236,14 @@ choice
>  		bool "SILO" if XSM_SILO
>  endchoice
>  
> +config XSM_DENIED_STRING
> +	string "xen_version denied string"

I guess inserting "hypercall" into this prompt would set better
context without needing to resort to the help text, i.e.
"xen_version hypercall denied string". Thoughts?

Jan

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

Re: [Xen-devel] [PATCH v3 1/2] xsm: add config option for denied string

[Durrant, Paul]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Jan
> Beulich
> Sent: 20 January 2020 09:51
> To: Sergey Dyasli <sergey.dyasli@citrix.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>; Julien Grall
> <julien@xen.org>; Wei Liu <wl@xen.org>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>; George Dunlap <George.Dunlap@eu.citrix.com>;
> Andrew Cooper <andrew.cooper3@citrix.com>; Doug Goldstein
> <cardoe@cardoe.com>; xen-devel@lists.xen.org; Daniel De Graaf
> <dgdegra@tycho.nsa.gov>; Ian Jackson <ian.jackson@eu.citrix.com>
> Subject: Re: [Xen-devel] [PATCH v3 1/2] xsm: add config option for denied
> string
> 
> On 17.01.2020 17:44, Sergey Dyasli wrote:
> > Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> 
> In principle
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> But I think it would be nice to have a non-empty description, at
> least to reason why the option addition is deemed useful.
> 
> > --- a/xen/common/Kconfig
> > +++ b/xen/common/Kconfig
> > @@ -236,6 +236,14 @@ choice
> >  		bool "SILO" if XSM_SILO
> >  endchoice
> >
> > +config XSM_DENIED_STRING
> > +	string "xen_version denied string"
> 
> I guess inserting "hypercall" into this prompt would set better
> context without needing to resort to the help text, i.e.
> "xen_version hypercall denied string". Thoughts?
>

"xen_version hypercall denied information replacement string"?

It's not like the hypercall as a whole is being denied, after all.

  Paul

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Jan Beulich]

On 17.01.2020 17:44, Sergey Dyasli wrote:
> v2 --> v3:
> - Remove hvmloader filtering

Why? Seeing the prior discussion, how about adding XENVER_denied to
return the "denied" string, allowing components which want to filter
to know exactly what to look for? And then re-add the filtering you
had? (The help text of the config option should then perhaps be
extended to make very clear that the chosen string should not match
anything that could potentially be returned by any of the XENVER_
sub-ops.)

Jan

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Sergey Dyasli]

On 20/01/2020 10:01, Jan Beulich wrote:
> On 17.01.2020 17:44, Sergey Dyasli wrote:
>> v2 --> v3:
>> - Remove hvmloader filtering
>
> Why? Seeing the prior discussion, how about adding XENVER_denied to
> return the "denied" string, allowing components which want to filter
> to know exactly what to look for? And then re-add the filtering you
> had? (The help text of the config option should then perhaps be
> extended to make very clear that the chosen string should not match
> anything that could potentially be returned by any of the XENVER_
> sub-ops.)

I had the following reasoning:

1. Most real-world users would set CONFIG_XSM_DENIED_STRING="" anyway.

2. Filtering in DMI tables is not a complete solution, since denied
string leaks elsewhere through the hypercall (PV guests, sysfs, driver
logs) as Andrew has pointed out in the previous discussion.

On the other hand, SMBios filtering slightly improves the situation for
HVM domains, so I can return it if maintainers find it worthy.

--
Thanks,
Sergey

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Jan Beulich]

On 22.01.2020 11:01, Sergey Dyasli wrote:
> On 20/01/2020 10:01, Jan Beulich wrote:
>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>> v2 --> v3:
>>> - Remove hvmloader filtering
>>
>> Why? Seeing the prior discussion, how about adding XENVER_denied to
>> return the "denied" string, allowing components which want to filter
>> to know exactly what to look for? And then re-add the filtering you
>> had? (The help text of the config option should then perhaps be
>> extended to make very clear that the chosen string should not match
>> anything that could potentially be returned by any of the XENVER_
>> sub-ops.)
> 
> I had the following reasoning:
> 
> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING="" anyway.
> 
> 2. Filtering in DMI tables is not a complete solution, since denied
> string leaks elsewhere through the hypercall (PV guests, sysfs, driver
> logs) as Andrew has pointed out in the previous discussion.

Well, that's Andrew's thinking. To me, getting back "<denied>" from
the hypercall is not a leaking of this string, but the intended
output. I continue to think that getting back a blank string there
is confusing, as it is far more likely to be mistaken to be actually
blank than it is for "<denied>" to be mistaken for actual data.
Where such a string wants filtering is to be determined for every
consumer of this interface separately; as said before, this is a UI
function, not something to be catered for in the hypervisor.

Jan

> On the other hand, SMBios filtering slightly improves the situation for
> HVM domains, so I can return it if maintainers find it worthy.
> 
> --
> Thanks,
> Sergey
> 


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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Julien Grall]

On 22/01/2020 10:01, Sergey Dyasli wrote:
> On 20/01/2020 10:01, Jan Beulich wrote:
>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>> v2 --> v3:
>>> - Remove hvmloader filtering
>>
>> Why? Seeing the prior discussion, how about adding XENVER_denied to
>> return the "denied" string, allowing components which want to filter
>> to know exactly what to look for? And then re-add the filtering you
>> had? (The help text of the config option should then perhaps be
>> extended to make very clear that the chosen string should not match
>> anything that could potentially be returned by any of the XENVER_
>> sub-ops.)
> 
> I had the following reasoning:
> 
> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING="" anyway.
> 
> 2. Filtering in DMI tables is not a complete solution, since denied
> string leaks elsewhere through the hypercall (PV guests, sysfs, driver
> logs) as Andrew has pointed out in the previous discussion.
> 
> On the other hand, SMBios filtering slightly improves the situation for
> HVM domains, so I can return it if maintainers find it worthy.

While I am not a maintainer of this code, my concern is you impose the 
conversion from "denied" to "" to all the users (include those who wants 
to keep "denied").

If you were doing any filtering in hvmloader, then it would be best if 
this is configurable. But this is a bit pointless if you already allow 
the user to configure the string at the hypervisor level :).

Cheers,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[George Dunlap]

On 1/22/20 10:14 AM, Julien Grall wrote:
> 
> 
> On 22/01/2020 10:01, Sergey Dyasli wrote:
>> On 20/01/2020 10:01, Jan Beulich wrote:
>>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>>> v2 --> v3:
>>>> - Remove hvmloader filtering
>>>
>>> Why? Seeing the prior discussion, how about adding XENVER_denied to
>>> return the "denied" string, allowing components which want to filter
>>> to know exactly what to look for? And then re-add the filtering you
>>> had? (The help text of the config option should then perhaps be
>>> extended to make very clear that the chosen string should not match
>>> anything that could potentially be returned by any of the XENVER_
>>> sub-ops.)
>>
>> I had the following reasoning:
>>
>> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING="" anyway.
>>
>> 2. Filtering in DMI tables is not a complete solution, since denied
>> string leaks elsewhere through the hypercall (PV guests, sysfs, driver
>> logs) as Andrew has pointed out in the previous discussion.
>>
>> On the other hand, SMBios filtering slightly improves the situation for
>> HVM domains, so I can return it if maintainers find it worthy.
> 
> While I am not a maintainer of this code, my concern is you impose the
> conversion from "denied" to "" to all the users (include those who wants
> to keep "denied").
> 
> If you were doing any filtering in hvmloader, then it would be best if
> this is configurable. But this is a bit pointless if you already allow
> the user to configure the string at the hypervisor level :).

So there are two things we're concerned about:
- Some people don't want to scare users with a "<denied>" string
- Some people don't want to "silently fail" with a "" string

The fact is, in *both cases*, this is a UI problem.  EVERY caller of
this interface should figure out independently what a graceful way of
handling failure is for their target UI.  Any caller who does not think
carefully about what to do in the failure case is buggy -- which
includes every single caller today.  The CONFIG_XSM_DENIED_STRING is a
gross hack fallback for buggy UIs.

Now, I don't like to tell other people to do work, and I certainly don't
plan on fixing hvmloader at the moment, because it's low-priority for
me.  But I do think that having hvmloader detect failure and explicitly
make a sensible decision is the right thing to do, regardless of the
availability of CONFIG_XSM_DENIED_STRING to work around buggy callers.

 -George

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Sergey Dyasli]

On 22/01/2020 10:14, Julien Grall wrote:
>
>
> On 22/01/2020 10:01, Sergey Dyasli wrote:
>> On 20/01/2020 10:01, Jan Beulich wrote:
>>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>>> v2 --> v3:
>>>> - Remove hvmloader filtering
>>>
>>> Why? Seeing the prior discussion, how about adding XENVER_denied to
>>> return the "denied" string, allowing components which want to filter
>>> to know exactly what to look for? And then re-add the filtering you
>>> had? (The help text of the config option should then perhaps be
>>> extended to make very clear that the chosen string should not match
>>> anything that could potentially be returned by any of the XENVER_
>>> sub-ops.)
>>
>> I had the following reasoning:
>>
>> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING="" anyway.
>>
>> 2. Filtering in DMI tables is not a complete solution, since denied
>> string leaks elsewhere through the hypercall (PV guests, sysfs, driver
>> logs) as Andrew has pointed out in the previous discussion.
>>
>> On the other hand, SMBios filtering slightly improves the situation for
>> HVM domains, so I can return it if maintainers find it worthy.
>
> While I am not a maintainer of this code, my concern is you impose the conversion from "denied" to "" to all the users (include those who wants to keep "denied").

This is not what's happening here: the default is still "<denied>" (as
per patch 1); but patch 2 makes XENVER_extraversion, XENVER_compile_info
and XENVER_changeset to return "<denied>" instead of the real values
which causes the UI / logs issues.

>
> If you were doing any filtering in hvmloader, then it would be best if this is configurable. But this is a bit pointless if you already allow the user to configure the string at the hypervisor level :).

--
Thanks,
Sergey

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Julien Grall]

On 22/01/2020 11:19, Sergey Dyasli wrote:
> On 22/01/2020 10:14, Julien Grall wrote:
>>
>>
>> On 22/01/2020 10:01, Sergey Dyasli wrote:
>>> On 20/01/2020 10:01, Jan Beulich wrote:
>>>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>>>> v2 --> v3:
>>>>> - Remove hvmloader filtering
>>>>
>>>> Why? Seeing the prior discussion, how about adding XENVER_denied to
>>>> return the "denied" string, allowing components which want to filter
>>>> to know exactly what to look for? And then re-add the filtering you
>>>> had? (The help text of the config option should then perhaps be
>>>> extended to make very clear that the chosen string should not match
>>>> anything that could potentially be returned by any of the XENVER_
>>>> sub-ops.)
>>>
>>> I had the following reasoning:
>>>
>>> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING="" anyway.
>>>
>>> 2. Filtering in DMI tables is not a complete solution, since denied
>>> string leaks elsewhere through the hypercall (PV guests, sysfs, driver
>>> logs) as Andrew has pointed out in the previous discussion.
>>>
>>> On the other hand, SMBios filtering slightly improves the situation for
>>> HVM domains, so I can return it if maintainers find it worthy.
>>
>> While I am not a maintainer of this code, my concern is you impose the conversion from "denied" to "" to all the users (include those who wants to keep "denied").
> 
> This is not what's happening here: the default is still "<denied>" (as
> per patch 1); but patch 2 makes XENVER_extraversion, XENVER_compile_info
> and XENVER_changeset to return "<denied>" instead of the real values
> which causes the UI / logs issues.

I was referring the SMBios filtering... I don't think doing a blank 
filtering is the right thing to do in the hvmloader for the reason 
explained above.

Regarding CONFIG_XSM_DENIED_STRING, I think this is a good step as it 
allows the vendor to configure it.

Cheers,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Sergey Dyasli]

On 22/01/2020 10:57, George Dunlap wrote:
> On 1/22/20 10:14 AM, Julien Grall wrote:
>>
>>
>> On 22/01/2020 10:01, Sergey Dyasli wrote:
>>> On 20/01/2020 10:01, Jan Beulich wrote:
>>>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>>>> v2 --> v3:
>>>>> - Remove hvmloader filtering
>>>>
>>>> Why? Seeing the prior discussion, how about adding XENVER_denied to
>>>> return the "denied" string, allowing components which want to filter
>>>> to know exactly what to look for? And then re-add the filtering you
>>>> had? (The help text of the config option should then perhaps be
>>>> extended to make very clear that the chosen string should not match
>>>> anything that could potentially be returned by any of the XENVER_
>>>> sub-ops.)
>>>
>>> I had the following reasoning:
>>>
>>> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING="" anyway.
>>>
>>> 2. Filtering in DMI tables is not a complete solution, since denied
>>> string leaks elsewhere through the hypercall (PV guests, sysfs, driver
>>> logs) as Andrew has pointed out in the previous discussion.
>>>
>>> On the other hand, SMBios filtering slightly improves the situation for
>>> HVM domains, so I can return it if maintainers find it worthy.
>>
>> While I am not a maintainer of this code, my concern is you impose the
>> conversion from "denied" to "" to all the users (include those who wants
>> to keep "denied").
>>
>> If you were doing any filtering in hvmloader, then it would be best if
>> this is configurable. But this is a bit pointless if you already allow
>> the user to configure the string at the hypervisor level :).
>
> So there are two things we're concerned about:
> - Some people don't want to scare users with a "<denied>" string
> - Some people don't want to "silently fail" with a "" string
>
> The fact is, in *both cases*, this is a UI problem.  EVERY caller of
> this interface should figure out independently what a graceful way of
> handling failure is for their target UI.  Any caller who does not think
> carefully about what to do in the failure case is buggy -- which
> includes every single caller today.  The CONFIG_XSM_DENIED_STRING is a
> gross hack fallback for buggy UIs.
>
> Now, I don't like to tell other people to do work, and I certainly don't
> plan on fixing hvmloader at the moment, because it's low-priority for
> me.  But I do think that having hvmloader detect failure and explicitly
> make a sensible decision is the right thing to do, regardless of the
> availability of CONFIG_XSM_DENIED_STRING to work around buggy callers.

It's not entirely clear to me what you suggest to do with hvmloader.
Could you elaborate a bit?

--
Thanks,
Sergey

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Julien Grall]

Hi George,

On 22/01/2020 10:57, George Dunlap wrote:
> On 1/22/20 10:14 AM, Julien Grall wrote:
>>
>>
>> On 22/01/2020 10:01, Sergey Dyasli wrote:
>>> On 20/01/2020 10:01, Jan Beulich wrote:
>>>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>>>> v2 --> v3:
>>>>> - Remove hvmloader filtering
>>>>
>>>> Why? Seeing the prior discussion, how about adding XENVER_denied to
>>>> return the "denied" string, allowing components which want to filter
>>>> to know exactly what to look for? And then re-add the filtering you
>>>> had? (The help text of the config option should then perhaps be
>>>> extended to make very clear that the chosen string should not match
>>>> anything that could potentially be returned by any of the XENVER_
>>>> sub-ops.)
>>>
>>> I had the following reasoning:
>>>
>>> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING="" anyway.
>>>
>>> 2. Filtering in DMI tables is not a complete solution, since denied
>>> string leaks elsewhere through the hypercall (PV guests, sysfs, driver
>>> logs) as Andrew has pointed out in the previous discussion.
>>>
>>> On the other hand, SMBios filtering slightly improves the situation for
>>> HVM domains, so I can return it if maintainers find it worthy.
>>
>> While I am not a maintainer of this code, my concern is you impose the
>> conversion from "denied" to "" to all the users (include those who wants
>> to keep "denied").
>>
>> If you were doing any filtering in hvmloader, then it would be best if
>> this is configurable. But this is a bit pointless if you already allow
>> the user to configure the string at the hypervisor level :).
> 
> So there are two things we're concerned about:
> - Some people don't want to scare users with a "<denied>" string
> - Some people don't want to "silently fail" with a "" string
> 
> The fact is, in *both cases*, this is a UI problem.  EVERY caller of
> this interface should figure out independently what a graceful way of
> handling failure is for their target UI.  Any caller who does not think
> carefully about what to do in the failure case is buggy -- which
> includes every single caller today.  The CONFIG_XSM_DENIED_STRING is a
> gross hack fallback for buggy UIs.

I agree that the two cases you explained are UI problems. However, I can 
see other use for the Kconfig option (with some tweaks).

At AWS, consistency accross two stable versions is very important. So 
most of the version strings exposed to the guest are fixed. Therefore a 
guest can be migrated seemlessly between two different versions without 
seen any change that may break it.

You could imagine using XSM to know whether you want to expose the guest 
the real or fixed version strings. Put it that way, this sounds less a 
gross hack over "<denied>".

The use case described would require multiple Kconfig options though.

> 
> Now, I don't like to tell other people to do work, and I certainly don't
> plan on fixing hvmloader at the moment, because it's low-priority for
> me.  But I do think that having hvmloader detect failure and explicitly
> make a sensible decision is the right thing to do, regardless of the
> availability of CONFIG_XSM_DENIED_STRING to work around buggy callers.

The lengthy discussion about returning "<denied>" shows that we 
(XenProject community) are not in position to decide what is the
sensible value here (even for filtering).

By allowing a vendor to configure the string in the hypervisor, you 
remove that decision from us.

Cheers,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Jan Beulich]

On 22.01.2020 13:05, Julien Grall wrote:
> Hi George,
> 
> On 22/01/2020 10:57, George Dunlap wrote:
>> On 1/22/20 10:14 AM, Julien Grall wrote:
>>>
>>>
>>> On 22/01/2020 10:01, Sergey Dyasli wrote:
>>>> On 20/01/2020 10:01, Jan Beulich wrote:
>>>>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>>>>> v2 --> v3:
>>>>>> - Remove hvmloader filtering
>>>>>
>>>>> Why? Seeing the prior discussion, how about adding XENVER_denied to
>>>>> return the "denied" string, allowing components which want to filter
>>>>> to know exactly what to look for? And then re-add the filtering you
>>>>> had? (The help text of the config option should then perhaps be
>>>>> extended to make very clear that the chosen string should not match
>>>>> anything that could potentially be returned by any of the XENVER_
>>>>> sub-ops.)
>>>>
>>>> I had the following reasoning:
>>>>
>>>> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING="" anyway.
>>>>
>>>> 2. Filtering in DMI tables is not a complete solution, since denied
>>>> string leaks elsewhere through the hypercall (PV guests, sysfs, driver
>>>> logs) as Andrew has pointed out in the previous discussion.
>>>>
>>>> On the other hand, SMBios filtering slightly improves the situation for
>>>> HVM domains, so I can return it if maintainers find it worthy.
>>>
>>> While I am not a maintainer of this code, my concern is you impose the
>>> conversion from "denied" to "" to all the users (include those who wants
>>> to keep "denied").
>>>
>>> If you were doing any filtering in hvmloader, then it would be best if
>>> this is configurable. But this is a bit pointless if you already allow
>>> the user to configure the string at the hypervisor level :).
>>
>> So there are two things we're concerned about:
>> - Some people don't want to scare users with a "<denied>" string
>> - Some people don't want to "silently fail" with a "" string
>>
>> The fact is, in *both cases*, this is a UI problem.  EVERY caller of
>> this interface should figure out independently what a graceful way of
>> handling failure is for their target UI.  Any caller who does not think
>> carefully about what to do in the failure case is buggy -- which
>> includes every single caller today.  The CONFIG_XSM_DENIED_STRING is a
>> gross hack fallback for buggy UIs.
> 
> I agree that the two cases you explained are UI problems. However, I can 
> see other use for the Kconfig option (with some tweaks).
> 
> At AWS, consistency accross two stable versions is very important. So 
> most of the version strings exposed to the guest are fixed. Therefore a 
> guest can be migrated seemlessly between two different versions without 
> seen any change that may break it.

A guest aware of being run on a hypervisor would also be aware
that it may be migrated, and hence that the version of the
underlying hypervisor may change (if it cares about versions
in the first place). A guest unaware of being run on a
hypervisor wouldn't care about version and alike strings at all.
Nevertheless I'm sure you play this game for a (real world)
reason, e.g. people making wrong assumptions. But is this
something you really think the upstream hypervisor should be
made care about?

Jan

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Julien Grall]

On 22/01/2020 12:32, Jan Beulich wrote:
> On 22.01.2020 13:05, Julien Grall wrote:
>> Hi George,
>>
>> On 22/01/2020 10:57, George Dunlap wrote:
>>> On 1/22/20 10:14 AM, Julien Grall wrote:
>>>>
>>>>
>>>> On 22/01/2020 10:01, Sergey Dyasli wrote:
>>>>> On 20/01/2020 10:01, Jan Beulich wrote:
>>>>>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>>>>>> v2 --> v3:
>>>>>>> - Remove hvmloader filtering
>>>>>>
>>>>>> Why? Seeing the prior discussion, how about adding XENVER_denied to
>>>>>> return the "denied" string, allowing components which want to filter
>>>>>> to know exactly what to look for? And then re-add the filtering you
>>>>>> had? (The help text of the config option should then perhaps be
>>>>>> extended to make very clear that the chosen string should not match
>>>>>> anything that could potentially be returned by any of the XENVER_
>>>>>> sub-ops.)
>>>>>
>>>>> I had the following reasoning:
>>>>>
>>>>> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING="" anyway.
>>>>>
>>>>> 2. Filtering in DMI tables is not a complete solution, since denied
>>>>> string leaks elsewhere through the hypercall (PV guests, sysfs, driver
>>>>> logs) as Andrew has pointed out in the previous discussion.
>>>>>
>>>>> On the other hand, SMBios filtering slightly improves the situation for
>>>>> HVM domains, so I can return it if maintainers find it worthy.
>>>>
>>>> While I am not a maintainer of this code, my concern is you impose the
>>>> conversion from "denied" to "" to all the users (include those who wants
>>>> to keep "denied").
>>>>
>>>> If you were doing any filtering in hvmloader, then it would be best if
>>>> this is configurable. But this is a bit pointless if you already allow
>>>> the user to configure the string at the hypervisor level :).
>>>
>>> So there are two things we're concerned about:
>>> - Some people don't want to scare users with a "<denied>" string
>>> - Some people don't want to "silently fail" with a "" string
>>>
>>> The fact is, in *both cases*, this is a UI problem.  EVERY caller of
>>> this interface should figure out independently what a graceful way of
>>> handling failure is for their target UI.  Any caller who does not think
>>> carefully about what to do in the failure case is buggy -- which
>>> includes every single caller today.  The CONFIG_XSM_DENIED_STRING is a
>>> gross hack fallback for buggy UIs.
>>
>> I agree that the two cases you explained are UI problems. However, I can
>> see other use for the Kconfig option (with some tweaks).
>>
>> At AWS, consistency accross two stable versions is very important. So
>> most of the version strings exposed to the guest are fixed. Therefore a
>> guest can be migrated seemlessly between two different versions without
>> seen any change that may break it.
> 
> A guest aware of being run on a hypervisor would also be aware
> that it may be migrated, and hence that the version of the
> underlying hypervisor may change (if it cares about versions
> in the first place).

If you use upstream-as-is yes. But with the on-going discussion 
regarding live udpate and guest transparent migration, a guest would 
seemlessly move between Xen versions without even been aware.

> A guest unaware of being run on a
> hypervisor wouldn't care about version and alike strings at all.
> Nevertheless I'm sure you play this game for a (real world)
> reason, e.g. people making wrong assumptions. But is this
> something you really think the upstream hypervisor should be
> made care about?

I agree that upstream does not necessarily needs it today. But this is 
an example on how configurable version strings could be useful by 
downstream users.

Cheers,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Sergey Dyasli]

On 22/01/2020 11:25, Julien Grall wrote:
>
>
> On 22/01/2020 11:19, Sergey Dyasli wrote:
>> On 22/01/2020 10:14, Julien Grall wrote:
>>>
>>>
>>> On 22/01/2020 10:01, Sergey Dyasli wrote:
>>>> On 20/01/2020 10:01, Jan Beulich wrote:
>>>>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>>>>> v2 --> v3:
>>>>>> - Remove hvmloader filtering
>>>>>
>>>>> Why? Seeing the prior discussion, how about adding XENVER_denied to
>>>>> return the "denied" string, allowing components which want to filter
>>>>> to know exactly what to look for? And then re-add the filtering you
>>>>> had? (The help text of the config option should then perhaps be
>>>>> extended to make very clear that the chosen string should not match
>>>>> anything that could potentially be returned by any of the XENVER_
>>>>> sub-ops.)
>>>>
>>>> I had the following reasoning:
>>>>
>>>> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING="" anyway.
>>>>
>>>> 2. Filtering in DMI tables is not a complete solution, since denied
>>>> string leaks elsewhere through the hypercall (PV guests, sysfs, driver
>>>> logs) as Andrew has pointed out in the previous discussion.
>>>>
>>>> On the other hand, SMBios filtering slightly improves the situation for
>>>> HVM domains, so I can return it if maintainers find it worthy.
>>>
>>> While I am not a maintainer of this code, my concern is you impose the conversion from "denied" to "" to all the users (include those who wants to keep "denied").
>>
>> This is not what's happening here: the default is still "<denied>" (as
>> per patch 1); but patch 2 makes XENVER_extraversion, XENVER_compile_info
>> and XENVER_changeset to return "<denied>" instead of the real values
>> which causes the UI / logs issues.
>
> I was referring the SMBios filtering... I don't think doing a blank filtering is the right thing to do in the hvmloader for the reason explained above.

Apologies for misunderstanding the context. But I disagree about
hvmloader. Returning "denied" from xen_version hypercall to guests is
one thing, but hvmloader and SMBios tables are parts of the hypervisor
and putting "denied" there is simply a terrible user experience.

>
> Regarding CONFIG_XSM_DENIED_STRING, I think this is a good step as it allows the vendor to configure it.

--
Thanks,
Sergey

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[George Dunlap]

On 1/22/20 11:44 AM, Sergey Dyasli wrote:
> On 22/01/2020 10:57, George Dunlap wrote:
>> On 1/22/20 10:14 AM, Julien Grall wrote:
>>>
>>>
>>> On 22/01/2020 10:01, Sergey Dyasli wrote:
>>>> On 20/01/2020 10:01, Jan Beulich wrote:
>>>>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>>>>> v2 --> v3:
>>>>>> - Remove hvmloader filtering
>>>>>
>>>>> Why? Seeing the prior discussion, how about adding XENVER_denied to
>>>>> return the "denied" string, allowing components which want to filter
>>>>> to know exactly what to look for? And then re-add the filtering you
>>>>> had? (The help text of the config option should then perhaps be
>>>>> extended to make very clear that the chosen string should not match
>>>>> anything that could potentially be returned by any of the XENVER_
>>>>> sub-ops.)
>>>>
>>>> I had the following reasoning:
>>>>
>>>> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING="" anyway.
>>>>
>>>> 2. Filtering in DMI tables is not a complete solution, since denied
>>>> string leaks elsewhere through the hypercall (PV guests, sysfs, driver
>>>> logs) as Andrew has pointed out in the previous discussion.
>>>>
>>>> On the other hand, SMBios filtering slightly improves the situation for
>>>> HVM domains, so I can return it if maintainers find it worthy.
>>>
>>> While I am not a maintainer of this code, my concern is you impose the
>>> conversion from "denied" to "" to all the users (include those who wants
>>> to keep "denied").
>>>
>>> If you were doing any filtering in hvmloader, then it would be best if
>>> this is configurable. But this is a bit pointless if you already allow
>>> the user to configure the string at the hypervisor level :).
>>
>> So there are two things we're concerned about:
>> - Some people don't want to scare users with a "<denied>" string
>> - Some people don't want to "silently fail" with a "" string
>>
>> The fact is, in *both cases*, this is a UI problem.  EVERY caller of
>> this interface should figure out independently what a graceful way of
>> handling failure is for their target UI.  Any caller who does not think
>> carefully about what to do in the failure case is buggy -- which
>> includes every single caller today.  The CONFIG_XSM_DENIED_STRING is a
>> gross hack fallback for buggy UIs.
>>
>> Now, I don't like to tell other people to do work, and I certainly don't
>> plan on fixing hvmloader at the moment, because it's low-priority for
>> me.  But I do think that having hvmloader detect failure and explicitly
>> make a sensible decision is the right thing to do, regardless of the
>> availability of CONFIG_XSM_DENIED_STRING to work around buggy callers.
> 
> It's not entirely clear to me what you suggest to do with hvmloader.
> Could you elaborate a bit?

Well, basically "think carefully about the user experience and decide
what to do".  Given your comment in response to Julien, I would think
that would mean checking for CONFIG_XSM_DENIED_STRING in hvmloader and
replacing it with nothing (or some other indication that's more
user-friendly).  Perhaps re-submitting your hvmloader patch as a
follow-up patch.

But as I said, it's just a suggestion.

 -George

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Julien Grall]

Hi,

On 23/01/2020 11:32, Sergey Dyasli wrote:
> On 22/01/2020 11:25, Julien Grall wrote:
>>
>>
>> On 22/01/2020 11:19, Sergey Dyasli wrote:
>>> On 22/01/2020 10:14, Julien Grall wrote:
>>>>
>>>>
>>>> On 22/01/2020 10:01, Sergey Dyasli wrote:
>>>>> On 20/01/2020 10:01, Jan Beulich wrote:
>>>>>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>>>>>> v2 --> v3:
>>>>>>> - Remove hvmloader filtering
>>>>>>
>>>>>> Why? Seeing the prior discussion, how about adding XENVER_denied to
>>>>>> return the "denied" string, allowing components which want to filter
>>>>>> to know exactly what to look for? And then re-add the filtering you
>>>>>> had? (The help text of the config option should then perhaps be
>>>>>> extended to make very clear that the chosen string should not match
>>>>>> anything that could potentially be returned by any of the XENVER_
>>>>>> sub-ops.)
>>>>>
>>>>> I had the following reasoning:
>>>>>
>>>>> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING="" anyway.
>>>>>
>>>>> 2. Filtering in DMI tables is not a complete solution, since denied
>>>>> string leaks elsewhere through the hypercall (PV guests, sysfs, driver
>>>>> logs) as Andrew has pointed out in the previous discussion.
>>>>>
>>>>> On the other hand, SMBios filtering slightly improves the situation for
>>>>> HVM domains, so I can return it if maintainers find it worthy.
>>>>
>>>> While I am not a maintainer of this code, my concern is you impose the conversion from "denied" to "" to all the users (include those who wants to keep "denied").
>>>
>>> This is not what's happening here: the default is still "<denied>" (as
>>> per patch 1); but patch 2 makes XENVER_extraversion, XENVER_compile_info
>>> and XENVER_changeset to return "<denied>" instead of the real values
>>> which causes the UI / logs issues.
>>
>> I was referring the SMBios filtering... I don't think doing a blank filtering is the right thing to do in the hvmloader for the reason explained above.
> 
> Apologies for misunderstanding the context. But I disagree about
> hvmloader. Returning "denied" from xen_version hypercall to guests is
> one thing, but hvmloader and SMBios tables are parts of the hypervisor
> and putting "denied" there is simply a terrible user experience.

I am not going to comment on the user experience because this is up to 
another bikeshedding. The question is which string will you use? Why an 
empty string over something different?

However, if an admin has control over the "deny" string, why would he 
ever want to filter it in hvmloader? Why can't he just replace the one 
in Kconfig?

Cheers,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[George Dunlap]

On 1/23/20 2:42 PM, Julien Grall wrote:
> Hi,
> 
> On 23/01/2020 11:32, Sergey Dyasli wrote:
>> On 22/01/2020 11:25, Julien Grall wrote:
>>>
>>>
>>> On 22/01/2020 11:19, Sergey Dyasli wrote:
>>>> On 22/01/2020 10:14, Julien Grall wrote:
>>>>>
>>>>>
>>>>> On 22/01/2020 10:01, Sergey Dyasli wrote:
>>>>>> On 20/01/2020 10:01, Jan Beulich wrote:
>>>>>>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>>>>>>> v2 --> v3:
>>>>>>>> - Remove hvmloader filtering
>>>>>>>
>>>>>>> Why? Seeing the prior discussion, how about adding XENVER_denied to
>>>>>>> return the "denied" string, allowing components which want to filter
>>>>>>> to know exactly what to look for? And then re-add the filtering you
>>>>>>> had? (The help text of the config option should then perhaps be
>>>>>>> extended to make very clear that the chosen string should not match
>>>>>>> anything that could potentially be returned by any of the XENVER_
>>>>>>> sub-ops.)
>>>>>>
>>>>>> I had the following reasoning:
>>>>>>
>>>>>> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING=""
>>>>>> anyway.
>>>>>>
>>>>>> 2. Filtering in DMI tables is not a complete solution, since denied
>>>>>> string leaks elsewhere through the hypercall (PV guests, sysfs,
>>>>>> driver
>>>>>> logs) as Andrew has pointed out in the previous discussion.
>>>>>>
>>>>>> On the other hand, SMBios filtering slightly improves the
>>>>>> situation for
>>>>>> HVM domains, so I can return it if maintainers find it worthy.
>>>>>
>>>>> While I am not a maintainer of this code, my concern is you impose
>>>>> the conversion from "denied" to "" to all the users (include those
>>>>> who wants to keep "denied").
>>>>
>>>> This is not what's happening here: the default is still "<denied>" (as
>>>> per patch 1); but patch 2 makes XENVER_extraversion,
>>>> XENVER_compile_info
>>>> and XENVER_changeset to return "<denied>" instead of the real values
>>>> which causes the UI / logs issues.
>>>
>>> I was referring the SMBios filtering... I don't think doing a blank
>>> filtering is the right thing to do in the hvmloader for the reason
>>> explained above.
>>
>> Apologies for misunderstanding the context. But I disagree about
>> hvmloader. Returning "denied" from xen_version hypercall to guests is
>> one thing, but hvmloader and SMBios tables are parts of the hypervisor
>> and putting "denied" there is simply a terrible user experience.
> 
> I am not going to comment on the user experience because this is up to
> another bikeshedding. The question is which string will you use? Why an
> empty string over something different?
> 
> However, if an admin has control over the "deny" string, why would he
> ever want to filter it in hvmloader? Why can't he just replace the one
> in Kconfig?

Most admins don't compile their own version of Xen...

 -George

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Julien Grall]

Hi,

On 23/01/2020 14:45, George Dunlap wrote:
> On 1/23/20 2:42 PM, Julien Grall wrote:
>> Hi,
>>
>> On 23/01/2020 11:32, Sergey Dyasli wrote:
>>> On 22/01/2020 11:25, Julien Grall wrote:
>>>>
>>>>
>>>> On 22/01/2020 11:19, Sergey Dyasli wrote:
>>>>> On 22/01/2020 10:14, Julien Grall wrote:
>>>>>>
>>>>>>
>>>>>> On 22/01/2020 10:01, Sergey Dyasli wrote:
>>>>>>> On 20/01/2020 10:01, Jan Beulich wrote:
>>>>>>>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>>>>>>>> v2 --> v3:
>>>>>>>>> - Remove hvmloader filtering
>>>>>>>>
>>>>>>>> Why? Seeing the prior discussion, how about adding XENVER_denied to
>>>>>>>> return the "denied" string, allowing components which want to filter
>>>>>>>> to know exactly what to look for? And then re-add the filtering you
>>>>>>>> had? (The help text of the config option should then perhaps be
>>>>>>>> extended to make very clear that the chosen string should not match
>>>>>>>> anything that could potentially be returned by any of the XENVER_
>>>>>>>> sub-ops.)
>>>>>>>
>>>>>>> I had the following reasoning:
>>>>>>>
>>>>>>> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING=""
>>>>>>> anyway.
>>>>>>>
>>>>>>> 2. Filtering in DMI tables is not a complete solution, since denied
>>>>>>> string leaks elsewhere through the hypercall (PV guests, sysfs,
>>>>>>> driver
>>>>>>> logs) as Andrew has pointed out in the previous discussion.
>>>>>>>
>>>>>>> On the other hand, SMBios filtering slightly improves the
>>>>>>> situation for
>>>>>>> HVM domains, so I can return it if maintainers find it worthy.
>>>>>>
>>>>>> While I am not a maintainer of this code, my concern is you impose
>>>>>> the conversion from "denied" to "" to all the users (include those
>>>>>> who wants to keep "denied").
>>>>>
>>>>> This is not what's happening here: the default is still "<denied>" (as
>>>>> per patch 1); but patch 2 makes XENVER_extraversion,
>>>>> XENVER_compile_info
>>>>> and XENVER_changeset to return "<denied>" instead of the real values
>>>>> which causes the UI / logs issues.
>>>>
>>>> I was referring the SMBios filtering... I don't think doing a blank
>>>> filtering is the right thing to do in the hvmloader for the reason
>>>> explained above.
>>>
>>> Apologies for misunderstanding the context. But I disagree about
>>> hvmloader. Returning "denied" from xen_version hypercall to guests is
>>> one thing, but hvmloader and SMBios tables are parts of the hypervisor
>>> and putting "denied" there is simply a terrible user experience.
>>
>> I am not going to comment on the user experience because this is up to
>> another bikeshedding. The question is which string will you use? Why an
>> empty string over something different?
>>
>> However, if an admin has control over the "deny" string, why would he
>> ever want to filter it in hvmloader? Why can't he just replace the one
>> in Kconfig?
> 
> Most admins don't compile their own version of Xen...

Those admins are also unlikely going to build there own hvmloader, right?

Therefore, they will have to accept whatever string is reported by 
HVMLoader (or Xen). As you already allow Xen to configure it, why would 
that be a problem to change the one in Kconfig? Why do you need to fix 
it up in hvmloader as well?

Cheers,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[George Dunlap]

On 1/23/20 2:52 PM, Julien Grall wrote:
> Hi,
> 
> On 23/01/2020 14:45, George Dunlap wrote:
>> On 1/23/20 2:42 PM, Julien Grall wrote:
>>> Hi,
>>>
>>> On 23/01/2020 11:32, Sergey Dyasli wrote:
>>>> On 22/01/2020 11:25, Julien Grall wrote:
>>>>>
>>>>>
>>>>> On 22/01/2020 11:19, Sergey Dyasli wrote:
>>>>>> On 22/01/2020 10:14, Julien Grall wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 22/01/2020 10:01, Sergey Dyasli wrote:
>>>>>>>> On 20/01/2020 10:01, Jan Beulich wrote:
>>>>>>>>> On 17.01.2020 17:44, Sergey Dyasli wrote:
>>>>>>>>>> v2 --> v3:
>>>>>>>>>> - Remove hvmloader filtering
>>>>>>>>>
>>>>>>>>> Why? Seeing the prior discussion, how about adding
>>>>>>>>> XENVER_denied to
>>>>>>>>> return the "denied" string, allowing components which want to
>>>>>>>>> filter
>>>>>>>>> to know exactly what to look for? And then re-add the filtering
>>>>>>>>> you
>>>>>>>>> had? (The help text of the config option should then perhaps be
>>>>>>>>> extended to make very clear that the chosen string should not
>>>>>>>>> match
>>>>>>>>> anything that could potentially be returned by any of the XENVER_
>>>>>>>>> sub-ops.)
>>>>>>>>
>>>>>>>> I had the following reasoning:
>>>>>>>>
>>>>>>>> 1. Most real-world users would set CONFIG_XSM_DENIED_STRING=""
>>>>>>>> anyway.
>>>>>>>>
>>>>>>>> 2. Filtering in DMI tables is not a complete solution, since denied
>>>>>>>> string leaks elsewhere through the hypercall (PV guests, sysfs,
>>>>>>>> driver
>>>>>>>> logs) as Andrew has pointed out in the previous discussion.
>>>>>>>>
>>>>>>>> On the other hand, SMBios filtering slightly improves the
>>>>>>>> situation for
>>>>>>>> HVM domains, so I can return it if maintainers find it worthy.
>>>>>>>
>>>>>>> While I am not a maintainer of this code, my concern is you impose
>>>>>>> the conversion from "denied" to "" to all the users (include those
>>>>>>> who wants to keep "denied").
>>>>>>
>>>>>> This is not what's happening here: the default is still "<denied>"
>>>>>> (as
>>>>>> per patch 1); but patch 2 makes XENVER_extraversion,
>>>>>> XENVER_compile_info
>>>>>> and XENVER_changeset to return "<denied>" instead of the real values
>>>>>> which causes the UI / logs issues.
>>>>>
>>>>> I was referring the SMBios filtering... I don't think doing a blank
>>>>> filtering is the right thing to do in the hvmloader for the reason
>>>>> explained above.
>>>>
>>>> Apologies for misunderstanding the context. But I disagree about
>>>> hvmloader. Returning "denied" from xen_version hypercall to guests is
>>>> one thing, but hvmloader and SMBios tables are parts of the hypervisor
>>>> and putting "denied" there is simply a terrible user experience.
>>>
>>> I am not going to comment on the user experience because this is up to
>>> another bikeshedding. The question is which string will you use? Why an
>>> empty string over something different?
>>>
>>> However, if an admin has control over the "deny" string, why would he
>>> ever want to filter it in hvmloader? Why can't he just replace the one
>>> in Kconfig?
>>
>> Most admins don't compile their own version of Xen...
> 
> Those admins are also unlikely going to build there own hvmloader, right?
> 
> Therefore, they will have to accept whatever string is reported by
> HVMLoader (or Xen). As you already allow Xen to configure it, why would
> that be a problem to change the one in Kconfig? Why do you need to fix
> it up in hvmloader as well?

Right, the idea was perhaps as upstream, we should modify hvmloader to
*always* replace "<denied>" with "".  (Or potentially with a more benign
string, like "hypervisor build unknown".)

 -George

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

Re: [Xen-devel] [PATCH v3 2/2] xsm: hide detailed Xen version from unprivileged guests

[Jan Beulich]

On 23.01.2020 15:52, Julien Grall wrote:
> Therefore, they will have to accept whatever string is reported by 
> HVMLoader (or Xen). As you already allow Xen to configure it, why would 
> that be a problem to change the one in Kconfig? Why do you need to fix 
> it up in hvmloader as well?

Because, as stated before, hvmloader is actually the presentation
layer from the guest firmware pov. Hence what is sensibly coming
back as "<denied>" or "<hidden>" from the hypercall should not
propagate into the firmware tables the guest gets to see. Other
users of the hypercall may very well leave these strings
unfiltered, such that to consumers it's clear what has happened
(and from other context it would then typically also be clear
what exact piece of information it is which has got hidden).

Jan

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