Page MenuHomeFreeBSD

pf: Add modern NAT syntax
AbandonedPublic

Authored by kp on Mar 3 2025, 5:09 PM.
Tags
None
Referenced Files
F115799463: D49221.diff
Mon, Apr 28, 8:17 PM
F115799133: D49221.diff
Mon, Apr 28, 8:08 PM
F115792633: D49221.diff
Mon, Apr 28, 6:07 PM
Unknown Object (File)
Mon, Apr 21, 10:23 PM
Unknown Object (File)
Tue, Apr 8, 9:41 PM
Unknown Object (File)
Sun, Apr 6, 12:45 AM
Unknown Object (File)
Sat, Apr 5, 11:28 AM
Unknown Object (File)
Mar 26 2025, 11:19 PM

Details

Summary

Now that pfct has separate functions for parsing redirection pools and
ports, we can finally add support for nat-to and rdr-to filter_opts.
NAT and RDR actions are marked by having the respective pools filled in.

Function pf_rule_apply_nat() is responsible for both NAT/RDR and af-to
address translations. It is called both for match rules and the final
pass rule.

Use FreeBSD's original address translation code by splitting it into
pf_translate_compat(). Call this function for old-style NAT ruleset
and for modern NAT rules via pf_rule_apply_nat().

Initialize pfctl_rule's redirection pools on rule allocation, also for
code paths not using expand_rule(), so that they can be safely checked
for being empty in filter_consistent().

Move map-e NAT test to nat.sh for convenience, duplicate critical NAT
tests into _compat (for old-style NAT ruleset) and _pass (for match/
pass) variants.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

vegeta_tuxpowered.net changed the visibility from "vegeta_tuxpowered.net (Kajetan Staszkiewicz)" to "Public (No Login Required)".

I think we're missing the associated man page updates.

I'm going to revisit this tomorrow, but I've not seen any issues yet.

sbin/pfctl/parse.y
964

Is it worth factoring this out into a pfctl_init_rule() or something?

I have re-enabled OpenBSD tests. Some of them required modification to resolve port numbers to service names, include the "keep state" keyword, insert commas between table members, not auto-expand redirection table-like syntax into tables.

Looks good. It does still need the man page updates. We can probably stea^W borrow those from OpenBSD.

sys/netpfil/pf/pf.c
5998

Yeah, I think that makes sense. It'd save us doing the check in multiple places.
(There.s another one like this in pf_test_rule()).

sys/netpfil/pf/pf_lb.c
809

uint8_t in new code.

vegeta_tuxpowered.net added inline comments.
sbin/pfctl/parse.y
964

Yeah, why not.

vegeta_tuxpowered.net marked an inline comment as done.
vegeta_tuxpowered.net edited the summary of this revision. (Show Details)

Added binat support, modified pf.conf(5) man page to cover the new syntax, re-enabled a few more original OpenBSD pfctl tests.

I think we're about ready to have users try to break this.
Please fix the man page typo and date, and then it's approved to commit.

The other remarks for pf_translate_compat() can be handled in a follow-up commit.

share/man/man5/pf.conf.5
30 ↗(On Diff #154031)

Don't forget to bump the date when you commit.

1504 ↗(On Diff #154031)

loqok typo.

sys/netpfil/pf/pf.c
6364

If we use pd->sport/pd->dport we ought to be able to fold the UDP and TCP cases into one. It'd be fine to do that in a follow up commit too.

6424

I think we're missing the 'rewrite++' here.
We don't expect to change SCTP port numbers, but we're doing the work anyway. And that'd also let us fold it into the TCP/UDP code block more easily.

Oh, and I'd prefix the title with 'pf:' rather than 'pfctl:', because it includes kernel changes as well as pfctl changes.

vegeta_tuxpowered.net retitled this revision from pfctl: Add modern NAT syntax to pf: Add modern NAT syntax.Sun, Apr 27, 6:58 PM
vegeta_tuxpowered.net marked 2 inline comments as done.

pf.conf(5): fixed a typo, updated date.

LGTM. Ship it!
Approved. Commit it before I import more patches and force you to rebase again ;)

This revision is now accepted and ready to land.Mon, Apr 28, 2:13 PM
kp edited reviewers, added: vegeta_tuxpowered.net; removed: kp.

Commandeer to close.

This revision now requires review to proceed.Tue, Apr 29, 10:01 AM

Committed as https://cgit.freebsd.org/src/commit/?id=e0fe26691fc98a16cdda9d4f4beea9c5698ac64a

(Presumably the hook failed to fire because the title did not match. Bad Phabricator, no cookie for you.)