pf: dedupe layer 4 protocol code in pf_setup_pdesc()
ClosedPublic
Actions

Authored by kp on Sep 12 2024, 11:57 AM.

Details

Reviewers

None

Group Reviewers

network
pfsense

Commits

rG905db4aa8877: pf: dedupe layer 4 protocol code in pf_setup_pdesc()

Summary

In pf_setup_pdesc() the code for analysing TCP and UDP headers was
the same for v4 and v6. Deduplicate by moving the protocol switch
after the address family switch.
ok henning@ claudio@

Obtained from: OpenBSD, bluhm <bluhm@openbsd.org>, 72cf18cc6e
Sponsored by: Rubicon Communications, LLC ("Netgate")

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 59450
Build 56337: arc lint + arc unit

Event Timeline

kp created this revision.Sep 12 2024, 11:57 AM

Herald added subscribers: vegeta_tuxpowered.net, glebius, melifaro and 2 others. · View Herald TranscriptSep 12 2024, 11:57 AM

kp requested review of this revision.Sep 12 2024, 11:57 AM

Harbormaster completed remote builds in B59450: Diff 143272.Sep 12 2024, 11:57 AM

kp added a child revision: D46648: pf: remove pointless CURVNET_SET.Sep 12 2024, 11:57 AM

I'm a little nervous about this change. It seems invalid combinations such as IPv4/ICMPv6 and IPv6/ICMP are now possible after the change. Do we have some pre-checks about those combinations ?

In D46647#1063679, @zlei wrote:

I'm a little nervous about this change. It seems invalid combinations such as IPv4/ICMPv6 and IPv6/ICMP are now possible after the change. Do we have some pre-checks about those combinations ?

There's an explicit test in pf_test() where we drop such packets. It's introduced in D46649. That's after pf_setup_pdesc(), but I'd argue that that's fine. pf_setup_pdesc() is only meant to set up a context struct, it doesn't have to do the full packet validation.

This revision was not accepted when it landed; it landed in state Needs Review.Sep 25 2024, 12:35 PM

Closed by commit rG905db4aa8877: pf: dedupe layer 4 protocol code in pf_setup_pdesc() (authored by kp). · Explain Why

This revision was automatically updated to reflect the committed changes.

kp added a commit: rG905db4aa8877: pf: dedupe layer 4 protocol code in pf_setup_pdesc().