| Message ID | 65034694-62fb-3e6a-b15c-1176e65eae65@dtsystems.be |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | extensions : multiple to-dst/to-src arguments for ip6t_DNAT/SNAT not reported | expand |
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 -- 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
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
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! -- 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
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. -- 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
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
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. -- 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
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. -- 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
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
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! -- 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
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;
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(-)