Re: [Xen-devel] 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@xxxxxxxxxx>]

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.
>> * Extend do_xen_version() to take 3 parameters.  (It is curious that it
>> didn't take a length parameter before)
>> ** This is still ABI compatible, as existing subops simply ignore the
>> parameter.
> Or we can just use 1024 bytes space the XENVER_* use.

What 1024 bytes?

Each subop currently assumes the guest handle is a pointer to an
appropriately typed structure, which puts arbitrary and unnecessary
length restrictions on items.

~Andrew

>
>> * Introduce new XENVER_build_id subop which is documented to require the
>> 3-parameter version of the hypercall.
>> ** This subop falls into straight into privileged information.
>>
>> This will introduce build-id in its correct location, with appropriate
>> restrictions.
>>
>> Moving forwards, we really should have an audit of the existing XENVER_*
>> subops.  Some are clearly safe/required for domU to read, but some such
>> as XENVER_commandline have no business being accessible.  A separate
>> argument, from the repeatable build point of view, says that compilation
>> information isn't useful at all.
>>
>> Depending on how we wish to fix the issue, we could either do a blanket
>> move of the subops into the privileged XSM catagory, or introduce a 3rd
>> "legacy privileged" category to allow the admin to control access on a
>> per-vm basis.
> CC-ing Daniel. Changing title.
>> ~Andrew


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel