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 12:28:50PM -0400, Daniel De Graaf wrote:
> On 17/09/15 14: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.
> >
> >>* 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.
> 
> With XSM enabled, I think the correct thing to do is to have a distinct
> permission so that an admin can do per-VM controls.  Without XSM enabled,
> how common is the case where some VMs need to get this information and
> some need it hidden?  A global (command line controlled?) enable of the
> feature for domUs seems like a reasonable solution if this is uncommon.

The one (not in the tree) case we want to add is to expose the build-id
 which is an ldd linker type data. The right place to do it is in XENVER_
but having it exposed to the initial domain only makes sense.

However if you have XSM you could use that to expose it to other guests
as well?

But if we are going to add XSM we may as well add XSM for the other ones.
My preference would be to (which I believe is what you think is correct to):

 1). Allow some of the XENVER_ subops to be presented to guests without
     any XSM calls (aka, as it is now).

 2). The other ones (the existing ones), would be bundled under XSM_OTHER.

 3). The new one (build-id) would be XSM_PRIV  and if XSM is
     disabled would be only available for initial domain.

The XSM_PRIV is used in sysctl and we would expand the 'struct xsm_operations'
to have the 'build-id' member.

Hows that?

> 
> As far as the xsm_default_t value, this is really what XSM_OTHER is for,
> but if there are going to be many instances of this type of data, a new
> value like XSM_PRIV_INFOLEAK could be introduced.
> 
> -- 
> Daniel De Graaf
> National Security Agency