Page MenuHomeFreeBSD

pf: Let pf_state_insert() handle redirect state conflicts
ClosedPublic

Authored by markj on Sep 9 2024, 6:38 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 16, 5:40 AM
Unknown Object (File)
Tue, Dec 3, 5:48 PM
Unknown Object (File)
Nov 21 2024, 1:20 PM
Unknown Object (File)
Nov 21 2024, 9:25 AM
Unknown Object (File)
Nov 20 2024, 11:32 PM
Unknown Object (File)
Nov 19 2024, 3:09 PM
Unknown Object (File)
Nov 8 2024, 11:20 AM
Unknown Object (File)
Oct 7 2024, 1:07 PM

Details

Summary

When handling a redirect state conflict, pf_get_translation() tries
modifying the source port to avoid it. If it fails to find a free port,
the translation is aborted.

Instead, if we fail to find a free source port, simply press on with the
original source port and let pf_state_insert() handle the conflict as it
pleases, rather than second-guessing what it will do. In particular,
pf_state_insert() has special handling for TCP connections in a terminal
state, and might succeed despite a state conflict.

Sponsored by: Klara, Inc.
Sponsored by: Modirum

Diff Detail

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

Event Timeline

markj requested review of this revision.Sep 9 2024, 6:38 PM

That seems mostly reasonable. The only possible concern I have is that we implicitly change the reason from PFRES_MAPFAILED to PFRES_STATEINS. That in turn affects the pf status counters, and could make debugging nat issues a bit harder.

It's not a huge concern, but on the other hand, this also seems like it'd only rarely matter as well. We'd have to fail to find a free port and then happen to be lucky enough that the used port happens to be in timeout state. On the third hand, it's somewhat plausible that this is a client that's re-using source ports and in that case it's much more likely that the existing state is actually for a closed connection.

This revision is now accepted and ready to land.Sep 10 2024, 9:48 AM