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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Sep 22, 2015 at 02:33:23PM +0100, Andrew Cooper wrote:
> 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.

I am worried about some spinlock in the depths of XSM code.

But then I haven't looked in detail so perhaps this is not an issue after all.

> 
> 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.

But making something slower is surely not something we strive for.

> 
> ~Andrew