Hello,
This series fixes header length checks in the userspace datapath.
Version 1 went to the security team, who asked for it to be posted here
for review as one series.
Patches 1 to 10 are minimal fixes, one per bug, each with a test:
1 the ND option walk in packet_set_nd() and odp_set_nd()
2 the SCTP chunk header length check in conntrack
3 conntrack ICMP handlers picked by protocol number alone
4 the TCP and SCTP checksum update on a truncated quoted header
5-7 the GTP-U, ERSPAN and GRE tunnel pops
8 NSH set and pop
9 layer offsets and checksum offloads left by output truncation
10 set_arp() on a truncated ARP header
With AddressSanitizer, the test of each of patches 1 to 10 fails on the
current code and passes with its patch.
Patch 11 is hardening on top: it adds dp_packet_l3_hdr() and
dp_packet_l4_hdr(), which return a header only when the packet holds
it, and makes the L3 and L4 field setters use them. It is not needed
for backports.
The related testsuite sections pass with the whole series under
AddressSanitizer, and clang-analyze reports nothing new for any patch
of it.
Each patch mail carries its commit exactly as git format-patch wrote
it, as a plain-text attachment, with only the author line in its body,
because my mail setup rewraps message bodies. git am and patchwork
take each one as a single commit with the right author.
Please apply the patches as they are (git am), so that they keep their
author; change anything you like in review.
v3:
- Patch 1: assert the L4 header in the new check_set_nd() test, for
the clang-analyze warning the robot reported on v2.
- Patches 2 to 11 are unchanged, so the GTP-U extension header patch
posted on top of patch 5 still applies.
v2:
- Posted to ovs-dev as one series, as the security team asked.
- ND: fixed the loops in place, no new helper.
- set_arp(): new minimal fix (patch 10); the accessor patch is now
hardening only and comes last.
- Tests: raw hex packets with a comment per header,
OVS_WAIT_UNTIL_EQUAL, time/warp under AT_CHECK, add_of_ports.
- Shorter comments and commit messages.
Alejandro Pacheco (11):
packets: Fix ND option walk in packet_set_nd().
conntrack: Require a full chunk header in SCTP length check.
conntrack: Use ICMP handlers only for their own address family.
packets: Leave the checksum of a truncated TCP or SCTP header alone.
netdev-native-tnl: Check GTP-U header length on pop.
netdev-native-tnl: Check ERSPAN header length before reading it.
netdev-native-tnl: Check GRE base header length before reading it.
packets: Validate NSH header before set or pop.
dp-packet: Clear layer offsets and offloads on output truncation.
odp-execute: Check ARP header length in set_arp().
packets: Check header length in L3 and L4 field setters.
lib/conntrack.c | 27 +++-
lib/dp-packet.c | 56 ++++++++
lib/dp-packet.h | 34 ++++-
lib/dpif.c | 2 +-
lib/netdev-native-tnl.c | 56 +++++---
lib/odp-execute.c | 65 ++++++---
lib/packets.c | 161 +++++++++++++++-------
lib/packets.h | 27 ++++
tests/dpif-netdev.at | 64 +++++++++
tests/nsh.at | 36 +++++
tests/ofproto-dpif.at | 178 +++++++++++++++++++++++++
tests/test-packets.c | 244 ++++++++++++++++++++++++++++++++++
tests/tunnel-push-pop-ipv6.at | 14 ++
tests/tunnel-push-pop.at | 80 +++++++++++
14 files changed, 950 insertions(+), 94 deletions(-)
Alejandro Pacheco
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
