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

diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index dec9358..9d7e179 100644 (file)
--- 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);
+}