pf(4): Three possible bugs with syncookies

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

pf(4): Three possible bugs with syncookies

Jesper Wallin
Hi,

I sent a mail to henning@ about two weeks ago and thought I'd post it
here as well, assuming he's occupied with other things.

Since the syncookies are disabled by default, I thought I'd give it
some testing and set it to "always" on all the machines I operate for
a few weeks.  So far I've found three bugs/issues, assuming it's not
my configurations/understanding of it that's causing the issues.

The first one is that my vmd(8) guests can't connect to the outside
world.  I can connect to the host from the guest, but not outside the
host.  Thus, breaking 'nat-to'.  ICMP seems to work fine, using ping(8)
but TCP fails.  I get the "Connected to example.com." but no packets
actually arrive at example.com and the connection is stuck there.

The second issue was discovered when looking over my configuration for
redirecting traffic to spamd(8).  I noticed my configuration was using
the 'rdr-to' directive (which works! fine), but the example in the
manual for spamd(8) was suggesting 'divert-to'.  When using 'divert-to',
the rule behaves as a regular pass, without the 'divert-to', making the
clients reaching smtpd(8) instead of spamd(8), even if it's the divert
rule that match (according to pflog(4)).  I don't fully understand how
syncookies work, but my guess (and that's all it is, a guess) is that
it the algorithm to create the state when the SYN/ACK is received, only
handle the src/dst and src-port/dst-port, but leaves out the fact that
the connection should be diverted, thus accepting the connection as-is.

The third issue is more about pf.conf(5).  By default, the syncookies
feature is disabled, meaning that if nothing is specified in my pf.conf,
syncookies are disabled.  However, if I add "set syncookies always" and
flush everything (doas pfctl -F all -f /etc/pf.conf), then change my
mind and remove it or comment it out, then flush everything again, it's
still enabled.  In order to disable syncookies, I need to actively turn
it off by adding "set syncookies never" and flushing everything again.

I apologize for not providing any patches, but this is beyond my skills
and I'm not even sure if it's a bug or something to expect due to the
way NAT and such behaves.

All testing was done on amd64 and various snapshots within the last few
weeks.  To rule out that anything else might break my pf.conf, I've
reduced my ruleset to the following:

--8<--
set skip on lo
set syncookies always

pass out quick on egress received-on tap nat-to (egress)
pass in quick on tap proto { tcp udp } from 100.64.0.0/10 \
    to any port domain rdr-to 127.0.0.1
pass quick on tap

block log
pass out keep state
--8<--


Jesper Wallin

Reply | Threaded
Open this post in threaded view
|

Re: pf(4): Three possible bugs with syncookies

Alexandr Nedvedicky
Hello Jesper,

thanks for the bugreport.

</snip>

> The third issue is more about pf.conf(5).  By default, the syncookies
> feature is disabled, meaning that if nothing is specified in my pf.conf,
> syncookies are disabled.  However, if I add "set syncookies always" and
> flush everything (doas pfctl -F all -f /etc/pf.conf), then change my
> mind and remove it or comment it out, then flush everything again, it's
> still enabled.  In order to disable syncookies, I need to actively turn
> it off by adding "set syncookies never" and flushing everything again.

    I don't have working PF by hand at the moment, so can't try the patch
    below. Does patch below solve the 'pfctl -Fa ...' glitch?


thanks and
regards
sashan

--------8<---------------8<---------------8<------------------8<--------
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index f567b13b6d6..70ea905c728 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -2434,6 +2434,7 @@ pfctl_reset(int dev, int opts)
  pf.debug_set = 1;
  pf.reass_set = 1;
  pf.syncookieswat_set = 1;
+ pf.syncookies_set = 1;
  pf.ifname = strdup("none");
  if (pf.ifname == NULL)
  err(1, "%s: strdup", __func__);

Reply | Threaded
Open this post in threaded view
|

Re: pf(4): Three possible bugs with syncookies

Jesper Wallin
Hi Alexandr,

Indeed it does!  The syncookies now gets disabled when removing or
commenting out the 'set syncookies always' line and flushing all rules
and states.


Jesper Wallin

On Thu, Aug 15, 2019 at 11:20:53AM +0200, Alexandr Nedvedicky wrote:

> Hello Jesper,
>
> thanks for the bugreport.
>
> </snip>
>
> > The third issue is more about pf.conf(5).  By default, the syncookies
> > feature is disabled, meaning that if nothing is specified in my pf.conf,
> > syncookies are disabled.  However, if I add "set syncookies always" and
> > flush everything (doas pfctl -F all -f /etc/pf.conf), then change my
> > mind and remove it or comment it out, then flush everything again, it's
> > still enabled.  In order to disable syncookies, I need to actively turn
> > it off by adding "set syncookies never" and flushing everything again.
>
>     I don't have working PF by hand at the moment, so can't try the patch
>     below. Does patch below solve the 'pfctl -Fa ...' glitch?
>
>
> thanks and
> regards
> sashan
>
> --------8<---------------8<---------------8<------------------8<--------
> diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
> index f567b13b6d6..70ea905c728 100644
> --- a/sbin/pfctl/pfctl.c
> +++ b/sbin/pfctl/pfctl.c
> @@ -2434,6 +2434,7 @@ pfctl_reset(int dev, int opts)
>   pf.debug_set = 1;
>   pf.reass_set = 1;
>   pf.syncookieswat_set = 1;
> + pf.syncookies_set = 1;
>   pf.ifname = strdup("none");
>   if (pf.ifname == NULL)
>   err(1, "%s: strdup", __func__);

Reply | Threaded
Open this post in threaded view
|

Re: pf(4): Three possible bugs with syncookies

Klemens Nanni-2
In reply to this post by Alexandr Nedvedicky
On Thu, Aug 15, 2019 at 11:20:53AM +0200, Alexandr Nedvedicky wrote:
>     I don't have working PF by hand at the moment, so can't try the patch
>     below. Does patch below solve the 'pfctl -Fa ...' glitch?
OK kn

Unless a non-default value is specified explicitly, the default value
should be effective after loading pf.conf.  Most options already behave
this way (quickly tested with `set skip on $if`), but I'd have to double
check all available options to ensure we're not missing on anyting else.