* [nft PATCH] evaluate: Reject quoted strings containing only wildcard @ 2020-09-24 17:06 Phil Sutter 2020-09-28 8:19 ` Phil Sutter 0 siblings, 1 reply; 5+ messages in thread From: Phil Sutter @ 2020-09-24 17:06 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel 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 "*" | ^^^ While being at it, drop the 'datalen >= 1' clause from the following conditional as together with the added check for 'datalen == 0', all possible other values have been caught already. Signed-off-by: Phil Sutter <phil@nwl.cc> --- src/evaluate.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/evaluate.c b/src/evaluate.c index c8045e5ded729..5f17d7501ac0e 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -324,8 +324,11 @@ static int expr_evaluate_string(struct eval_ctx *ctx, struct expr **exprp) return 0; } - if (datalen >= 1 && - data[datalen - 1] == '\\') { + if (datalen == 0) + return expr_error(ctx->msgs, expr, + "All-wildcard strings are not supported"); + + if (data[datalen - 1] == '\\') { char unescaped_str[data_len]; memset(unescaped_str, 0, sizeof(unescaped_str)); -- 2.28.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [nft PATCH] evaluate: Reject quoted strings containing only wildcard 2020-09-24 17:06 [nft PATCH] evaluate: Reject quoted strings containing only wildcard Phil Sutter @ 2020-09-28 8:19 ` Phil Sutter 2020-09-30 10:20 ` Pablo Neira Ayuso 0 siblings, 1 reply; 5+ messages in thread From: Phil Sutter @ 2020-09-28 8:19 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel 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? Cheers, Phil ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [nft PATCH] evaluate: Reject quoted strings containing only wildcard 2020-09-28 8:19 ` Phil Sutter @ 2020-09-30 10:20 ` Pablo Neira Ayuso 2020-12-17 14:23 ` Phil Sutter 0 siblings, 1 reply; 5+ messages in thread From: Pablo Neira Ayuso @ 2020-09-30 10:20 UTC (permalink / raw) To: Phil Sutter, netfilter-devel 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [nft PATCH] evaluate: Reject quoted strings containing only wildcard 2020-09-30 10:20 ` Pablo Neira Ayuso @ 2020-12-17 14:23 ` Phil Sutter 2020-12-17 14:32 ` Pablo Neira Ayuso 0 siblings, 1 reply; 5+ messages in thread From: Phil Sutter @ 2020-12-17 14:23 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel 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? Thanks, Phil ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [nft PATCH] evaluate: Reject quoted strings containing only wildcard 2020-12-17 14:23 ` Phil Sutter @ 2020-12-17 14:32 ` Pablo Neira Ayuso 0 siblings, 0 replies; 5+ messages in thread From: Pablo Neira Ayuso @ 2020-12-17 14:32 UTC (permalink / raw) To: Phil Sutter, netfilter-devel 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-17 14:33 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-09-24 17:06 [nft PATCH] evaluate: Reject quoted strings containing only wildcard Phil Sutter 2020-09-28 8:19 ` Phil Sutter 2020-09-30 10:20 ` Pablo Neira Ayuso 2020-12-17 14:23 ` Phil Sutter 2020-12-17 14:32 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).