Index | Thread | Search

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

Download raw body.

Thread
  • Alexander Bluhm:

    pf af-to breaks traceroute

  • 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);
     				}
    
    
  • Alexander Bluhm:

    pf af-to breaks traceroute