Age | Commit message (Collapse) | Author |
|
retrieve rules from kernel. The current implementation requires
like O((n^2)/2) operation to read the complete rule set, because
each DIOCGETRULE operation must iterate over previous n
rules to find (n + 1)-th rule to read.
To address the issue diff introduces a pf_trans structure to keep
pointer to next rule to read, thus reading process does not need
to iterate from beginning of rule set to reach the next rule.
All transactions opened by process get closed either when process
is done (reads all rules) or when /dev/pf device is closed.
the diff also comes with lots of improvements from dlg@ and kn@
OK dlg@, kn@
|
|
This fixes a few KNF issues and ugly line wrapping by using a local
version of nitems(); fix two bsearch() on top.
ok claudio
|
|
fit into u_int8_t. Issue has been noticed and kindly reported by
amalinin _at_ bh0.amt.ru via bugs@.
OK bluhm@
|
|
On amd64 stack overflows for anchor rule with depth ~30. The tricky
thing is the 'safe' depth varies depending on kind of packet processed
by pf_match_rule(). For example for local outbound TCP packet stack
overflows when recursion if pf_match_rule() reaches depth 24.
Instead of lowering PF_ANCHOR_STACK_MAX to 20 and hoping it will
be enough on all platforms and for all packets I'd like to stop
calling pf_match_rule() recursively. This commit brings back
pf_anchor_stackframe array we used to have back in 2017. It also
revives patrick@'s idea to pre-allocate stack frame arrays
from per-cpu.
OK kn@
|
|
and interface groups are reported. The bug allowed
to enumerate the first 64 interfaces only.
The issue has been noticed and bug kindly reported
by Olivier Croquin.
OK kn@
|
|
"pfctl -s rules" omits expired rules but print newlines for them.
"pfctl -s rules -v" omits expired rules but print their stats.
Add the existing skip check to those two missing print logic places such
that expired rules won't cause any output at all, as expected, unless
debug ("-g") or more verbose ("-vv") mode are given, as documented.
OK sashan
|
|
- use imperative tense in the pf.conf(5) "once" part
- leave printing implementation details to pfctl(8)'s "-s rules" part
- use more markup
- debug mode also prints expired rules
OK jmc sashan
|
|
- show -t with -T in options list
- sort the -T commands
- small text/formatting tweaks
ok sthen
ok kn on an earlier version
|
|
let packet to mark 'once' rule as expired. The rule
will be removed by pfctl(8) when rules are updated.
OK kn@
|
|
reported to FreeBSD by Franco Fichtner; from Kristof Provost
|
|
Also memset the pfctl struct in pfctl_reset.
OK jan@
|
|
|
|
It was possible to exhaust kernel memory by repeatedly calling
pfioctl DIOCXBEGIN with different anchor names.
OK bluhm@
Reported-by: syzbot+9dd98cbce69e26f0fc11@syzkaller.appspotmail.com
|
|
apostrophe.
|
|
ok dlg@ guenther@
|
|
OK bluhm@
|
|
Issue reported by Kristof Provost from FreeBSD.
[ https://reviews.freebsd.org/D32488 ]
In order to fix the issue we must delay '$nr' macro
expansion after optimizer collapses ruleset.
OK kn@
|
|
functions that take "char *" arguments. Where such chars are
assigned to int or passed to ctype functions, explicitly cast them
to unsigned char.
For OpenBSD's clang, -Wpointer-sign has been disabled by default,
but when the parse.y code was built elsewhere, the compiler would
complain.
With help from millert@
ok benno@ deraadt@
|
|
this is a significant (and breaking) reworking of the policy based
routing that pf can do. the intention is to make it as easy as
nat/rdr to use, and more robust when it's operating.
the main reasons for this change are:
- route-to, reply-to, and dup-to do not work with pfsync
this is because the information about where to route-to is stored in
rules, and it is hard to have a ruleset synced between firewalls,
and impossible to have them synced 100% of the time.
- i can make my boxes panic in certain situations using route-to
yeah...
- the configuration and syntax for route-to rules are confusing.
the argument to route-to and co is an interace name with an optional
ip address. there are several problems with this. one is that people
tend to think about routing as sending packets to peers by their
address, not by the interface they're reachable on. another is that
we currently have no way to synchronise interface topology information
between firewalls, so using an interface to say where packets go
means we can't do failover of these states with pfsync. another
is that a change in routing topology means a host may become
reachable over a different interface. tying routing policy to
interfaces gets in the way of failover and load balancing.
this change does the following:
- stores the route info in the state instead of the pf rule
this allows route-to to keep working when the ruleset changes, and
allows route-to info to be sent over pfsync. there's enough spare bits
in pfsync messages that the protocol doesnt break.
the caveat is that route-to becomes tied to pass rules that create
state, like rdr-to and nat-to.
- the argument to route-to etc is a destination ip address
it's not limited to a next-hop address (thought a next-hop can be a
destination address). this allows for the failover and load balancing
referred to above.
- deprecates the address@interface host syntax in pfctl
because routing is done entirely by IPs, the interface is derived from
the route lookup, not pf. any attempt to use the @interface syntax
will fail now in all contexts.
there's enthusiasm from proctor@ jmatthew@ and others
ok sashan@ bluhm@
|
|
the rule did not specify it. Check the option again for the log
rule in case another rule has triggered a socket lookup. Remove
logopt group, it is not documented and cannot work as struct pfloghdr
does not contain a gid. Rename PF_LOG_SOCKET_LOOKUP to PF_LOG_USER
to express what it does. The lookup involved is only an implemntation
detail.
OK kn@ sashan@ mvs@
|
|
before accessing anything in ifa_addr.
ok claudio@
|
|
This makes pfctl(8) detect bogus ranges (with and without `-n') before
loading the ruleset and completes the previous commit.
OK sashan sthen
|
|
Ranges where the left boundary is bigger than the right one are always bogus
as they work like `port any' (`port 34<>12' means "all ports") or in way
that inverts the rule's action (`pass ... port 34:12' means "pass no port at
all").
Add checks for all ranges and invalidate those that yield no or all ports.
For this to work on redirections, make pfctl(8) pass the range's type,
otherwise boundary including ranges are not detected as such; that is to
say, `struct pf_pool's `port_op' member was unused in the kernel so far.
`rdr-to' rules with invalid ranges could panic the kernel when hit.
Reported-by: syzbot+9c309db201f06e39a8ba@syzkaller.appspotmail.com
OK sashan
|
|
issue noticed by sthen@. fix discussed with bluhm@ and procter@
OK bluhm@, kn@, procter@
|
|
in fact modify the string buffer.
ok kn@ sashan@
cVS: ----------------------------------------------------------------------
|
|
Unlike "... rtable N", pf.conf(5)'s "on rdomain N" does not alter packet
state and will always work no matter if rdomain N currently exists or not,
i.e. the rule "pass on rdomain 42" will simply match (and pass) packets if
rdomain 42 exists, and it will simply not match (neither pass nor block)
packets if 42 does not exist.
There's no need to reload the ruleset whenever routing domains are created
or deleted, which can already be observed now by creating an rdomain,
loading rules referencing it and deleting the same rdomain immediately
afterwards: pf will continue to work as expected.
Relax both pfctl(8)'s parser check as well as pf(4)'s copyin routine to
accept any valid routing domain ID without expecting it to exist at the time
of ruleset creation - this lifts the requirement to create rdomains before
referencing them in pf.conf while keeping pf behaviour unchanged.
Prompted by yasuoka's recent pfctl parse.y r1.702 commit requiring an rtable
to exist upon ruleset creation.
Discussed with claudio and bluhm at k2k20.
Feedback sashan
OK sashan yasouka claudio
|
|
ok kn
|
|
config. work with and diff from kn
ok kn
|
|
OPT is misleading and usually refers to command line arguments to pfctl
ok sashan kn
|
|
Complete the description of "-s info -v" such that grepping for them
in the manual pager yields something.
Feedback jmc
OK sashan
|
|
In pf(4), the pf_status.since timestamp is set with time_uptime(9).
This is a low-res snapshot of nanouptime(9). nanouptime(9) is used to
implement CLOCK_BOOTTIME for clock_gettime(2). It is not used to
implement CLOCK_UPTIME, though. The names are misleading.
Switch to CLOCK_BOOTTIME in places in userspace where we use
pf_status.since so we are working with the right clock.
Technically CLOCK_MONOTONIC is equivalent, but we shouldn't use that
here. CLOCK_MONOTONIC is not necessarily the "time since boot": the
standard says its absolute value is meaningless.
ok patrick@ bluhm@
|
|
OK kn@, sashan@, florian@
|
|
There is no reason to continue on anchor specific paths if the given
anchor does not exist.
OK sashan
|
|
Missed in previous
|
|
pf(4) returns EINVAL for DIOCGETRULE, DIOCGETRULES and DIOCGETRULESET if
the specified anchor does not exist.
Extend and rename {pfr -> pf}_strerror() to make error message more
consistent.
There are other occasions as well but those need additional tweaks;
that's stuff for another diff.
OK and rename from sashan
|
|
While code in pf/pfctl confusingly uses either anchor or ruleset
depending on the context, pfctl(8) (both manual and user interface)
should be consistent.
For users there are basically anchors only, so do not imply any
difference between the two terminologies.
OK sashan
|
|
Less nesting for clearer code.
OK sashan
|
|
OK sashan
|
|
OK sashan
|
|
OK kn@
|
|
recursive operation ("pfctl -a '*' ...") works for '-s' option already. This
change enables the same thing for '-F' option, so "pfctl -a '*' -Fa" will flush
everything from PF driver.
The idea was discussed with many on tech@ in spring 2019.
OK kn@
|
|
All optimizations work on actual rules; if there are none, return early.
While here, tell which ruleset/anchor is being optimized to make the debug
message actually useful.
OK mikeb
|
|
|
|
This is the userland portion. OK deraadt@ sashan@
|
|
The fix is the same as for other parse.y files in the tree (see bgpd(8) or
unwind(8))
ok bluhm@
|
|
(bug found and fix tested by Jesper Wallin)
OK deraadt OK kn
|
|
Prompted by and OK deraadt
|
|
for yyerror.
From Moritz Buhl
ok bluhm@ claudio@
|
|
value < 0. errno is only updated in this case. Change all (most?)
callers of syscalls to follow this better, and let's see if this strictness
helps us in the future.
|
|
(bug found and fixed by Petr Hoffmann _at_ oracle.com)
OK kn@
|