[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] libxl: assigned a default ssid_label (XSM label) to guests

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

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.

Example patch adding SECINITSID_DOMU, for testing/reference.

diff --git a/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 
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

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.