[PATCH net] net: netlink: filter particular protocols from analyzers

[PATCH net] net: netlink: filter particular protocols from analyzers

[Daniel Borkmann]

From: Daniel Borkmann <dborkmann@redhat.com>

Fix finer-grained control and let only a whitelist of allowed netlink
protocols pass, in our case related to networking. If later on, other
subsystems decide they want to add their protocol as well to the list
of allowed protocols they shall simply add it. While at it, we also
need to tell what protocol is in use otherwise BPF_S_ANC_PROTOCOL can
not pick it up (as it's not filled out).

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/netlink/af_netlink.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 0c61b59..350187e 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -168,16 +168,43 @@ int netlink_remove_tap(struct netlink_tap *nt)
 }
 EXPORT_SYMBOL_GPL(netlink_remove_tap);
 
+static bool netlink_filter_tap(const struct sk_buff *skb)
+{
+	struct sock *sk = skb->sk;
+	bool pass = false;
+
+	/* We take the more conservative approach and
+	 * whitelist socket protocols that may pass.
+	 */
+	switch (sk->sk_protocol) {
+	case NETLINK_ROUTE:
+	case NETLINK_USERSOCK:
+	case NETLINK_SOCK_DIAG:
+	case NETLINK_NFLOG:
+	case NETLINK_XFRM:
+	case NETLINK_FIB_LOOKUP:
+	case NETLINK_NETFILTER:
+	case NETLINK_GENERIC:
+		pass = true;
+		break;
+	}
+
+	return pass;
+}
+
 static int __netlink_deliver_tap_skb(struct sk_buff *skb,
 				     struct net_device *dev)
 {
 	struct sk_buff *nskb;
+	struct sock *sk = skb->sk;
 	int ret = -ENOMEM;
 
 	dev_hold(dev);
 	nskb = skb_clone(skb, GFP_ATOMIC);
 	if (nskb) {
 		nskb->dev = dev;
+		nskb->protocol = htons((u16) sk->sk_protocol);
+
 		ret = dev_queue_xmit(nskb);
 		if (unlikely(ret > 0))
 			ret = net_xmit_errno(ret);
@@ -192,6 +219,9 @@ static void __netlink_deliver_tap(struct sk_buff *skb)
 	int ret;
 	struct netlink_tap *tmp;
 
+	if (!netlink_filter_tap(skb))
+		return;
+
 	list_for_each_entry_rcu(tmp, &netlink_tap_all, list) {
 		ret = __netlink_deliver_tap_skb(skb, tmp->dev);
 		if (unlikely(ret))
-- 
1.8.3.1

Re: [PATCH net] net: netlink: filter particular protocols from analyzers

[David Miller]

From: Daniel Borkmann <dborkman@redhat.com>
Date: Thu,  5 Sep 2013 17:48:47 +0200

> From: Daniel Borkmann <dborkmann@redhat.com>
> 
> Fix finer-grained control and let only a whitelist of allowed netlink
> protocols pass, in our case related to networking. If later on, other
> subsystems decide they want to add their protocol as well to the list
> of allowed protocols they shall simply add it. While at it, we also
> need to tell what protocol is in use otherwise BPF_S_ANC_PROTOCOL can
> not pick it up (as it's not filled out).
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>

This takes away functionality that I'd be more interesting in using,
namely being able to listen to all netlink protocols using one tap.

Seriously, when I first saw this feature, that was the first way I'd
imagine myself using it, as a tcpdump for netlink traffic, all of
it.

If I just want to hear all netlink traffic, don't make me be forced to
know every single NETLINK_* protocol value and have to open that many
sockets just to do so.

It also makes it so that I can't listen to userlevel custom netlink
protocols, another minus of filtering.

At the very least, allow an sk_protocol of zero or similar to have this
meaning of "everything".

I'm not applying this patch, sorry.

Re: [PATCH net] net: netlink: filter particular protocols from analyzers

[Stephen Hemminger]

On Thu, 05 Sep 2013 14:44:42 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:

> From: Daniel Borkmann <dborkman@redhat.com>
> Date: Thu,  5 Sep 2013 17:48:47 +0200
> 
> > From: Daniel Borkmann <dborkmann@redhat.com>
> > 
> > Fix finer-grained control and let only a whitelist of allowed netlink
> > protocols pass, in our case related to networking. If later on, other
> > subsystems decide they want to add their protocol as well to the list
> > of allowed protocols they shall simply add it. While at it, we also
> > need to tell what protocol is in use otherwise BPF_S_ANC_PROTOCOL can
> > not pick it up (as it's not filled out).
> > 
> > Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> 
> This takes away functionality that I'd be more interesting in using,
> namely being able to listen to all netlink protocols using one tap.
> 
> Seriously, when I first saw this feature, that was the first way I'd
> imagine myself using it, as a tcpdump for netlink traffic, all of
> it.
> 
> If I just want to hear all netlink traffic, don't make me be forced to
> know every single NETLINK_* protocol value and have to open that many
> sockets just to do so.
> 
> It also makes it so that I can't listen to userlevel custom netlink
> protocols, another minus of filtering.
> 
> At the very least, allow an sk_protocol of zero or similar to have this
> meaning of "everything".
> 
> I'm not applying this patch, sorry.
> --
> 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


If you want filtering, why not add BPF (sk_filter) support to this?

Re: [PATCH net] net: netlink: filter particular protocols from analyzers

[David Miller]

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Thu, 5 Sep 2013 11:57:55 -0700

> If you want filtering, why not add BPF (sk_filter) support to this?

That's one idea.

But using sk_protocol is at least consistent with how AF_PACKET does
things.

Re: [PATCH net] net: netlink: filter particular protocols from analyzers

[Stephen Hemminger]

On Thu, 05 Sep 2013 15:03:41 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Thu, 5 Sep 2013 11:57:55 -0700
> 
> > If you want filtering, why not add BPF (sk_filter) support to this?
> 
> That's one idea.
> 
> But using sk_protocol is at least consistent with how AF_PACKET does
> things.

Why not both.

Re: [PATCH net] net: netlink: filter particular protocols from analyzers

[David Miller]

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Thu, 5 Sep 2013 12:44:37 -0700

> On Thu, 05 Sep 2013 15:03:41 -0400 (EDT)
> David Miller <davem@davemloft.net> wrote:
> 
>> From: Stephen Hemminger <stephen@networkplumber.org>
>> Date: Thu, 5 Sep 2013 11:57:55 -0700
>> 
>> > If you want filtering, why not add BPF (sk_filter) support to this?
>> 
>> That's one idea.
>> 
>> But using sk_protocol is at least consistent with how AF_PACKET does
>> things.
> 
> Why not both.

That's fine too.

Re: [PATCH net] net: netlink: filter particular protocols from analyzers

[Daniel Borkmann]

On 09/05/2013 08:44 PM, David Miller wrote:
> From: Daniel Borkmann <dborkman@redhat.com>
> Date: Thu,  5 Sep 2013 17:48:47 +0200
>
>> From: Daniel Borkmann <dborkmann@redhat.com>
>>
>> Fix finer-grained control and let only a whitelist of allowed netlink
>> protocols pass, in our case related to networking. If later on, other
>> subsystems decide they want to add their protocol as well to the list
>> of allowed protocols they shall simply add it. While at it, we also
>> need to tell what protocol is in use otherwise BPF_S_ANC_PROTOCOL can
>> not pick it up (as it's not filled out).
>>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>

To answer Stephen and Dave in one message ... ;-)

> This takes away functionality that I'd be more interesting in using,
> namely being able to listen to all netlink protocols using one tap.
>
> Seriously, when I first saw this feature, that was the first way I'd
> imagine myself using it, as a tcpdump for netlink traffic, all of
> it.
>
> If I just want to hear all netlink traffic, don't make me be forced to
> know every single NETLINK_* protocol value and have to open that many
> sockets just to do so.
>
> It also makes it so that I can't listen to userlevel custom netlink
> protocols, another minus of filtering.
>
> At the very least, allow an sk_protocol of zero or similar to have this
> meaning of "everything".

I agree with you with all the above and I think with having this in
tcpdump would be great to debug netlink traffic of course ...

With socket(PF_PACKET, ..., htons(ETH_P_ALL)) you will already get all
users from the suggested white-list of the patch, which is the majority
of netlink users I believe. Hence, you do not need to have one socket
per protocol. skbs from there should get dragged into pf_packet via
dev_queue_xmit_nit() which works on ptype_all list.

Plus, with the nskb->protocol addition in this patch, they can then either
take all netlink traffic from that, or filter with BPF e.g. for particular
protocols via BPF_S_ANC_PROTOCOL or more advanced stuff, for example. Also,
they can select particular dissectors with that information that comes in
via sll_protocol which is useful, of course.

I think for out-of-tree modules, we never really cared much, and they
could use generic netlink probably anyway. The thing why I wanted to
do this is to keep security related messages (audit, selinux) generally
under the radar, which is "more or less" what's remaining, so we still get
main users. This patch would accomplish those things, that's why I think
it's useful.

Re: [PATCH net] net: netlink: filter particular protocols from analyzers

[David Miller]

From: Daniel Borkmann <dborkman@redhat.com>
Date: Thu, 05 Sep 2013 21:48:00 +0200

> With socket(PF_PACKET, ..., htons(ETH_P_ALL)) you will already get
> all users from the suggested white-list of the patch, which is the
> majority of netlink users I believe. Hence, you do not need to have
> one socket per protocol. skbs from there should get dragged into
> pf_packet via dev_queue_xmit_nit() which works on ptype_all list.

What about user level netlink protocols?

Re: [PATCH net] net: netlink: filter particular protocols from analyzers

[Daniel Borkmann]

On 09/05/2013 09:54 PM, David Miller wrote:
> From: Daniel Borkmann <dborkman@redhat.com>
> Date: Thu, 05 Sep 2013 21:48:00 +0200
>
>> With socket(PF_PACKET, ..., htons(ETH_P_ALL)) you will already get
>> all users from the suggested white-list of the patch, which is the
>> majority of netlink users I believe. Hence, you do not need to have
>> one socket per protocol. skbs from there should get dragged into
>> pf_packet via dev_queue_xmit_nit() which works on ptype_all list.
>
> What about user level netlink protocols?

If you are referring to NETLINK_USERSOCK, then we let this pass here,
so nothing changes.

Re: [PATCH net] net: netlink: filter particular protocols from analyzers

[David Miller]

From: Daniel Borkmann <dborkman@redhat.com>
Date: Thu, 05 Sep 2013 21:59:28 +0200

> On 09/05/2013 09:54 PM, David Miller wrote:
>> From: Daniel Borkmann <dborkman@redhat.com>
>> Date: Thu, 05 Sep 2013 21:48:00 +0200
>>
>>> With socket(PF_PACKET, ..., htons(ETH_P_ALL)) you will already get
>>> all users from the suggested white-list of the patch, which is the
>>> majority of netlink users I believe. Hence, you do not need to have
>>> one socket per protocol. skbs from there should get dragged into
>>> pf_packet via dev_queue_xmit_nit() which works on ptype_all list.
>>
>> What about user level netlink protocols?
> 
> If you are referring to NETLINK_USERSOCK, then we let this pass here,
> so nothing changes.

Ok I need to think about this some more, I moved your patch back into under review
state.

Re: [PATCH net] net: netlink: filter particular protocols from analyzers

[David Miller]

From: Daniel Borkmann <dborkman@redhat.com>
Date: Thu, 05 Sep 2013 21:59:28 +0200

> On 09/05/2013 09:54 PM, David Miller wrote:
>> From: Daniel Borkmann <dborkman@redhat.com>
>> Date: Thu, 05 Sep 2013 21:48:00 +0200
>>
>>> With socket(PF_PACKET, ..., htons(ETH_P_ALL)) you will already get
>>> all users from the suggested white-list of the patch, which is the
>>> majority of netlink users I believe. Hence, you do not need to have
>>> one socket per protocol. skbs from there should get dragged into
>>> pf_packet via dev_queue_xmit_nit() which works on ptype_all list.
>>
>> What about user level netlink protocols?
> 
> If you are referring to NETLINK_USERSOCK, then we let this pass here,
> so nothing changes.

Ok, I've applied this, thanks Daniel.