State keys are trivially const in lookup routines, so annotate them as
such. No functional change intended.
Reviewed by: kp
MFC after: 1 week
Sponsored by: Klara, Inc.
Sponsored by: Modirum
Differential Revision: https://reviews.freebsd.org/D45671
Use roundup_pow_of_two in place of a local function that does the same thing.
Reviewed by: alc, markj
Differential Revision: https://reviews.freebsd.org/D45536
The machinery to support 64-bit counters even on 32-bit kernels had a
bug where it would unitentionally truncate the value back to 32-bits
when transferring to a new counter. This resulted in buggy be behavior
on 64-bit kernels as well.
Sponsored by: Rubicon Communications, LLC ("Netgate")
The data of a TCP packet must fit into the announced window, but this is not
required for the sequence number of the FIN. A packet with the FIN bit set and
containing data that fits exactly into the announced window was blocked. Our
stack generates such packets when the receive buffer size is set to 1024. Now
pf uses only the data lenght for window comparison.
OK henning@
Obtained From: OpenBSD
Sponsored by: Rubicon Communications, LLC ("Netgate")
pf was setting max_win to 0 and discarded retransmitted SYN-ACK segments without
wscale if the original SYN contained a wscale option. with gerhard@, ok
henning@
Obtained From: OpenBSD
Sponsored by: Rubicon Communications, LLC ("Netgate")
In several places, a loop tests for powers of two, or iterates through
powers of two. In those places, replace the loop with an invocation
of fls or ilog2 without changing the meaning of the code.
Reviewed by: alc, markj, kib, np, erj, avg (previous version)
Differential Revision: https://reviews.freebsd.org/D45494
On creating the pfsync(4) interface, pfsync_clone_create() does an
unconditional bpfattach(). Use bpf_peers_present() which was introduced
in commit 16d878cc99 [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. 16d878cc99 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
On creating the pflog(4) interface, pflog_clone_create() does an
unconditional bpfattach(). Use bpf_peers_present() which was introduced
in commit 16d878cc99 [1] to check the presence of bpf peers.
This will save a little CPU cycles when no bpf peers present. There
should be no functional change.
1. 16d878cc99 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/D45532
Add probe points to allow us to probe when we add or remove multihome addresses.
Example use:
pf:sctp:multihome:add
{
printf("Add tag %x address %x", arg0,
((struct pf_sctp_source *)arg1)->addr.v4.s_addr);
}
pf:sctp:multihome:remove
{
printf("Remove tag %x address %x", arg0,
((struct pf_sctp_source *)arg2)->addr.v4.s_addr);
}
MFC after: 1 week
Sponsored by: Orange Business Services
Use the v4/v6 union members rather than the uint32_t ones.
Export IN_ARE_MASKED_ADDR_EQUAL() in in_var.h and use it (and its IPv6
equivalent) for masked comparisons rather than hand-rolled code.
Event: Kitchener-Waterloo Hackathon 202406
It casts in_addr to pf_addr, which is smaller, so this isn't quite right.
Replace it with a function that will only read the actual address.
Reported by: CheriBSD
Event: Kitchener-Waterloo Hackathon 202406
The timeout array in struct pf_rule has PFTM_OLD_MAX entries, the one in
struct pf_krule has PFTM_MAX entries (and PFTM_MAX > PFTM_OLD_MAX).
Use the smaller of the sizes when copying.
Reported by: CheriBSD
MFC after: 1 week
Event: Kitchener-Waterloo Hackathon 202406
777a4702c changed how we copy out the anchor_call string, and
incorrectly limited it to 8 (4 on 32-bit systems) bytes. Fix that so we
get the full anchor path, rather than just the first few characters.
PR: 279225
Sponsored by: Rubicon Communications, LLC ("Netgate")
Users have reported crashes in pf_test_state_udp() where at least one state key
is NULL.
That suggests that pf_detach_state() ran concurrently with pf_test_state_udp().
pf_test_state_udp() holds the state lock (aka the id lock), but
pf_detach_state() does not.
The intent is that detached states are not returned by STATE_LOOKUP/
pf_find_state(), as the state's timeout is set to PFTM_UNLINKED and thus
pf_find_state() does not find the state.
There are other paths to pf_detach_state() (outside of pf_unlink_state())
though, where we did not set the timeout to PFTM_UNLINKED. Fix those, and assert
that the timeout is set correctly when we enter pf_detach_state().
MFC after: 1 week
See also: https://redmine.pfsense.org/issues/15413
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D45101
It's possible for states to be cleaned up (through pf_detach_state()) that
have not been fully attached. For example if there's an ID conflict during
pf_state_insert().
pflow exports states from pf_detach_state(), so it can get called on such
states, but did not account for this and could end up dereferencing a NULL
state key.
Check for this in export_pflow() and do not export unattached states.
See also: https://redmine.pfsense.org/issues/15446
Sponsored by: Rubicon Communications, LLC ("Netgate")
Introduce pfctl_get_status_h() because we need the pfctl_handle. In this variant
use netlink to obtain the information.
Sponsored by: Rubicon Communications, LLC ("Netgate")
pf.ko, when built as a module without 'options INET' but with 'options
VIMAGE', won't load:
link_elf_obj: symbol vnet_entry_in_loopback_mask undefined
This is because it uses IN_LOOPBACK(), which in the VIMAGE case uses
INET-specific symbols.
Fix by making this check conditional on #ifdef INET.
Reviewed by: imp
Pull Request: https://github.com/freebsd/freebsd-src/pull/1157
If we redirect a packet to localhost and it gets dummynet'd it may be
re-injected later (e.g. when delayed) which means it will be passed
through ip_input() again. ip_input() will then reject the packet because
it's directed to the loopback address, but did not arrive on a loopback
interface.
Fix this by having pf set the rcvif to V_iflo if we redirect to
loopback.
See also: https://redmine.pfsense.org/issues/15363
Sponsored by: Rubicon Communications, LLC ("Netgate")
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
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
If we fragment the packet in pf_route() the first transmitted packet
will free the pf_mtag we have stored in pf_pdesc (pd). Ensure we
update that pointer for every packet to avoid using a freed pointer in
pf_dummynet_route().
Reported by: CI KASAN, markj
MFC after: 1 week
While here also add a basic test case for it.
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D44368
Ensure that we pick the correct dummynet pipe (i.e. forward vs. reverse
direction) when applying route-to.
We mark the processing as outbound so that dummynet will re-inject in
the correct phase of processing after it's done with the packet, but
that will cause us to pick the wrong pipe number. Reverse them so that
the incorrect decision ends up picking the correct pipe.
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D44366
In some setups we end up with multiple states created for a single
packet, which in turn can mean we run the packet through dummynet
multiple times. That's not expected or intended. Mark each packet when
it goes through dummynet, and do not pass packet through dummynet if
they're marked as having already passed through.
See also: https://redmine.pfsense.org/issues/14854
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D44365
On reply-to we don't know what interface to bind to when we create
the state. Create any reply-to state as floating, but bind to the
appropriate interface once we're handling the reply.
See also: https://redmine.pfsense.org/issues/15220
Sponsored by: Rubicon Communications, LLC ("Netgate")
When we handle a packet via route-to (i.e. pf_route6()) we still need to
verify the MTU. However, we only run that check in the forwarding case.
Set the PFIL_FWD tag when running the pf_test6(PF_OUT) check from
pf_route6(). We are in fact forwarding, so should call the test function
as such. This will cause us to run the MTU check, and generate an ICMP6
packet-too-big error when required.
See also: 54c62e3e5d
See also: f1c0030bb0
See also: https://redmine.pfsense.org/issues/14290
Sponsored by: Rubicon Communications, LLC ("Netgate")
When a packet matches an existing dynamic rule for a keep-state rule,
the matching engine advances the "instruction pointer" to the action
portion of the rule skipping over the match conditions. However, the
code was merely breaking out of the switch statement rather than doing
a continue, so the remainder of the loop body after the switch was
still executed. If the first action opcode contains an F_NOT but not
an F_OR (such as an "untag" action), then match is toggled to 0, and
the code exits the inner loop via a break which aborts processing of
the actions.
To fix, just use a continue instead of a break.
PR: 276732
Reviewed by: jhb, ae
MFC after: 2 weeks
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
The redirection pool stored in r->rpool.cur is used for loadbalancing
and cur can change whenever loadbalancing happens, which is for every
new connection. Therefore it can't be trusted outside of pf_map_addr()
and the r->rpool->mtx mutex. After evaluating the ruleset, loadbalancing
decission is made in pf_map_addr() called from within pf_create_state()
and stored in the state itself.
This patch modifies BOUND_IFACE() so that it only uses the information
already stored in the state which has been obtained in a way which
respects the r->rpool->mtx mutex.
Reviewed by: kp
Differential Revision: https://reviews.freebsd.org/D43741
The redirection pool stored in r->rpool.cur is used for loadbalancing
and cur can change whenever loadbalancing happens, which is for every
new connection. Therefore it can't be trusted outside of pf_map_addr()
and the r->rpool->mtx mutex. After evaluating the ruleset, loadbalancing
decission is made in pf_map_addr() called from within pf_create_state()
and stored in the state itself.
This patch modifies BOUND_IFACE() so that it only uses the information
already stored in the state which has been obtained in a way which
respects the r->rpool->mtx mutex.
Reviewed by: kp
Differential Revision: https://reviews.freebsd.org/D43741
If we apply a route-to to an inbound packet pf_route() may hand that
packet over to dummynet. Dummynet may then delay the packet, and later
re-inject it. This re-injection (in dummynet_send()) needs to know
if the packet was inbound or outbound, to call the correct path for
continued processing.
That's done based on the pf_pdesc we pass along (through
pf_dummynet_route() and pf_pdesc_to_dnflow()). In the case of pf_route()
on inbound packets that may be wrong, because we're called in the input
path, and didn't update pf_pdesc->dir.
This can manifest in issues with fragmented packets. For example, a
fragmented packet will be re-fragmented in pf_route(), and if dummynet
makes different decisions for some of the fragments (that is, it delays
some and allows others to pass through directly) this will break.
The packets that pass through dummynet without delay will be transmitted
correctly (through the ifp->if_output() call in pf_route()), but
the delayed packets will be re-injected in the input path (and not
the output path, as they should be). These packets will pass through
pf_test(PF_IN) as they're tagged PF_MTAG_FLAG_DUMMYNET. However,
this tag is then removed and the packet will be routed and enter
pf_test(PF_OUT) where pf_reassemble() will hold them indefinitely
(as some fragments have been transmitted directly, and will never hit
pf_test(PF_OUT)).
The fix is simple: we must update pf_pfdesc->dir to PF_OUT before we
pass the packet to dummynet.
See also: https://redmine.pfsense.org/issues/15156
Reviewed by: rcm
Sponsored by: Rubicon Communications, LLC ("Netgate")
When we route-to the state should be bound to the route-to interface,
not the default route interface. However, we should only do so for
outbound traffic, because inbound traffic should bind on the arriving
interface, not the one we eventually transmit on.
Explicitly check for this in BOUND_IFACE().
We must also extend pf_find_state(), because subsequent packets within
the established state will attempt to match the original interface, not
the route-to interface.
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D43589
pfil hooks (i.e. firewalls) may pass, modify or free the mbuf passed
to them. (E.g. when rejecting a packet, or when gathering up packets
for reassembly).
If the hook returns PFIL_PASS the mbuf must still be present. Assert
this in pfil_mem_common() and ensure that ipfilter follows this
convention. pf and ipfw already did.
Similarly, if the hook returns PFIL_DROPPED or PFIL_CONSUMED the mbuf
must have been freed (or now be owned by the firewall for further
processing, like packet scheduling or reassembly).
This allows us to remove a few extraneous NULL checks.
Suggested by: tuexen
Reviewed by: tuexen, zlei
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D43617
Introduce a verbose output mode to pflowctl, and expose the status of
the socket to userspace. This can be helpful in debugging configuration
errors.
Sponsored by: Rubicon Communications, LLC ("Netgate")
When the packets are generated locally (i.e. PFIL_FWD is not set) we
might generate overly large packets and rely on the NIC to fragment it
for us. In that case we'd reject a valid packet.
Reported by: Herbert J. Skuhra <herbert@gojira.at>
Tested by: Herbert J. Skuhra <herbert@gojira.at>
Fixes: 54c62e3e5d
Sponsored by: Rubicon Communications, LLC ("Netgate")
