Commit Graph

101 Commits

Author SHA1 Message Date
kp
88e3fcf359 pfctl: Fix ifgroup check
We cannot just assume that any name which ends with a letter is a group
That's not been true since we allowed renaming of network interfaces. It's also
not true for things like epair0a.

Try to retrieve the group members for the name to check, since we'll get ENOENT
if the group doesn't exist.

MFC after:	1 week
Event:		Aberdeen hackathon 2019
2019-04-19 10:52:54 +00:00
pkelsey
6ce26106c0 Fix the fix added in r343287 for spurious HFSC bandwidth check errors
The logic added in r343287 to avoid false-positive
sum-of-child-bandwidth check errors for HFSC queues has a bug in it
that causes the upperlimit service curve of an HFSC queue to be pulled
down to its parent's linkshare service curve if it happens to be above
it.

Upon further inspection/reflection, this generic
sum-of-child-bandwidths check does not need to be fixed for HFSC - it
needs to be skipped.  For HFSC, the equivalent check is to ensure the
sum of child linkshare service curves are at or below the parent's
linkshare service curve, and this check is already being performed by
eval_pfqueue_hfsc().

This commit reverts the affected parts of r343287 and adds new logic
to skip the generic sum-of-child-bandwidths check for HFSC.

MFC after:	1 day
Sponsored by:	RG Nets
Differential Revision:	https://reviews.freebsd.org/D19124
2019-02-11 22:58:43 +00:00
kp
af0e61e653 pfctl: Fix ifa_grouplookup()
Setting the length of the request got lost in r343287, which means SIOCGIFGMEMB
gives us the required length, but does not copy the names of the group members.
As a result we don't get a correct list of group members, and 'set skip on
<ifgroup>' broke.

This produced all sorts of very unexpected results, because we would end up
applying 'set skip' to unexpected interfaces.

X-MFC-with:	r343287
2019-02-10 21:22:55 +00:00
kp
37106b5e69 pfctl: Point users to net.pf.request_maxcount if large requests are rejected
The kernel will reject very large tables to avoid resource exhaustion
attacks. Some users run into this limit with legitimate table
configurations.

The error message in this case was not very clear:

    pf.conf:1: cannot define table nets: Invalid argument
    pfctl: Syntax error in config file: pf rules not loaded

If a table definition fails we now check the request_maxcount sysctl,
and if we've tried to create more than that point the user at
net.pf.request_maxcount:

    pf.conf:1: cannot define table nets: too many elements.
    Consider increasing net.pf.request_maxcount.
    pfctl: Syntax error in config file: pf rules not loaded

PR:		235076
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D18909
2019-01-28 08:36:10 +00:00
pkelsey
da5a978b52 Remove unused function gsc_destroy()
gsc_destroy() is no longer needed as of r343287.

MFC after:	1 week
2019-01-22 02:53:59 +00:00
pkelsey
12803f533b Reduce pf.conf parsing cost for configs that define N queues from O(N^2) to O(N)
The number of syscalls made during parsing of any config that
defines tables is also reduced, and incorrect warnings that HFSC
parent queue bandwidths were smaller than the sum of their child
bandwidths have been fixed.

Reviewed by:	kp
MFC after:	1 week
Sponsored by:	RG Nets
Differential Revision:	https://reviews.freebsd.org/D18759
2019-01-22 00:50:24 +00:00
kp
dff9d64e7b pfctl: Fix 'set skip' handling for groups
When we skip on a group the kernel will automatically skip on the member
interfaces. We still need to update our own cache though, or we risk
overruling the kernel afterwards.

This manifested as 'set skip' working initially, then not working when
the rules were reloaded.

PR:		229241
MFC after:	1 week
2019-01-13 05:30:26 +00:00
kp
9dfb6fa95d pfctl: Populate ifname in ifa_lookup()
pfctl_adjust_skip_ifaces() relies on this name.

MFC after:	2 weeks
2018-11-08 21:53:09 +00:00
kp
0e6555c17f pfctl test: Add a test for macro names with a space in it 2018-10-28 05:50:04 +00:00
kp
bd5bde638e pfctl: Do not allow whitespace in macro names
i.e. "this is" = "a variable" is not valid. It was accepted by the
parser, but the variable could not be used afterwards.

Obtained from:	OpenBSD
2018-10-28 05:41:13 +00:00
kp
048eb33a87 pf: Make ':0' ignore link-local v6 addresses too
When users mark an interface to not use aliases they likely also don't
want to use the link-local v6 address there.

PR:		201695
Submitted by:	Russell Yount <Russell.Yount AT gmail.com>
Differential Revision:	https://reviews.freebsd.org/D17633
2018-10-28 05:32:50 +00:00
kp
f6bf99efac pf tests: Fix incorrect test for PR 231323
Fix r339466.  The test result file did not list the rdr rule.
Additionally, the route-to rule needs a redirection address.

X-MFC-with:	339466
2018-10-22 23:33:48 +00:00
kp
9ebffec59f pfctl: Fix line numbers when \ is used inside ""
PR:		201520
Obtained from:	OpenBSD
MFC after:	2 weeks
2018-10-22 04:12:51 +00:00
kp
89f2c3be02 pfctl tests: Basic test case for PR 231323
PR:		231323
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D17508
2018-10-20 18:18:28 +00:00
kp
b5745038bc pfctl: Dup strings
When we set the ifname we have to copy the string, rather than just keep
the pointer.

PR:		231323
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D17507
2018-10-20 18:11:46 +00:00
pkelsey
efa4592c55 Fix warning about crossing INT32_MAX boundary in computation of constant value. 2018-08-23 17:41:39 +00:00
pkelsey
5e80b3611a Extend tbrsize heuristic in pfctl(8) to provide a sensible value for
higher bandwidth interfaces.  The new value is used above 2.5 Gbps,
which is the highest standard rate that could be used prior to
r338209, so the default behavior for all existing systems should
remain the same.

The value of 128 chosen is a balance between being big enough to
reduce potential precision/quantization effects stemming from frequent
bucket refills over small time intervals and being small enough to
prevent a greedy driver from burst dequeuing more packets than it has
available hardware ring slots for whenever altq transitions from idle
to backlogged.

Reviewed by:	jmallett, kp
MFC after:	2 weeks
Sponsored by:	RG Nets
Differential Revision: https://reviews.freebsd.org/D16852
2018-08-23 16:10:28 +00:00
pkelsey
2e5630c90a Extended pf(4) ioctl interface and pfctl(8) to allow bandwidths of
2^32 bps or greater to be used.  Prior to this, bandwidth parameters
would simply wrap at the 2^32 boundary.  The computations in the HFSC
scheduler and token bucket regulator have been modified to operate
correctly up to at least 100 Gbps.  No other algorithms have been
examined or modified for correct operation above 2^32 bps (some may
have existing computation resolution or overflow issues at rates below
that threshold).  pfctl(8) will now limit non-HFSC bandwidth
parameters to 2^32 - 1 before passing them to the kernel.

The extensions to the pf(4) ioctl interface have been made in a
backwards-compatible way by versioning affected data structures,
supporting all versions in the kernel, and implementing macros that
will cause existing code that consumes that interface to use version 0
without source modifications.  If version 0 consumers of the interface
are used against a new kernel that has had bandwidth parameters of
2^32 or greater configured by updated tools, such bandwidth parameters
will be reported as 2^32 - 1 bps by those old consumers.

All in-tree consumers of the pf(4) interface have been updated.  To
update out-of-tree consumers to the latest version of the interface,
define PFIOC_USE_LATEST ahead of any includes and use the code of
pfctl(8) as a guide for the ioctls of interest.

PR:	211730
Reviewed by:	jmallett, kp, loos
MFC after:	2 weeks
Relnotes:	yes
Sponsored by:	RG Nets
Differential Revision:	https://reviews.freebsd.org/D16782
2018-08-22 19:38:48 +00:00
kp
3dfca0e2a4 pfctl: Remove unused function
Fix compile issues.

MFC after:	1 week
2018-08-22 08:27:49 +00:00
kp
459b3ecb25 pfctl: Improve set skip handling for groups
Rely on the kernel to appropriately mark group members as skipped.
Once a group is skipped we can clear the update flag on all the members.

PR:		229241
Submitted by:	Andreas Longwitz <longwitz AT incore.de>
MFC after:	1 week
2018-08-22 08:14:29 +00:00
brd
83e33a2658 Move pf.os to sbin/pfctl/
Approved by:	will (mentor)
Glanced at by:	kp
Sponsored by:	Essen Hackathon
Differential Revision:	https://reviews.freebsd.org/D16557
2018-08-11 13:58:26 +00:00
brd
4e657f5af0 Convert bsd.files.mk to support DIRS and simplify by only having one install
target.

Also update the pfctl tests Makefile to work with this change.

Approved by:	bapt (mentor)
Differential Revision:	https://reviews.freebsd.org/D16430
2018-07-26 17:05:33 +00:00
asomers
db69ec6635 Skip the pftcl(8) tests if pf isn't loaded.
Previously, they would simply fail.

MFC after:	2 weeks
2018-07-22 03:58:01 +00:00
kp
406cc909da pfctl: Don't retrieve interface list if '-n' is set
If '-n' is set we don't use the list of skip interfaces, so don't retrieve it.
This fixes issues if 'pfctl -n' is used before the pf module is loaded. This
was broken by r333181.

Reported by:	Jakub Chromy <hicks AT cgi.cz>
MFC after:	1 week
2018-07-03 08:50:49 +00:00
sbruno
ca26d83725 Assuming that the intent (from the white space) is that the fprintf()
be executed in the if() conditional.  If its not supposed to be printed
inside the conditional, then the braces should be removed and the extra
tabs on the fprintf() should be removed.

Noted by cross compilation with gcc-mips.
2018-06-24 21:19:08 +00:00
kp
80906faeff pf: Support "return" statements in passing rules when they fail.
Normally pf rules are expected to do one of two things: pass the traffic or
block it. Blocking can be silent - "drop", or loud - "return", "return-rst",
"return-icmp". Yet there is a 3rd category of traffic passing through pf:
Packets matching a "pass" rule but when applying the rule fails. This happens
when redirection table is empty or when src node or state creation fails. Such
rules always fail silently without notifying the sender.

Allow users to configure this behaviour too, so that pf returns an error packet
in these cases.

PR:		226850
Submitted by:	Kajetan Staszkiewicz <vegeta tuxpowered.net>
MFC after:	1 week
Sponsored by:	InnoGames GmbH
2018-06-22 21:59:30 +00:00
kp
86ab05f953 pf: Return non-zero from 'status' if pf is not enabled
In the pf rc.d script the output of `/etc/rc.d/pf status` or `/etc/rc.d/pf
onestatus` always provided an exit status of zero. This made it fiddly to
programmatically determine if pf was running or not.

Return a non-zero status if the pf module is not loaded, extend pfctl to have
an option to return an error status if pf is not enabled.

PR:		228632
Submitted by:	James Park-Watt <jimmypw AT gmail.com>
MFC after:	1 week
2018-06-06 19:36:37 +00:00
kp
62c9955538 pfctl: Don't break connections on skipped interfaces on reload
On reload we used to first flush everything, including the list of skipped
interfaces.  This can lead to termination of these connections if they send
packets before the new configuration is applied.

Note that this doesn't currently happen on 12 or 11, because of special EACCES
handling introduced in r315514. This special behaviour in tcp_output() may
change, hence the fix in pfctl.

PR:		214613
2018-04-28 13:16:58 +00:00
pfg
26ab50214a pfctl(8): Fix two wrong conditions.
Caught by gcc80's -Wtautological-compare option.

MFC after:	5 days
Approved by:	kp
Obtained from:	DragonFlyBSD (git e3cdbf6c)
Differential Revision:	https://reviews.freebsd.org/D14083
2018-01-27 22:57:01 +00:00
imp
1ccd04128b Free path before returnig.
CID: 977827
2017-12-28 05:33:54 +00:00
pfg
7551d83c35 various: general adoption of SPDX licensing ID tags.
Mainly focus on files that use BSD 2-Clause license, however the tool I
was using misidentified many licenses so this was mostly a manual - error
prone - task.

The Software Package Data Exchange (SPDX) group provides a specification
to make it easier for automated tools to detect and summarize well known
opensource licenses. We are gradually adopting the specification, noting
that the tags are considered only advisory and do not, in any way,
superceed or replace the license texts.

No functional change intended.
2017-11-27 15:37:16 +00:00
kp
aba79e1cd8 pfctl: teach route-to to deal with interfaces with multiple addresses
The route_host parsing code set the interface name, but only for the first
node_host in the list. If that one happened to be the inet6 address and the
rule wanted an inet address it'd get removed by remove_invalid_hosts() later
on, and we'd have no interface name.

We must set the interface name for all node_host entries in the list, not just
the first one.

PR:		223208
MFC after:	2 weeks
2017-11-15 12:27:02 +00:00
bdrewery
a598c4b809 DIRDEPS_BUILD: Update dependencies.
Sponsored by:	Dell EMC Isilon
2017-10-31 00:07:04 +00:00
bdrewery
a4164f0dcd DIRDEPS_BUILD: Connect new directories.
Sponsored by:	Dell EMC Isilon
2017-10-31 00:04:07 +00:00
ngie
d26727d972 Add HAS_TESTS to all Makefiles that are currently using the
`SUBDIR.${MK_TESTS}+= tests` idiom.

This is a follow up to r321912.
2017-08-02 08:50:42 +00:00
ngie
734d081ed1 MFhead@r321912 2017-08-02 08:38:36 +00:00
kp
34cfc1e7ed pfctl parser tests
Copy the most important test cases from OpenBSD's corresponding
src/regress/sbin/pfctl, those that run pfctl on a test input file and check
correctness of its output. We have also added some new tests using the same
format.

The tests consist of a collection of input files (pf*.in) and
corresponding output files (pf*.ok). We run pfctl -nv on the input
files and check that the output matches the output files. If any
discrepancy is discovered during future development in the source
tree, we know that a regression bug has been introduced into the tree.

Submitted by:	paggas
Sponsored by:	Google, Inc (GSoC 2017)
Differential Revision:	https://reviews.freebsd.org/D11322
2017-07-15 19:22:01 +00:00
ngie
3c4b37d990 sbin: normalize paths using SRCTOP-relative paths or :H when possible
This simplifies make logic/output

MFC after:	1 month
Sponsored by:	Dell EMC Isilon
2017-03-04 11:33:01 +00:00
loos
c411c7577d Fix the parsing of NPt binat rules.
In this specific case the src address can be set to any, which was not
accepted prior to this commit.

pfSense bug report:	https://redmine.pfsense.org/issues/6985
Reviewed by:	kp
Obtained from:	pfSense
MFC after:	2 weeks
Sponsored by:	Rubicon Communications, LLC (Netgate)
2016-12-28 16:16:48 +00:00
kp
df497bd192 pfctl: fix nested inline anchors
Import the OpenBSD fix for nested inline anchors.

PR:		196314
Submitted by:	krichy@cflinux.hu
Obtained from:	OpenBSD
2016-11-10 18:41:43 +00:00
bdrewery
30f99dbeef Fix improper use of "its".
Sponsored by:	Dell EMC Isilon
2016-11-08 23:59:41 +00:00
kp
3fbefcb2d1 pf: port extended DSCP support from OpenBSD
Ignore the ECN bits on 'tos' and 'set-tos' and allow to use
DCSP names instead of having to embed their TOS equivalents
as plain numbers.

Obtained from:	OpenBSD
Sponsored by:	OPNsense
Differential Revision:	https://reviews.freebsd.org/D8165
2016-10-13 20:34:44 +00:00
kp
077476b686 pf: remove fastroute tag
The tag fastroute came from ipf and was removed in OpenBSD in 2011. The code
allows to skip the in pfil hooks and completely removes the out pfil invoke,
albeit looking up a route that the IP stack will likely find on its own.
The code between IPv4 and IPv6 is also inconsistent and marked as "XXX"
for years.

Submitted by:	Franco Fichtner <franco@opnsense.org>
Differential Revision:	https://reviews.freebsd.org/D8058
2016-10-04 19:35:14 +00:00
sevan
c28389fc20 Note the version PF first appeared in FreeBSD & from which version it was ported from.
Address the contractions raised by igor.

PR:		212574
Approved by:	bcr (mentor)
MFC after:	4 days
Differential Revision:	https://reviews.freebsd.org/D8105
2016-10-03 00:35:14 +00:00
loos
e8dc3f01c2 Fix a regression in pf.conf while parsing the 'interval' keyword.
The bug was introduced by r287009.

PR:		210924
Submitted by:	kp@
Sponsored by:	Rubicon Communications (Netgate)
Pointy hat to:	loos
2016-08-05 02:19:03 +00:00
kp
86090567a8 pfctl: Make most global variables static.
This will make it easier to link as a library.

Submitted by:	Christian Mauderer <christian.mauderer@embedded-brains.de>
2016-08-04 19:24:44 +00:00
kp
7f74bdcca7 pfctl: Add missing __FBSDID to pfctl_osfp.c 2016-08-04 19:24:05 +00:00
kp
119d258974 pfctl: Use const where possible.
This adds const qualifiers where it is possible.

Submitted by:	Christian Mauderer <christian.mauderer@embedded-brains.de>
2016-08-02 20:32:02 +00:00
kp
7999df886d pfctl: Match prototype of pfctl_load_hostid.
The prototype and the implementation of the pfctl_load_hostid used a
different data type for one of the parameters.

Submitted by:	Christian Mauderer <christian.mauderer@embedded-brains.de>
2016-08-02 19:54:40 +00:00
kp
f9eb7c538a pfctl: Allow TOS bits to be cleared
TOS value 0 is valid, so use 256 as an invalid value rather than zero.
This allows users to enforce TOS == 0 with pf.

Reported by:	Radek Krejča <radek.krejca@starnet.cz>
2016-08-02 15:41:42 +00:00