From: Alejandro Pacheco <[email protected]>
parse_nsh() rejects an NSH header that does not fit in the packet, but
the flow keeps dl_type NSH, so a set(nsh) action can still be executed
for it. odp_set_nsh() then rewrites the base header and the MD type 1
context without checking that they are in the packet. pop_nsh() also
trusts the header length from the packet.
Add nsh_hdr_is_valid() with the checks of parse_nsh() and use it in
odp_set_nsh() and pop_nsh(). odp_set_nsh() now takes the MD type from
the md_type byte as parse_nsh() does, and zeroes the key and mask, as
the action may not carry the MD type 1 context.
Fixes: 3d2fbd70bda5 ("userspace: Add support for NSH MD1 match fields")
Fixes: 1fc11c5948cf ("Generic encap and decap support for NSH")
Fixes: 9a180f2c002a ("NSH: Adjust NSH wire format to the latest IETF draft")
Assisted-by: claude-opus-5-5, Claude Code
Signed-off-by: Alejandro Pacheco <[email protected]>
---
lib/odp-execute.c | 13 ++++++++++---
lib/packets.c | 4 ++++
lib/packets.h | 27 +++++++++++++++++++++++++++
tests/nsh.at | 36 ++++++++++++++++++++++++++++++++++++
4 files changed, 77 insertions(+), 3 deletions(-)
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 3bec8ee00..a1f338271 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -390,16 +390,23 @@ odp_set_nd(struct dp_packet *packet, const struct
ovs_key_nd *key,
}
}
-/* Set the NSH header. Assumes the NSH header is present and matches the
- * MD format of the key. The slow path must take case of that. */
+/* Set the NSH header. Leaves the packet unchanged if it does not hold a valid
+ * NSH header. Only the MD type 1 context is rewritten. */
static void
odp_set_nsh(struct dp_packet *packet, const struct nlattr *a, bool has_mask)
{
struct ovs_key_nsh key, mask;
struct nsh_hdr *nsh = dp_packet_l3(packet);
- uint8_t mdtype = nsh_md_type(nsh);
+ uint8_t mdtype;
ovs_be32 path_hdr;
+ if (!nsh || !nsh_hdr_is_valid(nsh, dp_packet_l3_size(packet))) {
+ return;
+ }
+ mdtype = nsh->md_type;
+
+ memset(&key, 0, sizeof key);
+ memset(&mask, 0, sizeof mask);
if (has_mask) {
odp_nsh_key_from_attr(a, &key, &mask, NULL);
} else {
diff --git a/lib/packets.c b/lib/packets.c
index 435aa8f9a..b0090fc44 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -530,6 +530,10 @@ pop_nsh(struct dp_packet *packet)
uint32_t next_pt;
if (packet->packet_type == htonl(PT_NSH) && nsh) {
+ if (!nsh_hdr_is_valid(nsh, dp_packet_l3_size(packet))) {
+ return false;
+ }
+
switch (nsh->next_proto) {
case NSH_P_ETHERNET:
next_pt = PT_ETH;
diff --git a/lib/packets.h b/lib/packets.h
index ea0bda840..189c5e283 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -453,6 +453,33 @@ void push_eth(struct dp_packet *packet, const struct
eth_addr *dst,
const struct eth_addr *src);
void pop_eth(struct dp_packet *packet);
+/* Returns true if the 'size' bytes at 'nsh' start with an NSH header that
+ * flow parsing accepts: version 0, a length that fits within 'size' and, for
+ * MD type 1, the full fixed-length context. */
+static inline bool
+nsh_hdr_is_valid(const struct nsh_hdr *nsh, size_t size)
+{
+ size_t length;
+
+ if (size < NSH_BASE_HDR_LEN || nsh_get_ver(nsh) != 0) {
+ return false;
+ }
+
+ length = nsh_hdr_len(nsh);
+ if (length > size) {
+ return false;
+ }
+
+ switch (nsh->md_type) {
+ case NSH_M_TYPE1:
+ return length == NSH_M_TYPE1_LEN;
+ case NSH_M_TYPE2:
+ return length >= NSH_BASE_HDR_LEN;
+ default:
+ return true;
+ }
+}
+
void push_nsh(struct dp_packet *packet, const struct nsh_hdr *nsh_hdr_src);
bool pop_nsh(struct dp_packet *packet);
diff --git a/tests/nsh.at b/tests/nsh.at
index 2440addc5..4f689c456 100644
--- a/tests/nsh.at
+++ b/tests/nsh.at
@@ -86,6 +86,42 @@ Datapath actions:
set(nsh(flags=2,ttl=63,mdtype=1,np=3,spi=0x123456,si=254,c1=0x
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([nsh - set on truncated header])
+OVS_VSWITCHD_START
+add_of_ports --pcap br0 1 2
+
+AT_DATA([flows.txt], [dnl
+table=0,in_port=1,dl_type=0x894f,actions=set_field:0x123456->nsh_spi,2
+])
+AT_CHECK([ovs-ofctl -Oopenflow13 add-flows br0 flows.txt])
+
+dnl MD type 1 header with a length of 24 bytes, but only 12 bytes present.
+m4_define([NSH_SHORT_PKT], m4_join([],
+dnl eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:02,type=0x894f)
+[000000000002000000000001894f],
+dnl nsh(ttl=63,len=6,mdtype=1,np=3,spi=0x1,si=255)
+[0fc60103000001ff],
+dnl First 4 of the 16 bytes of MD type 1 context.
+[11223344]))
+
+dnl Unknown MD type 0x11 with a length of 8 bytes. Only the base header
+dnl is rewritten.
+m4_define([NSH_MD11_PKT], m4_join([],
+dnl eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:02,type=0x894f)
+[000000000002000000000001894f],
+dnl nsh(ttl=63,len=2,mdtype=0x11,np=3,spi=0xa,si=255)
+[0fc2110300000aff]))
+
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 NSH_SHORT_PKT])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 NSH_MD11_PKT])
+
+dnl The first header is not valid, so that packet is forwarded unmodified.
+OVS_WAIT_UNTIL_EQUAL([ovs-pcap p2-tx.pcap], [NSH_SHORT_PKT
+m4_join([], [000000000002000000000001894f], [0fc21103123456ff])])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
### -----------------------------------------------------------------
### NSH MD1 on Ethernet encapsulation over veth link
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
