kern/183997: route-to rule passes traffic when no targets are specified

Kajetan Staszkiewicz vegeta at tuxpowered.net
Fri Nov 15 12:20:00 UTC 2013


>Number:         183997
>Category:       kern
>Synopsis:       route-to rule passes traffic when no targets are specified
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri Nov 15 12:20:00 UTC 2013
>Closed-Date:
>Last-Modified:
>Originator:     Kajetan Staszkiewicz
>Release:        9.1, 10.0-BETA
>Organization:
InnoGames GmbH
>Environment:
FreeBSD xxxxxxxx 9.1-RELEASE-p7 FreeBSD 9.1-RELEASE-p7 #93 r+cb64f52: Fri Oct 25 16:05:48 CEST 2013     root at xxxxxxx:/usr/obj/mnt/builder/freebsd.git/sys/IGLB3  amd64
>Description:
When rule's redirection pool is empty, a packet still gets forwarded but no redirection occurs. In case of using route-to the packet leaving the router will have original target MAC address and thus will be received by the rotuer itself again if public and internal interfaces are on the same network. Due to no TTL decrease in pf, this situation causes a network loop stealing bandwidth and CPU time.

Redirection pool can be emptied by misconfiguration or by some automatic healthchecking tool which decides that all targets are dead and there is nobody to forward the traffic to.
>How-To-Repeat:
Empty a table and have some traffic hit a route-to rule using this table.
>Fix:
Pass the status of pf_map_addr to pf_set_rt_ifp and then to pf_create_state. If the status shows that pf_map_addr has failed, abort creation of the state and try to delete the src_node it could have created.

The attached patch is for FreeBSD 10.0-BETA3 and was not yet tested under some bigger load, although the same approach works well for FreeBSD 9.1. I can provide the 9.1 patch too if requested.

Patch attached with submission follows:

# When rule's redirection pool is empty, a packet still gets forwarded but no
# redirection occurs. In case of using route-to the packet leaving the router
# will have original target MAC address and thus will be received by the
# rotuer itself again if public and internal interfaces are on the same
# network. Due to no TTL decrease in pf, this situation causes a network
# loop stealing bandwidth and CPU time.
#
# Pass the status of pf_map_addr to pf_set_rt_ifp and then to pf_create_state.
# If the status shows that pf_map_addr has failed, abort creation of the
# state and try to delete the src_node it could have created.
#
# kajetan.staszkiewicz at innogames.de
# Work sponsored by InnoGames GmbH
#
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index e5395e3..7a44f47 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1214,6 +1214,7 @@ struct pf_pdesc {
 #define PFRES_SRCLIMIT	13		/* Source node/conn limit */
 #define PFRES_SYNPROXY	14		/* SYN proxy */
 #define PFRES_MAX	15		/* total+1 */
+#define PFRES_MAPFAILED	16		/* pf_map_addr failed */
 
 #define PFRES_NAMES { \
 	"match", \
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 9da73c5..12d1e9a 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -267,8 +267,8 @@ static u_int16_t	 pf_get_mss(struct mbuf *, int, u_int16_t,
 			    sa_family_t);
 static u_int16_t	 pf_calc_mss(struct pf_addr *, sa_family_t,
 				int, u_int16_t);
-static void		 pf_set_rt_ifp(struct pf_state *,
-			    struct pf_addr *);
+static int		 pf_set_rt_ifp(struct pf_state *,
+			    struct pf_addr *, sa_family_t af);
 static int		 pf_check_proto_cksum(struct mbuf *, int, int,
 			    u_int8_t, sa_family_t);
 static void		 pf_print_state_parts(struct pf_state *,
@@ -2929,29 +2929,32 @@ pf_calc_mss(struct pf_addr *addr, sa_family_t af, int rtableid, u_int16_t offer)
 	return (mss);
 }
 
-static void
-pf_set_rt_ifp(struct pf_state *s, struct pf_addr *saddr)
+static int
+pf_set_rt_ifp(struct pf_state *s, struct pf_addr *saddr, sa_family_t af)
 {
 	struct pf_rule *r = s->rule.ptr;
 	struct pf_src_node *sn = NULL;
+	int map_status = 0;
 
 	s->rt_kif = NULL;
 	if (!r->rt || r->rt == PF_FASTROUTE)
-		return;
-	switch (s->key[PF_SK_WIRE]->af) {
+		return 0;
+	switch (af) {
 #ifdef INET
 	case AF_INET:
-		pf_map_addr(AF_INET, r, saddr, &s->rt_addr, NULL, &sn);
+		map_status = pf_map_addr(AF_INET, r, saddr, &s->rt_addr, NULL, &sn);
 		s->rt_kif = r->rpool.cur->kif;
 		break;
 #endif /* INET */
 #ifdef INET6
 	case AF_INET6:
-		pf_map_addr(AF_INET6, r, saddr, &s->rt_addr, NULL, &sn);
+		map_status = pf_map_addr(AF_INET6, r, saddr, &s->rt_addr, NULL, &sn);
 		s->rt_kif = r->rpool.cur->kif;
 		break;
 #endif /* INET6 */
 	}
+
+	return map_status;
 }
 
 static u_int32_t
@@ -3516,6 +3519,19 @@ pf_create_state(struct pf_rule *r, struct pf_rule *nr, struct pf_rule *a,
 		s->timeout = PFTM_OTHER_FIRST_PACKET;
 	}
 
+	/* Call pf_set_rt_ifp (and thus pf_map_addr). If pf_map_addr fails,
+	   remove the state and drop the packet. It makes no sense forwarding
+	   it if redirection mapping has failed. Do it before setting timeouts,
+	   csfailed won't remove the src_node otherwise. */
+	if (pf_set_rt_ifp(s, pd->src, pd->af)) {
+		REASON_SET(&reason, PFRES_MAPFAILED);
+		pf_src_tree_remove_state(s);
+		STATE_DEC_COUNTERS(s);
+		uma_zfree(V_pf_state_z, s);
+		/* Try to remove (nat_)src_node. */
+		goto csfailed;
+	}
+
 	s->creation = time_uptime;
 	s->expire = time_uptime;
 
@@ -3589,7 +3605,6 @@ pf_create_state(struct pf_rule *r, struct pf_rule *nr, struct pf_rule *a,
 	} else
 		*sm = s;
 
-	pf_set_rt_ifp(s, pd->src);	/* needs s->state_key set */
 	if (tag > 0)
 		s->tag = tag;
 	if (pd->proto == IPPROTO_TCP && (th->th_flags & (TH_SYN|TH_ACK)) ==


>Release-Note:
>Audit-Trail:
>Unformatted:


More information about the freebsd-bugs mailing list