Subject: Re: stopping PF NAT state from "floating" ?
To: None <tech-net@NetBSD.org>
From: David Young <dyoung@pobox.com>
List: tech-net
Date: 05/10/2007 02:04:39
--zYM0uCDKw75PZbzx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Wed, May 09, 2007 at 11:23:38PM -0500, David Young wrote:
> Right now, I am at a loss to explain how pfctl can add a rule with an
> if-bound flag, read the same rule back out with if-bound flag intact,
> and nevertheless ignore the if-bound flag when it creates state.
> Still investigating.

I figured it out.  PF uses the if-bound flag from filter rules, only.
I have made PF use both the 'if-bound'/'gr-bound'/'floating' properties
from the filter rule and the translation rule (if available).  Works so
far.  See patch (attached).

Dave

-- 
David Young             OJC Technologies
dyoung@ojctech.com      Urbana, IL * (217) 278-3933

--zYM0uCDKw75PZbzx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="pf.patch"

Index: dist/pf/sbin/pfctl/parse.y
===================================================================
RCS file: /cvsroot/src/dist/pf/sbin/pfctl/parse.y,v
retrieving revision 1.7
diff -p -u -u -p -r1.7 parse.y
--- dist/pf/sbin/pfctl/parse.y	27 Sep 2006 15:35:12 -0000	1.7
+++ dist/pf/sbin/pfctl/parse.y	10 May 2007 06:59:54 -0000
@@ -441,6 +441,7 @@ typedef struct {
 %type	<v.string>		label string tag
 %type	<v.keep_state>		keep
 %type	<v.state_opt>		state_opt_spec state_opt_list state_opt_item
+%type	<v.state_opt>		opt_statelock
 %type	<v.logquick>		logquick
 %type	<v.interface>		antispoof_ifspc antispoof_iflst antispoof_if
 %type	<v.qassign>		qname
@@ -1835,6 +1836,22 @@ pfrule		: action dir logquick interface 
 		}
 		;
 
+
+opt_statelock	: statelock
+		{
+			$$ = calloc(1, sizeof(struct node_state_opt));
+			if ($$ == NULL)
+				err(EXIT_FAILURE, "opt_statelock: calloc");
+			$$->type = PF_STATE_OPT_STATELOCK;
+			$$->data.statelock = $1;
+			$$->next = NULL;
+			$$->tail = $$;
+		}
+		| /* empty */	{
+			$$ = NULL;
+		}
+		;
+
 filter_opts	:	{ bzero(&filter_opts, sizeof filter_opts); }
 		    filter_opts_l
 			{ $$ = filter_opts; }
@@ -3224,7 +3241,7 @@ nataction	: no NAT natpass {
 		}
 		;
 
-natrule		: nataction interface af proto fromto tag tagged redirpool pool_opts
+natrule		: nataction interface af proto fromto tag tagged redirpool pool_opts opt_statelock
 		{
 			struct pf_rule	r;
 
@@ -3372,6 +3389,11 @@ natrule		: nataction interface af proto 
 				r.rpool.proxy_port[1] = 0;
 			}
 
+			if ($10 == NULL)
+				r.rule_flag |= default_statelock;
+			else
+				r.rule_flag |= $10->data.statelock;
+
 			expand_rule(&r, $2, $8 == NULL ? NULL : $8->host, $4,
 			    $5.src_os, $5.src.host, $5.src.port, $5.dst.host,
 			    $5.dst.port, 0, 0, 0, "");
Index: sys/dist/pf/net/pf.c
===================================================================
RCS file: /cvsroot/src/sys/dist/pf/net/pf.c,v
retrieving revision 1.37
diff -p -u -u -p -r1.37 pf.c
--- sys/dist/pf/net/pf.c	2 May 2007 20:40:22 -0000	1.37
+++ sys/dist/pf/net/pf.c	10 May 2007 06:59:55 -0000
@@ -274,10 +274,6 @@ struct pf_pool_limit pf_pool_limits[PF_L
 	  (s)->lan.addr.addr32[3] != (s)->gwy.addr.addr32[3])) || \
 	(s)->lan.port != (s)->gwy.port
 
-#define BOUND_IFACE(r, k) (((r)->rule_flag & PFRULE_IFBOUND) ? (k) :   \
-	((r)->rule_flag & PFRULE_GRBOUND) ? (k)->pfik_parent :	       \
-	(k)->pfik_parent->pfik_parent)
-
 #define STATE_INC_COUNTERS(s)				\
 	do {						\
 		s->rule.ptr->states++;			\
@@ -320,6 +316,24 @@ RB_GENERATE(pf_state_tree_id, pf_state,
 RB_GENERATE(pf_anchor_global, pf_anchor, entry_global, pf_anchor_compare);
 RB_GENERATE(pf_anchor_node, pf_anchor, entry_node, pf_anchor_compare);
 
+static inline struct pfi_kif *
+bound_iface(const struct pf_rule *r, const struct pf_rule *nr,
+    struct pfi_kif *k)
+{
+	uint32_t rule_flag;
+
+	rule_flag = r->rule_flag;
+	if (nr != NULL)
+		rule_flag |= nr->rule_flag;
+
+	if ((rule_flag & PFRULE_IFBOUND) != 0)
+		return k;
+	else if ((rule_flag & PFRULE_GRBOUND) != 0)
+		return k->pfik_parent;
+	else
+		return k->pfik_parent->pfik_parent;
+}
+
 static __inline int
 pf_src_compare(struct pf_src_node *a, struct pf_src_node *b)
 {
@@ -3091,7 +3105,7 @@ cleanup:
 			pool_put(&pf_state_pl, s);
 			return (PF_DROP);
 		}
-		if (pf_insert_state(BOUND_IFACE(r, kif), s)) {
+		if (pf_insert_state(bound_iface(r, nr, kif), s)) {
 			pf_normalize_tcp_cleanup(s);
 			REASON_SET(&reason, PFRES_STATEINS);
 			pf_src_tree_remove_state(s);
@@ -3396,7 +3410,7 @@ cleanup:
 			s->nat_src_node = nsn;
 			s->nat_src_node->states++;
 		}
-		if (pf_insert_state(BOUND_IFACE(r, kif), s)) {
+		if (pf_insert_state(bound_iface(r, nr, kif), s)) {
 			REASON_SET(&reason, PFRES_STATEINS);
 			pf_src_tree_remove_state(s);
 			STATE_DEC_COUNTERS(s);
@@ -3686,7 +3700,7 @@ cleanup:
 			s->nat_src_node = nsn;
 			s->nat_src_node->states++;
 		}
-		if (pf_insert_state(BOUND_IFACE(r, kif), s)) {
+		if (pf_insert_state(bound_iface(r, nr, kif), s)) {
 			REASON_SET(&reason, PFRES_STATEINS);
 			pf_src_tree_remove_state(s);
 			STATE_DEC_COUNTERS(s);
@@ -3959,7 +3973,7 @@ cleanup:
 			s->nat_src_node = nsn;
 			s->nat_src_node->states++;
 		}
-		if (pf_insert_state(BOUND_IFACE(r, kif), s)) {
+		if (pf_insert_state(bound_iface(r, nr, kif), s)) {
 			REASON_SET(&reason, PFRES_STATEINS);
 			pf_src_tree_remove_state(s);
 			STATE_DEC_COUNTERS(s);

--zYM0uCDKw75PZbzx--