On Tue, May 09, 2017 at 05:51:21PM +0200, Florian Westphal wrote: > after ip6 nexthdr also switch ip to meta l4proto instead of ip protocol. > > While its needed for ipv6 (due to extension headers) this isn't needed > for ip but it has the advantage that > > tcp dport 22 > > produces same expressions for ip/ip6/inet families.
Oh, this is coming later on... which is what I was asking before, patch order in this series is a bit confusing for review. Another comment below. > Signed-off-by: Florian Westphal <[email protected]> > --- > src/payload.c | 17 +++++++++++------ > src/proto.c | 3 ++- > 2 files changed, 13 insertions(+), 7 deletions(-) > > diff --git a/src/payload.c b/src/payload.c > index 3a3fe8de97cf..5331b5832fd2 100644 > --- a/src/payload.c > +++ b/src/payload.c > @@ -118,17 +118,22 @@ static const struct expr_ops payload_expr_ops = { > }; > > /* > - * ipv6 is special case, we normally use 'meta l4proto' to fetch the last > - * l4 header of the ipv6 extension header chain so we will also match > + * We normally use 'meta l4proto' to fetch the last l4 header of the > + * ipv6 extension header chain so we will also match > * tcp after a fragmentation header, for instance. > + * For consistency we also use meta l4proto for ipv4. > * > - * If user specifically asks for nexthdr x, treat is as a full > - * dependency rather than injecting another (useless) meta l4 one. > + * If user specifically asks for nexthdr x, don't add another (useless) > + * meta dependency. > */ > static bool proto_key_is_protocol(const struct proto_desc *desc, unsigned > int type) > { > - if (type == desc->protocol_key || > - (desc == &proto_ip6 && type == IP6HDR_NEXTHDR)) > + if (type == desc->protocol_key) > + return true; > + > + if (desc == &proto_ip6 && type == IP6HDR_NEXTHDR) > + return true; > + if (desc == &proto_ip && type == IPHDR_PROTOCOL) > return true; > > return false; > diff --git a/src/proto.c b/src/proto.c > index 3b20a5fd35b1..2afedf773045 100644 > --- a/src/proto.c > +++ b/src/proto.c > @@ -587,7 +587,6 @@ const struct proto_desc proto_ip = { > .name = "ip", > .base = PROTO_BASE_NETWORK_HDR, > .checksum_key = IPHDR_CHECKSUM, > - .protocol_key = IPHDR_PROTOCOL, > .protocols = { > PROTO_LINK(IPPROTO_ICMP, &proto_icmp), > PROTO_LINK(IPPROTO_ESP, &proto_esp), > @@ -600,6 +599,7 @@ const struct proto_desc proto_ip = { > PROTO_LINK(IPPROTO_SCTP, &proto_sctp), > }, > .templates = { > + [0] = PROTO_META_TEMPLATE("l4proto", &inet_protocol_type, > NFT_META_L4PROTO, 8), > [IPHDR_VERSION] = HDR_BITFIELD("version", > &integer_type, 0, 4), > [IPHDR_HDRLENGTH] = HDR_BITFIELD("hdrlength", > &integer_type, 4, 4), > [IPHDR_DSCP] = HDR_BITFIELD("dscp", &dscp_type, 8, > 6), > @@ -779,6 +779,7 @@ const struct proto_desc proto_inet_service = { > PROTO_LINK(IPPROTO_TCP, &proto_tcp), > PROTO_LINK(IPPROTO_DCCP, &proto_dccp), > PROTO_LINK(IPPROTO_SCTP, &proto_sctp), > + PROTO_LINK(IPPROTO_ICMP, &proto_icmp), > PROTO_LINK(IPPROTO_ICMPV6, &proto_icmp6), We have avoid adding ICMPv4 and ICMPv6 to proto_inet_service because of combinations like, IPv4 packet with ICMPv6 protocol should not match. Same with IPv6 packet with ICMPv4 protocol. So Arturo came up with the patch to solve this: commit 0011985554e269e1cc8f8e5b41eb9dcd795ebe8c Author: Arturo Borrero Gonzalez <[email protected]> Date: Wed Jan 25 12:51:08 2017 +0100 payload: explicit network ctx assignment for icmp/icmp6 in special families My concern here is that we should not match this invalid combination to allow fishy packets go through. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
