question about UNDEFINE/REDEFINE

question about UNDEFINE/REDEFINE

[David Fabian]

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

Hello,

we have a firewall written in bash (using iptables) that is organized by 
customer VLANs. Each VLAN has its own set of bash variables holding things 
like uplink iface names, gateway IPs, etc. We want to rewrite the firewall to 
nftables but are stuck on the fact that nft variables cannot be overridden in 
the same scope. We have each VLAN configuration in a separate file containing 
pre/post-routing, input, output and forward rules,and we include those files to 
a master firewall configuration. One solution is to rename all the variables 
with some VLAN specific (pre/su)ffix. But that is cumbersome.

I have made a small patch to nft which adds two new keywords - undefine and 
redefine. undefine simply undefines a variable from the current scope. redefine 
allows one to change a variable definition. The patch works against the latest 
fedora nft (version 0.7) but I believe it should work against master as well. 
I don't know how to properly send the patch to the project so I am attaching 
it here. I would like to know your opinion.

-- 
Best regards,

David Fabian
Cluster Design, s.r.o.

[-- Attachment #2: 0001-Added-undefine-redefine-keywords.patch --]
[-- Type: text/x-patch, Size: 2950 bytes --]

>From 43abd3a12670b54739f0a7f6500aa315b3905f08 Mon Sep 17 00:00:00 2001
From: David Fabian <david.fabian@bosson.cz>
Date: Mon, 22 Jan 2018 14:02:11 +0100
Subject: [PATCH] Added undefine/redefine keywords

---
 include/rule.h     |  1 +
 src/parser_bison.y | 23 +++++++++++++++++++++++
 src/rule.c         | 16 ++++++++++++++++
 src/scanner.l      |  2 ++
 4 files changed, 42 insertions(+)

diff --git a/include/rule.h b/include/rule.h
index b9b4a19..4524b4d 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -80,6 +80,7 @@ struct symbol {
 
 extern void symbol_bind(struct scope *scope, const char *identifier,
 			struct expr *expr);
+extern int symbol_unbind(struct scope *scope, const char *identifier);
 extern struct symbol *symbol_lookup(const struct scope *scope,
 				    const char *identifier);
 
diff --git a/src/parser_bison.y b/src/parser_bison.y
index deaaf06..4cc1b47 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -167,6 +167,8 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 
 %token INCLUDE			"include"
 %token DEFINE			"define"
+%token REDEFINE			"redefine"
+%token UNDEFINE			"undefine"
 
 %token FIB			"fib"
 
@@ -661,6 +663,27 @@ common_block		:	INCLUDE		QUOTED_STRING	stmt_seperator
 				symbol_bind(scope, $2, $4);
 				xfree($2);
 			}
+			|       REDEFINE                identifier      '='     initializer_expr        stmt_seperator
+			{
+				struct scope *scope = current_scope(state);
+
+				/* ignore missing identifier */
+				symbol_unbind(scope, $2);
+				symbol_bind(scope, $2, $4);
+				xfree($2);
+			}
+			|       UNDEFINE                identifier      stmt_seperator
+			{
+				struct scope *scope = current_scope(state);
+
+				if (symbol_unbind(scope, $2) < 0) {
+					erec_queue(error(&@2, "undefined symbol '%s'", $2),
+						   state->msgs);
+					YYERROR;
+			}
+
+				xfree($2);
+			}
 			|	error		stmt_seperator
 			{
 				if (++state->nerrs == max_errors)
diff --git a/src/rule.c b/src/rule.c
index f1bb6cf..f97c8e5 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -447,6 +447,22 @@ void symbol_bind(struct scope *scope, const char *identifier, struct expr *expr)
 	list_add_tail(&sym->list, &scope->symbols);
 }
 
+int symbol_unbind(struct scope *scope, const char *identifier)
+{
+	struct symbol *sym;
+
+	if ((sym = symbol_lookup(scope, identifier)) == NULL)
+	{
+		return -1;
+	}
+	list_del(&sym->list);
+	xfree(sym->identifier);
+	expr_free(sym->expr);
+	xfree(sym);
+	return 0;
+}
+
+
 struct symbol *symbol_lookup(const struct scope *scope, const char *identifier)
 {
 	struct symbol *sym;
diff --git a/src/scanner.l b/src/scanner.l
index 625023f..2000554 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -231,6 +231,8 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 
 "include"		{ return INCLUDE; }
 "define"		{ return DEFINE; }
+"redefine"		{ return REDEFINE; }
+"undefine"		{ return UNDEFINE; }
 
 "describe"		{ return DESCRIBE; }
 
-- 
2.14.3

Re: question about UNDEFINE/REDEFINE

[Pablo Neira Ayuso]

Hi David,

On Mon, Jan 22, 2018 at 02:53:09PM +0100, David Fabian wrote:
> Hello,
> 
> we have a firewall written in bash (using iptables) that is organized by 
> customer VLANs. Each VLAN has its own set of bash variables holding things 
> like uplink iface names, gateway IPs, etc. We want to rewrite the firewall to 
> nftables but are stuck on the fact that nft variables cannot be overridden in 
> the same scope. We have each VLAN configuration in a separate file containing 
> pre/post-routing, input, output and forward rules,and we include those files to 
> a master firewall configuration. One solution is to rename all the variables 
> with some VLAN specific (pre/su)ffix. But that is cumbersome.
> 
> I have made a small patch to nft which adds two new keywords - undefine and 
> redefine. undefine simply undefines a variable from the current scope. redefine 
> allows one to change a variable definition. The patch works against the latest 
> fedora nft (version 0.7) but I believe it should work against master as well. 
> I don't know how to properly send the patch to the project so I am attaching 
> it here. I would like to know your opinion.

Thanks for sending us this patch.

Question here: If we allow to pass variable definitions via -D option
from the command line, would that work for you too?

I'm asking here because I would need to understand better how you've
structured your scripts, if you could explain a bit more, we would
appreciate.

Thanks!

Re: question about UNDEFINE/REDEFINE

[David Fabian]

Hello Pablo,

Dne úterý 23. ledna 2018 12:07:28 CET, Pablo Neira Ayuso napsal(a):
> I'm asking here because I would need to understand better how you've
> structured your scripts, if you could explain a bit more, we would
> appreciate.

I have packed an excerpt of a playground FW with two VLANs 3 and 54. The 
configuration already uses my redefine keyword.

ftp://ftp.bosson.eu/pub/tmp/nftables_excerpt.tar.gz

The intended use case is to call nft -f fw-on and reload the firewall from 
scratch every time there is a config change. I don't know how a cmdline 
parameter would help us with it. Maybe if we would wrap nft calls with bash 
scripts but that would defeat the purpose of using the nft scripting 
capabilities in the first place.

The most important for us is to have the FW logically structured for every 
customer and every FW rule related to a customer should be in his/her VLAN 
config file.

-- 
Best regards,

David Fabian
Cluster Design, s.r.o.

Re: question about UNDEFINE/REDEFINE

[Pablo Neira Ayuso]

Hi David,

On Tue, Jan 23, 2018 at 01:40:03PM +0100, David Fabian wrote:
> Hello Pablo,
> 
> Dne úterý 23. ledna 2018 12:07:28 CET, Pablo Neira Ayuso napsal(a):
> > I'm asking here because I would need to understand better how you've
> > structured your scripts, if you could explain a bit more, we would
> > appreciate.
> 
> I have packed an excerpt of a playground FW with two VLANs 3 and 54. The 
> configuration already uses my redefine keyword.
> 
> ftp://ftp.bosson.eu/pub/tmp/nftables_excerpt.tar.gz

Thanks for this example ruleset.

> The intended use case is to call nft -f fw-on and reload the firewall from 
> scratch every time there is a config change. I don't know how a cmdline 
> parameter would help us with it.

Yes, command line won't help in this case.

> Maybe if we would wrap nft calls with bash scripts but that would
> defeat the purpose of using the nft scripting capabilities in the
> first place.

No bash, we don't want more shell scripts in place, the goal is indeed
to add scripting capabilities to nftables.

> The most important for us is to have the FW logically structured for every 
> customer and every FW rule related to a customer should be in his/her VLAN 
> config file.

Variables are bound to their scope, so the following example works
fine:

 # cat example.nft
 table ip xxx {
        define IP1 = 1.1.1.1
 }

 table ip yyy {
        define IP1 = 2.2.2.2
 }

Given IP1 is bound to the table definitions.

In your ruleset, you're using the flat ruleset notation, ie.

 add rule x y ip saddr $IP1 counter drop

I see another
three choices, apart from your 'redefine' proposal:

1) Add some explicit scoping, so you can do this, eg.

 begin-scope fw-vlan1
 define IP1 = 1.1.1.1

 add rule filter INPUT x y ip saddr $IP1 counter drop
 end-scope

 begin-scope fw-vlan32
 define IP1 = 2.2.2.2
 ...
 add rule filter INPUT x y ip saddr $IP1 counter drop
 end-scope

Hm, but this doesn't look very nice...

2) Probably even cleaner is to look at 'local' scopes like in bash.

 define local IP1 = 1.1.1.1

so the symbol is bound to this file - consider the content of a file
determines a given scope. This can be also useful to the nested
notation.

3) You rework your ruleset to use the notation with nesting :-). But I
think 2) can be useful for both the flat and nested notation.

I'm not asking you to do 2), but I would like to see how a patch that
adds explicit scoping for the flat ruleset representation looks like.

Re: question about UNDEFINE/REDEFINE

[Pablo Neira Ayuso]

On Fri, Jan 26, 2018 at 02:45:49PM +0100, Pablo Neira Ayuso wrote:
[...]
> 2) Probably even cleaner is to look at 'local' scopes like in bash.
> 
>  define local IP1 = 1.1.1.1
> 
> so the symbol is bound to this file - consider the content of a file
> determines a given scope. This can be also useful to the nested
> notation.

Actually, this would have different semantics to what bash offers,
since local variable are bound to the scope.

In the flat notation, there's one single scope for all of your
ruleset.

Re: question about UNDEFINE/REDEFINE

[Arturo Borrero Gonzalez]

On 23 January 2018 at 04:40, David Fabian <david.fabian@cldn.cz> wrote:
> Hello Pablo,
>
> Dne úterý 23. ledna 2018 12:07:28 CET, Pablo Neira Ayuso napsal(a):
>> I'm asking here because I would need to understand better how you've
>> structured your scripts, if you could explain a bit more, we would
>> appreciate.
>
> I have packed an excerpt of a playground FW with two VLANs 3 and 54. The
> configuration already uses my redefine keyword.
>
> ftp://ftp.bosson.eu/pub/tmp/nftables_excerpt.tar.gz
>
> The intended use case is to call nft -f fw-on and reload the firewall from
> scratch every time there is a config change. I don't know how a cmdline
> parameter would help us with it. Maybe if we would wrap nft calls with bash
> scripts but that would defeat the purpose of using the nft scripting
> capabilities in the first place.
>
> The most important for us is to have the FW logically structured for every
> customer and every FW rule related to a customer should be in his/her VLAN
> config file.
>

Your approach (redefining variables) doesn't save so much typing at
the end of the day.

My suggestion is to simply create one variable per value:

define INET_IFACES_VLAN43 = { bond0.x, bond3.y}
define INET_IFACES_VLAN3 = { bond3.x, bond3.y}
define XXX_VLAN43 = xxx
define XXX_VLAN3 = xxx

you could generate such a file, something like 'defines.nft' and
include it once in your main ruleset file.

If you will perform many updates to this file, you could even maintain
this in git to keep track of changes.
Example: https://wiki.nftables.org/wiki-nftables/index.php/Classic_perimetral_firewall_example

Other option is you create some kind of shell wrapper to replace
variable names before running nft -f (something like make .in files),
but that's even uglier? I don't know.

Re: question about UNDEFINE/REDEFINE

[David Fabian]

Hello Pablo,

Dne pátek 26. ledna 2018 14:45:49 CET, Pablo Neira Ayuso napsal(a):
> 2) Probably even cleaner is to look at 'local' scopes like in bash.
> 
>  define local IP1 = 1.1.1.1
> 
> so the symbol is bound to this file - consider the content of a file
> determines a given scope. This can be also useful to the nested
> notation.
> 
> 3) You rework your ruleset to use the notation with nesting :-). But I
> think 2) can be useful for both the flat and nested notation.
> 
> I'm not asking you to do 2), but I would like to see how a patch that
> adds explicit scoping for the flat ruleset representation looks like.

I know about scopes in the code but unfortunately as you said, the flat 
notation only has a single scope. Since we are talking about analogy to bash, 
bash allows me to redefine a variable in the same scope. Variables in nftables 
feel more like constants which is not necessarily bad as it can prevent some 
typos but is hard to work with in scripting as it's not that flexible.

From those options you listed I would strongly prefer to have an implicit 
scope for each file included in the flat notation. That way, defining a variable 
in one file would not collide with the same variable in a sibling include. 
Variables from outer scopes would still be available in inner scopes. For 
people that would want to have their "global" definitions in a separate 
include, I would recommend creating a new keyword like global or export that 
would tie a variable to the top-level scope and thus make it available to 
everyone. I don't think that would be that hard to implement and I may try to 
if we agree on it.

Anyway there should definitely be a way to de-register (undefine) a variable 
from a scope to prevent a misuse due to typos.

By the way, can we restructure the FW using nesting and still be able to 
retain all per-customer rules in a single file? Wouldn't that require us to 
split prerouting, postrouting, forward and other rules to separate scopes/
table definitions? That would be highly inconvenient.

-- 
S pozdravem,

David Fabian
Cluster Design, s.r.o.

Re: question about UNDEFINE/REDEFINE

[David Fabian]

Hello Arturo,

Dne pátek 26. ledna 2018 19:43:18 CET, Arturo Borrero Gonzalez napsal(a):
> My suggestion is to simply create one variable per value:
> 
> define INET_IFACES_VLAN43 = { bond0.x, bond3.y}
> define INET_IFACES_VLAN3 = { bond3.x, bond3.y}
> define XXX_VLAN43 = xxx
> define XXX_VLAN3 = xxx
> 
> you could generate such a file, something like 'defines.nft' and
> include it once in your main ruleset file.

that is exactly the boilerplate that we are trying to avoid. By using 
consistent (and non-unique) variable names we are able to freely move the 
rules from one customer to another without rewriting every use of a variable 
every time. We also do not want to build a code-generating harness in bash (or 
any other language) since that would sort of defeat the purpose of scripting 
in nftables in my eyes. the redefine keyword was just my first idea to solve the 
problem of a single flat variable scope. There may be a better approach but I 
think that if nftables wants to have scripting capabilities, some kind of 
variable scoping (even in flat notation) and more ubiquitous variable use 
within rules is necessary.

I event went so far and made some experimental patches that allowed me to use 
string variables and string concatenation in places like interface names and 
rule targets. With that I was able to create very generic rules and I tied 
them to a customer/VLAN just by changing one or two constants in the header of 
a file (e.g. the VLAN number). Of course, I had to use redefine in the header.

-- 
S pozdravem,

David Fabian
Cluster Design, s.r.o.

Re: question about UNDEFINE/REDEFINE

[David Fabian]

Hello Pablo,

what do you think about this proposal?

-- 
S pozdravem,

David Fabian
Cluster Design, s.r.o.

Dne úterý 30. ledna 2018 12:05:48 CET, David Fabian napsal(a):
> Hello Pablo,
> 
> Dne pátek 26. ledna 2018 14:45:49 CET, Pablo Neira Ayuso napsal(a):
> > 2) Probably even cleaner is to look at 'local' scopes like in bash.
> > 
> >  define local IP1 = 1.1.1.1
> > 
> > so the symbol is bound to this file - consider the content of a file
> > determines a given scope. This can be also useful to the nested
> > notation.
> > 
> > 3) You rework your ruleset to use the notation with nesting :-). But I
> > think 2) can be useful for both the flat and nested notation.
> > 
> > I'm not asking you to do 2), but I would like to see how a patch that
> > adds explicit scoping for the flat ruleset representation looks like.
> 
> I know about scopes in the code but unfortunately as you said, the flat
> notation only has a single scope. Since we are talking about analogy to
> bash, bash allows me to redefine a variable in the same scope. Variables in
> nftables feel more like constants which is not necessarily bad as it can
> prevent some typos but is hard to work with in scripting as it's not that
> flexible.
> 
> From those options you listed I would strongly prefer to have an implicit
> scope for each file included in the flat notation. That way, defining a
> variable in one file would not collide with the same variable in a sibling
> include. Variables from outer scopes would still be available in inner
> scopes. For people that would want to have their "global" definitions in a
> separate include, I would recommend creating a new keyword like global or
> export that would tie a variable to the top-level scope and thus make it
> available to everyone. I don't think that would be that hard to implement
> and I may try to if we agree on it.
> 
> Anyway there should definitely be a way to de-register (undefine) a variable
> from a scope to prevent a misuse due to typos.
> 
> By the way, can we restructure the FW using nesting and still be able to
> retain all per-customer rules in a single file? Wouldn't that require us to
> split prerouting, postrouting, forward and other rules to separate scopes/
> table definitions? That would be highly inconvenient.