selinux: match extended permissions to their base permissions

In commit d1d991efaf34 ("selinux: Add netlink xperm support") a new
extended permission was added ("nlmsg"). This was the second extended
permission implemented in selinux ("ioctl" being the first one).

Extended permissions are associated with a base permission. It was found
that, in the access vector cache (avc), the extended permission did not
keep track of its base permission. This is an issue for a domain that is
using both extended permissions (i.e., a domain calling ioctl() on a
netlink socket). In this case, the extended permissions were
overlapping.

Keep track of the base permission in the cache. A new field "base_perm"
is added to struct extended_perms_decision to make sure that the
extended permission refers to the correct policy permission. A new field
"base_perms" is added to struct extended_perms to quickly decide if
extended permissions apply.

While it is in theory possible to retrieve the base permission from the
access vector, the same base permission may not be mapped to the same
bit for each class (e.g., "nlmsg" is mapped to a different bit for
"netlink_route_socket" and "netlink_audit_socket"). Instead, use a
constant (AVC_EXT_IOCTL or AVC_EXT_NLMSG) provided by the caller.

Fixes: d1d991efaf34 ("selinux: Add netlink xperm support")
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>

diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index cc0b0af20296170be8f001ae539eb29eae04e871..1f2680bcc43a42301e06a573df053328b8a83087 100644 (file)
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -174,13 +174,15 @@ int avc_get_hash_stats(char *page)
  * using a linked list for extended_perms_decision lookup because the list is
  * always small. i.e. less than 5, typically 1
  */
-static struct extended_perms_decision *avc_xperms_decision_lookup(u8 driver,
-                                       struct avc_xperms_node *xp_node)
+static struct extended_perms_decision *
+avc_xperms_decision_lookup(u8 driver, u8 base_perm,
+                          struct avc_xperms_node *xp_node)
 {
        struct avc_xperms_decision_node *xpd_node;
 
        list_for_each_entry(xpd_node, &xp_node->xpd_head, xpd_list) {
-               if (xpd_node->xpd.driver == driver)
+               if (xpd_node->xpd.driver == driver &&
+                   xpd_node->xpd.base_perm == base_perm)
                        return &xpd_node->xpd;
        }
        return NULL;
@@ -205,11 +207,12 @@ avc_xperms_has_perm(struct extended_perms_decision *xpd,
 }
 
 static void avc_xperms_allow_perm(struct avc_xperms_node *xp_node,
-                               u8 driver, u8 perm)
+                                 u8 driver, u8 base_perm, u8 perm)
 {
        struct extended_perms_decision *xpd;
        security_xperm_set(xp_node->xp.drivers.p, driver);
-       xpd = avc_xperms_decision_lookup(driver, xp_node);
+       xp_node->xp.base_perms |= base_perm;
+       xpd = avc_xperms_decision_lookup(driver, base_perm, xp_node);
        if (xpd && xpd->allowed)
                security_xperm_set(xpd->allowed->p, perm);
 }
@@ -245,6 +248,7 @@ static void avc_xperms_free(struct avc_xperms_node *xp_node)
 static void avc_copy_xperms_decision(struct extended_perms_decision *dest,
                                        struct extended_perms_decision *src)
 {
+       dest->base_perm = src->base_perm;
        dest->driver = src->driver;
        dest->used = src->used;
        if (dest->used & XPERMS_ALLOWED)
@@ -272,6 +276,7 @@ static inline void avc_quick_copy_xperms_decision(u8 perm,
         */
        u8 i = perm >> 5;
 
+       dest->base_perm = src->base_perm;
        dest->used = src->used;
        if (dest->used & XPERMS_ALLOWED)
                dest->allowed->p[i] = src->allowed->p[i];
@@ -357,6 +362,7 @@ static int avc_xperms_populate(struct avc_node *node,
 
        memcpy(dest->xp.drivers.p, src->xp.drivers.p, sizeof(dest->xp.drivers.p));
        dest->xp.len = src->xp.len;
+       dest->xp.base_perms = src->xp.base_perms;
 
        /* for each source xpd allocate a destination xpd and copy */
        list_for_each_entry(src_xpd, &src->xpd_head, xpd_list) {
@@ -807,6 +813,7 @@ out:
  * @event : Updating event
  * @perms : Permission mask bits
  * @driver: xperm driver information
+ * @base_perm: the base permission associated with the extended permission
  * @xperm: xperm permissions
  * @ssid: AVC entry source sid
  * @tsid: AVC entry target sid
@@ -820,10 +827,9 @@ out:
  * otherwise, this function updates the AVC entry. The original AVC-entry object
  * will release later by RCU.
  */
-static int avc_update_node(u32 event, u32 perms, u8 driver, u8 xperm, u32 ssid,
-                          u32 tsid, u16 tclass, u32 seqno,
-                          struct extended_perms_decision *xpd,
-                          u32 flags)
+static int avc_update_node(u32 event, u32 perms, u8 driver, u8 base_perm,
+                          u8 xperm, u32 ssid, u32 tsid, u16 tclass, u32 seqno,
+                          struct extended_perms_decision *xpd, u32 flags)
 {
        u32 hvalue;
        int rc = 0;
@@ -880,7 +886,7 @@ static int avc_update_node(u32 event, u32 perms, u8 driver, u8 xperm, u32 ssid,
        case AVC_CALLBACK_GRANT:
                node->ae.avd.allowed |= perms;
                if (node->ae.xp_node && (flags & AVC_EXTENDED_PERMS))
-                       avc_xperms_allow_perm(node->ae.xp_node, driver, xperm);
+                       avc_xperms_allow_perm(node->ae.xp_node, driver, base_perm, xperm);
                break;
        case AVC_CALLBACK_TRY_REVOKE:
        case AVC_CALLBACK_REVOKE:
@@ -987,10 +993,9 @@ static noinline void avc_compute_av(u32 ssid, u32 tsid, u16 tclass,
        avc_insert(ssid, tsid, tclass, avd, xp_node);
 }
 
-static noinline int avc_denied(u32 ssid, u32 tsid,
-                              u16 tclass, u32 requested,
-                              u8 driver, u8 xperm, unsigned int flags,
-                              struct av_decision *avd)
+static noinline int avc_denied(u32 ssid, u32 tsid, u16 tclass, u32 requested,
+                              u8 driver, u8 base_perm, u8 xperm,
+                              unsigned int flags, struct av_decision *avd)
 {
        if (flags & AVC_STRICT)
                return -EACCES;
@@ -999,7 +1004,7 @@ static noinline int avc_denied(u32 ssid, u32 tsid,
            !(avd->flags & AVD_FLAGS_PERMISSIVE))
                return -EACCES;
 
-       avc_update_node(AVC_CALLBACK_GRANT, requested, driver,
+       avc_update_node(AVC_CALLBACK_GRANT, requested, driver, base_perm,
                        xperm, ssid, tsid, tclass, avd->seqno, NULL, flags);
        return 0;
 }
@@ -1012,7 +1017,8 @@ static noinline int avc_denied(u32 ssid, u32 tsid,
  * driver field is used to specify which set contains the permission.
  */
 int avc_has_extended_perms(u32 ssid, u32 tsid, u16 tclass, u32 requested,
-                          u8 driver, u8 xperm, struct common_audit_data *ad)
+                          u8 driver, u8 base_perm, u8 xperm,
+                          struct common_audit_data *ad)
 {
        struct avc_node *node;
        struct av_decision avd;
@@ -1047,22 +1053,23 @@ int avc_has_extended_perms(u32 ssid, u32 tsid, u16 tclass, u32 requested,
        local_xpd.auditallow = &auditallow;
        local_xpd.dontaudit = &dontaudit;
 
-       xpd = avc_xperms_decision_lookup(driver, xp_node);
+       xpd = avc_xperms_decision_lookup(driver, base_perm, xp_node);
        if (unlikely(!xpd)) {
                /*
                 * Compute the extended_perms_decision only if the driver
-                * is flagged
+                * is flagged and the base permission is known.
                 */
-               if (!security_xperm_test(xp_node->xp.drivers.p, driver)) {
+               if (!security_xperm_test(xp_node->xp.drivers.p, driver) ||
+                   !(xp_node->xp.base_perms & base_perm)) {
                        avd.allowed &= ~requested;
                        goto decision;
                }
                rcu_read_unlock();
-               security_compute_xperms_decision(ssid, tsid, tclass,
-                                                driver, &local_xpd);
+               security_compute_xperms_decision(ssid, tsid, tclass, driver,
+                                                base_perm, &local_xpd);
                rcu_read_lock();
-               avc_update_node(AVC_CALLBACK_ADD_XPERMS, requested,
-                               driver, xperm, ssid, tsid, tclass, avd.seqno,
+               avc_update_node(AVC_CALLBACK_ADD_XPERMS, requested, driver,
+                               base_perm, xperm, ssid, tsid, tclass, avd.seqno,
                                &local_xpd, 0);
        } else {
                avc_quick_copy_xperms_decision(xperm, &local_xpd, xpd);
@@ -1075,8 +1082,8 @@ int avc_has_extended_perms(u32 ssid, u32 tsid, u16 tclass, u32 requested,
 decision:
        denied = requested & ~(avd.allowed);
        if (unlikely(denied))
-               rc = avc_denied(ssid, tsid, tclass, requested,
-                               driver, xperm, AVC_EXTENDED_PERMS, &avd);
+               rc = avc_denied(ssid, tsid, tclass, requested, driver,
+                               base_perm, xperm, AVC_EXTENDED_PERMS, &avd);
 
        rcu_read_unlock();
 
@@ -1110,7 +1117,7 @@ static noinline int avc_perm_nonode(u32 ssid, u32 tsid, u16 tclass,
        avc_compute_av(ssid, tsid, tclass, avd, &xp_node);
        denied = requested & ~(avd->allowed);
        if (unlikely(denied))
-               return avc_denied(ssid, tsid, tclass, requested, 0, 0,
+               return avc_denied(ssid, tsid, tclass, requested, 0, 0, 0,
                                  flags, avd);
        return 0;
 }
@@ -1158,7 +1165,7 @@ inline int avc_has_perm_noaudit(u32 ssid, u32 tsid,
        rcu_read_unlock();
 
        if (unlikely(denied))
-               return avc_denied(ssid, tsid, tclass, requested, 0, 0,
+               return avc_denied(ssid, tsid, tclass, requested, 0, 0, 0,
                                  flags, avd);
        return 0;
 }

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index f5a08f94e09402b6b0b1538fae1a7a3f5af19fe6..011d9121b3ab5c7eee5ecd219db5a3cafef09cc0 100644 (file)
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3688,8 +3688,8 @@ static int ioctl_has_perm(const struct cred *cred, struct file *file,
                return 0;
 
        isec = inode_security(inode);
-       rc = avc_has_extended_perms(ssid, isec->sid, isec->sclass,
-                                   requested, driver, xperm, &ad);
+       rc = avc_has_extended_perms(ssid, isec->sid, isec->sclass, requested,
+                                   driver, AVC_EXT_IOCTL, xperm, &ad);
 out:
        return rc;
 }
@@ -5952,7 +5952,7 @@ static int nlmsg_sock_has_extended_perms(struct sock *sk, u32 perms, u16 nlmsg_t
        xperm = nlmsg_type & 0xff;
 
        return avc_has_extended_perms(current_sid(), sksec->sid, sksec->sclass,
-                       perms, driver, xperm, &ad);
+                                     perms, driver, AVC_EXT_NLMSG, xperm, &ad);
 }
 
 static int selinux_netlink_send(struct sock *sk, struct sk_buff *skb)

diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h
index 96a614d47df8d1d67b0b033ebb90954653dfeab7..281f4010366336c32731f42b3501ea13ca05b1d8 100644 (file)
--- a/security/selinux/include/avc.h
+++ b/security/selinux/include/avc.h
@@ -136,8 +136,11 @@ int avc_has_perm_noaudit(u32 ssid, u32 tsid, u16 tclass, u32 requested,
 int avc_has_perm(u32 ssid, u32 tsid, u16 tclass, u32 requested,
                 struct common_audit_data *auditdata);
 
+#define AVC_EXT_IOCTL  (1 << 0) /* Cache entry for an ioctl extended permission */
+#define AVC_EXT_NLMSG  (1 << 1) /* Cache entry for an nlmsg extended permission */
 int avc_has_extended_perms(u32 ssid, u32 tsid, u16 tclass, u32 requested,
-                          u8 driver, u8 perm, struct common_audit_data *ad);
+                          u8 driver, u8 base_perm, u8 perm,
+                          struct common_audit_data *ad);
 
 u32 avc_policy_seqno(void);
 

diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index c7f2731abd03e831e010b80403102d3b422ac94b..700bd6c8bb3865defb19cf824bd7bc243c04e43d 100644 (file)
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -239,6 +239,7 @@ struct extended_perms_data {
 struct extended_perms_decision {
        u8 used;
        u8 driver;
+       u8 base_perm;
        struct extended_perms_data *allowed;
        struct extended_perms_data *auditallow;
        struct extended_perms_data *dontaudit;
@@ -246,6 +247,7 @@ struct extended_perms_decision {
 
 struct extended_perms {
        u16 len; /* length associated decision chain */
+       u8 base_perms; /* which base permissions are covered */
        struct extended_perms_data drivers; /* flag drivers that are used */
 };
 
@@ -257,6 +259,7 @@ void security_compute_av(u32 ssid, u32 tsid, u16 tclass,
                         struct extended_perms *xperms);
 
 void security_compute_xperms_decision(u32 ssid, u32 tsid, u16 tclass, u8 driver,
+                                     u8 base_perm,
                                      struct extended_perms_decision *xpermd);
 
 void security_compute_av_user(u32 ssid, u32 tsid, u16 tclass,

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 3d5c563cfc4c86a4ca38ace8008a500595767ce3..d9f58b5d0f49f04e7ff52921634a2d9f66f61550 100644 (file)
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -582,7 +582,7 @@ static void type_attribute_bounds_av(struct policydb *policydb,
 }
 
 /*
- * Flag which drivers have permissions.
+ * Flag which drivers have permissions and which base permissions are covered.
  */
 void services_compute_xperms_drivers(
                struct extended_perms *xperms,
@@ -592,12 +592,19 @@ void services_compute_xperms_drivers(
 
        switch (node->datum.u.xperms->specified) {
        case AVTAB_XPERMS_IOCTLDRIVER:
+               xperms->base_perms |= AVC_EXT_IOCTL;
                /* if one or more driver has all permissions allowed */
                for (i = 0; i < ARRAY_SIZE(xperms->drivers.p); i++)
                        xperms->drivers.p[i] |= node->datum.u.xperms->perms.p[i];
                break;
        case AVTAB_XPERMS_IOCTLFUNCTION:
+               xperms->base_perms |= AVC_EXT_IOCTL;
+               /* if allowing permissions within a driver */
+               security_xperm_set(xperms->drivers.p,
+                                       node->datum.u.xperms->driver);
+               break;
        case AVTAB_XPERMS_NLMSG:
+               xperms->base_perms |= AVC_EXT_NLMSG;
                /* if allowing permissions within a driver */
                security_xperm_set(xperms->drivers.p,
                                        node->datum.u.xperms->driver);
@@ -631,8 +638,7 @@ static void context_struct_compute_av(struct policydb *policydb,
        avd->auditallow = 0;
        avd->auditdeny = 0xffffffff;
        if (xperms) {
-               memset(&xperms->drivers, 0, sizeof(xperms->drivers));
-               xperms->len = 0;
+               memset(xperms, 0, sizeof(*xperms));
        }
 
        if (unlikely(!tclass || tclass > policydb->p_classes.nprim)) {
@@ -969,13 +975,19 @@ void services_compute_xperms_decision(struct extended_perms_decision *xpermd,
 {
        switch (node->datum.u.xperms->specified) {
        case AVTAB_XPERMS_IOCTLFUNCTION:
-       case AVTAB_XPERMS_NLMSG:
-               if (xpermd->driver != node->datum.u.xperms->driver)
+               if (xpermd->base_perm != AVC_EXT_IOCTL ||
+                   xpermd->driver != node->datum.u.xperms->driver)
                        return;
                break;
        case AVTAB_XPERMS_IOCTLDRIVER:
-               if (!security_xperm_test(node->datum.u.xperms->perms.p,
-                                       xpermd->driver))
+               if (xpermd->base_perm != AVC_EXT_IOCTL ||
+                   !security_xperm_test(node->datum.u.xperms->perms.p,
+                                        xpermd->driver))
+                       return;
+               break;
+       case AVTAB_XPERMS_NLMSG:
+               if (xpermd->base_perm != AVC_EXT_NLMSG ||
+                   xpermd->driver != node->datum.u.xperms->driver)
                        return;
                break;
        default:
@@ -1010,6 +1022,7 @@ void security_compute_xperms_decision(u32 ssid,
                                      u32 tsid,
                                      u16 orig_tclass,
                                      u8 driver,
+                                     u8 base_perm,
                                      struct extended_perms_decision *xpermd)
 {
        struct selinux_policy *policy;
@@ -1023,6 +1036,7 @@ void security_compute_xperms_decision(u32 ssid,
        struct ebitmap_node *snode, *tnode;
        unsigned int i, j;
 
+       xpermd->base_perm = base_perm;
        xpermd->driver = driver;
        xpermd->used = 0;
        memset(xpermd->allowed->p, 0, sizeof(xpermd->allowed->p));