git: a6e1dde287eb - main - pf: Do not corrupt neighbour advertisements when translating

The branch main has been updated by rcm:

URL: https://cgit.FreeBSD.org/src/commit/?id=a6e1dde287eb3854873224200cea40996b0b5cbf

commit a6e1dde287eb3854873224200cea40996b0b5cbf
Author:     R. Christian McDonald <rcm@FreeBSD.org>
AuthorDate: 2026-10-13 15:58:55 +0000
Commit:     R. Christian McDonald <rcm@FreeBSD.org>
CommitDate: 2026-10-13 16:02:39 +0000

    pf: Do not corrupt neighbour advertisements when translating
    
    When an ICMPv6 query or reply matches a translated state,
    pf_test_state_icmp() copies the state's virtual id into icmp6_id.  Only
    echo messages have an identifier there.  For all other types the
    virtual id is 0 and those two bytes hold something else; in a neighbour
    advertisement they hold the R, S and O flags.  The checksum was not
    updated either, so icmp6_input() dropped the advertisement as a bad
    checksum.
    
    Unicast NUD probes to a link-local neighbour, such as the default
    router, are sent from the link-local address.  With a nat rule that
    matches it (e.g. "from any"), every advertisement answering a probe was
    dropped: NUD failed, the neighbour cache entry was deleted, and traffic
    to the neighbour stalled until it was resolved again.
    
    Only rewrite the id of echo messages, as the ICMPv6 error path already
    does, and update the checksum, as the IPv4 path does.
    
    Add a regression test.
    
    Reviewed by:            kp
    Fixes:                  f1ddd7f1dae6 ("pf: add forgotten fixup for icmp6 id's when translating")
    See also:               https://redmine.pfsense.org/issues/16236
    MFC after:              1 week
    Sponsored by:           Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D60542
---
 sys/netpfil/pf/pf.c           | 14 ++++++++-
 tests/sys/netpfil/pf/icmp6.sh | 73 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 817790ba0ef0..00990e17d0c8 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -8856,9 +8856,21 @@ pf_test_state_icmp(struct pf_kstate **state, struct pf_pdesc *pd,
 					    &pd->hdr.icmp6.icmp6_cksum,
 					    &nk->addr[didx], 0);
 
-				if (nk->port[iidx] != pd->hdr.icmp6.icmp6_id)
+				/*
+				 * Only echo has an identifier.  Other
+				 * types use a virtual id of 0 and keep
+				 * other fields here (e.g. the NA flags).
+				 */
+				if (virtual_type == htons(ICMP6_ECHO_REQUEST) &&
+				    nk->port[iidx] != pd->hdr.icmp6.icmp6_id) {
+					pd->hdr.icmp6.icmp6_cksum =
+					    pf_cksum_fixup(
+					    pd->hdr.icmp6.icmp6_cksum,
+					    pd->hdr.icmp6.icmp6_id,
+					    nk->port[iidx], 0);
 					pd->hdr.icmp6.icmp6_id =
 					    nk->port[iidx];
+				}
 
 				m_copyback(pd->m, pd->off, sizeof(struct icmp6_hdr),
 				    (caddr_t )&pd->hdr.icmp6);
diff --git a/tests/sys/netpfil/pf/icmp6.sh b/tests/sys/netpfil/pf/icmp6.sh
index c55af906e3a6..acce09dd45c5 100644
--- a/tests/sys/netpfil/pf/icmp6.sh
+++ b/tests/sys/netpfil/pf/icmp6.sh
@@ -196,9 +196,82 @@ repeat_cleanup()
 	pft_cleanup
 }
 
+atf_test_case "nat_nud" "cleanup"
+nat_nud_head()
+{
+	atf_set descr 'Test that NUD works when pf translates the probes'
+	atf_set require.user root
+}
+
+nat_nud_body()
+{
+	pft_init
+
+	epair=$(vnet_mkepair)
+
+	vnet_mkjail alcatraz ${epair}a
+	jexec alcatraz ifconfig ${epair}a inet6 no_dad
+	jexec alcatraz ifconfig ${epair}a inet6 2001:db8::1/64 up
+	# Probe a stale neighbour one second after using it, not five.
+	jexec alcatraz sysctl net.inet6.icmp6.nd6_delay=1
+
+	vnet_mkjail singsing ${epair}b
+	jexec singsing ifconfig ${epair}b inet6 no_dad
+	jexec singsing ifconfig ${epair}b inet6 2001:db8::2/64 up
+
+	lla=$(jexec alcatraz ifconfig ${epair}a inet6 | \
+	    awk '/fe80:/ { sub(/%.*/, "", $2); print $2 }')
+	llb=$(jexec singsing ifconfig ${epair}b inet6 | \
+	    awk '/fe80:/ { sub(/%.*/, "", $2); print $2 }')
+
+	# NUD probes are sent from the link-local address, so this translates
+	# them, and the advertisements answering them come back to 2001:db8::1.
+	jexec alcatraz pfctl -e
+	pft_set_rules alcatraz \
+	    "nat on ${epair}a inet6 from fe80::/10 to any -> 2001:db8::1" \
+	    "pass"
+
+	# singsing resolving alcatraz leaves it stale in alcatraz's neighbour
+	# cache, and alcatraz answering uses it, so alcatraz will probe it.
+	atf_check -s exit:0 -o ignore \
+	    jexec singsing ping -6 -c 1 ${lla}%${epair}b
+
+	# The advertisement answering the probe makes the neighbour reachable.
+	# A corrupted one is dropped: alcatraz probes twice more, then deletes
+	# the entry.
+	state=
+	for i in $(seq 1 15); do
+		state=$(jexec alcatraz ndp -n ${llb}%${epair}a 2>/dev/null | \
+		    awk 'NR == 2 { print $5 }')
+		if [ "${state}" = "R" ]; then
+			break
+		fi
+		sleep 1
+	done
+
+	jexec alcatraz ndp -an
+	jexec alcatraz pfctl -ss -vv
+
+	if [ "${state}" != "R" ]; then
+		atf_fail "NUD failed, neighbour state '${state:-no entry}'"
+	fi
+	atf_check -o match:"^[[:space:]]*0 bad checksums$" \
+	    jexec alcatraz netstat -s -p icmp6
+
+	# The probe was translated
+	atf_check -o match:"2001:db8::1 \(${lla}\) -> ${llb}\[135\]" \
+	    jexec alcatraz pfctl -ss
+}
+
+nat_nud_cleanup()
+{
+	pft_cleanup
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case "zero_id"
 	atf_add_test_case "ttl_exceeded"
 	atf_add_test_case "repeat"
+	atf_add_test_case "nat_nud"
 }