rustybird / corridor

Tor traffic whitelisting gateway
ISC License
68 stars 6 forks source link

port to netfilter-persistent #8

Closed adrelanos closed 7 years ago

adrelanos commented 8 years ago

https://packages.debian.org/jessie/netfilter-persistent is a reliable way to load firewall rules.

It is better than inventing custom firewall loading mechanisms.

https://github.com/rustybird/corridor/blob/master/systemd/corridor-init-forwarding.service.in

rustybird commented 8 years ago

https://anonscm.debian.org/cgit/collab-maint/iptables-persistent.git/tree/systemd/netfilter-persistent.service doesn't look reliable: It orders itself before the wrong target (should be Before=network-pre.target), and seems to make no provision to block network or system startup if any of the firewall scripts or the wrapper script itself fail.

adrelanos commented 8 years ago

Rusty Bird:

https://anonscm.debian.org/cgit/collab-maint/iptables-persistent.git/tree/systemd/netfilter-persistent.service doesn't look reliable:

Then let's see if we can get these sorted out upstream.

I am very glad to see an attempt by netfilter-persistent to provide canonical ways to reliably load firewall rules. And the ability for stackable firewall scripts is also great.

It orders itself before the wrong target (should be Before=network-pre.target)

I agree this is bad. Credited you and reported a bug upstream:

netfilter-persistent loads firewall rules too late http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=829640

and seems to make no provision to block network or system startup if any of the firewall scripts or the wrapper script itself fail.

Should it block the network if a firewall script fails? Why shouldn't it be up to the firewall scripts to block the network? Each firewall script that desires so could have an error handler that locks the network if failure condition is met.

Blocking the system startup is not really useful? Why should one be locked out of the whole system just because of a mistake in a firewall script?

Given proper firewall scripts, the wrapper script should not fail, should it? Or what are conditions where the wrapper script would fail?

Do you suggest netfilter-persistent should run some firewall-locking script in case of a failure condition? How should that look like? Set iptables and ip6tables policy drop for all chains?

I think setting all network policies to drop is problematic. Imagine the case where the user is setting up something less security relevant such as playing with Qubes firewall rules for caching proxies. And keeping to restart netfilter persistent for testing purposes. Now, a mistake was made in some script, failure condition. Set all to policy drop. In that case, how would the iptables policy be set back to allow without ignoring what other firewalls scripts may desire about that?

rustybird commented 8 years ago

I am very glad to see an attempt by netfilter-persistent to provide canonical ways to reliably load firewall rules. And the ability for stackable firewall scripts is also great.

I'd agree that a defined order for iptables scripts is currently useful, if only for the maddening limitations of iptables rules. Maybe nftables and its tooling can facilitate writing modular rulesets without caring so much about the order in which they are loaded? Still haven't checked it out.

But how does netfilter-persistent make things more reliable? Is it even possible anymore on modern kernels/userspace that concurrent iptables-restore -n and iptables -w calls fail? If so, this is what should be fixed because there can always be some init script or something that just wants to run a quick iptables command without plugging into a firewall ruleset loader... https://imgs.xkcd.com/comics/standards.png

Blocking the system startup is not really useful? Why should one be locked out of the whole system just because of a mistake in a firewall script?

OnFailure=emergency.target
OnFailureJobMode=replace-irreversibly

is sometimes used for security critical services, but yes, it's a horrible option, especially in a Qubes VM context (because it would block qrexec-agent). Its only advantage is not having to know the name of the particular network management service(s), but netfilter-persistent is Debian specific anyway.

Given proper firewall scripts, the wrapper script should not fail, should it? Or what are conditions where the wrapper script would fail?

For example, whoami or run-parts could be corrupted on disk or otherwise broken.

Imagine the case where the user is setting up something less security relevant such as playing with Qubes firewall rules for caching proxies. And keeping to restart netfilter persistent for testing purposes. Now, a mistake was made in some script, failure condition.

Yes, failing closed should probably only happen on system startup (where it's easy for a firewall to fail without anyone noticing), which is another reason the emergency.target thing isn't used in corridor.

rustybird commented 8 years ago

netfilter-persistent loads firewall rules too late http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=829640

They also have to add Wants=network-pre.target then, I forgot this myself recently :( https://www.freedesktop.org/wiki/Software/systemd/NetworkTarget/

adrelanos commented 8 years ago

Rusty Bird:

netfilter-persistent loads firewall rules too late http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=829640

They also have to add Wants=network-pre.target then, I forgot this myself recently :( https://www.freedesktop.org/wiki/Software/systemd/NetworkTarget/

I relayed that suggestion to the bug report.

rustybird commented 8 years ago

Thanks, appreciated.

adrelanos commented 8 years ago

Rusty Bird:

But how does netfilter-persistent make things more reliable?

As long as these issues are not fixed, it does not. Once it does, and once multiple people use netfilter-persistent to get their firewall rules reliably loaded in a sane way (early enough...), it will be.

Is it even possible anymore on modern kernels/userspace that concurrent iptables-restore -n and iptables -w calls fail?

iptables-restore could fail because it does not support -w / --wait.

Given proper firewall scripts, the wrapper script should not fail, should it? Or what are conditions where the wrapper script would fail?

For example, whoami or run-parts could be corrupted on disk or otherwise broken.

My answer below will apply.

Imagine the case where the user is setting up something less security relevant such as playing with Qubes firewall rules for caching proxies. And keeping to restart netfilter persistent for testing purposes. Now, a mistake was made in some script, failure condition.

Yes, failing closed should probably only happen on system startup (where it's easy for a firewall to fail without anyone noticing), which is another reason the emergency.target thing isn't used in corridor.

Good idea. I've credited you and submitted a bug report to Debian.

netfilter-persistent bug report: netfilter-persistent systemd service does not lock the network if netfilter-persistent wrapper is failing at system bootup http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=829752

adrelanos commented 8 years ago

systemd feature request: please provide a firewall scripts drop-in folder https://github.com/systemd/systemd/issues/3661

adrelanos commented 8 years ago

systemd developer Lennart Poettering said, that this does not belong into the systemd project, but perhaps into the netfilter project. Source: https://github.com/systemd/systemd/issues/3661

Posted a feature request against netfilter: please provide a firewall scripts drop-in folder https://bugzilla.netfilter.org/show_bug.cgi?id=1078

adrelanos commented 7 years ago

I would like to suggest solutions to these Debian bugs. Before I do that, I would very much like your opinion.

Suggested fix for http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=829640:

[Unit]
Description=netfilter persistent configuration
DefaultDependencies=no

Wants=network-pre.target
Before=network-pre.target

Wants=systemd-modules-load.service local-fs.target
After=systemd-modules-load.service local-fs.target

Conflicts=shutdown.target

[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=/usr/sbin/netfilter-persistent start
ExecStop=/usr/sbin/netfilter-persistent stop

[Install]
WantedBy=multi-user.target

Do you think this is good?

adrelanos commented 7 years ago

Suggested fix for http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=829752:

/lib/systemd/system/networking.service.d/30_netfilter-persistent.conf

[Unit]
## Fail Closed Mechanism.
## When the firewall systemd service failed, do not bring up the network.
After=netfilter-persistent.service
Requires=netfilter-persistent.service

Do you think this is good?

rustybird commented 7 years ago
Conflicts=shutdown.target

Hmm, is it even necessary to stop the firewall on shutdown?

After=netfilter-persistent.service

Redundant, because networking.service correctly orders itself after network-pre.target in /lib/systemd/system/networking.service.d/network-pre.conf.

LGTM otherwise

adrelanos commented 7 years ago

Hmm, is it even necessary to stop the firewall on shutdown?

In case of netfilter-persistent the stop action is sane since it does not flush. Doesn't make a big difference either way. Without it, systemd would automatically kill it on shutdown. With the stop action it is a little bit more correct since it does not involve the killing process of stale processes / units on shutdown, I think.

(Also discussed here btw: https://phabricator.whonix.org/T528#9665)

rustybird commented 7 years ago

Ok, then you should also add Before=shutdown.target according to https://www.freedesktop.org/software/systemd/man/systemd.special.html#shutdown.target

rustybird commented 7 years ago

Closing this, because netfilter-persistent is Debian specific

adrelanos commented 7 years ago

Okay, agreed.

Done, suggestions attached to the Debian bug reports.