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 a3d8d63..7bd6dfd 100644 (file)
--- 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 1331970..e9e80f6 100644 (file)
--- 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 */