pf: allow the use of tables in ethernet rules

Allow tables to be used for the l3 source/destination matching.
This requires taking the PF_RULES read lock.

Sponsored by:	Rubicon Communications, LLC ("Netgate")
Differential Revision:	https://reviews.freebsd.org/D34917

7 files changed, 99 insertions, 12 deletions

diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index 5f84a39371f3..21729fc7ba4e 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -3262,13 +3262,15 @@ l3fromto	: /* empty */			{
 		}
 		| L3 fromto			{
 			if ($2.src.host != NULL &&
-			    $2.src.host->addr.type != PF_ADDR_ADDRMASK) {
-				yyerror("from must be an address");
+			    $2.src.host->addr.type != PF_ADDR_ADDRMASK &&
+			    $2.src.host->addr.type != PF_ADDR_TABLE) {
+				yyerror("from must be an address or table");
 				YYERROR;
 			}
 			if ($2.dst.host != NULL &&
-			    $2.dst.host->addr.type != PF_ADDR_ADDRMASK) {
-				yyerror("to must be an address");
+			    $2.dst.host->addr.type != PF_ADDR_ADDRMASK &&
+			    $2.dst.host->addr.type != PF_ADDR_TABLE) {
+				yyerror("to must be an address or table");
 				YYERROR;
 			}
 			$$ = $2;
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index db6b5c22f07f..adcf69905698 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -2153,6 +2153,8 @@ int	pfr_pool_get(struct pfr_ktable *, int *, struct pf_addr *, sa_family_t);
 void	pfr_dynaddr_update(struct pfr_ktable *, struct pfi_dynaddr *);
 struct pfr_ktable *
 	pfr_attach_table(struct pf_kruleset *, char *);
+struct pfr_ktable *
+	pfr_eth_attach_table(struct pf_keth_ruleset *, char *);
 void	pfr_detach_table(struct pfr_ktable *);
 int	pfr_clr_tables(struct pfr_table *, int *, int);
 int	pfr_add_tables(struct pfr_table *, int, int *, int);
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index a150c7e86e5e..d27e6f30f945 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -3845,6 +3845,8 @@ pf_test_eth_rule(int dir, struct pfi_kkif *kif, struct mbuf **m0)
 	MPASS(kif->pfik_ifp->if_vnet == curvnet);
 	NET_EPOCH_ASSERT();
 
+	PF_RULES_RLOCK_TRACKER;
+
 	SDT_PROBE3(pf, eth, test_rule, entry, dir, kif->pfik_ifp, m);
 
 	ruleset = V_pf_keth;
@@ -3892,6 +3894,8 @@ pf_test_eth_rule(int dir, struct pfi_kkif *kif, struct mbuf **m0)
 	e = mtod(m, struct ether_header *);
 	*m0 = m;
 
+	PF_RULES_RLOCK();
+
 	while (r != NULL) {
 		counter_u64_add(r->evaluations, 1);
 		SDT_PROBE2(pf, eth, test_rule, test, r->nr, r);
@@ -3959,20 +3963,25 @@ pf_test_eth_rule(int dir, struct pfi_kkif *kif, struct mbuf **m0)
 	SDT_PROBE2(pf, eth, test_rule, final_match, (r != NULL ? r->nr : -1), r);
 
 	/* Default to pass. */
-	if (r == NULL)
+	if (r == NULL) {
+		PF_RULES_RUNLOCK();
 		return (PF_PASS);
+	}
 
 	/* Execute action. */
 	counter_u64_add(r->packets[dir == PF_OUT], 1);
 	counter_u64_add(r->bytes[dir == PF_OUT], m_length(m, NULL));
 
 	/* Shortcut. Don't tag if we're just going to drop anyway. */
-	if (r->action == PF_DROP)
+	if (r->action == PF_DROP) {
+		PF_RULES_RUNLOCK();
 		return (PF_DROP);
+	}
 
 	if (r->tag > 0) {
 		mtag = pf_get_mtag(m);
 		if (mtag == NULL) {
+			PF_RULES_RUNLOCK();
 			counter_u64_add(V_pf_status.counters[PFRES_MEMORY], 1);
 			return (PF_DROP);
 		}
@@ -3982,6 +3991,7 @@ pf_test_eth_rule(int dir, struct pfi_kkif *kif, struct mbuf **m0)
 	if (r->qid != 0) {
 		mtag = pf_get_mtag(m);
 		if (mtag == NULL) {
+			PF_RULES_RUNLOCK();
 			counter_u64_add(V_pf_status.counters[PFRES_MEMORY], 1);
 			return (PF_DROP);
 		}
@@ -3997,6 +4007,7 @@ pf_test_eth_rule(int dir, struct pfi_kkif *kif, struct mbuf **m0)
 		 **/
 		mtag = pf_get_mtag(m);
 		if (mtag == NULL) {
+			PF_RULES_RUNLOCK();
 			counter_u64_add(V_pf_status.counters[PFRES_MEMORY], 1);
 			return (PF_DROP);
 		}
@@ -4006,6 +4017,8 @@ pf_test_eth_rule(int dir, struct pfi_kkif *kif, struct mbuf **m0)
 
 	action = r->action;
 
+	PF_RULES_RUNLOCK();
+
 	return (action);
 }
 
diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
index 49fdb6046098..86cbf7aa3808 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -520,6 +520,11 @@ pf_free_eth_rule(struct pf_keth_rule *rule)
 	if (rule->kif)
 		pfi_kkif_unref(rule->kif);
 
+	if (rule->ipsrc.addr.type == PF_ADDR_TABLE)
+		pfr_detach_table(rule->ipsrc.addr.p.tbl);
+	if (rule->ipdst.addr.type == PF_ADDR_TABLE)
+		pfr_detach_table(rule->ipdst.addr.p.tbl);
+
 	counter_u64_free(rule->evaluations);
 	for (int i = 0; i < 2; i++) {
 		counter_u64_free(rule->packets[i]);
@@ -1428,6 +1433,24 @@ pf_setup_pfsync_matching(struct pf_kruleset *rs)
 }
 
 static int
+pf_eth_addr_setup(struct pf_keth_ruleset *ruleset, struct pf_addr_wrap *addr)
+{
+	int error = 0;
+
+	switch (addr->type) {
+	case PF_ADDR_TABLE:
+		addr->p.tbl = pfr_eth_attach_table(ruleset, addr->v.tblname);
+		if (addr->p.tbl == NULL)
+			error = ENOMEM;
+		break;
+	default:
+		error = EINVAL;
+	}
+
+	return (error);
+}
+
+static int
 pf_addr_setup(struct pf_kruleset *ruleset, struct pf_addr_wrap *addr,
     sa_family_t af)
 {
@@ -2843,6 +2866,11 @@ DIOCGETETHRULE_error:
 			if ((rule->tag = pf_tagname2tag(rule->tagname)) == 0)
 				error = EBUSY;
 
+		if (error == 0 && rule->ipdst.addr.type == PF_ADDR_TABLE)
+			error = pf_eth_addr_setup(ruleset, &rule->ipdst.addr);
+		if (error == 0 && rule->ipsrc.addr.type == PF_ADDR_TABLE)
+			error = pf_eth_addr_setup(ruleset, &rule->ipsrc.addr);
+
 		if (error) {
 			pf_free_eth_rule(rule);
 			PF_RULES_WUNLOCK();
diff --git a/sys/netpfil/pf/pf_nv.c b/sys/netpfil/pf/pf_nv.c
index 85666a5ee111..4dcb5441f835 100644
--- a/sys/netpfil/pf/pf_nv.c
+++ b/sys/netpfil/pf/pf_nv.c
@@ -1147,7 +1147,8 @@ pf_nveth_rule_to_keth_rule(const nvlist_t *nvl,
 		if (error != 0)
 			return (error);
 
-		if (krule->ipsrc.addr.type != PF_ADDR_ADDRMASK)
+		if (krule->ipsrc.addr.type != PF_ADDR_ADDRMASK &&
+		    krule->ipsrc.addr.type != PF_ADDR_TABLE)
 			return (EINVAL);
 	}
 
@@ -1157,7 +1158,8 @@ pf_nveth_rule_to_keth_rule(const nvlist_t *nvl,
 		if (error != 0)
 			return (error);
 
-		if (krule->ipdst.addr.type != PF_ADDR_ADDRMASK)
+		if (krule->ipdst.addr.type != PF_ADDR_ADDRMASK &&
+		    krule->ipdst.addr.type != PF_ADDR_TABLE)
 			return (EINVAL);
 	}
 
diff --git a/sys/netpfil/pf/pf_table.c b/sys/netpfil/pf/pf_table.c
index a2afd5c24cff..9b048d899810 100644
--- a/sys/netpfil/pf/pf_table.c
+++ b/sys/netpfil/pf/pf_table.c
@@ -2152,6 +2152,44 @@ pfr_update_stats(struct pfr_ktable *kt, struct pf_addr *a, sa_family_t af,
 }
 
 struct pfr_ktable *
+pfr_eth_attach_table(struct pf_keth_ruleset *rs, char *name)
+{
+	struct pfr_ktable	*kt, *rt;
+	struct pfr_table	 tbl;
+	struct pf_keth_anchor	*ac = rs->anchor;
+
+	PF_RULES_WASSERT();
+
+	bzero(&tbl, sizeof(tbl));
+	strlcpy(tbl.pfrt_name, name, sizeof(tbl.pfrt_name));
+	if (ac != NULL)
+		strlcpy(tbl.pfrt_anchor, ac->path, sizeof(tbl.pfrt_anchor));
+	kt = pfr_lookup_table(&tbl);
+	if (kt == NULL) {
+		kt = pfr_create_ktable(&tbl, time_second, 1);
+		if (kt == NULL)
+			return (NULL);
+		if (ac != NULL) {
+			bzero(tbl.pfrt_anchor, sizeof(tbl.pfrt_anchor));
+			rt = pfr_lookup_table(&tbl);
+			if (rt == NULL) {
+				rt = pfr_create_ktable(&tbl, 0, 1);
+				if (rt == NULL) {
+					pfr_destroy_ktable(kt, 0);
+					return (NULL);
+				}
+				pfr_insert_ktable(rt);
+			}
+			kt->pfrkt_root = rt;
+		}
+		pfr_insert_ktable(kt);
+	}
+	if (!kt->pfrkt_refcnt[PFR_REFCNT_RULE]++)
+		pfr_setflags_ktable(kt, kt->pfrkt_flags|PFR_TFLAG_REFERENCED);
+	return (kt);
+}
+
+struct pfr_ktable *
 pfr_attach_table(struct pf_kruleset *rs, char *name)
 {
 	struct pfr_ktable	*kt, *rt;
diff --git a/tests/sys/netpfil/pf/ether.sh b/tests/sys/netpfil/pf/ether.sh
index 7a7f91844148..da936c9cfaeb 100644
--- a/tests/sys/netpfil/pf/ether.sh
+++ b/tests/sys/netpfil/pf/ether.sh
@@ -537,10 +537,12 @@ ip_body()
 		"ether block out l3 to 192.0.2.3"
 	atf_check -s exit:2 -o ignore ping -c 1 192.0.2.2
 
-	# We can't use tables in these rules
-	echo "ether pass out l3 from <test>" | \
-	    atf_check -s exit:1 -o ignore -e ignore \
-	    jexec alcatraz pfctl -g -f -
+	# Test table
+	pft_set_rules alcatraz \
+		"table <tbl> { 192.0.2.3 }" \
+		"ether pass" \
+		"ether block out l3 to <tbl>"
+	atf_check -s exit:2 -o ignore ping -c 1 192.0.2.2
 }
 
 ip_cleanup()