Re: Failing to construct a 'set' for TCP Flag filtering.

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

[-- Attachment #1: Type: text/plain, Size: 1917 bytes --]

On Wed, Sep 02, 2020 at 05:44:11PM +0100, Chris Hall wrote:
> 
> I have constructed the file 'tryx' containing:
> 
> #---------------------------------------------------
> flush ruleset
> add table ip MAIN
> 
> add set ip MAIN tcp_good_flags { type tcp_flag ; flags constant ; \
> elements = {                  \
>   ( 0 | 0 | 0 |ack| 0 | 0 ),  \
>   ( 0 | 0 | 0 |ack| 0 |urg),  \
>   ( 0 | 0 | 0 |ack|psh| 0 ),  \
>   ( 0 | 0 | 0 |ack|psh|urg),  \
>   ( 0 | 0 |rst| 0 | 0 | 0 ),  \
>   ( 0 | 0 |rst|ack| 0 | 0 ),  \
>   ( 0 | 0 |rst|ack| 0 |urg),  \
>   ( 0 | 0 |rst|ack|psh| 0 ),  \
>   ( 0 | 0 |rst|ack|psh|urg),  \
>   ( 0 |syn| 0 | 0 | 0 | 0 ),  \
>   ( 0 |syn| 0 |ack| 0 | 0 ),  \
>   ( 0 |syn| 0 |ack| 0 |urg),  \
>   ( 0 |syn| 0 |ack|psh| 0 ),  \
>   ( 0 |syn| 0 |ack|psh|urg),  \
>   (fin| 0 | 0 |ack| 0 | 0 ),  \
>   (fin| 0 | 0 |ack| 0 |urg),  \
>   (fin| 0 | 0 |ack|psh| 0 ),  \
>   (fin| 0 | 0 |ack|psh|urg)   \
> } ; }
> 
> list ruleset
> #---------------------------------------------------
> 
> With a view to using this to filter out invalid combinations of TCP flags.
> 
> When I give that to nft I get:
> 
> # nft -f tryx
> table ip MAIN {
>         set tcp_good_flags {
>                 type tcp_flag
>                 flags constant
>                 elements = { ack, 0x30, 0x18, 0x38, rst,
>                              0x14, 0x34, 0x1c, 0x3c, syn,
>                              0x12, 0x32, 0x1a, 0x3a, 0x11,
>                              0x31, 0x19, 0x39 }
>         }
> }
> 
> ...which suggests that all is well so far... but it isn't.  And if I try to
> check the state of the ruleset again:
> 
> # nft list ruleset
> BUG: Unknown expression binop
> nft: mergesort.c:47: expr_msort_cmp: Assertion `0' failed.
> Aborted (core dumped)
> 
> Indeed, nothing seems to work until I 'flush ruleset' again !
> 
> Am I asking for the impossible here ?

It's a bug, I'll apply this patch to git.


[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 621 bytes --]

diff --git a/src/mergesort.c b/src/mergesort.c
index 02094b486aeb..4d5bdbd414db 100644
--- a/src/mergesort.c
+++ b/src/mergesort.c
@@ -44,7 +44,15 @@ static int expr_msort_cmp(const struct expr *e1, const struct expr *e2)
 	case EXPR_MAPPING:
 		return expr_msort_cmp(e1->left, e2->left);
 	case EXPR_BINOP:
-		return expr_msort_cmp(e1->left, e2->left);
+		switch (e2->etype) {
+		case EXPR_VALUE:
+			return expr_msort_cmp(e1->left, e2);
+		case EXPR_BINOP:
+			return expr_msort_cmp(e1->left, e2->left);
+		default:
+			break;
+		}
+		/* fall through */
 	default:
 		BUG("Unknown expression %s\n", expr_name(e1));
 	}