| Message ID | 1366818756-4234-4-git-send-email-nicolas.dichtel@6wind.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 2013-04-24 at 17:52 +0200, Nicolas Dichtel wrote: > This patch allows to dump BPF filters attached to a socket with > SO_ATTACH_FILTER. In other words, users allowing to open netlink sockets can > see filters set on a socket (when the diag module of the socket family is > loaded). To my knowledge, opening netlink sockets is not restricted. I do not want user lambda being able to see my BPF filters. I am root, and was assuming user lambda could not spy on me. $ cat /proc/net/packet sk RefCnt Type Proto Iface R Rmem User Inode 0000000000000000 3 10 0003 3 1 0 0 1089989 0000000000000000 3 10 0003 2 1 0 0 1050535 0000000000000000 3 2 888e 3 1 0 0 1041970 With this information, it seems safe enough, but the whole BPF could give interesting ideas to user lambda. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Wed, 24 Apr 2013 09:22:45 -0700 > On Wed, 2013-04-24 at 17:52 +0200, Nicolas Dichtel wrote: >> This patch allows to dump BPF filters attached to a socket with >> SO_ATTACH_FILTER. In other words, users allowing to open netlink sockets can >> see filters set on a socket (when the diag module of the socket family is >> loaded). > > To my knowledge, opening netlink sockets is not restricted. > > I do not want user lambda being able to see my BPF filters. I agree, this change is not reasonable. And therefore the notifiers added in the subsequent patches of this series are not appropriate either. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le 24/04/2013 18:22, Eric Dumazet a écrit : > On Wed, 2013-04-24 at 17:52 +0200, Nicolas Dichtel wrote: >> This patch allows to dump BPF filters attached to a socket with >> SO_ATTACH_FILTER. In other words, users allowing to open netlink sockets can >> see filters set on a socket (when the diag module of the socket family is >> loaded). > > To my knowledge, opening netlink sockets is not restricted. > > I do not want user lambda being able to see my BPF filters. > > I am root, and was assuming user lambda could not spy on me. > > $ cat /proc/net/packet > sk RefCnt Type Proto Iface R Rmem User Inode > 0000000000000000 3 10 0003 3 1 0 0 1089989 > 0000000000000000 3 10 0003 2 1 0 0 1050535 > 0000000000000000 3 2 888e 3 1 0 0 1041970 > > > With this information, it seems safe enough, but the whole BPF could > give interesting ideas to user lambda. I agree. But then you just have to avoid loading the module packet_diag. This module already give some clue to users, because it sends the socket pointer through netlink. Maybe I'm wrong, but I was thinking that this module is used for debug purpose. If the module is not loaded, my patch has no effect on the system. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Nicolas Dichtel <nicolas.dichtel@6wind.com> Date: Thu, 25 Apr 2013 10:37:00 +0200 > Maybe I'm wrong, but I was thinking that this module is used for debug > purpose. No, it's a normal tool. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
The goal of this patchset is to be able to monitor packet sockets.
The two first patches add new attributes for packet_diag subsystem, so that all
information exported via the /proc/net/packet are also exported via netlink.
The third patch avoid to disclose socket pointer to all user. Via the /proc,
kptr_restrict is used.
The last patch allows allowed users to get details about filter attached to a
packet socket.
As usual, the patch against iproute2 will be sent once the patches are included
and net-next merged. I can send it on demand.
v2: add sock_diag_notify_del() to avoid confusion of the meaning of the second
arg of __sock_diag_notify()
enhance commitlog of patch 3/5
v3: drop previous 4/5 and 5/5 patches
add patch 3/4 (sock_diag: do not disclose sock ptr to all users)
disclose filters only to allowed users
include/linux/sock_diag.h | 5 ++++-
include/uapi/linux/packet_diag.h | 5 +++++
net/core/sock_diag.c | 45 +++++++++++++++++++++++++++++++++++++---
net/ipv4/inet_diag.c | 13 +++++++-----
net/netlink/diag.c | 5 ++++-
net/packet/diag.c | 29 ++++++++++++++++++++------
net/unix/diag.c | 14 ++++++++-----
7 files changed, 95 insertions(+), 21 deletions(-)
Comments are welcome.
Regards,
Nicolas
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2013-04-25 at 10:37 +0200, Nicolas Dichtel wrote: > I agree. But then you just have to avoid loading the module packet_diag. This > module already give some clue to users, because it sends the socket pointer > through netlink. You probably missed a lot of discussion about this socket pointer being leaked. Supposedly security guys wanted to remove this at one point. Thats why we should make sure to add yet another security issue. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/sock_diag.h b/include/linux/sock_diag.h index e8d702e..3957c14 100644 --- a/include/linux/sock_diag.h +++ b/include/linux/sock_diag.h @@ -22,5 +22,7 @@ int sock_diag_check_cookie(void *sk, __u32 *cookie); void sock_diag_save_cookie(void *sk, __u32 *cookie); int sock_diag_put_meminfo(struct sock *sk, struct sk_buff *skb, int attr); +int sock_diag_put_filterinfo(struct sock *sk, struct sk_buff *skb, + int attrtype); #endif diff --git a/include/uapi/linux/packet_diag.h b/include/uapi/linux/packet_diag.h index c0802c1..b2cc0cd 100644 --- a/include/uapi/linux/packet_diag.h +++ b/include/uapi/linux/packet_diag.h @@ -17,6 +17,7 @@ struct packet_diag_req { #define PACKET_SHOW_RING_CFG 0x00000004 /* Rings configuration parameters */ #define PACKET_SHOW_FANOUT 0x00000008 #define PACKET_SHOW_MEMINFO 0x00000010 +#define PACKET_SHOW_FILTER 0x00000020 struct packet_diag_msg { __u8 pdiag_family; @@ -35,6 +36,7 @@ enum { PACKET_DIAG_FANOUT, PACKET_DIAG_UID, PACKET_DIAG_MEMINFO, + PACKET_DIAG_FILTER, __PACKET_DIAG_MAX, }; diff --git a/net/core/sock_diag.c b/net/core/sock_diag.c index a29e90c..51e75f4 100644 --- a/net/core/sock_diag.c +++ b/net/core/sock_diag.c @@ -49,6 +49,33 @@ int sock_diag_put_meminfo(struct sock *sk, struct sk_buff *skb, int attrtype) } EXPORT_SYMBOL_GPL(sock_diag_put_meminfo); +int sock_diag_put_filterinfo(struct sock *sk, struct sk_buff *skb, int attrtype) +{ + struct nlattr *attr; + struct sk_filter *filter; + unsigned int len; + int err = 0; + + rcu_read_lock(); + + filter = rcu_dereference(sk->sk_filter); + len = filter ? filter->len * sizeof(struct sock_filter) : 0; + + attr = nla_reserve(skb, attrtype, len); + if (attr == NULL) { + err = -EMSGSIZE; + goto out; + } + + if (filter) + memcpy(nla_data(attr), filter->insns, len); + +out: + rcu_read_unlock(); + return err; +} +EXPORT_SYMBOL(sock_diag_put_filterinfo); + void sock_diag_register_inet_compat(int (*fn)(struct sk_buff *skb, struct nlmsghdr *nlh)) { mutex_lock(&sock_diag_table_mutex); diff --git a/net/packet/diag.c b/net/packet/diag.c index 822fe9b..ec8b6e8 100644 --- a/net/packet/diag.c +++ b/net/packet/diag.c @@ -170,6 +170,10 @@ static int sk_diag_fill(struct sock *sk, struct sk_buff *skb, sock_diag_put_meminfo(sk, skb, PACKET_DIAG_MEMINFO)) goto out_nlmsg_trim; + if ((req->pdiag_show & PACKET_SHOW_FILTER) && + sock_diag_put_filterinfo(sk, skb, PACKET_DIAG_FILTER)) + goto out_nlmsg_trim; + return nlmsg_end(skb, nlh); out_nlmsg_trim:
This patch allows to dump BPF filters attached to a socket with SO_ATTACH_FILTER. In other words, users allowing to open netlink sockets can see filters set on a socket (when the diag module of the socket family is loaded). For now, only AF_PACKET sockets use this feature. Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> --- include/linux/sock_diag.h | 2 ++ include/uapi/linux/packet_diag.h | 2 ++ net/core/sock_diag.c | 27 +++++++++++++++++++++++++++ net/packet/diag.c | 4 ++++ 4 files changed, 35 insertions(+)