Re: Is: Make XENVER_* use XSM, seperate the different ops in smaller security domains. Was:Re: [PATCH v1 5/5] xsplice: Use ld-embedded build-ids

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 22/09/15 14:22, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 18, 2015 at 12:40:46PM +0100, Andrew Cooper wrote:
>> On 17/09/15 19:45, Konrad Rzeszutek Wilk wrote:
>>> . snip..
>>>>>>>> The build id of the current running hypervisor should belong in the
>>>>>>>> xeninfo hypercall.  It is not specific to xsplice.
>>>>>>> However in the previous reviews it was pointed out that it should only be accessible to dom0.
>>>>>>>
>>>>>>> Or to any domains as long as the XSM allows (and is turned on) - so not the default dummy one.
>>>>>>>
>>>>>>> That is a bit of 'if' extra complexity which I am not sure is worth it?
>>>>>> DomU can already read the build information such as changeset, compile
>>>>>> time, etc.  Build-id is no more special or revealing.
>>>>> I would take this as an argument *against* giving DomU access to those
>>>>> pieces of information in details and not as an argument for
>>>>> *additionally* giving it access to build-id.
>>>>>
>>>>> With build-id we have the chance to shape a not-yet-established API and
>>>>> I would like to follow the Principle of least privilege wherever it
>>>>> makes sense.
>>>>>
>>>>> To reach a similar security level with the existing API, it might make
>>>>> sense to limit DomU access to compile date, compile time, compiled by,
>>>>> compiled domain, compiler version and command line details, xen extra
>>>>> version, and xen changeset.  Basically anything that might help DomUs to
>>>>> uniquely identify a Xen build.
>>>>>
>>>>> The approach can not directly drop access to those fields, as that would
>>>>> break an existing API, but it could restrict the detail level handed out
>>>>> to DomU.
>>>> These are all valid arguments to be made, but please lets fix the issue
>>>> properly rather than hacking build-id on the side of an unrelated component.
>>>>
>>>>  From my point of view, the correct course of action is this:
>>>>
>>>> * Split the current XSM model to contain separate attributes for general
>>>> and privileged information.
>>>> ** For current compatibility, all existing XENVER_* subops fall into general
>>>> * Apply an XSM check at the very start of the hypercall.
> That would introduce a performance regression I fear. Linux pvops does this:
>
>                                                                                
> /*
>   * Force a proper event-channel callback from Xen after clearing the
>   * callback mask. We do this in a very simple manner, by making a call
>   * down into Xen. The pending flag will be checked by Xen on return.
>   */
> void xen_force_evtchn_callback(void)
> {
>          (void)HYPERVISOR_xen_version(0, NULL);
> }
>
> quite often, which now will have to do the XSM check which is extra code.
>
>
> I would say that the XENVER_compile_info (/sys/hypervisor/compilation),
> XENVER_changeset (/sys/hypervisor/compilation) should go over
> the XSM check.
>
> While:XENVER_version, XENVER_extraversion,XENVER_capabilities,
> XENVER_platform_parameters, XENVER_get_features,XENVER_pagesize
>
> should have no XSM check.

The XSM check will fall into the noise, performance wise, compared to 
the context switch to make the hypercall in the first place.  It is just 
another switch statement.  Also, selectively applying XSM checks will 
incur even more overhead than doing a blanket XSM check.

Also, I really don't care if you can measure a performance hit (not that 
I reckon you could).  How Linux chooses to behave itself has absolutely 
no bearing on how we go about securing the hypercall.

~Andrew