Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 26 Apr 2021 12:28:04 GMT
From:      Kristof Provost <kp@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 6fcc8e042ac4 - main - pf: Allow multiple labels to be set on a rule
Message-ID:  <202104261228.13QCS4HQ045561@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=6fcc8e042ac480f9276177339f7de1ad0f95c1b0

commit 6fcc8e042ac480f9276177339f7de1ad0f95c1b0
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-04-20 09:04:48 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-04-26 12:14:21 +0000

    pf: Allow multiple labels to be set on a rule
    
    Allow up to 5 labels to be set on each rule.
    This offers more flexibility in using labels. For example, it replaces
    the customer 'schedule' keyword used by pfSense to terminate states
    according to a schedule.
    
    Reviewed by:    glebius
    MFC after:      2 weeks
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D29936
---
 lib/libpfctl/libpfctl.c                   | 18 ++++++++--
 lib/libpfctl/libpfctl.h                   |  2 +-
 sbin/pfctl/parse.y                        | 59 ++++++++++++++++++-------------
 sbin/pfctl/pfctl.c                        | 16 ++++++---
 sbin/pfctl/pfctl_parser.c                 |  5 +--
 sys/net/pfvar.h                           |  2 +-
 sys/netpfil/pf/pf.h                       |  1 +
 sys/netpfil/pf/pf_ioctl.c                 | 55 ++++++++++++++++++++++++----
 usr.sbin/bsnmpd/modules/snmp_pf/pf_snmp.c |  2 +-
 9 files changed, 116 insertions(+), 44 deletions(-)

diff --git a/lib/libpfctl/libpfctl.c b/lib/libpfctl/libpfctl.c
index 86117268a9d0..024244d7bea8 100644
--- a/lib/libpfctl/libpfctl.c
+++ b/lib/libpfctl/libpfctl.c
@@ -301,7 +301,8 @@ static void
 pf_nvrule_to_rule(const nvlist_t *nvl, struct pfctl_rule *rule)
 {
 	const uint64_t *skip;
-	size_t skipcount;
+	const char *const *labels;
+	size_t skipcount, labelcount;
 
 	rule->nr = nvlist_get_number(nvl, "nr");
 
@@ -314,7 +315,10 @@ pf_nvrule_to_rule(const nvlist_t *nvl, struct pfctl_rule *rule)
 	for (int i = 0; i < PF_SKIP_COUNT; i++)
 		rule->skip[i].nr = skip[i];
 
-	strlcpy(rule->label, nvlist_get_string(nvl, "label"), PF_RULE_LABEL_SIZE);
+	labels = nvlist_get_string_array(nvl, "labels", &labelcount);
+	assert(labelcount <= PF_RULE_MAX_LABEL_COUNT);
+	for (size_t i = 0; i < labelcount; i++)
+		strlcpy(rule->label[i], labels[i], PF_RULE_LABEL_SIZE);
 	strlcpy(rule->ifname, nvlist_get_string(nvl, "ifname"), IFNAMSIZ);
 	strlcpy(rule->qname, nvlist_get_string(nvl, "qname"), PF_QNAME_SIZE);
 	strlcpy(rule->pqname, nvlist_get_string(nvl, "pqname"), PF_QNAME_SIZE);
@@ -404,6 +408,7 @@ pfctl_add_rule(int dev, const struct pfctl_rule *r, const char *anchor,
 	u_int64_t timeouts[PFTM_MAX];
 	u_int64_t set_prio[2];
 	nvlist_t *nvl, *nvlr;
+	size_t labelcount;
 	int ret;
 
 	nvl = nvlist_create(0);
@@ -418,7 +423,14 @@ pfctl_add_rule(int dev, const struct pfctl_rule *r, const char *anchor,
 	pfctl_nv_add_rule_addr(nvlr, "src", &r->src);
 	pfctl_nv_add_rule_addr(nvlr, "dst", &r->dst);
 
-	nvlist_add_string(nvlr, "label", r->label);
+	labelcount = 0;
+	while (r->label[labelcount][0] != 0 &&
+	    labelcount < PF_RULE_MAX_LABEL_COUNT) {
+		nvlist_append_string_array(nvlr, "labels",
+		    r->label[labelcount]);
+		labelcount++;
+	}
+
 	nvlist_add_string(nvlr, "ifname", r->ifname);
 	nvlist_add_string(nvlr, "qname", r->qname);
 	nvlist_add_string(nvlr, "pqname", r->pqname);
diff --git a/lib/libpfctl/libpfctl.h b/lib/libpfctl/libpfctl.h
index 2c498189a6e4..e19187fc2526 100644
--- a/lib/libpfctl/libpfctl.h
+++ b/lib/libpfctl/libpfctl.h
@@ -53,7 +53,7 @@ struct pfctl_rule {
 	struct pf_rule_addr	 src;
 	struct pf_rule_addr	 dst;
 	union pf_rule_ptr	 skip[PF_SKIP_COUNT];
-	char			 label[PF_RULE_LABEL_SIZE];
+	char			 label[PF_RULE_MAX_LABEL_COUNT][PF_RULE_LABEL_SIZE];
 	char			 ifname[IFNAMSIZ];
 	char			 qname[PF_QNAME_SIZE];
 	char			 pqname[PF_QNAME_SIZE];
diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index e0314241eec3..6acfefbf5ad3 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -55,6 +55,7 @@ __FBSDID("$FreeBSD$");
 #include <net/altq/altq_hfsc.h>
 #include <net/altq/altq_fairq.h>
 
+#include <assert.h>
 #include <stdio.h>
 #include <unistd.h>
 #include <stdlib.h>
@@ -241,7 +242,8 @@ static struct filter_opts {
 	} keep;
 	int			 fragment;
 	int			 allowopts;
-	char			*label;
+	char			*label[PF_RULE_MAX_LABEL_COUNT];
+	int			 labelcount;
 	struct node_qassign	 queues;
 	char			*tag;
 	char			*match_tag;
@@ -256,7 +258,8 @@ static struct filter_opts {
 } filter_opts;
 
 static struct antispoof_opts {
-	char			*label;
+	char			*label[PF_RULE_MAX_LABEL_COUNT];
+	int			 labelcount;
 	u_int			 rtableid;
 } antispoof_opts;
 
@@ -349,7 +352,7 @@ int		 expand_skip_interface(struct node_if *);
 
 int	 check_rulestate(int);
 int	 getservice(char *);
-int	 rule_label(struct pfctl_rule *, char *);
+int	 rule_label(struct pfctl_rule *, char *s[PF_RULE_MAX_LABEL_COUNT]);
 int	 rt_tableid_max(void);
 
 void	 mv_rules(struct pfctl_ruleset *, struct pfctl_ruleset *);
@@ -887,7 +890,8 @@ anchorrule	: ANCHOR anchorname dir quick interface af proto fromto
 			r.match_tag_not = $9.match_tag_not;
 			if (rule_label(&r, $9.label))
 				YYERROR;
-			free($9.label);
+			for (int i = 0; i < PF_RULE_MAX_LABEL_COUNT; i++)
+				free($9.label[i]);
 			r.flags = $9.flags.b1;
 			r.flagset = $9.flags.b2;
 			if (($9.flags.b1 & $9.flags.b2) != $9.flags.b1) {
@@ -1333,7 +1337,8 @@ antispoof	: ANTISPOOF logquick antispoof_ifspc af antispoof_opts {
 				} else
 					free(hh);
 			}
-			free($5.label);
+			for (int i = 0; i < PF_RULE_MAX_LABEL_COUNT; i++)
+				free($5.label[i]);
 		}
 		;
 
@@ -1374,11 +1379,11 @@ antispoof_opts_l	: antispoof_opts_l antispoof_opt
 			;
 
 antispoof_opt	: label	{
-			if (antispoof_opts.label) {
-				yyerror("label cannot be redefined");
+			if (antispoof_opts.labelcount >= PF_RULE_MAX_LABEL_COUNT) {
+				yyerror("label can only be used %d times", PF_RULE_MAX_LABEL_COUNT);
 				YYERROR;
 			}
-			antispoof_opts.label = $1;
+			antispoof_opts.label[antispoof_opts.labelcount++] = $1;
 		}
 		| RTABLE NUMBER				{
 			if ($2 < 0 || $2 > rt_tableid_max()) {
@@ -2093,7 +2098,8 @@ pfrule		: action dir logquick interface route af proto fromto
 			r.match_tag_not = $9.match_tag_not;
 			if (rule_label(&r, $9.label))
 				YYERROR;
-			free($9.label);
+			for (int i = 0; i < PF_RULE_MAX_LABEL_COUNT; i++)
+				free($9.label[i]);
 			r.flags = $9.flags.b1;
 			r.flagset = $9.flags.b2;
 			if (($9.flags.b1 & $9.flags.b2) != $9.flags.b1) {
@@ -2531,11 +2537,11 @@ filter_opt	: USER uids {
 			filter_opts.allowopts = 1;
 		}
 		| label	{
-			if (filter_opts.label) {
-				yyerror("label cannot be redefined");
+			if (filter_opts.labelcount >= PF_RULE_MAX_LABEL_COUNT) {
+				yyerror("label can only be used %d times", PF_RULE_MAX_LABEL_COUNT);
 				YYERROR;
 			}
-			filter_opts.label = $1;
+			filter_opts.label[filter_opts.labelcount++] = $1;
 		}
 		| qname	{
 			if (filter_opts.queues.qname) {
@@ -5316,15 +5322,15 @@ expand_rule(struct pfctl_rule *r,
 	sa_family_t		 af = r->af;
 	int			 added = 0, error = 0;
 	char			 ifname[IF_NAMESIZE];
-	char			 label[PF_RULE_LABEL_SIZE];
+	char			 label[PF_RULE_MAX_LABEL_COUNT][PF_RULE_LABEL_SIZE];
 	char			 tagname[PF_TAG_NAME_SIZE];
 	char			 match_tagname[PF_TAG_NAME_SIZE];
 	struct pf_pooladdr	*pa;
 	struct node_host	*h;
 	u_int8_t		 flags, flagset, keep_state;
 
-	if (strlcpy(label, r->label, sizeof(label)) >= sizeof(label))
-		errx(1, "expand_rule: strlcpy");
+	memcpy(label, r->label, sizeof(r->label));
+	assert(sizeof(r->label) == sizeof(label));
 	if (strlcpy(tagname, r->tagname, sizeof(tagname)) >= sizeof(tagname))
 		errx(1, "expand_rule: strlcpy");
 	if (strlcpy(match_tagname, r->match_tagname, sizeof(match_tagname)) >=
@@ -5373,17 +5379,17 @@ expand_rule(struct pfctl_rule *r,
 		else
 			memset(r->ifname, '\0', sizeof(r->ifname));
 
-		if (strlcpy(r->label, label, sizeof(r->label)) >=
-		    sizeof(r->label))
-			errx(1, "expand_rule: strlcpy");
+		memcpy(r->label, label, sizeof(r->label));
 		if (strlcpy(r->tagname, tagname, sizeof(r->tagname)) >=
 		    sizeof(r->tagname))
 			errx(1, "expand_rule: strlcpy");
 		if (strlcpy(r->match_tagname, match_tagname,
 		    sizeof(r->match_tagname)) >= sizeof(r->match_tagname))
 			errx(1, "expand_rule: strlcpy");
-		expand_label(r->label, PF_RULE_LABEL_SIZE, r->ifname, r->af,
-		    src_host, src_port, dst_host, dst_port, proto->proto);
+		for (int i = 0; i < PF_RULE_MAX_LABEL_COUNT; i++)
+			expand_label(r->label[i], PF_RULE_LABEL_SIZE,
+			    r->ifname, r->af, src_host, src_port, dst_host,
+			    dst_port, proto->proto);
 		expand_label(r->tagname, PF_TAG_NAME_SIZE, r->ifname, r->af,
 		    src_host, src_port, dst_host, dst_port, proto->proto);
 		expand_label(r->match_tagname, PF_TAG_NAME_SIZE, r->ifname,
@@ -6273,13 +6279,16 @@ getservice(char *n)
 }
 
 int
-rule_label(struct pfctl_rule *r, char *s)
+rule_label(struct pfctl_rule *r, char *s[PF_RULE_MAX_LABEL_COUNT])
 {
-	if (s) {
-		if (strlcpy(r->label, s, sizeof(r->label)) >=
-		    sizeof(r->label)) {
+	for (int i = 0; i < PF_RULE_MAX_LABEL_COUNT; i++) {
+		if (s[i] == NULL)
+			return (0);
+
+		if (strlcpy(r->label[i], s[i], sizeof(r->label[0])) >=
+		    sizeof(r->label[0])) {
 			yyerror("rule label too long (max %d chars)",
-			    sizeof(r->label)-1);
+			    sizeof(r->label[0])-1);
 			return (-1);
 		}
 	}
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 82af047e7571..af2ae6fe3bf0 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -996,11 +996,18 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format,
 			goto error;
 
 		switch (format) {
-		case PFCTL_SHOW_LABELS:
-			if (rule.label[0]) {
-				printf("%s %llu %llu %llu %llu"
+		case PFCTL_SHOW_LABELS: {
+			bool show = false;
+			int i = 0;
+
+			while (rule.label[i][0]) {
+				printf("%s ", rule.label[i++]);
+				show = true;
+			}
+
+			if (show) {
+				printf("%llu %llu %llu %llu"
 				    " %llu %llu %llu %ju\n",
-				    rule.label,
 				    (unsigned long long)rule.evaluations,
 				    (unsigned long long)(rule.packets[0] +
 				    rule.packets[1]),
@@ -1013,6 +1020,7 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format,
 				    (uintmax_t)rule.states_tot);
 			}
 			break;
+		}
 		case PFCTL_SHOW_RULES:
 			brace = 0;
 			if (rule.label[0] && (opts & PF_OPT_SHOWALL))
diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
index 422e93460a43..282a0922bec7 100644
--- a/sbin/pfctl/pfctl_parser.c
+++ b/sbin/pfctl/pfctl_parser.c
@@ -1019,8 +1019,9 @@ print_rule(struct pfctl_rule *r, const char *anchor_call, int verbose, int numer
 
 		printf(" fragment reassemble");
 	}
-	if (r->label[0])
-		printf(" label \"%s\"", r->label);
+	i = 0;
+	while (r->label[i][0])
+		printf(" label \"%s\"", r->label[i++]);
 	if (r->qname[0] && r->pqname[0])
 		printf(" queue(%s, %s)", r->qname, r->pqname);
 	else if (r->qname[0])
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 3a79a281f916..5261bfe3bfb1 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -324,7 +324,7 @@ struct pf_krule {
 	struct pf_rule_addr	 src;
 	struct pf_rule_addr	 dst;
 	union pf_krule_ptr	 skip[PF_SKIP_COUNT];
-	char			 label[PF_RULE_LABEL_SIZE];
+	char			 label[PF_RULE_MAX_LABEL_COUNT][PF_RULE_LABEL_SIZE];
 	char			 ifname[IFNAMSIZ];
 	char			 qname[PF_QNAME_SIZE];
 	char			 pqname[PF_QNAME_SIZE];
diff --git a/sys/netpfil/pf/pf.h b/sys/netpfil/pf/pf.h
index 4192b1a5bc95..6df1426c9e0c 100644
--- a/sys/netpfil/pf/pf.h
+++ b/sys/netpfil/pf/pf.h
@@ -450,6 +450,7 @@ struct pf_rule {
 #define PF_SKIP_COUNT		8
 	union pf_rule_ptr	 skip[PF_SKIP_COUNT];
 #define PF_RULE_LABEL_SIZE	 64
+#define PF_RULE_MAX_LABEL_COUNT	 5
 	char			 label[PF_RULE_LABEL_SIZE];
 	char			 ifname[IFNAMSIZ];
 	char			 qname[PF_QNAME_SIZE];
diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
index 6158a6c9b0c6..fa78c98eca19 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -992,7 +992,8 @@ pf_hash_rule(MD5_CTX *ctx, struct pf_krule *rule)
 
 	pf_hash_rule_addr(ctx, &rule->src);
 	pf_hash_rule_addr(ctx, &rule->dst);
-	PF_MD5_UPD_STR(rule, label);
+	for (int i = 0; i < PF_RULE_MAX_LABEL_COUNT; i++)
+		PF_MD5_UPD_STR(rule, label[i]);
 	PF_MD5_UPD_STR(rule, ifname);
 	PF_MD5_UPD_STR(rule, match_tagname);
 	PF_MD5_UPD_HTONS(rule, match_tag, x); /* dup? */
@@ -1556,7 +1557,7 @@ pf_krule_to_rule(const struct pf_krule *krule, struct pf_rule *rule)
 			rule->skip[i].nr = krule->skip[i].ptr->nr;
 	}
 
-	strlcpy(rule->label, krule->label, sizeof(rule->label));
+	strlcpy(rule->label, krule->label[0], sizeof(rule->label));
 	strlcpy(rule->ifname, krule->ifname, sizeof(rule->ifname));
 	strlcpy(rule->qname, krule->qname, sizeof(rule->qname));
 	strlcpy(rule->pqname, krule->pqname, sizeof(rule->pqname));
@@ -1977,7 +1978,30 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule)
 	PFNV_CHK(pf_nvrule_addr_to_rule_addr(nvlist_get_nvlist(nvl, "dst"),
 	    &rule->dst));
 
-	PFNV_CHK(pf_nvstring(nvl, "label", rule->label, sizeof(rule->label)));
+	if (nvlist_exists_string(nvl, "label")) {
+		PFNV_CHK(pf_nvstring(nvl, "label", rule->label[0],
+		    sizeof(rule->label[0])));
+	} else if (nvlist_exists_string_array(nvl, "labels")) {
+		const char *const *strs;
+		size_t items;
+		int ret;
+
+		strs = nvlist_get_string_array(nvl, "labels", &items);
+		if (items > PF_RULE_MAX_LABEL_COUNT) {
+			error = E2BIG;
+			goto errout;
+		}
+
+		for (size_t i = 0; i < items; i++) {
+			ret = strlcpy(rule->label[i], strs[i],
+			    sizeof(rule->label[0]));
+			if (ret >= sizeof(rule->label[0])) {
+				error = E2BIG;
+				goto errout;
+			}
+		}
+	}
+
 	PFNV_CHK(pf_nvstring(nvl, "ifname", rule->ifname,
 	    sizeof(rule->ifname)));
 	PFNV_CHK(pf_nvstring(nvl, "qname", rule->qname, sizeof(rule->qname)));
@@ -2151,7 +2175,10 @@ pf_krule_to_nvrule(const struct pf_krule *rule)
 		    rule->skip[i].ptr ? rule->skip[i].ptr->nr : -1);
 	}
 
-	nvlist_add_string(nvl, "label", rule->label);
+	for (int i = 0; i < PF_RULE_MAX_LABEL_COUNT; i++) {
+		nvlist_append_string_array(nvl, "labels", rule->label[i]);
+	}
+	nvlist_add_string(nvl, "label", rule->label[0]);
 	nvlist_add_string(nvl, "ifname", rule->ifname);
 	nvlist_add_string(nvl, "qname", rule->qname);
 	nvlist_add_string(nvl, "pqname", rule->pqname);
@@ -2284,7 +2311,7 @@ pf_rule_to_krule(const struct pf_rule *rule, struct pf_krule *krule)
 	bcopy(&rule->src, &krule->src, sizeof(rule->src));
 	bcopy(&rule->dst, &krule->dst, sizeof(rule->dst));
 
-	strlcpy(krule->label, rule->label, sizeof(rule->label));
+	strlcpy(krule->label[0], rule->label, sizeof(rule->label));
 	strlcpy(krule->ifname, rule->ifname, sizeof(rule->ifname));
 	strlcpy(krule->qname, rule->qname, sizeof(rule->qname));
 	strlcpy(krule->pqname, rule->pqname, sizeof(rule->pqname));
@@ -2522,6 +2549,20 @@ errout_unlocked:
 	return (error);
 }
 
+static bool
+pf_label_match(const struct pf_krule *rule, const char *label)
+{
+	int i = 0;
+
+	while (*rule->label[i]) {
+		if (strcmp(rule->label[i], label) == 0)
+			return (true);
+		i++;
+	}
+
+	return (false);
+}
+
 static int
 pf_killstates_row(struct pfioc_state_kill *psk, struct pf_idhash *ih)
 {
@@ -2571,8 +2612,8 @@ relock_DIOCKILLSTATES:
 		    psk->psk_dst.port[0], psk->psk_dst.port[1], dstport))
 			continue;
 
-		if (psk->psk_label[0] && (! s->rule.ptr->label[0] ||
-		    strcmp(psk->psk_label, s->rule.ptr->label)))
+		if (psk->psk_label[0] &&
+		    ! pf_label_match(s->rule.ptr, psk->psk_label))
 			continue;
 
 		if (psk->psk_ifname[0] && strcmp(psk->psk_ifname,
diff --git a/usr.sbin/bsnmpd/modules/snmp_pf/pf_snmp.c b/usr.sbin/bsnmpd/modules/snmp_pf/pf_snmp.c
index 018f3751ca57..51d940f20c99 100644
--- a/usr.sbin/bsnmpd/modules/snmp_pf/pf_snmp.c
+++ b/usr.sbin/bsnmpd/modules/snmp_pf/pf_snmp.c
@@ -1545,7 +1545,7 @@ pfl_scan_ruleset(const char *path)
 			strlcpy(e->name, path, sizeof(e->name));
 			if (path[0])
 				strlcat(e->name, "/", sizeof(e->name));
-			strlcat(e->name, rule.label, sizeof(e->name));
+			strlcat(e->name, rule.label[0], sizeof(e->name));
 
 			e->evals = rule.evaluations;
 			e->bytes[IN] = rule.bytes[IN];



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202104261228.13QCS4HQ045561>