[PATCH] src: Add option '-D' to define variables from command-line

[PATCH] src: Add option '-D' to define variables from command-line

[Harsha Sharma]

This patch adds option '-D' with optarg in form test="foo" to define
variable to be referenced from input file.

For eg.
nft -D test="foo" -f /tmp/test1

Signed-off-by: Harsha Sharma <harshasharmaiitr@gmail.com>
---
This patch passes identifier and its value as nft_ctx struct members
and binds variable with symbol_bind and when symbol_lookup is called
while parsing, then comparison returns true, but this patch causes 
segmentation fault.

 include/nftables.h          |  6 ++++++
 include/nftables/nftables.h |  2 ++
 include/parser.h            |  3 ++-
 src/libnftables.c           | 11 +++++++++--
 src/main.c                  | 18 +++++++++++++++++-
 src/parser_bison.y          |  7 ++++++-
 6 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/include/nftables.h b/include/nftables.h
index 3bfa33e..50b8102 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -21,6 +21,11 @@ struct nft_cache {
 	uint32_t		seqnum;
 };
 
+struct nft_variable {
+	const char 		*identifier;
+	const char 		*expr_value;
+};
+
 struct mnl_socket;
 
 struct nft_ctx {
@@ -32,6 +37,7 @@ struct nft_ctx {
 	struct output_ctx	output;
 	bool			check;
 	struct nft_cache	cache;
+	struct nft_variable	variable;
 	uint32_t		flags;
 };
 
diff --git a/include/nftables/nftables.h b/include/nftables/nftables.h
index 8e59f2b..a06a202 100644
--- a/include/nftables/nftables.h
+++ b/include/nftables/nftables.h
@@ -55,6 +55,8 @@ bool nft_ctx_output_get_handle(struct nft_ctx *ctx);
 void nft_ctx_output_set_handle(struct nft_ctx *ctx, bool val);
 bool nft_ctx_output_get_echo(struct nft_ctx *ctx);
 void nft_ctx_output_set_echo(struct nft_ctx *ctx, bool val);
+void nft_ctx_output_set_variable(struct nft_ctx *ctx, char *identifier,
+				 char *expr_value);
 
 FILE *nft_ctx_set_output(struct nft_ctx *ctx, FILE *fp);
 int nft_ctx_add_include_path(struct nft_ctx *ctx, const char *path);
diff --git a/include/parser.h b/include/parser.h
index 0bdb3fa..46981fe 100644
--- a/include/parser.h
+++ b/include/parser.h
@@ -33,7 +33,8 @@ struct mnl_socket;
 
 extern void parser_init(struct mnl_socket *nf_sock, struct nft_cache *cache,
 			struct parser_state *state, struct list_head *msgs,
-			unsigned int debug_level, struct output_ctx *octx);
+			unsigned int debug_level, struct output_ctx *octx,
+			struct nft_variable *variable);
 extern int nft_parse(struct nft_ctx *ctx, void *, struct parser_state *state);
 
 extern void *scanner_init(struct parser_state *state);
diff --git a/src/libnftables.c b/src/libnftables.c
index c86d894..b34cea3 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -269,6 +269,13 @@ void nft_ctx_output_set_echo(struct nft_ctx *ctx, bool val)
 	ctx->output.echo = val;
 }
 
+void nft_ctx_output_set_variable(struct nft_ctx *ctx, char *identifier,
+				 char *expr_value)
+{
+	ctx->variable.identifier = identifier;
+	ctx->variable.expr_value = expr_value;
+}
+
 static const struct input_descriptor indesc_cmdline = {
 	.type	= INDESC_BUFFER,
 	.name	= "<cmdline>",
@@ -283,7 +290,7 @@ int nft_run_cmd_from_buffer(struct nft_ctx *nft, char *buf, size_t buflen)
 	FILE *fp;
 
 	parser_init(nft->nf_sock, &nft->cache, &state,
-		    &msgs, nft->debug_mask, &nft->output);
+		    &msgs, nft->debug_mask, &nft->output, &nft->variable);
 	scanner = scanner_init(&state);
 	scanner_push_buffer(scanner, &indesc_cmdline, buf);
 
@@ -313,7 +320,7 @@ int nft_run_cmd_from_filename(struct nft_ctx *nft, const char *filename)
 		return -1;
 
 	parser_init(nft->nf_sock, &nft->cache, &state,
-		    &msgs, nft->debug_mask, &nft->output);
+		    &msgs, nft->debug_mask, &nft->output, &nft->variable);
 	scanner = scanner_init(&state);
 	if (scanner_read_file(scanner, filename, &internal_location) < 0) {
 		rc = -1;
diff --git a/src/main.c b/src/main.c
index 353b87b..d9402cf 100644
--- a/src/main.c
+++ b/src/main.c
@@ -37,10 +37,11 @@ enum opt_vals {
 	OPT_DEBUG		= 'd',
 	OPT_HANDLE_OUTPUT	= 'a',
 	OPT_ECHO		= 'e',
+	OPT_VARIABLE		= 'D',
 	OPT_INVALID		= '?',
 };
 
-#define OPTSTRING	"hvcf:iI:vnsNae"
+#define OPTSTRING	"hvcf:iI:vnsNaeD:"
 
 static const struct option options[] = {
 	{
@@ -95,6 +96,11 @@ static const struct option options[] = {
 		.val		= OPT_ECHO,
 	},
 	{
+		.name		= "variable",
+		.val		= OPT_VARIABLE,
+		.has_arg	= 1,
+	},
+	{
 		.name		= NULL
 	}
 };
@@ -119,6 +125,7 @@ static void show_help(const char *name)
 "  -N				Translate IP addresses to names.\n"
 "  -a, --handle			Output rule handle.\n"
 "  -e, --echo			Echo what has been added, inserted or replaced.\n"
+"  -D, --define 		Define variable names from command line for using in nft input file.\n"
 "  -I, --includepath <directory>	Add <directory> to the paths searched for include files. Default is: %s\n"
 "  --debug <level [,level...]>	Specify debugging level (scanner, parser, eval, netlink, mnl, proto-ctx, segtree, all)\n"
 "\n",
@@ -166,6 +173,7 @@ static const struct {
 int main(int argc, char * const *argv)
 {
 	char *buf = NULL, *filename = NULL;
+	char *identifier = NULL, *expr_value = NULL;
 	enum nft_numeric_level numeric;
 	bool interactive = false;
 	unsigned int debug_mask;
@@ -255,6 +263,14 @@ int main(int argc, char * const *argv)
 		case OPT_ECHO:
 			nft_ctx_output_set_echo(nft, true);
 			break;
+		case OPT_VARIABLE:
+			{
+				const char *search = "=";
+				identifier = strtok(optarg, search);
+				expr_value = strtok(NULL, search);
+				nft_ctx_output_set_variable(nft, identifier, expr_value);
+				break;
+			}
 		case OPT_INVALID:
 			exit(EXIT_FAILURE);
 		}
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 6e85a62..f382add 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -37,7 +37,8 @@
 
 void parser_init(struct mnl_socket *nf_sock, struct nft_cache *cache,
 		 struct parser_state *state, struct list_head *msgs,
-		 unsigned int debug_mask, struct output_ctx *octx)
+		 unsigned int debug_mask, struct output_ctx *octx,
+		 struct nft_variable *variable)
 {
 	memset(state, 0, sizeof(*state));
 	init_list_head(&state->cmds);
@@ -49,6 +50,10 @@ void parser_init(struct mnl_socket *nf_sock, struct nft_cache *cache,
 	state->ectx.nf_sock = nf_sock;
 	state->ectx.debug_mask = debug_mask;
 	state->ectx.octx = octx;
+	if (variable->identifier != NULL) {
+		struct expr *expr = (struct expr *) variable->expr_value;
+		symbol_bind(state->scopes[state->scope], variable->identifier, expr);
+	}
 }
 
 static void yyerror(struct location *loc, struct nft_ctx *nft, void *scanner,
-- 
2.11.0

Re: [PATCH] src: Add option '-D' to define variables from command-line

[Pablo Neira Ayuso]

On Mon, Dec 04, 2017 at 04:22:59PM +0530, Harsha Sharma wrote:
> This patch adds option '-D' with optarg in form test="foo" to define
> variable to be referenced from input file.
> 
> For eg.
> nft -D test="foo" -f /tmp/test1

Could you generalize this to take comma separated list of variables,
ie.

nft -D test="foo",test2="bar" -f /tmp/test1

> Signed-off-by: Harsha Sharma <harshasharmaiitr@gmail.com>
> ---
> This patch passes identifier and its value as nft_ctx struct members
> and binds variable with symbol_bind and when symbol_lookup is called
> while parsing, then comparison returns true, but this patch causes 
> segmentation fault.

Valgrind can help you know where the crash is going on.

# cat /tmp/x
add table x
add chain x y
add rule x y ip saddr $test

# valgrind nft -D test="1.1.1.1" -f /tmp/x
[...]
==23100== Invalid read of size 8
==23100==    at 0x40E31A: expr_clone (expression.c:52)
==23100==    by 0x439826: expr_evaluate_symbol (evaluate.c:191)
==23100==    by 0x439826: expr_evaluate (evaluate.c:1790)
==23100==    by 0x43855D: expr_evaluate_relational (evaluate.c:1569)
==23100==    by 0x43855D: expr_evaluate (evaluate.c:1830)
==23100==    by 0x436F93: stmt_evaluate_expr (evaluate.c:1843)
==23100==    by 0x436F93: stmt_evaluate (evaluate.c:2780)
==23100==    by 0x43B20E: rule_evaluate (evaluate.c:2915)
==23100==    by 0x42FD44: nft_parse (parser_bison.y:727)
==23100==    by 0x40768D: nft_run (libnftables.c:86)
==23100==    by 0x407DF8: nft_run_cmd_from_filename
(libnftables.c:330)
==23100==    by 0x406FC5: main (main.c:292)

> @@ -49,6 +50,10 @@ void parser_init(struct mnl_socket *nf_sock, struct nft_cache *cache,
>  	state->ectx.nf_sock = nf_sock;
>  	state->ectx.debug_mask = debug_mask;
>  	state->ectx.octx = octx;
> +	if (variable->identifier != NULL) {
> +		struct expr *expr = (struct expr *) variable->expr_value;

This casting is not correct. You need to allocate a symbol expression, eg.

        $$ = symbol_expr_alloc(&@$, SYMBOL_DEFINE, scope, $2);

> +		symbol_bind(state->scopes[state->scope], variable->identifier, expr);
> +	}