pf: add a dedicated pf pool for route options

As suggested by henning.
Which unbreaks ie route-to after the recent pf changes.

With much help debugging and pointing out of missing bits from claudio

pf: add a dedicated pf pool for route options

As suggested by henning.
Which unbreaks ie route-to after the recent pf changes.

With much help debugging and pointing out of missing bits from claudio@

ok claudio@ "looks good" henning@

Obtained from: OpenBSD, jsg <jsg@openbsd.org>, 7fa5c09028
Sponsored by: Rubicon Communications, LLC ("Netgate")

show more ...

pf: partially import OpenBSD's NAT rewrite

We won't follow this fully, because it involves breaking syntax changes
(removing nat/rdr rules and moving this functionality into regular rules) as
well a

pf: partially import OpenBSD's NAT rewrite

We won't follow this fully, because it involves breaking syntax changes
(removing nat/rdr rules and moving this functionality into regular rules) as
well as behaviour changes because NAT is now done after the rules evaluation,
rather than before it.

We import some related changes anyway, because it paves the way for nat64
support.
This change introduces a new pf_kpool in struct pf_krule, for nat. It is not yet
used (but will be for nat64) and renames the existing 'rpool' to 'rdr'.

Obtained from: OpenBSD, henning <henning@openbsd.org>, 0ef3d4febe
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D47783

show more ...

pf: Don't pfsync states with unrecoverable routing information

States created by route-to rules can't be trusted when received with
pfsync version 1301 as they lack the rt and rt_kif information. Th

pf: Don't pfsync states with unrecoverable routing information

States created by route-to rules can't be trusted when received with
pfsync version 1301 as they lack the rt and rt_kif information. They
are imported, though, and pf_route() function attempts to recover
the missing information for every forwarded packet.

Move the recovery operation to pfsync_state_import() so that it's
performed only once and if it's impossible don't import the state.
Add an additional check for cases when recovery might produce wrong
results.

Reviewed by: kp
Approved by: kp (mentor)
Sponsored by: InnoGames GmbH
Differential Revision: https://reviews.freebsd.org/D47906

show more ...

pf: Move route-to information to rule actions

Route-to redirection information (rt, rt_kif, rt_addr) can be considered an
action of a rule. This information is duplicated in struct pf_kstate which
m

pf: Move route-to information to rule actions

Route-to redirection information (rt, rt_kif, rt_addr) can be considered an
action of a rule. This information is duplicated in struct pf_kstate which
means that the pf_route() function must always figure out where to get this
information from: state for stateful forwarding, or rule for stateless.

Create the necessary members in struct pf_rule_action. Fill them in right after
parsing the ruleset, similar for how NAT redirection is applied right after
parsing the NAT ruleset. Remove the logic for finding the right source for
route-to redirection from pf_route().

As a bonus simplify pf_map_addr_sn() and source node handling. Both for the
NAT and the filter ruleset there is now only one path:
1. parse the rules
2. apply redirection either from an existing source node or by load balancing
for the last matching rule
3. create the source node using the redirection if the node does not yet exist

Reviewed by: kp
Approved by: kp (mentor)
Sponsored by: InnoGames GmbH
Differential Revision: https://reviews.freebsd.org/D47827

show more ...

pf: replace union pf_krule_ptr with struct pf_krule in in-kernel structs

There is no need for the union pf_krule_ptr for kernel-only structs like
pf_kstate and pf_ksrc_node. The rules are always acc

pf: replace union pf_krule_ptr with struct pf_krule in in-kernel structs

There is no need for the union pf_krule_ptr for kernel-only structs like
pf_kstate and pf_ksrc_node. The rules are always accessed by pointer. The rule
numbers are a leftover from using the same structure for pfctl(8) and pf(4).

Reviewed by: kp
Differential Revision: https://reviews.freebsd.org/D46868

show more ...

pf: vnet-ify pf_hashsize, pf_hashmask, pf_srchashsize and V_pf_srchashmask

These variables are tunables, so in principle they never change at runtime.
That would mean they don't need to be tracked p

pf: vnet-ify pf_hashsize, pf_hashmask, pf_srchashsize and V_pf_srchashmask

These variables are tunables, so in principle they never change at runtime.
That would mean they don't need to be tracked per-vnet.

However, they both can be decreased (back to their default values) if the
memory allocations for their respective tables fail, and these allocations are
per-vnet. That is, it's possible for a few vnets to be started and have the
tuned size for the hash and srchash tables only to have later vnets fail the
initial allocation and fall back to smaller allocations. That would confuse
the previously created vnets (because their actual table size and size/mask
variables would no longer match).

Avoid this by turning these into per-vnet variables.

MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")

show more ...

if_pfsync: lock buckets during pfsync_drop()

We failed to lock buckets while dropping messages, which could potentially lead
to crashes, and is the likely cause of panics like:

> pfsync_drop: st->s

if_pfsync: lock buckets during pfsync_drop()

We failed to lock buckets while dropping messages, which could potentially lead
to crashes, and is the likely cause of panics like:

> pfsync_drop: st->sync_state == q
> # pfsync_drop
> # pfsync_q_ins
> # pfsync_insert_state
> # pf_state_insert
> ...

Handle this by only handling the currently relevant (and this locked) bucket.
This ensures that the bucket is locked while we manipulate it.
While here also log slightly more information in the KASSERT().

MFC after: 2 weeks
Sponsored by: Orange Business Services

show more ...

net: Remove unneeded NULL check for the allocated ifnet

Change 4787572d0580 made if_alloc_domain() never fail, then also do the
wrappers if_alloc(), if_alloc_dev(), and if_gethandle().

No functiona

net: Remove unneeded NULL check for the allocated ifnet

Change 4787572d0580 made if_alloc_domain() never fail, then also do the
wrappers if_alloc(), if_alloc_dev(), and if_gethandle().

No functional change intended.

Reviewed by: kp, imp, glebius, stevek
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D45740

show more ...

pfsync: Correctly check if bpf peers are present

On creating the pfsync(4) interface, pfsync_clone_create() does an
unconditional bpfattach(). Use bpf_peers_present() which was introduced
in commit

pfsync: Correctly check if bpf peers are present

On creating the pfsync(4) interface, pfsync_clone_create() does an
unconditional bpfattach(). Use bpf_peers_present() which was introduced
in commit 16d878cc99ef [1] to check the presence of bpf peers.

This will save a little CPU cycles and memory usage when the
synchronisation interface is not configured and there is no bpf peers
present. There should be no functional change.

1. 16d878cc99ef Fix the following bpf(4) race condition which can result in a panic

Reviewed by: kp
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D45533

show more ...

pfsync: cope with multiple pending plus messages

It's possible for pfsync to add a plus message when one is already queued.
Append both, rather than overwriting the already pending one.

MFC after:

pfsync: cope with multiple pending plus messages

It's possible for pfsync to add a plus message when one is already queued.
Append both, rather than overwriting the already pending one.

MFC after: 1 week

show more ...

pfsync: fix use of invalidated stack variable

Calls to pfsync_send_plus() pass pointers to stack variables.
If pfsync_sendout() then fails it retains the pointer to these stack
variables, accesing t

pfsync: fix use of invalidated stack variable

Calls to pfsync_send_plus() pass pointers to stack variables.
If pfsync_sendout() then fails it retains the pointer to these stack
variables, accesing them later.

Allocate a buffer and copy the data instead, so that we can retain the
pointer safely.

Reported by: CI KASAN, markj
MFC after: 1 week

show more ...

pfsync: Fix offset calculation

Even though message version is automatically recognized and the top of
the struct is identical for different versions, when iterating over
multiple messages proper mes

pfsync: Fix offset calculation

Even though message version is automatically recognized and the top of
the struct is identical for different versions, when iterating over
multiple messages proper message length must be used. That's the length
of an union member for given version, not of the union itself.

Reviewed by: kp
Differential Revision: https://reviews.freebsd.org/D43862

show more ...

pf: store state creation/expiration timestamps with milisecond precision

The primary beneficiary is pflow(4), which expects milisecond precision
in timestamps.

Sponsored by: Rubicon Communications,

pf: store state creation/expiration timestamps with milisecond precision

The primary beneficiary is pflow(4), which expects milisecond precision
in timestamps.

Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D43112

show more ...

pfsync: Avoid transmitting uninitialized bytes in pfsync_sendout()

When IPv6 support was added to pfsync, PFSYNC_MINPKT increased such that
we always allocate enough space for either IPv4 or IPv6 he

pfsync: Avoid transmitting uninitialized bytes in pfsync_sendout()

When IPv6 support was added to pfsync, PFSYNC_MINPKT increased such that
we always allocate enough space for either IPv4 or IPv6 headers. IPv6
headers are 20 bytes larger than IPv4 headers. When pfsync_sendout()
does its thing, it ends up allocating enough space for either; thus when
transmitting an IPv4 packet, the last 20 bytes of the buffer are left
uninitialized.

Fix the problem by stashing the length in a local variable and adjusting
it depending on the address family in use.

While here, just zero the entire buffer in one go rather than being
careful to initialize each subheader. This seems simpler and less error
prone.

Reported by: KMSAN
Reviewed by: kp
Fixes: 6fc7fc2dbb2b ("pfsync: transport over IPv6")
MFC after: 3 days
Differential Revision: https://reviews.freebsd.org/D42461

show more ...

pfsync: fix state leak

If we receive a state with a route-to interface name set and we can't
find the interface we do not insert the state. However, in that case we
must still clean up the state (an

pfsync: fix state leak

If we receive a state with a route-to interface name set and we can't
find the interface we do not insert the state. However, in that case we
must still clean up the state (and state keys).
Do so, so we do not leak states.

Reviewed by: Kajetan Staszkiewicz <vegeta@tuxpowered.net>
MFC after: 3 days
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D41779

show more ...

sys: Remove $FreeBSD$: one-line .c pattern

Remove /^[\s*]*__FBSDID\("\$FreeBSD\$"\);?\s*\n/

pfsync: fix NOINET6 build

While here also fix a few minor style(9) issues.

pfsync: transport over IPv6

Implement pfsync over IPv6.

Submitted by: Luiz Amaral <email@luiz.eng.br>
Submitted by: Naman Sood <naman@freebsdfoundation.org>
Reviewed by: kp
Sponsored by: InnoGames

pfsync: transport over IPv6

Implement pfsync over IPv6.

Submitted by: Luiz Amaral <email@luiz.eng.br>
Submitted by: Naman Sood <naman@freebsdfoundation.org>
Reviewed by: kp
Sponsored by: InnoGames GmbH
Differential Revision: https://reviews.freebsd.org/D40102

show more ...

pf: Simplify rule actions logic

Actions applied to a processed packet come in case of stateless
firewalling from a rule or in case of statefull firewalling from a
state. The state obtains the action

pf: Simplify rule actions logic

Actions applied to a processed packet come in case of stateless
firewalling from a rule or in case of statefull firewalling from a
state. The state obtains the actions from a rule when it is created by a
rule or by pfsync. The logic for deciding if actions come from a rule or
a state is spread across many places in pf.

There already is struct pf_rule_actions in struct pf_pdesc and thus it
can be used as a central place for storing actions and their parameters.
OpenBSD does something similar: they also store the actions in struct
pf_pdesc and have no variables in pf_test() but they use separate
variables instead of a structure. By using struct pf_rule_actions we can
simplify the code even further. Applying of actions is done *only* in
pf_rule_to_actions() no matter if for the legacy scrub rules or for the
normal match / pass rules. The logic of choosing if rule or state
actions are used is applied only once in pf_test() by copying the whole
struct.

Reviewed by: kp
Sponsored by: InnoGames GmbH
Differential Revision: https://reviews.freebsd.org/D41009

show more ...

pf: fix build without VIMAGE

Remove the name conflict between the pfsync_defer_tmo variable and
function.

This worked fine in kernels with VIMAGE (the default), but not in those
without.

Reported

pf: fix build without VIMAGE

Remove the name conflict between the pfsync_defer_tmo variable and
function.

This worked fine in kernels with VIMAGE (the default), but not in those
without.

Reported by: des@
Sponsored by: Rubicon Communications, LLC ("Netgate")

show more ...

pf: allow defer timeout to be configured

Add the net.pfsync.defer_delay sysctl to allow the defer timeout (i.e.
how long pf holds onto packets waiting for the peer to ack the new
state) to be change

pf: allow defer timeout to be configured

Add the net.pfsync.defer_delay sysctl to allow the defer timeout (i.e.
how long pf holds onto packets waiting for the peer to ack the new
state) to be changed.

This is intended to make testing of the defer code more robust, by
allowing longer timeouts to mitigate scheduling/measurement jitter.

Sponsored by: Rubicon Communications, LLC ("Netgate")

show more ...

pf: make contents of struct pfsync_state configurable

Make struct pfsync_state contents configurable by sending out new
versions of the structure in separate subheader actions. Both old and
new vers

pf: make contents of struct pfsync_state configurable

Make struct pfsync_state contents configurable by sending out new
versions of the structure in separate subheader actions. Both old and
new version of struct pfsync_state can be understood, so replication of
states from a system running an older kernel is possible. The version
being sent out is configured using ifconfig pfsync0 … version XXXX. The
version is an user-friendly string - 1301 stands for FreeBSD 13.1 (I
have checked synchronization against a host running 13.1), 1400 stands
for 14.0.

A host running an older kernel will just ignore the messages and count
them as "packets discarded for bad action".

Reviewed by: kp
Sponsored by: InnoGames GmbH
Differential Revision: https://reviews.freebsd.org/D39392

show more ...

pfsync: Remove deletion of states using the full pfsync_state struct

State deletions are sent over pfsync using struct pfsync_del_c.

Remove the code for receiving state deletions using struct pfsyn

pfsync: Remove deletion of states using the full pfsync_state struct

State deletions are sent over pfsync using struct pfsync_del_c.

Remove the code for receiving state deletions using struct pfsync_state
as such deletions are never sent. Rename functions and constants so that
only the "compressed" versions remain.

Reviewed by: kp
Sponsored by: InnoGames GmbH
Differential Revision: https://reviews.freebsd.org/D40004

show more ...

spdx: The BSD-2-Clause-FreeBSD identifier is obsolete, drop -FreeBSD

The SPDX folks have obsoleted the BSD-2-Clause-FreeBSD identifier. Catch
up to that fact and revert to their recommended match of

spdx: The BSD-2-Clause-FreeBSD identifier is obsolete, drop -FreeBSD

The SPDX folks have obsoleted the BSD-2-Clause-FreeBSD identifier. Catch
up to that fact and revert to their recommended match of BSD-2-Clause.

Discussed with: pfg
MFC After: 3 days
Sponsored by: Netflix

show more ...

pf: remove NULL check before uma_zfree()

uma_zfree() can be called on a NULL pointer. Simplify the pf code a
little by removing the redundant checks.

Sponsored by: Rubicon Communications, LLC ("Net

pf: remove NULL check before uma_zfree()

uma_zfree() can be called on a NULL pointer. Simplify the pf code a
little by removing the redundant checks.

Sponsored by: Rubicon Communications, LLC ("Netgate")

show more ...