Re: [External] : don't run dup-to generated packets through pf_test in pf_route{,6}

2021-01-26 Thread Alexandr Nedvedicky
On Wed, Jan 27, 2021 at 11:31:27AM +1000, David Gwynne wrote:
> this was discussed as part of the big route-to issues thread. i think
> it's easy to break out and handle separately now.
> 
> the diff does what the subject line says. it seems to work as expected
> for me. i don't see weird state issues anymore when i dup my ssh session
> out over a tunnel interface.
> 
> sasha suggested setting PF_TAG_GENERATED on the duplicated packet, but i
> didn't set it in this diff. the reason is that i can't see
> PF_TAG_GENERATED get cleared anywhere. this means that if you dup-to a
> host over a tunnel (eg, gif, gre, etc), the encapsulated packet still
> has that tag, which means pf doesn't run against the encapsulated
> packet.

I'm taking a note to take a look. this feels like a bug to me.

> 
> ok?

OK sashan@
> 
> Index: pf.c
> ===
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.1101
> diff -u -p -r1.1101 pf.c
> --- pf.c  19 Jan 2021 22:22:23 -  1.1101
> +++ pf.c  27 Jan 2021 01:21:24 -
> @@ -6039,7 +6041,7 @@ pf_route(struct pf_pdesc *pd, struct pf_
>   if (ifp == NULL)
>   goto bad;
>  
> - if (pd->kif->pfik_ifp != ifp) {
> + if (r->rt != PF_DUPTO && pd->kif->pfik_ifp != ifp) {
>   if (pf_test(AF_INET, PF_OUT, ifp, &m0) != PF_PASS)
>   goto bad;
>   else if (m0 == NULL)
> @@ -6194,7 +6195,7 @@ pf_route6(struct pf_pdesc *pd, struct pf
>   if (ifp == NULL)
>   goto bad;
>  
> - if (pd->kif->pfik_ifp != ifp) {
> + if (r->rt != PF_DUPTO && pd->kif->pfik_ifp != ifp) {
>   if (pf_test(AF_INET6, PF_OUT, ifp, &m0) != PF_PASS)
>   goto bad;
>   else if (m0 == NULL)
> 



Re: don't run dup-to generated packets through pf_test in pf_route{,6}

2021-01-26 Thread Alexander Bluhm
On Wed, Jan 27, 2021 at 11:31:27AM +1000, David Gwynne wrote:
> this was discussed as part of the big route-to issues thread. i think
> it's easy to break out and handle separately now.
> 
> the diff does what the subject line says. it seems to work as expected
> for me. i don't see weird state issues anymore when i dup my ssh session
> out over a tunnel interface.
> 
> sasha suggested setting PF_TAG_GENERATED on the duplicated packet, but i
> didn't set it in this diff. the reason is that i can't see
> PF_TAG_GENERATED get cleared anywhere. this means that if you dup-to a
> host over a tunnel (eg, gif, gre, etc), the encapsulated packet still
> has that tag, which means pf doesn't run against the encapsulated
> packet.
> 
> ok?

OK bluhm@

> Index: pf.c
> ===
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.1101
> diff -u -p -r1.1101 pf.c
> --- pf.c  19 Jan 2021 22:22:23 -  1.1101
> +++ pf.c  27 Jan 2021 01:21:24 -
> @@ -6039,7 +6041,7 @@ pf_route(struct pf_pdesc *pd, struct pf_
>   if (ifp == NULL)
>   goto bad;
>  
> - if (pd->kif->pfik_ifp != ifp) {
> + if (r->rt != PF_DUPTO && pd->kif->pfik_ifp != ifp) {
>   if (pf_test(AF_INET, PF_OUT, ifp, &m0) != PF_PASS)
>   goto bad;
>   else if (m0 == NULL)
> @@ -6194,7 +6195,7 @@ pf_route6(struct pf_pdesc *pd, struct pf
>   if (ifp == NULL)
>   goto bad;
>  
> - if (pd->kif->pfik_ifp != ifp) {
> + if (r->rt != PF_DUPTO && pd->kif->pfik_ifp != ifp) {
>   if (pf_test(AF_INET6, PF_OUT, ifp, &m0) != PF_PASS)
>   goto bad;
>   else if (m0 == NULL)



don't run dup-to generated packets through pf_test in pf_route{,6}

2021-01-26 Thread David Gwynne
this was discussed as part of the big route-to issues thread. i think
it's easy to break out and handle separately now.

the diff does what the subject line says. it seems to work as expected
for me. i don't see weird state issues anymore when i dup my ssh session
out over a tunnel interface.

sasha suggested setting PF_TAG_GENERATED on the duplicated packet, but i
didn't set it in this diff. the reason is that i can't see
PF_TAG_GENERATED get cleared anywhere. this means that if you dup-to a
host over a tunnel (eg, gif, gre, etc), the encapsulated packet still
has that tag, which means pf doesn't run against the encapsulated
packet.

ok?

Index: pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.1101
diff -u -p -r1.1101 pf.c
--- pf.c19 Jan 2021 22:22:23 -  1.1101
+++ pf.c27 Jan 2021 01:21:24 -
@@ -6039,7 +6041,7 @@ pf_route(struct pf_pdesc *pd, struct pf_
if (ifp == NULL)
goto bad;
 
-   if (pd->kif->pfik_ifp != ifp) {
+   if (r->rt != PF_DUPTO && pd->kif->pfik_ifp != ifp) {
if (pf_test(AF_INET, PF_OUT, ifp, &m0) != PF_PASS)
goto bad;
else if (m0 == NULL)
@@ -6194,7 +6195,7 @@ pf_route6(struct pf_pdesc *pd, struct pf
if (ifp == NULL)
goto bad;
 
-   if (pd->kif->pfik_ifp != ifp) {
+   if (r->rt != PF_DUPTO && pd->kif->pfik_ifp != ifp) {
if (pf_test(AF_INET6, PF_OUT, ifp, &m0) != PF_PASS)
goto bad;
else if (m0 == NULL)