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: Thu, 7 Nov 2024 18:04:52 +0100

On Thu, Nov 07, 2024 at 04:50:07PM +0100, Alexandr Nedvedicky wrote:
> Hello,
> 
> On Tue, Nov 05, 2024 at 04:14:24PM +0100, Alexander Bluhm wrote:
> > On Sun, Aug 25, 2024 at 02:07:21PM +0200, Alexandr Nedvedicky wrote:
> </sbip>
> > > 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
> 
>     yes, you arr right. updated diff is below.
>     thanks for spotting that.
> 
> regards
> sashan

OK bluhm@

> diff --git a/sys/net/pf.c b/sys/net/pf.c
> index 29aee94f42f..cf4cbba48bd 100644
> --- a/sys/net/pf.c
> +++ b/sys/net/pf.c
> @@ -5753,6 +5753,7 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **stp,
>  				return (PF_DROP);
>  
>  			pd2.tot_len = ntohs(h2.ip_len);
> +			pd2.ttl = h2.ip_ttl;
>  			pd2.src = (struct pf_addr *)&h2.ip_src;
>  			pd2.dst = (struct pf_addr *)&h2.ip_dst;
>  			break;
> @@ -5773,6 +5774,7 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **stp,
>  
>  			pd2.tot_len = ntohs(h2_6.ip6_plen) +
>  			    sizeof(struct ip6_hdr);
> +			pd2.ttl = h2_6.ip6_hlim;
>  			pd2.src = (struct pf_addr *)&h2_6.ip6_src;
>  			pd2.dst = (struct pf_addr *)&h2_6.ip6_dst;
>  			break;