Hi,
> [...]
>
> Sorry, but I don't agree with the following reasons. Let me briefly explain
> why.
>
> All files under '/usr/share/firewall4/includes.d' are uci files. I can see
> all relevant options.
One problem with your suggested implementation is that you introduce a generic
directory (/usr/share/firewall4/includes.d) containing uci partials that do
look like a full /etc/config/firewall but actually ignore all section types
except `config include` ones, that will lead to confusion.
> I can set in the includes my own path. This is
> relevant for packages that updates the ruleset on events. If I do not want
> to be this rules persistent saved I could use a tmp file in the system
> (strongswan).
Using my proposal you could achieve the same by placing a symlink to a
temporary file path
> Also the new include add by you already does have the state file feature.
> Which is relevant on reloading the ruleset.
Not sure what you mean with that.
> In addition, it would make the ruleset.uc file confusing if I added the
> same feature again.
The ruleset.uc file wouldn't change at all. The implementation would be
confined to fw4.uc, like your approach.
> Also, I would have to add two sections on include to support temporary
> rules, which I would not have to store under
> '/usr/share/firewall4/includes.d/' but under '/tmp/firewall4/includes.d/'
> for example to support the not persistent feature.
ln -s /tmp/strongswan/nftables.d/pre-input.nft \
/usr/share/nftables.d/chain-pre/input/10_strongswan.nft
ln -s /tmp/strongswan/nftables.d/pre-output.nft \
/usr/share/nftables.d/chain-pre/output/10_strongswan.nft
ln -s /tmp/strongswan/nftables.d/pre-forward.nft \
/usr/share/nftables.d/chain-pre/forward/10_strongswan.nft
> We also use the include to add the helpers.
Can you elaborate?
> Last but not leased, if we now add an other possibility, then I don't
> think anyone knows where which file adds which rule!
The same applies to /usr/share/firewall4/includes.d/ as well. You do need to
be aware of it to know that includes can stem from there.
> From my point of view, it makes sense to use the include function from you
> with my extension. So I think the include feature is the better and
> unified solution.
What I do not like about it are several facts:
- It introduces uci in a non-standard location
- It implies configurability where there isn't any (it is package managed
resource files, like init scripts. Technically you can edit init scripts
but they're certainly not configuration but package integration code).
- It introduces overhead; you do have to read and parse a uci file to extract
a path from it which you then write into the ruleset so that nftables can
include it
- It creates the false impression of being a general-purpose uci partial
solution to be able to modularize /etc/config/firewall which it isn't since
it'll silently ignore any non-include section type being put there
I have attached my proposal as patch for reference.
Kind regards,
Jo
From 5ab0f61350f02590c5e6c1981bce4531510517de Mon Sep 17 00:00:00 2001
From: Jo-Philipp Wich
Date: Thu, 11 Aug 2022 13:48:14 +0200
Subject: [PATCH] fw4: support automatic includes
Introduce a new directory tree /usr/share/nftables/includes.d/ which may
contain partial nftables files being included into the rendered ruleset.
The include position is derived from the file path;
- Files in .../includes.d/table-pre/ and .../includes.d/table-post/ are
included before and after the `table inet fw4 { ... }` declaration
respectively
- Files in .../includes.d/ruleset-pre/ and .../includes.d/ruleset-post/
are included before the first chain and after the last chain
declaration within the fw4 table respectively
- Files in .../includes.d/chain-pre/${chain}/ and .../chain-post/${chain}/
are included before the first and after the last rule within the mentioned
chain of the fw4 table respectively
Automatic includes can be disabled by setting the `auto_includes` option to
`0` in the global defaults section.
Signed-off-by: Jo-Philipp Wich
---
root/usr/share/ucode/fw4.uc | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/root/usr/share/ucode/fw4.uc b/root/usr/share/ucode/fw4.uc
index 2dc44ac..86e3331 100644
--- a/root/usr/share/ucode/fw4.uc
+++ b/root/usr/share/ucode/fw4.uc
@@ -733,6 +733,19 @@ return {
this.cursor.foreach("firewall", "include", i => self.parse_include(i));
+ //
+ // Discover automatic includes
+ //
+
+ if (this.default_option("auto_includes")) {
+ for (let position in [ 'ruleset-pre', 'ruleset-post', 'table-pre', 'table-post', 'chain-pre', 'chain-post' ])
+for (let chain in (position in [ 'chain-pre', 'chain-post' ]) ? fs.lsdir(`/usr/share/nftables/includes.d/${position}`) : [ null ])
+ for (let path in fs.glob(`/usr/share/nftables/includes.d/${position}/${chain ?? ''}/*.nft`))
+ if (fs.access(path))
+ this.parse_include({
