All of lore.kernel.org
 help / color / mirror / Atom feed
* [nft PATCH] payload: use explicit network ctx assignation for icmp/icmp6 in inet family
@ 2017-01-20 12:20 Arturo Borrero Gonzalez
  2017-01-24 19:48 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 5+ messages in thread
From: Arturo Borrero Gonzalez @ 2017-01-20 12:20 UTC (permalink / raw)
  To: netfilter-devel

In the inet family, we can add rules like these:

% nft add rule inet t c ip protocol icmp icmp type echo-request
% nft add rule inet t c ip6 nexthdr icmpv6 icmpv6 type echo-request

However, when we print the ruleset:

% nft list ruleset
table inet t {
	chain c {
		icmpv6 type echo-request
		icmp type echo-request
	}
}

These rules we obtain can't be added again:

% nft add rule inet t c icmp type echo-request
<cmdline>:1:19-27: Error: conflicting protocols specified: inet-service vs. icmp
add rule inet t c icmp type echo-request
                  ^^^^^^^^^

% nft add rule inet t c icmpv6 type echo-request
<cmdline>:1:19-29: Error: conflicting protocols specified: inet-service vs. icmpv6
add rule inet t c icmpv6 type echo-request
                  ^^^^^^^^^^^

Since I wouldn't expect an IP packet carrying ICMPv6, or IPv6 packet
carrying ICMP, if the link layer is inet, the network layer protocol context
can be safely update to 'ip' or 'ip6'.

Moreover, nft currently generates a 'meta nfproto ipvX' depedency when
using icmp or icmp6 in the inet family.

Fixes: https://bugzilla.netfilter.org/show_bug.cgi?id=1073
Signed-off-by: Arturo Borrero Gonzalez <arturo@debian.org>
---
 src/payload.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/payload.c b/src/payload.c
index af533b2..9cca838 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -284,7 +284,12 @@ int payload_gen_dependency(struct eval_ctx *ctx, const struct expr *expr,
 				desc = &proto_inet;
 				break;
 			case PROTO_BASE_TRANSPORT_HDR:
-				desc = &proto_inet_service;
+				if (expr->payload.desc == &proto_icmp)
+					desc = &proto_ip;
+				else if (expr->payload.desc == &proto_icmp6)
+					desc = &proto_ip6;
+				else
+					desc = &proto_inet_service;
 				break;
 			default:
 				break;


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [nft PATCH] payload: use explicit network ctx assignation for icmp/icmp6 in inet family
  2017-01-20 12:20 [nft PATCH] payload: use explicit network ctx assignation for icmp/icmp6 in inet family Arturo Borrero Gonzalez
@ 2017-01-24 19:48 ` Pablo Neira Ayuso
  2017-01-24 20:12   ` Pablo Neira Ayuso
  0 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2017-01-24 19:48 UTC (permalink / raw)
  To: Arturo Borrero Gonzalez; +Cc: netfilter-devel

On Fri, Jan 20, 2017 at 01:20:11PM +0100, Arturo Borrero Gonzalez wrote:
> In the inet family, we can add rules like these:
> 
> % nft add rule inet t c ip protocol icmp icmp type echo-request
> % nft add rule inet t c ip6 nexthdr icmpv6 icmpv6 type echo-request
> 
> However, when we print the ruleset:
> 
> % nft list ruleset
> table inet t {
> 	chain c {
> 		icmpv6 type echo-request
> 		icmp type echo-request
> 	}
> }
> 
> These rules we obtain can't be added again:
> 
> % nft add rule inet t c icmp type echo-request
> <cmdline>:1:19-27: Error: conflicting protocols specified: inet-service vs. icmp
> add rule inet t c icmp type echo-request
>                   ^^^^^^^^^
> 
> % nft add rule inet t c icmpv6 type echo-request
> <cmdline>:1:19-29: Error: conflicting protocols specified: inet-service vs. icmpv6
> add rule inet t c icmpv6 type echo-request
>                   ^^^^^^^^^^^
> 
> Since I wouldn't expect an IP packet carrying ICMPv6, or IPv6 packet
> carrying ICMP, if the link layer is inet, the network layer protocol context
> can be safely update to 'ip' or 'ip6'.
> 
> Moreover, nft currently generates a 'meta nfproto ipvX' depedency when
> using icmp or icmp6 in the inet family.

Applied, thanks Arturo.

BTW, it would be great if you can cook a patch with new tests/py lines
covering this case.

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [nft PATCH] payload: use explicit network ctx assignation for icmp/icmp6 in inet family
  2017-01-24 19:48 ` Pablo Neira Ayuso
@ 2017-01-24 20:12   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2017-01-24 20:12 UTC (permalink / raw)
  To: Arturo Borrero Gonzalez; +Cc: netfilter-devel

On Tue, Jan 24, 2017 at 08:48:54PM +0100, Pablo Neira Ayuso wrote:
> On Fri, Jan 20, 2017 at 01:20:11PM +0100, Arturo Borrero Gonzalez wrote:
> > In the inet family, we can add rules like these:
> > 
> > % nft add rule inet t c ip protocol icmp icmp type echo-request
> > % nft add rule inet t c ip6 nexthdr icmpv6 icmpv6 type echo-request
> > 
> > However, when we print the ruleset:
> > 
> > % nft list ruleset
> > table inet t {
> > 	chain c {
> > 		icmpv6 type echo-request
> > 		icmp type echo-request
> > 	}
> > }
> > 
> > These rules we obtain can't be added again:
> > 
> > % nft add rule inet t c icmp type echo-request
> > <cmdline>:1:19-27: Error: conflicting protocols specified: inet-service vs. icmp
> > add rule inet t c icmp type echo-request
> >                   ^^^^^^^^^
> > 
> > % nft add rule inet t c icmpv6 type echo-request
> > <cmdline>:1:19-29: Error: conflicting protocols specified: inet-service vs. icmpv6
> > add rule inet t c icmpv6 type echo-request
> >                   ^^^^^^^^^^^
> > 
> > Since I wouldn't expect an IP packet carrying ICMPv6, or IPv6 packet
> > carrying ICMP, if the link layer is inet, the network layer protocol context
> > can be safely update to 'ip' or 'ip6'.
> > 
> > Moreover, nft currently generates a 'meta nfproto ipvX' depedency when
> > using icmp or icmp6 in the inet family.
> 
> Applied, thanks Arturo.
> 
> BTW, it would be great if you can cook a patch with new tests/py lines
> covering this case.

Wait. This only solves the inet case. Bridge and netdev still remain
broken.

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [nft PATCH] payload: use explicit network ctx assignation for icmp/icmp6 in inet family
  2017-01-20 12:02 Arturo Borrero Gonzalez
@ 2017-01-22 19:59 ` Arturo Borrero Gonzalez
  0 siblings, 0 replies; 5+ messages in thread
From: Arturo Borrero Gonzalez @ 2017-01-22 19:59 UTC (permalink / raw)
  To: Netfilter Development Mailing list

On 20 January 2017 at 13:02, Arturo Borrero Gonzalez
<arturo.borrero.glez@gmail.com> wrote:
> From: Arturo Borrero Gonzalez <arturo@debian.org>
>

duplicated, sorry.

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [nft PATCH] payload: use explicit network ctx assignation for icmp/icmp6 in inet family
@ 2017-01-20 12:02 Arturo Borrero Gonzalez
  2017-01-22 19:59 ` Arturo Borrero Gonzalez
  0 siblings, 1 reply; 5+ messages in thread
From: Arturo Borrero Gonzalez @ 2017-01-20 12:02 UTC (permalink / raw)
  To: netfilter-devel

From: Arturo Borrero Gonzalez <arturo@debian.org>

In the inet family, we can add rules like these:

% nft add rule inet t c ip protocol icmp icmp type echo-request
% nft add rule inet t c ip6 nexthdr icmpv6 icmpv6 type echo-request

However, when we print the ruleset:

% nft list ruleset
table inet t {
	chain c {
		icmpv6 type echo-request
		icmp type echo-request
	}
}

These rules we obtain can't be added again:

% nft add rule inet t c icmp type echo-request
<cmdline>:1:19-27: Error: conflicting protocols specified: inet-service vs. icmp
add rule inet t c icmp type echo-request
                  ^^^^^^^^^

% nft add rule inet t c icmpv6 type echo-request
<cmdline>:1:19-29: Error: conflicting protocols specified: inet-service vs. icmpv6
add rule inet t c icmpv6 type echo-request
                  ^^^^^^^^^^^

Since I wouldn't expect an IP packet carrying ICMPv6, or IPv6 packet
carrying ICMP, if the link layer is inet, the network layer protocol context
can be safely update to 'ip' or 'ip6'.

Moreover, nft currently generates a 'meta nfproto ipvX' depedency when
using icmp or icmp6 in the inet family.

Fixes: https://bugzilla.netfilter.org/show_bug.cgi?id=1073
Signed-off-by: Arturo Borrero Gonzalez <arturo@debian.org>
---
 src/payload.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/payload.c b/src/payload.c
index af533b2..9cca838 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -284,7 +284,12 @@ int payload_gen_dependency(struct eval_ctx *ctx, const struct expr *expr,
 				desc = &proto_inet;
 				break;
 			case PROTO_BASE_TRANSPORT_HDR:
-				desc = &proto_inet_service;
+				if (expr->payload.desc == &proto_icmp)
+					desc = &proto_ip;
+				else if (expr->payload.desc == &proto_icmp6)
+					desc = &proto_ip6;
+				else
+					desc = &proto_inet_service;
 				break;
 			default:
 				break;


^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-01-24 20:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-20 12:20 [nft PATCH] payload: use explicit network ctx assignation for icmp/icmp6 in inet family Arturo Borrero Gonzalez
2017-01-24 19:48 ` Pablo Neira Ayuso
2017-01-24 20:12   ` Pablo Neira Ayuso
  -- strict thread matches above, loose matches on Subject: below --
2017-01-20 12:02 Arturo Borrero Gonzalez
2017-01-22 19:59 ` Arturo Borrero Gonzalez

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.