Re: kdbus: add connection, queue handling and message validation code

2 messages, 2 authors, 2014-10-30 · open the first message on its own page

Re: kdbus: add connection, queue handling and message validation code

From: Andy Lutomirski <luto@amacapital.net>
Date: 2014-10-30 03:56:23

On Wed, Oct 29, 2014 at 8:47 PM, Eric W. Biederman
[off-list ref] wrote:
Greg Kroah-Hartman [off-list ref] writes:
quoted
From: Daniel Mack <redacted>

This patch adds code to create and destroy connections, to validate
incoming messages and to maintain the queue of messages that are
associated with a connection.

Note that connection and queue have a 1:1 relation, the code is only
split in two parts for cleaner separation and better readability.
You are not performing capability checks at open time.

As such this API is suceptible to a host of file descriptor passing attacks.
To be fair, write(2) doesn't work on these fds, so the usual attacks
don't work.  But who knows what absurd things kdbus clients will do
with fd passing?

--Andy
quoted
Signed-off-by: Daniel Mack <redacted>
Signed-off-by: Greg Kroah-Hartman <redacted>
---
quoted
+/*
+ * Check for maximum number of messages per individual user. This
+ * should prevent a single user from being able to fill the receiver's
+ * queue.
+ */
+static int kdbus_conn_queue_user_quota(struct kdbus_conn *conn,
+                                    const struct kdbus_conn *conn_src,
+                                    struct kdbus_queue_entry *entry)
+{
+     unsigned int user;
+
+     if (!conn_src)
+             return 0;
+
+     if (ns_capable(&init_user_ns, CAP_IPC_OWNER))
+             return 0;


-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: kdbus: add connection, queue handling and message validation code

From: Djalal Harouni <hidden>
Date: 2014-10-30 09:06:59

On Wed, Oct 29, 2014 at 08:55:58PM -0700, Andy Lutomirski wrote:
On Wed, Oct 29, 2014 at 8:47 PM, Eric W. Biederman
[off-list ref] wrote:
quoted
Greg Kroah-Hartman [off-list ref] writes:
quoted
From: Daniel Mack <daniel@zonque.org>

This patch adds code to create and destroy connections, to validate
incoming messages and to maintain the queue of messages that are
associated with a connection.

Note that connection and queue have a 1:1 relation, the code is only
split in two parts for cleaner separation and better readability.
You are not performing capability checks at open time.

As such this API is suceptible to a host of file descriptor passing attacks.
To be fair, write(2) doesn't work on these fds, so the usual attacks
don't work.  But who knows what absurd things kdbus clients will do
with fd passing?
Yes, we use ioctl() so we are safe here! if there is a a suid process
that does perform arbitrary ioctl() on intrusted passed fds,
then we are already in truble given all the already available ioctl()
(not only kdbus, all available ioctl()... we blame the client), so yes
usual write()/read() do not work here.

But we do perform the creds check against the cred of connection
creation time, if you open the fd you do not have the connection,
you still need a KDBUS_CMD_HELLO ioctl() on the fd, and during that time
we store the creds, and we perform all the TALK, SEE and OWN against
those creds (uid/gid). It is like a second connect() call, unless you
perform the KDBUS_CMD_HELLO you are not connected, and after turning
your fd to a connection, a service can restrict its access (TALK, OWN
and SEE) policies, not all connected peers can TALK (send messages) to
a service.


-- 
Djalal Harouni
http://opendz.org
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help