From: Alejandro Pacheco <[email protected]>
extract_l4() validates the ICMP header only for IPPROTO_ICMP over IPv4
and the ICMPv6 header only for IPPROTO_ICMPV6 over IPv6. Any other
combination, such as protocol 58 in an IPv4 header, is accepted as a
protocol without L4 keys and with no length check. The handler table
is indexed by the protocol number alone, though, so such a packet gets
the ICMP or ICMPv6 handler, whose valid_new() reads the type from an
L4 header that may not be in the packet.
Add l4_proto_get(), which picks a handler from both the address family
and the protocol in the same way extract_l4() does, and use it for every
handler lookup. Other combinations now use the generic handler, which
does not look at the L4 header.
Fixes: a27d70a89848 ("conntrack: add generic IP protocol support")
Assisted-by: claude-opus-5-5, Claude Code
Signed-off-by: Alejandro Pacheco <[email protected]>
---
lib/conntrack.c | 25 ++++++++++++++++++++-----
tests/ofproto-dpif.at | 41 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+), 5 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index d40870c63..a81072f33 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -157,6 +157,21 @@ expectation_clean(struct conntrack *ct, const struct
conn_key *parent_key);
static struct ct_l4_proto *l4_protos[UINT8_MAX + 1];
+/* Returns the l4 protocol handler for 'key'. ICMP and ICMPv6 handlers are
+ * used only with their own address family, since extract_l4() validates the
+ * ICMP headers only in that case. */
+static struct ct_l4_proto *
+l4_proto_get(const struct conn_key *key)
+{
+ if ((key->nw_proto == IPPROTO_ICMP
+ && key->dl_type != htons(ETH_TYPE_IP))
+ || (key->nw_proto == IPPROTO_ICMPV6
+ && key->dl_type != htons(ETH_TYPE_IPV6))) {
+ return &ct_proto_other;
+ }
+ return l4_protos[key->nw_proto];
+}
+
static void
handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
struct dp_packet *pkt, struct conn *ec, long long now,
@@ -2667,9 +2682,9 @@ conn_update(struct conntrack *ct, struct conn *conn,
struct dp_packet *pkt,
struct conn_lookup_ctx *ctx, long long now)
{
ovs_mutex_lock(&conn->lock);
- uint8_t nw_proto = conn->key_node[CT_DIR_FWD].key.nw_proto;
enum ct_update_res update_res =
- l4_protos[nw_proto]->conn_update(ct, conn, pkt, ctx->reply, now);
+ l4_proto_get(&conn->key_node[CT_DIR_FWD].key)->conn_update(
+ ct, conn, pkt, ctx->reply, now);
ovs_mutex_unlock(&conn->lock);
return update_res;
}
@@ -2703,14 +2718,14 @@ conn_expired(const struct conn *conn, long long now)
static bool
valid_new(struct dp_packet *pkt, struct conn_key *key)
{
- return l4_protos[key->nw_proto]->valid_new(pkt);
+ return l4_proto_get(key)->valid_new(pkt);
}
static struct conn *
new_conn(struct conntrack *ct, struct dp_packet *pkt, struct conn_key *key,
long long now, uint32_t tp_id)
{
- return l4_protos[key->nw_proto]->new_conn(ct, pkt, now, tp_id);
+ return l4_proto_get(key)->new_conn(ct, pkt, now, tp_id);
}
static void
@@ -2837,7 +2852,7 @@ conn_to_ct_dpif_entry(const struct conn *conn, struct
ct_dpif_entry *entry,
long long expiration = conn_expiration(conn) - now;
- struct ct_l4_proto *class = l4_protos[key->nw_proto];
+ struct ct_l4_proto *class = l4_proto_get(key);
if (class->conn_get_protoinfo) {
class->conn_get_protoinfo(conn, &entry->protoinfo);
}
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 8598ffa39..fb2bdcc56 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -12674,6 +12674,47 @@ NXST_FLOW reply:
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto-dpif - conntrack - icmp protocol of the other address
family])
+OVS_VSWITCHD_START
+
+add_of_ports br0 1 2
+
+AT_DATA([flows.txt], [dnl
+table=0,priority=10,ip,ct_state=-trk,action=ct(table=1)
+table=0,priority=10,ipv6,ct_state=-trk,action=ct(table=1)
+table=1,priority=10,ct_state=+trk+inv,action=drop
+table=1,priority=10,ct_state=+trk+new,action=output:2
+])
+
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+dnl IPv4 with protocol ICMPv6 and IPv6 with next header ICMP, both
+dnl without L4 payload, are tracked like any other protocol.
+m4_define([IP_ICMP6_PKT], m4_join([],
+dnl eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a,type=0x0800)
+[50540000000a5054000000090800],
+dnl ipv4(src=10.0.0.1,dst=10.0.0.2,proto=58,tot_len=20)
+[4500001400000000403a66ae0a0000010a000002]))
+
+m4_define([IP6_ICMP_PKT], m4_join([],
+dnl eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a,type=0x86dd)
+[50540000000a50540000000986dd],
+dnl ipv6(src=2001:db8::1,dst=2001:db8::2,nh=1,plen=0)
+[600000000000014020010db800000000000000000000000120010db8000000000000000000000002]))
+
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 IP_ICMP6_PKT])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 IP6_ICMP_PKT])
+
+AT_CHECK([ovs-appctl revalidator/purge])
+AT_CHECK([ovs-ofctl dump-flows br0 table=1 | ofctl_strip | sort], [0], [dnl
+ table=1, n_packets=2, n_bytes=88, priority=10,ct_state=+new+trk
actions=output:2
+ table=1, priority=10,ct_state=+inv+trk actions=drop
+NXST_FLOW reply:
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([ofproto-dpif - conntrack - zones])
OVS_VSWITCHD_START
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
