Page MenuHomeFreeBSD

Fix IPv6 flow label match in ipfw
ClosedPublic

Authored by lytboris_gmail.com on Thu, May 7, 4:23 PM.
Tags
None
Referenced Files
F157411548: D56869.id177622.diff
Thu, May 21, 3:05 AM
F157378723: D56869.diff
Wed, May 20, 7:34 PM
Unknown Object (File)
Tue, May 19, 12:26 PM
Unknown Object (File)
Mon, May 18, 7:30 AM
Unknown Object (File)
Sun, May 17, 5:37 AM
Unknown Object (File)
Sat, May 16, 4:43 PM
Unknown Object (File)
Wed, May 13, 9:38 PM
Unknown Object (File)
Wed, May 13, 12:31 PM

Details

Summary

Address a number of IPv6 flow label bugs scattered across different parts of ipfw:

  • kernel module: IPV6_FLOWLABEL_MASK is not being applied before comparison in flow6id_match() so flow-id opcode never matches a flow label alone (one need to take protocol version and traffic class into account)
  • kernel module: off-by-one bug leading to out-of-bounds read
  • sbin/ipfw: flow-id opcode allows just ip6 as a proto while ipv6-icmp, tcp, udp should be fine too.
Test Plan

ipv6-flow-id.sh test is provided

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

pouria added subscribers: ziaee, pouria.

I'm not an expert on IPFW. But the code, and logic LGTM.

tests/sys/netpfil/ipfw/ipv6-flow-id.sh
2–28

Please use new format.
recently, @ziaee updated the preferred format:
https://docs.freebsd.org/en/articles/license-guide/

Even that - at the first line is no longer required.

43

Could be nice to use ifconfig -j instead. Of course, I understand this pattern is repeated across the ipfw tests.

tests/sys/netpfil/ipfw/lookup.sh
59 ↗(On Diff #177369)

This is not really related to this change.
I suggest to separate it into another commit/review.

144 ↗(On Diff #177369)

This is not really related to this change.
I suggest to separate it into another commit/review.

This revision is now accepted and ready to land.Thu, May 7, 7:22 PM
This revision now requires review to proceed.Fri, May 8, 5:43 AM
This revision was not accepted when it landed; it landed in state Needs Review.Tue, May 12, 7:49 AM
This revision was automatically updated to reflect the committed changes.