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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Nov 10, 2015 at 02:51:35PM -0500, Daniel De Graaf wrote:
> 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.

<nods>
> 
> [...]
> >  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.

<nods>
> 
> >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.

Ok, modified.
> 
> -- 
> Daniel De Graaf
> National Security Agency