[nft PATCH 3/4 v2] delinearize: list the icmpx reason with the string associated

[Alvaro Neira Ayuso]

If you add the rule:
nft add rule inet filter input reject with icmpx type host-unreachable
nft list table inet filter

shows:
table inet filter {
chain input {
reject with icmpx type 2
}
}

We have to attach the icmpx datatype when we list the rules that use it. With
this patch if we list the ruleset, the output is:

table inet filter {
chain input {
reject with icmpx type host-unreachable
}
}

Signed-off-by: Alvaro Neira Ayuso <***@gmail.com>
---
[no changes in v2]

src/netlink_delinearize.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 4bb4697..3e7aed4 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -928,8 +928,10 @@ static void stmt_reject_postprocess(struct rule_pp_ctx rctx, struct stmt *stmt)
stmt->reject.expr->dtype = &icmpv6_code_type;
break;
case NFPROTO_INET:
- if (stmt->reject.type == NFT_REJECT_ICMPX_UNREACH)
+ if (stmt->reject.type == NFT_REJECT_ICMPX_UNREACH) {
+ stmt->reject.expr->dtype = &icmpx_code_type;
break;
+ }
base = rctx.pctx.protocol[PROTO_BASE_LL_HDR].desc;
desc = rctx.pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
protocol = proto_find_num(base, desc);
@@ -944,8 +946,10 @@ static void stmt_reject_postprocess(struct rule_pp_ctx rctx, struct stmt *stmt)
stmt->reject.family = protocol;
break;
case NFPROTO_BRIDGE:
- if (stmt->reject.type == NFT_REJECT_ICMPX_UNREACH)
+ if (stmt->reject.type == NFT_REJECT_ICMPX_UNREACH) {
+ stmt->reject.expr->dtype = &icmpx_code_type;
break;
+ }
base = rctx.pctx.protocol[PROTO_BASE_LL_HDR].desc;
desc = rctx.pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
protocol = proto_find_num(base, desc);

[Alvaro Neira Ayuso]

Example:

nft add rule inet filter input meta l4proto udp reject with tcp reset

When we check if the transport protocol is tcp, we use the network context.
If we don't have this network context, we have a crash.

Signed-off-by: Alvaro Neira Ayuso <***@gmail.com>
---
[no changes in v2]

src/evaluate.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/src/evaluate.c b/src/evaluate.c
index 4b7bda9..2f71e9b 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1339,6 +1339,13 @@ static int stmt_evaluate_reset(struct eval_ctx *ctx, struct stmt *stmt)
if (desc == NULL)
return 0;

+ if (base == NULL) {
+ if (strcmp(desc->name, "tcp") == 0)
+ return 0;
+ else
+ return stmt_error(ctx, stmt,
+ "you cannot use tcp reset with this protocol");
+ }
protonum = proto_find_num(base, desc);
switch (protonum) {
case IPPROTO_TCP:

[Pablo Neira Ayuso]

Post by Alvaro Neira Ayuso
nft add rule inet filter input meta l4proto udp reject with tcp reset
When we check if the transport protocol is tcp, we use the network context.
If we don't have this network context, we have a crash.
---
[no changes in v2]
src/evaluate.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/evaluate.c b/src/evaluate.c
index 4b7bda9..2f71e9b 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1339,6 +1339,13 @@ static int stmt_evaluate_reset(struct eval_ctx *ctx, struct stmt *stmt)
if (desc == NULL)
return 0;
+ if (base == NULL) {
+ if (strcmp(desc->name, "tcp") == 0)
+ return 0;
+ else
+ return stmt_error(ctx, stmt,
+ "you cannot use tcp reset with this protocol");
+ }

Can you give a try to this?

if (base == NULL &&
ctx->table.handle.family == NFPROTO_INET)
base = &proto_inet_service;

Post by Alvaro Neira Ayuso
protonum = proto_find_num(base, desc);
switch (protonum) {
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

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

[Álvaro Neira Ayuso]

Post by Pablo Neira Ayuso

nft add rule inet filter input meta l4proto udp reject with tcp rese=

t

Post by Pablo Neira Ayuso

When we check if the transport protocol is tcp, we use the network c=

ontext.

Post by Pablo Neira Ayuso

If we don't have this network context, we have a crash.
---
[no changes in v2]
src/evaluate.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/evaluate.c b/src/evaluate.c
index 4b7bda9..2f71e9b 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1339,6 +1339,13 @@ static int stmt_evaluate_reset(struct eval_ct=

x *ctx, struct stmt *stmt)

Post by Pablo Neira Ayuso

if (desc =3D=3D NULL)
return 0;
+ if (base =3D=3D NULL) {
+ if (strcmp(desc->name, "tcp") =3D=3D 0)
+ return 0;
+ else
+ return stmt_error(ctx, stmt,
+ "you cannot use tcp reset with this protocol");
+ }

Can you give a try to this?
if (base =3D=3D NULL &&
ctx->table.handle.family =3D=3D NFPROTO_INET)
base =3D &proto_inet_service;

It works. That was another solution that I thought. But we don't need t=
o=20
compare the family because the base can be NULL only with Inet and=20
Bridge tables.

Post by Pablo Neira Ayuso

protonum =3D proto_find_num(base, desc);
switch (protonum) {
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe netfilter-=

devel" in

Post by Pablo Neira Ayuso

More majordomo info at http://vger.kernel.org/majordomo-info.html

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

[Pablo Neira Ayuso]

Post by Álvaro Neira Ayuso

Post by Pablo Neira Ayuso

nft add rule inet filter input meta l4proto udp reject with tcp res=

et

Post by Álvaro Neira Ayuso

Post by Pablo Neira Ayuso

When we check if the transport protocol is tcp, we use the network =

context.

Post by Álvaro Neira Ayuso

Post by Pablo Neira Ayuso

If we don't have this network context, we have a crash.
---
[no changes in v2]
src/evaluate.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/evaluate.c b/src/evaluate.c
index 4b7bda9..2f71e9b 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1339,6 +1339,13 @@ static int stmt_evaluate_reset(struct eval_c=

tx *ctx, struct stmt *stmt)

Post by Álvaro Neira Ayuso

Post by Pablo Neira Ayuso

if (desc =3D=3D NULL)
return 0;
+ if (base =3D=3D NULL) {
+ if (strcmp(desc->name, "tcp") =3D=3D 0)
+ return 0;
+ else
+ return stmt_error(ctx, stmt,
+ "you cannot use tcp reset with this protocol");
+ }

Can you give a try to this?
if (base =3D=3D NULL &&
ctx->table.handle.family =3D=3D NFPROTO_INET)
base =3D &proto_inet_service;

=20
It works. That was another solution that I thought. But we don't
need to compare the family because the base can be NULL only with
Inet and Bridge tables.

OK, but better you still check for bridge and inet there. We may
introduce changes later on that may easily break this code because of
this assumption.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-dev=
el" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Álvaro Neira Ayuso]

Post by Pablo Neira Ayuso

Post by Álvaro Neira Ayuso

Post by Pablo Neira Ayuso

nft add rule inet filter input meta l4proto udp reject with tcp re=

set

Post by Pablo Neira Ayuso

Post by Álvaro Neira Ayuso

Post by Pablo Neira Ayuso

When we check if the transport protocol is tcp, we use the network=

context.

Post by Pablo Neira Ayuso

Post by Álvaro Neira Ayuso

Post by Pablo Neira Ayuso

If we don't have this network context, we have a crash.
---
[no changes in v2]
src/evaluate.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/evaluate.c b/src/evaluate.c
index 4b7bda9..2f71e9b 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1339,6 +1339,13 @@ static int stmt_evaluate_reset(struct eval_=

ctx *ctx, struct stmt *stmt)

Post by Pablo Neira Ayuso

Post by Álvaro Neira Ayuso

Post by Pablo Neira Ayuso

if (desc =3D=3D NULL)
return 0;
+ if (base =3D=3D NULL) {
+ if (strcmp(desc->name, "tcp") =3D=3D 0)
+ return 0;
+ else
+ return stmt_error(ctx, stmt,
+ "you cannot use tcp reset with this protocol");
+ }

Can you give a try to this?
if (base =3D=3D NULL &&
ctx->table.handle.family =3D=3D NFPROTO_INET)
base =3D &proto_inet_service;

It works. That was another solution that I thought. But we don't
need to compare the family because the base can be NULL only with
Inet and Bridge tables.

OK, but better you still check for bridge and inet there. We may
introduce changes later on that may easily break this code because of
this assumption.

Perfect. That's true. Thanks Pablo.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-dev=
el" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Alvaro Neira Ayuso]

Signed-off-by: Alvaro Neira Ayuso <***@gmail.com>
---
[changes in v2]
* Changed the format and added the rules with all the posible reasons

tests/regression/bridge/reject.t | 30 ++++++++++++++++++++++++++++++
tests/regression/inet/reject.t | 28 ++++++++++++++++++++++++++++
tests/regression/ip/reject.t | 11 ++++++++++-
tests/regression/ip6/reject.t | 9 ++++++++-
4 files changed, 76 insertions(+), 2 deletions(-)
create mode 100644 tests/regression/bridge/reject.t
create mode 100644 tests/regression/inet/reject.t

diff --git a/tests/regression/bridge/reject.t b/tests/regression/bridge/reject.t
new file mode 100644
index 0000000..68e6051
--- /dev/null
+++ b/tests/regression/bridge/reject.t
@@ -0,0 +1,30 @@
+*bridge;test-bridge
+:input;type filter hook input priority 0
+
+reject with icmp type host-unreachable;ok;ether type ip reject with icmp type host-unreachable
+reject with icmp type net-unreachable;ok;ether type ip reject with icmp type net-unreachable
+reject with icmp type prot-unreachable;ok;ether type ip reject with icmp type prot-unreachable
+reject with icmp type port-unreachable;ok;ether type ip reject
+reject with icmp type net-prohibited;ok;ether type ip reject with icmp type net-prohibited
+reject with icmp type host-prohibited;ok;ether type ip reject with icmp type host-prohibited
+reject with icmp type admin-prohibited;ok;ether type ip reject with icmp type admin-prohibited
+
+reject with icmpv6 type no-route;ok;ether type ip6 reject with icmpv6 type no-route
+reject with icmpv6 type admin-prohibited;ok;ether type ip6 reject with icmpv6 type admin-prohibited
+reject with icmpv6 type addr-unreachable;ok;ether type ip6 reject with icmpv6 type addr-unreachable
+reject with icmpv6 type port-unreachable;ok;ether type ip6 reject
+
+ip protocol tcp reject with tcp reset;ok;ip protocol 6 reject with tcp reset
+
+reject;ok
+reject with icmpx type host-unreachable;ok
+reject with icmpx type no-route;ok
+reject with icmpx type admin-prohibited;ok
+reject with icmpx type port-unreachable;ok;reject
+
+ether type ipv6 reject with icmp type host-unreachable;fail
+ether type ip6 reject with icmp type host-unreachable;fail
+ether type ip reject with icmpv6 type no-route;fail
+ether type vlan reject;fail
+ether type arp reject;fail
+ip protocol udp reject with tcp reset;fail
diff --git a/tests/regression/inet/reject.t b/tests/regression/inet/reject.t
new file mode 100644
index 0000000..7dd4598
--- /dev/null
+++ b/tests/regression/inet/reject.t
@@ -0,0 +1,28 @@
+*inet;test-inet
+:input;type filter hook input priority 0
+
+reject with icmp type host-unreachable;ok;meta nfproto ipv4 reject with icmp type host-unreachable
+reject with icmp type net-unreachable;ok;meta nfproto ipv4 reject with icmp type net-unreachable
+reject with icmp type prot-unreachable;ok;meta nfproto ipv4 reject with icmp type prot-unreachable
+reject with icmp type port-unreachable;ok;meta nfproto ipv4 reject
+reject with icmp type net-prohibited;ok;meta nfproto ipv4 reject with icmp type net-prohibited
+reject with icmp type host-prohibited;ok;meta nfproto ipv4 reject with icmp type host-prohibited
+reject with icmp type admin-prohibited;ok;meta nfproto ipv4 reject with icmp type admin-prohibited
+
+reject with icmpv6 type no-route;ok;meta nfproto ipv6 reject with icmpv6 type no-route
+reject with icmpv6 type admin-prohibited;ok;meta nfproto ipv6 reject with icmpv6 type admin-prohibited
+reject with icmpv6 type addr-unreachable;ok;meta nfproto ipv6 reject with icmpv6 type addr-unreachable
+reject with icmpv6 type port-unreachable;ok;meta nfproto ipv6 reject
+
+reject with tcp reset;ok;meta l4proto 6 reject with tcp reset
+
+reject;ok
+reject with icmpx type host-unreachable;ok
+reject with icmpx type no-route;ok
+reject with icmpx type admin-prohibited;ok
+reject with icmpx type port-unreachable;ok;reject
+
+meta nfproto ipv6 reject with icmp type host-unreachable;fail
+meta nfproto ipv4 ip protocol icmp reject with icmpv6 type no-route;fail
+meta nfproto ipv6 ip protocol icmp reject with icmp type host-unreachable;fail
+ip protocol udp reject with tcp reset;fail
diff --git a/tests/regression/ip/reject.t b/tests/regression/ip/reject.t
index e7fb15b..70a63a0 100644
--- a/tests/regression/ip/reject.t
+++ b/tests/regression/ip/reject.t
@@ -1,5 +1,14 @@
*ip;test-ip4
-*ip;test-inet
:output;type filter hook output priority 0

reject;ok
+reject with icmp type host-unreachable;ok
+reject with icmp type net-unreachable;ok
+reject with icmp type prot-unreachable;ok
+reject with icmp type port-unreachable;ok;reject
+reject with icmp type net-prohibited;ok
+reject with icmp type host-prohibited;ok
+reject with icmp type admin-prohibited;ok
+
+reject with icmp type no-route;fail
+reject with icmpv6 type no-route;fail
diff --git a/tests/regression/ip6/reject.t b/tests/regression/ip6/reject.t
index b49c50b..60dec90 100644
--- a/tests/regression/ip6/reject.t
+++ b/tests/regression/ip6/reject.t
@@ -1,5 +1,12 @@
*ip6;test-ip6
-*inet;test-inet
:output;type filter hook output priority 0

reject;ok
+reject with icmpv6 type no-route;ok
+reject with icmpv6 type admin-prohibited;ok
+reject with icmpv6 type addr-unreachable;ok
+reject with icmpv6 type port-unreachable;ok;reject
+reject with tcp reset;ok;ip6 nexthdr 6 reject with tcp reset
+
+reject with icmpv6 type host-unreachable;fail
+reject with icmp type host-unreachable;fail

[Pablo Neira Ayuso]

nft add rule bridge filter input \
ether type ip reject with icmp type host-unreachable
nft add rule inet filter input \
meta nfproto ipv4 reject with icmp type host-unreachable
we have a segfault because we add a network dependency when we already have
network context.
---
[changes in v2]
* Fixed a incorrect refactor when we check the family in bridge
src/evaluate.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 56 insertions(+), 1 deletion(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index 83ef749..4b7bda9 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -19,6 +19,7 @@
#include <linux/netfilter/nf_tables.h>
#include <netinet/ip_icmp.h>
#include <netinet/icmp6.h>
+#include <net/ethernet.h>
#include <expression.h>
#include <statement.h>
@@ -1193,6 +1194,8 @@ static int stmt_reject_gen_dependency(struct eval_ctx *ctx, struct stmt *stmt,
BUG("cannot generate reject dependency for type %d",
stmt->reject.type);
}
+ if (payload == NULL)
+ return 0;

Why this check?

if (payload_gen_dependency(ctx, payload, &nstmt) < 0)
return -1;

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

[Álvaro Neira Ayuso]

Post by Pablo Neira Ayuso

nft add rule bridge filter input \
ether type ip reject with icmp type host-unreachable
nft add rule inet filter input \
meta nfproto ipv4 reject with icmp type host-unreachable
we have a segfault because we add a network dependency when we alrea=

dy have

Post by Pablo Neira Ayuso

network context.
---
[changes in v2]
* Fixed a incorrect refactor when we check the family in bridge
src/evaluate.c | 57 ++++++++++++++++++++++++++++++++++++++++++++=

+++++++++++-

Post by Pablo Neira Ayuso

1 file changed, 56 insertions(+), 1 deletion(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index 83ef749..4b7bda9 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -19,6 +19,7 @@
#include <linux/netfilter/nf_tables.h>
#include <netinet/ip_icmp.h>
#include <netinet/icmp6.h>
+#include <net/ethernet.h>
#include <expression.h>
#include <statement.h>
@@ -1193,6 +1194,8 @@ static int stmt_reject_gen_dependency(struct e=

val_ctx *ctx, struct stmt *stmt,

Post by Pablo Neira Ayuso

BUG("cannot generate reject dependency for type %d",
stmt->reject.type);
}
+ if (payload =3D=3D NULL)
+ return 0;

Why this check?

If we already have context, the previously functions return a NULL=20
payload. Therefore, if we try to create a dependency with this NULL=20
payload, we have a crash.

Post by Pablo Neira Ayuso

if (payload_gen_dependency(ctx, payload, &nstmt) < 0)
return -1;

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

[Pablo Neira Ayuso]

Post by Álvaro Neira Ayuso

Post by Pablo Neira Ayuso

nft add rule bridge filter input \
ether type ip reject with icmp type host-unreachable
nft add rule inet filter input \
meta nfproto ipv4 reject with icmp type host-unreachable
we have a segfault because we add a network dependency when we already have
network context.
---
[changes in v2]
* Fixed a incorrect refactor when we check the family in bridge
src/evaluate.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 56 insertions(+), 1 deletion(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index 83ef749..4b7bda9 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -19,6 +19,7 @@
#include <linux/netfilter/nf_tables.h>
#include <netinet/ip_icmp.h>
#include <netinet/icmp6.h>
+#include <net/ethernet.h>
#include <expression.h>
#include <statement.h>
@@ -1193,6 +1194,8 @@ static int stmt_reject_gen_dependency(struct eval_ctx *ctx, struct stmt *stmt,
BUG("cannot generate reject dependency for type %d",
stmt->reject.type);
}
+ if (payload == NULL)
+ return 0;

Why this check?

If we already have context, the previously functions return a NULL
payload. Therefore, if we try to create a dependency with this NULL
payload, we have a crash.

I prefer if you can change the return value logic in
reject_payload_gen_dependency*() to:

1: payload dependency was created
0: no payload dependency needed
-1: error

See patch attached.

[Álvaro Neira Ayuso]

Post by Pablo Neira Ayuso

Post by Álvaro Neira Ayuso

Post by Pablo Neira Ayuso

nft add rule bridge filter input \
ether type ip reject with icmp type host-unreachable
nft add rule inet filter input \
meta nfproto ipv4 reject with icmp type host-unreachable
we have a segfault because we add a network dependency when we alr=

eady have

Post by Pablo Neira Ayuso

Post by Álvaro Neira Ayuso

Post by Pablo Neira Ayuso

network context.
---
[changes in v2]
* Fixed a incorrect refactor when we check the family in bridge
src/evaluate.c | 57 ++++++++++++++++++++++++++++++++++++++++++=

+++++++++++++-

Post by Pablo Neira Ayuso

Post by Álvaro Neira Ayuso

Post by Pablo Neira Ayuso

1 file changed, 56 insertions(+), 1 deletion(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index 83ef749..4b7bda9 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -19,6 +19,7 @@
#include <linux/netfilter/nf_tables.h>
#include <netinet/ip_icmp.h>
#include <netinet/icmp6.h>
+#include <net/ethernet.h>
#include <expression.h>
#include <statement.h>
@@ -1193,6 +1194,8 @@ static int stmt_reject_gen_dependency(struct=

eval_ctx *ctx, struct stmt *stmt,

Post by Pablo Neira Ayuso

Post by Álvaro Neira Ayuso

Post by Pablo Neira Ayuso

BUG("cannot generate reject dependency for type %d",
stmt->reject.type);
}
+ if (payload =3D=3D NULL)
+ return 0;

Why this check?

If we already have context, the previously functions return a NULL
payload. Therefore, if we try to create a dependency with this NULL
payload, we have a crash.

I prefer if you can change the return value logic in
1: payload dependency was created
0: no payload dependency needed
-1: error
See patch attached.

Nice idea. Looks good to me.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-dev=
el" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Pablo Neira Ayuso]

Post by Alvaro Neira Ayuso
nft add rule inet filter input reject with icmpx type host-unreachable
nft list table inet filter
table inet filter {
chain input {
reject with icmpx type 2
}
}

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