Re: [PATCH v4] common: dom0less-bindings: introduce XSM labels

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 12-Oct-26 12:15, Sergiy Kibrik wrote:
> Add "seclabel" property to be able to specify security label for a domain
> when XSM Flask is enabled, similar to xl configuration files.
> 
> Currently guest domain can't be created by Xen in dom0less configuration when
> Flask is enabled, as domain is assigned "system_u:system_r:unlabeled_t" label
> by default, which Flask denies to create according to current policy.
> 
> Because code from outside of flask can't directly execute its internal API
> a new XSM hook lookup_sid() introduced as part of XSM API exposed
> to rest of Xen.
> 
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
> CC: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: Michal Orzel <michal.orzel@xxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>
Please move the CC: lines below the --- so that they don't end up in the
commit message.

> ---
> changes in v4:
>  - introduce XSM hook `lookup_sid`
>  - at SID lookup return error if security server not initialized
>  - document panic if DTS property is used and FLASK disabled
>  - per Michal's request panic if FLASK disabled is left unchanged
> changes in v3:
>  - panic if `seclabel` property found but FLASK is disabled
> changes in v2:
>  - add & use flask_context_to_sid() wrapper
> ---
>  docs/misc/arm/device-tree/booting.txt      | 8 ++++++++
>  xen/common/device-tree/dom0less-bindings.c | 7 +++++++
>  xen/include/xsm/dummy.h                    | 7 +++++++
>  xen/include/xsm/hooks.h                    | 2 ++
>  xen/xsm/flask/hooks.c                      | 7 +++++++
>  5 files changed, 31 insertions(+)
> 
> diff --git a/docs/misc/arm/device-tree/booting.txt 
> b/docs/misc/arm/device-tree/booting.txt
> index bcb06bc796..2a69d2537d 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -345,6 +345,12 @@ with the following properties:
>      not passed. This configuration requires static allocation 
> (xen,static-mem)
>      and direct mapping (direct-map).
>  
> +- seclabel
> +
> +    A string property specifying an XSM security label to this domain. 
> Domains
> +    will be classified “unlabeled” if this property not specified.
NIT: Use regular ASCII quotes.
Also, s/property not/property is not/

> +    Note: boot will fail if this property is present but FLASK is disabled.
> +
>  Under the "xen,domain" compatible node, one or more sub-nodes are present
>  for the DomU kernel and ramdisk.
>  
> @@ -422,6 +428,7 @@ chosen {
>          memory = <0 131072>;
>          cpus = <2>;
>          vpl011;
> +        seclabel = "system_u:system_r:domU_t";
>  
>          vcpu0 {
>              compatible = "xen,vcpu";
> @@ -453,6 +460,7 @@ chosen {
>          #size-cells = <0x1>;
>          memory = <0 65536>;
>          cpus = <1>;
> +        seclabel = "system_u:system_r:domU_t";
>  
>          module@0x4c000000 {
>              compatible = "multiboot,kernel", "multiboot,module";
> diff --git a/xen/common/device-tree/dom0less-bindings.c 
> b/xen/common/device-tree/dom0less-bindings.c
> index 41d72d0d58..b03cee0e8c 100644
> --- a/xen/common/device-tree/dom0less-bindings.c
> +++ b/xen/common/device-tree/dom0less-bindings.c
> @@ -11,6 +11,8 @@
>  #include <public/bootfdt.h>
>  #include <public/domctl.h>
>  
> +#include <xsm/xsm.h>
> +
>  int __init parse_dom0less_node(struct dt_device_node *node,
>                                 struct boot_domain *bd)
>  {
> @@ -21,6 +23,7 @@ int __init parse_dom0less_node(struct dt_device_node *node,
>      bool has_dtb = false;
>      bool iommu = false;
>      const char *dom0less_iommu = NULL;
> +    const char *xsmlabel = NULL;
Please drop a dead assignment.

>  
>      if ( !dt_device_is_compatible(node, "xen,domain") )
>          return -ENOENT;
> @@ -141,5 +144,9 @@ int __init parse_dom0less_node(struct dt_device_node 
> *node,
>          panic("'llc-colors' found, but LLC coloring is disabled\n");
>  #endif
>  
> +    if ( !dt_property_read_string(node, "seclabel", &xsmlabel) &&
> +         xsm_lookup_sid(XSM_HOOK, xsmlabel, strlen(xsmlabel), 
> &d_cfg->ssidref) )
xsm_lookup_sid returns an error code that we could print on panic message.
Please do so.

> +        panic("Invalid security context for domain: %s\n", xsmlabel);
> +
>      return arch_parse_dom0less_node(node, bd);
>  }
> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> index 6331859afa..029cddd5cd 100644
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -823,4 +823,11 @@ static XSM_INLINE int xsm_domain_resource_map(
>      return xsm_default_action(action, current->domain, d);
>  }
>  
> +static XSM_INLINE int xsm_lookup_sid(
> +    XSM_DEFAULT_ARG const char *ctx, uint32_t len, uint32_t *sid)
> +{
We need XSM_ASSERT_ACTION as the top level comment requires.

With the changes above:
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

~Michal