Page MenuHomePhabricator

fix qubes-whonix-firewall systemd service start in Q2198 branch
Closed, ResolvedPublic

Description

This currently blocks pretty much everything.

This branch is supposed to fix various bugs such as Tor starting/failing for multiple times and qubes-whonix-torified-updates-proxy still sometimes failing.

It does not get mentioned in sudo journalctl -b at all.

user@host:~$ sudo service qubes-whonix-firewall status
● qubes-whonix-firewall.service - Qubes Whonix firewall updater
   Loaded: loaded (/lib/systemd/system/qubes-whonix-firewall.service; disabled)
   Active: inactive (dead)

Manual start sudo service qubes-whonix-firewall start works without issues.

It was changed from WantedBy=multi-user.target to WantedBy=network-pre.target.

The updated qubes-whonix-network.service [which also has this change] works, which baffles me.

For debugging purposes I also removed all After= and Before= from these both services to no avail.

sudo systemctl enable qubes-whonix-firewall (or reenable) does not help either.

Details

Impact
High

Event Timeline

Patrick created this task.Jul 28 2016, 1:49 AM

(Old systemd targets wants symlinks are cleaned up.)

With WantedBy=multi-user.target qubes-whonix-firewall systemd service auto starts as usual. With WantedBy=network-pre.target issue happens as described above.

Patrick added a comment.EditedJul 28 2016, 2:56 AM

qubes-whonix-network.service contains:
Alias=qubes-network.service

Which results in an additional file:

user@host:~$ ls -la /etc/systemd/system/qubes-network.service 
lrwxrwxrwx 1 root root 48 Jul 28  2016 /etc/systemd/system/qubes-network.service -> /lib/systemd/system/qubes-whonix-network.service

Without that file / without that alias, both qubes-whonix-network.service and qubes-whonix-firewall.service end up in this strange loaded, enabled, inactive status.

Maybe nothing pulls in network-pre.target? I think network-pre.target is only for ordering, not for starting services (so, only After= or Before= dependencies, but no WantedBy=). Generally quite good description is in systemd.special man page.

In T528#9599, @Patrick wrote:

qubes-whonix-network.service contains:
Alias=qubes-network.service
Which results in an additional file:

user@host:~$ ls -la /etc/systemd/system/qubes-network.service 
lrwxrwxrwx 1 root root 48 Jul 28  2016 /etc/systemd/system/qubes-network.service -> /lib/systemd/system/qubes-whonix-network.service

Without that file / without that alias, both qubes-whonix-network.service and qubes-whonix-firewall.service end up in this strange loaded, enabled, inactive status.

This is expected - this is how Alias= works.

Also a super simple new service I added just for testing purposes on my local machine only with WanntedBy=network-pre.target did not load.

In T528#9600, @marmarek wrote:

Maybe nothing pulls in network-pre.target? I think network-pre.target is only for ordering, not for starting services (so, only After= or Before= dependencies, but no WantedBy=).

Yes, quite possible.

I am currently looking for some recommended/secure or even canonical way to load firewall rules.

[systemd-devel] How to securely load a firewall before networking gets up?
https://lists.freedesktop.org/archives/systemd-devel/2016-July/037236.html

I finally have a working setup. Will commit soon.

I am still running into systemd ordering cycles.

For debugging purposes, I've masked qubes-whonix-postinit.service, qubes-whonix-network.service and qubes-whonix-firewall.service. No more ordering cycles then.

Then I just unmasked a minimal qubes-whonix-firewall.service and again ordering cycles.

qubes-whonix-firewall.service

[Unit]
Description=Qubes Whonix firewall updater

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

## Preventing race condition with
## /etc/xdg/autostart/qubes-whonixsetup.desktop.
## TODO:
## Not the most efficient / clean solution.
## https://phabricator.whonix.org/T424
#Before=qubes-gui-agent.service

## For /rw/whonix_firewall.d.
#After=local-fs.target

[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=/usr/lib/qubes-whonix/init/enable-firewall
StandardOutput=syslog

[Install]
WantedBy=multi-user.target
Jul 28 21:54:45 host systemd[1]: Found ordering cycle on basic.target/start
Jul 28 21:54:45 host systemd[1]: Found dependency on paths.target/start
Jul 28 21:54:45 host systemd[1]: Found dependency on acpid.path/start
Jul 28 21:54:45 host systemd[1]: Found dependency on sysinit.target/start
Jul 28 21:54:45 host systemd[1]: Found dependency on networking.service/start
Jul 28 21:54:45 host systemd[1]: Found dependency on network-pre.target/start
Jul 28 21:54:45 host systemd[1]: Found dependency on qubes-whonix-firewall.service/start
Jul 28 21:54:45 host systemd[1]: Found dependency on basic.target/start
Jul 28 21:54:45 host systemd[1]: Breaking ordering cycle by deleting job paths.target/start
Jul 28 21:54:45 host systemd[1]: Job paths.target/start deleted to break ordering cycle starting with basic.target/start
Jul 28 21:54:45 host systemd[1]: Found ordering cycle on basic.target/start
Jul 28 21:54:45 host systemd[1]: Found dependency on sysinit.target/start
Jul 28 21:54:45 host systemd[1]: Found dependency on networking.service/start
Jul 28 21:54:45 host systemd[1]: Found dependency on network-pre.target/start
Jul 28 21:54:45 host systemd[1]: Found dependency on qubes-whonix-firewall.service/start
Jul 28 21:54:45 host systemd[1]: Found dependency on basic.target/start
Jul 28 21:54:45 host systemd[1]: Breaking ordering cycle by deleting job networking.service/start
Jul 28 21:54:45 host systemd[1]: Job networking.service/start deleted to break ordering cycle starting with basic.target/start

Any idea?

Apparently network-pre.target is implicitly ordered before basic.target, which is automatically added (as After=) by DefaultDependencies=yes. Which would mean that any service with Before=network-pre.target also needs 'DefaultDependencies=no`.
I can't find any other example with Before=netowork-pre.target, but all the units I can find on Debian with Before=network.target also have DefaultDependencies=no. On the other hand, on Fedora I see multiple services with Before=network.target, but with DefaultDependencies=yes, so this looks like something Debian-specific. I guess it's about networking.service:

$ systemctl show networking.service |grep 'After=\|Before=\|DefaultDependencies='
Before=sysinit.target shutdown.target network.target
After=mountkernfs.service local-fs.target systemd-random-seed.service network-pre.target systemd-journald.socket system.slice
DefaultDependencies=no

This impose ordering: networ-pre.target -> networking.service -> sysinit.target (which is before basic.target).
I have no idea whether it is some design decision, or unexpected side effect...

Sounds like a bug. Reported one against Debian. Let's see.

usage of network-pre.target results in systemd ordering cycle
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=832802

Patrick added a comment.EditedJul 29 2016, 5:15 PM

Got a reply:
https://lists.freedesktop.org/archives/systemd-devel/2016-July/037242.html

Right, looks like DefaultDependencies=no cannot be prevented. I had hope it can be, since that makes things more difficult to get right.

Debian systemd question:
How to securely load a firewall before networking gets up?
http://lists.alioth.debian.org/pipermail/pkg-systemd-maintainers/2016-July/012253.html

Getting closer. Will commit soon.

networking.service causes a systemd ordering cycle. Would it be sane to add Alias=networking.service to qubes-whonix-network.service? This is what I currently have. I am asking, because Qubes might want to use the default upstream networking tools at some point and I would not want to make migration more difficult than necessary then. @marmarek

I think that's fine. Because it serves very similar purpose as
networking.service. But this alone probably will not fix all the
problems, as it will have the same dependencies, which are added using
drop-ins:

user@host:~$ systemctl status networking
● networking.service - LSB: Raise network interfaces.
   Loaded: loaded (/etc/init.d/networking)
  Drop-In: /lib/systemd/system/networking.service.d
           └─40_qubes.conf
        /run/systemd/generator/networking.service.d
           └─50-insserv.conf-$network.conf
        /lib/systemd/system/networking.service.d
           └─network-pre.conf
   Active: inactive (dead)
           start condition failed at Tue 2016-07-26 22:45:03 UTC; 3 days ago
           ConditionPathExists=!/usr/lib/qubes-whonix was not met

You can probably have the same result by setting the same dependencies
in qubes-whonix-network.service itself. And the risk of some conflicts
will be lower.

marmarek (Marek Marczykowski-Górecki):

marmarek added a comment.

I think that's fine. Because it serves very similar purpose as
networking.service.

Good, so this will be it.

But this alone probably will not fix all the

problems, as it will have the same dependencies, which are added using
drop-ins:

I feared that yesterday also, but my test result is, that these drop-ins
are ignored once qubes-whonix-network.service had
Alias=networking.service. I will double check today that this is not the
case.

Works well. The updated qubes-whonix package is in Whonix developers repository now and will soon be migrated into the testing and jessie-proposed-updates repository.

for x in qubes-whonix-firewall.service qubes-whonix-network.service qubes-whonix-postinit.service qubes-whonix-torified-updates-proxy-check.service ; do
   echo "$x"
   systemctl show -p After,Before,Wants,Requires,RequiresOverridable \
                  -p Requisite,RequisiteOverridable,BindsTo,PartOf   \
                  -p Conflicts,DefaultDependencies "$x"
   echo "#####"
done | cat

old:

qubes-whonix-firewall.service
Requires=basic.target
RequiresOverridable=
Requisite=
RequisiteOverridable=
Wants=system.slice
BindsTo=
PartOf=
Conflicts=shutdown.target
Before=qubes-whonix-torified-updates-proxy-check.service network.target qubes-gui-agent.service shutdown.target multi-user.target
After=qubes-whonix-postinit.service qubes-whonix-network.service systemd-journald.socket basic.target system.slice
DefaultDependencies=yes
#####
qubes-whonix-network.service
Requires=basic.target
RequiresOverridable=
Requisite=
RequisiteOverridable=
Wants=system.slice
BindsTo=
PartOf=
Conflicts=shutdown.target
Before=qubes-whonix-torified-updates-proxy-check.service network.target qubes-update-check.service qubes-update-check.timer shutdown.target multi-user.target qubes-whonix-firewall.service qubes-firewall.service
After=iptables.service systemd-journald.socket basic.target system.slice qubes-whonix-postinit.service
DefaultDependencies=yes
#####
qubes-whonix-postinit.service
Requires=basic.target
RequiresOverridable=
Requisite=
RequisiteOverridable=
Wants=system.slice
BindsTo=
PartOf=
Conflicts=shutdown.target
Before=qubes-whonix-network.service qubes-gui-agent.service tor.service rinetd.service control-port-filter-python.service qubes-whonix-firewall.service whonixcheck.service sdwdate.service anon-ws-disable-stacked-tor.service shutdown.target multi-user.target whonix-legacy.service
After=qubes-mount-dirs.service qubes-whonix-sysinit.service qubes-mount-home.service systemd-journald.socket basic.target system.slice
DefaultDependencies=yes
#####
qubes-whonix-torified-updates-proxy-check.service
Requires=basic.target
RequiresOverridable=
Requisite=
RequisiteOverridable=
Wants=system.slice
BindsTo=
PartOf=
Conflicts=shutdown.target
Before=shutdown.target multi-user.target
After=qubes-whonix-firewall.service network.target systemd-journald.socket basic.target system.slice qubes-whonix-network.service
DefaultDependencies=yes
#####

new:

qubes-whonix-firewall.service
Requires=
RequiresOverridable=
Requisite=
RequisiteOverridable=
Wants=network-pre.target system.slice
BindsTo=
PartOf=
Conflicts=
Before=network-pre.target qubes-gui-agent.service
After=local-fs.target qubes-mount-dirs.service qubes-mount-home.service sysinit.target systemd-journald.socket system.slice
DefaultDependencies=no
#####
qubes-whonix-network.service
Requires=
RequiresOverridable=
Requisite=
RequisiteOverridable=
Wants=network.target system.slice
BindsTo=
PartOf=
Conflicts=
Before=network.target
After=sysinit.target network-pre.target systemd-journald.socket system.slice
DefaultDependencies=no
#####
qubes-whonix-postinit.service
Requires=basic.target
RequiresOverridable=
Requisite=
RequisiteOverridable=
Wants=system.slice
BindsTo=
PartOf=
Conflicts=shutdown.target
Before=qubes-gui-agent.service tor.service tor@default.service rinetd.service control-port-filter-python.service whonixcheck.service sdwdate.service anon-ws-disable-stacked-tor.service shutdown.target multi-user.target whonix-legacy.service
After=qubes-whonix-sysinit.service local-fs.target qubes-mount-dirs.service qubes-mount-home.service systemd-journald.socket basic.target system.slice
DefaultDependencies=yes
#####
qubes-whonix-torified-updates-proxy-check.service
Requires=basic.target
RequiresOverridable=
Requisite=
RequisiteOverridable=
Wants=network.target network-online.target system.slice
BindsTo=
PartOf=
Conflicts=shutdown.target
Before=qubes-updates-proxy.service qubes-updates-cache.service shutdown.target multi-user.target
After=network.target network-online.target systemd-journald.socket basic.target system.slice
DefaultDependencies=yes
#####

qubes-whonix-network.service and qubes-whonix-firewall.service are longer using Conflicts=shutdown.target. Is it worth another update? I am wondering if the missing Conflicts=shutdown.target should be considered a bug or feature. At least for the firewall that may even be an advantage.

It means the service will not be stopped at VM shutdown. In case of
firewall indeed it may be ok, but for network setup I think it may be a
bug.

Note that Conflict= is the same type as Wants= - it affect whether the
service is started/stopped, but do not affect ordering - this is taken
from Before=/After= (in case of shutdown - in reverse order).

Okay.

Done:
https://github.com/Whonix/qubes-whonix/commit/bbfe3682c718b4fd38907a9165972f8c773b5748

Previous merge btw:
https://github.com/Whonix/qubes-whonix/commit/bcdc2e6a15b0f3558ca34407f0716af33dacbebe

(Stopping the firewall does not unload the firewall rules so using Conflicts=shutdown.target does no harm but is more correct and perhaps a few milliseconds faster shutdown.)

Updated qubes-whonix package version soon. (5.7-1)

Patrick changed the task status from Open to Review.Aug 1 2016, 3:36 AM

qubs-whonix 5.7-1 with Conflicts=shutdown.target.

qubes-whonix-firewall.service
Requires=
RequiresOverridable=
Requisite=
RequisiteOverridable=
Wants=network-pre.target system.slice
BindsTo=
PartOf=
Conflicts=shutdown.target
Before=network-pre.target qubes-gui-agent.service
After=local-fs.target qubes-mount-dirs.service qubes-mount-home.service sysinit.target systemd-journald.socket system.slice
DefaultDependencies=no
#####
qubes-whonix-network.service
Requires=
RequiresOverridable=
Requisite=
RequisiteOverridable=
Wants=network.target system.slice
BindsTo=
PartOf=
Conflicts=shutdown.target
Before=network.target qubes-firewall.service
After=sysinit.target network-pre.target systemd-journald.socket system.slice
DefaultDependencies=no
#####
qubes-whonix-postinit.service
Requires=basic.target
RequiresOverridable=
Requisite=
RequisiteOverridable=
Wants=system.slice
BindsTo=
PartOf=
Conflicts=shutdown.target
Before=qubes-gui-agent.service tor.service tor@default.service rinetd.service control-port-filter-python.service whonixcheck.service sdwdate.service anon-ws-disable-stacked-tor.service shutdown.target multi-user.target whonix-legacy.service
After=qubes-whonix-sysinit.service local-fs.target qubes-mount-dirs.service qubes-mount-home.service systemd-journald.socket basic.target system.slice
DefaultDependencies=yes
#####
qubes-whonix-torified-updates-proxy-check.service
Requires=basic.target
RequiresOverridable=
Requisite=
RequisiteOverridable=
Wants=network.target network-online.target system.slice
BindsTo=
PartOf=
Conflicts=shutdown.target
Before=qubes-updates-proxy.service qubes-updates-cache.service shutdown.target multi-user.target
After=network.target network-online.target systemd-journald.socket basic.target system.slice
DefaultDependencies=yes
#####
Patrick closed this task as Resolved.Aug 16 2016, 12:49 AM
Patrick claimed this task.