Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-09 Thread Alexandr Nedvedicky
Hello Klemens,

On Tue, Apr 09, 2019 at 04:02:06PM +0200, Klemens Nanni wrote:
> OK either way, but see below.
> 
> On Mon, Apr 08, 2019 at 09:56:46AM +0200, Alexandr Nedvedicky wrote:
> > +   pf.ifname = strdup("none");
> > +   if (pf.ifname == NULL)
> > +   err(1, "%s: strdup", __func__);
> > +   else
> > +   pf.ifname_set = 1;
> This branch is redundant and confusing, that pattern is also rarely
> seen in the tree.  The following is more obvious to the reader and
> resembles the code flow more clearly, I'd say:
> 
>   pf.ifname = strdup("none");
>   if (pf.ifname == NULL)
>   err(1, "%s: strdup", __func__);
>   pf.ifname_set = 1;

you are absolutely right.

Looks like I need glasses, but not the glasses with beer.

thanks and
regards
sashan

8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.8 b/sbin/pfctl/pfctl.8
index 48b2893cfcd..00bd27c200a 100644
--- a/sbin/pfctl/pfctl.8
+++ b/sbin/pfctl/pfctl.8
@@ -197,6 +197,8 @@ Flush the filter information (statistics that are not bound 
to rules).
 Flush the tables.
 .It Fl F Cm osfp
 Flush the passive operating system fingerprints.
+.It Fl F Cm Reset
+Reset limits, timeouts and options back to default settings.
 .It Fl F Cm all
 Flush all of the above.
 .El
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 493ff47af2f..17461a4bf77 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -105,6 +105,7 @@ int  pfctl_load_rule(struct pfctl *, char *, struct pf_rule 
*, int);
 const char *pfctl_lookup_option(char *, const char **);
 void   pfctl_state_store(int, const char *);
 void   pfctl_state_load(int, const char *);
+void   pfctl_reset(int, int);
 
 const char *clearopt;
 char   *rulesopt;
@@ -205,7 +206,8 @@ static const struct {
 };
 
 static const char *clearopt_list[] = {
-   "rules", "Sources", "states", "info", "Tables", "osfp", "all", NULL
+   "rules", "Sources", "states", "info", "Tables", "osfp", "Reset",
+   "all", NULL
 };
 
 static const char *showopt_list[] = {
@@ -2232,6 +2234,44 @@ pfctl_state_load(int dev, const char *file)
fclose(f);
 }
 
+void
+pfctl_reset(int dev, int opts)
+{
+   struct pfctlpf;
+   struct pfr_buffer t;
+   int i;
+
+   pf.dev = dev;
+   pfctl_init_options(&pf);
+
+   /* Force reset upon pfctl_load_options() */
+   pf.debug_set = 1;
+   pf.reass_set = 1;
+   pf.syncookieswat_set = 1;
+   pf.ifname = strdup("none");
+   if (pf.ifname == NULL)
+   err(1, "%s: strdup", __func__);
+   pf.ifname_set = 1;
+
+   memset(&t, 0, sizeof(t));
+   t.pfrb_type = PFRB_TRANS;
+   if (pfctl_trans(dev, &t, DIOCXBEGIN, 0))
+   err(1, "%s: DIOCXBEGIN", __func__);
+
+   for (i = 0; pf_limits[i].name; i++)
+   pf.limit_set[pf_limits[i].index] = 1;
+
+   for (i = 0; pf_timeouts[i].name; i++)
+   pf.timeout_set[pf_timeouts[i].timeout] = 1;
+
+   pfctl_load_options(&pf);
+
+   if (pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
+   err(1, "%s: DIOCXCOMMIT", __func__);
+
+   pfctl_clear_interface_flags(dev, opts);
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -2557,7 +2597,7 @@ main(int argc, char *argv[])
pfctl_clear_src_nodes(dev, opts);
pfctl_clear_stats(dev, ifaceopt, opts);
pfctl_clear_fingerprints(dev, opts);
-   pfctl_clear_interface_flags(dev, opts);
+   pfctl_reset(dev, opts);
}
break;
case 'o':
@@ -2566,6 +2606,9 @@ main(int argc, char *argv[])
case 'T':
pfctl_clear_tables(anchorname, opts);
break;
+   case 'R':
+   pfctl_reset(dev, opts);
+   break;
}
}
if (state_killers) {
diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5
index 247ceef40a5..dfa8d15d37a 100644
--- a/share/man/man5/pf.conf.5
+++ b/share/man/man5/pf.conf.5
@@ -1129,12 +1129,24 @@ can be used.
 .Xr pf 4
 may be tuned for various situations using the
 .Ic set
-command.
+command. Two sorts of options should be distinguished.
+.Em runtime
+options, which define parameters for 
+.Xr pf 4
+driver and
+.Em parser
+options, which fine-tune interpretation of rules, while
+they are being loaded from file. The runtime options
+may be restored to their default values using:
+.Pp
+.Dl # pfctl -FReset
+.Pp
+ 
 .Bl -tag -width Ds
 .It Ic set Cm block-policy drop | return
 The
 .Cm block-policy
-option sets the default behaviour for the packet
+parser option sets the default behaviour for the packet
 .Ic block
 action:
 .Pp
@@ -1146,8 +1158,13 @@ A TCP RST is returned for blocked TCP packets,
 an ICMP UNREACHABLE is returned fo

Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-09 Thread Klemens Nanni
OK either way, but see below.

On Mon, Apr 08, 2019 at 09:56:46AM +0200, Alexandr Nedvedicky wrote:
> + pf.ifname = strdup("none");
> + if (pf.ifname == NULL)
> + err(1, "%s: strdup", __func__);
> + else
> + pf.ifname_set = 1;
This branch is redundant and confusing, that pattern is also rarely
seen in the tree.  The following is more obvious to the reader and
resembles the code flow more clearly, I'd say:

pf.ifname = strdup("none");
if (pf.ifname == NULL)
err(1, "%s: strdup", __func__);
pf.ifname_set = 1;



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-08 Thread Alexandr Nedvedicky
Hello,



> We should fail hard as in almost all other strdup(3) use cases.
> Failure means the system ran out of memory, there's no point in going
> any further.
> 
> So just something like
> 
>   pf.ifname = strdup("none");
>   if (pf.ifname == NULL)
>   err(1, "%s: strdup", __func__);
> 

yes, you are right. so this is the change:

8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 470c54644ba..28f8723cac3 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -2250,14 +2250,14 @@ pfctl_reset(int dev, int opts)
pf.syncookieswat_set = 1;
pf.ifname = strdup("none");
if (pf.ifname == NULL)
-   warn("%s: Warning: can't reset loginterface\n", __func__);
+   err(1, "%s: strdup", __func__);
else
pf.ifname_set = 1;
 
memset(&t, 0, sizeof(t));
t.pfrb_type = PFRB_TRANS;
if (pfctl_trans(dev, &t, DIOCXBEGIN, 0))
-   warn("%s, DIOCXBEGIN", __func__);
+   err(1, "%s: DIOCXBEGIN", __func__);
 
for (i = 0; pf_limits[i].name; i++)
pf.limit_set[pf_limits[i].index] = 1;
@@ -2268,7 +2268,7 @@ pfctl_reset(int dev, int opts)
pfctl_load_options(&pf);
 
if (pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
-   warn("%s, DIOCXCOMMIT", __func__);
+   err(1, "%s: DIOCXCOMMIT", __func__);
 
pfctl_clear_interface_flags(dev, opts);
 }
8<---8<---8<--8<

updated patch follows.

thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.8 b/sbin/pfctl/pfctl.8
index 48b2893cfcd..00bd27c200a 100644
--- a/sbin/pfctl/pfctl.8
+++ b/sbin/pfctl/pfctl.8
@@ -197,6 +197,8 @@ Flush the filter information (statistics that are not bound 
to rules).
 Flush the tables.
 .It Fl F Cm osfp
 Flush the passive operating system fingerprints.
+.It Fl F Cm Reset
+Reset limits, timeouts and options back to default settings.
 .It Fl F Cm all
 Flush all of the above.
 .El
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 493ff47af2f..28f8723cac3 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -105,6 +105,7 @@ int  pfctl_load_rule(struct pfctl *, char *, struct pf_rule 
*, int);
 const char *pfctl_lookup_option(char *, const char **);
 void   pfctl_state_store(int, const char *);
 void   pfctl_state_load(int, const char *);
+void   pfctl_reset(int, int);
 
 const char *clearopt;
 char   *rulesopt;
@@ -205,7 +206,8 @@ static const struct {
 };
 
 static const char *clearopt_list[] = {
-   "rules", "Sources", "states", "info", "Tables", "osfp", "all", NULL
+   "rules", "Sources", "states", "info", "Tables", "osfp", "Reset",
+   "all", NULL
 };
 
 static const char *showopt_list[] = {
@@ -2232,6 +2234,45 @@ pfctl_state_load(int dev, const char *file)
fclose(f);
 }
 
+void
+pfctl_reset(int dev, int opts)
+{
+   struct pfctlpf;
+   struct pfr_buffer t;
+   int i;
+
+   pf.dev = dev;
+   pfctl_init_options(&pf);
+
+   /* Force reset upon pfctl_load_options() */
+   pf.debug_set = 1;
+   pf.reass_set = 1;
+   pf.syncookieswat_set = 1;
+   pf.ifname = strdup("none");
+   if (pf.ifname == NULL)
+   err(1, "%s: strdup", __func__);
+   else
+   pf.ifname_set = 1;
+
+   memset(&t, 0, sizeof(t));
+   t.pfrb_type = PFRB_TRANS;
+   if (pfctl_trans(dev, &t, DIOCXBEGIN, 0))
+   err(1, "%s: DIOCXBEGIN", __func__);
+
+   for (i = 0; pf_limits[i].name; i++)
+   pf.limit_set[pf_limits[i].index] = 1;
+
+   for (i = 0; pf_timeouts[i].name; i++)
+   pf.timeout_set[pf_timeouts[i].timeout] = 1;
+
+   pfctl_load_options(&pf);
+
+   if (pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
+   err(1, "%s: DIOCXCOMMIT", __func__);
+
+   pfctl_clear_interface_flags(dev, opts);
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -2557,7 +2598,7 @@ main(int argc, char *argv[])
pfctl_clear_src_nodes(dev, opts);
pfctl_clear_stats(dev, ifaceopt, opts);
pfctl_clear_fingerprints(dev, opts);
-   pfctl_clear_interface_flags(dev, opts);
+   pfctl_reset(dev, opts);
}
break;
case 'o':
@@ -2566,6 +2607,9 @@ main(int argc, char *argv[])
case 'T':
pfctl_clear_tables(anchorname, opts);
break;
+   case 'R':
+   pfctl_reset(dev, opts);
+   break;
}
}
if (state_killers) {



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-06 Thread Klemens Nanni
On Sat, Apr 06, 2019 at 02:37:05AM +0200, Alexandr Nedvedicky wrote:
> updated diff is attached. I'll commit the change after unlock.
OK kn with comments inline.

> + pf.ifname = strdup("none");
> + if (pf.ifname == NULL)
> + warn("%s: Warning: can't reset loginterface\n", __func__);
> + else
> + pf.ifname_set = 1;

We should fail hard as in almost all other strdup(3) use cases.
Failure means the system ran out of memory, there's no point in going
any further.

So just something like

pf.ifname = strdup("none");
if (pf.ifname == NULL)
err(1, "%s: strdup", __func__);

> + if (pfctl_trans(dev, &t, DIOCXBEGIN, 0))
> + warn("%s, DIOCXBEGIN", __func__);
Turn this comma into a double colon.

> + if (pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
> + warn("%s, DIOCXCOMMIT", __func__);
Same here.



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-06 Thread Gleydson Soares
 +void
> +pfctl_reset(int dev, int opts)
> +{
> + struct pfctlpf;
> + struct pfr_buffer t;
> + int i;
> +
> + pf.dev = dev;
> + pfctl_init_options(&pf);
> +
> + /* Force reset upon pfctl_load_options() */
> + pf.debug_set = 1;
> + pf.reass_set = 1;
> + pf.syncookieswat_set = 1;
> + pf.ifname = strdup("none");
> + if (pf.ifname == NULL)
> + warn("%s: Warning: can't reset loginterface\n", __func__);

do you really need this
extra newline here?
warn() itself already includes
one.
> + else
> + pf.ifname_set = 1;
> +
> + memset(&t, 0, sizeof(t));
> + t.pfrb_type = PFRB_TRANS;
> + if (pfctl_trans(dev, &t, DIOCXBEGIN, 0))
> + warn("%s, DIOCXBEGIN", __func__);
> +
> + for (i = 0; pf_limits[i].name; i++)
> + pf.limit_set[pf_limits[i].index] = 1;
> +
> + for (i = 0; pf_timeouts[i].name; i++)
> + pf.timeout_set[pf_timeouts[i].timeout] = 1;
> +
> + pfctl_load_options(&pf);
> +
> + if (pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
> + warn("%s, DIOCXCOMMIT", __func__);
> +
> + pfctl_clear_interface_flags(dev, opts);
> +}



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-05 Thread Alexandr Nedvedicky
Hello,


> > +void
> > +pfctl_reset(int dev, int opts)
> > +{
> > +   struct pfctlpf;
> > +   struct pfr_buffer t;
> > +   int i;
> > +
> > +   pf.dev = dev;
> > +   pfctl_init_options(&pf);
> > +
> > +   /* Force reset upon pfctl_load_options() */
> > +   pf.debug_set = 1;
> > +   pf.reass_set = 1;
> > +   pf.syncookieswat_set = 1;
> > +   pf.ifname = strdup("none");
> 
> I think strdup should be checked for NULL.

good point. does something like this look good?

8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 032fdd08b57..88d31d6190e 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -2249,7 +2249,10 @@ pfctl_reset(int dev, int opts)
pf.reass_set = 1;
pf.syncookieswat_set = 1;
pf.ifname = strdup("none");
-   pf.ifname_set = 1;
+   if (pf.ifname == NULL)
+   warn("%s: Warning: can't reset loginterface\n", __func__);
+   else
+   pf.ifname_set = 1;
 
memset(&t, 0, sizeof(t));
t.pfrb_type = PFRB_TRANS;
8<---8<---8<--8<

> 
> > +   pf.ifname_set = 1;
> > +
> > +   memset(&t, 0, sizeof(t));
> > +   t.pfrb_type = PFRB_TRANS;
> > +   if (pfctl_trans(dev, &t, DIOCXBEGIN, 0))
> > +   warn("%s, DIOCXBEGIN", __func__);
> > +
> > +
> 
> There is an extra white-space line here.

fixed.

updated diff is attached. I'll commit the change after unlock.

thanks and
regards
sasha

8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.8 b/sbin/pfctl/pfctl.8
index 48b2893cfcd..00bd27c200a 100644
--- a/sbin/pfctl/pfctl.8
+++ b/sbin/pfctl/pfctl.8
@@ -197,6 +197,8 @@ Flush the filter information (statistics that are not bound 
to rules).
 Flush the tables.
 .It Fl F Cm osfp
 Flush the passive operating system fingerprints.
+.It Fl F Cm Reset
+Reset limits, timeouts and options back to default settings.
 .It Fl F Cm all
 Flush all of the above.
 .El
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 493ff47af2f..470c54644ba 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -105,6 +105,7 @@ int  pfctl_load_rule(struct pfctl *, char *, struct pf_rule 
*, int);
 const char *pfctl_lookup_option(char *, const char **);
 void   pfctl_state_store(int, const char *);
 void   pfctl_state_load(int, const char *);
+void   pfctl_reset(int, int);
 
 const char *clearopt;
 char   *rulesopt;
@@ -205,7 +206,8 @@ static const struct {
 };
 
 static const char *clearopt_list[] = {
-   "rules", "Sources", "states", "info", "Tables", "osfp", "all", NULL
+   "rules", "Sources", "states", "info", "Tables", "osfp", "Reset",
+   "all", NULL
 };
 
 static const char *showopt_list[] = {
@@ -2232,6 +2234,45 @@ pfctl_state_load(int dev, const char *file)
fclose(f);
 }
 
+void
+pfctl_reset(int dev, int opts)
+{
+   struct pfctlpf;
+   struct pfr_buffer t;
+   int i;
+
+   pf.dev = dev;
+   pfctl_init_options(&pf);
+
+   /* Force reset upon pfctl_load_options() */
+   pf.debug_set = 1;
+   pf.reass_set = 1;
+   pf.syncookieswat_set = 1;
+   pf.ifname = strdup("none");
+   if (pf.ifname == NULL)
+   warn("%s: Warning: can't reset loginterface\n", __func__);
+   else
+   pf.ifname_set = 1;
+
+   memset(&t, 0, sizeof(t));
+   t.pfrb_type = PFRB_TRANS;
+   if (pfctl_trans(dev, &t, DIOCXBEGIN, 0))
+   warn("%s, DIOCXBEGIN", __func__);
+
+   for (i = 0; pf_limits[i].name; i++)
+   pf.limit_set[pf_limits[i].index] = 1;
+
+   for (i = 0; pf_timeouts[i].name; i++)
+   pf.timeout_set[pf_timeouts[i].timeout] = 1;
+
+   pfctl_load_options(&pf);
+
+   if (pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
+   warn("%s, DIOCXCOMMIT", __func__);
+
+   pfctl_clear_interface_flags(dev, opts);
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -2557,7 +2598,7 @@ main(int argc, char *argv[])
pfctl_clear_src_nodes(dev, opts);
pfctl_clear_stats(dev, ifaceopt, opts);
pfctl_clear_fingerprints(dev, opts);
-   pfctl_clear_interface_flags(dev, opts);
+   pfctl_reset(dev, opts);
}
break;
case 'o':
@@ -2566,6 +2607,9 @@ main(int argc, char *argv[])
case 'T':
pfctl_clear_tables(anchorname, opts);
break;
+   case 'R':
+   pfctl_reset(dev, opts);
+   break;
}
}
if (state_killers) {



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-05 Thread Hiltjo Posthuma
On Wed, Apr 03, 2019 at 11:10:21AM +0200, Alexandr Nedvedicky wrote:
> Hello,
> 
> On Tue, Apr 02, 2019 at 11:28:43AM +0200, Petr Hoffmann wrote:
> > Hi,
> > 
> > seeing this in the manpage
> > --8<--
> > +.It Fl F Cm Reset
> > +Reset limits, timeouts and options back to default settings.
> > -->8--
> > would make me believe everything mentioned as OPTIONS in pf.conf(5)
> > is about to be reset. I see e.g. the debug level is reset, but what
> > about the other stuff like fingerprints, 'skip on' and other options
> > set via the 'set' command? Maybe the manpage should be more precise
> > here?
> > 
> 
> I did look at pf.conf(5) manpage yesterday. It requires more updates, 
> which
> I would like to leave for another diff. For example pf.conf(5) does not
> mention default values for limits and time outs. I expect some discussion
> on how far we want to get with details. So let's exclude pf.conf(5) this
> time.
> 
> below is the diff I'd like to commit.
> 
> thanks and
> regards
> sashan
> 
> 8<---8<---8<--8<
> diff --git a/sbin/pfctl/pfctl.8 b/sbin/pfctl/pfctl.8
> index 48b2893cfcd..00bd27c200a 100644
> --- a/sbin/pfctl/pfctl.8
> +++ b/sbin/pfctl/pfctl.8
> @@ -197,6 +197,8 @@ Flush the filter information (statistics that are not 
> bound to rules).
>  Flush the tables.
>  .It Fl F Cm osfp
>  Flush the passive operating system fingerprints.
> +.It Fl F Cm Reset
> +Reset limits, timeouts and options back to default settings.
>  .It Fl F Cm all
>  Flush all of the above.
>  .El
> diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
> index 493ff47af2f..032fdd08b57 100644
> --- a/sbin/pfctl/pfctl.c
> +++ b/sbin/pfctl/pfctl.c
> @@ -105,6 +105,7 @@ intpfctl_load_rule(struct pfctl *, char *, struct 
> pf_rule *, int);
>  const char   *pfctl_lookup_option(char *, const char **);
>  void pfctl_state_store(int, const char *);
>  void pfctl_state_load(int, const char *);
> +void pfctl_reset(int, int);
>  
>  const char   *clearopt;
>  char *rulesopt;
> @@ -205,7 +206,8 @@ static const struct {
>  };
>  
>  static const char *clearopt_list[] = {
> - "rules", "Sources", "states", "info", "Tables", "osfp", "all", NULL
> + "rules", "Sources", "states", "info", "Tables", "osfp", "Reset",
> + "all", NULL
>  };
>  
>  static const char *showopt_list[] = {
> @@ -2232,6 +2234,43 @@ pfctl_state_load(int dev, const char *file)
>   fclose(f);
>  }
>  
> +void
> +pfctl_reset(int dev, int opts)
> +{
> + struct pfctlpf;
> + struct pfr_buffer t;
> + int i;
> +
> + pf.dev = dev;
> + pfctl_init_options(&pf);
> +
> + /* Force reset upon pfctl_load_options() */
> + pf.debug_set = 1;
> + pf.reass_set = 1;
> + pf.syncookieswat_set = 1;
> + pf.ifname = strdup("none");

I think strdup should be checked for NULL.

> + pf.ifname_set = 1;
> +
> + memset(&t, 0, sizeof(t));
> + t.pfrb_type = PFRB_TRANS;
> + if (pfctl_trans(dev, &t, DIOCXBEGIN, 0))
> + warn("%s, DIOCXBEGIN", __func__);
> +
> +

There is an extra white-space line here.

> + for (i = 0; pf_limits[i].name; i++)
> + pf.limit_set[pf_limits[i].index] = 1;
> +
> + for (i = 0; pf_timeouts[i].name; i++)
> + pf.timeout_set[pf_timeouts[i].timeout] = 1;
> +
> + pfctl_load_options(&pf);
> +
> + if (pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
> + warn("%s, DIOCXCOMMIT", __func__);
> +
> + pfctl_clear_interface_flags(dev, opts);
> +}
> +
>  int
>  main(int argc, char *argv[])
>  {
> @@ -2557,7 +2596,7 @@ main(int argc, char *argv[])
>   pfctl_clear_src_nodes(dev, opts);
>   pfctl_clear_stats(dev, ifaceopt, opts);
>   pfctl_clear_fingerprints(dev, opts);
> - pfctl_clear_interface_flags(dev, opts);
> + pfctl_reset(dev, opts);
>   }
>   break;
>   case 'o':
> @@ -2566,6 +2605,9 @@ main(int argc, char *argv[])
>   case 'T':
>   pfctl_clear_tables(anchorname, opts);
>   break;
> + case 'R':
> + pfctl_reset(dev, opts);
> + break;
>   }
>   }
>   if (state_killers) {
> 

-- 
Kind regards,
Hiltjo



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-03 Thread Ted Unangst
Alexandr Nedvedicky wrote:
> below is the diff I'd like to commit.

this is fine with me.



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-03 Thread Klemens Nanni
On Wed, Apr 03, 2019 at 11:10:21AM +0200, Alexandr Nedvedicky wrote:
> I did look at pf.conf(5) manpage yesterday. It requires more updates, 
> which
> I would like to leave for another diff. For example pf.conf(5) does not
> mention default values for limits and time outs. I expect some discussion
> on how far we want to get with details. So let's exclude pf.conf(5) this
> time.
Yes, this bothered me as well;  I also agree on tackling this with
separate diffs.

> below is the diff I'd like to commit.
OK kn



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-03 Thread Alexandr Nedvedicky
Hello,

On Tue, Apr 02, 2019 at 11:28:43AM +0200, Petr Hoffmann wrote:
> Hi,
> 
> seeing this in the manpage
> --8<--
> +.It Fl F Cm Reset
> +Reset limits, timeouts and options back to default settings.
> -->8--
> would make me believe everything mentioned as OPTIONS in pf.conf(5)
> is about to be reset. I see e.g. the debug level is reset, but what
> about the other stuff like fingerprints, 'skip on' and other options
> set via the 'set' command? Maybe the manpage should be more precise
> here?
> 

I did look at pf.conf(5) manpage yesterday. It requires more updates, which
I would like to leave for another diff. For example pf.conf(5) does not
mention default values for limits and time outs. I expect some discussion
on how far we want to get with details. So let's exclude pf.conf(5) this
time.

below is the diff I'd like to commit.

thanks and
regards
sashan

8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.8 b/sbin/pfctl/pfctl.8
index 48b2893cfcd..00bd27c200a 100644
--- a/sbin/pfctl/pfctl.8
+++ b/sbin/pfctl/pfctl.8
@@ -197,6 +197,8 @@ Flush the filter information (statistics that are not bound 
to rules).
 Flush the tables.
 .It Fl F Cm osfp
 Flush the passive operating system fingerprints.
+.It Fl F Cm Reset
+Reset limits, timeouts and options back to default settings.
 .It Fl F Cm all
 Flush all of the above.
 .El
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 493ff47af2f..032fdd08b57 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -105,6 +105,7 @@ int  pfctl_load_rule(struct pfctl *, char *, struct pf_rule 
*, int);
 const char *pfctl_lookup_option(char *, const char **);
 void   pfctl_state_store(int, const char *);
 void   pfctl_state_load(int, const char *);
+void   pfctl_reset(int, int);
 
 const char *clearopt;
 char   *rulesopt;
@@ -205,7 +206,8 @@ static const struct {
 };
 
 static const char *clearopt_list[] = {
-   "rules", "Sources", "states", "info", "Tables", "osfp", "all", NULL
+   "rules", "Sources", "states", "info", "Tables", "osfp", "Reset",
+   "all", NULL
 };
 
 static const char *showopt_list[] = {
@@ -2232,6 +2234,43 @@ pfctl_state_load(int dev, const char *file)
fclose(f);
 }
 
+void
+pfctl_reset(int dev, int opts)
+{
+   struct pfctlpf;
+   struct pfr_buffer t;
+   int i;
+
+   pf.dev = dev;
+   pfctl_init_options(&pf);
+
+   /* Force reset upon pfctl_load_options() */
+   pf.debug_set = 1;
+   pf.reass_set = 1;
+   pf.syncookieswat_set = 1;
+   pf.ifname = strdup("none");
+   pf.ifname_set = 1;
+
+   memset(&t, 0, sizeof(t));
+   t.pfrb_type = PFRB_TRANS;
+   if (pfctl_trans(dev, &t, DIOCXBEGIN, 0))
+   warn("%s, DIOCXBEGIN", __func__);
+
+
+   for (i = 0; pf_limits[i].name; i++)
+   pf.limit_set[pf_limits[i].index] = 1;
+
+   for (i = 0; pf_timeouts[i].name; i++)
+   pf.timeout_set[pf_timeouts[i].timeout] = 1;
+
+   pfctl_load_options(&pf);
+
+   if (pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
+   warn("%s, DIOCXCOMMIT", __func__);
+
+   pfctl_clear_interface_flags(dev, opts);
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -2557,7 +2596,7 @@ main(int argc, char *argv[])
pfctl_clear_src_nodes(dev, opts);
pfctl_clear_stats(dev, ifaceopt, opts);
pfctl_clear_fingerprints(dev, opts);
-   pfctl_clear_interface_flags(dev, opts);
+   pfctl_reset(dev, opts);
}
break;
case 'o':
@@ -2566,6 +2605,9 @@ main(int argc, char *argv[])
case 'T':
pfctl_clear_tables(anchorname, opts);
break;
+   case 'R':
+   pfctl_reset(dev, opts);
+   break;
}
}
if (state_killers) {



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-02 Thread Klemens Nanni
On Tue, Apr 02, 2019 at 02:01:05PM +0200, Alexandr Nedvedicky wrote:
> I think Petr is right here. my patch requires yet another finishing touch:
Fair enough, but it should be noted that this somewhat changes behaviour
of the existing interface:

> 8<---8<---8<--8<
> diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
> index 40929d90530..032fdd08b57 100644
> --- a/sbin/pfctl/pfctl.c
> +++ b/sbin/pfctl/pfctl.c
> @@ -2267,6 +2267,8 @@ pfctl_reset(int dev, int opts)
>  
> if (pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
> warn("%s, DIOCXCOMMIT", __func__);
> +
> +   pfctl_clear_interface_flags(dev, opts);
Now this is done with `-F reset' and therefore `-F all'...

>  }
>  
>  int
> @@ -2594,7 +2596,6 @@ main(int argc, char *argv[])
> pfctl_clear_src_nodes(dev, opts);
> pfctl_clear_stats(dev, ifaceopt, opts);
> pfctl_clear_fingerprints(dev, opts);
> -   pfctl_clear_interface_flags(dev, opts);
Where previously, without being documented, only `-F all' would do so.

> pfctl_reset(dev, opts);
> }

I think that is fine in this particular case, but clearing things in
specific flush commands that were previously only touched by the `all'
hammer can be dangerous.



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-02 Thread Alexandr Nedvedicky
Hello,


On Tue, Apr 02, 2019 at 12:59:33PM +0200, Petr Hoffmann wrote:
> On 02.04.2019 12:06, Klemens Nanni wrote:
> >On Tue, Apr 02, 2019 at 11:28:43AM +0200, Petr Hoffmann wrote:
> >>would make me believe everything mentioned as OPTIONS in pf.conf(5) is about
> >>to be reset. I see e.g. the debug level is reset, but what about the other
> >>stuff like fingerprints, 'skip on' and other options set via the 'set'
> >>command? Maybe the manpage should be more precise here?
> >Seems fine to me, given that a) some options are not persisted in the
> >driver but only effective during ruleset parsing and b) stuff like
> >fingerprints is already flushed separately, see pfctl(8) `-F osfp'.
> For me, forcing the user to think what is meant by 'options' is not
> very friendly, though I understand the idea behind *some* options
> being used only while parsing. Let's assume I'm the smart user who
> is able to distinguish them. But then, 'set skip on' is the
> persistent one, right, but still not reset, I guess.
> 

I think Petr is right here. my patch requires yet another finishing touch:

8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 40929d90530..032fdd08b57 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -2267,6 +2267,8 @@ pfctl_reset(int dev, int opts)
 
if (pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
warn("%s, DIOCXCOMMIT", __func__);
+
+   pfctl_clear_interface_flags(dev, opts);
 }
 
 int
@@ -2594,7 +2596,6 @@ main(int argc, char *argv[])
pfctl_clear_src_nodes(dev, opts);
pfctl_clear_stats(dev, ifaceopt, opts);
pfctl_clear_fingerprints(dev, opts);
-   pfctl_clear_interface_flags(dev, opts);
pfctl_reset(dev, opts);
}
break;
8<---8<---8<--8<

I'll walk through my change one more time to check if there are similar
oversights.

thanks and
regards
sasha



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-02 Thread Petr Hoffmann

On 02.04.2019 12:06, Klemens Nanni wrote:

On Tue, Apr 02, 2019 at 11:28:43AM +0200, Petr Hoffmann wrote:

would make me believe everything mentioned as OPTIONS in pf.conf(5) is about
to be reset. I see e.g. the debug level is reset, but what about the other
stuff like fingerprints, 'skip on' and other options set via the 'set'
command? Maybe the manpage should be more precise here?

Seems fine to me, given that a) some options are not persisted in the
driver but only effective during ruleset parsing and b) stuff like
fingerprints is already flushed separately, see pfctl(8) `-F osfp'.
For me, forcing the user to think what is meant by 'options' is not very 
friendly, though I understand the idea behind *some* options being used 
only while parsing. Let's assume I'm the smart user who is able to 
distinguish them. But then, 'set skip on' is the persistent one, right, 
but still not reset, I guess.


Petr


On 02.04.2019 12:06, Klemens Nanni wrote:

On Tue, Apr 02, 2019 at 11:28:43AM +0200, Petr Hoffmann wrote:

would make me believe everything mentioned as OPTIONS in pf.conf(5) is about
to be reset. I see e.g. the debug level is reset, but what about the other
stuff like fingerprints, 'skip on' and other options set via the 'set'
command? Maybe the manpage should be more precise here?

Seems fine to me, given that a) some options are not persisted in the
driver but only effective during ruleset parsing and b) stuff like
fingerprints is already flushed separately, see pfctl(8) `-F osfp'.





Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-02 Thread Klemens Nanni
On Tue, Apr 02, 2019 at 11:28:43AM +0200, Petr Hoffmann wrote:
> would make me believe everything mentioned as OPTIONS in pf.conf(5) is about
> to be reset. I see e.g. the debug level is reset, but what about the other
> stuff like fingerprints, 'skip on' and other options set via the 'set'
> command? Maybe the manpage should be more precise here?
Seems fine to me, given that a) some options are not persisted in the
driver but only effective during ruleset parsing and b) stuff like
fingerprints is already flushed separately, see pfctl(8) `-F osfp'.



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-02 Thread Petr Hoffmann

Hi,

seeing this in the manpage
--8<--
+.It Fl F Cm Reset
+Reset limits, timeouts and options back to default settings.
-->8--
would make me believe everything mentioned as OPTIONS in pf.conf(5) is 
about to be reset. I see e.g. the debug level is reset, but what about 
the other stuff like fingerprints, 'skip on' and other options set via 
the 'set' command? Maybe the manpage should be more precise here?


PH

On 02.04.2019 9:40, Alexandr Nedvedicky wrote:

Hello,

below is diff I plan to commit. I did add a comment to pfctl_reset()
and wording in manpage.

thanks and
regards
sashan

8<---8<---8<---8<---
diff --git a/sbin/pfctl/pfctl.8 b/sbin/pfctl/pfctl.8
index 48b2893cfcd..00bd27c200a 100644
--- a/sbin/pfctl/pfctl.8
+++ b/sbin/pfctl/pfctl.8
@@ -197,6 +197,8 @@ Flush the filter information (statistics that are not bound 
to rules).
  Flush the tables.
  .It Fl F Cm osfp
  Flush the passive operating system fingerprints.
+.It Fl F Cm Reset
+Reset limits, timeouts and options back to default settings.
  .It Fl F Cm all
  Flush all of the above.
  .El
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 493ff47af2f..40929d90530 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -105,6 +105,7 @@ int  pfctl_load_rule(struct pfctl *, char *, struct pf_rule 
*, int);
  const char*pfctl_lookup_option(char *, const char **);
  void  pfctl_state_store(int, const char *);
  void  pfctl_state_load(int, const char *);
+void   pfctl_reset(int, int);
  
  const char	*clearopt;

  char  *rulesopt;
@@ -205,7 +206,8 @@ static const struct {
  };
  
  static const char *clearopt_list[] = {

-   "rules", "Sources", "states", "info", "Tables", "osfp", "all", NULL
+   "rules", "Sources", "states", "info", "Tables", "osfp", "Reset",
+   "all", NULL
  };
  
  static const char *showopt_list[] = {

@@ -2232,6 +2234,41 @@ pfctl_state_load(int dev, const char *file)
fclose(f);
  }
  
+void

+pfctl_reset(int dev, int opts)
+{
+   struct pfctlpf;
+   struct pfr_buffer t;
+   int i;
+
+   pf.dev = dev;
+   pfctl_init_options(&pf);
+
+   /* Force reset upon pfctl_load_options() */
+   pf.debug_set = 1;
+   pf.reass_set = 1;
+   pf.syncookieswat_set = 1;
+   pf.ifname = strdup("none");
+   pf.ifname_set = 1;
+
+   memset(&t, 0, sizeof(t));
+   t.pfrb_type = PFRB_TRANS;
+   if (pfctl_trans(dev, &t, DIOCXBEGIN, 0))
+   warn("%s, DIOCXBEGIN", __func__);
+
+
+   for (i = 0; pf_limits[i].name; i++)
+   pf.limit_set[pf_limits[i].index] = 1;
+
+   for (i = 0; pf_timeouts[i].name; i++)
+   pf.timeout_set[pf_timeouts[i].timeout] = 1;
+
+   pfctl_load_options(&pf);
+
+   if (pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
+   warn("%s, DIOCXCOMMIT", __func__);
+}
+
  int
  main(int argc, char *argv[])
  {
@@ -2558,6 +2595,7 @@ main(int argc, char *argv[])
pfctl_clear_stats(dev, ifaceopt, opts);
pfctl_clear_fingerprints(dev, opts);
pfctl_clear_interface_flags(dev, opts);
+   pfctl_reset(dev, opts);
}
break;
case 'o':
@@ -2566,6 +2604,9 @@ main(int argc, char *argv[])
case 'T':
pfctl_clear_tables(anchorname, opts);
break;
+   case 'R':
+   pfctl_reset(dev, opts);
+   break;
}
}
if (state_killers) {





Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-02 Thread Alexandr Nedvedicky
Hello,

below is diff I plan to commit. I did add a comment to pfctl_reset()
and wording in manpage.

thanks and
regards
sashan

8<---8<---8<---8<---
diff --git a/sbin/pfctl/pfctl.8 b/sbin/pfctl/pfctl.8
index 48b2893cfcd..00bd27c200a 100644
--- a/sbin/pfctl/pfctl.8
+++ b/sbin/pfctl/pfctl.8
@@ -197,6 +197,8 @@ Flush the filter information (statistics that are not bound 
to rules).
 Flush the tables.
 .It Fl F Cm osfp
 Flush the passive operating system fingerprints.
+.It Fl F Cm Reset
+Reset limits, timeouts and options back to default settings.
 .It Fl F Cm all
 Flush all of the above.
 .El
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 493ff47af2f..40929d90530 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -105,6 +105,7 @@ int  pfctl_load_rule(struct pfctl *, char *, struct pf_rule 
*, int);
 const char *pfctl_lookup_option(char *, const char **);
 void   pfctl_state_store(int, const char *);
 void   pfctl_state_load(int, const char *);
+void   pfctl_reset(int, int);
 
 const char *clearopt;
 char   *rulesopt;
@@ -205,7 +206,8 @@ static const struct {
 };
 
 static const char *clearopt_list[] = {
-   "rules", "Sources", "states", "info", "Tables", "osfp", "all", NULL
+   "rules", "Sources", "states", "info", "Tables", "osfp", "Reset",
+   "all", NULL
 };
 
 static const char *showopt_list[] = {
@@ -2232,6 +2234,41 @@ pfctl_state_load(int dev, const char *file)
fclose(f);
 }
 
+void
+pfctl_reset(int dev, int opts)
+{
+   struct pfctlpf;
+   struct pfr_buffer t;
+   int i;
+
+   pf.dev = dev;
+   pfctl_init_options(&pf);
+
+   /* Force reset upon pfctl_load_options() */
+   pf.debug_set = 1;
+   pf.reass_set = 1;
+   pf.syncookieswat_set = 1;
+   pf.ifname = strdup("none");
+   pf.ifname_set = 1;
+
+   memset(&t, 0, sizeof(t));
+   t.pfrb_type = PFRB_TRANS;
+   if (pfctl_trans(dev, &t, DIOCXBEGIN, 0))
+   warn("%s, DIOCXBEGIN", __func__);
+
+
+   for (i = 0; pf_limits[i].name; i++)
+   pf.limit_set[pf_limits[i].index] = 1;
+
+   for (i = 0; pf_timeouts[i].name; i++)
+   pf.timeout_set[pf_timeouts[i].timeout] = 1;
+
+   pfctl_load_options(&pf);
+
+   if (pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
+   warn("%s, DIOCXCOMMIT", __func__);
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -2558,6 +2595,7 @@ main(int argc, char *argv[])
pfctl_clear_stats(dev, ifaceopt, opts);
pfctl_clear_fingerprints(dev, opts);
pfctl_clear_interface_flags(dev, opts);
+   pfctl_reset(dev, opts);
}
break;
case 'o':
@@ -2566,6 +2604,9 @@ main(int argc, char *argv[])
case 'T':
pfctl_clear_tables(anchorname, opts);
break;
+   case 'R':
+   pfctl_reset(dev, opts);
+   break;
}
}
if (state_killers) {



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-03-28 Thread Alexandr Nedvedicky
Hello,



> > +Flush all of the above (+ reset settings).
> This is fine as is, I think.
> 
> > +void   pfctl_restore_defaults(int, int);
> Why not simply pfctl_reset()?

I had used pfctl_reset() at some point of history, then
I stopped to like it. Now I like it again.

updated diff below.

thanks and
regards
sashan

8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.8 b/sbin/pfctl/pfctl.8
index 48b2893cfcd..00bd27c200a 100644
--- a/sbin/pfctl/pfctl.8
+++ b/sbin/pfctl/pfctl.8
@@ -197,6 +197,8 @@ Flush the filter information (statistics that are not bound 
to rules).
 Flush the tables.
 .It Fl F Cm osfp
 Flush the passive operating system fingerprints.
+.It Fl F Cm Reset
+Reset limits, timeouts and options back to default settings.
 .It Fl F Cm all
 Flush all of the above.
 .El
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 493ff47af2f..1932649defc 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -105,6 +105,7 @@ int  pfctl_load_rule(struct pfctl *, char *, struct pf_rule 
*, int);
 const char *pfctl_lookup_option(char *, const char **);
 void   pfctl_state_store(int, const char *);
 void   pfctl_state_load(int, const char *);
+void   pfctl_reset(int, int);
 
 const char *clearopt;
 char   *rulesopt;
@@ -205,7 +206,8 @@ static const struct {
 };
 
 static const char *clearopt_list[] = {
-   "rules", "Sources", "states", "info", "Tables", "osfp", "all", NULL
+   "rules", "Sources", "states", "info", "Tables", "osfp", "Reset",
+   "all", NULL
 };
 
 static const char *showopt_list[] = {
@@ -2232,6 +2234,40 @@ pfctl_state_load(int dev, const char *file)
fclose(f);
 }
 
+void
+pfctl_reset(int dev, int opts)
+{
+   struct pfctlpf;
+   struct pfr_buffer t;
+   int i;
+
+   pf.dev = dev;
+   pfctl_init_options(&pf);
+
+   pf.debug_set = 1;
+   pf.reass_set = 1;
+   pf.syncookieswat_set = 1;
+   pf.ifname = strdup("none");
+   pf.ifname_set = 1;
+
+   memset(&t, 0, sizeof(t));
+   t.pfrb_type = PFRB_TRANS;
+   if (pfctl_trans(dev, &t, DIOCXBEGIN, 0))
+   warn("%s, DIOCXBEGIN", __func__);
+
+
+   for (i = 0; pf_limits[i].name; i++)
+   pf.limit_set[pf_limits[i].index] = 1;
+
+   for (i = 0; pf_timeouts[i].name; i++)
+   pf.timeout_set[pf_timeouts[i].timeout] = 1;
+
+   pfctl_load_options(&pf);
+
+   if (pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
+   warn("%s, DIOCXCOMMIT", __func__);
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -2558,6 +2594,7 @@ main(int argc, char *argv[])
pfctl_clear_stats(dev, ifaceopt, opts);
pfctl_clear_fingerprints(dev, opts);
pfctl_clear_interface_flags(dev, opts);
+   pfctl_reset(dev, opts);
}
break;
case 'o':
@@ -2566,6 +2603,9 @@ main(int argc, char *argv[])
case 'T':
pfctl_clear_tables(anchorname, opts);
break;
+   case 'R':
+   pfctl_reset(dev, opts);
+   break;
}
}
if (state_killers) {



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-03-28 Thread Klemens Nanni
On Wed, Mar 27, 2019 at 02:17:03AM +0100, Alexandr Nedvedicky wrote:
> tedu@ has planted idea for diff below here [1].  That particular email is part
> of thread [2], where various cleanup/unconfigure options for PF are discussed.
> To keep progressing in small steps I've decided to factor out the first diff
> here, which introduces '-FR' (a.k.a. reset settings) for pfctl(8).
A bit late, but I generally agree on "reset" and reusing `-F'.

Diff looks sane to me, two comments:
 
> -Flush all of the above.
> +Flush all of the above (+ reset settings).
This is fine as is, I think.

> +void pfctl_restore_defaults(int, int);
Why not simply pfctl_reset()?



introduce 'pfctl -FR' to reset settings to defaults

2019-03-26 Thread Alexandr Nedvedicky
Hello,

tedu@ has planted idea for diff below here [1].  That particular email is part
of thread [2], where various cleanup/unconfigure options for PF are discussed.
To keep progressing in small steps I've decided to factor out the first diff
here, which introduces '-FR' (a.k.a. reset settings) for pfctl(8).

OK?

thanks and
regards
sashan

[1] https://marc.info/?l=openbsd-tech&m=155356735115005&w=2

[2] https://marc.info/?l=openbsd-tech&m=155341612701577&w=2
[ this is a good start point where to gather the context ]

8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.8 b/sbin/pfctl/pfctl.8
index 48b2893cfcd..ab1693e5854 100644
--- a/sbin/pfctl/pfctl.8
+++ b/sbin/pfctl/pfctl.8
@@ -197,8 +197,10 @@ Flush the filter information (statistics that are not 
bound to rules).
 Flush the tables.
 .It Fl F Cm osfp
 Flush the passive operating system fingerprints.
+.It Fl F Cm Reset
+Reset limits, timeouts and options back to default settings.
 .It Fl F Cm all
-Flush all of the above.
+Flush all of the above (+ reset settings).
 .El
 .It Fl f Ar file
 Replace the current ruleset with
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 493ff47af2f..a6cf265451c 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -105,6 +105,7 @@ int  pfctl_load_rule(struct pfctl *, char *, struct pf_rule 
*, int);
 const char *pfctl_lookup_option(char *, const char **);
 void   pfctl_state_store(int, const char *);
 void   pfctl_state_load(int, const char *);
+void   pfctl_restore_defaults(int, int);
 
 const char *clearopt;
 char   *rulesopt;
@@ -205,7 +206,8 @@ static const struct {
 };
 
 static const char *clearopt_list[] = {
-   "rules", "Sources", "states", "info", "Tables", "osfp", "all", NULL
+   "rules", "Sources", "states", "info", "Tables", "osfp", "Reset",
+   "all", NULL
 };
 
 static const char *showopt_list[] = {
@@ -2232,6 +2234,40 @@ pfctl_state_load(int dev, const char *file)
fclose(f);
 }
 
+void
+pfctl_restore_defaults(int dev, int opts)
+{
+   struct pfctlpf;
+   struct pfr_buffer t;
+   int i;
+
+   pf.dev = dev;
+   pfctl_init_options(&pf);
+
+   pf.debug_set = 1;
+   pf.reass_set = 1;
+   pf.syncookieswat_set = 1;
+   pf.ifname = strdup("none");
+   pf.ifname_set = 1;
+
+   memset(&t, 0, sizeof(t));
+   t.pfrb_type = PFRB_TRANS;
+   if (pfctl_trans(dev, &t, DIOCXBEGIN, 0))
+   warn("%s, DIOCXBEGIN", __func__);
+
+
+   for (i = 0; pf_limits[i].name; i++)
+   pf.limit_set[pf_limits[i].index] = 1;
+
+   for (i = 0; pf_timeouts[i].name; i++)
+   pf.timeout_set[pf_timeouts[i].timeout] = 1;
+
+   pfctl_load_options(&pf);
+
+   if (pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
+   warn("%s, DIOCXCOMMIT", __func__);
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -2558,6 +2594,7 @@ main(int argc, char *argv[])
pfctl_clear_stats(dev, ifaceopt, opts);
pfctl_clear_fingerprints(dev, opts);
pfctl_clear_interface_flags(dev, opts);
+   pfctl_restore_defaults(dev, opts);
}
break;
case 'o':
@@ -2566,6 +2603,9 @@ main(int argc, char *argv[])
case 'T':
pfctl_clear_tables(anchorname, opts);
break;
+   case 'R':
+   pfctl_restore_defaults(dev, opts);
+   break;
}
}
if (state_killers) {