On Tue, Aug 31, 2021 at 07:33:40AM +0200, Alexandr Nedvedicky wrote:
> Hello,
> 
> On Tue, Aug 31, 2021 at 02:40:57PM +1000, David Gwynne wrote:
> > handling the "no" option with a token, and "yes" via a string made my
> > eye twitch.
> > 
> > ok? or is the helpful yyerror a nice feature?
> > 
> 
>     I actually think it's a nice feature. below is output
>     for current pfctl we have in tree:

it is nice, but the implementation isn't... rigorous.

> 
>       lumpy$ pfctl -n -f /tmp/pf.conf
>       /tmp/pf.conf:6: invalid value 'nope', expected 'yes' or 'no'
> 
>     and output with diff applied:
> 
>       lumpy$ ./pfctl -n -f /tmp/pf.conf
>       /tmp/pf.conf:6: syntax error

but if you try to use a keyword instead of a string, you get this:

dlg@kbuild ~$ echo "set reassemble yes" | pfctl -vnf -
set reassemble yes 
dlg@kbuild ~$ echo "set reassemble no" | pfctl -vnf -  
set reassemble no 
dlg@kbuild ~$ echo "set reassemble nope" | pfctl -vnf -
stdin:1: invalid value 'nope', expected 'yes' or 'no'
dlg@kbuild ~$ echo "set reassemble block" | pfctl -vnf -
stdin:1: syntax error
dlg@kbuild ~$ 

if the tokeniser exposed the buffer it was working on, we could make it
consistent for all arguments:

dlg@kbuild pfctl$ echo "set reassemble yes" | ./obj/pfctl -vnf -   
set reassemble yes 
dlg@kbuild pfctl$ echo "set reassemble no" | ./obj/pfctl -vnf -  
set reassemble no 
dlg@kbuild pfctl$ echo "set reassemble nope" | ./obj/pfctl -vnf -
stdin:1: syntax error
stdin:1: invalid value 'nope', expected 'yes' or 'no'
dlg@kbuild pfctl$ echo "set reassemble block" | ./obj/pfctl -vnf -
stdin:1: syntax error
stdin:1: invalid value 'block', expected 'yes' or 'no'

the extremely rough PoC diff for pfctl that implements this is
below. because the tokeniser handles some operators without using the
buffer, if you give "set reassemble" an operator then you get confusing
output:

dlg@kbuild pfctl$ echo "set reassemble <" | ./obj/pfctl -vnf -     
stdin:1: syntax error
stdin:1: invalid value 'reassemble', expected 'yes' or 'no'

anyway, it might be easier to drop the diff for now.

Index: parse.y
===================================================================
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.709
diff -u -p -r1.709 parse.y
--- parse.y     1 Feb 2021 00:31:04 -0000       1.709
+++ parse.y     31 Aug 2021 09:20:38 -0000
@@ -458,6 +458,8 @@ typedef struct {
        int lineno;
 } YYSTYPE;
 
+static u_char   *yytext;
+
 #define PPORT_RANGE    1
 #define PPORT_STAR     2
 int    parseport(char *, struct range *r, int);
@@ -471,7 +473,7 @@ int parseport(char *, struct range *r, i
 %token PASS BLOCK MATCH SCRUB RETURN IN OS OUT LOG QUICK ON FROM TO FLAGS
 %token RETURNRST RETURNICMP RETURNICMP6 PROTO INET INET6 ALL ANY ICMPTYPE
 %token ICMP6TYPE CODE KEEP MODULATE STATE PORT BINATTO NODF
-%token MINTTL ERROR ALLOWOPTS FILENAME ROUTETO DUPTO REPLYTO NO LABEL
+%token MINTTL ERROR ALLOWOPTS FILENAME ROUTETO DUPTO REPLYTO YES NO LABEL
 %token NOROUTE URPFFAILED FRAGMENT USER GROUP MAXMSS MAXIMUM TTL TOS DROP TABLE
 %token REASSEMBLE ANCHOR SYNCOOKIES
 %token SET OPTIMIZATION TIMEOUT LIMIT LOGINTERFACE BLOCKPOLICY RANDOMID
@@ -3754,16 +3756,11 @@ comma           : ','
                ;
 
 yesno          : NO                    { $$ = 0; }
-               | STRING                {
-                       if (!strcmp($1, "yes"))
-                               $$ = 1;
-                       else {
-                               yyerror("invalid value '%s', expected 'yes' "
-                                   "or 'no'", $1);
-                               free($1);
-                               YYERROR;
-                       }
-                       free($1);
+               | YES                   { $$ = 1; }
+               | error                 {
+                       yyerror("invalid value '%s', expected 'yes' or 'no'",
+                           yytext);
+                       YYABORT;
                }
                ;
 
@@ -5048,6 +5045,7 @@ lookup(char *s)
                { "urpf-failed",        URPFFAILED},
                { "user",               USER},
                { "weight",             WEIGHT},
+               { "yes",                YES},
        };
        const struct keywords   *p;
 
@@ -5170,10 +5168,12 @@ findeol(void)
 int
 yylex(void)
 {
-       u_char   buf[8096];
+       static u_char buf[8192];
        u_char  *p, *val;
        int      quotec, next, c;
        int      token;
+
+       yytext = buf;
 
 top:
        p = buf;



Reply via email to