Re: pfctl: prevent modifying internal anchors through their tables

2018-09-14 Thread Klemens Nanni
On Wed, Sep 12, 2018 at 02:05:25PM +0200, Alexander Bluhm wrote:
> On Tue, Sep 11, 2018 at 12:17:05PM +0200, Klemens Nanni wrote:
> > Now `t' under the anonymous anchors (internally named "_1") must not be
> > modified through pfctl:
> > 
> > # pfctl -a _1 -t t -T flush
> > 0 addresses deleted.
> 
> Why do you think that this semantic is wrong?  Why should tables
> within an anonoumus anchor be constant?
Because that's what I count as modifying reserved anchors from the
command line, similar to how adding/removing rules or further anchors
below it.

Thinking about it after your question made me realise that I'm not
checking whether the table is used exclusively within the reserved
anchor. Contrary to rules, the same table may be used in multiple places
and my diff would rather naively prevent write access to them.

Given the case of changing reserved anchors on the command line is
already a corner case, trying to prevent users from editing
automatically generated tables within them is even more so.

Thanks for your feedback.



Re: pfctl: prevent modifying internal anchors through their tables

2018-09-12 Thread Stuart Henderson
On 2018/09/12 17:38, Jason McIntyre wrote:
> On Wed, Sep 12, 2018 at 02:05:25PM +0200, Alexander Bluhm wrote:
> > 
> > > + warnx("anchors apply to -f, -F, -t and -s only");
> > 
> > If I understand English comma rules correctly, there is also a comma
> > before the " and".  At least this is what we do in the man page.
> > 
> > bluhm
> > 
> 
> hi.
> 
> just to note: it is not wrong to omit the comma. i personally use them
> there, because it eliminates some potential confusion in lists. but it
> does sometime look/read better without. it's a matter of taste i guess.
> 
> jmc
> 

btw (as jmc will already know of course!) this is known as an "Oxford comma"
or "serial comma", you will find a *lot* of references and arguments in both
directions if you search ;)



Re: pfctl: prevent modifying internal anchors through their tables

2018-09-12 Thread Jason McIntyre
On Wed, Sep 12, 2018 at 02:05:25PM +0200, Alexander Bluhm wrote:
> 
> > +   warnx("anchors apply to -f, -F, -t and -s only");
> 
> If I understand English comma rules correctly, there is also a comma
> before the " and".  At least this is what we do in the man page.
> 
> bluhm
> 

hi.

just to note: it is not wrong to omit the comma. i personally use them
there, because it eliminates some potential confusion in lists. but it
does sometime look/read better without. it's a matter of taste i guess.

jmc



Re: pfctl: prevent modifying internal anchors through their tables

2018-09-12 Thread Alexander Bluhm
On Tue, Sep 11, 2018 at 12:17:05PM +0200, Klemens Nanni wrote:
> Now `t' under the anonymous anchors (internally named "_1") must not be
> modified through pfctl:
> 
>   # pfctl -a _1 -t t -T flush
>   0 addresses deleted.

Why do you think that this semantic is wrong?  Why should tables
within an anonoumus anchor be constant?

> + warnx("anchors apply to -f, -F, -t and -s only");

If I understand English comma rules correctly, there is also a comma
before the " and".  At least this is what we do in the man page.

bluhm



Re: pfctl: prevent modifying internal anchors through their tables

2018-09-11 Thread Klemens Nanni
On Tue, Sep 11, 2018 at 12:17:05PM +0200, Klemens Nanni wrote:
> Anchor names beginning with '_' are reserved for internal use, but this
> particular case still works:
My example is not exclusive; this effects all tables within special
anchors including those automatically created by the ruleset optimizer.

New diff flags mentioned in the same order as they appear in the
synopsis.

OK?

Index: pfctl.8
===
RCS file: /cvs/src/sbin/pfctl/pfctl.8,v
retrieving revision 1.171
diff -u -p -r1.171 pfctl.8
--- pfctl.8 11 Aug 2017 22:30:38 -  1.171
+++ pfctl.8 11 Sep 2018 21:59:21 -
@@ -94,8 +94,9 @@ The options are as follows:
 Apply flags
 .Fl f ,
 .Fl F ,
+.Fl s ,
 and
-.Fl s
+.Fl t
 only to the rules in the specified
 .Ar anchor .
 In addition to the main ruleset,
Index: pfctl.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl.c,v
retrieving revision 1.359
diff -u -p -r1.359 pfctl.c
--- pfctl.c 8 Sep 2018 14:45:55 -   1.359
+++ pfctl.c 11 Sep 2018 21:59:21 -
@@ -2498,8 +2498,8 @@ main(int argc, char *argv[])
 
memset(anchorname, 0, sizeof(anchorname));
if (anchoropt != NULL) {
-   if (mode == O_RDONLY && showopt == NULL) {
-   warnx("anchors apply to -f, -F and -s only");
+   if (mode == O_RDONLY && showopt == NULL && tblcmdopt == NULL) {
+   warnx("anchors apply to -f, -F, -s and -t only");
usage();
}
if (mode == O_RDWR &&