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 <fw@xxxxxxxxx>
> ---
> 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 <arturo@xxxxxxxxxx>
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
