Re: af-to on pass out should be a parser error

[Henning Brauer]

* Mike Belopuhov  [2016-06-20 00:33]:
> rdr-to/nat-to are not checked on purpose.  i'm not certain about
> route-to/reply-to.

indeed, rdr-to/nat-to in the "unnatural" direction DO work, with
caveats. route-to and af-to are different.

as others already pointed out the check should be != PF_IN and not ==
PF_OUT, to catch unspecified direction.

With that, ok with me.

> as far as i'm concerned, af-to should be restricted to "pass in".
> but it would be nice to know if "pass out route-to" and "pass in
> reply-to" produce working configurations to restrict them as well
> if they don't.

ack - I dunno either otoh

-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS. Virtual & Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/

Re: af-to on pass out should be a parser error

[Mike Belopuhov]

On Mon, Jun 20, 2016 at 00:27 +0200, Sebastian Benoit wrote:
> Mike Belopuhov(m...@belopuhov.com) on 2016.06.20 00:11:03 +0200:
> > On Sun, Jun 19, 2016 at 23:43 +0200, Sebastian Benoit wrote:
> > > manpage documents that af-to does not work on pass out rules, but the
> > > pf.conf parser allows it, which leads a non working configuration being
> > > loaded.
> > > 
> > > this changes the parser to make pass out .. af-to an error.
> > > 
> > > ok?
> > > 
> > 
> > forgot to mention in my previous mail that af-to follows route-to
> > in this regard.  you can say "pass out route-to" but in fact it's
> > sort of pointless since the routing decision has already been made
> > by the forwarding code.  i'm not certain doing route-to at this
> > point produces a working result regarding created states, but that
> > would indeed contrast with af-to where this is not a supported
> > configuration.
> > 
> > to some extent "pass out af-to" also follows "pass out rdr-to" and
> > "pass in nat-to" in a sense that they're not common and might not
> > produce results one would expect, yet are parsed and installed into
> > the kernel successfully.
> 
> yes,
> 
> i thought these were checked, but there is only a check to make sure
> rdr/nat-to have a direction, not which one. i'll look at that tomorrow.
> 
> thanks.

rdr-to/nat-to are not checked on purpose.  i'm not certain about
route-to/reply-to.

as far as i'm concerned, af-to should be restricted to "pass in".
but it would be nice to know if "pass out route-to" and "pass in
reply-to" produce working configurations to restrict them as well
if they don't.

Re: af-to on pass out should be a parser error

[Sebastian Benoit]

Mike Belopuhov(m...@belopuhov.com) on 2016.06.20 00:11:03 +0200:
> On Sun, Jun 19, 2016 at 23:43 +0200, Sebastian Benoit wrote:
> > manpage documents that af-to does not work on pass out rules, but the
> > pf.conf parser allows it, which leads a non working configuration being
> > loaded.
> > 
> > this changes the parser to make pass out .. af-to an error.
> > 
> > ok?
> > 
> 
> forgot to mention in my previous mail that af-to follows route-to
> in this regard.  you can say "pass out route-to" but in fact it's
> sort of pointless since the routing decision has already been made
> by the forwarding code.  i'm not certain doing route-to at this
> point produces a working result regarding created states, but that
> would indeed contrast with af-to where this is not a supported
> configuration.
> 
> to some extent "pass out af-to" also follows "pass out rdr-to" and
> "pass in nat-to" in a sense that they're not common and might not
> produce results one would expect, yet are parsed and installed into
> the kernel successfully.

yes,

i thought these were checked, but there is only a check to make sure
rdr/nat-to have a direction, not which one. i'll look at that tomorrow.

thanks.

Re: af-to on pass out should be a parser error

[Sebastian Benoit]

Mike Belopuhov(m...@belopuhov.com) on 2016.06.20 00:01:28 +0200:
> On Sun, Jun 19, 2016 at 23:43 +0200, Sebastian Benoit wrote:
> > manpage documents that af-to does not work on pass out rules, but the
> > pf.conf parser allows it, which leads a non working configuration being
> > loaded.
> > 
> > this changes the parser to make pass out .. af-to an error.
> >
> 
> what happens if the direction is not specified?

this works better i hope.

diff --git sbin/pfctl/parse.y sbin/pfctl/parse.y
index 934438c..c491b8e 100644
--- sbin/pfctl/parse.y
+++ sbin/pfctl/parse.y
@@ -1518,6 +1518,9 @@ pfrule: action dir logquick interface af 
proto fromto
}
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;
r.af = $5;
 
if ($8.tag)

Re: af-to on pass out should be a parser error

[Mike Belopuhov]

On Sun, Jun 19, 2016 at 23:43 +0200, Sebastian Benoit wrote:
> manpage documents that af-to does not work on pass out rules, but the
> pf.conf parser allows it, which leads a non working configuration being
> loaded.
> 
> this changes the parser to make pass out .. af-to an error.
> 
> ok?
> 

forgot to mention in my previous mail that af-to follows route-to
in this regard.  you can say "pass out route-to" but in fact it's
sort of pointless since the routing decision has already been made
by the forwarding code.  i'm not certain doing route-to at this
point produces a working result regarding created states, but that
would indeed contrast with af-to where this is not a supported
configuration.

to some extent "pass out af-to" also follows "pass out rdr-to" and
"pass in nat-to" in a sense that they're not common and might not
produce results one would expect, yet are parsed and installed into
the kernel successfully.

Re: af-to on pass out should be a parser error

[Mike Belopuhov]

On Sun, Jun 19, 2016 at 23:43 +0200, Sebastian Benoit wrote:
> manpage documents that af-to does not work on pass out rules, but the
> pf.conf parser allows it, which leads a non working configuration being
> loaded.
> 
> this changes the parser to make pass out .. af-to an error.
>

what happens if the direction is not specified?

> ok?
>

i'm not a native speaker but there should be a verb somewhere :-)
how about "af-to can only be applied to inbound rules"?

> diff --git sbin/pfctl/parse.y sbin/pfctl/parse.y
> index 934438c..0fecba8 100644
> --- sbin/pfctl/parse.y
> +++ sbin/pfctl/parse.y
> @@ -1518,6 +1518,9 @@ pfrule  : action dir logquick interface af 
> proto fromto
>   }
>   if ($8.marker & FOM_AFTO)
>   r.rule_flag |= PFRULE_AFTO;
> + if ($8.marker & FOM_AFTO && r.direction == PF_OUT)
> + yyerror("af-to not possible with direction 
> out");
> + YYERROR;
>   r.af = $5;
>  
>   if ($8.tag)

af-to on pass out should be a parser error

[Sebastian Benoit]

manpage documents that af-to does not work on pass out rules, but the
pf.conf parser allows it, which leads a non working configuration being
loaded.

this changes the parser to make pass out .. af-to an error.

ok?

diff --git sbin/pfctl/parse.y sbin/pfctl/parse.y
index 934438c..0fecba8 100644
--- sbin/pfctl/parse.y
+++ sbin/pfctl/parse.y
@@ -1518,6 +1518,9 @@ pfrule: action dir logquick interface af 
proto fromto
}
if ($8.marker & FOM_AFTO)
r.rule_flag |= PFRULE_AFTO;
+   if ($8.marker & FOM_AFTO && r.direction == PF_OUT)
+   yyerror("af-to not possible with direction 
out");
+   YYERROR;
r.af = $5;
 
if ($8.tag)