Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 20 Jun 2023 11:57:47 -0400
From:      Matteo Riondato <matteo@freebsd.org>
To:        Doug Rabson <dfr@freebsd.org>, Kristof Provost <kp@freebsd.org>
Cc:        src-committers@freebsd.org, dev-commits-src-all@freebsd.org,  dev-commits-src-main@freebsd.org
Subject:   Re: git: 3a1f834b5228 - main - pf: Add code to enable filtering for locally delivered packets
Message-ID:  <bpp7osufd7dbvoo35ffldieku3ctkv6p2evbfygisfq7hixiic@j7xyvx5blomy>
In-Reply-To: <202306201435.35KEZtHN062484@gitrepo.freebsd.org>
References:  <202306201435.35KEZtHN062484@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

[-- Attachment #1 --]
On 2023-06-20 at 10:35 EDT, Doug Rabson <dfr@FreeBSD.org> wrote:

>The branch main has been updated by dfr:
>
>URL: https://cgit.FreeBSD.org/src/commit/?id=3a1f834b5228986a7c14fd60da13cf2700e80996
>
>commit 3a1f834b5228986a7c14fd60da13cf2700e80996
>Author:     Doug Rabson <dfr@FreeBSD.org>
>AuthorDate: 2023-06-20 13:01:58 +0000
>Commit:     Doug Rabson <dfr@FreeBSD.org>
>CommitDate: 2023-06-20 14:34:01 +0000
>
>    pf: Add code to enable filtering for locally delivered packets
>
>    This is disabled by default since it potentially changes the behavior of
>    existing filter rule sets. To enable this extra filter for packets being
>    delivered locally, use:
>
>            sysctl net.pf.filter_local=1
>            service pf restart
>
>    PR:             268717
>    Reviewed-by:    kp
>    MFC-after:      2 weeks
>    Differential Revision: https://reviews.freebsd.org/D40373
>---
> UPDATING                                     | 12 ++++++++++++
> sys/netpfil/pf/pf_ioctl.c                    | 20 ++++++++++++++++++++
> tests/sys/netpfil/common/utils.subr          |  3 +--
> tests/sys/netpfil/pf/fragmentation_compat.sh |  3 ++-
> tests/sys/netpfil/pf/fragmentation_pass.sh   |  3 ++-
> tests/sys/netpfil/pf/killstate.sh            | 24 ++++++++++++++++--------
> tests/sys/netpfil/pf/map_e.sh                |  3 ++-
> tests/sys/netpfil/pf/pass_block.sh           |  3 ++-
> tests/sys/netpfil/pf/pfsync.sh               |  1 +
> tests/sys/netpfil/pf/route_to.sh             |  3 ++-
> tests/sys/netpfil/pf/set_skip.sh             |  2 +-
> tests/sys/netpfil/pf/table.sh                |  6 ++++--
> 12 files changed, 65 insertions(+), 18 deletions(-)
>
>diff --git a/UPDATING b/UPDATING
>index 1980411c1853..f4e13d97006d 100644
>--- a/UPDATING
>+++ b/UPDATING
>@@ -27,6 +27,18 @@ NOTE TO PEOPLE WHO THINK THAT FreeBSD 14.x IS SLOW:
> 	world, or to merely disable the most expensive debugging functionality
> 	at runtime, run "ln -s 'abort:false,junk:false' /etc/malloc.conf".)
>
>+20230619:
>+	To enable pf rdr rules for connections initiated from the host, pf
>+	filter rules can be optionally enabled for packets delivered
>+	locally. This can change the behavior of rules which match packets
>+	delivered to lo0. To enable this feature:
>+
>+		sysctl net.pf.filter_local=1
>+		service pf restart

It seems a bit weird to suggest an action that is not permanent (does 
not survive reboot). See proposed rewording below.

>+
>+	When enabled, its best to ensure that packets delivered locally are not

s/its/it is/

>+	filtered, e.g. by adding a 'skip on lo' rule.

TBH, I find the phrasing a bit confusing: "to enable pf rdr rules for 
connections …, pf filter rules can *optionally* be  enabled for packets 
delivered locally". That "optionally" makes it sound as if it is not 
*required* to enable pf filter rules for packets delivered locally in 
order to enable pf rdr rules for connections etc etc., but, given this 
change, I assume it is.

Perhaps a better phrasing (assuming I understand the feature) would be:

"The new sysctl net.pf.filter_local controls whether PF filter rules are 
enabled for packets originating from localhost and delivered locally.

This feature can be useful for, e.g., enabling rdr rules for connections 
initiated from localhost and redirected to a different port on 
localhost. Setting the sysctl to 1 may change the behavior of rules 
which match packets delivered to lo0, so it may be necessary to add 
enable the "skip on lo" option."

Note that "skip on" is not a rule, even if it is translated to a pair of 
rules: it's part of the options, and requires "set" before it, per 
pf.conf(5). Also, I'm assuming (and mention in the rewording) we are 
talking about rdr rules for port remapping, not rdr rules that redirect 
to other destinations, but please confirm or adjust.

More generally, this new feature should likely also be documented 
somewhere else (pf(4) ? pfctl(8)? pf.conf(5)?). 

But apart from the above, I'm a little puzzled: does it mean that until 
now (and continuing to do so, unless one sets the sysctl to 1), packets 
originating locally and destined locally were not filtered by pf? I.e., 
that filtering rules on lo0 had no effect on incoming traffic from 
localhost?

Thanks,
Matteo

[-- Attachment #2 --]
-----BEGIN PGP SIGNATURE-----

iQJHBAABCgAxFiEEa9uKZL0hP4E8Nl5vGwL9SVQlVQEFAmSRzGgTGGhrcHM6Ly9w
Z3AubWl0LmVkdQAKCRAbAv1JVCVVAd/yD/98jK6+btKowzD4NlfP/QTksFfjWia1
s86lfjfk85NSSdG4Y0mcE5WvcPYD79p38UCK2f3wxfW5u+Wu8NifMVcTuntarw1u
BIl31Rx7lzDi61R9cTZVn4VDFMX+65Ln69uOpwp+WNze8BNxogGiRO9h6lE5fiGd
c0dfxB+50U/JqPHOFvTNRB+Z4cQHvUZhH3iOS2L47qRC0THhtM5+0zJMkD4OC8V+
qnjxusRxJ8EfLQ9GfcZ8Va4+riqiz0ZPwTgZUtGtyLzs1Y5muW127Np2zbkZi24P
pZ4xCmWKI6NKlFV0aBGGmpQoWBnXoWtYRGvTqdZBN9sZBw9wb6FmA7l6bj/YLXOH
mmB+UjGO9PJohGZ2FXbvNm3B3mCyZ8PJdgruW/v1Rike0+kdtfe2VpJbz8FJ/nft
7WLJEIUfhPSaey4QwAGLLvsnxe+n0Cbmf54k9++y5cWMCajzKKxcIvKCLJCs3baN
pzPIktkUZPc0I6IRRXLXm7zxMTo5n2JC2mAuFerYXrGUsXSWL1mNdb+ZGk35V5TA
4IE9oFgvmBU1pY1BZHPjC+zdNI1MXhlMjXhCH8vLoWa2b9thcVD5DeZ5CbBLwK1M
Q5s3vyh0+inVTxBk+Giu2h8C0oL33b1PbNes6kK8IP2k6yub1yCGiddPpvIIp7YP
JZIT0cTnE7gK6g==
=VFn6
-----END PGP SIGNATURE-----

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bpp7osufd7dbvoo35ffldieku3ctkv6p2evbfygisfq7hixiic>