Re: [Xen-devel] [PATCH 11/25] xsm, argo: XSM control for argo register operation, argo_mac bootparam

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Christopher Clark [mailto:christopher.w.clark@xxxxxxxxx]
> Sent: 01 December 2018 01:33
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
> <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Jan
> Beulich <jbeulich@xxxxxxxx>; Julien Grall <julien.grall@xxxxxxx>; Konrad
> Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>; Paul Durrant
> <Paul.Durrant@xxxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>;
> Tim (Xen.org) <tim@xxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Daniel De
> Graaf <dgdegra@xxxxxxxxxxxxx>; Rich Persaud <persaur@xxxxxxxxx>; Ross
> Philipson <ross.philipson@xxxxxxxxx>; Eric Chanudet
> <eric.chanudet@xxxxxxxxx>; James McKenzie <voreekf@xxxxxxxxxxxxx>; Jason
> Andryuk <jandryuk@xxxxxxxxx>; Daniel Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> Subject: [PATCH 11/25] xsm, argo: XSM control for argo register operation,
> argo_mac bootparam
> 
> XSM hooks implement distinct permissions for these two distinct cases of
> Argo ring registration:
> 
> * Single source:  registering a ring for communication to receive messages
>                   from a specified single other domain.
>   Default policy: allow.
> 
> * Any source:     registering a ring for communication to receive messages
>                   from any, or all, other domains (ie. wildcard).
>   Default policy: deny, with runtime policy configuration via new
> bootparam.
> 
> The reason why the default for wildcard rings is 'deny' is that there is
> currently no means other than XSM to protect the ring from DoS by a noisy
> domain spamming the ring, reducing the ability of other domains to send to
> it.
> Using XSM at least allows per-domain control over access to the send
> permission, to limit communication to domains that can be trusted.
> 
> Since denying access to any-sender rings unless a flask XSM policy is
> active
> will prevent many users from using a key Argo feature, also introduce a
> bootparam
> that can override this constraint:
>  "argo_mac" variable has allowed values: 'permissive' and 'enforcing'.
> Even though this is a boolean variable, use these descriptive strings in
> order
> to make it obvious to an administrator that this has potential security
> impact.
> 
> Signed-off-by: Christopher Clark <christopher.clark6@xxxxxxxxxxxxxx>
> ---
>  xen/common/argo.c                     | 15 +++++++++++++++
>  xen/include/xsm/dummy.h               | 15 +++++++++++++++
>  xen/include/xsm/xsm.h                 | 17 +++++++++++++++++
>  xen/xsm/dummy.c                       |  4 ++++
>  xen/xsm/flask/hooks.c                 | 19 +++++++++++++++++++
>  xen/xsm/flask/policy/access_vectors   | 11 +++++++++++
>  xen/xsm/flask/policy/security_classes |  1 +
>  7 files changed, 82 insertions(+)
> 
> diff --git a/xen/common/argo.c b/xen/common/argo.c
> index 82fab36..2a95e09 100644
> --- a/xen/common/argo.c
> +++ b/xen/common/argo.c
> @@ -32,6 +32,21 @@ DEFINE_XEN_GUEST_HANDLE(argo_ring_t);
>  static bool __read_mostly opt_argo_enabled = 0;
>  boolean_param("argo", opt_argo_enabled);
> 
> +/* Xen command line option for conservative or relaxed access control */
> +bool __read_mostly argo_mac_bootparam_enforcing = true;
> +
> +static int __init parse_argo_mac_param(const char *s)
> +{
> +    if ( !strncmp(s, "enforcing", 10) )
> +        argo_mac_bootparam_enforcing = true;
> +    else if ( !strncmp(s, "permissive", 11) )
> +        argo_mac_bootparam_enforcing = false;
> +    else

Do you really want to parse e.g. 'enforcingfoobar' as 'enforcing'?

  Paul

> +        return -EINVAL;
> +    return 0;
> +}
> +custom_param("argo_mac", parse_argo_mac_param);
> +
>  struct argo_pending_ent
>  {
>      struct hlist_node node;
> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> index a29d1ef..55113c3 100644
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -720,6 +720,21 @@ static XSM_INLINE int xsm_dm_op(XSM_DEFAULT_ARG
> struct domain *d)
> 
>  #endif /* CONFIG_X86 */
> 
> +#ifdef CONFIG_ARGO
> +static XSM_INLINE int xsm_argo_register_single_source(struct domain *d,
> +                                                      struct domain *t)
> +{
> +    return 0;
> +}
> +
> +static XSM_INLINE int xsm_argo_register_any_source(struct domain *d,
> +                                                   bool strict)
> +{
> +    return strict ? -EPERM : 0;
> +}
> +
> +#endif /* CONFIG_ARGO */
> +
>  #include <public/version.h>
>  static XSM_INLINE int xsm_xen_version (XSM_DEFAULT_ARG uint32_t op)
>  {
> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
> index 3b192b5..65577fd 100644
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -181,6 +181,10 @@ struct xsm_operations {
>  #endif
>      int (*xen_version) (uint32_t cmd);
>      int (*domain_resource_map) (struct domain *d);
> +#ifdef CONFIG_ARGO
> +    int (*argo_register_single_source) (struct domain *d, struct domain
> *t);
> +    int (*argo_register_any_source) (struct domain *d);
> +#endif
>  };
> 
>  #ifdef CONFIG_XSM
> @@ -698,6 +702,19 @@ static inline int
> xsm_domain_resource_map(xsm_default_t def, struct domain *d)
>      return xsm_ops->domain_resource_map(d);
>  }
> 
> +#ifdef CONFIG_ARGO
> +static inline xsm_argo_register_single_source(struct domain *d, struct
> domain *t)
> +{
> +    return xsm_ops->argo_register_single_source(d, t);
> +}
> +
> +static inline xsm_argo_register_any_source(struct domain *d, bool strict)
> +{
> +    return xsm_ops->argo_register_any_source(d);
> +}
> +
> +#endif /* CONFIG_ARGO */
> +
>  #endif /* XSM_NO_WRAPPERS */
> 
>  #ifdef CONFIG_MULTIBOOT
> diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
> index 5701047..ed236b0 100644
> --- a/xen/xsm/dummy.c
> +++ b/xen/xsm/dummy.c
> @@ -152,4 +152,8 @@ void __init xsm_fixup_ops (struct xsm_operations *ops)
>  #endif
>      set_to_dummy_if_null(ops, xen_version);
>      set_to_dummy_if_null(ops, domain_resource_map);
> +#ifdef CONFIG_ARGO
> +    set_to_dummy_if_null(ops, argo_register_single_source);
> +    set_to_dummy_if_null(ops, argo_register_any_source);
> +#endif
>  }
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index 96d31aa..3166561 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1717,6 +1717,21 @@ static int flask_domain_resource_map(struct domain
> *d)
>      return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__RESOURCE_MAP);
>  }
> 
> +#ifdef CONFIG_ARGO
> +static int flask_argo_register_single_source(struct domain *d,
> +                                             struct domain *t)
> +{
> +    return domain_has_perm(d, t, SECCLASS_ARGO,
> +                           ARGO__REGISTER_SINGLE_SOURCE);
> +}
> +
> +static int flask_argo_register_any_source(struct domain *d)
> +{
> +    return avc_has_perm(domain_sid(d), SECINITSID_XEN, SECCLASS_ARGO,
> +                        ARGO__REGISTER_ANY_SOURCE, NULL);
> +}
> +#endif
> +
>  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);
> 
> @@ -1851,6 +1866,10 @@ static struct xsm_operations flask_ops = {
>  #endif
>      .xen_version = flask_xen_version,
>      .domain_resource_map = flask_domain_resource_map,
> +#ifdef CONFIG_ARGO
> +    .argo_register_single_source = flask_argo_register_single_source,
> +    .argo_register_any_source = flask_argo_register_any_source,
> +#endif
>  };
> 
>  void __init flask_init(const void *policy_buffer, size_t policy_size)
> diff --git a/xen/xsm/flask/policy/access_vectors
> b/xen/xsm/flask/policy/access_vectors
> index 6fecfda..fb95c97 100644
> --- a/xen/xsm/flask/policy/access_vectors
> +++ b/xen/xsm/flask/policy/access_vectors
> @@ -531,3 +531,14 @@ class version
>  # Xen build id
>      xen_build_id
>  }
> +
> +# Class argo is used to describe the Argo interdomain communication
> system.
> +class argo
> +{
> +    # Domain requesting registration of a communication ring
> +    # to receive messages from a specific other domain.
> +    register_single_source
> +    # Domain requesting registration of a communication ring
> +    # to receive messages from any other domain.
> +    register_any_source
> +}
> diff --git a/xen/xsm/flask/policy/security_classes
> b/xen/xsm/flask/policy/security_classes
> index cde4e1a..50ecbab 100644
> --- a/xen/xsm/flask/policy/security_classes
> +++ b/xen/xsm/flask/policy/security_classes
> @@ -19,5 +19,6 @@ class event
>  class grant
>  class security
>  class version
> +class argo
> 
>  # FLASK
> --
> 2.1.4


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