Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 18 Jul 2023 18:50:44 -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:  <7vbajkgkvsxkbsl2am3wnpn2ogh6tsty6tquurko2we22rjcjm@ilb45u4llxsv>
In-Reply-To: <bpp7osufd7dbvoo35ffldieku3ctkv6p2evbfygisfq7hixiic@j7xyvx5blomy>
References:  <202306201435.35KEZtHN062484@gitrepo.freebsd.org> <bpp7osufd7dbvoo35ffldieku3ctkv6p2evbfygisfq7hixiic@j7xyvx5blomy>

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

[-- Attachment #1 --]
On 2023-06-20 at 11:57 EDT, Matteo Riondato <matteo@freebsd.org> wrote:

>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?

Hi Doug and Kristof,

A ping about what I said below, as I think there is a need to better 
document this sysctl.

Additionally, I'm also a little worried about the name of the sysctl, 
which seems extremely generic, while its use may be much more specific. 
If the name could be made more specific, that should probably be done 
before 14 is branched.

Thanks,
Matteo

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

iQJHBAABCgAxFiEEa9uKZL0hP4E8Nl5vGwL9SVQlVQEFAmS3FzsTGGhrcHM6Ly9w
Z3AubWl0LmVkdQAKCRAbAv1JVCVVAQv2EACPHxidWvhTdzuSrm4tt/HgNC474rjG
OJ6zPG+Io2Z6GMIfyanZWjnaXX5pzDsA0ggjNE8qp+15I+jJEMnsaqHAC0LHyWQI
zz7fSp7K+CPjs4GJvmK81JB0ntuZgOb2/++ZiQtcfFxWnhgMGxjHyc7A5S+QEFao
RilE6E8yh4OaFFroq6rfM+DhOpxQC3PH184ZuBKGsBItPu1eJBRHd6izKiwFidhs
SxTh+RVhm1WuqHMREWo7d+snSJBNsIPKHxDHmOZX5uDjURwfd/sUqU8s0wVm7CQX
y+po+5NPdaFBFQ8gyvrLZ3aLpeIprqrMAsTTaNlYWWJ5LMOGNbi1KlM1x/2Is3FH
MOjFpStZ1EteMQB7/jCCl6eDq4Ps39WASPJ3YEXpZjo9kNOwQYi9MdEfYiCgX3Fv
MtxGGvqzl7dZlFEGw68LHFOvL/XoF82Rtmg3pU79hc5VS8rLdOd4cTP9wB7hj4NH
0SdD8R9j3aKYRn5DbiALK6zh68cr6ovLS2mf1n/vic+H9ooWYf6+lXkX090oiITG
U/jcxz06YakFIzi0mvKnZ7NbKMfzn82lUSSrxBIBNU1fvVIoBB/toXiHc4kPqhCd
vJyl5OuxQqaPi/4gsjW4udBskH4gMIMz1yA07ClBzPRuNR/IUXNalvE6cG+e5AXC
6Ir3vpW/Gx2zSw==
=NHyY
-----END PGP SIGNATURE-----

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