[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 2/2] flask: implement xsm_transtion_running
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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |