Fix for PR3983

- Add a new PFSTATE_STALE flag to uncompressed state updates sent as a result
  of a stale state being detected, and prevent updates with this flag from
  generating similar messages.
- For the specific case where the state->src in the recieved update is ok but
  the state.dst is not, take the partial update, then "fail" to let the other
  peers pick up the better data that we have. From Chris Pascoe.

ok dhartmei@

3 files changed, 43 insertions, 18 deletions

diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c
index 0bf5f902769..aefa4aa19de 100644
--- a/sys/net/if_pfsync.c
+++ b/sys/net/if_pfsync.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pfsync.c,v 1.38 2004/09/17 21:49:15 mcbride Exp $	*/
+/*	$OpenBSD: if_pfsync.c,v 1.39 2004/11/16 20:07:56 mcbride Exp $	*/
 
 /*
  * Copyright (c) 2002 Michael Shalayeff
@@ -388,6 +388,8 @@ pfsync_input(struct mbuf *m, ...)
 		s = splsoftnet();
 		for (i = 0, sp = (struct pfsync_state *)(mp->m_data + offp);
 		    i < count; i++, sp++) {
+			int flags = PFSYNC_FLAG_STALE;
+
 			/* check for invalid values */
 			if (sp->timeout >= PFTM_MAX ||
 			    sp->src.state > PF_TCPS_PROXY_DST ||
@@ -420,12 +422,21 @@ pfsync_input(struct mbuf *m, ...)
 				    (st->src.state < PF_TCPS_PROXY_SRC ||
 				    sp->src.state >= PF_TCPS_PROXY_SRC))
 					sfail = 1;
-				else if (st->dst.state > sp->dst.state)
-					sfail = 2;
 				else if (SEQ_GT(st->src.seqlo,
 				    ntohl(sp->src.seqlo)))
 					sfail = 3;
-				else if (st->dst.state >= TCPS_SYN_SENT &&
+				else if (st->dst.state > sp->dst.state) {
+					/* There might still be useful
+					 * information about the src state here,
+					 * so import that part of the update,
+					 * then "fail" so we send the updated
+					 * state back to the peer who is missing
+					 * our what we know. */
+					pf_state_peer_ntoh(&sp->src, &st->src);
+					/* XXX do anything with timeouts? */
+					sfail = 7;
+					flags = 0;
+				} else if (st->dst.state >= TCPS_SYN_SENT &&
 				    SEQ_GT(st->dst.seqlo, ntohl(sp->dst.seqlo)))
 					sfail = 4;
 			} else {
@@ -440,18 +451,23 @@ pfsync_input(struct mbuf *m, ...)
 			}
 			if (sfail) {
 				if (pf_status.debug >= PF_DEBUG_MISC)
-					printf("pfsync: ignoring stale update "
+					printf("pfsync: %s stale update "
 					    "(%d) id: %016llx "
-					    "creatorid: %08x\n", sfail,
+					    "creatorid: %08x\n",
+					    (sfail < 7 ?  "ignoring"
+					     : "partial"), sfail,
 					    betoh64(st->id),
 					    ntohl(st->creatorid));
 				pfsyncstats.pfsyncs_badstate++;
 
-				/* we have a better state, send it out */
-				if (sc->sc_mbuf != NULL && !stale)
-					pfsync_sendout(sc);
-				stale++;
-				pfsync_pack_state(PFSYNC_ACT_UPD, st, 0);
+				if (!(sp->sync_flags & PFSTATE_STALE)) {
+					/* we have a better state, send it */
+					if (sc->sc_mbuf != NULL && !stale)
+						pfsync_sendout(sc);
+					stale++;
+					pfsync_pack_state(PFSYNC_ACT_UPD, st,
+					    flags);
+				}
 				continue;
 			}
 			pf_state_peer_ntoh(&sp->src, &st->src);
@@ -575,7 +591,8 @@ pfsync_input(struct mbuf *m, ...)
 					update_requested = 0;
 				}
 				stale++;
-				pfsync_pack_state(PFSYNC_ACT_UPD, st, 0);
+				pfsync_pack_state(PFSYNC_ACT_UPD, st,
+				    PFSYNC_FLAG_STALE);
 				continue;
 			}
 			pf_state_peer_ntoh(&up->src, &st->src);
@@ -939,7 +956,7 @@ pfsync_get_mbuf(struct pfsync_softc *sc, u_int8_t action, void **sp)
 }
 
 int
-pfsync_pack_state(u_int8_t action, struct pf_state *st, int compress)
+pfsync_pack_state(u_int8_t action, struct pf_state *st, int flags)
 {
 	struct ifnet *ifp = &pfsyncif.sc_if;
 	struct pfsync_softc *sc = ifp->if_softc;
@@ -1057,6 +1074,8 @@ pfsync_pack_state(u_int8_t action, struct pf_state *st, int compress)
 		sp->timeout = st->timeout;
 
 		sp->sync_flags = st->sync_flags & PFSTATE_NOSYNC;
+		if (flags & PFSYNC_FLAG_STALE)
+			sp->sync_flags = st->sync_flags & PFSTATE_STALE;
 	}
 
 	pf_state_peer_hton(&st->src, &sp->src);
@@ -1068,7 +1087,7 @@ pfsync_pack_state(u_int8_t action, struct pf_state *st, int compress)
 		sp->expire = htonl(st->expire - secs);
 
 	/* do we need to build "compressed" actions for network transfer? */
-	if (sc->sc_sync_ifp && compress) {
+	if (sc->sc_sync_ifp && flags & PFSYNC_FLAG_COMPRESS) {
 		switch (action) {
 		case PFSYNC_ACT_UPD:
 			newaction = PFSYNC_ACT_UPD_C;
diff --git a/sys/net/if_pfsync.h b/sys/net/if_pfsync.h
index 76cebbe7d2b..a908017ca70 100644
--- a/sys/net/if_pfsync.h
+++ b/sys/net/if_pfsync.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pfsync.h,v 1.16 2004/08/03 05:32:28 mcbride Exp $	*/
+/*	$OpenBSD: if_pfsync.h,v 1.17 2004/11/16 20:07:56 mcbride Exp $	*/
 
 /*
  * Copyright (c) 2001 Michael Shalayeff
@@ -85,6 +85,9 @@ struct pfsync_state {
 	u_int8_t	 updates;
 } __packed;
 
+#define PFSYNC_FLAG_COMPRESS 	0x01
+#define PFSYNC_FLAG_STALE	0x02
+
 struct pfsync_state_upd {
 	u_int32_t		id[2];
 	struct pfsync_state_peer	src;
@@ -271,12 +274,14 @@ int pfsync_pack_state(u_int8_t, struct pf_state *, int);
 } while (0)
 #define pfsync_update_state(st) do {				\
 	if (!st->sync_flags)					\
-		pfsync_pack_state(PFSYNC_ACT_UPD, (st), 1);	\
+		pfsync_pack_state(PFSYNC_ACT_UPD, (st), 	\
+		    PFSYNC_FLAG_COMPRESS);			\
 	st->sync_flags &= ~PFSTATE_FROMSYNC;			\
 } while (0)
 #define pfsync_delete_state(st) do {				\
 	if (!st->sync_flags)					\
-		pfsync_pack_state(PFSYNC_ACT_DEL, (st), 1);	\
+		pfsync_pack_state(PFSYNC_ACT_DEL, (st),		\
+		    PFSYNC_FLAG_COMPRESS);			\
 	st->sync_flags &= ~PFSTATE_FROMSYNC;			\
 } while (0)
 #endif
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 6bd93c7a714..208e6ac0af5 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfvar.h,v 1.203 2004/09/21 16:59:12 aaron Exp $ */
+/*	$OpenBSD: pfvar.h,v 1.204 2004/11/16 20:07:57 mcbride Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -645,6 +645,7 @@ struct pf_state {
 	u_int8_t	 sync_flags;
 #define	PFSTATE_NOSYNC	 0x01
 #define	PFSTATE_FROMSYNC 0x02
+#define	PFSTATE_STALE	 0x04
 	u_int8_t	 pad;
 };