Re: [nft PATCH] evaluate: Reject quoted strings containing only wildcard

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Dec 17, 2020 at 03:23:40PM +0100, Phil Sutter wrote:
> On Wed, Sep 30, 2020 at 12:20:33PM +0200, Pablo Neira Ayuso wrote:
> > On Mon, Sep 28, 2020 at 10:19:25AM +0200, Phil Sutter wrote:
> > > On Thu, Sep 24, 2020 at 07:06:39PM +0200, Phil Sutter wrote:
> > > > Fix for an assertion fail when trying to match against an all-wildcard
> > > > interface name:
> > > > 
> > > > | % nft add rule t c iifname '"*"'
> > > > | nft: expression.c:402: constant_expr_alloc: Assertion `(((len) + (8) - 1) / (8)) > 0' failed.
> > > > | zsh: abort      nft add rule t c iifname '"*"'
> > > > 
> > > > Fix this by detecting the string in expr_evaluate_string() and returning
> > > > an error message:
> > > > 
> > > > | % nft add rule t c iifname '"*"'
> > > > | Error: All-wildcard strings are not supported
> > > > | add rule t c iifname "*"
> > > > |                      ^^^
> > > > 
> > > 
> > > Note that all this is pretty inconsistent: The above happens only for
> > > quoted asterisks. Unquoted ones cause a different error (at least no
> > > assertion fail):
> > > 
> > > | % nft add rule t c iifname '*'
> > > | Error: datatype mismatch, expected network interface name, expression has type integer
> > > | add rule t c iifname *
> > > |              ~~~~~~~ ^
> > > 
> > > What puzzles me is that we have:
> > > 
> > > | wildcard_expr           :       ASTERISK
> > > |                         {
> > > |                                 struct expr *expr;
> > > | 
> > > |                                 expr = constant_expr_alloc(&@$, &integer_type,
> > > |                                                            BYTEORDER_HOST_ENDIAN,
> > > |                                                            0, NULL);
> > > |                                 $$ = prefix_expr_alloc(&@$, expr, 0);
> > > |                         }
> > > |                         ;
> > > 
> > > Yet when trying to use it as a prefix, it is rejected:
> > > 
> > > | % nft add rule t c ip saddr '*'
> > > | Error: datatype mismatch, expected IPv4 address, expression has type integer
> > > | add rule t c ip saddr *
> > > |              ~~~~~~~~ ^
> > > 
> > > So is this wildcard_expr simply broken or didn't I find correct way to use it
> > > yet?
> > 
> > This looks like some preliminary support for wildcard matching in set
> > elements, but my impression is that this is broken. I don't remember
> > to have seen any tests covering this.
> 
> OK. If it needs fixing, I guess that's a different issue. Are you fine
> with the "fix" for asterisk-only interface names for the time being?

I think so, yes.