From mboxrd@z Thu Jan  1 00:00:00 1970
From: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Subject: Re: [PATCH v2 1/3] xsm/xen_version: Add XSM for the
 xen_version hypercall.
Date: Tue, 10 Nov 2015 14:51:35 -0500
Message-ID: <56424AC7.7040200@tycho.nsa.gov>
References: <1446838577-7563-1-git-send-email-konrad.wilk@oracle.com>
	<1446838577-7563-2-git-send-email-konrad.wilk@oracle.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta14.messagelabs.com ([193.109.254.103])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <dgdegra@tycho.nsa.gov>) id 1ZwExF-0001Sq-Cs
	for xen-devel@lists.xenproject.org; Tue, 10 Nov 2015 19:51:45 +0000
In-Reply-To: <1446838577-7563-2-git-send-email-konrad.wilk@oracle.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, xen-devel@lists.xenproject.org
Cc: wei.liu2@citrix.com, ian.campbell@citrix.com, andrew.cooper3@citrix.com, ian.jackson@eu.citrix.com, mpohlack@amazon.de, JBeulich@suse.com
List-Id: xen-devel@lists.xenproject.org

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