From patchwork Tue Oct 26 10:06:05 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cheng Li X-Patchwork-Id: 1546369 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=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (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 bilbo.ozlabs.org (Postfix) with ESMTPS id 4HdnhB1F5mz9sfG for ; Tue, 26 Oct 2021 21:12:56 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id A295B40050; Tue, 26 Oct 2021 10:12:51 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org 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 H27d_K6BfeSE; Tue, 26 Oct 2021 10:12:50 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp4.osuosl.org (Postfix) with ESMTPS id D1A4140210; Tue, 26 Oct 2021 10:12:49 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 8E063C0019; Tue, 26 Oct 2021 10:12:49 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 00898C000E for ; Tue, 26 Oct 2021 10:12:48 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id E456580F42 for ; Tue, 26 Oct 2021 10:12:47 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id l20aIV5f0DGE for ; Tue, 26 Oct 2021 10:12:45 +0000 (UTC) X-Greylist: delayed 00:06:34 by SQLgrey-1.8.0 Received: from chinatelecom.cn (prt-mail.chinatelecom.cn [42.123.76.220]) by smtp1.osuosl.org (Postfix) with ESMTP id 3343481011 for ; Tue, 26 Oct 2021 10:12:45 +0000 (UTC) HMM_SOURCE_IP: 172.18.0.218:40602.487411740 HMM_ATTACHE_NUM: 0000 HMM_SOURCE_TYPE: SMTP Received: from clientip-182.150.57.243 (unknown [172.18.0.218]) by chinatelecom.cn (HERMES) with SMTP id 5B6A6280088 for ; Tue, 26 Oct 2021 18:06:02 +0800 (CST) X-189-SAVE-TO-SEND: lic121@chinatelecom.cn Received: from ([172.18.0.218]) by app0025 with ESMTP id 7c876594439d44b080e93cf2566c172e for dev@openvswitch.org; Tue, 26 Oct 2021 18:06:03 CST X-Transaction-ID: 7c876594439d44b080e93cf2566c172e X-Real-From: lic121@chinatelecom.cn X-Receive-IP: 172.18.0.218 Date: Tue, 26 Oct 2021 18:06:05 +0800 From: "lic121@chinatelecom.cn" To: "dev@openvswitch.org" X-Priority: 3 X-Has-Attach: no X-Mailer: Foxmail 7.2.19.158[cn] Mime-Version: 1.0 Message-ID: <2021102618060472779916@chinatelecom.cn> X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: [ovs-dev] [PATCH 1/2] upcall: prevent from installing flows when inconsistence 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" In ovs kernel datapath upcall, the *key* and packet are passed to userspace. The key contains the fields/meta extracted from packet. Once the ovs-vswitchd receives the upcall, the packet is extracted again into *flow*. Next, the flow is used to match openflow rules to generate the wildcard(wc). At last, vswitchd installs a mega_flow in datapath(mega_flow = key/wc,action) We can see that vswitchd generate wc from flow while it installs dp flow with key. If the key is not consistent with the flow [1], we get bad mega_flow. Let's assume we have the flowing rules, means to block tcp port 0-0xf, but allow other ports. "table=0,priority=100,tcp,tp_dst=0x0/0xfff0 actions=drop" "table=0,priority=90,tcp actions=p1" good case: If a packet has tcp dst=0x10, generated `mega_flow=0x10/0xfff0,out:p1`, this is expected. bad case: If a packet has tcp dst=0x10 but not pass tcphdr_ok [1], generated wc and action are `0xfff0,out:p1`. The mega_flow will be `0x0/0xfff0,out:p1`, bacause mega_flow=key/wc,action. This allows packets with tcp port 0-0xf pass by mistake. The following scapy3 script triggers the issue: ```py eth=Ether(src="fa:16:3e:5e:e3:57",dst="be:95:df:40:fb:57") ip=IP(src="10.10.10.10",dst="20.20.20.20") tcp=TCP(sport=100,dport=16,dataofs=1) sendp(eth/ip/tcp) ``` This patch is to prevent from installing datapath flow if the key is not consistant with the flow. [1] https://github.com/openvswitch/ovs/blob/v2.16.1/datapath/flow.c#L601 Signed-off-by: lic121 --- ofproto/ofproto-dpif-upcall.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 1c9c720..ad2e63b 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -244,6 +244,7 @@ struct upcall { size_t key_len; /* Datapath flow key length. */ const struct nlattr *out_tun_key; /* Datapath output tunnel key. */ + const struct flow *key_as_flow; /* converted from key. */ struct user_action_cookie cookie; uint64_t odp_actions_stub[1024 / 8]; /* Stub for odp_actions. */ @@ -810,6 +811,7 @@ recv_upcalls(struct handler *handler) struct dpif_upcall dupcalls[UPCALL_MAX_BATCH]; struct upcall upcalls[UPCALL_MAX_BATCH]; struct flow flows[UPCALL_MAX_BATCH]; + struct flow key_as_flows[UPCALL_MAX_BATCH]; size_t n_upcalls, i; n_upcalls = 0; @@ -818,6 +820,7 @@ recv_upcalls(struct handler *handler) struct dpif_upcall *dupcall = &dupcalls[n_upcalls]; struct upcall *upcall = &upcalls[n_upcalls]; struct flow *flow = &flows[n_upcalls]; + struct flow *key_as_flow = &key_as_flows[n_upcalls]; unsigned int mru = 0; uint64_t hash = 0; int error; @@ -830,7 +833,7 @@ recv_upcalls(struct handler *handler) } upcall->fitness = odp_flow_key_to_flow(dupcall->key, dupcall->key_len, - flow, NULL); + key_as_flow, NULL); if (upcall->fitness == ODP_FIT_ERROR) { goto free_dupcall; } @@ -842,7 +845,8 @@ recv_upcalls(struct handler *handler) if (dupcall->hash) { hash = nl_attr_get_u64(dupcall->hash); } - + /* Fill flow with key_as_flow as upcall_receive needs packet flow info */ + *flow = *key_as_flow; error = upcall_receive(upcall, udpif->backer, &dupcall->packet, dupcall->type, dupcall->userdata, flow, mru, &dupcall->ufid, PMD_ID_NULL); @@ -856,20 +860,21 @@ recv_upcalls(struct handler *handler) dupcall->key_len, NULL, 0, NULL, 0, &dupcall->ufid, PMD_ID_NULL, NULL); VLOG_INFO_RL(&rl, "received packet on unassociated datapath " - "port %"PRIu32, flow->in_port.odp_port); + "port %"PRIu32, key_as_flow->in_port.odp_port); } goto free_dupcall; } upcall->key = dupcall->key; upcall->key_len = dupcall->key_len; + upcall->key_as_flow = key_as_flow; upcall->ufid = &dupcall->ufid; upcall->hash = hash; upcall->out_tun_key = dupcall->out_tun_key; upcall->actions = dupcall->actions; - pkt_metadata_from_flow(&dupcall->packet.md, flow); + pkt_metadata_from_flow(&dupcall->packet.md, key_as_flow); flow_extract(&dupcall->packet, flow); error = process_upcall(udpif, upcall, @@ -1332,6 +1337,19 @@ should_install_flow(struct udpif *udpif, struct upcall *upcall) return false; } + /* For linux kernel datapath, the "key" extracted by kernel may be + inconsistent with the flow extracted from packet by ovs. If that + is the case, twe can't install the datapth flow (key/wc) */ + if (upcall->key_len){ + if (!flow_equal_except(upcall->key_as_flow, upcall->flow, &upcall->wc)){ + VLOG_INFO_RL(&rl, "upcall: inconsistent on datapath key and vswitchd" + " extracted key. Datapath flow will not be installed\n" + "datapath key: %s \nvswitchd extracted key: %s", + flow_to_string(upcall->key_as_flow, NULL), + flow_to_string(upcall->flow, NULL)); + return false; + } + } return true; } From patchwork Tue Oct 26 10:07:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cheng Li X-Patchwork-Id: 1546370 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=2605:bc80:3010::133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) (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 bilbo.ozlabs.org (Postfix) with ESMTPS id 4Hdnqy65p6z9sP7 for ; Tue, 26 Oct 2021 21:19:42 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 41E064025F; Tue, 26 Oct 2021 10:19:39 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id NFGsKEXVTMhq; Tue, 26 Oct 2021 10:19:38 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp2.osuosl.org (Postfix) with ESMTPS id 499D640017; Tue, 26 Oct 2021 10:19:37 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0B7CBC0012; Tue, 26 Oct 2021 10:19:37 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id EEE49C000E for ; Tue, 26 Oct 2021 10:19:34 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id E9C43608F2 for ; Tue, 26 Oct 2021 10:19:34 +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 dbOjt2irZKMb for ; Tue, 26 Oct 2021 10:19:33 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.8.0 Received: from chinatelecom.cn (prt-mail.chinatelecom.cn [42.123.76.222]) by smtp3.osuosl.org (Postfix) with ESMTP id 0D32160775 for ; Tue, 26 Oct 2021 10:19:32 +0000 (UTC) HMM_SOURCE_IP: 172.18.0.218:38700.343539712 HMM_ATTACHE_NUM: 0000 HMM_SOURCE_TYPE: SMTP Received: from clientip-182.150.57.243 (unknown [172.18.0.218]) by chinatelecom.cn (HERMES) with SMTP id C69F92800A7 for ; Tue, 26 Oct 2021 18:07:26 +0800 (CST) X-189-SAVE-TO-SEND: lic121@chinatelecom.cn Received: from ([172.18.0.218]) by app0025 with ESMTP id a4e5dd970e2c49a7b00973adee0d4aad for dev@openvswitch.org; Tue, 26 Oct 2021 18:07:27 CST X-Transaction-ID: a4e5dd970e2c49a7b00973adee0d4aad X-Real-From: lic121@chinatelecom.cn X-Receive-IP: 172.18.0.218 Date: Tue, 26 Oct 2021 18:07:29 +0800 From: "lic121@chinatelecom.cn" To: "dev@openvswitch.org" X-Priority: 3 X-Has-Attach: no X-Mailer: Foxmail 7.2.19.158[cn] Mime-Version: 1.0 Message-ID: <2021102618070792615017@chinatelecom.cn> X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: [ovs-dev] [PATCH 2/2] upcall: considering dataofs when parsing tcp pkt 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" dataofs field of tcp header indicates the tcp header len. The len should be >= 20 bytes/4. This patch is to test dataofs, and don't parse layer 4 fields when meet ba dataofs. This behave is the consistent with openvswitch kenrel module. Signed-off-by: lic121 --- lib/flow.c | 18 ++++++++++-------- tests/ofproto-dpif.at | 31 +++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/lib/flow.c b/lib/flow.c index 89837de..16d04fc 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -1006,14 +1006,16 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst) if (OVS_LIKELY(nw_proto == IPPROTO_TCP)) { if (OVS_LIKELY(size >= TCP_HEADER_LEN)) { const struct tcp_header *tcp = data; - - miniflow_push_be32(mf, arp_tha.ea[2], 0); - miniflow_push_be32(mf, tcp_flags, - TCP_FLAGS_BE32(tcp->tcp_ctl)); - miniflow_push_be16(mf, tp_src, tcp->tcp_src); - miniflow_push_be16(mf, tp_dst, tcp->tcp_dst); - miniflow_push_be16(mf, ct_tp_src, ct_tp_src); - miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst); + size_t tcp_hdr_len = TCP_OFFSET(tcp->tcp_ctl) * 4; + if (tcp_hdr_len >= TCP_HEADER_LEN){ + miniflow_push_be32(mf, arp_tha.ea[2], 0); + miniflow_push_be32(mf, tcp_flags, + TCP_FLAGS_BE32(tcp->tcp_ctl)); + miniflow_push_be16(mf, tp_src, tcp->tcp_src); + miniflow_push_be16(mf, tp_dst, tcp->tcp_dst); + miniflow_push_be16(mf, ct_tp_src, ct_tp_src); + miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst); + } } } else if (OVS_LIKELY(nw_proto == IPPROTO_UDP)) { if (OVS_LIKELY(size >= UDP_HEADER_LEN)) { diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 31fb163..0f372ae 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -4862,6 +4862,37 @@ recirc_id(0),in_port(90),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,fr OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([ofproto-dpif - malformed packets handling - upcall]) +OVS_VSWITCHD_START +add_of_ports br0 1 90 +dnl drop packet has tcp port 0-f but allow other tcp packets +AT_DATA([flows.txt], [dnl +priority=75 tcp tp_dst=0/0xfff0 actions=drop +priority=50 tcp actions=output:1 +]) +AT_CHECK([ovs-ofctl replace-flows br0 flows.txt]) +dnl good tcp pkt, tcp(sport=100,dpor=16) +pkt1="be95df40fb57fa163e5ee3570800450000280001000040063e940a0a0a0a141414140064001000000000000000005002200053330000" +dnl malformed tcp pkt, tcp(sport=100,dport=16,dataofs=1) +pkt2="be95df40fb57fa163e5ee3570800450000280001000040063e940a0a0a0a141414140064001000000000000000001002200093330000" +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) +mode=normal +AT_CHECK([ovs-appctl netdev-dummy/receive p90 "$pkt1"], [0], [stdout]) +dnl for good tcp pkt, ovs can extract the tp_dst=16 +AT_CHECK([ovs-appctl dpctl/dump-flows filter=in_port\(90\),tcp], [0], [dnl +flow-dump from the main thread: +recirc_id(0),in_port(90),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,frag=no),tcp(dst=16/0xfff0), packets:0, bytes:0, used:never, actions:1 +]) +AT_CHECK([ovs-appctl dpctl/del-flows], [0], [stdout]) +AT_CHECK([ovs-appctl netdev-dummy/receive p90 "$pkt2"], [0], [stdout]) +dnl for malformed tcp pkt, ovs can use default value tp_dst=0 +AT_CHECK([ovs-appctl dpctl/dump-flows filter=in_port\(90\),tcp], [0], [dnl +flow-dump from the main thread: +recirc_id(0),in_port(90),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,frag=no),tcp(dst=0/0xfff0), packets:0, bytes:0, used:never, actions:drop +]) +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ofproto-dpif - exit]) OVS_VSWITCHD_START add_of_ports br0 1 2 3 10 11 12 13 14