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
--dcahvzwmiwrnfdrr Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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=3D3a1f834b5228986a7c14fd60da= 13cf2700e80996 >> >>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=20 >> existing filter rule sets. To enable this extra filter for packets bei= ng=20 >> delivered locally, use: >> >> sysctl net.pf.filter_local=3D1 >> 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=20 >>++++++++++++++++-------- >>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=20 >> 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=3D1 >>+ service pf restart > >It seems a bit weird to suggest an action that is not permanent (does=20 >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=20 >connections =E2=80=A6, pf filter rules can *optionally* be enabled for=20 >packets delivered locally". That "optionally" makes it sound as if it=20 >is not *required* to enable pf filter rules for packets delivered=20 >locally in order to enable pf rdr rules for connections etc etc., but,=20 >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=20 >are enabled for packets originating from localhost and delivered=20 >locally. > >This feature can be useful for, e.g., enabling rdr rules for=20 >connections initiated from localhost and redirected to a different=20 >port on localhost. Setting the sysctl to 1 may change the behavior of=20 >rules which match packets delivered to lo0, so it may be necessary to=20 >add enable the "skip on lo" option." > >Note that "skip on" is not a rule, even if it is translated to a pair=20 >of rules: it's part of the options, and requires "set" before it, per=20 >pf.conf(5). Also, I'm assuming (and mention in the rewording) we are=20 >talking about rdr rules for port remapping, not rdr rules that=20 >redirect to other destinations, but please confirm or adjust. > >More generally, this new feature should likely also be documented=20 >somewhere else (pf(4) ? pfctl(8)? pf.conf(5)?). > >But apart from the above, I'm a little puzzled: does it mean that=20 >until now (and continuing to do so, unless one sets the sysctl to 1),=20 >packets originating locally and destined locally were not filtered by=20 >pf? I.e., that filtering rules on lo0 had no effect on incoming=20 >traffic from localhost? Hi Doug and Kristof, A ping about what I said below, as I think there is a need to better=20 document this sysctl. Additionally, I'm also a little worried about the name of the sysctl,=20 which seems extremely generic, while its use may be much more specific.=20 If the name could be made more specific, that should probably be done=20 before 14 is branched. Thanks, Matteo --dcahvzwmiwrnfdrr Content-Type: application/pgp-signature; name="signature.asc" -----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----- --dcahvzwmiwrnfdrr--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?7vbajkgkvsxkbsl2am3wnpn2ogh6tsty6tquurko2we22rjcjm>