* [nft PATCH] tests/shell: add chain validations tests
@ 2016-03-22 13:06 Arturo Borrero Gonzalez
2016-03-22 19:20 ` Pablo Neira Ayuso
2016-03-23 0:09 ` Mart Frauenlob
0 siblings, 2 replies; 6+ messages in thread
From: Arturo Borrero Gonzalez @ 2016-03-22 13:06 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
Some basic test regarding chains: jumps and validations.
Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
NOTE: the testcases/chains/0009masquerade_jump_1 file fails, seems like a bug
in the kernel validation. Needs more investigation.
tests/shell/testcases/chains/0001jumps_0 | 17 +++++++++++++++
tests/shell/testcases/chains/0002jumps_1 | 22 ++++++++++++++++++++
tests/shell/testcases/chains/0003jump_loop_1 | 21 +++++++++++++++++++
tests/shell/testcases/chains/0004busy_1 | 11 ++++++++++
tests/shell/testcases/chains/0005busy_map_1 | 11 ++++++++++
tests/shell/testcases/chains/0006masquerade_0 | 7 ++++++
tests/shell/testcases/chains/0007masquerade_1 | 9 ++++++++
tests/shell/testcases/chains/0008masquerade_jump_1 | 11 ++++++++++
tests/shell/testcases/chains/0009masquerade_jump_1 | 11 ++++++++++
9 files changed, 120 insertions(+)
create mode 100755 tests/shell/testcases/chains/0001jumps_0
create mode 100755 tests/shell/testcases/chains/0002jumps_1
create mode 100755 tests/shell/testcases/chains/0003jump_loop_1
create mode 100755 tests/shell/testcases/chains/0004busy_1
create mode 100755 tests/shell/testcases/chains/0005busy_map_1
create mode 100755 tests/shell/testcases/chains/0006masquerade_0
create mode 100755 tests/shell/testcases/chains/0007masquerade_1
create mode 100755 tests/shell/testcases/chains/0008masquerade_jump_1
create mode 100755 tests/shell/testcases/chains/0009masquerade_jump_1
diff --git a/tests/shell/testcases/chains/0001jumps_0 b/tests/shell/testcases/chains/0001jumps_0
new file mode 100755
index 0000000..b39df38
--- /dev/null
+++ b/tests/shell/testcases/chains/0001jumps_0
@@ -0,0 +1,17 @@
+#!/bin/bash
+
+set -e
+
+MAX_JUMPS=16
+
+$NFT add table t
+
+for i in $(seq 1 $MAX_JUMPS)
+do
+ $NFT add chain t c${i}
+done
+
+for i in $(seq 1 $((MAX_JUMPS - 1)))
+do
+ $NFT add rule t c${i} jump c$((i + 1))
+done
diff --git a/tests/shell/testcases/chains/0002jumps_1 b/tests/shell/testcases/chains/0002jumps_1
new file mode 100755
index 0000000..0cc8928
--- /dev/null
+++ b/tests/shell/testcases/chains/0002jumps_1
@@ -0,0 +1,22 @@
+#!/bin/bash
+
+set -e
+
+MAX_JUMPS=16
+
+$NFT add table t
+
+for i in $(seq 1 $MAX_JUMPS)
+do
+ $NFT add chain t c${i}
+done
+
+for i in $(seq 1 $((MAX_JUMPS - 1)))
+do
+ $NFT add rule t c${i} jump c$((i + 1))
+done
+
+# this last jump should fail: too many links
+$NFT add chain t c$((MAX_JUMPS + 1))
+$NFT add rule t c${MAX_JUMPS} jump c$((MAX_JUMPS + 1)) 2>/dev/null
+echo "E: max jumps ignored?" >&2
diff --git a/tests/shell/testcases/chains/0003jump_loop_1 b/tests/shell/testcases/chains/0003jump_loop_1
new file mode 100755
index 0000000..f74361f
--- /dev/null
+++ b/tests/shell/testcases/chains/0003jump_loop_1
@@ -0,0 +1,21 @@
+#!/bin/bash
+
+set -e
+
+MAX_JUMPS=16
+
+$NFT add table t
+
+for i in $(seq 1 $MAX_JUMPS)
+do
+ $NFT add chain t c${i}
+done
+
+for i in $(seq 1 $((MAX_JUMPS - 1)))
+do
+ $NFT add rule t c${i} jump c$((i + 1))
+done
+
+# this last jump should fail: loop
+$NFT add rule t c${MAX_JUMPS} jump c1 2>/dev/null
+echo "E: loop of jumps ignored?" >&2
diff --git a/tests/shell/testcases/chains/0004busy_1 b/tests/shell/testcases/chains/0004busy_1
new file mode 100755
index 0000000..cc9a0da
--- /dev/null
+++ b/tests/shell/testcases/chains/0004busy_1
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+$NFT add table t
+$NFT add chain t c1
+$NFT add chain t c2
+$NFT add rule t c1 jump c2
+# kernel should return EBUSY
+$NFT delete chain t c2 2>/dev/null
+echo "E: deleted a busy chain?" >&2
diff --git a/tests/shell/testcases/chains/0005busy_map_1 b/tests/shell/testcases/chains/0005busy_map_1
new file mode 100755
index 0000000..93eca82
--- /dev/null
+++ b/tests/shell/testcases/chains/0005busy_map_1
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+$NFT add table t
+$NFT add chain t c1
+$NFT add chain t c2
+$NFT add rule t c1 tcp dport vmap { 1 : jump c2 }
+# kernel should return EBUSY
+$NFT delete chain t c2 2>/dev/null
+echo "E: deleted a busy chain?" >&2
diff --git a/tests/shell/testcases/chains/0006masquerade_0 b/tests/shell/testcases/chains/0006masquerade_0
new file mode 100755
index 0000000..7934998
--- /dev/null
+++ b/tests/shell/testcases/chains/0006masquerade_0
@@ -0,0 +1,7 @@
+#!/bin/bash
+
+set -e
+
+$NFT add table t
+$NFT add chain t c1 {type nat hook postrouting priority 0 \; }
+$NFT add rule t c1 masquerade
diff --git a/tests/shell/testcases/chains/0007masquerade_1 b/tests/shell/testcases/chains/0007masquerade_1
new file mode 100755
index 0000000..4e98d10
--- /dev/null
+++ b/tests/shell/testcases/chains/0007masquerade_1
@@ -0,0 +1,9 @@
+#!/bin/bash
+
+set -e
+
+$NFT add table t
+$NFT add chain t c1 {type filter hook output priority 0 \; }
+# wrong hook output, only postrouting is valid
+$NFT add rule t c1 masquerade 2>/dev/null
+echo "E: accepted masquerade in output hook" >&2
diff --git a/tests/shell/testcases/chains/0008masquerade_jump_1 b/tests/shell/testcases/chains/0008masquerade_jump_1
new file mode 100755
index 0000000..7754ed0
--- /dev/null
+++ b/tests/shell/testcases/chains/0008masquerade_jump_1
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+$NFT add table t
+$NFT add chain t output {type nat hook output priority 0 \; }
+$NFT add chain t c1
+$NFT add rule t c1 masquerade
+# kernel should return EOPNOTSUPP
+$NFT add rule t output jump c1 2>/dev/null
+echo "E: accepted masquerade in output hook" >&2
diff --git a/tests/shell/testcases/chains/0009masquerade_jump_1 b/tests/shell/testcases/chains/0009masquerade_jump_1
new file mode 100755
index 0000000..684d441
--- /dev/null
+++ b/tests/shell/testcases/chains/0009masquerade_jump_1
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+$NFT add table t
+$NFT add chain t output {type nat hook output priority 0 \; }
+$NFT add chain t c1
+$NFT add rule t c1 masquerade
+# kernel should return EOPNOTSUPP
+$NFT add rule t output tcp dport vmap {1 :jump c1 } 2>/dev/null
+echo "E: accepted masquerade in output hook in a vmap" >&2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [nft PATCH] tests/shell: add chain validations tests
2016-03-22 13:06 [nft PATCH] tests/shell: add chain validations tests Arturo Borrero Gonzalez
@ 2016-03-22 19:20 ` Pablo Neira Ayuso
2016-03-23 7:44 ` Arturo Borrero Gonzalez
2016-03-23 0:09 ` Mart Frauenlob
1 sibling, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2016-03-22 19:20 UTC (permalink / raw)
To: Arturo Borrero Gonzalez; +Cc: netfilter-devel
On Tue, Mar 22, 2016 at 02:06:09PM +0100, Arturo Borrero Gonzalez wrote:
> Some basic test regarding chains: jumps and validations.
>
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
> NOTE: the testcases/chains/0009masquerade_jump_1 file fails, seems like a bug
> in the kernel validation. Needs more investigation.
I can see this there:
> +$NFT add chain t output {type nat hook output priority 0 \; }
We only support masquerade from postrouting.
static struct xt_target masquerade_tg_reg __read_mostly = {
.name = "MASQUERADE",
.family = NFPROTO_IPV4,
.target = masquerade_tg,
.targetsize = sizeof(struct nf_nat_ipv4_multi_range_compat),
.table = "nat",
.hooks = 1 << NF_INET_POST_ROUTING,
BTW, it would be good to add more tests to exercise the chain loop
detection code.
Please, fix and resubmit, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [nft PATCH] tests/shell: add chain validations tests
2016-03-22 19:20 ` Pablo Neira Ayuso
@ 2016-03-23 7:44 ` Arturo Borrero Gonzalez
2016-03-29 11:16 ` Pablo Neira Ayuso
0 siblings, 1 reply; 6+ messages in thread
From: Arturo Borrero Gonzalez @ 2016-03-23 7:44 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Netfilter Development Mailing list
On 22 March 2016 at 20:20, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Tue, Mar 22, 2016 at 02:06:09PM +0100, Arturo Borrero Gonzalez wrote:
>> Some basic test regarding chains: jumps and validations.
>>
>> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
>> ---
>> NOTE: the testcases/chains/0009masquerade_jump_1 file fails, seems like a bug
>> in the kernel validation. Needs more investigation.
>
> I can see this there:
>
>> +$NFT add chain t output {type nat hook output priority 0 \; }
>
> We only support masquerade from postrouting.
>
> static struct xt_target masquerade_tg_reg __read_mostly = {
> .name = "MASQUERADE",
> .family = NFPROTO_IPV4,
> .target = masquerade_tg,
> .targetsize = sizeof(struct nf_nat_ipv4_multi_range_compat),
> .table = "nat",
> .hooks = 1 << NF_INET_POST_ROUTING,
>
> BTW, it would be good to add more tests to exercise the chain loop
> detection code.
>
> Please, fix and resubmit, thanks.
Probably mi description of the problem was poor.
The offending testcase is testing, in fact, that we can add a rule
with a jump to a chain with a masquerade rule, thus connecting
masquerade to a output hook:
$NFT add table t
$NFT add chain t output {type nat hook output priority 0 \; }
$NFT add chain t c1
$NFT add rule t c1 masquerade
$NFT add rule t output tcp dport vmap {1 :jump c1 }
this don't fail, and that's the problem indeed.
--
Arturo Borrero González
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [nft PATCH] tests/shell: add chain validations tests
2016-03-23 7:44 ` Arturo Borrero Gonzalez
@ 2016-03-29 11:16 ` Pablo Neira Ayuso
0 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2016-03-29 11:16 UTC (permalink / raw)
To: Arturo Borrero Gonzalez; +Cc: Netfilter Development Mailing list
On Wed, Mar 23, 2016 at 08:44:31AM +0100, Arturo Borrero Gonzalez wrote:
> On 22 March 2016 at 20:20, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Tue, Mar 22, 2016 at 02:06:09PM +0100, Arturo Borrero Gonzalez wrote:
> >> Some basic test regarding chains: jumps and validations.
> >>
> >> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> >> ---
> >> NOTE: the testcases/chains/0009masquerade_jump_1 file fails, seems like a bug
> >> in the kernel validation. Needs more investigation.
> >
> > I can see this there:
> >
> >> +$NFT add chain t output {type nat hook output priority 0 \; }
> >
> > We only support masquerade from postrouting.
> >
> > static struct xt_target masquerade_tg_reg __read_mostly = {
> > .name = "MASQUERADE",
> > .family = NFPROTO_IPV4,
> > .target = masquerade_tg,
> > .targetsize = sizeof(struct nf_nat_ipv4_multi_range_compat),
> > .table = "nat",
> > .hooks = 1 << NF_INET_POST_ROUTING,
> >
> > BTW, it would be good to add more tests to exercise the chain loop
> > detection code.
> >
> > Please, fix and resubmit, thanks.
>
> Probably mi description of the problem was poor.
>
> The offending testcase is testing, in fact, that we can add a rule
> with a jump to a chain with a masquerade rule, thus connecting
> masquerade to a output hook:
>
> $NFT add table t
> $NFT add chain t output {type nat hook output priority 0 \; }
> $NFT add chain t c1
> $NFT add rule t c1 masquerade
> $NFT add rule t output tcp dport vmap {1 :jump c1 }
>
> this don't fail, and that's the problem indeed.
Right, I'm applying this so we keep this in the radar, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [nft PATCH] tests/shell: add chain validations tests
2016-03-22 13:06 [nft PATCH] tests/shell: add chain validations tests Arturo Borrero Gonzalez
2016-03-22 19:20 ` Pablo Neira Ayuso
@ 2016-03-23 0:09 ` Mart Frauenlob
2016-03-23 8:02 ` Arturo Borrero Gonzalez
1 sibling, 1 reply; 6+ messages in thread
From: Mart Frauenlob @ 2016-03-23 0:09 UTC (permalink / raw)
To: Arturo Borrero Gonzalez; +Cc: netfilter-devel, pablo
Good day,
On 22.03.2016 14:06, Arturo Borrero Gonzalez wrote:
> Some basic test regarding chains: jumps and validations.
>
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
> NOTE: the testcases/chains/0009masquerade_jump_1 file fails, seems like a bug
> in the kernel validation. Needs more investigation.
>
> tests/shell/testcases/chains/0001jumps_0 | 17 +++++++++++++++
> tests/shell/testcases/chains/0002jumps_1 | 22 ++++++++++++++++++++
> tests/shell/testcases/chains/0003jump_loop_1 | 21 +++++++++++++++++++
> tests/shell/testcases/chains/0004busy_1 | 11 ++++++++++
> tests/shell/testcases/chains/0005busy_map_1 | 11 ++++++++++
> tests/shell/testcases/chains/0006masquerade_0 | 7 ++++++
> tests/shell/testcases/chains/0007masquerade_1 | 9 ++++++++
> tests/shell/testcases/chains/0008masquerade_jump_1 | 11 ++++++++++
> tests/shell/testcases/chains/0009masquerade_jump_1 | 11 ++++++++++
> 9 files changed, 120 insertions(+)
> create mode 100755 tests/shell/testcases/chains/0001jumps_0
> create mode 100755 tests/shell/testcases/chains/0002jumps_1
> create mode 100755 tests/shell/testcases/chains/0003jump_loop_1
> create mode 100755 tests/shell/testcases/chains/0004busy_1
> create mode 100755 tests/shell/testcases/chains/0005busy_map_1
> create mode 100755 tests/shell/testcases/chains/0006masquerade_0
> create mode 100755 tests/shell/testcases/chains/0007masquerade_1
> create mode 100755 tests/shell/testcases/chains/0008masquerade_jump_1
> create mode 100755 tests/shell/testcases/chains/0009masquerade_jump_1
>
> diff --git a/tests/shell/testcases/chains/0001jumps_0 b/tests/shell/testcases/chains/0001jumps_0
> new file mode 100755
> index 0000000..b39df38
> --- /dev/null
> +++ b/tests/shell/testcases/chains/0001jumps_0
> @@ -0,0 +1,17 @@
> +#!/bin/bash
I've not looked up the code calling this, but:
First: bash only?
Second: It's not granted to be in /bin.
Third: May not be the wanted version.
So a shebang like:
#!/usr/bin/env bash
or
#!/urs/bin/env sh
should be more compatible and fail proof.
> +
> +set -e
> +
> +MAX_JUMPS=16
> +
> +$NFT add table t
Unquoted variable, may fail if, unlikely but possible, the name contains
i.e. spaces.
> +
> +for i in $(seq 1 $MAX_JUMPS)
> +do
> + $NFT add chain t c${i}
> +done
Requires `seq' binary.
I think for ((i=1; i<=$MAX_JUMPS; i++)) is more portable.
> +
> +for i in $(seq 1 $((MAX_JUMPS - 1)))
> +do
> + $NFT add rule t c${i} jump c$((i + 1))
> +done
Why not add functions? i.e.
runft() {
"$NFT" "$@"
}
nfat() {
runft add table "$@"
}
nfac() {
runft add chain "$@"
}
....
[...]
Best regards,
Mart
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [nft PATCH] tests/shell: add chain validations tests
2016-03-23 0:09 ` Mart Frauenlob
@ 2016-03-23 8:02 ` Arturo Borrero Gonzalez
0 siblings, 0 replies; 6+ messages in thread
From: Arturo Borrero Gonzalez @ 2016-03-23 8:02 UTC (permalink / raw)
To: mart.frauenlob; +Cc: Netfilter Development Mailing list, Pablo Neira Ayuso
On 23 March 2016 at 01:09, Mart Frauenlob <mart.frauenlob@chello.at> wrote:
>
> I've not looked up the code calling this, but:
> First: bash only?
> Second: It's not granted to be in /bin.
> Third: May not be the wanted version.
>
> So a shebang like:
> #!/usr/bin/env bash
> or
> #!/urs/bin/env sh
> should be more compatible and fail proof.
>
>> +
>> +set -e
>> +
>> +MAX_JUMPS=16
>> +
>> +$NFT add table t
>
> Unquoted variable, may fail if, unlikely but possible, the name contains
> i.e. spaces.
>
>> +
>> +for i in $(seq 1 $MAX_JUMPS)
>> +do
>> + $NFT add chain t c${i}
>> +done
>
> Requires `seq' binary.
> I think for ((i=1; i<=$MAX_JUMPS; i++)) is more portable.
>
>> +
>> +for i in $(seq 1 $((MAX_JUMPS - 1)))
>> +do
>> + $NFT add rule t c${i} jump c$((i + 1))
>> +done
>
>
> Why not add functions? i.e.
>
Hi Mart,
yes, bash only :-) I think it's a very good shell. I'm not using any
bash operand which depends on the version... I don't expect anybody to
run this testsuite in bash v1 (20 years old already).
I didn't really care about portability... this is an internal script
to perform internal tests, this is going to be run in very few
systems.
Also, no functions, because I want the nft commands to be really
really clear, so debugging is easier.
Is this testsuite failing for you? Have you run it?
I would really like to review patches to incrementally improve the
testsuite, which has been deliberately simplified to focus on what
matter for us: testing nftables :-)
thanks, best regards.
--
Arturo Borrero González
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-29 11:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-22 13:06 [nft PATCH] tests/shell: add chain validations tests Arturo Borrero Gonzalez
2016-03-22 19:20 ` Pablo Neira Ayuso
2016-03-23 7:44 ` Arturo Borrero Gonzalez
2016-03-29 11:16 ` Pablo Neira Ayuso
2016-03-23 0:09 ` Mart Frauenlob
2016-03-23 8:02 ` Arturo Borrero Gonzalez
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).