From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel De Graaf Subject: Re: [PATCH] libxl: assigned a default ssid_label (XSM label) to guests Date: Thu, 14 May 2015 19:09:02 -0400 Message-ID: <55552B0E.8050807@tycho.nsa.gov> References: <1431599625-9572-1-git-send-email-ian.campbell@citrix.com> <55548553.7060700@citrix.com> <1431604483.13579.60.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1431604483.13579.60.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Julien Grall Cc: wei.liu2@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 05/14/2015 07:54 AM, Ian Campbell wrote: > On Thu, 2015-05-14 at 12:21 +0100, Julien Grall wrote: >> Hi Ian, >> >> On 14/05/15 11:33, Ian Campbell wrote: >>> system_u:system_r:domU_t is defined in the default policy and makes as >>> much sense as anything for a default. >> >> So you rule out the possibility to run an unlabelled domain? This is >> possible if the policy explicitly authorized it. That's a significant >> change in the libxl behavior. > > I didn't realise this was a possibility, wouldn't such a domain be > system_u:system_r:unlabeled_t> or something? Yes. FLASK resolves any numeric SID value that is unused (including zero) to the unlabeled sid (defined in tools/flask/policy/policy/initial_sids to be system_u:system_r:unlabeled_t). Because this could be the result of an error (in the hypervisor, toolstack, etc), the use of unlabled_t for real objects is discouraged in SELinux and XSM/FLASK. > Note that this won't override a label which is just '' (i.e. an empty > string rather than NULL). I don't know if that results in the behaviour > you want. > > When this was discussed before (in a conversation Wei started, but which > I can't find, maybe it was IRC rather than email) it seemed that > consensus was that by default things should Just Work as if XSM weren't > disabled, which is what I've implemented here. I agree that this is a useful feature. It is possible to extend the initial_sids list with new entries that are used by the toolstack instead of by the hypervisor, which could be used to define SECINITSID_DOMU as the default label for a domU created by a toolstack without a label. This is better than hard-coding a string that may not be valid in a given security policy, and it can be associated with a label that better reflects how the policy wishes to treat domains with an "incomplete" configuration file. The header file defining these SIDs is buried in the hypervisor source tree (xen/xsm/flask/include/flask.h) and is only generated during a build with XSM enabled. It may be simpler to define the value in a shared header and add a BUILD_BUG_ON somewhere in the flask code to check for mismatches. >> IHMO, having a default policy doesn't mean libxl should set a default >> ssid to make XSM transparent to the user. >> >> The explicit ssid makes clear that the guest is using a ssid foo and if >> it's not provided then it will fail to boot. >> >> Setting a default value may hide a bigger issue and take the wrong >> policy the user forgot to set up an ssid. > > Does domU_t really have so many privileges that this is an issue? I'd > expect it to be almost totally privilegeless apart from things which any > domU needs. > > The benefits of XSM seem to mainly apply to the various service domains. > > Daniel, do you have an opinion here? In the example policy, domU_t should have the same level of access as a normal domain (i.e. not device model stubdom) has with XSM disabled. The only real difference is that the example policy does not allow any domain to act as a device model to domU_t; it uses domHVM_t and dm_dom_t for this. If you want to use configurations with device model stubdoms that also do not assign labels in the configuration, this distinction will need to be removed. >>> This change required moving the call to domain_create_info_setdefault >>> to be before the ssid_label is translated into ssidref, which also >>> moves it before some other stuff which consumes things from c_info, >>> which is correct since setdefault should always be called first. Apart >>> from the SSID handling there should be no functional change (since >>> setdefault doesn't actually act on anything which that other stuff >>> uses). >>> >>> There is no need to set exec_ssid_label since the default is to leave >>> the domain using the ssid_label after build. >> >> By setting a ssid label, libxl will print a new warning on Xen not built >> with XSM which will confuse the user: >> >> libxl: warning: libxl_create.c:813:initiate_domain_create: XSM Disabled: >> init_seclabel not supported > > Ah, I didn't try that case. I'll see if I can work out a way to suppress > that warning. I would be fine with removing that warning completely; someone trying to use XSM without it enabled will likely be able to figure out the problem without this error, likely by noticing the "-" labels in xl list -v/-Z. ------------->8----------------- Example patch adding SECINITSID_DOMU, for testing/reference. --- diff --git a/tools/flask/policy/policy/initial_sids b/tools/flask/policy/policy/initial_sids index 5de0bbf..48aad17 100644 --- a/tools/flask/policy/policy/initial_sids +++ b/tools/flask/policy/policy/initial_sids @@ -12,3 +12,4 @@ sid irq gen_context(system_u:object_r:irq_t,s0) sid iomem gen_context(system_u:object_r:iomem_t,s0) sid ioport gen_context(system_u:object_r:ioport_t,s0) sid device gen_context(system_u:object_r:device_t,s0) +sid domU gen_context(system_u:system_r:domU_t,s0) diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index f0da7dc..0c3d4ed 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -815,6 +815,8 @@ static void initiate_domain_create(libxl__egc *egc, goto error_out; } } + } else { + d_config->c_info.ssidref = 11; /* SECINITSID_DOMU */ } if (d_config->b_info.exec_ssid_label) { diff --git a/xen/xsm/flask/policy/initial_sids b/xen/xsm/flask/policy/initial_sids index e508bde..a442a38 100644 --- a/xen/xsm/flask/policy/initial_sids +++ b/xen/xsm/flask/policy/initial_sids @@ -13,4 +13,5 @@ sid ioport sid iomem sid irq sid device +sid domU # FLASK