packages/openvswitch
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix CVE-2023-3966

Browse Source 
(cherry picked from commit c99552111ab98cfd61d0714873fed83d8780dd00)
This commit is contained in:
zppzhangpan 2024-02-20 16:23:31 +08:00 committed by openeuler-sync-bot
parent b4a471b15e
commit d33bacf764
4 changed files with 399 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

160
backport-0001-CVE-2023-3966.patch Normal file
View File

@ -0,0 +1,160 @@
From 87f191a3a398ae495fa55d70ca5fe5d813bd284f Mon Sep 17 00:00:00 2001
From: Ilya Maximets <i.maximets@ovn.org>
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 <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
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

90
backport-0002-CVE-2023-3966.patch Normal file
View File

@ -0,0 +1,90 @@
From 6f322ccf8a9989905b7c29420239d5f6d81f0002 Mon Sep 17 00:00:00 2001
From: Ilya Maximets <i.maximets@ovn.org>
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 <echaudro@redhat.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
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

138
backport-0003-CVE-2023-3966.patch Normal file
View File

@ -0,0 +1,138 @@
From b8657dada9641fbd2bd3a3f882e0862448d60910 Mon Sep 17 00:00:00 2001
From: Timothy Redaelli <tredaelli@redhat.com>
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 <hakhande@redhat.com>
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
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

15
openvswitch.spec
View File

@ -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 <zhangpan103@h-pattners.com> - 2.12.4-7
- fix CVE-2023-3966
* Thu Dec 14 2023 zhouwenpei <zhouwenpei1@h-partners.com> - 2.12.4-6
- Type:bugfix
- Id:NA