From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752046AbaDAOPp (ORCPT ); Tue, 1 Apr 2014 10:15:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38023 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751240AbaDAOPi (ORCPT ); Tue, 1 Apr 2014 10:15:38 -0400 From: Richard Guy Briggs To: linux-audit@redhat.com, linux-kernel@vger.kernel.org, netfilter-devel@vger.kernel.org, netdev@vger.kernel.org Cc: Richard Guy Briggs , eparis@redhat.com, sgrubb@redhat.com, hadi@mojatatu.com, davem@davemloft.net Subject: [PATCH 2/3][v7] netlink: have netlink per-protocol bind function return an error code. Date: Tue, 1 Apr 2014 10:14:57 -0400 Message-Id: <05279f8fb61d0f79a17b6936b110ad102f27440a.1396317747.git.rgb@redhat.com> In-Reply-To: <20140324183406.GE28666@madcap2.tricolour.ca> References: <20140324183406.GE28666@madcap2.tricolour.ca> In-Reply-To: References: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --- 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. v7: fix formatting and rework logic to flatten indentation. v6: *really* fixed a logic error (missing braces) and a C99 error. v3: added netlink_unbind() option and unwound netlink_insert(). --- include/linux/netlink.h | 3 ++- net/netfilter/nfnetlink.c | 3 ++- net/netlink/af_netlink.c | 42 +++++++++++++++++++++++++++++------------- net/netlink/af_netlink.h | 6 ++++-- 4 files changed, 37 insertions(+), 17 deletions(-) diff --git a/include/linux/netlink.h b/include/linux/netlink.h index 7a6c396..0ba2edd 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 1da5ef1..14c8ac8 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,6 +402,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 bca50b9..9ae6e0f 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -1198,7 +1198,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; @@ -1224,6 +1225,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) @@ -1240,6 +1242,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; @@ -1293,6 +1296,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; } @@ -1441,6 +1445,25 @@ 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) - @@ -1449,15 +1472,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; ingroups; i++) { - if (test_bit(i, nlk->groups)) - nlk->netlink_bind(i); - } - } - return 0; } @@ -2095,14 +2109,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..2aeae8a 100644 --- a/net/netlink/af_netlink.h +++ b/net/netlink/af_netlink.h @@ -37,7 +37,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; @@ -73,7 +74,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