On 10/07/2015 07:08 PM, Paul Moore wrote:
Add LSM access control hooks to kdbus; several new hooks are added and
the existing security_file_receive() hook is reused. The new hooks
are listed below:
* security_kdbus_conn_new
Check if the current task is allowed to create a new kdbus
connection.
* security_kdbus_own_name
Check if a connection is allowed to own a kdbus service name.
* security_kdbus_conn_talk
Check if a connection is allowed to talk to a kdbus peer.
* security_kdbus_conn_see
Check if a connection can see a kdbus peer.
* security_kdbus_conn_see_name
Check if a connection can see a kdbus service name.
* security_kdbus_conn_see_notification
Check if a connection can receive notifications.
* security_kdbus_proc_permission
Check if a connection can access another task's pid namespace info.
* security_kdbus_init_inode
Set the security label on a kdbusfs inode
Signed-off-by: Paul Moore <pmo...@redhat.com>
---
ChangeLog:
- v3
* Ported to the 4.3-rc4 based kdbus tree
- v2
* Implemented suggestions by Stephen Smalley
* call security_kdbus_conn_new() sooner
* reworked hook inside kdbus_conn_policy_own_name()
* fixed if-conditional in kdbus_conn_policy_talk()
* reworked hook inside kdbus_conn_policy_see_name_unlocked()
* reworked hook inside kdbus_conn_policy_see()
* reworked hook inside kdbus_conn_policy_see_notification()
* added the security_kdbus_init_inode() hook
- v1
* Initial draft
---
include/linux/lsm_hooks.h | 63 +++++++++++++++++++++++++++++++++++++++
include/linux/security.h | 71 ++++++++++++++++++++++++++++++++++++++++++++
ipc/kdbus/connection.c | 73 +++++++++++++++++++++++++++++----------------
ipc/kdbus/fs.c | 6 ++++
ipc/kdbus/message.c | 19 +++++++++---
ipc/kdbus/metadata.c | 6 +---
security/security.c | 62 ++++++++++++++++++++++++++++++++++++++
7 files changed, 265 insertions(+), 35 deletions(-)
diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index ef63d65..1cb87b3 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -26,6 +26,7 @@
#include <linux/path.h>
#include <linux/poll.h>
#include <linux/sched.h>
+#include <linux/security.h>
#include <linux/shmem_fs.h>
#include <linux/sizes.h>
#include <linux/slab.h>
@@ -108,6 +109,14 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep
*ep,
if (!owner && (creds || pids || seclabel))
return ERR_PTR(-EPERM);
+ ret = security_kdbus_conn_new(get_cred(file->f_cred),
You only need to use get_cred() if saving a reference; otherwise, you'll
leak one here. Also, do we want file->f_cred here or
ep->bus->node.creds (the latter is what is used by their own checks; the
former is typically the same as current cred IIUC). For that matter,
what about ep->node.creds vs ep->bus->node.creds vs.
ep->bus->domain->node.creds? Can they differ? Do we care?
+ creds, pids, seclabel,
+ owner, privileged,
+ is_activator, is_monitor,
+ is_policy_holder);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
ret = kdbus_sanitize_attach_flags(hello->attach_flags_send,
&attach_flags_send);
if (ret < 0)
@@ -1435,12 +1444,12 @@ bool kdbus_conn_policy_own_name(struct kdbus_conn *conn,
return false;
}
- if (conn->owner)
- return true;
+ if (!conn->owner &&
+ kdbus_policy_query(&conn->ep->bus->policy_db, conn_creds, name,
+ hash) < KDBUS_POLICY_OWN)
+ return false;
- res = kdbus_policy_query(&conn->ep->bus->policy_db, conn_creds,
- name, hash);
- return res >= KDBUS_POLICY_OWN;
+ return (security_kdbus_own_name(conn_creds, name) == 0);
Similar question here. conn_creds is the credentials of the creator of
the connection, typically the client/sender, right?
conn->ep->bus->node.creds are the credentials of the bus owner, so don't
we want to ask "Can I own this name on this bus?". Note that their
policy checks are based on conn->ep->policy_db, i.e. the policy
associated with the endpoint, and conn->owner is only true if the
connection creator has the same uid as the bus.
}
/**
@@ -1465,14 +1474,13 @@ bool kdbus_conn_policy_talk(struct kdbus_conn *conn,
to, KDBUS_POLICY_TALK))
return false;
- if (conn->owner)
- return true;
- if (uid_eq(conn_creds->euid, to->cred->uid))
- return true;
+ if (!conn->owner && !uid_eq(conn_creds->euid, to->cred->uid) &&
+ !kdbus_conn_policy_query_all(conn, conn_creds,
+ &conn->ep->bus->policy_db, to,
+ KDBUS_POLICY_TALK))
+ return false;
- return kdbus_conn_policy_query_all(conn, conn_creds,
- &conn->ep->bus->policy_db, to,
- KDBUS_POLICY_TALK);
+ return (security_kdbus_conn_talk(conn_creds, to->cred) == 0);
Here at least we have a notion of client and peer. But we still aren't
considering conn->ep or conn->ep->bus, whereas they are querying both
policy dbs for their decision. The parallel would be checking access to
the labels of both I suppose, unless we institute a control up front
over the relationship between the label of the endpoint and the label of
the bus.
}
/**
@@ -1491,19 +1499,19 @@ bool kdbus_conn_policy_see_name_unlocked(struct
kdbus_conn *conn,
const struct cred *conn_creds,
const char *name)
{
- int res;
+ if (!conn_creds)
+ conn_creds = conn->cred;
/*
* By default, all names are visible on a bus. SEE policies can only be
* installed on custom endpoints, where by default no name is visible.
*/
- if (!conn->ep->user)
- return true;
+ if (conn->ep->user &&
+ kdbus_policy_query_unlocked(&conn->ep->policy_db, conn_creds, name,
+ kdbus_strhash(name)) < KDBUS_POLICY_SEE)
+ return false;
- res = kdbus_policy_query_unlocked(&conn->ep->policy_db,
- conn_creds ? : conn->cred,
- name, kdbus_strhash(name));
- return res >= KDBUS_POLICY_SEE;
+ return (security_kdbus_conn_see_name(conn_creds, name) == 0);
Here they only define policy based on endpoints, not bus. Not sure what
we want, but we need at least one of their creds. Same for the rest.
}
static bool kdbus_conn_policy_see_name(struct kdbus_conn *conn,
@@ -1523,6 +1531,9 @@ static bool kdbus_conn_policy_see(struct kdbus_conn *conn,
const struct cred *conn_creds,
struct kdbus_conn *whom)
{
+ if (!conn_creds)
+ conn_creds = conn->cred;
+
/*
* By default, all names are visible on a bus, so a connection can
* always see other connections. SEE policies can only be installed on
@@ -1530,10 +1541,13 @@ static bool kdbus_conn_policy_see(struct kdbus_conn
*conn,
* peers from each other, unless you see at least _one_ name of the
* peer.
*/
- return !conn->ep->user ||
- kdbus_conn_policy_query_all(conn, conn_creds,
- &conn->ep->policy_db, whom,
- KDBUS_POLICY_SEE);
+ if (conn->ep->user &&
+ !kdbus_conn_policy_query_all(conn, conn_creds,
+ &conn->ep->policy_db, whom,
+ KDBUS_POLICY_SEE))
+ return false;
+
+ return (security_kdbus_conn_see(conn_creds, whom->cred) == 0);
}
/**
@@ -1551,6 +1565,9 @@ bool kdbus_conn_policy_see_notification(struct kdbus_conn
*conn,
const struct cred *conn_creds,
const struct kdbus_msg *msg)
{
+ if (!conn_creds)
+ conn_creds = conn->cred;
+
/*
* Depending on the notification type, broadcasted kernel notifications
* have to be filtered:
@@ -1567,18 +1584,22 @@ bool kdbus_conn_policy_see_notification(struct
kdbus_conn *conn,
case KDBUS_ITEM_NAME_ADD:
case KDBUS_ITEM_NAME_REMOVE:
case KDBUS_ITEM_NAME_CHANGE:
- return kdbus_conn_policy_see_name(conn, conn_creds,
- msg->items[0].name_change.name);
+ if (!kdbus_conn_policy_see_name(conn, conn_creds,
+ msg->items[0].name_change.name))
+ return false;
case KDBUS_ITEM_ID_ADD:
case KDBUS_ITEM_ID_REMOVE:
- return true;
+ /* fall through for the LSM check */
+ break;
default:
WARN(1, "Invalid type for notification broadcast: %llu\n",
(unsigned long long)msg->items[0].type);
return false;
}
+
+ return (security_kdbus_conn_see_notification(conn_creds) == 0);
}
/**
diff --git a/ipc/kdbus/fs.c b/ipc/kdbus/fs.c
index 68818a8..4e84e89 100644
--- a/ipc/kdbus/fs.c
+++ b/ipc/kdbus/fs.c
@@ -23,6 +23,7 @@
#include <linux/namei.h>
#include <linux/pagemap.h>
#include <linux/sched.h>
+#include <linux/security.h>
#include <linux/slab.h>
#include "bus.h"
@@ -192,6 +193,7 @@ static const struct inode_operations fs_inode_iops = {
static struct inode *fs_inode_get(struct super_block *sb,
struct kdbus_node *node)
{
+ int ret;
struct inode *inode;
inode = iget_locked(sb, node->id);
@@ -200,6 +202,10 @@ static struct inode *fs_inode_get(struct super_block *sb,
if (!(inode->i_state & I_NEW))
return inode;
+ ret = security_kdbus_init_inode(inode, node->creds);
+ if (ret)
+ return ERR_PTR(ret);
Need to put the inode.
+
inode->i_private = kdbus_node_ref(node);
inode->i_mapping->a_ops = &empty_aops;
inode->i_mode = node->mode & S_IALLUGO;
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
