From patchwork Mon Aug 26 16:06:35 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Darrell Ball X-Patchwork-Id: 1153285 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="s0j78ArP"; dkim-atps=neutral Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 46HH4t75Kgz9sDB for ; Tue, 27 Aug 2019 02:09:18 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id EEB6719B2; Mon, 26 Aug 2019 16:09:09 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 32DF018A3 for ; Mon, 26 Aug 2019 16:06:45 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pf1-f193.google.com (mail-pf1-f193.google.com [209.85.210.193]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 9E176710 for ; Mon, 26 Aug 2019 16:06:44 +0000 (UTC) Received: by mail-pf1-f193.google.com with SMTP id y9so11712615pfl.4 for ; Mon, 26 Aug 2019 09:06:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=w0ROd1zIYcJK49tWPe3onY7tz9xad64Akb24Z1ceu+4=; b=s0j78ArPYg8xzxUu4Omp2Cofc3fMw0AoFxsV/NxnteON7JF/P416PfP1IjJYZayLjd OclYiN8UbrboBi1BUkhygxSBop7LrIAL+gqJDcd4jCVn4hSayH6vIE6PZORIC58FfmsT oKqtmp2+KmvuBBj2ZkFQVQyyeRWX726HgZdfF1HmGZGt8moTTSVyE00chzQa10t7Zabp aPTN9V90EU5R76bbrKufjj8xvhkXM+jfa6k1+vuHQXvrx3tTDssRgXjpMIGcEG3IZzkP TT2CGTqM7EI9hdAZm9106tC2kPKtqsNJcWP6Wkl+rrZRk1DrupW94driyw2BdaGHmxg3 YjAQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=w0ROd1zIYcJK49tWPe3onY7tz9xad64Akb24Z1ceu+4=; b=UijZDQW2FIqLbCCBrBjyFnQvqYQsZxD9eScrDcYrdRX+PCDnfojtlkaXIxDoS/4P6H 1nq/WbKvl9SOxg6AcVqCBxS+aszNwOym5dnDNtD8a81f0xGc0u/RZq2yd1w5cFBHiF6B 4XTvpOeQZyj+p1PqanlYHdwNWBZi5fherJgeOXi5h+B+GTMquZo53uQvOXJoBUucCB5t 2SLKAv5ViDyr0TLdgnitM8yObxfxzBrj32Y4LsKD68g1ZYIz2Su9o/AiyHoISv/2PLAT zvrFkx+/4V+Ihm5TkzlUGsWPIrB8kn6JNQJOBtl8vGFD9sNk/cwlNaoOphxkaghvTS2O 3guA== X-Gm-Message-State: APjAAAU/K989+OmPPoeJ+h1a1Th9r15G1rmSVco3m9lz7IMdlXmycqXf aT3htZlECIAYyDH0DIOXE0U= X-Google-Smtp-Source: APXvYqxZdq2jD/QS8IMqm4/J+nu2NB0wu5r/ntMhdS4yUVTur3Dds/i2xr90/YBwMELO4Ov/+asy+Q== X-Received: by 2002:a17:90a:cb88:: with SMTP id a8mr7798786pju.111.1566835604281; Mon, 26 Aug 2019 09:06:44 -0700 (PDT) Received: from ubuntu.localdomain (c-76-102-76-212.hsd1.ca.comcast.net. [76.102.76.212]) by smtp.gmail.com with ESMTPSA id t4sm13489421pfd.109.2019.08.26.09.06.43 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 26 Aug 2019 09:06:43 -0700 (PDT) From: Darrell Ball To: dlu998@gmail.com, dev@openvswitch.org Date: Mon, 26 Aug 2019 09:06:35 -0700 Message-Id: <1566835595-72476-1-git-send-email-dlu998@gmail.com> X-Mailer: git-send-email 1.9.1 X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Daniele Di Proietto Subject: [ovs-dev] [patch v1] conntrack: Fix ICMPV4 error data L4 length check. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org The ICMP error data L4 length check was found to be too strict for TCP, expecting a minimum of 20 rather than 8 bytes. This worked by hapenstance for other inner protocols. The approach is to explicitly handle the ICMPV4 error data L4 length check and to do this for all supported inner protocols in the same way. Making the code common between protocols also allows the existing ICMP related UDP tests to cover TCP and ICMP cases. Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.") CC: Daniele Di Proietto Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2019-August/361949.html Reported-by: Vishal Deep Ajmera Signed-off-by: Vishal Deep Ajmera Co-authored-by: Vishal Deep Ajmera Signed-off-by: Darrell Ball --- lib/conntrack.c | 38 ++++++++++++++++++++++---------------- lib/packets.h | 3 +++ 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index ecf3bcc..de0ab9b 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -1565,9 +1565,10 @@ check_l4_icmp6(const struct conn_key *key, const void *data, size_t size, } 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, + size_t *chk_len) { - if (OVS_UNLIKELY(size < TCP_HEADER_LEN)) { + if (OVS_UNLIKELY(size < (chk_len ? *chk_len : TCP_HEADER_LEN))) { return false; } @@ -1580,9 +1581,10 @@ extract_l4_tcp(struct conn_key *key, const void *data, size_t size) } static inline bool -extract_l4_udp(struct conn_key *key, const void *data, size_t size) +extract_l4_udp(struct conn_key *key, const void *data, size_t size, + size_t *chk_len) { - if (OVS_UNLIKELY(size < UDP_HEADER_LEN)) { + if (OVS_UNLIKELY(size < (chk_len ? *chk_len : UDP_HEADER_LEN))) { return false; } @@ -1596,7 +1598,7 @@ extract_l4_udp(struct conn_key *key, const void *data, size_t size) static inline bool extract_l4(struct conn_key *key, const void *data, size_t size, bool *related, const void *l3, - bool validate_checksum); + bool validate_checksum, size_t *chk_len); static uint8_t reverse_icmp_type(uint8_t type) @@ -1628,9 +1630,9 @@ reverse_icmp_type(uint8_t type) * possible */ static inline int extract_l4_icmp(struct conn_key *key, const void *data, size_t size, - bool *related) + bool *related, size_t *chk_len) { - if (OVS_UNLIKELY(size < ICMP_HEADER_LEN)) { + if (OVS_UNLIKELY(size < (chk_len ? *chk_len : ICMP_HEADER_LEN))) { return false; } @@ -1681,8 +1683,9 @@ extract_l4_icmp(struct conn_key *key, const void *data, size_t size, key->src = inner_key.src; key->dst = inner_key.dst; key->nw_proto = inner_key.nw_proto; + size_t check_len = ICMP_ERROR_DATA_L4_LEN; - ok = extract_l4(key, l4, tail - l4, NULL, l3, false); + ok = extract_l4(key, l4, tail - l4, NULL, l3, false, &check_len); if (ok) { conn_key_reverse(key); *related = true; @@ -1769,7 +1772,7 @@ extract_l4_icmp6(struct conn_key *key, const void *data, size_t size, key->dst = inner_key.dst; key->nw_proto = inner_key.nw_proto; - ok = extract_l4(key, l4, tail - l4, NULL, l3, false); + ok = extract_l4(key, l4, tail - l4, NULL, l3, false, NULL); if (ok) { conn_key_reverse(key); *related = true; @@ -1797,23 +1800,25 @@ extract_l4_icmp6(struct conn_key *key, const void *data, size_t size, * in an ICMP error. In this case, we skip checksum and length validation. */ static inline bool extract_l4(struct conn_key *key, const void *data, size_t size, bool *related, - const void *l3, bool validate_checksum) + const void *l3, bool validate_checksum, size_t *chk_len) { 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, chk_len); } else if (key->nw_proto == IPPROTO_UDP) { return (!related || check_l4_udp(key, data, size, l3, - validate_checksum)) && extract_l4_udp(key, data, size); + validate_checksum)) + && extract_l4_udp(key, data, size, chk_len); } else if (key->dl_type == htons(ETH_TYPE_IP) && key->nw_proto == IPPROTO_ICMP) { return (!related || check_l4_icmp(data, size, validate_checksum)) - && extract_l4_icmp(key, data, size, related); + && extract_l4_icmp(key, data, size, related, chk_len); } else if (key->dl_type == htons(ETH_TYPE_IPV6) && key->nw_proto == IPPROTO_ICMPV6) { return (!related || check_l4_icmp6(key, data, size, l3, - validate_checksum)) && extract_l4_icmp6(key, data, size, - related); + validate_checksum)) + && extract_l4_icmp6(key, data, size, related); } else { return false; } @@ -1892,7 +1897,8 @@ conn_key_extract(struct conntrack *ct, struct dp_packet *pkt, ovs_be16 dl_type, bool hwol_good_l4_csum = dp_packet_l4_checksum_valid(pkt); /* Validate the checksum only when hwol is not supported. */ if (extract_l4(&ctx->key, l4, dp_packet_l4_size(pkt), - &ctx->icmp_related, l3, !hwol_good_l4_csum)) { + &ctx->icmp_related, l3, !hwol_good_l4_csum, + NULL)) { ctx->hash = conn_key_hash(&ctx->key, ct->hash_basis); return true; } diff --git a/lib/packets.h b/lib/packets.h index 0b3e3a2..c78defb 100644 --- a/lib/packets.h +++ b/lib/packets.h @@ -780,6 +780,9 @@ struct icmp_header { }; BUILD_ASSERT_DECL(ICMP_HEADER_LEN == sizeof(struct icmp_header)); +/* ICMPV4 */ +#define ICMP_ERROR_DATA_L4_LEN 8 + #define IGMP_HEADER_LEN 8 struct igmp_header { uint8_t igmp_type;