Aaron Conole <acon...@redhat.com> writes:

> Mike Pattrick via dev <ovs-dev@openvswitch.org> writes:
>
>> Currently conntrack will refuse to extract metadata from fragmented
>> IPv4 packets. Usually the fragments would be processed by the ipf
>> module, but this isn't the case for ICMP related packets. The current
>> handling will result in these being incorrectly processed.
>>
>> This patch checks for a frag offset instead of just frag flags, which is
>> similar to how conntrack handles fragments in the kernel.
>>
>> Reported-at: https://issues.redhat.com/browse/FDP-136
>> Reported-by: Ales Musil <amu...@redhat.com>
>> Signed-off-by: Mike Pattrick <m...@redhat.com>

Missed that this should also have:

Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.")

>> ---
>>  lib/conntrack.c       |  2 +-
>>  tests/ofproto-dpif.at | 32 ++++++++++++++++++++++++++++++++
>>  2 files changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/conntrack.c b/lib/conntrack.c
>> index fe1ecc159..b02174ec4 100644
>> --- a/lib/conntrack.c
>> +++ b/lib/conntrack.c
>> @@ -1705,7 +1705,7 @@ extract_l3_ipv4(struct conn_key *key, const void 
>> *data, size_t size,
>>          return false;
>>      }
>>  
>> -    if (IP_IS_FRAGMENT(ip->ip_frag_off)) {
>> +    if (ip->ip_frag_off & htons(IP_FRAG_OFF_MASK)) {
>
> Stylistic, but we may want to have something like the following
> (untested code) in lib/packets.h::
>
> #define IP_IS_FIRST_FRAG(ip_frag_off) \
>    (((ip_frag_off) & htons(IP_MORE_FRAGMENTS | IP_FRAG_OFF_MASK)) == \
>     htons(IP_MORE_FRAGMENTS))
>
> And the update this block to say:
>
>     if (!IP_IS_FIRST_FRAG(ip->ip_frag_off))
>
> Because the first frag should be allowed to be processed to extract
> these details.
>
> It may also be a good idea to update extract_l4() to check that we are
> not the first frag and exit there as well (since the l4 data won't be
> available - it can be considered an early shunt).
>
>>          return false;
>>      }
>>  
>> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
>> index 3d84618b3..7930b3f29 100644
>> --- a/tests/ofproto-dpif.at
>> +++ b/tests/ofproto-dpif.at
>> @@ -11597,6 +11597,38 @@ 
>> udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=4,dport=3),reply=(src=10.1.1.1,dst=10.
>>  OVS_VSWITCHD_STOP
>>  AT_CLEANUP
>>  
>> +AT_SETUP([ofproto-dpif - conntrack - related nat])
>> +OVS_VSWITCHD_START
>> +
>> +add_of_ports --pcap br0 1 2
>> +
>> +AT_DATA([flows.txt], [dnl
>> +table=0,priority=100,arp,action=normal
>> +table=0,priority=10,ip,in_port=1,udp,action=ct(commit,table=1,nat(dst=1.2.3.4:10000-10000))
>> +table=0,priority=10,ip,in_port=2,action=ct(nat,table=1)
>> +table=0,priority=1,action=drop
>> +table=1,priority=10,in_port=1,ct_state=+trk,action=2
>> +table=1,priority=10,in_port=2,ct_state=+trk,action=1
>> +table=1,priority=1,action=drop
>> +])
>> +
>> +AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
>> +
>> +dnl 1. Send and UDP packet to port 5555.
>> +packet1=c6f94ecb72dbe64c473528c9080045000021317040004011b138ac100001ac100002a28e15b3000d20966369616f0a
>> +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=$packet1 
>> actions=resubmit(,0)"])
>> +
>> +dnl 2. Send an fragmented ICMP related reply from 1.2.3.4:10000.
>> +packet2=e64c473528c9c6f94ecb72db080045c000382e87000040019b6701020304ac1000010303ad2d000000004500001c317020004011794aac10000101020304a28e2710000d8623
>> +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=$packet2 
>> actions=resubmit(,0)"])
>> +
>> +dnl 3. Check that the inner packet is translated.
>> +packet3=e64c473528c9c6f94ecb72db080045c000382e8700004001f35aac100002ac1000010303553a000000004500001c317020004011d13dac100001ac100002a28e15b3000def73
>> +OVS_WAIT_UNTIL([ovs-pcap p1-tx.pcap | grep -q "$packet3"])
>> +
>> +OVS_VSWITCHD_STOP
>> +AT_CLEANUP
>> +
>>  AT_SETUP([ofproto-dpif - conntrack - ipv6])
>>  OVS_VSWITCHD_START

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to