[ovs-dev] [PATCH] conntrack: Correct length check for tcp packet inside ICMP data.

Darrell Ball dlu998 at gmail.com
Mon Aug 26 16:20:28 UTC 2019


On Fri, Aug 23, 2019 at 9:09 AM Darrell Ball <dlu998 at gmail.com> wrote:

> Thanks for the patch
>
> Goes back to release 2.6/day one :-).
>
> I'll provide more feedback after today.
>

I sent an alternative patch here

https://mail.openvswitch.org/pipermail/ovs-dev/2019-August/362013.html

pls take a look.


>
> On Fri, Aug 23, 2019 at 6:20 AM Vishal Deep Ajmera <
> vishal.deep.ajmera at ericsson.com> wrote:
>
>> An ICMP packet with type destination or host not reachable also carries
>> 28 bytes of ICMP data field. This data field contains IP header and TCP
>> header (partial first 8 bytes) of the original packet for which ICMP
>> is being generated.
>>
>> Conntrack module when processing these ICMP packets checks for TCP header
>> length (20 bytes). Since TCP header is partial the length check fails and
>> packet is erroneously dropped.
>>
>> This patch fixes length check for TCP header when processing ICMP data
>> fields.
>>
>> Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera at ericsson.com>
>> ---
>>  lib/conntrack.c | 14 +++++++++++---
>>  lib/packets.h   |  1 +
>>  2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/conntrack.c b/lib/conntrack.c
>> index 5f60fea..0618fdd 100644
>> --- a/lib/conntrack.c
>> +++ b/lib/conntrack.c
>> @@ -1513,10 +1513,18 @@ check_l4_icmp6(const struct conn_key *key, const
>> void *data, size_t size,
>>      return validate_checksum ? checksum_valid(key, data, size, l3) :
>> true;
>>  }
>>
>> +/* If related is NULL, we are parsing nested TCP header  inside ICMP
>> packet.
>> + * Only 8 bytes of TCP header is required by RFC to be present in such
>> case.
>> + */
>>  static inline bool
>> -extract_l4_tcp(struct conn_key *key, const void *data, size_t size)
>> +extract_l4_tcp(struct conn_key *key, const void *data, size_t size,
>> +               bool *related)
>>  {
>> -    if (OVS_UNLIKELY(size < TCP_HEADER_LEN)) {
>> +    if (!related) {
>> +        if (size < ICMP_L4_DATA_LEN) {
>> +            return false;
>> +        }
>> +    } else if (size < TCP_HEADER_LEN) {
>>          return false;
>>      }
>>
>> @@ -1750,7 +1758,7 @@ extract_l4(struct conn_key *key, const void *data,
>> size_t size, bool *related,
>>  {
>>      if (key->nw_proto == IPPROTO_TCP) {
>>          return (!related || check_l4_tcp(key, data, size, l3,
>> -                validate_checksum)) && extract_l4_tcp(key, data, size);
>> +              validate_checksum)) && extract_l4_tcp(key, data, size,
>> related);
>>      } else if (key->nw_proto == IPPROTO_UDP) {
>>          return (!related || check_l4_udp(key, data, size, l3,
>>                  validate_checksum)) && extract_l4_udp(key, data, size);
>> diff --git a/lib/packets.h b/lib/packets.h
>> index a4bee38..2bc65c9 100644
>> --- a/lib/packets.h
>> +++ b/lib/packets.h
>> @@ -886,6 +886,7 @@ struct tcp_header {
>>      ovs_be16 tcp_urg;
>>  };
>>  BUILD_ASSERT_DECL(TCP_HEADER_LEN == sizeof(struct tcp_header));
>> +#define ICMP_L4_DATA_LEN 8
>>
>>  /* Connection states.
>>   *
>> --
>> 1.9.1
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>


More information about the dev mailing list