[PATCH nft v4 1/2] jump: Introduce chain_expr in jump and goto statements

[PATCH nft v4 1/2] jump: Introduce chain_expr in jump and goto statements

[Fernando Fernandez Mancera]

Now we can introduce expressions as a chain in jump and goto statements. This
is going to be used to support variables as a chain in the following patches.

Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
v1: initial patch
v2: fixes in the parser
v3: fixes rare usecases
v4: add json support
---
 include/expression.h |  4 ++--
 src/datatype.c       | 22 ++++++++++++++++++++--
 src/evaluate.c       |  4 ++++
 src/expression.c     | 12 ++++++------
 src/json.c           |  6 +++---
 src/netlink.c        | 26 +++++++++++++++++++++-----
 src/parser_bison.y   | 17 +++++++++++++----
 src/parser_json.c    |  9 +++++++--
 8 files changed, 76 insertions(+), 24 deletions(-)

diff --git a/include/expression.h b/include/expression.h
index 6416ac0..ef41255 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -240,7 +240,7 @@ struct expr {
 		struct {
 			/* EXPR_VERDICT */
 			int			verdict;
-			const char		*chain;
+			struct expr		*chain;
 		};
 		struct {
 			/* EXPR_VALUE */
@@ -403,7 +403,7 @@ extern void relational_expr_pctx_update(struct proto_ctx *ctx,
 					const struct expr *expr);
 
 extern struct expr *verdict_expr_alloc(const struct location *loc,
-				       int verdict, const char *chain);
+				       int verdict, struct expr *chain);
 
 extern struct expr *symbol_expr_alloc(const struct location *loc,
 				      enum symbol_types type, struct scope *scope,
diff --git a/src/datatype.c b/src/datatype.c
index ac9f2af..10f185b 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -254,6 +254,8 @@ const struct datatype invalid_type = {
 
 static void verdict_type_print(const struct expr *expr, struct output_ctx *octx)
 {
+	char chain[NFT_CHAIN_MAXNAMELEN];
+
 	switch (expr->verdict) {
 	case NFT_CONTINUE:
 		nft_print(octx, "continue");
@@ -262,10 +264,26 @@ static void verdict_type_print(const struct expr *expr, struct output_ctx *octx)
 		nft_print(octx, "break");
 		break;
 	case NFT_JUMP:
-		nft_print(octx, "jump %s", expr->chain);
+		if (expr->chain->etype == EXPR_VALUE) {
+			mpz_export_data(chain, expr->chain->value,
+					BYTEORDER_HOST_ENDIAN,
+					NFT_CHAIN_MAXNAMELEN);
+			nft_print(octx, "jump %s", chain);
+		} else {
+			nft_print(octx, "jump ");
+			expr_print(expr->chain, octx);
+		}
 		break;
 	case NFT_GOTO:
-		nft_print(octx, "goto %s", expr->chain);
+		if (expr->chain->etype == EXPR_VALUE) {
+			mpz_export_data(chain, expr->chain->value,
+					BYTEORDER_HOST_ENDIAN,
+					NFT_CHAIN_MAXNAMELEN);
+			nft_print(octx, "goto %s", chain);
+		} else {
+			nft_print(octx, "goto ");
+			expr_print(expr->chain, octx);
+		}
 		break;
 	case NFT_RETURN:
 		nft_print(octx, "return");
diff --git a/src/evaluate.c b/src/evaluate.c
index 21d9e14..8394037 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1947,6 +1947,10 @@ static int stmt_evaluate_verdict(struct eval_ctx *ctx, struct stmt *stmt)
 	case EXPR_VERDICT:
 		if (stmt->expr->verdict != NFT_CONTINUE)
 			stmt->flags |= STMT_F_TERMINAL;
+		if (stmt->expr->chain != NULL) {
+			if (expr_evaluate(ctx, &stmt->expr->chain) < 0)
+				return -1;
+		}
 		break;
 	case EXPR_MAP:
 		break;
diff --git a/src/expression.c b/src/expression.c
index eece12e..a41e2da 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -207,22 +207,22 @@ static bool verdict_expr_cmp(const struct expr *e1, const struct expr *e2)
 
 	if ((e1->verdict == NFT_JUMP ||
 	     e1->verdict == NFT_GOTO) &&
-	    strcmp(e1->chain, e2->chain))
-		return false;
+	     expr_cmp(e1->chain, e2->chain))
+		return true;
 
-	return true;
+	return false;
 }
 
 static void verdict_expr_clone(struct expr *new, const struct expr *expr)
 {
 	new->verdict = expr->verdict;
 	if (expr->chain != NULL)
-		new->chain = xstrdup(expr->chain);
+		mpz_init_set(new->chain->value, expr->chain->value);
 }
 
 static void verdict_expr_destroy(struct expr *expr)
 {
-	xfree(expr->chain);
+	expr_free(expr->chain);
 }
 
 static const struct expr_ops verdict_expr_ops = {
@@ -236,7 +236,7 @@ static const struct expr_ops verdict_expr_ops = {
 };
 
 struct expr *verdict_expr_alloc(const struct location *loc,
-				int verdict, const char *chain)
+				int verdict, struct expr *chain)
 {
 	struct expr *expr;
 
diff --git a/src/json.c b/src/json.c
index ff79b0c..a503a97 100644
--- a/src/json.c
+++ b/src/json.c
@@ -673,14 +673,14 @@ json_t *verdict_expr_json(const struct expr *expr, struct output_ctx *octx)
 		{ NF_QUEUE, "queue", false },
 	};
 	const char *name = NULL;
-	const char *chain = NULL;
+	json_t *chain = NULL;
 	unsigned int i;
 
 	for (i = 0; i < array_size(verdict_tbl); i++) {
 		if (expr->verdict == verdict_tbl[i].verdict) {
 			name = verdict_tbl[i].name;
 			if (verdict_tbl[i].chain && expr->chain)
-				chain = expr->chain;
+				chain = expr_print_json(expr->chain, octx);
 			break;
 		}
 	}
@@ -689,7 +689,7 @@ json_t *verdict_expr_json(const struct expr *expr, struct output_ctx *octx)
 		return NULL;
 	}
 	if (chain)
-		return json_pack("{s:{s:s}}", name, "target", chain);
+		return json_pack("{s:{s:o}}", name, "target", chain);
 	else
 		return json_pack("{s:n}", name);
 }
diff --git a/src/netlink.c b/src/netlink.c
index c051ae6..ef12cb0 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -218,12 +218,17 @@ static void netlink_gen_constant_data(const struct expr *expr,
 static void netlink_gen_verdict(const struct expr *expr,
 				struct nft_data_linearize *data)
 {
+	char chain[NFT_CHAIN_MAXNAMELEN];
+
 	data->verdict = expr->verdict;
 
 	switch (expr->verdict) {
 	case NFT_JUMP:
 	case NFT_GOTO:
-		snprintf(data->chain, NFT_CHAIN_MAXNAMELEN, "%s", expr->chain);
+		mpz_export_data(chain, expr->chain->value,
+				BYTEORDER_HOST_ENDIAN,
+				NFT_CHAIN_MAXNAMELEN);
+		snprintf(data->chain, NFT_CHAIN_MAXNAMELEN, "%s", chain);
 		data->chain[NFT_CHAIN_MAXNAMELEN-1] = '\0';
 		break;
 	}
@@ -253,12 +258,15 @@ struct expr *netlink_alloc_value(const struct location *loc,
 static struct expr *netlink_alloc_verdict(const struct location *loc,
 					  const struct nft_data_delinearize *nld)
 {
-	char *chain;
+	struct expr *chain;
 
 	switch (nld->verdict) {
 	case NFT_JUMP:
 	case NFT_GOTO:
-		chain = xstrdup(nld->chain);
+		chain = constant_expr_alloc(loc, &string_type,
+					    BYTEORDER_HOST_ENDIAN,
+					    NFT_CHAIN_MAXNAMELEN *
+					    BITS_PER_BYTE, nld->chain);
 		break;
 	default:
 		chain = NULL;
@@ -1153,14 +1161,22 @@ static void trace_print_expr(const struct nftnl_trace *nlt, unsigned int attr,
 static void trace_print_verdict(const struct nftnl_trace *nlt,
 				 struct output_ctx *octx)
 {
+	struct expr *chain_expr = NULL;
 	const char *chain = NULL;
 	unsigned int verdict;
 	struct expr *expr;
 
 	verdict = nftnl_trace_get_u32(nlt, NFTNL_TRACE_VERDICT);
-	if (nftnl_trace_is_set(nlt, NFTNL_TRACE_JUMP_TARGET))
+	if (nftnl_trace_is_set(nlt, NFTNL_TRACE_JUMP_TARGET)) {
 		chain = xstrdup(nftnl_trace_get_str(nlt, NFTNL_TRACE_JUMP_TARGET));
-	expr = verdict_expr_alloc(&netlink_location, verdict, chain);
+		chain_expr = constant_expr_alloc(&netlink_location,
+						 &string_type,
+						 BYTEORDER_HOST_ENDIAN,
+						 NFT_CHAIN_MAXNAMELEN
+						 * BITS_PER_BYTE,
+						 chain);
+	}
+	expr = verdict_expr_alloc(&netlink_location, verdict, chain_expr);
 
 	nft_print(octx, "verdict ");
 	expr_print(expr, octx);
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 9e632c0..b1e29a8 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -618,8 +618,8 @@ int nft_lex(void *, void *, void *);
 %type <stmt>			meter_stmt meter_stmt_alloc flow_stmt_legacy_alloc
 %destructor { stmt_free($$); }	meter_stmt meter_stmt_alloc flow_stmt_legacy_alloc
 
-%type <expr>			symbol_expr verdict_expr integer_expr variable_expr
-%destructor { expr_free($$); }	symbol_expr verdict_expr integer_expr variable_expr
+%type <expr>			symbol_expr verdict_expr integer_expr variable_expr chain_expr
+%destructor { expr_free($$); }	symbol_expr verdict_expr integer_expr variable_expr chain_expr
 %type <expr>			primary_expr shift_expr and_expr
 %destructor { expr_free($$); }	primary_expr shift_expr and_expr
 %type <expr>			exclusive_or_expr inclusive_or_expr
@@ -3827,11 +3827,11 @@ verdict_expr		:	ACCEPT
 			{
 				$$ = verdict_expr_alloc(&@$, NFT_CONTINUE, NULL);
 			}
-			|	JUMP			identifier
+			|	JUMP			chain_expr
 			{
 				$$ = verdict_expr_alloc(&@$, NFT_JUMP, $2);
 			}
-			|	GOTO			identifier
+			|	GOTO			chain_expr
 			{
 				$$ = verdict_expr_alloc(&@$, NFT_GOTO, $2);
 			}
@@ -3841,6 +3841,15 @@ verdict_expr		:	ACCEPT
 			}
 			;
 
+chain_expr		:	identifier
+			{
+				$$ = constant_expr_alloc(&@$, &string_type,
+							 BYTEORDER_HOST_ENDIAN,
+							 NFT_CHAIN_MAXNAMELEN *
+							 BITS_PER_BYTE, $1);
+			}
+			;
+
 meta_expr		:	META	meta_key
 			{
 				$$ = meta_expr_alloc(&@$, $2);
diff --git a/src/parser_json.c b/src/parser_json.c
index 1b3e086..80f17d8 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -1068,10 +1068,16 @@ static struct expr *json_parse_verdict_expr(struct json_ctx *ctx,
 		{ NF_ACCEPT, "accept", false },
 		{ NF_DROP, "drop", false },
 	};
+	struct expr *chain_expr = NULL;
 	const char *chain = NULL;
 	unsigned int i;
 
 	json_unpack(root, "{s:s}", "target", &chain);
+	if (!chain)
+		chain_expr = constant_expr_alloc(int_loc, &string_type,
+						 BYTEORDER_HOST_ENDIAN,
+						 NFT_CHAIN_MAXNAMELEN *
+						 BITS_PER_BYTE, chain);
 
 	for (i = 0; i < array_size(verdict_tbl); i++) {
 		if (strcmp(type, verdict_tbl[i].name))
@@ -1082,8 +1088,7 @@ static struct expr *json_parse_verdict_expr(struct json_ctx *ctx,
 			return NULL;
 
 		return verdict_expr_alloc(int_loc,
-					  verdict_tbl[i].verdict,
-					  chain ? xstrdup(chain) : NULL);
+					  verdict_tbl[i].verdict, chain_expr);
 	}
 	json_error(ctx, "Unknown verdict '%s'.", type);
 	return NULL;
-- 
2.20.1

[PATCH nft v4 2/2] jump: Allow goto and jump to a variable using nft input files

[Fernando Fernandez Mancera]

This patch introduces the use of nft input files variables in 'jump' and 'goto'
statements, e.g.

define dest = ber

add table ip foo
add chain ip foo bar {type filter hook input priority 0;}
add chain ip foo ber
add rule ip foo ber counter
add rule ip foo bar jump $dest

table ip foo {
        chain bar {
                type filter hook input priority filter; policy accept;
                jump ber
        }

        chain ber {
                counter packets 71 bytes 6664
        }
}

Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
v1: initial patch
v2: fixes parser
v3: add rare usecases tests
v4: nothing new in this patch
---
 src/datatype.c                                | 11 ++++++++++
 src/evaluate.c                                |  7 +++++++
 src/parser_bison.y                            |  3 ++-
 .../shell/testcases/nft-f/0018jump_variable_0 | 19 ++++++++++++++++++
 .../shell/testcases/nft-f/0019jump_variable_1 | 20 +++++++++++++++++++
 .../shell/testcases/nft-f/0020jump_variable_1 | 20 +++++++++++++++++++
 .../nft-f/dumps/0018jump_variable_0.nft       |  8 ++++++++
 7 files changed, 87 insertions(+), 1 deletion(-)
 create mode 100755 tests/shell/testcases/nft-f/0018jump_variable_0
 create mode 100755 tests/shell/testcases/nft-f/0019jump_variable_1
 create mode 100755 tests/shell/testcases/nft-f/0020jump_variable_1
 create mode 100644 tests/shell/testcases/nft-f/dumps/0018jump_variable_0.nft

diff --git a/src/datatype.c b/src/datatype.c
index 10f185b..1d5ed6f 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -309,11 +309,22 @@ static void verdict_type_print(const struct expr *expr, struct output_ctx *octx)
 	}
 }
 
+static struct error_record *verdict_type_parse(const struct expr *sym,
+					       struct expr **res)
+{
+	*res = constant_expr_alloc(&sym->location, &string_type,
+				   BYTEORDER_HOST_ENDIAN,
+				   (strlen(sym->identifier) + 1) * BITS_PER_BYTE,
+				   sym->identifier);
+	return NULL;
+}
+
 const struct datatype verdict_type = {
 	.type		= TYPE_VERDICT,
 	.name		= "verdict",
 	.desc		= "netfilter verdict",
 	.print		= verdict_type_print,
+	.parse		= verdict_type_parse,
 };
 
 static const struct symbol_table nfproto_tbl = {
diff --git a/src/evaluate.c b/src/evaluate.c
index 8394037..55fb3b6 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1950,6 +1950,13 @@ static int stmt_evaluate_verdict(struct eval_ctx *ctx, struct stmt *stmt)
 		if (stmt->expr->chain != NULL) {
 			if (expr_evaluate(ctx, &stmt->expr->chain) < 0)
 				return -1;
+			if ((stmt->expr->chain->etype != EXPR_SYMBOL &&
+			    stmt->expr->chain->etype != EXPR_VALUE) ||
+			    stmt->expr->chain->symtype != SYMBOL_VALUE) {
+				return stmt_error(ctx, stmt,
+						  "invalid verdict chain expression %s\n",
+						  expr_name(stmt->expr->chain));
+			}
 		}
 		break;
 	case EXPR_MAP:
diff --git a/src/parser_bison.y b/src/parser_bison.y
index b1e29a8..0fea3c6 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -3841,7 +3841,8 @@ verdict_expr		:	ACCEPT
 			}
 			;
 
-chain_expr		:	identifier
+chain_expr		:	variable_expr
+			|	identifier
 			{
 				$$ = constant_expr_alloc(&@$, &string_type,
 							 BYTEORDER_HOST_ENDIAN,
diff --git a/tests/shell/testcases/nft-f/0018jump_variable_0 b/tests/shell/testcases/nft-f/0018jump_variable_0
new file mode 100755
index 0000000..003a1bd
--- /dev/null
+++ b/tests/shell/testcases/nft-f/0018jump_variable_0
@@ -0,0 +1,19 @@
+#!/bin/bash
+
+# Tests use of variables in jump statements
+
+set -e
+
+RULESET="
+define dest = ber
+
+table ip foo {
+	chain bar {
+		jump \$dest
+	}
+
+	chain ber {
+	}
+}"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/nft-f/0019jump_variable_1 b/tests/shell/testcases/nft-f/0019jump_variable_1
new file mode 100755
index 0000000..bda861c
--- /dev/null
+++ b/tests/shell/testcases/nft-f/0019jump_variable_1
@@ -0,0 +1,20 @@
+#!/bin/bash
+
+# Tests use of variables in jump statements
+
+set -e
+
+RULESET="
+define dest = { 1024 }
+
+table ip foo {
+	chain bar {
+		jump \$dest
+	}
+
+	chain ber {
+	}
+}"
+
+$NFT -f - <<< "$RULESET" && exit 1
+exit 0
diff --git a/tests/shell/testcases/nft-f/0020jump_variable_1 b/tests/shell/testcases/nft-f/0020jump_variable_1
new file mode 100755
index 0000000..f753058
--- /dev/null
+++ b/tests/shell/testcases/nft-f/0020jump_variable_1
@@ -0,0 +1,20 @@
+#!/bin/bash
+
+# Tests use of variables in jump statements
+
+set -e
+
+RULESET="
+define dest = *
+
+table ip foo {
+	chain bar {
+		jump \$dest
+	}
+
+	chain ber {
+	}
+}"
+
+$NFT -f - <<< "$RULESET" && exit 1
+exit 0
diff --git a/tests/shell/testcases/nft-f/dumps/0018jump_variable_0.nft b/tests/shell/testcases/nft-f/dumps/0018jump_variable_0.nft
new file mode 100644
index 0000000..0ddaf07
--- /dev/null
+++ b/tests/shell/testcases/nft-f/dumps/0018jump_variable_0.nft
@@ -0,0 +1,8 @@
+table ip foo {
+	chain bar {
+		jump ber
+	}
+
+	chain ber {
+	}
+}
-- 
2.20.1

Re: [PATCH nft v4 1/2] jump: Introduce chain_expr in jump and goto statements

[Pablo Neira Ayuso]

On Fri, May 24, 2019 at 03:06:47PM +0200, Fernando Fernandez Mancera wrote:
> Now we can introduce expressions as a chain in jump and goto statements. This
> is going to be used to support variables as a chain in the following patches.

Applied, thanks Fernando.

Re: [PATCH nft v4 2/2] jump: Allow goto and jump to a variable using nft input files

[Pablo Neira Ayuso]

On Fri, May 24, 2019 at 03:06:50PM +0200, Fernando Fernandez Mancera wrote:
> This patch introduces the use of nft input files variables in 'jump' and 'goto'
> statements, e.g.
> 
> define dest = ber
> 
> add table ip foo
> add chain ip foo bar {type filter hook input priority 0;}
> add chain ip foo ber
> add rule ip foo ber counter
> add rule ip foo bar jump $dest
> 
> table ip foo {
>         chain bar {
>                 type filter hook input priority filter; policy accept;
>                 jump ber
>         }
> 
>         chain ber {
>                 counter packets 71 bytes 6664
>         }
> }

Also applied, thanks.