pfctl_show_nat() is called recursively to print nat anchors. This passes the
anchor path, but this path was modified by pfctl_show_nat(), leading to issues
printing the anchors.
Make a copy of the path ('npath') before we modify it. Ensure we do this
correctly by sprinking in 'const', and add a test case to verify that we do now
print things correctly.
Reported by: Thomas Pasqualini <thomas.pasqualini@orange.com>
MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 58164dcb55)
m_unshare() can return a new mbuf pointer. We update the 'm' pointer in
ovpn_udp_input(), but if we decide to pass on the packet (e.g. because it's for
an unknown peer) the caller (udp_append()) continues with the old 'm' pointer,
eventually resulting in a use-after-free.
Re-order operations in ovpn_udp_input() so that we don't modify the 'm' pointer
until we're committed to keeping the packet.
PR: 283426
Test case by: takahiro.kurosawa@gmail.com
MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
See commit 4f02a7d739 for more background.
I cannot see a good reason to continue ignoring mismatching UIDs when
binding to INADDR_ANY. Looking at the sdr.V2.4a7n sources (mentioned in
bugzilla PR 7713), there is a CANT_MCAST_BIND hack wherein the
application binds to INADDR_ANY instead of a multicast address, but
CANT_MCAST_BIND isn't defined for FreeBSD builds.
It seems unlikely that we still have a use-case for allowing sockets
from different UIDs to bind to the same port when binding to the
unspecified address. And, as noted in D47832, applications like sdr
would have been broken by the inverted SO_REUSEPORT check removed in
that revision, apparently without any bug reports. Let's break
compatibility and simply disallow this case outright.
Also, add some comments, remove a hack in a regression test which tests
this funtionality, and add a new regression test to exercise the
remaining checks that were added in commit 4658dc8325.
MFC after: 1 month
Sponsored by: Klara, Inc.
Sponsored by: Stormshield
Differential Revision: https://reviews.freebsd.org/D47870
(cherry picked from commit c9756953bd)
This check for SO_REUSEPORT was added way back in commit 52b65dbe85.
Per the commit log, this commit restricted this port-stealing check to
unicast addresses, and then only if the existing socket does not have
SO_REUSEPORT set. In other words, if there exists a socket bound to
INADDR_ANY, and we bind a socket to INADDR_ANY with the same port, then
the two sockets need not be owned by the same user if the existing
socket has SO_REUSEPORT set.
This is a surprising semantic; bugzilla PR 7713 gives some additional
context. That PR makes a case for the behaviour described above when
binding to a multicast address. But, the SO_REUSEPORT check is only
applied when binding to a non-multicast address, so it doesn't really
make sense. In the PR the committer notes that "unicast applications
don't set SO_REUSEPORT", which makes some sense, but also refers to
"multicast applications that bind to INADDR_ANY", which sounds a bit
suspicious.
OpenBSD performs the multicast check, but not the SO_REUSEPORT check.
DragonflyBSD removed the SO_REUSEPORT (and INADDR_ANY) checks back in
2014 (commit 0323d5fde12a4). NetBSD explicitly copied our logic and
still has it.
The plot thickens: 20 years later, SO_REUSEPORT_LB was ported from
DragonflyBSD: this option provides similar semantics to SO_REUSEPORT,
but for unicast addresses it causes incoming connections/datagrams to be
distributed among all sockets in the group. This commit (1a43cff92a)
inverted the check for SO_REUSEPORT while adding one for
SO_REUSEPORT_LB; this appears to have been inadvertent. However:
- apparently no one has noticed that the semantics were changed;
- sockets belonging to different users can now be bound to the same port
so long as they belong to a single lbgroup bound to INADDR_ANY, which
is not correct.
Simply remove the SO_REUSEPORT(_LB) checks, as their original
justification was dubious and their current implementation is wrong; add
some tests.
Reviewed by: glebius
MFC after: 1 month
Sponsored by: Klara, Inc.
Sponsored by: Stormshield
Differential Revision: https://reviews.freebsd.org/D47832
(cherry picked from commit 4f02a7d739)
IPv4 packets can be routed via an IPv6 nexthop, so the handling of the
parsed address family is more strict than it needs to be. If we have a
valid header that matches a known peer, then we have no reason to
decline the packet.
Convert it to an assertion that it matches the destination as viewed by
the stack below it, instead. `dst` may be the gateway instead of the
destination in the case of a nexthop, so the `af` assignment must be
switched to use the destination in all cases.
Add a test case that approximates a setup like in the PR and
demonstrates the issue.
PR: 284857
Reviewed by: markj (earlier version), zlei
(cherry picked from commit 2bef0d54f7)
Scapy is not needed to run this test. Remove it from the required
programs list.
Reviewed by: kp
Approved by: emaste (mentor)
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D49006
(cherry picked from commit 1f1963bd32)
Add more test cases for pf fragment hole counter. Also look into
final fragment of echo reply and check total length of IP packet.
MFC after: 1 week
Obtained from: OpenBSD, bluhm <bluhm@openbsd.org>, 640736615b
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit db100bd930)
The reassembly state will be dropped after timeout, all related
fragments are dropped until that. This is conforming to RFC 5722.
- Sort pf_fragment fields while there.
- If the fr_queue is empty, we had overlapping fragments, don't add
new ones.
- If we detect overlapping IPv6 fragments, flush the fr_queue and
drop all fragments immediately.
- Rearrange debug output, to make clear what happens.
- An IPv4 fragment that is totaly overlapped does not inclease the
bad fragment counter.
- Put an KASSERT into pf_isfull_fragment() to make sure that the
fr_queue is never emtpy there.
discussed with Fernando Gont; ok henning@
Obtained from: OpenBSD, bluhm <bluhm@openbsd.org>, 8b45f36762
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 6a3266f72e)
Much like we already do for TCP and UDP we should also parse SCTP-in-ICMP
messages to see if they apply to an SCTP connection we've already allowed. If so
we should allow the ICMP packet to pass, even if we'd otherwise block it.
Add a test case where we generate an 'ICMP unreachable - need to frag' packet
and check that it passes through pf.
MFC after: 2 weeks
Sponsored by: Orange Business Services
(cherry picked from commit 7d5e02b015)
Allow users to choose to allow permitted SCTP connections to set up additional
multihomed connections regardless of the ruleset. That is, allow an already
established connection to set up flows that would otherwise be disallowed.
In case of if-bound connections we initially set the extra associations to
be floating, because we don't know what path they'll be taking when they're
created. Once we see the first traffic we can bind them.
MFC after: 2 weeks
Sponsored by: Orange Business Services
Differential Revision: https://reviews.freebsd.org/D48453
(cherry picked from commit e4f2733df8)
Verify that we reset the cleared time when we zero an address' counters in
a table.
PR: 282877, 282984
Reviewed by: kp
MFC after: 2 weeks
Signed-off-by: Leonid Evdokimov <leon@darkk.net.ru>
Differential Revision: https://reviews.freebsd.org/D48242
(cherry picked from commit 0749d81343)
This will make it easier for scripts to detect idle hosts in tables.
PR: 282984
Reviewed by: kp
MFC after: 2 weeks
(cherry picked from commit 5b59b0c61e)
The ioctl DIOCRCLRASTATS provides the functionality of clearing stats
not only for the whole table for for addresses stored in that table. The
functionality was missing from pfctl, though. Add it now.
PR: 282877
Obtained from: OpenBSD, kirill <kirill@openbsd.org>, e496dff3a7
MFC after: 3 weeks
(cherry picked from commit 6463b6b591)
If NAT rules cause inbound connections to different external IPs to be
mapped to the same internal IP, and some application uses the same
source port for multiple such connections, rdr translation may result in
conflicts that cause some of the connections to be dropped.
Address this by letting rdr rules detect state conflicts and modulate
the source port to avoid them.
Reviewed by: kp, allanjude
MFC after: 3 months
Sponsored by: Klara, Inc.
Sponsored by: Modirum
Differential Revision: https://reviews.freebsd.org/D44488
(cherry picked from commit 9897a66923)
When processing an SCTP ASCONF we re-run the rules processing to check
if the new state should be allowed as well. We used to do so against the
'all' interface, to allow new connections to use any interface.
This is problematic for two reasons, the first being it may unexpectedly
bypass interface restrictions. The more important one is that it
can trigger panics. If the ruleset contains a rule which filters on
interface group we'd attempt to process the group list for the 'all'
interface. As this isn't a real interface it doesn't have an associated
struct ifnet, and we end up dereferencing a NULL pointer.
Solve this by not overriding the interface, instead leaving the physical
interface the SCTP ASCONF arrived on. This implies that we may end up
binding to that interface (if if-bound), and thus denying traffic on
other interfaces. Users can allow this anyway by setting 'state-policy
floating' on the relevant SCTP rules. This arguably better reflects user
intent as well. That is, we'll consider SCTP multihomed states to be
floating if we're in floating mode, and if-bound if we're if-bound.
Update the test cases to account for this, while adding a "pass on
lo" (i.e. pass on an interface group") rule to provoke this issue. Add
separate test cases for the floating and if-bound scenarios.
Reported by: Franco Fichtner <franco@opnsense.org>
MFC after: 3 weeks
Sponsored by: Orange Business Services
Without this patch, vdevs faulted via AUX state would not be replaced
once the appropriate drive is replaced. ZFS does not internally use
that state, but a drive can be manually forced into such a state with a
command like zinject.
Submitted by: Goran Mekić <meka@tilda.center>
Sponsored by: ConnectWise
Differential Revision: https://reviews.freebsd.org/D46866
(cherry picked from commit 618ae939b7)
The update to py-twisted 24.7.0 broke EPSV mode in twisted's ftp server.
Work around this by disabling EPSV (and thus using PASV).
PR: 282154
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 9cd6ab45a4)
From scapy 2.6.0 it now removes (sets to None) the IP checksum in its defrag
code. We typically ask for IPv4 or IPv6 defragmentation, which causes check
function failures.
Just skip the checksum verificiation here, because we're going to notice
incorrect checksums even if the tests don't directly verify it.
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 6b8096fa5f)
Many of the modified tests add epair interfaces to the host, though most
of the tests themselves run in a VNET jail. scapy in particular becomes
unhappy when the list of interfaces changes as it is running, so, to
improve reliability of parallel test runs, isolate tests which add
epairs to the host.
Also serialize arp tests, as they examine the dmesg as part of the test.
The list of modified tests is not exhaustive, it was determined by
running the test suite with parallelization enabled and looking at
failures. It may be easier to just automatically enable VNET jailing of
all netinet tests, but let's be more particular for now.
Reviewed by: kp
MFC after: 1 month
Differential Revision: https://reviews.freebsd.org/D46606
(cherry picked from commit b103fc49ae)
Approved by: kp (mentor), markj (mentor)
Now that kyua can isolate tests in their own vnets we can run these tests in
parallel.
Reviewed by: markj
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46600
(cherry picked from commit a05decec2d)
Approved by: kp (mentor), markj (mentor)
Run these tests in their own (vnet) jail so we don't have to worry about IP
range or jail name conflicts.
Reviewed by: markj
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46040
(cherry picked from commit 4f752a1583)
Approved by: kp (mentor), markj (mentor)
If we fail to change the vlan id we have to undo the removal (and vlan id
change) in the error path. Otherwise we'll have removed the vlan object from the
hash table, and have the wrong vlan id as well. Subsequent modification attempts
will then try to remove an entry which doesn't exist, and panic.
Undo the vlan id modification if the insertion in the hash table fails, and
re-insert it under the original vlan id.
PR: 279195
Reviewed by: zlei
MFC atfer: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D45285
(cherry picked from commit bdd12889ea)
If the FUSE_GETATTR issued to query a file's size during
fuse_vnop_deallocate failed for any reason, then fuse_vnop_deallocate
would attempt to destroy an uninitialized fuse_dispatcher struct, with a
crash the likely result. This bug only affects FUSE file systems that
implement FUSE_FALLOCATE, and is unlikely to be seen on those that don't
disable attribute caching.
Reported by: Coverity Scan
CID: 1505308
(cherry picked from commit f93a50d69d)
The in-tree ZFS test suite is somewhat outdated and I see a number of
failures there. I tend to think that we want to integrate the OpenZFS
test suite somehow, replacing the legacy one, though it's also possible
to run that as a separate test suite.
In any case, if one wants to run the OpenZFS test suite separately, it's
useful to be able to disable installation of the legacy ZFS test suite,
so let's provide a src.conf option to do that.
Reviewed by: asomers
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D46476
(cherry picked from commit 24affded3d)
The test suite runs the same tests twice, as different users, and these
can trample over each other when run in parallel, causing spurious test
failures.
MFC after: 1 week
(cherry picked from commit 41ece3c036)
Background:
If a user does pathconf(_, _PC_MIN_HOLE_SIZE) on a fusefs file system,
the kernel must actually issue a FUSE_LSEEK operation in order to
determine whether the server supports it. We cache that result, so we
only have to send FUSE_LSEEK the first time that _PC_MIN_HOLE_SIZE is
requested on any given mountpoint.
Problem 1:
Unlike fpathconf, pathconf operates on files that may not be open. But
FUSE_LSEEK requires the file to be open. As described in PR 278135,
FUSE_LSEEK cannot be sent for unopened files, causing _PC_MIN_HOLE_size
to wrongly report EINVAL. We never noticed that before because the
fusefs test suite only uses fpathconf, not pathconf. Fix this bug by
opening the file if necessary.
Problem 2:
On a completely sparse file, with no data blocks at all, FUSE_LSEEK with
SEEK_DATA would fail to ENXIO. That's correct behavior, but
fuse_vnop_pathconf wrongly interpreted that as "FUSE_LSEEK not
supported". Fix the interpretation.
PR: 278135
Sponsored by: Axcient
Differential Revision: https://reviews.freebsd.org/D44618
(cherry picked from commit 6efba04df3)
Some SCTP implementations will abort connections and then later re-use the same
port numbers (i.e. both src and dst) for a new connection, before pf has fully
purged the old connection.
Apply the same hack we already have for similarly misbehaving TCP
implementations and forcibly remove the old state so we can create a new one.
MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 82e021443a)
Test whether the SNDSTIOC_ADD_USER_DEVS IOCTL (registers a userland
device to /dev/sndstat) works properly.
Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Reviewed by: dev_submerge.ch
Differential Revision: https://reviews.freebsd.org/D46228
(cherry picked from commit 2668e76d6e)
We're going to start running many of the vnet tests in nested jails (so they
can run in parallel). That means the tests won't be able to load kernel modules,
which we commonly do for if_epair and if_bridge.
Just assume that all vnet tests need this, because so many of them do that we
don't want to manually annotate all of them.
This is essentially a no-op on non-nested tests.
Do the same for the python test framework.
While here also have pflog_init actually call pft_init. While having pflog
loaded implies we have pf too pft_init also checks for vimage support, and now
for if_epair.
Reviewed by: markj
MFC after: 1 month
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D46039
(cherry picked from commit ae8d588140)
Many of the tests create temporary files. pid files, log files, tcpdump
captures, ... We should take care to ensure they're stored in the temporary
working directory Kyua creates rather than in the root directory.
This ensures there are no conflicts between simultaneously running tests, and
also keeps the root directory clean.
MFC after: 1 month
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit b0fcf4d522)
(e.g. traceroute with icmp)
ok henning, jsing
Also extend the test case to cover this scenario.
PR: 280701
Obtained from: OpenBSD
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 89f6723288)
This serves as a regression test for commit a48df53e42.
Reviewed by: bz, allanjude
MFC after: 2 weeks
Sponsored by: Klara, Inc.
Sponsored by: Bell Tower Integration
Differential Revision: https://reviews.freebsd.org/D46136
(cherry picked from commit feda329622)
These ones were unambiguous cases where the Foundation was the only
listed copyright holder.
Sponsored by: The FreeBSD Foundation
(cherry picked from commit 5c2bc3db20)
When creating a state for ICMP (v4 or v6) packets we only used the ID, which
means that we could confuse different ICMP types. For example, if we allowed
neighbour discovery but not ICMPv6 echo requests an ND packet could create a
state that the echo request would match.
Test that this is now fixed.
Reported by: Enrico Bassetti <e.bassetti@tudelft.nl>
MFC after: 1 day
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit b9f0dbc3d0)
It relies on being able inspect the dmesg buffer to see whether the
kernel logged certain messages. If it's interleaved with tests that
generate large amounts of console output (e.g., GELI tests), then it can
fail spuriously.
MFC after: 2 weeks
(cherry picked from commit ab27e232b1)
