pf: Add a sysctl to limit work done for rdr source port rewriting

It was pointed out that the current approach of exhaustively searching
for a free source port might be very time consuming.  Limit the amount
of work that we might do before giving up.

Reviewed by:	kp
Reported by:	Eirik Øverby <ltning-freebsd@anduin.net>
MFC after:	3 months
Sponsored by:	Klara, Inc.
Sponsored by:	Modirum
Differential Revision:	https://reviews.freebsd.org/D46495

(cherry picked from commit 339a1977c32414f3d23733504955245ca6f3802d)

3 files changed, 26 insertions, 3 deletions

diff --git a/share/man/man4/pf.4 b/share/man/man4/pf.4
index b757376e0183..4938e719b17e 100644
--- a/share/man/man4/pf.4
+++ b/share/man/man4/pf.4
@@ -87,6 +87,11 @@ Default value is 131072.
 Size of hash table that store source nodes.
 Should be power of 2.
 Default value is 32768.
+.It Va net.pf.rdr_srcport_rewrite_tries
+The maximum number of times to try and find a free source port when handling
+redirects.
+Such rules are typically applied to external traffic, so an exhaustive search
+may be too expensive.
 .El
 .Pp
 Read only
diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5
index d9e9127f8a84..935a70301d88 100644
--- a/share/man/man5/pf.conf.5
+++ b/share/man/man5/pf.conf.5
@@ -1410,7 +1410,11 @@ 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.
+A random source port in the range 50001-65535 is chosen in this case; to
+avoid excessive CPU consumption, the number of searches for a free port is
+limited by the
+.Va net.pf.rdr_srcport_rewrite_tries
+sysctl.
 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 3f5b3f90a4e4..2623a22db86b 100644
--- a/sys/netpfil/pf/pf_lb.c
+++ b/sys/netpfil/pf/pf_lb.c
@@ -52,6 +52,13 @@
 #include <net/pfvar.h>
 #include <net/if_pflog.h>
 
+/*
+ * Limit the amount of work we do to find a free source port for redirects that
+ * introduce a state conflict.
+ */
+#define	V_pf_rdr_srcport_rewrite_tries	VNET(pf_rdr_srcport_rewrite_tries)
+VNET_DEFINE_STATIC(int, pf_rdr_srcport_rewrite_tries) = 16;
+
 #define DPFPRINTF(n, x)	if (V_pf_status.debug >= (n)) printf x
 
 static void		 pf_hash(struct pf_addr *, struct pf_addr *,
@@ -756,6 +763,7 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off,
 		break;
 	case PF_RDR: {
 		struct pf_state_key_cmp key;
+		int tries;
 		uint16_t cut, low, high, nport;
 
 		reason = pf_map_addr(pd->af, r, saddr, naddr, NULL, NULL, sn);
@@ -807,11 +815,15 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off,
 		if (!pf_find_state_all_exists(&key, PF_OUT))
 			break;
 
+		tries = 0;
+
 		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++) {
+		    tmp <= high && tmp <= UINT16_MAX &&
+		    tries < V_pf_rdr_srcport_rewrite_tries;
+		    tmp++, tries++) {
 			key.port[0] = htons(tmp);
 			if (!pf_find_state_all_exists(&key, PF_OUT)) {
 				/* Update the source port. */
@@ -819,7 +831,9 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off,
 				goto out;
 			}
 		}
-		for (uint32_t tmp = cut - 1; tmp >= low; tmp--) {
+		for (uint32_t tmp = cut - 1;
+		    tmp >= low && tries < V_pf_rdr_srcport_rewrite_tries;
+		    tmp--, tries++) {
 			key.port[0] = htons(tmp);
 			if (!pf_find_state_all_exists(&key, PF_OUT)) {
 				/* Update the source port. */