fix anchor rules with filter opts, introduce filteropts_to_rule()

Some filter options were parsed but not set on anchor rules due to missing
copies of the respective struct members:

	$ cat pf.conf
	queue rq on trunk0 bandwidth 1G
	queue dq parent rq bandwidth 1G default
	anchor a set queue dq
	$ pfctl -vnf pf.conf | fgrep queue
	anchor "a" all

Fix this by moving common code from `anchorrule' and `pfrule' into a new
helper filteropts_to_rule().

Input from henning and benno
OK henning sashan jca

1 files changed, 124 insertions, 140 deletions

diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index dec93585d31..9d7e179cb5b 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.y,v 1.676 2018/07/09 15:07:06 kn Exp $	*/
+/*	$OpenBSD: parse.y,v 1.677 2018/07/10 08:44:55 kn Exp $	*/
 
 /*
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
@@ -391,6 +391,7 @@ u_int16_t parseicmpspec(char *, sa_family_t);
 int	 kw_casecmp(const void *, const void *);
 int	 map_tos(char *string, int *);
 int	 rdomain_exists(u_int);
+int	 filteropts_to_rule(struct pf_rule *, struct filter_opts *);
 
 TAILQ_HEAD(loadanchorshead, loadanchors)
     loadanchorshead = TAILQ_HEAD_INITIALIZER(loadanchorshead);
@@ -902,35 +903,7 @@ anchorrule	: ANCHOR anchorname dir quick interface af proto fromto
 			r.direction = $3;
 			r.quick = $4.quick;
 			r.af = $6;
-			r.prob = $9.prob;
-			r.rtableid = $9.rtableid;
-			r.pktrate.limit = $9.pktrate.limit;
-			r.pktrate.seconds = $9.pktrate.seconds;
-
-			if ($9.tag)
-				if (strlcpy(r.tagname, $9.tag,
-				    PF_TAG_NAME_SIZE) >= PF_TAG_NAME_SIZE) {
-					yyerror("tag too long, max %u chars",
-					    PF_TAG_NAME_SIZE - 1);
-					YYERROR;
-				}
-			if ($9.match_tag)
-				if (strlcpy(r.match_tagname, $9.match_tag,
-				    PF_TAG_NAME_SIZE) >= PF_TAG_NAME_SIZE) {
-					yyerror("tag too long, max %u chars",
-					    PF_TAG_NAME_SIZE - 1);
-					YYERROR;
-				}
-			r.match_tag_not = $9.match_tag_not;
-			if (rule_label(&r, $9.label))
-				YYERROR;
-			free($9.label);
-			r.flags = $9.flags.b1;
-			r.flagset = $9.flags.b2;
-			if (($9.flags.b1 & $9.flags.b2) != $9.flags.b1) {
-				yyerror("flags always false");
-				YYERROR;
-			}
+
 			if ($9.flags.b1 || $9.flags.b2 || $8.src_os) {
 				for (proto = $7; proto != NULL &&
 				    proto->proto != IPPROTO_TCP;
@@ -948,7 +921,8 @@ anchorrule	: ANCHOR anchorname dir quick interface af proto fromto
 				}
 			}
 
-			r.tos = $9.tos;
+			if (filteropts_to_rule(&r, &$9))
+				YYERROR;
 
 			if ($9.keep.action) {
 				yyerror("cannot specify state handling "
@@ -968,26 +942,6 @@ anchorrule	: ANCHOR anchorname dir quick interface af proto fromto
 				YYERROR;
 			}
 
-			if ($9.match_tag)
-				if (strlcpy(r.match_tagname, $9.match_tag,
-				    PF_TAG_NAME_SIZE) >= PF_TAG_NAME_SIZE) {
-					yyerror("tag too long, max %u chars",
-					    PF_TAG_NAME_SIZE - 1);
-					YYERROR;
-				}
-			r.match_tag_not = $9.match_tag_not;
-			if ($9.marker & FOM_PRIO) {
-				if ($9.prio == 0)
-					r.prio = PF_PRIO_ZERO;
-				else
-					r.prio = $9.prio;
-			}
-			if ($9.marker & FOM_SETPRIO) {
-				r.set_prio[0] = $9.set_prio[0];
-				r.set_prio[1] = $9.set_prio[1];
-				r.scrub_flags |= PFSTATE_SETPRIO;
-			}
-
 			decide_address_family($8.src.host, &r.af);
 			decide_address_family($8.dst.host, &r.af);
 
@@ -1590,34 +1544,11 @@ pfrule		: action dir logquick interface af proto fromto
 			r.log = $3.log;
 			r.logif = $3.logif;
 			r.quick = $3.quick;
-			r.prob = $8.prob;
-			r.rtableid = $8.rtableid;
-
-			if ($8.nodf)
-				r.scrub_flags |= PFSTATE_NODF;
-			if ($8.randomid)
-				r.scrub_flags |= PFSTATE_RANDOMID;
-			if ($8.minttl)
-				r.min_ttl = $8.minttl;
-			if ($8.max_mss)
-				r.max_mss = $8.max_mss;
-			if ($8.marker & FOM_SETTOS) {
-				r.scrub_flags |= PFSTATE_SETTOS;
-				r.set_tos = $8.settos;
-			}
-			if ($8.marker & FOM_SCRUB_TCP)
-				r.scrub_flags |= PFSTATE_SCRUB_TCP;
-			if ($8.marker & FOM_PRIO) {
-				if ($8.prio == 0)
-					r.prio = PF_PRIO_ZERO;
-				else
-					r.prio = $8.prio;
-			}
-			if ($8.marker & FOM_SETPRIO) {
-				r.set_prio[0] = $8.set_prio[0];
-				r.set_prio[1] = $8.set_prio[1];
-				r.scrub_flags |= PFSTATE_SETPRIO;
-			}
+			r.af = $5;
+
+			if (filteropts_to_rule(&r, &$8))
+				YYERROR;
+
 			if ($8.marker & FOM_ONCE) {
 				if (r.action == PF_MATCH) {
 					yyerror("can't specify once for "
@@ -1626,43 +1557,7 @@ pfrule		: action dir logquick interface af proto fromto
 				}
 				r.rule_flag |= PFRULE_ONCE;
 			}
-			if ($8.marker & FOM_AFTO)
-				r.rule_flag |= PFRULE_AFTO;
-			if (($8.marker & FOM_AFTO) && r.direction != PF_IN) {
-				yyerror("af-to can only be used with direction in");
-				YYERROR;
-			}
-			if (($8.marker & FOM_AFTO) && $8.route.rt) {
-				yyerror("af-to cannot be used together with "
-				    "route-to, reply-to, dup-to");
-				YYERROR;
-			}
-			r.af = $5;
 
-			if ($8.tag)
-				if (strlcpy(r.tagname, $8.tag,
-				    PF_TAG_NAME_SIZE) >= PF_TAG_NAME_SIZE) {
-					yyerror("tag too long, max %u chars",
-					    PF_TAG_NAME_SIZE - 1);
-					YYERROR;
-				}
-			if ($8.match_tag)
-				if (strlcpy(r.match_tagname, $8.match_tag,
-				    PF_TAG_NAME_SIZE) >= PF_TAG_NAME_SIZE) {
-					yyerror("tag too long, max %u chars",
-					    PF_TAG_NAME_SIZE - 1);
-					YYERROR;
-				}
-			r.match_tag_not = $8.match_tag_not;
-			if (rule_label(&r, $8.label))
-				YYERROR;
-			free($8.label);
-			r.flags = $8.flags.b1;
-			r.flagset = $8.flags.b2;
-			if (($8.flags.b1 & $8.flags.b2) != $8.flags.b1) {
-				yyerror("flags always false");
-				YYERROR;
-			}
 			if ($8.flags.b1 || $8.flags.b2 || $7.src_os) {
 				for (proto = $6; proto != NULL &&
 				    proto->proto != IPPROTO_TCP;
@@ -1680,9 +1575,6 @@ pfrule		: action dir logquick interface af proto fromto
 				}
 			}
 
-			r.tos = $8.tos;
-			r.pktrate.limit = $8.pktrate.limit;
-			r.pktrate.seconds = $8.pktrate.seconds;
 			r.keep_state = $8.keep.action;
 			o = $8.keep.options;
 
@@ -1910,10 +1802,6 @@ pfrule		: action dir logquick interface af proto fromto
 			if (r.keep_state && !statelock)
 				r.rule_flag |= default_statelock;
 
-			if ($8.fragment)
-				r.rule_flag |= PFRULE_FRAGMENT;
-			r.allow_opts = $8.allowopts;
-
 			decide_address_family($7.src.host, &r.af);
 			decide_address_family($7.dst.host, &r.af);
 
@@ -1949,24 +1837,7 @@ pfrule		: action dir logquick interface af proto fromto
 					err(1, "$8.rroute.rdr");
 				$8.rroute.rdr->host = $8.route.host;
 			}
-			if ($8.queues.qname != NULL) {
-				if (strlcpy(r.qname, $8.queues.qname,
-				    sizeof(r.qname)) >= sizeof(r.qname)) {
-					yyerror("rule qname too long (max "
-					    "%d chars)", sizeof(r.qname)-1);
-					YYERROR;
-				}
-				free($8.queues.qname);
-			}
-			if ($8.queues.pqname != NULL) {
-				if (strlcpy(r.pqname, $8.queues.pqname,
-				    sizeof(r.pqname)) >= sizeof(r.pqname)) {
-					yyerror("rule pqname too long (max "
-					    "%d chars)", sizeof(r.pqname)-1);
-					YYERROR;
-				}
-				free($8.queues.pqname);
-			}
+
 			if (expand_divertspec(&r, &$8.divert))
 				YYERROR;
 
@@ -5979,3 +5850,116 @@ rdomain_exists(u_int rdomain)
 	/* rdomain is a table, but not an rdomain */
 	return 0;
 }
+
+int
+filteropts_to_rule(struct pf_rule *r, struct filter_opts *opts)
+{
+
+	r->keep_state = opts->keep.action;
+	r->pktrate.limit = opts->pktrate.limit;
+	r->pktrate.seconds = opts->pktrate.seconds;
+	r->prob = opts->prob;
+	r->rtableid = opts->rtableid;
+	r->tos = opts->tos;
+
+	if (opts->nodf)
+		r->scrub_flags |= PFSTATE_NODF;
+	if (opts->randomid)
+		r->scrub_flags |= PFSTATE_RANDOMID;
+	if (opts->minttl)
+		r->min_ttl = opts->minttl;
+	if (opts->max_mss)
+		r->max_mss = opts->max_mss;
+
+	if (opts->tag)
+		if (strlcpy(r->tagname, opts->tag,
+		    PF_TAG_NAME_SIZE) >= PF_TAG_NAME_SIZE) {
+			yyerror("tag too long, max %u chars",
+			    PF_TAG_NAME_SIZE - 1);
+			return (1);
+		}
+	if (opts->match_tag)
+		if (strlcpy(r->match_tagname, opts->match_tag,
+		    PF_TAG_NAME_SIZE) >= PF_TAG_NAME_SIZE) {
+			yyerror("tag too long, max %u chars",
+			    PF_TAG_NAME_SIZE - 1);
+			return (1);
+		}
+	r->match_tag_not = opts->match_tag_not;
+
+	if (rule_label(r, opts->label))
+		return (1);
+	free(opts->label);
+
+	if (opts->marker & FOM_AFTO)
+		r->rule_flag |= PFRULE_AFTO;
+	if ((opts->marker & FOM_AFTO) && r->direction != PF_IN) {
+		yyerror("af-to can only be used with direction in");
+		return (1);
+	}
+	if ((opts->marker & FOM_AFTO) && opts->route.rt) {
+		yyerror("af-to cannot be used together with "
+		    "route-to, reply-to, dup-to");
+		return (1);
+	}
+	if (opts->marker & FOM_SCRUB_TCP)
+		r->scrub_flags |= PFSTATE_SCRUB_TCP;
+	if (opts->marker & FOM_PRIO) {
+		if (opts->prio == 0)
+			r->prio = PF_PRIO_ZERO;
+		else
+			r->prio = opts->prio;
+	}
+	if (opts->marker & FOM_SETPRIO) {
+		r->set_prio[0] = opts->set_prio[0];
+		r->set_prio[1] = opts->set_prio[1];
+		r->scrub_flags |= PFSTATE_SETPRIO;
+	}
+	if (opts->marker & FOM_SETTOS) {
+		r->scrub_flags |= PFSTATE_SETTOS;
+		r->set_tos = opts->settos;
+	}
+	if (opts->marker & FOM_PRIO) {
+		if (opts->prio == 0)
+			r->prio = PF_PRIO_ZERO;
+		else
+			r->prio = opts->prio;
+	}
+	if (opts->marker & FOM_SETPRIO) {
+		r->set_prio[0] = opts->set_prio[0];
+		r->set_prio[1] = opts->set_prio[1];
+		r->scrub_flags |= PFSTATE_SETPRIO;
+	}
+
+	r->flags = opts->flags.b1;
+	r->flagset = opts->flags.b2;
+	if ((opts->flags.b1 & opts->flags.b2) != opts->flags.b1) {
+		yyerror("flags always false");
+		return (1);
+	}
+
+	if (opts->queues.qname != NULL) {
+		if (strlcpy(r->qname, opts->queues.qname,
+		    sizeof(r->qname)) >= sizeof(r->qname)) {
+			yyerror("rule qname too long (max "
+			    "%d chars)", sizeof(r->qname)-1);
+			return (1);
+		}
+		free(opts->queues.qname);
+	}
+	if (opts->queues.pqname != NULL) {
+		if (strlcpy(r->pqname, opts->queues.pqname,
+		    sizeof(r->pqname)) >= sizeof(r->pqname)) {
+			yyerror("rule pqname too long (max "
+			    "%d chars)", sizeof(r->pqname)-1);
+			return (1);
+		}
+		free(opts->queues.qname);
+	}
+
+	if (opts->fragment)
+		r->rule_flag |= PFRULE_FRAGMENT;
+	r->allow_opts = opts->allowopts;
+
+	return (0);
+}