Commit Graph

101 Commits

Author SHA1 Message Date
Kristof Provost
7296d6c9bf 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
Patrick Kelsey
997667302f 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
Kristof Provost
4c8fb952b5 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
Kristof Provost
542feeff96 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
Patrick Kelsey
0e4ef871dc 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
Patrick Kelsey
1d34c9dac8 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
Kristof Provost
b1f3ab0051 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
Kristof Provost
0f9e47a9c4 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
Kristof Provost
c964e17bfe pfctl test: Add a test for macro names with a space in it 2018-10-28 05:50:04 +00:00
Kristof Provost
d3f6532494 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
Kristof Provost
99eb00558a 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
Kristof Provost
db2380870c 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
Kristof Provost
4a8e4793ed 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
Kristof Provost
63488b6e4e 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
Kristof Provost
a2a90d6ee5 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
Patrick Kelsey
ef9afde438 Fix warning about crossing INT32_MAX boundary in computation of constant value. 2018-08-23 17:41:39 +00:00
Patrick Kelsey
ff7b3f731a 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
Patrick Kelsey
249cc75fd1 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
Kristof Provost
cd615bf560 pfctl: Remove unused function
Fix compile issues.

MFC after:	1 week
2018-08-22 08:27:49 +00:00
Kristof Provost
7a831fecc8 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
Brad Davis
6d76ed56a0 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
Brad Davis
7b821f55bd 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
Alan Somers
8707733f71 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
Kristof Provost
7a2bf4c52e 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
Sean Bruno
2d0730b243 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
Kristof Provost
150182e309 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
Kristof Provost
fa1d4439f9 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
Kristof Provost
c5e105655b 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
Pedro F. Giffuni
e2d84d5adc 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
Warner Losh
076950009f Free path before returnig.
CID: 977827
2017-12-28 05:33:54 +00:00
Pedro F. Giffuni
1de7b4b805 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
Kristof Provost
58c8430a32 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
Bryan Drewery
ea825d0274 DIRDEPS_BUILD: Update dependencies.
Sponsored by:	Dell EMC Isilon
2017-10-31 00:07:04 +00:00
Bryan Drewery
3806950135 DIRDEPS_BUILD: Connect new directories.
Sponsored by:	Dell EMC Isilon
2017-10-31 00:04:07 +00:00
Enji Cooper
d511b20a69 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
Enji Cooper
4b330699f8 Convert traditional ${MK_TESTS} conditional idiom for including test
directories to SUBDIR.${MK_TESTS} idiom

This is being done to pave the way for future work (and homogenity) in
^/projects/make-check-sandbox .

No functional change intended.

MFC after:	1 weeks
2017-08-02 08:35:51 +00:00
Kristof Provost
4d7709ddf6 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
Enji Cooper
22289a8c3d 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
Luiz Otavio O Souza
1e93588b1d 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
Kristof Provost
2fd47dea4e 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
Bryan Drewery
28323add09 Fix improper use of "its".
Sponsored by:	Dell EMC Isilon
2016-11-08 23:59:41 +00:00
Kristof Provost
1f4955785d 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
Kristof Provost
813196a11a 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 Janiyan
eb6d64f86c 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
Luiz Otavio O Souza
7f8af000a9 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
Kristof Provost
13cfafabf4 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
Kristof Provost
233e1432b7 pfctl: Add missing __FBSDID to pfctl_osfp.c 2016-08-04 19:24:05 +00:00
Kristof Provost
72a3cf0f41 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
Kristof Provost
9a5e33be57 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
Kristof Provost
0cd7a91aa7 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