Re: [ptxdist] [PATCH] Add persistant iptable-rules via systemd

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

Hello,

even though I said for (implict) v2 that I'm lucky now,
I still found a few things to critize/ask.

You could be a still better ptxdist citizen if you used -v3 for
git format-patch (or git send-email if you didn't do the explicit
format-patch step). For additional karma add a section like:

	Changes since v2:

	 - reformat rule file
	 - ...

after the --- below.

$Subject ~= s/iptable/iptables/

On Mon, Apr 11, 2016 at 02:19:28PM +0200, Gavin Schenk wrote:
> Supports ipv4 and ipv6 and both options can be selected in menuconfig
> by IPTABLES_IPV6_SYSTEMD_UNIT and/or IPTABLES_IPV4_SYSTEMD_UNIT
> 
> If you select IPTABLES_IPV4_SYSTEMD_UNIT a systemd unit is started on
> multiuser.target that set the iptable rules from file:
> /etc/iptables/rules.v4
> 
> If you select IPTABLES_IPV6_SYSTEMD_UNIT a systemd unit is started on
> multiuser.target that set the iptable rules from the file:
> /etc/iptables/rules.v6
> 
> The Package provides empty files. If you want to add custom rules, you

s/P/p/

> have to provide your own files. The rule files can be generated with
> the utils iptables-save ip6tables-save from the iptables package.

	the utils iptables-save or ip6tables-save respectively.

Pointing out "from the iptables package" doesn't add much because the
two new config items are part of the iptables package, too.

> Example:
> Generating a rule file, that drops port 5000 on interface eth0 for ipv4
> 
> 1.) iptables -A INPUT -i eth0 -p TCP --dport 5000 -j DROP
> 2.) iptables-save > /etc/iptables/rules.v4
> 
> The basic idea was taken from https://github.com/gronke/systemd-iptables
> written by Stefan Grönke <stefan@gronke.net> in 2015.
> 
> Signed-off-by: Gavin Schenk <g.schenk@eckelmann.de>
> ---
>  projectroot/etc/iptables/rules.v4                |  0
>  projectroot/etc/iptables/rules.v6                |  0
>  projectroot/lib/systemd/system/ip6tables.service | 14 ++++++++++++++
>  projectroot/lib/systemd/system/iptables.service  | 14 ++++++++++++++
>  projectroot/usr/sbin/ip6tables-flush             | 19 +++++++++++++++++++
>  projectroot/usr/sbin/iptables-flush              | 19 +++++++++++++++++++
>  rules/iptables.in                                | 10 ++++++++++
>  rules/iptables.make                              | 21 +++++++++++++++++++++
>  8 files changed, 97 insertions(+)
>  create mode 100644 projectroot/etc/iptables/rules.v4
>  create mode 100644 projectroot/etc/iptables/rules.v6
>  create mode 100644 projectroot/lib/systemd/system/ip6tables.service
>  create mode 100644 projectroot/lib/systemd/system/iptables.service
>  create mode 100755 projectroot/usr/sbin/ip6tables-flush
>  create mode 100755 projectroot/usr/sbin/iptables-flush
> 
> diff --git a/projectroot/etc/iptables/rules.v4 b/projectroot/etc/iptables/rules.v4
> new file mode 100644
> index 0000000..e69de29

These files are never used, are they? Either you want them filled, then
you provide your own version in the BSP; or you disable
IPTABLES_IPV4_SYSTEMD_UNIT.
Hm, thinking again, not adding the files breaks an all-yes compile test.
Michael, what do you think?

> diff --git a/projectroot/etc/iptables/rules.v6 b/projectroot/etc/iptables/rules.v6
> new file mode 100644
> index 0000000..e69de29
> diff --git a/projectroot/lib/systemd/system/ip6tables.service b/projectroot/lib/systemd/system/ip6tables.service
> new file mode 100644
> index 0000000..e842cc1
> --- /dev/null
> +++ b/projectroot/lib/systemd/system/ip6tables.service
> @@ -0,0 +1,14 @@
> +[Unit]
> +Description=Packet Filtering Framework
> +DefaultDependencies=no
> +After=systemd-sysctl.service
> +Before=sysinit.target
> +ConditionFileNotEmpty=/etc/iptables/rules.v6
> +[Service]
> +Type=oneshot
> +ExecStart=/usr/sbin/ip6tables-restore /etc/iptables/rules.v6
> +ExecReload=/usr/sbin/ip6tables-restore /etc/iptables/rules.v6
> +ExecStop=/usr/sbin/iptables/ip6tables-flush
> +RemainAfterExit=yes
> +[Install]
> +WantedBy=multi-user.target
> diff --git a/projectroot/lib/systemd/system/iptables.service b/projectroot/lib/systemd/system/iptables.service
> new file mode 100644
> index 0000000..fa4a8b3
> --- /dev/null
> +++ b/projectroot/lib/systemd/system/iptables.service
> @@ -0,0 +1,14 @@
> +[Unit]
> +Description=Packet Filtering Framework
> +DefaultDependencies=no
> +After=systemd-sysctl.service
> +Before=sysinit.target
> +ConditionFileNotEmpty=/etc/iptables/rules.v4
> +[Service]
> +Type=oneshot
> +ExecStart=/usr/sbin/iptables-restore /etc/iptables/rules.v4
> +ExecReload=/usr/sbin/iptables-restore /etc/iptables/rules.v4
> +ExecStop=/usr/sbin/iptables-flush
> +RemainAfterExit=yes
> +[Install]
> +WantedBy=multi-user.target

I wonder if we want this to run earlier (not sure how to enforce this).
Like this a service that is blocked by the firewall is reachable between
start of respective service and loading of the firewall.

I'm not sure about the ordering of the services, is

	After=systemd-sysctl.service

late enough that all relevant kernel modules are there that provide the
ethernet devices? If the module is missing that provides eth0
referencing eth0 in the rules doesn't work IIRC.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de