[nft PATCH 1/2] files: Drop shebangs from config files

[nft PATCH 1/2] files: Drop shebangs from config files

[Phil Sutter]

These are not meant to be executed as is but instead loaded via
'nft -f' - all-in-one.nft even points this out in header comment.
While being at it, drop two spelling mistakes found along the way.

Consequently remove executable bits - being registered in automake as
dist_pkgsysconf_DATA, they're changed to 644 upon installation anyway.

Also there is obviously no need for replacement of nft binary path
anymore, drop that bit from Makefile.am.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 files/nftables/Makefile.am        | 3 ---
 files/nftables/all-in-one.nft     | 4 +---
 files/nftables/arp-filter.nft     | 2 --
 files/nftables/bridge-filter.nft  | 2 --
 files/nftables/inet-filter.nft    | 2 --
 files/nftables/inet-nat.nft       | 2 --
 files/nftables/ipv4-filter.nft    | 2 --
 files/nftables/ipv4-mangle.nft    | 2 --
 files/nftables/ipv4-nat.nft       | 2 --
 files/nftables/ipv4-raw.nft       | 2 --
 files/nftables/ipv6-filter.nft    | 2 --
 files/nftables/ipv6-mangle.nft    | 2 --
 files/nftables/ipv6-nat.nft       | 2 --
 files/nftables/ipv6-raw.nft       | 2 --
 files/nftables/netdev-ingress.nft | 4 +---
 15 files changed, 2 insertions(+), 33 deletions(-)
 mode change 100755 => 100644 files/nftables/all-in-one.nft
 mode change 100755 => 100644 files/nftables/arp-filter.nft
 mode change 100755 => 100644 files/nftables/bridge-filter.nft
 mode change 100755 => 100644 files/nftables/inet-filter.nft
 mode change 100755 => 100644 files/nftables/inet-nat.nft
 mode change 100755 => 100644 files/nftables/ipv4-filter.nft
 mode change 100755 => 100644 files/nftables/ipv4-mangle.nft
 mode change 100755 => 100644 files/nftables/ipv4-nat.nft
 mode change 100755 => 100644 files/nftables/ipv4-raw.nft
 mode change 100755 => 100644 files/nftables/ipv6-filter.nft
 mode change 100755 => 100644 files/nftables/ipv6-mangle.nft
 mode change 100755 => 100644 files/nftables/ipv6-nat.nft
 mode change 100755 => 100644 files/nftables/ipv6-raw.nft
 mode change 100755 => 100644 files/nftables/netdev-ingress.nft

diff --git a/files/nftables/Makefile.am b/files/nftables/Makefile.am
index 2a511cd1729c1..fc8b94eac1d01 100644
--- a/files/nftables/Makefile.am
+++ b/files/nftables/Makefile.am
@@ -13,6 +13,3 @@ dist_pkgsysconf_DATA =	all-in-one.nft		\
 			ipv6-nat.nft		\
 			ipv6-raw.nft		\
 			netdev-ingress.nft
-
-install-data-hook:
-	${SED} -i 's|@sbindir[@]|${sbindir}/|g' ${DESTDIR}${pkgsysconfdir}/*.nft
diff --git a/files/nftables/all-in-one.nft b/files/nftables/all-in-one.nft
old mode 100755
new mode 100644
index d3aa7f37f29f1..15ac22e2d676f
--- a/files/nftables/all-in-one.nft
+++ b/files/nftables/all-in-one.nft
@@ -1,12 +1,10 @@
-#!@sbindir@nft -f
-
 # Here is an example of different families, hooks and priorities in the
 # nftables framework, all mixed together.
 #
 # more examples are located in files/examples in nftables source.
 # For up-to-date information please visit https://wiki.nftables.org
 #
-# This script is mean to be loaded with `nft -f <file>`
+# This script is meant to be loaded with `nft -f <file>`
 
 # clear all prior state
 flush ruleset
diff --git a/files/nftables/arp-filter.nft b/files/nftables/arp-filter.nft
old mode 100755
new mode 100644
index 8a350b1eba8aa..6e4c62489ba9c
--- a/files/nftables/arp-filter.nft
+++ b/files/nftables/arp-filter.nft
@@ -1,5 +1,3 @@
-#!@sbindir@nft -f
-
 table arp filter {
 	chain input		{ type filter hook input priority 0; }
 	chain output		{ type filter hook output priority 0; }
diff --git a/files/nftables/bridge-filter.nft b/files/nftables/bridge-filter.nft
old mode 100755
new mode 100644
index 93efe86423011..f071205e3d0fa
--- a/files/nftables/bridge-filter.nft
+++ b/files/nftables/bridge-filter.nft
@@ -1,5 +1,3 @@
-#!@sbindir@nft -f
-
 table bridge filter {
 	chain input		{ type filter hook input priority -200; }
 	chain forward		{ type filter hook forward priority -200; }
diff --git a/files/nftables/inet-filter.nft b/files/nftables/inet-filter.nft
old mode 100755
new mode 100644
index 7be447fd4df5f..bfe43b4fade7c
--- a/files/nftables/inet-filter.nft
+++ b/files/nftables/inet-filter.nft
@@ -1,5 +1,3 @@
-#!@sbindir@nft -f
-
 table inet filter {
 	chain input		{ type filter hook input priority 0; }
 	chain forward		{ type filter hook forward priority 0; }
diff --git a/files/nftables/inet-nat.nft b/files/nftables/inet-nat.nft
old mode 100755
new mode 100644
index 52fcdb543ddab..babd7f00de32a
--- a/files/nftables/inet-nat.nft
+++ b/files/nftables/inet-nat.nft
@@ -1,5 +1,3 @@
-#!@sbindir@nft -f
-
 table inet nat {
 	chain prerouting	{ type nat hook prerouting priority -100; }
 	chain input		{ type nat hook input priority 100; }
diff --git a/files/nftables/ipv4-filter.nft b/files/nftables/ipv4-filter.nft
old mode 100755
new mode 100644
index 51c060f62cf42..ab62024f2cc67
--- a/files/nftables/ipv4-filter.nft
+++ b/files/nftables/ipv4-filter.nft
@@ -1,5 +1,3 @@
-#!@sbindir@nft -f
-
 table filter {
 	chain input		{ type filter hook input priority 0; }
 	chain forward		{ type filter hook forward priority 0; }
diff --git a/files/nftables/ipv4-mangle.nft b/files/nftables/ipv4-mangle.nft
old mode 100755
new mode 100644
index dba8888c06adf..07da5bd90b12c
--- a/files/nftables/ipv4-mangle.nft
+++ b/files/nftables/ipv4-mangle.nft
@@ -1,5 +1,3 @@
-#!@sbindir@nft -f
-
 table mangle {
 	chain output		{ type route hook output priority -150; }
 }
diff --git a/files/nftables/ipv4-nat.nft b/files/nftables/ipv4-nat.nft
old mode 100755
new mode 100644
index 6754e5eede6a5..2c9ce7c5b7f4f
--- a/files/nftables/ipv4-nat.nft
+++ b/files/nftables/ipv4-nat.nft
@@ -1,5 +1,3 @@
-#!@sbindir@nft -f
-
 table nat {
 	chain prerouting	{ type nat hook prerouting priority -100; }
 	chain input		{ type nat hook input priority 100; }
diff --git a/files/nftables/ipv4-raw.nft b/files/nftables/ipv4-raw.nft
old mode 100755
new mode 100644
index c3fed1919cfba..2318e8758bfa8
--- a/files/nftables/ipv4-raw.nft
+++ b/files/nftables/ipv4-raw.nft
@@ -1,5 +1,3 @@
-#!@sbindir@nft -f
-
 table raw {
 	chain prerouting	{ type filter hook prerouting priority -300; }
 	chain output		{ type filter hook output priority -300; }
diff --git a/files/nftables/ipv6-filter.nft b/files/nftables/ipv6-filter.nft
old mode 100755
new mode 100644
index 266bed365671b..383d075d9ae24
--- a/files/nftables/ipv6-filter.nft
+++ b/files/nftables/ipv6-filter.nft
@@ -1,5 +1,3 @@
-#!@sbindir@nft -f
-
 table ip6 filter {
 	chain input		{ type filter hook input priority 0; }
 	chain forward		{ type filter hook forward priority 0; }
diff --git a/files/nftables/ipv6-mangle.nft b/files/nftables/ipv6-mangle.nft
old mode 100755
new mode 100644
index 6b3e20dcd458a..88c51e5247dbb
--- a/files/nftables/ipv6-mangle.nft
+++ b/files/nftables/ipv6-mangle.nft
@@ -1,5 +1,3 @@
-#!@sbindir@nft -f
-
 table ip6 mangle {
 	chain output		{ type route hook output priority -150; }
 }
diff --git a/files/nftables/ipv6-nat.nft b/files/nftables/ipv6-nat.nft
old mode 100755
new mode 100644
index ce0391df24756..6a356f1e49f3e
--- a/files/nftables/ipv6-nat.nft
+++ b/files/nftables/ipv6-nat.nft
@@ -1,5 +1,3 @@
-#!@sbindir@nft -f
-
 table ip6 nat {
 	chain prerouting	{ type nat hook prerouting priority -100; }
 	chain input 		{ type nat hook input priority 100; }
diff --git a/files/nftables/ipv6-raw.nft b/files/nftables/ipv6-raw.nft
old mode 100755
new mode 100644
index 504fb3e5c851b..f92668be272a6
--- a/files/nftables/ipv6-raw.nft
+++ b/files/nftables/ipv6-raw.nft
@@ -1,5 +1,3 @@
-#!@sbindir@nft -f
-
 table ip6 raw {
 	chain prerouting	{ type filter hook prerouting priority -300; }
 	chain output		{ type filter hook output priority -300; }
diff --git a/files/nftables/netdev-ingress.nft b/files/nftables/netdev-ingress.nft
old mode 100755
new mode 100644
index 9e46b15a7e596..3ed881af21d37
--- a/files/nftables/netdev-ingress.nft
+++ b/files/nftables/netdev-ingress.nft
@@ -1,6 +1,4 @@
-#!@sbindir@nft -f
-
-# mind the NIC, it must exists
+# mind the NIC, it must exist
 table netdev filter {
         chain loinput { type filter hook ingress device lo priority 0; }
 }
-- 
2.24.0

[nft PATCH 2/2] files: Install sample scripts from files/examples

[Phil Sutter]

Assuming these are still relevant and useful as a source of inspiration,
install them into DATAROOTDIR/doc/nftables/examples.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 configure.ac               | 1 +
 files/Makefile.am          | 1 +
 files/examples/Makefile.am | 4 ++++
 3 files changed, 6 insertions(+)
 create mode 100644 files/examples/Makefile.am

diff --git a/configure.ac b/configure.ac
index 170b609321458..3a512e0295dc9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -137,6 +137,7 @@ AC_CONFIG_FILES([					\
 		include/linux/netfilter_ipv4/Makefile	\
 		include/linux/netfilter_ipv6/Makefile	\
 		files/Makefile				\
+		files/examples/Makefile			\
 		files/nftables/Makefile			\
 		files/osf/Makefile			\
 		doc/Makefile				\
diff --git a/files/Makefile.am b/files/Makefile.am
index 4f41b664e9db7..7deec15129772 100644
--- a/files/Makefile.am
+++ b/files/Makefile.am
@@ -1,2 +1,3 @@
 SUBDIRS =	nftables \
+		examples \
 		osf
diff --git a/files/examples/Makefile.am b/files/examples/Makefile.am
new file mode 100644
index 0000000000000..c40e041e43578
--- /dev/null
+++ b/files/examples/Makefile.am
@@ -0,0 +1,4 @@
+pkgdocdir = ${docdir}/examples
+dist_pkgdoc_SCRIPTS = ct_helpers.nft \
+		load_balancing.nft \
+		sets_and_maps.nft
-- 
2.24.0

Re: [nft PATCH 2/2] files: Install sample scripts from files/examples

[Pablo Neira Ayuso]

Hi,

On Thu, Nov 07, 2019 at 12:45:16PM +0100, Phil Sutter wrote:
> Assuming these are still relevant and useful as a source of inspiration,
> install them into DATAROOTDIR/doc/nftables/examples.

I think I found the intention of this update, it's something that
Arturo made IIRC. I forgot about this. The idea with this shebang is
to allow for this.

        # ./x.nft

to allow to restore a ruleset without invoking nft -f.

You have to give execution permission to nft script.

Re: [nft PATCH 2/2] files: Install sample scripts from files/examples

[Phil Sutter]

Hi Pablo,

On Thu, Nov 07, 2019 at 01:06:04PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Nov 07, 2019 at 12:45:16PM +0100, Phil Sutter wrote:
> > Assuming these are still relevant and useful as a source of inspiration,
> > install them into DATAROOTDIR/doc/nftables/examples.
> 
> I think I found the intention of this update, it's something that
> Arturo made IIRC. I forgot about this. The idea with this shebang is
> to allow for this.
> 
>         # ./x.nft
> 
> to allow to restore a ruleset without invoking nft -f.
> 
> You have to give execution permission to nft script.

Yes, that's correct. I've used dist_pkgdoc_SCRIPTS variable
intentionally, this makes them being installed with executable bit set.

Cheers, Phil

Re: [nft PATCH 2/2] files: Install sample scripts from files/examples

[Arturo Borrero Gonzalez]

On 11/7/19 12:45 PM, Phil Sutter wrote:
> Assuming these are still relevant and useful as a source of inspiration,
> install them into DATAROOTDIR/doc/nftables/examples.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  configure.ac               | 1 +
>  files/Makefile.am          | 1 +
>  files/examples/Makefile.am | 4 ++++
>  3 files changed, 6 insertions(+)
>  create mode 100644 files/examples/Makefile.am

I assume the files will end in somewhere like:
 /usr/share/doc/nftables/examples/sets_and_maps.nft

Depending on the system of course. But anyway that looks pretty convenient.

Acked-by: Arturo Borrero Gonzalez <arturo@netfilter.org>

Re: [nft PATCH 1/2] files: Drop shebangs from config files

[Arturo Borrero Gonzalez]

On 11/7/19 12:45 PM, Phil Sutter wrote:
> These are not meant to be executed as is but instead loaded via
> 'nft -f' - all-in-one.nft even points this out in header comment.
> While being at it, drop two spelling mistakes found along the way.
> 
> Consequently remove executable bits - being registered in automake as
> dist_pkgsysconf_DATA, they're changed to 644 upon installation anyway.
> 
> Also there is obviously no need for replacement of nft binary path
> anymore, drop that bit from Makefile.am.

If you drop the shebang, the shell may not know how to execute these files. Why
not executing them with the python interpreter instead of `nft -f`?

As pablo commented, the intention was to allow simple use cases like:

root@server:~# ./load-my-ruleset.nft

This use case would still be allowed after this patch but it would be a little
less obvious (less examples). So I'm not sure about ACK'ing this patch.

Re: [nft PATCH 1/2] files: Drop shebangs from config files

[Phil Sutter]

Hi Arturo,

On Tue, Nov 12, 2019 at 12:15:07PM +0100, Arturo Borrero Gonzalez wrote:
> On 11/7/19 12:45 PM, Phil Sutter wrote:
> > These are not meant to be executed as is but instead loaded via
> > 'nft -f' - all-in-one.nft even points this out in header comment.
> > While being at it, drop two spelling mistakes found along the way.
> > 
> > Consequently remove executable bits - being registered in automake as
> > dist_pkgsysconf_DATA, they're changed to 644 upon installation anyway.
> > 
> > Also there is obviously no need for replacement of nft binary path
> > anymore, drop that bit from Makefile.am.
> 
> If you drop the shebang, the shell may not know how to execute these files. Why
> not executing them with the python interpreter instead of `nft -f`?

Even without dropping it, shell won't execute them because we don't
install them with executable bit set.

> As pablo commented, the intention was to allow simple use cases like:
> 
> root@server:~# ./load-my-ruleset.nft
> 
> This use case would still be allowed after this patch but it would be a little
> less obvious (less examples). So I'm not sure about ACK'ing this patch.

While it is inconvenient for users to set the file executable first,
adding a shebang is certainly beyond that. IMO, we basically have two
options:

A) Apply my patch and stick to all-in-one.nft's header comment ("This
   script is meant to be loaded with `nft -f <file>`").

B) Ignore my patch and declare the configs as dist_pkgsysconf_SCRIPTS
   (untested) so they are installed with executable bit set.

Personally I find it awkward to directly execute files in /etc other
than sysv init scripts, hence why I prefer (A). For an example of "real"
nft scripts, there are the samples in files/examples/ which get
installed into $docdir/examples/ with executable bit set if my other
patch is applied.

But for me, (B) is fine as well. I just think we should be consistent.
:)

Cheers, Phil

Re: [nft PATCH 1/2] files: Drop shebangs from config files

[Arturo Borrero Gonzalez]

On 11/12/19 12:36 PM, Phil Sutter wrote:
> 
> But for me, (B) is fine as well. I just think we should be consistent.
> :)
> 

On second thought, I don't have strong opinion :-) so go ahead with whatever
approach you see fit.

Re: [nft PATCH 1/2] files: Drop shebangs from config files

[Pablo Neira Ayuso]

On Thu, Nov 07, 2019 at 12:45:15PM +0100, Phil Sutter wrote:
> These are not meant to be executed as is but instead loaded via
> 'nft -f' - all-in-one.nft even points this out in header comment.
> While being at it, drop two spelling mistakes found along the way.
> 
> Consequently remove executable bits - being registered in automake as
> dist_pkgsysconf_DATA, they're changed to 644 upon installation anyway.
> 
> Also there is obviously no need for replacement of nft binary path
> anymore, drop that bit from Makefile.am.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

No object from Arturo, so:

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Re: [nft PATCH 2/2] files: Install sample scripts from files/examples

[Pablo Neira Ayuso]

On Thu, Nov 07, 2019 at 12:45:16PM +0100, Phil Sutter wrote:
> Assuming these are still relevant and useful as a source of inspiration,
> install them into DATAROOTDIR/doc/nftables/examples.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>