[PATCH net-next 0/2] netfilter: IPv4/v6 IPcomp match support

[PATCH net-next 0/2] netfilter: IPv4/v6 IPcomp match support

[Fan Du]

Hi,

This patchset adds IPv4/v6 IPComp 'match' plugin to enables user setting
ACTONs for IPcomp flows sepecified with SPI value.

Corresponding iptables patchset will be sent here after soon.

Fan Du (2):
  netfilter: add IPv4 IPComp extension match support
  netfilter: add IPv6 IPComp extension match support

 include/uapi/linux/netfilter_ipv4/ipt_comp.h  |   18 ++++
 include/uapi/linux/netfilter_ipv6/ip6t_comp.h |   18 ++++
 net/ipv4/netfilter/Kconfig                    |    9 ++
 net/ipv4/netfilter/Makefile                   |    1 +
 net/ipv4/netfilter/ipt_comp.c                 |  101 +++++++++++++++++++++++
 net/ipv6/netfilter/Kconfig                    |    8 ++
 net/ipv6/netfilter/Makefile                   |    1 +
 net/ipv6/netfilter/ip6t_comp.c                |  110 +++++++++++++++++++++++++
 8 files changed, 266 insertions(+)
 create mode 100644 include/uapi/linux/netfilter_ipv4/ipt_comp.h
 create mode 100644 include/uapi/linux/netfilter_ipv6/ip6t_comp.h
 create mode 100644 net/ipv4/netfilter/ipt_comp.c
 create mode 100644 net/ipv6/netfilter/ip6t_comp.c

-- 
1.7.9.5

[PATCH 1/2] netfilter: add IPv4 IPComp extension match support

[Fan Du]

With this plugin, user could specify IPComp tagged with certain
CPI that host not interested will be DROPped or any other action.

For example:
iptables -A INPUT -p 108 -m ipcomp --ipcompspi 0x87 -j DROP

Then input IPComp packet with CPI equates 0x87 will not reach
upper layer anymore.

Signed-off-by: Fan Du <fan.du@windriver.com>
---
 include/uapi/linux/netfilter_ipv4/ipt_comp.h |   18 +++++
 net/ipv4/netfilter/Kconfig                   |    9 +++
 net/ipv4/netfilter/Makefile                  |    1 +
 net/ipv4/netfilter/ipt_comp.c                |  101 ++++++++++++++++++++++++++
 4 files changed, 129 insertions(+)
 create mode 100644 include/uapi/linux/netfilter_ipv4/ipt_comp.h
 create mode 100644 net/ipv4/netfilter/ipt_comp.c

diff --git a/include/uapi/linux/netfilter_ipv4/ipt_comp.h b/include/uapi/linux/netfilter_ipv4/ipt_comp.h
new file mode 100644
index 0000000..37172fa
--- /dev/null
+++ b/include/uapi/linux/netfilter_ipv4/ipt_comp.h
@@ -0,0 +1,18 @@
+#ifndef _IPT_COMP_H
+#define _IPT_COMP_H
+
+#include <linux/types.h>
+
+struct ipt_comp {
+	__u32 spis[2];	/* Security Parameter Index */
+	__u8 invflags;	/* Inverse flags */
+	__u8 hdrres;	/* Test of the Reserved Filed */
+};
+
+
+
+/* Values for "invflags" field in struct ipt_comp. */
+#define IPT_IPCOMP_INV_SPI	0x01	/* Invert the sense of spi. */
+#define IPT_IPCOMP_INV_MASK	0x01	/* All possible flags. */
+
+#endif /*_IPT_COMP_H*/
diff --git a/net/ipv4/netfilter/Kconfig b/net/ipv4/netfilter/Kconfig
index 40d5607..f71cf7b 100644
--- a/net/ipv4/netfilter/Kconfig
+++ b/net/ipv4/netfilter/Kconfig
@@ -81,6 +81,15 @@ config IP_NF_MATCH_AH
 
 	  To compile it as a module, choose M here.  If unsure, say N.
 
+config IP_NF_MATCH_IPCOMP
+	tristate '"ipcomp" match support'
+	depends on NETFILTER_ADVANCED
+	help
+	  This match extension allows you to match a range of SPIs (Actually it's
+	  Compression Parameter Index(CPI) inside IPComp header of IPSec packets.
+
+	  To compile it as a module, choose M here.  If unsure, say N.
+
 config IP_NF_MATCH_ECN
 	tristate '"ecn" match support'
 	depends on NETFILTER_ADVANCED
diff --git a/net/ipv4/netfilter/Makefile b/net/ipv4/netfilter/Makefile
index 19df72b..b67ecb8 100644
--- a/net/ipv4/netfilter/Makefile
+++ b/net/ipv4/netfilter/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_IP_NF_SECURITY) += iptable_security.o
 
 # matches
 obj-$(CONFIG_IP_NF_MATCH_AH) += ipt_ah.o
+obj-$(CONFIG_IP_NF_MATCH_IPCOMP) += ipt_comp.o
 obj-$(CONFIG_IP_NF_MATCH_RPFILTER) += ipt_rpfilter.o
 
 # targets
diff --git a/net/ipv4/netfilter/ipt_comp.c b/net/ipv4/netfilter/ipt_comp.c
new file mode 100644
index 0000000..7796e6e
--- /dev/null
+++ b/net/ipv4/netfilter/ipt_comp.c
@@ -0,0 +1,101 @@
+/*  Kernel module to match IPComp parameters
+ *
+ *  Copyright (C) 2013 WindRiver
+ *
+ *  Author:
+ *  Fan Du <fan.du@windriver.com>
+ *
+ *  Based on:
+ *  net/ipv4/netfilter/ipt_ah.c
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/in.h>
+#include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/ip.h>
+
+#include <linux/netfilter_ipv4/ipt_comp.h>
+#include <linux/netfilter/x_tables.h>
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Fan Du <fan.du@windriver.com>");
+MODULE_DESCRIPTION("Xtables: IPv4 IPsec-IPComp SPI match");
+
+/* Returns 1 if the spi is matched by the range, 0 otherwise */
+static inline bool
+spi_match(u_int32_t min, u_int32_t max, u_int32_t spi, bool invert)
+{
+	bool r;
+	pr_debug("spi_match:%c 0x%x <= 0x%x <= 0x%x\n",
+		 invert ? '!' : ' ', min, spi, max);
+	r = (spi >= min && spi <= max) ^ invert;
+	pr_debug(" result %s\n", r ? "PASS" : "FAILED");
+	return r;
+}
+
+static bool comp_mt(const struct sk_buff *skb, struct xt_action_param *par)
+{
+	struct ip_comp_hdr _comphdr;
+	const struct ip_comp_hdr *chdr;
+	const struct ipt_comp *compinfo = par->matchinfo;
+
+	/* Must not be a fragment. */
+	if (par->fragoff != 0)
+		return false;
+
+	chdr = skb_header_pointer(skb, par->thoff, sizeof(_comphdr), &_comphdr);
+	if (chdr == NULL) {
+		/* We've been asked to examine this packet, and we
+		 * can't.  Hence, no choice but to drop.
+		 */
+		pr_debug("Dropping evil IPComp tinygram.\n");
+		par->hotdrop = true;
+		return 0;
+	}
+
+	return spi_match(compinfo->spis[0], compinfo->spis[1],
+			 ntohl(chdr->cpi << 16),
+			 !!(compinfo->invflags & IPT_IPCOMP_INV_SPI));
+}
+
+static int comp_mt_check(const struct xt_mtchk_param *par)
+{
+	const struct ipt_comp *compinfo = par->matchinfo;
+
+	/* Must specify no unknown invflags */
+	if (compinfo->invflags & ~IPT_IPCOMP_INV_MASK) {
+		pr_debug("unknown flags %X\n", compinfo->invflags);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static struct xt_match comp_mt_reg __read_mostly = {
+	.name		= "ipcomp",
+	.family		= NFPROTO_IPV4,
+	.match		= comp_mt,
+	.matchsize	= sizeof(struct ipt_comp),
+	.proto		= IPPROTO_COMP,
+	.checkentry	= comp_mt_check,
+	.me		= THIS_MODULE,
+};
+
+static int __init comp_mt_init(void)
+{
+	return xt_register_match(&comp_mt_reg);
+}
+
+static void __exit comp_mt_exit(void)
+{
+	xt_unregister_match(&comp_mt_reg);
+}
+
+module_init(comp_mt_init);
+module_exit(comp_mt_exit);
-- 
1.7.9.5

[PATCH 2/2] netfilter: add IPv6 IPComp extension match support

[Fan Du]

With this plugin, user could specify IPComp tagged with certain
CPI that host not interested will be DROPped or any other action.

For example:
ip6tables -A INPUT -p 108 -m ipcomp --ipcompspi 0x87 -j DROP

Then input IPComp packet with CPI equates 0x87 will not reach
upper layer anymore.

Signed-off-by: Fan Du <fan.du@windriver.com>
---
 include/uapi/linux/netfilter_ipv6/ip6t_comp.h |   18 ++++
 net/ipv6/netfilter/Kconfig                    |    8 ++
 net/ipv6/netfilter/Makefile                   |    1 +
 net/ipv6/netfilter/ip6t_comp.c                |  110 +++++++++++++++++++++++++
 4 files changed, 137 insertions(+)
 create mode 100644 include/uapi/linux/netfilter_ipv6/ip6t_comp.h
 create mode 100644 net/ipv6/netfilter/ip6t_comp.c

diff --git a/include/uapi/linux/netfilter_ipv6/ip6t_comp.h b/include/uapi/linux/netfilter_ipv6/ip6t_comp.h
new file mode 100644
index 0000000..f2eecdd
--- /dev/null
+++ b/include/uapi/linux/netfilter_ipv6/ip6t_comp.h
@@ -0,0 +1,18 @@
+#ifndef _IP6T_COMP_H
+#define _IP6T_COMP_H
+
+#include <linux/types.h>
+
+struct ip6t_comp {
+	__u32 spis[2];			/* Security Parameter Index */
+	__u8  hdrres;			/* Test of the Reserved Filed */
+	__u8  invflags;			/* Inverse flags */
+};
+
+#define IP6T_IPCOMP_SPI 0x01
+#define IP6T_IPCOMP_RES 0x02
+
+#define IP6T_IPCOMP_INV_SPI	0x01	/* Invert the sense of spi. */
+#define IP6T_IPCOMP_INV_MASK	0x03	/* All possible flags. */
+
+#endif /*_IP6T_COMP_H*/
diff --git a/net/ipv6/netfilter/Kconfig b/net/ipv6/netfilter/Kconfig
index 7702f9e..3f5e603 100644
--- a/net/ipv6/netfilter/Kconfig
+++ b/net/ipv6/netfilter/Kconfig
@@ -62,6 +62,14 @@ config IP6_NF_MATCH_AH
 
 	  To compile it as a module, choose M here.  If unsure, say N.
 
+config IP6_NF_MATCH_IPCOMP
+	tristate '"ipcomp" match support'
+	depends on NETFILTER_ADVANCED
+	help
+	  This module allows one to match IPcomp packets.
+
+	  To compile it as a module, choose M here.  If unsure, say N.
+
 config IP6_NF_MATCH_EUI64
 	tristate '"eui64" address check'
 	depends on NETFILTER_ADVANCED
diff --git a/net/ipv6/netfilter/Makefile b/net/ipv6/netfilter/Makefile
index d1b4928..602ab70 100644
--- a/net/ipv6/netfilter/Makefile
+++ b/net/ipv6/netfilter/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_NFT_CHAIN_NAT_IPV6) += nft_chain_nat_ipv6.o
 
 # matches
 obj-$(CONFIG_IP6_NF_MATCH_AH) += ip6t_ah.o
+obj-$(CONFIG_IP6_NF_MATCH_IPCOMP) += ip6t_comp.o
 obj-$(CONFIG_IP6_NF_MATCH_EUI64) += ip6t_eui64.o
 obj-$(CONFIG_IP6_NF_MATCH_FRAG) += ip6t_frag.o
 obj-$(CONFIG_IP6_NF_MATCH_IPV6HEADER) += ip6t_ipv6header.o
diff --git a/net/ipv6/netfilter/ip6t_comp.c b/net/ipv6/netfilter/ip6t_comp.c
new file mode 100644
index 0000000..bd4a0ae
--- /dev/null
+++ b/net/ipv6/netfilter/ip6t_comp.c
@@ -0,0 +1,110 @@
+/*  Kernel module to match IPComp parameters
+ *
+ *  Copyright (C) 2013 WindRiver
+ *
+ *  Author:
+ *  Fan Du <fan.du@windriver.com>
+ *
+ *  Based on:
+ *  net/ipv6/netfilter/ip6t_ah.c
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/types.h>
+#include <net/checksum.h>
+#include <net/ipv6.h>
+
+#include <linux/netfilter/x_tables.h>
+#include <linux/netfilter_ipv6/ip6_tables.h>
+#include <linux/netfilter_ipv6/ip6t_comp.h>
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Fan Du <fan.du@windriver.com>");
+MODULE_DESCRIPTION("Xtables: IPv6 IPsec-IPComp SPI match");
+
+
+/* Returns 1 if the spi is matched by the range, 0 otherwise */
+static inline bool
+spi_match(u_int32_t min, u_int32_t max, u_int32_t spi, bool invert)
+{
+	bool r;
+
+	pr_debug("spi_match:%c 0x%x <= 0x%x <= 0x%x\n",
+		 invert ? '!' : ' ', min, spi, max);
+	r = (spi >= min && spi <= max) ^ invert;
+	pr_debug(" result %s\n", r ? "PASS" : "FAILED");
+	return r;
+}
+
+static bool comp_mt6(const struct sk_buff *skb, struct xt_action_param *par)
+{
+	struct ip_comp_hdr _comp;
+	const struct ip_comp_hdr *comp;
+	const struct ip6t_comp *compinfo = par->matchinfo;
+
+	comp = skb_header_pointer(skb, par->thoff, sizeof(_comp), &_comp);
+	if (comp == NULL) {
+		par->hotdrop = true;
+		return false;
+	}
+
+	pr_debug("IPv6 IPcomp SPI %u %04X ", ntohs(comp->cpi), ntohs(comp->cpi));
+	pr_debug("RES %02X \n", comp->flags);
+
+	pr_debug("IPv6 IPcomp spi %02X ",
+		 spi_match(compinfo->spis[0], compinfo->spis[1],
+			   ntohs(comp->cpi),
+			   !!(compinfo->invflags & IP6T_IPCOMP_INV_SPI)));
+	pr_debug("res %02X %04X %02X\n",
+		 compinfo->hdrres, comp->flags,
+		 !(compinfo->hdrres && comp->flags));
+
+	return (comp != NULL) &&
+		spi_match(compinfo->spis[0], compinfo->spis[1],
+			  ntohs(comp->cpi),
+			  !!(compinfo->invflags & IP6T_IPCOMP_INV_SPI)) &&
+			  !(compinfo->hdrres && comp->flags);
+}
+
+static int comp_mt6_check(const struct xt_mtchk_param *par)
+{
+	const struct ip6t_comp *compinfo = par->matchinfo;
+
+	if (compinfo->invflags & ~IP6T_IPCOMP_INV_MASK) {
+		pr_debug("unknown flags %X\n", compinfo->invflags);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static struct xt_match comp_mt6_reg __read_mostly = {
+	.name		= "ipcomp",
+	.family		= NFPROTO_IPV6,
+	.match		= comp_mt6,
+	.matchsize	= sizeof(struct ip6t_comp),
+	.checkentry	= comp_mt6_check,
+	.me		= THIS_MODULE,
+};
+
+static int __init comp_mt6_init(void)
+{
+	return xt_register_match(&comp_mt6_reg);
+}
+
+static void __exit comp_mt6_exit(void)
+{
+	xt_unregister_match(&comp_mt6_reg);
+}
+
+module_init(comp_mt6_init);
+module_exit(comp_mt6_exit);
-- 
1.7.9.5

Re: [PATCH net-next 0/2] netfilter: IPv4/v6 IPcomp match support

[Pablo Neira Ayuso]

On Fri, Dec 13, 2013 at 08:18:00PM +0800, Fan Du wrote:
> Hi,
> 
> This patchset adds IPv4/v6 IPComp 'match' plugin to enables user setting
> ACTONs for IPcomp flows sepecified with SPI value.
> 
> Corresponding iptables patchset will be sent here after soon.
> 
> Fan Du (2):
>   netfilter: add IPv4 IPComp extension match support
>   netfilter: add IPv6 IPComp extension match support

This looks good, but I have to ask you to merge those two modules into
one single xt_ipcomp, they are fairly small and we can save the
overhead of having two different modules. Moreover, at quick glance I
don't see any dependency with IPv4/IPv6 exported symbols that may
cause ifdef pollution.

Please, see net/netfilter/xt_tcpudp.c as reference to rework this.
Thanks.

Re: [PATCH net-next 0/2] netfilter: IPv4/v6 IPcomp match support

[Fan Du]

On 2013年12月17日 21:05, Pablo Neira Ayuso wrote:
> On Fri, Dec 13, 2013 at 08:18:00PM +0800, Fan Du wrote:
>> Hi,
>>
>> This patchset adds IPv4/v6 IPComp 'match' plugin to enables user setting
>> ACTONs for IPcomp flows sepecified with SPI value.
>>
>> Corresponding iptables patchset will be sent here after soon.
>>
>> Fan Du (2):
>>    netfilter: add IPv4 IPComp extension match support
>>    netfilter: add IPv6 IPComp extension match support
>
> This looks good, but I have to ask you to merge those two modules into
> one single xt_ipcomp, they are fairly small and we can save the
> overhead of having two different modules. Moreover, at quick glance I
> don't see any dependency with IPv4/IPv6 exported symbols that may
> cause ifdef pollution.
>
> Please, see net/netfilter/xt_tcpudp.c as reference to rework this.
> Thanks.
>

I noticed netfilter ipv4/v6 AH support also has split implementation,
so far, by my understanding, it's fairly enough to consolidate those
two implementations into one as well, as IPv4/6 AH head format are
identical.

If you don't mind or it won't break anything internal for netfilter,
I plan to combine them into one piece.

-- 
浮沉随浪只记今朝笑

--fan
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 0/2] netfilter: IPv4/v6 IPcomp match support

[Pablo Neira Ayuso]

On Thu, Dec 19, 2013 at 11:30:09AM +0800, Fan Du wrote:
> 
> 
> On 2013年12月17日 21:05, Pablo Neira Ayuso wrote:
> >On Fri, Dec 13, 2013 at 08:18:00PM +0800, Fan Du wrote:
> >>Hi,
> >>
> >>This patchset adds IPv4/v6 IPComp 'match' plugin to enables user setting
> >>ACTONs for IPcomp flows sepecified with SPI value.
> >>
> >>Corresponding iptables patchset will be sent here after soon.
> >>
> >>Fan Du (2):
> >>   netfilter: add IPv4 IPComp extension match support
> >>   netfilter: add IPv6 IPComp extension match support
> >
> >This looks good, but I have to ask you to merge those two modules into
> >one single xt_ipcomp, they are fairly small and we can save the
> >overhead of having two different modules. Moreover, at quick glance I
> >don't see any dependency with IPv4/IPv6 exported symbols that may
> >cause ifdef pollution.
> >
> >Please, see net/netfilter/xt_tcpudp.c as reference to rework this.
> >Thanks.
> >
> 
> I noticed netfilter ipv4/v6 AH support also has split implementation,
> so far, by my understanding, it's fairly enough to consolidate those
> two implementations into one as well, as IPv4/6 AH head format are
> identical.
>
> If you don't mind or it won't break anything internal for netfilter,
> I plan to combine them into one piece.

We can merge those two, but you'll have to handle dependencies with
IPv6 core via ifdefs and Kconfig to avoid problems.

AH is not the last header, so we still have to use ipv6_find_hdr() to
find the good header instead of par->thoff. Note that the ip6_tables
sets par->thoff to the last IPv6 extension header.

This rises some concerns regarding your ipcomp, I think that if you
use this with ah and esp, the ordering of the headers is
ah+ipcomp+esp, right? In that case, you need to use ipv6_find_hdr()
since par->thoff would point to the esp header.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 0/2] netfilter: IPv4/v6 IPcomp match support

[Fan Du]

On 2013年12月20日 17:04, Pablo Neira Ayuso wrote:
> On Thu, Dec 19, 2013 at 11:30:09AM +0800, Fan Du wrote:
>>
>>
>> On 2013年12月17日 21:05, Pablo Neira Ayuso wrote:
>>> On Fri, Dec 13, 2013 at 08:18:00PM +0800, Fan Du wrote:
>>>> Hi,
>>>>
>>>> This patchset adds IPv4/v6 IPComp 'match' plugin to enables user setting
>>>> ACTONs for IPcomp flows sepecified with SPI value.
>>>>
>>>> Corresponding iptables patchset will be sent here after soon.
>>>>
>>>> Fan Du (2):
>>>>    netfilter: add IPv4 IPComp extension match support
>>>>    netfilter: add IPv6 IPComp extension match support
>>>
>>> This looks good, but I have to ask you to merge those two modules into
>>> one single xt_ipcomp, they are fairly small and we can save the
>>> overhead of having two different modules. Moreover, at quick glance I
>>> don't see any dependency with IPv4/IPv6 exported symbols that may
>>> cause ifdef pollution.
>>>
>>> Please, see net/netfilter/xt_tcpudp.c as reference to rework this.
>>> Thanks.
>>>
>>
>> I noticed netfilter ipv4/v6 AH support also has split implementation,
>> so far, by my understanding, it's fairly enough to consolidate those
>> two implementations into one as well, as IPv4/6 AH head format are
>> identical.
>>
>> If you don't mind or it won't break anything internal for netfilter,
>> I plan to combine them into one piece.
>
> We can merge those two, but you'll have to handle dependencies with
> IPv6 core via ifdefs and Kconfig to avoid problems.
>
> AH is not the last header, so we still have to use ipv6_find_hdr() to
> find the good header instead of par->thoff. Note that the ip6_tables
> sets par->thoff to the last IPv6 extension header.

I'm quite new to the internal of netfiler, especially about this part.
I will take a look at the code later.

> This rises some concerns regarding your ipcomp, I think that if you
> use this with ah and esp, the ordering of the headers is
> ah+ipcomp+esp, right?

This depends on the user land configuration of encapsulation order.
It can be one of the three types only(ah, esp, ipcomp), the most commonly
used is ah(outer)+esp(inner).

I barely see ipcomp used in production, but I remember RFC says ipcomp
should be done first before esp, because after encryption in esp, the data
is polluted, i.e., not suitable for compressed anymore(I'm not sure the
details theory behind this statement.)

Anyway, no matter how ah,esp,ipcomp are ordered, I think it only makes
sense for the outer header when using any one of those three filters, and
that's what I can imagine how it's been used.

-- 
浮沉随浪只记今朝笑

--fan

Re: [PATCH net-next 0/2] netfilter: IPv4/v6 IPcomp match support

[Pablo Neira Ayuso]

Hi,

On Fri, Dec 20, 2013 at 05:21:05PM +0800, Fan Du wrote:
[...]
> >AH is not the last header, so we still have to use ipv6_find_hdr() to
> >find the good header instead of par->thoff. Note that the ip6_tables
> >sets par->thoff to the last IPv6 extension header.
> 
> I'm quite new to the internal of netfiler, especially about this part.
> I will take a look at the code later.
> 
> >This rises some concerns regarding your ipcomp, I think that if you
> >use this with ah and esp, the ordering of the headers is
> >ah+ipcomp+esp, right?
> 
> This depends on the user land configuration of encapsulation order.
> It can be one of the three types only(ah, esp, ipcomp), the most commonly
> used is ah(outer)+esp(inner).
> 
> I barely see ipcomp used in production, but I remember RFC says ipcomp
> should be done first before esp, because after encryption in esp, the data
> is polluted, i.e., not suitable for compressed anymore(I'm not sure the
> details theory behind this statement.)

In that case we have to use ipv6_find_hdr(..., IPPROTO_IPCOMP, ...),
since par->thoff will point to the last header which is esp. After
this change, the ipcomp ipv6 match will look very similar to what you
have in ah_mt6(...) in net/ipv6/netfilter/ip6t_ah.c. Please, rework
that in your ipcomp match patch and resend. Thanks.

Re: [PATCH net-next 0/2] netfilter: IPv4/v6 IPcomp match support

[Fan Du]

On 2013年12月23日 20:13, Pablo Neira Ayuso wrote:
> Hi,
>
> On Fri, Dec 20, 2013 at 05:21:05PM +0800, Fan Du wrote:
> [...]
>>> AH is not the last header, so we still have to use ipv6_find_hdr() to
>>> find the good header instead of par->thoff. Note that the ip6_tables
>>> sets par->thoff to the last IPv6 extension header.
>>
>> I'm quite new to the internal of netfiler, especially about this part.
>> I will take a look at the code later.
>>
>>> This rises some concerns regarding your ipcomp, I think that if you
>>> use this with ah and esp, the ordering of the headers is
>>> ah+ipcomp+esp, right?
>>
>> This depends on the user land configuration of encapsulation order.
>> It can be one of the three types only(ah, esp, ipcomp), the most commonly
>> used is ah(outer)+esp(inner).
>>
>> I barely see ipcomp used in production, but I remember RFC says ipcomp
>> should be done first before esp, because after encryption in esp, the data
>> is polluted, i.e., not suitable for compressed anymore(I'm not sure the
>> details theory behind this statement.)
>
> In that case we have to use ipv6_find_hdr(..., IPPROTO_IPCOMP, ...),
> since par->thoff will point to the last header which is esp. After
> this change, the ipcomp ipv6 match will look very similar to what you
> have in ah_mt6(...) in net/ipv6/netfilter/ip6t_ah.c. Please, rework
> that in your ipcomp match patch and resend. Thanks.
>

Hi Pablo

I think we don't need to rework this patch set back to v1 by using ipv6_find_hdr
for IPv6 part, because IPcomp shared the same characteristic as esp, that's hiding
upper layer protocol.

For a packet encapsulated in order of ah->esp->ah->original packet, as you said
par->thoff is set at esp, that's why netfilter esp has a unified implementation
in net/netfilter/xt_esp.c, because it's always the last parse header netfilter
can reach.

The same rule apply with IPcomp, for example,
(1) ah->ipcomp->original packet
          ^par->thoff
(2) ipcomp->ah->original packet
       ^par->thoff

Both cases (1) and (2) par->thoff can only point into IPcomp header, so in such
circumstance, a unified implementation for both IPv4/6 is feasible, and I have
tested (2) in such implementation, it works anyway.

IMO, a unified implementation suggested by you previous is ok for this round review.


-- 
浮沉随浪只记今朝笑

--fan
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 0/2] netfilter: IPv4/v6 IPcomp match support

[Pablo Neira Ayuso]

On Tue, Dec 24, 2013 at 02:19:57PM +0800, Fan Du wrote:
> For a packet encapsulated in order of ah->esp->ah->original packet, as you said
> par->thoff is set at esp, that's why netfilter esp has a unified implementation
> in net/netfilter/xt_esp.c, because it's always the last parse header netfilter
> can reach.
> 
> The same rule apply with IPcomp, for example,
> (1) ah->ipcomp->original packet
>          ^par->thoff
> (2) ipcomp->ah->original packet
>       ^par->thoff
> 
> Both cases (1) and (2) par->thoff can only point into IPcomp header, so in such
> circumstance, a unified implementation for both IPv4/6 is feasible, and I have
> tested (2) in such implementation, it works anyway.
> 
> IMO, a unified implementation suggested by you previous is ok for this round review.

Your right, I forgot the fact that ipcomp "hides" what it
encapsulates, so it's basically the last header we can see.