Re: [iptables PATCH 1/3] extensions: libipt_icmp: Fix confusion between 255/255 and any

[Phil Sutter <phil@nwl.cc>]

On Wed, Aug 02, 2023 at 10:31:16AM +0200, Jan Engelhardt wrote:
> On Wednesday 2023-08-02 04:05, Phil Sutter wrote:
> >
> >It is not entirely clear what the fixed commit was trying to establish,
> >but the save output is certainly not correct (especially since print
> >callback gets things right).
> >
> >Fixes: fc9237da4e845 ("Fix '-p icmp -m icmp' issue (Closes: #37)")
> 
> """(libipt_icmp.c):
>  * Up to kernel <=2.4.20 the problem was:
>  * '-p icmp ' matches all icmp packets
>  * '-p icmp -m icmp' matches _only_ ICMP type 0 :(
>  * This is now fixed by initializing the field * to icmp type 0xFF
> """
> 
> But also:
> 
> v1.2.7a-35-gfc9237da missed touching *libip6t_icmp6.c*, so
> it was never updated with the same "bug".
> 
> In icmp6, --icmpv6-type was (and still is to this date) mandatory, which means
> `-p icmp6 -m icmp6` would *not* implicitly go match ICMP(v6) type 0 like its
> IPv4 counterpart.
> 
> Then, in v1.4.10-115-g1b8db4f4, libipt_icmp.c more or less accidentally gained
> XTOPT_MAND (possible spill from IPv6 code), therefore `-p icmp -m icmp` would
> also stop implicitly doing ICMP type "any".

Thanks for the forensics.

One could extend icmp6 match (in kernel and user space) to support this
"any" type, though it seems not guaranteed the value 255 won't be used
for a real message at some point. So a proper solution was to support type
ranges like ebtables does. Then "any" type is 0:255/0:255.

Apart from the above, the three patches of this series should be fine,
right?

Thanks, Phil