[PATCH] extensions : multiple to-dst/to-src arguments for ip6t_DNAT/SNAT not reported

[PATCH] extensions : multiple to-dst/to-src arguments for ip6t_DNAT/SNAT not reported

[Thierry Du Tre]

This patch is fixing the detection of multiple '--to-destination' in a DNAT rule and '--to-source' in SNAT rule for IPv6.
Currently, when defining multiple values for these, only the last will be used and others ignored silently.

The checks for (cb->xflags & F_X_TO_[DEST/SRC]) always fails because the flags are never set before.
It seems to be a copy-paste artefact since introduction of the IPv6 DNAT/SNAT extensions based on IPv4 code.

I also removed the kernel_version checks because they seem useless. Extensions for IPv6 DNAT/SNAT are using xt_target with revision 1.
That seems only added since kernel version 3.7-rc1 and therefore the check for > v2.6.10 will always return true.
The check is probably also coming from the IPv4 copy-paste.

Signed-off-by: Thierry Du Tre <thierry@dtsystems.be>

---
 extensions/libip6t_DNAT.c | 12 +++++-------
 extensions/libip6t_SNAT.c |  8 +++-----
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/extensions/libip6t_DNAT.c b/extensions/libip6t_DNAT.c
index 08d920d..c3ba621 100644
--- a/extensions/libip6t_DNAT.c
+++ b/extensions/libip6t_DNAT.c
@@ -163,13 +163,11 @@ static void DNAT_parse(struct xt_option_call *cb)
 	switch (cb->entry->id) {
 	case O_TO_DEST:
 		if (cb->xflags & F_X_TO_DEST) {
-			if (!kernel_version)
-				get_kernel_version();
-			if (kernel_version > LINUX_VERSION(2, 6, 10))
-				xtables_error(PARAMETER_PROBLEM,
-					   "DNAT: Multiple --to-destination not supported");
+		  xtables_error(PARAMETER_PROBLEM,
+				   "DNAT: Multiple --to-destination not supported");
 		}
 		parse_to(cb->arg, portok, range);
+		cb->xflags |= F_X_TO_DEST;
 		break;
 	case O_PERSISTENT:
 		range->flags |= NF_NAT_RANGE_PERSISTENT;
@@ -281,7 +279,7 @@ static int DNAT_xlate(struct xt_xlate *xl,
 	return 1;
 }
 
-static struct xtables_target snat_tg_reg = {
+static struct xtables_target dnat_tg_reg = {
 	.name		= "DNAT",
 	.version	= XTABLES_VERSION,
 	.family		= NFPROTO_IPV6,
@@ -299,5 +297,5 @@ static struct xtables_target snat_tg_reg = {
 
 void _init(void)
 {
-	xtables_register_target(&snat_tg_reg);
+	xtables_register_target(&dnat_tg_reg);
 }
diff --git a/extensions/libip6t_SNAT.c b/extensions/libip6t_SNAT.c
index 671ac61..8eeadc1 100644
--- a/extensions/libip6t_SNAT.c
+++ b/extensions/libip6t_SNAT.c
@@ -166,13 +166,11 @@ static void SNAT_parse(struct xt_option_call *cb)
 	switch (cb->entry->id) {
 	case O_TO_SRC:
 		if (cb->xflags & F_X_TO_SRC) {
-			if (!kernel_version)
-				get_kernel_version();
-			if (kernel_version > LINUX_VERSION(2, 6, 10))
-				xtables_error(PARAMETER_PROBLEM,
-					   "SNAT: Multiple --to-source not supported");
+			xtables_error(PARAMETER_PROBLEM,
+				   "SNAT: Multiple --to-source not supported");
 		}
 		parse_to(cb->arg, portok, range);
+		cb->xflags |= F_X_TO_SRC;
 		break;
 	case O_PERSISTENT:
 		range->flags |= NF_NAT_RANGE_PERSISTENT;
-- 
2.7.4

Re: [PATCH] extensions : multiple to-dst/to-src arguments for ip6t_DNAT/SNAT not reported

[Pablo Neira Ayuso]

Hi Thierry,

On Tue, Jan 16, 2018 at 01:44:37PM +0100, Thierry Du Tre wrote:
> This patch is fixing the detection of multiple '--to-destination' in a DNAT rule and '--to-source' in SNAT rule for IPv6.
> Currently, when defining multiple values for these, only the last will be used and others ignored silently.
> 
> The checks for (cb->xflags & F_X_TO_[DEST/SRC]) always fails because the flags are never set before.
> It seems to be a copy-paste artefact since introduction of the IPv6 DNAT/SNAT extensions based on IPv4 code.
> 
> I also removed the kernel_version checks because they seem useless. Extensions for IPv6 DNAT/SNAT are using xt_target with revision 1.
> That seems only added since kernel version 3.7-rc1 and therefore the check for > v2.6.10 will always return true.
> The check is probably also coming from the IPv4 copy-paste.

Thanks for fixing up this.

Would you also send us a patch to add tests:

        extensions/libip6t_DNAT.t

Re: [PATCH] extensions : multiple to-dst/to-src arguments for ip6t_DNAT/SNAT not reported

[Thierry Du Tre]

Op 16/01/2018 om 14:06 schreef Pablo Neira Ayuso:
> Hi Thierry,
> 
> On Tue, Jan 16, 2018 at 01:44:37PM +0100, Thierry Du Tre wrote:
>> This patch is fixing the detection of multiple '--to-destination' in a DNAT rule and '--to-source' in SNAT rule for IPv6.
>> Currently, when defining multiple values for these, only the last will be used and others ignored silently.
>>
>> The checks for (cb->xflags & F_X_TO_[DEST/SRC]) always fails because the flags are never set before.
>> It seems to be a copy-paste artefact since introduction of the IPv6 DNAT/SNAT extensions based on IPv4 code.
>>
>> I also removed the kernel_version checks because they seem useless. Extensions for IPv6 DNAT/SNAT are using xt_target with revision 1.
>> That seems only added since kernel version 3.7-rc1 and therefore the check for > v2.6.10 will always return true.
>> The check is probably also coming from the IPv4 copy-paste.
> 
> Thanks for fixing up this.
> 
> Would you also send us a patch to add tests:
> 
>         extensions/libip6t_DNAT.t
> 

The following should cover this patch.
(without patch, libip6t_SNAT.t and libip6t_DNAT.t will fail)

---
 extensions/libip6t_DNAT.t | 2 ++
 extensions/libip6t_SNAT.t | 2 ++
 extensions/libipt_DNAT.t  | 2 ++
 extensions/libipt_SNAT.t  | 2 ++
 4 files changed, 8 insertions(+)

diff --git a/extensions/libip6t_DNAT.t b/extensions/libip6t_DNAT.t
index 3141c29..4a6d09a 100644
--- a/extensions/libip6t_DNAT.t
+++ b/extensions/libip6t_DNAT.t
@@ -2,7 +2,9 @@
 *nat
 -j DNAT --to-destination dead::beef;=;OK
 -j DNAT --to-destination dead::beef-dead::fee7;=;OK
+-j DNAT --to-destination [dead::beef]:1025-65535;FAIL
 -p tcp -j DNAT --to-destination [dead::beef]:1025-65535;=;OK
 -p tcp -j DNAT --to-destination [dead::beef-dead::fee7]:1025-65535;=;OK
 -p tcp -j DNAT --to-destination [dead::beef-dead::fee7]:1025-65536;;FAIL
+-p tcp -j DNAT --to-destination [dead::beef-dead::fee7]:1025-65535 --to-destination [dead::beef-dead::fee8]:1025-65535;;FAIL
 -j DNAT;;FAIL
diff --git a/extensions/libip6t_SNAT.t b/extensions/libip6t_SNAT.t
index bb08049..0f14894 100644
--- a/extensions/libip6t_SNAT.t
+++ b/extensions/libip6t_SNAT.t
@@ -2,7 +2,9 @@
 *nat
 -j SNAT --to-source dead::beef;=;OK
 -j SNAT --to-source dead::beef-dead::fee7;=;OK
+-j SNAT --to-source [dead::beef]:1025-65535;;FAIL
 -p tcp -j SNAT --to-source [dead::beef]:1025-65535;=;OK
 -p tcp -j SNAT --to-source [dead::beef-dead::fee7]:1025-65535;=;OK
 -p tcp -j SNAT --to-source [dead::beef-dead::fee7]:1025-65536;;FAIL
+-p tcp -j SNAT --to-source [dead::beef-dead::fee7]:1025-65535 --to-source [dead::beef-dead::fee8]:1025-65535;;FAIL
 -j SNAT;;FAIL
diff --git a/extensions/libipt_DNAT.t b/extensions/libipt_DNAT.t
index e3fd563..a7a45e9 100644
--- a/extensions/libipt_DNAT.t
+++ b/extensions/libipt_DNAT.t
@@ -2,7 +2,9 @@
 *nat
 -j DNAT --to-destination 1.1.1.1;=;OK
 -j DNAT --to-destination 1.1.1.1-1.1.1.10;=;OK
+-j DNAT --to-destination 1.1.1.1:1025-65535;;FAIL
 -p tcp -j DNAT --to-destination 1.1.1.1:1025-65535;=;OK
 -p tcp -j DNAT --to-destination 1.1.1.1-1.1.1.10:1025-65535;=;OK
 -p tcp -j DNAT --to-destination 1.1.1.1-1.1.1.10:1025-65536;;FAIL
+-p tcp -j DNAT --to-destination 1.1.1.1-1.1.1.10:1025-65535 --to-destination 2.2.2.2-2.2.2.20:1025-65535;;FAIL
 -j DNAT;;FAIL
diff --git a/extensions/libipt_SNAT.t b/extensions/libipt_SNAT.t
index 73071bb..34c5822 100644
--- a/extensions/libipt_SNAT.t
+++ b/extensions/libipt_SNAT.t
@@ -2,7 +2,9 @@
 *nat
 -j SNAT --to-source 1.1.1.1;=;OK
 -j SNAT --to-source 1.1.1.1-1.1.1.10;=;OK
+-j SNAT --to-source 1.1.1.1:1025-65535;;FAIL
 -p tcp -j SNAT --to-source 1.1.1.1:1025-65535;=;OK
 -p tcp -j SNAT --to-source 1.1.1.1-1.1.1.10:1025-65535;=;OK
 -p tcp -j SNAT --to-source 1.1.1.1-1.1.1.10:1025-65536;;FAIL
+-p tcp -j SNAT --to-source 1.1.1.1-1.1.1.10:1025-65535 --to-source 2.2.2.2-2.2.2.20:1025-65535;;FAIL
 -j SNAT;;FAIL
-- 
2.7.4

Re: [PATCH] extensions : multiple to-dst/to-src arguments for ip6t_DNAT/SNAT not reported

[Pablo Neira Ayuso]

On Tue, Jan 16, 2018 at 04:06:27PM +0100, Thierry Du Tre wrote:
> Op 16/01/2018 om 14:06 schreef Pablo Neira Ayuso:
> > Hi Thierry,
> > 
> > On Tue, Jan 16, 2018 at 01:44:37PM +0100, Thierry Du Tre wrote:
> >> This patch is fixing the detection of multiple '--to-destination' in a DNAT rule and '--to-source' in SNAT rule for IPv6.
> >> Currently, when defining multiple values for these, only the last will be used and others ignored silently.
> >>
> >> The checks for (cb->xflags & F_X_TO_[DEST/SRC]) always fails because the flags are never set before.
> >> It seems to be a copy-paste artefact since introduction of the IPv6 DNAT/SNAT extensions based on IPv4 code.
> >>
> >> I also removed the kernel_version checks because they seem useless. Extensions for IPv6 DNAT/SNAT are using xt_target with revision 1.
> >> That seems only added since kernel version 3.7-rc1 and therefore the check for > v2.6.10 will always return true.
> >> The check is probably also coming from the IPv4 copy-paste.
> > 
> > Thanks for fixing up this.
> > 
> > Would you also send us a patch to add tests:
> > 
> >         extensions/libip6t_DNAT.t
> > 
> 
> The following should cover this patch.
> (without patch, libip6t_SNAT.t and libip6t_DNAT.t will fail)

Folded to your patch to fix this.

> ---
>  extensions/libip6t_DNAT.t | 2 ++
>  extensions/libip6t_SNAT.t | 2 ++
>  extensions/libipt_DNAT.t  | 2 ++
>  extensions/libipt_SNAT.t  | 2 ++
>  4 files changed, 8 insertions(+)
> 
> diff --git a/extensions/libip6t_DNAT.t b/extensions/libip6t_DNAT.t
> index 3141c29..4a6d09a 100644
> --- a/extensions/libip6t_DNAT.t
> +++ b/extensions/libip6t_DNAT.t
> @@ -2,7 +2,9 @@
>  *nat
>  -j DNAT --to-destination dead::beef;=;OK
>  -j DNAT --to-destination dead::beef-dead::fee7;=;OK
> +-j DNAT --to-destination [dead::beef]:1025-65535;FAIL
                                                   ^
No problem, just a missing semicolon here. I have fixed it, please
run:

        python iptables-test.py

next time.

Applied!

Re: [PATCH] extensions : multiple to-dst/to-src arguments for ip6t_DNAT/SNAT not reported

[Pablo Neira Ayuso]

On Tue, Jan 16, 2018 at 01:44:37PM +0100, Thierry Du Tre wrote:
> This patch is fixing the detection of multiple '--to-destination' in a DNAT rule and '--to-source' in SNAT rule for IPv6.
> Currently, when defining multiple values for these, only the last will be used and others ignored silently.
> 
> The checks for (cb->xflags & F_X_TO_[DEST/SRC]) always fails because the flags are never set before.
> It seems to be a copy-paste artefact since introduction of the IPv6 DNAT/SNAT extensions based on IPv4 code.
> 
> I also removed the kernel_version checks because they seem useless. Extensions for IPv6 DNAT/SNAT are using xt_target with revision 1.
> That seems only added since kernel version 3.7-rc1 and therefore the check for > v2.6.10 will always return true.
> The check is probably also coming from the IPv4 copy-paste.

Applied, thanks Thierry.

Re: [PATCH] extensions : multiple to-dst/to-src arguments for ip6t_DNAT/SNAT not reported

[Thierry Du Tre]

Op 16/01/2018 om 16:06 schreef Thierry Du Tre:
> Op 16/01/2018 om 14:06 schreef Pablo Neira Ayuso:
>> Hi Thierry,
>>
>> On Tue, Jan 16, 2018 at 01:44:37PM +0100, Thierry Du Tre wrote:
>>> This patch is fixing the detection of multiple '--to-destination' in a DNAT rule and '--to-source' in SNAT rule for IPv6.
>>> Currently, when defining multiple values for these, only the last will be used and others ignored silently.
>>>
>>> The checks for (cb->xflags & F_X_TO_[DEST/SRC]) always fails because the flags are never set before.
>>> It seems to be a copy-paste artefact since introduction of the IPv6 DNAT/SNAT extensions based on IPv4 code.
>>>
>>> I also removed the kernel_version checks because they seem useless. Extensions for IPv6 DNAT/SNAT are using xt_target with revision 1.
>>> That seems only added since kernel version 3.7-rc1 and therefore the check for > v2.6.10 will always return true.
>>> The check is probably also coming from the IPv4 copy-paste.
>>
>> Thanks for fixing up this.
>>
>> Would you also send us a patch to add tests:
>>
>>         extensions/libip6t_DNAT.t
>>
> 
> The following should cover this patch.
> (without patch, libip6t_SNAT.t and libip6t_DNAT.t will fail)

Added another test + fixed typo (too late)

---
 extensions/libip6t_DNAT.t | 3 +++
 extensions/libip6t_SNAT.t | 3 +++
 extensions/libipt_DNAT.t  | 3 +++
 extensions/libipt_SNAT.t  | 3 +++
 4 files changed, 12 insertions(+)

diff --git a/extensions/libip6t_DNAT.t b/extensions/libip6t_DNAT.t
index 3141c29..6d8f1da 100644
--- a/extensions/libip6t_DNAT.t
+++ b/extensions/libip6t_DNAT.t
@@ -2,7 +2,10 @@
 *nat
 -j DNAT --to-destination dead::beef;=;OK
 -j DNAT --to-destination dead::beef-dead::fee7;=;OK
+-j DNAT --to-destination [dead::beef]:1025-65535;;FAIL
+-j DNAT --to-destination [dead::beef] --to-destination [dead::fee7];;FAIL
 -p tcp -j DNAT --to-destination [dead::beef]:1025-65535;=;OK
 -p tcp -j DNAT --to-destination [dead::beef-dead::fee7]:1025-65535;=;OK
 -p tcp -j DNAT --to-destination [dead::beef-dead::fee7]:1025-65536;;FAIL
+-p tcp -j DNAT --to-destination [dead::beef-dead::fee7]:1025-65535 --to-destination [dead::beef-dead::fee8]:1025-65535;;FAIL
 -j DNAT;;FAIL
diff --git a/extensions/libip6t_SNAT.t b/extensions/libip6t_SNAT.t
index bb08049..d188a6b 100644
--- a/extensions/libip6t_SNAT.t
+++ b/extensions/libip6t_SNAT.t
@@ -2,7 +2,10 @@
 *nat
 -j SNAT --to-source dead::beef;=;OK
 -j SNAT --to-source dead::beef-dead::fee7;=;OK
+-j SNAT --to-source [dead::beef]:1025-65535;;FAIL
+-j SNAT --to-source [dead::beef] --to-source [dead::fee7];;FAIL
 -p tcp -j SNAT --to-source [dead::beef]:1025-65535;=;OK
 -p tcp -j SNAT --to-source [dead::beef-dead::fee7]:1025-65535;=;OK
 -p tcp -j SNAT --to-source [dead::beef-dead::fee7]:1025-65536;;FAIL
+-p tcp -j SNAT --to-source [dead::beef-dead::fee7]:1025-65535 --to-source [dead::beef-dead::fee8]:1025-65535;;FAIL
 -j SNAT;;FAIL
diff --git a/extensions/libipt_DNAT.t b/extensions/libipt_DNAT.t
index e3fd563..1959801 100644
--- a/extensions/libipt_DNAT.t
+++ b/extensions/libipt_DNAT.t
@@ -2,7 +2,10 @@
 *nat
 -j DNAT --to-destination 1.1.1.1;=;OK
 -j DNAT --to-destination 1.1.1.1-1.1.1.10;=;OK
+-j DNAT --to-destination 1.1.1.1:1025-65535;;FAIL
+-j DNAT --to-destination 1.1.1.1 --to-destination 2.2.2.2;;FAIL
 -p tcp -j DNAT --to-destination 1.1.1.1:1025-65535;=;OK
 -p tcp -j DNAT --to-destination 1.1.1.1-1.1.1.10:1025-65535;=;OK
 -p tcp -j DNAT --to-destination 1.1.1.1-1.1.1.10:1025-65536;;FAIL
+-p tcp -j DNAT --to-destination 1.1.1.1-1.1.1.10:1025-65535 --to-destination 2.2.2.2-2.2.2.20:1025-65535;;FAIL
 -j DNAT;;FAIL
diff --git a/extensions/libipt_SNAT.t b/extensions/libipt_SNAT.t
index 73071bb..186e1cb 100644
--- a/extensions/libipt_SNAT.t
+++ b/extensions/libipt_SNAT.t
@@ -2,7 +2,10 @@
 *nat
 -j SNAT --to-source 1.1.1.1;=;OK
 -j SNAT --to-source 1.1.1.1-1.1.1.10;=;OK
+-j SNAT --to-source 1.1.1.1:1025-65535;;FAIL
+-j SNAT --to-source 1.1.1.1 --to-source 2.2.2.2;;FAIL
 -p tcp -j SNAT --to-source 1.1.1.1:1025-65535;=;OK
 -p tcp -j SNAT --to-source 1.1.1.1-1.1.1.10:1025-65535;=;OK
 -p tcp -j SNAT --to-source 1.1.1.1-1.1.1.10:1025-65536;;FAIL
+-p tcp -j SNAT --to-source 1.1.1.1-1.1.1.10:1025-65535 --to-source 2.2.2.2-2.2.2.20:1025-65535;;FAIL
 -j SNAT;;FAIL
-- 
2.7.4

Re: [PATCH] extensions : multiple to-dst/to-src arguments for ip6t_DNAT/SNAT not reported

[Pablo Neira Ayuso]

On Tue, Jan 16, 2018 at 04:20:40PM +0100, Thierry Du Tre wrote:
> Op 16/01/2018 om 16:06 schreef Thierry Du Tre:
> > Op 16/01/2018 om 14:06 schreef Pablo Neira Ayuso:
> >> Hi Thierry,
> >>
> >> On Tue, Jan 16, 2018 at 01:44:37PM +0100, Thierry Du Tre wrote:
> >>> This patch is fixing the detection of multiple '--to-destination' in a DNAT rule and '--to-source' in SNAT rule for IPv6.
> >>> Currently, when defining multiple values for these, only the last will be used and others ignored silently.
> >>>
> >>> The checks for (cb->xflags & F_X_TO_[DEST/SRC]) always fails because the flags are never set before.
> >>> It seems to be a copy-paste artefact since introduction of the IPv6 DNAT/SNAT extensions based on IPv4 code.
> >>>
> >>> I also removed the kernel_version checks because they seem useless. Extensions for IPv6 DNAT/SNAT are using xt_target with revision 1.
> >>> That seems only added since kernel version 3.7-rc1 and therefore the check for > v2.6.10 will always return true.
> >>> The check is probably also coming from the IPv4 copy-paste.
> >>
> >> Thanks for fixing up this.
> >>
> >> Would you also send us a patch to add tests:
> >>
> >>         extensions/libip6t_DNAT.t
> >>
> > 
> > The following should cover this patch.
> > (without patch, libip6t_SNAT.t and libip6t_DNAT.t will fail)
> 
> Added another test + fixed typo (too late)

No problem, send a follow up patch with more tests.

Thanks.

Re: [PATCH] extensions : multiple to-dst/to-src arguments for ip6t_DNAT/SNAT not reported

[Pablo Neira Ayuso]

On Tue, Jan 16, 2018 at 04:23:20PM +0100, Pablo Neira Ayuso wrote:
> On Tue, Jan 16, 2018 at 04:20:40PM +0100, Thierry Du Tre wrote:
> > Op 16/01/2018 om 16:06 schreef Thierry Du Tre:
> > > Op 16/01/2018 om 14:06 schreef Pablo Neira Ayuso:
> > >> Hi Thierry,
> > >>
> > >> On Tue, Jan 16, 2018 at 01:44:37PM +0100, Thierry Du Tre wrote:
> > >>> This patch is fixing the detection of multiple '--to-destination' in a DNAT rule and '--to-source' in SNAT rule for IPv6.
> > >>> Currently, when defining multiple values for these, only the last will be used and others ignored silently.
> > >>>
> > >>> The checks for (cb->xflags & F_X_TO_[DEST/SRC]) always fails because the flags are never set before.
> > >>> It seems to be a copy-paste artefact since introduction of the IPv6 DNAT/SNAT extensions based on IPv4 code.
> > >>>
> > >>> I also removed the kernel_version checks because they seem useless. Extensions for IPv6 DNAT/SNAT are using xt_target with revision 1.
> > >>> That seems only added since kernel version 3.7-rc1 and therefore the check for > v2.6.10 will always return true.
> > >>> The check is probably also coming from the IPv4 copy-paste.
> > >>
> > >> Thanks for fixing up this.
> > >>
> > >> Would you also send us a patch to add tests:
> > >>
> > >>         extensions/libip6t_DNAT.t
> > >>
> > > 
> > > The following should cover this patch.
> > > (without patch, libip6t_SNAT.t and libip6t_DNAT.t will fail)
> > 
> > Added another test + fixed typo (too late)
> 
> No problem, send a follow up patch with more tests.

BTW, the new patch to add more tests needs to go on top of your
previous patch.

Re: [PATCH] extensions : multiple to-dst/to-src arguments for ip6t_DNAT/SNAT not reported

[Thierry Du Tre]

Op 16/01/2018 om 16:24 schreef Pablo Neira Ayuso:
> On Tue, Jan 16, 2018 at 04:23:20PM +0100, Pablo Neira Ayuso wrote:
>> On Tue, Jan 16, 2018 at 04:20:40PM +0100, Thierry Du Tre wrote:
>>> Op 16/01/2018 om 16:06 schreef Thierry Du Tre:
>>>> Op 16/01/2018 om 14:06 schreef Pablo Neira Ayuso:
>>>>> Hi Thierry,
>>>>>
>>>>> On Tue, Jan 16, 2018 at 01:44:37PM +0100, Thierry Du Tre wrote:
>>>>>> This patch is fixing the detection of multiple '--to-destination' in a DNAT rule and '--to-source' in SNAT rule for IPv6.
>>>>>> Currently, when defining multiple values for these, only the last will be used and others ignored silently.
>>>>>>
>>>>>> The checks for (cb->xflags & F_X_TO_[DEST/SRC]) always fails because the flags are never set before.
>>>>>> It seems to be a copy-paste artefact since introduction of the IPv6 DNAT/SNAT extensions based on IPv4 code.
>>>>>>
>>>>>> I also removed the kernel_version checks because they seem useless. Extensions for IPv6 DNAT/SNAT are using xt_target with revision 1.
>>>>>> That seems only added since kernel version 3.7-rc1 and therefore the check for > v2.6.10 will always return true.
>>>>>> The check is probably also coming from the IPv4 copy-paste.
>>>>>
>>>>> Thanks for fixing up this.
>>>>>
>>>>> Would you also send us a patch to add tests:
>>>>>
>>>>>         extensions/libip6t_DNAT.t
>>>>>
>>>>
>>>> The following should cover this patch.
>>>> (without patch, libip6t_SNAT.t and libip6t_DNAT.t will fail)
>>>
>>> Added another test + fixed typo (too late)
>>
>> No problem, send a follow up patch with more tests.
> 
> BTW, the new patch to add more tests needs to go on top of your
> previous patch.

I hope this works :

---
 extensions/libip6t_DNAT.t | 1 +
 extensions/libip6t_SNAT.t | 1 +
 extensions/libipt_DNAT.t  | 1 +
 extensions/libipt_SNAT.t  | 1 +
 4 files changed, 4 insertions(+)

diff --git a/extensions/libip6t_DNAT.t b/extensions/libip6t_DNAT.t
index 0ba96bf..6d8f1da 100644
--- a/extensions/libip6t_DNAT.t
+++ b/extensions/libip6t_DNAT.t
@@ -3,6 +3,7 @@
 -j DNAT --to-destination dead::beef;=;OK
 -j DNAT --to-destination dead::beef-dead::fee7;=;OK
 -j DNAT --to-destination [dead::beef]:1025-65535;;FAIL
+-j DNAT --to-destination [dead::beef] --to-destination [dead::fee7];;FAIL
 -p tcp -j DNAT --to-destination [dead::beef]:1025-65535;=;OK
 -p tcp -j DNAT --to-destination [dead::beef-dead::fee7]:1025-65535;=;OK
 -p tcp -j DNAT --to-destination [dead::beef-dead::fee7]:1025-65536;;FAIL
diff --git a/extensions/libip6t_SNAT.t b/extensions/libip6t_SNAT.t
index 0f14894..d188a6b 100644
--- a/extensions/libip6t_SNAT.t
+++ b/extensions/libip6t_SNAT.t
@@ -3,6 +3,7 @@
 -j SNAT --to-source dead::beef;=;OK
 -j SNAT --to-source dead::beef-dead::fee7;=;OK
 -j SNAT --to-source [dead::beef]:1025-65535;;FAIL
+-j SNAT --to-source [dead::beef] --to-source [dead::fee7];;FAIL
 -p tcp -j SNAT --to-source [dead::beef]:1025-65535;=;OK
 -p tcp -j SNAT --to-source [dead::beef-dead::fee7]:1025-65535;=;OK
 -p tcp -j SNAT --to-source [dead::beef-dead::fee7]:1025-65536;;FAIL
diff --git a/extensions/libipt_DNAT.t b/extensions/libipt_DNAT.t
index a7a45e9..1959801 100644
--- a/extensions/libipt_DNAT.t
+++ b/extensions/libipt_DNAT.t
@@ -3,6 +3,7 @@
 -j DNAT --to-destination 1.1.1.1;=;OK
 -j DNAT --to-destination 1.1.1.1-1.1.1.10;=;OK
 -j DNAT --to-destination 1.1.1.1:1025-65535;;FAIL
+-j DNAT --to-destination 1.1.1.1 --to-destination 2.2.2.2;;FAIL
 -p tcp -j DNAT --to-destination 1.1.1.1:1025-65535;=;OK
 -p tcp -j DNAT --to-destination 1.1.1.1-1.1.1.10:1025-65535;=;OK
 -p tcp -j DNAT --to-destination 1.1.1.1-1.1.1.10:1025-65536;;FAIL
diff --git a/extensions/libipt_SNAT.t b/extensions/libipt_SNAT.t
index 34c5822..186e1cb 100644
--- a/extensions/libipt_SNAT.t
+++ b/extensions/libipt_SNAT.t
@@ -3,6 +3,7 @@
 -j SNAT --to-source 1.1.1.1;=;OK
 -j SNAT --to-source 1.1.1.1-1.1.1.10;=;OK
 -j SNAT --to-source 1.1.1.1:1025-65535;;FAIL
+-j SNAT --to-source 1.1.1.1 --to-source 2.2.2.2;;FAIL
 -p tcp -j SNAT --to-source 1.1.1.1:1025-65535;=;OK
 -p tcp -j SNAT --to-source 1.1.1.1-1.1.1.10:1025-65535;=;OK
 -p tcp -j SNAT --to-source 1.1.1.1-1.1.1.10:1025-65536;;FAIL
-- 
2.7.4

Re: [PATCH] extensions : multiple to-dst/to-src arguments for ip6t_DNAT/SNAT not reported

[Pablo Neira Ayuso]

On Tue, Jan 16, 2018 at 04:31:30PM +0100, Thierry Du Tre wrote:
> Op 16/01/2018 om 16:24 schreef Pablo Neira Ayuso:
> > On Tue, Jan 16, 2018 at 04:23:20PM +0100, Pablo Neira Ayuso wrote:
> >> On Tue, Jan 16, 2018 at 04:20:40PM +0100, Thierry Du Tre wrote:
> >>> Op 16/01/2018 om 16:06 schreef Thierry Du Tre:
> >>>> Op 16/01/2018 om 14:06 schreef Pablo Neira Ayuso:
> >>>>> Hi Thierry,
> >>>>>
> >>>>> On Tue, Jan 16, 2018 at 01:44:37PM +0100, Thierry Du Tre wrote:
> >>>>>> This patch is fixing the detection of multiple '--to-destination' in a DNAT rule and '--to-source' in SNAT rule for IPv6.
> >>>>>> Currently, when defining multiple values for these, only the last will be used and others ignored silently.
> >>>>>>
> >>>>>> The checks for (cb->xflags & F_X_TO_[DEST/SRC]) always fails because the flags are never set before.
> >>>>>> It seems to be a copy-paste artefact since introduction of the IPv6 DNAT/SNAT extensions based on IPv4 code.
> >>>>>>
> >>>>>> I also removed the kernel_version checks because they seem useless. Extensions for IPv6 DNAT/SNAT are using xt_target with revision 1.
> >>>>>> That seems only added since kernel version 3.7-rc1 and therefore the check for > v2.6.10 will always return true.
> >>>>>> The check is probably also coming from the IPv4 copy-paste.
> >>>>>
> >>>>> Thanks for fixing up this.
> >>>>>
> >>>>> Would you also send us a patch to add tests:
> >>>>>
> >>>>>         extensions/libip6t_DNAT.t
> >>>>>
> >>>>
> >>>> The following should cover this patch.
> >>>> (without patch, libip6t_SNAT.t and libip6t_DNAT.t will fail)
> >>>
> >>> Added another test + fixed typo (too late)
> >>
> >> No problem, send a follow up patch with more tests.
> > 
> > BTW, the new patch to add more tests needs to go on top of your
> > previous patch.
> 
> I hope this works :
> 
> ---
>  extensions/libip6t_DNAT.t | 1 +
>  extensions/libip6t_SNAT.t | 1 +
>  extensions/libipt_DNAT.t  | 1 +
>  extensions/libipt_SNAT.t  | 1 +
>  4 files changed, 4 insertions(+)

Applied, thanks.

Please, always send me patches in git-format-patch next time, thanks!