fake state

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Henning Brauer
Subject: fake state
Date: Friday, September 24, 2010 - 1:59 am

those freaking code pathes for stateless are annoying as hell and tend
to be buggy since everybody does stateful anyway...
so here's the deal: always get us a state back when we actually pass
the packet, but don't link it into the state table. late in pf_test
throw it away if we want stateless. makes things easier and allows for
some future simplifications (yes, i marked some in this diff to not
forget)
speaking of forgetting: anakin main tree

Index: pf.c
===================================================================
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.713
diff -u -p -r1.713 pf.c
--- pf.c	24 Sep 2010 02:28:10 -0000	1.713
+++ pf.c	24 Sep 2010 08:52:38 -0000
@@ -1200,7 +1200,7 @@ pf_free_state(struct pf_state *cur)
 	if (pfsync_state_in_use(cur))
 		return;
 #endif
-	KASSERT(cur->timeout == PFTM_UNLINKED);
+	KASSERT(cur->timeout == PFTM_UNLINKED || cur->key[PF_SK_STACK] == NULL);
 	if (--cur->rule.ptr->states_cur <= 0 &&
 	    cur->rule.ptr->src_nodes <= 0)
 		pf_rm_rule(NULL, cur->rule.ptr);
@@ -1215,8 +1215,10 @@ pf_free_state(struct pf_state *cur)
 		pool_put(&pf_rule_item_pl, ri);
 	}
 	pf_normalize_tcp_cleanup(cur);
-	pfi_kif_unref(cur->kif, PFI_KIF_REF_STATE);
-	TAILQ_REMOVE(&state_list, cur, entry_list);
+	if (cur->key[PF_SK_STACK] != NULL) {
+		pfi_kif_unref(cur->kif, PFI_KIF_REF_STATE);
+		TAILQ_REMOVE(&state_list, cur, entry_list);
+	}
 	if (cur->tag)
 		pf_tag_unref(cur->tag);
 	pool_put(&pf_state_pl, cur);
@@ -2789,7 +2791,7 @@ pf_test_rule(struct pf_rule **rm, struct
 	int			 tag = -1;
 	int			 asd = 0;
 	int			 match = 0;
-	int			 state_icmp = 0, icmp_dir, multi;
+	int			 state_icmp = 0, icmp_dir, multi, action;
 	u_int16_t		 virtual_type, virtual_id;
 	u_int8_t		 icmptype = 0, icmpcode = 0;
 
@@ -3033,46 +3035,37 @@ pf_test_rule(struct pf_rule **rm, struct
 	if (r->action == PF_DROP)
 		goto cleanup;
 
-	if (pf_tag_packet(m, tag, act.rtableid)) {
+	if (pf_tag_packet(m, tag, act.rtableid)) { /* XXX kill */
 		REASON_SET(&reason, PFRES_MEMORY);
 		goto cleanup;
 	}
-	if (act.rtableid >= 0 &&
+	if (act.rtableid >= 0 &&		/* XXX kill */
 	    rtable_l2(act.rtableid) != pd->rdomain)
 		pd->destchg = 1;
+	if (state_icmp || !r->keep_state)
+		act.fake_state = 1;
+	if (!act.fake_state && r->rule_flag & PFRULE_SRCTRACK &&
+	    pf_insert_src_node(&sns[PF_SN_NONE], r, PF_SN_NONE, pd->af,
+	    pd->src, NULL, 0) != 0) {
+		REASON_SET(&reason, PFRES_SRCLIMIT);
+		goto cleanup;
+	}
 
-	if (!state_icmp && r->keep_state) {
-		int action;
-
-		if (r->rule_flag & PFRULE_SRCTRACK &&
-		    pf_insert_src_node(&sns[PF_SN_NONE], r, PF_SN_NONE, pd->af,
-		    pd->src, NULL, 0) != 0) {
-			REASON_SET(&reason, PFRES_SRCLIMIT);
-			goto cleanup;
-		}
-
-		action = pf_create_state(r, a, pd, &skw, &sks, m, off,
-		    &rewrite, kif, sm, tag, hdrlen, &rules, &act, sns);
-
-		if (action != PF_PASS)
+	if ((action = pf_create_state(r, a, pd, &skw, &sks, m, off, &rewrite,
+	    kif, sm, tag, hdrlen, &rules, &act, sns)) != PF_PASS)
 			return (action);
-		if (sks != skw) {
-			struct pf_state_key	*sk;
+	/* XXX do the rewrite in pf_test */
+	if (!act.fake_state && sks != skw) {
+		struct pf_state_key	*sk;
 
-			if (pd->dir == PF_IN)
-				sk = sks;
-			else
-				sk = skw;
-			rewrite += pf_translate(pd,
-			    &sk->addr[pd->sidx], sk->port[pd->sidx],
-			    &sk->addr[pd->didx], sk->port[pd->didx],
-			    virtual_type, icmp_dir, m);
-		}
-	} else {
-		while ((ri = SLIST_FIRST(&rules))) {
-			SLIST_REMOVE_HEAD(&rules, entry);
-			pool_put(&pf_rule_item_pl, ri);
-		}
+		if (pd->dir == PF_IN)
+			sk = sks;
+		else
+			sk = skw;
+		rewrite += pf_translate(pd,
+		    &sk->addr[pd->sidx], sk->port[pd->sidx],
+		    &sk->addr[pd->didx], sk->port[pd->didx],
+		    virtual_type, icmp_dir, m);
 	}
 
 	/* copy back packet headers if we performed NAT operations */
@@ -3080,8 +3073,8 @@ pf_test_rule(struct pf_rule **rm, struct
 		m_copyback(m, off, hdrlen, pd->hdr.any, M_NOWAIT);
 
 #if NPFSYNC > 0
-	if (*sm != NULL && !ISSET((*sm)->state_flags, PFSTATE_NOSYNC) &&
-	    direction == PF_OUT && pfsync_up()) {
+	if (!act.fake_state && *sm != NULL && direction == PF_OUT &&
+	    pfsync_up() && !ISSET((*sm)->state_flags, PFSTATE_NOSYNC)) {
 		/*
 		 * We want the state created, but we dont
 		 * want to send this in case a partner
@@ -3222,6 +3215,11 @@ pf_create_state(struct pf_rule *r, struc
 	}
 	s->direction = pd->dir;
 
+	if (act->fake_state) {
+		*sm = s;
+		return (PF_PASS);
+	}
+
 	if (pf_state_key_setup(pd, skw, sks, act->rtableid)) {
 		REASON_SET(&reason, PFRES_MEMORY);
 		goto csfailed;
@@ -5826,16 +5824,17 @@ int
 pf_test(int dir, struct ifnet *ifp, struct mbuf **m0,
     struct ether_header *eh)
 {
-	struct pfi_kif		*kif;
-	u_short			 action, reason = 0, pflog = 0;
 	struct mbuf		*m = *m0;
 	struct ip		*h;
+	struct pfi_kif		*kif;
+	struct pf_divert	*divert;
 	struct pf_rule		*a = NULL, *r = &pf_default_rule;
 	struct pf_state		*s = NULL;
 	struct pf_ruleset	*ruleset = NULL;
 	struct pf_pdesc		 pd;
 	int			 off, hdrlen;
 	u_int32_t		 qid, pqid = 0;
+	u_short			 action, reason = 0, pflog = 0;
 
 	if (!pf_status.running)
 		return (PF_PASS);
@@ -5919,11 +5918,8 @@ pf_test(int dir, struct ifnet *ifp, stru
 			action = pf_test_rule(&r, &s, dir, kif,
 			    m, off, h, &pd, &a, &ruleset, &ipintrq, hdrlen);
 
-		if (s) {
-			if (s->max_mss)
-				pf_normalize_mss(m, off, &pd, s->max_mss);
-		} else if (r->max_mss)
-			pf_normalize_mss(m, off, &pd, r->max_mss);
+		if (s && s->max_mss)
+			pf_normalize_mss(m, off, &pd, s->max_mss);
 
 		break;
 	}
@@ -5982,69 +5978,57 @@ pf_test(int dir, struct ifnet *ifp, stru
 	}
 
 done:
-	if (action == PF_PASS && h->ip_hl > 5 &&
-	    !((s && s->state_flags & PFSTATE_ALLOWOPTS) || r->allow_opts)) {
-		action = PF_DROP;
-		REASON_SET(&reason, PFRES_IPOPTIONS);
-		pflog |= PF_LOG_FORCE;
-		DPFPRINTF(LOG_NOTICE,
-		    "pf: dropping packet with ip options");
-	}
-
-	if (s) {
+	if (action == PF_PASS) {
+		if (h->ip_hl > 5 && !(s->state_flags & PFSTATE_ALLOWOPTS)) {
+			action = PF_DROP;
+			REASON_SET(&reason, PFRES_IPOPTIONS);
+			pflog |= PF_LOG_FORCE;
+			DPFPRINTF(LOG_NOTICE,
+			    "pf: dropping packet with ip options");
+		}
 		pf_scrub_ip(&m, s->state_flags, s->min_ttl, s->set_tos);
 		pf_tag_packet(m, s->tag, s->rtableid[pd.didx]);
 		if (pqid || (pd.tos & IPTOS_LOWDELAY))
 			qid = s->pqid;
 		else
 			qid = s->qid;
-	} else {
-		pf_scrub_ip(&m, r->scrub_flags, r->min_ttl, r->set_tos);
-		if (pqid || (pd.tos & IPTOS_LOWDELAY))
-			qid = r->pqid;
-		else
-			qid = r->qid;
-	}
 
-	if (dir == PF_IN && s && s->key[PF_SK_STACK])
-		m->m_pkthdr.pf.statekey = s->key[PF_SK_STACK];
+		if (dir == PF_IN && s->key[PF_SK_STACK])
+			m->m_pkthdr.pf.statekey = s->key[PF_SK_STACK];
 
 #ifdef ALTQ
-	if (action == PF_PASS && qid) {
-		m->m_pkthdr.pf.qid = qid;
-		m->m_pkthdr.pf.hdr = h;	/* hints for ecn */
-	}
+		if (qid) {
+			m->m_pkthdr.pf.qid = qid;
+			m->m_pkthdr.pf.hdr = h;	/* hints for ecn */
+		}
 #endif /* ALTQ */
 
-	/*
-	 * connections redirected to loopback should not match sockets
-	 * bound specifically to loopback due to security implications,
-	 * see tcp_input() and in_pcblookup_listen().
-	 */
-	if (pd.destchg &&
-	    (ntohl(pd.dst->v4.s_addr) >> IN_CLASSA_NSHIFT) == IN_LOOPBACKNET)
-		m->m_pkthdr.pf.flags |= PF_TAG_TRANSLATE_LOCALHOST;
-	/* We need to redo the route lookup on outgoing routes. */
-	if (pd.destchg && dir == PF_OUT)
-		m->m_pkthdr.pf.flags |= PF_TAG_REROUTE;
-
-	if (dir == PF_IN && action == PF_PASS && r->divert.port) {
-		struct pf_divert *divert;
-
-		if ((divert = pf_get_divert(m))) {
-			m->m_pkthdr.pf.flags |= PF_TAG_DIVERTED;
-			divert->port = r->divert.port;
-			divert->addr.ipv4 = r->divert.addr.v4;
+		/*
+		 * connections redirected to loopback should not match sockets
+		 * bound specifically to loopback due to security implications,
+		 * see tcp_input() and in_pcblookup_listen().
+		 */
+		if (pd.destchg && (ntohl(pd.dst->v4.s_addr) >>
+		    IN_CLASSA_NSHIFT) == IN_LOOPBACKNET)
+			m->m_pkthdr.pf.flags |= PF_TAG_TRANSLATE_LOCALHOST;
+		/* We need to redo the route lookup on outgoing routes. */
+		if (pd.destchg && dir == PF_OUT)
+			m->m_pkthdr.pf.flags |= PF_TAG_REROUTE;
+
+		if (dir == PF_IN && r->divert.port) {
+			if ((divert = pf_get_divert(m))) {
+				m->m_pkthdr.pf.flags |= PF_TAG_DIVERTED;
+				divert->port = r->divert.port;
+				divert->addr.ipv4 = r->divert.addr.v4;
+			}
 		}
-	}
-
-	if (action == PF_PASS && r->divert_packet.port) {
-		struct pf_divert *divert;
 
-		if ((divert = pf_get_divert(m)))
-			divert->port = r->divert_packet.port;
-
-		action = PF_DIVERT;
+		if (r->divert_packet.port) {
+			struct pf_divert *divert;
+			if ((divert = pf_get_divert(m)))
+				divert->port = r->divert_packet.port;
+			action = PF_DIVERT;
+		}
 	}
 
 	if (pflog) {
@@ -6053,16 +6037,19 @@ done:
 		if (pflog & PF_LOG_FORCE || r->log & PF_LOG_ALL)
 			PFLOG_PACKET(kif, h, m, AF_INET, dir, reason, r, a,
 			    ruleset, &pd);
-		if (s) {
-			SLIST_FOREACH(ri, &s->match_rules, entry)
-				if (ri->r->log & PF_LOG_ALL)
-					PFLOG_PACKET(kif, h, m, AF_INET, dir,
-					    reason, ri->r, a, ruleset, &pd);
-		}
+		SLIST_FOREACH(ri, &s->match_rules, entry)
+			if (ri->r->log & PF_LOG_ALL)
+				PFLOG_PACKET(kif, h, m, AF_INET, dir,
+				    reason, ri->r, a, ruleset, &pd);
 	}
 
 	pf_counters_inc(dir, action, &pd, kif, s, r, a);
 
+	if (s && s->key[PF_SK_STACK] == NULL) {
+		pf_free_state(s);
+		s = NULL;
+	}
+
 	switch (action) {
 	case PF_SYNPROXY_DROP:
 		m_freem(*m0);
@@ -6091,15 +6078,16 @@ int
 pf_test6(int dir, struct ifnet *ifp, struct mbuf **m0,
     struct ether_header *eh)
 {
-	struct pfi_kif		*kif;
-	u_short			 action, reason = 0, pflog = 0;
 	struct mbuf		*m = *m0, *n = NULL;
 	struct ip6_hdr		*h;
+	struct pfi_kif		*kif;
+	struct pf_divert	*divert;
 	struct pf_rule		*a = NULL, *r = &pf_default_rule;
 	struct pf_state		*s = NULL;
 	struct pf_ruleset	*ruleset = NULL;
 	struct pf_pdesc		 pd;
 	int			 off, hdrlen;
+	u_short			 action, reason = 0, pflog = 0;
 
 	if (!pf_status.running)
 		return (PF_PASS);
@@ -6188,12 +6176,8 @@ pf_test6(int dir, struct ifnet *ifp, str
 			action = pf_test_rule(&r, &s, dir, kif,
 			    m, off, h, &pd, &a, &ruleset, &ip6intrq, hdrlen);
 
-		if (s) {
-			if (s->max_mss)
-				pf_normalize_mss(m, off, &pd, s->max_mss);
-		} else if (r->max_mss)
-			pf_normalize_mss(m, off, &pd, r->max_mss);
-
+		if (s && s->max_mss)
+			pf_normalize_mss(m, off, &pd, s->max_mss);
 		break;
 	}
 
@@ -6257,62 +6241,54 @@ done:
 		n = NULL;
 	}
 
-	/* handle dangerous IPv6 extension headers. */
-	if (action == PF_PASS && pd.rh_cnt &&
-	    !((s && s->state_flags & PFSTATE_ALLOWOPTS) || r->allow_opts)) {
-		action = PF_DROP;
-		REASON_SET(&reason, PFRES_IPOPTIONS);
-		pflog |= PF_LOG_FORCE;
-		DPFPRINTF(LOG_NOTICE,
-		    "pf: dropping packet with dangerous v6 headers");
-	}
-
-	if (s)
+	if (action == PF_PASS) {
+		/* handle dangerous IPv6 extension headers. */
+		if (action == PF_PASS && pd.rh_cnt &&
+		    !(s->state_flags & PFSTATE_ALLOWOPTS)) {
+			action = PF_DROP;
+			REASON_SET(&reason, PFRES_IPOPTIONS);
+			pflog |= PF_LOG_FORCE;
+			DPFPRINTF(LOG_NOTICE,
+			    "pf: dropping packet with dangerous v6 headers");
+		}
 		pf_scrub_ip6(&m, s->min_ttl);
-	else
-		pf_scrub_ip6(&m, r->min_ttl);
+		if (s->tag)
+			pf_tag_packet(m, s->tag, s->rtableid[pd.didx]);
 
-	if (s && s->tag)
-		pf_tag_packet(m, s ? s->tag : 0, s->rtableid[pd.didx]);
-
-	if (dir == PF_IN && s && s->key[PF_SK_STACK])
-		m->m_pkthdr.pf.statekey = s->key[PF_SK_STACK];
+		if (dir == PF_IN && s->key[PF_SK_STACK])
+			m->m_pkthdr.pf.statekey = s->key[PF_SK_STACK];
 
 #ifdef ALTQ
-	if (action == PF_PASS && s && s->qid) {
-		if (pd.tos & IPTOS_LOWDELAY)
-			m->m_pkthdr.pf.qid = s->pqid;
-		else
-			m->m_pkthdr.pf.qid = s->qid;
-		/* add hints for ecn */
-		m->m_pkthdr.pf.hdr = h;
-	}
+		if (s->qid) {
+			if (pd.tos & IPTOS_LOWDELAY)
+				m->m_pkthdr.pf.qid = s->pqid;
+			else
+				m->m_pkthdr.pf.qid = s->qid;
+			/* add hints for ecn */
+			m->m_pkthdr.pf.hdr = h;
+		}
 #endif /* ALTQ */
 
-	if (pd.destchg &&
-	    IN6_IS_ADDR_LOOPBACK(&pd.dst->v6))
-		m->m_pkthdr.pf.flags |= PF_TAG_TRANSLATE_LOCALHOST;
-	/* We need to redo the route lookup on outgoing routes. */
-	if (pd.destchg && dir == PF_OUT)
-		m->m_pkthdr.pf.flags |= PF_TAG_REROUTE;
-
-	if (dir == PF_IN && action == PF_PASS && r->divert.port) {
-		struct pf_divert *divert;
-
-		if ((divert = pf_get_divert(m))) {
-			m->m_pkthdr.pf.flags |= PF_TAG_DIVERTED;
-			divert->port = r->divert.port;
-			divert->addr.ipv6 = r->divert.addr.v6;
+		if (pd.destchg &&
+		    IN6_IS_ADDR_LOOPBACK(&pd.dst->v6))
+			m->m_pkthdr.pf.flags |= PF_TAG_TRANSLATE_LOCALHOST;
+		/* We need to redo the route lookup on outgoing routes. */
+		if (pd.destchg && dir == PF_OUT)
+			m->m_pkthdr.pf.flags |= PF_TAG_REROUTE;
+
+		if (dir == PF_IN && r->divert.port) {
+			if ((divert = pf_get_divert(m))) {
+				m->m_pkthdr.pf.flags |= PF_TAG_DIVERTED;
+				divert->port = r->divert.port;
+				divert->addr.ipv6 = r->divert.addr.v6;
+			}
 		}
-	}
-
-	if (action == PF_PASS && r->divert_packet.port) {
-		struct pf_divert *divert;
-
-		if ((divert = pf_get_divert(m)))
-			divert->port = r->divert_packet.port;
 
-		action = PF_DIVERT;
+		if (r->divert_packet.port) {
+			if ((divert = pf_get_divert(m)))
+				divert->port = r->divert_packet.port;
+			action = PF_DIVERT;
+		}
 	}
 
 	if (pflog) {
@@ -6321,15 +6297,18 @@ done:
 		if (pflog & PF_LOG_FORCE || r->log & PF_LOG_ALL)
 			PFLOG_PACKET(kif, h, m, AF_INET6, dir, reason, r, a,
 			    ruleset, &pd);
-		if (s) {
-			SLIST_FOREACH(ri, &s->match_rules, entry)
-				if (ri->r->log & PF_LOG_ALL)
-					PFLOG_PACKET(kif, h, m, AF_INET6, dir,
-					    reason, ri->r, a, ruleset, &pd);
-		}
+		SLIST_FOREACH(ri, &s->match_rules, entry)
+			if (ri->r->log & PF_LOG_ALL)
+				PFLOG_PACKET(kif, h, m, AF_INET6, dir,
+				    reason, ri->r, a, ruleset, &pd);
 	}
 
 	pf_counters_inc(dir, action, &pd, kif, s, r, a);
+
+	if (s && s->key[PF_SK_STACK] == NULL) {
+		pf_free_state(s);
+		s = NULL;
+	}
 
 	switch (action) {
 	case PF_SYNPROXY_DROP:
Index: pfvar.h
===================================================================
RCS file: /cvs/src/sys/net/pfvar.h,v
retrieving revision 1.315
diff -u -p -r1.315 pfvar.h
--- pfvar.h	22 Sep 2010 05:58:29 -0000	1.315
+++ pfvar.h	24 Sep 2010 08:52:38 -0000
@@ -527,7 +527,7 @@ struct pf_rule_actions {
 	u_int8_t	log;
 	u_int8_t	set_tos;
 	u_int8_t	min_ttl;
-	u_int8_t	pad[1];
+	u_int8_t	fake_state;
 	u_int16_t	flags;
 };
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
fake state, Henning Brauer, (Fri Sep 24, 1:59 am)