From: Alexandr Nedvedicky <sashan@fastmail.net>
Subject: Re: pf af-to breaks traceroute
To: Alexander Bluhm <alexander.bluhm@gmx.net>
Cc: Kristof Provost <kp@freebsd.org>, tech@openbsd.org
Date: Mon, 3 Mar 2025 01:28:43 +0100

Hello,

thanks for giving it a shot on regress. The failure was
caused by the first chunk in earlier diff. I've just
added the change to branch if (pf_icmp_mappin(...) == 0)
which deals with echo request/reply. This was wrong and
I think Kristof also has pointed that out.

The updated diff passes pf_foward test. 

I've also re-arranged code in ICMP branch. Hope I got
your suggestion right.

OK to commit diff below?

thanks and
regards
sashan

--------8<---------------8<---------------8<------------------8<--------

diff --git a/sys/net/pf.c b/sys/net/pf.c
index 6d6dbfd3c14..783b83df45c 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -5925,17 +5925,27 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **stp,
 					    pd, &pd2, &nk->addr[sidx],
 					    &nk->addr[didx], pd->af, nk->af))
 						return (PF_DROP);
-					if (nk->af == AF_INET)
-						pd->proto = IPPROTO_ICMP;
-					else
-						pd->proto = IPPROTO_ICMPV6;
 					pd->m->m_pkthdr.ph_rtableid =
 					    nk->rdomain;
 					pd->destchg = 1;
-					pf_addrcpy(&pd->nsaddr,
-					    &nk->addr[pd2.sidx], nk->af);
 					pf_addrcpy(&pd->ndaddr,
 					    &nk->addr[pd2.didx], nk->af);
+					pf_addrcpy(&pd->nsaddr,
+					    &nk->addr[pd2.sidx], nk->af);
+					if (nk->af == AF_INET) {
+						pd->proto = IPPROTO_ICMP;
+					} else {
+						pd->proto = IPPROTO_ICMPV6;
+						/*
+						 * IPv4 becomes IPv6 so we must
+						 * copy IPv4 src addr to least
+						 * 32bits in IPv6 address to
+						 * keep traceroute/icmp
+						 * working.
+						 */
+						pd->nsaddr.addr32[3] =
+						    pd->src->addr32[0];
+					}
 					pd->naf = nk->af;
 
 					pf_patch_16(pd,
@@ -6045,17 +6055,27 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **stp,
 					    pd, &pd2, &nk->addr[sidx],
 					    &nk->addr[didx], pd->af, nk->af))
 						return (PF_DROP);
-					if (nk->af == AF_INET)
-						pd->proto = IPPROTO_ICMP;
-					else
-						pd->proto = IPPROTO_ICMPV6;
 					pd->m->m_pkthdr.ph_rtableid =
 					    nk->rdomain;
 					pd->destchg = 1;
-					pf_addrcpy(&pd->nsaddr,
-					    &nk->addr[pd2.sidx], nk->af);
 					pf_addrcpy(&pd->ndaddr,
 					    &nk->addr[pd2.didx], nk->af);
+					pf_addrcpy(&pd->nsaddr,
+					    &nk->addr[pd2.sidx], nk->af);
+					if (nk->af == AF_INET) {
+						pd->proto = IPPROTO_ICMP;
+					} else {
+						pd->proto = IPPROTO_ICMPV6;
+						/*
+						 * IPv4 becomes IPv6 so we must
+						 * copy IPv4 src addr to least
+						 * 32bits in IPv6 address to
+						 * keep traceroute/icmp
+						 * working.
+						 */
+						pd->nsaddr.addr32[3] =
+						    pd->src->addr32[0];
+					}
 					pd->naf = nk->af;
 
 					pf_patch_16(pd,
@@ -6186,6 +6206,13 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **stp,
 					    &nk->addr[pd2.sidx], nk->af);
 					pf_addrcpy(&pd->ndaddr,
 					    &nk->addr[pd2.didx], nk->af);
+					/*
+					 * IPv4 becomes IPv6 so we must copy
+					 * IPv4 src addr to least 32bits in
+					 * IPv6 address to keep traceroute
+					 * working.
+					 */
+					pd->nsaddr.addr32[3] = pd->src->addr32[0];
 					pd->naf = nk->af;
 					return (PF_AFRT);
 				}