[PATCH 0/5] audit: add restricted capability read-only netlink multicast socket

[PATCH 0/5] audit: add restricted capability read-only netlink multicast socket

[Richard Guy Briggs]

Hi, 

This patch set adds a restricted capability read-only netlink multicast socket
to kaudit to enable userspace clients such as systemd to consume audit logs, in
addition to the existing bidirectional auditd userspace client. 
    
Currently, auditd has the CAP_AUDIT_CONTROL and CAP_AUDIT_WRITE capabilities
(both use CAP_NET_ADMIN).  The CAP_AUDIT_READ capability will be added for use
by read-only AUDIT_NLGRP_READLOG multicast group clients to the kaudit
subsystem.
  
This is accomplished by modifying the optional netlink per-protocol bind
function to return an error code.

https://bugzilla.redhat.com/show_bug.cgi?id=887992 

It needs a bit of massage to get past checkpatch.pl...

First posted:	https://www.redhat.com/archives/linux-audit/2013-January/msg00008.html
		https://lkml.org/lkml/2013/1/27/279

Richard Guy Briggs (5):
  audit: move kaudit thread start from auditd registration to kaudit
    init
  netlink: have netlink per-protocol bind function return an error
    code.
  audit: add netlink audit protocol bind to check capabilities on
    multicast join
  audit: add netlink multicast group for log read
  audit: send multicast messages only if there are listeners

 include/linux/netlink.h             |    2 +-
 include/uapi/linux/audit.h          |    8 ++++
 include/uapi/linux/capability.h     |    7 +++-
 kernel/audit.c                      |   66 +++++++++++++++++++++++++++++-----
 net/netfilter/nfnetlink.c           |    6 ++-
 net/netlink/af_netlink.c            |   30 +++++++++-------
 net/netlink/af_netlink.h            |    4 +-
 security/selinux/include/classmap.h |    2 +-
 8 files changed, 95 insertions(+), 30 deletions(-)

[PATCH 1/5] audit: move kaudit thread start from auditd registration to kaudit init

[Richard Guy Briggs]

The kauditd_thread() task was started only after the auditd userspace daemon
registers itself with kaudit.  This was fine when only auditd consumed messages
from the kaudit netlink unicast socket.  With the addition of a multicast group
to that socket it is more convenient to have the thread start on init of the
kaudit kernel subsystem.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 34c5a23..b5b2f72 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -768,16 +768,6 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 	if (err)
 		return err;
 
-	/* As soon as there's any sign of userspace auditd,
-	 * start kauditd to talk to it */
-	if (!kauditd_task) {
-		kauditd_task = kthread_run(kauditd_thread, NULL, "kauditd");
-		if (IS_ERR(kauditd_task)) {
-			err = PTR_ERR(kauditd_task);
-			kauditd_task = NULL;
-			return err;
-		}
-	}
 	seq  = nlh->nlmsg_seq;
 	data = nlmsg_data(nlh);
 
@@ -1116,6 +1106,11 @@ static int __init audit_init(void)
 	audit_enabled = audit_default;
 	audit_ever_enabled |= !!audit_default;
 
+	kauditd_task = kthread_run(kauditd_thread, NULL, "kauditd");
+	if (IS_ERR(kauditd_task))
+		printk(KERN_ERR "audit: error starting kauditd_thread (%ld)\n",
+			PTR_ERR(kauditd_task));
+
 	audit_log(NULL, GFP_KERNEL, AUDIT_KERNEL, "initialized");
 
 	for (i = 0; i < AUDIT_INODE_BUCKETS; i++)
-- 
1.7.1

[PATCH 2/5] netlink: have netlink per-protocol bind function return an error code.

[Richard Guy Briggs]

Have the netlink per-protocol optional bind function return an error code
rather than void to signal a failure.

This will enable netlink protocols to perform extra checks including
capabilities and permissions verifications when updating memberships in
multicast groups.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 include/linux/netlink.h   |    2 +-
 net/netfilter/nfnetlink.c |    6 ++++--
 net/netlink/af_netlink.c  |   30 +++++++++++++++++-------------
 net/netlink/af_netlink.h  |    4 ++--
 4 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 7a6c396..4402653 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -45,7 +45,7 @@ struct netlink_kernel_cfg {
 	unsigned int	flags;
 	void		(*input)(struct sk_buff *skb);
 	struct mutex	*cb_mutex;
-	void		(*bind)(int group);
+	int		(*bind)(int group);
 	bool		(*compare)(struct net *net, struct sock *sk);
 };
 
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 046aa13..0edc4d6 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -392,7 +392,7 @@ static void nfnetlink_rcv(struct sk_buff *skb)
 }
 
 #ifdef CONFIG_MODULES
-static void nfnetlink_bind(int group)
+static int nfnetlink_bind(int group)
 {
 	const struct nfnetlink_subsystem *ss;
 	int type = nfnl_group2type[group];
@@ -402,9 +402,11 @@ static void nfnetlink_bind(int group)
 	if (!ss) {
 		rcu_read_unlock();
 		request_module("nfnetlink-subsys-%d", type);
-		return;
+		return 0;
 	}
 	rcu_read_unlock();
+
+	return 0;
 }
 #endif
 
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index bca50b9..755912f 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1198,7 +1198,7 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol,
 	struct module *module = NULL;
 	struct mutex *cb_mutex;
 	struct netlink_sock *nlk;
-	void (*bind)(int group);
+	int (*bind)(int group);
 	int err = 0;
 
 	sock->state = SS_UNCONNECTED;
@@ -1441,6 +1441,17 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr,
 	if (!nladdr->nl_groups && (nlk->groups == NULL || !(u32)nlk->groups[0]))
 		return 0;
 
+	if (nlk->netlink_bind && nladdr->nl_groups) {
+		int i;
+
+		for (i=0; i<nlk->ngroups; i++)
+			if (test_bit(i, (long unsigned int *)&nladdr->nl_groups)) {
+				err = nlk->netlink_bind(i);
+				if (err)
+					return err;
+			}
+	}
+
 	netlink_table_grab();
 	netlink_update_subscriptions(sk, nlk->subscriptions +
 					 hweight32(nladdr->nl_groups) -
@@ -1449,15 +1460,6 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr,
 	netlink_update_listeners(sk);
 	netlink_table_ungrab();
 
-	if (nlk->netlink_bind && nlk->groups[0]) {
-		int i;
-
-		for (i=0; i<nlk->ngroups; i++) {
-			if (test_bit(i, nlk->groups))
-				nlk->netlink_bind(i);
-		}
-	}
-
 	return 0;
 }
 
@@ -2095,14 +2097,16 @@ static int netlink_setsockopt(struct socket *sock, int level, int optname,
 			return err;
 		if (!val || val - 1 >= nlk->ngroups)
 			return -EINVAL;
+		if (nlk->netlink_bind) {
+			err = nlk->netlink_bind(val);
+			if (err)
+				return err;
+		}
 		netlink_table_grab();
 		netlink_update_socket_mc(nlk, val,
 					 optname == NETLINK_ADD_MEMBERSHIP);
 		netlink_table_ungrab();
 
-		if (nlk->netlink_bind)
-			nlk->netlink_bind(val);
-
 		err = 0;
 		break;
 	}
diff --git a/net/netlink/af_netlink.h b/net/netlink/af_netlink.h
index acbd774..0edb8d5 100644
--- a/net/netlink/af_netlink.h
+++ b/net/netlink/af_netlink.h
@@ -37,7 +37,7 @@ struct netlink_sock {
 	struct mutex		*cb_mutex;
 	struct mutex		cb_def_mutex;
 	void			(*netlink_rcv)(struct sk_buff *skb);
-	void			(*netlink_bind)(int group);
+	int			(*netlink_bind)(int group);
 	struct module		*module;
 #ifdef CONFIG_NETLINK_MMAP
 	struct mutex		pg_vec_lock;
@@ -73,7 +73,7 @@ struct netlink_table {
 	unsigned int		groups;
 	struct mutex		*cb_mutex;
 	struct module		*module;
-	void			(*bind)(int group);
+	int			(*bind)(int group);
 	bool			(*compare)(struct net *net, struct sock *sock);
 	int			registered;
 };
-- 
1.7.1

[PATCH 3/5] audit: add netlink audit protocol bind to check capabilities on multicast join

[Richard Guy Briggs]

Register a netlink per-protocol bind fuction for audit to check userspace
process capabilities before allowing a multicast group connection.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index b5b2f72..f2d2d61 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1050,10 +1050,20 @@ static void audit_receive(struct sk_buff  *skb)
 	mutex_unlock(&audit_cmd_mutex);
 }
 
+/* Run custom bind function on netlink socket group connect or bind requests. */
+static int audit_bind(int group)
+{
+	if (!capable(CAP_AUDIT_READ))
+		return -EPERM;
+
+	return 0;
+}
+
 static int __net_init audit_net_init(struct net *net)
 {
 	struct netlink_kernel_cfg cfg = {
 		.input	= audit_receive,
+		.bind	= audit_bind,
 	};
 
 	struct audit_net *aunet = net_generic(net, audit_net_id);
-- 
1.7.1

Re: [PATCH 3/5] audit: add netlink audit protocol bind to check capabilities on multicast join

[Eric Paris]

On Wed, 2014-02-19 at 13:08 -0500, Richard Guy Briggs wrote:
> Register a netlink per-protocol bind fuction for audit to check userspace
> process capabilities before allowing a multicast group connection.
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/audit.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index b5b2f72..f2d2d61 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1050,10 +1050,20 @@ static void audit_receive(struct sk_buff  *skb)
>  	mutex_unlock(&audit_cmd_mutex);
>  }
>  
> +/* Run custom bind function on netlink socket group connect or bind requests. */
> +static int audit_bind(int group)
> +{
> +	if (!capable(CAP_AUDIT_READ))

Not a great idea to use CAP_AUDIT_READ before you define it in patch4

> +		return -EPERM;
> +
> +	return 0;
> +}
> +
>  static int __net_init audit_net_init(struct net *net)
>  {
>  	struct netlink_kernel_cfg cfg = {
>  		.input	= audit_receive,
> +		.bind	= audit_bind,
>  	};
>  
>  	struct audit_net *aunet = net_generic(net, audit_net_id);

Re: [PATCH 3/5] audit: add netlink audit protocol bind to check capabilities on multicast join

[Richard Guy Briggs]

On 14/02/19, Eric Paris wrote:
> On Wed, 2014-02-19 at 13:08 -0500, Richard Guy Briggs wrote:
> > Register a netlink per-protocol bind fuction for audit to check userspace
> > process capabilities before allowing a multicast group connection.
> > 
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >  kernel/audit.c |   10 ++++++++++
> >  1 files changed, 10 insertions(+), 0 deletions(-)
> > 
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index b5b2f72..f2d2d61 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -1050,10 +1050,20 @@ static void audit_receive(struct sk_buff  *skb)
> >  	mutex_unlock(&audit_cmd_mutex);
> >  }
> >  
> > +/* Run custom bind function on netlink socket group connect or bind requests. */
> > +static int audit_bind(int group)
> > +{
> > +	if (!capable(CAP_AUDIT_READ))
> 
> Not a great idea to use CAP_AUDIT_READ before you define it in patch4

<blush>

> > +		return -EPERM;
> > +
> > +	return 0;
> > +}
> > +
> >  static int __net_init audit_net_init(struct net *net)
> >  {
> >  	struct netlink_kernel_cfg cfg = {
> >  		.input	= audit_receive,
> > +		.bind	= audit_bind,
> >  	};
> >  
> >  	struct audit_net *aunet = net_generic(net, audit_net_id);

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

[PATCH 4/5] audit: add netlink multicast group for log read

[Richard Guy Briggs]

Add a netlink multicast socket with one group to kaudit for "best-effort"
delivery to read-only userspace clients such as systemd, in addition to the
existing bidirectional unicast auditd userspace client.

Currently, auditd is intended to use the CAP_AUDIT_CONTROL and CAP_AUDIT_WRITE
capabilities, but actually uses CAP_NET_ADMIN.  The CAP_AUDIT_READ capability
is added for use by read-only AUDIT_NLGRP_READLOG netlink multicast group
clients to the kaudit subsystem.

This will safely give access to services such as systemd to consume audit logs
while ensuring write access remains restricted for integrity.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 include/uapi/linux/audit.h          |    8 +++++++
 include/uapi/linux/capability.h     |    7 +++++-
 kernel/audit.c                      |   39 +++++++++++++++++++++++++++++++++++
 security/selinux/include/classmap.h |    2 +-
 4 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 2d48fe1..8aba976 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -372,6 +372,14 @@ enum {
  */
 #define AUDIT_MESSAGE_TEXT_MAX	8560
 
+/* Multicast Netlink socket groups (default up to 32) */
+enum audit_nlgrps {
+	AUDIT_NLGRP_NONE,	/* Group 0 not used */
+	AUDIT_NLGRP_READLOG,	/* "best effort" read only socket */
+	__AUDIT_NLGRP_MAX
+};
+#define AUDIT_NLGRP_MAX                (__AUDIT_NLGRP_MAX - 1)
+
 struct audit_status {
 	__u32		mask;		/* Bit mask for valid entries */
 	__u32		enabled;	/* 1 = enabled, 0 = disabled */
diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h
index 154dd6d..12c37a1 100644
--- a/include/uapi/linux/capability.h
+++ b/include/uapi/linux/capability.h
@@ -347,7 +347,12 @@ struct vfs_cap_data {
 
 #define CAP_BLOCK_SUSPEND    36
 
-#define CAP_LAST_CAP         CAP_BLOCK_SUSPEND
+/* Allow reading the audit log via multicast netlink socket */
+
+#define CAP_AUDIT_READ		37
+
+
+#define CAP_LAST_CAP         CAP_AUDIT_READ
 
 #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
 
diff --git a/kernel/audit.c b/kernel/audit.c
index f2d2d61..0da57b6 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -424,6 +424,37 @@ static void kauditd_send_skb(struct sk_buff *skb)
 }
 
 /*
+ * kauditd_send_multicast_skb - send the skb to multicast userspace listeners
+ *
+ * This function doesn't consume an skb as might be expected since it has to
+ * copy it anyways.
+ */
+static void kauditd_send_multicast_skb(struct sk_buff *skb)
+{
+	struct sk_buff *copy;
+	struct nlmsghdr *nlh;
+
+	/*
+	 * The seemingly wasteful skb_copy() is necessary because the original
+	 * kaudit unicast socket sends up messages with nlmsg_len set to the
+	 * payload length rather than the entire message length.  This breaks
+	 * the standard set by netlink.  The existing auditd daemon assumes
+	 * this breakage.  Fixing this would require co-ordinating a change in
+	 * the established protocol between the kaudit kernel subsystem and
+	 * the auditd userspace code.  There is no reason for new multicast
+	 * clients to continue with this non-compliance.
+	 */
+	copy = skb_copy(skb, GFP_KERNEL);
+	if (!copy)
+		return;
+
+	nlh = nlmsg_hdr(copy);
+	nlh->nlmsg_len = copy->len;
+
+	nlmsg_multicast(audit_sock, copy, 0, AUDIT_NLGRP_READLOG, GFP_KERNEL);
+}
+
+/*
  * flush_hold_queue - empty the hold queue if auditd appears
  *
  * If auditd just started, drain the queue of messages already
@@ -474,6 +505,12 @@ static int kauditd_thread(void *dummy)
 		skb = skb_dequeue(&audit_skb_queue);
 
 		if (skb) {
+			/* Don't bump skb refcount for multicast send since
+			 * kauditd_send_multicast_skb() copies the skb anyway
+			 * due to audit unicast netlink protocol
+			 * non-compliance.
+			 */
+			kauditd_send_multicast_skb(skb);
 			if (skb_queue_len(&audit_skb_queue) <= audit_backlog_limit)
 				wake_up(&audit_backlog_wait);
 			if (audit_pid)
@@ -1064,6 +1101,8 @@ static int __net_init audit_net_init(struct net *net)
 	struct netlink_kernel_cfg cfg = {
 		.input	= audit_receive,
 		.bind	= audit_bind,
+		.flags	= NL_CFG_F_NONROOT_RECV,
+		.groups	= AUDIT_NLGRP_MAX,
 	};
 
 	struct audit_net *aunet = net_generic(net, audit_net_id);
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 14d04e6..be491a7 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -147,7 +147,7 @@ struct security_class_mapping secclass_map[] = {
 	{ "peer", { "recv", NULL } },
 	{ "capability2",
 	  { "mac_override", "mac_admin", "syslog", "wake_alarm", "block_suspend",
-	    NULL } },
+	    "audit_read", NULL } },
 	{ "kernel_service", { "use_as_override", "create_files_as", NULL } },
 	{ "tun_socket",
 	  { COMMON_SOCK_PERMS, "attach_queue", NULL } },
-- 
1.7.1

Re: [PATCH 4/5] audit: add netlink multicast group for log read

[Steve Grubb]

On Wednesday, February 19, 2014 01:08:22 PM Richard Guy Briggs wrote:
> Add a netlink multicast socket with one group to kaudit for "best-effort"
> delivery to read-only userspace clients such as systemd, in addition to the
> existing bidirectional unicast auditd userspace client.

One question...we do have to have the ability to separate of secadm_r and 
sysadm_r. By allowing this we will leak to a sysadmin that he is being audited 
by the security officer. In a lot of cases, they are one in the same person. But 
for others, they are not. I have a feeling this will cause problems for MLS 
systems.

-Steve


> Currently, auditd is intended to use the CAP_AUDIT_CONTROL and
> CAP_AUDIT_WRITE capabilities, but actually uses CAP_NET_ADMIN.  The
> CAP_AUDIT_READ capability is added for use by read-only AUDIT_NLGRP_READLOG
> netlink multicast group clients to the kaudit subsystem.
> 
> This will safely give access to services such as systemd to consume audit
> logs while ensuring write access remains restricted for integrity.
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  include/uapi/linux/audit.h          |    8 +++++++
>  include/uapi/linux/capability.h     |    7 +++++-
>  kernel/audit.c                      |   39
> +++++++++++++++++++++++++++++++++++ security/selinux/include/classmap.h |  
>  2 +-
>  4 files changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 2d48fe1..8aba976 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -372,6 +372,14 @@ enum {
>   */
>  #define AUDIT_MESSAGE_TEXT_MAX	8560
> 
> +/* Multicast Netlink socket groups (default up to 32) */
> +enum audit_nlgrps {
> +	AUDIT_NLGRP_NONE,	/* Group 0 not used */
> +	AUDIT_NLGRP_READLOG,	/* "best effort" read only socket */
> +	__AUDIT_NLGRP_MAX
> +};
> +#define AUDIT_NLGRP_MAX                (__AUDIT_NLGRP_MAX - 1)
> +
>  struct audit_status {
>  	__u32		mask;		/* Bit mask for valid entries */
>  	__u32		enabled;	/* 1 = enabled, 0 = disabled */
> diff --git a/include/uapi/linux/capability.h
> b/include/uapi/linux/capability.h index 154dd6d..12c37a1 100644
> --- a/include/uapi/linux/capability.h
> +++ b/include/uapi/linux/capability.h
> @@ -347,7 +347,12 @@ struct vfs_cap_data {
> 
>  #define CAP_BLOCK_SUSPEND    36
> 
> -#define CAP_LAST_CAP         CAP_BLOCK_SUSPEND
> +/* Allow reading the audit log via multicast netlink socket */
> +
> +#define CAP_AUDIT_READ		37
> +
> +
> +#define CAP_LAST_CAP         CAP_AUDIT_READ
> 
>  #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index f2d2d61..0da57b6 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -424,6 +424,37 @@ static void kauditd_send_skb(struct sk_buff *skb)
>  }
> 
>  /*
> + * kauditd_send_multicast_skb - send the skb to multicast userspace
> listeners + *
> + * This function doesn't consume an skb as might be expected since it has
> to + * copy it anyways.
> + */
> +static void kauditd_send_multicast_skb(struct sk_buff *skb)
> +{
> +	struct sk_buff *copy;
> +	struct nlmsghdr *nlh;
> +
> +	/*
> +	 * The seemingly wasteful skb_copy() is necessary because the original
> +	 * kaudit unicast socket sends up messages with nlmsg_len set to the
> +	 * payload length rather than the entire message length.  This breaks
> +	 * the standard set by netlink.  The existing auditd daemon assumes
> +	 * this breakage.  Fixing this would require co-ordinating a change in
> +	 * the established protocol between the kaudit kernel subsystem and
> +	 * the auditd userspace code.  There is no reason for new multicast
> +	 * clients to continue with this non-compliance.
> +	 */
> +	copy = skb_copy(skb, GFP_KERNEL);
> +	if (!copy)
> +		return;
> +
> +	nlh = nlmsg_hdr(copy);
> +	nlh->nlmsg_len = copy->len;
> +
> +	nlmsg_multicast(audit_sock, copy, 0, AUDIT_NLGRP_READLOG, GFP_KERNEL);
> +}
> +
> +/*
>   * flush_hold_queue - empty the hold queue if auditd appears
>   *
>   * If auditd just started, drain the queue of messages already
> @@ -474,6 +505,12 @@ static int kauditd_thread(void *dummy)
>  		skb = skb_dequeue(&audit_skb_queue);
> 
>  		if (skb) {
> +			/* Don't bump skb refcount for multicast send since
> +			 * kauditd_send_multicast_skb() copies the skb anyway
> +			 * due to audit unicast netlink protocol
> +			 * non-compliance.
> +			 */
> +			kauditd_send_multicast_skb(skb);
>  			if (skb_queue_len(&audit_skb_queue) <= audit_backlog_limit)
>  				wake_up(&audit_backlog_wait);
>  			if (audit_pid)
> @@ -1064,6 +1101,8 @@ static int __net_init audit_net_init(struct net *net)
>  	struct netlink_kernel_cfg cfg = {
>  		.input	= audit_receive,
>  		.bind	= audit_bind,
> +		.flags	= NL_CFG_F_NONROOT_RECV,
> +		.groups	= AUDIT_NLGRP_MAX,
>  	};
> 
>  	struct audit_net *aunet = net_generic(net, audit_net_id);
> diff --git a/security/selinux/include/classmap.h
> b/security/selinux/include/classmap.h index 14d04e6..be491a7 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -147,7 +147,7 @@ struct security_class_mapping secclass_map[] = {
>  	{ "peer", { "recv", NULL } },
>  	{ "capability2",
>  	  { "mac_override", "mac_admin", "syslog", "wake_alarm", "block_suspend",
> -	    NULL } },
> +	    "audit_read", NULL } },
>  	{ "kernel_service", { "use_as_override", "create_files_as", NULL } },
>  	{ "tun_socket",
>  	  { COMMON_SOCK_PERMS, "attach_queue", NULL } },

Re: [PATCH 4/5] audit: add netlink multicast group for log read

[Eric Paris]

On Wed, 2014-03-12 at 08:55 -0400, Steve Grubb wrote:
> On Wednesday, February 19, 2014 01:08:22 PM Richard Guy Briggs wrote:
> > Add a netlink multicast socket with one group to kaudit for "best-effort"
> > delivery to read-only userspace clients such as systemd, in addition to the
> > existing bidirectional unicast auditd userspace client.
> 
> One question...we do have to have the ability to separate of secadm_r and 
> sysadm_r. By allowing this we will leak to a sysadmin that he is being audited 
> by the security officer. In a lot of cases, they are one in the same person. But 
> for others, they are not. I have a feeling this will cause problems for MLS 
> systems.

A good question.  But easily solved in policy.  Don't give
CAP_AUDIT_READ to sysadm_t if you don't want sysadm_t to be able to read
from the multicast socket.

As to what others who read from the journal I guess we can just make
sure it is a config option whether to collect or not.  Most everyone
would want to collect, but some configs might obviously not.

I'll roll around in the back of my head the ability for auditctl to
disable the multicasting, but CAP_AUDIT_READ takes care of that a whole
lot more nicely...

Re: [PATCH 4/5] audit: add netlink multicast group for log read

[Steve Grubb]

On Wednesday, March 12, 2014 09:18:14 AM Eric Paris wrote:
> On Wed, 2014-03-12 at 08:55 -0400, Steve Grubb wrote:
> > On Wednesday, February 19, 2014 01:08:22 PM Richard Guy Briggs wrote:
> > > Add a netlink multicast socket with one group to kaudit for
> > > "best-effort"
> > > delivery to read-only userspace clients such as systemd, in addition to
> > > the
> > > existing bidirectional unicast auditd userspace client.
> > 
> > One question...we do have to have the ability to separate of secadm_r and
> > sysadm_r. By allowing this we will leak to a sysadmin that he is being
> > audited by the security officer. In a lot of cases, they are one in the
> > same person. But for others, they are not. I have a feeling this will
> > cause problems for MLS systems.
> 
> A good question.  But easily solved in policy.  Don't give
> CAP_AUDIT_READ to sysadm_t if you don't want sysadm_t to be able to read
> from the multicast socket.

That also means that we probably want an audit event for any successful and 
unsuccessful attempts to connect for _reading_ audit events.

-Steve

> As to what others who read from the journal I guess we can just make
> sure it is a config option whether to collect or not.  Most everyone
> would want to collect, but some configs might obviously not.
>
> I'll roll around in the back of my head the ability for auditctl to
> disable the multicasting, but CAP_AUDIT_READ takes care of that a whole
> lot more nicely...

Re: [PATCH 4/5] audit: add netlink multicast group for log read

[Richard Guy Briggs]

On 14/03/12, Steve Grubb wrote:
> On Wednesday, March 12, 2014 09:18:14 AM Eric Paris wrote:
> > On Wed, 2014-03-12 at 08:55 -0400, Steve Grubb wrote:
> > > On Wednesday, February 19, 2014 01:08:22 PM Richard Guy Briggs wrote:
> > > > Add a netlink multicast socket with one group to kaudit for
> > > > "best-effort"
> > > > delivery to read-only userspace clients such as systemd, in addition to
> > > > the
> > > > existing bidirectional unicast auditd userspace client.
> > > 
> > > One question...we do have to have the ability to separate of secadm_r and
> > > sysadm_r. By allowing this we will leak to a sysadmin that he is being
> > > audited by the security officer. In a lot of cases, they are one in the
> > > same person. But for others, they are not. I have a feeling this will
> > > cause problems for MLS systems.

At first I had no idea what you were talking about but Eric's reply
helps to understand the context.

> > A good question.  But easily solved in policy.  Don't give
> > CAP_AUDIT_READ to sysadm_t if you don't want sysadm_t to be able to read
> > from the multicast socket.

This seems like an easy one.

> That also means that we probably want an audit event for any successful and 
> unsuccessful attempts to connect for _reading_ audit events.

That could easily be added to the new custom netlink bind function.

> -Steve
> 
> > As to what others who read from the journal I guess we can just make
> > sure it is a config option whether to collect or not.  Most everyone
> > would want to collect, but some configs might obviously not.

This would be easy to add as a "feature", I'm guessing...

> > I'll roll around in the back of my head the ability for auditctl to
> > disable the multicasting, but CAP_AUDIT_READ takes care of that a whole
> > lot more nicely...

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

[PATCH 5/5] audit: send multicast messages only if there are listeners

[Richard Guy Briggs]

Test first to see if there are any userspace multicast listeners bound to the
socket before starting the multicast send work.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 0da57b6..6e8d137 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -434,6 +434,8 @@ static void kauditd_send_multicast_skb(struct sk_buff *skb)
 	struct sk_buff *copy;
 	struct nlmsghdr *nlh;
 
+	if (!netlink_has_listeners(audit_sock, AUDIT_NLGRP_READLOG))
+		return;
 	/*
 	 * The seemingly wasteful skb_copy() is necessary because the original
 	 * kaudit unicast socket sends up messages with nlmsg_len set to the
-- 
1.7.1