From patchwork Thu Jun 17 20:21:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aaron Conole X-Patchwork-Id: 1493930 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Pw91Ceav; dkim-atps=neutral Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4G5YNp18Ctz9sSn for ; Fri, 18 Jun 2021 06:21:30 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 25A2D60E9C; Thu, 17 Jun 2021 20:21:28 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id JIjdDLRW35l4; Thu, 17 Jun 2021 20:21:27 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 3A58C60E94; Thu, 17 Jun 2021 20:21:26 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1BAF0C000D; Thu, 17 Jun 2021 20:21:26 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 71F81C000B for ; Thu, 17 Jun 2021 20:21:25 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 6486742279 for ; Thu, 17 Jun 2021 20:21:25 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp4.osuosl.org (amavisd-new); dkim=pass (1024-bit key) header.d=redhat.com Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id lRkjtcjts7QL for ; Thu, 17 Jun 2021 20:21:24 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by smtp4.osuosl.org (Postfix) with ESMTPS id 2ADC442273 for ; Thu, 17 Jun 2021 20:21:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623961282; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=0fHUPKKZE51TxnznNTqg0EgYJ2lYdTqo8m0g9Wm7UpM=; b=Pw91CeavUNTIE0w5SEs4K8ZCFh+oHJH5KksOdt60p76+88HfsGDVYUM83CiJ3EC8s2vnBy EskuMigaXGGzp+Y+CANkBgeBoI6boLVgFRawF4Gu/cZ7Y4nN2lL6iH9XVdynE7pWnH8DOS 26TZQY62bJ6XW7iOFxIVOgtpayZyybU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-593-klkH0XqON72GIqZCGxrO2w-1; Thu, 17 Jun 2021 16:21:21 -0400 X-MC-Unique: klkH0XqON72GIqZCGxrO2w-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 67255180DE06; Thu, 17 Jun 2021 20:21:20 +0000 (UTC) Received: from RHTPC1VM0NT.lan (ovpn-113-104.rdu2.redhat.com [10.10.113.104]) by smtp.corp.redhat.com (Postfix) with ESMTP id C497D1000324; Thu, 17 Jun 2021 20:21:19 +0000 (UTC) From: Aaron Conole To: dev@openvswitch.org Date: Thu, 17 Jun 2021 16:21:19 -0400 Message-Id: <20210617202119.104055-1-aconole@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=aconole@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH v1] conntrack: Add state and sequence validation X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" During testing, there was an edge condition that was found during packet pickup where userspace can improperly advance the TCP state machine during connection exstablishment and bypass the 3whs. This can pollute the TCP sequence windows. Add a fix to ensure that we move the state machine when we see the appropriate flags, and include a test to show the error condition. Signed-off-by: Aaron Conole --- NOTE: I haven't done as much testing for 'learn existing connections' as I would want. I expect that I may have missed a case there, and hope to include a test for it when I work on the tcp_loose mode support in the userspace conntrack lib/conntrack-tcp.c | 7 +++-- tests/system-traffic.at | 62 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c index 8a7c98cc45..1f6cc4368d 100644 --- a/lib/conntrack-tcp.c +++ b/lib/conntrack-tcp.c @@ -224,6 +224,7 @@ tcp_conn_update(struct conntrack *ct, struct conn *conn_, end = seq + p_len; if (tcp_flags & TCP_SYN) { + src->state = CT_DPIF_TCPS_SYN_SENT; /* SYN_SENT by src */ end++; if (dst->wscale & CT_WSCALE_FLAG) { src->wscale = tcp_get_wscale(tcp); @@ -245,7 +246,6 @@ tcp_conn_update(struct conntrack *ct, struct conn *conn_, } src->seqlo = seq; - src->state = CT_DPIF_TCPS_SYN_SENT; /* * May need to slide the window (seqhi may have been set by * the crappy stack check or if we picked up the connection @@ -329,7 +329,10 @@ tcp_conn_update(struct conntrack *ct, struct conn *conn_, } if (tcp_flags & TCP_ACK) { if (dst->state == CT_DPIF_TCPS_SYN_SENT) { - dst->state = CT_DPIF_TCPS_ESTABLISHED; + if (src->state == CT_DPIF_TCPS_SYN_SENT) { + /* Move to EST only once SRC things this is okay */ + dst->state = CT_DPIF_TCPS_ESTABLISHED; + } } else if (dst->state == CT_DPIF_TCPS_CLOSING) { dst->state = CT_DPIF_TCPS_FIN_WAIT_2; } diff --git a/tests/system-traffic.at b/tests/system-traffic.at index c73bbc420f..4e849085bb 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -6025,6 +6025,68 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | OFPROTO_CLEAR_DURATION_IDLE OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([conntrack - Out of order TCP state transition]) +dnl This can happen due to buggy TCP implementations that reuse ephemeral +dnl ports - this test will check that some invalid parameters don't advance +dnl the state machine +CHECK_CONNTRACK() +OVS_TRAFFIC_VSWITCHD_START() +OVS_CHECK_CT_CLEAR() + +ADD_NAMESPACES(at_ns0, at_ns1) +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "f0:00:00:01:01:01") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "f0:00:00:01:01:02") + +dnl setup ct flows +AT_DATA([flows.txt], [dnl +table=0,priority=10 arp action=normal +table=0,priority=10 ip,tcp,ct_state=-trk action=ct(table=1) +table=0,priority=1 action=drop +dnl dst ns2 +table=1,priority=20 ip,ct_state=+new+trk,nw_dst=10.1.1.2 action=ct(commit),output:ovs-p1 +table=1,priority=20 ip,ct_state=+est+trk,nw_dst=10.1.1.2 action=output:ovs-p1 +dnl dst ns1 +table=1,priority=10 ip,ct_state=+trk+est,nw_dst=10.1.1.1 action=output:ovs-p0 +table=1,priority=10 ip,ct_state=+trk+new,nw_dst=10.1.1.1 action=output:ovs-p0 +table=1,priority=1 ip,ct_state=+trk+inv action=drop +]) + +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) + +dnl kill tcp packets - this will suppress RST/RST-ACK messages +NS_CHECK_EXEC([at_ns0], [iptables -I INPUT 1 -p tcp --sport 6667 -j DROP]) +NS_CHECK_EXEC([at_ns1], [iptables -I INPUT 1 -p tcp --dport 6667 -j DROP]) + +dnl Send TCP SYN +NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 f0 00 00 01 01 02 f0 00 00 01 01 01 08 00 45 00 00 3c c9 c8 40 00 40 06 5a ef 0a 01 01 01 0a 01 01 02 b2 2a 1a 0b d3 78 6f 81 00 00 00 00 a0 02 fa f0 7a 0b 00 00 02 04 05 b4 04 02 08 0a 6d 35 40 9a 00 00 00 00 01 03 03 07 > /dev/null]) + +dnl Send TCP PSH|URG +NS_CHECK_EXEC([at_ns1], [$PYTHON3 $srcdir/sendpkt.py p1 f0 00 00 01 01 01 f0 00 00 01 01 02 08 00 45 00 00 34 c9 c9 40 00 40 06 5a f6 0a 01 01 02 0a 01 01 01 b2 2a 1a 0b d3 78 6f 82 0a 0b d5 33 80 28 01 f6 72 bb 00 00 01 01 08 0a 6d 35 40 9a 11 90 3e 20 > /dev/null]) + +dnl Check that we haven't advanced the ct state machine +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "dst=10.1.1.1" | sort | uniq], [0], [dnl +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=45610,dport=6667),reply=(src=10.1.1.2,dst=10.1.1.1,sport=6667,dport=45610),protoinfo=(state=SYN_SENT) +]) + +dnl Send TCP ACK without syn, and with garbage ack values +NS_CHECK_EXEC([at_ns1], [$PYTHON3 $srcdir/sendpkt.py p1 f0 00 00 01 01 01 f0 00 00 01 01 02 08 00 45 00 00 34 73 7a 40 00 40 06 b1 45 0a 01 01 02 0a 01 01 01 1a 0b b2 2a 0a 0b e5 33 d3 78 6f 87 80 10 01 fe 4b fa 00 00 01 01 08 0a 11 90 49 86 6d 35 4c 00 > /dev/null]) + +dnl Check that we haven't advanced the ct state machine +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "dst=10.1.1.1" | sort | uniq], [0], [dnl +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=45610,dport=6667),reply=(src=10.1.1.2,dst=10.1.1.1,sport=6667,dport=45610),protoinfo=(state=SYN_SENT) +]) + +dnl Now send a valid SYN|ACK +NS_CHECK_EXEC([at_ns1], [$PYTHON3 $srcdir/sendpkt.py p1 f0 00 00 01 01 01 f0 00 00 01 01 02 08 00 45 00 00 3c 00 00 40 00 40 06 24 b8 0a 01 01 02 0a 01 01 01 1a 0b b2 2a 0a 0b d5 32 d3 78 6f 82 a0 12 fe 88 47 74 00 00 02 04 05 b4 04 02 08 0a 11 90 3e 20 6d 35 40 9a 01 03 03 07 > /dev/null]) + +dnl Now check that we are in ESTABLISHED +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "dst=10.1.1.1" | sort | uniq], [0], [dnl +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=45610,dport=6667),reply=(src=10.1.1.2,dst=10.1.1.1,sport=6667,dport=45610),protoinfo=(state=ESTABLISHED) +]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + AT_BANNER([802.1ad]) AT_SETUP([802.1ad - vlan_limit])