pfsync: when importing a state clear take the interface name into account

When one pfsync host clears states it informs its peers about this.
While processing such messages, in pfsync_in_clr() we failed to take the
interface name into account.
This meant that if one host cleared states on one interface the peers
would clear all states, not just those on the affected interface.

Actually check for the interface in pfsync_in_clr()

Sponsored by:	Rubicon Communications, LLC ("Netgate")

sys/netpfil/pf/if_pfsync.c

@@ -1143,6 +1143,7 @@ static int
 pfsync_in_clr(struct mbuf *m, int offset, int count, int flags, int action)
 {
 	struct pfsync_clr *clr;
+	struct pfi_kkif *kif = NULL;
 	struct mbuf *mp;
 	int len = sizeof(*clr) * count;
 	int i, offp;
@@ -1159,7 +1160,7 @@ pfsync_in_clr(struct mbuf *m, int offset, int count, int flags, int action)
 		creatorid = clr[i].creatorid;
 
 		if (clr[i].ifname[0] != '\0' &&
-		    pfi_kkif_find(clr[i].ifname) == NULL)
+		    (kif = pfi_kkif_find(clr[i].ifname)) == NULL)
 			continue;
 
 		for (int i = 0; i <= V_pf_hashmask; i++) {
@@ -1168,11 +1169,14 @@ pfsync_in_clr(struct mbuf *m, int offset, int count, int flags, int action)
 relock:
 			PF_HASHROW_LOCK(ih);
 			LIST_FOREACH(s, &ih->states, entry) {
-				if (s->creatorid == creatorid) {
-					s->state_flags |= PFSTATE_NOSYNC;
-					pf_remove_state(s);
-					goto relock;
-				}
+				if (s->creatorid != creatorid)
+					continue;
+				if (kif != NULL && kif != s->kif)
+					continue;
+
+				s->state_flags |= PFSTATE_NOSYNC;
+				pf_remove_state(s);
+				goto relock;
 			}
 			PF_HASHROW_UNLOCK(ih);
 		}

tests/sys/netpfil/pf/pfsync.sh

@@ -1657,6 +1657,91 @@ rt_af_cleanup()
 	pfsynct_cleanup
 }
 
+atf_test_case "flush_by_intf" "cleanup"
+flush_by_intf_head()
+{
+	atf_set descr 'Test flushing states by interface'
+	atf_set require.user root
+}
+
+flush_by_intf_body()
+{
+	pfsynct_init
+
+	epair_sync=$(vnet_mkepair)
+	epair_one=$(vnet_mkepair)
+	epair_two=$(vnet_mkepair)
+
+	vnet_mkjail one ${epair_one}a ${epair_sync}a
+	vnet_mkjail two ${epair_two}a ${epair_sync}b
+
+	# pfsync interface
+	jexec one ifconfig ${epair_sync}a 192.0.2.1/24 up
+	jexec one ifconfig ${epair_one}a name epair_foo
+	jexec one ifconfig epair_foo 198.51.100.1/24 up
+	jexec one ifconfig pfsync0 \
+		syncdev ${epair_sync}a \
+		maxupd 1 \
+		up
+	jexec two ifconfig ${epair_sync}b 192.0.2.2/24 up
+	jexec two ifconfig ${epair_two}a name epair_foo
+	jexec two ifconfig epair_foo 198.51.100.2/24 up
+	jexec two ifconfig pfsync0 \
+		syncdev ${epair_sync}b \
+		maxupd 1 \
+		up
+
+	jexec one pfctl -e
+	pft_set_rules one \
+		"set state-policy if-bound" \
+		"set skip on ${epair_sync}a" \
+		"pass out keep state"
+	jexec two pfctl -e
+	pft_set_rules two \
+		"set state-policy if-bound" \
+		"set skip on ${epair_sync}b" \
+		"pass out keep state"
+
+	ifconfig ${epair_one}b 198.51.100.254/24 up
+
+	ping -c 1 -S 198.51.100.254 198.51.100.1
+
+	# Give pfsync time to do its thing
+	sleep 2
+
+	if ! jexec two pfctl -s states | grep icmp | grep 198.51.100.1 | \
+	    grep 198.51.100.254 ; then
+		atf_fail "state not found on synced host"
+	fi
+
+	# Now flush states in one, with an interface specified
+	# (Note the interface must exist on both hosts for the bug to manifest)
+	jexec one pfctl -i lo0 -Fs
+
+	sleep 2
+
+	if ! jexec two pfctl -s states | grep icmp | grep 198.51.100.1 | \
+	    grep 198.51.100.254 ; then
+		atf_fail "state was removed on synced host!"
+	fi
+
+	# Now flush on the interface the state is actually on, this should get passed to
+	# the peer.
+	jexec one pfctl -i epair_foo -Fs
+
+	sleep 2
+
+	if jexec two pfctl -s states | grep icmp | grep 198.51.100.1 | \
+	    grep 198.51.100.254 ; then
+		atf_fail "state was not removed on synced host!"
+	fi
+}
+
+flush_by_intf_cleanup()
+{
+	pfsynct_cleanup
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case "basic"
@@ -1681,4 +1766,5 @@ atf_init_test_cases()
 	atf_add_test_case "tag"
 	atf_add_test_case "altq_queues"
 	atf_add_test_case "rt_af"
+	atf_add_test_case "flush_by_intf"
 }