pf: Fix source node locking

Source nodes are created quite early in pf_create_state(), even before
the state is allocated, locked and inserted into its hash row. They are
prone to being freed by sou

pf: Fix source node locking

Source nodes are created quite early in pf_create_state(), even before
the state is allocated, locked and inserted into its hash row. They are
prone to being freed by source node killing or clearing ioctl while
pf_create_state() is still running.

The function pf_map_addr_sn() can be called in two very different paths.

One is for filter rules where it is called from
pf_create_state() after pf_insert_src_node(). In this case it is called
with a given source node and does not perform its own search and must
return the source node.

The other one is for NAT rules where it is called from
pf_get_translation() or its descendants. In this case it is called with
no known source node and performs its own search for source nodes. This
source node is then passed back to pf_create_state() without locking.

The states property of source node is increased in pf_find_src_node()
which allows for the counter to increase when a packet matches the NAT
rule but not a pass keep state rule.

The function pf_map_addr() operates on unlocked source node.

Modify pf_find_src_node() to return locked on source node found, so
that any subsequent operations can access the source node safely.

Move sn->states++ counter increase to pf_insert_src_node() to ensure
that it's called only from pf_create_state() and not from NAT ruleset
path, and have it increased only if the source node has really been
inserted or found, simplifying the cleanup.

Add locking in pf_src_connlimit() and pf_map_addr(). Sprinkle mutex
assertions in pf_map_addr().

Add a function pf_src_node_exists() to check a known source node is
still valid. Use it in pf_create_state() where it's impossible to hold
locks from pf_insert_src_node() because that would cause LoR (nodes
first, then state) against pf_src_connlimit() (state first, then node).

Don't propagate the source node found while parsing the NAT ruleset to
pf_create_state() because it must be found again and locked or created.

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

show more ...

pf: handle IPv6 fragmentation for route-to

If a fragmented IPv6 packet hits a route-to rule we have to first prevent
the pf_test(PF_OUT) check in pf_route6() from refragmenting (and calling
ip6_outp

pf: handle IPv6 fragmentation for route-to

If a fragmented IPv6 packet hits a route-to rule we have to first prevent
the pf_test(PF_OUT) check in pf_route6() from refragmenting (and calling
ip6_output()/ip6_forward()). We then have to refragment in pf_route6() and
transmit the packets on the route-to interface.

Split pf_refragment6() into two parts, the first to perform the refragmentation,
the second to call ip6_output()/ip6_forward() and call the former from
pf_route6().

Add a test case for route-to-ing fragmented IPv6 packets to verify this works
as expected.

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

show more ...

pf: Fix handling of v6 loopback connections with pf syncookies enabled

The SYN|ACK generated by pf needs to inherit M_LOOP from the original
SYN, otherwise it gets dropped by ip6_input().

Fix this

pf: Fix handling of v6 loopback connections with pf syncookies enabled

The SYN|ACK generated by pf needs to inherit M_LOOP from the original
SYN, otherwise it gets dropped by ip6_input().

Fix this by adding an mbuf_flags argument to pf_build_tcp() that can be
used to set both M_SKIP_FIREWALL and M_LOOP as needed. Set M_LOOP on
the output mbuf if it was generated in response to an mbuf with M_LOOP
set.

Add a regression test case. The v4 case had no problems, but the v6
case fails without this change.

Reviewed by: kp
MFC after: 1 month
Sponsored by: Klara, Inc.
Sponsored by: Zenarmor
Differential Revision: https://reviews.freebsd.org/D47257

show more ...

pf: move the mbuf into struct pf_pdesc too

As requested by henning, move the mbuf pointer into struct pf_pdesc.
Also sort pd to the beginning of the functions' parameter lists for
consistency.
ok he

pf: move the mbuf into struct pf_pdesc too

As requested by henning, move the mbuf pointer into struct pf_pdesc.
Also sort pd to the beginning of the functions' parameter lists for
consistency.
ok henning

Obtained from: OpenBSD, bluhm <bluhm@openbsd.org>, 776f210a75
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46941

show more ...

pf: put kif into struct pf_pdesc

Put kif and dir into pdesc an use this instead of passing the values
around. This is a mechanical change. Initialize pd2 and use it
where appropriate.
ok henning o

pf: put kif into struct pf_pdesc

Put kif and dir into pdesc an use this instead of passing the values
around. This is a mechanical change. Initialize pd2 and use it
where appropriate.
ok henning on an earlier version; ok mpf

Obtained from: OpenBSD, bluhm <bluhm@openbsd.org>, 47de5c193e
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46939

show more ...

pf: convert DIOCGETRULESET to netlink

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

pf: consolidate pf function parameters

Move off and hdrlen into pdesc
and change their type from int to u_int32_t. Do not pass struct
tcphdr *th and sa_family_t af, it is in pd anyway. Do not use

pf: consolidate pf function parameters

Move off and hdrlen into pdesc
and change their type from int to u_int32_t. Do not pass struct
tcphdr *th and sa_family_t af, it is in pd anyway. Do not use af
and pd->af intermixed, the latter makes clear where it comes from.
Do not calculate the packet length again if pd already has it. Use
pd2.off instead of off2.
go go go go don't stop henning@ mpf@

Obtained from: OpenBSD, bluhm <bluhm@openbsd.org>, 110e53770d
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46937

show more ...

pf: move pf_test_rule() out of pf_setup_pdesc()

Move the call to pf_test_rule() for fragments that have not been
reassembled by normalization from pf_setup_pdesc() to pf_test().
This simplifies the

pf: move pf_test_rule() out of pf_setup_pdesc()

Move the call to pf_test_rule() for fragments that have not been
reassembled by normalization from pf_setup_pdesc() to pf_test().
This simplifies the paramter list of pf_setup_pdesc() as it can
concentrate on its job filling the pf_pdesc struct.
ok henning mpf

Obtained from: OpenBSD, bluhm <bluhm@openbsd.org>, fb9fe53b92
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46935

show more ...

pf: remove the last hand-rolled IPv6 extension header loop

Replace the IPv6 header walking loop in pf_test_state_icmp() with
the common function pf_walk_header6(). For that, pf_walk_header6()
can n

pf: remove the last hand-rolled IPv6 extension header loop

Replace the IPv6 header walking loop in pf_test_state_icmp() with
the common function pf_walk_header6(). For that, pf_walk_header6()
can now extract both the information wether it is a fragment and
the final protocol if it is the first fragment. This allows to
match the icmp6 too big packet of a first fragment to the reassembled
packet's state. This is neccesary if a refragmented fragment is
to big for the Path-MTU.
Note that pd.proto contains the real protocol number for the first
fragment and IPPROTO_FRAGMENT for later fragments. pd.virtual_protocol
is set to PF_VPROTO_FRAGMENT for all fragments.
ok mcbride@

Obtained from: OpenBSD, bluhm <bluhm@openbsd.org>, 90b3c57e94
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46931

show more ...

pf: convert DIOCGETRULESETS to netlink

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

pf: add ttl to pf_pdesc

This simplifies the code a little, because we can now avoid looking at the
IP(v6) header at all in pf_test() itself.

We do have to move the Jumbogram check into pf_setup_pde

pf: add ttl to pf_pdesc

This simplifies the code a little, because we can now avoid looking at the
IP(v6) header at all in pf_test() itself.

We do have to move the Jumbogram check into pf_setup_pdesc as well.

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

show more ...

pf: reduce IPv6 header parsing code duplication

There were two loops in pf_setup_pdesc() and pf_normalize_ip6()
walking over the IPv6 header chain. Merge them into one loop,
adjust some length chec

pf: reduce IPv6 header parsing code duplication

There were two loops in pf_setup_pdesc() and pf_normalize_ip6()
walking over the IPv6 header chain. Merge them into one loop,
adjust some length checks and fix IPv6 jumbo option handling. Also
allow strange but legal IPv6 packets with plen=0 passing through
pf. IPv6 jumbo packets still get dropped.
testing dhill@; ok mcbride@ henning@

Obtained from: OpenBSD, bluhm <bluhm@openbsd.org>, d68283bbf0
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46925

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: Remove struct pf_pdesc->nat_rule

The variable struct pd->nat_rule is set only during rule evaluation, that
is only for the first packet of a connection. Use struct pf_kstate->nat_rule
instead.

pf: Remove struct pf_pdesc->nat_rule

The variable struct pd->nat_rule is set only during rule evaluation, that
is only for the first packet of a connection. Use struct pf_kstate->nat_rule
instead.

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

show more ...

pf: fold pf_test_fragment() into pf_test_rule()

Reduces code and fixes a bunch of bugs with fragment handling not being in sync
with the rest of the ruleset.

Much feedback from mpf, bluhm & markus

pf: fold pf_test_fragment() into pf_test_rule()

Reduces code and fixes a bunch of bugs with fragment handling not being in sync
with the rest of the ruleset.

Much feedback from mpf, bluhm & markus
Thanks to Tony Sarendal for help with testing

ok bluhm; various previous versions ok henning, claudio, mpf, markus

Note that while this changes the order of src addr/src port/dst addr/dst port
skips this doesn't actually affect the kernel/userspace ABI. The kernel always
recalculates skip steps. As a result we have to fix one of the pfctl parser
tests. Note that this is an order change that does not affect what packets are
acceppted or dropped.

Obtained from: OpenBSD, mcbride <mcbride@openbsd.org>, 04c69899a7
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46705

show more ...

pf: Split pf_map_addr()

Split pf_map_addr() into 2 functions:
- pf_map_addr() handles just the address mapping algorithms, it is used
in pf_route() and pf_route6() in case of stateless route-to, w

pf: Split pf_map_addr()

Split pf_map_addr() into 2 functions:
- pf_map_addr() handles just the address mapping algorithms, it is used
in pf_route() and pf_route6() in case of stateless route-to, where
source nodes would never be created
- pf_map_addr_sn() handles source nodes and calls pf_map_addr() for
address mapping algorightms, it is used everywhere else, like NAT
rules, which are always stateful

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

show more ...

pf: make pf_setup_pdesc() a static function

We only ever call it from pf_test(), and implement it in the same source file.

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

pf: make pf_setup_pdesc() a static function

We only ever call it from pf_test(), and implement it in the same source file.

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

show more ...

pf: rename the pf_pdesc field rh_cnt to badopts

It is also used for IPv4 options now.
ok mcbride@ henning@

Reviewed by: zlei
Obtained from: OpenBSD, bluhm <bluhm@openbsd.org>, 4fe6abd45f
Sponsored

pf: rename the pf_pdesc field rh_cnt to badopts

It is also used for IPv4 options now.
ok mcbride@ henning@

Reviewed by: zlei
Obtained from: OpenBSD, bluhm <bluhm@openbsd.org>, 4fe6abd45f
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46650

show more ...

pf: merge pf_test() and pf_test6()

Bye bye pf_test6(). Only one pf_test function for both IPv4 and v6.
The functions were 95% identical anyway.
OK bluhm@ mcbride@ and most probably henning@ as well

pf: merge pf_test() and pf_test6()

Bye bye pf_test6(). Only one pf_test function for both IPv4 and v6.
The functions were 95% identical anyway.
OK bluhm@ mcbride@ and most probably henning@ as well

Obtained from: OpenBSD, claudio <claudio@openbsd.org>, c8bc4f6e29
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46649

show more ...

pf: remove unused argument 'h' from various function

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

pf: merge pf_scrub_ip() and pf_scrub_ip6()

Merge pf_scrub_ip() and pf_scrub_ip6() into a single function. Call
pf_scrub with the right arugments in the rule case so that match
rules will work as ex

pf: merge pf_scrub_ip() and pf_scrub_ip6()

Merge pf_scrub_ip() and pf_scrub_ip6() into a single function. Call
pf_scrub with the right arugments in the rule case so that match
rules will work as expected.

OK henning@

Obtained from: OpenBSD, claudio <claudio@openbsd.org>, 48c45e6969
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46597

show more ...

pf: do not pass af to PFLOG_PACKET

Do not pass AF specific information to pf_test_rule() and PFLOG_PACKET()
because either the info is already available in struct pd or easy
to figure out. Makes pf

pf: do not pass af to PFLOG_PACKET

Do not pass AF specific information to pf_test_rule() and PFLOG_PACKET()
because either the info is already available in struct pd or easy
to figure out. Makes pf_test() and pf_test6() even more similar
(with the target to remove one of them in the near future).
OK henning@

Reviewed by: zlei
Obtained from: OpenBSD, claudio <claudio@openbsd.org>, 5480721ed1
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46596

show more ...

pf: exclude link local address from the dynamic interface address pool

so that rules like "pass out on vr1 inet6 nat-to (vr1)" won't map
to the non routable ipv6 link local address; with suggestions

pf: exclude link local address from the dynamic interface address pool

so that rules like "pass out on vr1 inet6 nat-to (vr1)" won't map
to the non routable ipv6 link local address; with suggestions and
ok claudio, henning

Reviewed by: zlei
Obtained from: OpenBSD, mikeb <mikeb@openbsd.org>, e41548933f
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46594

show more ...

pf: remove unneeded double pointer to pf_scrub_ip(6)()

pf_scrub_ip() does not modify the given mbuf pointer. So don't
pass a pointer to a pointer to make the code in pf_test() clearer.
ok henning@

pf: remove unneeded double pointer to pf_scrub_ip(6)()

pf_scrub_ip() does not modify the given mbuf pointer. So don't
pass a pointer to a pointer to make the code in pf_test() clearer.
ok henning@

Reviewed by: zlei
Obtained from: OpenBSD, bluhm <bluhm@openbsd.org>, aac78b59b9
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46593

show more ...

pf: factor out pf_setup_pdesc()

factor our the code to set up pf_pdesc, a central structure in pf carrying
information about the packet we're currently dealing with, into its own
function. ok ryan d

pf: factor out pf_setup_pdesc()

factor our the code to set up pf_pdesc, a central structure in pf carrying
information about the packet we're currently dealing with, into its own
function. ok ryan dlg and additional testing sthen

Obtained from: OpenBSD, henning <henning@openbsd.org>, c4202972a3
Obtained from: OpenBSD, claudio <claudio@openbsd.org>, 78d25123ea
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46586

show more ...