[RFC PATCH v1 0/3] Another take on the kdbus LSM hooks

[RFC PATCH v1 0/3] Another take on the kdbus LSM hooks

[Paul Moore]

A different take on the previous kdbus LSM hooks, intended to be much
simpler and more in line with what we currently do for binder and
other IPC mechanisms.  This patchset has three patches, the first
patch contains the LSM hooks and the last two patches are SELinux
specific implementations of those hooks.  Paul/Lukasz, please take a
look and see if this simplified set of hooks works for you; I'm hoping
it will.

This patchset is based of Greg's char-misc#kdbus tree which is a
little out of date with respect to LSM development, but that shouldn't
be a problem at this early stage of review.

I've intentionally only sent this to the SELinux and LSM list for the
time being; once we resolve our own concerns with the different
approaches we can start including the kdbus developers and other
relevant lists.  I'm also hoping that once we have a patchset which
contains the necessary SELinux/Smack/etc. support we can push this to
Greg for inclusion in the kdbus branch so we have at least some
kdbus/LSM support if/when kdbus is ever merged into Linus' tree.

You can find these patches in the working-kdbus-v1 branch of the
SELinux tree:

 * git://git.infradead.org/users/pcmoore/selinux

---

Paul Moore (3):
      lsm: introduce hooks for kdbus
      selinux: introduce kdbus names into the policy
      selinux: introduce kdbus access controls


 include/linux/security.h            |  113 +++++++++++++++++++++++++++++++++
 ipc/kdbus/connection.c              |   73 ++++++++++++++-------
 ipc/kdbus/message.c                 |   19 ++++-
 ipc/kdbus/metadata.c                |    6 +-
 security/security.c                 |   45 +++++++++++++
 security/selinux/hooks.c            |  121 ++++++++++++++++++++++++++++++++++-
 security/selinux/include/classmap.h |    4 +
 security/selinux/include/security.h |    6 +-
 security/selinux/ss/policydb.c      |   59 +++++++++++++++++
 security/selinux/ss/policydb.h      |    3 +
 security/selinux/ss/services.c      |   38 +++++++++++
 11 files changed, 449 insertions(+), 38 deletions(-)

[RFC PATCH v1 1/3] lsm: introduce hooks for kdbus

[Paul Moore]

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.

Signed-off-by: Paul Moore <pmoore@redhat.com>
---
 include/linux/security.h |  113 ++++++++++++++++++++++++++++++++++++++++++++++
 ipc/kdbus/connection.c   |   73 ++++++++++++++++++++----------
 ipc/kdbus/message.c      |   19 ++++++--
 ipc/kdbus/metadata.c     |    6 +-
 security/security.c      |   45 ++++++++++++++++++
 5 files changed, 223 insertions(+), 33 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 18264ea..ff3559f 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -53,6 +53,8 @@ struct msg_queue;
 struct xattr;
 struct xfrm_sec_ctx;
 struct mm_struct;
+struct kdbus_creds;
+struct kdbus_pids;
 
 /* Maximum number of letters for an LSM name string */
 #define SECURITY_NAME_MAX	10
@@ -1300,6 +1302,41 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
  *	@file contains the struct file being transferred.
  *	@to contains the task_struct for the receiving task.
  *
+ * @kdbus_conn_new
+ *	Check if the current task is allowed to create a new kdbus connection.
+ *	@creds credentials for the new connection
+ *	@fake_creds kdbus faked credentials
+ *	@fake_pids kdbus faked pids
+ *	@fake_seclabel kdbus faked security label
+ *	@owner kdbus owner
+ *	@privileged kdbus privileged
+ *	@is_activator kdbus activator boolean
+ *	@is_monitor kdbus monitor boolean
+ *	@is_policy_holder kdbus policy holder boolean
+ * @kdbus_own_name
+ *	Check if a connection is allowed to own a kdbus service name.
+ *	@creds requestor's credentials
+ *	@name service name
+ * @kdbus_conn_talk
+ *	Check if a connection is allowed to talk to a kdbus peer.
+ *	@creds requestor's credentials
+ *	@creds_peer peer credentials
+ * @kdbus_conn_see
+ *	Check if a connection can see a kdbus peer.
+ *	@creds requestor's credentials
+ *	@creds_peer peer credentials
+ * @kdbus_conn_see_name
+ *	Check if a connection can see a kdbus service name.
+ *	@creds requestor's credentials
+ *	@name service name
+ * @kdbus_conn_see_notification
+ *	Check if a connection can receive notifications.
+ *	@creds requestor's credentials
+ * @kdbus_proc_permission
+ *	Check if a connection can access another task's pid namespace info.
+ *	@cred requestor's credentials
+ *	@pid target task's pid struct
+ *
  * @ptrace_access_check:
  *	Check permission before allowing the current process to trace the
  *	@child process.
@@ -1468,6 +1505,21 @@ struct security_operations {
 	int (*binder_transfer_file) (struct task_struct *from,
 				     struct task_struct *to, struct file *file);
 
+	int (*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 (*kdbus_own_name)(const struct cred *creds, const char *name);
+	int (*kdbus_conn_talk)(const struct cred *creds,
+			       const struct cred *creds_peer);
+	int (*kdbus_conn_see)(const struct cred *creds,
+			      const struct cred *creds_peer);
+	int (*kdbus_conn_see_name)(const struct cred *creds, const char *name);
+	int (*kdbus_conn_see_notification)(const struct cred *creds);
+	int (*kdbus_proc_permission)(const struct cred *creds, struct pid *pid);
+
 	int (*ptrace_access_check) (struct task_struct *child, unsigned int mode);
 	int (*ptrace_traceme) (struct task_struct *parent);
 	int (*capget) (struct task_struct *target,
@@ -1772,6 +1824,20 @@ int security_binder_transfer_binder(struct task_struct *from,
 				    struct task_struct *to);
 int security_binder_transfer_file(struct task_struct *from,
 				  struct task_struct *to, struct file *file);
+int security_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 security_kdbus_own_name(const struct cred *creds, const char *name);
+int security_kdbus_conn_talk(const struct cred *creds,
+			     const struct cred *creds_peer);
+int security_kdbus_conn_see(const struct cred *creds,
+			    const struct cred *creds_peer);
+int security_kdbus_conn_see_name(const struct cred *creds, const char *name);
+int security_kdbus_conn_see_notification(const struct cred *creds);
+int security_kdbus_proc_permission(const struct cred *creds, struct pid *pid);
 int security_ptrace_access_check(struct task_struct *child, unsigned int mode);
 int security_ptrace_traceme(struct task_struct *parent);
 int security_capget(struct task_struct *target,
@@ -1984,6 +2050,53 @@ static inline int security_binder_transfer_file(struct task_struct *from,
 	return 0;
 }
 
+static inline int security_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)
+{
+	return 0;
+}
+
+static inline int security_kdbus_own_name(const struct cred *creds,
+					  const char *name)
+{
+	return 0;
+}
+
+static inline int security_kdbus_conn_talk(const struct cred *creds,
+					   const struct cred *creds_peer)
+{
+	return 0;
+}
+
+static inline int security_kdbus_conn_see(const struct cred *creds,
+					  const struct cred *creds_peer)
+{
+	return 0;
+}
+
+static inline int security_kdbus_conn_see_name(const struct cred *creds,
+					       const char *name)
+{
+	return 0;
+}
+
+static inline int security_kdbus_conn_see_notification(const struct cred *creds)
+{
+	return 0;
+}
+
+static inline int security_kdbus_proc_permission)(const struct cred *creds,
+						  struct pid *pid)
+{
+	return 0;
+}
+
 static inline int security_ptrace_access_check(struct task_struct *child,
 					     unsigned int mode)
 {
diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index ef63d65..be8d210 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -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>
@@ -213,6 +214,13 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep,
 			goto exit_unref;
 	}
 
+	ret = security_kdbus_conn_new(conn->cred, creds, pids, seclabel,
+				      owner, privileged,
+				      is_activator, is_monitor,
+				      is_policy_holder);
+	if (ret < 0)
+		goto exit_unref;
+
 	/*
 	 * Account the connection against the current user (UID), or for
 	 * custom endpoints use the anonymous user assigned to the endpoint.
@@ -1435,12 +1443,12 @@ bool kdbus_conn_policy_own_name(struct kdbus_conn *conn,
 			return false;
 	}
 
-	if (conn->owner)
-		return true;
-
 	res = kdbus_policy_query(&conn->ep->bus->policy_db, conn_creds,
 				 name, hash);
-	return res >= KDBUS_POLICY_OWN;
+	if (conn->owner || res >= KDBUS_POLICY_OWN)
+		return security_kdbus_own_name(conn_creds, name) == 0;
+
+	return false;
 }
 
 /**
@@ -1465,14 +1473,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) == 0)
+		return security_kdbus_conn_talk(conn_creds, to->cred) == 0;
 
-	return kdbus_conn_policy_query_all(conn, conn_creds,
-					   &conn->ep->bus->policy_db, to,
-					   KDBUS_POLICY_TALK);
+	return false;
 }
 
 /**
@@ -1493,17 +1500,19 @@ bool kdbus_conn_policy_see_name_unlocked(struct kdbus_conn *conn,
 {
 	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;
-
-	res = kdbus_policy_query_unlocked(&conn->ep->policy_db,
-					  conn_creds ? : conn->cred,
+	res = kdbus_policy_query_unlocked(&conn->ep->policy_db, conn_creds,
 					  name, kdbus_strhash(name));
-	return res >= KDBUS_POLICY_SEE;
+	if (!conn->ep->user || res >= KDBUS_POLICY_SEE)
+		return security_kdbus_conn_see_name(conn_creds, name) == 0;
+
+	return false;
 }
 
 static bool kdbus_conn_policy_see_name(struct kdbus_conn *conn,
@@ -1523,6 +1532,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 +1542,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 security_kdbus_conn_see(conn_creds, whom->cred) == 0;
+
+	return false;
 }
 
 /**
@@ -1551,6 +1566,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 +1585,25 @@ 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;
 	}
+
+	if (security_kdbus_conn_see_notification(conn_creds) < 0)
+		return false;
+
+	return true;
 }
 
 /**
diff --git a/ipc/kdbus/message.c b/ipc/kdbus/message.c
index ae565cd..acbe981 100644
--- a/ipc/kdbus/message.c
+++ b/ipc/kdbus/message.c
@@ -150,12 +150,17 @@ int kdbus_gaps_install(struct kdbus_gaps *gaps, struct kdbus_pool_slice *slice,
 		for (i = 0; i < gaps->n_fds; ++i) {
 			int fd;
 
+			ret = security_file_receive(gaps->fd_files[i]);
+			if (ret) {
+				incomplete_fds = true;
+				fds[n_fds++] = -1;
+				continue;
+			}
+
 			fd = get_unused_fd_flags(O_CLOEXEC);
 			if (fd < 0)
 				incomplete_fds = true;
 
-			WARN_ON(!gaps->fd_files[i]);
-
 			fds[n_fds++] = fd < 0 ? -1 : fd;
 		}
 
@@ -178,6 +183,13 @@ int kdbus_gaps_install(struct kdbus_gaps *gaps, struct kdbus_pool_slice *slice,
 	for (i = 0; i < gaps->n_memfds; ++i) {
 		int memfd;
 
+		ret = security_file_receive(gaps->memfd_files[i]);
+		if (ret) {
+			incomplete_fds = true;
+			fds[n_fds++] = -1;
+			continue;
+		}
+
 		memfd = get_unused_fd_flags(O_CLOEXEC);
 		if (memfd < 0) {
 			incomplete_fds = true;
@@ -194,9 +206,6 @@ int kdbus_gaps_install(struct kdbus_gaps *gaps, struct kdbus_pool_slice *slice,
 		 * message.
 		 */
 
-		WARN_ON(!gaps->memfd_offsets[i]);
-		WARN_ON(!gaps->memfd_files[i]);
-
 		kvec.iov_base = &memfd;
 		kvec.iov_len = sizeof(memfd);
 		ret = kdbus_pool_slice_copy_kvec(slice, gaps->memfd_offsets[i],
diff --git a/ipc/kdbus/metadata.c b/ipc/kdbus/metadata.c
index 71ca475..07c45d7 100644
--- a/ipc/kdbus/metadata.c
+++ b/ipc/kdbus/metadata.c
@@ -1182,11 +1182,9 @@ static unsigned int kdbus_proc_permission(const struct pid_namespace *pid_ns,
 					  const struct cred *cred,
 					  struct pid *target)
 {
-	if (pid_ns->hide_pid < 1)
-		return KDBUS_META_PROC_NORMAL;
-
 	/* XXX: we need groups_search() exported for aux-groups */
-	if (gid_eq(cred->egid, pid_ns->pid_gid))
+	if ((pid_ns->hide_pid < 1 || gid_eq(cred->egid, pid_ns->pid_gid)) &&
+	    security_kdbus_proc_permission(cred, target) == 0)
 		return KDBUS_META_PROC_NORMAL;
 
 	/*
diff --git a/security/security.c b/security/security.c
index 8e9b1f4..16cfa08 100644
--- a/security/security.c
+++ b/security/security.c
@@ -158,6 +158,51 @@ int security_binder_transfer_file(struct task_struct *from,
 	return security_ops->binder_transfer_file(from, to, file);
 }
 
+int security_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)
+{
+	return security_ops->kdbus_conn_new(creds, fake_creds, fake_pids,
+					    fake_seclabel, owner, privileged,
+					    is_activator, is_monitor,
+					    is_policy_holder);
+}
+
+int security_kdbus_own_name(const struct cred *creds, const char *name)
+{
+	return security_ops->kdbus_own_name(creds, name);
+}
+
+int security_kdbus_conn_talk(const struct cred *creds,
+			     const struct cred *creds_peer)
+{
+	return security_ops->kdbus_conn_talk(creds, creds_peer);
+}
+
+int security_kdbus_conn_see(const struct cred *creds,
+			    const struct cred *creds_peer)
+{
+	return security_ops->kdbus_conn_see(creds, creds_peer);
+}
+
+int security_kdbus_conn_see_name(const struct cred *creds, const char *name)
+{
+	return security_ops->kdbus_conn_see_name(creds, name);
+}
+
+int security_kdbus_conn_see_notification(const struct cred *creds)
+{
+	return security_ops->kdbus_conn_see_notification(creds);
+}
+
+int security_kdbus_proc_permission(const struct cred *creds, struct pid *pid)
+{
+	return security_ops->kdbus_proc_permission(creds, pid);
+}
+
 int security_ptrace_access_check(struct task_struct *child, unsigned int mode)
 {
 #ifdef CONFIG_SECURITY_YAMA_STACKED

[RFC PATCH v1 2/3] selinux: introduce kdbus names into the policy

[Paul Moore]

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 <pmoore@redhat.com>
---
 security/selinux/include/security.h |    6 +++-
 security/selinux/ss/policydb.c      |   59 ++++++++++++++++++++++++++++++++++-
 security/selinux/ss/policydb.h      |    3 +-
 security/selinux/ss/services.c      |   38 +++++++++++++++++++++++
 4 files changed, 103 insertions(+), 3 deletions(-)

diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index d1e0b23..693cb58 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -35,13 +35,15 @@
 #define POLICYDB_VERSION_NEW_OBJECT_DEFAULTS	27
 #define POLICYDB_VERSION_DEFAULT_TYPE	28
 #define POLICYDB_VERSION_CONSTRAINT_NAMES	29
+/* XXX - merge collision with POLICYDB_VERSION_XPERMS_IOCTL in v4.3-rc1 */
+#define POLICYDB_VERSION_KDBUS		30
 
 /* Range of policy versions we understand*/
 #define POLICYDB_VERSION_MIN   POLICYDB_VERSION_BASE
 #ifdef CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX
 #define POLICYDB_VERSION_MAX	CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE
 #else
-#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_CONSTRAINT_NAMES
+#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_KDBUS
 #endif
 
 /* Mask for just the mount related flags */
@@ -183,6 +185,8 @@ int security_fs_use(struct super_block *sb);
 int security_genfs_sid(const char *fstype, char *name, u16 sclass,
 	u32 *sid);
 
+int security_kdbus_sid(const char *name, u32 *sid);
+
 #ifdef CONFIG_NETLABEL
 int security_netlbl_secattr_to_sid(struct netlbl_lsm_secattr *secattr,
 				   u32 *sid);
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 74aa224..1fd3562 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -148,6 +148,12 @@ static struct policydb_compat_info policydb_compat[] = {
 		.sym_num	= SYM_NUM,
 		.ocon_num	= OCON_NUM,
 	},
+	/* NOTE: missing POLICYDB_VERSION_XPERMS_IOCTL */
+	{
+		.version	= POLICYDB_VERSION_KDBUS,
+		.sym_num	= SYM_NUM,
+		.ocon_num	= OCON_NUM,
+	},
 };
 
 static struct policydb_compat_info *policydb_lookup_compat(int version)
@@ -2106,7 +2112,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++) {
@@ -2125,6 +2131,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) {
@@ -2214,6 +2221,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;
+						if (last == NULL)
+							p->ocontexts[i] = c;
+						else
+							last->next = c;
+						break;
+					}
+					last = iter;
+					iter = iter->next;
+				}
+				break;
+			}
 			}
 		}
 	}
@@ -3142,6 +3186,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;
 			}
 		}
 	}
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index 725d594..ee9c120 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -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 {
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 9e2d820..efaf426 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2363,6 +2363,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
  */

[RFC PATCH v1 3/3] selinux: introduce kdbus access controls

[Paul Moore]

Add the SELinux access control implementation for the new kdbus LSM
hooks.

Signed-off-by: Paul Moore <pmoore@redhat.com>
---
 security/selinux/hooks.c            |  121 ++++++++++++++++++++++++++++++++++-
 security/selinux/include/classmap.h |    4 +
 2 files changed, 123 insertions(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 7dade28..1257ba4 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -9,8 +9,10 @@
  *	      James Morris <jmorris@redhat.com>
  *
  *  Copyright (C) 2001,2002 Networks Associates Technology, Inc.
- *  Copyright (C) 2003-2008 Red Hat, Inc., James Morris <jmorris@redhat.com>
- *					   Eric Paris <eparis@redhat.com>
+ *  Copyright (C) 2003-2008,2015 Red Hat, Inc.
+ *					James Morris <jmorris@redhat.com>
+ *					Eric Paris <eparis@redhat.com>
+ *					Paul Moore <paul@paul-moore.com>
  *  Copyright (C) 2004-2005 Trusted Computer Solutions, Inc.
  *			    <dgoeddel@trustedcs.com>
  *  Copyright (C) 2006, 2007, 2009 Hewlett-Packard Development Company, L.P.
@@ -1987,6 +1989,113 @@ 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;
+
+	rc = security_kdbus_sid(name, &name_sid);
+	if (rc)
+		return rc;
+
+	return avc_has_perm(cred_sid(creds), name_sid,
+			    SECCLASS_KDBUS, KDBUS__OWN, NULL);
+}
+
+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;
+
+	rc = security_kdbus_sid(name, &name_sid);
+	if (rc)
+		return rc;
+
+	return avc_has_perm(cred_sid(creds), name_sid,
+			    SECCLASS_KDBUS, KDBUS__SEE_NAME, NULL);
+}
+
+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_ptrace_access_check(struct task_struct *child,
 				     unsigned int mode)
 {
@@ -5848,6 +5957,14 @@ static struct security_operations selinux_ops = {
 	.binder_transfer_binder =	selinux_binder_transfer_binder,
 	.binder_transfer_file =		selinux_binder_transfer_file,
 
+	.kdbus_conn_new =		selinux_kdbus_conn_new,
+	.kdbus_own_name =		selinux_kdbus_own_name,
+	.kdbus_conn_talk =		selinux_kdbus_conn_talk,
+	.kdbus_conn_see_name = 		selinux_kdbus_conn_see_name,
+	.kdbus_conn_see =		selinux_kdbus_conn_see,
+	.kdbus_conn_see_notification =	selinux_kdbus_conn_see_notification,
+	.kdbus_proc_permission =	selinux_kdbus_proc_permission,
+
 	.ptrace_access_check =		selinux_ptrace_access_check,
 	.ptrace_traceme =		selinux_ptrace_traceme,
 	.capget =			selinux_capget,
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index eccd61b..31e4435 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -153,5 +153,9 @@ struct security_class_mapping secclass_map[] = {
 	  { COMMON_SOCK_PERMS, "attach_queue", NULL } },
 	{ "binder", { "impersonate", "call", "set_context_mgr", "transfer",
 		      NULL } },
+	{ "kdbus", { "impersonate", "fakecreds", "fakepids", "owner",
+		     "privileged", "activator", "monitor", "policy_holder",
+		     "connect", "own", "talk", "see", "see_name",
+		     "see_notification" } },
 	{ NULL }
   };

Re: [RFC PATCH v1 1/3] lsm: introduce hooks for kdbus

[Stephen Smalley]

On 09/23/2015 05:44 PM, Paul Moore wrote:
> 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.
> 
> Signed-off-by: Paul Moore <pmoore@redhat.com>
> ---
>  include/linux/security.h |  113 ++++++++++++++++++++++++++++++++++++++++++++++
>  ipc/kdbus/connection.c   |   73 ++++++++++++++++++++----------
>  ipc/kdbus/message.c      |   19 ++++++--
>  ipc/kdbus/metadata.c     |    6 +-
>  security/security.c      |   45 ++++++++++++++++++
>  5 files changed, 223 insertions(+), 33 deletions(-)
> 

> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index ef63d65..be8d210 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -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>
> @@ -213,6 +214,13 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep,
>  			goto exit_unref;
>  	}
>  
> +	ret = security_kdbus_conn_new(conn->cred, creds, pids, seclabel,
> +				      owner, privileged,
> +				      is_activator, is_monitor,
> +				      is_policy_holder);
> +	if (ret < 0)
> +		goto exit_unref;
> +

I think this could be moved up much earlier, right after the existing if
(!owner && (creds || pids || seclabel)) return ERR_PTR(-EPERM);, aside
from needing to pass file->f_cred instead of conn->cred at that point.
The advantage is that you can then just return the result immediately,
as there won't have been any allocation or setup of the conn structure,
and if permission was denied, you won't have wasted all of that
unnecessary processing.

>  	/*
>  	 * Account the connection against the current user (UID), or for
>  	 * custom endpoints use the anonymous user assigned to the endpoint.
> @@ -1435,12 +1443,12 @@ bool kdbus_conn_policy_own_name(struct kdbus_conn *conn,
>  			return false;
>  	}
>  
> -	if (conn->owner)
> -		return true;
> -
>  	res = kdbus_policy_query(&conn->ep->bus->policy_db, conn_creds,
>  				 name, hash);
> -	return res >= KDBUS_POLICY_OWN;
> +	if (conn->owner || res >= KDBUS_POLICY_OWN)
> +		return security_kdbus_own_name(conn_creds, name) == 0;
> +
> +	return false;

I could see them possibly objecting to this change on performance
grounds, as normally they wouldn't pay the cost of a
kdbus_policy_query() if conn->owner.  We could make it something like:
	if (!conn->owner) {
		res = kdbus_policy_query(...);
		if (res < KDBUS_POLICY_OWN)
			return false;
	}
	return (security_kdbus_own_name(conn_creds, name) == 0);
or even get rid of res and just coalesce it as:
	if (!conn->owner && kdbus_policy_query(...) < KDBUS_POLICY_OWN)
		return false;
	return (security_kdbus_own_name(conn_creds, name) == 0);

This has the side benefit of making the security hook call the tail of
the function.

>  }
>  
>  /**
> @@ -1465,14 +1473,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) == 0)

kdbus_conn_policy_query_all() returns a bool (false/0 on denial; true/1
on allowed).  So I think the test is wrong above?

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

This might be more readable as:
	if (!conn->owner && !uid_eq(conn_creds->euid, to->cred->uid) &&
		!kdbus_conn_policy_query_all(...))
		return false;
        return (security_kdbus_conn_talk(conn_creds, to->cred) == 0);
		
>  }
>  
>  /**
> @@ -1493,17 +1500,19 @@ bool kdbus_conn_policy_see_name_unlocked(struct kdbus_conn *conn,
>  {
>  	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;
> -
> -	res = kdbus_policy_query_unlocked(&conn->ep->policy_db,
> -					  conn_creds ? : conn->cred,
> +	res = kdbus_policy_query_unlocked(&conn->ep->policy_db, conn_creds,
>  					  name, kdbus_strhash(name));
> -	return res >= KDBUS_POLICY_SEE;
> +	if (!conn->ep->user || res >= KDBUS_POLICY_SEE)
> +		return security_kdbus_conn_see_name(conn_creds, name) == 0;
> +
> +	return false;

Similar to the first one, we could avoid the cost of
kdbus_policy_query_unlocked() here via:
	if (!conn->ep->user &&
		kdbus_policy_query_unlocked(...) < KDBUS_POLICY_SEE)
		return false;
	return (security_kdbus_conn_see_name(conn_creds, name) == 0);

>  }
>  
>  static bool kdbus_conn_policy_see_name(struct kdbus_conn *conn,
> @@ -1523,6 +1532,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 +1542,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 security_kdbus_conn_see(conn_creds, whom->cred) == 0;
> +
> +	return false;

Could restructure the same way, ala:
	if (!conn->ep->user && !kdbus_conn_policy_query_all(...))
		return false;
	return security_kdbus_conn_see(conn_creds, whom->cred) == 0);

>  }
>  
>  /**
> @@ -1551,6 +1566,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 +1585,25 @@ 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;
>  	}
> +
> +	if (security_kdbus_conn_see_notification(conn_creds) < 0)
> +		return false;
> +
> +	return true;

Could make this just:
	return (security_kdbus_conn_see_notification(conn_creds) == 0);

>  }
>  
>  /**

> diff --git a/ipc/kdbus/metadata.c b/ipc/kdbus/metadata.c
> index 71ca475..07c45d7 100644
> --- a/ipc/kdbus/metadata.c
> +++ b/ipc/kdbus/metadata.c
> @@ -1182,11 +1182,9 @@ static unsigned int kdbus_proc_permission(const struct pid_namespace *pid_ns,
>  					  const struct cred *cred,
>  					  struct pid *target)
>  {
> -	if (pid_ns->hide_pid < 1)
> -		return KDBUS_META_PROC_NORMAL;
> -
>  	/* XXX: we need groups_search() exported for aux-groups */
> -	if (gid_eq(cred->egid, pid_ns->pid_gid))
> +	if ((pid_ns->hide_pid < 1 || gid_eq(cred->egid, pid_ns->pid_gid)) &&
> +	    security_kdbus_proc_permission(cred, target) == 0)
>  		return KDBUS_META_PROC_NORMAL;

Not your fault, but I have to wonder why this function can't just return
a bool like the policy functions; it has only two return values.  Hardly
worth an enum.

Re: [RFC PATCH v1 1/3] lsm: introduce hooks for kdbus

[Stephen Smalley]

On 09/23/2015 05:44 PM, Paul Moore wrote:
> 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.
> 
> Signed-off-by: Paul Moore <pmoore@redhat.com>
> ---
>  include/linux/security.h |  113 ++++++++++++++++++++++++++++++++++++++++++++++
>  ipc/kdbus/connection.c   |   73 ++++++++++++++++++++----------
>  ipc/kdbus/message.c      |   19 ++++++--
>  ipc/kdbus/metadata.c     |    6 +-
>  security/security.c      |   45 ++++++++++++++++++
>  5 files changed, 223 insertions(+), 33 deletions(-)
> 

> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index ef63d65..be8d210 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -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>
> @@ -213,6 +214,13 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep,
>  			goto exit_unref;
>  	}
>  
> +	ret = security_kdbus_conn_new(conn->cred, creds, pids, seclabel,
> +				      owner, privileged,
> +				      is_activator, is_monitor,
> +				      is_policy_holder);
> +	if (ret < 0)
> +		goto exit_unref;
> +
>  	/*
>  	 * Account the connection against the current user (UID), or for
>  	 * custom endpoints use the anonymous user assigned to the endpoint.

It occurs to me that we aren't passing any information about the
endpoint or bus here.  conn->cred will be file->f_cred.  That will
generally be the same as the current cred since the process will have
opened the endpoint file in kdbusfs and then invoked ioctl with the
KDBUS_CMD_HELLO command on that open file.  So if I do a check against
that cred, it will always be to my own label, not to the actual creator
of the bus or the endpoint.

In comparison, kdbus_ep_is_owner(), which is called by this function,
checks file->f_cred->euid against ep->bus->node.uid.  Unfortunately the
kdbus_node struct does not contain a complete cred, just uid/gid/mode
information that gets used in various places, including for setting up
the pseudo file in kdbusfs that is used to access the bus or endpoint.
That seems to be kdbus' basic means of isolating different users; by
default, any bus or endpoint I create will only be accessible by my own
uid unless I choose to make it group or world accessible. current_euid()
and current_egid() are saved in the nodes when the bus or endpoint is
created, and later used in populating the inode and for uid comparisons.

If we want the same for MAC, I guess we either need kdbus_node to hold a
ref to a cred (and then we can pass ep->node->cred to the hooks), or
just add our own security field to kdbus_node.  The former seems cleaner
to me; we can then just take an additional reference to the bus or
endpoint creator's cred at creation time.  And then we need kdbusfs to
call a new hook on the inode and the cred in order to set the
inode->i_security to something appropriate for the bus or endpoint
creator.  Otherwise, we don't get any control over the ability to open
any given endpoint or bus in kdbusfs, as that is only subject to the
inode permission checks.

Re: [RFC PATCH v1 1/3] lsm: introduce hooks for kdbus

[Paul Moore]

On Thursday, September 24, 2015 11:57:07 AM Stephen Smalley wrote:
> On 09/23/2015 05:44 PM, Paul Moore wrote:

...

> > diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> > index ef63d65..be8d210 100644
> > --- a/ipc/kdbus/connection.c
> > +++ b/ipc/kdbus/connection.c
> > @@ -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>
> > 
> > @@ -213,6 +214,13 @@ static struct kdbus_conn *kdbus_conn_new(struct
> > kdbus_ep *ep,> 
> >  			goto exit_unref;
> >  	
> >  	}
> > 
> > +	ret = security_kdbus_conn_new(conn->cred, creds, pids, seclabel,
> > +				      owner, privileged,
> > +				      is_activator, is_monitor,
> > +				      is_policy_holder);
> > +	if (ret < 0)
> > +		goto exit_unref;
> > +
> 
> I think this could be moved up much earlier ...

All good suggestions, thank you.  I've incorporated all your suggestions into 
the patchset, I'll send a new version once I've addressed a few other things.

> > diff --git a/ipc/kdbus/metadata.c b/ipc/kdbus/metadata.c
> > index 71ca475..07c45d7 100644
> > --- a/ipc/kdbus/metadata.c
> > +++ b/ipc/kdbus/metadata.c
> > @@ -1182,11 +1182,9 @@ static unsigned int kdbus_proc_permission(const
> > struct pid_namespace *pid_ns,> 
> >  					  const struct cred *cred,
> >  					  struct pid *target)
> >  
> >  {
> > 
> > -	if (pid_ns->hide_pid < 1)
> > -		return KDBUS_META_PROC_NORMAL;
> > -
> > 
> >  	/* XXX: we need groups_search() exported for aux-groups */
> > 
> > -	if (gid_eq(cred->egid, pid_ns->pid_gid))
> > +	if ((pid_ns->hide_pid < 1 || gid_eq(cred->egid, pid_ns->pid_gid)) &&
> > +	    security_kdbus_proc_permission(cred, target) == 0)
> > 
> >  		return KDBUS_META_PROC_NORMAL;
> 
> Not your fault, but I have to wonder why this function can't just return
> a bool like the policy functions; it has only two return values.  Hardly
> worth an enum.

Agreed, I wondered about that too, however, in the interest of keep things 
small and focused I'm going to stay away from changing it.

-- 
paul moore
security @ redhat

Re: [RFC PATCH v1 1/3] lsm: introduce hooks for kdbus

[Paul Moore]

On Thursday, September 24, 2015 02:01:33 PM Stephen Smalley wrote:
> If we want the same for MAC, I guess we either need kdbus_node to hold a
> ref to a cred (and then we can pass ep->node->cred to the hooks), or
> just add our own security field to kdbus_node.  The former seems cleaner
> to me; we can then just take an additional reference to the bus or
> endpoint creator's cred at creation time.

Agreed.

> And then we need kdbusfs to call a new hook on the inode and the cred in
> order to set the inode->i_security to something appropriate for the bus or
> endpoint creator.  Otherwise, we don't get any control over the ability to
> open any given endpoint or bus in kdbusfs, as that is only subject to the
> inode permission checks.

I'll work on something and send out an updated patchset.

-- 
paul moore
security @ redhat