linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/8] Introduce LSM to KDBUS
@ 2015-07-08 10:25 Paul Osmialowski
  2015-07-08 10:25 ` [RFC 1/8] lsm: make security_file_receive available for external modules Paul Osmialowski
                   ` (8 more replies)
  0 siblings, 9 replies; 30+ messages in thread
From: Paul Osmialowski @ 2015-07-08 10:25 UTC (permalink / raw)
  To: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Paul Osmialowski, Lukasz Skalski

This patchset partially summarizes effects of collective work by
Karol Lewandowski and Paul Moore towards introduction of LSM into KDBUS.

These patches originate from following git repositories:

git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)

https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd-v212)

https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)

Since kdbus made its way to linux-next tree, I was kindly asked by
Karol Lewandowski to fit his work into the current kdbus code existing
there.

As both kdbus and security related code changed a bit, so are my changes
quite substantial in places.

Note that SELinux kdbus access control patches are absent - only SMACK part
of original work is included.

I've also made some changes to kdbus test suite. In order to see LSM hooks
in action we need to be able to run tests from different executable
binaries holding different security labels.

Therefore I added ability to select execution of particular test by
executed binary name. This is essential for running newly added 'send' test
which should communicate with 'daemon' test running in another process.

Karol Lewandowski (1):
  lsm: make security_file_receive available for external modules

Paul Osmialowski (7):
  lsm: smack: Make ipc/kdbus includes visible so smack callbacks could
    see them
  lsm: kdbus security hooks
  lsm: smack: smack callbacks for kdbus security hooks
  kdbus: use LSM hooks in kdbus code
  kdbus: TEST_CREATE_CONN now does no depend on TEST_CREATE_BUS
  kdbus: selftests extended
  kdbus: Ability to run kdbus test by executable binary name

 include/linux/lsm_hooks.h                        |  67 +++++++++++++
 include/linux/security.h                         |  99 +++++++++++++++++++
 ipc/kdbus/bus.c                                  |  12 ++-
 ipc/kdbus/bus.h                                  |   3 +
 ipc/kdbus/connection.c                           |  54 +++++++++++
 ipc/kdbus/connection.h                           |   4 +
 ipc/kdbus/domain.c                               |   9 +-
 ipc/kdbus/domain.h                               |   2 +
 ipc/kdbus/endpoint.c                             |  11 +++
 ipc/kdbus/names.c                                |  11 +++
 ipc/kdbus/queue.c                                |  30 ++++--
 security/security.c                              | 118 +++++++++++++++++++++++
 security/smack/Makefile                          |   2 +
 security/smack/smack_lsm.c                       |  68 +++++++++++++
 tools/testing/selftests/kdbus/Makefile           |   1 +
 tools/testing/selftests/kdbus/kdbus-test.c       |  37 ++++++-
 tools/testing/selftests/kdbus/kdbus-test.h       |   1 +
 tools/testing/selftests/kdbus/kdbus-util.c       |  37 ++++---
 tools/testing/selftests/kdbus/kdbus-util.h       |   2 +-
 tools/testing/selftests/kdbus/test-activator.c   |  20 ++--
 tools/testing/selftests/kdbus/test-chat.c        |   6 +-
 tools/testing/selftests/kdbus/test-connection.c  |   8 +-
 tools/testing/selftests/kdbus/test-fd.c          |   2 +-
 tools/testing/selftests/kdbus/test-message.c     |  69 ++++++++-----
 tools/testing/selftests/kdbus/test-metadata-ns.c |  10 +-
 tools/testing/selftests/kdbus/test-monitor.c     |   9 +-
 tools/testing/selftests/kdbus/test-policy-ns.c   |   8 +-
 tools/testing/selftests/kdbus/test-policy-priv.c |  48 +++++----
 tools/testing/selftests/kdbus/test-send.c        |  84 ++++++++++++++++
 tools/testing/selftests/kdbus/test-sync.c        |   2 +-
 tools/testing/selftests/kdbus/test-timeout.c     |   2 +-
 31 files changed, 732 insertions(+), 104 deletions(-)
 create mode 100644 tools/testing/selftests/kdbus/test-send.c

-- 
1.9.1


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFC 1/8] lsm: make security_file_receive available for external modules
  2015-07-08 10:25 [RFC 0/8] Introduce LSM to KDBUS Paul Osmialowski
@ 2015-07-08 10:25 ` Paul Osmialowski
  2015-07-08 10:25 ` [RFC 2/8] lsm: smack: Make ipc/kdbus includes visible so smack callbacks could see them Paul Osmialowski
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 30+ messages in thread
From: Paul Osmialowski @ 2015-07-08 10:25 UTC (permalink / raw)
  To: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Paul Osmialowski, Lukasz Skalski

From: Karol Lewandowski <k.lewandowsk@samsung.com>

This is required for using filedesc related LSM hooks in kdbus code
if it is built as a module.

Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
---
 security/security.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/security/security.c b/security/security.c
index 595fffa..b1e935b 100644
--- a/security/security.c
+++ b/security/security.c
@@ -837,6 +837,7 @@ int security_file_receive(struct file *file)
 {
 	return call_int_hook(file_receive, 0, file);
 }
+EXPORT_SYMBOL(security_file_receive);
 
 int security_file_open(struct file *file, const struct cred *cred)
 {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 2/8] lsm: smack: Make ipc/kdbus includes visible so smack callbacks could see them
  2015-07-08 10:25 [RFC 0/8] Introduce LSM to KDBUS Paul Osmialowski
  2015-07-08 10:25 ` [RFC 1/8] lsm: make security_file_receive available for external modules Paul Osmialowski
@ 2015-07-08 10:25 ` Paul Osmialowski
  2015-07-08 16:43   ` Daniel Mack
  2015-07-08 10:25 ` [RFC 3/8] lsm: kdbus security hooks Paul Osmialowski
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 30+ messages in thread
From: Paul Osmialowski @ 2015-07-08 10:25 UTC (permalink / raw)
  To: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Paul Osmialowski, Lukasz Skalski

Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
---
 security/smack/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/security/smack/Makefile b/security/smack/Makefile
index ee2ebd5..bd6927c 100644
--- a/security/smack/Makefile
+++ b/security/smack/Makefile
@@ -6,3 +6,5 @@ obj-$(CONFIG_SECURITY_SMACK) := smack.o
 
 smack-y := smack_lsm.o smack_access.o smackfs.o
 smack-$(CONFIG_SECURITY_SMACK_NETFILTER) += smack_netfilter.o
+
+ccflags-y += -Iipc
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 3/8] lsm: kdbus security hooks
  2015-07-08 10:25 [RFC 0/8] Introduce LSM to KDBUS Paul Osmialowski
  2015-07-08 10:25 ` [RFC 1/8] lsm: make security_file_receive available for external modules Paul Osmialowski
  2015-07-08 10:25 ` [RFC 2/8] lsm: smack: Make ipc/kdbus includes visible so smack callbacks could see them Paul Osmialowski
@ 2015-07-08 10:25 ` Paul Osmialowski
  2015-07-08 11:00   ` Lukasz Pawelczyk
  2015-07-08 14:14   ` Greg Kroah-Hartman
  2015-07-08 10:25 ` [RFC 4/8] lsm: smack: smack callbacks for " Paul Osmialowski
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 30+ messages in thread
From: Paul Osmialowski @ 2015-07-08 10:25 UTC (permalink / raw)
  To: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Paul Osmialowski, Lukasz Skalski

This is combination of the work by Karol Lewandowski and Paul Moore
on LSM hooks for kdbus.

Originates from:

git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)
commit: 7050f206a79564886938d0edc4e1e9da5972c72d

https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd-v212)
commit: a9fe4c33b6e5ab25a243e0590df406aabb6add12

Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
Signed-off-by: Paul Moore <pmoore@redhat.com>
Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
---
 include/linux/lsm_hooks.h |  67 ++++++++++++++++++++++++++
 include/linux/security.h  |  99 +++++++++++++++++++++++++++++++++++++++
 security/security.c       | 117 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 283 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 9429f05..2a8d8fc 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1297,6 +1297,36 @@
  *	@inode we wish to get the security context of.
  *	@ctx is a pointer in which to place the allocated security context.
  *	@ctxlen points to the place to put the length of @ctx.
+ *
+ * @kdbus_domain_alloc:
+ *	Allocate kdbus domain.
+ * @kdbus_domain_free:
+ *	Deallocate kdbus domain.
+ * @kdbus_bus_alloc:
+ *	Allocate kdbus bus.
+ * @kdbus_bus_free:
+ *	Deallocate kdbus bus.
+ * @kdbus_send:
+ *	Send message.
+ * @kdbus_recv:
+ *	Receive message.
+ * @kdbus_name_acquire:
+ *	Request a well-known bus name to associate with the connection.
+ * @kdbus_name_list:
+ *	Retrieve the list of all currently registered well-known and unique
+ *	names.
+ * @kdbus_ep_create:
+ *	Endpoint create
+ * @kdbus_connect:
+ *	Connect
+ * @kdbus_conn_free:
+ *	Deallocate connection
+ * @kdbus_conn_info:
+ *	Retrieve credentials and properties of the initial creator of the
+ *	connection.
+ * @kdbus_talk:
+ *	Talk to a given peer.
+ *
  * This is the main security structure.
  */
 
@@ -1520,6 +1550,29 @@ union security_list_options {
 	int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen);
 	int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);
 
+	int (*kdbus_domain_alloc)(struct kdbus_domain *domain);
+	void (*kdbus_domain_free)(struct kdbus_domain *domain);
+
+	int (*kdbus_bus_alloc)(struct kdbus_bus *bus);
+	void (*kdbus_bus_free)(struct kdbus_bus *bus);
+	int (*kdbus_send)(const struct kdbus_conn *conn,
+			  const struct kdbus_bus *bus);
+	int (*kdbus_recv)(const struct kdbus_conn *conn,
+			  const struct kdbus_bus *bus);
+	int (*kdbus_name_acquire)(const struct kdbus_conn *conn,
+			  const char *name);
+	int (*kdbus_name_list)(const struct kdbus_bus *bus);
+
+	int (*kdbus_ep_create)(const struct kdbus_bus *bus);
+	int (*kdbus_ep_setpolicy)(const struct kdbus_bus *bus);
+
+	int (*kdbus_connect)(struct kdbus_conn *conn,
+			     const char *secctx, u32 seclen);
+	void (*kdbus_conn_free)(struct kdbus_conn *conn);
+	int (*kdbus_conn_info)(const struct kdbus_conn *conn);
+	int (*kdbus_talk)(const struct kdbus_conn *src,
+			  const struct kdbus_conn *dst);
+
 #ifdef CONFIG_SECURITY_NETWORK
 	int (*unix_stream_connect)(struct sock *sock, struct sock *other,
 					struct sock *newsk);
@@ -1760,6 +1813,20 @@ struct security_hook_heads {
 	struct list_head inode_notifysecctx;
 	struct list_head inode_setsecctx;
 	struct list_head inode_getsecctx;
+	struct list_head kdbus_domain_alloc;
+	struct list_head kdbus_domain_free;
+	struct list_head kdbus_bus_alloc;
+	struct list_head kdbus_bus_free;
+	struct list_head kdbus_send;
+	struct list_head kdbus_recv;
+	struct list_head kdbus_name_acquire;
+	struct list_head kdbus_name_list;
+	struct list_head kdbus_ep_create;
+	struct list_head kdbus_ep_setpolicy;
+	struct list_head kdbus_connect;
+	struct list_head kdbus_conn_free;
+	struct list_head kdbus_conn_info;
+	struct list_head kdbus_talk;
 #ifdef CONFIG_SECURITY_NETWORK
 	struct list_head unix_stream_connect;
 	struct list_head unix_may_send;
diff --git a/include/linux/security.h b/include/linux/security.h
index 79d85dd..5f257b9 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -53,6 +53,10 @@ struct msg_queue;
 struct xattr;
 struct xfrm_sec_ctx;
 struct mm_struct;
+struct kdbus_ep;
+struct kdbus_bus;
+struct kdbus_conn;
+struct kdbus_domain;
 
 /* If capable should audit the security request */
 #define SECURITY_CAP_NOAUDIT 0
@@ -356,6 +360,28 @@ void security_release_secctx(char *secdata, u32 seclen);
 int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
 int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
 int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
+
+int security_kdbus_domain_alloc(struct kdbus_domain *domain);
+void security_kdbus_domain_free(struct kdbus_domain *domain);
+
+int security_kdbus_bus_alloc(struct kdbus_bus *bus);
+void security_kdbus_bus_free(struct kdbus_bus *bus);
+int security_kdbus_send(const struct kdbus_conn *conn,
+			const struct kdbus_bus *bus);
+int security_kdbus_recv(const struct kdbus_conn *conn,
+			const struct kdbus_bus *bus);
+int security_kdbus_name_acquire(const struct kdbus_conn *conn,
+				const char *name);
+int security_kdbus_name_list(const struct kdbus_bus *bus);
+int security_kdbus_ep_create(struct kdbus_bus *bus);
+int security_kdbus_ep_setpolicy(struct kdbus_bus *bus);
+int security_kdbus_connect(struct kdbus_conn *conn,
+			   const char *secctx, u32 seclen);
+void security_kdbus_conn_free(struct kdbus_conn *conn);
+int security_kdbus_conn_info(const struct kdbus_conn *conn);
+int security_kdbus_talk(const struct kdbus_conn *src,
+			const struct kdbus_conn *dst);
+
 #else /* CONFIG_SECURITY */
 struct security_mnt_opts {
 };
@@ -1105,6 +1131,79 @@ static inline int security_inode_getsecctx(struct inode *inode, void **ctx, u32
 {
 	return -EOPNOTSUPP;
 }
+
+static inline int security_kdbus_domain_alloc(struct kdbus_domain *domain)
+{
+	return 0;
+}
+
+static inline void security_kdbus_domain_free(struct kdbus_domain *domain)
+{
+}
+
+static inline int security_kdbus_bus_alloc(struct kdbus_bus *bus)
+{
+	return 0;
+}
+
+static inline void security_kdbus_bus_free(struct kdbus_bus *bus)
+{
+}
+
+static inline int security_kdbus_send(const struct kdbus_conn *conn,
+				      const struct kdbus_bus *bus)
+{
+	return 0;
+}
+
+static inline int security_kdbus_recv(const struct kdbus_conn *conn,
+				      const struct kdbus_bus *bus)
+{
+	return 0;
+}
+
+static inline int security_kdbus_name_acquire(const struct kdbus_conn *conn,
+					      const char *name)
+{
+	return 0;
+}
+
+static inline int security_kdbus_name_list(const struct kdbus_bus *bus)
+{
+	return 0;
+}
+
+static inline int security_kdbus_ep_create(const struct kdbus_bus *bus)
+{
+	return 0;
+}
+
+static inline int security_kdbus_ep_setpolicy(const struct kdbus_bus *bus)
+{
+	return 0;
+}
+
+static inline int security_kdbus_connect(struct kdbus_conn *conn,
+					 const char *secctx, u32 seclen)
+{
+	return 0;
+}
+
+static inline void security_kdbus_conn_free(struct kdbus_conn *conn)
+{
+}
+
+static inline int security_kdbus_conn_info(const struct kdbus_conn *conn)
+{
+	return 0;
+}
+
+static inline int security_kdbus_talk(const struct kdbus_conn *src,
+				      const struct kdbus_conn *dst)
+{
+	return 0;
+}
+
 #endif	/* CONFIG_SECURITY */
 
 #ifdef CONFIG_SECURITY_NETWORK
diff --git a/security/security.c b/security/security.c
index b1e935b..7fb46d1 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1185,6 +1185,95 @@ int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
 }
 EXPORT_SYMBOL(security_inode_getsecctx);
 
+int security_kdbus_domain_alloc(struct kdbus_domain *domain)
+{
+	return call_int_hook(kdbus_domain_alloc, 0, domain);
+}
+EXPORT_SYMBOL(security_kdbus_domain_alloc);
+
+void security_kdbus_domain_free(struct kdbus_domain *domain)
+{
+	call_void_hook(kdbus_domain_free, domain);
+}
+EXPORT_SYMBOL(security_kdbus_domain_free);
+
+int security_kdbus_bus_alloc(struct kdbus_bus *bus)
+{
+	return call_int_hook(kdbus_bus_alloc, 0, bus);
+}
+EXPORT_SYMBOL(security_kdbus_bus_alloc);
+
+void security_kdbus_bus_free(struct kdbus_bus *bus)
+{
+	call_void_hook(kdbus_bus_free, bus);
+}
+EXPORT_SYMBOL(security_kdbus_bus_free);
+
+int security_kdbus_send(const struct kdbus_conn *conn,
+			const struct kdbus_bus *bus)
+{
+	return call_int_hook(kdbus_send, 0, conn, bus);
+}
+EXPORT_SYMBOL(security_kdbus_send);
+
+int security_kdbus_recv(const struct kdbus_conn *conn,
+			const struct kdbus_bus *bus)
+{
+	return call_int_hook(kdbus_recv, 0, conn, bus);
+}
+EXPORT_SYMBOL(security_kdbus_recv);
+
+int security_kdbus_name_acquire(const struct kdbus_conn *conn,
+				const char *name)
+{
+	return call_int_hook(kdbus_name_acquire, 0, conn, name);
+}
+EXPORT_SYMBOL(security_kdbus_name_acquire);
+
+int security_kdbus_name_list(const struct kdbus_bus *bus)
+{
+	return call_int_hook(kdbus_name_list, 0, bus);
+}
+EXPORT_SYMBOL(security_kdbus_name_list);
+
+int security_kdbus_ep_create(struct kdbus_bus *bus)
+{
+	return call_int_hook(kdbus_ep_create, 0, bus);
+}
+EXPORT_SYMBOL(security_kdbus_ep_create);
+
+int security_kdbus_ep_setpolicy(struct kdbus_bus *bus)
+{
+	return call_int_hook(kdbus_ep_setpolicy, 0, bus);
+}
+EXPORT_SYMBOL(security_kdbus_ep_setpolicy);
+
+int security_kdbus_connect(struct kdbus_conn *conn,
+			   const char *secctx, u32 seclen)
+{
+	return call_int_hook(kdbus_connect, 0, conn, secctx, seclen);
+}
+EXPORT_SYMBOL(security_kdbus_connect);
+
+void security_kdbus_conn_free(struct kdbus_conn *conn)
+{
+	call_void_hook(kdbus_conn_free, conn);
+}
+EXPORT_SYMBOL(security_kdbus_conn_free);
+
+int security_kdbus_conn_info(const struct kdbus_conn *conn)
+{
+	return call_int_hook(kdbus_conn_info, 0, conn);
+}
+EXPORT_SYMBOL(security_kdbus_conn_info);
+
+int security_kdbus_talk(const struct kdbus_conn *src,
+			const struct kdbus_conn *dst)
+{
+	return call_int_hook(kdbus_talk, 0, src, dst);
+}
+EXPORT_SYMBOL(security_kdbus_talk);
+
 #ifdef CONFIG_SECURITY_NETWORK
 
 int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk)
@@ -1774,6 +1863,34 @@ struct security_hook_heads security_hook_heads = {
 		LIST_HEAD_INIT(security_hook_heads.inode_setsecctx),
 	.inode_getsecctx =
 		LIST_HEAD_INIT(security_hook_heads.inode_getsecctx),
+	.kdbus_domain_alloc =
+		LIST_HEAD_INIT(security_hook_heads.kdbus_domain_alloc),
+	.kdbus_domain_free =
+		LIST_HEAD_INIT(security_hook_heads.kdbus_domain_free),
+	.kdbus_bus_alloc =
+		LIST_HEAD_INIT(security_hook_heads.kdbus_bus_alloc),
+	.kdbus_bus_free =
+		LIST_HEAD_INIT(security_hook_heads.kdbus_bus_free),
+	.kdbus_send =
+		LIST_HEAD_INIT(security_hook_heads.kdbus_send),
+	.kdbus_recv =
+		LIST_HEAD_INIT(security_hook_heads.kdbus_recv),
+	.kdbus_name_acquire =
+		LIST_HEAD_INIT(security_hook_heads.kdbus_name_acquire),
+	.kdbus_name_list =
+		LIST_HEAD_INIT(security_hook_heads.kdbus_name_list),
+	.kdbus_ep_create =
+		LIST_HEAD_INIT(security_hook_heads.kdbus_ep_create),
+	.kdbus_ep_setpolicy =
+		LIST_HEAD_INIT(security_hook_heads.kdbus_ep_setpolicy),
+	.kdbus_connect =
+		LIST_HEAD_INIT(security_hook_heads.kdbus_connect),
+	.kdbus_conn_free =
+		LIST_HEAD_INIT(security_hook_heads.kdbus_conn_free),
+	.kdbus_conn_info =
+		LIST_HEAD_INIT(security_hook_heads.kdbus_conn_info),
+	.kdbus_talk =
+		LIST_HEAD_INIT(security_hook_heads.kdbus_talk),
 #ifdef CONFIG_SECURITY_NETWORK
 	.unix_stream_connect =
 		LIST_HEAD_INIT(security_hook_heads.unix_stream_connect),
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 4/8] lsm: smack: smack callbacks for kdbus security hooks
  2015-07-08 10:25 [RFC 0/8] Introduce LSM to KDBUS Paul Osmialowski
                   ` (2 preceding siblings ...)
  2015-07-08 10:25 ` [RFC 3/8] lsm: kdbus security hooks Paul Osmialowski
@ 2015-07-08 10:25 ` Paul Osmialowski
  2015-07-08 13:42   ` Stephen Smalley
  2015-07-09 10:08   ` Sergei Zviagintsev
  2015-07-08 10:25 ` [RFC 5/8] kdbus: use LSM hooks in kdbus code Paul Osmialowski
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 30+ messages in thread
From: Paul Osmialowski @ 2015-07-08 10:25 UTC (permalink / raw)
  To: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Paul Osmialowski, Lukasz Skalski

This adds implementation of three smack callbacks sitting behind kdbus
security hooks as proposed by Karol Lewandowski.

Originates from:

git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)
commit: fc3505d058c001fe72a6f66b833e0be5b2d118f3

https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd-v212)
commit: 103c26fd27d1ec8c32d85dd3d85681f936ac66fb

Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
---
 security/smack/smack_lsm.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index a143328..033b756 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -41,6 +41,7 @@
 #include <linux/msg.h>
 #include <linux/shm.h>
 #include <linux/binfmts.h>
+#include <kdbus/connection.h>
 #include "smack.h"
 
 #define TRANS_TRUE	"TRUE"
@@ -3336,6 +3337,69 @@ static int smack_setprocattr(struct task_struct *p, char *name,
 }
 
 /**
+ * smack_kdbus_connect - Set the security blob for a KDBus connection
+ * @conn: the connection
+ * @secctx: smack label
+ * @seclen: smack label length
+ *
+ * Returns 0
+ */
+static int smack_kdbus_connect(struct kdbus_conn *conn,
+			       const char *secctx, u32 seclen)
+{
+	struct smack_known *skp;
+
+	if (secctx && seclen > 0)
+		skp = smk_import_entry(secctx, seclen);
+	else
+		skp = smk_of_current();
+	conn->security = skp;
+
+	return 0;
+}
+
+/**
+ * smack_kdbus_conn_free - Clear the security blob for a KDBus connection
+ * @conn: the connection
+ *
+ * Clears the blob pointer
+ */
+static void smack_kdbus_conn_free(struct kdbus_conn *conn)
+{
+	conn->security = NULL;
+}
+
+/**
+ * smack_kdbus_talk - Smack access on KDBus
+ * @src: source kdbus connection
+ * @dst: destination kdbus connection
+ *
+ * Return 0 if a subject with the smack of sock could access
+ * an object with the smack of other, otherwise an error code
+ */
+static int smack_kdbus_talk(const struct kdbus_conn *src,
+			    const struct kdbus_conn *dst)
+{
+	struct smk_audit_info ad;
+	struct smack_known *sskp = src->security;
+	struct smack_known *dskp = dst->security;
+	int ret;
+
+	BUG_ON(sskp == NULL);
+	BUG_ON(dskp == NULL);
+
+	if (smack_privileged(CAP_MAC_OVERRIDE))
+		return 0;
+
+	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_NONE);
+
+	ret = smk_access(sskp, dskp, MAY_WRITE, &ad);
+	if (ret)
+		return ret;
+	return 0;
+}
+
+/**
  * smack_unix_stream_connect - Smack access on UDS
  * @sock: one sock
  * @other: the other sock
@@ -4393,6 +4457,10 @@ struct security_hook_list smack_hooks[] = {
 	LSM_HOOK_INIT(inode_notifysecctx, smack_inode_notifysecctx),
 	LSM_HOOK_INIT(inode_setsecctx, smack_inode_setsecctx),
 	LSM_HOOK_INIT(inode_getsecctx, smack_inode_getsecctx),
+
+	LSM_HOOK_INIT(kdbus_connect, smack_kdbus_connect),
+	LSM_HOOK_INIT(kdbus_conn_free, smack_kdbus_conn_free),
+	LSM_HOOK_INIT(kdbus_talk, smack_kdbus_talk),
 };
 
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 5/8] kdbus: use LSM hooks in kdbus code
  2015-07-08 10:25 [RFC 0/8] Introduce LSM to KDBUS Paul Osmialowski
                   ` (3 preceding siblings ...)
  2015-07-08 10:25 ` [RFC 4/8] lsm: smack: smack callbacks for " Paul Osmialowski
@ 2015-07-08 10:25 ` Paul Osmialowski
  2015-07-08 11:06   ` Lukasz Pawelczyk
                     ` (4 more replies)
  2015-07-08 10:25 ` [RFC 6/8] kdbus: TEST_CREATE_CONN now does no depend on TEST_CREATE_BUS Paul Osmialowski
                   ` (3 subsequent siblings)
  8 siblings, 5 replies; 30+ messages in thread
From: Paul Osmialowski @ 2015-07-08 10:25 UTC (permalink / raw)
  To: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Paul Osmialowski, Lukasz Skalski

Originates from:

https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)
commit: aa0885489d19be92fa41c6f0a71df28763228a40

Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
---
 ipc/kdbus/bus.c        | 12 ++++++++++-
 ipc/kdbus/bus.h        |  3 +++
 ipc/kdbus/connection.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
 ipc/kdbus/connection.h |  4 ++++
 ipc/kdbus/domain.c     |  9 ++++++++-
 ipc/kdbus/domain.h     |  2 ++
 ipc/kdbus/endpoint.c   | 11 ++++++++++
 ipc/kdbus/names.c      | 11 ++++++++++
 ipc/kdbus/queue.c      | 30 ++++++++++++++++++----------
 9 files changed, 124 insertions(+), 12 deletions(-)

diff --git a/ipc/kdbus/bus.c b/ipc/kdbus/bus.c
index bbdf0f2..9894895 100644
--- a/ipc/kdbus/bus.c
+++ b/ipc/kdbus/bus.c
@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/uio.h>
+#include <linux/security.h>
 
 #include "bus.h"
 #include "notify.h"
@@ -51,6 +52,7 @@ static void kdbus_bus_free(struct kdbus_node *node)
 	kdbus_domain_unref(bus->domain);
 	kdbus_policy_db_clear(&bus->policy_db);
 	kdbus_meta_proc_unref(bus->creator_meta);
+	security_kdbus_bus_free(bus);
 	kfree(bus);
 }
 
@@ -161,6 +163,12 @@ static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain *domain,
 		goto exit_unref;
 	}
 
+	ret = security_kdbus_bus_alloc(b);
+	if (ret) {
+		ret = -EPERM;
+		goto exit_unref;
+	}
+
 	/*
 	 * Bus-limits of the creator are accounted on its real UID, just like
 	 * all other per-user limits.
@@ -169,11 +177,13 @@ static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain *domain,
 	if (IS_ERR(b->creator)) {
 		ret = PTR_ERR(b->creator);
 		b->creator = NULL;
-		goto exit_unref;
+		goto exit_free_security;
 	}
 
 	return b;
 
+exit_free_security:
+	security_kdbus_bus_free(b);
 exit_unref:
 	kdbus_node_deactivate(&b->node);
 	kdbus_node_unref(&b->node);
diff --git a/ipc/kdbus/bus.h b/ipc/kdbus/bus.h
index 5bea5ef..03e4a54 100644
--- a/ipc/kdbus/bus.h
+++ b/ipc/kdbus/bus.h
@@ -53,6 +53,7 @@ struct kdbus_user;
  * @notify_list:	List of pending kernel-generated messages
  * @notify_lock:	Notification list lock
  * @notify_flush_lock:	Notification flushing lock
+ * @security:		LSM security blob
  */
 struct kdbus_bus {
 	struct kdbus_node node;
@@ -81,6 +82,8 @@ struct kdbus_bus {
 	struct list_head notify_list;
 	spinlock_t notify_lock;
 	struct mutex notify_flush_lock;
+
+	void *security;
 };
 
 struct kdbus_bus *kdbus_bus_ref(struct kdbus_bus *bus);
diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index 9993753..b85cdc7 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -31,6 +31,7 @@
 #include <linux/slab.h>
 #include <linux/syscalls.h>
 #include <linux/uio.h>
+#include <linux/security.h>
 
 #include "bus.h"
 #include "connection.h"
@@ -73,6 +74,8 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
 	bool is_activator;
 	bool is_monitor;
 	struct kvec kvec;
+	u32 sid, len;
+	char *label;
 	int ret;
 
 	struct {
@@ -222,6 +225,14 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
 		}
 	}
 
+	security_task_getsecid(current, &sid);
+	security_secid_to_secctx(sid, &label, &len);
+	ret = security_kdbus_connect(conn, label, len);
+	if (ret) {
+		ret = -EPERM;
+		goto exit_unref;
+	}
+
 	if (atomic_inc_return(&conn->user->connections) > KDBUS_USER_MAX_CONN) {
 		/* decremented by destructor as conn->user is valid */
 		ret = -EMFILE;
@@ -276,6 +287,7 @@ static void __kdbus_conn_free(struct kref *kref)
 	kdbus_pool_free(conn->pool);
 	kdbus_ep_unref(conn->ep);
 	put_cred(conn->cred);
+	security_kdbus_conn_free(conn);
 	kfree(conn->description);
 	kfree(conn->quota);
 	kfree(conn);
@@ -1107,6 +1119,12 @@ static int kdbus_conn_reply(struct kdbus_conn *src, struct kdbus_kmsg *kmsg)
 	if (ret < 0)
 		goto exit;
 
+	ret = security_kdbus_talk(src, dst);
+	if (ret) {
+		ret = -EPERM;
+		goto exit;
+	}
+
 	mutex_lock(&dst->lock);
 	reply = kdbus_reply_find(src, dst, kmsg->msg.cookie_reply);
 	if (reply) {
@@ -1187,6 +1205,12 @@ static struct kdbus_reply *kdbus_conn_call(struct kdbus_conn *src,
 	if (ret < 0)
 		goto exit;
 
+	ret = security_kdbus_talk(src, dst);
+	if (ret) {
+		ret = -EPERM;
+		goto exit;
+	}
+
 	if (!kdbus_conn_policy_talk(src, current_cred(), dst)) {
 		ret = -EPERM;
 		goto exit;
@@ -1248,6 +1272,12 @@ static int kdbus_conn_unicast(struct kdbus_conn *src, struct kdbus_kmsg *kmsg)
 	if (ret < 0)
 		goto exit;
 
+	ret = security_kdbus_talk(src, dst);
+	if (ret) {
+		ret = -EPERM;
+		goto exit;
+	}
+
 	if (is_signal) {
 		/* like broadcasts we eavesdrop even if the msg is dropped */
 		kdbus_bus_eavesdrop(bus, src, kmsg);
@@ -1639,6 +1669,12 @@ struct kdbus_conn *kdbus_cmd_hello(struct kdbus_ep *ep, bool privileged,
 		if (ret < 0)
 			goto exit;
 
+		ret = security_kdbus_ep_setpolicy(c->ep->bus);
+		if (ret) {
+			ret = -EPERM;
+			goto exit;
+		}
+
 		ret = kdbus_policy_set(&c->ep->bus->policy_db, args.items,
 				       args.items_size, 1,
 				       kdbus_conn_is_policy_holder(c), c);
@@ -1732,6 +1768,10 @@ int kdbus_cmd_conn_info(struct kdbus_conn *conn, void __user *argp)
 	if (ret != 0)
 		return ret;
 
+	ret = security_kdbus_conn_info(conn);
+	if (ret)
+		return -EPERM;
+
 	/* registry must be held throughout lookup *and* collecting data */
 	down_read(&bus->name_registry->rwlock);
 
@@ -1905,6 +1945,12 @@ int kdbus_cmd_update(struct kdbus_conn *conn, void __user *argp)
 	/* now that we verified the input, update the connection */
 
 	if (item_policy) {
+		ret = security_kdbus_ep_setpolicy(conn->ep->bus);
+		if (ret) {
+			ret = -EPERM;
+			goto exit;
+		}
+
 		ret = kdbus_policy_set(&conn->ep->bus->policy_db, cmd->items,
 				       KDBUS_ITEMS_SIZE(cmd, items),
 				       1, true, conn);
@@ -1948,6 +1994,10 @@ int kdbus_cmd_send(struct kdbus_conn *conn, struct file *f, void __user *argp)
 		.argc = ARRAY_SIZE(argv),
 	};
 
+	ret = security_kdbus_send(conn, conn->ep->bus);
+	if (ret)
+		return -EPERM;
+
 	if (!kdbus_conn_is_ordinary(conn))
 		return -EOPNOTSUPP;
 
@@ -2044,6 +2094,10 @@ int kdbus_cmd_recv(struct kdbus_conn *conn, void __user *argp)
 		.argc = ARRAY_SIZE(argv),
 	};
 
+	ret = security_kdbus_recv(conn, conn->ep->bus);
+	if (ret)
+		return -EPERM;
+
 	if (!kdbus_conn_is_ordinary(conn) &&
 	    !kdbus_conn_is_monitor(conn) &&
 	    !kdbus_conn_is_activator(conn))
diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
index d1ffe90..1f91d39 100644
--- a/ipc/kdbus/connection.h
+++ b/ipc/kdbus/connection.h
@@ -19,6 +19,7 @@
 #include <linux/kref.h>
 #include <linux/lockdep.h>
 #include <linux/path.h>
+#include <uapi/linux/kdbus.h>
 
 #include "limits.h"
 #include "metadata.h"
@@ -73,6 +74,7 @@ struct kdbus_kmsg;
  * @names_queue_list:	Well-known names this connection waits for
  * @privileged:		Whether this connection is privileged on the bus
  * @faked_meta:		Whether the metadata was faked on HELLO
+ * @security:		LSM security blob
  */
 struct kdbus_conn {
 	struct kref kref;
@@ -113,6 +115,8 @@ struct kdbus_conn {
 
 	bool privileged:1;
 	bool faked_meta:1;
+
+	void *security;
 };
 
 struct kdbus_conn *kdbus_conn_ref(struct kdbus_conn *conn);
diff --git a/ipc/kdbus/domain.c b/ipc/kdbus/domain.c
index ac9f760..da9cdab 100644
--- a/ipc/kdbus/domain.c
+++ b/ipc/kdbus/domain.c
@@ -20,6 +20,7 @@
 #include <linux/sizes.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
+#include <linux/security.h>
 
 #include "bus.h"
 #include "domain.h"
@@ -73,6 +74,7 @@ static void kdbus_domain_free(struct kdbus_node *node)
 	put_user_ns(domain->user_namespace);
 	ida_destroy(&domain->user_ida);
 	idr_destroy(&domain->user_idr);
+	security_kdbus_domain_free(domain);
 	kfree(domain);
 }
 
@@ -104,6 +106,10 @@ struct kdbus_domain *kdbus_domain_new(unsigned int access)
 	idr_init(&d->user_idr);
 	ida_init(&d->user_ida);
 
+	ret = security_kdbus_domain_alloc(d);
+	if (ret)
+		return ERR_PTR(-EPERM);
+
 	/* Pin user namespace so we can guarantee domain-unique bus * names. */
 	d->user_namespace = get_user_ns(current_user_ns());
 
@@ -116,6 +122,7 @@ struct kdbus_domain *kdbus_domain_new(unsigned int access)
 exit_unref:
 	kdbus_node_deactivate(&d->node);
 	kdbus_node_unref(&d->node);
+	security_kdbus_domain_free(d);
 	return ERR_PTR(ret);
 }
 
@@ -264,7 +271,7 @@ static void __kdbus_user_free(struct kref *kref)
 	if (uid_valid(user->uid))
 		idr_remove(&user->domain->user_idr, __kuid_val(user->uid));
 	mutex_unlock(&user->domain->lock);
-
+	security_kdbus_domain_free(user->domain);
 	kdbus_domain_unref(user->domain);
 	kfree(user);
 }
diff --git a/ipc/kdbus/domain.h b/ipc/kdbus/domain.h
index 447a2bd..3db06d8 100644
--- a/ipc/kdbus/domain.h
+++ b/ipc/kdbus/domain.h
@@ -31,6 +31,7 @@
  * @user_ida:		Set of all users to compute small indices
  * @user_namespace:	User namespace, pinned at creation time
  * @dentry:		Root dentry of VFS mount (don't use outside of kdbusfs)
+ * @security:		LSM security blob
  */
 struct kdbus_domain {
 	struct kdbus_node node;
@@ -40,6 +41,7 @@ struct kdbus_domain {
 	struct ida user_ida;
 	struct user_namespace *user_namespace;
 	struct dentry *dentry;
+	void *security;
 };
 
 /**
diff --git a/ipc/kdbus/endpoint.c b/ipc/kdbus/endpoint.c
index 9a95a5e..380228f 100644
--- a/ipc/kdbus/endpoint.c
+++ b/ipc/kdbus/endpoint.c
@@ -21,6 +21,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/uio.h>
+#include <linux/security.h>
 
 #include "bus.h"
 #include "connection.h"
@@ -122,6 +123,12 @@ struct kdbus_ep *kdbus_ep_new(struct kdbus_bus *bus, const char *name,
 	kdbus_policy_db_init(&e->policy_db);
 	e->bus = kdbus_bus_ref(bus);
 
+	ret = security_kdbus_ep_create(bus);
+	if (ret) {
+		ret = -EPERM;
+		goto exit_unref;
+	}
+
 	ret = kdbus_node_link(&e->node, &bus->node, name);
 	if (ret < 0)
 		goto exit_unref;
@@ -265,6 +272,10 @@ int kdbus_cmd_ep_update(struct kdbus_ep *ep, void __user *argp)
 		.argc = ARRAY_SIZE(argv),
 	};
 
+	ret = security_kdbus_ep_setpolicy(ep->bus);
+	if (ret)
+		return -EPERM;
+
 	ret = kdbus_args_parse(&args, argp, &cmd);
 	if (ret != 0)
 		return ret;
diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
index d77ee08..dd20bea 100644
--- a/ipc/kdbus/names.c
+++ b/ipc/kdbus/names.c
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/uio.h>
+#include <linux/security.h>
 
 #include "bus.h"
 #include "connection.h"
@@ -503,6 +504,12 @@ int kdbus_cmd_name_acquire(struct kdbus_conn *conn, void __user *argp)
 		goto exit;
 	}
 
+	ret = security_kdbus_name_acquire(conn, item_name);
+	if (ret) {
+		ret = -EPERM;
+		goto exit;
+	}
+
 	/*
 	 * Do atomic_inc_return here to reserve our slot, then decrement
 	 * it before returning.
@@ -724,6 +731,10 @@ int kdbus_cmd_list(struct kdbus_conn *conn, void __user *argp)
 	if (ret != 0)
 		return ret;
 
+	ret = security_kdbus_name_list(conn->ep->bus);
+	if (ret)
+		return -EPERM;
+
 	/* lock order: domain -> bus -> ep -> names -> conn */
 	down_read(&reg->rwlock);
 	down_read(&conn->ep->bus->conn_rwlock);
diff --git a/ipc/kdbus/queue.c b/ipc/kdbus/queue.c
index 25bb3ad..9872fb4 100644
--- a/ipc/kdbus/queue.c
+++ b/ipc/kdbus/queue.c
@@ -28,6 +28,7 @@
 #include <linux/slab.h>
 #include <linux/syscalls.h>
 #include <linux/uio.h>
+#include <linux/security.h>
 
 #include "util.h"
 #include "domain.h"
@@ -514,12 +515,17 @@ int kdbus_queue_entry_install(struct kdbus_queue_entry *entry,
 
 		for (i = 0; i < res->fds_count; i++) {
 			if (install_fds) {
-				fds[i] = get_unused_fd_flags(O_CLOEXEC);
-				if (fds[i] >= 0)
-					fd_install(fds[i],
-						   get_file(res->fds[i]));
-				else
+				if (security_file_receive(res->fds[i])) {
+					fds[i] = -1;
 					incomplete_fds = true;
+				} else {
+					fds[i] = get_unused_fd_flags(O_CLOEXEC);
+					if (fds[i] >= 0)
+						fd_install(fds[i],
+							get_file(res->fds[i]));
+					else
+						incomplete_fds = true;
+				}
 			} else {
 				fds[i] = -1;
 			}
@@ -557,13 +563,17 @@ int kdbus_queue_entry_install(struct kdbus_queue_entry *entry,
 		m.fd = -1;
 
 		if (install_fds) {
-			m.fd = get_unused_fd_flags(O_CLOEXEC);
-			if (m.fd < 0) {
-				m.fd = -1;
+			if (security_file_receive(d->memfd.file)) {
 				incomplete_fds = true;
 			} else {
-				fd_install(m.fd,
-					   get_file(d->memfd.file));
+				m.fd = get_unused_fd_flags(O_CLOEXEC);
+				if (m.fd < 0) {
+					m.fd = -1;
+					incomplete_fds = true;
+				} else {
+					fd_install(m.fd,
+						get_file(d->memfd.file));
+				}
 			}
 		}
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 6/8] kdbus: TEST_CREATE_CONN now does no depend on TEST_CREATE_BUS
  2015-07-08 10:25 [RFC 0/8] Introduce LSM to KDBUS Paul Osmialowski
                   ` (4 preceding siblings ...)
  2015-07-08 10:25 ` [RFC 5/8] kdbus: use LSM hooks in kdbus code Paul Osmialowski
@ 2015-07-08 10:25 ` Paul Osmialowski
  2015-07-08 10:25 ` [RFC 7/8] kdbus: selftests extended Paul Osmialowski
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 30+ messages in thread
From: Paul Osmialowski @ 2015-07-08 10:25 UTC (permalink / raw)
  To: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Paul Osmialowski, Lukasz Skalski

Without this patch, it is impossible to specify test case able to
connect to a bus already created (e.g. by 'test-daemon' test case), you can
only specify:

1) TEST_CREATE_BUS which creates new bus, or
2) TEST_CREATE_CONN OR'ed with TEST_CREATE_BUS which creates new bus and
creates connection to it.

This patch adds the missing ability to specify TEST_CREATE_CONN alone.

It will be used by a new test case (will be added by separate commit) which
is supposed to connect to already started test-daemon case.

Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
---
 tools/testing/selftests/kdbus/kdbus-test.c | 21 +++++++++++++++++++++
 tools/testing/selftests/kdbus/kdbus-util.c | 18 +++++++++++-------
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kdbus/kdbus-test.c b/tools/testing/selftests/kdbus/kdbus-test.c
index 294e82a..d3a656f 100644
--- a/tools/testing/selftests/kdbus/kdbus-test.c
+++ b/tools/testing/selftests/kdbus/kdbus-test.c
@@ -309,6 +309,27 @@ static int test_prepare_env(const struct kdbus_test *t,
 	}
 
 	if (t->flags & TEST_CREATE_CONN) {
+		if (!env->buspath) {
+			char *s = NULL;
+			char *n = NULL;
+			int ret;
+
+			if (!args->busname) {
+				n = unique_name("test-bus");
+				ASSERT_RETURN(n);
+			}
+
+			ret = kdbus_create_bus(-1,
+					       args->busname ?: n,
+					       0,
+					       0, &s);
+			free(n);
+			ASSERT_RETURN(ret == 0);
+
+			asprintf(&env->buspath, "%s/%s/bus", args->root, s);
+			free(s);
+		}
+		ASSERT_RETURN(env->buspath);
 		env->conn = kdbus_hello(env->buspath, 0, NULL, 0);
 		ASSERT_RETURN(env->conn);
 	}
diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
index 29a0cb1..26104ca 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.c
+++ b/tools/testing/selftests/kdbus/kdbus-util.c
@@ -141,7 +141,7 @@ int kdbus_create_bus(int control_fd, const char *name,
 			char str[64];
 		} name;
 	} bus_make;
-	int ret;
+	int ret = 0;
 
 	memset(&bus_make, 0, sizeof(bus_make));
 	bus_make.bp.size = sizeof(bus_make.bp);
@@ -171,13 +171,17 @@ int kdbus_create_bus(int control_fd, const char *name,
 			     bus_make.attach[1].size +
 			     bus_make.name.size;
 
-	kdbus_printf("Creating bus with name >%s< on control fd %d ...\n",
-		     name, control_fd);
+	if (control_fd != -1) {
+		kdbus_printf(
+			"Creating bus with name >%s< on control fd %d ...\n",
+			name, control_fd);
 
-	ret = kdbus_cmd_bus_make(control_fd, &bus_make.cmd);
-	if (ret < 0) {
-		kdbus_printf("--- error when making bus: %d (%m)\n", ret);
-		return ret;
+		ret = kdbus_cmd_bus_make(control_fd, &bus_make.cmd);
+		if (ret < 0) {
+			kdbus_printf("--- error when making bus: %d (%m)\n",
+				     ret);
+			return ret;
+		}
 	}
 
 	if (ret == 0 && path)
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 7/8] kdbus: selftests extended
  2015-07-08 10:25 [RFC 0/8] Introduce LSM to KDBUS Paul Osmialowski
                   ` (5 preceding siblings ...)
  2015-07-08 10:25 ` [RFC 6/8] kdbus: TEST_CREATE_CONN now does no depend on TEST_CREATE_BUS Paul Osmialowski
@ 2015-07-08 10:25 ` Paul Osmialowski
  2015-07-08 10:25 ` [RFC 8/8] kdbus: Ability to run kdbus test by executable binary name Paul Osmialowski
  2015-07-08 16:46 ` [RFC 0/8] Introduce LSM to KDBUS Casey Schaufler
  8 siblings, 0 replies; 30+ messages in thread
From: Paul Osmialowski @ 2015-07-08 10:25 UTC (permalink / raw)
  To: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Paul Osmialowski, Lukasz Skalski

The 'test-send' test case should connect to an already running test-daemon
which creates a bus with known name.

The main goal of this test case is to verify that messages as well as
file descriptors (opened with different open modes) can be transferred
properly.

In order to achieve its goals, this test case opens three files
(/tmp/kdbus-test-send.rd, /tmp/kdbus-test-send.wr,
/tmp/kdbus-test-send.rdwr) with three different open modes (O_RDONLY,
O_WRONLY, O_RDWR). If any of these files exists it is used 'as is',
otherwise it is created with some default content. Then this test tries
to send simple message followed by three messages each one of them
containing an array of opened file descriptors (single element array in the
first message, two element array in the second, three element array in the
third).

The ability to send array of file descriptors required changes in almost
all of the test case files.

Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
---
 tools/testing/selftests/kdbus/Makefile           |  1 +
 tools/testing/selftests/kdbus/kdbus-test.c       | 11 +++-
 tools/testing/selftests/kdbus/kdbus-test.h       |  1 +
 tools/testing/selftests/kdbus/kdbus-util.c       | 19 ++++--
 tools/testing/selftests/kdbus/kdbus-util.h       |  2 +-
 tools/testing/selftests/kdbus/test-activator.c   | 20 +++---
 tools/testing/selftests/kdbus/test-chat.c        |  6 +-
 tools/testing/selftests/kdbus/test-connection.c  |  8 ++-
 tools/testing/selftests/kdbus/test-fd.c          |  2 +-
 tools/testing/selftests/kdbus/test-message.c     | 69 +++++++++++--------
 tools/testing/selftests/kdbus/test-metadata-ns.c | 10 +--
 tools/testing/selftests/kdbus/test-monitor.c     |  9 +--
 tools/testing/selftests/kdbus/test-policy-ns.c   |  8 +--
 tools/testing/selftests/kdbus/test-policy-priv.c | 48 ++++++++------
 tools/testing/selftests/kdbus/test-send.c        | 84 ++++++++++++++++++++++++
 tools/testing/selftests/kdbus/test-sync.c        |  2 +-
 tools/testing/selftests/kdbus/test-timeout.c     |  2 +-
 17 files changed, 217 insertions(+), 85 deletions(-)
 create mode 100644 tools/testing/selftests/kdbus/test-send.c

diff --git a/tools/testing/selftests/kdbus/Makefile b/tools/testing/selftests/kdbus/Makefile
index 8f36cb5..f400756 100644
--- a/tools/testing/selftests/kdbus/Makefile
+++ b/tools/testing/selftests/kdbus/Makefile
@@ -27,6 +27,7 @@ OBJS= \
 	test-policy.o		\
 	test-policy-ns.o	\
 	test-policy-priv.o	\
+	test-send.o		\
 	test-sync.o		\
 	test-timeout.o
 
diff --git a/tools/testing/selftests/kdbus/kdbus-test.c b/tools/testing/selftests/kdbus/kdbus-test.c
index d3a656f..5a296da 100644
--- a/tools/testing/selftests/kdbus/kdbus-test.c
+++ b/tools/testing/selftests/kdbus/kdbus-test.c
@@ -25,6 +25,7 @@
 enum {
 	TEST_CREATE_BUS		= 1 << 0,
 	TEST_CREATE_CONN	= 1 << 1,
+	TEST_CREATE_CAN_FAIL	= 1 << 2,
 };
 
 struct kdbus_test {
@@ -148,6 +149,12 @@ static const struct kdbus_test tests[] = {
 		.flags	= TEST_CREATE_BUS,
 	},
 	{
+		.name	= "send",
+		.desc	= "send",
+		.func	= kdbus_test_send,
+		.flags	= TEST_CREATE_CONN | TEST_CREATE_CAN_FAIL,
+	},
+	{
 		.name	= "sync-byebye",
 		.desc	= "synchronous replies vs. BYEBYE",
 		.func	= kdbus_test_sync_byebye,
@@ -302,7 +309,7 @@ static int test_prepare_env(const struct kdbus_test *t,
 				       _KDBUS_ATTACH_ALL,
 				       _KDBUS_ATTACH_ALL, &s);
 		free(n);
-		ASSERT_RETURN(ret == 0);
+		ASSERT_RETURN((ret == 0) || (t->flags & TEST_CREATE_CAN_FAIL));
 
 		asprintf(&env->buspath, "%s/%s/bus", args->root, s);
 		free(s);
@@ -331,7 +338,7 @@ static int test_prepare_env(const struct kdbus_test *t,
 		}
 		ASSERT_RETURN(env->buspath);
 		env->conn = kdbus_hello(env->buspath, 0, NULL, 0);
-		ASSERT_RETURN(env->conn);
+		ASSERT_RETURN(env->conn || (t->flags & TEST_CREATE_CAN_FAIL));
 	}
 
 	env->root = args->root;
diff --git a/tools/testing/selftests/kdbus/kdbus-test.h b/tools/testing/selftests/kdbus/kdbus-test.h
index a5c6ae8..604c25c 100644
--- a/tools/testing/selftests/kdbus/kdbus-test.h
+++ b/tools/testing/selftests/kdbus/kdbus-test.h
@@ -75,6 +75,7 @@ int kdbus_test_name_queue(struct kdbus_test_env *env);
 int kdbus_test_policy(struct kdbus_test_env *env);
 int kdbus_test_policy_ns(struct kdbus_test_env *env);
 int kdbus_test_policy_priv(struct kdbus_test_env *env);
+int kdbus_test_send(struct kdbus_test_env *env);
 int kdbus_test_sync_byebye(struct kdbus_test_env *env);
 int kdbus_test_sync_reply(struct kdbus_test_env *env);
 int kdbus_test_timeout(struct kdbus_test_env *env);
diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
index 26104ca..622ff5a 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.c
+++ b/tools/testing/selftests/kdbus/kdbus-util.c
@@ -454,7 +454,9 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
 			    int64_t priority,
 			    uint64_t dst_id,
 			    uint64_t cmd_flags,
-			    int cancel_fd)
+			    int cancel_fd,
+			    int fds_count,
+			    int fds[])
 {
 	struct kdbus_cmd_send *cmd = NULL;
 	struct kdbus_msg *msg = NULL;
@@ -467,6 +469,7 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
 	int ret;
 
 	size = sizeof(*msg) + 3 * KDBUS_ITEM_SIZE(sizeof(struct kdbus_vec));
+	size += fds_count > 0 ? KDBUS_ITEM_SIZE(sizeof(int) * fds_count) : 0;
 
 	if (dst_id == KDBUS_DST_ID_BROADCAST)
 		size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_bloom_filter)) + 64;
@@ -564,6 +567,13 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
 	}
 	item = KDBUS_ITEM_NEXT(item);
 
+	if (fds_count > 0) {
+		item->type = KDBUS_ITEM_FDS;
+		item->size = KDBUS_ITEM_HEADER_SIZE + sizeof(int) * fds_count;
+		memcpy(&item->fds, fds, sizeof(int) * fds_count);
+		item = KDBUS_ITEM_NEXT(item);
+	}
+
 	size = sizeof(*cmd);
 	if (cancel_fd != -1)
 		size += KDBUS_ITEM_SIZE(sizeof(cancel_fd));
@@ -620,10 +630,10 @@ out:
 
 int kdbus_msg_send(const struct kdbus_conn *conn, const char *name,
 		   uint64_t cookie, uint64_t flags, uint64_t timeout,
-		   int64_t priority, uint64_t dst_id)
+		   int64_t priority, uint64_t dst_id, int fds_count, int fds[])
 {
 	return __kdbus_msg_send(conn, name, cookie, flags, timeout, priority,
-				dst_id, 0, -1);
+				dst_id, 0, -1, fds_count, fds);
 }
 
 int kdbus_msg_send_sync(const struct kdbus_conn *conn, const char *name,
@@ -631,7 +641,8 @@ int kdbus_msg_send_sync(const struct kdbus_conn *conn, const char *name,
 			int64_t priority, uint64_t dst_id, int cancel_fd)
 {
 	return __kdbus_msg_send(conn, name, cookie, flags, timeout, priority,
-				dst_id, KDBUS_SEND_SYNC_REPLY, cancel_fd);
+				dst_id, KDBUS_SEND_SYNC_REPLY, cancel_fd,
+				0, NULL);
 }
 
 int kdbus_msg_send_reply(const struct kdbus_conn *conn,
diff --git a/tools/testing/selftests/kdbus/kdbus-util.h b/tools/testing/selftests/kdbus/kdbus-util.h
index d1a0f1b..7bb28e6 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.h
+++ b/tools/testing/selftests/kdbus/kdbus-util.h
@@ -172,7 +172,7 @@ int kdbus_create_bus(int control_fd, const char *name,
 		     char **path);
 int kdbus_msg_send(const struct kdbus_conn *conn, const char *name,
 		   uint64_t cookie, uint64_t flags, uint64_t timeout,
-		   int64_t priority, uint64_t dst_id);
+		   int64_t priority, uint64_t dst_id, int fds_count, int fds[]);
 int kdbus_msg_send_sync(const struct kdbus_conn *conn, const char *name,
 			uint64_t cookie, uint64_t flags, uint64_t timeout,
 			int64_t priority, uint64_t dst_id, int cancel_fd);
diff --git a/tools/testing/selftests/kdbus/test-activator.c b/tools/testing/selftests/kdbus/test-activator.c
index 3d1b763..c70fafe 100644
--- a/tools/testing/selftests/kdbus/test-activator.c
+++ b/tools/testing/selftests/kdbus/test-activator.c
@@ -84,13 +84,13 @@ static int kdbus_priv_activator(struct kdbus_test_env *env)
 	ret = RUN_UNPRIVILEGED_CONN(unpriv, env->buspath, ({
 		/* Try to talk using the ID */
 		ret = kdbus_msg_send(unpriv, NULL, 0xdeadbeef, 0, 0,
-				     0, activator->id);
+				     0, activator->id, 0, NULL);
 		ASSERT_EXIT(ret == -ENXIO);
 
 		/* Try to talk to the name */
 		ret = kdbus_msg_send(unpriv, "foo.priv.activator",
 				     0xdeadbeef, 0, 0, 0,
-				     KDBUS_DST_ID_NAME);
+				     KDBUS_DST_ID_NAME, 0, NULL);
 		ASSERT_EXIT(ret == -EPERM);
 	}));
 	ASSERT_RETURN(ret >= 0);
@@ -110,7 +110,7 @@ static int kdbus_priv_activator(struct kdbus_test_env *env)
 
 	cookie++;
 	ret = kdbus_msg_send(service, "foo.priv.activator", cookie,
-			     0, 0, 0, KDBUS_DST_ID_NAME);
+			     0, 0, 0, KDBUS_DST_ID_NAME, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	ret = kdbus_starter_poll(activator);
@@ -141,7 +141,7 @@ static int kdbus_priv_activator(struct kdbus_test_env *env)
 		/* Try to talk to the name */
 		ret = kdbus_msg_send(unpriv, "foo.priv.activator",
 				     cookie, 0, 0, 0,
-				     KDBUS_DST_ID_NAME);
+				     KDBUS_DST_ID_NAME, 0, NULL);
 		ASSERT_EXIT(ret == -EPERM);
 	}));
 	ASSERT_RETURN(ret >= 0);
@@ -155,7 +155,7 @@ static int kdbus_priv_activator(struct kdbus_test_env *env)
 
 	cookie++;
 	ret = kdbus_msg_send(client, "foo.priv.activator", cookie,
-			     0, 0, 0, KDBUS_DST_ID_NAME);
+			     0, 0, 0, KDBUS_DST_ID_NAME, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 	ret = kdbus_msg_recv_poll(service, 100, &msg, NULL);
 	ASSERT_RETURN(ret == 0 && msg->cookie == cookie);
@@ -170,7 +170,8 @@ static int kdbus_priv_activator(struct kdbus_test_env *env)
 		/* Try to talk to the name */
 		ret = kdbus_msg_send(unpriv, "foo.priv.activator",
 				     cookie, 0, 0, 0,
-				     KDBUS_DST_ID_NAME);
+				     KDBUS_DST_ID_NAME,
+				     0, NULL);
 		ASSERT_EXIT(ret == -EPERM);
 	}));
 	ASSERT_RETURN(ret >= 0);
@@ -181,7 +182,7 @@ static int kdbus_priv_activator(struct kdbus_test_env *env)
 	/* Same user is able to TALK */
 	cookie++;
 	ret = kdbus_msg_send(client, "foo.priv.activator", cookie,
-			     0, 0, 0, KDBUS_DST_ID_NAME);
+			     0, 0, 0, KDBUS_DST_ID_NAME, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 	ret = kdbus_msg_recv_poll(service, 100, &msg, NULL);
 	ASSERT_RETURN(ret == 0 && msg->cookie == cookie);
@@ -207,7 +208,8 @@ static int kdbus_priv_activator(struct kdbus_test_env *env)
 		/* Try to talk to the name */
 		ret = kdbus_msg_send(unpriv, "foo.priv.activator",
 				     cookie, 0, 0, 0,
-				     KDBUS_DST_ID_NAME);
+				     KDBUS_DST_ID_NAME,
+				     0, NULL);
 		ASSERT_EXIT(ret == 0);
 	}));
 	ASSERT_RETURN(ret >= 0);
@@ -258,7 +260,7 @@ int kdbus_test_activator(struct kdbus_test_env *env)
 	ASSERT_RETURN(ret == 0);
 
 	ret = kdbus_msg_send(env->conn, "foo.test.activator", 0xdeafbeef,
-			     0, 0, 0, KDBUS_DST_ID_NAME);
+			     0, 0, 0, KDBUS_DST_ID_NAME, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	fds[0].fd = activator->fd;
diff --git a/tools/testing/selftests/kdbus/test-chat.c b/tools/testing/selftests/kdbus/test-chat.c
index 71a92d8..e19f595 100644
--- a/tools/testing/selftests/kdbus/test-chat.c
+++ b/tools/testing/selftests/kdbus/test-chat.c
@@ -64,7 +64,7 @@ int kdbus_test_chat(struct kdbus_test_env *env)
 
 	cookie = 0;
 	ret = kdbus_msg_send(conn_b, NULL, 0xc0000000 | cookie, 0, 0, 0,
-			     KDBUS_DST_ID_BROADCAST);
+			     KDBUS_DST_ID_BROADCAST, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	fds[0].fd = conn_a->fd;
@@ -91,7 +91,7 @@ int kdbus_test_chat(struct kdbus_test_env *env)
 			ASSERT_RETURN(ret == 0);
 			ret = kdbus_msg_send(conn_a, NULL,
 					     0xc0000000 | cookie++,
-					     0, 0, 0, conn_b->id);
+					     0, 0, 0, conn_b->id, 0, NULL);
 			ASSERT_RETURN(ret == 0);
 		}
 
@@ -100,7 +100,7 @@ int kdbus_test_chat(struct kdbus_test_env *env)
 			ASSERT_RETURN(ret == 0);
 			ret = kdbus_msg_send(conn_b, NULL,
 					     0xc0000000 | cookie++,
-					     0, 0, 0, conn_a->id);
+					     0, 0, 0, conn_a->id, 0, NULL);
 			ASSERT_RETURN(ret == 0);
 		}
 
diff --git a/tools/testing/selftests/kdbus/test-connection.c b/tools/testing/selftests/kdbus/test-connection.c
index e7c4866..779166f 100644
--- a/tools/testing/selftests/kdbus/test-connection.c
+++ b/tools/testing/selftests/kdbus/test-connection.c
@@ -127,7 +127,7 @@ int kdbus_test_byebye(struct kdbus_test_env *env)
 
 	/* send over 1st connection */
 	ret = kdbus_msg_send(env->conn, NULL, 0, 0, 0, 0,
-			     KDBUS_DST_ID_BROADCAST);
+			     KDBUS_DST_ID_BROADCAST, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	/* say byebye on the 2nd, which must fail */
@@ -513,7 +513,8 @@ int kdbus_test_conn_update(struct kdbus_test_env *env)
 	conn = kdbus_hello(env->buspath, 0, NULL, 0);
 	ASSERT_RETURN(conn);
 
-	ret = kdbus_msg_send(env->conn, NULL, 0x12345678, 0, 0, 0, conn->id);
+	ret = kdbus_msg_send(env->conn, NULL, 0x12345678, 0, 0, 0, conn->id,
+			     0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	ret = kdbus_msg_recv(conn, &msg, NULL);
@@ -536,7 +537,8 @@ int kdbus_test_conn_update(struct kdbus_test_env *env)
 					     ~KDBUS_ATTACH_TIMESTAMP);
 	ASSERT_RETURN(ret == 0);
 
-	ret = kdbus_msg_send(env->conn, NULL, 0x12345678, 0, 0, 0, conn->id);
+	ret = kdbus_msg_send(env->conn, NULL, 0x12345678, 0, 0, 0, conn->id,
+			     0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	ret = kdbus_msg_recv(conn, &msg, NULL);
diff --git a/tools/testing/selftests/kdbus/test-fd.c b/tools/testing/selftests/kdbus/test-fd.c
index 2ae0f5a..bfd8980 100644
--- a/tools/testing/selftests/kdbus/test-fd.c
+++ b/tools/testing/selftests/kdbus/test-fd.c
@@ -466,7 +466,7 @@ static int kdbus_test_no_fds(struct kdbus_test_env *env,
 	 */
 	cookie++;
 	ret = kdbus_msg_send(conn_src, NULL, cookie, 0, 0, 0,
-			     conn_dst->id);
+			     conn_dst->id, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	ret = waitpid(pid, &status, 0);
diff --git a/tools/testing/selftests/kdbus/test-message.c b/tools/testing/selftests/kdbus/test-message.c
index f1615da..d9b8fbd 100644
--- a/tools/testing/selftests/kdbus/test-message.c
+++ b/tools/testing/selftests/kdbus/test-message.c
@@ -51,7 +51,7 @@ int kdbus_test_message_basic(struct kdbus_test_env *env)
 
 	/* send over 1st connection */
 	ret = kdbus_msg_send(sender, NULL, cookie, 0, 0, 0,
-			     KDBUS_DST_ID_BROADCAST);
+			     KDBUS_DST_ID_BROADCAST, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	/* Make sure that we do not get our own broadcasts */
@@ -67,7 +67,7 @@ int kdbus_test_message_basic(struct kdbus_test_env *env)
 
 	/* Msgs that expect a reply must have timeout and cookie */
 	ret = kdbus_msg_send(sender, NULL, 0, KDBUS_MSG_EXPECT_REPLY,
-			     0, 0, conn->id);
+			     0, 0, conn->id, 0, NULL);
 	ASSERT_RETURN(ret == -EINVAL);
 
 	/* Faked replies with a valid reply cookie are rejected */
@@ -123,23 +123,36 @@ int kdbus_test_message_prio(struct kdbus_test_env *env)
 {
 	struct kdbus_conn *a, *b;
 	uint64_t cookie = 0;
+	int ret;
 
 	a = kdbus_hello(env->buspath, 0, NULL, 0);
 	b = kdbus_hello(env->buspath, 0, NULL, 0);
 	ASSERT_RETURN(a && b);
 
-	ASSERT_RETURN(kdbus_msg_send(b, NULL, ++cookie, 0, 0,   25, a->id) == 0);
-	ASSERT_RETURN(kdbus_msg_send(b, NULL, ++cookie, 0, 0, -600, a->id) == 0);
-	ASSERT_RETURN(kdbus_msg_send(b, NULL, ++cookie, 0, 0,   10, a->id) == 0);
-	ASSERT_RETURN(kdbus_msg_send(b, NULL, ++cookie, 0, 0,  -35, a->id) == 0);
-	ASSERT_RETURN(kdbus_msg_send(b, NULL, ++cookie, 0, 0, -100, a->id) == 0);
-	ASSERT_RETURN(kdbus_msg_send(b, NULL, ++cookie, 0, 0,   20, a->id) == 0);
-	ASSERT_RETURN(kdbus_msg_send(b, NULL, ++cookie, 0, 0,  -15, a->id) == 0);
-	ASSERT_RETURN(kdbus_msg_send(b, NULL, ++cookie, 0, 0, -800, a->id) == 0);
-	ASSERT_RETURN(kdbus_msg_send(b, NULL, ++cookie, 0, 0, -150, a->id) == 0);
-	ASSERT_RETURN(kdbus_msg_send(b, NULL, ++cookie, 0, 0,   10, a->id) == 0);
-	ASSERT_RETURN(kdbus_msg_send(b, NULL, ++cookie, 0, 0, -800, a->id) == 0);
-	ASSERT_RETURN(kdbus_msg_send(b, NULL, ++cookie, 0, 0,  -10, a->id) == 0);
+	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0,   25, a->id, 0, NULL);
+	ASSERT_RETURN(ret == 0);
+	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0, -600, a->id, 0, NULL);
+	ASSERT_RETURN(ret == 0);
+	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0,   10, a->id, 0, NULL);
+	ASSERT_RETURN(ret == 0);
+	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0,  -35, a->id, 0, NULL);
+	ASSERT_RETURN(ret == 0);
+	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0, -100, a->id, 0, NULL);
+	ASSERT_RETURN(ret == 0);
+	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0,   20, a->id, 0, NULL);
+	ASSERT_RETURN(ret == 0);
+	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0,  -15, a->id, 0, NULL);
+	ASSERT_RETURN(ret == 0);
+	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0, -800, a->id, 0, NULL);
+	ASSERT_RETURN(ret == 0);
+	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0, -150, a->id, 0, NULL);
+	ASSERT_RETURN(ret == 0);
+	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0,   10, a->id, 0, NULL);
+	ASSERT_RETURN(ret == 0);
+	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0, -800, a->id, 0, NULL);
+	ASSERT_RETURN(ret == 0);
+	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0,  -10, a->id, 0, NULL);
+	ASSERT_RETURN(ret == 0);
 
 	ASSERT_RETURN(msg_recv_prio(a, -200, -800) == 0);
 	ASSERT_RETURN(msg_recv_prio(a, -100, -800) == 0);
@@ -195,7 +208,8 @@ static int kdbus_test_notify_kernel_quota(struct kdbus_test_env *env)
 	 * Now the reader queue is full with kernel notfications,
 	 * but as a user we still have room to push our messages.
 	 */
-	ret = kdbus_msg_send(conn, NULL, 0xdeadbeef, 0, 0, 0, reader->id);
+	ret = kdbus_msg_send(conn, NULL, 0xdeadbeef, 0, 0, 0, reader->id,
+			     0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	/* More ID kernel notifications that will be lost */
@@ -315,7 +329,8 @@ static int kdbus_test_activator_quota(struct kdbus_test_env *env)
 	for (i = 0; i < KDBUS_CONN_MAX_MSGS; i++) {
 		ret = kdbus_msg_send(sender, "foo.test.activator",
 				     cookie++, 0, 0, 0,
-				     KDBUS_DST_ID_NAME);
+				     KDBUS_DST_ID_NAME,
+				     0, NULL);
 		if (ret < 0)
 			break;
 		activator_msgs_count++;
@@ -328,17 +343,18 @@ static int kdbus_test_activator_quota(struct kdbus_test_env *env)
 	/* Good, activator queue is full now */
 
 	/* ENXIO on direct send (activators can never be addressed by ID) */
-	ret = kdbus_msg_send(conn, NULL, cookie++, 0, 0, 0, activator->id);
+	ret = kdbus_msg_send(conn, NULL, cookie++, 0, 0, 0, activator->id,
+			     0, NULL);
 	ASSERT_RETURN(ret == -ENXIO);
 
 	/* can't queue more */
 	ret = kdbus_msg_send(conn, "foo.test.activator", cookie++,
-			     0, 0, 0, KDBUS_DST_ID_NAME);
+			     0, 0, 0, KDBUS_DST_ID_NAME, 0, NULL);
 	ASSERT_RETURN(ret == -ENOBUFS);
 
 	/* no match installed, so the broadcast will not inc dropped_msgs */
 	ret = kdbus_msg_send(sender, NULL, cookie++, 0, 0, 0,
-			     KDBUS_DST_ID_BROADCAST);
+			     KDBUS_DST_ID_BROADCAST, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	/* Check activator queue */
@@ -357,7 +373,7 @@ static int kdbus_test_activator_quota(struct kdbus_test_env *env)
 	/* Consume the connection pool memory */
 	for (i = 0; i < KDBUS_CONN_MAX_MSGS; i++) {
 		ret = kdbus_msg_send(sender, NULL,
-				     cookie++, 0, 0, 0, conn->id);
+				     cookie++, 0, 0, 0, conn->id, 0, NULL);
 		if (ret < 0)
 			break;
 	}
@@ -424,7 +440,7 @@ static int kdbus_test_activator_quota(struct kdbus_test_env *env)
 
 	/* This one is lost but it is not accounted */
 	ret = kdbus_msg_send(sender, NULL,
-			     cookie++, 0, 0, 0, conn->id);
+			     cookie++, 0, 0, 0, conn->id, 0, NULL);
 	ASSERT_RETURN(ret == -ENOBUFS);
 
 	/* Acquire the name again */
@@ -477,7 +493,8 @@ static int kdbus_test_expected_reply_quota(struct kdbus_test_env *env)
 			ret = kdbus_msg_send(conn, NULL, cookie++,
 					     KDBUS_MSG_EXPECT_REPLY,
 					     100000000ULL, 0,
-					     connections[i]->id);
+					     connections[i]->id,
+					     0, NULL);
 			if (ret < 0)
 				break;
 
@@ -497,7 +514,7 @@ static int kdbus_test_expected_reply_quota(struct kdbus_test_env *env)
 	 * no further requests are allowed
 	 */
 	ret = kdbus_msg_send(conn, NULL, cookie++, KDBUS_MSG_EXPECT_REPLY,
-			     1000000000ULL, 0, connections[8]->id);
+			     1000000000ULL, 0, connections[8]->id, 0, NULL);
 	ASSERT_RETURN(ret == -EMLINK);
 
 	for (i = 0; i < 9; i++)
@@ -575,7 +592,7 @@ int kdbus_test_pool_quota(struct kdbus_test_env *env)
 	ASSERT_RETURN(ret == -ENOBUFS);
 
 	/* We still can pass small messages */
-	ret = kdbus_msg_send(b, NULL, cookie++, 0, 0, 0, c->id);
+	ret = kdbus_msg_send(b, NULL, cookie++, 0, 0, 0, c->id, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	for (i = size; i < (POOL_SIZE / 2 / 3); i += size) {
@@ -630,7 +647,7 @@ int kdbus_test_message_quota(struct kdbus_test_env *env)
 	ret = kdbus_fill_conn_queue(b, a->id, KDBUS_CONN_MAX_MSGS);
 	ASSERT_RETURN(ret == KDBUS_CONN_MAX_MSGS);
 
-	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0, 0, a->id);
+	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0, 0, a->id, 0, NULL);
 	ASSERT_RETURN(ret == -ENOBUFS);
 
 	for (i = 0; i < KDBUS_CONN_MAX_MSGS; ++i) {
@@ -644,7 +661,7 @@ int kdbus_test_message_quota(struct kdbus_test_env *env)
 	ret = kdbus_fill_conn_queue(b, a->id, KDBUS_CONN_MAX_MSGS + 1);
 	ASSERT_RETURN(ret == KDBUS_CONN_MAX_MSGS);
 
-	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0, 0, a->id);
+	ret = kdbus_msg_send(b, NULL, ++cookie, 0, 0, 0, a->id, 0, NULL);
 	ASSERT_RETURN(ret == -ENOBUFS);
 
 	kdbus_conn_free(a);
diff --git a/tools/testing/selftests/kdbus/test-metadata-ns.c b/tools/testing/selftests/kdbus/test-metadata-ns.c
index ccdfae0..4f264ac 100644
--- a/tools/testing/selftests/kdbus/test-metadata-ns.c
+++ b/tools/testing/selftests/kdbus/test-metadata-ns.c
@@ -115,7 +115,7 @@ static int __kdbus_clone_userns_test(const char *bus,
 	 */
 
 	ret = kdbus_msg_send(unpriv_conn, NULL, cookie, 0, 0,
-			     0, conn->id);
+			     0, conn->id, 0, NULL);
 	ASSERT_EXIT(ret == 0);
 
 	/*
@@ -151,7 +151,7 @@ static int __kdbus_clone_userns_test(const char *bus,
 
 		cookie++;
 		ret = kdbus_msg_send(userns_conn, NULL, cookie,
-				     0, 0, 0, conn->id);
+				     0, 0, 0, conn->id, 0, NULL);
 		ASSERT_EXIT(ret == 0);
 
 		/* Parent did send */
@@ -394,7 +394,7 @@ static int kdbus_clone_userns_test(const char *bus,
 	 * Sending to unprivileged connections a unicast
 	 */
 	ret = kdbus_msg_send(conn, NULL, 0xdeadbeef, 0, 0,
-			     0, unpriv_conn_id);
+			     0, unpriv_conn_id, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	/* signal to child that is in its userns */
@@ -406,14 +406,14 @@ static int kdbus_clone_userns_test(const char *bus,
 	 * connections a unicast
 	 */
 	ret = kdbus_msg_send(conn, NULL, 0xdeadbeef, 0, 0,
-			     0, userns_conn_id);
+			     0, userns_conn_id, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	/*
 	 * Sending to unprivileged connections a broadcast
 	 */
 	ret = kdbus_msg_send(conn, NULL, 0xdeadbeef, 0, 0,
-			     0, KDBUS_DST_ID_BROADCAST);
+			     0, KDBUS_DST_ID_BROADCAST, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 
diff --git a/tools/testing/selftests/kdbus/test-monitor.c b/tools/testing/selftests/kdbus/test-monitor.c
index e00d738..ae87d87 100644
--- a/tools/testing/selftests/kdbus/test-monitor.c
+++ b/tools/testing/selftests/kdbus/test-monitor.c
@@ -56,7 +56,8 @@ int kdbus_test_monitor(struct kdbus_test_env *env)
 	ret = kdbus_name_acquire(monitor, "foo.bar.baz", NULL);
 	ASSERT_RETURN(ret == -EOPNOTSUPP);
 
-	ret = kdbus_msg_send(env->conn, NULL, cookie, 0, 0,  0, conn->id);
+	ret = kdbus_msg_send(env->conn, NULL, cookie, 0, 0,  0, conn->id,
+			     0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	/* the recipient should have gotten the message */
@@ -80,7 +81,7 @@ int kdbus_test_monitor(struct kdbus_test_env *env)
 
 	cookie++;
 	ret = kdbus_msg_send(env->conn, NULL, cookie, 0, 0, 0,
-			     KDBUS_DST_ID_BROADCAST);
+			     KDBUS_DST_ID_BROADCAST, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	/* The monitor should get the message. */
@@ -103,7 +104,7 @@ int kdbus_test_monitor(struct kdbus_test_env *env)
 
 	cookie++;
 	ret = kdbus_msg_send(env->conn, NULL, cookie, 0, 0, 0,
-			     KDBUS_DST_ID_BROADCAST);
+			     KDBUS_DST_ID_BROADCAST, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	ret = kdbus_msg_recv_poll(monitor, 100, &msg, &offset);
@@ -128,7 +129,7 @@ int kdbus_test_monitor(struct kdbus_test_env *env)
 
 	cookie++;
 	ret = kdbus_msg_send(env->conn, NULL, cookie, 0, 0, 0,
-			     KDBUS_DST_ID_BROADCAST);
+			     KDBUS_DST_ID_BROADCAST, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	ret = kdbus_msg_recv_poll(monitor, 100, &msg, &offset);
diff --git a/tools/testing/selftests/kdbus/test-policy-ns.c b/tools/testing/selftests/kdbus/test-policy-ns.c
index 3437012..2d70ee4 100644
--- a/tools/testing/selftests/kdbus/test-policy-ns.c
+++ b/tools/testing/selftests/kdbus/test-policy-ns.c
@@ -180,7 +180,7 @@ static int kdbus_recv_in_threads(const char *bus, const char *name,
 			break;
 
 		ret = kdbus_msg_send(conn_db[i], name, cookie++,
-				     0, 0, 0, dst_id);
+				     0, 0, 0, dst_id, 0, NULL);
 		if (ret < 0) {
 			/*
 			 * Receivers are not reading their messages,
@@ -295,7 +295,7 @@ static int kdbus_fork_test_by_id(const char *bus,
 		 * EXIT_FAILURE.
 		 */
 		ret = kdbus_msg_send(conn_src, NULL, cookie,
-				     0, 0, 0, conn_db[0]->id);
+				     0, 0, 0, conn_db[0]->id, 0, NULL);
 		ASSERT_EXIT(ret == child_status);
 
 		ret = kdbus_msg_recv_poll(conn_src, 100, NULL, NULL);
@@ -331,7 +331,7 @@ static int kdbus_fork_test_by_id(const char *bus,
 	}
 
 	ret = kdbus_msg_send(conn_db[0], NULL, ++cookie,
-			     0, 0, 0, msg->src_id);
+			     0, 0, 0, msg->src_id, 0, NULL);
 	/*
 	 * parent_status is checked against send operations,
 	 * on failures always return TEST_ERR.
@@ -454,7 +454,7 @@ static int __kdbus_clone_userns_test(const char *bus,
 		ASSERT_EXIT(ret == 0);
 
 		ret = kdbus_msg_send(conn_src, name, 0xabcd1234,
-				     0, 0, 0, KDBUS_DST_ID_NAME);
+				     0, 0, 0, KDBUS_DST_ID_NAME, 0, NULL);
 		kdbus_conn_free(conn_src);
 
 		_exit(ret == expected_status ? EXIT_SUCCESS : EXIT_FAILURE);
diff --git a/tools/testing/selftests/kdbus/test-policy-priv.c b/tools/testing/selftests/kdbus/test-policy-priv.c
index a318ccc..14a09c6 100644
--- a/tools/testing/selftests/kdbus/test-policy-priv.c
+++ b/tools/testing/selftests/kdbus/test-policy-priv.c
@@ -29,7 +29,7 @@ static int test_policy_priv_by_id(const char *bus,
 	ret = RUN_UNPRIVILEGED_CONN(unpriv, bus, ({
 		ret = kdbus_msg_send(unpriv, NULL,
 				     expected_cookie, 0, 0, 0,
-				     conn_dst->id);
+				     conn_dst->id, 0, NULL);
 		ASSERT_EXIT(ret == child_status);
 	}));
 	ASSERT_RETURN(ret >= 0);
@@ -92,7 +92,8 @@ static int test_policy_priv_by_broadcast(const char *bus,
 
 		/* Use expected_cookie since 'msg' might be NULL */
 		ret = kdbus_msg_send(child, NULL, expected_cookie + 1,
-				     0, 0, 0, KDBUS_DST_ID_BROADCAST);
+				     0, 0, 0, KDBUS_DST_ID_BROADCAST,
+				     0, NULL);
 		ASSERT_EXIT(ret == 0);
 
 		kdbus_msg_free(msg);
@@ -107,7 +108,8 @@ static int test_policy_priv_by_broadcast(const char *bus,
 
 			ret = kdbus_msg_send(child_2, NULL,
 					     expected_cookie, 0, 0, 0,
-					     KDBUS_DST_ID_BROADCAST);
+					     KDBUS_DST_ID_BROADCAST,
+					     0, NULL);
 			ASSERT_RETURN(ret == 0);
 
 			/* Use a little bit high time */
@@ -142,7 +144,8 @@ static int test_policy_priv_by_broadcast(const char *bus,
 
 				ret = kdbus_msg_send(child_2, NULL,
 						expected_cookie, 0, 0, 0,
-						KDBUS_DST_ID_BROADCAST);
+						KDBUS_DST_ID_BROADCAST,
+						0, NULL);
 				ASSERT_EXIT(ret == 0);
 
 				/* Use a little bit high time */
@@ -218,7 +221,8 @@ static int test_priv_before_policy_upload(struct kdbus_test_env *env)
 		ret = kdbus_msg_send(unpriv, NULL, 0xdeadbeef,
 				     KDBUS_MSG_EXPECT_REPLY,
 				     5000000000ULL, 0,
-				     KDBUS_DST_ID_BROADCAST);
+				     KDBUS_DST_ID_BROADCAST,
+				     0, NULL);
 		ASSERT_EXIT(ret == -ENOTUNIQ);
 	}));
 	ASSERT_RETURN(ret == 0);
@@ -405,7 +409,7 @@ static int test_broadcast_after_policy_upload(struct kdbus_test_env *env)
 
 	++expected_cookie;
 	ret = kdbus_msg_send(owner_a, NULL, expected_cookie, 0,
-			     0, 0, KDBUS_DST_ID_BROADCAST);
+			     0, 0, KDBUS_DST_ID_BROADCAST, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	ret = kdbus_msg_recv_poll(owner_b, 100, &msg, NULL);
@@ -499,7 +503,8 @@ static int test_broadcast_after_policy_upload(struct kdbus_test_env *env)
 		ret = kdbus_msg_send(unpriv_owner, NULL,
 				     expected_cookie,
 				     0, 0, 0,
-				     KDBUS_DST_ID_BROADCAST);
+				     KDBUS_DST_ID_BROADCAST,
+				     0, NULL);
 		ASSERT_EXIT(ret == 0);
 
 		/*
@@ -609,7 +614,7 @@ static int test_broadcast_after_policy_upload(struct kdbus_test_env *env)
 					 NULL);
 		ASSERT_EXIT(ret >= 0);
 		ret = kdbus_msg_send(unpriv, NULL, expected_cookie,
-				     0, 0, 0, KDBUS_DST_ID_BROADCAST);
+				     0, 0, 0, KDBUS_DST_ID_BROADCAST, 0, NULL);
 		ASSERT_EXIT(ret == 0);
 	}));
 	ASSERT_RETURN(ret == 0);
@@ -641,7 +646,7 @@ static int test_broadcast_after_policy_upload(struct kdbus_test_env *env)
 					 NULL);
 		ASSERT_EXIT(ret >= 0);
 		ret = kdbus_msg_send(unpriv, NULL, expected_cookie,
-				     0, 0, 0, KDBUS_DST_ID_BROADCAST);
+				     0, 0, 0, KDBUS_DST_ID_BROADCAST, 0, NULL);
 		ASSERT_EXIT(ret == 0);
 	}));
 	ASSERT_RETURN(ret == 0);
@@ -953,7 +958,8 @@ static int test_policy_priv(struct kdbus_test_env *env)
 	conn = kdbus_hello(env->buspath, 0, NULL, 0);
 	ASSERT_RETURN(conn);
 
-	ret = kdbus_msg_send(conn, "com.example.b", 0xdeadbeef, 0, 0, 0, 0);
+	ret = kdbus_msg_send(conn, "com.example.b", 0xdeadbeef, 0, 0, 0, 0,
+			     0, NULL);
 	ASSERT_EXIT(ret >= 0);
 
 	ret = kdbus_msg_recv_poll(conn_b, 300, NULL, NULL);
@@ -967,7 +973,7 @@ static int test_policy_priv(struct kdbus_test_env *env)
 
 	ret = RUN_UNPRIVILEGED_CONN(unpriv, env->buspath, ({
 		ret = kdbus_msg_send(unpriv, "com.example.b", 0xdeadbeef, 0, 0,
-				     0, 0);
+				     0, 0, 0, NULL);
 		ASSERT_EXIT(ret == -EPERM);
 	}));
 	ASSERT_RETURN(ret >= 0);
@@ -996,7 +1002,7 @@ static int test_policy_priv(struct kdbus_test_env *env)
 		ASSERT_EXIT(ret >= 0);
 
 		ret = kdbus_msg_send(unpriv, "com.example.c", 0xdeadbeef, 0, 0,
-				     0, 0);
+				     0, 0, 0, NULL);
 		ASSERT_EXIT(ret >= 0);
 		ret = kdbus_msg_recv_poll(owner, 100, NULL, NULL);
 		ASSERT_EXIT(ret >= 0);
@@ -1021,7 +1027,7 @@ static int test_policy_priv(struct kdbus_test_env *env)
 
 	ret = RUN_UNPRIVILEGED_CONN(unpriv, env->buspath, ({
 		ret = kdbus_msg_send(unpriv, "com.example.b", 0xdeadbeef, 0, 0,
-				     0, 0);
+				     0, 0, 0, NULL);
 		ASSERT_EXIT(ret >= 0);
 	}));
 	ASSERT_RETURN(ret >= 0);
@@ -1045,7 +1051,7 @@ static int test_policy_priv(struct kdbus_test_env *env)
 
 	ret = RUN_UNPRIVILEGED_CONN(unpriv, env->buspath, ({
 		ret = kdbus_msg_send(unpriv, "com.example.b", 0xdeadbeef, 0, 0,
-				     0, 0);
+				     0, 0, 0, NULL);
 		ASSERT_EXIT(ret >= 0);
 	}));
 	ASSERT_RETURN(ret >= 0);
@@ -1069,7 +1075,7 @@ static int test_policy_priv(struct kdbus_test_env *env)
 
 	ret = RUN_UNPRIVILEGED_CONN(unpriv, env->buspath, ({
 		ret = kdbus_msg_send(unpriv, "com.example.b", 0xdeadbeef, 0, 0,
-				     0, 0);
+				     0, 0, 0, NULL);
 		ASSERT_EXIT(ret >= 0);
 	}));
 	ASSERT_RETURN(ret >= 0);
@@ -1116,7 +1122,7 @@ static int test_policy_priv(struct kdbus_test_env *env)
 
 	ret = RUN_UNPRIVILEGED_CONN(unpriv, env->buspath, ({
 		ret = kdbus_msg_send(unpriv, "com.example.b", 0xdeadbeef, 0, 0,
-				     0, 0);
+				     0, 0, 0, NULL);
 		ASSERT_EXIT(ret == -EPERM);
 	}));
 	ASSERT_RETURN(ret >= 0);
@@ -1137,7 +1143,7 @@ static int test_policy_priv(struct kdbus_test_env *env)
 
 	ret = RUN_UNPRIVILEGED_CONN(unpriv, env->buspath, ({
 		ret = kdbus_msg_send(unpriv, "com.example.b", 0xdeadbeef, 0, 0,
-				     0, 0);
+				     0, 0, 0, NULL);
 		ASSERT_EXIT(ret >= 0);
 	}));
 	ASSERT_RETURN(ret >= 0);
@@ -1161,7 +1167,7 @@ static int test_policy_priv(struct kdbus_test_env *env)
 
 	ret = RUN_UNPRIVILEGED_CONN(unpriv, env->buspath, ({
 		ret = kdbus_msg_send(unpriv, "com.example.b", 0xdeadbeef, 0, 0,
-				     0, 0);
+				     0, 0, 0, NULL);
 		ASSERT_EXIT(ret >= 0);
 
 		ret = kdbus_msg_recv_poll(conn_b, 100, NULL, NULL);
@@ -1172,7 +1178,7 @@ static int test_policy_priv(struct kdbus_test_env *env)
 		ASSERT_RETURN(ret == 0);
 
 		ret = kdbus_msg_send(unpriv, "com.example.b", 0xdeadbeef, 0, 0,
-				     0, 0);
+				     0, 0, 0, NULL);
 		ASSERT_EXIT(ret == -EPERM);
 	}));
 	ASSERT_RETURN(ret >= 0);
@@ -1213,7 +1219,7 @@ static int test_policy_priv(struct kdbus_test_env *env)
 		ASSERT_RETURN(unpriv);
 
 		ret = kdbus_msg_send(unpriv, "com.example.c", 0xdeadbeef, 0, 0,
-				     0, 0);
+				     0, 0, 0, NULL);
 		ASSERT_EXIT(ret >= 0);
 
 		ret = kdbus_msg_recv_poll(owner, 100, NULL, NULL);
@@ -1223,7 +1229,7 @@ static int test_policy_priv(struct kdbus_test_env *env)
 		kdbus_conn_free(conn);
 
 		ret = kdbus_msg_send(unpriv, "com.example.c", 0xdeadbeef, 0, 0,
-				     0, 0);
+				     0, 0, 0, NULL);
 		ASSERT_EXIT(ret == -EPERM);
 
 		kdbus_conn_free(unpriv);
diff --git a/tools/testing/selftests/kdbus/test-send.c b/tools/testing/selftests/kdbus/test-send.c
new file mode 100644
index 0000000..9bd8c8f
--- /dev/null
+++ b/tools/testing/selftests/kdbus/test-send.c
@@ -0,0 +1,84 @@
+#include <stdio.h>
+#include <string.h>
+#include <time.h>
+#include <fcntl.h>
+#include <stdlib.h>
+/* Use in conjunction with test-kdbus-daemon */
+
+#include <unistd.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <errno.h>
+#include <assert.h>
+#include <poll.h>
+#include <sys/ioctl.h>
+
+#include "kdbus-test.h"
+#include "kdbus-util.h"
+#include "kdbus-enum.h"
+
+int get_file(const char *fname, int flags, const char *content)
+{
+	FILE *f;
+
+	if (access(fname, F_OK) < 0) {
+		f = fopen(fname, "w");
+		if (!f)
+			return -1;
+		fprintf(f, "%s\n", content);
+		fclose(f);
+	}
+
+	return open(fname, flags);
+}
+
+int kdbus_test_send(struct kdbus_test_env *env)
+{
+	int ret;
+	int serial = 1;
+	int fds[3];
+	size_t i;
+
+	if (!env->conn)
+		return EXIT_FAILURE;
+
+	fds[0] = get_file("/tmp/kdbus-test-send.rd", O_RDONLY, "foo");
+	fds[1] = get_file("/tmp/kdbus-test-send.wr", O_WRONLY, "bar");
+	fds[2] = get_file("/tmp/kdbus-test-send.rdwr", O_RDWR, "baz");
+
+	for (i = 0; i < ELEMENTSOF(fds); i++) {
+		if (fds[i] < 0) {
+			fprintf(stderr, "Unable to open data/fileN file(s)\n");
+			return EXIT_FAILURE;
+		}
+	}
+
+	ret = kdbus_msg_send(env->conn, "com.example.kdbus-test", serial++,
+			     0, 0, 0, 0, 0, NULL);
+	if (ret < 0)
+		fprintf(stderr, "error sending simple message: %d (%m)\n",
+			ret);
+
+	ret = kdbus_msg_send(env->conn, "com.example.kdbus-test", serial++,
+			     0, 0, 0, 0, 1, fds);
+	if (ret < 0)
+		fprintf(stderr, "error sending message with 1 fd: %d (%m)\n",
+			ret);
+
+	ret = kdbus_msg_send(env->conn, "com.example.kdbus-test", serial++,
+			     0, 0, 0, 0, 2, fds);
+	if (ret < 0)
+		fprintf(stderr, "error sending message with 2 fds: %d (%m)\n",
+			ret);
+
+	ret = kdbus_msg_send(env->conn, "com.example.kdbus-test", serial++,
+			     0, 0, 0, 0, 3, fds);
+	if (ret < 0)
+		fprintf(stderr, "error sending message with 3 fds: %d (%m)\n",
+			ret);
+
+	for (i = 0; i < ELEMENTSOF(fds); i++)
+		close(fds[i]);
+
+	return EXIT_SUCCESS;
+}
diff --git a/tools/testing/selftests/kdbus/test-sync.c b/tools/testing/selftests/kdbus/test-sync.c
index e2be910..77c3c41 100644
--- a/tools/testing/selftests/kdbus/test-sync.c
+++ b/tools/testing/selftests/kdbus/test-sync.c
@@ -113,7 +113,7 @@ static int close_epipe_sync(const char *bus)
 	}
 
 	ret = kdbus_msg_send(conn_dst, NULL, cookie, 0, 0, 0,
-			     KDBUS_DST_ID_BROADCAST);
+			     KDBUS_DST_ID_BROADCAST, 0, NULL);
 	ASSERT_RETURN(ret == 0);
 
 	cookie++;
diff --git a/tools/testing/selftests/kdbus/test-timeout.c b/tools/testing/selftests/kdbus/test-timeout.c
index cfd1930..a5cdaf2 100644
--- a/tools/testing/selftests/kdbus/test-timeout.c
+++ b/tools/testing/selftests/kdbus/test-timeout.c
@@ -69,7 +69,7 @@ int kdbus_test_timeout(struct kdbus_test_env *env)
 		ASSERT_RETURN(kdbus_msg_send(conn_b, NULL, cookie,
 			      KDBUS_MSG_EXPECT_REPLY,
 			      (i + 1) * 100ULL * 1000000ULL, 0,
-			      conn_a->id) == 0);
+			      conn_a->id, 0, NULL) == 0);
 		expected |= 1ULL << cookie;
 	}
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC 8/8] kdbus: Ability to run kdbus test by executable binary name
  2015-07-08 10:25 [RFC 0/8] Introduce LSM to KDBUS Paul Osmialowski
                   ` (6 preceding siblings ...)
  2015-07-08 10:25 ` [RFC 7/8] kdbus: selftests extended Paul Osmialowski
@ 2015-07-08 10:25 ` Paul Osmialowski
  2015-07-08 14:16   ` Greg Kroah-Hartman
  2015-07-08 16:46 ` [RFC 0/8] Introduce LSM to KDBUS Casey Schaufler
  8 siblings, 1 reply; 30+ messages in thread
From: Paul Osmialowski @ 2015-07-08 10:25 UTC (permalink / raw)
  To: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Paul Osmialowski, Lukasz Skalski

With this applied, you can do following:

$ cp kdbus-test daemon
$ cp kdbus-test send

Then run 'daemon' in one shell session:

$ ./daemon --bus test

...and 'send' in another:

$ ./send --bus test

Useful for testing features introduced by previous patches.

Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
---
 tools/testing/selftests/kdbus/kdbus-test.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/testing/selftests/kdbus/kdbus-test.c b/tools/testing/selftests/kdbus/kdbus-test.c
index 5a296da..9efbc5f 100644
--- a/tools/testing/selftests/kdbus/kdbus-test.c
+++ b/tools/testing/selftests/kdbus/kdbus-test.c
@@ -829,6 +829,7 @@ int main(int argc, char *argv[])
 		ARG_UIDMAP,
 		ARG_GIDMAP,
 	};
+	char *exec = basename(argv[0]);
 
 	kdbus_args = malloc(sizeof(*kdbus_args));
 	if (!kdbus_args) {
@@ -858,6 +859,10 @@ int main(int argc, char *argv[])
 
 	srand(time(NULL));
 
+	if (strcmp(exec, "kdbus-test") != 0) {
+		kdbus_args->test = exec;
+	}
+
 	while ((t = getopt_long(argc, argv, "hxfm:r:t:b:w:a", options, NULL)) >= 0) {
 		switch (t) {
 		case 'x':
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [RFC 3/8] lsm: kdbus security hooks
  2015-07-08 10:25 ` [RFC 3/8] lsm: kdbus security hooks Paul Osmialowski
@ 2015-07-08 11:00   ` Lukasz Pawelczyk
  2015-07-08 14:14   ` Greg Kroah-Hartman
  1 sibling, 0 replies; 30+ messages in thread
From: Lukasz Pawelczyk @ 2015-07-08 11:00 UTC (permalink / raw)
  To: Paul Osmialowski, Paul Moore, James Morris, Casey Schaufler,
	Serge E. Hallyn, Kees Cook, Tetsuo Handa, Stephen Smalley,
	Neil Brown, Mark Rustad, Greg Kroah-Hartman, Daniel Mack,
	David Herrmann, Djalal Harouni, Shuah Khan, Al Viro,
	linux-security-module, linux-kernel, linux-api
  Cc: Karol Lewandowski, Lukasz Skalski

On śro, 2015-07-08 at 12:25 +0200, Paul Osmialowski wrote:
> This is combination of the work by Karol Lewandowski and Paul Moore
> on LSM hooks for kdbus.
> 
> Originates from:
> 
> git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)
> commit: 7050f206a79564886938d0edc4e1e9da5972c72d
> 
> https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd
> -v212)
> commit: a9fe4c33b6e5ab25a243e0590df406aabb6add12
> 
> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
> Signed-off-by: Paul Moore <pmoore@redhat.com>
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
>  include/linux/lsm_hooks.h |  67 ++++++++++++++++++++++++++
>  include/linux/security.h  |  99 
> +++++++++++++++++++++++++++++++++++++++
>  security/security.c       | 117 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 283 insertions(+)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 9429f05..2a8d8fc 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1297,6 +1297,36 @@
>   *	@inode we wish to get the security context of.
>   *	@ctx is a pointer in which to place the allocated security 
> context.
>   *	@ctxlen points to the place to put the length of @ctx.
> + *
> + * @kdbus_domain_alloc:
> + *	Allocate kdbus domain.
> + * @kdbus_domain_free:
> + *	Deallocate kdbus domain.
> + * @kdbus_bus_alloc:
> + *	Allocate kdbus bus.
> + * @kdbus_bus_free:
> + *	Deallocate kdbus bus.
> + * @kdbus_send:
> + *	Send message.
> + * @kdbus_recv:
> + *	Receive message.
> + * @kdbus_name_acquire:
> + *	Request a well-known bus name to associate with the 
> connection.
> + * @kdbus_name_list:
> + *	Retrieve the list of all currently registered well-known and 
> unique
> + *	names.
> + * @kdbus_ep_create:
> + *	Endpoint create
> + * @kdbus_connect:
> + *	Connect
> + * @kdbus_conn_free:
> + *	Deallocate connection
> + * @kdbus_conn_info:
> + *	Retrieve credentials and properties of the initial creator 
> of the
> + *	connection.
> + * @kdbus_talk:
> + *	Talk to a given peer.
> + *
>   * This is the main security structure.
>   */
>  
> @@ -1520,6 +1550,29 @@ union security_list_options {
>  	int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 
> ctxlen);
>  	int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 
> *ctxlen);
>  
> +	int (*kdbus_domain_alloc)(struct kdbus_domain *domain);
> +	void (*kdbus_domain_free)(struct kdbus_domain *domain);

Shouldn't all of this be inside some #ifdef CONFIG_KDBUS (or whatever
the CONFIG for kdbus is)?

> +
> +	int (*kdbus_bus_alloc)(struct kdbus_bus *bus);
> +	void (*kdbus_bus_free)(struct kdbus_bus *bus);
> +	int (*kdbus_send)(const struct kdbus_conn *conn,
> +			  const struct kdbus_bus *bus);
> +	int (*kdbus_recv)(const struct kdbus_conn *conn,
> +			  const struct kdbus_bus *bus);
> +	int (*kdbus_name_acquire)(const struct kdbus_conn *conn,
> +			  const char *name);
> +	int (*kdbus_name_list)(const struct kdbus_bus *bus);
> +
> +	int (*kdbus_ep_create)(const struct kdbus_bus *bus);
> +	int (*kdbus_ep_setpolicy)(const struct kdbus_bus *bus);
> +
> +	int (*kdbus_connect)(struct kdbus_conn *conn,
> +			     const char *secctx, u32 seclen);
> +	void (*kdbus_conn_free)(struct kdbus_conn *conn);
> +	int (*kdbus_conn_info)(const struct kdbus_conn *conn);
> +	int (*kdbus_talk)(const struct kdbus_conn *src,
> +			  const struct kdbus_conn *dst);
> +
>  #ifdef CONFIG_SECURITY_NETWORK
>  	int (*unix_stream_connect)(struct sock *sock, struct sock 
> *other,
>  					struct sock *newsk);
> @@ -1760,6 +1813,20 @@ struct security_hook_heads {
>  	struct list_head inode_notifysecctx;
>  	struct list_head inode_setsecctx;
>  	struct list_head inode_getsecctx;
> +	struct list_head kdbus_domain_alloc;
> +	struct list_head kdbus_domain_free;
> +	struct list_head kdbus_bus_alloc;
> +	struct list_head kdbus_bus_free;
> +	struct list_head kdbus_send;
> +	struct list_head kdbus_recv;
> +	struct list_head kdbus_name_acquire;
> +	struct list_head kdbus_name_list;
> +	struct list_head kdbus_ep_create;
> +	struct list_head kdbus_ep_setpolicy;
> +	struct list_head kdbus_connect;
> +	struct list_head kdbus_conn_free;
> +	struct list_head kdbus_conn_info;
> +	struct list_head kdbus_talk;
>  #ifdef CONFIG_SECURITY_NETWORK
>  	struct list_head unix_stream_connect;
>  	struct list_head unix_may_send;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 79d85dd..5f257b9 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -53,6 +53,10 @@ struct msg_queue;
>  struct xattr;
>  struct xfrm_sec_ctx;
>  struct mm_struct;
> +struct kdbus_ep;
> +struct kdbus_bus;
> +struct kdbus_conn;
> +struct kdbus_domain;
>  
>  /* If capable should audit the security request */
>  #define SECURITY_CAP_NOAUDIT 0
> @@ -356,6 +360,28 @@ void security_release_secctx(char *secdata, u32 
> seclen);
>  int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 
> ctxlen);
>  int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 
> ctxlen);
>  int security_inode_getsecctx(struct inode *inode, void **ctx, u32 
> *ctxlen);
> +
> +int security_kdbus_domain_alloc(struct kdbus_domain *domain);
> +void security_kdbus_domain_free(struct kdbus_domain *domain);
> +
> +int security_kdbus_bus_alloc(struct kdbus_bus *bus);
> +void security_kdbus_bus_free(struct kdbus_bus *bus);
> +int security_kdbus_send(const struct kdbus_conn *conn,
> +			const struct kdbus_bus *bus);
> +int security_kdbus_recv(const struct kdbus_conn *conn,
> +			const struct kdbus_bus *bus);
> +int security_kdbus_name_acquire(const struct kdbus_conn *conn,
> +				const char *name);
> +int security_kdbus_name_list(const struct kdbus_bus *bus);
> +int security_kdbus_ep_create(struct kdbus_bus *bus);
> +int security_kdbus_ep_setpolicy(struct kdbus_bus *bus);
> +int security_kdbus_connect(struct kdbus_conn *conn,
> +			   const char *secctx, u32 seclen);
> +void security_kdbus_conn_free(struct kdbus_conn *conn);
> +int security_kdbus_conn_info(const struct kdbus_conn *conn);
> +int security_kdbus_talk(const struct kdbus_conn *src,
> +			const struct kdbus_conn *dst);
> +
>  #else /* CONFIG_SECURITY */
>  struct security_mnt_opts {
>  };
> @@ -1105,6 +1131,79 @@ static inline int 
> security_inode_getsecctx(struct inode *inode, void **ctx, u32
>  {
>  	return -EOPNOTSUPP;
>  }
> +
> +static inline int security_kdbus_domain_alloc(struct kdbus_domain 
> *domain)
> +{
> +	return 0;
> +}
> +
> +static inline void security_kdbus_domain_free(struct kdbus_domain 
> *domain)
> +{
> +}
> +
> +static inline int security_kdbus_bus_alloc(struct kdbus_bus *bus)
> +{
> +	return 0;
> +}
> +
> +static inline void security_kdbus_bus_free(struct kdbus_bus *bus)
> +{
> +}
> +
> +static inline int security_kdbus_send(const struct kdbus_conn *conn,
> +				      const struct kdbus_bus *bus)
> +{
> +	return 0;
> +}
> +
> +static inline int security_kdbus_recv(const struct kdbus_conn *conn,
> +				      const struct kdbus_bus *bus)
> +{
> +	return 0;
> +}
> +
> +static inline int security_kdbus_name_acquire(const struct 
> kdbus_conn *conn,
> +					      const char *name)
> +{
> +	return 0;
> +}
> +
> +static inline int security_kdbus_name_list(const struct kdbus_bus 
> *bus)
> +{
> +	return 0;
> +}
> +
> +static inline int security_kdbus_ep_create(const struct kdbus_bus 
> *bus)
> +{
> +	return 0;
> +}
> +
> +static inline int security_kdbus_ep_setpolicy(const struct kdbus_bus 
> *bus)
> +{
> +	return 0;
> +}
> +
> +static inline int security_kdbus_connect(struct kdbus_conn *conn,
> +					 const char *secctx, u32 
> seclen)
> +{
> +	return 0;
> +}
> +
> +static inline void security_kdbus_conn_free(struct kdbus_conn *conn)
> +{
> +}
> +
> +static inline int security_kdbus_conn_info(const struct kdbus_conn 
> *conn)
> +{
> +	return 0;
> +}
> +
> +static inline int security_kdbus_talk(const struct kdbus_conn *src,
> +				      const struct kdbus_conn *dst)
> +{
> +	return 0;
> +}
> +
>  #endif	/* CONFIG_SECURITY */
>  
>  #ifdef CONFIG_SECURITY_NETWORK
> diff --git a/security/security.c b/security/security.c
> index b1e935b..7fb46d1 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1185,6 +1185,95 @@ int security_inode_getsecctx(struct inode 
> *inode, void **ctx, u32 *ctxlen)
>  }
>  EXPORT_SYMBOL(security_inode_getsecctx);
>  
> +int security_kdbus_domain_alloc(struct kdbus_domain *domain)
> +{
> +	return call_int_hook(kdbus_domain_alloc, 0, domain);
> +}
> +EXPORT_SYMBOL(security_kdbus_domain_alloc);
> +
> +void security_kdbus_domain_free(struct kdbus_domain *domain)
> +{
> +	call_void_hook(kdbus_domain_free, domain);
> +}
> +EXPORT_SYMBOL(security_kdbus_domain_free);
> +
> +int security_kdbus_bus_alloc(struct kdbus_bus *bus)
> +{
> +	return call_int_hook(kdbus_bus_alloc, 0, bus);
> +}
> +EXPORT_SYMBOL(security_kdbus_bus_alloc);
> +
> +void security_kdbus_bus_free(struct kdbus_bus *bus)
> +{
> +	call_void_hook(kdbus_bus_free, bus);
> +}
> +EXPORT_SYMBOL(security_kdbus_bus_free);
> +
> +int security_kdbus_send(const struct kdbus_conn *conn,
> +			const struct kdbus_bus *bus)
> +{
> +	return call_int_hook(kdbus_send, 0, conn, bus);
> +}
> +EXPORT_SYMBOL(security_kdbus_send);
> +
> +int security_kdbus_recv(const struct kdbus_conn *conn,
> +			const struct kdbus_bus *bus)
> +{
> +	return call_int_hook(kdbus_recv, 0, conn, bus);
> +}
> +EXPORT_SYMBOL(security_kdbus_recv);
> +
> +int security_kdbus_name_acquire(const struct kdbus_conn *conn,
> +				const char *name)
> +{
> +	return call_int_hook(kdbus_name_acquire, 0, conn, name);
> +}
> +EXPORT_SYMBOL(security_kdbus_name_acquire);
> +
> +int security_kdbus_name_list(const struct kdbus_bus *bus)
> +{
> +	return call_int_hook(kdbus_name_list, 0, bus);
> +}
> +EXPORT_SYMBOL(security_kdbus_name_list);
> +
> +int security_kdbus_ep_create(struct kdbus_bus *bus)
> +{
> +	return call_int_hook(kdbus_ep_create, 0, bus);
> +}
> +EXPORT_SYMBOL(security_kdbus_ep_create);
> +
> +int security_kdbus_ep_setpolicy(struct kdbus_bus *bus)
> +{
> +	return call_int_hook(kdbus_ep_setpolicy, 0, bus);
> +}
> +EXPORT_SYMBOL(security_kdbus_ep_setpolicy);
> +
> +int security_kdbus_connect(struct kdbus_conn *conn,
> +			   const char *secctx, u32 seclen)
> +{
> +	return call_int_hook(kdbus_connect, 0, conn, secctx, 
> seclen);
> +}
> +EXPORT_SYMBOL(security_kdbus_connect);
> +
> +void security_kdbus_conn_free(struct kdbus_conn *conn)
> +{
> +	call_void_hook(kdbus_conn_free, conn);
> +}
> +EXPORT_SYMBOL(security_kdbus_conn_free);
> +
> +int security_kdbus_conn_info(const struct kdbus_conn *conn)
> +{
> +	return call_int_hook(kdbus_conn_info, 0, conn);
> +}
> +EXPORT_SYMBOL(security_kdbus_conn_info);
> +
> +int security_kdbus_talk(const struct kdbus_conn *src,
> +			const struct kdbus_conn *dst)
> +{
> +	return call_int_hook(kdbus_talk, 0, src, dst);
> +}
> +EXPORT_SYMBOL(security_kdbus_talk);
> +
>  #ifdef CONFIG_SECURITY_NETWORK
>  
>  int security_unix_stream_connect(struct sock *sock, struct sock 
> *other, struct sock *newsk)
> @@ -1774,6 +1863,34 @@ struct security_hook_heads security_hook_heads 
> = {
>  		LIST_HEAD_INIT(security_hook_heads.inode_setsecctx),
>  	.inode_getsecctx =
>  		LIST_HEAD_INIT(security_hook_heads.inode_getsecctx),
> +	.kdbus_domain_alloc =
> +		LIST_HEAD_INIT(security_hook_heads.kdbus_domain_allo
> c),
> +	.kdbus_domain_free =
> +		LIST_HEAD_INIT(security_hook_heads.kdbus_domain_free
> ),
> +	.kdbus_bus_alloc =
> +		LIST_HEAD_INIT(security_hook_heads.kdbus_bus_alloc),
> +	.kdbus_bus_free =
> +		LIST_HEAD_INIT(security_hook_heads.kdbus_bus_free),
> +	.kdbus_send =
> +		LIST_HEAD_INIT(security_hook_heads.kdbus_send),
> +	.kdbus_recv =
> +		LIST_HEAD_INIT(security_hook_heads.kdbus_recv),
> +	.kdbus_name_acquire =
> +		LIST_HEAD_INIT(security_hook_heads.kdbus_name_acquir
> e),
> +	.kdbus_name_list =
> +		LIST_HEAD_INIT(security_hook_heads.kdbus_name_list),
> +	.kdbus_ep_create =
> +		LIST_HEAD_INIT(security_hook_heads.kdbus_ep_create),
> +	.kdbus_ep_setpolicy =
> +		LIST_HEAD_INIT(security_hook_heads.kdbus_ep_setpolic
> y),
> +	.kdbus_connect =
> +		LIST_HEAD_INIT(security_hook_heads.kdbus_connect),
> +	.kdbus_conn_free =
> +		LIST_HEAD_INIT(security_hook_heads.kdbus_conn_free),
> +	.kdbus_conn_info =
> +		LIST_HEAD_INIT(security_hook_heads.kdbus_conn_info),
> +	.kdbus_talk =
> +		LIST_HEAD_INIT(security_hook_heads.kdbus_talk),
>  #ifdef CONFIG_SECURITY_NETWORK
>  	.unix_stream_connect =
>  		LIST_HEAD_INIT(security_hook_heads.unix_stream_conne
> ct),
-- 
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics




^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
  2015-07-08 10:25 ` [RFC 5/8] kdbus: use LSM hooks in kdbus code Paul Osmialowski
@ 2015-07-08 11:06   ` Lukasz Pawelczyk
  2015-07-08 11:09   ` Lukasz Pawelczyk
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 30+ messages in thread
From: Lukasz Pawelczyk @ 2015-07-08 11:06 UTC (permalink / raw)
  To: Paul Osmialowski, Paul Moore, James Morris, Casey Schaufler,
	Serge E. Hallyn, Kees Cook, Tetsuo Handa, Stephen Smalley,
	Neil Brown, Mark Rustad, Greg Kroah-Hartman, Daniel Mack,
	David Herrmann, Djalal Harouni, Shuah Khan, Al Viro,
	linux-security-module, linux-kernel, linux-api
  Cc: Karol Lewandowski, Lukasz Skalski

On śro, 2015-07-08 at 12:25 +0200, Paul Osmialowski wrote:
> Originates from:
> 
> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd
> -v212)
> commit: aa0885489d19be92fa41c6f0a71df28763228a40
> 
> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
>  ipc/kdbus/bus.c        | 12 ++++++++++-
>  ipc/kdbus/bus.h        |  3 +++
>  ipc/kdbus/connection.c | 54 
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  ipc/kdbus/connection.h |  4 ++++
>  ipc/kdbus/domain.c     |  9 ++++++++-
>  ipc/kdbus/domain.h     |  2 ++
>  ipc/kdbus/endpoint.c   | 11 ++++++++++
>  ipc/kdbus/names.c      | 11 ++++++++++
>  ipc/kdbus/queue.c      | 30 ++++++++++++++++++----------
>  9 files changed, 124 insertions(+), 12 deletions(-)
> 
> diff --git a/ipc/kdbus/bus.c b/ipc/kdbus/bus.c
> index bbdf0f2..9894895 100644
> --- a/ipc/kdbus/bus.c
> +++ b/ipc/kdbus/bus.c
> @@ -22,6 +22,7 @@
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "notify.h"
> @@ -51,6 +52,7 @@ static void kdbus_bus_free(struct kdbus_node *node)
>  	kdbus_domain_unref(bus->domain);
>  	kdbus_policy_db_clear(&bus->policy_db);
>  	kdbus_meta_proc_unref(bus->creator_meta);
> +	security_kdbus_bus_free(bus);
>  	kfree(bus);
>  }
>  
> @@ -161,6 +163,12 @@ static struct kdbus_bus *kdbus_bus_new(struct 
> kdbus_domain *domain,
>  		goto exit_unref;
>  	}
>  
> +	ret = security_kdbus_bus_alloc(b);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit_unref;
> +	}
> +
>  	/*
>  	 * Bus-limits of the creator are accounted on its real UID, 
> just like
>  	 * all other per-user limits.
> @@ -169,11 +177,13 @@ static struct kdbus_bus *kdbus_bus_new(struct 
> kdbus_domain *domain,
>  	if (IS_ERR(b->creator)) {
>  		ret = PTR_ERR(b->creator);
>  		b->creator = NULL;
> -		goto exit_unref;
> +		goto exit_free_security;
>  	}
>  
>  	return b;
>  
> +exit_free_security:
> +	security_kdbus_bus_free(b);
>  exit_unref:
>  	kdbus_node_deactivate(&b->node);
>  	kdbus_node_unref(&b->node);
> diff --git a/ipc/kdbus/bus.h b/ipc/kdbus/bus.h
> index 5bea5ef..03e4a54 100644
> --- a/ipc/kdbus/bus.h
> +++ b/ipc/kdbus/bus.h
> @@ -53,6 +53,7 @@ struct kdbus_user;
>   * @notify_list:	List of pending kernel-generated messages
>   * @notify_lock:	Notification list lock
>   * @notify_flush_lock:	Notification flushing lock
> + * @security:		LSM security blob
>   */
>  struct kdbus_bus {
>  	struct kdbus_node node;
> @@ -81,6 +82,8 @@ struct kdbus_bus {
>  	struct list_head notify_list;
>  	spinlock_t notify_lock;
>  	struct mutex notify_flush_lock;
> +
> +	void *security;

Security blobs are usually inside #ifdef CONFIG_SECURITY

>  };
>  
>  struct kdbus_bus *kdbus_bus_ref(struct kdbus_bus *bus);
> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index 9993753..b85cdc7 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -31,6 +31,7 @@
>  #include <linux/slab.h>
>  #include <linux/syscalls.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "connection.h"
> @@ -73,6 +74,8 @@ static struct kdbus_conn *kdbus_conn_new(struct 
> kdbus_ep *ep, bool privileged,
>  	bool is_activator;
>  	bool is_monitor;
>  	struct kvec kvec;
> +	u32 sid, len;
> +	char *label;
>  	int ret;
>  
>  	struct {
> @@ -222,6 +225,14 @@ static struct kdbus_conn *kdbus_conn_new(struct 
> kdbus_ep *ep, bool privileged,
>  		}
>  	}
>  
> +	security_task_getsecid(current, &sid);
> +	security_secid_to_secctx(sid, &label, &len);
> +	ret = security_kdbus_connect(conn, label, len);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit_unref;
> +	}
> +
>  	if (atomic_inc_return(&conn->user->connections) > 
> KDBUS_USER_MAX_CONN) {
>  		/* decremented by destructor as conn->user is valid 
> */
>  		ret = -EMFILE;
> @@ -276,6 +287,7 @@ static void __kdbus_conn_free(struct kref *kref)
>  	kdbus_pool_free(conn->pool);
>  	kdbus_ep_unref(conn->ep);
>  	put_cred(conn->cred);
> +	security_kdbus_conn_free(conn);
>  	kfree(conn->description);
>  	kfree(conn->quota);
>  	kfree(conn);
> @@ -1107,6 +1119,12 @@ static int kdbus_conn_reply(struct kdbus_conn 
> *src, struct kdbus_kmsg *kmsg)
>  	if (ret < 0)
>  		goto exit;
>  
> +	ret = security_kdbus_talk(src, dst);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit;
> +	}
> +
>  	mutex_lock(&dst->lock);
>  	reply = kdbus_reply_find(src, dst, kmsg->msg.cookie_reply);
>  	if (reply) {
> @@ -1187,6 +1205,12 @@ static struct kdbus_reply 
> *kdbus_conn_call(struct kdbus_conn *src,
>  	if (ret < 0)
>  		goto exit;
>  
> +	ret = security_kdbus_talk(src, dst);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit;
> +	}
> +
>  	if (!kdbus_conn_policy_talk(src, current_cred(), dst)) {
>  		ret = -EPERM;
>  		goto exit;
> @@ -1248,6 +1272,12 @@ static int kdbus_conn_unicast(struct 
> kdbus_conn *src, struct kdbus_kmsg *kmsg)
>  	if (ret < 0)
>  		goto exit;
>  
> +	ret = security_kdbus_talk(src, dst);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit;
> +	}
> +
>  	if (is_signal) {
>  		/* like broadcasts we eavesdrop even if the msg is 
> dropped */
>  		kdbus_bus_eavesdrop(bus, src, kmsg);
> @@ -1639,6 +1669,12 @@ struct kdbus_conn *kdbus_cmd_hello(struct 
> kdbus_ep *ep, bool privileged,
>  		if (ret < 0)
>  			goto exit;
>  
> +		ret = security_kdbus_ep_setpolicy(c->ep->bus);
> +		if (ret) {
> +			ret = -EPERM;
> +			goto exit;
> +		}
> +
>  		ret = kdbus_policy_set(&c->ep->bus->policy_db, 
> args.items,
>  				       args.items_size, 1,
>  				      
>  kdbus_conn_is_policy_holder(c), c);
> @@ -1732,6 +1768,10 @@ int kdbus_cmd_conn_info(struct kdbus_conn 
> *conn, void __user *argp)
>  	if (ret != 0)
>  		return ret;
>  
> +	ret = security_kdbus_conn_info(conn);
> +	if (ret)
> +		return -EPERM;
> +
>  	/* registry must be held throughout lookup *and* collecting 
> data */
>  	down_read(&bus->name_registry->rwlock);
>  
> @@ -1905,6 +1945,12 @@ int kdbus_cmd_update(struct kdbus_conn *conn, 
> void __user *argp)
>  	/* now that we verified the input, update the connection */
>  
>  	if (item_policy) {
> +		ret = security_kdbus_ep_setpolicy(conn->ep->bus);
> +		if (ret) {
> +			ret = -EPERM;
> +			goto exit;
> +		}
> +
>  		ret = kdbus_policy_set(&conn->ep->bus->policy_db, 
> cmd->items,
>  				       KDBUS_ITEMS_SIZE(cmd, items),
>  				       1, true, conn);
> @@ -1948,6 +1994,10 @@ int kdbus_cmd_send(struct kdbus_conn *conn, 
> struct file *f, void __user *argp)
>  		.argc = ARRAY_SIZE(argv),
>  	};
>  
> +	ret = security_kdbus_send(conn, conn->ep->bus);
> +	if (ret)
> +		return -EPERM;
> +
>  	if (!kdbus_conn_is_ordinary(conn))
>  		return -EOPNOTSUPP;
>  
> @@ -2044,6 +2094,10 @@ int kdbus_cmd_recv(struct kdbus_conn *conn, 
> void __user *argp)
>  		.argc = ARRAY_SIZE(argv),
>  	};
>  
> +	ret = security_kdbus_recv(conn, conn->ep->bus);
> +	if (ret)
> +		return -EPERM;
> +
>  	if (!kdbus_conn_is_ordinary(conn) &&
>  	    !kdbus_conn_is_monitor(conn) &&
>  	    !kdbus_conn_is_activator(conn))
> diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
> index d1ffe90..1f91d39 100644
> --- a/ipc/kdbus/connection.h
> +++ b/ipc/kdbus/connection.h
> @@ -19,6 +19,7 @@
>  #include <linux/kref.h>
>  #include <linux/lockdep.h>
>  #include <linux/path.h>
> +#include <uapi/linux/kdbus.h>
>  
>  #include "limits.h"
>  #include "metadata.h"
> @@ -73,6 +74,7 @@ struct kdbus_kmsg;
>   * @names_queue_list:	Well-known names this connection waits for
>   * @privileged:		Whether this connection is 
> privileged on the bus
>   * @faked_meta:		Whether the metadata was faked on 
> HELLO
> + * @security:		LSM security blob
>   */
>  struct kdbus_conn {
>  	struct kref kref;
> @@ -113,6 +115,8 @@ struct kdbus_conn {
>  
>  	bool privileged:1;
>  	bool faked_meta:1;
> +
> +	void *security;
>  };
>  
>  struct kdbus_conn *kdbus_conn_ref(struct kdbus_conn *conn);
> diff --git a/ipc/kdbus/domain.c b/ipc/kdbus/domain.c
> index ac9f760..da9cdab 100644
> --- a/ipc/kdbus/domain.c
> +++ b/ipc/kdbus/domain.c
> @@ -20,6 +20,7 @@
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "domain.h"
> @@ -73,6 +74,7 @@ static void kdbus_domain_free(struct kdbus_node 
> *node)
>  	put_user_ns(domain->user_namespace);
>  	ida_destroy(&domain->user_ida);
>  	idr_destroy(&domain->user_idr);
> +	security_kdbus_domain_free(domain);
>  	kfree(domain);
>  }
>  
> @@ -104,6 +106,10 @@ struct kdbus_domain *kdbus_domain_new(unsigned 
> int access)
>  	idr_init(&d->user_idr);
>  	ida_init(&d->user_ida);
>  
> +	ret = security_kdbus_domain_alloc(d);
> +	if (ret)
> +		return ERR_PTR(-EPERM);
> +
>  	/* Pin user namespace so we can guarantee domain-unique bus 
> * names. */
>  	d->user_namespace = get_user_ns(current_user_ns());
>  
> @@ -116,6 +122,7 @@ struct kdbus_domain *kdbus_domain_new(unsigned 
> int access)
>  exit_unref:
>  	kdbus_node_deactivate(&d->node);
>  	kdbus_node_unref(&d->node);
> +	security_kdbus_domain_free(d);
>  	return ERR_PTR(ret);
>  }
>  
> @@ -264,7 +271,7 @@ static void __kdbus_user_free(struct kref *kref)
>  	if (uid_valid(user->uid))
>  		idr_remove(&user->domain->user_idr, __kuid_val(user
> ->uid));
>  	mutex_unlock(&user->domain->lock);
> -
> +	security_kdbus_domain_free(user->domain);
>  	kdbus_domain_unref(user->domain);
>  	kfree(user);
>  }
> diff --git a/ipc/kdbus/domain.h b/ipc/kdbus/domain.h
> index 447a2bd..3db06d8 100644
> --- a/ipc/kdbus/domain.h
> +++ b/ipc/kdbus/domain.h
> @@ -31,6 +31,7 @@
>   * @user_ida:		Set of all users to compute small indices
>   * @user_namespace:	User namespace, pinned at creation time
>   * @dentry:		Root dentry of VFS mount (don't use outside 
> of kdbusfs)
> + * @security:		LSM security blob
>   */
>  struct kdbus_domain {
>  	struct kdbus_node node;
> @@ -40,6 +41,7 @@ struct kdbus_domain {
>  	struct ida user_ida;
>  	struct user_namespace *user_namespace;
>  	struct dentry *dentry;
> +	void *security;
>  };
>  
>  /**
> diff --git a/ipc/kdbus/endpoint.c b/ipc/kdbus/endpoint.c
> index 9a95a5e..380228f 100644
> --- a/ipc/kdbus/endpoint.c
> +++ b/ipc/kdbus/endpoint.c
> @@ -21,6 +21,7 @@
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "connection.h"
> @@ -122,6 +123,12 @@ struct kdbus_ep *kdbus_ep_new(struct kdbus_bus 
> *bus, const char *name,
>  	kdbus_policy_db_init(&e->policy_db);
>  	e->bus = kdbus_bus_ref(bus);
>  
> +	ret = security_kdbus_ep_create(bus);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit_unref;
> +	}
> +
>  	ret = kdbus_node_link(&e->node, &bus->node, name);
>  	if (ret < 0)
>  		goto exit_unref;
> @@ -265,6 +272,10 @@ int kdbus_cmd_ep_update(struct kdbus_ep *ep, 
> void __user *argp)
>  		.argc = ARRAY_SIZE(argv),
>  	};
>  
> +	ret = security_kdbus_ep_setpolicy(ep->bus);
> +	if (ret)
> +		return -EPERM;
> +
>  	ret = kdbus_args_parse(&args, argp, &cmd);
>  	if (ret != 0)
>  		return ret;
> diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
> index d77ee08..dd20bea 100644
> --- a/ipc/kdbus/names.c
> +++ b/ipc/kdbus/names.c
> @@ -24,6 +24,7 @@
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "connection.h"
> @@ -503,6 +504,12 @@ int kdbus_cmd_name_acquire(struct kdbus_conn 
> *conn, void __user *argp)
>  		goto exit;
>  	}
>  
> +	ret = security_kdbus_name_acquire(conn, item_name);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit;
> +	}
> +
>  	/*
>  	 * Do atomic_inc_return here to reserve our slot, then 
> decrement
>  	 * it before returning.
> @@ -724,6 +731,10 @@ int kdbus_cmd_list(struct kdbus_conn *conn, void 
> __user *argp)
>  	if (ret != 0)
>  		return ret;
>  
> +	ret = security_kdbus_name_list(conn->ep->bus);
> +	if (ret)
> +		return -EPERM;
> +
>  	/* lock order: domain -> bus -> ep -> names -> conn */
>  	down_read(&reg->rwlock);
>  	down_read(&conn->ep->bus->conn_rwlock);
> diff --git a/ipc/kdbus/queue.c b/ipc/kdbus/queue.c
> index 25bb3ad..9872fb4 100644
> --- a/ipc/kdbus/queue.c
> +++ b/ipc/kdbus/queue.c
> @@ -28,6 +28,7 @@
>  #include <linux/slab.h>
>  #include <linux/syscalls.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "util.h"
>  #include "domain.h"
> @@ -514,12 +515,17 @@ int kdbus_queue_entry_install(struct 
> kdbus_queue_entry *entry,
>  
>  		for (i = 0; i < res->fds_count; i++) {
>  			if (install_fds) {
> -				fds[i] = 
> get_unused_fd_flags(O_CLOEXEC);
> -				if (fds[i] >= 0)
> -					fd_install(fds[i],
> -						   get_file(res
> ->fds[i]));
> -				else
> +				if (security_file_receive(res
> ->fds[i])) {
> +					fds[i] = -1;
>  					incomplete_fds = true;
> +				} else {
> +					fds[i] = 
> get_unused_fd_flags(O_CLOEXEC);
> +					if (fds[i] >= 0)
> +						fd_install(fds[i],
> +							get_file(res
> ->fds[i]));
> +					else
> +						incomplete_fds = 
> true;
> +				}
>  			} else {
>  				fds[i] = -1;
>  			}
> @@ -557,13 +563,17 @@ int kdbus_queue_entry_install(struct 
> kdbus_queue_entry *entry,
>  		m.fd = -1;
>  
>  		if (install_fds) {
> -			m.fd = get_unused_fd_flags(O_CLOEXEC);
> -			if (m.fd < 0) {
> -				m.fd = -1;
> +			if (security_file_receive(d->memfd.file)) {
>  				incomplete_fds = true;
>  			} else {
> -				fd_install(m.fd,
> -					   get_file(d->memfd.file));
> +				m.fd = 
> get_unused_fd_flags(O_CLOEXEC);
> +				if (m.fd < 0) {
> +					m.fd = -1;
> +					incomplete_fds = true;
> +				} else {
> +					fd_install(m.fd,
> +						get_file(d
> ->memfd.file));
> +				}
>  			}
>  		}
>  
-- 
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics




^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
  2015-07-08 10:25 ` [RFC 5/8] kdbus: use LSM hooks in kdbus code Paul Osmialowski
  2015-07-08 11:06   ` Lukasz Pawelczyk
@ 2015-07-08 11:09   ` Lukasz Pawelczyk
  2015-07-08 12:12     ` Paul Osmialowski
  2015-07-08 13:37   ` Stephen Smalley
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 30+ messages in thread
From: Lukasz Pawelczyk @ 2015-07-08 11:09 UTC (permalink / raw)
  To: Paul Osmialowski, Paul Moore, James Morris, Casey Schaufler,
	Serge E. Hallyn, Kees Cook, Tetsuo Handa, Stephen Smalley,
	Neil Brown, Mark Rustad, Greg Kroah-Hartman, Daniel Mack,
	David Herrmann, Djalal Harouni, Shuah Khan, Al Viro,
	linux-security-module, linux-kernel, linux-api
  Cc: Karol Lewandowski, Lukasz Skalski

On śro, 2015-07-08 at 12:25 +0200, Paul Osmialowski wrote:
> Originates from:
> 
> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd
> -v212)
> commit: aa0885489d19be92fa41c6f0a71df28763228a40
> 
> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
>  ipc/kdbus/bus.c        | 12 ++++++++++-
>  ipc/kdbus/bus.h        |  3 +++
>  ipc/kdbus/connection.c | 54 
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  ipc/kdbus/connection.h |  4 ++++
>  ipc/kdbus/domain.c     |  9 ++++++++-
>  ipc/kdbus/domain.h     |  2 ++
>  ipc/kdbus/endpoint.c   | 11 ++++++++++
>  ipc/kdbus/names.c      | 11 ++++++++++
>  ipc/kdbus/queue.c      | 30 ++++++++++++++++++----------
>  9 files changed, 124 insertions(+), 12 deletions(-)
> 
> diff --git a/ipc/kdbus/bus.c b/ipc/kdbus/bus.c
> index bbdf0f2..9894895 100644
> --- a/ipc/kdbus/bus.c
> +++ b/ipc/kdbus/bus.c
> @@ -22,6 +22,7 @@
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "notify.h"
> @@ -51,6 +52,7 @@ static void kdbus_bus_free(struct kdbus_node *node)
>  	kdbus_domain_unref(bus->domain);
>  	kdbus_policy_db_clear(&bus->policy_db);
>  	kdbus_meta_proc_unref(bus->creator_meta);
> +	security_kdbus_bus_free(bus);
>  	kfree(bus);
>  }
>  
> @@ -161,6 +163,12 @@ static struct kdbus_bus *kdbus_bus_new(struct 
> kdbus_domain *domain,
>  		goto exit_unref;
>  	}
>  
> +	ret = security_kdbus_bus_alloc(b);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit_unref;
> +	}

Also, why aren't the error codes passed from LSM? LSM can return
anything, from EINVAL, to ENOMEM. Returning EPERM can be misleading.

> +
>  	/*
>  	 * Bus-limits of the creator are accounted on its real UID, 
> just like
>  	 * all other per-user limits.
> @@ -169,11 +177,13 @@ static struct kdbus_bus *kdbus_bus_new(struct 
> kdbus_domain *domain,
>  	if (IS_ERR(b->creator)) {
>  		ret = PTR_ERR(b->creator);
>  		b->creator = NULL;
> -		goto exit_unref;
> +		goto exit_free_security;
>  	}
>  
>  	return b;
>  
> +exit_free_security:
> +	security_kdbus_bus_free(b);
>  exit_unref:
>  	kdbus_node_deactivate(&b->node);
>  	kdbus_node_unref(&b->node);
> diff --git a/ipc/kdbus/bus.h b/ipc/kdbus/bus.h
> index 5bea5ef..03e4a54 100644
> --- a/ipc/kdbus/bus.h
> +++ b/ipc/kdbus/bus.h
> @@ -53,6 +53,7 @@ struct kdbus_user;
>   * @notify_list:	List of pending kernel-generated messages
>   * @notify_lock:	Notification list lock
>   * @notify_flush_lock:	Notification flushing lock
> + * @security:		LSM security blob
>   */
>  struct kdbus_bus {
>  	struct kdbus_node node;
> @@ -81,6 +82,8 @@ struct kdbus_bus {
>  	struct list_head notify_list;
>  	spinlock_t notify_lock;
>  	struct mutex notify_flush_lock;
> +
> +	void *security;
>  };
>  
>  struct kdbus_bus *kdbus_bus_ref(struct kdbus_bus *bus);
> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index 9993753..b85cdc7 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -31,6 +31,7 @@
>  #include <linux/slab.h>
>  #include <linux/syscalls.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "connection.h"
> @@ -73,6 +74,8 @@ static struct kdbus_conn *kdbus_conn_new(struct 
> kdbus_ep *ep, bool privileged,
>  	bool is_activator;
>  	bool is_monitor;
>  	struct kvec kvec;
> +	u32 sid, len;
> +	char *label;
>  	int ret;
>  
>  	struct {
> @@ -222,6 +225,14 @@ static struct kdbus_conn *kdbus_conn_new(struct 
> kdbus_ep *ep, bool privileged,
>  		}
>  	}
>  
> +	security_task_getsecid(current, &sid);
> +	security_secid_to_secctx(sid, &label, &len);
> +	ret = security_kdbus_connect(conn, label, len);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit_unref;
> +	}
> +
>  	if (atomic_inc_return(&conn->user->connections) > 
> KDBUS_USER_MAX_CONN) {
>  		/* decremented by destructor as conn->user is valid 
> */
>  		ret = -EMFILE;
> @@ -276,6 +287,7 @@ static void __kdbus_conn_free(struct kref *kref)
>  	kdbus_pool_free(conn->pool);
>  	kdbus_ep_unref(conn->ep);
>  	put_cred(conn->cred);
> +	security_kdbus_conn_free(conn);
>  	kfree(conn->description);
>  	kfree(conn->quota);
>  	kfree(conn);
> @@ -1107,6 +1119,12 @@ static int kdbus_conn_reply(struct kdbus_conn 
> *src, struct kdbus_kmsg *kmsg)
>  	if (ret < 0)
>  		goto exit;
>  
> +	ret = security_kdbus_talk(src, dst);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit;
> +	}
> +
>  	mutex_lock(&dst->lock);
>  	reply = kdbus_reply_find(src, dst, kmsg->msg.cookie_reply);
>  	if (reply) {
> @@ -1187,6 +1205,12 @@ static struct kdbus_reply 
> *kdbus_conn_call(struct kdbus_conn *src,
>  	if (ret < 0)
>  		goto exit;
>  
> +	ret = security_kdbus_talk(src, dst);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit;
> +	}
> +
>  	if (!kdbus_conn_policy_talk(src, current_cred(), dst)) {
>  		ret = -EPERM;
>  		goto exit;
> @@ -1248,6 +1272,12 @@ static int kdbus_conn_unicast(struct 
> kdbus_conn *src, struct kdbus_kmsg *kmsg)
>  	if (ret < 0)
>  		goto exit;
>  
> +	ret = security_kdbus_talk(src, dst);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit;
> +	}
> +
>  	if (is_signal) {
>  		/* like broadcasts we eavesdrop even if the msg is 
> dropped */
>  		kdbus_bus_eavesdrop(bus, src, kmsg);
> @@ -1639,6 +1669,12 @@ struct kdbus_conn *kdbus_cmd_hello(struct 
> kdbus_ep *ep, bool privileged,
>  		if (ret < 0)
>  			goto exit;
>  
> +		ret = security_kdbus_ep_setpolicy(c->ep->bus);
> +		if (ret) {
> +			ret = -EPERM;
> +			goto exit;
> +		}
> +
>  		ret = kdbus_policy_set(&c->ep->bus->policy_db, 
> args.items,
>  				       args.items_size, 1,
>  				      
>  kdbus_conn_is_policy_holder(c), c);
> @@ -1732,6 +1768,10 @@ int kdbus_cmd_conn_info(struct kdbus_conn 
> *conn, void __user *argp)
>  	if (ret != 0)
>  		return ret;
>  
> +	ret = security_kdbus_conn_info(conn);
> +	if (ret)
> +		return -EPERM;
> +
>  	/* registry must be held throughout lookup *and* collecting 
> data */
>  	down_read(&bus->name_registry->rwlock);
>  
> @@ -1905,6 +1945,12 @@ int kdbus_cmd_update(struct kdbus_conn *conn, 
> void __user *argp)
>  	/* now that we verified the input, update the connection */
>  
>  	if (item_policy) {
> +		ret = security_kdbus_ep_setpolicy(conn->ep->bus);
> +		if (ret) {
> +			ret = -EPERM;
> +			goto exit;
> +		}
> +
>  		ret = kdbus_policy_set(&conn->ep->bus->policy_db, 
> cmd->items,
>  				       KDBUS_ITEMS_SIZE(cmd, items),
>  				       1, true, conn);
> @@ -1948,6 +1994,10 @@ int kdbus_cmd_send(struct kdbus_conn *conn, 
> struct file *f, void __user *argp)
>  		.argc = ARRAY_SIZE(argv),
>  	};
>  
> +	ret = security_kdbus_send(conn, conn->ep->bus);
> +	if (ret)
> +		return -EPERM;
> +
>  	if (!kdbus_conn_is_ordinary(conn))
>  		return -EOPNOTSUPP;
>  
> @@ -2044,6 +2094,10 @@ int kdbus_cmd_recv(struct kdbus_conn *conn, 
> void __user *argp)
>  		.argc = ARRAY_SIZE(argv),
>  	};
>  
> +	ret = security_kdbus_recv(conn, conn->ep->bus);
> +	if (ret)
> +		return -EPERM;
> +
>  	if (!kdbus_conn_is_ordinary(conn) &&
>  	    !kdbus_conn_is_monitor(conn) &&
>  	    !kdbus_conn_is_activator(conn))
> diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
> index d1ffe90..1f91d39 100644
> --- a/ipc/kdbus/connection.h
> +++ b/ipc/kdbus/connection.h
> @@ -19,6 +19,7 @@
>  #include <linux/kref.h>
>  #include <linux/lockdep.h>
>  #include <linux/path.h>
> +#include <uapi/linux/kdbus.h>
>  
>  #include "limits.h"
>  #include "metadata.h"
> @@ -73,6 +74,7 @@ struct kdbus_kmsg;
>   * @names_queue_list:	Well-known names this connection waits for
>   * @privileged:		Whether this connection is 
> privileged on the bus
>   * @faked_meta:		Whether the metadata was faked on 
> HELLO
> + * @security:		LSM security blob
>   */
>  struct kdbus_conn {
>  	struct kref kref;
> @@ -113,6 +115,8 @@ struct kdbus_conn {
>  
>  	bool privileged:1;
>  	bool faked_meta:1;
> +
> +	void *security;
>  };
>  
>  struct kdbus_conn *kdbus_conn_ref(struct kdbus_conn *conn);
> diff --git a/ipc/kdbus/domain.c b/ipc/kdbus/domain.c
> index ac9f760..da9cdab 100644
> --- a/ipc/kdbus/domain.c
> +++ b/ipc/kdbus/domain.c
> @@ -20,6 +20,7 @@
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "domain.h"
> @@ -73,6 +74,7 @@ static void kdbus_domain_free(struct kdbus_node 
> *node)
>  	put_user_ns(domain->user_namespace);
>  	ida_destroy(&domain->user_ida);
>  	idr_destroy(&domain->user_idr);
> +	security_kdbus_domain_free(domain);
>  	kfree(domain);
>  }
>  
> @@ -104,6 +106,10 @@ struct kdbus_domain *kdbus_domain_new(unsigned 
> int access)
>  	idr_init(&d->user_idr);
>  	ida_init(&d->user_ida);
>  
> +	ret = security_kdbus_domain_alloc(d);
> +	if (ret)
> +		return ERR_PTR(-EPERM);
> +
>  	/* Pin user namespace so we can guarantee domain-unique bus 
> * names. */
>  	d->user_namespace = get_user_ns(current_user_ns());
>  
> @@ -116,6 +122,7 @@ struct kdbus_domain *kdbus_domain_new(unsigned 
> int access)
>  exit_unref:
>  	kdbus_node_deactivate(&d->node);
>  	kdbus_node_unref(&d->node);
> +	security_kdbus_domain_free(d);
>  	return ERR_PTR(ret);
>  }
>  
> @@ -264,7 +271,7 @@ static void __kdbus_user_free(struct kref *kref)
>  	if (uid_valid(user->uid))
>  		idr_remove(&user->domain->user_idr, __kuid_val(user
> ->uid));
>  	mutex_unlock(&user->domain->lock);
> -
> +	security_kdbus_domain_free(user->domain);
>  	kdbus_domain_unref(user->domain);
>  	kfree(user);
>  }
> diff --git a/ipc/kdbus/domain.h b/ipc/kdbus/domain.h
> index 447a2bd..3db06d8 100644
> --- a/ipc/kdbus/domain.h
> +++ b/ipc/kdbus/domain.h
> @@ -31,6 +31,7 @@
>   * @user_ida:		Set of all users to compute small indices
>   * @user_namespace:	User namespace, pinned at creation time
>   * @dentry:		Root dentry of VFS mount (don't use outside 
> of kdbusfs)
> + * @security:		LSM security blob
>   */
>  struct kdbus_domain {
>  	struct kdbus_node node;
> @@ -40,6 +41,7 @@ struct kdbus_domain {
>  	struct ida user_ida;
>  	struct user_namespace *user_namespace;
>  	struct dentry *dentry;
> +	void *security;
>  };
>  
>  /**
> diff --git a/ipc/kdbus/endpoint.c b/ipc/kdbus/endpoint.c
> index 9a95a5e..380228f 100644
> --- a/ipc/kdbus/endpoint.c
> +++ b/ipc/kdbus/endpoint.c
> @@ -21,6 +21,7 @@
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "connection.h"
> @@ -122,6 +123,12 @@ struct kdbus_ep *kdbus_ep_new(struct kdbus_bus 
> *bus, const char *name,
>  	kdbus_policy_db_init(&e->policy_db);
>  	e->bus = kdbus_bus_ref(bus);
>  
> +	ret = security_kdbus_ep_create(bus);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit_unref;
> +	}
> +
>  	ret = kdbus_node_link(&e->node, &bus->node, name);
>  	if (ret < 0)
>  		goto exit_unref;
> @@ -265,6 +272,10 @@ int kdbus_cmd_ep_update(struct kdbus_ep *ep, 
> void __user *argp)
>  		.argc = ARRAY_SIZE(argv),
>  	};
>  
> +	ret = security_kdbus_ep_setpolicy(ep->bus);
> +	if (ret)
> +		return -EPERM;
> +
>  	ret = kdbus_args_parse(&args, argp, &cmd);
>  	if (ret != 0)
>  		return ret;
> diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
> index d77ee08..dd20bea 100644
> --- a/ipc/kdbus/names.c
> +++ b/ipc/kdbus/names.c
> @@ -24,6 +24,7 @@
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "connection.h"
> @@ -503,6 +504,12 @@ int kdbus_cmd_name_acquire(struct kdbus_conn 
> *conn, void __user *argp)
>  		goto exit;
>  	}
>  
> +	ret = security_kdbus_name_acquire(conn, item_name);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit;
> +	}
> +
>  	/*
>  	 * Do atomic_inc_return here to reserve our slot, then 
> decrement
>  	 * it before returning.
> @@ -724,6 +731,10 @@ int kdbus_cmd_list(struct kdbus_conn *conn, void 
> __user *argp)
>  	if (ret != 0)
>  		return ret;
>  
> +	ret = security_kdbus_name_list(conn->ep->bus);
> +	if (ret)
> +		return -EPERM;
> +
>  	/* lock order: domain -> bus -> ep -> names -> conn */
>  	down_read(&reg->rwlock);
>  	down_read(&conn->ep->bus->conn_rwlock);
> diff --git a/ipc/kdbus/queue.c b/ipc/kdbus/queue.c
> index 25bb3ad..9872fb4 100644
> --- a/ipc/kdbus/queue.c
> +++ b/ipc/kdbus/queue.c
> @@ -28,6 +28,7 @@
>  #include <linux/slab.h>
>  #include <linux/syscalls.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "util.h"
>  #include "domain.h"
> @@ -514,12 +515,17 @@ int kdbus_queue_entry_install(struct 
> kdbus_queue_entry *entry,
>  
>  		for (i = 0; i < res->fds_count; i++) {
>  			if (install_fds) {
> -				fds[i] = 
> get_unused_fd_flags(O_CLOEXEC);
> -				if (fds[i] >= 0)
> -					fd_install(fds[i],
> -						   get_file(res
> ->fds[i]));
> -				else
> +				if (security_file_receive(res
> ->fds[i])) {
> +					fds[i] = -1;
>  					incomplete_fds = true;
> +				} else {
> +					fds[i] = 
> get_unused_fd_flags(O_CLOEXEC);
> +					if (fds[i] >= 0)
> +						fd_install(fds[i],
> +							get_file(res
> ->fds[i]));
> +					else
> +						incomplete_fds = 
> true;
> +				}
>  			} else {
>  				fds[i] = -1;
>  			}
> @@ -557,13 +563,17 @@ int kdbus_queue_entry_install(struct 
> kdbus_queue_entry *entry,
>  		m.fd = -1;
>  
>  		if (install_fds) {
> -			m.fd = get_unused_fd_flags(O_CLOEXEC);
> -			if (m.fd < 0) {
> -				m.fd = -1;
> +			if (security_file_receive(d->memfd.file)) {
>  				incomplete_fds = true;
>  			} else {
> -				fd_install(m.fd,
> -					   get_file(d->memfd.file));
> +				m.fd = 
> get_unused_fd_flags(O_CLOEXEC);
> +				if (m.fd < 0) {
> +					m.fd = -1;
> +					incomplete_fds = true;
> +				} else {
> +					fd_install(m.fd,
> +						get_file(d
> ->memfd.file));
> +				}
>  			}
>  		}
>  
-- 
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics




^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
  2015-07-08 11:09   ` Lukasz Pawelczyk
@ 2015-07-08 12:12     ` Paul Osmialowski
  2015-07-09 10:55       ` Sergei Zviagintsev
  0 siblings, 1 reply; 30+ messages in thread
From: Paul Osmialowski @ 2015-07-08 12:12 UTC (permalink / raw)
  To: Lukasz Pawelczyk
  Cc: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api, Karol Lewandowski, Lukasz Skalski

[-- Attachment #1: Type: TEXT/PLAIN, Size: 299 bytes --]

You're right about it. To be fixed. Thanks for spotting this.

On Wed, 8 Jul 2015, Lukasz Pawelczyk wrote:

>> +		ret = -EPERM;
>> +		goto exit_unref;
>> +	}
>
> Also, why aren't the error codes passed from LSM? LSM can return
> anything, from EINVAL, to ENOMEM. Returning EPERM can be misleading.
>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/x-diff; name=0005-kdbus-use-LSM-hooks-in-kdbus-code.patch, Size: 13167 bytes --]

From 9c37667302f104e0f412ba2727c4d3df81fb59da Mon Sep 17 00:00:00 2001
From: Paul Osmialowski <p.osmialowsk@samsung.com>
Date: Tue, 26 May 2015 12:00:33 +0200
Subject: [RFC 5/8] kdbus: use LSM hooks in kdbus code

Originates from:

https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)
commit: aa0885489d19be92fa41c6f0a71df28763228a40

Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
---
 ipc/kdbus/bus.c        | 11 ++++++++++-
 ipc/kdbus/bus.h        |  5 +++++
 ipc/kdbus/connection.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 ipc/kdbus/connection.h |  6 ++++++
 ipc/kdbus/domain.c     |  9 ++++++++-
 ipc/kdbus/domain.h     |  4 ++++
 ipc/kdbus/endpoint.c   | 10 ++++++++++
 ipc/kdbus/names.c      | 10 ++++++++++
 ipc/kdbus/queue.c      | 30 ++++++++++++++++++++----------
 9 files changed, 121 insertions(+), 12 deletions(-)

diff --git a/ipc/kdbus/bus.c b/ipc/kdbus/bus.c
index bbdf0f2..be7ed81 100644
--- a/ipc/kdbus/bus.c
+++ b/ipc/kdbus/bus.c
@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/uio.h>
+#include <linux/security.h>
 
 #include "bus.h"
 #include "notify.h"
@@ -51,6 +52,7 @@ static void kdbus_bus_free(struct kdbus_node *node)
 	kdbus_domain_unref(bus->domain);
 	kdbus_policy_db_clear(&bus->policy_db);
 	kdbus_meta_proc_unref(bus->creator_meta);
+	security_kdbus_bus_free(bus);
 	kfree(bus);
 }
 
@@ -161,6 +163,11 @@ static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain *domain,
 		goto exit_unref;
 	}
 
+	ret = security_kdbus_bus_alloc(b);
+	if (ret) {
+		goto exit_unref;
+	}
+
 	/*
 	 * Bus-limits of the creator are accounted on its real UID, just like
 	 * all other per-user limits.
@@ -169,11 +176,13 @@ static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain *domain,
 	if (IS_ERR(b->creator)) {
 		ret = PTR_ERR(b->creator);
 		b->creator = NULL;
-		goto exit_unref;
+		goto exit_free_security;
 	}
 
 	return b;
 
+exit_free_security:
+	security_kdbus_bus_free(b);
 exit_unref:
 	kdbus_node_deactivate(&b->node);
 	kdbus_node_unref(&b->node);
diff --git a/ipc/kdbus/bus.h b/ipc/kdbus/bus.h
index 5bea5ef..521e692 100644
--- a/ipc/kdbus/bus.h
+++ b/ipc/kdbus/bus.h
@@ -53,6 +53,7 @@ struct kdbus_user;
  * @notify_list:	List of pending kernel-generated messages
  * @notify_lock:	Notification list lock
  * @notify_flush_lock:	Notification flushing lock
+ * @security:		LSM security blob
  */
 struct kdbus_bus {
 	struct kdbus_node node;
@@ -81,6 +82,10 @@ struct kdbus_bus {
 	struct list_head notify_list;
 	spinlock_t notify_lock;
 	struct mutex notify_flush_lock;
+
+#ifdef CONFIG_SECURITY
+	void *security;
+#endif
 };
 
 struct kdbus_bus *kdbus_bus_ref(struct kdbus_bus *bus);
diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index 9993753..4b9f0b6 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -31,6 +31,7 @@
 #include <linux/slab.h>
 #include <linux/syscalls.h>
 #include <linux/uio.h>
+#include <linux/security.h>
 
 #include "bus.h"
 #include "connection.h"
@@ -73,6 +74,8 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
 	bool is_activator;
 	bool is_monitor;
 	struct kvec kvec;
+	u32 sid, len;
+	char *label;
 	int ret;
 
 	struct {
@@ -222,6 +225,13 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
 		}
 	}
 
+	security_task_getsecid(current, &sid);
+	security_secid_to_secctx(sid, &label, &len);
+	ret = security_kdbus_connect(conn, label, len);
+	if (ret) {
+		goto exit_unref;
+	}
+
 	if (atomic_inc_return(&conn->user->connections) > KDBUS_USER_MAX_CONN) {
 		/* decremented by destructor as conn->user is valid */
 		ret = -EMFILE;
@@ -276,6 +286,7 @@ static void __kdbus_conn_free(struct kref *kref)
 	kdbus_pool_free(conn->pool);
 	kdbus_ep_unref(conn->ep);
 	put_cred(conn->cred);
+	security_kdbus_conn_free(conn);
 	kfree(conn->description);
 	kfree(conn->quota);
 	kfree(conn);
@@ -1107,6 +1118,11 @@ static int kdbus_conn_reply(struct kdbus_conn *src, struct kdbus_kmsg *kmsg)
 	if (ret < 0)
 		goto exit;
 
+	ret = security_kdbus_talk(src, dst);
+	if (ret) {
+		goto exit;
+	}
+
 	mutex_lock(&dst->lock);
 	reply = kdbus_reply_find(src, dst, kmsg->msg.cookie_reply);
 	if (reply) {
@@ -1187,6 +1203,11 @@ static struct kdbus_reply *kdbus_conn_call(struct kdbus_conn *src,
 	if (ret < 0)
 		goto exit;
 
+	ret = security_kdbus_talk(src, dst);
+	if (ret) {
+		goto exit;
+	}
+
 	if (!kdbus_conn_policy_talk(src, current_cred(), dst)) {
 		ret = -EPERM;
 		goto exit;
@@ -1248,6 +1269,11 @@ static int kdbus_conn_unicast(struct kdbus_conn *src, struct kdbus_kmsg *kmsg)
 	if (ret < 0)
 		goto exit;
 
+	ret = security_kdbus_talk(src, dst);
+	if (ret) {
+		goto exit;
+	}
+
 	if (is_signal) {
 		/* like broadcasts we eavesdrop even if the msg is dropped */
 		kdbus_bus_eavesdrop(bus, src, kmsg);
@@ -1639,6 +1665,11 @@ struct kdbus_conn *kdbus_cmd_hello(struct kdbus_ep *ep, bool privileged,
 		if (ret < 0)
 			goto exit;
 
+		ret = security_kdbus_ep_setpolicy(c->ep->bus);
+		if (ret) {
+			goto exit;
+		}
+
 		ret = kdbus_policy_set(&c->ep->bus->policy_db, args.items,
 				       args.items_size, 1,
 				       kdbus_conn_is_policy_holder(c), c);
@@ -1732,6 +1763,10 @@ int kdbus_cmd_conn_info(struct kdbus_conn *conn, void __user *argp)
 	if (ret != 0)
 		return ret;
 
+	ret = security_kdbus_conn_info(conn);
+	if (ret)
+		return ret;
+
 	/* registry must be held throughout lookup *and* collecting data */
 	down_read(&bus->name_registry->rwlock);
 
@@ -1905,6 +1940,11 @@ int kdbus_cmd_update(struct kdbus_conn *conn, void __user *argp)
 	/* now that we verified the input, update the connection */
 
 	if (item_policy) {
+		ret = security_kdbus_ep_setpolicy(conn->ep->bus);
+		if (ret) {
+			goto exit;
+		}
+
 		ret = kdbus_policy_set(&conn->ep->bus->policy_db, cmd->items,
 				       KDBUS_ITEMS_SIZE(cmd, items),
 				       1, true, conn);
@@ -1948,6 +1988,10 @@ int kdbus_cmd_send(struct kdbus_conn *conn, struct file *f, void __user *argp)
 		.argc = ARRAY_SIZE(argv),
 	};
 
+	ret = security_kdbus_send(conn, conn->ep->bus);
+	if (ret)
+		return ret;
+
 	if (!kdbus_conn_is_ordinary(conn))
 		return -EOPNOTSUPP;
 
@@ -2044,6 +2088,10 @@ int kdbus_cmd_recv(struct kdbus_conn *conn, void __user *argp)
 		.argc = ARRAY_SIZE(argv),
 	};
 
+	ret = security_kdbus_recv(conn, conn->ep->bus);
+	if (ret)
+		return ret;
+
 	if (!kdbus_conn_is_ordinary(conn) &&
 	    !kdbus_conn_is_monitor(conn) &&
 	    !kdbus_conn_is_activator(conn))
diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
index d1ffe90..951f9b0 100644
--- a/ipc/kdbus/connection.h
+++ b/ipc/kdbus/connection.h
@@ -19,6 +19,7 @@
 #include <linux/kref.h>
 #include <linux/lockdep.h>
 #include <linux/path.h>
+#include <uapi/linux/kdbus.h>
 
 #include "limits.h"
 #include "metadata.h"
@@ -73,6 +74,7 @@ struct kdbus_kmsg;
  * @names_queue_list:	Well-known names this connection waits for
  * @privileged:		Whether this connection is privileged on the bus
  * @faked_meta:		Whether the metadata was faked on HELLO
+ * @security:		LSM security blob
  */
 struct kdbus_conn {
 	struct kref kref;
@@ -113,6 +115,10 @@ struct kdbus_conn {
 
 	bool privileged:1;
 	bool faked_meta:1;
+
+#ifdef CONFIG_SECURITY
+	void *security;
+#endif
 };
 
 struct kdbus_conn *kdbus_conn_ref(struct kdbus_conn *conn);
diff --git a/ipc/kdbus/domain.c b/ipc/kdbus/domain.c
index ac9f760..e79080f 100644
--- a/ipc/kdbus/domain.c
+++ b/ipc/kdbus/domain.c
@@ -20,6 +20,7 @@
 #include <linux/sizes.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
+#include <linux/security.h>
 
 #include "bus.h"
 #include "domain.h"
@@ -73,6 +74,7 @@ static void kdbus_domain_free(struct kdbus_node *node)
 	put_user_ns(domain->user_namespace);
 	ida_destroy(&domain->user_ida);
 	idr_destroy(&domain->user_idr);
+	security_kdbus_domain_free(domain);
 	kfree(domain);
 }
 
@@ -104,6 +106,10 @@ struct kdbus_domain *kdbus_domain_new(unsigned int access)
 	idr_init(&d->user_idr);
 	ida_init(&d->user_ida);
 
+	ret = security_kdbus_domain_alloc(d);
+	if (ret)
+		return ERR_PTR(ret);
+
 	/* Pin user namespace so we can guarantee domain-unique bus * names. */
 	d->user_namespace = get_user_ns(current_user_ns());
 
@@ -116,6 +122,7 @@ struct kdbus_domain *kdbus_domain_new(unsigned int access)
 exit_unref:
 	kdbus_node_deactivate(&d->node);
 	kdbus_node_unref(&d->node);
+	security_kdbus_domain_free(d);
 	return ERR_PTR(ret);
 }
 
@@ -264,7 +271,7 @@ static void __kdbus_user_free(struct kref *kref)
 	if (uid_valid(user->uid))
 		idr_remove(&user->domain->user_idr, __kuid_val(user->uid));
 	mutex_unlock(&user->domain->lock);
-
+	security_kdbus_domain_free(user->domain);
 	kdbus_domain_unref(user->domain);
 	kfree(user);
 }
diff --git a/ipc/kdbus/domain.h b/ipc/kdbus/domain.h
index 447a2bd..4a935aa 100644
--- a/ipc/kdbus/domain.h
+++ b/ipc/kdbus/domain.h
@@ -31,6 +31,7 @@
  * @user_ida:		Set of all users to compute small indices
  * @user_namespace:	User namespace, pinned at creation time
  * @dentry:		Root dentry of VFS mount (don't use outside of kdbusfs)
+ * @security:		LSM security blob
  */
 struct kdbus_domain {
 	struct kdbus_node node;
@@ -40,6 +41,9 @@ struct kdbus_domain {
 	struct ida user_ida;
 	struct user_namespace *user_namespace;
 	struct dentry *dentry;
+#ifdef CONFIG_SECURITY
+	void *security;
+#endif
 };
 
 /**
diff --git a/ipc/kdbus/endpoint.c b/ipc/kdbus/endpoint.c
index 9a95a5e..6ff74ee 100644
--- a/ipc/kdbus/endpoint.c
+++ b/ipc/kdbus/endpoint.c
@@ -21,6 +21,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/uio.h>
+#include <linux/security.h>
 
 #include "bus.h"
 #include "connection.h"
@@ -122,6 +123,11 @@ struct kdbus_ep *kdbus_ep_new(struct kdbus_bus *bus, const char *name,
 	kdbus_policy_db_init(&e->policy_db);
 	e->bus = kdbus_bus_ref(bus);
 
+	ret = security_kdbus_ep_create(bus);
+	if (ret) {
+		goto exit_unref;
+	}
+
 	ret = kdbus_node_link(&e->node, &bus->node, name);
 	if (ret < 0)
 		goto exit_unref;
@@ -265,6 +271,10 @@ int kdbus_cmd_ep_update(struct kdbus_ep *ep, void __user *argp)
 		.argc = ARRAY_SIZE(argv),
 	};
 
+	ret = security_kdbus_ep_setpolicy(ep->bus);
+	if (ret)
+		return ret;
+
 	ret = kdbus_args_parse(&args, argp, &cmd);
 	if (ret != 0)
 		return ret;
diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
index d77ee08..fcc1a49 100644
--- a/ipc/kdbus/names.c
+++ b/ipc/kdbus/names.c
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/uio.h>
+#include <linux/security.h>
 
 #include "bus.h"
 #include "connection.h"
@@ -503,6 +504,11 @@ int kdbus_cmd_name_acquire(struct kdbus_conn *conn, void __user *argp)
 		goto exit;
 	}
 
+	ret = security_kdbus_name_acquire(conn, item_name);
+	if (ret) {
+		goto exit;
+	}
+
 	/*
 	 * Do atomic_inc_return here to reserve our slot, then decrement
 	 * it before returning.
@@ -724,6 +730,10 @@ int kdbus_cmd_list(struct kdbus_conn *conn, void __user *argp)
 	if (ret != 0)
 		return ret;
 
+	ret = security_kdbus_name_list(conn->ep->bus);
+	if (ret)
+		return ret;
+
 	/* lock order: domain -> bus -> ep -> names -> conn */
 	down_read(&reg->rwlock);
 	down_read(&conn->ep->bus->conn_rwlock);
diff --git a/ipc/kdbus/queue.c b/ipc/kdbus/queue.c
index 25bb3ad..9872fb4 100644
--- a/ipc/kdbus/queue.c
+++ b/ipc/kdbus/queue.c
@@ -28,6 +28,7 @@
 #include <linux/slab.h>
 #include <linux/syscalls.h>
 #include <linux/uio.h>
+#include <linux/security.h>
 
 #include "util.h"
 #include "domain.h"
@@ -514,12 +515,17 @@ int kdbus_queue_entry_install(struct kdbus_queue_entry *entry,
 
 		for (i = 0; i < res->fds_count; i++) {
 			if (install_fds) {
-				fds[i] = get_unused_fd_flags(O_CLOEXEC);
-				if (fds[i] >= 0)
-					fd_install(fds[i],
-						   get_file(res->fds[i]));
-				else
+				if (security_file_receive(res->fds[i])) {
+					fds[i] = -1;
 					incomplete_fds = true;
+				} else {
+					fds[i] = get_unused_fd_flags(O_CLOEXEC);
+					if (fds[i] >= 0)
+						fd_install(fds[i],
+							get_file(res->fds[i]));
+					else
+						incomplete_fds = true;
+				}
 			} else {
 				fds[i] = -1;
 			}
@@ -557,13 +563,17 @@ int kdbus_queue_entry_install(struct kdbus_queue_entry *entry,
 		m.fd = -1;
 
 		if (install_fds) {
-			m.fd = get_unused_fd_flags(O_CLOEXEC);
-			if (m.fd < 0) {
-				m.fd = -1;
+			if (security_file_receive(d->memfd.file)) {
 				incomplete_fds = true;
 			} else {
-				fd_install(m.fd,
-					   get_file(d->memfd.file));
+				m.fd = get_unused_fd_flags(O_CLOEXEC);
+				if (m.fd < 0) {
+					m.fd = -1;
+					incomplete_fds = true;
+				} else {
+					fd_install(m.fd,
+						get_file(d->memfd.file));
+				}
 			}
 		}
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
  2015-07-08 10:25 ` [RFC 5/8] kdbus: use LSM hooks in kdbus code Paul Osmialowski
  2015-07-08 11:06   ` Lukasz Pawelczyk
  2015-07-08 11:09   ` Lukasz Pawelczyk
@ 2015-07-08 13:37   ` Stephen Smalley
  2015-07-10 16:56     ` Stephen Smalley
  2015-07-10 18:20     ` Stephen Smalley
  2015-07-08 14:13   ` Greg Kroah-Hartman
  2015-07-08 16:24   ` Casey Schaufler
  4 siblings, 2 replies; 30+ messages in thread
From: Stephen Smalley @ 2015-07-08 13:37 UTC (permalink / raw)
  To: Paul Osmialowski, Paul Moore, James Morris, Casey Schaufler,
	Serge E. Hallyn, Kees Cook, Tetsuo Handa, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Lukasz Skalski

On 07/08/2015 06:25 AM, Paul Osmialowski wrote:
> Originates from:
> 
> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)
> commit: aa0885489d19be92fa41c6f0a71df28763228a40
> 
> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
>  ipc/kdbus/bus.c        | 12 ++++++++++-
>  ipc/kdbus/bus.h        |  3 +++
>  ipc/kdbus/connection.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  ipc/kdbus/connection.h |  4 ++++
>  ipc/kdbus/domain.c     |  9 ++++++++-
>  ipc/kdbus/domain.h     |  2 ++
>  ipc/kdbus/endpoint.c   | 11 ++++++++++
>  ipc/kdbus/names.c      | 11 ++++++++++
>  ipc/kdbus/queue.c      | 30 ++++++++++++++++++----------
>  9 files changed, 124 insertions(+), 12 deletions(-)
> 
>

> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index 9993753..b85cdc7 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -31,6 +31,7 @@
>  #include <linux/slab.h>
>  #include <linux/syscalls.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "connection.h"
> @@ -73,6 +74,8 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
>  	bool is_activator;
>  	bool is_monitor;
>  	struct kvec kvec;
> +	u32 sid, len;
> +	char *label;
>  	int ret;
>  
>  	struct {
> @@ -222,6 +225,14 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
>  		}
>  	}
>  
> +	security_task_getsecid(current, &sid);
> +	security_secid_to_secctx(sid, &label, &len);
> +	ret = security_kdbus_connect(conn, label, len);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit_unref;
> +	}

This seems convoluted and expensive.  If you always want the label of
the current task here, then why not just have security_kdbus_connect()
internally extract the label of the current task?

> @@ -1107,6 +1119,12 @@ static int kdbus_conn_reply(struct kdbus_conn *src, struct kdbus_kmsg *kmsg)
>  	if (ret < 0)
>  		goto exit;
>  
> +	ret = security_kdbus_talk(src, dst);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit;
> +	}

Where does kdbus apply its uid-based or other restrictions on
connections?  Why do we need to insert separate hooks into each of these
functions?  Is there no central chokepoint already for permission
checking that we can hook?

> diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
> index d1ffe90..1f91d39 100644
> --- a/ipc/kdbus/connection.h
> +++ b/ipc/kdbus/connection.h
> @@ -19,6 +19,7 @@
>  #include <linux/kref.h>
>  #include <linux/lockdep.h>
>  #include <linux/path.h>
> +#include <uapi/linux/kdbus.h>
>  
>  #include "limits.h"
>  #include "metadata.h"
> @@ -73,6 +74,7 @@ struct kdbus_kmsg;
>   * @names_queue_list:	Well-known names this connection waits for
>   * @privileged:		Whether this connection is privileged on the bus
>   * @faked_meta:		Whether the metadata was faked on HELLO
> + * @security:		LSM security blob
>   */
>  struct kdbus_conn {
>  	struct kref kref;
> @@ -113,6 +115,8 @@ struct kdbus_conn {
>  
>  	bool privileged:1;
>  	bool faked_meta:1;
> +
> +	void *security;
>  };

Unless I missed it, you may have missed the most important thing of all:
 controlling kdbus's notion of "privileged".  kdbus sets privileged to
true if the process has CAP_IPC_OWNER or the process euid matches the
uid of the bus creator, and then it allows those processes to do many
dangerous things, including monitoring all traffic, impersonating
credentials, pids, or seclabel, etc.

I don't believe we should ever permit impersonating seclabel information.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 4/8] lsm: smack: smack callbacks for kdbus security hooks
  2015-07-08 10:25 ` [RFC 4/8] lsm: smack: smack callbacks for " Paul Osmialowski
@ 2015-07-08 13:42   ` Stephen Smalley
  2015-07-08 16:38     ` Casey Schaufler
  2015-07-09 10:08   ` Sergei Zviagintsev
  1 sibling, 1 reply; 30+ messages in thread
From: Stephen Smalley @ 2015-07-08 13:42 UTC (permalink / raw)
  To: Paul Osmialowski, Paul Moore, James Morris, Casey Schaufler,
	Serge E. Hallyn, Kees Cook, Tetsuo Handa, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Lukasz Skalski

On 07/08/2015 06:25 AM, Paul Osmialowski wrote:
> This adds implementation of three smack callbacks sitting behind kdbus
> security hooks as proposed by Karol Lewandowski.
> 
> Originates from:
> 
> git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)
> commit: fc3505d058c001fe72a6f66b833e0be5b2d118f3
> 
> https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd-v212)
> commit: 103c26fd27d1ec8c32d85dd3d85681f936ac66fb
> 
> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
>  security/smack/smack_lsm.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index a143328..033b756 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -41,6 +41,7 @@
>  #include <linux/msg.h>
>  #include <linux/shm.h>
>  #include <linux/binfmts.h>
> +#include <kdbus/connection.h>
>  #include "smack.h"
>  
>  #define TRANS_TRUE	"TRUE"
> @@ -3336,6 +3337,69 @@ static int smack_setprocattr(struct task_struct *p, char *name,
>  }
>  
>  /**
> + * smack_kdbus_connect - Set the security blob for a KDBus connection
> + * @conn: the connection
> + * @secctx: smack label
> + * @seclen: smack label length
> + *
> + * Returns 0
> + */
> +static int smack_kdbus_connect(struct kdbus_conn *conn,
> +			       const char *secctx, u32 seclen)
> +{
> +	struct smack_known *skp;
> +
> +	if (secctx && seclen > 0)
> +		skp = smk_import_entry(secctx, seclen);
> +	else
> +		skp = smk_of_current();
> +	conn->security = skp;
> +
> +	return 0;
> +}
> +
> +/**
> + * smack_kdbus_conn_free - Clear the security blob for a KDBus connection
> + * @conn: the connection
> + *
> + * Clears the blob pointer
> + */
> +static void smack_kdbus_conn_free(struct kdbus_conn *conn)
> +{
> +	conn->security = NULL;
> +}
> +
> +/**
> + * smack_kdbus_talk - Smack access on KDBus
> + * @src: source kdbus connection
> + * @dst: destination kdbus connection
> + *
> + * Return 0 if a subject with the smack of sock could access
> + * an object with the smack of other, otherwise an error code
> + */
> +static int smack_kdbus_talk(const struct kdbus_conn *src,
> +			    const struct kdbus_conn *dst)
> +{
> +	struct smk_audit_info ad;
> +	struct smack_known *sskp = src->security;
> +	struct smack_known *dskp = dst->security;
> +	int ret;
> +
> +	BUG_ON(sskp == NULL);
> +	BUG_ON(dskp == NULL);
> +
> +	if (smack_privileged(CAP_MAC_OVERRIDE))
> +		return 0;
> +
> +	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_NONE);
> +
> +	ret = smk_access(sskp, dskp, MAY_WRITE, &ad);
> +	if (ret)
> +		return ret;
> +	return 0;
> +}
> +
> +/**
>   * smack_unix_stream_connect - Smack access on UDS
>   * @sock: one sock
>   * @other: the other sock
> @@ -4393,6 +4457,10 @@ struct security_hook_list smack_hooks[] = {
>  	LSM_HOOK_INIT(inode_notifysecctx, smack_inode_notifysecctx),
>  	LSM_HOOK_INIT(inode_setsecctx, smack_inode_setsecctx),
>  	LSM_HOOK_INIT(inode_getsecctx, smack_inode_getsecctx),
> +
> +	LSM_HOOK_INIT(kdbus_connect, smack_kdbus_connect),
> +	LSM_HOOK_INIT(kdbus_conn_free, smack_kdbus_conn_free),
> +	LSM_HOOK_INIT(kdbus_talk, smack_kdbus_talk),
>  };

If Smack only truly needs 3 hooks, then it begs the question of why
there are so many other hooks defined.  Are the other hooks just to
support finer-grained distinctions, or is Smack's coverage incomplete?


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
  2015-07-08 10:25 ` [RFC 5/8] kdbus: use LSM hooks in kdbus code Paul Osmialowski
                     ` (2 preceding siblings ...)
  2015-07-08 13:37   ` Stephen Smalley
@ 2015-07-08 14:13   ` Greg Kroah-Hartman
  2015-07-08 16:24   ` Casey Schaufler
  4 siblings, 0 replies; 30+ messages in thread
From: Greg Kroah-Hartman @ 2015-07-08 14:13 UTC (permalink / raw)
  To: Paul Osmialowski
  Cc: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Daniel Mack, David Herrmann, Djalal Harouni,
	Shuah Khan, Al Viro, linux-security-module, linux-kernel,
	linux-api, Karol Lewandowski, Lukasz Skalski

On Wed, Jul 08, 2015 at 12:25:07PM +0200, Paul Osmialowski wrote:
> Originates from:
> 
> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)
> commit: aa0885489d19be92fa41c6f0a71df28763228a40

That's a horrid changelog entry, what is it supposed to mean?  Don't
refer to a different tree, which will not be around for forever, and
have no information here at all as to what is going on in the change.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 3/8] lsm: kdbus security hooks
  2015-07-08 10:25 ` [RFC 3/8] lsm: kdbus security hooks Paul Osmialowski
  2015-07-08 11:00   ` Lukasz Pawelczyk
@ 2015-07-08 14:14   ` Greg Kroah-Hartman
  1 sibling, 0 replies; 30+ messages in thread
From: Greg Kroah-Hartman @ 2015-07-08 14:14 UTC (permalink / raw)
  To: Paul Osmialowski
  Cc: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Daniel Mack, David Herrmann, Djalal Harouni,
	Shuah Khan, Al Viro, linux-security-module, linux-kernel,
	linux-api, Karol Lewandowski, Lukasz Skalski

On Wed, Jul 08, 2015 at 12:25:05PM +0200, Paul Osmialowski wrote:
> This is combination of the work by Karol Lewandowski and Paul Moore
> on LSM hooks for kdbus.
> 
> Originates from:
> 
> git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)
> commit: 7050f206a79564886938d0edc4e1e9da5972c72d
> 
> https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd-v212)
> commit: a9fe4c33b6e5ab25a243e0590df406aabb6add12

Again, horrid commit log entries :(

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 8/8] kdbus: Ability to run kdbus test by executable binary name
  2015-07-08 10:25 ` [RFC 8/8] kdbus: Ability to run kdbus test by executable binary name Paul Osmialowski
@ 2015-07-08 14:16   ` Greg Kroah-Hartman
  2015-07-08 14:58     ` Paul Osmialowski
  0 siblings, 1 reply; 30+ messages in thread
From: Greg Kroah-Hartman @ 2015-07-08 14:16 UTC (permalink / raw)
  To: Paul Osmialowski
  Cc: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Daniel Mack, David Herrmann, Djalal Harouni,
	Shuah Khan, Al Viro, linux-security-module, linux-kernel,
	linux-api, Karol Lewandowski, Lukasz Skalski

On Wed, Jul 08, 2015 at 12:25:10PM +0200, Paul Osmialowski wrote:
> With this applied, you can do following:
> 
> $ cp kdbus-test daemon
> $ cp kdbus-test send
> 
> Then run 'daemon' in one shell session:
> 
> $ ./daemon --bus test
> 
> ...and 'send' in another:
> 
> $ ./send --bus test
> 
> Useful for testing features introduced by previous patches.
> 
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
>  tools/testing/selftests/kdbus/kdbus-test.c | 5 +++++
>  1 file changed, 5 insertions(+)

Why not create symlinks automatically so this works without having to
manually run 'cp'?  But really, is spelling out the command line option
really a big issue?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 8/8] kdbus: Ability to run kdbus test by executable binary name
  2015-07-08 14:16   ` Greg Kroah-Hartman
@ 2015-07-08 14:58     ` Paul Osmialowski
  0 siblings, 0 replies; 30+ messages in thread
From: Paul Osmialowski @ 2015-07-08 14:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Daniel Mack, David Herrmann, Djalal Harouni,
	Shuah Khan, Al Viro, linux-security-module, linux-kernel,
	linux-api, Karol Lewandowski, Lukasz Skalski

Hi Greg,

Thanks for your comments,

On Wed, 8 Jul 2015, Greg Kroah-Hartman wrote:

>
> Why not create symlinks automatically so this works without having to
> manually run 'cp'?

It can be anything on which one can apply security label. Symlinks can do.

> But really, is spelling out the command line option
> really a big issue?

Maybe. I couldn't think about any easier (and easy to present) use case 
that could explain the purpose of this change.

>
> thanks,
>
> greg k-h
>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
  2015-07-08 10:25 ` [RFC 5/8] kdbus: use LSM hooks in kdbus code Paul Osmialowski
                     ` (3 preceding siblings ...)
  2015-07-08 14:13   ` Greg Kroah-Hartman
@ 2015-07-08 16:24   ` Casey Schaufler
  4 siblings, 0 replies; 30+ messages in thread
From: Casey Schaufler @ 2015-07-08 16:24 UTC (permalink / raw)
  To: Paul Osmialowski, Paul Moore, James Morris, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Lukasz Skalski

On 7/8/2015 3:25 AM, Paul Osmialowski wrote:
> Originates from:
>
> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)
> commit: aa0885489d19be92fa41c6f0a71df28763228a40
>
> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
>  ipc/kdbus/bus.c        | 12 ++++++++++-
>  ipc/kdbus/bus.h        |  3 +++
>  ipc/kdbus/connection.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  ipc/kdbus/connection.h |  4 ++++
>  ipc/kdbus/domain.c     |  9 ++++++++-
>  ipc/kdbus/domain.h     |  2 ++
>  ipc/kdbus/endpoint.c   | 11 ++++++++++
>  ipc/kdbus/names.c      | 11 ++++++++++
>  ipc/kdbus/queue.c      | 30 ++++++++++++++++++----------
>  9 files changed, 124 insertions(+), 12 deletions(-)
>
> diff --git a/ipc/kdbus/bus.c b/ipc/kdbus/bus.c
> index bbdf0f2..9894895 100644
> --- a/ipc/kdbus/bus.c
> +++ b/ipc/kdbus/bus.c
> @@ -22,6 +22,7 @@
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "notify.h"
> @@ -51,6 +52,7 @@ static void kdbus_bus_free(struct kdbus_node *node)
>  	kdbus_domain_unref(bus->domain);
>  	kdbus_policy_db_clear(&bus->policy_db);
>  	kdbus_meta_proc_unref(bus->creator_meta);
> +	security_kdbus_bus_free(bus);
>  	kfree(bus);
>  }
>  
> @@ -161,6 +163,12 @@ static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain *domain,
>  		goto exit_unref;
>  	}
>  
> +	ret = security_kdbus_bus_alloc(b);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit_unref;
> +	}
> +
>  	/*
>  	 * Bus-limits of the creator are accounted on its real UID, just like
>  	 * all other per-user limits.
> @@ -169,11 +177,13 @@ static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain *domain,
>  	if (IS_ERR(b->creator)) {
>  		ret = PTR_ERR(b->creator);
>  		b->creator = NULL;
> -		goto exit_unref;
> +		goto exit_free_security;
>  	}
>  
>  	return b;
>  
> +exit_free_security:
> +	security_kdbus_bus_free(b);
>  exit_unref:
>  	kdbus_node_deactivate(&b->node);
>  	kdbus_node_unref(&b->node);
> diff --git a/ipc/kdbus/bus.h b/ipc/kdbus/bus.h
> index 5bea5ef..03e4a54 100644
> --- a/ipc/kdbus/bus.h
> +++ b/ipc/kdbus/bus.h
> @@ -53,6 +53,7 @@ struct kdbus_user;
>   * @notify_list:	List of pending kernel-generated messages
>   * @notify_lock:	Notification list lock
>   * @notify_flush_lock:	Notification flushing lock
> + * @security:		LSM security blob
>   */
>  struct kdbus_bus {
>  	struct kdbus_node node;
> @@ -81,6 +82,8 @@ struct kdbus_bus {
>  	struct list_head notify_list;
>  	spinlock_t notify_lock;
>  	struct mutex notify_flush_lock;
> +
> +	void *security;
>  };
>  
>  struct kdbus_bus *kdbus_bus_ref(struct kdbus_bus *bus);
> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index 9993753..b85cdc7 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -31,6 +31,7 @@
>  #include <linux/slab.h>
>  #include <linux/syscalls.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "connection.h"
> @@ -73,6 +74,8 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
>  	bool is_activator;
>  	bool is_monitor;
>  	struct kvec kvec;
> +	u32 sid, len;
> +	char *label;
>  	int ret;
>  
>  	struct {
> @@ -222,6 +225,14 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
>  		}
>  	}
>  
> +	security_task_getsecid(current, &sid);
> +	security_secid_to_secctx(sid, &label, &len);
> +	ret = security_kdbus_connect(conn, label, len);

This is horrid. It's maximally inefficient for everyone.
Change the label argument to a task, and pass current. Let
the module decide what to do with it. While Smack and SELinux
will be making the decision based on the label, you can't
count on that for all modules.

If you think you need to do *anything* using secids you are
mistaken. The only case where a secid should be used is in
legacy networking code where the maintainer has decided that
security is only worth 32 bits and can never be worth more.

> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit_unref;
> +	}
> +
>  	if (atomic_inc_return(&conn->user->connections) > KDBUS_USER_MAX_CONN) {
>  		/* decremented by destructor as conn->user is valid */
>  		ret = -EMFILE;
> @@ -276,6 +287,7 @@ static void __kdbus_conn_free(struct kref *kref)
>  	kdbus_pool_free(conn->pool);
>  	kdbus_ep_unref(conn->ep);
>  	put_cred(conn->cred);
> +	security_kdbus_conn_free(conn);
>  	kfree(conn->description);
>  	kfree(conn->quota);
>  	kfree(conn);
> @@ -1107,6 +1119,12 @@ static int kdbus_conn_reply(struct kdbus_conn *src, struct kdbus_kmsg *kmsg)
>  	if (ret < 0)
>  		goto exit;
>  
> +	ret = security_kdbus_talk(src, dst);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit;
> +	}

Why not return what the module reports?
Why EPERM? EPERM means "you needed privilege".
EACCES means "you were denied access by normal means".

> +
>  	mutex_lock(&dst->lock);
>  	reply = kdbus_reply_find(src, dst, kmsg->msg.cookie_reply);
>  	if (reply) {
> @@ -1187,6 +1205,12 @@ static struct kdbus_reply *kdbus_conn_call(struct kdbus_conn *src,
>  	if (ret < 0)
>  		goto exit;
>  
> +	ret = security_kdbus_talk(src, dst);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit;
> +	}
> +
>  	if (!kdbus_conn_policy_talk(src, current_cred(), dst)) {
>  		ret = -EPERM;
>  		goto exit;
> @@ -1248,6 +1272,12 @@ static int kdbus_conn_unicast(struct kdbus_conn *src, struct kdbus_kmsg *kmsg)
>  	if (ret < 0)
>  		goto exit;
>  
> +	ret = security_kdbus_talk(src, dst);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit;
> +	}
> +
>  	if (is_signal) {
>  		/* like broadcasts we eavesdrop even if the msg is dropped */
>  		kdbus_bus_eavesdrop(bus, src, kmsg);
> @@ -1639,6 +1669,12 @@ struct kdbus_conn *kdbus_cmd_hello(struct kdbus_ep *ep, bool privileged,
>  		if (ret < 0)
>  			goto exit;
>  
> +		ret = security_kdbus_ep_setpolicy(c->ep->bus);
> +		if (ret) {
> +			ret = -EPERM;
> +			goto exit;
> +		}
> +
>  		ret = kdbus_policy_set(&c->ep->bus->policy_db, args.items,
>  				       args.items_size, 1,
>  				       kdbus_conn_is_policy_holder(c), c);
> @@ -1732,6 +1768,10 @@ int kdbus_cmd_conn_info(struct kdbus_conn *conn, void __user *argp)
>  	if (ret != 0)
>  		return ret;
>  
> +	ret = security_kdbus_conn_info(conn);
> +	if (ret)
> +		return -EPERM;
> +
>  	/* registry must be held throughout lookup *and* collecting data */
>  	down_read(&bus->name_registry->rwlock);
>  
> @@ -1905,6 +1945,12 @@ int kdbus_cmd_update(struct kdbus_conn *conn, void __user *argp)
>  	/* now that we verified the input, update the connection */
>  
>  	if (item_policy) {
> +		ret = security_kdbus_ep_setpolicy(conn->ep->bus);
> +		if (ret) {
> +			ret = -EPERM;
> +			goto exit;
> +		}
> +
>  		ret = kdbus_policy_set(&conn->ep->bus->policy_db, cmd->items,
>  				       KDBUS_ITEMS_SIZE(cmd, items),
>  				       1, true, conn);
> @@ -1948,6 +1994,10 @@ int kdbus_cmd_send(struct kdbus_conn *conn, struct file *f, void __user *argp)
>  		.argc = ARRAY_SIZE(argv),
>  	};
>  
> +	ret = security_kdbus_send(conn, conn->ep->bus);
> +	if (ret)
> +		return -EPERM;
> +
>  	if (!kdbus_conn_is_ordinary(conn))
>  		return -EOPNOTSUPP;
>  
> @@ -2044,6 +2094,10 @@ int kdbus_cmd_recv(struct kdbus_conn *conn, void __user *argp)
>  		.argc = ARRAY_SIZE(argv),
>  	};
>  
> +	ret = security_kdbus_recv(conn, conn->ep->bus);
> +	if (ret)
> +		return -EPERM;
> +
>  	if (!kdbus_conn_is_ordinary(conn) &&
>  	    !kdbus_conn_is_monitor(conn) &&
>  	    !kdbus_conn_is_activator(conn))
> diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
> index d1ffe90..1f91d39 100644
> --- a/ipc/kdbus/connection.h
> +++ b/ipc/kdbus/connection.h
> @@ -19,6 +19,7 @@
>  #include <linux/kref.h>
>  #include <linux/lockdep.h>
>  #include <linux/path.h>
> +#include <uapi/linux/kdbus.h>
>  
>  #include "limits.h"
>  #include "metadata.h"
> @@ -73,6 +74,7 @@ struct kdbus_kmsg;
>   * @names_queue_list:	Well-known names this connection waits for
>   * @privileged:		Whether this connection is privileged on the bus
>   * @faked_meta:		Whether the metadata was faked on HELLO
> + * @security:		LSM security blob
>   */
>  struct kdbus_conn {
>  	struct kref kref;
> @@ -113,6 +115,8 @@ struct kdbus_conn {
>  
>  	bool privileged:1;
>  	bool faked_meta:1;
> +
> +	void *security;
>  };
>  
>  struct kdbus_conn *kdbus_conn_ref(struct kdbus_conn *conn);
> diff --git a/ipc/kdbus/domain.c b/ipc/kdbus/domain.c
> index ac9f760..da9cdab 100644
> --- a/ipc/kdbus/domain.c
> +++ b/ipc/kdbus/domain.c
> @@ -20,6 +20,7 @@
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "domain.h"
> @@ -73,6 +74,7 @@ static void kdbus_domain_free(struct kdbus_node *node)
>  	put_user_ns(domain->user_namespace);
>  	ida_destroy(&domain->user_ida);
>  	idr_destroy(&domain->user_idr);
> +	security_kdbus_domain_free(domain);
>  	kfree(domain);
>  }
>  
> @@ -104,6 +106,10 @@ struct kdbus_domain *kdbus_domain_new(unsigned int access)
>  	idr_init(&d->user_idr);
>  	ida_init(&d->user_ida);
>  
> +	ret = security_kdbus_domain_alloc(d);
> +	if (ret)
> +		return ERR_PTR(-EPERM);
> +
>  	/* Pin user namespace so we can guarantee domain-unique bus * names. */
>  	d->user_namespace = get_user_ns(current_user_ns());
>  
> @@ -116,6 +122,7 @@ struct kdbus_domain *kdbus_domain_new(unsigned int access)
>  exit_unref:
>  	kdbus_node_deactivate(&d->node);
>  	kdbus_node_unref(&d->node);
> +	security_kdbus_domain_free(d);
>  	return ERR_PTR(ret);
>  }
>  
> @@ -264,7 +271,7 @@ static void __kdbus_user_free(struct kref *kref)
>  	if (uid_valid(user->uid))
>  		idr_remove(&user->domain->user_idr, __kuid_val(user->uid));
>  	mutex_unlock(&user->domain->lock);
> -
> +	security_kdbus_domain_free(user->domain);
>  	kdbus_domain_unref(user->domain);
>  	kfree(user);
>  }
> diff --git a/ipc/kdbus/domain.h b/ipc/kdbus/domain.h
> index 447a2bd..3db06d8 100644
> --- a/ipc/kdbus/domain.h
> +++ b/ipc/kdbus/domain.h
> @@ -31,6 +31,7 @@
>   * @user_ida:		Set of all users to compute small indices
>   * @user_namespace:	User namespace, pinned at creation time
>   * @dentry:		Root dentry of VFS mount (don't use outside of kdbusfs)
> + * @security:		LSM security blob
>   */
>  struct kdbus_domain {
>  	struct kdbus_node node;
> @@ -40,6 +41,7 @@ struct kdbus_domain {
>  	struct ida user_ida;
>  	struct user_namespace *user_namespace;
>  	struct dentry *dentry;
> +	void *security;
>  };
>  
>  /**
> diff --git a/ipc/kdbus/endpoint.c b/ipc/kdbus/endpoint.c
> index 9a95a5e..380228f 100644
> --- a/ipc/kdbus/endpoint.c
> +++ b/ipc/kdbus/endpoint.c
> @@ -21,6 +21,7 @@
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "connection.h"
> @@ -122,6 +123,12 @@ struct kdbus_ep *kdbus_ep_new(struct kdbus_bus *bus, const char *name,
>  	kdbus_policy_db_init(&e->policy_db);
>  	e->bus = kdbus_bus_ref(bus);
>  
> +	ret = security_kdbus_ep_create(bus);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit_unref;
> +	}
> +
>  	ret = kdbus_node_link(&e->node, &bus->node, name);
>  	if (ret < 0)
>  		goto exit_unref;
> @@ -265,6 +272,10 @@ int kdbus_cmd_ep_update(struct kdbus_ep *ep, void __user *argp)
>  		.argc = ARRAY_SIZE(argv),
>  	};
>  
> +	ret = security_kdbus_ep_setpolicy(ep->bus);
> +	if (ret)
> +		return -EPERM;
> +
>  	ret = kdbus_args_parse(&args, argp, &cmd);
>  	if (ret != 0)
>  		return ret;
> diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
> index d77ee08..dd20bea 100644
> --- a/ipc/kdbus/names.c
> +++ b/ipc/kdbus/names.c
> @@ -24,6 +24,7 @@
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "bus.h"
>  #include "connection.h"
> @@ -503,6 +504,12 @@ int kdbus_cmd_name_acquire(struct kdbus_conn *conn, void __user *argp)
>  		goto exit;
>  	}
>  
> +	ret = security_kdbus_name_acquire(conn, item_name);
> +	if (ret) {
> +		ret = -EPERM;
> +		goto exit;
> +	}
> +
>  	/*
>  	 * Do atomic_inc_return here to reserve our slot, then decrement
>  	 * it before returning.
> @@ -724,6 +731,10 @@ int kdbus_cmd_list(struct kdbus_conn *conn, void __user *argp)
>  	if (ret != 0)
>  		return ret;
>  
> +	ret = security_kdbus_name_list(conn->ep->bus);
> +	if (ret)
> +		return -EPERM;
> +
>  	/* lock order: domain -> bus -> ep -> names -> conn */
>  	down_read(&reg->rwlock);
>  	down_read(&conn->ep->bus->conn_rwlock);
> diff --git a/ipc/kdbus/queue.c b/ipc/kdbus/queue.c
> index 25bb3ad..9872fb4 100644
> --- a/ipc/kdbus/queue.c
> +++ b/ipc/kdbus/queue.c
> @@ -28,6 +28,7 @@
>  #include <linux/slab.h>
>  #include <linux/syscalls.h>
>  #include <linux/uio.h>
> +#include <linux/security.h>
>  
>  #include "util.h"
>  #include "domain.h"
> @@ -514,12 +515,17 @@ int kdbus_queue_entry_install(struct kdbus_queue_entry *entry,
>  
>  		for (i = 0; i < res->fds_count; i++) {
>  			if (install_fds) {
> -				fds[i] = get_unused_fd_flags(O_CLOEXEC);
> -				if (fds[i] >= 0)
> -					fd_install(fds[i],
> -						   get_file(res->fds[i]));
> -				else
> +				if (security_file_receive(res->fds[i])) {
> +					fds[i] = -1;
>  					incomplete_fds = true;
> +				} else {
> +					fds[i] = get_unused_fd_flags(O_CLOEXEC);
> +					if (fds[i] >= 0)
> +						fd_install(fds[i],
> +							get_file(res->fds[i]));
> +					else
> +						incomplete_fds = true;
> +				}
>  			} else {
>  				fds[i] = -1;
>  			}
> @@ -557,13 +563,17 @@ int kdbus_queue_entry_install(struct kdbus_queue_entry *entry,
>  		m.fd = -1;
>  
>  		if (install_fds) {
> -			m.fd = get_unused_fd_flags(O_CLOEXEC);
> -			if (m.fd < 0) {
> -				m.fd = -1;
> +			if (security_file_receive(d->memfd.file)) {
>  				incomplete_fds = true;
>  			} else {
> -				fd_install(m.fd,
> -					   get_file(d->memfd.file));
> +				m.fd = get_unused_fd_flags(O_CLOEXEC);
> +				if (m.fd < 0) {
> +					m.fd = -1;
> +					incomplete_fds = true;
> +				} else {
> +					fd_install(m.fd,
> +						get_file(d->memfd.file));
> +				}
>  			}
>  		}
>  


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 4/8] lsm: smack: smack callbacks for kdbus security hooks
  2015-07-08 13:42   ` Stephen Smalley
@ 2015-07-08 16:38     ` Casey Schaufler
  2015-07-08 20:07       ` Paul Moore
  0 siblings, 1 reply; 30+ messages in thread
From: Casey Schaufler @ 2015-07-08 16:38 UTC (permalink / raw)
  To: Stephen Smalley, Paul Osmialowski, Paul Moore, James Morris,
	Serge E. Hallyn, Kees Cook, Tetsuo Handa, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Lukasz Skalski

On 7/8/2015 6:42 AM, Stephen Smalley wrote:
> On 07/08/2015 06:25 AM, Paul Osmialowski wrote:
>> This adds implementation of three smack callbacks sitting behind kdbus
>> security hooks as proposed by Karol Lewandowski.
>>
>> Originates from:
>>
>> git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)
>> commit: fc3505d058c001fe72a6f66b833e0be5b2d118f3
>>
>> https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd-v212)
>> commit: 103c26fd27d1ec8c32d85dd3d85681f936ac66fb
>>
>> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
>> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
>> ---
>>  security/smack/smack_lsm.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 68 insertions(+)
>>
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index a143328..033b756 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -41,6 +41,7 @@
>>  #include <linux/msg.h>
>>  #include <linux/shm.h>
>>  #include <linux/binfmts.h>
>> +#include <kdbus/connection.h>
>>  #include "smack.h"
>>  
>>  #define TRANS_TRUE	"TRUE"
>> @@ -3336,6 +3337,69 @@ static int smack_setprocattr(struct task_struct *p, char *name,
>>  }
>>  
>>  /**
>> + * smack_kdbus_connect - Set the security blob for a KDBus connection
>> + * @conn: the connection
>> + * @secctx: smack label
>> + * @seclen: smack label length
>> + *
>> + * Returns 0
>> + */
>> +static int smack_kdbus_connect(struct kdbus_conn *conn,
>> +			       const char *secctx, u32 seclen)
>> +{
>> +	struct smack_known *skp;
>> +
>> +	if (secctx && seclen > 0)
>> +		skp = smk_import_entry(secctx, seclen);
>> +	else
>> +		skp = smk_of_current();
>> +	conn->security = skp;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * smack_kdbus_conn_free - Clear the security blob for a KDBus connection
>> + * @conn: the connection
>> + *
>> + * Clears the blob pointer
>> + */
>> +static void smack_kdbus_conn_free(struct kdbus_conn *conn)
>> +{
>> +	conn->security = NULL;
>> +}
>> +
>> +/**
>> + * smack_kdbus_talk - Smack access on KDBus
>> + * @src: source kdbus connection
>> + * @dst: destination kdbus connection
>> + *
>> + * Return 0 if a subject with the smack of sock could access
>> + * an object with the smack of other, otherwise an error code
>> + */
>> +static int smack_kdbus_talk(const struct kdbus_conn *src,
>> +			    const struct kdbus_conn *dst)
>> +{
>> +	struct smk_audit_info ad;
>> +	struct smack_known *sskp = src->security;
>> +	struct smack_known *dskp = dst->security;
>> +	int ret;
>> +
>> +	BUG_ON(sskp == NULL);
>> +	BUG_ON(dskp == NULL);
>> +
>> +	if (smack_privileged(CAP_MAC_OVERRIDE))
>> +		return 0;
>> +
>> +	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_NONE);
>> +
>> +	ret = smk_access(sskp, dskp, MAY_WRITE, &ad);
>> +	if (ret)
>> +		return ret;
>> +	return 0;
>> +}
>> +
>> +/**
>>   * smack_unix_stream_connect - Smack access on UDS
>>   * @sock: one sock
>>   * @other: the other sock
>> @@ -4393,6 +4457,10 @@ struct security_hook_list smack_hooks[] = {
>>  	LSM_HOOK_INIT(inode_notifysecctx, smack_inode_notifysecctx),
>>  	LSM_HOOK_INIT(inode_setsecctx, smack_inode_setsecctx),
>>  	LSM_HOOK_INIT(inode_getsecctx, smack_inode_getsecctx),
>> +
>> +	LSM_HOOK_INIT(kdbus_connect, smack_kdbus_connect),
>> +	LSM_HOOK_INIT(kdbus_conn_free, smack_kdbus_conn_free),
>> +	LSM_HOOK_INIT(kdbus_talk, smack_kdbus_talk),
>>  };
> If Smack only truly needs 3 hooks, then it begs the question of why
> there are so many other hooks defined.  Are the other hooks just to
> support finer-grained distinctions, or is Smack's coverage incomplete?

I haven't been following kdbus closely for a while, but the original
intent for Smack and kdbus was that it Smack controls would be on the
objects involved, and that to accomplish that only a small number of
hooks would be necessary. After all, Smack uses fewer hooks than SELinux
on other things. I do agree that without a user there is no point in
having hooks. If SELinux requires the other hooks we might want to
hold off on asking for the hooks until the SELinux implementation is
exposed. I also think that AppArmor should be examined as a potential
user of the hooks, just to make sure the hooks aren't excessively
oriented toward subject/object based security modules.

>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 2/8] lsm: smack: Make ipc/kdbus includes visible so smack callbacks could see them
  2015-07-08 10:25 ` [RFC 2/8] lsm: smack: Make ipc/kdbus includes visible so smack callbacks could see them Paul Osmialowski
@ 2015-07-08 16:43   ` Daniel Mack
  0 siblings, 0 replies; 30+ messages in thread
From: Daniel Mack @ 2015-07-08 16:43 UTC (permalink / raw)
  To: Paul Osmialowski, Paul Moore, James Morris, Casey Schaufler,
	Serge E. Hallyn, Kees Cook, Tetsuo Handa, Stephen Smalley,
	Neil Brown, Mark Rustad, Greg Kroah-Hartman, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Lukasz Skalski

On 07/08/2015 06:25 AM, Paul Osmialowski wrote:
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
>  security/smack/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/security/smack/Makefile b/security/smack/Makefile
> index ee2ebd5..bd6927c 100644
> --- a/security/smack/Makefile
> +++ b/security/smack/Makefile
> @@ -6,3 +6,5 @@ obj-$(CONFIG_SECURITY_SMACK) := smack.o
>  
>  smack-y := smack_lsm.o smack_access.o smackfs.o
>  smack-$(CONFIG_SECURITY_SMACK_NETFILTER) += smack_netfilter.o
> +
> +ccflags-y += -Iipc
> 

I would really like to avoid exposing the internal structures of kdbus
to LSM modules. This is going to get messy if any internals change in
the future.

The callbacks for smack currently only use the passed structure to
access the opaque 'security' pointer. Can't you just pass that directly
(as void**), along with other parameters the LSMs needs to know about?


Thanks,
Daniel


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 0/8] Introduce LSM to KDBUS
  2015-07-08 10:25 [RFC 0/8] Introduce LSM to KDBUS Paul Osmialowski
                   ` (7 preceding siblings ...)
  2015-07-08 10:25 ` [RFC 8/8] kdbus: Ability to run kdbus test by executable binary name Paul Osmialowski
@ 2015-07-08 16:46 ` Casey Schaufler
  8 siblings, 0 replies; 30+ messages in thread
From: Casey Schaufler @ 2015-07-08 16:46 UTC (permalink / raw)
  To: Paul Osmialowski, Paul Moore, James Morris, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Lukasz Skalski

On 7/8/2015 3:25 AM, Paul Osmialowski wrote:
> This patchset partially summarizes effects of collective work by
> Karol Lewandowski and Paul Moore towards introduction of LSM into KDBUS.
>
> These patches originate from following git repositories:
>
> git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)
>
> https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd-v212)
>
> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)
>
> Since kdbus made its way to linux-next tree, I was kindly asked by
> Karol Lewandowski to fit his work into the current kdbus code existing
> there.
>
> As both kdbus and security related code changed a bit, so are my changes
> quite substantial in places.
>
> Note that SELinux kdbus access control patches are absent - only SMACK part
> of original work is included.

Patches 2 and 3 need to be reversed. You can't add the Smack hooks
until you've added the infrastructure for them.

My comments should in no way be construed as an endorsement of kdbus.

>
> I've also made some changes to kdbus test suite. In order to see LSM hooks
> in action we need to be able to run tests from different executable
> binaries holding different security labels.
>
> Therefore I added ability to select execution of particular test by
> executed binary name. This is essential for running newly added 'send' test
> which should communicate with 'daemon' test running in another process.
>
> Karol Lewandowski (1):
>   lsm: make security_file_receive available for external modules
>
> Paul Osmialowski (7):
>   lsm: smack: Make ipc/kdbus includes visible so smack callbacks could
>     see them
>   lsm: kdbus security hooks
>   lsm: smack: smack callbacks for kdbus security hooks
>   kdbus: use LSM hooks in kdbus code
>   kdbus: TEST_CREATE_CONN now does no depend on TEST_CREATE_BUS
>   kdbus: selftests extended
>   kdbus: Ability to run kdbus test by executable binary name
>
>  include/linux/lsm_hooks.h                        |  67 +++++++++++++
>  include/linux/security.h                         |  99 +++++++++++++++++++
>  ipc/kdbus/bus.c                                  |  12 ++-
>  ipc/kdbus/bus.h                                  |   3 +
>  ipc/kdbus/connection.c                           |  54 +++++++++++
>  ipc/kdbus/connection.h                           |   4 +
>  ipc/kdbus/domain.c                               |   9 +-
>  ipc/kdbus/domain.h                               |   2 +
>  ipc/kdbus/endpoint.c                             |  11 +++
>  ipc/kdbus/names.c                                |  11 +++
>  ipc/kdbus/queue.c                                |  30 ++++--
>  security/security.c                              | 118 +++++++++++++++++++++++
>  security/smack/Makefile                          |   2 +
>  security/smack/smack_lsm.c                       |  68 +++++++++++++
>  tools/testing/selftests/kdbus/Makefile           |   1 +
>  tools/testing/selftests/kdbus/kdbus-test.c       |  37 ++++++-
>  tools/testing/selftests/kdbus/kdbus-test.h       |   1 +
>  tools/testing/selftests/kdbus/kdbus-util.c       |  37 ++++---
>  tools/testing/selftests/kdbus/kdbus-util.h       |   2 +-
>  tools/testing/selftests/kdbus/test-activator.c   |  20 ++--
>  tools/testing/selftests/kdbus/test-chat.c        |   6 +-
>  tools/testing/selftests/kdbus/test-connection.c  |   8 +-
>  tools/testing/selftests/kdbus/test-fd.c          |   2 +-
>  tools/testing/selftests/kdbus/test-message.c     |  69 ++++++++-----
>  tools/testing/selftests/kdbus/test-metadata-ns.c |  10 +-
>  tools/testing/selftests/kdbus/test-monitor.c     |   9 +-
>  tools/testing/selftests/kdbus/test-policy-ns.c   |   8 +-
>  tools/testing/selftests/kdbus/test-policy-priv.c |  48 +++++----
>  tools/testing/selftests/kdbus/test-send.c        |  84 ++++++++++++++++
>  tools/testing/selftests/kdbus/test-sync.c        |   2 +-
>  tools/testing/selftests/kdbus/test-timeout.c     |   2 +-
>  31 files changed, 732 insertions(+), 104 deletions(-)
>  create mode 100644 tools/testing/selftests/kdbus/test-send.c
>


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 4/8] lsm: smack: smack callbacks for kdbus security hooks
  2015-07-08 16:38     ` Casey Schaufler
@ 2015-07-08 20:07       ` Paul Moore
  0 siblings, 0 replies; 30+ messages in thread
From: Paul Moore @ 2015-07-08 20:07 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: Stephen Smalley, Paul Osmialowski, James Morris, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Neil Brown, Mark Rustad,
	Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni,
	Shuah Khan, Al Viro, linux-security-module, linux-kernel,
	linux-api, Karol Lewandowski, Lukasz Skalski

On Wednesday, July 08, 2015 09:38:25 AM Casey Schaufler wrote:
> On 7/8/2015 6:42 AM, Stephen Smalley wrote:
> > On 07/08/2015 06:25 AM, Paul Osmialowski wrote:

...

> > If Smack only truly needs 3 hooks, then it begs the question of why
> > there are so many other hooks defined.  Are the other hooks just to
> > support finer-grained distinctions, or is Smack's coverage incomplete?
> 
> I haven't been following kdbus closely for a while, but the original
> intent for Smack and kdbus was that it Smack controls would be on the
> objects involved, and that to accomplish that only a small number of
> hooks would be necessary. After all, Smack uses fewer hooks than SELinux
> on other things. I do agree that without a user there is no point in
> having hooks. If SELinux requires the other hooks we might want to
> hold off on asking for the hooks until the SELinux implementation is
> exposed. I also think that AppArmor should be examined as a potential
> user of the hooks, just to make sure the hooks aren't excessively
> oriented toward subject/object based security modules.

In Paul O.'s defense, we did have some discussion about the reasons for these 
hooks, although that seems like ages ago and I would need to dig through the 
archives (my inbox?) to find the reasoning for each.

However, I don't remember being very comfortable with the hooks back them 
largely due to uncertainty about how we were treating kdbus with respect to 
subjects/objects.  I think it's worth restarting that discussion now before we 
nit pick the patches themselves.

-- 
paul moore
security @ redhat


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 4/8] lsm: smack: smack callbacks for kdbus security hooks
  2015-07-08 10:25 ` [RFC 4/8] lsm: smack: smack callbacks for " Paul Osmialowski
  2015-07-08 13:42   ` Stephen Smalley
@ 2015-07-09 10:08   ` Sergei Zviagintsev
  2015-07-09 15:24     ` Casey Schaufler
  1 sibling, 1 reply; 30+ messages in thread
From: Sergei Zviagintsev @ 2015-07-09 10:08 UTC (permalink / raw)
  To: Paul Osmialowski
  Cc: Paul Moore, James Morris, Casey Schaufler, Serge E. Hallyn,
	Kees Cook, Tetsuo Handa, Stephen Smalley, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api, Karol Lewandowski, Lukasz Skalski

Hi,

On Wed, Jul 08, 2015 at 12:25:06PM +0200, Paul Osmialowski wrote:
> This adds implementation of three smack callbacks sitting behind kdbus
> security hooks as proposed by Karol Lewandowski.
> 
> Originates from:
> 
> git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)
> commit: fc3505d058c001fe72a6f66b833e0be5b2d118f3
> 
> https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd-v212)
> commit: 103c26fd27d1ec8c32d85dd3d85681f936ac66fb
> 
> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
>  security/smack/smack_lsm.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index a143328..033b756 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -41,6 +41,7 @@
>  #include <linux/msg.h>
>  #include <linux/shm.h>
>  #include <linux/binfmts.h>
> +#include <kdbus/connection.h>
>  #include "smack.h"
>  
>  #define TRANS_TRUE	"TRUE"
> @@ -3336,6 +3337,69 @@ static int smack_setprocattr(struct task_struct *p, char *name,
>  }
>  
>  /**
> + * smack_kdbus_connect - Set the security blob for a KDBus connection
> + * @conn: the connection
> + * @secctx: smack label
> + * @seclen: smack label length
> + *
> + * Returns 0
> + */
> +static int smack_kdbus_connect(struct kdbus_conn *conn,
> +			       const char *secctx, u32 seclen)
> +{
> +	struct smack_known *skp;
> +
> +	if (secctx && seclen > 0)
> +		skp = smk_import_entry(secctx, seclen);
> +	else
> +		skp = smk_of_current();
> +	conn->security = skp;
> +
> +	return 0;
> +}
> +
> +/**
> + * smack_kdbus_conn_free - Clear the security blob for a KDBus connection
> + * @conn: the connection
> + *
> + * Clears the blob pointer
> + */
> +static void smack_kdbus_conn_free(struct kdbus_conn *conn)
> +{
> +	conn->security = NULL;
> +}
> +
> +/**
> + * smack_kdbus_talk - Smack access on KDBus
> + * @src: source kdbus connection
> + * @dst: destination kdbus connection
> + *
> + * Return 0 if a subject with the smack of sock could access
> + * an object with the smack of other, otherwise an error code
> + */
> +static int smack_kdbus_talk(const struct kdbus_conn *src,
> +			    const struct kdbus_conn *dst)
> +{
> +	struct smk_audit_info ad;
> +	struct smack_known *sskp = src->security;
> +	struct smack_known *dskp = dst->security;
> +	int ret;
> +
> +	BUG_ON(sskp == NULL);
> +	BUG_ON(dskp == NULL);

I am not familiar with the smack code so far, but I see that current
security/smack/smack_lsm.c contains only one BUG_ON and this patch adds
another two.

> +
> +	if (smack_privileged(CAP_MAC_OVERRIDE))
> +		return 0;
> +
> +	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_NONE);
> +
> +	ret = smk_access(sskp, dskp, MAY_WRITE, &ad);
> +	if (ret)
> +		return ret;
> +	return 0;

Three redundant lines here.

> +}
> +
> +/**
>   * smack_unix_stream_connect - Smack access on UDS
>   * @sock: one sock
>   * @other: the other sock
> @@ -4393,6 +4457,10 @@ struct security_hook_list smack_hooks[] = {
>  	LSM_HOOK_INIT(inode_notifysecctx, smack_inode_notifysecctx),
>  	LSM_HOOK_INIT(inode_setsecctx, smack_inode_setsecctx),
>  	LSM_HOOK_INIT(inode_getsecctx, smack_inode_getsecctx),
> +
> +	LSM_HOOK_INIT(kdbus_connect, smack_kdbus_connect),
> +	LSM_HOOK_INIT(kdbus_conn_free, smack_kdbus_conn_free),
> +	LSM_HOOK_INIT(kdbus_talk, smack_kdbus_talk),
>  };
>  
>  
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
  2015-07-08 12:12     ` Paul Osmialowski
@ 2015-07-09 10:55       ` Sergei Zviagintsev
  2015-07-09 11:28         ` Paul Osmialowski
  0 siblings, 1 reply; 30+ messages in thread
From: Sergei Zviagintsev @ 2015-07-09 10:55 UTC (permalink / raw)
  To: Paul Osmialowski
  Cc: Lukasz Pawelczyk, Paul Moore, James Morris, Casey Schaufler,
	Serge E. Hallyn, Kees Cook, Tetsuo Handa, Stephen Smalley,
	Neil Brown, Mark Rustad, Greg Kroah-Hartman, Daniel Mack,
	David Herrmann, Djalal Harouni, Shuah Khan, Al Viro,
	linux-security-module, linux-kernel, linux-api,
	Karol Lewandowski, Lukasz Skalski

Hi,

On Wed, Jul 08, 2015 at 02:12:03PM +0200, Paul Osmialowski wrote:
[...]
>  
> +	ret = security_kdbus_bus_alloc(b);
> +	if (ret) {
> +		goto exit_unref;
> +	}

checkpatch.pl will complain about this


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
  2015-07-09 10:55       ` Sergei Zviagintsev
@ 2015-07-09 11:28         ` Paul Osmialowski
  0 siblings, 0 replies; 30+ messages in thread
From: Paul Osmialowski @ 2015-07-09 11:28 UTC (permalink / raw)
  To: Sergei Zviagintsev
  Cc: Lukasz Pawelczyk, Paul Moore, James Morris, Casey Schaufler,
	Serge E. Hallyn, Kees Cook, Tetsuo Handa, Stephen Smalley,
	Neil Brown, Mark Rustad, Greg Kroah-Hartman, Daniel Mack,
	David Herrmann, Djalal Harouni, Shuah Khan, Al Viro,
	linux-security-module, linux-kernel, linux-api,
	Karol Lewandowski, Lukasz Skalski, Paul Osmialowski



On Thu, 9 Jul 2015, Sergei Zviagintsev wrote:

> Hi,
>
> On Wed, Jul 08, 2015 at 02:12:03PM +0200, Paul Osmialowski wrote:
> [...]
>>
>> +	ret = security_kdbus_bus_alloc(b);
>> +	if (ret) {
>> +		goto exit_unref;
>> +	}
>
> checkpatch.pl will complain about this
>
>

It should but it didn't:

$ scripts/checkpatch.pl 0005-kdbus-use-LSM-hooks-in-kdbus-code.patch
total: 0 errors, 0 warnings, 365 lines checked

0005-kdbus-use-LSM-hooks-in-kdbus-code.patch has no obvious style problems 
and is ready for submission.


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 4/8] lsm: smack: smack callbacks for kdbus security hooks
  2015-07-09 10:08   ` Sergei Zviagintsev
@ 2015-07-09 15:24     ` Casey Schaufler
  0 siblings, 0 replies; 30+ messages in thread
From: Casey Schaufler @ 2015-07-09 15:24 UTC (permalink / raw)
  To: Sergei Zviagintsev, Paul Osmialowski
  Cc: Paul Moore, James Morris, Serge E. Hallyn, Kees Cook,
	Tetsuo Handa, Stephen Smalley, Neil Brown, Mark Rustad,
	Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni,
	Shuah Khan, Al Viro, linux-security-module, linux-kernel,
	linux-api, Karol Lewandowski, Lukasz Skalski

On 7/9/2015 3:08 AM, Sergei Zviagintsev wrote:
> Hi,
>
> On Wed, Jul 08, 2015 at 12:25:06PM +0200, Paul Osmialowski wrote:
>> This adds implementation of three smack callbacks sitting behind kdbus
>> security hooks as proposed by Karol Lewandowski.
>>
>> Originates from:
>>
>> git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)
>> commit: fc3505d058c001fe72a6f66b833e0be5b2d118f3
>>
>> https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd-v212)
>> commit: 103c26fd27d1ec8c32d85dd3d85681f936ac66fb
>>
>> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
>> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
>> ---
>>  security/smack/smack_lsm.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 68 insertions(+)
>>
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index a143328..033b756 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -41,6 +41,7 @@
>>  #include <linux/msg.h>
>>  #include <linux/shm.h>
>>  #include <linux/binfmts.h>
>> +#include <kdbus/connection.h>
>>  #include "smack.h"
>>  
>>  #define TRANS_TRUE	"TRUE"
>> @@ -3336,6 +3337,69 @@ static int smack_setprocattr(struct task_struct *p, char *name,
>>  }
>>  
>>  /**
>> + * smack_kdbus_connect - Set the security blob for a KDBus connection
>> + * @conn: the connection
>> + * @secctx: smack label
>> + * @seclen: smack label length
>> + *
>> + * Returns 0
>> + */
>> +static int smack_kdbus_connect(struct kdbus_conn *conn,
>> +			       const char *secctx, u32 seclen)
>> +{
>> +	struct smack_known *skp;
>> +
>> +	if (secctx && seclen > 0)
>> +		skp = smk_import_entry(secctx, seclen);
>> +	else
>> +		skp = smk_of_current();
>> +	conn->security = skp;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * smack_kdbus_conn_free - Clear the security blob for a KDBus connection
>> + * @conn: the connection
>> + *
>> + * Clears the blob pointer
>> + */
>> +static void smack_kdbus_conn_free(struct kdbus_conn *conn)
>> +{
>> +	conn->security = NULL;
>> +}
>> +
>> +/**
>> + * smack_kdbus_talk - Smack access on KDBus
>> + * @src: source kdbus connection
>> + * @dst: destination kdbus connection
>> + *
>> + * Return 0 if a subject with the smack of sock could access
>> + * an object with the smack of other, otherwise an error code
>> + */
>> +static int smack_kdbus_talk(const struct kdbus_conn *src,
>> +			    const struct kdbus_conn *dst)
>> +{
>> +	struct smk_audit_info ad;
>> +	struct smack_known *sskp = src->security;
>> +	struct smack_known *dskp = dst->security;
>> +	int ret;
>> +
>> +	BUG_ON(sskp == NULL);
>> +	BUG_ON(dskp == NULL);
> I am not familiar with the smack code so far, but I see that current
> security/smack/smack_lsm.c contains only one BUG_ON and this patch adds
> another two.

Smack does not use BUG_ON for routine error checking. If there
is reason to expect that these values will not be set the case
should be checked for and handled. Otherwise, I don't believe
in cluttering the code with assertions.

>
>> +
>> +	if (smack_privileged(CAP_MAC_OVERRIDE))
>> +		return 0;
>> +
>> +	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_NONE);
>> +
>> +	ret = smk_access(sskp, dskp, MAY_WRITE, &ad);
>> +	if (ret)
>> +		return ret;
>> +	return 0;
> Three redundant lines here.
>
>> +}
>> +
>> +/**
>>   * smack_unix_stream_connect - Smack access on UDS
>>   * @sock: one sock
>>   * @other: the other sock
>> @@ -4393,6 +4457,10 @@ struct security_hook_list smack_hooks[] = {
>>  	LSM_HOOK_INIT(inode_notifysecctx, smack_inode_notifysecctx),
>>  	LSM_HOOK_INIT(inode_setsecctx, smack_inode_setsecctx),
>>  	LSM_HOOK_INIT(inode_getsecctx, smack_inode_getsecctx),
>> +
>> +	LSM_HOOK_INIT(kdbus_connect, smack_kdbus_connect),
>> +	LSM_HOOK_INIT(kdbus_conn_free, smack_kdbus_conn_free),
>> +	LSM_HOOK_INIT(kdbus_talk, smack_kdbus_talk),
>>  };
>>  
>>  
>> -- 
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
  2015-07-08 13:37   ` Stephen Smalley
@ 2015-07-10 16:56     ` Stephen Smalley
  2015-07-10 18:20     ` Stephen Smalley
  1 sibling, 0 replies; 30+ messages in thread
From: Stephen Smalley @ 2015-07-10 16:56 UTC (permalink / raw)
  To: Paul Osmialowski, Paul Moore, James Morris, Casey Schaufler,
	Serge E. Hallyn, Kees Cook, Tetsuo Handa, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Lukasz Skalski

On 07/08/2015 09:37 AM, Stephen Smalley wrote:
> On 07/08/2015 06:25 AM, Paul Osmialowski wrote:
>> Originates from:
>>
>> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)
>> commit: aa0885489d19be92fa41c6f0a71df28763228a40
>>
>> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
>> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
>> ---
>>  ipc/kdbus/bus.c        | 12 ++++++++++-
>>  ipc/kdbus/bus.h        |  3 +++
>>  ipc/kdbus/connection.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  ipc/kdbus/connection.h |  4 ++++
>>  ipc/kdbus/domain.c     |  9 ++++++++-
>>  ipc/kdbus/domain.h     |  2 ++
>>  ipc/kdbus/endpoint.c   | 11 ++++++++++
>>  ipc/kdbus/names.c      | 11 ++++++++++
>>  ipc/kdbus/queue.c      | 30 ++++++++++++++++++----------
>>  9 files changed, 124 insertions(+), 12 deletions(-)
>>
>>
> 
>> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
>> index 9993753..b85cdc7 100644
>> --- a/ipc/kdbus/connection.c
>> +++ b/ipc/kdbus/connection.c
>> @@ -31,6 +31,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/syscalls.h>
>>  #include <linux/uio.h>
>> +#include <linux/security.h>
>>  
>>  #include "bus.h"
>>  #include "connection.h"
>> @@ -73,6 +74,8 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
>>  	bool is_activator;
>>  	bool is_monitor;
>>  	struct kvec kvec;
>> +	u32 sid, len;
>> +	char *label;
>>  	int ret;
>>  
>>  	struct {
>> @@ -222,6 +225,14 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
>>  		}
>>  	}
>>  
>> +	security_task_getsecid(current, &sid);
>> +	security_secid_to_secctx(sid, &label, &len);
>> +	ret = security_kdbus_connect(conn, label, len);
>> +	if (ret) {
>> +		ret = -EPERM;
>> +		goto exit_unref;
>> +	}
> 
> This seems convoluted and expensive.  If you always want the label of
> the current task here, then why not just have security_kdbus_connect()
> internally extract the label of the current task?
> 
>> @@ -1107,6 +1119,12 @@ static int kdbus_conn_reply(struct kdbus_conn *src, struct kdbus_kmsg *kmsg)
>>  	if (ret < 0)
>>  		goto exit;
>>  
>> +	ret = security_kdbus_talk(src, dst);
>> +	if (ret) {
>> +		ret = -EPERM;
>> +		goto exit;
>> +	}
> 
> Where does kdbus apply its uid-based or other restrictions on
> connections?  Why do we need to insert separate hooks into each of these
> functions?  Is there no central chokepoint already for permission
> checking that we can hook?

For example, why wouldn't you insert a single hook into
kdbus_conn_policy_talk() where they perform their DAC checking?
You would need to restructure it slightly to ensure that the security
hook is only called if it passes the DAC (privileged || uid_eq) check so
that we do not trigger MAC denials when DAC wouldn't have allowed it
anyway.  Also, kdbus_conn_policy_talk() takes a separate conn_creds
argument - that should be passed through to the hook as well.



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code
  2015-07-08 13:37   ` Stephen Smalley
  2015-07-10 16:56     ` Stephen Smalley
@ 2015-07-10 18:20     ` Stephen Smalley
  1 sibling, 0 replies; 30+ messages in thread
From: Stephen Smalley @ 2015-07-10 18:20 UTC (permalink / raw)
  To: Paul Osmialowski, Paul Moore, James Morris, Casey Schaufler,
	Serge E. Hallyn, Kees Cook, Tetsuo Handa, Neil Brown,
	Mark Rustad, Greg Kroah-Hartman, Daniel Mack, David Herrmann,
	Djalal Harouni, Shuah Khan, Al Viro, linux-security-module,
	linux-kernel, linux-api
  Cc: Karol Lewandowski, Lukasz Skalski

On 07/08/2015 09:37 AM, Stephen Smalley wrote:
> On 07/08/2015 06:25 AM, Paul Osmialowski wrote:
>> Originates from:
>>
>> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212)
>> commit: aa0885489d19be92fa41c6f0a71df28763228a40
>>
>> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
>> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
>> ---
>>  ipc/kdbus/bus.c        | 12 ++++++++++-
>>  ipc/kdbus/bus.h        |  3 +++
>>  ipc/kdbus/connection.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  ipc/kdbus/connection.h |  4 ++++
>>  ipc/kdbus/domain.c     |  9 ++++++++-
>>  ipc/kdbus/domain.h     |  2 ++
>>  ipc/kdbus/endpoint.c   | 11 ++++++++++
>>  ipc/kdbus/names.c      | 11 ++++++++++
>>  ipc/kdbus/queue.c      | 30 ++++++++++++++++++----------
>>  9 files changed, 124 insertions(+), 12 deletions(-)
>>
>>
> 
>> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
>> index 9993753..b85cdc7 100644
>> --- a/ipc/kdbus/connection.c
>> +++ b/ipc/kdbus/connection.c
>> @@ -31,6 +31,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/syscalls.h>
>>  #include <linux/uio.h>
>> +#include <linux/security.h>
>>  
>>  #include "bus.h"
>>  #include "connection.h"
>> @@ -73,6 +74,8 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
>>  	bool is_activator;
>>  	bool is_monitor;
>>  	struct kvec kvec;
>> +	u32 sid, len;
>> +	char *label;
>>  	int ret;
>>  
>>  	struct {
>> @@ -222,6 +225,14 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
>>  		}
>>  	}
>>  
>> +	security_task_getsecid(current, &sid);
>> +	security_secid_to_secctx(sid, &label, &len);
>> +	ret = security_kdbus_connect(conn, label, len);
>> +	if (ret) {
>> +		ret = -EPERM;
>> +		goto exit_unref;
>> +	}
> 
> This seems convoluted and expensive.  If you always want the label of
> the current task here, then why not just have security_kdbus_connect()
> internally extract the label of the current task?

Furthermore, why do we need a separate security field and copy of the
current label in the conn->security, when we already have
conn->cred->security available to us?

I don't think we need new security fields unless we are going to assign
some kind of object labeling to these structures separate from their
cred, and offhand I don't see why we would do that.

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2015-07-10 19:23 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-08 10:25 [RFC 0/8] Introduce LSM to KDBUS Paul Osmialowski
2015-07-08 10:25 ` [RFC 1/8] lsm: make security_file_receive available for external modules Paul Osmialowski
2015-07-08 10:25 ` [RFC 2/8] lsm: smack: Make ipc/kdbus includes visible so smack callbacks could see them Paul Osmialowski
2015-07-08 16:43   ` Daniel Mack
2015-07-08 10:25 ` [RFC 3/8] lsm: kdbus security hooks Paul Osmialowski
2015-07-08 11:00   ` Lukasz Pawelczyk
2015-07-08 14:14   ` Greg Kroah-Hartman
2015-07-08 10:25 ` [RFC 4/8] lsm: smack: smack callbacks for " Paul Osmialowski
2015-07-08 13:42   ` Stephen Smalley
2015-07-08 16:38     ` Casey Schaufler
2015-07-08 20:07       ` Paul Moore
2015-07-09 10:08   ` Sergei Zviagintsev
2015-07-09 15:24     ` Casey Schaufler
2015-07-08 10:25 ` [RFC 5/8] kdbus: use LSM hooks in kdbus code Paul Osmialowski
2015-07-08 11:06   ` Lukasz Pawelczyk
2015-07-08 11:09   ` Lukasz Pawelczyk
2015-07-08 12:12     ` Paul Osmialowski
2015-07-09 10:55       ` Sergei Zviagintsev
2015-07-09 11:28         ` Paul Osmialowski
2015-07-08 13:37   ` Stephen Smalley
2015-07-10 16:56     ` Stephen Smalley
2015-07-10 18:20     ` Stephen Smalley
2015-07-08 14:13   ` Greg Kroah-Hartman
2015-07-08 16:24   ` Casey Schaufler
2015-07-08 10:25 ` [RFC 6/8] kdbus: TEST_CREATE_CONN now does no depend on TEST_CREATE_BUS Paul Osmialowski
2015-07-08 10:25 ` [RFC 7/8] kdbus: selftests extended Paul Osmialowski
2015-07-08 10:25 ` [RFC 8/8] kdbus: Ability to run kdbus test by executable binary name Paul Osmialowski
2015-07-08 14:16   ` Greg Kroah-Hartman
2015-07-08 14:58     ` Paul Osmialowski
2015-07-08 16:46 ` [RFC 0/8] Introduce LSM to KDBUS Casey Schaufler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).