Re: [PATCH v2 1/3] xsm/xen_version: Add XSM for the xen_version hypercall.

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 06/11/15 14:36, Konrad Rzeszutek Wilk wrote:
> All of XENVER_* have now an XSM check.
>
> The subops for XENVER_[compile_info|changeset|commandline|
> extraversion] are now priviliged operations. To not break
> guests we still return an string - but it is just '<denied>'.
>
> The rest: XENVER_[version|capabilities|
> parameters|get_features|page_size|guest_handle] behave
> as before - allowed by default for all guests.
>
> This is with the XSM default policy and with the dummy ones.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Comments below, inline.

[...]
> diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te
> index d35ae22..1ca0e65 100644
> --- a/tools/flask/policy/policy/modules/xen/xen.te
> +++ b/tools/flask/policy/policy/modules/xen/xen.te
> @@ -73,6 +73,12 @@ allow dom0_t xen_t:xen2 {
>       pmu_ctrl
>       get_symbol
>   };
> +
> +# Allow dom0 to use XENVER_compile_info|changeset|commandline]extraversion
> +allow dom0_t xen_t:xen2 {
> +    version_priv
> +};
> +
>   allow dom0_t xen_t:mmu memorymap;
>
>   # Allow dom0 to use these domctls on itself. For domctls acting on other

Minor tweak: if you don't want to add the new to the block a few lines above,
the one-line permission syntax without braces (as seen below) looks better.

[...]
>   DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>   {
> +    bool_t deny = !!xsm_version_op(XSM_HOOK, cmd);
> +

Since this call produces denials in the default policy, it should be marked
as XSM_OTHER.

> diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
> index effb59f..273459f 100644
> --- a/xen/xsm/flask/policy/access_vectors
> +++ b/xen/xsm/flask/policy/access_vectors
> @@ -93,6 +93,8 @@ class xen2
>       pmu_ctrl
>   # PMU use (domains, including unprivileged ones, will be using this operation)
>       pmu_use
> +# XENVER_[compile_info|changeset|commandline|extraversion] usage.
> +   version_priv
>   }
>
>   # Classes domain and domain2 consist of operations that a domain performs on
> @@ -242,6 +244,8 @@ class domain2
>       mem_sharing
>   # XEN_DOMCTL_psr_cat_op
>       psr_cat_op
> +# XENVER_[version|capabilities|parameters|get_features|page_size|guest_handle].
> +    version_use
>   }
>
>   # Similar to class domain, but primarily contains domctls related to HVM domains
>

I think that both version_priv and version_use belong in the same access
vector (xen2) rather than placing version_use in domain2.

-- 
Daniel De Graaf
National Security Agency