netfilter: socket lookup function refactoring, cgroup match fixes

This series is based on work done by Daniel Borkmann a little while ago:

http://article.gmane.org/gmane.comp.security.firewalls.netfilter.devel/56877

I addressed the feedback from that thread and factored out the socket lookup code into own modules, one for ipv4, one for ipv6. These modules are now selected in kbuild by code that uses it.

Also, a patch was added to fix nft_meta cgroup match rules in a similar fashion as it's now done for xt_cgroup.

Feedback welcome!

Thanks,
Daniel

Daniel Borkmann (1):
netfilter: x_tables: fix cgroup's NF_INET_LOCAL_IN sk lookups

Daniel Mack (2):
netfilter: factor out helpers from xt_socket into separate modules
netfilter: nft_meta: fix cgroup socket lookups

 include/linux/netfilter_ipv4.h    |   6 +
 include/linux/netfilter_ipv6.h    |   5 +
 net/ipv4/netfilter/Makefile       |   3 +
 net/ipv4/netfilter/nf_sock_ipv4.c | 169 +++++++++++++++++++++++
 net/ipv6/netfilter/Makefile       |   3 +
 net/ipv6/netfilter/nf_sock_ipv6.c | 152 +++++++++++++++++++++
 net/netfilter/Kconfig             |  12 ++
 net/netfilter/nft_meta.c          |  35 ++++-
 net/netfilter/xt_cgroup.c         |  95 ++++++++++---
 net/netfilter/xt_socket.c         | 278 ++------------------------------------
 10 files changed, 467 insertions(+), 291 deletions(-)
 create mode 100644 net/ipv4/netfilter/nf_sock_ipv4.c
 create mode 100644 net/ipv6/netfilter/nf_sock_ipv6.c

2.4.0

The socket lookup helpers are also needed for fixing xt_cgroups and nft_meta, therefore factor them out.

Based on a patch from Daniel Borkmann, and reworked so that the code now lives in net/ipv[46]/netfilter/nf_sock_ipv[46].c and is linked into separate modules.

---
 include/linux/netfilter_ipv4.h    |   6 +
 include/linux/netfilter_ipv6.h    |   5 +
 net/ipv4/netfilter/Makefile       |   3 +
 net/ipv4/netfilter/nf_sock_ipv4.c | 169 +++++++++++++++++++++++
 net/ipv6/netfilter/Makefile       |   3 +
 net/ipv6/netfilter/nf_sock_ipv6.c | 153 +++++++++++++++++++++
 net/netfilter/Kconfig             |   8 ++
 net/netfilter/xt_socket.c         | 278 ++------------------------------------
 8 files changed, 355 insertions(+), 270 deletions(-)
 create mode 100644 net/ipv4/netfilter/nf_sock_ipv4.c
 create mode 100644 net/ipv6/netfilter/nf_sock_ipv6.c

index 6e4591b..f13fbc2 100644
@@ -9,4 +9,10 @@
 int ip_route_me_harder(struct sk_buff *skb, unsigned addr_type);
 __sum16 nf_ip_checksum(struct sk_buff *skb, unsigned int hook,
 		       unsigned int dataoff, u_int8_t protocol);
+
+#if IS_ENABLED(CONFIG_NF_SOCK_IPV4)
+struct sock *nf_socket_lookup_v4(const struct sk_buff *skb,
+				 const struct net_device *indev);
+#endif
+
 #endif /*__LINUX_IP_NETFILTER_H*/

index 8b7d28f..d5b3a07 100644
@@ -41,4 +41,9 @@ static inline int ipv6_netfilter_init(void) { return 0; }
 static inline void ipv6_netfilter_fini(void) { return; }
 #endif /* CONFIG_NETFILTER */
 
+#if IS_ENABLED(CONFIG_NF_SOCK_IPV6)
+struct sock *nf_socket_lookup_v6(const struct sk_buff *skb,
+				 const struct net_device *indev);
+#endif
+
 #endif /*__LINUX_IP6_NETFILTER_H*/

index 7fe6c70..c036c18 100644
@@ -35,6 +35,9 @@ obj-$(CONFIG_NF_NAT_MASQUERADE_IPV4) += nf_nat_masquerade_ipv4.o
 # NAT protocols (nf_nat)
 obj-$(CONFIG_NF_NAT_PROTO_GRE) += nf_nat_proto_gre.o
 
+# Socket helpers
+obj-$(CONFIG_NF_SOCK_IPV4) += nf_sock_ipv4.o
+
 obj-$(CONFIG_NF_TABLES_IPV4) += nf_tables_ipv4.o
 obj-$(CONFIG_NFT_CHAIN_ROUTE_IPV4) += nft_chain_route_ipv4.o
 obj-$(CONFIG_NFT_CHAIN_NAT_IPV4) += nft_chain_nat_ipv4.o

index c36e0a5..3a8cf8d 100644
@@ -30,6 +30,9 @@ obj-$(CONFIG_NF_LOG_IPV6) += nf_log_ipv6.o
 # reject
 obj-$(CONFIG_NF_REJECT_IPV6) += nf_reject_ipv6.o
 
+# Socket helpers
+obj-$(CONFIG_NF_SOCK_IPV6) += nf_sock_ipv6.o
+
 # nf_tables
 obj-$(CONFIG_NF_TABLES_IPV6) += nf_tables_ipv6.o
 obj-$(CONFIG_NFT_CHAIN_ROUTE_IPV6) += nft_chain_route_ipv6.o

index fbc8d15..f08e7a8 100644
@@ -12,6 +12,12 @@ config NETFILTER_INGRESS
 config NETFILTER_NETLINK
 	tristate
 
+config NF_SOCK_IPV4
+	tristate
+
+config NF_SOCK_IPV6
+	tristate
+
 config NETFILTER_NETLINK_ACCT
 tristate "Netfilter NFACCT over NFNETLINK interface"
 	depends on NETFILTER_ADVANCED

@@ -1373,6 +1379,8 @@ config NETFILTER_XT_MATCH_SOCKET
 	depends on (IP6_NF_IPTABLES || IP6_NF_IPTABLES=n)
 	select NF_DEFRAG_IPV4
 	select NF_DEFRAG_IPV6 if IP6_NF_IPTABLES
+	select NF_SOCK_IPV4
+	select NF_SOCK_IPV6 if IP6_NF_IPTABLES
 	help
 	  This option adds a `socket' match, which can be used to match
 	  packets for which a TCP or UDP socket lookup finds a valid socket.

2.4.0

From: Daniel Borkmann <[email protected]>

While originally only being intended for outgoing traffic, commit a00e76349f35 ("netfilter: x_tables: allow to use cgroup match for LOCAL_IN nf hooks") enabled xt_cgroups for the NF_INET_LOCAL_IN hook as well, in order to allow for nfacct accounting.

This basically was under the assumption that socket early demux will resolve it. It's correct that demux happens after PRE_ROUTING, but before LOCAL_IN.

However, that as-is only partially works, i.e. it works for the case of established TCP and connected UDP sockets when early demux is enabled, but not for various other ingress scenarios.

Instead of reverting commit a00e76349f35, I think it's worth to fix it up as there are applications requiring xt_cgroup to match on ingress and egress side. In order to do so, we need to perform a full lookup on skb->sk (ingress) miss, similarly as being done in xt_socket.

Therefore, we need to make use of shared helpers nf_socket_lookup_v4() and nf_socket_lookup_v6().

Reference: http://thread.gmane.org/gmane.linux.network/355527

---
 net/netfilter/Kconfig     |  2 +
 net/netfilter/xt_cgroup.c | 95 ++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 79 insertions(+), 18 deletions(-)

index f08e7a8..11c7e37 100644
@@ -981,6 +981,8 @@ config NETFILTER_XT_MATCH_CGROUP
 	tristate '"control group" match support'
 	depends on NETFILTER_ADVANCED
 	depends on CGROUPS
+	select NF_SOCK_IPV4
+	select NF_SOCK_IPV6 if IP6_NF_IPTABLES
 	select CGROUP_NET_CLASSID
 	---help---
 	Socket/process control group matching allows you to match locally

2.4.0

The cgroup matching logic in nft_meta currently bails out early if skb->sk == NULL, ie, when the check is run on a socket that has not yet been looked up by early demux.

To fix this, use the socket lookup helpers to determine the receiver socket in such cases.

---
 net/netfilter/Kconfig    |  2 ++
 net/netfilter/nft_meta.c | 35 ++++++++++++++++++++++++++++++++---
 2 files changed, 34 insertions(+), 3 deletions(-)

index 11c7e37..2d3294c 100644
@@ -475,6 +475,8 @@ config NFT_EXTHDR
 
 config NFT_META
 	tristate "Netfilter nf_tables meta module"
+	select NF_SOCK_IPV4
+	select NF_SOCK_IPV6 if IP6_NF_IPTABLES
 	help
 	  This option adds the "meta" expression that you can use to match and
 	  to set packet metainformation such as the packet mark.

index 52561e1..ecc3de8 100644
@@ -13,6 +13,8 @@
 #include <linux/module.h>
 #include <linux/netlink.h>
 #include <linux/netfilter.h>
+#include <linux/netfilter_ipv4.h>
+#include <linux/netfilter_ipv6.h>
 #include <linux/netfilter/nf_tables.h>
 #include <linux/in.h>
 #include <linux/ip.h>

@@ -166,11 +168,38 @@ void nft_meta_get_eval(const struct nft_expr *expr,
 			goto err;
 		*dest = out->group;
 		break;
-	case NFT_META_CGROUP:
-		if (skb->sk == NULL || !sk_fullsock(skb->sk))
+	case NFT_META_CGROUP: {
+		struct sock *sk;
+
+		if (skb->sk && sk_fullsock(skb->sk)) {
+			*dest = skb->sk->sk_classid;
+			break;
+		}
+
+		if (!in)
 			goto err;
-		*dest = skb->sk->sk_classid;
+
+		switch (pkt->ops->pf) {
+		case NFPROTO_IPV4:
+			sk = nf_socket_lookup_v4(skb, in);
+			break;
+		case NFPROTO_IPV6:
+			sk = nf_socket_lookup_v6(skb, in);
+			break;
+		default:
+			goto err;
+		}
+
+		if (!sk)
+			goto err;
+
+		*dest = sk->sk_classid;
+
+		if (!sk_fullsock(sk))
+			sock_gen_put(sk);
+
 		break;
+	}
 	default:
 		WARN_ON(1);
 		goto err;

2.4.0

Then, you probably missed these:

http://permalink.gmane.org/gmane.comp.security.firewalls.netfilter.devel/56879

That means that we will not take any rework of the existing socket infrastructure in netfilter/iptables. The socket match was designed for TPROXY, not for firewalling as it will only work under very specific assumptions.

The possible ideas that we (Patrick and me) have discussed boil down to:

1) Placing the INPUT hook at later stage, from the layer 4 protocols.

or

2) Add a new socket hook in the path from layer 4 protocols.

In both cases, the idea is that we're always 100% sure that we have a valid sk in place, including UDP multicast.

Hi Pablo,

Thanks a lot for the feedback!

Yeah, right. The thing is that cgroup matching is an incredible useful feature. It's a pity it isn't currently usable for ingress packets.

That would be great, but the consequences for other matching functions are beyond my current knowledge of the netfilter code.

That sounds reasonable too. That option would at least work for nft then, right? Is anyone working on that yet?

Thanks,
Daniel

[...]

Not yet, but Daniel Borkmann will likely bring up this issue during the Netfilter Workshop that will happen the next week.

On 06/17/2015 12:34 PM, Pablo Neira Ayuso wrote:
...

Yes, added a slot on this topic for Thursday.

Thanks,
Daniel