Download raw body.
pf af-to breaks traceroute
This diff break 2 test cases in regress/sys/net/pf_forward
*** Error 1 in . (Makefile:269 'run-traceroute-icmp-inet-AF_IN')
FAILED
--
*** Error 1 in . (Makefile:178 'run-ping-inet6-AF_IN')
FAILED
--
*** Error 1 in . (Makefile:269 'run-traceroute-icmp-inet6-AF_IN')
An obvious problem is that it changes the source addres of the echo
reply packet.
root@ot1:.../pf_forward# make run-ping-inet6-AF_IN
Check ping AF_IN6:
ping6 -n -c 1 fdd7:e83e:66bc:87::18
PING fdd7:e83e:66bc:87::18 (fdd7:e83e:66bc:87::18): 56 data bytes
--- fdd7:e83e:66bc:87::18 ping statistics ---
1 packets transmitted, 0 packets received, 100.0% packet loss
*** Error 1 in /usr/src/regress/sys/net/pf_forward (Makefile:178 'run-ping-inet6-AF_IN')
tcpdump on PF host:
16:08:43.074888 fdd7:e83e:66bc:81::21 > fdd7:e83e:66bc:87::18: icmp6: echo request
16:08:43.074986 10.188.82.22 > 10.188.83.24: icmp: echo request (DF)
16:08:43.075318 10.188.83.24 > 10.188.82.22: icmp: echo reply (DF)
16:08:43.075383 fdd7:e83e:66bc:87::abc:5318 > fdd7:e83e:66bc:81::21: icmp6: echo reply
16:08:43.075534 fdd7:e83e:66bc:81::21 > fdd7:e83e:66bc:87::abc:5318: icmp6: fdd7:e83e:66bc:81::21 protocol 58 port 16085 unreachable
The address fdd7:e83e:66bc:87::abc:5318 is not recocnized by the
pinging host. While translated source address is good for traceroute,
ping does not work that way.
Also in the final chunk if (nk->af == AF_INET) { pd->proto =
IPPROTO_ICMP; is not necessary. We are in case case IPPROTO_ICMP,
note that there is an unconditinal pd->proto = IPPROTO_ICMPV6 a few
lines above.
Maybe you should net reorder the code to keep it consistent with
case IPPROTO_ICMPV6.
In general I like your idea.
bluhm
On Thu, Feb 27, 2025 at 02:22:16AM +0100, Alexandr Nedvedicky wrote:
> --------8<---------------8<---------------8<------------------8<--------
> diff --git a/sys/net/pf.c b/sys/net/pf.c
> index 6d6dbfd3c14..ce2cb7c2a9c 100644
> --- a/sys/net/pf.c
> +++ b/sys/net/pf.c
> @@ -5650,6 +5650,16 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **stp,
> if (afto) {
> pf_addrcpy(&pd->nsaddr, &nk->addr[sidx],
> nk->af);
> + if (nk->af == AF_INET6) {
> + /*
> + * IPv4 becomes IPv6 so we must put
> + * IPv4 src addr to least 32bits in
> + * IPv6 address to keep traceroute/icmp
> + * working.
> + */
> + pd->nsaddr.addr32[3] =
> + pd->src->addr32[0];
> + }
> pf_addrcpy(&pd->ndaddr, &nk->addr[didx],
> nk->af);
> pd->naf = nk->af;
> @@ -5925,17 +5935,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
> + * put 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 +6065,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
> + * put 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,
> @@ -6182,10 +6212,24 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **stp,
> 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
> + * put 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;
> return (PF_AFRT);
> }
pf af-to breaks traceroute