Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 22 Oct 2013 09:58:31 +0000 (UTC)
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org
Subject:   svn commit: r256882 - stable/9/sys/contrib/pf/net
Message-ID:  <201310220958.r9M9wVqx082402@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: glebius
Date: Tue Oct 22 09:58:31 2013
New Revision: 256882
URL: http://svnweb.freebsd.org/changeset/base/256882

Log:
  Merge r255143 from head:
    Merge 1.12 of pf_lb.c from OpenBSD, with some changes. Original commit:
  
      date: 2010/02/04 14:10:12;  author: sthen;  state: Exp;  lines: +24 -19;
      pf_get_sport() picks a random port from the port range specified in a
      nat rule. It should check to see if it's in-use (i.e. matches an existing
      PF state), if it is, it cycles sequentially through other ports until
      it finds a free one. However the check was being done with the state
      keys the wrong way round so it was never actually finding the state
      to be in-use.
  
      - switch the keys to correct this, avoiding random state collisions
      with nat. Fixes PR 6300 and problems reported by robert@ and viq.
  
      - check pf_get_sport() return code in pf_test(); if port allocation
      fails the packet should be dropped rather than sent out untranslated.
  
      Help/ok claudio@.
  
    Some additional changes to 1.12:
  
    - We also need to bzero() the key to zero padding, otherwise key
      won't match.
    - Collapse two if blocks into one with ||, since both conditions
      lead to the same processing.
    - Only naddr changes in the cycle, so move initialization of other
      fields above the cycle.
    - s/u_intXX_t/uintXX_t/g
  
    PR:             kern/181690
    Submitted by:   Olivier Cochard-Labbé <olivier cochard.me>
    Sponsored by:   Nginx, Inc.

Modified:
  stable/9/sys/contrib/pf/net/pf_lb.c
Directory Properties:
  stable/9/sys/   (props changed)
  stable/9/sys/contrib/pf/   (props changed)

Modified: stable/9/sys/contrib/pf/net/pf_lb.c
==============================================================================
--- stable/9/sys/contrib/pf/net/pf_lb.c	Tue Oct 22 09:43:14 2013	(r256881)
+++ stable/9/sys/contrib/pf/net/pf_lb.c	Tue Oct 22 09:58:31 2013	(r256882)
@@ -165,8 +165,8 @@ struct pf_rule		*pf_match_translation(st
 			    struct pf_addr *, u_int16_t, struct pf_addr *,
 			    u_int16_t, int);
 int			 pf_get_sport(sa_family_t, u_int8_t, struct pf_rule *,
-			    struct pf_addr *, struct pf_addr *, u_int16_t,
-			    struct pf_addr *, u_int16_t*, u_int16_t, u_int16_t,
+			    struct pf_addr *, uint16_t, struct pf_addr *, uint16_t,
+			    struct pf_addr *, uint16_t*, uint16_t, uint16_t,
 			    struct pf_src_node **);
 
 #define mix(a,b,c) \
@@ -318,13 +318,13 @@ pf_match_translation(struct pf_pdesc *pd
 
 int
 pf_get_sport(sa_family_t af, u_int8_t proto, struct pf_rule *r,
-    struct pf_addr *saddr, struct pf_addr *daddr, u_int16_t dport,
-    struct pf_addr *naddr, u_int16_t *nport, u_int16_t low, u_int16_t high,
-    struct pf_src_node **sn)
+    struct pf_addr *saddr, uint16_t sport, struct pf_addr *daddr,
+    uint16_t dport, struct pf_addr *naddr, uint16_t *nport, uint16_t low,
+    uint16_t high, struct pf_src_node **sn)
 {
 	struct pf_state_key_cmp	key;
 	struct pf_addr		init_addr;
-	u_int16_t		cut;
+	uint16_t		cut;
 
 	bzero(&init_addr, sizeof(init_addr));
 	if (pf_map_addr(af, r, saddr, naddr, &init_addr, sn))
@@ -335,34 +335,38 @@ pf_get_sport(sa_family_t af, u_int8_t pr
 		high = 65535;
 	}
 
+	bzero(&key,sizeof(key));
+	key.af = af;
+	key.proto = proto;
+	key.port[0] = dport;
+	PF_ACPY(&key.addr[0], daddr, key.af);
+
 	do {
-		key.af = af;
-		key.proto = proto;
-		PF_ACPY(&key.addr[1], daddr, key.af);
-		PF_ACPY(&key.addr[0], naddr, key.af);
-		key.port[1] = dport;
+		PF_ACPY(&key.addr[1], naddr, key.af);
 
 		/*
 		 * port search; start random, step;
 		 * similar 2 portloop in in_pcbbind
 		 */
 		if (!(proto == IPPROTO_TCP || proto == IPPROTO_UDP ||
-		    proto == IPPROTO_ICMP)) {
-			key.port[0] = dport;
-			if (pf_find_state_all(&key, PF_IN, NULL) == NULL)
-				return (0);
-		} else if (low == 0 && high == 0) {
-			key.port[0] = *nport;
-			if (pf_find_state_all(&key, PF_IN, NULL) == NULL)
+		    proto == IPPROTO_ICMP) || (low == 0 && high == 0)) {
+			/*
+			* XXX bug: icmp state don't use the id on both sides.
+			* (traceroute -l through nat)
+			*/
+			key.port[1] = sport;
+			if (pf_find_state_all(&key, PF_IN, NULL) == NULL) {
+				*nport = sport;
 				return (0);
+			}
 		} else if (low == high) {
-			key.port[0] = htons(low);
+			key.port[1] = htons(low);
 			if (pf_find_state_all(&key, PF_IN, NULL) == NULL) {
 				*nport = htons(low);
 				return (0);
 			}
 		} else {
-			u_int16_t tmp;
+			uint16_t tmp;
 
 			if (low > high) {
 				tmp = low;
@@ -377,7 +381,7 @@ pf_get_sport(sa_family_t af, u_int8_t pr
 #endif
 			/* low <= cut <= high */
 			for (tmp = cut; tmp <= high; ++(tmp)) {
-				key.port[0] = htons(tmp);
+				key.port[1] = htons(tmp);
 				if (pf_find_state_all(&key, PF_IN, NULL) ==
 #ifdef __FreeBSD__
 				    NULL) {
@@ -389,7 +393,7 @@ pf_get_sport(sa_family_t af, u_int8_t pr
 				}
 			}
 			for (tmp = cut - 1; tmp >= low; --(tmp)) {
-				key.port[0] = htons(tmp);
+				key.port[1] = htons(tmp);
 				if (pf_find_state_all(&key, PF_IN, NULL) ==
 #ifdef __FreeBSD__
 				    NULL) {
@@ -655,8 +659,8 @@ pf_get_translation(struct pf_pdesc *pd, 
 		case PF_NORDR:
 			return (NULL);
 		case PF_NAT:
-			if (pf_get_sport(pd->af, pd->proto, r, saddr,
-			    daddr, dport, naddr, nport, r->rpool.proxy_port[0],
+			if (pf_get_sport(pd->af, pd->proto, r, saddr, sport, daddr,
+			    dport, naddr, nport, r->rpool.proxy_port[0],
 			    r->rpool.proxy_port[1], sn)) {
 				DPFPRINTF(PF_DEBUG_MISC,
 				    ("pf: NAT proxy port allocation "



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201310220958.r9M9wVqx082402>