Hi, On Fri, Mar 30, 2018 at 12:17:56PM +0200, Pablo Neira Ayuso wrote: > On Thu, Mar 29, 2018 at 08:39:40AM +0200, Phil Sutter wrote: [...] > > Sounds good! I think the most intuitive behaviour would be: > > > > family | rule | effect > > --------------------------------------------------------------- > > ip | icmp type foo | icmp-in-ipv4 > > | icmpv6 type foo | icmpv6-in-ipv4 > > --------------------------------------------------------------- > > ip6 | icmp type foo | icmp-in-ipv6 > > | icmpv6 type foo | icmpv6-in-ipv6 > > These two above look good to me. > > > --------------------------------------------------------------- > > inet | icmp type foo | icmp-in-ipv4 or icmp-in-ipv6 > > | icmpv6 type foo | icmpv6-in-ipv4 or icmpv6-in-ipv4 > > So you mean, we just look at the transport protocol field? > > That would simplify things since we would not need dependencies at all. > > We discussed that we should actually enforce ipv4 if you ask for 'icmp > type foo' from inet, otherwise we would just let things like a forged > IPv6 header with ICMP go through, which may be a bit counterintuitive > to users? I mean, for a tight ruleset from inet chains, users would > need to do 'meta protocol ip icmp type foo' to make sure forged > packets don't go through. Yes, that approach makes sense in that it avoids pitfalls and simplifies things for users. OTOH it is inconsistent: | add rule inet t c tcp dport 22 accept will allow SSH via IPv4 as well as IPv6, but | add rule inet t c icmp type echo-request accept will allow ICMP pings via IPv4 only. Sure, since ICMP/ICMPv6 are so tightly bound to IPv4/IPv6, I guess it's OK to have a special case for them. > > --------------------------------------------------------------- > > > > I guess this differs from the current state only in the 'or' part of > > inet family, right? Or does nftables reject plain icmp/icmpv6 payload > > matches in inet family if l3proto has not been specified? > > If l3proto is not specified, the idea is to infer it from icmp. > > We should still allow things like: > > meta protocol ip6 icmp type foo which is broken right now, since we > should allow users to make any arbitrary matching, even if it will be > unlikely to see this packet. So if we stick to this path, this needs > to be fixed as it is broken right now. So from my perspective the (sensible) special treatment of icmp/icmpv6 expressions in inet family adds complexity to the code (due to dependency generation/elimination) and is buggy (as you stated). If we drop it, code is simplified and the bugs are "fixed" along the way. To me that's motivation enough to go that path and maybe reconsider it if someone complains. After all, the concept of inet family is new in Linux firewalling, so I'd expect users to give their ruleset an extra thought when migrating to it. Cheers, Phil -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
