| Message ID | 20220420222834.5478-3-dpsmith@apertussolutions.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Adds starting the idle domain privileged | expand |
On 21.04.2022 00:28, Daniel P. Smith wrote: > --- a/xen/xsm/flask/hooks.c > +++ b/xen/xsm/flask/hooks.c > @@ -168,7 +168,7 @@ static int cf_check flask_domain_alloc_security(struct domain *d) > switch ( d->domain_id ) > { > case DOMID_IDLE: > - dsec->sid = SECINITSID_XEN; > + dsec->sid = SECINITSID_XENBOOT; > break; > case DOMID_XEN: > dsec->sid = SECINITSID_DOMXEN; > @@ -188,6 +188,7 @@ static int cf_check flask_domain_alloc_security(struct domain *d) > > static void cf_check flask_transition_running(void) > { > + struct domain_security_struct *dsec; > struct domain *d = current->domain; > > if ( d->domain_id != DOMID_IDLE ) > @@ -198,6 +199,10 @@ static void cf_check flask_transition_running(void) > * set to false for the consistency check(s) in the setup code. > */ > d->is_privileged = false; > + > + dsec = d->ssid; > + dsec->sid = SECINITSID_XEN; > + dsec->self_sid = dsec->sid; > } If replacing SIDs is an okay thing to do, perhaps assert that the values haven't changed from SECINITSID_XENBOOT prior to replacing them? Jan
On 4/21/22 05:22, Jan Beulich wrote: > On 21.04.2022 00:28, Daniel P. Smith wrote: >> --- a/xen/xsm/flask/hooks.c >> +++ b/xen/xsm/flask/hooks.c >> @@ -168,7 +168,7 @@ static int cf_check flask_domain_alloc_security(struct domain *d) >> switch ( d->domain_id ) >> { >> case DOMID_IDLE: >> - dsec->sid = SECINITSID_XEN; >> + dsec->sid = SECINITSID_XENBOOT; >> break; >> case DOMID_XEN: >> dsec->sid = SECINITSID_DOMXEN; >> @@ -188,6 +188,7 @@ static int cf_check flask_domain_alloc_security(struct domain *d) >> >> static void cf_check flask_transition_running(void) >> { >> + struct domain_security_struct *dsec; >> struct domain *d = current->domain; >> >> if ( d->domain_id != DOMID_IDLE ) >> @@ -198,6 +199,10 @@ static void cf_check flask_transition_running(void) >> * set to false for the consistency check(s) in the setup code. >> */ >> d->is_privileged = false; >> + >> + dsec = d->ssid; >> + dsec->sid = SECINITSID_XEN; >> + dsec->self_sid = dsec->sid; >> } > > If replacing SIDs is an okay thing to do, perhaps assert that the > values haven't changed from SECINITSID_XENBOOT prior to replacing > them? Yes, changing a domain's SID is a legitimate action that could be done today via the FLASK_RELABEL_DOMAIN subop of xsm_op hypercall that ends up calling flask_relabel_domain(), when using flask policy. This is where Jason was concerned if I was going to be using that call to change the SID, which would require a policy rule to allow xenboot_t to relabel itself as xen_t. As flask works today, the system domains use initial SIDs which are effectively compile-time labels, which means the policy rule is a static, fixed rule, i.e. it is not possible to use a different set of labels, that must always be present. This also introduces the risk for a custom policy writer to inadvertently leave the xenboot_t to xen_t transitional rule out resulting in a failed access attempt which would lead to a panic. This is unnecessary pain when we can just handle the transition internal to the hypervisor as that where it is all really occurring. As for the ASSERT, that is a good point since there is a specific state we are expecting to enter the hook. Pair that with some thinking I have had to do in answering Jason, Roger, and yourself, I am going to rewire the hook to return a success/error return value and move the panic outside the check. v/r, dps
diff --git a/tools/flask/policy/modules/xen.if b/tools/flask/policy/modules/xen.if index 5e2aa472b6..4ec676fff1 100644 --- a/tools/flask/policy/modules/xen.if +++ b/tools/flask/policy/modules/xen.if @@ -62,6 +62,12 @@ define(`create_domain_common', ` setparam altp2mhvm altp2mhvm_op dm }; ') +# xen_build_domain(target) +# Allow a domain to be created at boot by the hypervisor +define(`xen_build_domain', ` + allow xenboot_t $1_channel:event create; +') + # create_domain(priv, target) # Allow a domain to be created directly define(`create_domain', ` diff --git a/tools/flask/policy/modules/xen.te b/tools/flask/policy/modules/xen.te index 3dbf93d2b8..de98206fdd 100644 --- a/tools/flask/policy/modules/xen.te +++ b/tools/flask/policy/modules/xen.te @@ -24,6 +24,7 @@ attribute mls_priv; ################################################################################ # The hypervisor itself +type xenboot_t, xen_type, mls_priv; type xen_t, xen_type, mls_priv; # Domain 0 diff --git a/tools/flask/policy/policy/initial_sids b/tools/flask/policy/policy/initial_sids index 6b7b7eff21..ec729d3ba3 100644 --- a/tools/flask/policy/policy/initial_sids +++ b/tools/flask/policy/policy/initial_sids @@ -2,6 +2,7 @@ # objects created before the policy is loaded or for objects that do not have a # label defined in some other manner. +sid xenboot gen_context(system_u:system_r:xenboot_t,s0) sid xen gen_context(system_u:system_r:xen_t,s0) sid dom0 gen_context(system_u:system_r:dom0_t,s0) sid domxen gen_context(system_u:system_r:domxen_t,s0) diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index decebc8231..0643654aba 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -168,7 +168,7 @@ static int cf_check flask_domain_alloc_security(struct domain *d) switch ( d->domain_id ) { case DOMID_IDLE: - dsec->sid = SECINITSID_XEN; + dsec->sid = SECINITSID_XENBOOT; break; case DOMID_XEN: dsec->sid = SECINITSID_DOMXEN; @@ -188,6 +188,7 @@ static int cf_check flask_domain_alloc_security(struct domain *d) static void cf_check flask_transition_running(void) { + struct domain_security_struct *dsec; struct domain *d = current->domain; if ( d->domain_id != DOMID_IDLE ) @@ -198,6 +199,10 @@ static void cf_check flask_transition_running(void) * set to false for the consistency check(s) in the setup code. */ d->is_privileged = false; + + dsec = d->ssid; + dsec->sid = SECINITSID_XEN; + dsec->self_sid = dsec->sid; } static void cf_check flask_domain_free_security(struct domain *d) diff --git a/xen/xsm/flask/policy/initial_sids b/xen/xsm/flask/policy/initial_sids index 7eca70d339..e8b55b8368 100644 --- a/xen/xsm/flask/policy/initial_sids +++ b/xen/xsm/flask/policy/initial_sids @@ -3,6 +3,7 @@ # # Define initial security identifiers # +sid xenboot sid xen sid dom0 sid domio
This commit implements full support for starting the idle domain privileged by introducing a new flask label xenboot_t which the idle domain is labeled with at creation. It then provides the implementation for the XSM hook xsm_transition_running to relabel the idle domain to the existing xen_t flask label. In the reference flask policy a new macro, xen_build_domain(target), is introduced for creating policies for dom0less/hyperlaunch allowing the hypervisor to create and assign the necessary resources for domain construction. Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> --- tools/flask/policy/modules/xen.if | 6 ++++++ tools/flask/policy/modules/xen.te | 1 + tools/flask/policy/policy/initial_sids | 1 + xen/xsm/flask/hooks.c | 7 ++++++- xen/xsm/flask/policy/initial_sids | 1 + 5 files changed, 15 insertions(+), 1 deletion(-)