* Re: [nft PATCH] src: Quote user-defined names
2020-01-23 22:45 ` [nft PATCH] src: Quote user-defined names Phil Sutter
@ 2020-06-09 17:36 ` Phil Sutter
0 siblings, 0 replies; 2+ messages in thread
From: Phil Sutter @ 2020-06-09 17:36 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel, fw
Hi,
On Thu, Jan 23, 2020 at 11:45:48PM +0100, Phil Sutter wrote:
> On Thu, Feb 14, 2019 at 06:43:03PM +0100, Phil Sutter wrote:
> > On Thu, Feb 14, 2019 at 12:10:54PM +0100, Pablo Neira Ayuso wrote:
> > > Another spin on this, let's try to make a final decision on this asap.
> > >
> > > In this case, this patch passes the quoted string to the kernel, so
> > > the listing shows it again.
> > >
> > > Still, problem here is that the shell is stripping off the quotes
> > > unless I escape them, ie.
> > >
> > > nft add chain "x" "y"
> > >
> > > enters the unquoted path from the scannner. So I have to use:
> > >
> > > nft add chain \"x\" \"y\"
> > >
> > > or:
> > >
> > > nft add chain x '"y"'
> > >
> > > I think your patch fixes the problem with using keywords as object
> > > names, which we could also fix via a rule that deals with this.
> > >
> > > The problem with using any arbitrary name would be still there, unless
> > > the user escapes the quotes.
> > >
> > > On the other hand, if we quote the string in the listing by default,
> > > we clearly show that these are user-defined, which is not a bad idea.
> > > However, we don't get much from showing quotes by default on listings,
> > > I mean, this is not solving the arbitrary name problem from the input
> > > path, which I think it the real problem.
> > >
> > > Then, enforcing quotes since the beginning would not have helped
> > > either, because of the shell behaviour.
> > >
> > > Exploring another patch here to allow to use keywords without quotes
> > > as object names, it won't look nice in bison, since we will need
> > > something similar to what we do in primary_rhs_expr for TCP, UDP...
> > > but it will work.
> >
> > Are you sure about that? Flex would still recognize the keyword as such
> > and you won't get STRING type in Bison. Or am I missing the point?
> >
> > Personally, I'm totally fine with people having to escape the quotes.
> > This is how shells work, and we have the same problem in other situation
> > requiring the quotes, too. My shell for instance catches the curly
> > braces and semi-colons, as well if not escaped.
> >
> > Quoting all user-defined names on output merely helps with the case
> > where a user *really* wanted to produce a confusing ruleset and to avoid
> > ruleset restore after dump from failing miserably because the names are
> > not quoted in output.
>
> Getting back to this dusty topic again: I played with extending
> 'identifier' in parser_bison.c like so:
>
> | @@ -2183,6 +2183,7 @@ chain_policy : ACCEPT { $$ = NF_ACCEPT; }
> | ;
> |
> | identifier : STRING
> | + | HOUR { $$ = strdup("hour"); }
> | ;
> |
> | string : STRING
>
> I am able to create a table named 'hour'. The approach has two problems
> though AFAICT:
>
> 1) When adding TABLE as identifier, bison spews shift/reduce conflict
> warnings.
>
> 2) In order to allow for really arbitrary names, we would have to add
> all defined keywords to identifier. This is tedious, ugly and (most
> importantly) a moving target.
I had another idea, namely teaching flex to recognize a bit of context
by defining e.g. 'table <something>' as a keyword. But this turns into a
mess when it comes to optional family specifiers, line breaks, etc. Also
this is probably not how lex/yacc was meant to be used.
The reason why I keep getting back to this is the increasing impact on
users over time: Whenever we introduce a new keyword, we potentially
break someone's working ruleset.
With bison not even accepting quoted names everywhere, there is no way
out even for cautious users.
Can we please restart discussions on how to solve this issue?
Cheers, Phil
^ permalink raw reply [flat|nested] 2+ messages in thread