From ccad8b61bb8bebeabc588275be585e378c305291 Mon Sep 17 00:00:00 2001 From: zppzhangpan Date: Tue, 20 Feb 2024 16:23:31 +0800 Subject: [PATCH] fix CVE-2023-3966 (cherry picked from commit c99552111ab98cfd61d0714873fed83d8780dd00) --- backport-0001-CVE-2023-3966.patch | 160 ++++++++++++++++++++++++++++++ backport-0002-CVE-2023-3966.patch | 90 +++++++++++++++++ backport-0003-CVE-2023-3966.patch | 138 ++++++++++++++++++++++++++ openvswitch.spec | 15 ++- 4 files changed, 399 insertions(+), 4 deletions(-) create mode 100644 backport-0001-CVE-2023-3966.patch create mode 100644 backport-0002-CVE-2023-3966.patch create mode 100644 backport-0003-CVE-2023-3966.patch diff --git a/backport-0001-CVE-2023-3966.patch b/backport-0001-CVE-2023-3966.patch new file mode 100644 index 0000000..3e105cf --- /dev/null +++ b/backport-0001-CVE-2023-3966.patch @@ -0,0 +1,160 @@ +From 87f191a3a398ae495fa55d70ca5fe5d813bd284f Mon Sep 17 00:00:00 2001 +From: Ilya Maximets +Date: Sun, 14 Aug 2022 16:45:59 +0200 +Subject: [PATCH] netdev-offload-tc: Fix the mask for tunnel metadata length. + +'wc.masks.tunnel.metadata.present.len' must be a mask for the same +field in the flow key, but in the tc_flower structure it's the real +length of metadata masks. + +That is correctly handled for the individual opt->length, setting all +the masks to 0x1f like it's done in the tun_metadata_to_geneve_mask__(), +but not handled for the main 'len' field. + +Fix that by setting the mask to 0xff like it's done during the flow +translation in xlate_actions() and during the flow dump in the +tun_metadata_from_geneve_nlattr(). + +Also, flower always has an exact match on the present.len field +regardless of its value and regardless of this field being masked +by OVS flow translation layer while installing the flow. Hence, +all tunnel flows dumped from TC should have an exact match on +present.len and also UDPIF flag, because present.len doesn't make +sense without that flag. Without the change, zero-length options +match is incorrectly reported as a wildcard match. The side effect +though is that zero-length match on geneve options is reported even +for other tunnel types, e.g. vxlan. But that should be fairly +harmless. To avoid reporting a match on empty geneve options for +vxlan/etc. tunnels we'll need to check the tunnel port type, there +is no enough information in the TUNNEL attribute itself. + +Extra checks and comments added around the code to better explain +what is going on. + +Fixes: a468645c6d33 ("lib/tc: add geneve with option match offload") +Reviewed-by: Roi Dayan +Signed-off-by: Ilya Maximets + +Reference:https://github.com/openvswitch/ovs/commit/87f191a3a398ae495fa55d70ca5fe5d813bd284f +Conflict:Context adaptation + +--- + lib/netdev-offload-tc.c | 33 +++++++++++++++++++++++---------- + lib/tc.c | 13 ++++++++++--- + 2 files changed, 33 insertions(+), 13 deletions(-) + +diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c +index 68c4c7e..2383fe2 100644 +--- a/lib/netdev-offload-tc.c ++++ b/lib/netdev-offload-tc.c +@@ -476,30 +476,42 @@ flower_tun_opt_to_match(struct match *match, struct tc_flower *flower) + struct geneve_opt *opt, *opt_mask; + int len, cnt = 0; + ++ /* Options are always in UDPIF format in the 'flower'. */ ++ match->flow.tunnel.flags |= FLOW_TNL_F_UDPIF; ++ match->wc.masks.tunnel.flags |= FLOW_TNL_F_UDPIF; ++ ++ match->flow.tunnel.metadata.present.len = ++ flower->key.tunnel.metadata.present.len; ++ /* In the 'flower' mask len is an actual length, not a mask. But in the ++ * 'match' it is an actual mask, so should be an exact match, because TC ++ * will always match on the exact value. */ ++ match->wc.masks.tunnel.metadata.present.len = 0xff; ++ ++ if (!flower->key.tunnel.metadata.present.len) { ++ /* No options present. */ ++ return; ++ } ++ + memcpy(match->flow.tunnel.metadata.opts.gnv, + flower->key.tunnel.metadata.opts.gnv, + flower->key.tunnel.metadata.present.len); +- match->flow.tunnel.metadata.present.len = +- flower->key.tunnel.metadata.present.len; +- match->flow.tunnel.flags |= FLOW_TNL_F_UDPIF; + memcpy(match->wc.masks.tunnel.metadata.opts.gnv, + flower->mask.tunnel.metadata.opts.gnv, + flower->mask.tunnel.metadata.present.len); + ++ /* Fixing up 'length' fields of particular options, since these are ++ * also not masks, but actual lengths in the 'flower' structure. */ + len = flower->key.tunnel.metadata.present.len; + while (len) { + opt = &match->flow.tunnel.metadata.opts.gnv[cnt]; + opt_mask = &match->wc.masks.tunnel.metadata.opts.gnv[cnt]; + ++ /* "Exact" match as set in tun_metadata_to_geneve_mask__(). */ + opt_mask->length = 0x1f; + + cnt += sizeof(struct geneve_opt) / 4 + opt->length; + len -= sizeof(struct geneve_opt) + opt->length * 4; + } +- +- match->wc.masks.tunnel.metadata.present.len = +- flower->mask.tunnel.metadata.present.len; +- match->wc.masks.tunnel.flags |= FLOW_TNL_F_UDPIF; + } + + static int +@@ -621,9 +633,8 @@ parse_tc_flower_to_match(struct tc_flower *flower, + if (flower->key.tunnel.tp_dst) { + match_set_tun_tp_dst(match, flower->key.tunnel.tp_dst); + } +- if (flower->key.tunnel.metadata.present.len) { +- flower_tun_opt_to_match(match, flower); +- } ++ ++ flower_tun_opt_to_match(match, flower); + } + + act_off = nl_msg_start_nested(buf, OVS_FLOW_ATTR_ACTIONS); +@@ -1079,6 +1090,8 @@ flower_match_to_tun_opt(struct tc_flower *flower, const struct flow_tnl *tnl, + len -= sizeof(struct geneve_opt) + opt->length * 4; + } + ++ /* Copying from the key and not from the mask, since in the 'flower' ++ * the length for a mask is not a mask, but the actual length. */ + flower->mask.tunnel.metadata.present.len = tnl->metadata.present.len; + } + +diff --git a/lib/tc.c b/lib/tc.c +index 1e9810f..ef6138f 100644 +--- a/lib/tc.c ++++ b/lib/tc.c +@@ -588,15 +588,17 @@ flower_tun_geneve_opt_check_len(struct tun_metadata *key, + const struct geneve_opt *opt, *opt_mask; + int len, cnt = 0; + ++ if (key->present.len != mask->present.len) { ++ goto bad_length; ++ } ++ + len = key->present.len; + while (len) { + opt = &key->opts.gnv[cnt]; + opt_mask = &mask->opts.gnv[cnt]; + + if (opt->length != opt_mask->length) { +- VLOG_ERR_RL(&error_rl, +- "failed to parse tun options; key/mask length differ"); +- return EINVAL; ++ goto bad_length; + } + + cnt += sizeof(struct geneve_opt) / 4 + opt->length; +@@ -604,6 +606,11 @@ flower_tun_geneve_opt_check_len(struct tun_metadata *key, + } + + return 0; ++ ++bad_length: ++ VLOG_ERR_RL(&error_rl, ++ "failed to parse tun options; key/mask length differ"); ++ return EINVAL; + } + + static int +-- +2.33.0 + diff --git a/backport-0002-CVE-2023-3966.patch b/backport-0002-CVE-2023-3966.patch new file mode 100644 index 0000000..8c2bf62 --- /dev/null +++ b/backport-0002-CVE-2023-3966.patch @@ -0,0 +1,90 @@ +From 6f322ccf8a9989905b7c29420239d5f6d81f0002 Mon Sep 17 00:00:00 2001 +From: Ilya Maximets +Date: Fri, 19 Aug 2022 21:51:27 +0200 +Subject: [PATCH] netdev-offload-tc: Parse tunnel options only for geneve + ports. + +Cited commit correctly fixed the issue of incorrect reporting +of zero-length geneve option matches as wildcarded. But as a +side effect, exact match on the metadata length was added to +every tunnel flow match, even if the tunnel is not geneve. +That doesn't generate any functional issues, but it maybe +confusing to see 'tunnel(...,geneve(),...)' while looking at +datapath flow dumps for, e.g., vxlan tunnel flows. + +Fix that by checking the port type before parsing geneve options. +tunnel() attribute itself doesn't have enough information to +figure out the tunnel type. + +Fixes: 7a6c8074c5d2 ("netdev-offload-tc: Fix the mask for tunnel metadata length.") +Acked-by: Eelco Chaudron +Reviewed-by: Roi Dayan +Signed-off-by: Ilya Maximets + +Reference:https://github.com/openvswitch/ovs/commit/6f322ccf8a9989905b7c29420239d5f6d81f0002 +Conflict: Implement terse dump support: The parse_tc_flower_to_match function has an extra parameter. + +--- + lib/netdev-offload-tc.c | 18 ++++++++++++------ + 1 file changed, 12 insertions(+), 6 deletions(-) + +diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c +index 2383fe2..b8c3738 100644 +--- a/lib/netdev-offload-tc.c ++++ b/lib/netdev-offload-tc.c +@@ -515,7 +515,8 @@ flower_tun_opt_to_match(struct match *match, struct tc_flower *flower) + } + + static int +-parse_tc_flower_to_match(struct tc_flower *flower, ++parse_tc_flower_to_match(const struct netdev *netdev, ++ struct tc_flower *flower, + struct match *match, + struct nlattr **actions, + struct dpif_flow_stats *stats, +@@ -634,7 +635,9 @@ parse_tc_flower_to_match(struct tc_flower *flower, + match_set_tun_tp_dst(match, flower->key.tunnel.tp_dst); + } + +- flower_tun_opt_to_match(match, flower); ++ if (!strcmp(netdev_get_type(netdev), "geneve")) { ++ flower_tun_opt_to_match(match, flower); ++ } + } + + act_off = nl_msg_start_nested(buf, OVS_FLOW_ATTR_ACTIONS); +@@ -760,8 +763,8 @@ netdev_tc_flow_dump_next(struct netdev_flow_dump *dump, + continue; + } + +- if (parse_tc_flower_to_match(&flower, match, actions, stats, attrs, +- wbuffer)) { ++ if (parse_tc_flower_to_match(netdev, &flower, match, actions, ++ stats, attrs, wbuffer)) { + continue; + } + +@@ -1145,7 +1148,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, + flower.mask.tunnel.tos = tnl_mask->ip_tos; + flower.mask.tunnel.ttl = tnl_mask->ip_ttl; + flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ? tnl_mask->tun_id : 0; +- flower_match_to_tun_opt(&flower, tnl, tnl_mask); ++ if (!strcmp(netdev_get_type(netdev), "geneve")) { ++ flower_match_to_tun_opt(&flower, tnl, tnl_mask); ++ } + flower.tunnel = true; + } + memset(&mask->tunnel, 0, sizeof mask->tunnel); +@@ -1458,7 +1463,8 @@ netdev_tc_flow_get(struct netdev *netdev OVS_UNUSED, + } + + in_port = netdev_ifindex_to_odp_port(ifindex); +- parse_tc_flower_to_match(&flower, match, actions, stats, attrs, buf); ++ parse_tc_flower_to_match(netdev, &flower, match, actions, ++ stats, attrs, buf); + + match->wc.masks.in_port.odp_port = u32_to_odp(UINT32_MAX); + match->flow.in_port.odp_port = in_port; +-- +2.33.0 + diff --git a/backport-0003-CVE-2023-3966.patch b/backport-0003-CVE-2023-3966.patch new file mode 100644 index 0000000..46b7b4c --- /dev/null +++ b/backport-0003-CVE-2023-3966.patch @@ -0,0 +1,138 @@ +From b8657dada9641fbd2bd3a3f882e0862448d60910 Mon Sep 17 00:00:00 2001 +From: Timothy Redaelli +Date: Thu, 23 Nov 2023 19:47:54 +0100 +Subject: [PATCH] netdev-offload-tc: Check geneve metadata length. + +Currently ovs-vswitchd crashes, with hw offloading enabled, if a geneve +packet with corrupted metadata is received, because the metadata header +is not verified correctly. + +This commit adds a check for geneve metadata length and, if the header +is wrong, the packet is not sent to flower. + +It also includes a system-traffic test for geneve packets with corrupted +metadata. + +Fixes: a468645c6d33 ("lib/tc: add geneve with option match offload") +Reported-by: Haresh Khandelwal +Signed-off-by: Timothy Redaelli +Signed-off-by: Ilya Maximets + +Reference:https://github.com/openvswitch/ovs/commit/b8657dada9641fbd2bd3a3f882e0862448d60910 +Conflict:The return value is not modified in flower_match_to_tun_opt function + +--- + lib/netdev-offload-tc.c | 21 +++++++++++++++++--- + tests/system-offloads-traffic.at | 34 ++++++++++++++++++++++++++++++++ + 2 files changed, 52 insertions(+), 3 deletions(-) + +diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c +index b8c3738..e248d0b 100644 +--- a/lib/netdev-offload-tc.c ++++ b/lib/netdev-offload-tc.c +@@ -42,6 +42,7 @@ + VLOG_DEFINE_THIS_MODULE(netdev_offload_tc); + + static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(60, 5); ++static struct vlog_rate_limit warn_rl = VLOG_RATE_LIMIT_INIT(10, 2); + + static struct hmap ufid_tc = HMAP_INITIALIZER(&ufid_tc); + static bool multi_mask_per_prio = false; +@@ -1068,12 +1069,12 @@ test_key_and_mask(struct match *match) + return 0; + } + +-static void ++static int + flower_match_to_tun_opt(struct tc_flower *flower, const struct flow_tnl *tnl, + const struct flow_tnl *tnl_mask) + { + struct geneve_opt *opt, *opt_mask; +- int len, cnt = 0; ++ int tot_opt_len, len, cnt = 0; + + memcpy(flower->key.tunnel.metadata.opts.gnv, tnl->metadata.opts.gnv, + tnl->metadata.present.len); +@@ -1084,7 +1085,16 @@ flower_match_to_tun_opt(struct tc_flower *flower, const struct flow_tnl *tnl, + + len = flower->key.tunnel.metadata.present.len; + while (len) { ++ if (len < sizeof *opt) { ++ return EOPNOTSUPP; ++ } ++ + opt = &flower->key.tunnel.metadata.opts.gnv[cnt]; ++ tot_opt_len = sizeof *opt + opt->length * 4; ++ if (len < tot_opt_len) { ++ return EOPNOTSUPP; ++ } ++ + opt_mask = &flower->mask.tunnel.metadata.opts.gnv[cnt]; + + opt_mask->length = opt->length; +@@ -1096,6 +1106,7 @@ flower_match_to_tun_opt(struct tc_flower *flower, const struct flow_tnl *tnl, + /* Copying from the key and not from the mask, since in the 'flower' + * the length for a mask is not a mask, but the actual length. */ + flower->mask.tunnel.metadata.present.len = tnl->metadata.present.len; ++ return 0; + } + + static int +@@ -1149,7 +1160,11 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, + flower.mask.tunnel.ttl = tnl_mask->ip_ttl; + flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ? tnl_mask->tun_id : 0; + if (!strcmp(netdev_get_type(netdev), "geneve")) { +- flower_match_to_tun_opt(&flower, tnl, tnl_mask); ++ err = flower_match_to_tun_opt(&flower, tnl, tnl_mask); ++ if (err) { ++ VLOG_WARN_RL(&warn_rl, "Unable to parse geneve options"); ++ return err; ++ } + } + flower.tunnel = true; + } +diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at +index bfad66e..9643535 100644 +--- a/tests/system-offloads-traffic.at ++++ b/tests/system-offloads-traffic.at +@@ -116,3 +116,37 @@ matchall + ]) + OVS_TRAFFIC_VSWITCHD_STOP + AT_CLEANUP ++ ++AT_SETUP([offloads - handling of geneve corrupted metadata - offloads enabled]) ++OVS_CHECK_GENEVE() ++ ++OVS_TRAFFIC_VSWITCHD_START( ++ [_ADD_BR([br-underlay]) -- \ ++ set bridge br0 other-config:hwaddr=f2:ff:00:00:00:01 -- \ ++ set bridge br-underlay other-config:hwaddr=f2:ff:00:00:00:02]) ++ ++AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true]) ++ ++AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) ++AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"]) ++ ++ADD_NAMESPACES(at_ns0) ++ ++dnl Set up underlay link from host into the namespace using veth pair. ++ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24", f2:ff:00:00:00:03) ++AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"]) ++AT_CHECK([ip link set dev br-underlay up]) ++ ++dnl Set up tunnel endpoints on OVS outside the namespace and with a native ++dnl linux device inside the namespace. ++ADD_OVS_TUNNEL([geneve], [br0], [at_gnv0], [172.31.1.1], [10.1.1.100/24]) ++ADD_NATIVE_TUNNEL([geneve], [ns_gnv0], [at_ns0], [172.31.1.100], [10.1.1.1/24], ++ [vni 0], [address f2:ff:00:00:00:04]) ++ ++NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 f2 ff 00 00 00 02 f2 ff 00 00 00 03 08 00 45 00 00 52 00 01 00 00 40 11 1f f7 ac 1f 01 01 ac 1f 01 64 de c1 17 c1 00 3e 59 e9 01 00 65 58 00 00 00 00 00 03 00 02 f2 ff 00 00 00 01 f2 ff 00 00 00 04 08 00 45 00 00 1c 00 01 00 00 40 01 64 7a 0a 01 01 01 0a 01 01 64 08 00 f7 ff 00 00 00 00 > /dev/null]) ++ ++OVS_WAIT_UNTIL([grep -q 'Invalid Geneve tunnel metadata' ovs-vswitchd.log]) ++ ++OVS_TRAFFIC_VSWITCHD_STOP(["/Invalid Geneve tunnel metadata on bridge br0 while processing icmp,in_port=1,vlan_tci=0x0000,dl_src=f2:ff:00:00:00:04,dl_dst=f2:ff:00:00:00:01,nw_src=10.1.1.1,nw_dst=10.1.1.100,nw_tos=0,nw_ecn=0,nw_ttl=64,nw_frag=no,icmp_type=8,icmp_code=0/d ++/Unable to parse geneve options/d"]) ++AT_CLEANUP +-- +2.33.0 + diff --git a/openvswitch.spec b/openvswitch.spec index 8ca126f..b7a9b5b 100644 --- a/openvswitch.spec +++ b/openvswitch.spec @@ -9,15 +9,19 @@ Summary: Production Quality, Multilayer Open Virtual Switch URL: http://www.openvswitch.org/ Version: 2.12.4 License: ASL 2.0 and ISC -Release: 6 +Release: 7 Source: https://www.openvswitch.org/releases/openvswitch-%{version}.tar.gz Buildroot: /tmp/openvswitch-rpm Patch0000: 0000-openvswitch-add-stack-protector-strong.patch Patch0002: 0002-Remove-unsupported-permission-names.patch Patch0003: 0003-Fallback-to-read-proc-net-dev-on-linux.patch -Patch0004: backport-CVE-2022-4338.patch -Patch0005: backport-CVE-2023-1668.patch -Patch0006: backport-CVE-2023-5366.patch + +Patch6000: backport-CVE-2022-4338.patch +Patch6001: backport-CVE-2023-1668.patch +Patch6002: backport-CVE-2023-5366.patch +Patch6003: backport-0001-CVE-2023-3966.patch +Patch6004: backport-0002-CVE-2023-3966.patch +Patch6005: backport-0003-CVE-2023-3966.patch Patch9000: fix-selinux-err.patch @@ -294,6 +298,9 @@ exit 0 %doc README.rst NEWS rhel/README.RHEL.rst %changelog +* Tue Feb 20 2024 zhangpan - 2.12.4-7 +- fix CVE-2023-3966 + * Thu Dec 14 2023 zhouwenpei - 2.12.4-6 - Type:bugfix - Id:NA -- Gitee