git: 829a69db855b - main - pf: change pf_route so pf only runs when packets enter and leave the stack.

[Kubilay Kocak]

On 5/04/2021 9:44 pm, Kristof Provost wrote:
> The branch main has been updated by kp:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=829a69db855b48ff7e8242b95e193a0783c489d9
> 
> commit 829a69db855b48ff7e8242b95e193a0783c489d9
> Author:     Kristof Provost <kp at FreeBSD.org>
> AuthorDate: 2021-04-02 10:23:42 +0000
> Commit:     Kristof Provost <kp at FreeBSD.org>
> CommitDate: 2021-04-05 07:57:06 +0000
> 
>      pf: change pf_route so pf only runs when packets enter and leave the stack.
>      
>      before this change pf_route operated on the semantic that pf runs
>      when packets go over an interface, so when pf_route changed which
>      interface the packet was on it would run pf_test again. this change
>      changes (restores) the semantic that pf is only supposed to run
>      when packets go in or out of the network stack, even if route-to
>      is responsibly for short circuiting past the network stack.
>      
>      just to be clear, for normal packets (ie, those not touched by
>      route-to/reply-to/dup-to), there isn't a difference between running
>      pf when packets enter or leave the stack, or having pf run when a
>      packet goes over an interface.
>      
>      the main reason for this change is that running the same packet
>      through pf multiple times creates confusion for the state table.
>      by default, pf states are floating, meaning that packets are matched
>      to states regardless of which interface they're going over. if a
>      packet leaving on em0 is rerouted out em1, both traversals will end
>      up using the same state, which at best will make the accounting
>      look weird, or at worst fail some checks in the state and get
>      dropped.
>      
>      another reason for this commit is is to make handling of the changes
>      that route-to makes consistent with other changes that are made to
>      packet. eg, when nat is applied to a packet, we don't run pf_test
>      again with the new addresses.
>      
>      the main caveat with this diff is you can't have one rule that
>      pushes a packet out a different interface, and then have a rule on
>      that second interface that NATs the packet. i'm not convinced this
>      ever worked reliably or was used much anyway, so we don't think
>      it's a big concern.
>      
>      discussed with many, with special thanks to bluhm@, sashan@ and
>      sthen@ for weathering most of that pain.
>      ok claudio@ sashan@ jmatthew@
>      
>      Obtained from:  OpenBSD
>      MFC after:      2 weeks
>      Sponsored by:   Rubicon Communications, LLC ("Netgate")
>      Differential Revision:  https://reviews.freebsd.org/D29554

Relnotes: Yes

For the rule semantics change?


> ---
>   sys/netpfil/pf/pf.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
> index 50bf4b3871c5..5b41be4ad683 100644
> --- a/sys/netpfil/pf/pf.c
> +++ b/sys/netpfil/pf/pf.c
> @@ -5549,7 +5549,7 @@ pf_route(struct mbuf **m, struct pf_krule *r, int dir, struct ifnet *oifp,
>   	if (ifp == NULL)
>   		goto bad;
>   
> -	if (oifp != ifp) {
> +	if (dir == PF_IN) {
>   		if (pf_test(PF_OUT, 0, ifp, &m0, inp) != PF_PASS)
>   			goto bad;
>   		else if (m0 == NULL)
> @@ -5738,7 +5738,7 @@ pf_route6(struct mbuf **m, struct pf_krule *r, int dir, struct ifnet *oifp,
>   	if (ifp == NULL)
>   		goto bad;
>   
> -	if (oifp != ifp) {
> +	if (dir == PF_IN) {
>   		if (pf_test6(PF_OUT, PFIL_FWD, ifp, &m0, inp) != PF_PASS)
>   			goto bad;
>   		else if (m0 == NULL)
> _______________________________________________
> dev-commits-src-main at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main
> To unsubscribe, send any mail to "dev-commits-src-main-unsubscribe at freebsd.org"
>