Discussion:
[PATCH 0/6] audit: implement multicast socket for journald
(too old to reply)
Richard Guy Briggs
2014-04-18 17:34:04 UTC
Permalink
This is a patch set Eric Paris and I have been working on to add a restricted
capability read-only netlink multicast socket to kernel audit to enable
userspace clients such as systemd/journald to receive audit logs, in addition
to the bidirectional auditd userspace client.

Currently, auditd has the CAP_AUDIT_CONTROL and CAP_AUDIT_WRITE capabilities
(but uses 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 will remove the dependence on CAP_NET_ADMIN for the multicast
read-only socket.


Patches 1-3 provide a way for per-protocol bind functions to
signal an error and to be able to clean up after themselves.

The first netfilter cleanup patch has already been accepted by a netfilter
maintainer, though I don't see it upstream yet, so it is included for
completeness.

The second patch adds the per-protocol bind function return code to signal to
the netlink code that no further processing should be done and to undo the work
already done. This rev fixes a bug introduced by flattening the code in the
last posting.

The third provides a way per protocol to undo bind actions on DROP.


Patches 4-6 implement the audit multicast socket with capability checking.

The fourth patch adds the bind function capability check to multicast join
requests for audit.

The fifth patch adds the audit log read multicast group. An assumption has
been made that systemd/journald reside in the initial network namespace. This
could be changed to check the actual network namespace of systemd/journald
should this assumption no longer be true since audit now supports all network
namespaces. This version of the patch now directly sends the broadcast when
the packet is ready rather than waiting until it passes the queue.

The sixth checks if any clients actually exist before sending.


Since the net tree is busier than the audit tree, conflicts are more likely and
the audit patches depend on the net patches, it is proposed to have the net
tree carry this entire patchset for 3.16. Are the net maintainers ok with this?


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

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

Please find source for a test program at:
http://people.redhat.com/rbriggs/audit-multicast-listen/


Richard Guy Briggs (6):
netlink: simplify nfnetlink_bind
netlink: have netlink per-protocol bind function return an error
code.
netlink: implement unbind to netlink_setsockopt
NETLINK_DROP_MEMBERSHIP
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 | 3 +-
include/uapi/linux/audit.h | 8 ++++
include/uapi/linux/capability.h | 7 +++-
kernel/audit.c | 64 ++++++++++++++++++++++++++++++++--
net/netfilter/nfnetlink.c | 10 ++---
net/netlink/af_netlink.c | 45 +++++++++++++++++-------
net/netlink/af_netlink.h | 6 ++-
security/selinux/include/classmap.h | 2 +-
8 files changed, 117 insertions(+), 28 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Richard Guy Briggs
2014-04-18 17:34:06 UTC
Permalink
Have the netlink per-protocol optional bind function return an int 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.

In netlink_bind() and netlink_setsockopt() the call to the per-protocol bind
function was moved above the multicast group update to prevent any access to
the multicast socket groups before checking with the per-protocol bind
function. This will enable the per-protocol bind function to be used to check
permissions which could be denied before making them available, and to avoid
the messy job of undoing the addition should the per-protocol bind function
fail.

The netfilter subsystem seems to be the only one currently using the
per-protocol bind function.

Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
In particular, the audit subsystem (NETLINK_AUDIT protocol) could benefit by
being able to check specific capabilities for each multicast group before
granting membership to the requesting socket. Currently, all NETLINK_AUDIT
sockets must have the capability CAP_NET_ADMIN. No other capabilities are
required to join a multicast group. This capability is too broad allowing
access to this socket by many applications that must not have access to this
information. It is proposed to add capability CAP_AUDIT_READ to allow this
access while dropping the exessively broad capability CAP_NET_ADMIN.

There has also been some interest expressed by IETF ForCES folk.
---
include/linux/netlink.h | 3 ++-
net/netfilter/nfnetlink.c | 3 ++-
net/netlink/af_netlink.c | 43 ++++++++++++++++++++++++++++++-------------
net/netlink/af_netlink.h | 6 ++++--
4 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index aad8eea..5146ce0 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -45,7 +45,8 @@ 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);
+ void (*unbind)(int group);
bool (*compare)(struct net *net, struct sock *sk);
};

diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 0df800a..6e42dcf 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -400,7 +400,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];
@@ -410,6 +410,7 @@ static void nfnetlink_bind(int group)
rcu_read_unlock();
if (!ss)
request_module("nfnetlink-subsys-%d", type);
+ return 0;
}
#endif

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 894cda0..3f43e5a 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1206,7 +1206,8 @@ 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);
+ void (*unbind)(int group);
int err = 0;

sock->state = SS_UNCONNECTED;
@@ -1232,6 +1233,7 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol,
err = -EPROTONOSUPPORT;
cb_mutex = nl_table[protocol].cb_mutex;
bind = nl_table[protocol].bind;
+ unbind = nl_table[protocol].unbind;
netlink_unlock_table();

if (err < 0)
@@ -1248,6 +1250,7 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol,
nlk = nlk_sk(sock->sk);
nlk->module = module;
nlk->netlink_bind = bind;
+ nlk->netlink_unbind = unbind;
out:
return err;

@@ -1301,6 +1304,7 @@ static int netlink_release(struct socket *sock)
kfree_rcu(old, rcu);
nl_table[sk->sk_protocol].module = NULL;
nl_table[sk->sk_protocol].bind = NULL;
+ nl_table[sk->sk_protocol].unbind = NULL;
nl_table[sk->sk_protocol].flags = 0;
nl_table[sk->sk_protocol].registered = 0;
}
@@ -1449,6 +1453,26 @@ 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++) {
+ int undo;
+
+ if (!test_bit(i, (long unsigned int *)&nladdr->nl_groups))
+ continue;
+ err = nlk->netlink_bind(i);
+ if (!err)
+ continue;
+ if (!nlk->portid)
+ netlink_remove(sk);
+ for (undo = 0; undo < i; undo++)
+ if (nlk->netlink_unbind)
+ nlk->netlink_unbind(undo);
+ return err;
+ }
+ }
+
netlink_table_grab();
netlink_update_subscriptions(sk, nlk->subscriptions +
hweight32(nladdr->nl_groups) -
@@ -1457,15 +1481,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;
}

@@ -2103,14 +2118,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 ed13a79..0b59d44 100644
--- a/net/netlink/af_netlink.h
+++ b/net/netlink/af_netlink.h
@@ -38,7 +38,8 @@ 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);
+ void (*netlink_unbind)(int group);
struct module *module;
#ifdef CONFIG_NETLINK_MMAP
struct mutex pg_vec_lock;
@@ -74,7 +75,8 @@ struct netlink_table {
unsigned int groups;
struct mutex *cb_mutex;
struct module *module;
- void (*bind)(int group);
+ int (*bind)(int group);
+ void (*unbind)(int group);
bool (*compare)(struct net *net, struct sock *sock);
int registered;
};
--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
David Miller
2014-04-22 20:19:04 UTC
Permalink
From: Richard Guy Briggs <***@redhat.com>
Date: Fri, 18 Apr 2014 13:34:06 -0400
Post by Richard Guy Briggs
@@ -1449,6 +1453,26 @@ 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++) {
+ int undo;
+
+ if (!test_bit(i, (long unsigned int *)&nladdr->nl_groups))
+ continue;
+ err = nlk->netlink_bind(i);
+ if (!err)
+ continue;
+ if (!nlk->portid)
+ netlink_remove(sk);
+ for (undo = 0; undo < i; undo++)
+ if (nlk->netlink_unbind)
+ nlk->netlink_unbind(undo);
+ return err;
+ }
+ }
+
It took me a while to figure out why you need to do the netlink_remove() in
the error path.

I think it's really asking for trouble to allow the socket to have temporary
visibility if we end up signalling an error.

It seems safest if we only do the autobind/insert once we are absolutely
certain that the bind() will fully succeed. This means that you have to
do this bind validation loop before autobind/insert.

Please make this change and resubmit this series, thanks.
Richard Guy Briggs
2014-04-23 01:30:05 UTC
Permalink
Post by David Miller
Date: Fri, 18 Apr 2014 13:34:06 -0400
Post by Richard Guy Briggs
@@ -1449,6 +1453,26 @@ 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++) {
+ int undo;
+
+ if (!test_bit(i, (long unsigned int *)&nladdr->nl_groups))
+ continue;
+ err = nlk->netlink_bind(i);
+ if (!err)
+ continue;
+ if (!nlk->portid)
+ netlink_remove(sk);
+ for (undo = 0; undo < i; undo++)
+ if (nlk->netlink_unbind)
+ nlk->netlink_unbind(undo);
+ return err;
+ }
+ }
+
It took me a while to figure out why you need to do the netlink_remove() in
the error path.
I think it's really asking for trouble to allow the socket to have temporary
visibility if we end up signalling an error.
The risk seems small, but I agree it is sloppy.
Post by David Miller
It seems safest if we only do the autobind/insert once we are absolutely
certain that the bind() will fully succeed. This means that you have to
do this bind validation loop before autobind/insert.
Ok, done. This revision ends up being a bit cleaner than the previous
one because I've refactored out the unbind loop into netlink_unbind()
due to it needing to be called if the netlink_insert/autobind() fails as
well. In the factorization process I noticed that unrequested groups
were being unnecessarily unbound. It also provoked assigning the var
"groups" from "nladdr->nl_groups" making things easier to read.
Post by David Miller
Please make this change and resubmit this series, thanks.
Compiled and tested. New patchset to follow.

- RGB

--
Richard Guy Briggs <***@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
Richard Guy Briggs
2014-04-23 01:31:55 UTC
Permalink
Call the per-protocol unbind function rather than bind function on
NETLINK_DROP_MEMBERSHIP in netlink_setsockopt().

Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
net/netlink/af_netlink.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 7e8d229..92f4b69 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2129,7 +2129,7 @@ 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) {
+ if (optname == NETLINK_ADD_MEMBERSHIP && nlk->netlink_bind) {
err = nlk->netlink_bind(val);
if (err)
return err;
@@ -2138,6 +2138,8 @@ static int netlink_setsockopt(struct socket *sock, int level, int optname,
netlink_update_socket_mc(nlk, val,
optname == NETLINK_ADD_MEMBERSHIP);
netlink_table_ungrab();
+ if (optname == NETLINK_DROP_MEMBERSHIP && nlk->netlink_unbind)
+ nlk->netlink_unbind(val);

err = 0;
break;
--
1.7.1
Richard Guy Briggs
2014-04-23 01:31:54 UTC
Permalink
Have the netlink per-protocol optional bind function return an int 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.

In netlink_bind() and netlink_setsockopt() the call to the per-protocol bind
function was moved above the multicast group update to prevent any access to
the multicast socket groups before checking with the per-protocol bind
function. This will enable the per-protocol bind function to be used to check
permissions which could be denied before making them available, and to avoid
the messy job of undoing the addition should the per-protocol bind function
fail.

The netfilter subsystem seems to be the only one currently using the
per-protocol bind function.

Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
V1: This rev fixes a bug introduced by flattening the code in the last posting.
*V2: This rev moves the per-protocol bind call above the socket exposure call
and refactors out the unbind procedure.

In particular, the audit subsystem (NETLINK_AUDIT protocol) could benefit by
being able to check specific capabilities for each multicast group before
granting membership to the requesting socket. Currently, all NETLINK_AUDIT
sockets must have the capability CAP_NET_ADMIN. No other capabilities are
required to join a multicast group. This capability is too broad allowing
access to this socket by many applications that must not have access to this
information. It is proposed to add capability CAP_AUDIT_READ to allow this
access while dropping the exessively broad capability CAP_NET_ADMIN.

There has also been some interest expressed by IETF ForCES folk.
---
include/linux/netlink.h | 3 +-
net/netfilter/nfnetlink.c | 3 +-
net/netlink/af_netlink.c | 68 +++++++++++++++++++++++++++++++-------------
net/netlink/af_netlink.h | 6 ++-
4 files changed, 56 insertions(+), 24 deletions(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index aad8eea..5146ce0 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -45,7 +45,8 @@ 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);
+ void (*unbind)(int group);
bool (*compare)(struct net *net, struct sock *sk);
};

diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 0df800a..6e42dcf 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -400,7 +400,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];
@@ -410,6 +410,7 @@ static void nfnetlink_bind(int group)
rcu_read_unlock();
if (!ss)
request_module("nfnetlink-subsys-%d", type);
+ return 0;
}
#endif

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 894cda0..7e8d229 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1206,7 +1206,8 @@ 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);
+ void (*unbind)(int group);
int err = 0;

sock->state = SS_UNCONNECTED;
@@ -1232,6 +1233,7 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol,
err = -EPROTONOSUPPORT;
cb_mutex = nl_table[protocol].cb_mutex;
bind = nl_table[protocol].bind;
+ unbind = nl_table[protocol].unbind;
netlink_unlock_table();

if (err < 0)
@@ -1248,6 +1250,7 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol,
nlk = nlk_sk(sock->sk);
nlk->module = module;
nlk->netlink_bind = bind;
+ nlk->netlink_unbind = unbind;
out:
return err;

@@ -1301,6 +1304,7 @@ static int netlink_release(struct socket *sock)
kfree_rcu(old, rcu);
nl_table[sk->sk_protocol].module = NULL;
nl_table[sk->sk_protocol].bind = NULL;
+ nl_table[sk->sk_protocol].unbind = NULL;
nl_table[sk->sk_protocol].flags = 0;
nl_table[sk->sk_protocol].registered = 0;
}
@@ -1411,6 +1415,19 @@ static int netlink_realloc_groups(struct sock *sk)
return err;
}

+static void netlink_unbind(int group, long unsigned int groups,
+ struct netlink_sock *nlk)
+{
+ int undo;
+
+ if (!nlk->netlink_unbind)
+ return;
+
+ for (undo = 0; undo < group; undo++)
+ if (test_bit(group, &groups))
+ nlk->netlink_unbind(undo);
+}
+
static int netlink_bind(struct socket *sock, struct sockaddr *addr,
int addr_len)
{
@@ -1419,6 +1436,7 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr,
struct netlink_sock *nlk = nlk_sk(sk);
struct sockaddr_nl *nladdr = (struct sockaddr_nl *)addr;
int err;
+ long unsigned int groups = nladdr->nl_groups;

if (addr_len < sizeof(struct sockaddr_nl))
return -EINVAL;
@@ -1427,7 +1445,7 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr,
return -EINVAL;

/* Only superuser is allowed to listen multicasts */
- if (nladdr->nl_groups) {
+ if (groups) {
if (!netlink_capable(sock, NL_CFG_F_NONROOT_RECV))
return -EPERM;
err = netlink_realloc_groups(sk);
@@ -1435,37 +1453,45 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr,
return err;
}

- if (nlk->portid) {
+ if (nlk->portid)
if (nladdr->nl_pid != nlk->portid)
return -EINVAL;
- } else {
+
+ if (nlk->netlink_bind && groups) {
+ int group;
+
+ for (group = 0; group < nlk->ngroups; group++) {
+ if (!test_bit(group, &groups))
+ continue;
+ err = nlk->netlink_bind(group);
+ if (!err)
+ continue;
+ netlink_unbind(group, groups, nlk);
+ return err;
+ }
+ }
+
+ if (!nlk->portid) {
err = nladdr->nl_pid ?
netlink_insert(sk, net, nladdr->nl_pid) :
netlink_autobind(sock);
- if (err)
+ if (err) {
+ netlink_unbind(nlk->ngroups - 1, groups, nlk);
return err;
+ }
}

- if (!nladdr->nl_groups && (nlk->groups == NULL || !(u32)nlk->groups[0]))
+ if (!groups && (nlk->groups == NULL || !(u32)nlk->groups[0]))
return 0;

netlink_table_grab();
netlink_update_subscriptions(sk, nlk->subscriptions +
- hweight32(nladdr->nl_groups) -
+ hweight32(groups) -
hweight32(nlk->groups[0]));
- nlk->groups[0] = (nlk->groups[0] & ~0xffffffffUL) | nladdr->nl_groups;
+ nlk->groups[0] = (nlk->groups[0] & ~0xffffffffUL) | groups;
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;
}

@@ -2103,14 +2129,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 ed13a79..0b59d44 100644
--- a/net/netlink/af_netlink.h
+++ b/net/netlink/af_netlink.h
@@ -38,7 +38,8 @@ 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);
+ void (*netlink_unbind)(int group);
struct module *module;
#ifdef CONFIG_NETLINK_MMAP
struct mutex pg_vec_lock;
@@ -74,7 +75,8 @@ struct netlink_table {
unsigned int groups;
struct mutex *cb_mutex;
struct module *module;
- void (*bind)(int group);
+ int (*bind)(int group);
+ void (*unbind)(int group);
bool (*compare)(struct net *net, struct sock *sock);
int registered;
};
--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Richard Guy Briggs
2014-04-23 01:31:52 UTC
Permalink
This is a patch set Eric Paris and I have been working on to add a restricted
capability read-only netlink multicast socket to kernel audit to enable
userspace clients such as systemd/journald to receive audit logs, in addition
to the bidirectional auditd userspace client.

Currently, auditd has the CAP_AUDIT_CONTROL and CAP_AUDIT_WRITE capabilities
(but uses 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 will remove the dependence on CAP_NET_ADMIN for the multicast
read-only socket.


Patches 1-3 provide a way for per-protocol bind functions to
signal an error and to be able to clean up after themselves.

The first netfilter cleanup patch has already been accepted by a netfilter
maintainer, though I don't see it upstream yet, so it is included for
completeness.

The second patch adds the per-protocol bind function return code to signal to
the netlink code that no further processing should be done and to undo the work
already done.
V1: This rev fixes a bug introduced by flattening the code in the last posting.
*V2: This rev moves the per-protocol bind call above the socket exposure call
and refactors out the unbind procedure.

The third provides a way per protocol to undo bind actions on DROP.


Patches 4-6 implement the audit multicast socket with capability checking.

The fourth patch adds the bind function capability check to multicast join
requests for audit.

The fifth patch adds the audit log read multicast group. An assumption has
been made that systemd/journald reside in the initial network namespace. This
could be changed to check the actual network namespace of systemd/journald
should this assumption no longer be true since audit now supports all network
namespaces. This version of the patch now directly sends the broadcast when
the packet is ready rather than waiting until it passes the queue.

The sixth checks if any clients actually exist before sending.


Since the net tree is busier than the audit tree, conflicts are more likely and
the audit patches depend on the net patches, it is proposed to have the net
tree carry this entire patchset for 3.16. Are the net maintainers ok with this?


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

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

Please find source for a test program at:
http://people.redhat.com/rbriggs/audit-multicast-listen/


Richard Guy Briggs (6):
netlink: simplify nfnetlink_bind
netlink: have netlink per-protocol bind function return an error
code.
netlink: implement unbind to netlink_setsockopt
NETLINK_DROP_MEMBERSHIP
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 | 3 +-
include/uapi/linux/audit.h | 8 ++++
include/uapi/linux/capability.h | 7 +++-
kernel/audit.c | 64 ++++++++++++++++++++++++++++++--
net/netfilter/nfnetlink.c | 10 ++---
net/netlink/af_netlink.c | 70 +++++++++++++++++++++++++----------
net/netlink/af_netlink.h | 6 ++-
security/selinux/include/classmap.h | 2 +-
8 files changed, 135 insertions(+), 35 deletions(-)
David Miller
2014-04-23 01:43:00 UTC
Permalink
From: Richard Guy Briggs <***@redhat.com>
Date: Tue, 22 Apr 2014 21:31:52 -0400
Post by Richard Guy Briggs
This is a patch set Eric Paris and I have been working on to add a restricted
capability read-only netlink multicast socket to kernel audit to enable
userspace clients such as systemd/journald to receive audit logs, in addition
to the bidirectional auditd userspace client.
Series applied, thanks Richard.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Richard Guy Briggs
2014-04-23 01:49:29 UTC
Permalink
Post by David Miller
Date: Tue, 22 Apr 2014 21:31:52 -0400
Post by Richard Guy Briggs
This is a patch set Eric Paris and I have been working on to add a restricted
capability read-only netlink multicast socket to kernel audit to enable
userspace clients such as systemd/journald to receive audit logs, in addition
to the bidirectional auditd userspace client.
Series applied, thanks Richard.
Thanks for your patience, David. Can I assume you adopted the 3 audit
patches too, becuase of the dependence?

- RGB

--
Richard Guy Briggs <***@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
David Miller
2014-04-23 03:55:15 UTC
Permalink
From: Richard Guy Briggs <***@redhat.com>
Date: Tue, 22 Apr 2014 21:49:29 -0400
Post by Richard Guy Briggs
Post by David Miller
Date: Tue, 22 Apr 2014 21:31:52 -0400
Post by Richard Guy Briggs
This is a patch set Eric Paris and I have been working on to add a restricted
capability read-only netlink multicast socket to kernel audit to enable
userspace clients such as systemd/journald to receive audit logs, in addition
to the bidirectional auditd userspace client.
Series applied, thanks Richard.
Thanks for your patience, David. Can I assume you adopted the 3 audit
patches too, becuase of the dependence?
Yes.
Steve Grubb
2014-04-23 02:25:55 UTC
Permalink
Post by Richard Guy Briggs
This is a patch set Eric Paris and I have been working on to add a
restricted capability read-only netlink multicast socket to kernel audit to
enable userspace clients such as systemd/journald to receive audit logs, in
addition to the bidirectional auditd userspace client.
Do 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.

Also, shouldn't we have an audit event for every attempt to connect to this
socket? We really need to know where this information is getting leaked to.

-Steve
Post by Richard Guy Briggs
Currently, auditd has the CAP_AUDIT_CONTROL and CAP_AUDIT_WRITE capabilities
(but uses 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 will remove the dependence on CAP_NET_ADMIN for the
multicast read-only socket.
Patches 1-3 provide a way for per-protocol bind functions to
signal an error and to be able to clean up after themselves.
The first netfilter cleanup patch has already been accepted by a netfilter
maintainer, though I don't see it upstream yet, so it is included for
completeness.
The second patch adds the per-protocol bind function return code to signal
to the netlink code that no further processing should be done and to undo
the work already done.
V1: This rev fixes a bug introduced by flattening the code in the last
posting. *V2: This rev moves the per-protocol bind call above the socket
exposure call and refactors out the unbind procedure.
The third provides a way per protocol to undo bind actions on DROP.
Patches 4-6 implement the audit multicast socket with capability checking.
The fourth patch adds the bind function capability check to multicast join
requests for audit.
The fifth patch adds the audit log read multicast group. An assumption has
been made that systemd/journald reside in the initial network namespace.
This could be changed to check the actual network namespace of
systemd/journald should this assumption no longer be true since audit now
supports all network namespaces. This version of the patch now directly
sends the broadcast when the packet is ready rather than waiting until it
passes the queue.
The sixth checks if any clients actually exist before sending.
Since the net tree is busier than the audit tree, conflicts are more likely
and the audit patches depend on the net patches, it is proposed to have the
net tree carry this entire patchset for 3.16. Are the net maintainers ok
with this?
https://bugzilla.redhat.com/show_bug.cgi?id=887992
https://www.redhat.com/archives/linux-audit/2013-January/msg00008.html
https://lkml.org/lkml/2013/1/27/279
http://people.redhat.com/rbriggs/audit-multicast-listen/
netlink: simplify nfnetlink_bind
netlink: have netlink per-protocol bind function return an error
code.
netlink: implement unbind to netlink_setsockopt
NETLINK_DROP_MEMBERSHIP
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 | 3 +-
include/uapi/linux/audit.h | 8 ++++
include/uapi/linux/capability.h | 7 +++-
kernel/audit.c | 64 ++++++++++++++++++++++++++++++--
net/netfilter/nfnetlink.c | 10 ++---
net/netlink/af_netlink.c | 70
+++++++++++++++++++++++++---------- net/netlink/af_netlink.h |
6 ++-
security/selinux/include/classmap.h | 2 +-
8 files changed, 135 insertions(+), 35 deletions(-)
Eric Paris
2014-04-23 03:57:55 UTC
Permalink
Post by Steve Grubb
Post by Richard Guy Briggs
This is a patch set Eric Paris and I have been working on to add a
restricted capability read-only netlink multicast socket to kernel audit to
enable userspace clients such as systemd/journald to receive audit logs, in
addition to the bidirectional auditd userspace client.
Do 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.
Why? This requires CAP_AUDIT_READ. Just don't give CAP_AUDIT_READ to
places you don't want to have read permission. Exactly the same as you
don't give CAP_AUDIT_CONTROL to sysadm_r. (If we are giving
CAP_AUDIT_CONTROL to sysadm_r and you think that any file protections
on /var/log/audit/audit.log are adequate we are fooling ourselves!)
Post by Steve Grubb
Also, shouldn't we have an audit event for every attempt to connect to this
socket? We really need to know where this information is getting leaked to.
We certainly can. What would you like to see in that event?

-Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Daniel J Walsh
2014-04-23 13:40:34 UTC
Permalink
Here are the capabilities we currently give to sysadm_t with
sysadm_secadm 1.0.0 Disabled

allow sysadm_t sysadm_t : capability { chown dac_override
dac_read_search fowner fsetid kill setgid setuid setpcap linux_immutable
net_bind_service net_broadcast net_admin net_raw ipc_lock ipc_owner
sys_rawio sys_chroot sys_ptrace sys_pacct sys_admin sys_boot sys_nice
sys_resource sys_time sys_tty_config mknod lease audit_write setfcap } ;
allow sysadm_t sysadm_t : capability { setgid setuid sys_chroot }

allow sysadm_t sysadm_t : capability2 { syslog block_suspend } ;

cap_audit_write might be a problem?
Post by Eric Paris
Post by Steve Grubb
Post by Richard Guy Briggs
This is a patch set Eric Paris and I have been working on to add a
restricted capability read-only netlink multicast socket to kernel audit to
enable userspace clients such as systemd/journald to receive audit logs, in
addition to the bidirectional auditd userspace client.
Do 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.
Why? This requires CAP_AUDIT_READ. Just don't give CAP_AUDIT_READ to
places you don't want to have read permission. Exactly the same as you
don't give CAP_AUDIT_CONTROL to sysadm_r. (If we are giving
CAP_AUDIT_CONTROL to sysadm_r and you think that any file protections
on /var/log/audit/audit.log are adequate we are fooling ourselves!)
Post by Steve Grubb
Also, shouldn't we have an audit event for every attempt to connect to this
socket? We really need to know where this information is getting leaked to.
We certainly can. What would you like to see in that event?
-Eric
_______________________________________________
Selinux mailing list
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Eric Paris
2014-04-23 14:42:32 UTC
Permalink
Post by Daniel J Walsh
Here are the capabilities we currently give to sysadm_t with
sysadm_secadm 1.0.0 Disabled
allow sysadm_t sysadm_t : capability { chown dac_override
dac_read_search fowner fsetid kill setgid setuid setpcap linux_immutable
net_bind_service net_broadcast net_admin net_raw ipc_lock ipc_owner
sys_rawio sys_chroot sys_ptrace sys_pacct sys_admin sys_boot sys_nice
sys_resource sys_time sys_tty_config mknod lease audit_write setfcap } ;
allow sysadm_t sysadm_t : capability { setgid setuid sys_chroot }
allow sysadm_t sysadm_t : capability2 { syslog block_suspend } ;
cap_audit_write might be a problem?
cap_audit_write is fine.

syslogd_t (aka journal) is going to need the new permission
cap_audit_read. Also, as steve pointed out, someone may be likely to
want to be able to disable that permission easily.

-Eric
Daniel J Walsh
2014-04-23 15:36:50 UTC
Permalink
I guess the problem would be that the sysadm_t would be able to look at
the journal which would now contain the audit content.
Post by Eric Paris
Post by Daniel J Walsh
Here are the capabilities we currently give to sysadm_t with
sysadm_secadm 1.0.0 Disabled
allow sysadm_t sysadm_t : capability { chown dac_override
dac_read_search fowner fsetid kill setgid setuid setpcap linux_immutable
net_bind_service net_broadcast net_admin net_raw ipc_lock ipc_owner
sys_rawio sys_chroot sys_ptrace sys_pacct sys_admin sys_boot sys_nice
sys_resource sys_time sys_tty_config mknod lease audit_write setfcap } ;
allow sysadm_t sysadm_t : capability { setgid setuid sys_chroot }
allow sysadm_t sysadm_t : capability2 { syslog block_suspend } ;
cap_audit_write might be a problem?
cap_audit_write is fine.
syslogd_t (aka journal) is going to need the new permission
cap_audit_read. Also, as steve pointed out, someone may be likely to
want to be able to disable that permission easily.
-Eric
Eric Paris
2014-04-23 15:37:38 UTC
Permalink
Post by Daniel J Walsh
I guess the problem would be that the sysadm_t would be able to look at
the journal which would now contain the audit content.
right. so include it in the sysadm_secadm bool
Post by Daniel J Walsh
Post by Eric Paris
Post by Daniel J Walsh
Here are the capabilities we currently give to sysadm_t with
sysadm_secadm 1.0.0 Disabled
allow sysadm_t sysadm_t : capability { chown dac_override
dac_read_search fowner fsetid kill setgid setuid setpcap linux_immutable
net_bind_service net_broadcast net_admin net_raw ipc_lock ipc_owner
sys_rawio sys_chroot sys_ptrace sys_pacct sys_admin sys_boot sys_nice
sys_resource sys_time sys_tty_config mknod lease audit_write setfcap } ;
allow sysadm_t sysadm_t : capability { setgid setuid sys_chroot }
allow sysadm_t sysadm_t : capability2 { syslog block_suspend } ;
cap_audit_write might be a problem?
cap_audit_write is fine.
syslogd_t (aka journal) is going to need the new permission
cap_audit_read. Also, as steve pointed out, someone may be likely to
want to be able to disable that permission easily.
-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Daniel J Walsh
2014-04-23 15:52:20 UTC
Permalink
Meaning looking at the journal would be equivalent to looking at
/var/log/audit/audit.log.
Post by Eric Paris
Post by Daniel J Walsh
I guess the problem would be that the sysadm_t would be able to look at
the journal which would now contain the audit content.
right. so include it in the sysadm_secadm bool
Post by Daniel J Walsh
Post by Eric Paris
Post by Daniel J Walsh
Here are the capabilities we currently give to sysadm_t with
sysadm_secadm 1.0.0 Disabled
allow sysadm_t sysadm_t : capability { chown dac_override
dac_read_search fowner fsetid kill setgid setuid setpcap linux_immutable
net_bind_service net_broadcast net_admin net_raw ipc_lock ipc_owner
sys_rawio sys_chroot sys_ptrace sys_pacct sys_admin sys_boot sys_nice
sys_resource sys_time sys_tty_config mknod lease audit_write setfcap } ;
allow sysadm_t sysadm_t : capability { setgid setuid sys_chroot }
allow sysadm_t sysadm_t : capability2 { syslog block_suspend } ;
cap_audit_write might be a problem?
cap_audit_write is fine.
syslogd_t (aka journal) is going to need the new permission
cap_audit_read. Also, as steve pointed out, someone may be likely to
want to be able to disable that permission easily.
-Eric
_______________________________________________
Selinux mailing list
Eric Paris
2014-04-24 13:22:27 UTC
Permalink
They would be equivalent if and only if journald had CAP_AUDIT_READ.

I suggest you take CAP_AUDIT_READ away from journald on systems which
need the secadm/sysadmin split (which is a ridiculously stupid split
anyway, but who am I to complain?)
Post by Daniel J Walsh
Meaning looking at the journal would be equivalent to looking at
/var/log/audit/audit.log.
Post by Eric Paris
Post by Daniel J Walsh
I guess the problem would be that the sysadm_t would be able to look at
the journal which would now contain the audit content.
right. so include it in the sysadm_secadm bool
Post by Daniel J Walsh
Post by Eric Paris
Post by Daniel J Walsh
Here are the capabilities we currently give to sysadm_t with
sysadm_secadm 1.0.0 Disabled
allow sysadm_t sysadm_t : capability { chown dac_override
dac_read_search fowner fsetid kill setgid setuid setpcap linux_immutable
net_bind_service net_broadcast net_admin net_raw ipc_lock ipc_owner
sys_rawio sys_chroot sys_ptrace sys_pacct sys_admin sys_boot sys_nice
sys_resource sys_time sys_tty_config mknod lease audit_write setfcap } ;
allow sysadm_t sysadm_t : capability { setgid setuid sys_chroot }
allow sysadm_t sysadm_t : capability2 { syslog block_suspend } ;
cap_audit_write might be a problem?
cap_audit_write is fine.
syslogd_t (aka journal) is going to need the new permission
cap_audit_read. Also, as steve pointed out, someone may be likely to
want to be able to disable that permission easily.
-Eric
_______________________________________________
Selinux mailing list
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Daniel J Walsh
2014-04-24 14:59:04 UTC
Permalink
I don't disagree. I would think the real solution to this would be to
not allow sysadm_t to get to SystemHigh, where all of the logging data
will be stored.
Post by Eric Paris
They would be equivalent if and only if journald had CAP_AUDIT_READ.
I suggest you take CAP_AUDIT_READ away from journald on systems which
need the secadm/sysadmin split (which is a ridiculously stupid split
anyway, but who am I to complain?)
Post by Daniel J Walsh
Meaning looking at the journal would be equivalent to looking at
/var/log/audit/audit.log.
Post by Eric Paris
Post by Daniel J Walsh
I guess the problem would be that the sysadm_t would be able to look at
the journal which would now contain the audit content.
right. so include it in the sysadm_secadm bool
Post by Daniel J Walsh
Post by Eric Paris
Post by Daniel J Walsh
Here are the capabilities we currently give to sysadm_t with
sysadm_secadm 1.0.0 Disabled
allow sysadm_t sysadm_t : capability { chown dac_override
dac_read_search fowner fsetid kill setgid setuid setpcap linux_immutable
net_bind_service net_broadcast net_admin net_raw ipc_lock ipc_owner
sys_rawio sys_chroot sys_ptrace sys_pacct sys_admin sys_boot sys_nice
sys_resource sys_time sys_tty_config mknod lease audit_write setfcap } ;
allow sysadm_t sysadm_t : capability { setgid setuid sys_chroot }
allow sysadm_t sysadm_t : capability2 { syslog block_suspend } ;
cap_audit_write might be a problem?
cap_audit_write is fine.
syslogd_t (aka journal) is going to need the new permission
cap_audit_read. Also, as steve pointed out, someone may be likely to
want to be able to disable that permission easily.
-Eric
_______________________________________________
Selinux mailing list
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
_______________________________________________
Selinux mailing list
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Eric Paris
2014-04-24 15:03:00 UTC
Permalink
Post by Daniel J Walsh
I don't disagree. I would think the real solution to this would be to
not allow sysadm_t to get to SystemHigh, where all of the logging data
will be stored.
make journalctl a userspace object manager and do selinux checks on if
it can see individual records? so secadm_t running journalctl would see
them and sysadm running journalctl wouldn't see them?

Sounds elegant. Who is going to code it? *NOT IT!*
Post by Daniel J Walsh
Post by Eric Paris
They would be equivalent if and only if journald had CAP_AUDIT_READ.
I suggest you take CAP_AUDIT_READ away from journald on systems which
need the secadm/sysadmin split (which is a ridiculously stupid split
anyway, but who am I to complain?)
Post by Daniel J Walsh
Meaning looking at the journal would be equivalent to looking at
/var/log/audit/audit.log.
Post by Eric Paris
Post by Daniel J Walsh
I guess the problem would be that the sysadm_t would be able to look at
the journal which would now contain the audit content.
right. so include it in the sysadm_secadm bool
Post by Daniel J Walsh
Post by Eric Paris
Post by Daniel J Walsh
Here are the capabilities we currently give to sysadm_t with
sysadm_secadm 1.0.0 Disabled
allow sysadm_t sysadm_t : capability { chown dac_override
dac_read_search fowner fsetid kill setgid setuid setpcap linux_immutable
net_bind_service net_broadcast net_admin net_raw ipc_lock ipc_owner
sys_rawio sys_chroot sys_ptrace sys_pacct sys_admin sys_boot sys_nice
sys_resource sys_time sys_tty_config mknod lease audit_write setfcap } ;
allow sysadm_t sysadm_t : capability { setgid setuid sys_chroot }
allow sysadm_t sysadm_t : capability2 { syslog block_suspend } ;
cap_audit_write might be a problem?
cap_audit_write is fine.
syslogd_t (aka journal) is going to need the new permission
cap_audit_read. Also, as steve pointed out, someone may be likely to
want to be able to disable that permission easily.
-Eric
_______________________________________________
Selinux mailing list
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
_______________________________________________
Selinux mailing list
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Daniel J Walsh
2014-04-24 16:03:57 UTC
Permalink
Yes that would be the long term fix. But it would involve journal
labelling individual data records. IE Records from audit.log would be
audit_log_t, while messages from syslog would be var_log_t, Or some
other kind of crazyness.
Post by Eric Paris
Post by Daniel J Walsh
I don't disagree. I would think the real solution to this would be to
not allow sysadm_t to get to SystemHigh, where all of the logging data
will be stored.
make journalctl a userspace object manager and do selinux checks on if
it can see individual records? so secadm_t running journalctl would see
them and sysadm running journalctl wouldn't see them?
Sounds elegant. Who is going to code it? *NOT IT!*
Post by Daniel J Walsh
Post by Eric Paris
They would be equivalent if and only if journald had CAP_AUDIT_READ.
I suggest you take CAP_AUDIT_READ away from journald on systems which
need the secadm/sysadmin split (which is a ridiculously stupid split
anyway, but who am I to complain?)
Post by Daniel J Walsh
Meaning looking at the journal would be equivalent to looking at
/var/log/audit/audit.log.
Post by Eric Paris
Post by Daniel J Walsh
I guess the problem would be that the sysadm_t would be able to look at
the journal which would now contain the audit content.
right. so include it in the sysadm_secadm bool
Post by Daniel J Walsh
Post by Eric Paris
Post by Daniel J Walsh
Here are the capabilities we currently give to sysadm_t with
sysadm_secadm 1.0.0 Disabled
allow sysadm_t sysadm_t : capability { chown dac_override
dac_read_search fowner fsetid kill setgid setuid setpcap linux_immutable
net_bind_service net_broadcast net_admin net_raw ipc_lock ipc_owner
sys_rawio sys_chroot sys_ptrace sys_pacct sys_admin sys_boot sys_nice
sys_resource sys_time sys_tty_config mknod lease audit_write setfcap } ;
allow sysadm_t sysadm_t : capability { setgid setuid sys_chroot }
allow sysadm_t sysadm_t : capability2 { syslog block_suspend } ;
cap_audit_write might be a problem?
cap_audit_write is fine.
syslogd_t (aka journal) is going to need the new permission
cap_audit_read. Also, as steve pointed out, someone may be likely to
want to be able to disable that permission easily.
-Eric
_______________________________________________
Selinux mailing list
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
_______________________________________________
Selinux mailing list
_______________________________________________
Selinux mailing list
Richard Guy Briggs
2014-04-23 01:31:58 UTC
Permalink
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 <***@redhat.com>
---
kernel/audit.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index d272cc1..33531d7 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -435,6 +435,9 @@ static void kauditd_send_multicast_skb(struct sk_buff *skb)
struct audit_net *aunet = net_generic(&init_net, audit_net_id);
struct sock *sock = aunet->nlsk;

+ if (!netlink_has_listeners(sock, AUDIT_NLGRP_READLOG))
+ return;
+
/*
* The seemingly wasteful skb_copy() rather than bumping the refcount
* using skb_get() is necessary because non-standard mods are made to
--
1.7.1
Richard Guy Briggs
2014-04-23 01:31:53 UTC
Permalink
Remove duplicity and simplify code flow by moving the rcu_read_unlock() above
the condition and let the flow control exit naturally at the end of the
function.

Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
net/netfilter/nfnetlink.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index e8138da..0df800a 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -407,12 +407,9 @@ static void nfnetlink_bind(int group)

rcu_read_lock();
ss = nfnetlink_get_subsys(type);
- if (!ss) {
- rcu_read_unlock();
- request_module("nfnetlink-subsys-%d", type);
- return;
- }
rcu_read_unlock();
+ if (!ss)
+ request_module("nfnetlink-subsys-%d", type);
}
#endif
--
1.7.1
Richard Guy Briggs
2014-04-23 01:31:57 UTC
Permalink
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 <***@redhat.com>
---
include/uapi/linux/audit.h | 8 +++++++
kernel/audit.c | 51 ++++++++++++++++++++++++++++++++++++++++---
2 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 11917f7..dfa4c86 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -373,6 +373,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/kernel/audit.c b/kernel/audit.c
index 223cb74..d272cc1 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -424,6 +424,35 @@ 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 audit_net *aunet = net_generic(&init_net, audit_net_id);
+ struct sock *sock = aunet->nlsk;
+
+ /*
+ * The seemingly wasteful skb_copy() rather than bumping the refcount
+ * using skb_get() is necessary because non-standard mods are made to
+ * the skb by the original kaudit unicast socket send routine. 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;
+
+ nlmsg_multicast(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
@@ -1090,6 +1119,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);
@@ -1911,10 +1942,10 @@ out:
* audit_log_end - end one audit record
* @ab: the audit_buffer
*
- * The netlink_* functions cannot be called inside an irq context, so
- * the audit buffer is placed on a queue and a tasklet is scheduled to
- * remove them from the queue outside the irq context. May be called in
- * any context.
+ * netlink_unicast() cannot be called inside an irq context because it blocks
+ * (last arg, flags, is not set to MSG_DONTWAIT), so the audit buffer is placed
+ * on a queue and a tasklet is scheduled to remove them from the queue outside
+ * the irq context. May be called in any context.
*/
void audit_log_end(struct audit_buffer *ab)
{
@@ -1924,6 +1955,18 @@ void audit_log_end(struct audit_buffer *ab)
audit_log_lost("rate limit exceeded");
} else {
struct nlmsghdr *nlh = nlmsg_hdr(ab->skb);
+
+ kauditd_send_multicast_skb(ab->skb);
+
+ /*
+ * 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.
+ */
nlh->nlmsg_len = ab->skb->len - NLMSG_HDRLEN;

if (audit_pid) {
--
1.7.1
Richard Guy Briggs
2014-04-23 01:31:56 UTC
Permalink
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 <***@redhat.com>
---
include/uapi/linux/capability.h | 7 ++++++-
kernel/audit.c | 10 ++++++++++
security/selinux/include/classmap.h | 2 +-
3 files changed, 17 insertions(+), 2 deletions(-)

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 7c28936..223cb74 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1076,10 +1076,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);
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
Richard Guy Briggs
2014-04-18 17:34:05 UTC
Permalink
Remove duplicity and simplify code flow by moving the rcu_read_unlock() above
the condition and let the flow control exit naturally at the end of the
function.

Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
net/netfilter/nfnetlink.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index e8138da..0df800a 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -407,12 +407,9 @@ static void nfnetlink_bind(int group)

rcu_read_lock();
ss = nfnetlink_get_subsys(type);
- if (!ss) {
- rcu_read_unlock();
- request_module("nfnetlink-subsys-%d", type);
- return;
- }
rcu_read_unlock();
+ if (!ss)
+ request_module("nfnetlink-subsys-%d", type);
}
#endif
--
1.7.1
Richard Guy Briggs
2014-04-18 17:34:10 UTC
Permalink
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 <***@redhat.com>
---
kernel/audit.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index d272cc1..33531d7 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -435,6 +435,9 @@ static void kauditd_send_multicast_skb(struct sk_buff *skb)
struct audit_net *aunet = net_generic(&init_net, audit_net_id);
struct sock *sock = aunet->nlsk;

+ if (!netlink_has_listeners(sock, AUDIT_NLGRP_READLOG))
+ return;
+
/*
* The seemingly wasteful skb_copy() rather than bumping the refcount
* using skb_get() is necessary because non-standard mods are made to
--
1.7.1
Richard Guy Briggs
2014-04-18 17:34:09 UTC
Permalink
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 <***@redhat.com>
---
include/uapi/linux/audit.h | 8 +++++++
kernel/audit.c | 51 ++++++++++++++++++++++++++++++++++++++++---
2 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 11917f7..dfa4c86 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -373,6 +373,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/kernel/audit.c b/kernel/audit.c
index 223cb74..d272cc1 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -424,6 +424,35 @@ 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 audit_net *aunet = net_generic(&init_net, audit_net_id);
+ struct sock *sock = aunet->nlsk;
+
+ /*
+ * The seemingly wasteful skb_copy() rather than bumping the refcount
+ * using skb_get() is necessary because non-standard mods are made to
+ * the skb by the original kaudit unicast socket send routine. 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;
+
+ nlmsg_multicast(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
@@ -1090,6 +1119,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);
@@ -1911,10 +1942,10 @@ out:
* audit_log_end - end one audit record
* @ab: the audit_buffer
*
- * The netlink_* functions cannot be called inside an irq context, so
- * the audit buffer is placed on a queue and a tasklet is scheduled to
- * remove them from the queue outside the irq context. May be called in
- * any context.
+ * netlink_unicast() cannot be called inside an irq context because it blocks
+ * (last arg, flags, is not set to MSG_DONTWAIT), so the audit buffer is placed
+ * on a queue and a tasklet is scheduled to remove them from the queue outside
+ * the irq context. May be called in any context.
*/
void audit_log_end(struct audit_buffer *ab)
{
@@ -1924,6 +1955,18 @@ void audit_log_end(struct audit_buffer *ab)
audit_log_lost("rate limit exceeded");
} else {
struct nlmsghdr *nlh = nlmsg_hdr(ab->skb);
+
+ kauditd_send_multicast_skb(ab->skb);
+
+ /*
+ * 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.
+ */
nlh->nlmsg_len = ab->skb->len - NLMSG_HDRLEN;

if (audit_pid) {
--
1.7.1
Richard Guy Briggs
2014-04-18 17:34:08 UTC
Permalink
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 <***@redhat.com>
---
include/uapi/linux/capability.h | 7 ++++++-
kernel/audit.c | 10 ++++++++++
security/selinux/include/classmap.h | 2 +-
3 files changed, 17 insertions(+), 2 deletions(-)

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 7c28936..223cb74 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1076,10 +1076,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);
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

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Richard Guy Briggs
2014-04-18 17:34:07 UTC
Permalink
Call the per-protocol unbind function rather than bind function on
NETLINK_DROP_MEMBERSHIP in netlink_setsockopt().

Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
net/netlink/af_netlink.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 3f43e5a..cdb236f 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2118,7 +2118,7 @@ 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) {
+ if (optname == NETLINK_ADD_MEMBERSHIP && nlk->netlink_bind) {
err = nlk->netlink_bind(val);
if (err)
return err;
@@ -2127,6 +2127,8 @@ static int netlink_setsockopt(struct socket *sock, int level, int optname,
netlink_update_socket_mc(nlk, val,
optname == NETLINK_ADD_MEMBERSHIP);
netlink_table_ungrab();
+ if (optname == NETLINK_DROP_MEMBERSHIP && nlk->netlink_unbind)
+ nlk->netlink_unbind(val);

err = 0;
break;
--
1.7.1

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