Bug 1266512 - Restarting firewalld.service should restart fail2ban.service
Restarting firewalld.service should restart fail2ban.service
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: fail2ban (Show other bugs)
24
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Orion Poplawski
Fedora Extras Quality Assurance
:
Depends On: 1317240 1317243 1317244 1317246
Blocks:
  Show dependency treegraph
 
Reported: 2015-09-25 09:03 EDT by Marcos Mello
Modified: 2016-08-29 14:55 EDT (History)
6 users (show)

See Also:
Fixed In Version: fail2ban-0.9.4-5.fc24
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-08-29 14:55:25 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Marcos Mello 2015-09-25 09:03:27 EDT
This way fail2ban will reblock its previous database IPs.

Possible solution: put into fail2ban-firewalld a systemd snippet for the ordering/binding.

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

('Wants=' is to enforce ordering making firewalld.service start if it is not running)

Then, add %systemd_postun to fail2ban-firewalld %post and %postun to tell systemd reload its configuration.

Btw, /usr/lib/systemd/system/fail2ban.service has firewalld.service duplicated.

Please backport this to EPEL 7.
Comment 1 Marcos Mello 2015-09-25 13:45:58 EDT
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
---
Comment 2 Jonathan Underwood 2015-09-25 18:26:18 EDT
What if the machine in question isn't using the firewalld ban action?
Comment 3 Marcos Mello 2015-09-25 18:34:30 EDT
It will not have fail2ban-firewalld installed. No changes then.
Comment 4 Orion Poplawski 2015-09-25 18:49:40 EDT
Marcos - can you file this upstream as well?
Comment 5 Jonathan Underwood 2015-09-25 19:06:35 EDT
(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.
Comment 6 Marcos Mello 2015-09-26 06:08:18 EDT
(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.
Comment 7 Marcos Mello 2015-10-03 06:45:49 EDT
https://github.com/fail2ban/fail2ban/issues/1005

If implemented, it will allow us drop the 'PartOf=' part.
Comment 8 Marcos Mello 2016-01-26 05:50:21 EST
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.
Comment 9 Jan Kurik 2016-02-24 08:47:35 EST
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
Comment 10 Fedora Update System 2016-03-09 16:35:56 EST
fail2ban-0.9.4-2.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-478f5063bc
Comment 11 Orion Poplawski 2016-03-09 16:36:16 EST
(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?
Comment 12 Marcos Mello 2016-03-09 17:26:32 EST
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.
Comment 13 Jonathan Underwood 2016-03-09 17:59:34 EST
(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.
Comment 14 Marcos Mello 2016-03-09 19:01:55 EST
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.
Comment 15 Fedora Update System 2016-03-09 20:54:39 EST
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
Comment 16 Orion Poplawski 2016-03-09 22:50:45 EST
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.
Comment 17 Jonathan Underwood 2016-03-10 08:51:32 EST
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
Comment 18 Jonathan Underwood 2016-03-10 09:02:59 EST
Ah, I see that's fixed in the next commit - I guess the update will need editing.
Comment 19 Marcos Mello 2016-03-10 09:43:33 EST
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?
Comment 20 Jonathan Underwood 2016-03-10 10:02:52 EST
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
Comment 21 Marcos Mello 2016-03-11 14:14:06 EST
(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
Comment 22 Jonathan Underwood 2016-03-13 06:07:49 EDT
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.
Comment 23 Fedora Update System 2016-04-05 11:47:55 EDT
fail2ban-0.9.4-5.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-478f5063bc
Comment 24 Fedora Update System 2016-04-06 13:54:26 EDT
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
Comment 25 Fedora Update System 2016-08-29 14:55:17 EDT
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.

Note You need to log in before you can comment on or make changes to this bug.