[PATCH] add systemd service file

[PATCH] add systemd service file

[Jörg Thalheim]

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

Signed-off-by: Jörg Thalheim <joerg@higgsboson.tk>
---
 configure.ac                   | 34 ++++++++++++++++++++++++++++++++++
 files/Makefile.am              |  3 ++-
 files/nftables/nftables.conf   |  0
 files/systemd/Makefile.am      | 12 ++++++++++++
 files/systemd/nftables-reload  | 16 ++++++++++++++++
 files/systemd/nftables.service | 12 ++++++++++++
 6 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 files/nftables/nftables.conf
 create mode 100644 files/systemd/Makefile.am
 create mode 100755 files/systemd/nftables-reload
 create mode 100644 files/systemd/nftables.service

diff --git a/configure.ac b/configure.ac
index 57ea99d..5adb223 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,6 +13,8 @@ AC_CONFIG_MACRO_DIR([m4])
 AM_INIT_AUTOMAKE([-Wall foreign subdir-objects
         tar-pax no-dist-gzip dist-bzip2 1.6])
 
+AC_PATH_TOOL(PKGCONFIG, pkg-config)
+
 dnl kernel style compile messages
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
 
@@ -112,6 +114,36 @@ AC_TYPE_UINT16_T
 AC_TYPE_UINT32_T
 AC_TYPE_UINT64_T
 
+AC_ARG_WITH(systemd, [  --with-systemd          set directory for systemd service files],
+        [systemd_unitdir="$withval"; with_systemd=yes],
+        [systemd_unitdir=""; with_systemd=no])
+AC_SUBST(systemd_unitdir)
+
+AC_ARG_WITH(systemdutildir, [  --with-systemdutildir   set directory for systemd helper scripts],
+        [systemd_utildir="$withval"], [systemd_utildir=""])
+AC_SUBST(systemd_utildir)
+
+AM_CONDITIONAL([INSTALL_SYSTEMD], [test "x$with_systemd" != xno])
+AM_COND_IF([INSTALL_SYSTEMD],
+       [AS_IF([test "x$PKGCONFIG" = "x"],
+             [AC_MSG_ERROR(Need pkg-config to enable systemd support.)],
+
+             [AC_MSG_CHECKING(for systemd)
+              AS_IF([$PKGCONFIG --exists systemd],
+                    [AC_MSG_RESULT(yes)
+                     AS_IF([$PKGCONFIG --exists systemd],
+                           [AS_IF([test "x$systemd_unit_dir" = "x"],
+                                  [ systemd_unitdir="`$PKGCONFIG --variable=systemdsystemunitdir systemd`"])
+                            AS_IF([test "x$systemd_util_dir" = "x"],
+                                  [ systemd_utildir="`$PKGCONFIG --variable=systemdutildir systemd`"])
+                           ])
+                    ]
+                    [AC_MSG_RESULT(no)])
+             ]
+
+       )]
+)
+
 # Checks for library functions.
 AC_CHECK_FUNCS([memmove memset strchr strdup strerror strtoull])
 
@@ -124,10 +156,12 @@ AC_CONFIG_FILES([					\
 		doc/Makefile				\
 		files/Makefile				\
 		files/nftables/Makefile			\
+		files/systemd/Makefile			\
 		])
 AC_OUTPUT
 
 echo "
 nft configuration:
   cli support:			${with_cli}
+  systemd support:		${with_systemd}
   enable debugging:		${with_debug}"
diff --git a/files/Makefile.am b/files/Makefile.am
index a8394c0..4dc0027 100644
--- a/files/Makefile.am
+++ b/files/Makefile.am
@@ -1 +1,2 @@
-SUBDIRS = nftables
+SUBDIRS =	nftables	\
+					systemd
diff --git a/files/nftables/nftables.conf b/files/nftables/nftables.conf
new file mode 100644
index 0000000..e69de29
diff --git a/files/systemd/Makefile.am b/files/systemd/Makefile.am
new file mode 100644
index 0000000..c0a56c8
--- /dev/null
+++ b/files/systemd/Makefile.am
@@ -0,0 +1,12 @@
+
+if INSTALL_SYSTEMD
+systemd_unit_DATA = nftables.service
+
+systemd_scriptsdir = ${systemd_utildir}/scripts
+systemd_scripts_SCRIPTS = nftables-reload
+
+install-data-hook:
+	${SED} -i 's|@sbindir[@]|${sbindir}/|g;s|@sysconfdir[@]|${sysconfdir}/|g;s|@systemd_scriptsdir[@]|${systemd_scriptsdir}/|g' \
+		${DESTDIR}${systemd_scriptsdir}/nftables-reload \
+		${DESTDIR}${systemd_unitdir}/nftables.service
+endif
diff --git a/files/systemd/nftables-reload b/files/systemd/nftables-reload
new file mode 100755
index 0000000..0fcc2ad
--- /dev/null
+++ b/files/systemd/nftables-reload
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+set -e
+
+if [ ! -f @sysconfdir@nftables.conf ]
+then
+  echo "/etc/nftables.conf does not exist" >&2
+  exit 1
+fi
+
+tmp=${TMPDIR-/tmp}
+rules=`mktemp "$tmp/nftables.XXXXXXX"`
+trap "rm -f $tmpfile" 0 1 2 3 15
+echo "flush ruleset" > "$rules"
+cat @sysconfdir@nftables.conf >> "$rules"
+@sbindir@nft -f "$rules"
diff --git a/files/systemd/nftables.service b/files/systemd/nftables.service
new file mode 100644
index 0000000..f958bfc
--- /dev/null
+++ b/files/systemd/nftables.service
@@ -0,0 +1,12 @@
+[Unit]
+Description=nftables
+Documentation=man:nft(8)
+
+[Service]
+RemainAfterExit=yes
+ExecStart=@sbindir@nft -I /etc/nftables -f /etc/nftables.conf
+ExecStop=@sbindir@nft flush ruleset
+ExecReload=@systemd_scriptsdir@nftables-reload
+
+[Install]
+WantedBy=multi-user.target
-- 
2.2.0


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] add systemd service file

[Jörg Thalheim]

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

- fixes tempfile creation in the reload script

Anything left from your side?

This will be the first nftables init script with support for atomic reloading!

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] add systemd service file

[Pablo Neira Ayuso]

On Fri, Dec 19, 2014 at 02:02:58PM +0100, Jörg Thalheim wrote:
> Signed-off-by: Jörg Thalheim <joerg@higgsboson.tk>
> ---
>  configure.ac                   | 34 ++++++++++++++++++++++++++++++++++
>  files/Makefile.am              |  3 ++-
>  files/nftables/nftables.conf   |  0
>  files/systemd/Makefile.am      | 12 ++++++++++++
>  files/systemd/nftables-reload  | 16 ++++++++++++++++
>  files/systemd/nftables.service | 12 ++++++++++++
>  6 files changed, 76 insertions(+), 1 deletion(-)
>  create mode 100644 files/nftables/nftables.conf
>  create mode 100644 files/systemd/Makefile.am
>  create mode 100755 files/systemd/nftables-reload
>  create mode 100644 files/systemd/nftables.service

I'd prefer if distributors take care of this instead, I think these
scripts are glue integration code. Looking at what it happened to
other userspace netfilter codebase, my experience is that they tend to
get rotten unmaintained in the source code tree.

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

[PATCH] add systemd service file

[Jörg Thalheim]

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

Signed-off-by: Jörg Thalheim <joerg@higgsboson.tk>
---
 configure.ac                   | 34 ++++++++++++++++++++++++++++++++++
 files/Makefile.am              |  3 ++-
 files/nftables/nftables.conf   |  0
 files/systemd/Makefile.am      | 12 ++++++++++++
 files/systemd/nftables-reload  | 15 +++++++++++++++
 files/systemd/nftables.service | 12 ++++++++++++
 6 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 files/nftables/nftables.conf
 create mode 100644 files/systemd/Makefile.am
 create mode 100755 files/systemd/nftables-reload
 create mode 100644 files/systemd/nftables.service

diff --git a/configure.ac b/configure.ac
index 57ea99d..5adb223 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,6 +13,8 @@ AC_CONFIG_MACRO_DIR([m4])
 AM_INIT_AUTOMAKE([-Wall foreign subdir-objects
         tar-pax no-dist-gzip dist-bzip2 1.6])
 
+AC_PATH_TOOL(PKGCONFIG, pkg-config)
+
 dnl kernel style compile messages
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
 
@@ -112,6 +114,36 @@ AC_TYPE_UINT16_T
 AC_TYPE_UINT32_T
 AC_TYPE_UINT64_T
 
+AC_ARG_WITH(systemd, [  --with-systemd          set directory for systemd service files],
+        [systemd_unitdir="$withval"; with_systemd=yes],
+        [systemd_unitdir=""; with_systemd=no])
+AC_SUBST(systemd_unitdir)
+
+AC_ARG_WITH(systemdutildir, [  --with-systemdutildir   set directory for systemd helper scripts],
+        [systemd_utildir="$withval"], [systemd_utildir=""])
+AC_SUBST(systemd_utildir)
+
+AM_CONDITIONAL([INSTALL_SYSTEMD], [test "x$with_systemd" != xno])
+AM_COND_IF([INSTALL_SYSTEMD],
+       [AS_IF([test "x$PKGCONFIG" = "x"],
+             [AC_MSG_ERROR(Need pkg-config to enable systemd support.)],
+
+             [AC_MSG_CHECKING(for systemd)
+              AS_IF([$PKGCONFIG --exists systemd],
+                    [AC_MSG_RESULT(yes)
+                     AS_IF([$PKGCONFIG --exists systemd],
+                           [AS_IF([test "x$systemd_unit_dir" = "x"],
+                                  [ systemd_unitdir="`$PKGCONFIG --variable=systemdsystemunitdir systemd`"])
+                            AS_IF([test "x$systemd_util_dir" = "x"],
+                                  [ systemd_utildir="`$PKGCONFIG --variable=systemdutildir systemd`"])
+                           ])
+                    ]
+                    [AC_MSG_RESULT(no)])
+             ]
+
+       )]
+)
+
 # Checks for library functions.
 AC_CHECK_FUNCS([memmove memset strchr strdup strerror strtoull])
 
@@ -124,10 +156,12 @@ AC_CONFIG_FILES([					\
 		doc/Makefile				\
 		files/Makefile				\
 		files/nftables/Makefile			\
+		files/systemd/Makefile			\
 		])
 AC_OUTPUT
 
 echo "
 nft configuration:
   cli support:			${with_cli}
+  systemd support:		${with_systemd}
   enable debugging:		${with_debug}"
diff --git a/files/Makefile.am b/files/Makefile.am
index a8394c0..4dc0027 100644
--- a/files/Makefile.am
+++ b/files/Makefile.am
@@ -1 +1,2 @@
-SUBDIRS = nftables
+SUBDIRS =	nftables	\
+					systemd
diff --git a/files/nftables/nftables.conf b/files/nftables/nftables.conf
new file mode 100644
index 0000000..e69de29
diff --git a/files/systemd/Makefile.am b/files/systemd/Makefile.am
new file mode 100644
index 0000000..c0a56c8
--- /dev/null
+++ b/files/systemd/Makefile.am
@@ -0,0 +1,12 @@
+
+if INSTALL_SYSTEMD
+systemd_unit_DATA = nftables.service
+
+systemd_scriptsdir = ${systemd_utildir}/scripts
+systemd_scripts_SCRIPTS = nftables-reload
+
+install-data-hook:
+	${SED} -i 's|@sbindir[@]|${sbindir}/|g;s|@sysconfdir[@]|${sysconfdir}/|g;s|@systemd_scriptsdir[@]|${systemd_scriptsdir}/|g' \
+		${DESTDIR}${systemd_scriptsdir}/nftables-reload \
+		${DESTDIR}${systemd_unitdir}/nftables.service
+endif
diff --git a/files/systemd/nftables-reload b/files/systemd/nftables-reload
new file mode 100755
index 0000000..d688cb6
--- /dev/null
+++ b/files/systemd/nftables-reload
@@ -0,0 +1,15 @@
+#!/bin/sh
+
+set -e
+
+if [ ! -f @sysconfdir@nftables.conf ]
+then
+  echo "/etc/nftables.conf does not exist" >&2
+  exit 1
+fi
+
+rules="$(mktemp nftables.XXXXXXXX)"
+trap "rm -f $tmpfile" 0 1 2 3 15
+echo "flush ruleset" > "$rules"
+cat @sysconfdir@nftables.conf >> "$rules"
+@sbindir@nft -f "$rules"
diff --git a/files/systemd/nftables.service b/files/systemd/nftables.service
new file mode 100644
index 0000000..f958bfc
--- /dev/null
+++ b/files/systemd/nftables.service
@@ -0,0 +1,12 @@
+[Unit]
+Description=nftables
+Documentation=man:nft(8)
+
+[Service]
+RemainAfterExit=yes
+ExecStart=@sbindir@nft -I /etc/nftables -f /etc/nftables.conf
+ExecStop=@sbindir@nft flush ruleset
+ExecReload=@systemd_scriptsdir@nftables-reload

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] add systemd service file

[Jörg Thalheim]

* remove /etc/nftables configuration

Re: [PATCH] add systemd service file

[Jörg Thalheim]

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

I refactor the patch based on your feedback, however:
  
  $ nft flush ruleset

fails with:

  <cmdline>:1:1-13: Error: Could not process rule: Address family not supported by protocol 
  flush ruleset
  ^^^^^^^^^^^^^

  $ nft --version     
  nftables v0.4 (Support Edward Snowden)

  $ uname -a
  Linux turingmachine 3.17.6-1-ARCH #1 SMP PREEMPT Sun Dec 7 23:43:32 UTC 2014 x86_64 GNU/Linux

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] add systemd service file

[Pablo Neira Ayuso]

On Thu, Dec 18, 2014 at 01:47:08PM +0100, Jörg Thalheim wrote:
> I refactor the patch based on your feedback, however:
>   
>   $ nft flush ruleset
> 
> fails with:
> 
>   <cmdline>:1:1-13: Error: Could not process rule: Address family not supported by protocol 
>   flush ruleset
>   ^^^^^^^^^^^^^
> 
>   $ nft --version     
>   nftables v0.4 (Support Edward Snowden)
> 
>   $ uname -a
>   Linux turingmachine 3.17.6-1-ARCH #1 SMP PREEMPT Sun Dec 7 23:43:32 UTC 2014 x86_64 GNU/Linux

That is supported starting Linux kernel 3.18 as the nftables wiki
indicates.
--
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

[PATCH] add systemd service file

[Jörg Thalheim]

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

Signed-off-by: Jörg Thalheim <joerg@higgsboson.tk>
---
 configure.ac                   | 34 ++++++++++++++++++++++++++++++++++
 files/Makefile.am              | 10 +++++++++-
 files/nftables.conf            |  7 +++++++
 files/nftables/nftables.conf   |  0
 files/systemd/Makefile.am      | 12 ++++++++++++
 files/systemd/nftables-reload  | 15 +++++++++++++++
 files/systemd/nftables.service | 12 ++++++++++++
 7 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100644 files/nftables.conf
 create mode 100644 files/nftables/nftables.conf
 create mode 100644 files/systemd/Makefile.am
 create mode 100755 files/systemd/nftables-reload
 create mode 100644 files/systemd/nftables.service

diff --git a/configure.ac b/configure.ac
index 57ea99d..5adb223 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,6 +13,8 @@ AC_CONFIG_MACRO_DIR([m4])
 AM_INIT_AUTOMAKE([-Wall foreign subdir-objects
         tar-pax no-dist-gzip dist-bzip2 1.6])
 
+AC_PATH_TOOL(PKGCONFIG, pkg-config)
+
 dnl kernel style compile messages
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
 
@@ -112,6 +114,36 @@ AC_TYPE_UINT16_T
 AC_TYPE_UINT32_T
 AC_TYPE_UINT64_T
 
+AC_ARG_WITH(systemd, [  --with-systemd          set directory for
systemd service files],
+        [systemd_unitdir="$withval"; with_systemd=yes],
+        [systemd_unitdir=""; with_systemd=no])
+AC_SUBST(systemd_unitdir)
+
+AC_ARG_WITH(systemdutildir, [  --with-systemdutildir   set directory
for systemd helper scripts],
+        [systemd_utildir="$withval"], [systemd_utildir=""])
+AC_SUBST(systemd_utildir)
+
+AM_CONDITIONAL([INSTALL_SYSTEMD], [test "x$with_systemd" != xno])
+AM_COND_IF([INSTALL_SYSTEMD],
+       [AS_IF([test "x$PKGCONFIG" = "x"],
+             [AC_MSG_ERROR(Need pkg-config to enable systemd
support.)], +
+             [AC_MSG_CHECKING(for systemd)
+              AS_IF([$PKGCONFIG --exists systemd],
+                    [AC_MSG_RESULT(yes)
+                     AS_IF([$PKGCONFIG --exists systemd],
+                           [AS_IF([test "x$systemd_unit_dir" = "x"],
+                                  [ systemd_unitdir="`$PKGCONFIG
--variable=systemdsystemunitdir systemd`"])
+                            AS_IF([test "x$systemd_util_dir" = "x"],
+                                  [ systemd_utildir="`$PKGCONFIG
--variable=systemdutildir systemd`"])
+                           ])
+                    ]
+                    [AC_MSG_RESULT(no)])
+             ]
+
+       )]
+)
+
 # Checks for library functions.
 AC_CHECK_FUNCS([memmove memset strchr strdup strerror strtoull])
 
@@ -124,10 +156,12 @@
AC_CONFIG_FILES([					\
doc/Makefile				\
files/Makefile				\
files/nftables/Makefile			\
+		files/systemd/Makefile			\
 		])
 AC_OUTPUT
 
 echo "
 nft configuration:
   cli support:			${with_cli}
+  systemd support:		${with_systemd}
   enable debugging:		${with_debug}"
diff --git a/files/Makefile.am b/files/Makefile.am
index a8394c0..8045495 100644
--- a/files/Makefile.am
+++ b/files/Makefile.am
@@ -1 +1,9 @@
-SUBDIRS = nftables
+SUBDIRS =	nftables	\
+					systemd
+
+nftables_confdir = ${sysconfdir}
+nftables_conf_DATA = nftables.conf
+
+install-data-hook:
+	${SED} -i
's|@sbindir[@]|${sbindir}/|g;s|@sysconfdir[@]|${sysconfdir}/|g' \
+		${DESTDIR}${sysconfdir}/nftables.conf
diff --git a/files/nftables.conf b/files/nftables.conf
new file mode 100644
index 0000000..f572db5
--- /dev/null
+++ b/files/nftables.conf
@@ -0,0 +1,7 @@
+#! @sbindir@nft -f
+
+table inet filter {
+	chain input		{ type filter hook input priority
0; }
+	chain forward		{ type filter hook forward
priority 0; }
+	chain output		{ type filter hook output priority
0; } +}
diff --git a/files/nftables/nftables.conf b/files/nftables/nftables.conf
new file mode 100644
index 0000000..e69de29
diff --git a/files/systemd/Makefile.am b/files/systemd/Makefile.am
new file mode 100644
index 0000000..c0a56c8
--- /dev/null
+++ b/files/systemd/Makefile.am
@@ -0,0 +1,12 @@
+
+if INSTALL_SYSTEMD
+systemd_unit_DATA = nftables.service
+
+systemd_scriptsdir = ${systemd_utildir}/scripts
+systemd_scripts_SCRIPTS = nftables-reload
+
+install-data-hook:
+	${SED} -i
's|@sbindir[@]|${sbindir}/|g;s|@sysconfdir[@]|${sysconfdir}/|g;s|@systemd_scriptsdir[@]|${systemd_scriptsdir}/|g'
\
+		${DESTDIR}${systemd_scriptsdir}/nftables-reload \
+		${DESTDIR}${systemd_unitdir}/nftables.service
+endif
diff --git a/files/systemd/nftables-reload
b/files/systemd/nftables-reload new file mode 100755
index 0000000..d688cb6
--- /dev/null
+++ b/files/systemd/nftables-reload
@@ -0,0 +1,15 @@
+#!/bin/sh
+
+set -e
+
+if [ ! -f @sysconfdir@nftables.conf ]
+then
+  echo "/etc/nftables.conf does not exist" >&2
+  exit 1
+fi
+
+rules="$(mktemp nftables.XXXXXXXX)"
+trap "rm -f $tmpfile" 0 1 2 3 15
+echo "flush ruleset" > "$rules"
+cat @sysconfdir@nftables.conf >> "$rules"
+@sbindir@nft -f "$rules"
diff --git a/files/systemd/nftables.service
b/files/systemd/nftables.service new file mode 100644
index 0000000..f958bfc
--- /dev/null
+++ b/files/systemd/nftables.service
@@ -0,0 +1,12 @@
+[Unit]
+Description=nftables
+Documentation=man:nft(8)
+
+[Service]
+RemainAfterExit=yes
+ExecStart=@sbindir@nft -I /etc/nftables -f /etc/nftables.conf
+ExecStop=@sbindir@nft flush ruleset
+ExecReload=@systemd_scriptsdir@nftables-reload
+

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] add systemd service file

[Jan Engelhardt]

On Thursday 2014-12-18 13:47, Jörg Thalheim wrote:
>+
>+nftables_confdir = ${sysconfdir}
>+nftables_conf_DATA = nftables.conf

Per systemd and distro directions, upstream-provided files should really really be located in /usr, not /etc
(this location being reserved for machine- and user-specific tweaks).

That's why something.service is provided in /usr(/lib/systemd/system/) rather
rather than /etc/systemd/system, for example.
--
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

Re: [PATCH] add systemd service file

[Jörg Thalheim]

18. Dezember 2014 13:56 Uhr, "Jan Engelhardt" <jengelh@inai.de> schrieb: 
> On Thursday 2014-12-18 13:47, Jörg Thalheim wrote:
> 
>> +
>> +nftables_confdir = ${sysconfdir}
>> +nftables_conf_DATA = nftables.conf
> 
> Per systemd and distro directions, upstream-provided files should really really be located in /usr,
> not /etc
> (this location being reserved for machine- and user-specific tweaks).
> 
> That's why something.service is provided in /usr(/lib/systemd/system/) rather
> rather than /etc/systemd/system, for example.

This is what the code does (/usr/lib/systemd/system/nftables.service). 
In nftables.conf I have added some dummy rules, because the service file relies on it and 
will fails otherwise which is a bad default.

> --
> 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
--
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

Re: [PATCH] add systemd service file

[Jan Engelhardt]

On Thursday 2014-12-18 14:02, Jörg Thalheim wrote:
>18. Dezember 2014 13:56 Uhr, "Jan Engelhardt" <jengelh@inai.de> schrieb: 
>> On Thursday 2014-12-18 13:47, Jörg Thalheim wrote:
>>> +
>>> +nftables_confdir = ${sysconfdir}
>>> +nftables_conf_DATA = nftables.conf
>> 
>> Per systemd and distro directions, upstream-provided files should really really be located in /usr,
>> not /etc
>> (this location being reserved for machine- and user-specific tweaks).
>
>This is what the code does (/usr/lib/systemd/system/nftables.service). 
>In nftables.conf I have added some dummy rules, because the service file relies on it and 
>will fails otherwise which is a bad default.

I mean the shipped nftables.conf, it ought to be installed to
/usr(/share/nftables), similar to systemd services.
If and when the user decides to procure his own rules, he will do so via his
own nftables.conf, then located in /etc and - ideally - overriding the
same-named file in /usr.
--
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

[PATCH] add systemd service file

[Jörg Thalheim]

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

Signed-off-by: Jörg Thalheim <joerg@higgsboson.tk>
---
 .gitignore                |   2 +
 configure.ac              |  35 +++++++++++++
 files/Makefile.am         |   7 +++
 files/nftables.service.in |  12 +++++
 files/nftablesctl.in      | 129 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 185 insertions(+)
 create mode 100644 files/nftables.service.in
 create mode 100755 files/nftablesctl.in

diff --git a/.gitignore b/.gitignore
index 63ef1a2..e6f8065 100644
--- a/.gitignore
+++ b/.gitignore
@@ -23,6 +23,8 @@ depcomp
 ylwrap
 src/parser_bison.c
 src/parser_bison.h
+files/nftables.service
+files/nftablesctl
 
 # Debian package build temporary files
 build-stamp
diff --git a/configure.ac b/configure.ac
index 57ea99d..19980d1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,6 +13,8 @@ AC_CONFIG_MACRO_DIR([m4])
 AM_INIT_AUTOMAKE([-Wall foreign subdir-objects
         tar-pax no-dist-gzip dist-bzip2 1.6])
 
+AC_PATH_TOOL(PKGCONFIG, pkg-config)
+
 dnl kernel style compile messages
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
 
@@ -112,6 +114,36 @@ AC_TYPE_UINT16_T
 AC_TYPE_UINT32_T
 AC_TYPE_UINT64_T
 
+AC_ARG_WITH(systemd, [  --with-systemd          set directory for systemd service files],
+        [systemd_unitdir="$withval"; with_systemd=yes],
+        [systemd_unitdir=""; with_systemd=no])
+AC_SUBST(systemd_unitdir)
+
+AC_ARG_WITH(systemdutildir, [  --with-systemdutildir   set directory for systemd helper scripts],
+        [systemd_utildir="$withval"], [systemd_utildir=""])
+AC_SUBST(systemd_utildir)
+
+AM_CONDITIONAL([INSTALL_SYSTEMD], [test "x$with_systemd" != xno])
+AM_COND_IF([INSTALL_SYSTEMD],
+       [AS_IF([test "x$PKGCONFIG" = "x"],
+             [AC_MSG_ERROR(Need pkg-config to enable systemd support.)],
+
+             [AC_MSG_CHECKING(for systemd)
+              AS_IF([$PKGCONFIG --exists systemd],
+                    [AC_MSG_RESULT(yes)
+                     AS_IF([$PKGCONFIG --exists systemd],
+                           [AS_IF([test "x$systemd_unit_dir" = "x"],
+                                  [ systemd_unitdir="`$PKGCONFIG --variable=systemdsystemunitdir systemd`"])
+                            AS_IF([test "x$systemd_util_dir" = "x"],
+                                  [ systemd_utildir="`$PKGCONFIG --variable=systemdutildir systemd`"])
+                           ])
+                    ]
+                    [AC_MSG_RESULT(no)])
+             ]
+
+       )]
+)
+
 # Checks for library functions.
 AC_CHECK_FUNCS([memmove memset strchr strdup strerror strtoull])
 
@@ -124,10 +156,13 @@ AC_CONFIG_FILES([					\
 		doc/Makefile				\
 		files/Makefile				\
 		files/nftables/Makefile			\
+		files/nftables.service			\
+		files/nftablesctl			\
 		])
 AC_OUTPUT
 
 echo "
 nft configuration:
   cli support:			${with_cli}
+  systemd support:		${with_systemd}
   enable debugging:		${with_debug}"
diff --git a/files/Makefile.am b/files/Makefile.am
index a8394c0..4da6432 100644
--- a/files/Makefile.am
+++ b/files/Makefile.am
@@ -1 +1,8 @@
 SUBDIRS = nftables
+
+if INSTALL_SYSTEMD
+systemd_unit_DATA = nftables.service
+
+systemd_scriptsdir = ${systemd_utildir}/scripts
+systemd_scripts_SCRIPTS = nftablesctl
+endif
diff --git a/files/nftables.service.in b/files/nftables.service.in
new file mode 100644
index 0000000..3c8c921
--- /dev/null
+++ b/files/nftables.service.in
@@ -0,0 +1,12 @@
+[Unit]
+Description=nftables
+Documentation=man:nftables(8)
+
+[Service]
+Type=oneshot
+RemainAfterExit=yes
+ExecStart=@systemd_utildir@/scripts/nftablesctl start
+ExecStop=@systemd_utildir@/scripts/nftablesctl stop
+
+[Install]
+WantedBy=multi-user.target
diff --git a/files/nftablesctl.in b/files/nftablesctl.in
new file mode 100755
index 0000000..080f980
--- /dev/null
+++ b/files/nftablesctl.in
@@ -0,0 +1,129 @@
+#!/bin/sh
+
+set -e
+
+usage() {
+	name=$(basename "$0")
+	echo "Usage: $name start|stop|restart|list"
+	echo
+	echo "       $name start     load the rules"
+	echo "       $name stop      flush the rules"
+	echo "       $name restart   reload the rules"
+	echo "       $name list      list the loaded rules"
+	echo
+	echo "Using --confirm in the following manner will prompt you to check if"
+	echo "your network connection is working fine:"
+	echo
+	echo "       $name start --confirm"
+	echo "       $name restart --confirm"
+}
+
+if [ "$(id -u)" -ne 0 ]
+then
+	echo "Warning: Only root can run this script" >&2
+	echo
+	usage
+	exit 1
+fi
+
+if [ ! -d /etc/nftables ]
+then
+	echo "Rules directory /etc/nftables does not exist" >&2
+	exit 1
+fi
+
+ctrl_c() {
+	echo
+	echo "nftables rules successfully applied"
+	exit 0
+}
+
+nft_clear_table() {
+	@sbindir@nft flush table "$1" "$2"
+	@sbindir@nft list table "$1" "$2" \
+		| awk '/^[ \t]+chain/{ print $2 }' \
+		| xargs -r -L 1 @sbindir@nft delete chain "$1" "$2"
+	@sbindir@nft list sets "$1" "$2" \
+		| awk '/^[ \t]+set/{ print $2 }' \
+		| xargs -r -L 1 @sbindir@nft delete set "$1" "$2"
+}
+
+nft_delete_table() {
+	nft_clear_table "$1" "$2"
+	if @sbindir@nft list table "$1" "$2" > /dev/null
+	then
+		@sbindir@nft delete table "$1" "$2"
+	fi
+}
+
+nft_clear_protocol() {
+	for T in $(@sbindir@nft list tables "$1" | cut -d ' ' -f 2)
+	do
+		nft_delete_table "$1" "$T"
+	done
+}
+
+nft_list_protocol() {
+	for T in $(@sbindir@nft list tables "$1" | cut -d ' ' -f 2)
+	do
+		@sbindir@nft list table "$1" "$T"
+	done
+}
+
+nftables_start() {
+	find /etc/nftables -maxdepth 1 -type f -name '*.rules' -print0 | \
+		sort -z | xargs --null --no-run-if-empty --max-args=1 @sbindir@nft -f
+
+	if [ -t 0 ] && [ "$1" = "--confirm" ]
+	then
+		echo "Please confirm that your network connection is working and press Ctrl+C on success"
+		trap ctrl_c INT
+
+		sleep 20
+
+		echo "No response, flushing rules"
+		nftables_stop
+	fi
+}
+
+nftables_list() {
+	for P in ip inet ip6 arp bridge
+	do
+		nft_list_protocol "$P"
+	done
+}
+
+nftables_stop() {
+	for P in ip inet ip6 arp bridge
+	do
+		nft_clear_protocol "$P"
+	done
+}
+
+nftables_restart() {
+	nftables_stop
+	nftables_start "$1"
+}
+
+case "$1" in
+	start)
+		nftables_start "$2"
+		;;
+
+	stop)
+		nftables_stop
+		;;
+
+	restart)
+		nftables_restart "$2"
+		;;
+
+	list)
+		nftables_list
+		;;
+
+	*)
+		usage

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] add systemd service file

[Arturo Borrero Gonzalez]

On 17 December 2014 at 20:54, Jörg Thalheim <joerg@higgsboson.tk> wrote:
> Signed-off-by: Jörg Thalheim <joerg@higgsboson.tk>
> ---
>  .gitignore                |   2 +
>  configure.ac              |  35 +++++++++++++
>  files/Makefile.am         |   7 +++
>  files/nftables.service.in |  12 +++++
>  files/nftablesctl.in      | 129 ++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 185 insertions(+)
>  create mode 100644 files/nftables.service.in
>  create mode 100755 files/nftablesctl.in
>

Thanks for your work :-)

IMHO, this belongs to distributors, don't you?

Anyway, some inlined comments.


> +nftables_start() {
> +       find /etc/nftables -maxdepth 1 -type f -name '*.rules' -print0 | \
> +               sort -z | xargs --null --no-run-if-empty --max-args=1 @sbindir@nft -f
> +

You are making lot of assumptions, for example the '.rules' suffix.

> +       if [ -t 0 ] && [ "$1" = "--confirm" ]
> +       then
> +               echo "Please confirm that your network connection is working and press Ctrl+C on success"
> +               trap ctrl_c INT
> +
> +               sleep 20
> +
> +               echo "No response, flushing rules"
> +               nftables_stop
> +       fi
> +}
> +
> +nftables_list() {

Now (see latest v0.4 release) this is simply `nft list ruleset'.

> +       for P in ip inet ip6 arp bridge
> +       do
> +               nft_list_protocol "$P"
> +       done
> +}
> +
> +nftables_stop() {

Now this is simply `nft flush ruleset'.

> +       for P in ip inet ip6 arp bridge
> +       do
> +               nft_clear_protocol "$P"
> +       done
> +}
> +
> +nftables_restart() {
> +       nftables_stop
> +       nftables_start "$1"

Here, I think the time between the stop and start, there is not
ruleset in the kernel.
I guess we can do it better, flushing the old ruleset and loading the
new one in a single,atomic step.

-- 
Arturo Borrero González
--
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

Re: [PATCH] add systemd service file

[Jörg Thalheim]

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

Hi netfilter community,
Patrick McHardy ask me to submit our work on systemd support for
nftables (https://github.com/devkid/nftables-systemd)

It currently consists of a service file for systemd and a script to
load/remove nftables rules. The script does more than actually needed
for fundamental systemd support. 

It allows to test rules:

$ nftablesctl start --confirm

which resets after 20s if you accidentally kill your ssh connection.

It allows to list all rules with one command:

$ nftables list

Instead of storing all rules in a single file (/etc/nftables.conf), it
applys all files ending with .rules from directory /etc/nftables/ in
lexical order, which make it more useful for configuration management
like chef/puppet/ansible, where generating a single file from multiple
modules is a pain.

So the question is the nftables project wants such a script. If you are
just looking for a systemd service the following approach would be
much easier:

[Unit]
Description=Netfilter Tables
Documentation=man:nft(8)
Wants=network-pre.target
Before=network-pre.target

[Service]
Type=oneshot
ExecStart=/usr/bin/nft -f /etc/nftables.conf
ExecStop=/usr/lib/systemd/scripts/nftables-flush
RemainAfterExit=yes

[Install]
WantedBy=multi-user.target

where /usr/lib/systemd/scripts/nftables-flush would just drop all rules

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] add systemd service file

[Jan Engelhardt]

On Wednesday 2014-12-17 21:40, Jörg Thalheim wrote:
>
>Instead of storing all rules in a single file (/etc/nftables.conf), it
>applys all files ending with .rules from directory /etc/nftables/ in
>lexical order, which make it more useful for configuration management
>like chef/puppet/ansible, where generating a single file from multiple
>modules is a pain.

But it means there will be a time where only half of the ruleset is
loaded.
--
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

Re: [PATCH] add systemd service file

[Jörg Thalheim]

> But it means there will be a time where only half of the ruleset is
> loaded.

Good point, so i will replace it with a pipe into nft.

Re: [PATCH] add systemd service file

[Arturo Borrero Gonzalez]

On 17 December 2014 at 20:54, Jörg Thalheim <joerg@higgsboson.tk> wrote:
> +
> +nftables_start() {
> +       find /etc/nftables -maxdepth 1 -type f -name '*.rules' -print0 | \
> +               sort -z | xargs --null --no-run-if-empty --max-args=1 @sbindir@nft -f
> +
> +       if [ -t 0 ] && [ "$1" = "--confirm" ]
> +       then
> +               echo "Please confirm that your network connection is working and press Ctrl+C on success"
> +               trap ctrl_c INT
> +
> +               sleep 20
> +
> +               echo "No response, flushing rules"
> +               nftables_stop
> +       fi

Also, it would be nice to rollback to the old ruleset rather than
leaving the machine without firewall (think on mission critical
firewalls, where human mistakes happens after all)

-- 
Arturo Borrero González
--
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

Re: [PATCH] add systemd service file

[Jan Engelhardt]

On Wednesday 2014-12-17 20:54, Jörg Thalheim wrote:
>+
>+nft_clear_table() {
>+	@sbindir@nft flush table "$1" "$2"
>+	@sbindir@nft list table "$1" "$2" \
>+		| awk '/^[ \t]+chain/{ print $2 }' \
>+		| xargs -r -L 1 @sbindir@nft delete chain "$1" "$2"
>+	@sbindir@nft list sets "$1" "$2" \
>+		| awk '/^[ \t]+set/{ print $2 }' \
>+		| xargs -r -L 1 @sbindir@nft delete set "$1" "$2"
>+}

Loading an empty ruleset would be a lot better (and likely faster too) -
iptables was able to do that.

>+nft_delete_table() {
>+	nft_clear_table "$1" "$2"
>+	if @sbindir@nft list table "$1" "$2" > /dev/null
>+	then
>+		@sbindir@nft delete table "$1" "$2"
>+	fi
>+}

This too should perhaps become some single step in some way.


>+nft_clear_protocol() {
>+	for T in $(@sbindir@nft list tables "$1" | cut -d ' ' -f 2)
>+	do
>+		nft_delete_table "$1" "$T"
>+	done
>+}

as should this.
--
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

Re: [PATCH] add systemd service file

[Jörg Thalheim]

17. Dezember 2014 21:37 Uhr, "Arturo Borrero Gonzalez" <arturo.borrero.glez@gmail.com> schrieb: 
> On 17 December 2014 at 20:54, Jörg Thalheim <joerg@higgsboson.tk> wrote:
> 
>> Signed-off-by: Jörg Thalheim <joerg@higgsboson.tk>
>> ---
>> .gitignore                |   2 +
>> configure.ac              |  35 +++++++++++++
>> files/Makefile.am         |   7 +++
>> files/nftables.service.in |  12 +++++
>> files/nftablesctl.in      | 129 ++++++++++++++++++++++++++++++++++++++++++++++
>> 5 files changed, 185 insertions(+)
>> create mode 100644 files/nftables.service.in
>> create mode 100755 files/nftablesctl.in
> 
> Thanks for your work :-)
> 
> IMHO, this belongs to distributors, don't you?

this was in the old sysvinit world. Thesedays it is common for upstream projects to come along with service files, as 
they are more portable then shell scripts.

> 
> Anyway, some inlined comments.
> 
>> +nftables_start() {
>> +       find /etc/nftables -maxdepth 1 -type f -name '*.rules' -print0 | \
>> +               sort -z | xargs --null --no-run-if-empty --max-args=1 @sbindir@nft -f
>> +
> 
> You are making lot of assumptions, for example the '.rules' suffix.
> 
>> +       if [ -t 0 ] && [ "$1" = "--confirm" ]
>> +       then
>> +               echo "Please confirm that your network connection is working and press Ctrl+C on
>> success"
>> +               trap ctrl_c INT
>> +
>> +               sleep 20
>> +
>> +               echo "No response, flushing rules"
>> +               nftables_stop
>> +       fi
>> +}
>> +
>> +nftables_list() {
> 
> Now (see latest v0.4 release) this is simply `nft list ruleset'.
> 
>> +       for P in ip inet ip6 arp bridge
>> +       do
>> +               nft_list_protocol "$P"
>> +       done
>> +}
>> +
>> +nftables_stop() {
> 
> Now this is simply `nft flush ruleset'.

Ok. I did not have a look at latest release, when the script was written a year before, this was not possible.

> 
>> +       for P in ip inet ip6 arp bridge
>> +       do
>> +               nft_clear_protocol "$P"
>> +       done
>> +}
>> +
>> +nftables_restart() {
>> +       nftables_stop
>> +       nftables_start "$1"
> 
> Here, I think the time between the stop and start, there is not
> ruleset in the kernel.
> I guess we can do it better, flushing the old ruleset and loading the
> new one in a single,atomic step.

Is this possible with nft? If so, how?

> 
> -- 
> Arturo Borrero González
--
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

Re: [PATCH] add systemd service file

[Arturo Borrero Gonzalez]

On 17 December 2014 at 21:57, Jörg Thalheim <joerg@higgsboson.tk> wrote:
>>> +nftables_restart() {
>>> +       nftables_stop
>>> +       nftables_start "$1"
>>
>> Here, I think the time between the stop and start, there is not
>> ruleset in the kernel.
>> I guess we can do it better, flushing the old ruleset and loading the
>> new one in a single,atomic step.
>
> Is this possible with nft? If so, how?
>

add a heading 'flush ruleset' to the file to be loaded.

Also, to load multiple files you can use "include" statements, and
still be atomic B-)

-- 
Arturo Borrero González
--
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

Re: [PATCH] add systemd service file

[Jörg Thalheim]

17. Dezember 2014 22:11 Uhr, "Arturo Borrero Gonzalez" <arturo.borrero.glez@gmail.com> schrieb: 
> On 17 December 2014 at 21:57, Jörg Thalheim <joerg@higgsboson.tk> wrote:
> 
>>>> +nftables_restart() {
>>>> +       nftables_stop
>>>> +       nftables_start "$1"
>>> 
>>> Here, I think the time between the stop and start, there is not
>>> ruleset in the kernel.
>>> I guess we can do it better, flushing the old ruleset and loading the
>>> new one in a single,atomic step.
>> 
>> Is this possible with nft? If so, how?
> 
> add a heading 'flush ruleset' to the file to be loaded.
> 
> Also, to load multiple files you can use "include" statements, and
> still be atomic B-)

Is the "include" statement a new feature? Never saw this feature in the wild.
Does it works for directories too? Something like `include "/etc/nftables.d/*"` 
or `includedir "/etc/nftables/"` would be awesome.

> 
> -- 
> Arturo Borrero González
> --
> 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
--
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

Re: [PATCH] add systemd service file

[Jörg Thalheim]

> Is the "include" statement a new feature? Never saw this feature in the wild.
> Does it works for directories too? Something like `include "/etc/nftables.d/*"` 
> or `includedir "/etc/nftables/"` would be awesome.

I should have read the manpage before asking. Anyway glob(3) would be cool addition.

Re: [PATCH] add systemd service file

[Jörg Thalheim]

> You are making lot of assumptions, for example the '.rules' suffix.

Having some identifier is actually useful for administrators, this way you can just use 

$ mv /etc/nftables/{some.rules,some.rules-disabled}

to disable rules. Having an extension also allow editors to provide syntax highlighting.