Bug 1266512

If firewalld fails to start, fail2ban cannot work. We must use Requires instead of Wants.

Updated snippet:

/usr/lib/systemd/system/fail2ban.service.d/firewalld.conf
---
[Unit]
Requires=firewalld.service
PartOf=firewalld.service
---

What if the machine in question isn't using the firewalld ban action?

It will not have fail2ban-firewalld installed. No changes then.

Marcos - can you file this upstream as well?

(In reply to Marcos Mello from comment #3)
> It will not have fail2ban-firewalld installed. No changes then.

This seems fragile... It's possible to have fail2ban-firewalld installed, but not actually be using any firewalld ban actions in the active jails. In which case, we don't need firewalld to be running.

(In reply to Orion Poplawski from comment #4)
> Marcos - can you file this upstream as well?

I am not sure this is upstream material. They already have After=firewalld.service there and if they want all distro-specific firewall solutions can be added. After= is a "weak" ordering dependency. If the units listed are not running or missing systemd will not bother. Things are different when we add requirement deps. A missing one will cause a warning (Wants=) or refuse unit start (Requires=). Upstreaming will demand an ifdefery mess I am not willing to do.

(In reply to Jonathan Underwood from comment #5)
> (In reply to Marcos Mello from comment #3)
> > It will not have fail2ban-firewalld installed. No changes then.
> 
> This seems fragile... It's possible to have fail2ban-firewalld installed,
> but not actually be using any firewalld ban actions in the active jails. In
> which case, we don't need firewalld to be running.

Not fragile at all. The current setup is fragile, buggy, for people running firewalld. When you install fail2ban-firewalld, you tell it you will use firewalld and the system should behave correctly.

If you do not intend to use firewalld, simply uninstall fail2ban-firewalld. You still can test firewalld actions putting the right configuration in jails.local. fail2ban-firewalld is just a configuration file. All ban actions are still there in main package fail2ban-server.

fail2ban-firewalld description could be extended to explain better the situation. Something like:

This package enables support for manipulating firewalld rules. This is the
default firewall service in Fedora. It also enforces that firewalld will
start before fail2ban.

https://github.com/fail2ban/fail2ban/issues/1005

If implemented, it will allow us drop the 'PartOf=' part.

Upstream decided for adding PartOf=firewalld.service instead of firewall-cmd --permanent approach (will be in 0.9.4).

https://github.com/fail2ban/fail2ban/commit/9905396eb8bf0563a67ea9dc1d09d31a8af7e077
https://github.com/fail2ban/fail2ban/commit/dfaf82d68a39662e475b3a0403a64de04fda7e76

We still need a requirement dep as I explained in comment 6.

This bug appears to have been reported against 'rawhide' during the Fedora 24 development cycle.
Changing version to '24'.

More information and reason for this action is here:
https://fedoraproject.org/wiki/Fedora_Program_Management/HouseKeeping/Fedora24#Rawhide_Rebase

fail2ban-0.9.4-2.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-478f5063bc

(In reply to Marcos Mello from comment #8)
> We still need a requirement dep as I explained in comment 6.

I'm not sure I follow this.  What is still needed?

Add a requirement dependency (Requires=) between fail2ban.service and firewalld.service when fail2ban-firewalld is installed.

After= is just an ordering dependency. systemd *does not* guarantee units listed there will be started. Because fail2ban *will not* work out-of-box if firewalld is not running, we must enforce this via systemd. I proposed using a snippet config file:

/usr/lib/systemd/system/fail2ban.service.d/firewalld.conf
---
[Unit]
Requires=firewalld.service
---

(PartOf=firewalld.service is not needed as per comment 8)

Then add %systemd_postun to fail2ban-firewalld %post and %postun to tell systemd reload its configuration. Note: I am not following recent switch to RPM file triggers in systemd macros (it is v228+ stuff I think).

Requires=firewalld.service will:

- start firewalld.service even if it is disabled (only masked services are not started in this scenario)
- abort fail2ban start if firewalld start has failed (it cannot work in this case)

The objection so far is that systems not using firewalld would have it started anyway. Well, if you have fail2ban-firewalld installed, you already have fail2ban configured to use by default the firewalld actions. You should uninstall fail2ban-firewalld in this case. The snippet will be gone and you will be free to use any other firewall solution you want.

You can argue that it is admin responsibility certify that firewalld.service is enabled at least. I still think though we should use systemd features to bring us robustness.

(In reply to Marcos Mello from comment #12)
> Add a requirement dependency (Requires=) between fail2ban.service and
> firewalld.service when fail2ban-firewalld is installed.
> 
> After= is just an ordering dependency. systemd *does not* guarantee units
> listed there will be started. Because fail2ban *will not* work out-of-box if
> firewalld is not running, we must enforce this via systemd. I proposed using
> a snippet config file:
> 
> /usr/lib/systemd/system/fail2ban.service.d/firewalld.conf
> ---
> [Unit]
> Requires=firewalld.service
> ---
> 
> (PartOf=firewalld.service is not needed as per comment 8)
> 
> Then add %systemd_postun to fail2ban-firewalld %post and %postun to tell
> systemd reload its configuration. Note: I am not following recent switch to
> RPM file triggers in systemd macros (it is v228+ stuff I think).
> 
> Requires=firewalld.service will:
> 
> - start firewalld.service even if it is disabled (only masked services are
> not started in this scenario)
> - abort fail2ban start if firewalld start has failed (it cannot work in this
> case)
> 
> The objection so far is that systems not using firewalld would have it
> started anyway. Well, if you have fail2ban-firewalld installed, you already
> have fail2ban configured to use by default the firewalld actions. You should
> uninstall fail2ban-firewalld in this case. The snippet will be gone and you
> will be free to use any other firewall solution you want.
> 
> You can argue that it is admin responsibility certify that firewalld.service
> is enabled at least. I still think though we should use systemd features to
> bring us robustness.

I really don't like the thinking behind this. If I install fail2ban and fail2ban-firewalld and then at some point later decide to configure my jails to not use firewalld, I would be very surprised if restarting fail2ban kicked firewalld into life because I had forgotten to also uninstall the fail2ban-firewalld package. 

None of what you propose is needed at all. With the upstream change, if firewalld is running and is restarted, fail2ban will be restarted. That's really all that's needed. I see no use case for what you're proposing, unless I have really misunderstood what you're proposing.

It is similar if I forgot to enable firewalld.service. Or I messed up firewalld config (making it fail to start). My fail2ban service, despite running, would be useless.

We both made our points. I will be happy with Orion's decision. And I agree the main problem reported in this bug is solved.

fail2ban-0.9.4-2.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-478f5063bc

We seem to be in an area of marginal utility.  For the default case, fail2ban is installed with firewalld and firewalld is enabled by default.

So if we add the systemd requires statement, then if firewalld fails to start, fail2ban would fail to start as well.  I suppose having two failed services rather than one might have a better chance of getting attention, but I don't see it as critical.  And if someone disabled firewalld for whatever reason, and then decided to use fail2ban, it would work by default by starting firewalld.  This might be more useful, but hard to say.

The downside seems to be if someone switches to bare iptables by disabling firewalld (and not removing it), and then specifying a new banaction in jail.local (overriding the 00-firewalld.conf setting) rather than removing fail2ban-firewalld, then firewalld service would continue to be started.  I have no sense if this is a common way of working around the default configuration or not.

At this point I think I'm going to leave things as they are.  If problem reports start to surface about people complaining about fail2ban not working because firewalld was not started I'll reconsider.

Thank you Marcos for the detailed description, it was very helpful.

Unfortunately, the new packages actually have a problem:

# systemctl restart firewalld
Failed to restart firewalld.service: Transaction contains conflicting jobs 'restart' and 'stop' for fail2ban.service. Probably contradicting requirement dependencies configured.


# rpm -qa | grep fail2ban
fail2ban-0.9.4-2.fc23.noarch
fail2ban-server-0.9.4-2.fc23.noarch
fail2ban-firewalld-0.9.4-2.fc23.noarch
fail2ban-sendmail-0.9.4-2.fc23.noarch

Ah, I see that's fixed in the next commit - I guess the update will need editing.

Orion just removed a duplicated After=firewalld.service entry in that commit.

Hmmm, I cannot reproduce it on CentOS 7. What firewalld version do you have?

Ah, yes, good point.

$ rpm -qa | grep firewalld
firewalld-filesystem-0.4.0-2.fc23.noarch
fail2ban-firewalld-0.9.4-2.fc23.noarch
firewalld-0.4.0-2.fc23.noarch

(I can reproduce it now, I was using outdated fail2ban package, sorry for the noise)

Thanks for raising the issue on systemd-devel.

https://lists.freedesktop.org/archives/systemd-devel/2016-March/036010.html

I've filed a bunch of dependent RFE bugs to implement the suggested firewall.target approach outlined on systemd-devel (see the blockers of this bug).

For F23 and earlier I think we're going to need an imperfect compromise solution though. What I would favour is dropping the PartOf entry for iptables for fail2ban, so at least the fail2ban service is restarted correctly with the Fedora default firewall implementation.

fail2ban-0.9.4-5.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-478f5063bc

fail2ban-0.9.4-5.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-478f5063bc

fail2ban-0.9.4-5.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.