kdbus LSM/SELinux hooks

In order to effectively enforce LSM based access controls we need to have more information about the kdbus endpoint creator than the uid/gid currently stored in the kdbus_node_type struct. This patch replaces the uid/gid values with a reference to the node creator's credential struct which serves the needs of both the kdbus DAC access controls as well as the LSM's access controls.

Two macros have also been created, kdbus_node_[uid,gid](), which can be used to easily extract the euid/egid information from the new credential reference. The effective uid/gid is used as it was used in all areas of the previous kdbus code except for areas where the uid/gid was never set beyond the basic initialization to zero/root; I expect this was a bug that was never caught as the node creator in these cases was always expect to be root.

Signed-off-by Paul Moore <[email protected]>

---
ChangeLog:
- v3
* Ported to the 4.3-rc4 based kdbus tree
- v2
* Initial draft

---
  ipc/kdbus/bus.c      |   13 +++++--------
  ipc/kdbus/endpoint.c |   14 ++++----------
  ipc/kdbus/endpoint.h |    3 +--
  ipc/kdbus/fs.c       |    4 ++--
  ipc/kdbus/node.c     |   11 ++++-------
  ipc/kdbus/node.h     |    5 +++--
  6 files changed, 19 insertions(+), 31 deletions(-)

ipc/kdbus/node.c +1 -0

index 89f58bc..cd0c1a0 100644
@@ -12,6 +12,7 @@
   */
 
  #include <linux/atomic.h>
+#include <linux/cred.h>
  #include <linux/fs.h>
  #include <linux/idr.h>
  #include <linux/kdev_t.h>

Add LSM access control hooks to kdbus; several new hooks are added and the existing security_file_receive() hook is reused. The new hooks are listed below:

* security_kdbus_conn_new
Check if the current task is allowed to create a new kdbus connection.
* security_kdbus_own_name
Check if a connection is allowed to own a kdbus service name.
* security_kdbus_conn_talk
Check if a connection is allowed to talk to a kdbus peer.
* security_kdbus_conn_see
Check if a connection can see a kdbus peer.
* security_kdbus_conn_see_name
Check if a connection can see a kdbus service name.
* security_kdbus_conn_see_notification
Check if a connection can receive notifications.
* security_kdbus_proc_permission
Check if a connection can access another task's pid namespace info.
* security_kdbus_init_inode
Set the security label on a kdbusfs inode

Signed-off-by Paul Moore <[email protected]>

---
ChangeLog:
- v3
* Ported to the 4.3-rc4 based kdbus tree
- v2
* Implemented suggestions by Stephen Smalley
* call security_kdbus_conn_new() sooner
* reworked hook inside kdbus_conn_policy_own_name()
* fixed if-conditional in kdbus_conn_policy_talk()
* reworked hook inside kdbus_conn_policy_see_name_unlocked()
* reworked hook inside kdbus_conn_policy_see()
* reworked hook inside kdbus_conn_policy_see_notification()
* added the security_kdbus_init_inode() hook
- v1
* Initial draft

---
  include/linux/lsm_hooks.h |   63 +++++++++++++++++++++++++++++++++++++++
  include/linux/security.h  |   71 ++++++++++++++++++++++++++++++++++++++++++++
  ipc/kdbus/connection.c    |   73 +++++++++++++++++++++++++++++----------------
  ipc/kdbus/fs.c            |    6 ++++
  ipc/kdbus/message.c       |   19 +++++++++---
  ipc/kdbus/metadata.c      |    6 +---
  security/security.c       |   62 ++++++++++++++++++++++++++++++++++++++
  7 files changed, 265 insertions(+), 35 deletions(-)

ipc/kdbus/connection.c +1 -0

index ef63d65..1cb87b3 100644
@@ -26,6 +26,7 @@
  #include <linux/path.h>
  #include <linux/poll.h>
  #include <linux/sched.h>
+#include <linux/security.h>
  #include <linux/shmem_fs.h>
  #include <linux/sizes.h>
  #include <linux/slab.h>

ipc/kdbus/connection.c +7 -1

+				      creds, pids, seclabel,
+				      owner, privileged,
+				      is_activator, is_monitor,
+				      is_policy_holder);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
  	ret = kdbus_sanitize_attach_flags(hello->attach_flags_send,
  					  &attach_flags_send);
  	if (ret < 0)

@@ -1435,12 +1444,12 @@ bool kdbus_conn_policy_own_name(struct kdbus_conn *conn,
  			return false;
  	}
 
-	if (conn->owner)

ipc/kdbus/connection.c +0 -4

  }
 
  /**

@@ -1465,14 +1474,13 @@ bool kdbus_conn_policy_talk(struct kdbus_conn *conn,
  					 to, KDBUS_POLICY_TALK))
  		return false;
 
-	if (conn->owner)
-		return true;
-	if (uid_eq(conn_creds->euid, to->cred->uid))
-		return true;

ipc/kdbus/connection.c +2 -3

  }
 
  /**

@@ -1491,19 +1499,19 @@ bool kdbus_conn_policy_see_name_unlocked(struct kdbus_conn *conn,
  					 const struct cred *conn_creds,
  					 const char *name)
  {
-	int res;
+	if (!conn_creds)
+		conn_creds = conn->cred;
 
  	/*
  	 * By default, all names are visible on a bus. SEE policies can only be
  	 * installed on custom endpoints, where by default no name is visible.
  	 */
-	if (!conn->ep->user)
-		return true;

ipc/kdbus/connection.c +20 -7

  }
 
  static bool kdbus_conn_policy_see_name(struct kdbus_conn *conn,

@@ -1523,6 +1531,9 @@ static bool kdbus_conn_policy_see(struct kdbus_conn *conn,
  				  const struct cred *conn_creds,
  				  struct kdbus_conn *whom)
  {
+	if (!conn_creds)
+		conn_creds = conn->cred;
+
  	/*
  	 * By default, all names are visible on a bus, so a connection can
  	 * always see other connections. SEE policies can only be installed on

@@ -1530,10 +1541,13 @@ static bool kdbus_conn_policy_see(struct kdbus_conn *conn,
  	 * peers from each other, unless you see at least _one_ name of the
  	 * peer.
  	 */
-	return !conn->ep->user ||
-	       kdbus_conn_policy_query_all(conn, conn_creds,
-					   &conn->ep->policy_db, whom,
-					   KDBUS_POLICY_SEE);
+	if (conn->ep->user &&
+	    !kdbus_conn_policy_query_all(conn, conn_creds,
+					 &conn->ep->policy_db, whom,
+					 KDBUS_POLICY_SEE))
+		return false;
+
+	return (security_kdbus_conn_see(conn_creds, whom->cred) == 0);
  }
 
  /**

@@ -1551,6 +1565,9 @@ bool kdbus_conn_policy_see_notification(struct kdbus_conn *conn,
  					const struct cred *conn_creds,
  					const struct kdbus_msg *msg)
  {
+	if (!conn_creds)
+		conn_creds = conn->cred;
+
  	/*
  	 * Depending on the notification type, broadcasted kernel notifications
  	 * have to be filtered:

@@ -1567,18 +1584,22 @@ bool kdbus_conn_policy_see_notification(struct kdbus_conn *conn,
  	case KDBUS_ITEM_NAME_ADD:
  	case KDBUS_ITEM_NAME_REMOVE:
  	case KDBUS_ITEM_NAME_CHANGE:
-		return kdbus_conn_policy_see_name(conn, conn_creds,
-					msg->items[0].name_change.name);
+		if (!kdbus_conn_policy_see_name(conn, conn_creds,
+						msg->items[0].name_change.name))
+			return false;
 
  	case KDBUS_ITEM_ID_ADD:
  	case KDBUS_ITEM_ID_REMOVE:
-		return true;
+		/* fall through for the LSM check */
+		break;
 
  	default:
  		WARN(1, "Invalid type for notification broadcast: %llu\n",
  		     (unsigned long long)msg->items[0].type);
  		return false;
  	}
+
+	return (security_kdbus_conn_see_notification(conn_creds) == 0);
  }
 
  /**

ipc/kdbus/fs.c +2 -0

index 68818a8..4e84e89 100644
@@ -23,6 +23,7 @@
  #include <linux/namei.h>
  #include <linux/pagemap.h>
  #include <linux/sched.h>
+#include <linux/security.h>
  #include <linux/slab.h>
 
  #include "bus.h"

@@ -192,6 +193,7 @@ static const struct inode_operations fs_inode_iops = {
  static struct inode *fs_inode_get(struct super_block *sb,
  				  struct kdbus_node *node)
  {
+	int ret;
  	struct inode *inode;
 
  	inode = iget_locked(sb, node->id);

ipc/kdbus/fs.c +1 -0

+
  	inode->i_private = kdbus_node_ref(node);
  	inode->i_mapping->a_ops = &empty_aops;
  	inode->i_mode = node->mode & S_IALLUGO;

mode still remains

The kdbus service names will be recorded using 'service', similar to the existing dbus audit records.

Signed-off-by Paul Moore <[email protected]>

---
ChangeLog:
- v3
* Ported to the 4.3-rc4 based kdbus tree
- v2
* Initial draft

---
  include/linux/lsm_audit.h |    2 ++
  security/lsm_audit.c      |    4 ++++
  2 files changed, 6 insertions(+)

include/linux/lsm_audit.h +2 -0

index ffb9c9d..d6a656f 100644
@@ -59,6 +59,7 @@ struct common_audit_data {
  #define LSM_AUDIT_DATA_INODE	9
  #define LSM_AUDIT_DATA_DENTRY	10
  #define LSM_AUDIT_DATA_IOCTL_OP	11
+#define LSM_AUDIT_DATA_KDBUS	12
  	union 	{
  		struct path path;
  		struct dentry *dentry;

@@ -75,6 +76,7 @@ struct common_audit_data {
  #endif
  		char *kmod_name;
  		struct lsm_ioctlop_audit *op;
+		const char *kdbus_name;
  	} u;
  	/* this union contains LSM specific data */
  	union {

security/lsm_audit.c +0 -0

index cccbf30..0a3dc1b 100644

security/lsm_audit.c +2 -0

+		audit_log_untrustedstring(ab, a->u.kdbus_name);
+		break;
  	} /* switch (a->type) */
  }
 

_______________________________________________
Selinux mailing list
[email protected]
To unsubscribe, send email to [email protected].
To get help, send an email containing "help" to [email protected].

Add the SELinux access control implementation for the new kdbus LSM hooks using the new kdbus object class and the following permissions:

[NOTE: permissions below are based on kdbus code from Aug 2015]

* kdbus:impersonate
Send a different security label to kdbus peers.
* kdbus:fakecreds
Send different DAC credentials to kdbus peers.
* kdbus:fakepids
Send a different PID to kdbus peers.
* kdbus:owner
Act as a kdbus bus owner.
* kdbus:privileged
Act as a privileged endpoint.
* kdbus:activator
Act as a kdbus activator.
* kdbus:monitor
Act as a kdbus monitor.
* kdbus:policy_holder
Act as a kdbus policy holder.
* kdbus:connect
Create a new kdbus connection.
* kdbus:own
Own a kdbus service name.
* kdbus:talk
Talk between two kdbus endpoints.
* kdbus:see
See another kdbus endpoint.
* kdbus:see_name
See a kdbus service name.
* kdbus:see_notification
See a kdbus notification.

Signed-off-by Paul Moore <[email protected]>

---
ChangeLog:
- v3
* Ported to the 4.3-rc4 based kdbus tree
* Fix the missing NULL terminator in the kdbus obj class definition
- v2
* Add the selinux_kdbus_init_inode() hook
* Add some very basic info on the permissions to the description
* Add kdbus service name auditing in the AVC records
- v1
* Initial draft

---
  security/selinux/hooks.c            |  153 +++++++++++++++++++++++++++++++++++
  security/selinux/include/classmap.h |    4 +
  2 files changed, 155 insertions(+), 2 deletions(-)

security/selinux/hooks.c +127 -2

index e4369d8..5581990 100644
@@ -9,8 +9,10 @@
   *	      James Morris <[email protected]>
   *
   *  Copyright (C) 2001,2002 Networks Associates Technology, Inc.
- *  Copyright (C) 2003-2008 Red Hat, Inc., James Morris <[email protected]>
- *					   Eric Paris <[email protected]>
+ *  Copyright (C) 2003-2008,2015 Red Hat, Inc.
+ *					James Morris <[email protected]>
+ *					Eric Paris <[email protected]>
+ *					Paul Moore <[email protected]>
   *  Copyright (C) 2004-2005 Trusted Computer Solutions, Inc.
   *			    <[email protected]>
   *  Copyright (C) 2006, 2007, 2009 Hewlett-Packard Development Company, L.P.

@@ -2035,6 +2037,143 @@ static int selinux_binder_transfer_file(struct task_struct *from,
  			    &ad);
  }
 
+static int selinux_kdbus_conn_new(const struct cred *creds,
+				  const struct kdbus_creds *fake_creds,
+				  const struct kdbus_pids *fake_pids,
+				  const char *fake_seclabel,
+				  bool owner, bool privileged,
+				  bool is_activator, bool is_monitor,
+				  bool is_policy_holder)
+{
+	int rc;
+	u32 tsid = current_sid();
+	u32 av = KDBUS__CONNECT;
+
+	if (fake_creds)
+		av |= KDBUS__FAKECREDS;
+	if (fake_pids)
+		av |= KDBUS__FAKEPIDS;
+	if (owner)
+		av |= KDBUS__OWNER;
+	if (privileged)
+		av |= KDBUS__PRIVILEGED;
+	if (is_activator)
+		av |= KDBUS__ACTIVATOR;
+	if (is_monitor)
+		av |= KDBUS__MONITOR;
+	if (is_policy_holder)
+		av |= KDBUS__POLICY_HOLDER;
+
+	rc = avc_has_perm(tsid, cred_sid(creds), SECCLASS_KDBUS, av, NULL);
+	if (rc)
+		return rc;
+
+	if (fake_seclabel) {
+		u32 sid;
+		if (security_context_to_sid(fake_seclabel,
+					    strlen(fake_seclabel),
+					    &sid, GFP_KERNEL))
+			return -EINVAL;
+
+		rc = avc_has_perm(tsid, sid,
+				  SECCLASS_KDBUS, KDBUS__IMPERSONATE, NULL);
+	}
+
+	return rc;
+}
+
+static int selinux_kdbus_own_name(const struct cred *creds, const char *name)
+{
+	int rc;
+	u32 name_sid;
+	struct common_audit_data ad;
+
+	rc = security_kdbus_sid(name, &name_sid);
+	if (rc)
+		return rc;
+
+	ad.type = LSM_AUDIT_DATA_KDBUS;
+	ad.u.kdbus_name = name;
+
+	return avc_has_perm(cred_sid(creds), name_sid,
+			    SECCLASS_KDBUS, KDBUS__OWN, &ad);
+}
+
+static int selinux_kdbus_conn_talk(const struct cred *creds,
+				   const struct cred *creds_to)
+{
+	return avc_has_perm(cred_sid(creds), cred_sid(creds_to),
+			    SECCLASS_KDBUS, KDBUS__TALK, NULL);
+}
+
+static int selinux_kdbus_conn_see(const struct cred *creds,
+				  const struct cred *creds_whom)
+{
+	return avc_has_perm(cred_sid(creds), cred_sid(creds_whom),
+			    SECCLASS_KDBUS, KDBUS__SEE, NULL);
+}
+
+static int selinux_kdbus_conn_see_name(const struct cred *creds,
+				       const char *name)
+{
+	int rc;
+	u32 name_sid;
+	struct common_audit_data ad;
+
+	rc = security_kdbus_sid(name, &name_sid);
+	if (rc)
+		return rc;
+
+	ad.type = LSM_AUDIT_DATA_KDBUS;
+	ad.u.kdbus_name = name;
+
+	return avc_has_perm(cred_sid(creds), name_sid,
+			    SECCLASS_KDBUS, KDBUS__SEE_NAME, &ad);
+}
+
+static int selinux_kdbus_conn_see_notification(const struct cred *creds)
+{
+	return avc_has_perm(SECINITSID_KERNEL, cred_sid(creds),
+			    SECCLASS_KDBUS, KDBUS__SEE_NOTIFICATION, NULL);
+}
+
+static int selinux_kdbus_proc_permission(const struct cred *creds,
+					 struct pid *pid)
+{
+	int rc;
+	struct task_struct *task;
+
+	rcu_read_lock();
+	task = pid_task(pid, PIDTYPE_PID);
+	rc = avc_has_perm(cred_sid(creds), task_sid(task),
+			  SECCLASS_PROCESS, PROCESS__GETATTR, NULL);
+	rcu_read_unlock();
+
+	return rc;
+}
+
+static int selinux_kdbus_init_inode(struct inode *inode,
+				    const struct cred *creds)
+{
+	struct inode_security_struct *isec = inode->i_security;
+	u32 sid = cred_sid(creds);
+
+	/* XXX - this is very simple, e.g. no transitions, no special object
+	 *       class, etc. since this inode is basically an IPC socket ...

security/selinux/hooks.c +4 -0

+
+	return 0;
+}
+

Quoted 8 lines · Paul Moore

security/selinux/hooks.c +8 -0

+static int selinux_kdbus_init_inode(struct inode *inode,
+				    const struct cred *creds)
+{
+	struct inode_security_struct *isec = inode->i_security;
+	u32 sid = cred_sid(creds);
+
+	/* XXX - this is very simple, e.g. no transitions, no special object
+	 *       class, etc. since this inode is basically an IPC socket ...

security/selinux/hooks.c static int selinux_binder_transfer_file(struct task_struct *from, · line 2172

Quoted 10 lines

security/selinux/hooks.c +10 -0

+	 *       however, is this too simple?  do we want transitions?  if we
+	 *       do, we should do the transition in kdbus_node_init() and not
+	 *       here so that endpoint is labeled correctly and not just this
+	 *       inode */
+
+	isec->inode = inode;
+	isec->task_sid = sid;
+	isec->sid = sid;
+	isec->sclass = SECCLASS_FILE;
+	isec->initialized = 1;

These are used for files exposed in the filesystem namespace, unlike

sockets (sockfs can't be mounted by userspace, and the socket objects themselves have their own class, so there is no ambiguity). Currently the only such files that are labeled with the same SID as the associated task are /proc files. So if we label the kdbusfs files with the same SID, then you can't allow read/write to kdbusfs nodes owned by another task without also exposing its /proc/pid files in the same manner. Doubt we want that. Probably should compute a transition from the task SID and the kdbusfs SID.

Quoted 61 lines · Paul Moore

The kdbus service names will be recorded using 'service', similar to the existing dbus audit records.

Signed-off-by Paul Moore <[email protected]>

---
ChangeLog:
- v3

* Ported to the 4.3-rc4 based kdbus tree

- v2

* Initial draft

---

  include/linux/lsm_audit.h |    2 ++
  security/lsm_audit.c      |    4 ++++
  2 files changed, 6 insertions(+)

include/linux/lsm_audit.h +1 -0

index ffb9c9d..d6a656f 100644
@@ -59,6 +59,7 @@ struct common_audit_data {
 
  #define LSM_AUDIT_DATA_INODE	9
  #define LSM_AUDIT_DATA_DENTRY	10
  #define LSM_AUDIT_DATA_IOCTL_OP	11
 
+#define LSM_AUDIT_DATA_KDBUS	12
 

  	union 	{

struct path path;
struct dentry *dentry;

include/linux/lsm_audit.h +1 -0

@@ -75,6 +76,7 @@ struct common_audit_data {
 
  #endif
 
  		char *kmod_name;
  		struct lsm_ioctlop_audit *op;
 
+		const char *kdbus_name;

  	} u;
  	/* this union contains LSM specific data */
  	union {

security/lsm_audit.c +0 -0

index cccbf30..0a3dc1b 100644
@@ -397,6 +397,10 @@ static void dump_common_audit_data(struct

audit_buffer *ab,>
audit_log_format(ab, " kmod="); audit_log_untrustedstring(ab, a->u.kmod_name); break;

security/lsm_audit.c static void dump_common_audit_data(struct audit_buffer *ab, · line 401

Quoted 2 lines

security/lsm_audit.c +2 -0

+	case LSM_AUDIT_DATA_KDBUS:
+		audit_log_format(ab, " service=");

Not a major issue to me, but just wondering if this needs to be further qualified to indicate it is a kdbus service. service= is rather generic.

From the audit perspective, its fine as service. Too many names that mean the

Quoted 7 lines

security/lsm_audit.c +2 -0

+		audit_log_untrustedstring(ab, a->u.kdbus_name);
+		break;
 
  	} /* switch (a->type) */
 
  }

Quoted 6 lines

_______________________________________________
Selinux mailing list
[email protected]
To unsubscribe, send email to [email protected]. To get help, send an email containing "help" to [email protected].

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

SELinux treats kdbus service names as objects and therefore needs a mechanism to map service names to security labels. This patch adds support for loading kdbus name/label matches with the security policy.

The patch supports service name prefix matching to lessen the burden on the policy developers and reduce the size of the resulting policy.

Signed-off-by Paul Moore <[email protected]>

---
ChangeLog:
- v3
* Ported to the 4.3-rc4 based kdbus tree, v2 hacks removed
- v2
* Porting needed to work with ioctl xperms
- v1
* Initial draft

---
  security/selinux/include/security.h |    5 ++
  security/selinux/ss/policydb.c      |   88 +++++++++++++++++++++++++++++------
  security/selinux/ss/policydb.h      |    3 +
  security/selinux/ss/services.c      |   38 +++++++++++++++
  4 files changed, 116 insertions(+), 18 deletions(-)

security/selinux/ss/policydb.c +28 -1

index 992a315..9be2e6d 100644
@@ -2111,7 +2116,7 @@ static int ocontext_read(struct policydb *p, struct policydb_compat_info *info,
  	int i, j, rc;
  	u32 nel, len;
  	__le32 buf[3];
-	struct ocontext *l, *c;
+	struct ocontext *l, *l2, *c;
  	u32 nodebuf[8];
 
  	for (i = 0; i < info->ocon_num; i++) {

@@ -2130,6 +2135,7 @@ static int ocontext_read(struct policydb *p, struct policydb_compat_info *info,
  				l->next = c;
  			else
  				p->ocontexts[i] = c;
+			l2 = l;
  			l = c;
 
  			switch (i) {

@@ -2219,6 +2225,43 @@ static int ocontext_read(struct policydb *p, struct policydb_compat_info *info,
  					goto out;
  				break;
  			}
+			case OCON_KDBUS: {
+				struct ocontext *iter, *last;
+				u32 len2;
+
+				rc = next_entry(buf, fp, sizeof(u32));
+				if (rc)
+					goto out;
+				len = le32_to_cpu(buf[0]);
+				rc = str_read(&c->u.name, GFP_KERNEL, fp, len);
+				if (rc)
+					goto out;
+				rc = context_read_and_validate(&c->context[0], p, fp);
+				if (rc) {
+					kfree(c->u.name);
+					goto out;
+				}
+
+				/* sort by ->u.name length, longest first */
+				last = NULL;
+				iter = p->ocontexts[OCON_KDBUS];
+				while (iter != c) {
+					len2 = strlen(iter->u.name);
+					if (len > len2) {
+						if (l2)
+							l2->next = NULL;
+						c->next = iter;

security/selinux/ss/policydb.c +14 -0

+			}
  			}
  		}
  	}

@@ -3147,6 +3190,19 @@ static int ocontext_write(struct policydb *p, struct policydb_compat_info *info,
  				if (rc)
  					return rc;
  				break;
+			case OCON_KDBUS:
+				len = strlen(c->u.name);
+				buf[0] = cpu_to_le32(len);
+				rc = put_entry(buf, sizeof(u32), 1, fp);
+				if (rc)
+					return rc;
+				rc = put_entry(c->u.name, len, 1, fp);
+				if (rc)
+					return rc;
+				rc = context_write(p, &c->context[0], fp);
+				if (rc)
+					return rc;
+				break;
  			}
  		}
  	}

security/selinux/ss/policydb.h +2 -1

index 725d594..ee9c120 100644
@@ -222,7 +222,8 @@ struct genfs {
  #define OCON_NODE  4	/* nodes */
  #define OCON_FSUSE 5	/* fs_use */
  #define OCON_NODE6 6	/* IPv6 nodes */
-#define OCON_NUM   7
+#define OCON_KDBUS 7	/* kdbus names */
+#define OCON_NUM   8
 
  /* The policy database */
  struct policydb {

security/selinux/ss/services.c +38 -0

index b7df12b..ada2d28 100644
@@ -2536,6 +2536,44 @@ int security_genfs_sid(const char *fstype,
  }
 
  /**
+ * security_kdbus_sid - Obtain a SID for a kdbus name
+ * @name: kdbus name
+ * @sid: SID for the kdbus name
+ *
+ * Obtain a SID for the given kdbus service name.  Returns zero on success,
+ * negative values on error.
+ */
+int security_kdbus_sid(const char *name, u32 *sid)
+{
+	int rc = 0;
+	struct ocontext *c;
+
+	read_lock(&policy_rwlock);
+
+	c = policydb.ocontexts[OCON_KDBUS];
+	while (c) {
+		if (strncmp(c->u.name, name, strlen(c->u.name)) == 0)
+			break;
+		c = c->next;
+	}
+
+	if (c) {
+		if (!c->sid[0]) {
+			rc = sidtab_context_to_sid(&sidtab,
+						   &c->context[0], &c->sid[0]);
+			if (rc)
+				goto out;
+		}
+		*sid = c->sid[0];
+	} else
+		*sid = SECINITSID_UNLABELED;
+
+out:
+	read_unlock(&policy_rwlock);
+	return rc;
+}
+
+/**
   * security_fs_use - Determine how to handle labeling for a filesystem.
   * @sb: superblock in question
   */

_______________________________________________
Selinux mailing list
[email protected]
To unsubscribe, send email to [email protected].
To get help, send an email containing "help" to [email protected].

Quoted 67 lines · Stephen Smalley

Quoted 61 lines · Paul Moore

The kdbus service names will be recorded using 'service', similar to the existing dbus audit records.

Signed-off-by Paul Moore <[email protected]>

---
ChangeLog:
- v3

* Ported to the 4.3-rc4 based kdbus tree

- v2

* Initial draft

---

   include/linux/lsm_audit.h |    2 ++
   security/lsm_audit.c      |    4 ++++
   2 files changed, 6 insertions(+)

include/linux/lsm_audit.h +1 -0

index ffb9c9d..d6a656f 100644
@@ -59,6 +59,7 @@ struct common_audit_data {
 
   #define LSM_AUDIT_DATA_INODE	9
   #define LSM_AUDIT_DATA_DENTRY	10
   #define LSM_AUDIT_DATA_IOCTL_OP	11
 
+#define LSM_AUDIT_DATA_KDBUS	12
 

   	union 	{

   		struct path path;
   		struct dentry *dentry;

include/linux/lsm_audit.h +1 -0

@@ -75,6 +76,7 @@ struct common_audit_data {
 
   #endif
 
   		char *kmod_name;
   		struct lsm_ioctlop_audit *op;
 
+		const char *kdbus_name;

   	} u;
   	/* this union contains LSM specific data */
   	union {

security/lsm_audit.c +0 -0

index cccbf30..0a3dc1b 100644
@@ -397,6 +397,10 @@ static void dump_common_audit_data(struct

audit_buffer *ab,>

   		audit_log_format(ab, " kmod=");
   		audit_log_untrustedstring(ab, a->u.kmod_name);
   		break;

security/lsm_audit.c static void dump_common_audit_data(struct audit_buffer *ab, · line 401

Quoted 2 lines

security/lsm_audit.c +2 -0

+	case LSM_AUDIT_DATA_KDBUS:
+		audit_log_format(ab, " service=");

Not a major issue to me, but just wondering if this needs to be further qualified to indicate it is a kdbus service. service= is rather generic.

Quoted 1 line

From the audit perspective, its fine as service. Too many names that mean the

same thing causes string lookup tables to get big. Service is what dbus is currently using. So, it makes sense to re-use the field name. If the selinux tooling wants to know an AVC originated from kdbus activity, then maybe another name=value should be added.

Quoted 21 lines · Stephen Smalley

Quoted 8 lines · Paul Moore

security/selinux/hooks.c +8 -0

+static int selinux_kdbus_init_inode(struct inode *inode,
+				    const struct cred *creds)
+{
+	struct inode_security_struct *isec = inode->i_security;
+	u32 sid = cred_sid(creds);
+
+	/* XXX - this is very simple, e.g. no transitions, no special object
+	 *       class, etc. since this inode is basically an IPC socket ...

security/selinux/hooks.c static int selinux_binder_transfer_file(struct task_struct *from, · line 2172

Quoted 10 lines

security/selinux/hooks.c +10 -0

+	 *       however, is this too simple?  do we want transitions?  if we
+	 *       do, we should do the transition in kdbus_node_init() and not
+	 *       here so that endpoint is labeled correctly and not just this
+	 *       inode */
+
+	isec->inode = inode;
+	isec->task_sid = sid;
+	isec->sid = sid;
+	isec->sclass = SECCLASS_FILE;
+	isec->initialized = 1;

These are used for files exposed in the filesystem namespace, unlike

Quoted 8 lines

sockets (sockfs can't be mounted by userspace, and the socket objects themselves have their own class, so there is no ambiguity). Currently the only such files that are labeled with the same SID as the associated task are /proc files. So if we label the kdbusfs files with the same SID, then you can't allow read/write to kdbusfs nodes owned by another task without also exposing its /proc/pid files in the same manner. Doubt we want that. Probably should compute a transition from the task SID and the kdbusfs SID.

Okay, that was one of my main concerns; your suggestion makes sense to me.

I'm also thinking that is we do a file transition using the task label and the kdbusfs superblock label we should limit it to just the inode label and not the kdbus endpoint as I suggested in the comment above (the bit about kdbus_node_init()), yes?

Quoted 11 lines · Paul Moore

Quoted 10 lines · Stephen Smalley

security/selinux/hooks.c static int selinux_binder_transfer_file(struct task_struct *from,

Quoted 9 lines · Paul Moore

security/selinux/hooks.c +9 -0

+static int selinux_kdbus_init_inode(struct inode *inode,
+				    const struct cred *creds)
+{
+	struct inode_security_struct *isec = inode->i_security;
+	u32 sid = cred_sid(creds);
+
+	/* XXX - this is very simple, e.g. no transitions, no special object
+	 *       class, etc. since this inode is basically an IPC socket ...
+	 *       however, is this too simple?  do we want transitions?  if 

Quoted 1 line

Quoted 1 line

Review

Quoted 1 line

(quoted diff) +1 -0

+	 *       do, we should do the transition in kdbus_node_init() and 

Quoted 1 line

Quoted 1 line

Review

Quoted 1 line

(quoted diff) +1 -0

+	 *       here so that endpoint is labeled correctly and not just 

Quoted 20 lines

Quoted 9 lines

security/selinux/hooks.c static int selinux_binder_transfer_file(struct task_struct *from, · line 2172

Quoted 7 lines

security/selinux/hooks.c +7 -0

+	 *       inode */
+
+	isec->inode = inode;
+	isec->task_sid = sid;
+	isec->sid = sid;
+	isec->sclass = SECCLASS_FILE;
+	isec->initialized = 1;

These are used for files exposed in the filesystem namespace, unlike

Quoted 8 lines

sockets (sockfs can't be mounted by userspace, and the socket objects themselves have their own class, so there is no ambiguity). Currently the only such files that are labeled with the same SID as the associated task are /proc files. So if we label the kdbusfs files with the same SID, then you can't allow read/write to kdbusfs nodes owned by another task without also exposing its /proc/pid files in the same manner. Doubt we want that. Probably should compute a transition from the task SID and the kdbusfs SID.

Okay, that was one of my main concerns; your suggestion makes sense to me.

Quoted 4 lines

I'm also thinking that is we do a file transition using the task label and the kdbusfs superblock label we should limit it to just the inode label and not the kdbus endpoint as I suggested in the comment above (the bit about kdbus_node_init()), yes?

Yes, it only needs to be done for the inode, not the endpoint. Analogy with sockets: Can I write to the socket file (kdbus file) bound to the socket (endpoint)? Can I connectto/sendto the socket (endpoint)?

Quoted 8 lines · Paul Moore

ipc/kdbus/connection.c +0 -0

index ef63d65..1cb87b3 100644
@@ -108,6 +109,14 @@ static struct kdbus_conn *kdbus_conn_new(struct

kdbus_ep *ep,>
if (!owner && (creds || pids || seclabel))

ipc/kdbus/connection.c static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, · line 112

Quoted 3 lines

  		return ERR_PTR(-EPERM);

ipc/kdbus/connection.c +1 -0

+	ret = security_kdbus_conn_new(get_cred(file->f_cred),

You only need to use get_cred() if saving a reference; otherwise, you'll leak one here.

Also, do we want file->f_cred here or ep->bus->node.creds (the latter is what is used by their own checks; the former is typically the same as current cred IIUC). For that matter, what about ep->node.creds vs ep->bus- node.creds vs. ep->bus->domain->node.creds? Can they differ? Do we care?

Quoted 13 lines

ipc/kdbus/connection.c +4 -2

@@ -1435,12 +1444,12 @@ bool kdbus_conn_policy_own_name(struct kdbus_conn
 *conn,> 
  			return false;
 
  	}
 
-	if (conn->owner)
-		return true;
+	if (!conn->owner &&
+	    kdbus_policy_query(&conn->ep->bus->policy_db, conn_creds, name,
+			       hash) < KDBUS_POLICY_OWN)
+		return false;
 

ipc/kdbus/connection.c bool kdbus_conn_policy_own_name(struct kdbus_conn *conn, · line 1452

Quoted 4 lines

ipc/kdbus/connection.c +1 -3

-	res = kdbus_policy_query(&conn->ep->bus->policy_db, conn_creds,
-				 name, hash);
-	return res >= KDBUS_POLICY_OWN;
+	return (security_kdbus_own_name(conn_creds, name) == 0);

Similar question here. conn_creds is the credentials of the creator of the connection, typically the client/sender, right? conn->ep->bus->node.creds are the credentials of the bus owner, so don't we want to ask "Can I own this name on this bus?".

From a SELinux point of view I imagine we would want access controls along the

Note that their policy checks are based on conn->ep->policy_db, i.e. the policy associated with the endpoint, and conn->owner is only true if the connection creator has the same uid as the bus.

Quoted 16 lines

ipc/kdbus/connection.c +5 -4

@@ -1465,14 +1474,13 @@ bool kdbus_conn_policy_talk(struct kdbus_conn
 *conn,> 
  					 to, KDBUS_POLICY_TALK))
 
  		return false;
 
-	if (conn->owner)
-		return true;
-	if (uid_eq(conn_creds->euid, to->cred->uid))
-		return true;
+	if (!conn->owner && !uid_eq(conn_creds->euid, to->cred->uid) &&
+	    !kdbus_conn_policy_query_all(conn, conn_creds,
+					 &conn->ep->bus->policy_db, to,
+					 KDBUS_POLICY_TALK))
+		return false;
 

ipc/kdbus/connection.c bool kdbus_conn_policy_talk(struct kdbus_conn *conn, · line 1483

Quoted 4 lines

ipc/kdbus/connection.c +1 -3

-	return kdbus_conn_policy_query_all(conn, conn_creds,
-					   &conn->ep->bus->policy_db, to,
-					   KDBUS_POLICY_TALK);
+	return (security_kdbus_conn_talk(conn_creds, to->cred) == 0);

Here at least we have a notion of client and peer. But we still aren't considering conn->ep or conn->ep->bus, whereas they are querying both policy dbs for their decision. The parallel would be checking access to the labels of both I suppose, unless we institute a control up front over the relationship between the label of the endpoint and the label of the bus.

Quoted 24 lines

ipc/kdbus/connection.c +0 -0

@@ -1491,19 +1499,19 @@ bool kdbus_conn_policy_see_name_unlocked(struct

kdbus_conn *conn,>
const struct cred *conn_creds, const char *name)

  {

ipc/kdbus/connection.c +6 -3

-	int res;
+	if (!conn_creds)
+		conn_creds = conn->cred;
 
  	/*
 
  	 * By default, all names are visible on a bus. SEE policies can only be
  	 * installed on custom endpoints, where by default no name is visible.
  	 */
 
-	if (!conn->ep->user)
-		return true;
+	if (conn->ep->user &&
+	    kdbus_policy_query_unlocked(&conn->ep->policy_db, conn_creds, name,
+					kdbus_strhash(name)) < KDBUS_POLICY_SEE)
+		return false;

ipc/kdbus/connection.c bool kdbus_conn_policy_see_name_unlocked(struct kdbus_conn *conn, · line 1514

Quoted 5 lines

ipc/kdbus/connection.c +1 -4

-	res = kdbus_policy_query_unlocked(&conn->ep->policy_db,
-					  conn_creds ? : conn->cred,
-					  name, kdbus_strhash(name));
-	return res >= KDBUS_POLICY_SEE;
+	return (security_kdbus_conn_see_name(conn_creds, name) == 0);

Here they only define policy based on endpoints, not bus. Not sure what we want, but we need at least one of their creds. Same for the rest.

Quoted 18 lines

ipc/kdbus/connection.c +0 -0

@@ -1530,10 +1541,13 @@ static bool kdbus_conn_policy_see(struct

kdbus_conn *conn,>
* peers from each other, unless you see at least _one_ name of the
* peer.
*/

ipc/kdbus/connection.c +7 -4

-	return !conn->ep->user ||
-	       kdbus_conn_policy_query_all(conn, conn_creds,
-					   &conn->ep->policy_db, whom,
-					   KDBUS_POLICY_SEE);
+	if (conn->ep->user &&
+	    !kdbus_conn_policy_query_all(conn, conn_creds,
+					 &conn->ep->policy_db, whom,
+					 KDBUS_POLICY_SEE))
+		return false;
+
+	return (security_kdbus_conn_see(conn_creds, whom->cred) == 0);

Quoted 1 line

  }

Quoted 24 lines

ipc/kdbus/connection.c +0 -0

@@ -1567,18 +1584,22 @@ bool kdbus_conn_policy_see_notification(struct

kdbus_conn *conn,>
case KDBUS_ITEM_NAME_ADD:
case KDBUS_ITEM_NAME_REMOVE:

case KDBUS_ITEM_NAME_CHANGE:

ipc/kdbus/connection.c +5 -3

-		return kdbus_conn_policy_see_name(conn, conn_creds,
-					msg->items[0].name_change.name);
+		if (!kdbus_conn_policy_see_name(conn, conn_creds,
+						msg->items[0].name_change.name))
+			return false;
 
  	case KDBUS_ITEM_ID_ADD:
 
  	case KDBUS_ITEM_ID_REMOVE:
-		return true;
+		/* fall through for the LSM check */
+		break;
 
  	default:
  		WARN(1, "Invalid type for notification broadcast: %llu\n",
 
  		     (unsigned long long)msg->items[0].type);

ipc/kdbus/connection.c bool kdbus_conn_policy_see_notification(struct kdbus_conn *conn,

Quoted 8 lines

return false;

  	}

ipc/kdbus/connection.c +2 -0

+
+	return (security_kdbus_conn_see_notification(conn_creds) == 0);
 
  }

Quoted 8 lines

ipc/kdbus/fs.c +0 -0

index 68818a8..4e84e89 100644
@@ -200,6 +202,10 @@ static struct inode *fs_inode_get(struct super_block
 *sb,> 
  	if (!(inode->i_state & I_NEW))
 

ipc/kdbus/fs.c static struct inode *fs_inode_get(struct super_block *sb, · line 207

Quoted 5 lines

ipc/kdbus/fs.c +3 -0

  		return inode;
 
+	ret = security_kdbus_init_inode(inode, node->creds);
+	if (ret)
+		return ERR_PTR(ret);

Need to put the inode.

Quoted 9 lines · Stephen Smalley

Quoted 8 lines · Paul Moore

ipc/kdbus/connection.c +0 -0

index ef63d65..1cb87b3 100644
@@ -108,6 +109,14 @@ static struct kdbus_conn *kdbus_conn_new(struct

kdbus_ep *ep,>

    if (!owner && (creds || pids || seclabel))

Quoted 6 lines

Quoted 3 lines

            return ERR_PTR(-EPERM);

+ ret = security_kdbus_conn_new(get_cred(file->f_cred),

You only need to use get_cred() if saving a reference; otherwise, you'll leak one here.

Yes, that was a typo on my part, thanks.

Quoted 4 lines

Also, do we want file->f_cred here or ep->bus->node.creds (the latter is what is used by their own checks; the former is typically the same as current cred IIUC). For that matter, what about ep->node.creds vs ep->bus- node.creds vs. ep->bus->domain->node.creds? Can they differ? Do we care?

We don't want file->f_cred, per our previous discussions. I was working on this patchset in small chunks and while I added credential storing in the nodes, I forgot to update the hooks before I hit send, my apologies.

My current thinking is to pass both the endpoint and bus credentials, as I believe they can differ. Both the bus and the endpoint inherit their security labels from their creator and while I don't have any specifics, I think it is reasonable to imagine those two processes having different security labels. Assuming we pass both credentials down to the LSM, I'm currently thinking of the following SELinux access controls for this hook:

allow <current> bus_t:kdbus { connect };
allow <current> ep_t:kdbus { use privileged activator monitor policy };

... besides the additional label, I added the kdbus:use permission and dropped the kdbus:owner permission. Considering that the endpoint label, ep_t, in the examples above, could be different from the current process, it seemed reasonable to want to control that interaction and I felt the fd:use permission was the closest existing control so I reused the permission name. I decided to drop the "owner" permission as it really wasn't the useful for anything anymore, it simply indicates that the current task is the DAC owner of the endpoint.

Quoted 14 lines

Quoted 14 lines

ipc/kdbus/connection.c +4 -2

@@ -1435,12 +1444,12 @@ bool kdbus_conn_policy_own_name(struct kdbus_conn
 *conn,>
                    return false;
 
    }
 
-   if (conn->owner)
-           return true;
+   if (!conn->owner &&
+       kdbus_policy_query(&conn->ep->bus->policy_db, conn_creds, name,
+                          hash) < KDBUS_POLICY_OWN)
+           return false;
 

Quoted 9 lines

ipc/kdbus/connection.c bool kdbus_conn_policy_own_name(struct kdbus_conn *conn, · line 1452

Quoted 4 lines

ipc/kdbus/connection.c +1 -3

-   res = kdbus_policy_query(&conn->ep->bus->policy_db, conn_creds,
-                            name, hash);
-   return res >= KDBUS_POLICY_OWN;
+   return (security_kdbus_own_name(conn_creds, name) == 0);

Similar question here. conn_creds is the credentials of the creator of the connection, typically the client/sender, right? conn->ep->bus->node.creds are the credentials of the bus owner, so don't we want to ask "Can I own this name on this bus?".

Yes, I think so.

From a SELinux point of view I imagine we would want access controls along the lines of the following:

allow current name_t:kdbus { own_name };
allow current bus_t:kdbus { own_name };

... do we want to use different permissions? I doubt it would matter much either way.

Quoted 3 lines

Note that their policy checks are based on conn->ep->policy_db, i.e. the policy associated with the endpoint, and conn->owner is only true if the connection creator has the same uid as the bus.

I don't think this is significant for us.

Quoted 16 lines

Quoted 16 lines

ipc/kdbus/connection.c +5 -4

@@ -1465,14 +1474,13 @@ bool kdbus_conn_policy_talk(struct kdbus_conn
 *conn,>
                                     to, KDBUS_POLICY_TALK))
 
            return false;
 
-   if (conn->owner)
-           return true;
-   if (uid_eq(conn_creds->euid, to->cred->uid))
-           return true;
+   if (!conn->owner && !uid_eq(conn_creds->euid, to->cred->uid) &&
+       !kdbus_conn_policy_query_all(conn, conn_creds,
+                                    &conn->ep->bus->policy_db, to,
+                                    KDBUS_POLICY_TALK))
+           return false;
 

Quoted 11 lines

ipc/kdbus/connection.c bool kdbus_conn_policy_talk(struct kdbus_conn *conn, · line 1483

Quoted 4 lines

ipc/kdbus/connection.c +1 -3

-   return kdbus_conn_policy_query_all(conn, conn_creds,
-                                      &conn->ep->bus->policy_db, to,
-                                      KDBUS_POLICY_TALK);
+   return (security_kdbus_conn_talk(conn_creds, to->cred) == 0);

Here at least we have a notion of client and peer. But we still aren't considering conn->ep or conn->ep->bus, whereas they are querying both policy dbs for their decision. The parallel would be checking access to the labels of both I suppose, unless we institute a control up front over the relationship between the label of the endpoint and the label of the bus.

While accidental, as I forgot to update this patch as mentioned previously, I think the differences between kdbus DAC and kdbus MAC is okay. At this point we've already authorized the individual nodes to connect to the bus and now we are authorizing them to talk to each other; at this control point, it is the interaction between the two nodes that we care about, I don't think we care about what bus they use, do we?

Quoted 25 lines

Quoted 25 lines

ipc/kdbus/connection.c +0 -0

@@ -1491,19 +1499,19 @@ bool kdbus_conn_policy_see_name_unlocked(struct

kdbus_conn *conn,>
const struct cred *conn_creds, const char *name)

  {

ipc/kdbus/connection.c +6 -3

-   int res;
+   if (!conn_creds)
+           conn_creds = conn->cred;
 
    /*
 
     * By default, all names are visible on a bus. SEE policies can only be
     * installed on custom endpoints, where by default no name is visible.
     */
 
-   if (!conn->ep->user)
-           return true;
+   if (conn->ep->user &&
+       kdbus_policy_query_unlocked(&conn->ep->policy_db, conn_creds, name,
+                                   kdbus_strhash(name)) < KDBUS_POLICY_SEE)
+           return false;

Quoted 8 lines

ipc/kdbus/connection.c bool kdbus_conn_policy_see_name_unlocked(struct kdbus_conn *conn, · line 1514

Quoted 5 lines

ipc/kdbus/connection.c +1 -4

-   res = kdbus_policy_query_unlocked(&conn->ep->policy_db,
-                                     conn_creds ? : conn->cred,
-                                     name, kdbus_strhash(name));
-   return res >= KDBUS_POLICY_SEE;
+   return (security_kdbus_conn_see_name(conn_creds, name) == 0);

Here they only define policy based on endpoints, not bus. Not sure what we want, but we need at least one of their creds. Same for the rest.

Once again, I'm not sure we care about the underlying bus at this point, do we? We've already authorized both the service and the client to connect to the bus, now I think all we care about is can the client see the service name. Do we really care about the label of the bus here?

Quoted 31 lines · Paul Moore

Quoted 8 lines · Stephen Smalley

Quoted 7 lines · Paul Moore

ipc/kdbus/connection.c +0 -0

index ef63d65..1cb87b3 100644
@@ -108,6 +109,14 @@ static struct kdbus_conn *kdbus_conn_new(struct

kdbus_ep *ep,>

Quoted 8 lines

Quoted 5 lines

    if (!owner && (creds || pids || seclabel))
    
            return ERR_PTR(-EPERM);

+ ret = security_kdbus_conn_new(get_cred(file->f_cred),

You only need to use get_cred() if saving a reference; otherwise, you'll leak one here.

Yes, that was a typo on my part, thanks.

Quoted 6 lines

Also, do we want file->f_cred here or ep->bus->node.creds (the latter is what is used by their own checks; the former is typically the same as current cred IIUC). For that matter, what about ep->node.creds vs ep->bus-
node.creds vs. ep->bus->domain->node.creds? Can they differ? Do we care?

We don't want file->f_cred, per our previous discussions. I was working on this patchset in small chunks and while I added credential storing in the nodes, I forgot to update the hooks before I hit send, my apologies.

Quoted 9 lines

My current thinking is to pass both the endpoint and bus credentials, as I believe they can differ. Both the bus and the endpoint inherit their security labels from their creator and while I don't have any specifics, I think it is reasonable to imagine those two processes having different security labels. Assuming we pass both credentials down to the LSM, I'm currently thinking of>
the following SELinux access controls for this hook:
allow <current> bus_t:kdbus { connect };
allow <current> ep_t:kdbus { use privileged activator monitor policy };

I think it would be simpler to apply an associate check when the endpoint is created between the endpoint label and the bus label (which will typically be the same), and then only check based on endpoint label for all subsequent permission checks involving that endpoint. Then you don't have to worry about which label to use for all the other permission checks. And you get finer-grained control - per-endpoint rather than only per-bus.

Quoted 8 lines

... besides the additional label, I added the kdbus:use permission and dropped the kdbus:owner permission. Considering that the endpoint label, ep_t, in the examples above, could be different from the current process, it seemed reasonable to want to control that interaction and I felt the fd:use permission was the closest existing control so I reused the permission name. I decided to drop the "owner" permission as it really wasn't the useful for anything anymore, it simply indicates that the current task is the DAC owner of the endpoint.

Can you 'use' an endpoint in any way other than to connect via it? If not, I'd just call that connect (won't conflict if you get rid of the separate bus check above), or distinguish it via separate classes or as connectthrough vs connectto.

conn->owner is used to determine whether the caller can fake credentials, skip kdbus policy checking, create an activator, monitor, or policy holder connection, etc. Our options are:

1. Apply a SELinux check when it is set to see if the caller is allowed to own the bus based on MAC labels and policy, and if not, refuse to create the connection (that's what checking the owner permission was doing).

2. Separately apply MAC checks over each of those abilities (fake creds, override policy, create an activator, monitor, or policy holder, etc) when there is an attempt to exercise them (not all during connection creation), and selectively deny that ability. More invasive, more potential for breakage for applications that don't expect failure if they could create the connection in the first place.

3. Treat faking of DAC credentials and skipping of kdbus policy checking as not of interest to MAC, leaving it only controlled by the existing uid match or CAP_IPC_OWNER check. Simple, but seems to lose some of the potential benefit of using SELinux for confining processes using kdbus.

privileged actually seems less interesting than owner these days, although I haven't done a thorough analysis.

IIUC, creating a monitor or policy holder connection has bus-wide implications and we ought to control it so we can limit it to specific processes, not just all uid-0 processes for the system bus and not just all uid-<user> processes for the per-user bus.

I'm not actually clear on the implications of activator connections or why they are limited in the same way.

Quoted 27 lines

Quoted 15 lines

Quoted 15 lines

ipc/kdbus/connection.c +0 -0

@@ -1435,12 +1444,12 @@ bool kdbus_conn_policy_own_name(struct

kdbus_conn
*conn,>

                    return false;
    
    }

ipc/kdbus/connection.c +4 -2

-   if (conn->owner)
-           return true;
+   if (!conn->owner &&
+       kdbus_policy_query(&conn->ep->bus->policy_db, conn_creds, name,
+                          hash) < KDBUS_POLICY_OWN)
+           return false;

Quoted 9 lines

ipc/kdbus/connection.c bool kdbus_conn_policy_own_name(struct kdbus_conn *conn, · line 1452

Quoted 4 lines

ipc/kdbus/connection.c +1 -3

-   res = kdbus_policy_query(&conn->ep->bus->policy_db, conn_creds,
-                            name, hash);
-   return res >= KDBUS_POLICY_OWN;
+   return (security_kdbus_own_name(conn_creds, name) == 0);

Similar question here. conn_creds is the credentials of the creator of the connection, typically the client/sender, right? conn->ep->bus->node.creds are the credentials of the bus owner, so don't we want to ask "Can I own this name on this bus?".

Yes, I think so.

Quoted 7 lines

From a SELinux point of view I imagine we would want access controls along the lines of the following:
allow current name_t:kdbus { own_name };
allow current bus_t:kdbus { own_name };

... do we want to use different permissions? I doubt it would matter much either way.

If we keep them both, then they need separate classes or separate permissions; I can't think of another case where we reuse the same class and permission for two different objects.

Not clear that this kind of pairwise checking will work well. For example, I should be able to own any name I like on my own bus, but not on the system bus.

Quoted 37 lines

Quoted 36 lines

Quoted 28 lines

ipc/kdbus/connection.c +0 -0

@@ -1491,19 +1499,19 @@ bool kdbus_conn_policy_see_name_unlocked(struct

kdbus_conn *conn,>

const struct cred *conn_creds, const char *name)

  {

ipc/kdbus/connection.c +4 -3

-   int res;
+   if (!conn_creds)
+           conn_creds = conn->cred;
 
    /*
 
     * By default, all names are visible on a bus. SEE policies can
     only be
     * installed on custom endpoints, where by default no name is
     visible.
     */
 
-   if (!conn->ep->user)
-           return true;
+   if (conn->ep->user &&
+       kdbus_policy_query_unlocked(&conn->ep->policy_db, conn_creds,

name,
+ kdbus_strhash(name)) <
KDBUS_POLICY_SEE) + return false;

ipc/kdbus/connection.c bool kdbus_conn_policy_see_name_unlocked(struct kdbus_conn *conn, · line 1514

Quoted 5 lines

ipc/kdbus/connection.c +1 -4

-   res = kdbus_policy_query_unlocked(&conn->ep->policy_db,
-                                     conn_creds ? : conn->cred,
-                                     name, kdbus_strhash(name));
-   return res >= KDBUS_POLICY_SEE;
+   return (security_kdbus_conn_see_name(conn_creds, name) == 0);

Here they only define policy based on endpoints, not bus. Not sure what we want, but we need at least one of their creds. Same for the rest.

Quoted 4 lines

Once again, I'm not sure we care about the underlying bus at this point, do we? We've already authorized both the service and the client to connect to the bus, now I think all we care about is can the client see the service name. Do we really care about the label of the bus here?

Well, here we again have the issue that we should be able to see all names on our own bus, but not necessarily on the system bus.