Re: [Xen-devel] [PATCH v1 6/6] xsm: add tee access policy support

[Julien Grall <julien.grall@xxxxxxx>]

Hi Volodymyr,

On 08/22/2018 03:11 PM, Volodymyr Babchuk wrote:
> As we don't want any guest to access limited resources of TEE,
> we need a way to control who can work with it.
>
> Thus, new access vector class "tee" is added with only ony operation
> "call" so far. tee framework uses this to check if guest has a right
> to work with TEE.
>
> Also, example security context domU_with_tee_t was added.
>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
> ---
>   tools/flask/policy/modules/dom0.te          |  3 +++
>   tools/flask/policy/modules/domU_with_tee.te | 23 +++++++++++++++++++++++
>   tools/flask/policy/modules/modules.conf     |  1 +
>   tools/flask/policy/modules/xen.if           | 12 ++++++++++++
>   xen/arch/arm/tee/tee.c                      | 10 ++++++++++
>   xen/include/xsm/dummy.h                     | 10 ++++++++++
>   xen/include/xsm/xsm.h                       | 13 +++++++++++++
>   xen/xsm/dummy.c                             |  4 ++++
>   xen/xsm/flask/hooks.c                       | 15 +++++++++++++++
>   xen/xsm/flask/policy/access_vectors         |  7 +++++++
>   xen/xsm/flask/policy/security_classes       |  1 +
>   11 files changed, 99 insertions(+)
>   create mode 100644 tools/flask/policy/modules/domU_with_tee.te
>
> diff --git a/tools/flask/policy/modules/dom0.te 
> b/tools/flask/policy/modules/dom0.te
> index 1643b40..8bac4f3 100644
> --- a/tools/flask/policy/modules/dom0.te
> +++ b/tools/flask/policy/modules/dom0.te
> @@ -27,6 +27,9 @@ allow dom0_t xen_t:version {
>         xen_build_id
>   };
>   
> +# Allow dom0 to work with TEE
> +allow dom0_t xen_t:tee call;
> +
>   allow dom0_t xen_t:mmu memorymap;
>   
>   # Allow dom0 to use these domctls on itself. For domctls acting on other
> diff --git a/tools/flask/policy/modules/domU_with_tee.te 
> b/tools/flask/policy/modules/domU_with_tee.te
> new file mode 100644
> index 0000000..4d856b2
> --- /dev/null
> +++ b/tools/flask/policy/modules/domU_with_tee.te
> @@ -0,0 +1,23 @@
> +###############################################################################
> +#
> +# Domain creation
> +#
> +###############################################################################
> +
> +declare_domain(domU_with_tee_t)
> +domain_self_comms(domU_t)
> +create_domain(dom0_t, domU_with_tee_t)
> +manage_domain(dom0_t, domU_with_tee_t)
> +domain_comms(dom0_t, domU_with_tee_t)
> +domain_comms(domU_with_tee_t, domU_with_tee_t)
> +migrate_domain_out(dom0_t, domU_with_tee_t)
> +domain_self_comms(domU_with_tee_t)
> +
> +# This is required for PCI (or other device) passthrough
> +delegate_devices(dom0_t, domU_with_tee_t)
> +
> +# Both of these domain types can be created using the default (system) role
> +role system_r types { domU_with_tee_t dm_dom_t };
> +
> +# Allow to work with TEE
> +access_tee(domU_with_tee_t)
> diff --git a/tools/flask/policy/modules/modules.conf 
> b/tools/flask/policy/modules/modules.conf
> index 6dba0a3..9010d91 100644
> --- a/tools/flask/policy/modules/modules.conf
> +++ b/tools/flask/policy/modules/modules.conf
> @@ -29,6 +29,7 @@ domU = on
>   isolated_domU = on
>   prot_domU = on
>   nomigrate = on
> +domU_with_tee = on
>   
>   # Example device policy.  Also see policy/device_contexts.
>   nic_dev = on
> diff --git a/tools/flask/policy/modules/xen.if 
> b/tools/flask/policy/modules/xen.if
> index 5543749..e534179 100644
> --- a/tools/flask/policy/modules/xen.if
> +++ b/tools/flask/policy/modules/xen.if
> @@ -209,3 +209,15 @@ define(`admin_device', `
>   define(`delegate_devices', `
>       allow $1 $2:resource { add remove };
>   ')
> +
> +################################################################################
> +#
> +# Miscellaneous services
> +#
> +################################################################################
> +
> +# access_tee(domain)
> +
> +define(`access_tee', `
> +    allow $1 xen_t:tee call;
> +')
> diff --git a/xen/arch/arm/tee/tee.c b/xen/arch/arm/tee/tee.c
> index 6df3b09..06f5091 100644
> --- a/xen/arch/arm/tee/tee.c
> +++ b/xen/arch/arm/tee/tee.c
> @@ -19,6 +19,7 @@
>   #include <xen/errno.h>
>   #include <xen/types.h>
>   #include <asm/tee/tee.h>
> +#include <xsm/xsm.h>
>   
>   extern const struct tee_mediator_desc _steemediator[], _eteemediator[];
>   static const struct tee_mediator_ops *mediator_ops;
> @@ -42,6 +43,9 @@ bool tee_handle_smc(struct cpu_user_regs *regs)
>       if ( !mediator_ops )
>           return false;
>   
> +    if ( xsm_tee_op(XSM_PRIV, current->domain) )
> +        return false;
> +
>       return mediator_ops->handle_smc(regs);
>   }
>   
> @@ -50,6 +54,9 @@ int tee_domain_create(struct domain *d)
>       if ( !mediator_ops )
>           return -ENODEV;
>   
> +    if ( xsm_tee_op(XSM_PRIV, d) )
> +        return -EPERM;
I don't think we should use XSM to enforce the use of TEE. This 
contradictory to your next patch where you let the user configure OP-TEE 
for a given guest.
IHMO, XSM should only be used to restrict usage of calls in a fine 
grain. For an overall control, that should be go through a DOMCTL tell 
Xen to initialize OP-TEE for that domain.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel