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

Darrell Ball dlu998 at gmail.com
Fri Aug 23 16:09:07 UTC 2019


Thanks for the patch

Goes back to release 2.6/day one :-).

I'll provide more feedback after today.

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