Use time_uptime for expiration values as time_second can be skewed at

runtime while time_uptime is monotonic. Prevent underflows in
pfsync(4) and pflow(4) by using signed variables.  pfsync(4) problem
pointed out by camield.

Diff originally by dlg, frag and pflow bits by me.

feedback dlg
man page tweak jmc

Various versions of the pflow bits tested by Hrvoje Popovski
(hrvoje AT srce DOT hr), thanks!

ok benno, henning, dlg

6 files changed, 77 insertions, 64 deletions

diff --git a/sys/net/if_pflow.c b/sys/net/if_pflow.c
index 372098cd90b..f20e0b274e8 100644
--- a/sys/net/if_pflow.c
+++ b/sys/net/if_pflow.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pflow.c,v 1.20 2012/04/11 17:42:53 mikeb Exp $	*/
+/*	$OpenBSD: if_pflow.c,v 1.21 2012/10/30 12:09:05 florian Exp $	*/
 
 /*
  * Copyright (c) 2011 Florian Obser <florian@narrans.de>
@@ -553,12 +553,15 @@ copy_flow_data(struct pflow_flow *flow1, struct pflow_flow *flow2,
 	flow1->flow_octets = htonl(st->bytes[0]);
 	flow2->flow_octets = htonl(st->bytes[1]);
 
-	flow1->flow_start = flow2->flow_start =
-	    htonl(st->creation * 1000);
-	flow1->flow_finish = flow2->flow_finish =
-	    htonl((time_uptime - (st->rule.ptr->timeout[st->timeout] ?
-	    st->rule.ptr->timeout[st->timeout] :
-	    pf_default_rule.timeout[st->timeout])) * 1000);
+	/*
+	 * Pretend the flow was created or expired when the machine came up
+	 * when creation is in the future of the last time a package was seen
+	 * or was created / expired before this machine came up due to pfsync.
+	 */
+	flow1->flow_start = flow2->flow_start = st->creation < 0 ||
+	    st->creation > st->expire ? htonl(0) : htonl(st->creation * 1000);
+	flow1->flow_finish = flow2->flow_finish = st->expire < 0 ? htonl(0) :
+	    htonl(st->expire * 1000);
 	flow1->tcp_flags = flow2->tcp_flags = 0;
 	flow1->protocol = flow2->protocol = sk->proto;
 	flow1->tos = flow2->tos = st->rule.ptr->tos;
@@ -580,12 +583,15 @@ copy_flow4_data(struct pflow_flow4 *flow1, struct pflow_flow4 *flow2,
 	flow1->flow_octets = htobe64(st->bytes[0]);
 	flow2->flow_octets = htobe64(st->bytes[1]);
 
-	flow1->flow_start = flow2->flow_start =
-	    htonl(st->creation * 1000);
-	flow1->flow_finish = flow2->flow_finish =
-	    htonl((time_uptime - (st->rule.ptr->timeout[st->timeout] ?
-	    st->rule.ptr->timeout[st->timeout] :
-	    pf_default_rule.timeout[st->timeout])) * 1000);
+	/*
+	 * Pretend the flow was created or expired when the machine came up
+	 * when creation is in the future of the last time a package was seen
+	 * or was created / expired before this machine came up due to pfsync.
+	 */
+	flow1->flow_start = flow2->flow_start = st->creation < 0 ||
+	    st->creation > st->expire ? htonl(0) : htonl(st->creation * 1000);
+	flow1->flow_finish = flow2->flow_finish = st->expire < 0 ? htonl(0) :
+	    htonl(st->expire * 1000);
 
 	flow1->protocol = flow2->protocol = sk->proto;
 	flow1->tos = flow2->tos = st->rule.ptr->tos;
@@ -608,12 +614,15 @@ copy_flow6_data(struct pflow_flow6 *flow1, struct pflow_flow6 *flow2,
 	flow1->flow_octets = htobe64(st->bytes[0]);
 	flow2->flow_octets = htobe64(st->bytes[1]);
 
-	flow1->flow_start = flow2->flow_start =
-	    htonl(st->creation * 1000);
-	flow1->flow_finish = flow2->flow_finish =
-	    htonl((time_uptime - (st->rule.ptr->timeout[st->timeout] ?
-	    st->rule.ptr->timeout[st->timeout] :
-	    pf_default_rule.timeout[st->timeout])) * 1000);
+	/*
+	 * Pretend the flow was created or expired when the machine came up
+	 * when creation is in the future of the last time a package was seen
+	 * or was created / expired before this machine came up due to pfsync.
+	 */
+	flow1->flow_start = flow2->flow_start = st->creation < 0 ||
+	    st->creation > st->expire ? htonl(0) : htonl(st->creation * 1000);
+	flow1->flow_finish = flow2->flow_finish = st->expire < 0 ? htonl(0) :
+	    htonl(st->expire * 1000);
 
 	flow1->protocol = flow2->protocol = sk->proto;
 	flow1->tos = flow2->tos = st->rule.ptr->tos;
diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c
index eaf6c00161a..c9ae79f4f49 100644
--- a/sys/net/if_pfsync.c
+++ b/sys/net/if_pfsync.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pfsync.c,v 1.194 2012/10/09 11:16:28 markus Exp $	*/
+/*	$OpenBSD: if_pfsync.c,v 1.195 2012/10/30 12:09:05 florian Exp $	*/
 
 /*
  * Copyright (c) 2002 Michael Shalayeff
@@ -574,8 +574,8 @@ pfsync_state_import(struct pfsync_state *sp, int flags)
 	/* copy to state */
 	bcopy(&sp->rt_addr, &st->rt_addr, sizeof(st->rt_addr));
 	st->creation = time_uptime - ntohl(sp->creation);
-	st->expire = time_second;
-	if (sp->expire) {
+	st->expire = time_uptime;
+	if (ntohl(sp->expire)) {
 		u_int32_t timeout;
 
 		timeout = r->timeout[sp->timeout];
@@ -948,7 +948,7 @@ pfsync_in_upd(caddr_t buf, int len, int count, int flags)
 		if (sync < 2) {
 			pfsync_alloc_scrub_memory(&sp->dst, &st->dst);
 			pf_state_peer_ntoh(&sp->dst, &st->dst);
-			st->expire = time_second;
+			st->expire = time_uptime;
 			st->timeout = sp->timeout;
 		}
 		st->pfsync_time = time_uptime;
@@ -1022,7 +1022,7 @@ pfsync_in_upd_c(caddr_t buf, int len, int count, int flags)
 		if (sync < 2) {
 			pfsync_alloc_scrub_memory(&up->dst, &st->dst);
 			pf_state_peer_ntoh(&up->dst, &st->dst);
-			st->expire = time_second;
+			st->expire = time_uptime;
 			st->timeout = up->timeout;
 		}
 		st->pfsync_time = time_uptime;
diff --git a/sys/net/pf.c b/sys/net/pf.c
index f8fd5774176..eaff73afc6a 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf.c,v 1.813 2012/10/21 13:06:02 benno Exp $ */
+/*	$OpenBSD: pf.c,v 1.814 2012/10/30 12:09:05 florian Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -389,13 +389,13 @@ pf_init_threshold(struct pf_threshold *threshold,
 	threshold->limit = limit * PF_THRESHOLD_MULT;
 	threshold->seconds = seconds;
 	threshold->count = 0;
-	threshold->last = time_second;
+	threshold->last = time_uptime;
 }
 
 void
 pf_add_threshold(struct pf_threshold *threshold)
 {
-	u_int32_t t = time_second, diff = t - threshold->last;
+	u_int32_t t = time_uptime, diff = t - threshold->last;
 
 	if (diff >= threshold->seconds)
 		threshold->count = 0;
@@ -582,7 +582,7 @@ pf_insert_src_node(struct pf_src_node **sn, struct pf_rule *rule,
 void
 pf_remove_src_node(struct pf_src_node *sn)
 {
-	if (sn->states > 0 || sn->expire > time_second)
+	if (sn->states > 0 || sn->expire > time_uptime)
 		return;
 
 	if (sn->rule.ptr != NULL) {
@@ -1080,6 +1080,8 @@ pf_find_state_all(struct pf_state_key_cmp *key, u_int dir, int *more)
 void
 pf_state_export(struct pfsync_state *sp, struct pf_state *st)
 {
+	int32_t expire;
+
 	bzero(sp, sizeof(struct pfsync_state));
 
 	/* copy from state key */
@@ -1104,11 +1106,11 @@ pf_state_export(struct pfsync_state *sp, struct pf_state *st)
 	strlcpy(sp->ifname, st->kif->pfik_name, sizeof(sp->ifname));
 	bcopy(&st->rt_addr, &sp->rt_addr, sizeof(sp->rt_addr));
 	sp->creation = htonl(time_uptime - st->creation);
-	sp->expire = pf_state_expires(st);
-	if (sp->expire <= time_second)
+	expire = pf_state_expires(st);
+	if (expire <= time_uptime)
 		sp->expire = htonl(0);
 	else
-		sp->expire = htonl(sp->expire - time_second);
+		sp->expire = htonl(expire - time_uptime);
 
 	sp->direction = st->direction;
 	sp->log = st->log;
@@ -1169,22 +1171,25 @@ pf_purge_thread(void *v)
 	}
 }
 
-u_int32_t
+int32_t
 pf_state_expires(const struct pf_state *state)
 {
-	u_int32_t	timeout;
+	int32_t		timeout;
 	u_int32_t	start;
 	u_int32_t	end;
 	u_int32_t	states;
 
 	/* handle all PFTM_* > PFTM_MAX here */
 	if (state->timeout == PFTM_PURGE)
-		return (time_second);
+		return (0);
+
 	KASSERT(state->timeout != PFTM_UNLINKED);
 	KASSERT(state->timeout < PFTM_MAX);
+
 	timeout = state->rule.ptr->timeout[state->timeout];
 	if (!timeout)
 		timeout = pf_default_rule.timeout[state->timeout];
+
 	start = state->rule.ptr->timeout[PFTM_ADAPTIVE_START];
 	if (start) {
 		end = state->rule.ptr->timeout[PFTM_ADAPTIVE_END];
@@ -1195,12 +1200,12 @@ pf_state_expires(const struct pf_state *state)
 		states = pf_status.states;
 	}
 	if (end && states > start && start < end) {
-		if (states < end)
-			return (state->expire + timeout * (end - states) /
-			    (end - start));
-		else
-			return (time_second);
+		if (states >= end)
+			return (0);
+
+		timeout = timeout * (end - states) / (end - start);
 	}
+
 	return (state->expire + timeout);
 }
 
@@ -1213,7 +1218,7 @@ pf_purge_expired_src_nodes(int waslocked)
 	for (cur = RB_MIN(pf_src_tree, &tree_src_tracking); cur; cur = next) {
 	next = RB_NEXT(pf_src_tree, &tree_src_tracking, cur);
 
-		if (cur->states <= 0 && cur->expire <= time_second) {
+		if (cur->states <= 0 && cur->expire <= time_uptime) {
 			if (! locked) {
 				rw_enter_write(&pf_consistency_lock);
 				next = RB_NEXT(pf_src_tree,
@@ -1243,7 +1248,7 @@ pf_src_tree_remove_state(struct pf_state *s)
 			if (!timeout)
 				timeout =
 				    pf_default_rule.timeout[PFTM_SRC_NODE];
-			sni->sn->expire = time_second + timeout;
+			sni->sn->expire = time_uptime + timeout;
 		}
 		pool_put(&pf_sn_item_pl, sni);
 	}
@@ -1343,7 +1348,7 @@ pf_purge_expired_states(u_int32_t maxcheck)
 				locked = 1;
 			}
 			pf_free_state(cur);
-		} else if (pf_state_expires(cur) <= time_second) {
+		} else if (pf_state_expires(cur) <= time_uptime) {
 			/* unlink and free expired state */
 			pf_unlink_state(cur);
 			if (! locked) {
@@ -3758,7 +3763,7 @@ pf_create_state(struct pf_pdesc *pd, struct pf_rule *r, struct pf_rule *a,
 	}
 
 	s->creation = time_uptime;
-	s->expire = time_second;
+	s->expire = time_uptime;
 
 	if (pd->proto == IPPROTO_TCP) {
 		if (s->state_flags & PFSTATE_SCRUB_TCP &&
@@ -4195,7 +4200,7 @@ pf_tcp_track_full(struct pf_pdesc *pd, struct pf_state_peer *src,
 			src->state = dst->state = TCPS_TIME_WAIT;
 
 		/* update expire time */
-		(*state)->expire = time_second;
+		(*state)->expire = time_uptime;
 		if (src->state >= TCPS_FIN_WAIT_2 &&
 		    dst->state >= TCPS_FIN_WAIT_2)
 			(*state)->timeout = PFTM_TCP_CLOSED;
@@ -4372,7 +4377,7 @@ pf_tcp_track_sloppy(struct pf_pdesc *pd, struct pf_state_peer *src,
 		src->state = dst->state = TCPS_TIME_WAIT;
 
 	/* update expire time */
-	(*state)->expire = time_second;
+	(*state)->expire = time_uptime;
 	if (src->state >= TCPS_FIN_WAIT_2 &&
 	    dst->state >= TCPS_FIN_WAIT_2)
 		(*state)->timeout = PFTM_TCP_CLOSED;
@@ -4617,7 +4622,7 @@ pf_test_state_udp(struct pf_pdesc *pd, struct pf_state **state)
 		dst->state = PFUDPS_MULTIPLE;
 
 	/* update expire time */
-	(*state)->expire = time_second;
+	(*state)->expire = time_uptime;
 	if (src->state == PFUDPS_MULTIPLE && dst->state == PFUDPS_MULTIPLE)
 		(*state)->timeout = PFTM_UDP_MULTIPLE;
 	else
@@ -4762,7 +4767,7 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state,
 				return (ret);
 		}
 
-		(*state)->expire = time_second;
+		(*state)->expire = time_uptime;
 		(*state)->timeout = PFTM_ICMP_ERROR_REPLY;
 
 		/* translate source/destination address, if necessary */
@@ -5570,7 +5575,7 @@ pf_test_state_other(struct pf_pdesc *pd, struct pf_state **state)
 		dst->state = PFOTHERS_MULTIPLE;
 
 	/* update expire time */
-	(*state)->expire = time_second;
+	(*state)->expire = time_uptime;
 	if (src->state == PFOTHERS_MULTIPLE && dst->state == PFOTHERS_MULTIPLE)
 		(*state)->timeout = PFTM_OTHER_MULTIPLE;
 	else
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index f0db9cea7c1..426d8eaac01 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_ioctl.c,v 1.255 2012/09/20 09:43:49 camield Exp $ */
+/*	$OpenBSD: pf_ioctl.c,v 1.256 2012/10/30 12:09:05 florian Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -2338,7 +2338,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 
 		p = psn->psn_src_nodes;
 		RB_FOREACH(n, pf_src_tree, &tree_src_tracking) {
-			int	secs = time_second, diff;
+			int	secs = time_uptime, diff;
 
 			if ((nr + 1) * sizeof(*p) > (unsigned)psn->psn_len)
 				break;
@@ -2346,7 +2346,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 			bcopy(n, pstore, sizeof(*pstore));
 			if (n->rule.ptr != NULL)
 				pstore->rule.nr = n->rule.ptr->nr;
-			pstore->creation = time_uptime - pstore->creation;
+			pstore->creation = secs - pstore->creation;
 			if (pstore->expire > secs)
 				pstore->expire -= secs;
 			else
diff --git a/sys/net/pf_norm.c b/sys/net/pf_norm.c
index 45e0db4b6a5..5363366fb9f 100644
--- a/sys/net/pf_norm.c
+++ b/sys/net/pf_norm.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_norm.c,v 1.154 2012/05/12 13:08:48 mpf Exp $ */
+/*	$OpenBSD: pf_norm.c,v 1.155 2012/10/30 12:09:05 florian Exp $ */
 
 /*
  * Copyright 2001 Niels Provos <provos@citi.umich.edu>
@@ -96,7 +96,7 @@ struct pf_fragment {
 	RB_ENTRY(pf_fragment) fr_entry;
 	TAILQ_ENTRY(pf_fragment) frag_next;
 	TAILQ_HEAD(pf_fragq, pf_frent) fr_queue;
-	u_int32_t	fr_timeout;
+	int32_t		fr_timeout;
 	u_int16_t	fr_maxlen;	/* maximum length of single fragment */
 };
 
@@ -173,7 +173,7 @@ void
 pf_purge_expired_fragments(void)
 {
 	struct pf_fragment	*frag;
-	u_int32_t		 expire = time_second -
+	int32_t			 expire = time_uptime -
 				    pf_default_rule.timeout[PFTM_FRAG];
 
 	while ((frag = TAILQ_LAST(&pf_fragqueue, pf_fragqueue)) != NULL) {
@@ -238,7 +238,7 @@ pf_find_fragment(struct pf_fragment_cmp *key, struct pf_frag_tree *tree)
 	frag = RB_FIND(pf_frag_tree, tree, (struct pf_fragment *)key);
 	if (frag != NULL) {
 		/* XXX Are we sure we want to update the timeout? */
-		frag->fr_timeout = time_second;
+		frag->fr_timeout = time_uptime;
 		TAILQ_REMOVE(&pf_fragqueue, frag, frag_next);
 		TAILQ_INSERT_HEAD(&pf_fragqueue, frag, frag_next);
 	}
@@ -314,7 +314,7 @@ pf_fillup_fragment(struct pf_fragment_cmp *key, struct pf_frent *frent,
 
 		*(struct pf_fragment_cmp *)frag = *key;
 		TAILQ_INIT(&frag->fr_queue);
-		frag->fr_timeout = time_second;
+		frag->fr_timeout = time_uptime;
 		frag->fr_maxlen = frent->fe_len;
 
 		RB_INSERT(pf_frag_tree, &pf_frag_tree, frag);
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 2b4b83f4356..15bf7d8e28b 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfvar.h,v 1.371 2012/10/08 17:41:55 camield Exp $ */
+/*	$OpenBSD: pfvar.h,v 1.372 2012/10/30 12:09:05 florian Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -708,8 +708,8 @@ struct pf_src_node {
 	u_int32_t		 states;
 	u_int32_t		 conn;
 	struct pf_threshold	 conn_rate;
-	u_int32_t		 creation;
-	u_int32_t		 expire;
+	int32_t			 creation;
+	int32_t			 expire;
 	sa_family_t		 af;
 	sa_family_t		 naf;
 	u_int8_t		 type;
@@ -823,9 +823,9 @@ struct pf_state {
 	struct pfi_kif		*rt_kif;
 	u_int64_t		 packets[2];
 	u_int64_t		 bytes[2];
-	u_int32_t		 creation;
-	u_int32_t		 expire;
-	u_int32_t		 pfsync_time;
+	int32_t			 creation;
+	int32_t			 expire;
+	int32_t			 pfsync_time;
 	u_int16_t		 qid;
 	u_int16_t		 pqid;
 	u_int16_t		 tag;
@@ -1818,8 +1818,7 @@ int	pf_normalize_tcp_stateful(struct pf_pdesc *, u_short *,
 	    int *);
 int	pf_normalize_mss(struct pf_pdesc *, u_int16_t);
 void	pf_scrub(struct mbuf *, u_int16_t, sa_family_t, u_int8_t, u_int8_t);
-u_int32_t
-	pf_state_expires(const struct pf_state *);
+int32_t	pf_state_expires(const struct pf_state *);
 void	pf_purge_expired_fragments(void);
 int	pf_routable(struct pf_addr *addr, sa_family_t af, struct pfi_kif *,
 	    int);