| Message ID | 3622208.jy4NlOniyd@voxel |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | question about UNDEFINE/REDEFINE | expand |
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! -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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.
Hello Pablo, what do you think about this proposal?
On Tue, Feb 13, 2018 at 12:52:49PM +0100, David Fabian wrote: > Hello Pablo, > > what do you think about this proposal? Applied, thanks. It would be great if you could send us a patch to update documentation and add some tests to tests/shell/ folder too. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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