[xen staging] xsm/flask: correcting initial sid assignment on context allocation

[patchbot@xxxxxxx]

commit 3007efadf74d6146a1c0ff1c2fbbae6b53ce7898
Author:     Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
AuthorDate: Wed Sep 14 15:22:01 2022 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Wed Sep 14 15:22:01 2022 +0200

    xsm/flask: correcting initial sid assignment on context allocation
    
    The current flow for initial SID assignment is that the function
    flask_domain_alloc_security() allocates the security context and assigns an
    initial SID based on the limited state information it can access. 
Specifically
    the initial SID is determined by the domid of the domain, where it would 
assign
    the label for one of the domains the hypervisor constructed with the 
exception
    of initial domain (dom0). In the case of the initial domain and all other
    domains it would use the unlabeled_t SID.
    
    When it came to the SID for the initial domain, its assignment was managed 
by
    flask_domain_create() where it would be switched from unlabeled_t to dom0_t.
    This logic worked under the assumption that the first call to
    flask_domain_create() would be the hypervisor constructing the initial 
domain.
    After which it would be the toolstack constructing the domain, for which it 
is
    expected to provide an appropriate SID or else unlabeled_t would be used.
    
    The issue is that the assumptions upon which the current flow is built were
    weak and are invalid for PV shim and dom0less. Under the current flow even
    though the initial domain for PV shim is not set as privileged, flask would
    label the domain as dom0_t. For dom0less, the situation is two-fold. First 
is
    that every domain after the first domain creation will fail as they will be
    labeled as unlabeled_t. The second is that if the dom0less configuration 
does
    not include a "dom0", the first domain created would be labeled as dom0_t.
    
    This commit only seeks to address the situation for PV shim, by including a
    check for xenboot_t context in flask_domain_alloc_security() to determine if
    the domain is being constructed at system boot. Then a check for 
is_privilged
    and pv_shim is added to differentiate between a "dom0" initial domain and a 
PV
    shim initial domain.
    
    The logic for flask_domain_create() was altered to allow the incoming SID to
    override the initial label. This allows a domain builder, whether it is a
    toolstack, dom0less, or hyperlaunch, to provide the correct label for the
    domain at construction.
    
    The base policy was adjusted to allow the idle domain under the xenboot_t
    context the ability to construct domains of both types, dom0_t and domu_t.
    This will enable a hypervisor resident domain builder to construct domains
    beyond the initial domain,
    
    Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
    Reviewed-by: Jason Andryuk <jandryuk@xxxxxxxxx>
---
 tools/flask/policy/modules/dom0.te |  3 +++
 tools/flask/policy/modules/domU.te |  3 +++
 xen/xsm/flask/hooks.c              | 55 +++++++++++++++++++++++++++++---------
 3 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/tools/flask/policy/modules/dom0.te 
b/tools/flask/policy/modules/dom0.te
index 0a63ce15b6..f710ff9941 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -75,3 +75,6 @@ admin_device(dom0_t, ioport_t)
 admin_device(dom0_t, iomem_t)
 
 domain_comms(dom0_t, dom0_t)
+
+# Allow the hypervisor to build domains of type dom0_t
+xen_build_domain(dom0_t)
diff --git a/tools/flask/policy/modules/domU.te 
b/tools/flask/policy/modules/domU.te
index b77df29d56..3f269018f9 100644
--- a/tools/flask/policy/modules/domU.te
+++ b/tools/flask/policy/modules/domU.te
@@ -13,6 +13,9 @@ domain_comms(domU_t, domU_t)
 migrate_domain_out(dom0_t, domU_t)
 domain_self_comms(domU_t)
 
+# Allow the hypervisor to build domains of type domU_t
+xen_build_domain(domU_t)
+
 # Device model for domU_t.  You can define distinct types for device models for
 # domains of other types, or add more make_device_model lines for this type.
 declare_domain(dm_dom_t)
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 8bd56644ef..e7db7fe07a 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -38,6 +38,12 @@
 #include <conditional.h>
 #include "private.h"
 
+#ifdef CONFIG_X86
+#include <asm/pv/shim.h>
+#else
+#define pv_shim false
+#endif
+
 static uint32_t domain_sid(const struct domain *dom)
 {
     struct domain_security_struct *dsec = dom->ssid;
@@ -170,6 +176,9 @@ static int cf_check flask_domain_alloc_security(struct 
domain *d)
     if ( !dsec )
         return -ENOMEM;
 
+    /* Set as unlabeled then change as appropriate. */
+    dsec->sid = SECINITSID_UNLABELED;
+
     switch ( d->domain_id )
     {
     case DOMID_IDLE:
@@ -182,7 +191,13 @@ static int cf_check flask_domain_alloc_security(struct 
domain *d)
         dsec->sid = SECINITSID_DOMIO;
         break;
     default:
-        dsec->sid = SECINITSID_UNLABELED;
+        if ( domain_sid(current->domain) == SECINITSID_XENBOOT )
+        {
+            if ( d->is_privileged )
+                dsec->sid = SECINITSID_DOM0;
+            else if ( pv_shim )
+                dsec->sid = SECINITSID_DOMU;
+        }
     }
 
     dsec->self_sid = dsec->sid;
@@ -550,20 +565,36 @@ static int cf_check flask_domain_create(struct domain *d, 
uint32_t ssidref)
     struct domain_security_struct *dsec = d->ssid;
     static int dom0_created = 0;
 
-    if ( is_idle_domain(current->domain) && !dom0_created )
+    /*
+     * If the null label is passed, then use the label from security context
+     * allocation. NB: if the label from the allocated security context is also
+     * null, the security server will use unlabeled_t for the domain.
+     */
+    if ( ssidref == 0 )
+        ssidref = dsec->sid;
+
+    /*
+     * First check if the current domain is allowed to create the target domain
+     * type before making changes to the current state.
+     */
+    rc = avc_current_has_perm(ssidref, SECCLASS_DOMAIN, DOMAIN__CREATE, NULL);
+    if ( rc )
+        return rc;
+
+    /*
+     * The dom0_t label is expressed as a singleton label in the base policy.
+     * This cannot be enforced by the security server, therefore it will be
+     * enforced here.
+     */
+    if ( ssidref == SECINITSID_DOM0 )
     {
-        dsec->sid = SECINITSID_DOM0;
-        dom0_created = 1;
+        if ( !dom0_created )
+            dom0_created = 1;
+        else
+            return -EINVAL;
     }
-    else
-    {
-        rc = avc_current_has_perm(ssidref, SECCLASS_DOMAIN,
-                          DOMAIN__CREATE, NULL);
-        if ( rc )
-            return rc;
 
-        dsec->sid = ssidref;
-    }
+    dsec->sid = ssidref;
     dsec->self_sid = dsec->sid;
 
     rc = security_transition_sid(dsec->sid, dsec->sid, SECCLASS_DOMAIN,
--
generated by git-patchbot for /home/xen/git/xen.git#staging