From nobody Mon Aug 19 14:37:38 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4WnZv73dmyz5T8Gp; Mon, 19 Aug 2024 14:37:39 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4WnZv71Ll4z4ttt; Mon, 19 Aug 2024 14:37:39 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1724078259; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=YE/Xa2Ou7Z00oGYVesANvoKbVMj5wm2WudVJ3sFu2Ns=; b=Yl1T3ovuOtwEendOi7s/l17udJRJIg94SQNod6S1tid4cNhtf0TXKSbWmknnN84CsCYTTC AR/yL6I6n2HYXzVZ5o3E7mm2rm7c68Ht5oIec0WPBKPw02lCmMzwwcWxPS97qxRjggntoE XfzBG5K2SMBGqUpvCdPHqgu/zcJuaEGwRfACc/QltDcts2FQ24RozLS1Yl1ikmWtSodBGb /E24Ts2Wgqe6dJ+53CSnWhi/kTqP0xcOA6P04TvqA7KmsSbAVwu3SUIldvWQoyg0WVXNrU osw5oZtZK+9gZiTf7Hnraqn8PsVUel8pHCdVaK14bRmOF9F+x1rwPKSlZkSpEg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1724078259; a=rsa-sha256; cv=none; b=nkhx3zcNgbRMxvP+T1DkVuXlLFtaP0tSxNhv8HeO+3q0+lfNC7nV7ndGv0i1MffKEeIaHW qR+BSjGgAdm/TeWfJE3Cnc80X37pildaC4jcDUb5r2U3X5Cf/T0sEBOu8hPBksHx/84SFS TCTLYxTHq4FtfLtB/Rj6xNhyx4uJ82q8iPPlfvaUw86mGAgg90MkgZa3jZ9smQ/JhOYEbh tVmebsi6SoTtGlwDzRatNuBiyfjS1EBUY27txj4AHl1zVZjDs/QYRwbzSo78TIoKLAivSb k4DZ6QMkkOIDt9+FhiEdEKe3Df/7zTX7J6PCyJE5c3EPKNrtRhptwJc7Gmc03w== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1724078259; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=YE/Xa2Ou7Z00oGYVesANvoKbVMj5wm2WudVJ3sFu2Ns=; b=T4HoL4v2NKFAdT1NnKOcKFMle464pyj1ww3NAUHGO2jmJdz8CHGn14Vrm5nZAHnu5oE1pP 3hFoP7Aa9/k+sQtzc/YuSAkgIVw+ebgLtKAPsKZj1TWkNgfDiWp0gz93t3RhSzj5eqidqC 1MY5ryZws7UOc8Ptz8In7u1HKmj1uWSDJNun9++Gfh407FqjMXUk9kUH//dC/l2hJ5pcZx eXU1lNYdkocRdPSSDiKkI3x2y4nf8XnKxltKceLu3hyL97UwbjzljLY7Qrf9YGd1QgPdic Ze+VN7e4iIulLjkT8iikqnJgJCe+5pU9XDlqkNqjKuxiet2FPeJczAs1ILuKIA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4WnZv70bh4zL1W; Mon, 19 Aug 2024 14:37:39 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 47JEbcIQ094096; Mon, 19 Aug 2024 14:37:38 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 47JEbco3094093; Mon, 19 Aug 2024 14:37:38 GMT (envelope-from git) Date: Mon, 19 Aug 2024 14:37:38 GMT Message-Id: <202408191437.47JEbco3094093@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: 9897a66923a3 - main - pf: Let rdr rules modify the src port if doing so would avoid a conflict List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 9897a66923a3e79c22fcbd4bc80afae9eb9f277c Auto-Submitted: auto-generated The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=9897a66923a3e79c22fcbd4bc80afae9eb9f277c commit 9897a66923a3e79c22fcbd4bc80afae9eb9f277c Author: Mark Johnston AuthorDate: 2024-08-19 14:08:44 +0000 Commit: Mark Johnston CommitDate: 2024-08-19 14:37:27 +0000 pf: Let rdr rules modify the src port if doing so would avoid a conflict If NAT rules cause inbound connections to different external IPs to be mapped to the same internal IP, and some application uses the same source port for multiple such connections, rdr translation may result in conflicts that cause some of the connections to be dropped. Address this by letting rdr rules detect state conflicts and modulate the source port to avoid them. Reviewed by: kp, allanjude MFC after: 3 months Sponsored by: Klara, Inc. Sponsored by: Modirum Differential Revision: https://reviews.freebsd.org/D44488 --- share/man/man5/pf.conf.5 | 9 +++- sys/netpfil/pf/pf_lb.c | 70 ++++++++++++++++++++++--- tests/sys/netpfil/pf/Makefile | 1 + tests/sys/netpfil/pf/rdr-srcport.py | 20 ++++++++ tests/sys/netpfil/pf/rdr.sh | 100 ++++++++++++++++++++++++++++++++++++ 5 files changed, 191 insertions(+), 9 deletions(-) diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5 index da55f00293bb..f04b0799741e 100644 --- a/share/man/man5/pf.conf.5 +++ b/share/man/man5/pf.conf.5 @@ -27,7 +27,7 @@ .\" ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE .\" POSSIBILITY OF SUCH DAMAGE. .\" -.Dd June 6, 2024 +.Dd June 24, 2024 .Dt PF.CONF 5 .Os .Sh NAME @@ -1400,9 +1400,14 @@ or .Xr udp 4 connections; implicitly in the case of .Ar nat -rules and explicitly in the case of +rules and both implicitly and explicitly in the case of .Ar rdr rules. +A +.Ar rdr +rule may cause the source port to be modified if doing so avoids a conflict +with an existing connection. +A random source port in the range 50001-65535 is chosen in this case. Port numbers are never translated with a .Ar binat rule. diff --git a/sys/netpfil/pf/pf_lb.c b/sys/netpfil/pf/pf_lb.c index 4fcad7e578a8..4b703d3d02da 100644 --- a/sys/netpfil/pf/pf_lb.c +++ b/sys/netpfil/pf/pf_lb.c @@ -600,7 +600,7 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, { struct pf_krule *r = NULL; struct pf_addr *naddr; - uint16_t *nport; + uint16_t *nportp; uint16_t low, high; PF_RULES_RASSERT(); @@ -643,9 +643,8 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, return (NULL); } - /* XXX We only modify one side for now. */ naddr = &(*nkp)->addr[1]; - nport = &(*nkp)->port[1]; + nportp = &(*nkp)->port[1]; switch (r->action) { case PF_NAT: @@ -658,7 +657,7 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, } if (r->rpool.mape.offset > 0) { if (pf_get_mape_sport(pd->af, pd->proto, r, saddr, - sport, daddr, dport, naddr, nport, sn)) { + sport, daddr, dport, naddr, nportp, sn)) { DPFPRINTF(PF_DEBUG_MISC, ("pf: MAP-E port allocation (%u/%u/%u)" " failed\n", @@ -668,7 +667,7 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, goto notrans; } } else if (pf_get_sport(pd->af, pd->proto, r, saddr, sport, - daddr, dport, naddr, nport, low, high, sn)) { + daddr, dport, naddr, nportp, low, high, sn)) { DPFPRINTF(PF_DEBUG_MISC, ("pf: NAT proxy port allocation (%u-%u) failed\n", r->rpool.proxy_port[0], r->rpool.proxy_port[1])); @@ -742,6 +741,9 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, } break; case PF_RDR: { + struct pf_state_key_cmp key; + uint16_t cut, low, high, nport; + if (pf_map_addr(pd->af, r, saddr, naddr, NULL, NULL, sn)) goto notrans; if ((r->rpool.opts & PF_POOL_TYPEMASK) == PF_POOL_BITMASK) @@ -762,9 +764,63 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, /* Wrap around if necessary. */ if (tmp_nport > 65535) tmp_nport -= 65535; - *nport = htons((uint16_t)tmp_nport); + nport = htons((uint16_t)tmp_nport); } else if (r->rpool.proxy_port[0]) - *nport = htons(r->rpool.proxy_port[0]); + nport = htons(r->rpool.proxy_port[0]); + else + nport = dport; + + /* + * Update the destination port. + */ + *nportp = nport; + + /* + * Do we have a source port conflict in the stack state? Try to + * modulate the source port if so. Note that this is racy since + * the state lookup may not find any matches here but will once + * pf_create_state() actually instantiates the state. + */ + bzero(&key, sizeof(key)); + key.af = pd->af; + key.proto = pd->proto; + key.port[0] = sport; + PF_ACPY(&key.addr[0], saddr, key.af); + key.port[1] = nport; + PF_ACPY(&key.addr[1], naddr, key.af); + + if (!pf_find_state_all_exists(&key, PF_OUT)) + break; + + low = 50001; /* XXX-MJ PF_NAT_PROXY_PORT_LOW/HIGH */ + high = 65535; + cut = arc4random() % (1 + high - low) + low; + for (uint32_t tmp = cut; + tmp <= high && tmp <= UINT16_MAX; tmp++) { + key.port[0] = htons(tmp); + if (!pf_find_state_all_exists(&key, PF_OUT)) { + /* Update the source port. */ + (*nkp)->port[0] = htons(tmp); + goto out; + } + } + for (uint32_t tmp = cut - 1; tmp >= low; tmp--) { + key.port[0] = htons(tmp); + if (!pf_find_state_all_exists(&key, PF_OUT)) { + /* Update the source port. */ + (*nkp)->port[0] = htons(tmp); + goto out; + } + } + + DPFPRINTF(PF_DEBUG_MISC, + ("pf: RDR source port allocation failed\n")); + if (0) { +out: + DPFPRINTF(PF_DEBUG_MISC, + ("pf: RDR source port allocation %u->%u\n", + ntohs(sport), ntohs((*nkp)->port[0]))); + } break; } default: diff --git a/tests/sys/netpfil/pf/Makefile b/tests/sys/netpfil/pf/Makefile index 4a16642a967b..2b3cb9fbd858 100644 --- a/tests/sys/netpfil/pf/Makefile +++ b/tests/sys/netpfil/pf/Makefile @@ -73,6 +73,7 @@ ${PACKAGE}FILES+= CVE-2019-5597.py \ pfsync_defer.py \ pft_ether.py \ pft_read_ipfix.py \ + rdr-srcport.py \ utils.subr ${PACKAGE}FILESMODE_CVE-2019-5597.py= 0555 diff --git a/tests/sys/netpfil/pf/rdr-srcport.py b/tests/sys/netpfil/pf/rdr-srcport.py new file mode 100644 index 000000000000..633580582711 --- /dev/null +++ b/tests/sys/netpfil/pf/rdr-srcport.py @@ -0,0 +1,20 @@ +# +# A helper script which accepts TCP connections and writes the remote port +# number to the stream. +# + +import socket + +def main(): + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + s.bind(('0.0.0.0', 8888)) + s.listen(5) + + while True: + cs, addr = s.accept() + cs.sendall(str(addr[1]).encode()) + cs.close() + +if __name__ == '__main__': + main() diff --git a/tests/sys/netpfil/pf/rdr.sh b/tests/sys/netpfil/pf/rdr.sh index b7ec80b4d85e..135bfd42c1f4 100644 --- a/tests/sys/netpfil/pf/rdr.sh +++ b/tests/sys/netpfil/pf/rdr.sh @@ -121,7 +121,107 @@ tcp_v6_cleanup() pft_cleanup } + +atf_test_case "srcport" "cleanup" +srcport_head() +{ + atf_set descr 'TCP rdr srcport modulation' + atf_set require.user root + atf_set require.progs python3 + atf_set timeout 9999 +} + +# +# Test that rdr works for multiple TCP with same srcip and srcport. +# +# Four jails, a, b, c, d, are used: +# - jail d runs a server on port 8888, +# - jail a makes connections to the server, routed through jails b and c, +# - jail b uses NAT to rewrite source addresses and ports to the same 2-tuple, +# avoiding the need to use SO_REUSEADDR in jail a, +# - jail c uses a redirect rule to map the destination address to the same +# address and port, resulting in a NAT state conflict. +# +# In this case, the rdr rule should also rewrite the source port (again) to +# resolve the state conflict. +# +srcport_body() +{ + pft_init + + j="rdr:srcport" + epair1=$(vnet_mkepair) + epair2=$(vnet_mkepair) + epair3=$(vnet_mkepair) + + echo $epair_one + echo $epair_two + + vnet_mkjail ${j}a ${epair1}a + vnet_mkjail ${j}b ${epair1}b ${epair2}a + vnet_mkjail ${j}c ${epair2}b ${epair3}a + vnet_mkjail ${j}d ${epair3}b + + # configure addresses for a + jexec ${j}a ifconfig lo0 up + jexec ${j}a ifconfig ${epair1}a inet 198.51.100.50/24 up + jexec ${j}a ifconfig ${epair1}a inet alias 198.51.100.51/24 + jexec ${j}a ifconfig ${epair1}a inet alias 198.51.100.52/24 + + # configure addresses for b + jexec ${j}b ifconfig lo0 up + jexec ${j}b ifconfig ${epair1}b inet 198.51.100.1/24 up + jexec ${j}b ifconfig ${epair2}a inet 198.51.101.2/24 up + + # configure addresses for c + jexec ${j}c ifconfig lo0 up + jexec ${j}c ifconfig ${epair2}b inet 198.51.101.3/24 up + jexec ${j}c ifconfig ${epair2}b inet alias 198.51.101.4/24 + jexec ${j}c ifconfig ${epair2}b inet alias 198.51.101.5/24 + jexec ${j}c ifconfig ${epair3}a inet 203.0.113.1/24 up + + # configure addresses for d + jexec ${j}d ifconfig lo0 up + jexec ${j}d ifconfig ${epair3}b inet 203.0.113.50/24 up + + jexec ${j}b sysctl net.inet.ip.forwarding=1 + jexec ${j}c sysctl net.inet.ip.forwarding=1 + jexec ${j}b pfctl -e + jexec ${j}c pfctl -e + + pft_set_rules ${j}b \ + "set debug misc" \ + "nat on ${epair2}a inet from 198.51.100.0/24 to any -> ${epair2}a static-port" + + pft_set_rules ${j}c \ + "set debug misc" \ + "rdr on ${epair2}b proto tcp from any to ${epair2}b port 7777 -> 203.0.113.50 port 8888" + + jexec ${j}a route add default 198.51.100.1 + jexec ${j}c route add 198.51.100.0/24 198.51.101.2 + jexec ${j}d route add 198.51.101.0/24 203.0.113.1 + + jexec ${j}d python3 $(atf_get_srcdir)/rdr-srcport.py & + sleep 1 + + echo a | jexec ${j}a nc -w 3 -s 198.51.100.50 -p 1234 198.51.101.3 7777 > port1 + + jexec ${j}a nc -s 198.51.100.51 -p 1234 198.51.101.4 7777 > port2 & + jexec ${j}a nc -s 198.51.100.52 -p 1234 198.51.101.5 7777 > port3 & + sleep 1 + + atf_check -o inline:"1234" cat port1 + atf_check -o match:"[0-9]+" -o not-inline:"1234" cat port2 + atf_check -o match:"[0-9]+" -o not-inline:"1234" cat port3 +} + +srcport_cleanup() +{ + pft_cleanup +} + atf_init_test_cases() { atf_add_test_case "tcp_v6" + atf_add_test_case "srcport" }