[PATCH netfilter/iptables] Add new slavedev match extension

[PATCH netfilter/iptables] Add new slavedev match extension

[Martin Willi]

This patchset introduces a new Netfilter match extension to match input
interfaces that are associated to a layer 3 master device. The first 
patch adds the new match to the kernel, the other provides an extension 
to userspace iptables to make use of the new match.

The motivation for a new match is that in INPUT/FORWARD, a base match
for the input interface is done against the layer 3 master device if
the real input device is associated to such a device. This makes
filtering on input interfaces within VRFs difficult.

In output, the packet is passed to Netfilter with the real output
interface as well, so output interface matching in slavedev is not
required. Nonetheless are the arguments named explicitly for the input
interface, as it makes the meaning of these options more intuitive
and the match extensible.

An alternative approach for better filtering within VRFs could be to pass
the packet with the real interface to FORWARD/INPUT hooks, or even pass 
it twice similar to the output path. This is very likely to break 
existing rulesets, though, which should be no problem with a new match
extension.
--
2.20.1

[PATCH nf-next] netfilter: xt_slavedev: Add new L3master slave input device match

[Martin Willi]

When filtering in INPUT or FORWARD within a VRF domain, input interface
matching is done against the VRF device name instead of the real input
interface. This makes interface based filtering difficult if the interface
is associated to a VRF or some other layer 3 master device. While
PREROUTING can match against the real input interface, using it for
filtering is often inconvenient.

To allow filtering in INPUT/FORWARD against the real input interface,
add a match extension for this specific purpose. It is very similar
to the layer 2 slave device match implemented in xt_physdev to match
against bridge ports, but matches on layer 3 slave devices.

As an option, the user may specify a "strict" flag, limiting matches to
interfaces that strictly are layer 3 slave devices.

Signed-off-by: Martin Willi <martin@strongswan.org>
---
 include/net/ip.h                           |  2 +-
 include/uapi/linux/netfilter/xt_slavedev.h | 18 +++++
 net/netfilter/Kconfig                      | 12 ++++
 net/netfilter/Makefile                     |  1 +
 net/netfilter/xt_slavedev.c                | 80 ++++++++++++++++++++++
 5 files changed, 112 insertions(+), 1 deletion(-)
 create mode 100644 include/uapi/linux/netfilter/xt_slavedev.h
 create mode 100644 net/netfilter/xt_slavedev.c

diff --git a/include/net/ip.h b/include/net/ip.h
index 5b317c9f4470..12cd3971f4cf 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -98,7 +98,7 @@ static inline void ipcm_init_sk(struct ipcm_cookie *ipcm,
 #define PKTINFO_SKB_CB(skb) ((struct in_pktinfo *)((skb)->cb))
 
 /* return enslaved device index if relevant */
-static inline int inet_sdif(struct sk_buff *skb)
+static inline int inet_sdif(const struct sk_buff *skb)
 {
 #if IS_ENABLED(CONFIG_NET_L3_MASTER_DEV)
 	if (skb && ipv4_l3mdev_skb(IPCB(skb)->flags))
diff --git a/include/uapi/linux/netfilter/xt_slavedev.h b/include/uapi/linux/netfilter/xt_slavedev.h
new file mode 100644
index 000000000000..d35785b04c4b
--- /dev/null
+++ b/include/uapi/linux/netfilter/xt_slavedev.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_XT_SLAVEDEV_H
+#define _UAPI_XT_SLAVEDEV_H
+
+#include <linux/types.h>
+#include <linux/if.h>
+
+#define XT_SLAVEDEV_IN_INV	0x01	/* invert interface match */
+#define XT_SLAVEDEV_IN_STRICT	0x02	/* require iif to be enslaved */
+#define XT_SLAVEDEV_MASK	(0x04 - 1)
+
+struct xt_slavedev_info {
+	char iniface[IFNAMSIZ];
+	unsigned char iniface_mask[IFNAMSIZ];
+	__u8 flags;
+};
+
+#endif /* _UAPI_XT_SLAVEDEV_H */
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 91efae88e8c2..a259192bdb2e 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -1530,6 +1530,18 @@ config NETFILTER_XT_MATCH_SCTP
 	  If you want to compile it as a module, say M here and read
 	  <file:Documentation/kbuild/modules.rst>.  If unsure, say `N'.
 
+config NETFILTER_XT_MATCH_SLAVEDEV
+	tristate '"slavedev" match support'
+	depends on NET_L3_MASTER_DEV
+	depends on NETFILTER_ADVANCED
+	help
+	  Slavedev packet matching matches against the input slave interface
+	  if the IP packet arrived on a layer 3 master device. It allows
+	  matching against the input interface if that interface is
+	  associated to a VRF master device.
+
+	  To compile it as a module, choose M here.  If unsure, say N.
+
 config NETFILTER_XT_MATCH_SOCKET
 	tristate '"socket" match support'
 	depends on NETFILTER_XTABLES
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 5e9b2eb24349..9650d1d5216c 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -197,6 +197,7 @@ obj-$(CONFIG_NETFILTER_XT_MATCH_RATEEST) += xt_rateest.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_REALM) += xt_realm.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_RECENT) += xt_recent.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_SCTP) += xt_sctp.o
+obj-$(CONFIG_NETFILTER_XT_MATCH_SLAVEDEV) += xt_slavedev.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_SOCKET) += xt_socket.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_STATE) += xt_state.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_STATISTIC) += xt_statistic.o
diff --git a/net/netfilter/xt_slavedev.c b/net/netfilter/xt_slavedev.c
new file mode 100644
index 000000000000..a53466dfb10b
--- /dev/null
+++ b/net/netfilter/xt_slavedev.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <net/ip.h>
+#include <net/ipv6.h>
+#include <linux/module.h>
+#include <linux/netfilter/x_tables.h>
+#include <uapi/linux/netfilter/xt_slavedev.h>
+
+static bool
+slavedev_mt(const struct sk_buff *skb, struct xt_action_param *par)
+{
+	const struct xt_slavedev_info *info = par->matchinfo;
+	struct net_device *sd;
+	unsigned long ret = 1;
+	int sdif = 0;
+
+	switch (xt_family(par)) {
+	case NFPROTO_IPV4:
+		sdif = inet_sdif(skb);
+		break;
+	case NFPROTO_IPV6:
+		sdif = inet6_sdif(skb);
+		break;
+	}
+	if (sdif) {
+		sd = dev_get_by_index_rcu(xt_net(par), sdif);
+		if (sd)
+			ret = ifname_compare_aligned(sd->name, info->iniface,
+						     info->iniface_mask);
+	} else if (!(info->flags & XT_SLAVEDEV_IN_STRICT) && xt_in(par)) {
+		ret = ifname_compare_aligned(xt_inname(par), info->iniface,
+					     info->iniface_mask);
+	}
+
+	if (!ret ^ !(info->flags & XT_SLAVEDEV_IN_INV))
+		return false;
+
+	return true;
+}
+
+static int slavedev_mt_check(const struct xt_mtchk_param *par)
+{
+	const struct xt_slavedev_info *info = par->matchinfo;
+
+	if (info->flags & ~XT_SLAVEDEV_MASK)
+		return -EINVAL;
+
+	return 0;
+}
+
+static struct xt_match slavedev_mt_reg __read_mostly = {
+	.name		= "slavedev",
+	.revision	= 0,
+	.family		= NFPROTO_UNSPEC,
+	.checkentry	= slavedev_mt_check,
+	.match		= slavedev_mt,
+	.matchsize	= sizeof(struct xt_slavedev_info),
+	.hooks		= (1 << NF_INET_LOCAL_IN) |
+			  (1 << NF_INET_FORWARD),
+	.me		= THIS_MODULE,
+};
+
+static int __init slavedev_mt_init(void)
+{
+	return xt_register_match(&slavedev_mt_reg);
+}
+
+static void __exit slavedev_mt_exit(void)
+{
+	xt_unregister_match(&slavedev_mt_reg);
+}
+
+module_init(slavedev_mt_init);
+module_exit(slavedev_mt_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Martin Willi <martin@strongswan.org>");
+MODULE_DESCRIPTION("Xtables: L3master input slave device match");
+MODULE_ALIAS("ipt_slavedev");
+MODULE_ALIAS("ip6t_slavedev");
-- 
2.20.1

[PATCH iptables] extensions: Add new xt_slavedev input interface match extension

[Martin Willi]

When filtering in INPUT or FORWARD within a VRF domain, input interface
matching is done against the VRF device name instead of the real input
interface. This makes interface based filtering difficult if the interface
is associated to a VRF or some other layer 3 master device. While
PREROUTING can match against the real input interface, using it for
filtering is often inconvenient.

To allow filtering in INPUT/FORWARD against the real input interface,
add a match extension for this specific purpose. It is very similar
to the layer 2 slave device match implemented in xt_physdev to match
against bridge ports, but matches on layer 3 slave devices.

As an option, the user may specify a "strict" flag, limiting matches to
interfaces that strictly are layer 3 slave devices.

Signed-off-by: Martin Willi <martin@strongswan.org>
---
 extensions/libxt_slavedev.c           | 98 +++++++++++++++++++++++++++
 extensions/libxt_slavedev.man         | 19 ++++++
 extensions/libxt_slavedev.t           |  4 ++
 include/linux/netfilter/xt_slavedev.h | 18 +++++
 4 files changed, 139 insertions(+)
 create mode 100644 extensions/libxt_slavedev.c
 create mode 100644 extensions/libxt_slavedev.man
 create mode 100644 extensions/libxt_slavedev.t
 create mode 100644 include/linux/netfilter/xt_slavedev.h

diff --git a/extensions/libxt_slavedev.c b/extensions/libxt_slavedev.c
new file mode 100644
index 000000000000..d957c889e1a1
--- /dev/null
+++ b/extensions/libxt_slavedev.c
@@ -0,0 +1,98 @@
+#include <stdio.h>
+#include <string.h>
+#include <xtables.h>
+#include <linux/netfilter/xt_slavedev.h>
+
+enum {
+	O_SLAVEDEV_IN = 0,
+	O_SLAVEDEV_IN_STRICT,
+};
+
+static void slavedev_help(void)
+{
+	printf(
+"slavedev match options:\n"
+" [!] --slavedev-in inputname[+]	input device name ([+] for wildcard)\n"
+"     --slavedev-in-strict		input device must be layer 3 slave device\n");
+}
+
+static const struct xt_option_entry slavedev_opts[] = {
+	{.name = "slavedev-in", .id = O_SLAVEDEV_IN, .type = XTTYPE_STRING,
+	 .flags = XTOPT_INVERT | XTOPT_PUT,
+	 XTOPT_POINTER(struct xt_slavedev_info, iniface)},
+	{.name = "slavedev-in-strict", .id = O_SLAVEDEV_IN_STRICT,
+	 .type = XTTYPE_NONE},
+	XTOPT_TABLEEND,
+};
+
+static void slavedev_parse(struct xt_option_call *cb)
+{
+	struct xt_slavedev_info *info = cb->data;
+
+	xtables_option_parse(cb);
+	switch (cb->entry->id) {
+	case O_SLAVEDEV_IN:
+		xtables_parse_interface(cb->arg, info->iniface,
+					info->iniface_mask);
+		if (cb->invert)
+			info->flags |= XT_SLAVEDEV_IN_INV;
+		break;
+	case O_SLAVEDEV_IN_STRICT:
+		info->flags |= XT_SLAVEDEV_IN_STRICT;
+		break;
+	}
+}
+
+static bool slavedev_has_iniface(const struct xt_slavedev_info *info)
+{
+	int i;
+
+	for (i = 0; i < sizeof(info->iniface_mask); i++)
+		if (info->iniface_mask[i])
+			return true;
+	return false;
+}
+
+static void
+slavedev_print(const void *ip, const struct xt_entry_match *match, int numeric)
+{
+	const struct xt_slavedev_info *info = (const void *)match->data;
+
+	printf(" slavedev");
+	if (slavedev_has_iniface(info))
+		printf("%s in %s",
+		       (info->flags & XT_SLAVEDEV_IN_INV) ? " !" : "",
+		       info->iniface);
+	if (info->flags & XT_SLAVEDEV_IN_STRICT)
+		printf(" in-strict");
+}
+
+static void slavedev_save(const void *ip, const struct xt_entry_match *match)
+{
+	const struct xt_slavedev_info *info = (const void *)match->data;
+
+	if (slavedev_has_iniface(info))
+		printf("%s --slavedev-in %s",
+		       (info->flags & XT_SLAVEDEV_IN_INV) ? " !" : "",
+		       info->iniface);
+	if (info->flags & XT_SLAVEDEV_IN_STRICT)
+		printf(" --slavedev-in-strict");
+}
+
+static struct xtables_match slavedev_match = {
+	.family		= NFPROTO_UNSPEC,
+	.name		= "slavedev",
+	.version	= XTABLES_VERSION,
+	.size		= XT_ALIGN(sizeof(struct xt_slavedev_info)),
+	.userspacesize	= XT_ALIGN(sizeof(struct xt_slavedev_info)),
+	.help		= slavedev_help,
+	.print		= slavedev_print,
+	.save		= slavedev_save,
+	.x6_parse	= slavedev_parse,
+	.x6_options	= slavedev_opts,
+};
+
+void _init(void)
+{
+	xtables_register_match(&slavedev_match);
+}
diff --git a/extensions/libxt_slavedev.man b/extensions/libxt_slavedev.man
new file mode 100644
index 000000000000..127eab4872f4
--- /dev/null
+++ b/extensions/libxt_slavedev.man
@@ -0,0 +1,19 @@
+This module matches on the real input interface enslaved to layer 3 master
+devices. For devices associated to VRF interfaces, the standard matching in
+\fBINPUT\fP and \fBFORWARD\fP chains can match against the VRF interface
+name, only. The slavedev match can match against the real input interface
+instead.
+.PP
+The slavedef match is valid in the \fBINPUT\fP and \fBFORWARD\fP chains
+only, as \fBPREROUTING\fP always matches against the real input interface.
+.TP
+[\fB!\fP] \fB\-\-slavedev\-in\fP \fIname\fP
+Name of a slave device the packet has been received on. If the interface name
+ends in a "+", then any interface which begins with this name will match.
+Not specifying an input interface can be used to match any packet that
+was received over a layer 3 slave device if the strict flag below is given.
+.TP
+\fB\-\-slavedev\-in\-strict\fP
+Matches only if the input interface is actually a layer 3 slave device,
+i.e. is associated to a VRF domain. If this flag is omitted, it also
+matches input interfaces without a layer 3 master device.
diff --git a/extensions/libxt_slavedev.t b/extensions/libxt_slavedev.t
new file mode 100644
index 000000000000..80b0ff3b4264
--- /dev/null
+++ b/extensions/libxt_slavedev.t
@@ -0,0 +1,4 @@
+:INPUT,FORWARD
+-m slavedev --slavedev-in lo;=;OK
+-m slavedev --slavedev-in-strict;=;OK
+-m slavedev --slavedev-in eth+ --slavedev-in-strict;=;OK
diff --git a/include/linux/netfilter/xt_slavedev.h b/include/linux/netfilter/xt_slavedev.h
new file mode 100644
index 000000000000..d35785b04c4b
--- /dev/null
+++ b/include/linux/netfilter/xt_slavedev.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_XT_SLAVEDEV_H
+#define _UAPI_XT_SLAVEDEV_H
+
+#include <linux/types.h>
+#include <linux/if.h>
+
+#define XT_SLAVEDEV_IN_INV	0x01	/* invert interface match */
+#define XT_SLAVEDEV_IN_STRICT	0x02	/* require iif to be enslaved */
+#define XT_SLAVEDEV_MASK	(0x04 - 1)
+
+struct xt_slavedev_info {
+	char iniface[IFNAMSIZ];
+	unsigned char iniface_mask[IFNAMSIZ];
+	__u8 flags;
+};
+
+#endif /* _UAPI_XT_SLAVEDEV_H */
-- 
2.20.1

Re: [PATCH netfilter/iptables] Add new slavedev match extension

[Martin Willi]

Pablo,

> This patchset introduces a new Netfilter match extension to match
> input interfaces that are associated to a layer 3 master device. The
> first patch adds the new match to the kernel, the other provides an
> extension to userspace iptables to make use of the new match.

These patches have been marked as superseded in patchworks, likely due
to Florian's nftables version that has been merged.

While I very much appreciate the addition of VRF interface matching to
nftables, some users still depend on plain iptables for filtering. So I
guess there is some value in these patches for those users to extend
their filtering with VRF support.

Is there a chance of bringing in slavedev matching for iptables, or do
we have a policy to implement new features in nftables, only?

Thanks,
Martin 

Re: [PATCH netfilter/iptables] Add new slavedev match extension

[Pablo Neira Ayuso]

Hi Marti,

On Fri, Jan 10, 2020 at 05:34:12PM +0100, Martin Willi wrote:
> Pablo,
> 
> > This patchset introduces a new Netfilter match extension to match
> > input interfaces that are associated to a layer 3 master device. The
> > first patch adds the new match to the kernel, the other provides an
> > extension to userspace iptables to make use of the new match.
> 
> These patches have been marked as superseded in patchworks, likely due
> to Florian's nftables version that has been merged.
> 
> While I very much appreciate the addition of VRF interface matching to
> nftables, some users still depend on plain iptables for filtering. So I
> guess there is some value in these patches for those users to extend
> their filtering with VRF support.

A single xt_slavedev module only for this is too much overhead, if you
find an existing extension (via revision infrastructure) where you can
make this fit in, I would consider this.

Thanks.

Re: [PATCH netfilter/iptables] Add new slavedev match extension

[Martin Willi]

> > So I guess there is some value in these patches for those users to
> > extend their filtering with VRF support.
> 
> A single xt_slavedev module only for this is too much overhead, if
> you find an existing extension (via revision infrastructure) where
> you can make this fit in, I would consider this.

The only feasible candidate I see is the physdev match. However, there
is not much in common code-wise. And from a user perspective, slavedev
matching via physdev and the interaction between these functionalities
just makes that confusing.

So for now I'll keep the slavedev match out-of-tree, then.

Thanks,
Martin