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: Fri, 15 May 2015 13:09:26 -0400 Message-ID: <55562846.8030703@tycho.nsa.gov> References: <1431599625-9572-1-git-send-email-ian.campbell@citrix.com> <55548553.7060700@citrix.com> <1431604483.13579.60.camel@citrix.com> <55552B0E.8050807@tycho.nsa.gov> <1431682741.8943.45.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: <1431682741.8943.45.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 Cc: Julien Grall , wei.liu2@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 05/15/2015 05:39 AM, Ian Campbell wrote: > On Thu, 2015-05-14 at 19:09 -0400, Daniel De Graaf wrote: >> 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. > > OK, so I think this rules out using unlabelled_t as a default. > >>> 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. > > That sounds good. > >>>From the PoV of the code in libxl this would be done by writing ssidref > as a literal number rather than translating a string (judging from your > example patch). That works for me. > > While looking into this I noticed the existing code is > if (ssid_label) > ssid_ref = translate(ssid_label) > > So my first patch has another issue which is that it will override a > users attempt to use ssid_ref themselves (which they are entitled to > do). > > Is ssidref==0 "unused"? i.e. could we use it to differentiate whether > the field had been filled in by the user or not? If not is there some > other number? Or should we reserve one using the same technique you > suggest here for domU? Yes, a value of zero is "not populated"; no valid context resolves to a SID of zero (including unlabeled_t, which is properly 5 or so). >> 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. > > I was about to ask about this. Short of a pretty serious change to the > build a BUILD_BUG_ON seems like a reasonable approach. > >>>> 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. > > I'd be inclined to go the other way and either have a default ssid for > the DM or to fail if one isn't given (the latter would probably happen > anyway due to enforcement?). Yes, it would probably fail at xc_domain_set_target in enforcing mode. > Sounds like the default ssidref should be either ~= domU_t of domHVM_t > depending on the type of domain? (domU_t is really domPV_t?) The domU_t type also works for HVM domains with the device model in dom0. Looking at the problem again, I think a second initial SID for the device model would be preferable, removing domHVM_t completely. There are already other example types in the policy for domains that do not use a device model (isolated_domU_t is probably the best example), and the result more closely matches the permissions used in the hypervisor without XSM enabled. -- Daniel De Graaf National Security Agency