public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Herrmann <dh.herrmann@gmail.com>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Daniel Mack <daniel@zonque.org>,
	Djalal Harouni <tixxdz@opendz.org>,
	David Herrmann <dh.herrmann@gmail.com>
Subject: [PATCH 4/9] kdbus: move privilege checking in kdbus_conn_new()
Date: Thu,  6 Aug 2015 10:21:23 +0200	[thread overview]
Message-ID: <1438849288-18112-5-git-send-email-dh.herrmann@gmail.com> (raw)
In-Reply-To: <1438849288-18112-1-git-send-email-dh.herrmann@gmail.com>

Instead of relying on handle.c to perform privilege evaluation and
passing information along, move this into kdbus_conn_new(). This has the
benefit that we can now split 'owner' level and 'privileged' levels
apart. This will be required for following extensions that need to
distinguish both cases.

Also, pass on 'struct file*' from handle into kdbus_conn_new(). Most
kernel helpers cannot accept 'struct cred*' but instead only operate on
files (and access file->f_cred then).

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 ipc/kdbus/connection.c | 41 ++++++++++++++++++++++++++++++++---------
 ipc/kdbus/connection.h |  6 ++++--
 ipc/kdbus/handle.c     |  2 +-
 3 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index 2787bd7..243cbc7 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -52,7 +52,8 @@
 #define KDBUS_CONN_ACTIVE_BIAS	(INT_MIN + 2)
 #define KDBUS_CONN_ACTIVE_NEW	(INT_MIN + 1)
 
-static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
+static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep,
+					 struct file *file,
 					 struct kdbus_cmd_hello *hello,
 					 const char *name,
 					 const struct kdbus_creds *creds,
@@ -72,6 +73,8 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
 	bool is_policy_holder;
 	bool is_activator;
 	bool is_monitor;
+	bool privileged;
+	bool owner;
 	struct kvec kvec;
 	int ret;
 
@@ -81,6 +84,25 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
 		struct kdbus_bloom_parameter bloom;
 	} bloom_item;
 
+	/*
+	 * A connection is considered privileged, if, and only if, it didn't
+	 * connect through a custom endpoint *and* it has CAP_IPC_OWNER on the
+	 * namespace of the current domain.
+	 * Additionally, a connection is considered equivalent to the bus owner
+	 * if it didn't connect through a custom endpoint *and* it either is
+	 * privileged or the same user as the bus owner.
+	 *
+	 * Bus owners and alike can bypass bus policies. Privileged connections
+	 * can additionally change accounting, modify kernel resources and
+	 * perform restricted operations, as long as they're privileged on the
+	 * same level as the resources they touch.
+	 */
+	privileged = !ep->user &&
+		     file_ns_capable(file, ep->bus->domain->user_namespace,
+				     CAP_IPC_OWNER);
+	owner = !ep->user &&
+		(privileged || uid_eq(file->f_cred->euid, ep->bus->node.uid));
+
 	is_monitor = hello->flags & KDBUS_HELLO_MONITOR;
 	is_activator = hello->flags & KDBUS_HELLO_ACTIVATOR;
 	is_policy_holder = hello->flags & KDBUS_HELLO_POLICY_HOLDER;
@@ -97,9 +119,9 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
 		return ERR_PTR(-EINVAL);
 	if (is_monitor && ep->user)
 		return ERR_PTR(-EOPNOTSUPP);
-	if (!privileged && (is_activator || is_policy_holder || is_monitor))
+	if (!owner && (is_activator || is_policy_holder || is_monitor))
 		return ERR_PTR(-EPERM);
-	if ((creds || pids || seclabel) && !privileged)
+	if (!owner && (creds || pids || seclabel))
 		return ERR_PTR(-EPERM);
 
 	ret = kdbus_sanitize_attach_flags(hello->attach_flags_send,
@@ -129,12 +151,13 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
 	atomic_set(&conn->request_count, 0);
 	atomic_set(&conn->lost_count, 0);
 	INIT_DELAYED_WORK(&conn->work, kdbus_reply_list_scan_work);
-	conn->cred = get_current_cred();
+	conn->cred = get_cred(file->f_cred);
 	conn->pid = get_pid(task_pid(current));
 	get_fs_root(current->fs, &conn->root_path);
 	init_waitqueue_head(&conn->wait);
 	kdbus_queue_init(&conn->queue);
 	conn->privileged = privileged;
+	conn->owner = owner;
 	conn->ep = kdbus_ep_ref(ep);
 	conn->id = atomic64_inc_return(&bus->domain->last_id);
 	conn->flags = hello->flags;
@@ -1418,7 +1441,7 @@ bool kdbus_conn_policy_own_name(struct kdbus_conn *conn,
 			return false;
 	}
 
-	if (conn->privileged)
+	if (conn->owner)
 		return true;
 
 	res = kdbus_policy_query(&conn->ep->bus->policy_db, conn_creds,
@@ -1448,7 +1471,7 @@ bool kdbus_conn_policy_talk(struct kdbus_conn *conn,
 					 to, KDBUS_POLICY_TALK))
 		return false;
 
-	if (conn->privileged)
+	if (conn->owner)
 		return true;
 	if (uid_eq(conn_creds->euid, to->cred->uid))
 		return true;
@@ -1567,12 +1590,12 @@ bool kdbus_conn_policy_see_notification(struct kdbus_conn *conn,
 /**
  * kdbus_cmd_hello() - handle KDBUS_CMD_HELLO
  * @ep:			Endpoint to operate on
- * @privileged:		Whether the caller is privileged
+ * @file:		File this connection is opened on
  * @argp:		Command payload
  *
  * Return: NULL or newly created connection on success, ERR_PTR on failure.
  */
-struct kdbus_conn *kdbus_cmd_hello(struct kdbus_ep *ep, bool privileged,
+struct kdbus_conn *kdbus_cmd_hello(struct kdbus_ep *ep, struct file *file,
 				   void __user *argp)
 {
 	struct kdbus_cmd_hello *cmd;
@@ -1607,7 +1630,7 @@ struct kdbus_conn *kdbus_cmd_hello(struct kdbus_ep *ep, bool privileged,
 
 	item_name = argv[1].item ? argv[1].item->str : NULL;
 
-	c = kdbus_conn_new(ep, privileged, cmd, item_name,
+	c = kdbus_conn_new(ep, file, cmd, item_name,
 			   argv[2].item ? &argv[2].item->creds : NULL,
 			   argv[3].item ? &argv[3].item->pids : NULL,
 			   argv[4].item ? argv[4].item->str : NULL,
diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
index 5ee864e..8e0180a 100644
--- a/ipc/kdbus/connection.h
+++ b/ipc/kdbus/connection.h
@@ -73,7 +73,8 @@ struct kdbus_staging;
  * @activator_of:	Well-known name entry this connection acts as an
  * @names_list:		List of well-known names
  * @names_queue_list:	Well-known names this connection waits for
- * @privileged:		Whether this connection is privileged on the bus
+ * @privileged:		Whether this connection is privileged on the domain
+ * @owner:		Owned by the same user as the bus owner
  */
 struct kdbus_conn {
 	struct kref kref;
@@ -116,6 +117,7 @@ struct kdbus_conn {
 	struct list_head names_queue_list;
 
 	bool privileged:1;
+	bool owner:1;
 };
 
 struct kdbus_conn *kdbus_conn_ref(struct kdbus_conn *conn);
@@ -154,7 +156,7 @@ bool kdbus_conn_policy_see_notification(struct kdbus_conn *conn,
 					const struct kdbus_msg *msg);
 
 /* command dispatcher */
-struct kdbus_conn *kdbus_cmd_hello(struct kdbus_ep *ep, bool privileged,
+struct kdbus_conn *kdbus_cmd_hello(struct kdbus_ep *ep, struct file *file,
 				   void __user *argp);
 int kdbus_cmd_byebye_unlocked(struct kdbus_conn *conn, void __user *argp);
 int kdbus_cmd_conn_info(struct kdbus_conn *conn, void __user *argp);
diff --git a/ipc/kdbus/handle.c b/ipc/kdbus/handle.c
index e0e06b0..a93c385 100644
--- a/ipc/kdbus/handle.c
+++ b/ipc/kdbus/handle.c
@@ -431,7 +431,7 @@ static long kdbus_handle_ioctl_ep(struct file *file, unsigned int cmd,
 		break;
 
 	case KDBUS_CMD_HELLO:
-		conn = kdbus_cmd_hello(file_ep, handle->privileged, buf);
+		conn = kdbus_cmd_hello(file_ep, file, buf);
 		if (IS_ERR_OR_NULL(conn)) {
 			ret = PTR_ERR_OR_ZERO(conn);
 			break;
-- 
2.5.0


  parent reply	other threads:[~2015-08-06  8:24 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-06  8:21 [PATCH 0/9] kdbus: set of random fixes David Herrmann
2015-08-06  8:21 ` [PATCH 1/9] kdbus: return EBADSLT on replies without slot David Herrmann
2015-08-06  8:21 ` [PATCH 2/9] kdbus: reduce stack buffer to 256 bytes David Herrmann
2015-08-06  8:21 ` [PATCH 3/9] kdbus: use separate counter for message IDs David Herrmann
2015-08-06  8:21 ` David Herrmann [this message]
2015-08-06  8:21 ` [PATCH 5/9] kdbus: perform accounting on proxied uids David Herrmann
2015-08-06  8:21 ` [PATCH 6/9] kdbus: inline privilege checks David Herrmann
2015-08-06  8:21 ` [PATCH 7/9] kdbus: consolidate common code David Herrmann
2015-08-06  8:21 ` [PATCH 8/9] kdbus/samples: skip if __NR_memfd_create is not defined David Herrmann
2015-08-06  8:21 ` [PATCH 9/9] kdbus/tests: properly parse KDBUS_CMD_LIST objects David Herrmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1438849288-18112-5-git-send-email-dh.herrmann@gmail.com \
    --to=dh.herrmann@gmail.com \
    --cc=daniel@zonque.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tixxdz@opendz.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox