From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v1 5/5] xsplice: Use ld-embedded build-ids Date: Thu, 17 Sep 2015 10:35:32 +0100 Message-ID: <55FA8964.3010602@citrix.com> References: <1442437276-2620-1-git-send-email-konrad.wilk@oracle.com> <1442437276-2620-6-git-send-email-konrad.wilk@oracle.com> <55F9E206.6060508@citrix.com> <55F9EDA8.1010206@citrix.com> <55FA60A9.8060202@amazon.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZcVbS-0007pR-0p for xen-devel@lists.xenproject.org; Thu, 17 Sep 2015 09:35:42 +0000 In-Reply-To: <55FA60A9.8060202@amazon.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Martin Pohlack , Konrad Rzeszutek Wilk , xen-devel@lists.xenproject.org, msw@amazon.com, aliguori@amazon.com, amesserl@rackspace.com, rick.harris@rackspace.com, paul.voccio@rackspace.com, steven.wilson@rackspace.com, major.hayden@rackspace.com, josh.kearney@rackspace.com, jinsong.liu@alibaba-inc.com, xiantao.zxt@alibaba-inc.com, boris.ostrovsky@oracle.com, daniel.kiper@oracle.com, elena.ufimtseva@oracle.com, bob.liu@oracle.com, lars.kurth@citrix.com, hanweidong@huawei.com, peter.huangpeng@huawei.com, fanhenglong@huawei.com, liuyingdong@huawei.com, john.liuqiming@huawei.com, jbeulich@suse.com, ian.campbell@citrix.com Cc: Martin Pohlack List-Id: xen-devel@lists.xenproject.org On 17/09/15 07:41, Martin Pohlack wrote: > On 17.09.2015 00:31, Andrew Cooper wrote: >> On 16/09/2015 22:59, Konrad Rzeszutek Wilk wrote: >>> On September 16, 2015 5:41:26 PM EDT, Andrew Cooper wrote: >>>> On 16/09/2015 22:01, Konrad Rzeszutek Wilk wrote: >>>>> From: Martin Pohlack >>>>> >>>>> The mechanism to get this is via the XSPLICE_OP and >>>>> we add a new subsequent hypercall to retrieve the >>>>> binary build-id. The hypercall allows an arbirarty >>>>> size (the buffer is provided to the hypervisor) - however >>>>> by default the toolstack will allocate it up to 128 >>>>> bytes. >>>>> >>>>> We also add two places for the build-id to be printed: >>>>> - xsplice keyhandler. We cannot use 'hh' in the hypervisor >>>>> snprintf handler (as it is not implemented) so instead >>>>> we use an simpler way to print it. >>>>> - In the 'xen-xsplice' tool add an extra parameter - build-id >>>>> to print this as an human readable value. >>>>> >>>>> Note that one can also retrieve the value by 'readelf -h xen-syms'. >>>>> >>>>> Signed-off-by: Martin Pohlack >>>>> Signed-off-by: Konrad Rzeszutek Wilk >>>>> --- >>>>> tools/libxc/include/xenctrl.h | 1 + >>>>> tools/libxc/xc_misc.c | 26 +++++++++++++ >>>>> tools/misc/xen-xsplice.c | 39 ++++++++++++++++++++ >>>>> xen/arch/x86/Makefile | 4 +- >>>>> xen/arch/x86/xen.lds.S | 5 +++ >>>>> xen/common/xsplice.c | 86 >>>> +++++++++++++++++++++++++++++++++++++++++++ >>>>> xen/include/public/sysctl.h | 18 +++++++++ >>>>> xen/include/xen/version.h | 1 + >>>>> 8 files changed, 178 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/tools/libxc/include/xenctrl.h >>>> b/tools/libxc/include/xenctrl.h >>>>> index 2cd982d..946ddc0 100644 >>>>> --- a/tools/libxc/include/xenctrl.h >>>>> +++ b/tools/libxc/include/xenctrl.h >>>>> @@ -2860,6 +2860,7 @@ int xc_xsplice_apply(xc_interface *xch, char >>>> *id); >>>>> int xc_xsplice_revert(xc_interface *xch, char *id); >>>>> int xc_xsplice_unload(xc_interface *xch, char *id); >>>>> int xc_xsplice_check(xc_interface *xch, char *id); >>>>> +int xc_xsplice_build_id(xc_interface *xch, char *build_id, unsigned >>>> int max); >>>> >>>> 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. * 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. ~Andrew