| Message ID | 20141217205407.06558f65@turingmachine |
|---|---|
| State | RFC |
| Delegated to: | Pablo Neira |
| Headers | show |
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.
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
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)
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
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
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
> 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. -- 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 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-)
> 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.
--
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
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
> 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. -- 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
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
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