From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: af-nat of ICMP error messages leaves IP header ttl 0
To: Alexandr Nedvedicky <sashan@fastmail.net>
Cc: tech@openbsd.org
Date: Tue, 5 Nov 2024 16:14:24 +0100

On Sun, Aug 25, 2024 at 02:07:21PM +0200, Alexandr Nedvedicky wrote:
> Hello,
> 
> I've hit this bug when investigating an issue kindly reported
> by jhealy here [1]. I've noticed TTL in ip header of translated
> ICMP message is 0. This is caused by pf(4) when it translates
> ICMPv6 to ICMPv4 (and vice-versa) in function pf_test_state_icmp()
> where we call pf_change_icmp_af():
> 
>     6265
>     6266            if (afto) {
>     6267                    if (nk->af != AF_INET)
>     6268                            return (PF_DROP);
>     6269                    if (pf_translate_icmp_af(pd, nk->af,
>     6270                        &pd->hdr.icmp))
>     6271                            return (PF_DROP);
>     6272                    m_copyback(pd->m, pd->off,
>     6273                        sizeof(struct icmp6_hdr),
>     6274                        &pd->hdr.icmp6, M_NOWAIT);
>     6275                    if (pf_change_icmp_af(pd->m, ipoff2,
>     6276                        pd, &pd2, &nk->addr[sidx],
>     6277                        &nk->addr[didx], pd->af, nk->af))
>     6278                            return (PF_DROP);
> 
> the function pf_change_icmp_af() receives to pf_pdesc arguments
> pd and pd2. The ttl for packet header is grabbed from pd2.
> the ttl member in pd2 is zero. Oneliner below fixes that.
> I believe it fixes af-nat of ICMP error handling for TCP
> and UDP icmp error payloads too.
> 
> OK to commit?

pd->ttl is the outer ttl.  You need the inner ttl in pd2.
Try this

		case AF_INET:
			...
			pd2->ttl = h2.ip_ttl;
		case AF_INET6:
			...
			pd2->ttl = h2_6.ip6_hlim;

bluhm

> 
> thanks and
> regards
> sashan
> 
> [1] https://marc.info/?l=openbsd-bugs&m=172380201729124&w=2
> 
> --------8<---------------8<---------------8<------------------8<--------
> diff --git a/sys/net/pf.c b/sys/net/pf.c
> index a5eac9edbeb..1f211b13ac5 100644
> --- a/sys/net/pf.c
> +++ b/sys/net/pf.c
> @@ -5727,6 +5727,7 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **stp,
>  		/* Payload packet is from the opposite direction. */
>  		pd2.sidx = (pd2.dir == PF_IN) ? 1 : 0;
>  		pd2.didx = (pd2.dir == PF_IN) ? 0 : 1;
> +		pd2.ttl = pd->ttl;
>  		switch (pd->af) {
>  		case AF_INET:
>  			/* offset of h2 in mbuf chain */