Re: Extensions for ICMP[6] with sport, dport

[Duncan Roe <duncan_roe@optusnet.com.au>]

On Mon, Jun 08, 2020 at 07:31:01PM +0200, Rick van Rein wrote:
> Hello Patrick McHardy / NFT,
>
> I'm using NetFilter for static firewalling.  Ideally with ICMP, for
> which I found that a minor extension might be helpful, adding selectors
> for icmp|icmp6|l4proto sport|dport.  This avoids painstaking detail to
> carry ICMP, and may be helpful to have mature firewalls more easily.
> Would you agree that this is a useful extension?
>
> Interpretation of IP content is valid for error types; for ICMP, those
> are 3,11,12,31, for ICMP6, those are 1,2,3,4; this should be checked
> elsewhere in the ruleset.  The code supports "l4proto" selection of ICMP
> with the same rules as TCP et al.  (But a better implementation of
> "l4proto" in meta.c would skip IP option headers and ICMP headers with
> error types to actually arrive at layer 4, IMHO).
>
> A sketch of code is below; I am unsure about the [THDR_?PORT] but I
> think the "sport" and "dport" should be interpreted in reverse for ICMP,
> as it travels upstream.  That would match "l4proto sport" match ICMP
> along with the TCP, UDP, SCTP and DCCP to which it relates.  It also
> seems fair that ICMP with a "dport" targets the port at the ICMP target,
> so the originator of the initial message.
>
>
> If you want me to continue on this, I need to find a way into
> git.kernel.org and how to offer code.  Just point me to howto's.  I also
> could write a Wiki about Stateful Filter WHENTO-and-HOWTO.
>
>
> Cheers,
>  -Rick
>
>
> struct icmphdr_udphdr {
> 	struct icmphdr ih;
> 	struct udphdr uh;
> };
>
> const struct proto_desc proto_icmp = {
> 	???
>         .templates      = {
> 		???
> 		/* ICMP travels upstream; we reverse sport/dport for icmp/l4proto */
>                 [THDR_SPORT]            = INET_SERVICE(???sport", struct
> icmphdr_udphdr, uh.dest  ),
>                 [THDR_DPORT]            = INET_SERVICE(???dport", struct
> icmphdr_udphdr, uh.source),
> 		// Unsure about these indexes???
>         },
> 	???
> };
>
> struct icmp6hdr_udphdr {
> 	struct icmp6hdr ih;
> 	struct udphdr uh;
> };
>
>
> const struct proto_desc proto_icmp6 = {
> 	???
>         .templates      = {
> 		???
> 		/* ICMP travels upstream; we reverse sport/dport for icmp6/l4proto */
>                 [THDR_SPORT]            = INET_SERVICE(???sport", struct
> icmphdr_udphdr, uh.dest),
>                 [THDR_DPORT]            = INET_SERVICE(???dport", struct
> icmphdr_udphdr, uh.source),
> 		// Unsure about these indexes???
>         },
> 	???
> };
Hi Rick,

Usually people submit patches to netfilter-devel using git format-patch and
git send-email.

You should submit patches against the nf-next tree, which you can clone from
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git

Cheers ... Duncan.