Re: [Xen-devel] [PATCH v2 2/5] xen/libxc: Move TMEM_AUTH to XEN_SYSCTL_TMEM_OP_SET_AUTH

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Apr 04, 2017 at 03:10:14PM -0400, Konrad Rzeszutek Wilk wrote:
> which surprisingly (or maybe not) looks like
> XEN_SYSCTL_TMEM_OP_SET_POOLS.
> 
> This hypercall came about, as explained in docs/misc/tmem-internals.html:
> 
> When tmem was first proposed to the linux kernel mailing list
> (LKML), there was concern expressed about security of shared ephemeral
> pools.  The initial tmem implementation only
> required a client to provide a 128-bit UUID to identify a shared pool, and the
> linux-side tmem implementation obtained this UUID from the superblock of the
> shared filesystem (in ocfs2).  It was
> pointed out on LKML that the UUID was essentially a security key and any
> malicious domain that guessed it would have access to any data from the shared
> filesystem that found its way into tmem.
> ..
> As a result, a Xen boot option -- tmem_shared_auth; -- was
> added.  The option defaults to disabled,
> but when it is enabled, management tools must explicitly authenticate (or may
> explicitly deny) shared pool access to any client.
> On Xen, this is done with the xm tmem-shared-auth command.
> "
> 
> However the implementation has some rather large holes:
> 
> a) The hypercall was accessed from any guest.
> 
> b) If the ->domain id value is 0xFFFF then one can toggle the
>    tmem_global.shared_auth knob on/off. That with a)
>    made it pretty bad.
> 
> c) If one toggles the tmem_global.shared_auth off, then the
>   'tmem_shared_auth=1' bootup parameter is ignored and
>    one can join any shared pool (if UUID is known)!
> 
> d) If the 'tmem_shared_auth=1' and tmem_global.shared_auth is
>   set to 1, then one can only join an shared pool if the
>   UUID has been set by 'xl tmem-shared-auth'. Otherwise
>   the joining of a pool fails and a non-shared pool is
>   created (without errors to guest). Not exactly sure if
>   the shared pool creation at that point should error out
>   or not.
> 
> e) If a guest is migrated, the policy values (which UUID
>   can be shared, whether tmem_global.shared_auth is set, etc)
>   are completely ignored.
> 
> This patch only fixes a) and only allows the hypercall to
> be called by the control domain. Subsequent patches will
> fix the remaining issues.
> 
> We also have to call client_create as the guest at this
> point may not have done any tmem hypercalls - and hence
> the '->tmem' from 'struct domain' is still NULL. Us calling
> client_create fixes this.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> 
> ---
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> 
> v1: First posting
> v2: Drop 'idx' in tmemc_auth_pools
>     Check for 'n_pages' in tmemc_auth_pools.
>     Update comment for xen_tmem_pool_info_t.
> ---
>  tools/libxc/include/xenctrl.h  |  2 +-
>  tools/libxc/xc_tmem.c          | 52 
> +++++++++++++-----------------------------

Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>

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