Re: [RFC PATCH] flask: Remove magic SID setting

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 7/6/22 15:13, Jason Andryuk wrote:
> flask_domain_alloc_security and flask_domain_create has special code to
> magically label dom0 as dom0_t.  This can all be streamlined by making
> create_dom0 set ssidref before creating dom0.
Hmm, I wouldn't call it magical, it is the initialization policy for a 
domain labeling, which is specific to each policy module. I considered 
this approach already and my concern here is two fold. First, it now 
hard codes the concept of dom0 vs domU into the XSM API. There is an 
ever growing desire by solution providers to not have a dom0 and at most 
have a hardware domain if at all and this is a step backwards from that 
movement. Second, and related, is this now pushes the initial label 
policy up into the domain builder code away from the policy module and 
spreads it out. Hopefully Xen will evolve to have a richer set of 
initial domains and an appropriate initial label policy will be needed 
for this case. This approach will result in having to continually expand 
the XSM API for each new initial domain type.
> create_domU is also extended to create domains with domU_t.
>
> xsm_ssidref_domU and xsm_ssidref_dom0 are introduced to abstract away
> the details.
>
> Signed-off-by: Jason Andryuk <jandryuk@xxxxxxxxx>
> ---
> Untested on ARM.  Minimally tested on x86.  Needs your Flask permission
> changes for xenboot_t to create dom0_t and domU_t.
>
> This is what I was thinking would be a better way to handle SID
> assignment.
>
> Regards,
> Jason
> ---
>   xen/arch/arm/domain_build.c |  2 ++
>   xen/arch/x86/setup.c        |  1 +
>   xen/include/xsm/dummy.h     | 10 ++++++++++
>   xen/include/xsm/xsm.h       | 12 ++++++++++++
>   xen/xsm/dummy.c             |  2 ++
>   xen/xsm/flask/hooks.c       | 31 +++++++++++++++++--------------
>   6 files changed, 44 insertions(+), 14 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 3fd1186b53..a7e88944c2 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -3281,6 +3281,7 @@ void __init create_domUs(void)
>               .max_grant_frames = -1,
>               .max_maptrack_frames = -1,
>               .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
> +            .ssidref = xsm_ssidref_domU(),
>           };
>           unsigned int flags = 0U;
>   
> @@ -3438,6 +3439,7 @@ void __init create_dom0(void)
>           .max_grant_frames = gnttab_dom0_frames(),
>           .max_maptrack_frames = -1,
>           .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
> +        .ssidref = xsm_ssidref_dom0(),
>       };
>   
>       /* The vGIC for DOM0 is exactly emulating the hardware GIC */
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index f08b07b8de..5a6086cfe3 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -771,6 +771,7 @@ static struct domain *__init create_dom0(const module_t 
> *image,
>           .arch = {
>               .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
>           },
> +        .ssidref = xsm_ssidref_dom0(),
>       };
>       struct domain *d;
>       char *cmdline;
> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> index 77f27e7163..12fbc224d0 100644
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -124,6 +124,16 @@ static XSM_INLINE void cf_check xsm_security_domaininfo(
>       return;
>   }
>   
> +static XSM_INLINE int cf_check xsm_ssidref_dom0(XSM_DEFAULT_VOID)
> +{
> +    return 0;
> +}
> +
> +static XSM_INLINE int cf_check xsm_ssidref_domU(XSM_DEFAULT_VOID)
> +{
> +    return 0;
> +}
> +
>   static XSM_INLINE int cf_check xsm_domain_create(
>       XSM_DEFAULT_ARG struct domain *d, uint32_t ssidref)
>   {
> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
> index 8dad03fd3d..a6a4ffe05a 100644
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -55,6 +55,8 @@ struct xsm_ops {
>       int (*set_system_active)(void);
>       void (*security_domaininfo)(struct domain *d,
>                                   struct xen_domctl_getdomaininfo *info);
> +    int (*ssidref_dom0)(void);
> +    int (*ssidref_domU)(void);
>       int (*domain_create)(struct domain *d, uint32_t ssidref);
>       int (*getdomaininfo)(struct domain *d);
>       int (*domctl_scheduler_op)(struct domain *d, int op);
> @@ -220,6 +222,16 @@ static inline void xsm_security_domaininfo(
>       alternative_vcall(xsm_ops.security_domaininfo, d, info);
>   }
>   
> +static inline int xsm_ssidref_dom0(void)
> +{
> +    return alternative_call(xsm_ops.ssidref_dom0);
> +}
> +
> +static inline int xsm_ssidref_domU(void)
> +{
> +    return alternative_call(xsm_ops.ssidref_domU);
> +}
> +
>   static inline int xsm_domain_create(
>       xsm_default_t def, struct domain *d, uint32_t ssidref)
>   {
> diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
> index e6ffa948f7..d46cfef0ec 100644
> --- a/xen/xsm/dummy.c
> +++ b/xen/xsm/dummy.c
> @@ -16,6 +16,8 @@
>   static const struct xsm_ops __initconst_cf_clobber dummy_ops = {
>       .set_system_active             = xsm_set_system_active,
>       .security_domaininfo           = xsm_security_domaininfo,
> +    .ssidref_dom0                  = xsm_ssidref_dom0,
> +    .ssidref_domU                  = xsm_ssidref_domU,
>       .domain_create                 = xsm_domain_create,
>       .getdomaininfo                 = xsm_getdomaininfo,
>       .domctl_scheduler_op           = xsm_domctl_scheduler_op,
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index 8c9cd0f297..d6f786ea84 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -544,26 +544,27 @@ static void cf_check flask_security_domaininfo(
>       info->ssidref = domain_sid(d);
>   }
>   
> +static int cf_check flask_ssidref_dom0(void)
> +{
> +    return SECINITSID_DOM0;
> +}
> +
> +static int cf_check flask_ssidref_domU(void)
> +{
> +    return SECINITSID_DOMU;
> +}
> +
>   static int cf_check flask_domain_create(struct domain *d, uint32_t ssidref)
>   {
>       int rc;
>       struct domain_security_struct *dsec = d->ssid;
> -    static int dom0_created = 0;
>   
> -    if ( is_idle_domain(current->domain) && !dom0_created )
> -    {
> -        dsec->sid = SECINITSID_DOM0;
> -        dom0_created = 1;
> -    }
> -    else
> -    {
> -        rc = avc_current_has_perm(ssidref, SECCLASS_DOMAIN,
> -                          DOMAIN__CREATE, NULL);
> -        if ( rc )
> -            return rc;
> +    rc = avc_current_has_perm(ssidref, SECCLASS_DOMAIN,
> +                      DOMAIN__CREATE, NULL);
> +    if ( rc )
> +        return rc;
>   
> -        dsec->sid = ssidref;
> -    }
> +    dsec->sid = ssidref;
>       dsec->self_sid = dsec->sid;
>   
>       rc = security_transition_sid(dsec->sid, dsec->sid, SECCLASS_DOMAIN,
> @@ -1805,6 +1806,8 @@ static int cf_check flask_argo_send(
>   static const struct xsm_ops __initconst_cf_clobber flask_ops = {
>       .set_system_active = flask_set_system_active,
>       .security_domaininfo = flask_security_domaininfo,
> +    .ssidref_dom0 = flask_ssidref_dom0,
> +    .ssidref_domU = flask_ssidref_domU,
>       .domain_create = flask_domain_create,
>       .getdomaininfo = flask_getdomaininfo,
>       .domctl_scheduler_op = flask_domctl_scheduler_op,