From: Matt Corallo Date: Sun, 4 Apr 2021 17:14:08 +0000 (-0400) Subject: Correct second-frag L4 matching X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=b2597a12f3a4c500b43b9888b5174385b0227bc3;p=flowspec-xdp Correct second-frag L4 matching --- diff --git a/test.sh b/test.sh index 10ae086..f13d3f9 100755 --- a/test.sh +++ b/test.sh @@ -228,6 +228,17 @@ echo "$TEST_PKT" >> rules.h echo "#define TEST_EXP XDP_PASS" >> rules.h clang -std=c99 -fsanitize=address -pedantic -Wall -Wextra -Wno-pointer-arith -Wno-unused-variable -O0 -g xdp.c -o xdp && ./xdp +echo "flow6 { src 2620:6e:a007:233::1/128; dst 2001:470:0:503::2/128; fragment !is_fragment || first_fragment || !last_fragment; };" | ./genrules.py --ihl=drop-options --8021q=drop-vlan --v6frag=parse-frags +echo "$TEST_PKT" >> rules.h +echo "#define TEST_EXP XDP_PASS" >> rules.h +clang -std=c99 -fsanitize=address -pedantic -Wall -Wextra -Wno-pointer-arith -Wno-unused-variable -O0 -g xdp.c -o xdp && ./xdp + +# Note on the second fragment we don't know the ICMP header (though < 256 is trivially true) +echo "flow6 { icmp type < 256; };" | ./genrules.py --ihl=drop-options --8021q=drop-vlan --v6frag=parse-frags +echo "$TEST_PKT" >> rules.h +echo "#define TEST_EXP XDP_PASS" >> rules.h +clang -std=c99 -fsanitize=address -pedantic -Wall -Wextra -Wno-pointer-arith -Wno-unused-variable -O0 -g xdp.c -o xdp && ./xdp + #TODO Is nextheader frag correct to match on here? Should we support matching on any nexthdr? echo "flow6 { next header 44; };" | ./genrules.py --ihl=accept-options --8021q=accept-vlan --v6frag=parse-frags echo "$TEST_PKT" >> rules.h diff --git a/xdp.c b/xdp.c index 1709e00..74e8e8c 100644 --- a/xdp.c +++ b/xdp.c @@ -226,18 +226,20 @@ int xdp_drop_prog(struct xdp_md *ctx) l4hdr = pktdata + 5*4; #endif - if (ip->protocol == IP_PROTO_TCP) { - if (unlikely(l4hdr + sizeof(struct tcphdr) > data_end)) - DO_RETURN(PKT_LEN_DROP, XDP_DROP); - tcp = (struct tcphdr*) l4hdr; - } else if (ip->protocol == IP_PROTO_UDP) { - if (unlikely(l4hdr + sizeof(struct udphdr) > data_end)) - DO_RETURN(PKT_LEN_DROP, XDP_DROP); - udp = (struct udphdr*) l4hdr; - } else if (ip->protocol == IP_PROTO_ICMP) { - if (unlikely(l4hdr + sizeof(struct icmphdr) > data_end)) - DO_RETURN(PKT_LEN_DROP, XDP_DROP); - icmp = (struct icmphdr*) l4hdr; + if ((ip->frag_off & BE16(IP_OFFSET)) == 0) { + if (ip->protocol == IP_PROTO_TCP) { + if (unlikely(l4hdr + sizeof(struct tcphdr) > data_end)) + DO_RETURN(PKT_LEN_DROP, XDP_DROP); + tcp = (struct tcphdr*) l4hdr; + } else if (ip->protocol == IP_PROTO_UDP) { + if (unlikely(l4hdr + sizeof(struct udphdr) > data_end)) + DO_RETURN(PKT_LEN_DROP, XDP_DROP); + udp = (struct udphdr*) l4hdr; + } else if (ip->protocol == IP_PROTO_ICMP) { + if (unlikely(l4hdr + sizeof(struct icmphdr) > data_end)) + DO_RETURN(PKT_LEN_DROP, XDP_DROP); + icmp = (struct icmphdr*) l4hdr; + } } } #endif @@ -264,21 +266,23 @@ int xdp_drop_prog(struct xdp_md *ctx) #endif } #endif - - if (v6nexthdr == IP_PROTO_TCP) { - if (unlikely(l4hdr + sizeof(struct tcphdr) > data_end)) - DO_RETURN(PKT_LEN_DROP, XDP_DROP); - tcp = (struct tcphdr*) l4hdr; - } else if (v6nexthdr == IP_PROTO_UDP) { - if (unlikely(l4hdr + sizeof(struct udphdr) > data_end)) - DO_RETURN(PKT_LEN_DROP, XDP_DROP); - udp = (struct udphdr*) l4hdr; - } else if (v6nexthdr == IP6_PROTO_ICMPV6) { - if (unlikely(l4hdr + sizeof(struct icmp6hdr) > data_end)) - DO_RETURN(PKT_LEN_DROP, XDP_DROP); - icmpv6 = (struct icmp6hdr*) l4hdr; + // TODO: Handle more options? + + if (frag6 == NULL || (frag6->frag_off & BE16(IP6_FRAGOFF)) == 0) { + if (v6nexthdr == IP_PROTO_TCP) { + if (unlikely(l4hdr + sizeof(struct tcphdr) > data_end)) + DO_RETURN(PKT_LEN_DROP, XDP_DROP); + tcp = (struct tcphdr*) l4hdr; + } else if (v6nexthdr == IP_PROTO_UDP) { + if (unlikely(l4hdr + sizeof(struct udphdr) > data_end)) + DO_RETURN(PKT_LEN_DROP, XDP_DROP); + udp = (struct udphdr*) l4hdr; + } else if (v6nexthdr == IP6_PROTO_ICMPV6) { + if (unlikely(l4hdr + sizeof(struct icmp6hdr) > data_end)) + DO_RETURN(PKT_LEN_DROP, XDP_DROP); + icmpv6 = (struct icmp6hdr*) l4hdr; + } } - // TODO: Handle some options? } #endif