From: sashan <sashan@openbsd.org>
Date: Wed, 10 May 2023 22:42:51 +0000 (+0000)
Subject: nat-to may fail to insert state due to conflict on chosen source
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=e8d816753b9af4d465375cdb9e00ae6f5f109a36;p=openbsd

nat-to may fail to insert state due to conflict on chosen source
port number. This is typically indicated by 'wire key attach failed on...'
message when pf(4) debugging is enabled. The problem is caused by
glitch in pf_get_sport() which fails to discover conflict in advance.
In order to fix it we must also calculate toeplitz hash in
pf_get_sport() to initialize look up key properly.

the bug has been kindly reported by  joosepm _von_ gmail _dot_ com

OK dlg@
---

diff --git a/sys/net/pf_lb.c b/sys/net/pf_lb.c
index a3d8d6333cd..7bd6dfd607b 100644
--- a/sys/net/pf_lb.c
+++ b/sys/net/pf_lb.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_lb.c,v 1.73 2023/01/04 10:31:55 dlg Exp $ */
+/*	$OpenBSD: pf_lb.c,v 1.74 2023/05/10 22:42:51 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -196,18 +196,24 @@ pf_get_sport(struct pf_pdesc *pd, struct pf_rule *r,
 			/* XXX bug: icmp states dont use the id on both
 			 * XXX sides (traceroute -I through nat) */
 			key.port[sidx] = pd->nsport;
+			key.hash = pf_pkt_hash(key.af, key.proto, &key.addr[0],
+			    &key.addr[1], key.port[0], key.port[1]);
 			if (pf_find_state_all(&key, dir, NULL) == NULL) {
 				*nport = pd->nsport;
 				return (0);
 			}
 		} else if (low == 0 && high == 0) {
 			key.port[sidx] = pd->nsport;
+			key.hash = pf_pkt_hash(key.af, key.proto, &key.addr[0],
+			    &key.addr[1], key.port[0], key.port[1]);
 			if (pf_find_state_all(&key, dir, NULL) == NULL) {
 				*nport = pd->nsport;
 				return (0);
 			}
 		} else if (low == high) {
 			key.port[sidx] = htons(low);
+			key.hash = pf_pkt_hash(key.af, key.proto, &key.addr[0],
+			    &key.addr[1], key.port[0], key.port[1]);
 			if (pf_find_state_all(&key, dir, NULL) == NULL) {
 				*nport = htons(low);
 				return (0);
@@ -225,6 +231,9 @@ pf_get_sport(struct pf_pdesc *pd, struct pf_rule *r,
 			/* low <= cut <= high */
 			for (tmp = cut; tmp <= high && tmp <= 0xffff; ++tmp) {
 				key.port[sidx] = htons(tmp);
+				key.hash = pf_pkt_hash(key.af, key.proto,
+				    &key.addr[0], &key.addr[1], key.port[0],
+				    key.port[1]);
 				if (pf_find_state_all(&key, dir, NULL) ==
 				    NULL && !in_baddynamic(tmp, pd->proto)) {
 					*nport = htons(tmp);
@@ -234,6 +243,9 @@ pf_get_sport(struct pf_pdesc *pd, struct pf_rule *r,
 			tmp = cut;
 			for (tmp -= 1; tmp >= low && tmp <= 0xffff; --tmp) {
 				key.port[sidx] = htons(tmp);
+				key.hash = pf_pkt_hash(key.af, key.proto,
+				    &key.addr[0], &key.addr[1], key.port[0],
+				    key.port[1]);
 				if (pf_find_state_all(&key, dir, NULL) ==
 				    NULL && !in_baddynamic(tmp, pd->proto)) {
 					*nport = htons(tmp);
diff --git a/sys/net/pfvar_priv.h b/sys/net/pfvar_priv.h
index 13319709da1..e9e80f6196f 100644
--- a/sys/net/pfvar_priv.h
+++ b/sys/net/pfvar_priv.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfvar_priv.h,v 1.32 2023/05/08 23:52:36 dlg Exp $	*/
+/*	$OpenBSD: pfvar_priv.h,v 1.33 2023/05/10 22:42:51 sashan Exp $	*/
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -405,6 +405,9 @@ void			pf_state_peer_hton(const struct pf_state_peer *,
 			    struct pfsync_state_peer *);
 void			pf_state_peer_ntoh(const struct pfsync_state_peer *,
 			    struct pf_state_peer *);
+u_int16_t		pf_pkt_hash(sa_family_t, uint8_t,
+			    const struct pf_addr *, const struct pf_addr *,
+			    uint16_t, uint16_t);
 
 #endif /* _KERNEL */