Re: [PATCH v1 1/4] xsm/libxl/xen_version: Add XSM for some of the xen_version commands.

[Martin Pohlack <mpohlack@amazon.com>]

On 09.10.2015 04:56, Konrad Rzeszutek Wilk wrote:
> The XENVER_[compile_info|changeset|commandline] are now
> guarded by an XSM check.
> 
> The rest: XENVER_[version|extraversion|capabilities|
> parameters|get_features|page_size|guest_handle] behave
> as before (no XSM check).

I think extraversion should also be guarded, it helps too much deducing
specific builds.

> We allow the initial domain to see these while the other
> guests are not permitted.
> 
> As such we also modify the toolstack such that if we fail
> to get any data instead of printing (null) we just print "".
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  tools/flask/policy/policy/modules/xen/xen.te |  2 +-
>  tools/libxl/libxl.c                          |  3 +++
>  xen/common/kernel.c                          | 11 ++++++++++-
>  xen/include/xsm/dummy.h                      | 24 ++++++++++++++++++++++++
>  xen/include/xsm/xsm.h                        |  6 ++++++
>  xen/xsm/dummy.c                              |  1 +
>  xen/xsm/flask/hooks.c                        | 25 +++++++++++++++++++++++++
>  xen/xsm/flask/policy/access_vectors          |  2 ++
>  8 files changed, 72 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te
> index d35ae22..d0ad758 100644
> --- a/tools/flask/policy/policy/modules/xen/xen.te
> +++ b/tools/flask/policy/policy/modules/xen/xen.te
> @@ -86,7 +86,7 @@ allow dom0_t dom0_t:domain {
>  };
>  allow dom0_t dom0_t:domain2 {
>  	set_cpuid gettsc settsc setscheduler set_max_evtchn set_vnumainfo
> -	get_vnumainfo psr_cmt_op psr_cat_op
> +	get_vnumainfo psr_cmt_op psr_cat_op version_use
>  };
>  allow dom0_t dom0_t:resource { add remove };
>  
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 22bbc29..efa6462 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -5272,6 +5272,7 @@ const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
>      xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra);
>      info->xen_version_extra = strdup(u.xen_extra);
>  
> +    memset(&u.xen_cc, 0, sizeof(xen_compile_info_t));
>      xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc);
>      info->compiler = strdup(u.xen_cc.compiler);
>      info->compile_by = strdup(u.xen_cc.compile_by);
> @@ -5281,6 +5282,7 @@ const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
>      xc_version(ctx->xch, XENVER_capabilities, &u.xen_caps);
>      info->capabilities = strdup(u.xen_caps);
>  
> +    memset(&u.xen_cc, 0, sizeof(xen_changeset_info_t));
>      xc_version(ctx->xch, XENVER_changeset, &u.xen_chgset);
>      info->changeset = strdup(u.xen_chgset);
>  
> @@ -5289,6 +5291,7 @@ const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
>  
>      info->pagesize = xc_version(ctx->xch, XENVER_pagesize, NULL);
>  
> +    memset(&u.xen_commandline, 0, sizeof(xen_commandline_t));
>      xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline);
>      info->commandline = strdup(u.xen_commandline);
>  
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index 6a3196a..210ec99 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -13,6 +13,7 @@
>  #include <xen/nmi.h>
>  #include <xen/guest_access.h>
>  #include <xen/hypercall.h>
> +#include <xsm/xsm.h>
>  #include <asm/current.h>
>  #include <public/nmi.h>
>  #include <public/version.h>
> @@ -226,9 +227,17 @@ void __init do_initcalls(void)
>  /*
>   * Simple hypercalls.
>   */
> -
> +#define XENVER_CMD_XSM_CHECK    ( (1U << XENVER_compile_info) | \
> +                                  (1U << XENVER_changeset) | \
> +                                  (1U << XENVER_commandline) )
>  DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
> +    if ( ( 1 << cmd ) & XENVER_CMD_XSM_CHECK )

"1" -> "1U" as 3 lines above.

> +    {
> +        int rc = xsm_version_op(XSM_PRIV, cmd);
> +        if ( rc )
> +            return rc;
> +    }
>      switch ( cmd )
>      {
>      case XENVER_version:
> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> index 9fe372c..2ee102e 100644
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -720,4 +720,28 @@ static XSM_INLINE int xsm_pmu_op (XSM_DEFAULT_ARG struct domain *d, unsigned int
>      }
>  }
>  
> +#include <public/version.h>
> +static XSM_INLINE int xsm_version_op (XSM_DEFAULT_ARG uint32_t op)
> +{
> +    XSM_ASSERT_ACTION(XSM_PRIV);
> +    switch ( op )
> +    {
> +    case XENVER_compile_info:
> +    case XENVER_changeset:
> +    case XENVER_commandline:
> +        return xsm_default_action(XSM_PRIV, current->domain, NULL);
> +    case XENVER_version:
> +    case XENVER_extraversion:

See above.

> +    case XENVER_capabilities:
> +    case XENVER_platform_parameters:
> +    case XENVER_get_features:
> +    case XENVER_pagesize:
> +    case XENVER_guest_handle:
> +        /* The should _NEVER_ get here, but just in case. */
> +        return xsm_default_action(XSM_HOOK, current->domain, NULL);
> +    default:
> +        return -EPERM;
> +    }
> +}
> +
>  #endif /* CONFIG_X86 */
> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
> index ba3caed..cd3e9b8 100644
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -191,6 +191,7 @@ struct xsm_operations {
>      int (*ioport_mapping) (struct domain *d, uint32_t s, uint32_t e, uint8_t allow);
>      int (*pmu_op) (struct domain *d, unsigned int op);
>  #endif
> +    int (*version_op) (uint32_t cmd);
>  };
>  
>  #ifdef XSM_ENABLE
> @@ -727,6 +728,11 @@ static inline int xsm_pmu_op (xsm_default_t def, struct domain *d, unsigned int
>      return xsm_ops->pmu_op(d, op);
>  }
>  
> +static inline int xsm_version_op (xsm_default_t def, uint32_t op)
> +{
> +    return xsm_ops->version_op(op);
> +}
> +
>  #endif /* CONFIG_X86 */
>  
>  #endif /* XSM_NO_WRAPPERS */
> diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
> index 72eba40..6105c07 100644
> --- a/xen/xsm/dummy.c
> +++ b/xen/xsm/dummy.c
> @@ -162,4 +162,5 @@ void xsm_fixup_ops (struct xsm_operations *ops)
>      set_to_dummy_if_null(ops, ioport_mapping);
>      set_to_dummy_if_null(ops, pmu_op);
>  #endif
> +    set_to_dummy_if_null(ops, version_op);
>  }
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index 4180f3b..4f1269d 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -26,6 +26,7 @@
>  #include <public/xen.h>
>  #include <public/physdev.h>
>  #include <public/platform.h>
> +#include <public/version.h>
>  
>  #include <public/xsm/flask_op.h>
>  
> @@ -1621,6 +1622,29 @@ static int flask_pmu_op (struct domain *d, unsigned int op)
>  }
>  #endif /* CONFIG_X86 */
>  
> +static int flask_version_op (uint32_t op)
> +{
> +    switch ( op )
> +    {
> +    case XENVER_compile_info:
> +    case XENVER_changeset:
> +    case XENVER_commandline:
> +        return current_has_perm(current->domain, SECCLASS_DOMAIN2,
> +                                DOMAIN2__VERSION_USE);
> +    case XENVER_version:
> +    case XENVER_extraversion:

See above.

> +    case XENVER_capabilities:
> +    case XENVER_platform_parameters:
> +    case XENVER_get_features:
> +    case XENVER_pagesize:
> +    case XENVER_guest_handle:
> +        /* The XSM check is only for a subset of ops. The rest are allowed. */
> +        return 0;
> +    default:
> +        return -EPERM;
> +    }
> +}
> +
>  long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
>  int compat_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
>  
> @@ -1759,6 +1783,7 @@ static struct xsm_operations flask_ops = {
>      .ioport_mapping = flask_ioport_mapping,
>      .pmu_op = flask_pmu_op,
>  #endif
> +    .version_op = flask_version_op,
>  };
>  
>  static __init void flask_init(void)
> diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
> index effb59f..6e2e5d1 100644
> --- a/xen/xsm/flask/policy/access_vectors
> +++ b/xen/xsm/flask/policy/access_vectors
> @@ -242,6 +242,8 @@ class domain2
>      mem_sharing
>  # XEN_DOMCTL_psr_cat_op
>      psr_cat_op
> +# XENVER_[compile_info|changeset|commandline] usage. The rest is not checked.
> +   version_use

See above (+XENVER_extraversion).

>  }
>  
>  # Similar to class domain, but primarily contains domctls related to HVM domains
> 

Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B