Index | Thread | Search

From:
Alexandr Nedvedicky <sashan@fastmail.net>
Subject:
af-nat of ICMP error messages leaves IP header ttl 0
To:
tech@openbsd.org
Date:
Sun, 25 Aug 2024 14:07:21 +0200

Download raw body.

Thread
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?

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 */